diff --git a/pkg/helmexec/exec.go b/pkg/helmexec/exec.go index e2df4423..572db447 100644 --- a/pkg/helmexec/exec.go +++ b/pkg/helmexec/exec.go @@ -525,8 +525,32 @@ func (helm *execer) ChartPull(chart string, path string, flags ...string) error if helmVersionConstraint.Check(helm.version) { // in the 3.7.0 version, the chart pull has been replaced with helm pull // https://github.com/helm/helm/releases/tag/v3.7.0 - ociChartURL, _ := resolveOciChart(chart) - helmArgs = []string{"pull", ociChartURL, "--destination", path, "--untar"} + + // Check if chart already contains digest info (always use full URL for digests) + hasDigest := strings.Contains(chart, "@") + + // For version tags, only use full URL if it looks like a proper OCI registry URL + // (contains registry hostname with dots, not just repo/chart:version format) + hasVersionTag := !hasDigest && strings.LastIndex(chart, ":") > strings.LastIndex(chart, "/") + isFullOCIURL := false + if hasVersionTag { + // Check if this looks like a proper OCI registry URL by looking for hostname with dots + parts := strings.Split(chart, "/") + if len(parts) > 0 { + firstPart := parts[0] + // If first part contains a dot (domain) or explicitly has oci:// prefix, treat as full OCI URL + isFullOCIURL = strings.Contains(firstPart, ".") || strings.HasPrefix(chart, "oci://") + } + } + + if hasDigest || (hasVersionTag && isFullOCIURL) { + // Chart already contains version/digest for proper OCI registry, use full URL without resolving + helmArgs = []string{"pull", fmt.Sprintf("oci://%s", chart), "--destination", path, "--untar"} + } else { + // Chart doesn't contain version/digest or is legacy format, use original resolution + ociChartURL, _ := resolveOciChart(chart) + helmArgs = []string{"pull", ociChartURL, "--destination", path, "--untar"} + } helmArgs = append(helmArgs, flags...) } else { helmArgs = []string{"chart", "pull", chart} @@ -665,18 +689,37 @@ func (helm *execer) IsVersionAtLeast(versionStr string) bool { } func resolveOciChart(ociChart string) (ociChartURL, ociChartTag string) { + // Check for digest syntax (@sha256:...) + digestIndex := strings.Index(ociChart, "@") + hasDigest := digestIndex != -1 + var urlTagIndex int - // Get the last : index - // e.g., - // 1. registry:443/helm-charts - // 2. registry/helm-charts:latest - // 3. registry:443/helm-charts:latest - if strings.LastIndex(ociChart, ":") <= strings.LastIndex(ociChart, "/") { - urlTagIndex = len(ociChart) - ociChartTag = "" + if hasDigest { + // For charts with digest, check for version before digest + beforeDigest := ociChart[:digestIndex] + if strings.LastIndex(beforeDigest, ":") <= strings.LastIndex(beforeDigest, "/") { + // No version tag before digest, just digest + urlTagIndex = len(ociChart) + ociChartTag = "" + } else { + // Version tag before digest (e.g., chart:1.0.0@sha256:...) + urlTagIndex = strings.LastIndex(beforeDigest, ":") + ociChartTag = beforeDigest[urlTagIndex+1:] + } } else { - urlTagIndex = strings.LastIndex(ociChart, ":") - ociChartTag = ociChart[urlTagIndex+1:] + // Original logic for version tags + // Get the last : index + // e.g., + // 1. registry:443/helm-charts + // 2. registry/helm-charts:latest + // 3. registry:443/helm-charts:latest + if strings.LastIndex(ociChart, ":") <= strings.LastIndex(ociChart, "/") { + urlTagIndex = len(ociChart) + ociChartTag = "" + } else { + urlTagIndex = strings.LastIndex(ociChart, ":") + ociChartTag = ociChart[urlTagIndex+1:] + } } ociChartURL = fmt.Sprintf("oci://%s", ociChart[:urlTagIndex]) return ociChartURL, ociChartTag diff --git a/pkg/helmexec/exec_test.go b/pkg/helmexec/exec_test.go index bbf88808..5fcedeb1 100644 --- a/pkg/helmexec/exec_test.go +++ b/pkg/helmexec/exec_test.go @@ -841,6 +841,39 @@ exec: helm --kubeconfig config --kube-context dev pull oci://repo/helm-charts -- chartFlags: []string{"--untardir", "/tmp/dir", "--devel"}, listResult: `Pulling repo/helm-charts exec: helm --kubeconfig config --kube-context dev pull oci://repo/helm-charts --destination path1 --untar --untardir /tmp/dir --devel +`, + }, + { + name: "oci chart with version in URL", + helmBin: "helm", + helmVersion: "v3.10.0", + chartName: "ghcr.io/nginxinc/charts/nginx-ingress:2.0.0", + chartPath: "path1", + chartFlags: []string{"--untardir", "/tmp/dir"}, + listResult: `Pulling ghcr.io/nginxinc/charts/nginx-ingress:2.0.0 +exec: helm --kubeconfig config --kube-context dev pull oci://ghcr.io/nginxinc/charts/nginx-ingress:2.0.0 --destination path1 --untar --untardir /tmp/dir +`, + }, + { + name: "oci chart with digest in URL", + helmBin: "helm", + helmVersion: "v3.10.0", + chartName: "ghcr.io/nginxinc/charts/nginx-ingress@sha256:87ad282a8e7cc31913ce0543de2933ddb3f3eba80d6e5285f33b62ed720fc085", + chartPath: "path1", + chartFlags: []string{"--untardir", "/tmp/dir"}, + listResult: `Pulling ghcr.io/nginxinc/charts/nginx-ingress@sha256:87ad282a8e7cc31913ce0543de2933ddb3f3eba80d6e5285f33b62ed720fc085 +exec: helm --kubeconfig config --kube-context dev pull oci://ghcr.io/nginxinc/charts/nginx-ingress@sha256:87ad282a8e7cc31913ce0543de2933ddb3f3eba80d6e5285f33b62ed720fc085 --destination path1 --untar --untardir /tmp/dir +`, + }, + { + name: "oci chart with version and digest in URL", + helmBin: "helm", + helmVersion: "v3.10.0", + chartName: "ghcr.io/nginxinc/charts/nginx-ingress:2.0.0@sha256:87ad282a8e7cc31913ce0543de2933ddb3f3eba80d6e5285f33b62ed720fc085", + chartPath: "path1", + chartFlags: []string{"--untardir", "/tmp/dir"}, + listResult: `Pulling ghcr.io/nginxinc/charts/nginx-ingress:2.0.0@sha256:87ad282a8e7cc31913ce0543de2933ddb3f3eba80d6e5285f33b62ed720fc085 +exec: helm --kubeconfig config --kube-context dev pull oci://ghcr.io/nginxinc/charts/nginx-ingress:2.0.0@sha256:87ad282a8e7cc31913ce0543de2933ddb3f3eba80d6e5285f33b62ed720fc085 --destination path1 --untar --untardir /tmp/dir `, }, } @@ -1067,6 +1100,24 @@ func Test_resolveOciChart(t *testing.T) { ociChartURL: "oci://chart:5000/nginx", ociChartTag: "", }, + { + name: "digest only", + chartPath: "ghcr.io/nginxinc/charts/nginx-ingress@sha256:87ad282a8e7cc31913ce0543de2933ddb3f3eba80d6e5285f33b62ed720fc085", + ociChartURL: "oci://ghcr.io/nginxinc/charts/nginx-ingress@sha256:87ad282a8e7cc31913ce0543de2933ddb3f3eba80d6e5285f33b62ed720fc085", + ociChartTag: "", + }, + { + name: "version and digest", + chartPath: "ghcr.io/nginxinc/charts/nginx-ingress:2.0.0@sha256:87ad282a8e7cc31913ce0543de2933ddb3f3eba80d6e5285f33b62ed720fc085", + ociChartURL: "oci://ghcr.io/nginxinc/charts/nginx-ingress", + ociChartTag: "2.0.0", + }, + { + name: "port with digest", + chartPath: "registry:5000/chart@sha256:abc123", + ociChartURL: "oci://registry:5000/chart@sha256:abc123", + ociChartTag: "", + }, } for i := range tests { tt := tests[i] @@ -1075,7 +1126,7 @@ func Test_resolveOciChart(t *testing.T) { if tt.ociChartURL != url || tt.ociChartTag != tag { actual := fmt.Sprintf("ociChartURL->%s ociChartTag->%s", url, tag) expected := fmt.Sprintf("ociChartURL->%s ociChartTag->%s", tt.ociChartURL, tt.ociChartTag) - t.Errorf("resolveOciChart()\nactual = %v\nexpect = %v", actual, expected) + t.Errorf("resolveOciChart()\nactual = %v\nexpected = %v", actual, expected) } }) } diff --git a/pkg/state/state.go b/pkg/state/state.go index 60289117..bb77057a 100644 --- a/pkg/state/state.go +++ b/pkg/state/state.go @@ -4071,9 +4071,60 @@ func (st *HelmState) getOCIQualifiedChartName(release *ReleaseSpec, helm helmexe var qualifiedChartName, chartName string if strings.HasPrefix(release.Chart, "oci://") { - parts := strings.Split(release.Chart, "/") - chartName = parts[len(parts)-1] - qualifiedChartName = strings.Replace(fmt.Sprintf("%s:%s", release.Chart, chartVersion), "oci://", "", 1) + // Parse the chart URL to detect existing version/digest information + chartURL := release.Chart + urlWithoutProtocol := strings.TrimPrefix(chartURL, "oci://") + + // Check for digest (@sha256:...) + digestIndex := strings.Index(urlWithoutProtocol, "@") + hasDigest := digestIndex != -1 + + // Check for version tag (:version) + var versionInURL string + if hasDigest { + // Check for version before digest (e.g., chart:1.0.0@sha256:...) + beforeDigest := urlWithoutProtocol[:digestIndex] + if colonIndex := strings.LastIndex(beforeDigest, ":"); colonIndex > strings.LastIndex(beforeDigest, "/") { + versionInURL = beforeDigest[colonIndex+1:] + } + } else { + // Check for version at end (e.g., chart:1.0.0) + if colonIndex := strings.LastIndex(urlWithoutProtocol, ":"); colonIndex > strings.LastIndex(urlWithoutProtocol, "/") { + versionInURL = urlWithoutProtocol[colonIndex+1:] + } + } + + // Determine effective chart version and build qualified name + if hasDigest || versionInURL != "" { + // Chart URL already contains version/digest, use as-is without adding version + qualifiedChartName = urlWithoutProtocol + + // Extract chart name from URL + parts := strings.Split(urlWithoutProtocol, "/") + chartNamePart := parts[len(parts)-1] + + // Remove version/digest from chart name for extraction + if hasDigest { + chartNamePart = strings.Split(chartNamePart, "@")[0] + } + if colonIndex := strings.LastIndex(chartNamePart, ":"); colonIndex > 0 { + chartNamePart = chartNamePart[:colonIndex] + } + chartName = chartNamePart + + // Set effective version for return (use version from URL if available, otherwise indicate digest) + if versionInURL != "" { + chartVersion = versionInURL + } else { + // For digest-only, extract digest as version for helm compatibility + chartVersion = urlWithoutProtocol[digestIndex:] + } + } else { + // No version/digest in URL, use explicit version + parts := strings.Split(urlWithoutProtocol, "/") + chartName = parts[len(parts)-1] + qualifiedChartName = fmt.Sprintf("%s:%s", urlWithoutProtocol, chartVersion) + } } else { var repo *RepositorySpec repo, chartName = st.GetRepositoryAndNameFromChartName(release.Chart) diff --git a/pkg/state/state_test.go b/pkg/state/state_test.go index 4834ba1c..71d27980 100644 --- a/pkg/state/state_test.go +++ b/pkg/state/state_test.go @@ -3310,6 +3310,87 @@ func TestGetOCIQualifiedChartName(t *testing.T) { {"registry/chart-path/chart-name", "chart-name", ""}, }, }, + { + state: HelmState{ + ReleaseSetSpec: ReleaseSetSpec{ + Repositories: []RepositorySpec{}, + Releases: []ReleaseSpec{ + { + Chart: "oci://ghcr.io/nginxinc/charts/nginx-ingress:2.0.0", + }, + }, + }, + }, + helmVersion: "3.13.3", + expected: []struct { + qualifiedChartName string + chartName string + chartVersion string + }{ + {"ghcr.io/nginxinc/charts/nginx-ingress:2.0.0", "nginx-ingress", "2.0.0"}, + }, + }, + { + state: HelmState{ + ReleaseSetSpec: ReleaseSetSpec{ + Repositories: []RepositorySpec{}, + Releases: []ReleaseSpec{ + { + Chart: "oci://ghcr.io/nginxinc/charts/nginx-ingress@sha256:87ad282a8e7cc31913ce0543de2933ddb3f3eba80d6e5285f33b62ed720fc085", + }, + }, + }, + }, + helmVersion: "3.13.3", + expected: []struct { + qualifiedChartName string + chartName string + chartVersion string + }{ + {"ghcr.io/nginxinc/charts/nginx-ingress@sha256:87ad282a8e7cc31913ce0543de2933ddb3f3eba80d6e5285f33b62ed720fc085", "nginx-ingress", "@sha256:87ad282a8e7cc31913ce0543de2933ddb3f3eba80d6e5285f33b62ed720fc085"}, + }, + }, + { + state: HelmState{ + ReleaseSetSpec: ReleaseSetSpec{ + Repositories: []RepositorySpec{}, + Releases: []ReleaseSpec{ + { + Chart: "oci://ghcr.io/nginxinc/charts/nginx-ingress:2.0.0@sha256:87ad282a8e7cc31913ce0543de2933ddb3f3eba80d6e5285f33b62ed720fc085", + }, + }, + }, + }, + helmVersion: "3.13.3", + expected: []struct { + qualifiedChartName string + chartName string + chartVersion string + }{ + {"ghcr.io/nginxinc/charts/nginx-ingress:2.0.0@sha256:87ad282a8e7cc31913ce0543de2933ddb3f3eba80d6e5285f33b62ed720fc085", "nginx-ingress", "2.0.0"}, + }, + }, + { + state: HelmState{ + ReleaseSetSpec: ReleaseSetSpec{ + Repositories: []RepositorySpec{}, + Releases: []ReleaseSpec{ + { + Chart: "oci://ghcr.io/nginxinc/charts/nginx-ingress", + Version: "@sha256:87ad282a8e7cc31913ce0543de2933ddb3f3eba80d6e5285f33b62ed720fc085", + }, + }, + }, + }, + helmVersion: "3.13.3", + expected: []struct { + qualifiedChartName string + chartName string + chartVersion string + }{ + {"ghcr.io/nginxinc/charts/nginx-ingress:@sha256:87ad282a8e7cc31913ce0543de2933ddb3f3eba80d6e5285f33b62ed720fc085", "nginx-ingress", "@sha256:87ad282a8e7cc31913ce0543de2933ddb3f3eba80d6e5285f33b62ed720fc085"}, + }, + }, } for _, tt := range tests {