diff --git a/cmd/hauler/cli/store/add.go b/cmd/hauler/cli/store/add.go index 84dd50f..f7373a4 100644 --- a/cmd/hauler/cli/store/add.go +++ b/cmd/hauler/cli/store/add.go @@ -133,6 +133,7 @@ func storeImage(ctx context.Context, s *store.Layout, i v1.Image, platform strin } if rewrite != "" { + rawRewrite := rewrite rewrite = strings.TrimPrefix(rewrite, "/") if !strings.Contains(rewrite, ":") { if tag, ok := r.(name.Tag); ok { @@ -146,7 +147,7 @@ func storeImage(ctx context.Context, s *store.Layout, i v1.Image, platform strin if err != nil { return fmt.Errorf("unable to parse rewrite name [%s]: %w", rewrite, err) } - if err := rewriteReference(ctx, s, r, newRef); err != nil { + if err := rewriteReference(ctx, s, r, newRef, rawRewrite); err != nil { return err } } @@ -155,7 +156,7 @@ func storeImage(ctx context.Context, s *store.Layout, i v1.Image, platform strin return nil } -func rewriteReference(ctx context.Context, s *store.Layout, oldRef name.Reference, newRef name.Reference) error { +func rewriteReference(ctx context.Context, s *store.Layout, oldRef name.Reference, newRef name.Reference, rawRewrite string) error { l := log.FromContext(ctx) if err := s.OCI.LoadIndex(); err != nil { @@ -184,8 +185,9 @@ func rewriteReference(ctx context.Context, s *store.Layout, oldRef name.Referenc newRegistry := newRefContext.RegistryStr() // If user omitted a registry in the rewrite string, go-containerregistry defaults to // index.docker.io. Preserve the original registry when the source is non-docker. - if newRegistry == "index.docker.io" && oldRegistry != "index.docker.io" { + if newRegistry == "index.docker.io" && !strings.HasPrefix(rawRewrite, "docker.io") && !strings.HasPrefix(rawRewrite, "index.docker.io") { newRegistry = oldRegistry + newRepo = strings.TrimPrefix(newRepo, "library/") //if rewrite has library/ prefix in path it is stripped off unless registry specified in rewrite } oldTotal := oldRepo + ":" + oldTag newTotal := newRepo + ":" + newTag diff --git a/cmd/hauler/cli/store/add_test.go b/cmd/hauler/cli/store/add_test.go index b94cabb..a72f2a1 100644 --- a/cmd/hauler/cli/store/add_test.go +++ b/cmd/hauler/cli/store/add_test.go @@ -12,6 +12,7 @@ import ( "github.com/google/go-containerregistry/pkg/name" "github.com/google/go-containerregistry/pkg/registry" "github.com/google/go-containerregistry/pkg/v1/remote" + ocispec "github.com/opencontainers/image-spec/specs-go/v1" helmchart "helm.sh/helm/v3/pkg/chart" "hauler.dev/go/hauler/internal/flags" @@ -248,7 +249,9 @@ func TestRewriteReference(t *testing.T) { t.Fatalf("parse newRef: %v", err) } - if err := rewriteReference(ctx, s, oldRef, newRef); err != nil { + rawRewrite := newRef.String() + + if err := rewriteReference(ctx, s, oldRef, newRef, rawRewrite); err != nil { t.Fatalf("rewriteReference: %v", err) } @@ -259,8 +262,9 @@ func TestRewriteReference(t *testing.T) { s := newTestStore(t) oldRef, _ := name.NewTag("docker.io/missing/repo:v1") newRef, _ := name.NewTag("docker.io/new/repo:v2") + rawRewrite := newRef.String() - err := rewriteReference(ctx, s, oldRef, newRef) + err := rewriteReference(ctx, s, oldRef, newRef, rawRewrite) if err == nil { t.Fatal("expected error, got nil") } @@ -268,6 +272,86 @@ func TestRewriteReference(t *testing.T) { t.Errorf("expected 'could not find' in error, got: %v", err) } }) + + // Tests for the registry-preservation / library/-stripping logic (lines 188-191). + // go-containerregistry normalises bare single-name Docker Hub refs (e.g. "nginx:latest") + // to "index.docker.io/library/nginx:latest". When the rewrite string omits a registry, + // rewriteReference must (a) preserve the source registry and (b) strip the injected + // "library/" prefix so that the stored ref looks like "nginx:v2", not "library/nginx:v2". + + t.Run("path-only rewrite strips library/ prefix from docker hub official image", func(t *testing.T) { + s := newTestStore(t) + seedStoreDescriptor(t, s, map[string]string{ + ocispec.AnnotationRefName: "library/nginx:latest", + consts.ContainerdImageNameKey: "index.docker.io/library/nginx:latest", + }) + + oldRef, _ := name.NewTag("nginx:latest") // → index.docker.io/library/nginx:latest + newRef, _ := name.NewTag("nginx:v2") // → index.docker.io/library/nginx:v2 + rawRewrite := "nginx:v2" + + if err := rewriteReference(ctx, s, oldRef, newRef, rawRewrite); err != nil { + t.Fatalf("rewriteReference: %v", err) + } + // library/ must be stripped; registry stays index.docker.io + assertAnnotationsInStore(t, s, "nginx:v2", "index.docker.io/nginx:v2") + }) + + t.Run("explicit docker.io rewrite preserves library/ prefix", func(t *testing.T) { + s := newTestStore(t) + seedStoreDescriptor(t, s, map[string]string{ + ocispec.AnnotationRefName: "library/nginx:latest", + consts.ContainerdImageNameKey: "index.docker.io/library/nginx:latest", + }) + + oldRef, _ := name.NewTag("nginx:latest") + newRef, _ := name.NewTag("docker.io/nginx:v2") // → index.docker.io/library/nginx:v2 + rawRewrite := "docker.io/nginx:v2" + + if err := rewriteReference(ctx, s, oldRef, newRef, rawRewrite); err != nil { + t.Fatalf("rewriteReference: %v", err) + } + // rawRewrite starts with "docker.io" → condition must NOT fire → library/ preserved + assertAnnotationsInStore(t, s, "library/nginx:v2", "index.docker.io/library/nginx:v2") + }) + + t.Run("explicit index.docker.io rewrite preserves library/ prefix", func(t *testing.T) { + s := newTestStore(t) + seedStoreDescriptor(t, s, map[string]string{ + ocispec.AnnotationRefName: "library/nginx:latest", + consts.ContainerdImageNameKey: "index.docker.io/library/nginx:latest", + }) + + oldRef, _ := name.NewTag("nginx:latest") + newRef, _ := name.NewTag("index.docker.io/nginx:v2") // → index.docker.io/library/nginx:v2 + rawRewrite := "index.docker.io/nginx:v2" + + if err := rewriteReference(ctx, s, oldRef, newRef, rawRewrite); err != nil { + t.Fatalf("rewriteReference: %v", err) + } + // rawRewrite starts with "index.docker.io" → condition must NOT fire → library/ preserved + assertAnnotationsInStore(t, s, "library/nginx:v2", "index.docker.io/library/nginx:v2") + }) + + t.Run("non-docker source with path-only rewrite preserves original registry", func(t *testing.T) { + host, rOpts := newTestRegistry(t) + seedImage(t, host, "src/repo", "v1", rOpts...) + + s := newTestStore(t) + if err := s.AddImage(ctx, host+"/src/repo:v1", "", rOpts...); err != nil { + t.Fatalf("AddImage: %v", err) + } + + oldRef, _ := name.NewTag(host+"/src/repo:v1", name.Insecure) + newRef, _ := name.NewTag("newrepo/img:v2") // defaults to index.docker.io + rawRewrite := "newrepo/img:v2" + + if err := rewriteReference(ctx, s, oldRef, newRef, rawRewrite); err != nil { + t.Fatalf("rewriteReference: %v", err) + } + // condition fires → registry reverts to host, no library/ to strip + assertAnnotationsInStore(t, s, "newrepo/img:v2", host+"/newrepo/img:v2") + }) } // -------------------------------------------------------------------------- diff --git a/cmd/hauler/cli/store/testhelpers_test.go b/cmd/hauler/cli/store/testhelpers_test.go index c179cb4..27cbd60 100644 --- a/cmd/hauler/cli/store/testhelpers_test.go +++ b/cmd/hauler/cli/store/testhelpers_test.go @@ -22,6 +22,7 @@ import ( "github.com/google/go-containerregistry/pkg/v1/remote" "github.com/google/go-containerregistry/pkg/v1/static" gvtypes "github.com/google/go-containerregistry/pkg/v1/types" + digest "github.com/opencontainers/go-digest" ocispec "github.com/opencontainers/image-spec/specs-go/v1" "github.com/rs/zerolog" "helm.sh/helm/v3/pkg/action" @@ -282,6 +283,41 @@ func seedOCI11Referrer(t *testing.T, host, repo string, baseImg gcrv1.Image, opt } } +// seedStoreDescriptor injects a descriptor with the given annotations directly +// into the store index without requiring a real registry or blob. This is used +// to pre-populate the store for rewriteReference unit tests. +func seedStoreDescriptor(t *testing.T, s *store.Layout, annotations map[string]string) { + t.Helper() + desc := ocispec.Descriptor{ + MediaType: ocispec.MediaTypeImageManifest, + Digest: digest.Digest("sha256:" + strings.Repeat("a", 64)), + Size: 1, + Annotations: annotations, + } + if err := s.OCI.AddIndex(desc); err != nil { + t.Fatalf("seedStoreDescriptor: %v", err) + } +} + +// assertAnnotationsInStore walks the store and fails if no descriptor has both +// AnnotationRefName == refName AND ContainerdImageNameKey == containerdName. +func assertAnnotationsInStore(t *testing.T, s *store.Layout, refName, containerdName string) { + t.Helper() + found := false + if err := s.OCI.Walk(func(_ string, desc ocispec.Descriptor) error { + if desc.Annotations[ocispec.AnnotationRefName] == refName && + desc.Annotations[consts.ContainerdImageNameKey] == containerdName { + found = true + } + return nil + }); err != nil { + t.Fatalf("assertAnnotationsInStore walk: %v", err) + } + if !found { + t.Errorf("no artifact with AnnotationRefName=%q and ContainerdImageNameKey=%q found in store", refName, containerdName) + } +} + // assertReferrerInStore walks the store and fails if no descriptor has a kind // annotation with the KindAnnotationReferrers prefix and a ref containing refSubstring. func assertReferrerInStore(t *testing.T, s *store.Layout, refSubstring string) {