From 627f4d28a8cb793498ac6a9f93fd9166e4577300 Mon Sep 17 00:00:00 2001 From: Benoit Tigeot Date: Wed, 1 Oct 2025 08:37:18 +0200 Subject: [PATCH 01/10] Add basic registry BuildPushRef to be used in ocipusher We want an easy way to check for chart version provided Signed-off-by: Benoit Tigeot --- pkg/registry/pushref.go | 65 +++++++++++++++++++++ pkg/registry/pushref_test.go | 106 +++++++++++++++++++++++++++++++++++ 2 files changed, 171 insertions(+) create mode 100644 pkg/registry/pushref.go create mode 100644 pkg/registry/pushref_test.go diff --git a/pkg/registry/pushref.go b/pkg/registry/pushref.go new file mode 100644 index 000000000..bd2b2e537 --- /dev/null +++ b/pkg/registry/pushref.go @@ -0,0 +1,65 @@ +/* +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 ( + "fmt" + "path" + "strings" +) + +func BuildPushRef(href, chartName, chartVersion string) (string, error) { + ref, err := newReference(href) + if err != nil { + return "", err + } + + if ref.Digest != "" { + return "", fmt.Errorf("cannot push to a reference with a digest: %q. Only tags are allowed", href) + } + + // 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 + } + + // Ensure repository ends with the chart name once (avoid duplication) + finalRepo := ref.Repository + if chartName != "" { + last := chartName + // Extract last segment of current repository path + if idx := strings.LastIndex(finalRepo, "/"); idx >= 0 { + last = finalRepo[idx+1:] + } else { + last = finalRepo + } + if last != chartName { + finalRepo = path.Join(finalRepo, chartName) + } + } + + return fmt.Sprintf("%s/%s:%s", ref.Registry, finalRepo, finalTag), nil +} diff --git a/pkg/registry/pushref_test.go b/pkg/registry/pushref_test.go new file mode 100644 index 000000000..cfdcdb387 --- /dev/null +++ b/pkg/registry/pushref_test.go @@ -0,0 +1,106 @@ +/* +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 "helm.sh/helm/v4/pkg/registry" + +import ( + "reflect" + "testing" +) + +func TestBuildPushRef(t *testing.T) { + tests := []struct { + name string + registry string + chart string + version string + want string + wantErr bool + }{ + { + name: "simple case", + registry: "oci://my-registry.io/my-repo:1.0.0", + chart: "my-repo", + version: "1.0.0", + want: "my-registry.io/my-repo:1.0.0", + wantErr: false, + }, + { + name: "append chart name to repo", + registry: "oci://my-registry.io/ns", + chart: "my-repo", + version: "1.0.0", + want: "my-registry.io/ns/my-repo:1.0.0", + wantErr: false, + }, + { + name: "digest not allowed", + registry: "oci://my-registry.io/my-repo@sha256:abcdef1234567890", + chart: "my-repo", + version: "1.0.0", + want: "", + wantErr: true, + }, + { + name: "invalid registry", + registry: "invalid-registry", + chart: "my-repo", + version: "1.0.0", + want: "", + wantErr: true, + }, + { + name: "tag mismatch", + registry: "oci://my-registry.io/my-repo:2.0.0", + chart: "my-repo", + version: "1.0.0", + want: "", + wantErr: true, + }, + { + name: "plus to underscore normalization", + 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", + wantErr: false, + }, + } + + for _, tt := range tests { + + if tt.wantErr { + t.Run(tt.name, func(t *testing.T) { + _, err := BuildPushRef(tt.registry, tt.chart, tt.version) + if err == nil { + t.Errorf("BuildPushRef() error = %v, wantErr %v", err, tt.wantErr) + return + } + }) + } else { + t.Run(tt.name, func(t *testing.T) { + got, err := BuildPushRef(tt.registry, tt.chart, tt.version) + if err != nil { + t.Errorf("BuildPushRef() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("BuildPushRef() got = %v, want %v", got, tt.want) + } + }) + } + } +} From 9ba8d4641720ce7959b7debb0d3524ddea6e5a0d Mon Sep 17 00:00:00 2001 From: Benoit Tigeot Date: Wed, 1 Oct 2025 15:07:35 +0200 Subject: [PATCH 02/10] Simplify normalized version and use the proper function to validate oci Signed-off-by: Benoit Tigeot --- 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 2a12e09b4..9e6cc42a6 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, 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 b7d362681..ab2558f95 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 { From 5e8940a54f9c95fddd46215d1548d73cee681592 Mon Sep 17 00:00:00 2001 From: Benoit Tigeot Date: Wed, 1 Oct 2025 15:36:20 +0200 Subject: [PATCH 03/10] Add --version cmd flag to push cmd to validate chart version Signed-off-by: Benoit Tigeot --- pkg/action/push.go | 12 ++++++++++++ pkg/cmd/push.go | 15 +++++++++++++-- pkg/pusher/ocipusher.go | 5 +++++ pkg/pusher/ocipusher_test.go | 27 +++++++++++++++++++++++++++ pkg/pusher/pusher.go | 9 +++++++++ 5 files changed, 66 insertions(+), 2 deletions(-) diff --git a/pkg/action/push.go b/pkg/action/push.go index 0c7148f65..aeedc8df4 100644 --- a/pkg/action/push.go +++ b/pkg/action/push.go @@ -38,6 +38,7 @@ type Push struct { insecureSkipTLSVerify bool plainHTTP bool out io.Writer + expectedVersion string } // PushOpt is a type of function that sets options for a push action. @@ -80,6 +81,13 @@ func WithPushOptWriter(out io.Writer) PushOpt { } } +// WithExpectedVersion configures an expected chart version that must match Chart.yaml. +func WithExpectedVersion(version string) PushOpt { + return func(p *Push) { + p.expectedVersion = version + } +} + // NewPushWithOpts creates a new push, with configuration options. func NewPushWithOpts(opts ...PushOpt) *Push { p := &Push{} @@ -103,6 +111,10 @@ func (p *Push) Run(chartRef string, remote string) (string, error) { }, } + if p.expectedVersion != "" { + c.Options = append(c.Options, pusher.WithExpectedVersion(p.expectedVersion)) + } + if registry.IsOCI(remote) { // Don't use the default registry client if tls options are set. c.Options = append(c.Options, pusher.WithRegistryClient(p.cfg.RegistryClient)) diff --git a/pkg/cmd/push.go b/pkg/cmd/push.go index f32ce92be..21ce2c2bd 100644 --- a/pkg/cmd/push.go +++ b/pkg/cmd/push.go @@ -30,8 +30,16 @@ import ( const pushDesc = ` Upload a chart to a registry. -If the chart has an associated provenance file, -it will also be uploaded. +If the chart has an associated provenance file, it will also be uploaded. + +You can optionally specify --version or oci://...:version as a safety check. When provided, +it must match the version from Chart.yaml; otherwise the command will fail. + +Examples: + + $ helm push mychart-0.1.0.tgz oci://my-registry.io/helm/charts + $ helm push mychart-0.1.0.tgz oci://my-registry.io/helm/charts --version 0.1.0 + $ helm push mychart-0.1.0.tgz oci://my-registry.io/helm/charts:0.1.0 ` type registryPushOptions struct { @@ -42,6 +50,7 @@ type registryPushOptions struct { plainHTTP bool password string username string + version string } func newPushCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { @@ -84,6 +93,7 @@ func newPushCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { action.WithTLSClientConfig(o.certFile, o.keyFile, o.caFile), action.WithInsecureSkipTLSVerify(o.insecureSkipTLSVerify), action.WithPlainHTTP(o.plainHTTP), + action.WithExpectedVersion(o.version), action.WithPushOptWriter(out)) client.Settings = settings output, err := client.Run(chartRef, remote) @@ -103,6 +113,7 @@ func newPushCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { f.BoolVar(&o.plainHTTP, "plain-http", false, "use insecure HTTP connections for the chart upload") f.StringVar(&o.username, "username", "", "chart repository username where to locate the requested chart") f.StringVar(&o.password, "password", "", "chart repository password where to locate the requested chart") + f.StringVar(&o.version, "version", "", "specify the exact chart version to push. If this is not specified, the version from Chart.yaml is used") return cmd } diff --git a/pkg/pusher/ocipusher.go b/pkg/pusher/ocipusher.go index 9e6cc42a6..60492956d 100644 --- a/pkg/pusher/ocipusher.go +++ b/pkg/pusher/ocipusher.go @@ -59,6 +59,11 @@ func (pusher *OCIPusher) push(chartRef, href string) error { return err } + // If an expected version is specified via --version, enforce it matches Chart.yaml + if pusher.opts.expectedVersion != "" && pusher.opts.expectedVersion != meta.Metadata.Version { + return fmt.Errorf("specified --version %q does not match chart version %q", pusher.opts.expectedVersion, meta.Metadata.Version) + } + ref, err := registry.BuildPushRef(href, meta.Metadata.Name, meta.Metadata.Version) if err != nil { return err diff --git a/pkg/pusher/ocipusher_test.go b/pkg/pusher/ocipusher_test.go index ab2558f95..91d374a42 100644 --- a/pkg/pusher/ocipusher_test.go +++ b/pkg/pusher/ocipusher_test.go @@ -455,3 +455,30 @@ func TestOCIPusher_Push_MultipleOptions(t *testing.T) { t.Error("Expected insecureSkipTLSVerify option to be applied") } } + +func TestOCIPusher_Push_ExpectedVersionMismatch(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) + } + + p, err := NewOCIPusher() + if err != nil { + t.Fatal(err) + } + + // Provide an expected version that does not match the chart's version + err = p.Push(chartPath, "oci://localhost:5000/test", + WithExpectedVersion("0.2.0"), + ) + + if err == nil { + t.Fatal("Expected error when --version does not match chart version") + } + + if !strings.Contains(err.Error(), "specified --version") { + t.Errorf("Expected error to mention version mismatch check, got %q", err.Error()) + } +} diff --git a/pkg/pusher/pusher.go b/pkg/pusher/pusher.go index 8ce78b011..f2318cb01 100644 --- a/pkg/pusher/pusher.go +++ b/pkg/pusher/pusher.go @@ -34,6 +34,7 @@ type options struct { caFile string insecureSkipTLSVerify bool plainHTTP bool + expectedVersion string } // Option allows specifying various settings configurable by the user for overriding the defaults @@ -69,6 +70,14 @@ func WithPlainHTTP(plainHTTP bool) Option { } } +// WithExpectedVersion sets a specific chart version to check against the chart metadata. +// If the chart's version differs, the push will fail. +func WithExpectedVersion(version string) Option { + return func(opts *options) { + opts.expectedVersion = version + } +} + // Pusher is an interface to support upload to the specified URL. type Pusher interface { // Push file content by url string From a3fd30cbe8e752cb58303a339457105b8fe4d08d Mon Sep 17 00:00:00 2001 From: Benoit Tigeot Date: Wed, 1 Oct 2025 15:39:06 +0200 Subject: [PATCH 04/10] Linting Signed-off-by: Benoit Tigeot --- pkg/registry/pushref.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/registry/pushref.go b/pkg/registry/pushref.go index 17f2c5004..33c801d1a 100644 --- a/pkg/registry/pushref.go +++ b/pkg/registry/pushref.go @@ -42,8 +42,8 @@ func BuildPushRef(href, chartName, chartVersion string) (string, error) { // Ensure repository ends with the chart name once (avoid duplication) finalRepo := ref.Repository if chartName != "" { - last := chartName // Extract last segment of current repository path + var last string if idx := strings.LastIndex(finalRepo, "/"); idx >= 0 { last = finalRepo[idx+1:] } else { From dc9a97f9ca22cb71791c6fbdc33e6e895f6da698 Mon Sep 17 00:00:00 2001 From: Benoit Tigeot Date: Wed, 1 Oct 2025 16:03:58 +0200 Subject: [PATCH 05/10] Try to make doc less focused on version check It's hard Signed-off-by: Benoit Tigeot --- pkg/cmd/push.go | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/pkg/cmd/push.go b/pkg/cmd/push.go index 21ce2c2bd..eb582c60e 100644 --- a/pkg/cmd/push.go +++ b/pkg/cmd/push.go @@ -32,14 +32,23 @@ Upload a chart to a registry. If the chart has an associated provenance file, it will also be uploaded. -You can optionally specify --version or oci://...:version as a safety check. When provided, -it must match the version from Chart.yaml; otherwise the command will fail. +Remote target formats: +- oci://REGISTRY/REPO +- oci://REGISTRY/REPO/CHART +- oci://REGISTRY/REPO/CHART:VERSION + +When CHART is omitted, the chart name is derived from the package. When VERSION is omitted, +it comes from Chart.yaml. Use --version as an optional verification. If set, it must match +Chart.yaml or the command fails. + +Note: OCI tags do not support "+". Helm replaces "+" with "_" in tags when pushing and restores +"+" when pulling. Examples: $ helm push mychart-0.1.0.tgz oci://my-registry.io/helm/charts $ helm push mychart-0.1.0.tgz oci://my-registry.io/helm/charts --version 0.1.0 - $ helm push mychart-0.1.0.tgz oci://my-registry.io/helm/charts:0.1.0 + $ helm push mychart-0.1.0.tgz oci://my-registry.io/helm/charts/mychart:0.1.0 ` type registryPushOptions struct { @@ -113,7 +122,7 @@ func newPushCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { f.BoolVar(&o.plainHTTP, "plain-http", false, "use insecure HTTP connections for the chart upload") f.StringVar(&o.username, "username", "", "chart repository username where to locate the requested chart") f.StringVar(&o.password, "password", "", "chart repository password where to locate the requested chart") - f.StringVar(&o.version, "version", "", "specify the exact chart version to push. If this is not specified, the version from Chart.yaml is used") + f.StringVar(&o.version, "version", "", "verify the chart version; must match Chart.yaml (optional check)") return cmd } From f4b1d02b50f92578ea2e946887fc28f73a8ba2fa Mon Sep 17 00:00:00 2001 From: Benoit Tigeot Date: Wed, 1 Oct 2025 16:13:15 +0200 Subject: [PATCH 06/10] Avoid misleading user with ref.Tag with version dash as underscore on er Before ``` Error: tag "0.2.0_meta" does not match provided chart version "0.1.0+meta" ``` After ``` Error: tag "0.2.0+meta" does not match provided chart version "0.1.0+meta" ``` Signed-off-by: Benoit Tigeot --- pkg/registry/pushref.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/registry/pushref.go b/pkg/registry/pushref.go index 33c801d1a..eaa85d60b 100644 --- a/pkg/registry/pushref.go +++ b/pkg/registry/pushref.go @@ -36,7 +36,8 @@ func BuildPushRef(href, chartName, chartVersion string) (string, error) { normalizedVersion := strings.ReplaceAll(chartVersion, "+", "_") // 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) + normalizedTag := strings.ReplaceAll(ref.Tag, "_", "+") + return "", fmt.Errorf("tag %q does not match provided chart version %q", normalizedTag, chartVersion) } // Ensure repository ends with the chart name once (avoid duplication) From bdf9c09d6ca067a3e5cc8e895583bb3e54f251ef Mon Sep 17 00:00:00 2001 From: Benoit Tigeot Date: Wed, 1 Oct 2025 16:23:25 +0200 Subject: [PATCH 07/10] Reorganise tests and remove unnecessary config Signed-off-by: Benoit Tigeot --- pkg/pusher/ocipusher_test.go | 36 +++++++++++++++--------------------- 1 file changed, 15 insertions(+), 21 deletions(-) diff --git a/pkg/pusher/ocipusher_test.go b/pkg/pusher/ocipusher_test.go index 91d374a42..2e4ef837a 100644 --- a/pkg/pusher/ocipusher_test.go +++ b/pkg/pusher/ocipusher_test.go @@ -393,7 +393,7 @@ func TestOCIPusher_Push_ChartOperations(t *testing.T) { } } -func TestOCIPusher_Push_InvalidChartVersion(t *testing.T) { +func TestOCIPusher_Push_MultipleOptions(t *testing.T) { chartPath := "../../pkg/cmd/testdata/testcharts/compressedchart-0.1.0.tgz" // Skip test if chart file doesn't exist @@ -407,7 +407,7 @@ func TestOCIPusher_Push_InvalidChartVersion(t *testing.T) { } // Test that multiple options are applied correctly - err = pusher.Push(chartPath, "oci://localhost:5000/test:0.2.0", + err = pusher.Push(chartPath, "oci://localhost:5000/test", WithPlainHTTP(true), WithInsecureSkipTLSVerify(true), ) @@ -417,12 +417,17 @@ func TestOCIPusher_Push_InvalidChartVersion(t *testing.T) { 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") + // Verify options were applied + op := pusher.(*OCIPusher) + if !op.opts.plainHTTP { + t.Error("Expected plainHTTP option to be applied") + } + if !op.opts.insecureSkipTLSVerify { + t.Error("Expected insecureSkipTLSVerify option to be applied") } } -func TestOCIPusher_Push_MultipleOptions(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 @@ -435,24 +440,13 @@ func TestOCIPusher_Push_MultipleOptions(t *testing.T) { t.Fatal(err) } - // Test that multiple options are applied correctly - err = pusher.Push(chartPath, "oci://localhost:5000/test", - WithPlainHTTP(true), - WithInsecureSkipTLSVerify(true), - ) + err = pusher.Push(chartPath, "oci://localhost:5000/test:0.2.0") - // 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") } - - // Verify options were applied - op := pusher.(*OCIPusher) - if !op.opts.plainHTTP { - t.Error("Expected plainHTTP option to be applied") - } - if !op.opts.insecureSkipTLSVerify { - t.Error("Expected insecureSkipTLSVerify option to be applied") + if !strings.Contains(err.Error(), "does not match provided chart version") { + t.Error("Expected error to mention tag mismatch") } } @@ -464,13 +458,13 @@ func TestOCIPusher_Push_ExpectedVersionMismatch(t *testing.T) { t.Skipf("Test chart %s not found, skipping test", chartPath) } - p, err := NewOCIPusher() + pusher, err := NewOCIPusher() if err != nil { t.Fatal(err) } // Provide an expected version that does not match the chart's version - err = p.Push(chartPath, "oci://localhost:5000/test", + err = pusher.Push(chartPath, "oci://localhost:5000/test", WithExpectedVersion("0.2.0"), ) From d084625a0064db8051220355f692aa818b96c01b Mon Sep 17 00:00:00 2001 From: Benoit Tigeot Date: Thu, 2 Oct 2025 21:33:26 +0200 Subject: [PATCH 08/10] rename --version to --ensure-version to better reflect its purpose Good suggestion from Scott today during dev meeting https://docs.google.com/document/d/1d-6xJEx0C78csIYSPKJzRPeWaHG_8W1Hjl72OJggwdc/edit?pli=1&tab=t.0 Signed-off-by: Benoit Tigeot --- pkg/cmd/push.go | 12 ++++++------ pkg/pusher/ocipusher.go | 2 +- pkg/pusher/ocipusher_test.go | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/pkg/cmd/push.go b/pkg/cmd/push.go index eb582c60e..da7c07b64 100644 --- a/pkg/cmd/push.go +++ b/pkg/cmd/push.go @@ -38,8 +38,8 @@ Remote target formats: - oci://REGISTRY/REPO/CHART:VERSION When CHART is omitted, the chart name is derived from the package. When VERSION is omitted, -it comes from Chart.yaml. Use --version as an optional verification. If set, it must match -Chart.yaml or the command fails. +it comes from Chart.yaml. Use --ensure-version as an optional verification. If set, it +must match Chart.yaml or the command fails. Note: OCI tags do not support "+". Helm replaces "+" with "_" in tags when pushing and restores "+" when pulling. @@ -47,7 +47,7 @@ Note: OCI tags do not support "+". Helm replaces "+" with "_" in tags when pushi Examples: $ helm push mychart-0.1.0.tgz oci://my-registry.io/helm/charts - $ helm push mychart-0.1.0.tgz oci://my-registry.io/helm/charts --version 0.1.0 + $ helm push mychart-0.1.0.tgz oci://my-registry.io/helm/charts --ensure-version 0.1.0 $ helm push mychart-0.1.0.tgz oci://my-registry.io/helm/charts/mychart:0.1.0 ` @@ -59,7 +59,7 @@ type registryPushOptions struct { plainHTTP bool password string username string - version string + ensureVersion string } func newPushCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { @@ -102,7 +102,7 @@ func newPushCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { action.WithTLSClientConfig(o.certFile, o.keyFile, o.caFile), action.WithInsecureSkipTLSVerify(o.insecureSkipTLSVerify), action.WithPlainHTTP(o.plainHTTP), - action.WithExpectedVersion(o.version), + action.WithExpectedVersion(o.ensureVersion), action.WithPushOptWriter(out)) client.Settings = settings output, err := client.Run(chartRef, remote) @@ -122,7 +122,7 @@ func newPushCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { f.BoolVar(&o.plainHTTP, "plain-http", false, "use insecure HTTP connections for the chart upload") f.StringVar(&o.username, "username", "", "chart repository username where to locate the requested chart") f.StringVar(&o.password, "password", "", "chart repository password where to locate the requested chart") - f.StringVar(&o.version, "version", "", "verify the chart version; must match Chart.yaml (optional check)") + f.StringVar(&o.ensureVersion, "ensure-version", "", "verify the chart version; must match Chart.yaml (optional check)") return cmd } diff --git a/pkg/pusher/ocipusher.go b/pkg/pusher/ocipusher.go index 60492956d..753657d19 100644 --- a/pkg/pusher/ocipusher.go +++ b/pkg/pusher/ocipusher.go @@ -61,7 +61,7 @@ func (pusher *OCIPusher) push(chartRef, href string) error { // If an expected version is specified via --version, enforce it matches Chart.yaml if pusher.opts.expectedVersion != "" && pusher.opts.expectedVersion != meta.Metadata.Version { - return fmt.Errorf("specified --version %q does not match chart version %q", pusher.opts.expectedVersion, meta.Metadata.Version) + return fmt.Errorf("specified --ensure-version %q does not match chart version %q", pusher.opts.expectedVersion, meta.Metadata.Version) } ref, err := registry.BuildPushRef(href, meta.Metadata.Name, meta.Metadata.Version) diff --git a/pkg/pusher/ocipusher_test.go b/pkg/pusher/ocipusher_test.go index 2e4ef837a..27f6e075c 100644 --- a/pkg/pusher/ocipusher_test.go +++ b/pkg/pusher/ocipusher_test.go @@ -472,7 +472,7 @@ func TestOCIPusher_Push_ExpectedVersionMismatch(t *testing.T) { t.Fatal("Expected error when --version does not match chart version") } - if !strings.Contains(err.Error(), "specified --version") { + if !strings.Contains(err.Error(), "specified --ensure-version") { t.Errorf("Expected error to mention version mismatch check, got %q", err.Error()) } } From eee4fee92ec8fbcda2a1c78c72b6573255cdb6e0 Mon Sep 17 00:00:00 2001 From: Benoit Tigeot Date: Wed, 10 Jun 2026 17:47:48 +0200 Subject: [PATCH 09/10] Reject tagged push refs with mismatched chart name When the user provides an explicit tag (e.g. `oci://reg/repo/other:1.2.3`), the repo basename must match `chartName`. Previously `BuildPushRef` silently appended the chart name, pushing to a different path than intended. Signed-off-by: Benoit Tigeot --- pkg/cmd/push.go | 4 +++- pkg/pusher/ocipusher.go | 2 +- pkg/pusher/ocipusher_test.go | 4 ++-- pkg/registry/pushref.go | 3 +++ pkg/registry/pushref_test.go | 9 ++++++++- 5 files changed, 17 insertions(+), 5 deletions(-) diff --git a/pkg/cmd/push.go b/pkg/cmd/push.go index da7c07b64..31b03ac15 100644 --- a/pkg/cmd/push.go +++ b/pkg/cmd/push.go @@ -34,10 +34,12 @@ If the chart has an associated provenance file, it will also be uploaded. Remote target formats: - oci://REGISTRY/REPO +- oci://REGISTRY/REPO:VERSION - oci://REGISTRY/REPO/CHART - oci://REGISTRY/REPO/CHART:VERSION -When CHART is omitted, the chart name is derived from the package. When VERSION is omitted, +When CHART is omitted, the chart name is derived from the package (and must match the +repo basename when using oci://REGISTRY/REPO:VERSION). When VERSION is omitted, it comes from Chart.yaml. Use --ensure-version as an optional verification. If set, it must match Chart.yaml or the command fails. diff --git a/pkg/pusher/ocipusher.go b/pkg/pusher/ocipusher.go index 753657d19..4490c6d0c 100644 --- a/pkg/pusher/ocipusher.go +++ b/pkg/pusher/ocipusher.go @@ -59,7 +59,7 @@ func (pusher *OCIPusher) push(chartRef, href string) error { return err } - // If an expected version is specified via --version, enforce it matches Chart.yaml + // If an expected version is specified via --ensure-version, enforce it matches Chart.yaml if pusher.opts.expectedVersion != "" && pusher.opts.expectedVersion != meta.Metadata.Version { return fmt.Errorf("specified --ensure-version %q does not match chart version %q", pusher.opts.expectedVersion, meta.Metadata.Version) } diff --git a/pkg/pusher/ocipusher_test.go b/pkg/pusher/ocipusher_test.go index 27f6e075c..18f936f1d 100644 --- a/pkg/pusher/ocipusher_test.go +++ b/pkg/pusher/ocipusher_test.go @@ -443,7 +443,7 @@ func TestOCIPusher_Push_InvalidChartVersion(t *testing.T) { err = pusher.Push(chartPath, "oci://localhost:5000/test:0.2.0") if err == nil { - t.Fatal("Expected error when pushing without a valid registry") + t.Fatal("Expected error due to tag/chart version mismatch") } if !strings.Contains(err.Error(), "does not match provided chart version") { t.Error("Expected error to mention tag mismatch") @@ -469,7 +469,7 @@ func TestOCIPusher_Push_ExpectedVersionMismatch(t *testing.T) { ) if err == nil { - t.Fatal("Expected error when --version does not match chart version") + t.Fatal("Expected error when --ensure-version does not match chart version") } if !strings.Contains(err.Error(), "specified --ensure-version") { diff --git a/pkg/registry/pushref.go b/pkg/registry/pushref.go index eaa85d60b..86689d111 100644 --- a/pkg/registry/pushref.go +++ b/pkg/registry/pushref.go @@ -51,6 +51,9 @@ func BuildPushRef(href, chartName, chartVersion string) (string, error) { last = finalRepo } if last != chartName { + if ref.Tag != "" { + return "", fmt.Errorf("repository %q does not match chart name %q; the tagged reference %q indicates an explicit push target", finalRepo, chartName, href) + } finalRepo = path.Join(finalRepo, chartName) } } diff --git a/pkg/registry/pushref_test.go b/pkg/registry/pushref_test.go index 0bd29ae26..5408baf3b 100644 --- a/pkg/registry/pushref_test.go +++ b/pkg/registry/pushref_test.go @@ -85,10 +85,17 @@ func TestBuildPushRef(t *testing.T) { version: "1.0.0", want: "my-registry.io/namespace/my-repo:1.0.0", }, + { + name: "tagged href with mismatched chart name should error", + registry: "oci://my-registry.io/repo/other:1.2.3", + chart: "mychart", + version: "1.2.3", + want: "", + wantErr: true, + }, } for _, tt := range tests { - if tt.wantErr { t.Run(tt.name, func(t *testing.T) { _, err := BuildPushRef(tt.registry, tt.chart, tt.version) From dce5ae1707f39eb87af2c420735ba57945fb39ec Mon Sep 17 00:00:00 2001 From: Benoit Tigeot Date: Tue, 23 Jun 2026 17:14:04 +0200 Subject: [PATCH 10/10] Clarify normalizedVersion is for comparison only Prevent future refactors from switching the returned tag to the normalized form; the registry client handles '+' -> '_'. Signed-off-by: Benoit Tigeot --- pkg/registry/pushref.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/registry/pushref.go b/pkg/registry/pushref.go index 86689d111..a87d12551 100644 --- a/pkg/registry/pushref.go +++ b/pkg/registry/pushref.go @@ -32,7 +32,7 @@ func BuildPushRef(href, chartName, chartVersion string) (string, error) { return "", fmt.Errorf("cannot push to a reference with a digest: %q. Only tags are allowed", href) } - // Normalize chart version for tag comparison/build (registry tags cannot contain '+') + // Normalize for comparison only; the registry client converts '+' to '_' for the actual tag normalizedVersion := strings.ReplaceAll(chartVersion, "+", "_") // if href tag present, it must match normalized chart version if ref.Tag != "" && ref.Tag != normalizedVersion {