diff --git a/cmd/hauler/cli/store/add.go b/cmd/hauler/cli/store/add.go index 1acec5c..fe2c598 100644 --- a/cmd/hauler/cli/store/add.go +++ b/cmd/hauler/cli/store/add.go @@ -619,7 +619,13 @@ func rewriteReference(ctx context.Context, s *store.Layout, oldRef name.Referenc // index.docker.io. Preserve the original registry when the source is non-docker. 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 + rewriteRepo := strings.TrimPrefix(rawRewrite, "/") + if i := strings.LastIndex(rewriteRepo, ":"); i != -1 { + rewriteRepo = rewriteRepo[:i] + } + if !strings.HasPrefix(rewriteRepo, "library/") { + newRepo = strings.TrimPrefix(newRepo, "library/") + } } oldTotal := oldRepo + ":" + oldTag newTotal := newRepo + ":" + newTag @@ -1452,6 +1458,7 @@ func fetchChart(ctx context.Context, s *store.Layout, j chartJob, tempRoot strin // rewrite. A rewrite that omits a tag inherits ref's. func rewriteChartReference(ctx context.Context, s *store.Layout, ref name.Reference, rewrite string) error { rewrite = strings.TrimPrefix(rewrite, "/") + rawRewrite := rewrite newRef, err := name.ParseReference(rewrite) if err != nil { // error... don't continue with a bad reference @@ -1474,6 +1481,13 @@ func rewriteChartReference(ctx context.Context, s *store.Layout, ref name.Refere // rename chart name in store oldRepo := ref.Context().RepositoryStr() newRepo := newRef.Context().RepositoryStr() + rewriteRepo := rawRewrite + if i := strings.LastIndex(rewriteRepo, ":"); i != -1 { + rewriteRepo = rewriteRepo[:i] + } + if !strings.HasPrefix(rewriteRepo, "library/") { + newRepo = strings.TrimPrefix(newRepo, "library/") + } newTag := newRef.Identifier() if tag, ok := newRef.(name.Tag); ok { newTag = tag.TagStr() diff --git a/cmd/hauler/cli/store/add_test.go b/cmd/hauler/cli/store/add_test.go index e87ea64..8840802 100644 --- a/cmd/hauler/cli/store/add_test.go +++ b/cmd/hauler/cli/store/add_test.go @@ -375,6 +375,108 @@ func TestRewriteReference(t *testing.T) { // condition fires → registry reverts to host, no library/ to strip assertAnnotationsInStore(t, s, "newrepo/img:v2", host+"/newrepo/img:v2") }) + + // The library/-detection must look at rawRewrite's path (not the + // go-containerregistry-normalized newRepo, which always carries "library/" for + // single-segment repos), so that a rewrite which explicitly asks for + // "library/..." is honored instead of being unconditionally stripped. + + t.Run("path-only rewrite with explicit library/ prefix is preserved", 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("library/nginx:v2") + rawRewrite := "library/nginx:v2" + + if err := rewriteReference(ctx, s, oldRef, newRef, rawRewrite); err != nil { + t.Fatalf("rewriteReference: %v", err) + } + // rewriteRepo (derived from rawRewrite) starts with "library/" → must be kept + assertAnnotationsInStore(t, s, "library/nginx:v2", "index.docker.io/library/nginx:v2") + }) + + t.Run("leading slash rewrite with explicit library/ prefix is preserved", 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("library/nginx:v2") + // AddImageCmd passes the pre-trim rewrite string through as rawRewrite, so a + // leading "/" must still be handled correctly here. + rawRewrite := "/library/nginx:v2" + + if err := rewriteReference(ctx, s, oldRef, newRef, rawRewrite); err != nil { + t.Fatalf("rewriteReference: %v", err) + } + assertAnnotationsInStore(t, s, "library/nginx:v2", "index.docker.io/library/nginx:v2") + }) +} + +func TestRewriteChartReference(t *testing.T) { + ctx := newTestContext(t) + + // A chart rewritten to a bare single-segment name must not keep an erroneous + // "library/" prefix picked up from go-containerregistry's docker hub + // normalization, unless the rewrite explicitly asked for one. + + t.Run("path-only rewrite strips library/ prefix from docker hub normalization", func(t *testing.T) { + s := newTestStore(t) + seedStoreDescriptor(t, s, map[string]string{ + ocispec.AnnotationRefName: "library/mychart:1.0.0", + }) + + ref, _ := name.NewTag("mychart:1.0.0") + if err := rewriteChartReference(ctx, s, ref, "mychart:2.0.0"); err != nil { + t.Fatalf("rewriteChartReference: %v", err) + } + assertArtifactInStore(t, s, "mychart:2.0.0") + }) + + t.Run("explicit library/ prefix in rewrite is preserved", func(t *testing.T) { + s := newTestStore(t) + seedStoreDescriptor(t, s, map[string]string{ + ocispec.AnnotationRefName: "library/mychart:1.0.0", + }) + + ref, _ := name.NewTag("mychart:1.0.0") + if err := rewriteChartReference(ctx, s, ref, "library/mychart:2.0.0"); err != nil { + t.Fatalf("rewriteChartReference: %v", err) + } + assertArtifactInStore(t, s, "library/mychart:2.0.0") + }) + + t.Run("leading slash rewrite with explicit library/ prefix is preserved", func(t *testing.T) { + s := newTestStore(t) + seedStoreDescriptor(t, s, map[string]string{ + ocispec.AnnotationRefName: "library/mychart:1.0.0", + }) + + ref, _ := name.NewTag("mychart:1.0.0") + if err := rewriteChartReference(ctx, s, ref, "/library/mychart:2.0.0"); err != nil { + t.Fatalf("rewriteChartReference: %v", err) + } + assertArtifactInStore(t, s, "library/mychart:2.0.0") + }) + + t.Run("rewrite omitting tag inherits the source tag", func(t *testing.T) { + s := newTestStore(t) + seedStoreDescriptor(t, s, map[string]string{ + ocispec.AnnotationRefName: "library/mychart:1.0.0", + }) + + ref, _ := name.NewTag("mychart:1.0.0") + if err := rewriteChartReference(ctx, s, ref, "myneworg/mychart"); err != nil { + t.Fatalf("rewriteChartReference: %v", err) + } + assertArtifactInStore(t, s, "myneworg/mychart:1.0.0") + }) } // -------------------------------------------------------------------------- diff --git a/cmd/hauler/cli/store/info.go b/cmd/hauler/cli/store/info.go index b896293..0c02550 100644 --- a/cmd/hauler/cli/store/info.go +++ b/cmd/hauler/cli/store/info.go @@ -562,6 +562,26 @@ func newItemWithDigest(s *store.Layout, digestStr string, desc ocispec.Descripto return item } +// resolveDisplayReference returns the fully-qualified reference string to display +// for desc. ContainerdImageNameKey already holds the canonical "registry/repo:tag" +// string exactly as computed when the artifact was stored (see rewriteReference in +// cmd/hauler/cli/store/add.go), so it's used verbatim. Re-parsing it through +// name.ParseReference and calling .Name() would re-trigger go-containerregistry's +// Docker Hub "library/" namespace normalization for any single-segment repo under +// index.docker.io, undoing a rewrite like "hello-world-custom" back to +// "library/hello-world-custom". AnnotationRefName, used as a fallback, has no +// registry component, so it still needs reference.Parse to fill one in. +func resolveDisplayReference(desc ocispec.Descriptor) (string, error) { + if refName := desc.Annotations[consts.ContainerdImageNameKey]; refName != "" { + return refName, nil + } + ref, err := reference.Parse(desc.Annotations[ocispec.AnnotationRefName]) + if err != nil { + return "", err + } + return ref.Name(), nil +} + func newItem(s *store.Layout, desc ocispec.Descriptor, m ocispec.Manifest, plat string, o *flags.InfoOpts) item { var size int64 = 0 for _, l := range m.Layers { @@ -570,11 +590,7 @@ func newItem(s *store.Layout, desc ocispec.Descriptor, m ocispec.Manifest, plat ctype := resolveCtype(desc, m.Config.MediaType) - refName := desc.Annotations[consts.ContainerdImageNameKey] - if refName == "" { - refName = desc.Annotations[ocispec.AnnotationRefName] - } - ref, err := reference.Parse(refName) + refName, err := resolveDisplayReference(desc) if err != nil { return item{} } @@ -584,7 +600,7 @@ func newItem(s *store.Layout, desc ocispec.Descriptor, m ocispec.Manifest, plat } return item{ - Reference: ref.Name(), + Reference: refName, Type: ctype, Platform: plat, Digest: desc.Digest.String(), @@ -637,17 +653,13 @@ func fallbackItem(desc ocispec.Descriptor, plat string, problem store.BlobResult plat = "-" } - refName := desc.Annotations[consts.ContainerdImageNameKey] - if refName == "" { - refName = desc.Annotations[ocispec.AnnotationRefName] - } - ref, err := reference.Parse(refName) + refName, err := resolveDisplayReference(desc) if err != nil { return item{} } return item{ - Reference: ref.Name(), + Reference: refName, Type: resolveCtype(desc, ""), Platform: plat, Layers: 0, diff --git a/cmd/hauler/cli/store/info_test.go b/cmd/hauler/cli/store/info_test.go index a1912f6..55edd54 100644 --- a/cmd/hauler/cli/store/info_test.go +++ b/cmd/hauler/cli/store/info_test.go @@ -215,6 +215,82 @@ func TestNewItem(t *testing.T) { } } +func TestResolveDisplayReference(t *testing.T) { + // ContainerdImageNameKey already holds the fully-qualified reference exactly as + // computed by rewriteReference (see add.go), so it must be returned verbatim. + // Re-parsing it through the reference package would re-trigger + // go-containerregistry's docker hub "library/" normalization for a + // single-segment repo, undoing a rewrite like "hello-world-custom" back to + // "library/hello-world-custom". + t.Run("ContainerdImageNameKey is used verbatim, without re-injecting library/", func(t *testing.T) { + desc := ocispec.Descriptor{ + Annotations: map[string]string{ + consts.ContainerdImageNameKey: "index.docker.io/hello-world-custom:v2", + ocispec.AnnotationRefName: "hello-world-custom:v2", + }, + } + got, err := resolveDisplayReference(desc) + if err != nil { + t.Fatalf("resolveDisplayReference: %v", err) + } + if want := "index.docker.io/hello-world-custom:v2"; got != want { + t.Errorf("got %q, want %q", got, want) + } + }) + + t.Run("falls back to AnnotationRefName parsed when ContainerdImageNameKey absent", func(t *testing.T) { + desc := ocispec.Descriptor{ + Annotations: map[string]string{ + ocispec.AnnotationRefName: "hello-world-custom:v2", + }, + } + got, err := resolveDisplayReference(desc) + if err != nil { + t.Fatalf("resolveDisplayReference: %v", err) + } + if want := "hauler/hello-world-custom:v2"; got != want { + t.Errorf("got %q, want %q", got, want) + } + }) + + t.Run("returns error when fallback ref cannot be parsed", func(t *testing.T) { + desc := ocispec.Descriptor{Annotations: map[string]string{}} + if _, err := resolveDisplayReference(desc); err == nil { + t.Fatal("expected error, got nil") + } + }) +} + +func TestNewItem_ReferenceUsesContainerdImageNameVerbatim(t *testing.T) { + desc := ocispec.Descriptor{ + Annotations: map[string]string{ + consts.ContainerdImageNameKey: "index.docker.io/hello-world-custom:v2", + ocispec.AnnotationRefName: "hello-world-custom:v2", + }, + } + m := ocispec.Manifest{Config: ocispec.Descriptor{MediaType: consts.DockerConfigJSON}} + o := &flags.InfoOpts{TypeFilter: "all"} + + got := newItem(nil, desc, m, "linux/amd64", o) + if want := "index.docker.io/hello-world-custom:v2"; got.Reference != want { + t.Errorf("got Reference %q, want %q", got.Reference, want) + } +} + +func TestFallbackItem_ReferenceUsesContainerdImageNameVerbatim(t *testing.T) { + desc := ocispec.Descriptor{ + Annotations: map[string]string{ + consts.ContainerdImageNameKey: "index.docker.io/hello-world-custom:v2", + ocispec.AnnotationRefName: "hello-world-custom:v2", + }, + } + + got := fallbackItem(desc, "linux/amd64", store.BlobResult{}) + if want := "index.docker.io/hello-world-custom:v2"; got.Reference != want { + t.Errorf("got Reference %q, want %q", got.Reference, want) + } +} + func TestInfoCmd(t *testing.T) { ctx := newTestContext(t) s := newTestStore(t)