OCPBUGS-88461: fix(helm): tolerate v-prefix version mismatch in disk-to-mirror

internal/pkg/helm/local_stored_collector.go

@@ -11,6 +11,7 @@ import (
 	"path/filepath"
 	"strings"
 
+	"github.com/Masterminds/semver/v3"
 	helmchart "helm.sh/helm/v3/pkg/chart"
 	"helm.sh/helm/v3/pkg/chart/loader"
 	"helm.sh/helm/v3/pkg/chartutil"
@@ -168,8 +169,12 @@ func (o *LocalStorageCollector) HelmImageCollector(ctx context.Context) ([]v2alp
 			}
 
 			for _, chart := range charts {
-				src := filepath.Join(lsc.Opts.Global.WorkingDir, helmDir, helmChartDir)
-				path := filepath.Join(src, fmt.Sprintf("%s-%s.tgz", chart.Name, chart.Version))
+				chartDir := filepath.Join(lsc.Opts.Global.WorkingDir, helmDir, helmChartDir)
+				path, err := resolveChartPath(chartDir, chart.Name, chart.Version)
+				if err != nil {
+					errs = append(errs, err)
+					continue
+				}
 
 				imgs, err := getImages(path, chart.ImagePaths...)
 				if err != nil {
@@ -349,6 +354,74 @@ func getChartsFromIndex(indexURL string, indexFile helmrepo.IndexFile) ([]v2alph
 	return charts, nil
 }
 
+// resolveChartPath returns the filesystem path to a Helm chart tarball stored
+// under dir.  It normalises the version with semver (which strips any leading
+// "v") and then probes two candidate filenames — "{name}-{canonical}.tgz" and
+// "{name}-v{canonical}.tgz" — because Helm repositories differ on whether they
+// embed the "v" prefix in tarball URLs.  A descriptive error naming both
+// attempted paths is returned when neither file exists.
+func resolveChartPath(dir, name, version string) (string, error) {
+	ver, err := semver.NewVersion(version)
+	if err != nil {
+		return "", fmt.Errorf("invalid chart version %q: %w", version, err)
+	}
+	canonical := ver.String() // always without "v" prefix, regardless of how version was specified
+
+	baseDir := filepath.Clean(dir)
+	candidateVersions := []string{canonical, "v" + canonical}
+	var candidatePaths []string
+
+	for _, candidateVer := range candidateVersions {
+		path, err := buildChartCandidatePath(baseDir, name, candidateVer)
+		if err != nil {
+			return "", err
+		}
+		candidatePaths = append(candidatePaths, path)
+
+		found, err := chartPathExists(path)
+		if err != nil {
+			return "", err
+		}
+		if found {
+			if candidateVer != candidateVersions[0] {
+				lsc.Log.Debug("chart %s: %s not found, using %s (v-prefix variant)", name, filepath.Base(candidatePaths[0]), filepath.Base(path))
+			}
+			return path, nil
+		}
+	}
+
+	return "", fmt.Errorf("chart file not found for %s version %s: tried %s and %s",
+		name, version, filepath.Base(candidatePaths[0]), filepath.Base(candidatePaths[1]))
+}
+
+// chartPathExists reports whether the given path exists on disk.
+// It returns false (without error) for missing files and propagates
+// real I/O or permission errors so they are not silently swallowed.
+func chartPathExists(path string) (bool, error) {
+	_, err := os.Stat(path)
+	if err == nil {
+		return true, nil
+	}
+	if errors.Is(err, os.ErrNotExist) {
+		return false, nil
+	}
+	return false, fmt.Errorf("stat %s: %w", path, err)
+}
+
+// buildChartCandidatePath constructs the expected tarball path for a chart
+// version and validates it stays within baseDir to prevent path traversal.
+func buildChartCandidatePath(baseDir, name, ver string) (string, error) {
+	p := filepath.Join(baseDir, fmt.Sprintf("%s-%s.tgz", name, ver))
+	rel, err := filepath.Rel(baseDir, p)
+	if err != nil {
+		return "", fmt.Errorf("resolve chart path: %w", err)
+	}
+	if rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) {
+		return "", fmt.Errorf("invalid chart reference %q:%q escapes chart directory", name, ver)
+	}
+	return p, nil
+}
+
 func getImages(path string, imagePaths ...string) (images []v2alpha1.RelatedImage, err error) {
 	lsc.Log.Debug("Reading from path %s", path)
 

internal/pkg/helm/local_stored_collector_test.go

@@ -9,6 +9,7 @@ import (
 	"os"
 	"path"
 	"path/filepath"
+	"strings"
 	"testing"
 
 	"github.com/otiai10/copy"
@@ -715,6 +716,168 @@ func TestHelmImageCollector(t *testing.T) {
 	}
 }
 
+// TestResolveChartPath verifies that resolveChartPath tolerates mismatches
+// between the version string in the ImageSetConfiguration and the "v" prefix
+// that a Helm repository may embed in its tarball filenames.
+func TestResolveChartPath(t *testing.T) {
+	dir := t.TempDir()
+
+	// chartA-1.0.0.tgz  – version stored without "v"
+	// chartB-v2.0.0.tgz – version stored with "v"
+	chartAFile := filepath.Join(dir, "chartA-1.0.0.tgz")
+	chartBFile := filepath.Join(dir, "chartB-v2.0.0.tgz")
+	assert.NoError(t, os.WriteFile(chartAFile, []byte("placeholder"), 0600))
+	assert.NoError(t, os.WriteFile(chartBFile, []byte("placeholder"), 0600))
+
+	// resolveChartPath calls lsc.Log.Debug; initialise the global so the
+	// helper does not panic.
+	lsc = &LocalStorageCollector{Log: clog.New("trace")}
+
+	tests := []struct {
+		name      string
+		chartName string
+		version   string
+		wantPath  string
+		wantErr   bool
+	}{
+		{
+			name:      "exact match – no v prefix",
+			chartName: "chartA",
+			version:   "1.0.0",
+			wantPath:  chartAFile,
+		},
+		{
+			name:      "config omits v; file on disk has v prefix",
+			chartName: "chartB",
+			version:   "2.0.0",
+			wantPath:  chartBFile,
+		},
+		{
+			name:      "config has v; file on disk has no v prefix",
+			chartName: "chartA",
+			version:   "v1.0.0",
+			wantPath:  chartAFile,
+		},
+		{
+			name:      "neither candidate exists returns error",
+			chartName: "missing",
+			version:   "9.9.9",
+			wantErr:   true,
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			got, err := resolveChartPath(dir, tc.chartName, tc.version)
+			if tc.wantErr {
+				assert.Error(t, err)
+				// Both attempted candidate paths must appear in the error so
+				// callers can diagnose which filenames were probed.
+				assert.Contains(t, err.Error(), fmt.Sprintf("%s-%s.tgz", tc.chartName, tc.version))
+				altVersion := "v" + tc.version
+				if strings.HasPrefix(tc.version, "v") {
+					altVersion = strings.TrimPrefix(tc.version, "v")
+				}
+				assert.Contains(t, err.Error(), fmt.Sprintf("%s-%s.tgz", tc.chartName, altVersion))
+				return
+			}
+			assert.NoError(t, err)
+			assert.Equal(t, tc.wantPath, got)
+		})
+	}
+}
+
+// TestHelmImageCollectorVPrefixDiskToMirror ensures that the disk-to-mirror
+// collector succeeds even when the chart tarball on disk was saved by the Helm
+// downloader using a "v"-prefixed version (e.g. podinfo-v5.0.0.tgz) while the
+// ImageSetConfiguration specifies the version without the prefix (5.0.0), or
+// vice-versa.
+func TestHelmImageCollectorVPrefixDiskToMirror(t *testing.T) {
+	log := clog.New("trace")
+	ctx := context.Background()
+
+	tests := []struct {
+		name           string
+		configVersion  string
+		diskVersion    string
+		expectedImages []v2alpha1.CopyImageSchema
+	}{
+		{
+			name:          "config without v; disk file has v prefix",
+			configVersion: "5.0.0",
+			diskVersion:   "v5.0.0",
+			expectedImages: []v2alpha1.CopyImageSchema{
+				{
+					Source:      consts.DockerProtocol + testLocalStorageFQDN + "/stefanprodan/podinfo:5.0.0",
+					Destination: testDest + "/stefanprodan/podinfo:5.0.0",
+					Origin:      "ghcr.io/stefanprodan/podinfo:5.0.0",
+					Type:        v2alpha1.TypeHelmImage,
+				},
+			},
+		},
+		{
+			name:          "config with v; disk file has no v prefix",
+			configVersion: "v5.0.0",
+			diskVersion:   "5.0.0",
+			expectedImages: []v2alpha1.CopyImageSchema{
+				{
+					Source:      consts.DockerProtocol + testLocalStorageFQDN + "/stefanprodan/podinfo:5.0.0",
+					Destination: testDest + "/stefanprodan/podinfo:5.0.0",
+					Origin:      "ghcr.io/stefanprodan/podinfo:5.0.0",
+					Type:        v2alpha1.TypeHelmImage,
+				},
+			},
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			workingDir, err := prepareFolder(t.TempDir())
+			assert.NoError(t, err)
+
+			helmCfg := v2alpha1.Helm{
+				Repositories: []v2alpha1.Repository{
+					{
+						Name: "podinfo",
+						URL:  "https://stefanprodan.github.io/podinfo",
+						Charts: []v2alpha1.Chart{
+							{Name: "podinfo", Version: tc.configVersion},
+						},
+					},
+				},
+			}
+
+			testCfg := v2alpha1.ImageSetConfiguration{
+				ImageSetConfigurationSpec: v2alpha1.ImageSetConfigurationSpec{
+					Mirror: v2alpha1.Mirror{Helm: helmCfg},
+				},
+			}
+
+			_, srcOpts := mirror.ImageSrcFlags(nil, nil, nil, "src-", "screds")
+			opts := mirror.CopyOptions{
+				Mode:             mirror.DiskToMirror,
+				Global:           &mirror.GlobalOptions{WorkingDir: workingDir},
+				LocalStorageFQDN: testLocalStorageFQDN,
+				Destination:      testDest,
+				SrcImage:         srcOpts,
+			}
+
+			New(log, testCfg, opts, nil, nil, nil)
+
+			// Place the chart on disk using the downloader-supplied version
+			// string (which may differ from what the config specifies).
+			diskFile := filepath.Join(tempChartDir, fmt.Sprintf("podinfo-%s.tgz", tc.diskVersion))
+			assert.NoError(t, copy.Copy(filepath.Join(testChartsDataPath, "podinfo-5.0.0.tgz"), diskFile))
+
+			helmCollector := lsc
+			imgs, err := helmCollector.HelmImageCollector(ctx)
+
+			assert.NoError(t, err)
+			assert.ElementsMatch(t, tc.expectedImages, imgs)
+		})
+	}
+}
+
 func prepareDiskToMirror(testCase testCase) error {
 	for _, repo := range testCase.helmConfig.Repositories {
 		var err error