From 13eb26ca5c664bc582e8ed66b63fdbef517c0161 Mon Sep 17 00:00:00 2001 From: Stephan Butler Date: Wed, 13 May 2026 14:46:54 +0200 Subject: [PATCH] fix: better oci handling --- .../checker/engine_chart_rendering.go | 5 ++++ .../checker/engine_chart_rendering_test.go | 22 +++++++++++++++++ chartvalidator/checker/utils.go | 24 +++++++++++++++++++ chartvalidator/checker/utils_test.go | 23 +++++++++++++++++- 4 files changed, 73 insertions(+), 1 deletion(-) diff --git a/chartvalidator/checker/engine_chart_rendering.go b/chartvalidator/checker/engine_chart_rendering.go index 8976773..abdbe15 100644 --- a/chartvalidator/checker/engine_chart_rendering.go +++ b/chartvalidator/checker/engine_chart_rendering.go @@ -88,6 +88,11 @@ func (engine *ChartRenderingEngine) renderSingleChart(chart ChartRenderParams, w return nil, fmt.Errorf("values override file does not exist: %s", chart.ValuesOverride) } + // Normalize repoURL so that scheme-less OCI references (which ArgoCD + // ApplicationSets accept) are treated the same as explicit `oci://` URLs. + // See normalizeRepoURL for the rationale. + chart.RepoURL = normalizeRepoURL(chart.RepoURL) + args := []string{ "template", chart.ChartName, diff --git a/chartvalidator/checker/engine_chart_rendering_test.go b/chartvalidator/checker/engine_chart_rendering_test.go index 6d5819d..ad65c5d 100644 --- a/chartvalidator/checker/engine_chart_rendering_test.go +++ b/chartvalidator/checker/engine_chart_rendering_test.go @@ -73,6 +73,28 @@ func TestRenderOCIRepo(t *testing.T) { assert.Equal(t, "---\napiVersion: v1\nkind: ConfigMap\nmetadata:\n name: test\n", string(renderedManifest)) } +// Scheme-less repoURLs (as used in ArgoCD ApplicationSets that point at GAR / +// other OCI registries without the oci:// prefix) must be templated as OCI, +// matching the behaviour of explicit `oci://` URLs. +func TestRenderOCIRepoNoScheme(t *testing.T) { + mockExecutor := createMockExecutor() + mockExecutor.Output = []byte("---\napiVersion: v1\nkind: ConfigMap\nmetadata:\n name: test\n") + engine := createEngine(mockExecutor, false) + defer cleanupEngine(engine) + + testChart := createTestChart() + testChart.RepoURL = "europe-west4-docker.pkg.dev/wallet-dev-462809/interledger-helm-charts" + engine.inputChan <- testChart + + result := <-engine.resultChan + // Engine normalises RepoURL, so compare against the normalised form. + assert.Equal(t, "oci://europe-west4-docker.pkg.dev/wallet-dev-462809/interledger-helm-charts", result.Chart.RepoURL) + + expectedCommand := "helm template test-chart oci://europe-west4-docker.pkg.dev/wallet-dev-462809/interledger-helm-charts/test-chart -f values.yaml -f override.yaml --version 1.0.0 --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{ diff --git a/chartvalidator/checker/utils.go b/chartvalidator/checker/utils.go index 0f94c7c..03a2ff1 100644 --- a/chartvalidator/checker/utils.go +++ b/chartvalidator/checker/utils.go @@ -110,3 +110,27 @@ func findYAMLFiles(dir string) ([]string, error) { return strings.HasSuffix(name, ".yaml") || strings.HasSuffix(name, ".yml") }) } + +// normalizeRepoURL canonicalises an ApplicationSet `repoURL` for use with +// `helm template --repo`. +// +// ArgoCD's ApplicationSet multi-source supports OCI registries with the +// scheme omitted (e.g. `europe-west4-docker.pkg.dev/.../charts`), and some +// consumers rely on that form. The helm CLI, however, requires an explicit +// `oci://` scheme for OCI registries — otherwise it fails with +// "could not find protocol handler for: ". +// +// Heuristic: if the URL has no scheme (no `://`), treat it as OCI and +// prepend `oci://`. URLs that are clearly HTTP chart repos already carry +// `http://` or `https://`, and OCI URLs that already carry `oci://` are +// left untouched. +func normalizeRepoURL(repoURL string) string { + trimmed := strings.TrimSpace(repoURL) + if trimmed == "" { + return trimmed + } + if strings.Contains(trimmed, "://") { + return trimmed + } + return "oci://" + trimmed +} diff --git a/chartvalidator/checker/utils_test.go b/chartvalidator/checker/utils_test.go index ab403c0..35ddbd7 100644 --- a/chartvalidator/checker/utils_test.go +++ b/chartvalidator/checker/utils_test.go @@ -158,4 +158,25 @@ func createManifestValidationMockExecutor() *MockCommandExecutor { Output: []byte("mocked kubeconform output"), Error: nil, } -} \ No newline at end of file +} +func TestNormalizeRepoURL(t *testing.T) { + cases := []struct { + name string + in string + want string + }{ + {"empty", "", ""}, + {"whitespace", " ", ""}, + {"https passthrough", "https://example.com/charts", "https://example.com/charts"}, + {"http passthrough", "http://example.com/charts", "http://example.com/charts"}, + {"oci passthrough", "oci://europe-west4-docker.pkg.dev/p/r", "oci://europe-west4-docker.pkg.dev/p/r"}, + {"gar no scheme", "europe-west4-docker.pkg.dev/p/r", "oci://europe-west4-docker.pkg.dev/p/r"}, + {"ghcr no scheme", "ghcr.io/org/charts", "oci://ghcr.io/org/charts"}, + {"trims whitespace", " ghcr.io/org/charts ", "oci://ghcr.io/org/charts"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.want, normalizeRepoURL(tc.in)) + }) + } +}