Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 4 additions & 33 deletions chartvalidator/checker/applications.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,28 +114,11 @@ func extractApplicationCharts(doc map[string]any, envName, path string) []ChartR
chartName := str(src["chart"])
repoURL := str(src["repoURL"])

// Argo requires one of `path`, `chart`, or `ref` on every source, so
// a source with no chart field is a values-only ref source, or a git
// App-of-Apps path. It has nothing to render.
if chartName == "" {
// 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
}
continue
}

charts = append(charts, ChartRenderParams{
Expand All @@ -149,18 +132,6 @@ 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 {
Expand Down
50 changes: 0 additions & 50 deletions chartvalidator/checker/applications_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,56 +127,6 @@ 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
Expand Down
6 changes: 4 additions & 2 deletions chartvalidator/checker/engine_chart_rendering.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,8 +164,10 @@ func isOCIRepo(repoURL string) bool {
func resolveChartReference(chart ChartRenderParams) string {
if isOCIRepo(chart.RepoURL) {
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.
// An Argo OCI source names the chart twice: repoURL's last segment
// is the chart, and the `chart` field repeats it. Argo requires
// both, because it resolves the revision from repoURL alone, and
// its Application schema requires a `chart` field.
//
// Only append ChartName when repoURL is a registry or namespace
// path that still needs it. Otherwise the reference gets the
Expand Down
4 changes: 2 additions & 2 deletions chartvalidator/checker/engine_chart_rendering_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,8 @@ 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.
// repoURL can already carry the chart name as its last segment. Argo's OCI
// sources work this way, and they set the `chart` field to that same name.
// The chart reference must not repeat the chart name in that case.
func TestRenderOCIRepoChartEmbeddedInRepoURL(t *testing.T) {
mockExecutor := createMockExecutor()
Expand Down
Loading