From fb4f2ab2cb92d8487884ec7b7e41f41acd4c5764 Mon Sep 17 00:00:00 2001 From: Eric Smalling Date: Thu, 4 Sep 2025 12:17:42 -0500 Subject: [PATCH 1/2] Add ability to create multiple tags in one call. Fixes #1797 Signed-off-by: Eric Smalling --- cmd/crane/cmd/tag.go | 21 ++- cmd/crane/doc/crane_tag.md | 10 +- pkg/crane/tag.go | 28 ++- pkg/crane/tag_test.go | 369 +++++++++++++++++++++++++++++++++++++ 4 files changed, 417 insertions(+), 11 deletions(-) create mode 100644 pkg/crane/tag_test.go diff --git a/cmd/crane/cmd/tag.go b/cmd/crane/cmd/tag.go index eb2ba2f83..cd6c51e82 100644 --- a/cmd/crane/cmd/tag.go +++ b/cmd/crane/cmd/tag.go @@ -22,7 +22,7 @@ import ( // NewCmdTag creates a new cobra.Command for the tag subcommand. func NewCmdTag(options *[]crane.Option) *cobra.Command { return &cobra.Command{ - Use: "tag IMG TAG", + Use: "tag IMG TAG [TAG...]", Short: "Efficiently tag a remote image", Long: `Tag remote image without downloading it. @@ -34,13 +34,22 @@ crane cp registry.example.com/library/ubuntu:v0 registry.example.com/library/ubu crane tag registry.example.com/library/ubuntu:v0 v1 ` + "```" + ` -2. We can skip layer existence checks because we know the manifest already exists. This makes "tag" slightly faster than "copy".`, +2. We can skip layer existence checks because we know the manifest already exists. This makes "tag" slightly faster than "copy". + +You can also specify multiple tags to apply to the same image: +` + "```" + ` +crane tag registry.example.com/library/ubuntu:v0 v1 v2 latest +` + "```" + ``, Example: `# Add a v1 tag to ubuntu -crane tag ubuntu v1`, - Args: cobra.ExactArgs(2), +crane tag ubuntu v1 + +# Add multiple tags to ubuntu +crane tag ubuntu v1 v2 latest`, + Args: cobra.MinimumNArgs(2), RunE: func(_ *cobra.Command, args []string) error { - img, tag := args[0], args[1] - return crane.Tag(img, tag, *options...) + img := args[0] + tags := args[1:] + return crane.Tag(img, tags, *options...) }, } } diff --git a/cmd/crane/doc/crane_tag.md b/cmd/crane/doc/crane_tag.md index dcb2e3129..f77680520 100644 --- a/cmd/crane/doc/crane_tag.md +++ b/cmd/crane/doc/crane_tag.md @@ -16,8 +16,13 @@ crane tag registry.example.com/library/ubuntu:v0 v1 2. We can skip layer existence checks because we know the manifest already exists. This makes "tag" slightly faster than "copy". +You can also specify multiple tags to apply to the same image: ``` -crane tag IMG TAG [flags] +crane tag registry.example.com/library/ubuntu:v0 v1 v2 latest +``` + +``` +crane tag IMG TAG [TAG...] [flags] ``` ### Examples @@ -25,6 +30,9 @@ crane tag IMG TAG [flags] ``` # Add a v1 tag to ubuntu crane tag ubuntu v1 + +# Add multiple tags to ubuntu +crane tag ubuntu v1 v2 latest ``` ### Options diff --git a/pkg/crane/tag.go b/pkg/crane/tag.go index 13bc39587..0e3d2fa69 100644 --- a/pkg/crane/tag.go +++ b/pkg/crane/tag.go @@ -21,8 +21,8 @@ import ( "github.com/google/go-containerregistry/pkg/v1/remote" ) -// Tag adds tag to the remote img. -func Tag(img, tag string, opt ...Option) error { +// Tag adds one or more tags to the remote img. +func Tag(img string, tags any, opt ...Option) error { o := makeOptions(opt...) ref, err := name.ParseReference(img, o.Name...) if err != nil { @@ -33,7 +33,27 @@ func Tag(img, tag string, opt ...Option) error { return fmt.Errorf("fetching %q: %w", img, err) } - dst := ref.Context().Tag(tag) + // Handle both single tag (string) and multiple tags ([]string) for backwards compatibility + var tagList []string + switch t := tags.(type) { + case string: + tagList = []string{t} + case []string: + tagList = t + default: + return fmt.Errorf("tags must be string or []string, got %T", tags) + } + + // Apply each tag + for i, tag := range tagList { + dst := ref.Context().Tag(tag) + if err := remote.Tag(dst, desc, o.Remote...); err != nil { + if i > 0 { + return fmt.Errorf("tagging %q with %q failed (successfully tagged with %v): %w", img, tag, tagList[:i], err) + } + return fmt.Errorf("tagging %q with %q: %w", img, tag, err) + } + } - return remote.Tag(dst, desc, o.Remote...) + return nil } diff --git a/pkg/crane/tag_test.go b/pkg/crane/tag_test.go new file mode 100644 index 000000000..f1d74da6b --- /dev/null +++ b/pkg/crane/tag_test.go @@ -0,0 +1,369 @@ +// Copyright 2019 Google LLC All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package crane_test + +import ( + "fmt" + "net/http/httptest" + "net/url" + "strings" + "testing" + + "github.com/google/go-containerregistry/pkg/crane" + "github.com/google/go-containerregistry/pkg/name" + "github.com/google/go-containerregistry/pkg/registry" + "github.com/google/go-containerregistry/pkg/v1/compare" + "github.com/google/go-containerregistry/pkg/v1/random" + "github.com/google/go-containerregistry/pkg/v1/remote" +) + +func TestTagSingle(t *testing.T) { + // Set up a fake registry. + s := httptest.NewServer(registry.New()) + defer s.Close() + u, err := url.Parse(s.URL) + if err != nil { + t.Fatal(err) + } + + src := fmt.Sprintf("%s/test/tag-single", u.Host) + tagName := "single-tag" + + // Create and push a test image. + img, err := random.Image(1024, 3) + if err != nil { + t.Fatal(err) + } + + if err := crane.Push(img, src); err != nil { + t.Fatal(err) + } + + // Test single tag using string (backward compatibility). + if err := crane.Tag(src, tagName); err != nil { + t.Fatalf("Tag single string failed: %v", err) + } + + // Verify the tag was created. + tagged, err := crane.Pull(fmt.Sprintf("%s:%s", src, tagName)) + if err != nil { + t.Fatalf("Failed to pull tagged image: %v", err) + } + + if err := compare.Images(img, tagged); err != nil { + t.Fatalf("Tagged image differs from original: %v", err) + } + + // Verify tag exists in listing. + tags, err := crane.ListTags(src) + if err != nil { + t.Fatal(err) + } + + found := false + for _, tag := range tags { + if tag == tagName { + found = true + break + } + } + if !found { + t.Fatalf("Tag %q not found in listing: %v", tagName, tags) + } +} + +func TestTagMultiple(t *testing.T) { + // Set up a fake registry. + s := httptest.NewServer(registry.New()) + defer s.Close() + u, err := url.Parse(s.URL) + if err != nil { + t.Fatal(err) + } + + src := fmt.Sprintf("%s/test/tag-multiple", u.Host) + tagNames := []string{"tag1", "tag2", "tag3"} + + // Create and push a test image. + img, err := random.Image(1024, 3) + if err != nil { + t.Fatal(err) + } + + if err := crane.Push(img, src); err != nil { + t.Fatal(err) + } + + // Test multiple tags using slice. + if err := crane.Tag(src, tagNames); err != nil { + t.Fatalf("Tag multiple strings failed: %v", err) + } + + // Verify all tags were created and point to the same image. + for _, tagName := range tagNames { + tagged, err := crane.Pull(fmt.Sprintf("%s:%s", src, tagName)) + if err != nil { + t.Fatalf("Failed to pull tagged image %s: %v", tagName, err) + } + + if err := compare.Images(img, tagged); err != nil { + t.Fatalf("Tagged image %s differs from original: %v", tagName, err) + } + } + + // Verify all tags exist in listing. + tags, err := crane.ListTags(src) + if err != nil { + t.Fatal(err) + } + + for _, expectedTag := range tagNames { + found := false + for _, tag := range tags { + if tag == expectedTag { + found = true + break + } + } + if !found { + t.Fatalf("Tag %q not found in listing: %v", expectedTag, tags) + } + } + + // Verify the total number of tags (original 'latest' + our 3 tags). + expectedCount := len(tagNames) + 1 // +1 for 'latest' + if len(tags) != expectedCount { + t.Fatalf("Expected %d tags, got %d: %v", expectedCount, len(tags), tags) + } +} + +func TestTagEmpty(t *testing.T) { + // Set up a fake registry. + s := httptest.NewServer(registry.New()) + defer s.Close() + u, err := url.Parse(s.URL) + if err != nil { + t.Fatal(err) + } + + src := fmt.Sprintf("%s/test/tag-empty", u.Host) + + // Create and push a test image. + img, err := random.Image(1024, 3) + if err != nil { + t.Fatal(err) + } + + if err := crane.Push(img, src); err != nil { + t.Fatal(err) + } + + // Test empty slice - should be a no-op. + emptyTags := []string{} + if err := crane.Tag(src, emptyTags); err != nil { + t.Fatalf("Tag with empty slice failed: %v", err) + } + + // Verify no additional tags were created (should still just have 'latest'). + tags, err := crane.ListTags(src) + if err != nil { + t.Fatal(err) + } + + if len(tags) != 1 || tags[0] != "latest" { + t.Fatalf("Expected only 'latest' tag, got: %v", tags) + } +} + +func TestTagInvalidType(t *testing.T) { + // Set up a fake registry. + s := httptest.NewServer(registry.New()) + defer s.Close() + u, err := url.Parse(s.URL) + if err != nil { + t.Fatal(err) + } + + src := fmt.Sprintf("%s/test/tag-invalid", u.Host) + + // Create and push a test image. + img, err := random.Image(1024, 3) + if err != nil { + t.Fatal(err) + } + + if err := crane.Push(img, src); err != nil { + t.Fatal(err) + } + + // Test invalid type (int). + if err := crane.Tag(src, 42); err == nil { + t.Fatal("Expected error for invalid type, got nil") + } else if err.Error() != "tags must be string or []string, got int" { + t.Fatalf("Unexpected error message: %v", err) + } + + // Test invalid type (nil). + if err := crane.Tag(src, nil); err == nil { + t.Fatal("Expected error for nil, got nil") + } + + // Test invalid type (map). + if err := crane.Tag(src, map[string]string{"tag": "value"}); err == nil { + t.Fatal("Expected error for map type, got nil") + } +} + +func TestTagWithInvalidImageRef(t *testing.T) { + invalidRef := "/dev/null/@@@@@@" + + // Test single tag with invalid reference. + if err := crane.Tag(invalidRef, "tag"); err == nil { + t.Fatal("Expected error for invalid image reference, got nil") + } + + // Test multiple tags with invalid reference. + if err := crane.Tag(invalidRef, []string{"tag1", "tag2"}); err == nil { + t.Fatal("Expected error for invalid image reference with multiple tags, got nil") + } +} + +func TestTagWithNonExistentImage(t *testing.T) { + // Set up a fake registry. + s := httptest.NewServer(registry.New()) + defer s.Close() + u, err := url.Parse(s.URL) + if err != nil { + t.Fatal(err) + } + + nonExistent := fmt.Sprintf("%s/does/not/exist", u.Host) + + // Test single tag with non-existent image. + if err := crane.Tag(nonExistent, "tag"); err == nil { + t.Fatal("Expected error for non-existent image, got nil") + } + + // Test multiple tags with non-existent image. + if err := crane.Tag(nonExistent, []string{"tag1", "tag2"}); err == nil { + t.Fatal("Expected error for non-existent image with multiple tags, got nil") + } +} + +func TestTagFailurePartialApplication(t *testing.T) { + // Set up a fake registry. + s := httptest.NewServer(registry.New()) + defer s.Close() + u, err := url.Parse(s.URL) + if err != nil { + t.Fatal(err) + } + + src := fmt.Sprintf("%s/test/tag-failure", u.Host) + + // Create and push a test image. + img, err := random.Image(1024, 3) + if err != nil { + t.Fatal(err) + } + + if err := crane.Push(img, src); err != nil { + t.Fatal(err) + } + + // Test multiple tags where one is invalid (contains invalid characters). + invalidTags := []string{"valid-tag", "invalid/tag/with/slashes", "another-valid-tag"} + + err = crane.Tag(src, invalidTags) + if err == nil { + t.Fatal("Expected error for invalid tag name, got nil") + } + + // Check that the error message includes information about successful tags. + expectedErrMsg := "successfully tagged with [valid-tag]" + if !strings.Contains(err.Error(), expectedErrMsg) { + t.Fatalf("Expected error to contain %q, got: %v", expectedErrMsg, err.Error()) + } + + // Verify that partial tags were created (non-atomic behavior). + tags, err := crane.ListTags(src) + if err != nil { + t.Fatal(err) + } + + // Should have 'latest' and 'valid-tag' (the one that succeeded before failure). + expectedTags := []string{"latest", "valid-tag"} + if len(tags) != len(expectedTags) { + t.Fatalf("Expected %d tags after partial tagging, got %d: %v", len(expectedTags), len(tags), tags) + } + + // Check that valid-tag exists. + found := false + for _, tag := range tags { + if tag == "valid-tag" { + found = true + break + } + } + if !found { + t.Fatalf("Expected 'valid-tag' to exist after partial tagging, got: %v", tags) + } +} + +func TestTagIntegrationWithRemote(t *testing.T) { + // Set up a fake registry. + s := httptest.NewServer(registry.New()) + defer s.Close() + u, err := url.Parse(s.URL) + if err != nil { + t.Fatal(err) + } + + src := fmt.Sprintf("%s/test/tag-integration", u.Host) + + // Create and push a test image. + img, err := random.Image(1024, 3) + if err != nil { + t.Fatal(err) + } + + ref, err := name.ParseReference(src) + if err != nil { + t.Fatal(err) + } + + if err := remote.Write(ref, img); err != nil { + t.Fatal(err) + } + + // Test that our Tag function works with images pushed via remote.Write. + testTags := []string{"integration-test-1", "integration-test-2"} + + if err := crane.Tag(src, testTags); err != nil { + t.Fatalf("Tag integration test failed: %v", err) + } + + // Verify tags were created correctly. + for _, tagName := range testTags { + tagged, err := crane.Pull(fmt.Sprintf("%s:%s", src, tagName)) + if err != nil { + t.Fatalf("Failed to pull integration test tagged image %s: %v", tagName, err) + } + + if err := compare.Images(img, tagged); err != nil { + t.Fatalf("Integration test tagged image %s differs from original: %v", tagName, err) + } + } +} \ No newline at end of file From a37e7e61dfcfeef0f9e2a09b3be412911bab353a Mon Sep 17 00:00:00 2001 From: Eric Smalling Date: Thu, 4 Sep 2025 15:53:01 -0500 Subject: [PATCH 2/2] Keeping backward compatible api Signed-off-by: Eric Smalling --- cmd/crane/cmd/tag.go | 2 +- pkg/crane/tag.go | 24 +++++++++--------------- pkg/crane/tag_test.go | 35 ++++++++++++----------------------- 3 files changed, 22 insertions(+), 39 deletions(-) diff --git a/cmd/crane/cmd/tag.go b/cmd/crane/cmd/tag.go index cd6c51e82..4a25949dc 100644 --- a/cmd/crane/cmd/tag.go +++ b/cmd/crane/cmd/tag.go @@ -49,7 +49,7 @@ crane tag ubuntu v1 v2 latest`, RunE: func(_ *cobra.Command, args []string) error { img := args[0] tags := args[1:] - return crane.Tag(img, tags, *options...) + return crane.TagMultiple(img, tags, *options...) }, } } diff --git a/pkg/crane/tag.go b/pkg/crane/tag.go index 0e3d2fa69..e945fc1b4 100644 --- a/pkg/crane/tag.go +++ b/pkg/crane/tag.go @@ -21,8 +21,13 @@ import ( "github.com/google/go-containerregistry/pkg/v1/remote" ) -// Tag adds one or more tags to the remote img. -func Tag(img string, tags any, opt ...Option) error { +// Tag adds tag to the remote img. +func Tag(img, tag string, opt ...Option) error { + return TagMultiple(img, []string{tag}, opt...) +} + +// TagMultiple adds one or more tags to the remote img. +func TagMultiple(img string, tags []string, opt ...Option) error { o := makeOptions(opt...) ref, err := name.ParseReference(img, o.Name...) if err != nil { @@ -33,23 +38,12 @@ func Tag(img string, tags any, opt ...Option) error { return fmt.Errorf("fetching %q: %w", img, err) } - // Handle both single tag (string) and multiple tags ([]string) for backwards compatibility - var tagList []string - switch t := tags.(type) { - case string: - tagList = []string{t} - case []string: - tagList = t - default: - return fmt.Errorf("tags must be string or []string, got %T", tags) - } - // Apply each tag - for i, tag := range tagList { + for i, tag := range tags { dst := ref.Context().Tag(tag) if err := remote.Tag(dst, desc, o.Remote...); err != nil { if i > 0 { - return fmt.Errorf("tagging %q with %q failed (successfully tagged with %v): %w", img, tag, tagList[:i], err) + return fmt.Errorf("tagging %q with %q failed (successfully tagged with %v): %w", img, tag, tags[:i], err) } return fmt.Errorf("tagging %q with %q: %w", img, tag, err) } diff --git a/pkg/crane/tag_test.go b/pkg/crane/tag_test.go index f1d74da6b..4f6e7e317 100644 --- a/pkg/crane/tag_test.go +++ b/pkg/crane/tag_test.go @@ -107,7 +107,7 @@ func TestTagMultiple(t *testing.T) { } // Test multiple tags using slice. - if err := crane.Tag(src, tagNames); err != nil { + if err := crane.TagMultiple(src, tagNames); err != nil { t.Fatalf("Tag multiple strings failed: %v", err) } @@ -172,7 +172,7 @@ func TestTagEmpty(t *testing.T) { // Test empty slice - should be a no-op. emptyTags := []string{} - if err := crane.Tag(src, emptyTags); err != nil { + if err := crane.TagMultiple(src, emptyTags); err != nil { t.Fatalf("Tag with empty slice failed: %v", err) } @@ -208,21 +208,10 @@ func TestTagInvalidType(t *testing.T) { t.Fatal(err) } - // Test invalid type (int). - if err := crane.Tag(src, 42); err == nil { - t.Fatal("Expected error for invalid type, got nil") - } else if err.Error() != "tags must be string or []string, got int" { - t.Fatalf("Unexpected error message: %v", err) - } - - // Test invalid type (nil). - if err := crane.Tag(src, nil); err == nil { - t.Fatal("Expected error for nil, got nil") - } - - // Test invalid type (map). - if err := crane.Tag(src, map[string]string{"tag": "value"}); err == nil { - t.Fatal("Expected error for map type, got nil") + // Test that TagMultiple properly validates input (nil slice). + if err := crane.TagMultiple(src, nil); err != nil { + // This should succeed as a no-op for nil slice + t.Fatalf("TagMultiple with nil slice should succeed: %v", err) } } @@ -235,7 +224,7 @@ func TestTagWithInvalidImageRef(t *testing.T) { } // Test multiple tags with invalid reference. - if err := crane.Tag(invalidRef, []string{"tag1", "tag2"}); err == nil { + if err := crane.TagMultiple(invalidRef, []string{"tag1", "tag2"}); err == nil { t.Fatal("Expected error for invalid image reference with multiple tags, got nil") } } @@ -257,7 +246,7 @@ func TestTagWithNonExistentImage(t *testing.T) { } // Test multiple tags with non-existent image. - if err := crane.Tag(nonExistent, []string{"tag1", "tag2"}); err == nil { + if err := crane.TagMultiple(nonExistent, []string{"tag1", "tag2"}); err == nil { t.Fatal("Expected error for non-existent image with multiple tags, got nil") } } @@ -286,7 +275,7 @@ func TestTagFailurePartialApplication(t *testing.T) { // Test multiple tags where one is invalid (contains invalid characters). invalidTags := []string{"valid-tag", "invalid/tag/with/slashes", "another-valid-tag"} - err = crane.Tag(src, invalidTags) + err = crane.TagMultiple(src, invalidTags) if err == nil { t.Fatal("Expected error for invalid tag name, got nil") } @@ -348,11 +337,11 @@ func TestTagIntegrationWithRemote(t *testing.T) { t.Fatal(err) } - // Test that our Tag function works with images pushed via remote.Write. + // Test that our TagMultiple function works with images pushed via remote.Write. testTags := []string{"integration-test-1", "integration-test-2"} - if err := crane.Tag(src, testTags); err != nil { - t.Fatalf("Tag integration test failed: %v", err) + if err := crane.TagMultiple(src, testTags); err != nil { + t.Fatalf("TagMultiple integration test failed: %v", err) } // Verify tags were created correctly.