diff --git a/chartvalidator/checker/applications.go b/chartvalidator/checker/applications.go index 997b69c..103b050 100644 --- a/chartvalidator/checker/applications.go +++ b/chartvalidator/checker/applications.go @@ -112,13 +112,36 @@ func extractApplicationCharts(doc map[string]any, envName, path string) []ChartR var charts []ChartRenderParams for _, src := range sources { chartName := str(src["chart"]) + repoURL := str(src["repoURL"]) + if chartName == "" { - continue + // Argo's native OCI sources carry no separate `chart` field. + // The chart itself is the full repoURL, for example + // oci://ghcr.io/interledger/charts/merchant. The chart name + // is that URL's last path segment. + // + // This check only recognizes an explicit "oci://" scheme. It + // does not recognize the scheme-less form ApplicationSets + // accept (see normalizeRepoURL). A scheme-less git remote, + // for example git@github.com:org/repo.git, also has no + // "://". Reading it as OCI would misread it as a chart. + // + // A non-OCI source with no chart field is a values-only ref + // source, or a git App-of-Apps path. It has nothing to + // render. + if !isOCIRepo(repoURL) { + continue + } + chartName = lastPathSegment(repoURL) + if chartName == "" { + continue + } } + charts = append(charts, ChartRenderParams{ Env: envName, ChartName: chartName, - RepoURL: str(src["repoURL"]), + RepoURL: repoURL, ChartVersion: str(src["targetRevision"]), ValueFiles: applicationValueFiles(src, appName, path), }) @@ -126,6 +149,18 @@ func extractApplicationCharts(doc map[string]any, envName, path string) []ChartR return charts } +// lastPathSegment returns a URL or path's final "/"-separated, non-empty +// segment. It names a chart when repoURL already points at the chart +// itself. In that case, no separate chart name field exists. +func lastPathSegment(url string) string { + trimmed := strings.TrimRight(url, "/") + if trimmed == "" { + return "" + } + idx := strings.LastIndex(trimmed, "/") + return trimmed[idx+1:] +} + // applicationValueFiles maps an Application source's helm.valueFiles onto paths // this tool can read, which are relative to the checker's working directory. func applicationValueFiles(src map[string]any, appName, path string) []string { diff --git a/chartvalidator/checker/applications_test.go b/chartvalidator/checker/applications_test.go index 18d5a3f..5265bee 100644 --- a/chartvalidator/checker/applications_test.go +++ b/chartvalidator/checker/applications_test.go @@ -127,6 +127,56 @@ spec: assert.Equal(t, "example", charts[0].ChartName) } +// Argo's native OCI sources carry no separate `chart` field. repoURL is the +// full chart path. The chart name is its last path segment. +func TestChartsFromApplications_OCISourceWithoutChartField(t *testing.T) { + root := t.TempDir() + writeApplicationFile(t, root, "cards-playground", "merchant.yaml", `apiVersion: argoproj.io/v1alpha1 +kind: Application +metadata: + name: merchant +spec: + sources: + - repoURL: git@github.com:interledger/testnet-deploy.git + targetRevision: main + ref: values + - repoURL: oci://ghcr.io/interledger/charts/merchant + targetRevision: 0.0.4 + helm: + valueFiles: + - $values/env/cards-playground/merchant/merchant.yaml +`) + + charts, err := chartsFromApplications("cards-playground", filepath.Join(root, "cards-playground")) + require.NoError(t, err) + require.Len(t, charts, 1, "the values-only ref source must not yield a chart") + + c := charts[0] + assert.Equal(t, "merchant", c.ChartName) + assert.Equal(t, "oci://ghcr.io/interledger/charts/merchant", c.RepoURL) + assert.Equal(t, "0.0.4", c.ChartVersion) +} + +// A scheme-less repoURL with no chart field is not read as an OCI chart. +// It is indistinguishable from an SCP-style git remote, for example +// git@github.com:org/repo.git. That remote also has no "://". +func TestChartsFromApplications_SchemeLessRepoWithoutChartFieldIsSkipped(t *testing.T) { + root := t.TempDir() + writeApplicationFile(t, root, "cards-playground", "merchant.yaml", `apiVersion: argoproj.io/v1alpha1 +kind: Application +metadata: + name: merchant +spec: + source: + repoURL: ghcr.io/interledger/charts/merchant + targetRevision: 0.0.4 +`) + + charts, err := chartsFromApplications("cards-playground", filepath.Join(root, "cards-playground")) + require.NoError(t, err) + assert.Empty(t, charts) +} + func TestChartsFromApplications_MultipleChartSourcesInOneApp(t *testing.T) { root := t.TempDir() writeApplicationFile(t, root, "ilf-1", "two-charts.yaml", `apiVersion: argoproj.io/v1alpha1 diff --git a/chartvalidator/checker/engine_chart_rendering.go b/chartvalidator/checker/engine_chart_rendering.go index 5269ff3..62d4b71 100644 --- a/chartvalidator/checker/engine_chart_rendering.go +++ b/chartvalidator/checker/engine_chart_rendering.go @@ -163,7 +163,17 @@ func isOCIRepo(repoURL string) bool { func resolveChartReference(chart ChartRenderParams) string { if isOCIRepo(chart.RepoURL) { - return fmt.Sprintf("%s/%s", strings.TrimSuffix(chart.RepoURL, "/"), chart.ChartName) + trimmed := strings.TrimSuffix(chart.RepoURL, "/") + // Argo's native OCI sources already carry the chart name as + // repoURL's last segment. See lastPathSegment in applications.go. + // + // Only append ChartName when repoURL is a registry or namespace + // path that still needs it. Otherwise the reference gets the + // chart name twice. + if strings.HasSuffix(trimmed, "/"+chart.ChartName) { + return trimmed + } + return fmt.Sprintf("%s/%s", trimmed, chart.ChartName) } return chart.ChartName } diff --git a/chartvalidator/checker/engine_chart_rendering_test.go b/chartvalidator/checker/engine_chart_rendering_test.go index ad65c5d..184f023 100644 --- a/chartvalidator/checker/engine_chart_rendering_test.go +++ b/chartvalidator/checker/engine_chart_rendering_test.go @@ -95,6 +95,29 @@ func TestRenderOCIRepoNoScheme(t *testing.T) { assert.Equal(t, expectedCommand, actualCommand) } +// repoURL can already carry the chart name as its last segment. Argo's +// native OCI sources work this way; see lastPathSegment in applications.go. +// The chart reference must not repeat the chart name in that case. +func TestRenderOCIRepoChartEmbeddedInRepoURL(t *testing.T) { + mockExecutor := createMockExecutor() + mockExecutor.Output = []byte("Pulled: ghcr.io/interledger/charts/merchant:0.0.4\nDigest: sha256:abc123\n---\napiVersion: v1\nkind: ConfigMap\nmetadata:\n name: test\n") + engine := createEngine(mockExecutor, false) + defer cleanupEngine(engine) + + testChart := createTestChart() + testChart.ChartName = "merchant" + testChart.RepoURL = "oci://ghcr.io/interledger/charts/merchant" + testChart.ChartVersion = "0.0.4" + engine.inputChan <- testChart + + result := <-engine.resultChan + assertChartFieldsMatch(t, testChart, result.Chart) + + expectedCommand := "helm template merchant oci://ghcr.io/interledger/charts/merchant -f values.yaml -f override.yaml --version 0.0.4 --include-crds --kube-version 1.33.0 --api-versions something --api-versions something-else" + actualCommand := mockExecutor.GetFullCommand() + assert.Equal(t, expectedCommand, actualCommand) +} + func TestRenderBaseFileNotExist(t *testing.T) { mockExecutor := createMockExecutor() mockExecutor.FileExistsMap = map[string]bool{