From 207974a6836663a9986876588633286eb03196b9 Mon Sep 17 00:00:00 2001 From: Josh Wolf Date: Fri, 4 Sep 2026 14:19:46 -0600 Subject: [PATCH] feat(chelm): make the test org path and per-case registry configurable charts whose templates treat a dotted first path segment as a registry hostname silently drop the registry, and repository paths beginning with a domain name are common in practice. the hardcoded, dot-free `chainguard/test` hid that from every chart under test. `--test-repository` makes the path a flag; the default is unchanged, since flipping it will legitimately fail charts carrying that heuristic. `test.cases[].registry` is the registry a case's images must resolve under, so a case that sets a chart's registry override in `values` can assert it took effect instead of rendering the same refs as the case without it. --- chelm/cmd/test.go | 19 +- chelm/internal/chelm/parse.go | 6 + chelm/internal/chelm/schema.go | 5 + chelm/internal/chelm/values.go | 25 +- chelm/testdata/test_case_registry.txtar | 232 ++++++++++++++++++ chelm/testdata/test_dotted_org_path.txtar | 75 ++++++ .../test_dotted_org_path_drops_registry.txtar | 67 +++++ 7 files changed, 417 insertions(+), 12 deletions(-) create mode 100644 chelm/testdata/test_case_registry.txtar create mode 100644 chelm/testdata/test_dotted_org_path.txtar create mode 100644 chelm/testdata/test_dotted_org_path_drops_registry.txtar diff --git a/chelm/cmd/test.go b/chelm/cmd/test.go index 501e7188..6a215f1b 100644 --- a/chelm/cmd/test.go +++ b/chelm/cmd/test.go @@ -96,6 +96,7 @@ Exit code is non-zero if any test case fails.`, extraValuesStr, _ := cmd.Flags().GetString("extra-values") setFlags, _ := cmd.Flags().GetStringSlice("set") testRegistry, _ := cmd.Flags().GetString("test-registry") + testRepository, _ := cmd.Flags().GetString("test-repository") // Load cg.json f, err := os.Open(args[0]) @@ -140,8 +141,14 @@ Exit code is non-zero if any test case fails.`, for _, tc := range meta.Test.Cases { caseOut := CaseOutput{Name: tc.Name, Passed: true} + expectedRegistry := tc.Registry + if expectedRegistry == "" { + expectedRegistry = testRegistry + } + // Generate values - values, err := chelm.GenerateValues(meta, tc.Name, testRegistry, extraValues) + values, err := chelm.GenerateValues(meta, tc.Name, + chelm.TestParams{Registry: testRegistry, Repository: testRepository}, extraValues) if err != nil { caseOut.Error = fmt.Sprintf("generating values: %v", err) caseOut.Passed = false @@ -218,14 +225,17 @@ Exit code is non-zero if any test case fails.`, } // Check registry (case-insensitive per OCI spec) - if !strings.EqualFold(ref.Registry, testRegistry) { + if !strings.EqualFold(ref.Registry, expectedRegistry) { caseOut.Passed = false output.Passed = false continue } - // Check repository: must be {DefaultTestRepository}/{imageID} - repoPrefix := chelm.DefaultTestRepository + "/" + // Check repository: must be {testRepository}/{imageID}. The + // prefix holds even for a case that relocates the registry, so + // an override that stacks onto the existing host or swallows + // the first segment of the org path along with it is rejected. + repoPrefix := testRepository + "/" if !strings.HasPrefix(ref.Repo, repoPrefix) { caseOut.Passed = false output.Passed = false @@ -307,4 +317,5 @@ func init() { testCmd.Flags().String("extra-values", "", "Extra values YAML to merge") testCmd.Flags().StringSlice("set", nil, "Set values (passed to helm --set)") testCmd.Flags().String("test-registry", chelm.DefaultTestRegistry, "Registry for test marker images") + testCmd.Flags().String("test-repository", chelm.DefaultTestRepository, "Org path for test marker images") } diff --git a/chelm/internal/chelm/parse.go b/chelm/internal/chelm/parse.go index fcc58769..e09c15f1 100644 --- a/chelm/internal/chelm/parse.go +++ b/chelm/internal/chelm/parse.go @@ -8,6 +8,7 @@ import ( "strings" "chainguard.dev/sdk/helm/images" + "github.com/google/go-containerregistry/pkg/name" ) // Parse parses and validates a cg.json from the given reader. @@ -57,6 +58,11 @@ func (m *CGMeta) Validate() error { return fmt.Errorf("test case %q references unknown image %q", tc.Name, imgID) } } + if tc.Registry != "" { + if _, err := name.NewRegistry(tc.Registry); err != nil { + return fmt.Errorf("test case %q: invalid registry: %w", tc.Name, err) + } + } } return nil } diff --git a/chelm/internal/chelm/schema.go b/chelm/internal/chelm/schema.go index 1b34d507..03be0fba 100644 --- a/chelm/internal/chelm/schema.go +++ b/chelm/internal/chelm/schema.go @@ -22,4 +22,9 @@ type TestCase struct { Name string `json:"name"` Images []string `json:"images,omitempty"` // Image IDs to include in this case Values map[string]any `json:"values,omitempty"` // Case-specific values + // Registry every image must resolve under, defaulting to the test + // registry. A case that sets a chart's registry override in Values names + // the host it expects here, so an override the chart ignores fails instead + // of rendering the same refs as the case without it. + Registry string `json:"registry,omitempty"` } diff --git a/chelm/internal/chelm/values.go b/chelm/internal/chelm/values.go index 4b87bc27..3d661032 100644 --- a/chelm/internal/chelm/values.go +++ b/chelm/internal/chelm/values.go @@ -52,9 +52,15 @@ func DeclaresDigest(img *images.Image) bool { return found } +// TestParams are the base values that ${...} markers resolve against. +type TestParams struct { + Registry string // registry hostname, e.g. "cgr.test" + Repository string // org path every image ID hangs off, e.g. "chainguard/test" +} + // GenerateValues creates Helm values for a test case. // Merges in order: image values < global test values < case values < extra values -func GenerateValues(meta *CGMeta, caseName, testRegistry string, extra map[string]any) (map[string]any, error) { +func GenerateValues(meta *CGMeta, caseName string, p TestParams, extra map[string]any) (map[string]any, error) { // Find the test case var tc *TestCase for i := range meta.Test.Cases { @@ -68,7 +74,7 @@ func GenerateValues(meta *CGMeta, caseName, testRegistry string, extra map[strin } // Generate image values with test markers - imageVals, err := generateImageValues(&images.Mapping{Images: meta.Images}, testRegistry) + imageVals, err := generateImageValues(&images.Mapping{Images: meta.Images}, p) if err != nil { return nil, fmt.Errorf("generating image values: %w", err) } @@ -82,17 +88,20 @@ func GenerateValues(meta *CGMeta, caseName, testRegistry string, extra map[strin return result, nil } -func generateImageValues(m *images.Mapping, testRegistry string) (map[string]any, error) { +func generateImageValues(m *images.Mapping, p TestParams) (map[string]any, error) { if m == nil { return nil, nil } - registry, err := name.NewRegistry(testRegistry) + registry, err := name.NewRegistry(p.Registry) if err != nil { - return nil, fmt.Errorf("invalid marker base %q: %w", testRegistry, err) + return nil, fmt.Errorf("invalid marker base %q: %w", p.Registry, err) + } + if _, err := name.NewRepository(registry.Name() + "/" + p.Repository); err != nil { + return nil, fmt.Errorf("invalid test repository %q: %w", p.Repository, err) } - vals, err := m.Walk(testResolver(registry)) + vals, err := m.Walk(testResolver(registry, p.Repository)) if err != nil { return nil, err } @@ -100,9 +109,9 @@ func generateImageValues(m *images.Mapping, testRegistry string) (map[string]any } // testResolver returns a WalkFunc that substitutes markers with test values. -func testResolver(registry name.Registry) images.WalkFunc { +func testResolver(registry name.Registry, repository string) images.WalkFunc { return func(imageID string, tokens images.TokenList) (any, error) { - repo := registry.Repo(DefaultTestRepository, strings.ToLower(imageID)) + repo := registry.Repo(repository, strings.ToLower(imageID)) var sb strings.Builder for _, tok := range tokens { diff --git a/chelm/testdata/test_case_registry.txtar b/chelm/testdata/test_case_registry.txtar new file mode 100644 index 00000000..b9e83342 --- /dev/null +++ b/chelm/testdata/test_case_registry.txtar @@ -0,0 +1,232 @@ +# A case that engages a chart's registry override names the host it expects, +# so the override has to actually take effect. +chelm test cg.json --chart=chart --test-repository=chainguard/test +cmp stdout expected.json + +# An override the chart accepts but never reads renders the same refs as the +# case without it, which only the expected registry catches. +! chelm test ignored.json --chart=ignored --test-repository=chainguard/test +stdout '"passed": false' +stderr 'FAIL: case "relocated"' + +# An override that prepends instead of replacing the host it already carries +# is caught by the repository prefix, which stays fixed across cases. +! chelm test stacked.json --chart=stacked --test-repository=chainguard/test +stdout 'other.test/cgr.test/chainguard/test/app:v0.0.0' +stderr 'FAIL: case "relocated"' + +# A malformed registry fails when cg.json is parsed, not as a mismatch later. +! chelm test bad.json --chart=chart +stderr 'test case "relocated": invalid registry' + +-- cg.json -- +{ + "images": { + "app": { + "values": { + "image": { + "registry": "${registry}", + "repository": "${repo}", + "tag": "${tag}" + } + } + } + }, + "test": { + "cases": [ + {"name": "default", "images": ["app"]}, + { + "name": "relocated", + "images": ["app"], + "registry": "other.test", + "values": {"global": {"imageRegistry": "other.test"}} + } + ] + } +} + +-- bad.json -- +{ + "images": { + "app": { + "values": { + "image": { + "registry": "${registry}", + "repository": "${repo}", + "tag": "${tag}" + } + } + } + }, + "test": { + "cases": [ + {"name": "relocated", "images": ["app"], "registry": "not a registry"} + ] + } +} + +-- ignored.json -- +{ + "images": { + "app": { + "values": { + "image": { + "registry": "${registry}", + "repository": "${repo}", + "tag": "${tag}" + } + } + } + }, + "test": { + "cases": [ + { + "name": "relocated", + "images": ["app"], + "registry": "other.test", + "values": {"global": {"imageRegistry": "other.test"}} + } + ] + } +} + +-- stacked.json -- +{ + "images": { + "app": { + "values": { + "image": { + "repository": "${registry_repo}", + "tag": "${tag}" + } + } + } + }, + "test": { + "cases": [ + { + "name": "relocated", + "images": ["app"], + "registry": "other.test", + "values": {"global": {"imageRegistry": "other.test"}} + } + ] + } +} + +-- chart/Chart.yaml -- +apiVersion: v2 +name: test-chart +version: 0.1.0 + +-- chart/templates/deployment.yaml -- +apiVersion: apps/v1 +kind: Deployment +metadata: + name: test +spec: + template: + spec: + containers: + - name: app + image: {{ .Values.global.imageRegistry | default .Values.image.registry }}/{{ .Values.image.repository }}:{{ .Values.image.tag }} + +-- chart/values.yaml -- +global: + imageRegistry: "" +image: + registry: docker.io + repository: library/nginx + tag: latest + +-- ignored/Chart.yaml -- +apiVersion: v2 +name: test-chart +version: 0.1.0 + +-- ignored/templates/deployment.yaml -- +apiVersion: apps/v1 +kind: Deployment +metadata: + name: test +spec: + template: + spec: + containers: + - name: app + image: {{ .Values.image.registry }}/{{ .Values.image.repository }}:{{ .Values.image.tag }} + +-- ignored/values.yaml -- +global: + imageRegistry: "" +image: + registry: docker.io + repository: library/nginx + tag: latest + +-- stacked/Chart.yaml -- +apiVersion: v2 +name: test-chart +version: 0.1.0 + +-- stacked/templates/deployment.yaml -- +apiVersion: apps/v1 +kind: Deployment +metadata: + name: test +spec: + template: + spec: + containers: + - name: app + image: {{ with .Values.global.imageRegistry }}{{ . }}/{{ end }}{{ .Values.image.repository }}:{{ .Values.image.tag }} + +-- stacked/values.yaml -- +global: + imageRegistry: "" +image: + repository: docker.io/library/nginx + tag: latest + +-- expected.json -- +{ + "passed": true, + "cases": [ + { + "name": "default", + "passed": true, + "images": [ + "cgr.test/chainguard/test/app:v0.0.0" + ], + "expected": [ + "app" + ], + "extractors": { + "regex": [ + "cgr.test/chainguard/test/app:v0.0.0" + ], + "structured": [ + "cgr.test/chainguard/test/app:v0.0.0" + ] + } + }, + { + "name": "relocated", + "passed": true, + "images": [ + "other.test/chainguard/test/app:v0.0.0" + ], + "expected": [ + "app" + ], + "extractors": { + "regex": [ + "other.test/chainguard/test/app:v0.0.0" + ], + "structured": [ + "other.test/chainguard/test/app:v0.0.0" + ] + } + } + ] +} diff --git a/chelm/testdata/test_dotted_org_path.txtar b/chelm/testdata/test_dotted_org_path.txtar new file mode 100644 index 00000000..c1f9ed40 --- /dev/null +++ b/chelm/testdata/test_dotted_org_path.txtar @@ -0,0 +1,75 @@ +# A dotted org path renders like any other when the registry is authoritative +chelm test cg.json --chart=chart --test-repository=chainguard.test/org +cmp stdout expected.json + +# An org path that is not a valid repository component fails loudly +! chelm test cg.json --chart=chart --test-repository=Chainguard.Test +stdout 'invalid test repository' + +-- cg.json -- +{ + "images": { + "app": { + "values": { + "image": { + "registry": "${registry}", + "repository": "${repo}", + "tag": "${tag}" + } + } + } + }, + "test": { + "cases": [ + {"name": "default", "images": ["app"]} + ] + } +} + +-- chart/Chart.yaml -- +apiVersion: v2 +name: test-chart +version: 0.1.0 + +-- chart/templates/deployment.yaml -- +apiVersion: apps/v1 +kind: Deployment +metadata: + name: test +spec: + template: + spec: + containers: + - name: app + image: {{ .Values.image.registry }}/{{ .Values.image.repository }}:{{ .Values.image.tag }} + +-- chart/values.yaml -- +image: + registry: docker.io + repository: library/nginx + tag: latest + +-- expected.json -- +{ + "passed": true, + "cases": [ + { + "name": "default", + "passed": true, + "images": [ + "cgr.test/chainguard.test/org/app:v0.0.0" + ], + "expected": [ + "app" + ], + "extractors": { + "regex": [ + "cgr.test/chainguard.test/org/app:v0.0.0" + ], + "structured": [ + "cgr.test/chainguard.test/org/app:v0.0.0" + ] + } + } + ] +} diff --git a/chelm/testdata/test_dotted_org_path_drops_registry.txtar b/chelm/testdata/test_dotted_org_path_drops_registry.txtar new file mode 100644 index 00000000..e865ccde --- /dev/null +++ b/chelm/testdata/test_dotted_org_path_drops_registry.txtar @@ -0,0 +1,67 @@ +# A chart that treats a dotted first path segment as a registry hostname drops +# the registry for real org names, which are domains. The dot-free org path +# hides this entirely. +chelm test cg.json --chart=chart --test-repository=chainguard/test +stdout '"passed": true' +stdout 'cgr.test/chainguard/test/app:v0.0.0' + +! chelm test cg.json --chart=chart --test-repository=chainguard.test/org +stdout '"passed": false' +stdout '"chainguard.test/org/app:v0.0.0"' +stderr 'FAIL: case "default"' + +-- cg.json -- +{ + "images": { + "app": { + "values": { + "image": { + "registry": "${registry}", + "repository": "${repo}", + "tag": "${tag}" + } + } + } + }, + "test": { + "cases": [ + {"name": "default", "images": ["app"]} + ] + } +} + +-- chart/Chart.yaml -- +apiVersion: v2 +name: test-chart +version: 0.1.0 + +-- chart/templates/_helpers.tpl -- +{{/* Prepend the registry only when repository does not already look like it +carries a hostname, detected by a dot in the first segment. */}} +{{- define "test-chart.image" -}} +{{- $repository := .Values.image.repository -}} +{{- $first := (split "/" $repository)._0 -}} +{{- $prefix := "" -}} +{{- if and .Values.image.registry (not (contains "." $first)) -}} +{{- $prefix = printf "%s/" .Values.image.registry -}} +{{- end -}} +{{- printf "%s%s:%s" $prefix $repository .Values.image.tag -}} +{{- end -}} + +-- chart/templates/deployment.yaml -- +apiVersion: apps/v1 +kind: Deployment +metadata: + name: test +spec: + template: + spec: + containers: + - name: app + image: {{ include "test-chart.image" . }} + +-- chart/values.yaml -- +image: + registry: docker.io + repository: library/nginx + tag: latest