Merge 10c4c253a2 into f05c21b6b6

pkg/cmd/pull_test.go

@@ -506,3 +506,51 @@ 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.Run(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
+	expectedFile := filepath.Join(outdir, "oci-dependent-chart-0.1.0.tgz")
+	if _, err := os.Stat(expectedFile); err != nil {
+		// Try the digest-based filename
+		digestPart := result.PushedChart.Manifest.Digest[7:] // strip "sha256:"
+		expectedFile = filepath.Join(outdir, fmt.Sprintf("oci-dependent-chart@sha256-%s.tgz", digestPart))
+		if _, err := os.Stat(expectedFile); err != nil {
+			t.Errorf("expected chart file not found: %v", err)
+		}
+	}
+}

pkg/downloader/chart_downloader.go

@@ -125,7 +125,8 @@ 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
 		}
@@ -225,7 +226,8 @@ 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)
 	}
@@ -578,3 +580,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

@@ -485,3 +485,41 @@ func TestDownloadToCache(t *testing.T) {
 		c.Keyring = ""
 	})
 }
+
+func TestStripDigestAlgorithm(t *testing.T) {
+	tests := []struct {
+		name     string
+		input    string
+		expected string
+	}{
+		{
+			name:     "sha256 prefixed digest",
+			input:    "sha256:aef46c66a7f2d5a12a7e3f54a64790daf5c9a9e66af3f46955efdaa6c900341d",
+			expected: "aef46c66a7f2d5a12a7e3f54a64790daf5c9a9e66af3f46955efdaa6c900341d",
+		},
+		{
+			name:     "sha512 prefixed digest",
+			input:    "sha512:abcdef1234567890",
+			expected: "abcdef1234567890",
+		},
+		{
+			name:     "plain hex digest without prefix",
+			input:    "aef46c66a7f2d5a12a7e3f54a64790daf5c9a9e66af3f46955efdaa6c900341d",
+			expected: "aef46c66a7f2d5a12a7e3f54a64790daf5c9a9e66af3f46955efdaa6c900341d",
+		},
+		{
+			name:     "empty string",
+			input:    "",
+			expected: "",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := stripDigestAlgorithm(tt.input)
+			if result != tt.expected {
+				t.Errorf("stripDigestAlgorithm(%q) = %q, want %q", tt.input, result, tt.expected)
+			}
+		})
+	}
+}

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
@@ -209,7 +213,7 @@ func NewOCIServer(t *testing.T, dir string) (*OCIServer, error) {
 	}, nil
 }
 
-func (srv *OCIServer) Run(t *testing.T, opts ...OCIServerOpt) {
+func (srv *OCIServer) Run(t *testing.T, opts ...OCIServerOpt) *OCIServerRunResult {
 	t.Helper()
 	cfg := &OCIServerRunConfig{}
 	for _, fn := range opts {
@@ -284,7 +288,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 +314,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.