From 3c6695e35348a21fa88d859f492a9fbfadc820f8 Mon Sep 17 00:00:00 2001 From: Stephan Butler Date: Tue, 18 Aug 2026 10:33:38 +0200 Subject: [PATCH] feat: will now also consider stanalone applications --- chartvalidator/checker/applications.go | 158 ++++++++++ chartvalidator/checker/applications_test.go | 285 ++++++++++++++++++ chartvalidator/checker/appsets.go | 26 +- .../checker/engine_chart_rendering.go | 25 +- chartvalidator/checker/main.go | 14 +- chartvalidator/checker/types.go | 26 ++ 6 files changed, 512 insertions(+), 22 deletions(-) create mode 100644 chartvalidator/checker/applications.go create mode 100644 chartvalidator/checker/applications_test.go diff --git a/chartvalidator/checker/applications.go b/chartvalidator/checker/applications.go new file mode 100644 index 0000000..997b69c --- /dev/null +++ b/chartvalidator/checker/applications.go @@ -0,0 +1,158 @@ +package main + +import ( + "bytes" + "errors" + "fmt" + "io" + "os" + "path/filepath" + "regexp" + "strings" + + "gopkg.in/yaml.v3" +) + +// valuesRefPrefix matches the "$/" prefix ArgoCD uses in a +// multi-source Application's helm.valueFiles to point at a sibling source in +// the same Application (conventionally "$values/"). What follows the prefix is +// a path relative to the root of the referenced git repository. +var valuesRefPrefix = regexp.MustCompile(`^\$[A-Za-z0-9_.-]+/`) + +// chartsFromApplications scans ArgoCD Application manifests under +// /applications and returns one ChartRenderParams per Helm chart +// source found. This is the layout used by clusters, which declare their core +// charts as individual Applications rather than through an ApplicationSet. +// +// Applications that reference no chart are skipped rather than treated as an +// error: App-of-Apps entries point at a git path in another repository, and the +// values-only "ref" source of a multi-source Application has nothing to render. +func chartsFromApplications(envName, envPath string) ([]ChartRenderParams, error) { + appsPath := filepath.Join(envPath, "applications") + ok, err := existsDir(appsPath) + if err != nil || !ok { + return []ChartRenderParams{}, err + } + + // Recursive, so charts stay discoverable if applications/ grows subfolders. + files, err := findYAMLFiles(appsPath) + if err != nil { + return nil, err + } + + var charts []ChartRenderParams + for _, f := range files { + data, err := os.ReadFile(f) + if err != nil { + return nil, err + } + docs, err := decodeYAMLDocuments(data, f) + if err != nil { + return nil, err + } + for _, doc := range docs { + charts = append(charts, extractApplicationCharts(doc, envName, f)...) + } + } + return charts, nil +} + +// decodeYAMLDocuments decodes every mapping document in a possibly +// multi-document YAML file. +func decodeYAMLDocuments(data []byte, path string) ([]map[string]any, error) { + var out []map[string]any + dec := yaml.NewDecoder(bytes.NewReader(data)) + for { + var node any + err := dec.Decode(&node) + if errors.Is(err, io.EOF) { + break + } + if err != nil { + return nil, fmt.Errorf("failed to parse YAML %s: %w", path, err) + } + if m, ok := node.(map[string]any); ok { + out = append(out, m) + } + } + return out, nil +} + +// extractApplicationCharts pulls the Helm chart sources out of a single +// Application document. Both spec.source (single-source) and spec.sources +// (multi-source) are handled. +func extractApplicationCharts(doc map[string]any, envName, path string) []ChartRenderParams { + if str(doc["kind"]) != "Application" { + return nil + } + spec, _ := doc["spec"].(map[string]any) + if spec == nil { + return nil + } + + var sources []map[string]any + if list, ok := spec["sources"].([]any); ok { + for _, s := range list { + if m, ok := s.(map[string]any); ok { + sources = append(sources, m) + } + } + } + if single, ok := spec["source"].(map[string]any); ok { + sources = append(sources, single) + } + + appName := "" + if md, ok := doc["metadata"].(map[string]any); ok { + if n := str(md["name"]); n != "" { + appName = n + } + } + + var charts []ChartRenderParams + for _, src := range sources { + chartName := str(src["chart"]) + if chartName == "" { + continue + } + charts = append(charts, ChartRenderParams{ + Env: envName, + ChartName: chartName, + RepoURL: str(src["repoURL"]), + ChartVersion: str(src["targetRevision"]), + ValueFiles: applicationValueFiles(src, appName, path), + }) + } + return charts +} + +// 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 { + helm, _ := src["helm"].(map[string]any) + if helm == nil { + return nil + } + + if helm["values"] != nil || helm["valuesObject"] != nil { + fmt.Printf("WARNING: %s (%s): inline helm values are not applied by the chart checker; only valueFiles are rendered\n", appName, path) + } + + list, _ := helm["valueFiles"].([]any) + var out []string + for _, v := range list { + p := str(v) + if p == "" { + continue + } + prefix := valuesRefPrefix.FindString(p) + if prefix == "" { + // Without a "$ref/" prefix ArgoCD resolves the path inside the chart + // itself, so there is no corresponding file in this repository. + fmt.Printf("WARNING: %s (%s): skipping chart-internal values file %q\n", appName, path, p) + continue + } + out = append(out, srcPrefix+strings.TrimPrefix(p, prefix)) + } + return out +} diff --git a/chartvalidator/checker/applications_test.go b/chartvalidator/checker/applications_test.go new file mode 100644 index 0000000..18d5a3f --- /dev/null +++ b/chartvalidator/checker/applications_test.go @@ -0,0 +1,285 @@ +package main + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// writeApplicationFile writes content to //applications/. +func writeApplicationFile(t *testing.T, root, env, name, content string) { + t.Helper() + dir := filepath.Join(root, env, "applications") + require.NoError(t, os.MkdirAll(dir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, name), []byte(content), 0o644)) +} + +const multiSourceApp = `apiVersion: argoproj.io/v1alpha1 +kind: Application +metadata: + name: core-traefik-main + namespace: argocd +spec: + project: cluster-gateway + sources: + - repoURL: git@github.com:interledger/clusternation-deploy.git + targetRevision: main + ref: values + - repoURL: https://traefik.github.io/charts + chart: traefik + targetRevision: 41.2.0 + helm: + valueFiles: + - $values/base/values/blank.yaml + - $values/clusters/ilf-1/values/core-traefik.yaml + destination: + server: https://kubernetes.default.svc + namespace: traefik +` + +func TestChartsFromApplications_MultiSource(t *testing.T) { + root := t.TempDir() + writeApplicationFile(t, root, "ilf-1", "core-traefik.yaml", multiSourceApp) + + charts, err := chartsFromApplications("ilf-1", filepath.Join(root, "ilf-1")) + 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, "ilf-1", c.Env) + assert.Equal(t, "traefik", c.ChartName) + assert.Equal(t, "https://traefik.github.io/charts", c.RepoURL) + assert.Equal(t, "41.2.0", c.ChartVersion) + assert.Equal(t, []string{ + srcPrefix + "base/values/blank.yaml", + srcPrefix + "clusters/ilf-1/values/core-traefik.yaml", + }, c.ValueFiles) +} + +func TestChartsFromApplications_SingleSource(t *testing.T) { + root := t.TempDir() + writeApplicationFile(t, root, "ilf-1", "app.yaml", `apiVersion: argoproj.io/v1alpha1 +kind: Application +metadata: + name: single +spec: + source: + repoURL: https://prometheus-community.github.io/helm-charts + chart: kube-prometheus-stack + targetRevision: 88.3.0 +`) + + charts, err := chartsFromApplications("ilf-1", filepath.Join(root, "ilf-1")) + require.NoError(t, err) + require.Len(t, charts, 1) + assert.Equal(t, "kube-prometheus-stack", charts[0].ChartName) + assert.Equal(t, "88.3.0", charts[0].ChartVersion) + assert.Empty(t, charts[0].ValueFiles, "no helm block means no values files") +} + +// An App-of-Apps points at a git path in another repository; there is no chart +// to render, and that must not be an error. +func TestChartsFromApplications_SkipsNonChartSources(t *testing.T) { + root := t.TempDir() + writeApplicationFile(t, root, "ilf-1", "app-of-apps.yaml", `apiVersion: argoproj.io/v1alpha1 +kind: Application +metadata: + name: business-apps +spec: + source: + repoURL: git@github.com:interledger/some-other-repo.git + targetRevision: main + path: argocd/applications +`) + + charts, err := chartsFromApplications("ilf-1", filepath.Join(root, "ilf-1")) + require.NoError(t, err) + assert.Empty(t, charts) +} + +func TestChartsFromApplications_MultiDocumentAndOtherKinds(t *testing.T) { + root := t.TempDir() + writeApplicationFile(t, root, "ilf-1", "combined.yaml", `apiVersion: argoproj.io/v1alpha1 +kind: AppProject +metadata: + name: cluster-gateway +spec: + sourceRepos: + - '*' +--- +apiVersion: argoproj.io/v1alpha1 +kind: Application +metadata: + name: chart-app +spec: + source: + repoURL: https://example.com/charts + chart: example + targetRevision: 1.2.3 +`) + + charts, err := chartsFromApplications("ilf-1", filepath.Join(root, "ilf-1")) + require.NoError(t, err) + require.Len(t, charts, 1, "the AppProject document must be ignored") + assert.Equal(t, "example", charts[0].ChartName) +} + +func TestChartsFromApplications_MultipleChartSourcesInOneApp(t *testing.T) { + root := t.TempDir() + writeApplicationFile(t, root, "ilf-1", "two-charts.yaml", `apiVersion: argoproj.io/v1alpha1 +kind: Application +metadata: + name: two-charts +spec: + sources: + - repoURL: https://example.com/charts + chart: first + targetRevision: 1.0.0 + - repoURL: https://example.com/charts + chart: second + targetRevision: 2.0.0 +`) + + charts, err := chartsFromApplications("ilf-1", filepath.Join(root, "ilf-1")) + require.NoError(t, err) + require.Len(t, charts, 2) + assert.Equal(t, "first", charts[0].ChartName) + assert.Equal(t, "second", charts[1].ChartName) +} + +// A valueFiles entry without a "$ref/" prefix is resolved by ArgoCD inside the +// chart, so it cannot be located in the repository and is skipped. A custom ref +// name is honoured. +func TestChartsFromApplications_ValueFilePrefixHandling(t *testing.T) { + root := t.TempDir() + writeApplicationFile(t, root, "ilf-1", "prefixes.yaml", `apiVersion: argoproj.io/v1alpha1 +kind: Application +metadata: + name: prefixes +spec: + sources: + - repoURL: https://example.com/charts + chart: example + targetRevision: 1.0.0 + helm: + valueFiles: + - $custom-ref/clusters/ilf-1/values/example.yaml + - values-production.yaml +`) + + charts, err := chartsFromApplications("ilf-1", filepath.Join(root, "ilf-1")) + require.NoError(t, err) + require.Len(t, charts, 1) + assert.Equal(t, []string{srcPrefix + "clusters/ilf-1/values/example.yaml"}, charts[0].ValueFiles) +} + +func TestChartsFromApplications_RecursesIntoSubfolders(t *testing.T) { + root := t.TempDir() + dir := filepath.Join(root, "ilf-1", "applications", "nested") + require.NoError(t, os.MkdirAll(dir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "app.yaml"), []byte(`apiVersion: argoproj.io/v1alpha1 +kind: Application +metadata: + name: nested +spec: + source: + repoURL: https://example.com/charts + chart: nested-chart + targetRevision: 9.9.9 +`), 0o644)) + + charts, err := chartsFromApplications("ilf-1", filepath.Join(root, "ilf-1")) + require.NoError(t, err) + require.Len(t, charts, 1) + assert.Equal(t, "nested-chart", charts[0].ChartName) +} + +func TestChartsFromApplications_NoApplicationsDir(t *testing.T) { + root := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(root, "sandbox", "appsets"), 0o755)) + + charts, err := chartsFromApplications("sandbox", filepath.Join(root, "sandbox")) + require.NoError(t, err) + assert.Empty(t, charts) +} + +func TestChartsFromApplications_InvalidYAML(t *testing.T) { + root := t.TempDir() + writeApplicationFile(t, root, "ilf-1", "broken.yaml", "kind: Application\n\tbad indentation:\n") + + _, err := chartsFromApplications("ilf-1", filepath.Join(root, "ilf-1")) + require.Error(t, err) + assert.Contains(t, err.Error(), "failed to parse YAML") +} + +// findCharts must pick up both layouts, so a tree mixing ApplicationSets and +// Applications is fully covered. +func TestFindCharts_BothLayouts(t *testing.T) { + root := t.TempDir() + + appsets := filepath.Join(root, "mixed", "appsets") + require.NoError(t, os.MkdirAll(appsets, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(appsets, "cluster-appset.yaml"), []byte(`apiVersion: argoproj.io/v1alpha1 +kind: ApplicationSet +metadata: + name: cluster-appset +spec: + generators: + - list: + elements: + - name: from-appset + chartName: appset-chart + repoURL: https://example.com/charts + chartVersion: 1.0.0 + baseValuesFile: base/values/blank.yaml + valuesOverride: env/mixed/values/appset-chart.yaml +`), 0o644)) + + writeApplicationFile(t, root, "mixed", "app.yaml", `apiVersion: argoproj.io/v1alpha1 +kind: Application +metadata: + name: from-application +spec: + source: + repoURL: https://example.com/charts + chart: application-chart + targetRevision: 2.0.0 +`) + + charts, err := findCharts(root, "mixed") + require.NoError(t, err) + require.Len(t, charts, 2) + + names := []string{charts[0].ChartName, charts[1].ChartName} + assert.ElementsMatch(t, []string{"appset-chart", "application-chart"}, names) +} + +func TestResolvedValueFiles(t *testing.T) { + t.Run("falls back to the appset base/override pair", func(t *testing.T) { + refs := ChartRenderParams{ + BaseValuesFile: "../base/values/blank.yaml", + ValuesOverride: "../env/sandbox/values/traefik.yaml", + }.resolvedValueFiles() + + require.Len(t, refs, 2) + assert.Equal(t, "../base/values/blank.yaml", refs[0].path) + assert.Equal(t, "base values file", refs[0].label) + assert.Equal(t, "../env/sandbox/values/traefik.yaml", refs[1].path) + assert.Equal(t, "values override file", refs[1].label) + }) + + t.Run("ValueFiles replaces the pair", func(t *testing.T) { + refs := ChartRenderParams{ + BaseValuesFile: "ignored.yaml", + ValuesOverride: "ignored-too.yaml", + ValueFiles: []string{"a.yaml", "b.yaml", "c.yaml"}, + }.resolvedValueFiles() + + require.Len(t, refs, 3) + assert.Equal(t, []string{"a.yaml", "b.yaml", "c.yaml"}, + []string{refs[0].path, refs[1].path, refs[2].path}) + }) +} diff --git a/chartvalidator/checker/appsets.go b/chartvalidator/checker/appsets.go index 1c6dc96..ca22411 100644 --- a/chartvalidator/checker/appsets.go +++ b/chartvalidator/checker/appsets.go @@ -9,8 +9,10 @@ import ( "gopkg.in/yaml.v3" ) -// findChartsInAppsets scans ApplicationSet files and extracts chart information -func findChartsInAppsets(envDir, selectedEnv string) ([]ChartRenderParams, error) { +// findCharts scans an environment tree and extracts chart information from both +// supported layouts: ApplicationSets under appsets/ and Applications under +// applications/. envDir selects the tree (e.g. ../env or ../clusters). +func findCharts(envDir, selectedEnv string) ([]ChartRenderParams, error) { const suffix = "appset.yaml" var out []ChartRenderParams @@ -51,8 +53,26 @@ func findChartsInAppsets(envDir, selectedEnv string) ([]ChartRenderParams, error return out, nil } -// processEnvironment extracts charts from a single environment directory +// processEnvironment extracts charts from a single environment directory, from +// both the appsets/ and applications/ layouts. An environment that uses only +// one of them simply contributes nothing from the other. func processEnvironment(envName, envPath, suffix string) ([]ChartRenderParams, error) { + charts, err := chartsFromAppsets(envName, envPath, suffix) + if err != nil { + return nil, err + } + + appCharts, err := chartsFromApplications(envName, envPath) + if err != nil { + return nil, err + } + + return append(charts, appCharts...), nil +} + +// chartsFromAppsets extracts charts from the ApplicationSet list generators in +// /appsets. +func chartsFromAppsets(envName, envPath, suffix string) ([]ChartRenderParams, error) { appsetsPath := filepath.Join(envPath, "appsets") ok, err := existsDir(appsetsPath) if err != nil || !ok { diff --git a/chartvalidator/checker/engine_chart_rendering.go b/chartvalidator/checker/engine_chart_rendering.go index abdbe15..5269ff3 100644 --- a/chartvalidator/checker/engine_chart_rendering.go +++ b/chartvalidator/checker/engine_chart_rendering.go @@ -77,15 +77,13 @@ func (engine *ChartRenderingEngine) worker(workerId int) { func (engine *ChartRenderingEngine) renderSingleChart(chart ChartRenderParams, workerId int) (*RenderResult, error) { - if !engine.executor.FileExists(chart.BaseValuesFile) { - msg := fmt.Sprintf("base values file does not exist: %s", chart.BaseValuesFile) - logEngineWarning(engine.name, workerId, msg) - return nil, fmt.Errorf("base values file does not exist: %s", chart.BaseValuesFile) - } - if !engine.executor.FileExists(chart.ValuesOverride) { - msg := fmt.Sprintf("values override file does not exist: %s", chart.ValuesOverride) - logEngineWarning(engine.name, workerId, msg) - return nil, fmt.Errorf("values override file does not exist: %s", chart.ValuesOverride) + valueFiles := chart.resolvedValueFiles() + for _, vf := range valueFiles { + if !engine.executor.FileExists(vf.path) { + msg := fmt.Sprintf("%s does not exist: %s", vf.label, vf.path) + logEngineWarning(engine.name, workerId, msg) + return nil, fmt.Errorf("%s does not exist: %s", vf.label, vf.path) + } } // Normalize repoURL so that scheme-less OCI references (which ArgoCD @@ -97,12 +95,15 @@ func (engine *ChartRenderingEngine) renderSingleChart(chart ChartRenderParams, w "template", chart.ChartName, resolveChartReference(chart), - "-f", chart.BaseValuesFile, - "-f", chart.ValuesOverride, + } + for _, vf := range valueFiles { + args = append(args, "-f", vf.path) + } + args = append(args, "--version", chart.ChartVersion, "--include-crds", "--kube-version", kubernetesVersion, - } + ) if !isOCIRepo(chart.RepoURL) { args = append(args, "--repo", chart.RepoURL) diff --git a/chartvalidator/checker/main.go b/chartvalidator/checker/main.go index 0f2cb21..ffb2040 100644 --- a/chartvalidator/checker/main.go +++ b/chartvalidator/checker/main.go @@ -62,9 +62,9 @@ func runChartChecksCommand(args []string) { fs.Usage = func() { fmt.Println("Usage: run-manifest-checks run-checks [flags]") fmt.Println("") - fmt.Println("Will run a series of checks against all charts found in the ApplicationSets in the specified environment.") + fmt.Println("Will run a series of checks against all charts found in the ApplicationSets and Applications in the specified environment.") fmt.Println("Steps are as follows:") - fmt.Println(" 1. Find all charts referenced in ApplicationSets in the specified environment.") + fmt.Println(" 1. Find all charts referenced in ApplicationSets and Applications in the specified environment.") fmt.Println(" 2. Render each chart with its values using Helm.") fmt.Println(" 3. Validate the rendered manifests using kubeconform.") fmt.Println(" 4. Extract Docker image references from the manifests.") @@ -107,7 +107,7 @@ func runRenderOnlyCommand(args []string) { fs.Usage = func() { fmt.Println("Usage: run-manifest-checks render-only [flags]") fmt.Println("") - fmt.Println("Renders all charts found in the ApplicationSets in the specified environment and outputs the manifests to the specified output directory.") + fmt.Println("Renders all charts found in the ApplicationSets and Applications in the specified environment and outputs the manifests to the specified output directory.") fmt.Println("") fs.PrintDefaults() } @@ -132,9 +132,9 @@ func runRenderOnlyCommand(args []string) { func runAllChartRenders(singleEnv, envDir, outputDir string, apiVersions []string) error { fmt.Println("Starting chart renders...") - params, err := findChartsInAppsets(envDir, singleEnv) + params, err := findCharts(envDir, singleEnv) if err != nil { - return fmt.Errorf("failed to find charts in ApplicationSets: %w", err) + return fmt.Errorf("failed to find charts: %w", err) } fmt.Printf("Found %d charts to process.\n", len(params)) @@ -185,9 +185,9 @@ func runAllChartRenders(singleEnv, envDir, outputDir string, apiVersions []strin func runAllChartChecks(singleEnv, envDir, outputDir string, apiVersions []string) error { fmt.Println("Starting chart checks...") - params, err := findChartsInAppsets(envDir, singleEnv) + params, err := findCharts(envDir, singleEnv) if err != nil { - return fmt.Errorf("failed to find charts in ApplicationSets: %w", err) + return fmt.Errorf("failed to find charts: %w", err) } fmt.Printf("Found %d charts to process.\n", len(params)) diff --git a/chartvalidator/checker/types.go b/chartvalidator/checker/types.go index 095e151..1d6fb4f 100644 --- a/chartvalidator/checker/types.go +++ b/chartvalidator/checker/types.go @@ -31,6 +31,32 @@ type ChartRenderParams struct { ChartVersion string `json:"chartVersion"` BaseValuesFile string `json:"baseValuesFile"` ValuesOverride string `json:"valuesOverride"` + // ValueFiles is an ordered list of values files, used by charts discovered + // in Applications: their helm.valueFiles can hold any number of entries, + // whereas an ApplicationSet list element always carries the fixed + // BaseValuesFile/ValuesOverride pair above. When set it replaces that pair. + ValueFiles []string `json:"valueFiles,omitempty"` +} + +// valueFileRef is a values file plus the wording used to report it missing. +type valueFileRef struct { + path string + label string +} + +// resolvedValueFiles returns the values files to pass to helm, in order. +func (c ChartRenderParams) resolvedValueFiles() []valueFileRef { + if len(c.ValueFiles) > 0 { + refs := make([]valueFileRef, 0, len(c.ValueFiles)) + for _, p := range c.ValueFiles { + refs = append(refs, valueFileRef{path: p, label: "values file"}) + } + return refs + } + return []valueFileRef{ + {path: c.BaseValuesFile, label: "base values file"}, + {path: c.ValuesOverride, label: "values override file"}, + } } // task represents a validation task with a chart and command