Skip to content

Commit b37c98c

Browse files
aledbfclaude
andcommitted
fix(cli): fold the features lockfile into the cache key
read-configuration --cache-key is documented to hash the committed features lockfile, but computeCacheKey only used the lockfile when ReadLockfile's bool was true, and that bool reports the empty marker file, which comes back with a nil lock. A lockfile with content was therefore never hashed, so changing the pinned digests kept the key. Use the lockfile whenever ReadLockfile returns one. Keys for configurations without a lockfile, or with an empty one, are unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 59c2cc5 commit b37c98c

2 files changed

Lines changed: 65 additions & 1 deletion

File tree

‎internal/cli/cache_key.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,9 @@ func computeCacheKey(result *config.LoadResult, env map[string]string) (string,
5454
}
5555
}
5656

57-
if lock, ok, _ := features.ReadLockfile(cfg.ConfigFilePath); ok && lock != nil {
57+
// ReadLockfile's bool reports an empty marker file, which pins nothing; a
58+
// lockfile with content comes back as a non-nil lock.
59+
if lock, _, _ := features.ReadLockfile(cfg.ConfigFilePath); lock != nil {
5860
if lockJSON, lerr := json.Marshal(lock); lerr == nil {
5961
in.Lockfile = lockJSON
6062
}
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
package cli
2+
3+
import (
4+
"os"
5+
"path/filepath"
6+
"testing"
7+
)
8+
9+
// A committed features lockfile pins resolved digests, so changing it changes
10+
// the cache key (docs/go-only-features.md, read-configuration --cache-key); an
11+
// empty lockfile, which pins nothing, keys like no lockfile at all.
12+
func TestComputeCacheKeyFoldsInTheLockfile(t *testing.T) {
13+
dir := t.TempDir()
14+
cfgPath := filepath.Join(dir, "devcontainer.json")
15+
lockPath := filepath.Join(dir, "devcontainer-lock.json")
16+
key := func() string {
17+
t.Helper()
18+
k, err := computeCacheKey(imageResult("ubuntu:22.04", cfgPath), nil)
19+
if err != nil {
20+
t.Fatal(err)
21+
}
22+
return k
23+
}
24+
none := key()
25+
26+
if err := os.WriteFile(lockPath, []byte(""), 0o644); err != nil {
27+
t.Fatal(err)
28+
}
29+
if k := key(); k != none {
30+
t.Error("an empty lockfile changed the key")
31+
}
32+
33+
lock := func(digest string) string {
34+
return `{"features":{"ghcr.io/o/f:1":{"version":"1.0.0","resolved":"ghcr.io/o/f@` + digest + `","integrity":"` + digest + `"}}}`
35+
}
36+
if err := os.WriteFile(lockPath, []byte(lock("sha256:aaa")), 0o644); err != nil {
37+
t.Fatal(err)
38+
}
39+
pinnedA := key()
40+
if pinnedA == none {
41+
t.Error("a lockfile did not change the key")
42+
}
43+
if err := os.WriteFile(lockPath, []byte(lock("sha256:bbb")), 0o644); err != nil {
44+
t.Fatal(err)
45+
}
46+
if key() == pinnedA {
47+
t.Error("a lockfile pinning another digest kept the key")
48+
}
49+
}
50+
51+
// A proxy variable that is set but empty configures no proxy, so it keys like
52+
// an unset one.
53+
func TestComputeCacheKeyIgnoresEmptyProxy(t *testing.T) {
54+
cfgPath := filepath.Join(t.TempDir(), "devcontainer.json")
55+
base, err := computeCacheKey(imageResult("ubuntu:22.04", cfgPath), nil)
56+
if err != nil {
57+
t.Fatal(err)
58+
}
59+
if k, _ := computeCacheKey(imageResult("ubuntu:22.04", cfgPath), map[string]string{"HTTPS_PROXY": ""}); k != base {
60+
t.Error("an empty proxy variable changed the key")
61+
}
62+
}

0 commit comments

Comments
 (0)