From 901b5b0952a44c39ae93ea0bdec30b455f8c9de1 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Wed, 6 May 2026 17:00:20 -0500 Subject: [PATCH] feat(config): add oci:// prefix, multi-cloud auth, and digest caching to OCI extends Support the oci:// URI scheme in extends references, replace the bare DefaultKeychain with image.GetKeychain for multi-cloud authentication (AWS ECR, GCR, ACR, k8s in-cluster), and introduce SHA-256 digest-based caching that skips full pulls when the remote manifest is unchanged. --- pkg/devcontainer/config/extends.go | 12 +- pkg/devcontainer/config/extends_oci.go | 125 +++++++++++++++++--- pkg/devcontainer/config/extends_oci_test.go | 108 ++++++++++++++++- pkg/devcontainer/config/parse.go | 8 +- 4 files changed, 232 insertions(+), 21 deletions(-) diff --git a/pkg/devcontainer/config/extends.go b/pkg/devcontainer/config/extends.go index a718fd721..dee5196c8 100644 --- a/pkg/devcontainer/config/extends.go +++ b/pkg/devcontainer/config/extends.go @@ -1,6 +1,7 @@ package config import ( + "context" "encoding/json" "fmt" "maps" @@ -43,12 +44,13 @@ func (e *ExtendsRef) UnmarshalJSON(data []byte) error { // merging each on top of the previous result. The final merged config // is returned as the combined parent for the declaring file. func resolveExtendsArray( + ctx context.Context, refs ExtendsRef, declaringDir string, visited map[string]bool, ) (*DevContainerConfig, error) { var merged *DevContainerConfig for _, ref := range refs { - resolved, err := resolveExtendsSingle(ref, declaringDir, visited) + resolved, err := resolveExtendsSingle(ctx, ref, declaringDir, visited) if err != nil { return nil, err } @@ -64,11 +66,12 @@ func resolveExtendsArray( // resolveExtendsSingle resolves a single extends reference. // It dispatches to OCI resolution for registry refs or local file resolution otherwise. func resolveExtendsSingle( + ctx context.Context, extendsRef, declaringDir string, visited map[string]bool, ) (*DevContainerConfig, error) { if isOCIRef(extendsRef) { - return resolveOCIExtends(extendsRef, visited) + return resolveOCIExtends(ctx, extendsRef, visited) } refPath := extendsRef @@ -85,11 +88,12 @@ func resolveExtendsSingle( return nil, fmt.Errorf("extends: cycle detected, %q already in chain", absPath) } - return parseDevContainerJSONFileWithVisited(absPath, visited) + return parseDevContainerJSONFileWithVisited(ctx, absPath, visited) } // parseDevContainerJSONFileWithVisited parses a devcontainer.json and recursively resolves extends. func parseDevContainerJSONFileWithVisited( + ctx context.Context, path string, visited map[string]bool, ) (*DevContainerConfig, error) { @@ -121,7 +125,7 @@ func parseDevContainerJSONFileWithVisited( // Recursively resolve extends if !devContainer.Extends.IsEmpty() { declaringDir := filepath.Dir(absPath) - parent, err := resolveExtendsArray(devContainer.Extends, declaringDir, visited) + parent, err := resolveExtendsArray(ctx, devContainer.Extends, declaringDir, visited) if err != nil { return nil, err } diff --git a/pkg/devcontainer/config/extends_oci.go b/pkg/devcontainer/config/extends_oci.go index c376b59ab..9c05461e3 100644 --- a/pkg/devcontainer/config/extends_oci.go +++ b/pkg/devcontainer/config/extends_oci.go @@ -3,14 +3,21 @@ package config import ( "archive/tar" "compress/gzip" + "context" + "crypto/sha256" + "encoding/hex" "encoding/json" "errors" "fmt" "io" "net/http" + "os" + "path/filepath" "strings" "time" + pkgconfig "github.com/devsy-org/devsy/pkg/config" + "github.com/devsy-org/devsy/pkg/image" "github.com/google/go-containerregistry/pkg/authn" "github.com/google/go-containerregistry/pkg/name" v1 "github.com/google/go-containerregistry/pkg/v1" @@ -27,8 +34,11 @@ var ociExtendsBackoff = wait.Backoff{ } // isOCIRef returns true if the extends reference looks like an OCI image ref -// (e.g. "ghcr.io/owner/repo:tag") rather than a local file path. +// (e.g. "ghcr.io/owner/repo:tag" or "oci://ghcr.io/owner/repo:tag") rather than a local file path. func isOCIRef(ref string) bool { + if strings.HasPrefix(ref, "oci://") { + return true + } if strings.HasPrefix(ref, ".") || strings.HasPrefix(ref, "/") { return false } @@ -38,34 +48,41 @@ func isOCIRef(ref string) bool { return strings.Contains(ref, "/") } +// stripOCIPrefix removes the "oci://" prefix from an OCI reference if present. +func stripOCIPrefix(ref string) string { + return strings.TrimPrefix(ref, "oci://") +} + // resolveOCIExtends fetches a devcontainer.json from an OCI artifact and // recursively resolves any extends within it. func resolveOCIExtends( + ctx context.Context, ociRef string, visited map[string]bool, ) (*DevContainerConfig, error) { - if visited[ociRef] { - return nil, fmt.Errorf("extends: cycle detected, OCI ref %q already in chain", ociRef) + bare := stripOCIPrefix(ociRef) + if visited[bare] { + return nil, fmt.Errorf("extends: cycle detected, OCI ref %q already in chain", bare) } - visited[ociRef] = true + visited[bare] = true - data, err := pullOCIExtendsJSON(ociRef) + data, err := pullOCIExtendsJSON(ctx, bare) if err != nil { - return nil, fmt.Errorf("extends: fetch OCI %q: %w", ociRef, err) + return nil, fmt.Errorf("extends: fetch OCI %q: %w", bare, err) } devContainer := &DevContainerConfig{} normalized, err := hujson.Standardize(data) if err != nil { - return nil, fmt.Errorf("extends: parse jsonc from OCI %q: %w", ociRef, err) + return nil, fmt.Errorf("extends: parse jsonc from OCI %q: %w", bare, err) } if err := json.Unmarshal(normalized, devContainer); err != nil { - return nil, fmt.Errorf("extends: unmarshal OCI %q: %w", ociRef, err) + return nil, fmt.Errorf("extends: unmarshal OCI %q: %w", bare, err) } - devContainer.Origin = "oci://" + ociRef + devContainer.Origin = "oci://" + bare if !devContainer.Extends.IsEmpty() { - parent, err := resolveExtendsArray(devContainer.Extends, "", visited) + parent, err := resolveExtendsArray(ctx, devContainer.Extends, "", visited) if err != nil { return nil, err } @@ -76,24 +93,104 @@ func resolveOCIExtends( } // pullOCIExtendsJSON fetches an OCI image and extracts devcontainer.json -// from its first layer (expected to be a gzipped tarball). -func pullOCIExtendsJSON(ociRef string) ([]byte, error) { +// from its first layer (expected to be a gzipped tarball). Uses digest-based +// caching to avoid repeated pulls. +func pullOCIExtendsJSON(ctx context.Context, ociRef string) ([]byte, error) { ref, err := name.ParseReference(ociRef) if err != nil { return nil, fmt.Errorf("parse reference: %w", err) } + kc := getKeychain(ctx) + + cacheDir, cacheErr := extendsCacheDir(ociRef) + if cacheErr == nil { + if data, ok := checkExtendsCache(cacheDir, ref, kc); ok { + return data, nil + } + } + var img v1.Image err = retryOCIExtendsPull(func() error { var fetchErr error - img, fetchErr = remote.Image(ref, remote.WithAuthFromKeychain(authn.DefaultKeychain)) + img, fetchErr = remote.Image(ref, remote.WithAuthFromKeychain(kc)) return fetchErr }) if err != nil { return nil, fmt.Errorf("pull image: %w", err) } - return extractDevContainerJSON(img) + data, err := extractDevContainerJSON(img) + if err != nil { + return nil, err + } + + if cacheErr == nil { + writeExtendsCache(cacheDir, ref, kc, data) + } + + return data, nil +} + +func getKeychain(ctx context.Context) authn.Keychain { + kc, err := image.GetKeychain(ctx) + if err != nil { + return authn.DefaultKeychain + } + return kc +} + +func extendsCacheDir(ociRef string) (string, error) { + h := sha256.Sum256([]byte(ociRef)) + hashed := hex.EncodeToString(h[:]) + + base, err := pkgconfig.DefaultPathManager().CacheDir() + if err != nil { + return "", err + } + dir := filepath.Join(base, "extends", hashed) + if err := os.MkdirAll(dir, 0o700); err != nil { + return "", err + } + return dir, nil +} + +func checkExtendsCache(cacheDir string, ref name.Reference, kc authn.Keychain) ([]byte, bool) { + jsonPath := filepath.Join(cacheDir, "devcontainer.json") + digestPath := filepath.Join(cacheDir, "digest") + + // #nosec G304 -- paths derived from our own cache directory, not user input + storedDigest, err := os.ReadFile(digestPath) + if err != nil { + return nil, false + } + // #nosec G304 -- paths derived from our own cache directory, not user input + cachedJSON, err := os.ReadFile(jsonPath) + if err != nil { + return nil, false + } + + desc, err := remote.Head(ref, remote.WithAuthFromKeychain(kc)) + if err != nil { + return nil, false + } + + if desc.Digest.String() == strings.TrimSpace(string(storedDigest)) { + return cachedJSON, true + } + return nil, false +} + +func writeExtendsCache(cacheDir string, ref name.Reference, kc authn.Keychain, data []byte) { + desc, err := remote.Head(ref, remote.WithAuthFromKeychain(kc)) + if err != nil { + return + } + + jsonPath := filepath.Join(cacheDir, "devcontainer.json") + digestPath := filepath.Join(cacheDir, "digest") + _ = os.WriteFile(jsonPath, data, 0o600) + _ = os.WriteFile(digestPath, []byte(desc.Digest.String()), 0o600) } // extractDevContainerJSON reads the first layer of an OCI image as a diff --git a/pkg/devcontainer/config/extends_oci_test.go b/pkg/devcontainer/config/extends_oci_test.go index 5cfcddccd..c64c02ad6 100644 --- a/pkg/devcontainer/config/extends_oci_test.go +++ b/pkg/devcontainer/config/extends_oci_test.go @@ -4,7 +4,10 @@ import ( "archive/tar" "bytes" "compress/gzip" + "context" "net/http/httptest" + "os" + "path/filepath" "strings" "testing" @@ -27,6 +30,8 @@ func TestIsOCIRef(t *testing.T) { {"ghcr.io/owner/repo:tag", true}, {"docker.io/library/ubuntu:latest", true}, {"myregistry.com/org/devcontainer-base:1", true}, + {"oci://ghcr.io/org/repo:tag", true}, + {"oci://relative/path", true}, {"./base.json", false}, {"../shared/base.json", false}, {"/absolute/path.json", false}, @@ -95,7 +100,11 @@ func TestResolveOCIExtends_Integration(t *testing.T) { pushTestImage(t, regHost+"/test/devcontainer-base:latest", jsonContent) visited := map[string]bool{} - cfg, err := resolveOCIExtends(regHost+"/test/devcontainer-base:latest", visited) + cfg, err := resolveOCIExtends( + context.Background(), + regHost+"/test/devcontainer-base:latest", + visited, + ) if err != nil { t.Fatal(err) } @@ -113,11 +122,33 @@ func TestResolveOCIExtends_Integration(t *testing.T) { } } +func TestResolveOCIExtends_OCIPrefix(t *testing.T) { + srv := httptest.NewServer(registry.New()) + defer srv.Close() + + regHost := strings.TrimPrefix(srv.URL, "http://") + + jsonContent := `{"name": "oci-prefix-test", "image": "node:20"}` + pushTestImage(t, regHost+"/org/config:v1", jsonContent) + + visited := map[string]bool{} + cfg, err := resolveOCIExtends(context.Background(), "oci://"+regHost+"/org/config:v1", visited) + if err != nil { + t.Fatal(err) + } + if cfg.Name != "oci-prefix-test" { + t.Errorf("Name: got %q, want 'oci-prefix-test'", cfg.Name) + } + if cfg.Image != "node:20" { + t.Errorf("Image: got %q, want 'node:20'", cfg.Image) + } +} + func TestResolveOCIExtends_CycleDetection(t *testing.T) { ref := "ghcr.io/fake/cycle:1" visited := map[string]bool{ref: true} - _, err := resolveOCIExtends(ref, visited) + _, err := resolveOCIExtends(context.Background(), ref, visited) if err == nil { t.Fatal("expected cycle error") } @@ -126,6 +157,79 @@ func TestResolveOCIExtends_CycleDetection(t *testing.T) { } } +func TestResolveOCIExtends_CacheHit(t *testing.T) { + t.Setenv("XDG_CACHE_HOME", t.TempDir()) + + srv := httptest.NewServer(registry.New()) + defer srv.Close() + + regHost := strings.TrimPrefix(srv.URL, "http://") + ref := regHost + "/test/cache-hit:latest" + + pushTestImage(t, ref, `{"name": "cached", "image": "alpine:3"}`) + + visited := map[string]bool{} + cfg, err := resolveOCIExtends(context.Background(), ref, visited) + if err != nil { + t.Fatal(err) + } + if cfg.Name != "cached" { + t.Fatalf("first resolve: got name %q", cfg.Name) + } + + cacheDir, err := extendsCacheDir(ref) + if err != nil { + t.Fatal(err) + } + if _, err := os.Stat(filepath.Join(cacheDir, "devcontainer.json")); err != nil { + t.Fatal("cache file not written") + } + if _, err := os.Stat(filepath.Join(cacheDir, "digest")); err != nil { + t.Fatal("digest file not written") + } + + visited2 := map[string]bool{} + cfg2, err := resolveOCIExtends(context.Background(), ref, visited2) + if err != nil { + t.Fatal(err) + } + if cfg2.Name != "cached" { + t.Errorf("cache hit: got name %q, want 'cached'", cfg2.Name) + } +} + +func TestResolveOCIExtends_CacheInvalidation(t *testing.T) { + t.Setenv("XDG_CACHE_HOME", t.TempDir()) + + srv := httptest.NewServer(registry.New()) + defer srv.Close() + + regHost := strings.TrimPrefix(srv.URL, "http://") + ref := regHost + "/test/cache-invalidate:latest" + + pushTestImage(t, ref, `{"name": "version1", "image": "alpine:3"}`) + + visited := map[string]bool{} + cfg, err := resolveOCIExtends(context.Background(), ref, visited) + if err != nil { + t.Fatal(err) + } + if cfg.Name != "version1" { + t.Fatalf("first resolve: got name %q", cfg.Name) + } + + pushTestImage(t, ref, `{"name": "version2", "image": "alpine:3.18"}`) + + visited2 := map[string]bool{} + cfg2, err := resolveOCIExtends(context.Background(), ref, visited2) + if err != nil { + t.Fatal(err) + } + if cfg2.Name != "version2" { + t.Errorf("after invalidation: got name %q, want 'version2'", cfg2.Name) + } +} + func pushTestImage(t *testing.T, refStr, jsonContent string) { t.Helper() diff --git a/pkg/devcontainer/config/parse.go b/pkg/devcontainer/config/parse.go index 61e0335d1..d81aeb977 100644 --- a/pkg/devcontainer/config/parse.go +++ b/pkg/devcontainer/config/parse.go @@ -2,6 +2,7 @@ package config import ( "bufio" + "context" "encoding/json" "fmt" "os" @@ -100,7 +101,12 @@ func ParseDevContainerJSONFile(jsonFilePath string) (*DevContainerConfig, error) if !devContainer.Extends.IsEmpty() { visited := map[string]bool{path: true} declaringDir := filepath.Dir(path) - parent, err := resolveExtendsArray(devContainer.Extends, declaringDir, visited) + parent, err := resolveExtendsArray( + context.TODO(), + devContainer.Extends, + declaringDir, + visited, + ) if err != nil { return nil, err }