From b6c719dfece07b3ae641463bb0ca5d0e3b4bbbb4 Mon Sep 17 00:00:00 2001
From: Benoit Tigeot <benoit.tigeot@lifen.fr>
Date: Wed, 1 Oct 2025 15:07:35 +0200
Subject: [PATCH] Simplify normalized version and use the proper function to
 validate oci

Signed-off-by: Benoit Tigeot <benoit.tigeot@lifen.fr>
---
 pkg/pusher/ocipusher.go      | 11 +++++------
 pkg/pusher/ocipusher_test.go | 29 +++++++++++++++++++++++++++++
 pkg/registry/pushref.go      | 15 ++++-----------
 pkg/registry/pushref_test.go |  9 ++++++++-
 4 files changed, 46 insertions(+), 18 deletions(-)

diff --git a/pkg/pusher/ocipusher.go b/pkg/pusher/ocipusher.go
index 25682969b..1575164e1 100644
--- a/pkg/pusher/ocipusher.go
+++ b/pkg/pusher/ocipusher.go
@@ -22,8 +22,6 @@ import (
 	"net"
 	"net/http"
 	"os"
-	"path"
-	"strings"
 	"time"
 
 	"helm.sh/helm/v4/internal/tlsutil"
@@ -61,6 +59,11 @@ func (pusher *OCIPusher) push(chartRef, href string) error {
 		return err
 	}
 
+	ref, err := registry.BuildPushRef(href, meta.Metadata.Name, meta.Metadata.Version)
+	if err != nil {
+		return err
+	}
+
 	client := pusher.opts.registryClient
 	if client == nil {
 		c, err := pusher.newRegistryClient()
@@ -85,10 +88,6 @@ func (pusher *OCIPusher) push(chartRef, href string) error {
 		pushOpts = append(pushOpts, registry.PushOptProvData(provBytes))
 	}
 
-	ref := fmt.Sprintf("%s:%s",
-		path.Join(strings.TrimPrefix(href, fmt.Sprintf("%s://", registry.OCIScheme)), meta.Metadata.Name),
-		meta.Metadata.Version)
-
 	// The time the chart was "created" is semantically the time the chart archive file was last written(modified)
 	chartArchiveFileCreatedTime := stat.ModTime()
 	pushOpts = append(pushOpts, registry.PushOptCreationTime(chartArchiveFileCreatedTime.Format(time.RFC3339)))
diff --git a/pkg/pusher/ocipusher_test.go b/pkg/pusher/ocipusher_test.go
index 24f52a7ad..5ab8edae1 100644
--- a/pkg/pusher/ocipusher_test.go
+++ b/pkg/pusher/ocipusher_test.go
@@ -393,6 +393,35 @@ func TestOCIPusher_Push_ChartOperations(t *testing.T) {
 	}
 }
 
+func TestOCIPusher_Push_InvalidChartVersion(t *testing.T) {
+	chartPath := "../../pkg/cmd/testdata/testcharts/compressedchart-0.1.0.tgz"
+
+	// Skip test if chart file doesn't exist
+	if _, err := os.Stat(chartPath); err != nil {
+		t.Skipf("Test chart %s not found, skipping test", chartPath)
+	}
+
+	pusher, err := NewOCIPusher()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Test that multiple options are applied correctly
+	err = pusher.Push(chartPath, "oci://localhost:5000/test:0.2.0",
+		WithPlainHTTP(true),
+		WithInsecureSkipTLSVerify(true),
+	)
+
+	// We expect an error since we're not actually pushing to a registry
+	if err == nil {
+		t.Fatal("Expected error when pushing without a valid registry")
+	}
+
+	if !strings.Contains(err.Error(), "does not match provided chart version") {
+		t.Error("Expected error to mention tag mismatch")
+	}
+}
+
 func TestOCIPusher_Push_MultipleOptions(t *testing.T) {
 	chartPath := "../../pkg/cmd/testdata/testcharts/compressedchart-0.1.0.tgz"
 
diff --git a/pkg/registry/pushref.go b/pkg/registry/pushref.go
index bd2b2e537..17f2c5004 100644
--- a/pkg/registry/pushref.go
+++ b/pkg/registry/pushref.go
@@ -34,16 +34,9 @@ func BuildPushRef(href, chartName, chartVersion string) (string, error) {
 
 	// Normalize chart version for tag comparison/build (registry tags cannot contain '+')
 	normalizedVersion := strings.ReplaceAll(chartVersion, "+", "_")
-
-	// Determine final tag:
-	// - if href tag present, it must match normalized chart version
-	// - else use chart version
-	finalTag := normalizedVersion
-	if ref.Tag != "" {
-		if ref.Tag != normalizedVersion {
-			return "", fmt.Errorf("tag %q does not match provided chart version %q", ref.Tag, chartVersion)
-		}
-		finalTag = ref.Tag
+	// if href tag present, it must match normalized chart version
+	if ref.Tag != "" && ref.Tag != normalizedVersion {
+		return "", fmt.Errorf("tag %q does not match provided chart version %q", ref.Tag, chartVersion)
 	}
 
 	// Ensure repository ends with the chart name once (avoid duplication)
@@ -61,5 +54,5 @@ func BuildPushRef(href, chartName, chartVersion string) (string, error) {
 		}
 	}
 
-	return fmt.Sprintf("%s/%s:%s", ref.Registry, finalRepo, finalTag), nil
+	return fmt.Sprintf("%s/%s:%s", ref.Registry, finalRepo, chartVersion), nil
 }
diff --git a/pkg/registry/pushref_test.go b/pkg/registry/pushref_test.go
index cfdcdb387..0bd29ae26 100644
--- a/pkg/registry/pushref_test.go
+++ b/pkg/registry/pushref_test.go
@@ -75,9 +75,16 @@ func TestBuildPushRef(t *testing.T) {
 			registry: "oci://my-registry.io/my-repo:1.0.0_abc",
 			chart:    "my-repo",
 			version:  "1.0.0+abc",
-			want:     "my-registry.io/my-repo:1.0.0_abc",
+			want:     "my-registry.io/my-repo:1.0.0+abc",
 			wantErr:  false,
 		},
+		{
+			name:     "repo already includes chart name",
+			registry: "oci://my-registry.io/namespace/my-repo:1.0.0",
+			chart:    "my-repo",
+			version:  "1.0.0",
+			want:     "my-registry.io/namespace/my-repo:1.0.0",
+		},
 	}
 
 	for _, tt := range tests {