From 875e149d6b677f4a865c9ecd6d804fce0e2ff395 Mon Sep 17 00:00:00 2001
From: Benoit Tigeot <benoit.tigeot@lifen.fr>
Date: Wed, 21 May 2025 21:30:05 +0200
Subject: [PATCH] Prevent failure when resolving version tags in oras memory
 store

- The newReference() function transforms version tags by replacing + with _ for OCI compatibility
- But the code was using the original ref (with +) for TagBytes()
- Then it tries to find the tagged reference using parsedRef.String() (with _)
- This mismatch causes the Resolve method to fail with "not found"
- By using parsedRef.String() consistently in both places, the references will match and the lookup will succeed.

I extracted the TagBytes function to improve testability.
Push() includes several external calls that are hard to mock,
so isolating this logic makes testing more manageable.

Close: #30881
Signed-off-by: Benoit Tigeot <benoit.tigeot@lifen.fr>
---
 pkg/registry/client.go      | 40 ++++++++++++++++++++---------
 pkg/registry/client_test.go | 51 +++++++++++++++++++++++++++++++++++++
 2 files changed, 79 insertions(+), 12 deletions(-)
 create mode 100644 pkg/registry/client_test.go

diff --git a/pkg/registry/client.go b/pkg/registry/client.go
index 2d131dc47..d035609c2 100644
--- a/pkg/registry/client.go
+++ b/pkg/registry/client.go
@@ -685,19 +685,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, ref, configDescriptor,
+		layers, ociAnnotations)
 	if err != nil {
 		return nil, err
 	}
@@ -898,3 +888,29 @@ 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,
+	ref string, configDescriptor ocispec.Descriptor, layers []ocispec.Descriptor,
+	ociAnnotations map[string]string) (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
+	}
+
+	parsedRef, err := newReference(ref)
+	if err != nil {
+		return ocispec.Descriptor{}, err
+	}
+
+	return oras.TagBytes(ctx, memoryStore, ocispec.MediaTypeImageManifest,
+		manifestData, parsedRef.String())
+}
diff --git a/pkg/registry/client_test.go b/pkg/registry/client_test.go
new file mode 100644
index 000000000..bf1ce66da
--- /dev/null
+++ b/pkg/registry/client_test.go
@@ -0,0 +1,51 @@
+/*
+Copyright The Helm Authors.
+
+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 registry
+
+import (
+	"context"
+	"io"
+	"testing"
+
+	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
+	"github.com/stretchr/testify/require"
+	"oras.land/oras-go/v2/content/memory"
+)
+
+// 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}}
+
+	desc, err := client.tagManifest(ctx, memStore, refWithPlus, configDesc, layers, nil)
+	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 +")
+}