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..31b03ac15 100644 --- a/pkg/cmd/push.go +++ b/pkg/cmd/push.go @@ -30,8 +30,27 @@ 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. + +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 (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. + +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 --ensure-version 0.1.0 + $ helm push mychart-0.1.0.tgz oci://my-registry.io/helm/charts/mychart:0.1.0 ` type registryPushOptions struct { @@ -42,6 +61,7 @@ type registryPushOptions struct { plainHTTP bool password string username string + ensureVersion string } func newPushCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { @@ -84,6 +104,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.ensureVersion), action.WithPushOptWriter(out)) client.Settings = settings output, err := client.Run(chartRef, remote) @@ -103,6 +124,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.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 2a12e09b4..4490c6d0c 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,16 @@ func (pusher *OCIPusher) push(chartRef, href string) error { return err } + // 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) + } + + 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 +93,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..18f936f1d 100644 --- a/pkg/pusher/ocipusher_test.go +++ b/pkg/pusher/ocipusher_test.go @@ -426,3 +426,53 @@ func TestOCIPusher_Push_MultipleOptions(t *testing.T) { t.Error("Expected insecureSkipTLSVerify option to be applied") } } + +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) + } + + err = pusher.Push(chartPath, "oci://localhost:5000/test:0.2.0") + + if err == nil { + 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") + } +} + +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) + } + + pusher, err := NewOCIPusher() + if err != nil { + t.Fatal(err) + } + + // Provide an expected version that does not match the chart's version + err = pusher.Push(chartPath, "oci://localhost:5000/test", + WithExpectedVersion("0.2.0"), + ) + + if err == nil { + t.Fatal("Expected error when --ensure-version does not match chart version") + } + + if !strings.Contains(err.Error(), "specified --ensure-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 diff --git a/pkg/registry/pushref.go b/pkg/registry/pushref.go new file mode 100644 index 000000000..a87d12551 --- /dev/null +++ b/pkg/registry/pushref.go @@ -0,0 +1,62 @@ +/* +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 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 { + 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) + finalRepo := ref.Repository + if chartName != "" { + // Extract last segment of current repository path + var last string + if idx := strings.LastIndex(finalRepo, "/"); idx >= 0 { + last = finalRepo[idx+1:] + } else { + 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) + } + } + + 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 new file mode 100644 index 000000000..5408baf3b --- /dev/null +++ b/pkg/registry/pushref_test.go @@ -0,0 +1,120 @@ +/* +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, + }, + { + 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", + }, + { + 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) + 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) + } + }) + } + } +}