Merge branch 'helm:main' into feature/enhance-dry-run-for-helm-4

pkg/chart/common/util/jsonschema.go

@@ -83,7 +83,7 @@ func ValidateAgainstSchema(ch chart.Charter, values map[string]interface{}) erro
 		slog.Debug("chart name", "chart-name", chrt.Name())
 		err := ValidateAgainstSingleSchema(values, chrt.Schema())
 		if err != nil {
-			sb.WriteString(fmt.Sprintf("%s:\n", chrt.Name()))
+			fmt.Fprintf(&sb, "%s:\n", chrt.Name())
 			sb.WriteString(err.Error())
 		}
 	}
@@ -103,10 +103,8 @@ func ValidateAgainstSchema(ch chart.Charter, values map[string]interface{}) erro
 
 		subchartValues, ok := raw.(map[string]any)
 		if !ok {
-			sb.WriteString(fmt.Sprintf(
-				"%s:\ninvalid type for values: expected object (map), got %T\n",
-				sub.Name(), raw,
-			))
+			fmt.Fprintf(&sb, "%s:\ninvalid type for values: expected object (map), got %T\n",
+				sub.Name(), raw)
 			continue
 		}
 

pkg/cmd/pull_test.go

@@ -22,6 +22,7 @@ import (
 	"net/http/httptest"
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 
 	"helm.sh/helm/v4/pkg/repo/v1/repotest"
@@ -506,3 +507,54 @@ func TestPullFileCompletion(t *testing.T) {
 	checkFileCompletion(t, "pull", false)
 	checkFileCompletion(t, "pull repo/chart", false)
 }
+
+// TestPullOCIWithTagAndDigest tests pulling an OCI chart with both tag and digest specified.
+// This is a regression test for https://github.com/helm/helm/issues/31600
+func TestPullOCIWithTagAndDigest(t *testing.T) {
+	srv := repotest.NewTempServer(
+		t,
+		repotest.WithChartSourceGlob("testdata/testcharts/*.tgz*"),
+	)
+	defer srv.Stop()
+
+	ociSrv, err := repotest.NewOCIServer(t, srv.Root())
+	if err != nil {
+		t.Fatal(err)
+	}
+	result := ociSrv.RunWithReturn(t)
+
+	contentCache := t.TempDir()
+	outdir := t.TempDir()
+
+	// Test: pull with tag and digest (the fixed bug from issue #31600)
+	// Previously this failed with "encoding/hex: invalid byte: U+0073 's'"
+	ref := fmt.Sprintf("oci://%s/u/ocitestuser/oci-dependent-chart:0.1.0@%s",
+		ociSrv.RegistryURL, result.PushedChart.Manifest.Digest)
+
+	cmd := fmt.Sprintf("pull %s -d '%s' --registry-config %s --content-cache %s --plain-http",
+		ref,
+		outdir,
+		filepath.Join(srv.Root(), "config.json"),
+		contentCache,
+	)
+
+	_, _, err = executeActionCommand(cmd)
+	if err != nil {
+		t.Fatalf("pull with tag+digest failed: %v", err)
+	}
+
+	// Verify the file was downloaded
+	// When digest is present, the filename uses the digest format (e.g. chart@sha256-hex.tgz)
+	expectedFile := filepath.Join(outdir, "oci-dependent-chart-0.1.0.tgz")
+	if _, err := os.Stat(expectedFile); err != nil {
+		// Try the digest-based filename; parse algorithm:hex to avoid fixed-offset assumptions
+		algorithm, digestPart, ok := strings.Cut(result.PushedChart.Manifest.Digest, ":")
+		if !ok {
+			t.Fatalf("digest must be in algorithm:hex format, got %q", result.PushedChart.Manifest.Digest)
+		}
+		expectedFile = filepath.Join(outdir, fmt.Sprintf("oci-dependent-chart@%s-%s.tgz", algorithm, digestPart))
+		if _, err := os.Stat(expectedFile); err != nil {
+			t.Errorf("expected chart file not found: %v", err)
+		}
+	}
+}

pkg/downloader/chart_downloader.go

@@ -125,10 +125,15 @@ func (c *ChartDownloader) DownloadTo(ref, version, dest string) (string, *proven
 	var digest32 [32]byte
 	if hash != "" {
 		// if there is a hash, populate the other formats
-		digest, err = hex.DecodeString(hash)
+		// Strip the algorithm prefix (e.g., "sha256:") if present
+		digest, err = hex.DecodeString(stripDigestAlgorithm(hash))
 		if err != nil {
 			return "", nil, err
 		}
+		if len(digest) != 32 {
+			return "", nil, fmt.Errorf("invalid digest length: %d", len(digest))
+		}
+
 		copy(digest32[:], digest)
 		if pth, err := c.Cache.Get(digest32, CacheChart); err == nil {
 			fdata, err := os.ReadFile(pth)
@@ -180,7 +185,7 @@ func (c *ChartDownloader) DownloadTo(ref, version, dest string) (string, *proven
 			}
 		}
 		if !found {
-			body, err = g.Get(u.String() + ".prov")
+			body, err = g.Get(u.String()+".prov", c.Options...)
 			if err != nil {
 				if c.Verify == VerifyAlways {
 					return destfile, ver, fmt.Errorf("failed to fetch provenance %q", u.String()+".prov")
@@ -231,10 +236,14 @@ func (c *ChartDownloader) DownloadToCache(ref, version string) (string, *provena
 	c.Options = append(c.Options, getter.WithAcceptHeader("application/gzip,application/octet-stream"))
 
 	// Check the cache for the file
-	digest, err := hex.DecodeString(digestString)
+	// Strip the algorithm prefix (e.g., "sha256:") if present
+	digest, err := hex.DecodeString(stripDigestAlgorithm(digestString))
 	if err != nil {
 		return "", nil, fmt.Errorf("unable to decode digest: %w", err)
 	}
+	if digestString != "" && len(digest) != 32 {
+		return "", nil, fmt.Errorf("invalid digest length: %d", len(digest))
+	}
 	var digest32 [32]byte
 	copy(digest32[:], digest)
 
@@ -584,3 +593,12 @@ func loadRepoConfig(file string) (*repo.File, error) {
 	}
 	return r, nil
 }
+
+// stripDigestAlgorithm removes the algorithm prefix (e.g., "sha256:") from a digest string.
+// If no prefix is present, the original string is returned unchanged.
+func stripDigestAlgorithm(digest string) string {
+	if idx := strings.Index(digest, ":"); idx >= 0 {
+		return digest[idx+1:]
+	}
+	return digest
+}

pkg/downloader/chart_downloader_test.go

@@ -23,6 +23,7 @@ import (
 	"path/filepath"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
 	"helm.sh/helm/v4/internal/test/ensure"
@@ -486,3 +487,34 @@ func TestDownloadToCache(t *testing.T) {
 		c.Keyring = ""
 	})
 }
+
+func TestStripDigestAlgorithm(t *testing.T) {
+	tests := map[string]struct {
+		input    string
+		expected string
+	}{
+		"sha256 prefixed digest": {
+			input:    "sha256:aef46c66a7f2d5a12a7e3f54a64790daf5c9a9e66af3f46955efdaa6c900341d",
+			expected: "aef46c66a7f2d5a12a7e3f54a64790daf5c9a9e66af3f46955efdaa6c900341d",
+		},
+		"sha512 prefixed digest": {
+			input:    "sha512:abcdef1234567890",
+			expected: "abcdef1234567890",
+		},
+		"plain hex digest without prefix": {
+			input:    "aef46c66a7f2d5a12a7e3f54a64790daf5c9a9e66af3f46955efdaa6c900341d",
+			expected: "aef46c66a7f2d5a12a7e3f54a64790daf5c9a9e66af3f46955efdaa6c900341d",
+		},
+		"empty string": {
+			input:    "",
+			expected: "",
+		},
+	}
+
+	for name, tt := range tests {
+		t.Run(name, func(t *testing.T) {
+			result := stripDigestAlgorithm(tt.input)
+			assert.Equalf(t, tt.expected, result, "stripDigestAlgorithm(%q) = %q, want %q", tt.input, result, tt.expected)
+		})
+	}
+}

pkg/getter/getter_test.go

@@ -60,7 +60,8 @@ func TestProvidersWithTimeout(t *testing.T) {
 	if err != nil {
 		t.Error(err)
 	}
-	client, err := getter.(*HTTPGetter).httpClient()
+	httpGetter := getter.(*HTTPGetter)
+	client, err := httpGetter.httpClient(httpGetter.opts)
 	if err != nil {
 		t.Error(err)
 	}

pkg/getter/httpgetter.go

@@ -37,13 +37,15 @@ type HTTPGetter struct {
 
 // Get performs a Get from repo.Getter and returns the body.
 func (g *HTTPGetter) Get(href string, options ...Option) (*bytes.Buffer, error) {
+	// Create a local copy of options to avoid data races when Get is called concurrently
+	opts := g.opts
 	for _, opt := range options {
-		opt(&g.opts)
+		opt(&opts)
 	}
-	return g.get(href)
+	return g.get(href, opts)
 }
 
-func (g *HTTPGetter) get(href string) (*bytes.Buffer, error) {
+func (g *HTTPGetter) get(href string, opts getterOptions) (*bytes.Buffer, error) {
 	// Set a helm specific user agent so that a repo server and metrics can
 	// separate helm calls from other tools interacting with repos.
 	req, err := http.NewRequest(http.MethodGet, href, nil)
@@ -51,18 +53,18 @@ func (g *HTTPGetter) get(href string) (*bytes.Buffer, error) {
 		return nil, err
 	}
 
-	if g.opts.acceptHeader != "" {
-		req.Header.Set("Accept", g.opts.acceptHeader)
+	if opts.acceptHeader != "" {
+		req.Header.Set("Accept", opts.acceptHeader)
 	}
 
 	req.Header.Set("User-Agent", version.GetUserAgent())
-	if g.opts.userAgent != "" {
-		req.Header.Set("User-Agent", g.opts.userAgent)
+	if opts.userAgent != "" {
+		req.Header.Set("User-Agent", opts.userAgent)
 	}
 
 	// Before setting the basic auth credentials, make sure the URL associated
 	// with the basic auth is the one being fetched.
-	u1, err := url.Parse(g.opts.url)
+	u1, err := url.Parse(opts.url)
 	if err != nil {
 		return nil, fmt.Errorf("unable to parse getter URL: %w", err)
 	}
@@ -74,13 +76,13 @@ func (g *HTTPGetter) get(href string) (*bytes.Buffer, error) {
 	// Host on URL (returned from url.Parse) contains the port if present.
 	// This check ensures credentials are not passed between different
 	// services on different ports.
-	if g.opts.passCredentialsAll || (u1.Scheme == u2.Scheme && u1.Host == u2.Host) {
-		if g.opts.username != "" && g.opts.password != "" {
-			req.SetBasicAuth(g.opts.username, g.opts.password)
+	if opts.passCredentialsAll || (u1.Scheme == u2.Scheme && u1.Host == u2.Host) {
+		if opts.username != "" && opts.password != "" {
+			req.SetBasicAuth(opts.username, opts.password)
 		}
 	}
 
-	client, err := g.httpClient()
+	client, err := g.httpClient(opts)
 	if err != nil {
 		return nil, err
 	}
@@ -110,51 +112,52 @@ func NewHTTPGetter(options ...Option) (Getter, error) {
 	return &client, nil
 }
 
-func (g *HTTPGetter) httpClient() (*http.Client, error) {
-	if g.opts.transport != nil {
+func (g *HTTPGetter) httpClient(opts getterOptions) (*http.Client, error) {
+	if opts.transport != nil {
 		return &http.Client{
-			Transport: g.opts.transport,
-			Timeout:   g.opts.timeout,
+			Transport: opts.transport,
+			Timeout:   opts.timeout,
 		}, nil
 	}
 
-	g.once.Do(func() {
-		g.transport = &http.Transport{
+	// Check if we need custom TLS configuration
+	needsCustomTLS := (opts.certFile != "" && opts.keyFile != "") || opts.caFile != "" || opts.insecureSkipVerifyTLS
+
+	if needsCustomTLS {
+		// Create a new transport for custom TLS to avoid race conditions
+		transport := &http.Transport{
 			DisableCompression: true,
 			Proxy:              http.ProxyFromEnvironment,
-			// Being nil would cause the tls.Config default to be used
-			// "NewTLSConfig" modifies an empty TLS config, not the default one
-			TLSClientConfig: &tls.Config{},
 		}
-	})
 
-	if (g.opts.certFile != "" && g.opts.keyFile != "") || g.opts.caFile != "" || g.opts.insecureSkipVerifyTLS {
 		tlsConf, err := tlsutil.NewTLSConfig(
-			tlsutil.WithInsecureSkipVerify(g.opts.insecureSkipVerifyTLS),
-			tlsutil.WithCertKeyPairFiles(g.opts.certFile, g.opts.keyFile),
-			tlsutil.WithCAFile(g.opts.caFile),
+			tlsutil.WithInsecureSkipVerify(opts.insecureSkipVerifyTLS),
+			tlsutil.WithCertKeyPairFiles(opts.certFile, opts.keyFile),
+			tlsutil.WithCAFile(opts.caFile),
 		)
 		if err != nil {
 			return nil, fmt.Errorf("can't create TLS config for client: %w", err)
 		}
 
-		g.transport.TLSClientConfig = tlsConf
+		transport.TLSClientConfig = tlsConf
+
+		return &http.Client{
+			Transport: transport,
+			Timeout:   opts.timeout,
+		}, nil
 	}
 
-	if g.opts.insecureSkipVerifyTLS {
-		if g.transport.TLSClientConfig == nil {
-			g.transport.TLSClientConfig = &tls.Config{
-				InsecureSkipVerify: true,
-			}
-		} else {
-			g.transport.TLSClientConfig.InsecureSkipVerify = true
+	// Use shared transport for default case (no custom TLS)
+	g.once.Do(func() {
+		g.transport = &http.Transport{
+			DisableCompression: true,
+			Proxy:              http.ProxyFromEnvironment,
+			TLSClientConfig:    &tls.Config{},
 		}
-	}
+	})
 
-	client := &http.Client{
+	return &http.Client{
 		Transport: g.transport,
-		Timeout:   g.opts.timeout,
-	}
-
-	return client, nil
+		Timeout:   opts.timeout,
+	}, nil
 }

pkg/getter/httpgetter_test.go

@@ -577,7 +577,7 @@ func TestHttpClientInsecureSkipVerify(t *testing.T) {
 
 func verifyInsecureSkipVerify(t *testing.T, g *HTTPGetter, caseName string, expectedValue bool) *http.Transport {
 	t.Helper()
-	returnVal, err := g.httpClient()
+	returnVal, err := g.httpClient(g.opts)
 
 	if err != nil {
 		t.Fatal(err)
@@ -601,7 +601,7 @@ func verifyInsecureSkipVerify(t *testing.T, g *HTTPGetter, caseName string, expe
 func TestDefaultHTTPTransportReuse(t *testing.T) {
 	g := HTTPGetter{}
 
-	httpClient1, err := g.httpClient()
+	httpClient1, err := g.httpClient(g.opts)
 
 	if err != nil {
 		t.Fatal(err)
@@ -613,7 +613,7 @@ func TestDefaultHTTPTransportReuse(t *testing.T) {
 
 	transport1 := (httpClient1.Transport).(*http.Transport) //nolint:staticcheck
 
-	httpClient2, err := g.httpClient()
+	httpClient2, err := g.httpClient(g.opts)
 
 	if err != nil {
 		t.Fatal(err)
@@ -635,7 +635,7 @@ func TestHTTPTransportOption(t *testing.T) {
 
 	g := HTTPGetter{}
 	g.opts.transport = transport
-	httpClient1, err := g.httpClient()
+	httpClient1, err := g.httpClient(g.opts)
 
 	if err != nil {
 		t.Fatal(err)
@@ -651,7 +651,7 @@ func TestHTTPTransportOption(t *testing.T) {
 		t.Fatalf("Expected transport option to be applied")
 	}
 
-	httpClient2, err := g.httpClient()
+	httpClient2, err := g.httpClient(g.opts)
 
 	if err != nil {
 		t.Fatal(err)

pkg/repo/v1/repotest/server.go

@@ -153,6 +153,10 @@ type OCIServerRunConfig struct {
 
 type OCIServerOpt func(config *OCIServerRunConfig)
 
+type OCIServerRunResult struct {
+	PushedChart *ociRegistry.PushResult
+}
+
 func WithDependingChart(c *chart.Chart) OCIServerOpt {
 	return func(config *OCIServerRunConfig) {
 		config.DependingChart = c
@@ -210,6 +214,11 @@ func NewOCIServer(t *testing.T, dir string) (*OCIServer, error) {
 }
 
 func (srv *OCIServer) Run(t *testing.T, opts ...OCIServerOpt) {
+	t.Helper()
+	_ = srv.RunWithReturn(t, opts...)
+}
+
+func (srv *OCIServer) RunWithReturn(t *testing.T, opts ...OCIServerOpt) *OCIServerRunResult {
 	t.Helper()
 	cfg := &OCIServerRunConfig{}
 	for _, fn := range opts {
@@ -284,7 +293,9 @@ func (srv *OCIServer) Run(t *testing.T, opts ...OCIServerOpt) {
 	srv.Client = registryClient
 	c := cfg.DependingChart
 	if c == nil {
-		return
+		return &OCIServerRunResult{
+			PushedChart: result,
+		}
 	}
 
 	dependingRef := fmt.Sprintf("%s/u/ocitestuser/%s:%s",
@@ -308,6 +319,10 @@ func (srv *OCIServer) Run(t *testing.T, opts ...OCIServerOpt) {
 		result.Manifest.Digest, result.Manifest.Size,
 		result.Config.Digest, result.Config.Size,
 		result.Chart.Digest, result.Chart.Size)
+
+	return &OCIServerRunResult{
+		PushedChart: result,
+	}
 }
 
 // Root gets the docroot for the server.