Merge pull request #30906 from TerryHowe/backport-tag-fix

[v3 backport] Prevent push cmd failure in 3.18 by handling version tag resolution in ORAS memory store

pkg/registry/client.go

@@ -711,19 +711,9 @@ func (c *Client) Push(data []byte, ref string, options ...PushOption) (*PushResu
 	})
 
 	ociAnnotations := generateOCIAnnotations(meta, operation.creationTime)
-	manifest := ocispec.Manifest{
-		Versioned:   specs.Versioned{SchemaVersion: 2},
-		Config:      configDescriptor,
-		Layers:      layers,
-		Annotations: ociAnnotations,
-	}
 
-	manifestData, err := json.Marshal(manifest)
-	if err != nil {
-		return nil, err
-	}
-
-	manifestDescriptor, err := oras.TagBytes(ctx, memoryStore, ocispec.MediaTypeImageManifest, manifestData, ref)
+	manifestDescriptor, err := c.tagManifest(ctx, memoryStore, configDescriptor,
+		layers, ociAnnotations, parsedRef)
 	if err != nil {
 		return nil, err
 	}
@@ -924,3 +914,24 @@ func (c *Client) ValidateReference(ref, version string, u *url.URL) (*url.URL, e
 
 	return u, err
 }
+
+// tagManifest prepares and tags a manifest in memory storage
+func (c *Client) tagManifest(ctx context.Context, memoryStore *memory.Store,
+	configDescriptor ocispec.Descriptor, layers []ocispec.Descriptor,
+	ociAnnotations map[string]string, parsedRef reference) (ocispec.Descriptor, error) {
+
+	manifest := ocispec.Manifest{
+		Versioned:   specs.Versioned{SchemaVersion: 2},
+		Config:      configDescriptor,
+		Layers:      layers,
+		Annotations: ociAnnotations,
+	}
+
+	manifestData, err := json.Marshal(manifest)
+	if err != nil {
+		return ocispec.Descriptor{}, err
+	}
+
+	return oras.TagBytes(ctx, memoryStore, ocispec.MediaTypeImageManifest,
+		manifestData, parsedRef.String())
+}

pkg/registry/client_test.go

@@ -17,12 +17,15 @@ limitations under the License.
 package registry
 
 import (
+	"context"
 	"io"
 	"testing"
 
 	"github.com/containerd/containerd/remotes"
+	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"oras.land/oras-go/v2/content/memory"
 )
 
 func TestNewClientResolverNotSupported(t *testing.T) {
@@ -52,3 +55,30 @@ func TestStripURL(t *testing.T) {
 	assert.Equal(t, "127.0.0.1:15000", client.stripURL("127.0.0.1:15000/asdf/asdf"))
 	assert.Equal(t, "127.0.0.1:15000", client.stripURL("127.0.0.1:15000"))
 }
+
+// Inspired by oras test
+// https://github.com/oras-project/oras-go/blob/05a2b09cbf2eab1df691411884dc4df741ec56ab/content_test.go#L1802
+func TestTagManifestTransformsReferences(t *testing.T) {
+	memStore := memory.New()
+	client := &Client{out: io.Discard}
+	ctx := context.Background()
+
+	refWithPlus := "test-registry.io/charts/test:1.0.0+metadata"
+	expectedRef := "test-registry.io/charts/test:1.0.0_metadata" // + becomes _
+
+	configDesc := ocispec.Descriptor{MediaType: ConfigMediaType, Digest: "sha256:config", Size: 100}
+	layers := []ocispec.Descriptor{{MediaType: ChartLayerMediaType, Digest: "sha256:layer", Size: 200}}
+
+	parsedRef, err := newReference(refWithPlus)
+	require.NoError(t, err)
+
+	desc, err := client.tagManifest(ctx, memStore, configDesc, layers, nil, parsedRef)
+	require.NoError(t, err)
+
+	transformedDesc, err := memStore.Resolve(ctx, expectedRef)
+	require.NoError(t, err, "Should find the reference with _ instead of +")
+	require.Equal(t, desc.Digest, transformedDesc.Digest)
+
+	_, err = memStore.Resolve(ctx, refWithPlus)
+	require.Error(t, err, "Should NOT find the reference with the original +")
+}