From b4666d35f6df5c327ae4344c8b4809723a0a235d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=A4=D0=B5=D0=B4=D0=BE=D1=80=20=D0=91=D0=B0=D1=82=D0=BE?= =?UTF-8?q?=D0=BD=D0=BE=D0=B3=D0=BE=D0=B2?= Date: Fri, 16 Oct 2026 00:09:21 +0300 Subject: [PATCH 1/3] feat(charts): registryMirrors gains overridePath and skipFallback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The registryMirrors values knob could only carry endpoint URL strings, so a Harbor-style pull-through cache — which puts the registry name in the pull path and already includes /v2 in the endpoint URL — was not expressible: containerd appends /v2 again unless overridePath is set, and the cache answers with its HTML portal (pulls fail with "unexpected media type text/html"). Talos has supported the setting since v1.3 in the legacy machine.registries.mirrors schema and since v1.12 in the RegistryMirrorConfig document; talm's chart simply never emitted it. Each mirror now accepts optional per-mirror keys: registryMirrors: docker.io: endpoints: [https://mirror.example.com/v2/proxy-docker.io] overridePath: true skipFallback: true Per-mirror keys were chosen over map-form endpoints (endpoints: [{url, overridePath}]) because the legacy schema carries both settings only at mirror level — a per-endpoint values shape could not render there, and the knob is meant to keep one values file portable across both schemas. On the multidoc schema overridePath is stamped onto every endpoint of the mirror (RegistryMirrorConfig has no mirror-level form); all endpoints of one mirror share a path layout in practice. skipFallback is mirror-level in both schemas. Guards, in the existing talm.guard.registries style: - overridePath / skipFallback must be bools when set; - mapping endpoints are refused with a message naming the per-mirror shape (previously they hit the confusing "has no scheme" message); - skipFallback on a pinned pre-v1.9 contract fails fast (the key is unknown to older Talos and the strict node-side decode rejects the whole config). overridePath needs no such guard: present in the legacy schema since v1.3. Chart-only fix: the vendored machinery (v1.14) strict-decodes both fields in both schemas; no engine or vendored-machinery changes needed. When neither key is set the rendered output is byte-identical to before, so existing values files and golden renders are unaffected. Assisted-by: LLM Signed-off-by: Федор Батоногов --- charts/cozystack/values.yaml | 23 +++- charts/generic/values.yaml | 10 +- charts/talm/templates/_helpers.tpl | 38 +++++- docs/manual-test-plan.md | 15 +++ pkg/engine/contract_machine_test.go | 185 ++++++++++++++++++++++++++++ 5 files changed, 265 insertions(+), 6 deletions(-) diff --git a/charts/cozystack/values.yaml b/charts/cozystack/values.yaml index 361650ce..ed71b719 100644 --- a/charts/cozystack/values.yaml +++ b/charts/cozystack/values.yaml @@ -245,10 +245,27 @@ extraSchedulerArgs: {} # (e.g. ghcr.io) or point at a private pull-through cache. To emit NO # registry config at all, null the key out (`registryMirrors:` with no # value) — a per-node override merges deeply, so `{}` keeps the default -# docker.io mirror rather than clearing it. Example: +# docker.io mirror rather than clearing it. +# +# Each mirror also accepts two optional per-mirror settings: +# overridePath — use the endpoint URL exactly as written, without +# appending /v2 to its path. REQUIRED for pull-through caches that +# carry the registry name in the path and already include /v2 in the +# URL (Harbor proxy cache, e.g. +# https://mirror.example.com/v2/proxy-docker.io): without +# overridePath containerd appends /v2 again and the cache answers +# with its HTML portal — pulls fail with "unexpected media type +# text/html". On the multidoc schema it is stamped onto every +# endpoint of the mirror (the typed document has no mirror-level +# form); on the legacy schema it is a sibling of endpoints. +# skipFallback — do not fall back to the upstream registry when no +# mirror serves the image. Requires Talos v1.9+; on an older pinned +# talosVersion contract the render fails fast (an UNPINNED contract +# is trusted as current and skips that guard). +# Example: # registryMirrors: -# docker.io: { endpoints: [https://registry.example.com/v2/docker.io] } -# ghcr.io: { endpoints: [https://registry.example.com/v2/ghcr.io] } +# docker.io: { endpoints: [https://mirror.example.com/v2/proxy-docker.io], overridePath: true } +# ghcr.io: { endpoints: [https://mirror.example.com/v2/proxy-ghcr.io], overridePath: true, skipFallback: true } registryMirrors: docker.io: endpoints: diff --git a/charts/generic/values.yaml b/charts/generic/values.yaml index 12bd405f..6e189d7a 100644 --- a/charts/generic/values.yaml +++ b/charts/generic/values.yaml @@ -142,9 +142,15 @@ extraSchedulerArgs: {} # Container registry mirrors, keyed by registry host. The generic preset # ships no default (cozystack mirrors docker.io through gcr); this key -# exists so a values file stays portable between the two presets. Example: +# exists so a values file stays portable between the two presets. Each +# mirror also accepts optional per-mirror `overridePath` (use the +# endpoint URL as written, without appending /v2 — required by Harbor +# proxy-cache endpoints such as +# https://registry.example.com/v2/proxy-docker.io) and `skipFallback` +# (never fall back to the upstream registry; Talos v1.9+ — the fail-fast +# guard only applies when templateOptions.talosVersion pins a contract). Example: # registryMirrors: -# docker.io: { endpoints: [https://registry.example.com/v2/docker.io] } +# docker.io: { endpoints: [https://registry.example.com/v2/proxy-docker.io], overridePath: true } registryMirrors: {} # TLS posture per registry ENDPOINT host — the hostname of the endpoint diff --git a/charts/talm/templates/_helpers.tpl b/charts/talm/templates/_helpers.tpl index 59f0122a..11a38442 100644 --- a/charts/talm/templates/_helpers.tpl +++ b/charts/talm/templates/_helpers.tpl @@ -310,7 +310,8 @@ busPath: {{ .spec.busPath }} route, matching what the discovery path emits. */ -}} {{- /* Shared registry knobs, used by every preset so a values file stays portable between them. registryMirrors is keyed by registry host and - maps to endpoint URLs; registryTLS is keyed by the ENDPOINT host and + maps to endpoint URLs plus the optional per-mirror overridePath / + skipFallback settings; registryTLS is keyed by the ENDPOINT host and carries the TLS posture for it. Guards here rather than per preset, so the two copies cannot drift. */ -}} {{- define "talm.guard.registries" }} @@ -325,10 +326,26 @@ busPath: {{ .spec.busPath }} {{- fail (printf "talm: registryMirrors.%s endpoints must be a list (got %s) — a bare URL is the missing-dash typo. Example: %q: { endpoints: [https://mirror.example.com] }." $name (kindOf $cfg.endpoints) $name) }} {{- end }} {{- range $cfg.endpoints }} +{{- if kindIs "map" . }} +{{- fail (printf "talm: registryMirrors.%s endpoints entries are plain URL strings, not mappings (got %s). Per-mirror settings go next to endpoints: %q: { endpoints: [https://mirror.example.com], overridePath: true }." $name (kindOf .) $name) }} +{{- end }} {{- if not (or (hasPrefix "http://" (. | toString)) (hasPrefix "https://" (. | toString))) }} {{- fail (printf "talm: registryMirrors.%s endpoint %q has no scheme. Talos rejects it with \"unsupported scheme\"; write the full URL, e.g. https://%s." $name (. | toString) (. | toString)) }} {{- end }} {{- end }} +{{- if and (not (kindIs "invalid" $cfg.overridePath)) (not (kindIs "bool" $cfg.overridePath)) }} +{{- fail (printf "talm: registryMirrors.%s overridePath must be true or false (got %s). It tells containerd not to append /v2 to the endpoint path — required by pull-through caches that put the registry name in the path (Harbor proxy cache, e.g. https://mirror.example.com/v2/proxy-docker.io)." $name (kindOf $cfg.overridePath)) }} +{{- end }} +{{- if and (not (kindIs "invalid" $cfg.skipFallback)) (not (kindIs "bool" $cfg.skipFallback)) }} +{{- fail (printf "talm: registryMirrors.%s skipFallback must be true or false (got %s). It decides whether a pull falls back to the upstream registry when no mirror serves it, so an ambiguous YAML scalar is refused." $name (kindOf $cfg.skipFallback)) }} +{{- end }} +{{- /* skipFallback is an unknown key to Talos older than v1.9, and the + strict node-side decode rejects the whole config for it. overridePath + needs no such guard: it exists in the legacy schema since v1.3 and in + RegistryMirrorConfig since the document's introduction (v1.12). */}} +{{- if and (not (kindIs "invalid" $cfg.skipFallback)) $.TalosVersion (semverCompare "<1.9.0-0" $.TalosVersion) }} +{{- fail (printf "talm: registryMirrors.%s sets skipFallback, which Talos only understands from v1.9 on; templateOptions.talosVersion pins %s. Drop the key or raise the contract." $name $.TalosVersion) }} +{{- end }} {{- end }} {{- range $host, $cfg := .Values.registryTLS }} {{- if eq ($host | toString) "*" }} @@ -399,6 +416,17 @@ name: {{ $name | toYaml }} endpoints: {{- range $cfg.endpoints }} - url: {{ . }} +{{- /* RegistryMirrorConfig carries overridePath per endpoint; the values + knob is per-mirror (mirroring the legacy schema, which has no + per-endpoint form), so it is stamped onto every endpoint of the + mirror. All endpoints of one mirror share the same path layout in + practice, so the granularity loss is not observable. */}} +{{- if not (kindIs "invalid" $cfg.overridePath) }} + overridePath: {{ $cfg.overridePath }} +{{- end }} +{{- end }} +{{- if not (kindIs "invalid" $cfg.skipFallback) }} +skipFallback: {{ $cfg.skipFallback }} {{- end }} {{- end }} {{- range $host, $cfg := .Values.registryTLS }} @@ -437,6 +465,14 @@ insecureSkipVerify: {{ $cfg.insecureSkipVerify }} {{- range $cfg.endpoints }} - {{ . }} {{- end }} +{{- /* The legacy schema carries both settings as per-mirror siblings of + endpoints — the exact shape the values knob mirrors. */}} +{{- if not (kindIs "invalid" $cfg.overridePath) }} + overridePath: {{ $cfg.overridePath }} +{{- end }} +{{- if not (kindIs "invalid" $cfg.skipFallback) }} + skipFallback: {{ $cfg.skipFallback }} +{{- end }} {{- end }} {{- end }} {{- with .Values.registryTLS }} diff --git a/docs/manual-test-plan.md b/docs/manual-test-plan.md index 0ced2721..94366f5e 100644 --- a/docs/manual-test-plan.md +++ b/docs/manual-test-plan.md @@ -562,6 +562,21 @@ Expected (multidoc): one `RegistryMirrorConfig` document per host. The legacy sc Regression anchor: `TestContract_Machine_RegistryMirrors_CustomMirror_Cozystack`, `TestContract_Machine_Registries_DockerMirror_LegacyCozystack` (default), `TestContract_Machine_NoRegistriesOnGeneric`. +A Harbor-style pull-through cache puts the registry name in the pull path and already carries `/v2` in the endpoint URL, so containerd must not append `/v2` again — that is the per-mirror `overridePath` setting (Talos documents it in the pull-through-cache guide). `skipFallback` pins "never fall back to the upstream registry": + +```bash +# values.yaml: +# registryMirrors: +# docker.io: { endpoints: [https://mirror.example.com/v2/proxy-docker.io], overridePath: true, skipFallback: true } +talm template -f nodes/controlplane-0.yaml | yq 'select(.kind == "RegistryMirrorConfig")' +# legacy schema: re-render with templateOptions.talosVersion pinned to v1.11 +talm template -f nodes/controlplane-0.yaml | yq '.machine.registries.mirrors' +``` + +Expected (multidoc): every endpoint of the mirror carries `overridePath: true`, and the document carries `skipFallback: true`. Expected (legacy): both keys sit as siblings of `endpoints` under the mirror name. With neither key set the output is byte-identical to before the knob existed. A non-bool value, or `skipFallback` on a pinned pre-v1.9 contract (the key does not exist there), fails the render fast. A mapping endpoint (`{url: ..., overridePath: ...}` — the shape the DOCUMENT uses) is refused with a message pointing at the per-mirror keys, because the legacy schema has no per-endpoint form to render it into. + +Regression anchor: `TestContract_Machine_RegistryMirrors_OverridePath_Multidoc_Cozystack`, `TestContract_Machine_RegistryMirrors_OverridePath_Legacy_Cozystack`, `TestContract_Machine_RegistryMirrors_OverridePath_Unset_Omitted_Cozystack`, `TestContract_Machine_RegistryMirrors_OverridePath_FalseIsEmitted_Cozystack`, `TestContract_Machine_RegistryMirrors_NonBoolOverridePath_Fails_Cozystack`, `TestContract_Machine_RegistryMirrors_NonBoolSkipFallback_Fails_Cozystack`, `TestContract_Machine_RegistryMirrors_MapEndpoint_Fails_Cozystack`, `TestContract_Machine_RegistryMirrors_SkipFallback_Pre19Contract_Fails_Cozystack`. + A self-signed pull-through mirror also needs its TLS posture stated. `registryTLS` is keyed by the endpoint HOST, not the mirror name: ```bash diff --git a/pkg/engine/contract_machine_test.go b/pkg/engine/contract_machine_test.go index 67b32d0d..ed142653 100644 --- a/pkg/engine/contract_machine_test.go +++ b/pkg/engine/contract_machine_test.go @@ -1288,6 +1288,191 @@ func TestContract_Machine_RegistryMirrors_FallbackKey_Legacy_Cozystack(t *testin } } +// harborStyleMirror returns the pull-through-cache shape that needs +// overridePath: the endpoint URL already carries /v2 and the registry +// name in its path, so containerd must not append /v2 again. +func harborStyleMirror() map[string]any { + return map[string]any{ + "docker.io": map[string]any{ + "endpoints": []any{"https://mirror.example.com/v2/proxy-docker.io"}, + "overridePath": true, + "skipFallback": true, + }, + } +} + +// Contract: a per-mirror overridePath renders onto every endpoint of the +// multidoc RegistryMirrorConfig (the typed document has no mirror-level +// form), and skipFallback lands at mirror level. This is the Harbor +// proxy-cache shape from the Talos pull-through-cache guide. +func TestContract_Machine_RegistryMirrors_OverridePath_Multidoc_Cozystack(t *testing.T) { + out := renderCozystackWith(t, simpleNicLookup(), map[string]any{ + "advertisedSubnets": []any{testAdvertisedSubnet}, + "registryMirrors": harborStyleMirror(), + }) + assertContains(t, out, "kind: RegistryMirrorConfig") + assertContains(t, out, "- url: https://mirror.example.com/v2/proxy-docker.io\n overridePath: true") + assertContains(t, out, "skipFallback: true") + + // Pin decode validity, not just substrings: the rendered + // RegistryMirrorConfig document must load under machinery's strict + // decoder, or the node would reject it. (The full render is a patch + // stream with discovery placeholders, so only the typed document is + // fed to the loader.) + for _, doc := range strings.Split(out, "\n---\n") { + if !strings.Contains(doc, "kind: RegistryMirrorConfig") { + continue + } + if _, err := configloader.NewFromBytes([]byte(doc)); err != nil { + t.Fatalf("rendered RegistryMirrorConfig with overridePath must decode, got %v in:\n%s", err, doc) + } + } +} + +// Contract: on the legacy schema the same values land as per-mirror +// siblings of endpoints — machine.registries.mirrors..overridePath, +// the only shape the legacy schema has for them. +func TestContract_Machine_RegistryMirrors_OverridePath_Legacy_Cozystack(t *testing.T) { + out := renderLegacyChart(t, cozystackChartPath, "cozystack/templates/controlplane.yaml", simpleNicLookup(), map[string]any{ + "advertisedSubnets": []any{testAdvertisedSubnet}, + "registryMirrors": harborStyleMirror(), + }) + assertContains(t, out, "docker.io:") + assertContains(t, out, "- https://mirror.example.com/v2/proxy-docker.io") + assertContains(t, out, "overridePath: true") + assertContains(t, out, "skipFallback: true") +} + +// Contract: with neither key set the rendered output carries neither — +// existing values files render byte-identical output to before the knob +// existed. +func TestContract_Machine_RegistryMirrors_OverridePath_Unset_Omitted_Cozystack(t *testing.T) { + t.Run("multidoc", func(t *testing.T) { + out := renderCozystackWith(t, simpleNicLookup(), map[string]any{ + "advertisedSubnets": []any{testAdvertisedSubnet}, + "registryMirrors": map[string]any{ + "docker.io": map[string]any{"endpoints": []any{"https://mirror.gcr.io"}}, + }, + }) + assertNotContains(t, out, "overridePath") + assertNotContains(t, out, "skipFallback") + }) + t.Run("legacy", func(t *testing.T) { + out := renderLegacyChart(t, cozystackChartPath, "cozystack/templates/controlplane.yaml", simpleNicLookup(), map[string]any{ + "advertisedSubnets": []any{testAdvertisedSubnet}, + "registryMirrors": map[string]any{ + "docker.io": map[string]any{"endpoints": []any{"https://mirror.gcr.io"}}, + }, + }) + assertNotContains(t, out, "overridePath") + assertNotContains(t, out, "skipFallback") + }) +} + +// Contract: false is a stated choice and is emitted verbatim, matching the +// registryTLS.insecureSkipVerify behaviour — the knob can pin the default +// explicitly against a future chart default change. +func TestContract_Machine_RegistryMirrors_OverridePath_FalseIsEmitted_Cozystack(t *testing.T) { + out := renderCozystackWith(t, simpleNicLookup(), map[string]any{ + "advertisedSubnets": []any{testAdvertisedSubnet}, + "registryMirrors": map[string]any{ + "docker.io": map[string]any{ + "endpoints": []any{"https://mirror.example.com/v2/proxy-docker.io"}, + "overridePath": false, + }, + }, + }) + assertContains(t, out, "overridePath: false") +} + +// Contract: a non-bool overridePath (e.g. the YAML-ambiguous "yes") fails +// at render rather than emitting a scalar Talos rejects on decode. +func TestContract_Machine_RegistryMirrors_NonBoolOverridePath_Fails_Cozystack(t *testing.T) { + err := renderCozystackExpectError(t, simpleNicLookup(), map[string]any{ + "advertisedSubnets": []any{testAdvertisedSubnet}, + "registryMirrors": map[string]any{ + "docker.io": map[string]any{ + "endpoints": []any{"https://mirror.example.com/v2/proxy-docker.io"}, + "overridePath": "yes", + }, + }, + }) + if err == nil { + t.Fatal("expected a fail-fast for a non-bool overridePath") + } + if !strings.Contains(err.Error(), "overridePath") { + t.Errorf("error should name the offending key, got %v", err) + } +} + +// Contract: same fail-fast for a non-bool skipFallback — the guard is +// structurally identical to the overridePath one. +func TestContract_Machine_RegistryMirrors_NonBoolSkipFallback_Fails_Cozystack(t *testing.T) { + err := renderCozystackExpectError(t, simpleNicLookup(), map[string]any{ + "advertisedSubnets": []any{testAdvertisedSubnet}, + "registryMirrors": map[string]any{ + "docker.io": map[string]any{ + "endpoints": []any{"https://mirror.example.com/v2/proxy-docker.io"}, + "skipFallback": "yes", + }, + }, + }) + if err == nil { + t.Fatal("expected a fail-fast for a non-bool skipFallback") + } + if !strings.Contains(err.Error(), "skipFallback") { + t.Errorf("error should name the offending key, got %v", err) + } +} + +// Contract: a mapping endpoint ({url: ..., overridePath: ...}) is the +// shape the multidoc DOCUMENT uses, but the values knob keeps endpoints +// as plain strings with per-mirror settings next to them — the legacy +// schema has no per-endpoint form, so a per-endpoint values shape could +// not render there. The guard names the right shape instead of the +// confusing scheme message a map used to produce. +func TestContract_Machine_RegistryMirrors_MapEndpoint_Fails_Cozystack(t *testing.T) { + err := renderCozystackExpectError(t, simpleNicLookup(), map[string]any{ + "advertisedSubnets": []any{testAdvertisedSubnet}, + "registryMirrors": map[string]any{ + "docker.io": map[string]any{ + "endpoints": []any{map[string]any{"url": "https://mirror.example.com/v2/proxy-docker.io", "overridePath": true}}, + }, + }, + }) + if err == nil { + t.Fatal("expected a fail-fast for a mapping endpoint") + } + if !strings.Contains(err.Error(), "plain URL strings") { + t.Errorf("error should point at the per-mirror shape, got %v", err) + } +} + +// Contract: skipFallback is an unknown key to Talos older than v1.9, and +// the strict node-side decode would reject the whole config for it, so +// the render fails fast on a pinned pre-1.9 contract. overridePath needs +// no such guard — it exists in the legacy schema since v1.3. +func TestContract_Machine_RegistryMirrors_SkipFallback_Pre19Contract_Fails_Cozystack(t *testing.T) { + err := renderCozystackExpectError(t, simpleNicLookup(), map[string]any{ + "advertisedSubnets": []any{testAdvertisedSubnet}, + "registryMirrors": map[string]any{ + "docker.io": map[string]any{ + "endpoints": []any{"https://mirror.example.com/v2/proxy-docker.io"}, + "skipFallback": true, + }, + }, + }, "v1.8") + if err == nil { + t.Fatal("expected a fail-fast for skipFallback on a pre-1.9 contract") + } + if !strings.Contains(err.Error(), "skipFallback") { + t.Errorf("error should name the offending key, got %v", err) + } + if !strings.Contains(err.Error(), "v1.9") { + t.Errorf("error should name the version that introduced the key, got %v", err) + } +} + // Contract: Talos states the fallback key cannot be used for TLS config, // so the chart refuses it instead of emitting a document Talos ignores. func TestContract_Machine_RegistryTLS_FallbackKey_Fails_Cozystack(t *testing.T) { From 4d050b1824eb53baaf12d772cd65897a6099f562 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=A4=D0=B5=D0=B4=D0=BE=D1=80=20=D0=91=D0=B0=D1=82=D0=BE?= =?UTF-8?q?=D0=BD=D0=BE=D0=B3=D0=BE=D0=B2?= Date: Fri, 23 Oct 2026 05:14:37 +0300 Subject: [PATCH 2/3] test(charts): harden registryMirrors contract tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The guard and render shape were pinned only on the rejecting side and by substring, so template regressions kept the suite green: - exercise the skipFallback version guard on the passing side too: v1.8 with default mirrors and v1.9/v1.11 with skipFallback must render, so a widened (<1.10.0-0) or dropped guard condition fails; - parse the legacy render instead of substring checks: overridePath or skipFallback at a wrong indent now fails as a YAML shape error, and both keys are asserted next to endpoints; - give the Harbor-style fixture a second endpoint and assert overridePath on every parsed endpoint, catching a first-endpoint-only stamping regression; - pin skipFallback: false emission on both schemas, matching the overridePath stated-choice contract, so a truthiness-gated emission fails. Each mutation above was verified to fail the new assertions. Assisted-by: LLM Signed-off-by: Федор Батоногов --- pkg/engine/contract_machine_test.go | 211 ++++++++++++++++++++++++++-- pkg/engine/render_test.go | 12 +- 2 files changed, 207 insertions(+), 16 deletions(-) diff --git a/pkg/engine/contract_machine_test.go b/pkg/engine/contract_machine_test.go index ed142653..b8975722 100644 --- a/pkg/engine/contract_machine_test.go +++ b/pkg/engine/contract_machine_test.go @@ -31,6 +31,7 @@ import ( "encoding/base64" "os" "path/filepath" + "slices" "strings" "testing" @@ -1290,17 +1291,68 @@ func TestContract_Machine_RegistryMirrors_FallbackKey_Legacy_Cozystack(t *testin // harborStyleMirror returns the pull-through-cache shape that needs // overridePath: the endpoint URL already carries /v2 and the registry -// name in its path, so containerd must not append /v2 again. +// name in its path, so containerd must not append /v2 again. Two +// endpoints on purpose: a regression that stamps the per-mirror +// overridePath onto the first endpoint only must fail the contract +// tests, so the fixture always carries a second endpoint to omit. func harborStyleMirror() map[string]any { return map[string]any{ "docker.io": map[string]any{ - "endpoints": []any{"https://mirror.example.com/v2/proxy-docker.io"}, + "endpoints": []any{ + "https://mirror.example.com/v2/proxy-docker.io", + "https://mirror2.example.com/v2/proxy-docker.io", + }, "overridePath": true, "skipFallback": true, }, } } +// machineDoc splits a rendered patch stream into documents and returns +// the one carrying the legacy machine: section. yaml.Unmarshal alone +// would read only the first document, silently ignoring a render whose +// machine block moved or vanished. +func machineDoc(t *testing.T, out string) map[string]any { + t.Helper() + for _, doc := range strings.Split(out, "\n---\n") { + var v map[string]any + if err := yaml.Unmarshal([]byte(doc), &v); err != nil { + t.Fatalf("rendered document must be valid YAML, got %v in:\n%s", err, doc) + } + if _, ok := v["machine"]; ok { + return v + } + } + t.Fatalf("render must contain a document with a machine: section:\n%s", out) + return nil +} + +// nestedMap walks a decoded YAML mapping along keys, reporting whether +// every hop exists and the final value is itself a mapping. +func nestedMap(doc map[string]any, keys ...string) (map[string]any, bool) { + cur := doc + for _, k := range keys { + next, ok := cur[k].(map[string]any) + if !ok { + return nil, false + } + cur = next + } + return cur, true +} + +// registryMirrorConfigView is the generic decode target for a rendered +// RegistryMirrorConfig document: typed enough to assert where keys sit +// (a wrong indent moves overridePath out of the endpoint entry), without +// depending on machinery internals the strict loader already covers. +type registryMirrorConfigView struct { + Endpoints []struct { + URL string `yaml:"url"` + OverridePath *bool `yaml:"overridePath"` + } `yaml:"endpoints"` + SkipFallback *bool `yaml:"skipFallback"` +} + // Contract: a per-mirror overridePath renders onto every endpoint of the // multidoc RegistryMirrorConfig (the typed document has no mirror-level // form), and skipFallback lands at mirror level. This is the Harbor @@ -1311,22 +1363,47 @@ func TestContract_Machine_RegistryMirrors_OverridePath_Multidoc_Cozystack(t *tes "registryMirrors": harborStyleMirror(), }) assertContains(t, out, "kind: RegistryMirrorConfig") - assertContains(t, out, "- url: https://mirror.example.com/v2/proxy-docker.io\n overridePath: true") - assertContains(t, out, "skipFallback: true") - - // Pin decode validity, not just substrings: the rendered - // RegistryMirrorConfig document must load under machinery's strict - // decoder, or the node would reject it. (The full render is a patch - // stream with discovery placeholders, so only the typed document is - // fed to the loader.) + + // Pin the shape by parsing, not by substring: overridePath stamped + // on the first endpoint only, or at an indent that lifts it out of + // the endpoint entry, must fail here. + var parsed *registryMirrorConfigView for _, doc := range strings.Split(out, "\n---\n") { if !strings.Contains(doc, "kind: RegistryMirrorConfig") { continue } + var v registryMirrorConfigView + if err := yaml.Unmarshal([]byte(doc), &v); err != nil { + t.Fatalf("rendered RegistryMirrorConfig must be valid YAML, got %v in:\n%s", err, doc) + } + if parsed != nil { + t.Fatal("expected exactly one RegistryMirrorConfig document — asserting only the last one would silently weaken the test") + } + parsed = &v + + // Pin decode validity too: the rendered RegistryMirrorConfig + // document must load under machinery's strict decoder, or the + // node would reject it. (The full render is a patch stream with + // discovery placeholders, so only the typed document is fed to + // the loader.) if _, err := configloader.NewFromBytes([]byte(doc)); err != nil { t.Fatalf("rendered RegistryMirrorConfig with overridePath must decode, got %v in:\n%s", err, doc) } } + if parsed == nil { + t.Fatal("render must contain a RegistryMirrorConfig document") + } + if len(parsed.Endpoints) != 2 { + t.Fatalf("fixture carries two endpoints, got %d — the test stops detecting a first-endpoint-only overridePath regression without the second", len(parsed.Endpoints)) + } + for i, ep := range parsed.Endpoints { + if ep.OverridePath == nil || !*ep.OverridePath { + t.Errorf("endpoint %d (%s) must carry overridePath: true stamped from the per-mirror key", i, ep.URL) + } + } + if parsed.SkipFallback == nil || !*parsed.SkipFallback { + t.Error("skipFallback: true must sit at mirror level") + } } // Contract: on the legacy schema the same values land as per-mirror @@ -1337,10 +1414,42 @@ func TestContract_Machine_RegistryMirrors_OverridePath_Legacy_Cozystack(t *testi "advertisedSubnets": []any{testAdvertisedSubnet}, "registryMirrors": harborStyleMirror(), }) - assertContains(t, out, "docker.io:") - assertContains(t, out, "- https://mirror.example.com/v2/proxy-docker.io") - assertContains(t, out, "overridePath: true") - assertContains(t, out, "skipFallback: true") + + // Pin the shape by parsing, not by substring: a wrong indent turns + // the keys into a YAML error or a mirror named "skipFallback" — + // either way the navigation below fails, where substring checks + // stayed green. + doc := machineDoc(t, out) + mirror, ok := nestedMap(doc, "machine", "registries", "mirrors", "docker.io") + if !ok { + t.Fatalf("machine.registries.mirrors[\"docker.io\"] must be a mapping in:\n%s", out) + } + endpoints, ok := mirror["endpoints"].([]any) + if !ok || len(endpoints) != 2 { + t.Fatalf("mirror must list both fixture endpoints, got %v", mirror["endpoints"]) + } + got := make([]string, 0, len(endpoints)) + for _, e := range endpoints { + s, ok := e.(string) + if !ok { + t.Fatalf("endpoint entries must be plain URL strings, got %v", e) + } + got = append(got, s) + } + for _, want := range []string{ + "https://mirror.example.com/v2/proxy-docker.io", + "https://mirror2.example.com/v2/proxy-docker.io", + } { + if !slices.Contains(got, want) { + t.Errorf("endpoints must contain %s, got %v", want, got) + } + } + if v, ok := mirror["overridePath"].(bool); !ok || !v { + t.Errorf("overridePath: true must sit next to endpoints, got %v", mirror["overridePath"]) + } + if v, ok := mirror["skipFallback"].(bool); !ok || !v { + t.Errorf("skipFallback: true must sit next to endpoints, got %v", mirror["skipFallback"]) + } } // Contract: with neither key set the rendered output carries neither — @@ -1385,6 +1494,40 @@ func TestContract_Machine_RegistryMirrors_OverridePath_FalseIsEmitted_Cozystack( assertContains(t, out, "overridePath: false") } +// Contract: same stated-choice emission for skipFallback — an explicit +// false must survive the render on both schemas, not be dropped by a +// truthiness check, so the knob can pin the default against a future +// chart default change. Mirrors the overridePath contract above. +func TestContract_Machine_RegistryMirrors_SkipFallback_FalseIsEmitted_Cozystack(t *testing.T) { + mirror := map[string]any{ + "docker.io": map[string]any{ + "endpoints": []any{"https://mirror.example.com/v2/proxy-docker.io"}, + "skipFallback": false, + }, + } + t.Run("multidoc", func(t *testing.T) { + out := renderCozystackWith(t, simpleNicLookup(), map[string]any{ + "advertisedSubnets": []any{testAdvertisedSubnet}, + "registryMirrors": mirror, + }) + assertContains(t, out, "skipFallback: false") + }) + t.Run("legacy", func(t *testing.T) { + out := renderLegacyChart(t, cozystackChartPath, "cozystack/templates/controlplane.yaml", simpleNicLookup(), map[string]any{ + "advertisedSubnets": []any{testAdvertisedSubnet}, + "registryMirrors": mirror, + }) + doc := machineDoc(t, out) + m, ok := nestedMap(doc, "machine", "registries", "mirrors", "docker.io") + if !ok { + t.Fatalf("machine.registries.mirrors[\"docker.io\"] must be a mapping in:\n%s", out) + } + if v, ok := m["skipFallback"].(bool); !ok || v { + t.Errorf("skipFallback: false must be emitted verbatim, got %v", m["skipFallback"]) + } + }) +} + // Contract: a non-bool overridePath (e.g. the YAML-ambiguous "yes") fails // at render rather than emitting a scalar Talos rejects on decode. func TestContract_Machine_RegistryMirrors_NonBoolOverridePath_Fails_Cozystack(t *testing.T) { @@ -1473,6 +1616,46 @@ func TestContract_Machine_RegistryMirrors_SkipFallback_Pre19Contract_Fails_Cozys } } +// Contract: the skipFallback version guard must also stay green on the +// passing side, or a widened/dropped condition (<1.10.0-0, or dropping +// the skipFallback-is-set term) would ship silently: v1.8 renders with +// default mirrors, and v1.9/v1.11 render with skipFallback set. These +// versions take the legacy schema path, so the guard fires against +// $.TalosVersion there, not on the multidoc documents. +func TestContract_Machine_RegistryMirrors_SkipFallback_GuardPassingSide_Cozystack(t *testing.T) { + t.Run("v1.8 default mirrors", func(t *testing.T) { + out := renderCozystackWith(t, simpleNicLookup(), map[string]any{ + "advertisedSubnets": []any{testAdvertisedSubnet}, + "registryMirrors": map[string]any{ + "docker.io": map[string]any{"endpoints": []any{"https://mirror.gcr.io"}}, + }, + }, "v1.8") + assertContains(t, out, "- https://mirror.gcr.io") + assertNotContains(t, out, "skipFallback") + }) + for _, version := range []string{"v1.9", "v1.11"} { + t.Run(version+" with skipFallback", func(t *testing.T) { + out := renderCozystackWith(t, simpleNicLookup(), map[string]any{ + "advertisedSubnets": []any{testAdvertisedSubnet}, + "registryMirrors": map[string]any{ + "docker.io": map[string]any{ + "endpoints": []any{"https://mirror.example.com/v2/proxy-docker.io"}, + "skipFallback": true, + }, + }, + }, version) + doc := machineDoc(t, out) + mirror, ok := nestedMap(doc, "machine", "registries", "mirrors", "docker.io") + if !ok { + t.Fatalf("machine.registries.mirrors[\"docker.io\"] must be a mapping in:\n%s", out) + } + if v, ok := mirror["skipFallback"].(bool); !ok || !v { + t.Errorf("skipFallback: true must render on a %s contract, got %v", version, mirror["skipFallback"]) + } + }) + } +} + // Contract: Talos states the fallback key cannot be used for TLS config, // so the chart refuses it instead of emitting a document Talos ignores. func TestContract_Machine_RegistryTLS_FallbackKey_Fails_Cozystack(t *testing.T) { diff --git a/pkg/engine/render_test.go b/pkg/engine/render_test.go index e19487a1..42e9c3f7 100644 --- a/pkg/engine/render_test.go +++ b/pkg/engine/render_test.go @@ -7004,7 +7004,10 @@ func freshNicLookup() func(string, string, string) (map[string]any, error) { // against the supplied LookupFunc and values overrides, returning the // final template output or failing the test. Mirrors the pattern used // by the existing TestMultiDoc* suites. -func renderCozystackWith(t *testing.T, lookup func(string, string, string) (map[string]any, error), overrides map[string]any) string { +// +// Variadic talosVersion: defaults to v1.12 (multi-doc path); pass +// "v1.11" or earlier to exercise the legacy schema path. +func renderCozystackWith(t *testing.T, lookup func(string, string, string) (map[string]any, error), overrides map[string]any, talosVersion ...string) string { t.Helper() origLookup := helmEngine.LookupFunc t.Cleanup(func() { helmEngine.LookupFunc = origLookup }) @@ -7025,10 +7028,15 @@ func renderCozystackWith(t *testing.T, lookup func(string, string, string) (map[ } maps.Copy(values, overrides) + version := "v1.12" + if len(talosVersion) > 0 && talosVersion[0] != "" { + version = talosVersion[0] + } + eng := helmEngine.Engine{} out, err := eng.Render(chrt, common.Values{ "Values": values, - "TalosVersion": "v1.12", + "TalosVersion": version, }) if err != nil { t.Fatalf("render: %v", err) From 4ecd4369746b0ef9321a8b3fb0eebe17a74069ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=A4=D0=B5=D0=B4=D0=BE=D1=80=20=D0=91=D0=B0=D1=82=D0=BE?= =?UTF-8?q?=D0=BD=D0=BE=D0=B3=D0=BE=D0=B2?= Date: Mon, 26 Oct 2026 09:10:09 +0300 Subject: [PATCH 3/3] fix(charts): guard overridePath before Talos v1.3 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Talos added the legacy registry mirror overridePath field in v1.3. Rendering it for an older pinned contract produces a config the node strict decoder rejects, while talm explicitly supports older --talos-version contracts. Fail fast below v1.3 and cover both sides of the boundary: v1.2 rejects the key but renders without it, while v1.3 and v1.11 render it. Update the preset values and manual test plan to state the version contract. Assisted-by: LLM Signed-off-by: Федор Батоногов --- charts/cozystack/values.yaml | 3 +- charts/generic/values.yaml | 6 +-- charts/talm/templates/_helpers.tpl | 10 +++-- docs/manual-test-plan.md | 4 +- pkg/engine/contract_machine_test.go | 64 ++++++++++++++++++++++++++++- 5 files changed, 75 insertions(+), 12 deletions(-) diff --git a/charts/cozystack/values.yaml b/charts/cozystack/values.yaml index ed71b719..1aed966b 100644 --- a/charts/cozystack/values.yaml +++ b/charts/cozystack/values.yaml @@ -257,7 +257,8 @@ extraSchedulerArgs: {} # with its HTML portal — pulls fail with "unexpected media type # text/html". On the multidoc schema it is stamped onto every # endpoint of the mirror (the typed document has no mirror-level -# form); on the legacy schema it is a sibling of endpoints. +# form); on the legacy schema it is a sibling of endpoints. Requires +# Talos v1.3+; an older pinned talosVersion contract fails fast. # skipFallback — do not fall back to the upstream registry when no # mirror serves the image. Requires Talos v1.9+; on an older pinned # talosVersion contract the render fails fast (an UNPINNED contract diff --git a/charts/generic/values.yaml b/charts/generic/values.yaml index 6e189d7a..14b8f4f0 100644 --- a/charts/generic/values.yaml +++ b/charts/generic/values.yaml @@ -146,9 +146,9 @@ extraSchedulerArgs: {} # mirror also accepts optional per-mirror `overridePath` (use the # endpoint URL as written, without appending /v2 — required by Harbor # proxy-cache endpoints such as -# https://registry.example.com/v2/proxy-docker.io) and `skipFallback` -# (never fall back to the upstream registry; Talos v1.9+ — the fail-fast -# guard only applies when templateOptions.talosVersion pins a contract). Example: +# https://registry.example.com/v2/proxy-docker.io; Talos v1.3+) and +# `skipFallback` (never fall back to the upstream registry; Talos v1.9+). +# Older pinned templateOptions.talosVersion contracts fail fast. Example: # registryMirrors: # docker.io: { endpoints: [https://registry.example.com/v2/proxy-docker.io], overridePath: true } registryMirrors: {} diff --git a/charts/talm/templates/_helpers.tpl b/charts/talm/templates/_helpers.tpl index 11a38442..17d2bda8 100644 --- a/charts/talm/templates/_helpers.tpl +++ b/charts/talm/templates/_helpers.tpl @@ -339,10 +339,12 @@ busPath: {{ .spec.busPath }} {{- if and (not (kindIs "invalid" $cfg.skipFallback)) (not (kindIs "bool" $cfg.skipFallback)) }} {{- fail (printf "talm: registryMirrors.%s skipFallback must be true or false (got %s). It decides whether a pull falls back to the upstream registry when no mirror serves it, so an ambiguous YAML scalar is refused." $name (kindOf $cfg.skipFallback)) }} {{- end }} -{{- /* skipFallback is an unknown key to Talos older than v1.9, and the - strict node-side decode rejects the whole config for it. overridePath - needs no such guard: it exists in the legacy schema since v1.3 and in - RegistryMirrorConfig since the document's introduction (v1.12). */}} +{{- /* overridePath and skipFallback are unknown keys to Talos older than + v1.3 and v1.9 respectively, and the strict node-side decode rejects + the whole config for them. */}} +{{- if and (not (kindIs "invalid" $cfg.overridePath)) $.TalosVersion (semverCompare "<1.3.0-0" $.TalosVersion) }} +{{- fail (printf "talm: registryMirrors.%s sets overridePath, which Talos only understands from v1.3 on; templateOptions.talosVersion pins %s. Drop the key or raise the contract." $name $.TalosVersion) }} +{{- end }} {{- if and (not (kindIs "invalid" $cfg.skipFallback)) $.TalosVersion (semverCompare "<1.9.0-0" $.TalosVersion) }} {{- fail (printf "talm: registryMirrors.%s sets skipFallback, which Talos only understands from v1.9 on; templateOptions.talosVersion pins %s. Drop the key or raise the contract." $name $.TalosVersion) }} {{- end }} diff --git a/docs/manual-test-plan.md b/docs/manual-test-plan.md index 94366f5e..0728dfbc 100644 --- a/docs/manual-test-plan.md +++ b/docs/manual-test-plan.md @@ -573,9 +573,9 @@ talm template -f nodes/controlplane-0.yaml | yq 'select(.kind == "RegistryMirror talm template -f nodes/controlplane-0.yaml | yq '.machine.registries.mirrors' ``` -Expected (multidoc): every endpoint of the mirror carries `overridePath: true`, and the document carries `skipFallback: true`. Expected (legacy): both keys sit as siblings of `endpoints` under the mirror name. With neither key set the output is byte-identical to before the knob existed. A non-bool value, or `skipFallback` on a pinned pre-v1.9 contract (the key does not exist there), fails the render fast. A mapping endpoint (`{url: ..., overridePath: ...}` — the shape the DOCUMENT uses) is refused with a message pointing at the per-mirror keys, because the legacy schema has no per-endpoint form to render it into. +Expected (multidoc): every endpoint of the mirror carries `overridePath: true`, and the document carries `skipFallback: true`. Expected (legacy): both keys sit as siblings of `endpoints` under the mirror name. With neither key set the output is byte-identical to before the knob existed. A non-bool value fails the render fast, as do `overridePath` on a pinned pre-v1.3 contract and `skipFallback` on a pinned pre-v1.9 contract (the keys do not exist there). A mapping endpoint (`{url: ..., overridePath: ...}` — the shape the DOCUMENT uses) is refused with a message pointing at the per-mirror keys, because the legacy schema has no per-endpoint form to render it into. -Regression anchor: `TestContract_Machine_RegistryMirrors_OverridePath_Multidoc_Cozystack`, `TestContract_Machine_RegistryMirrors_OverridePath_Legacy_Cozystack`, `TestContract_Machine_RegistryMirrors_OverridePath_Unset_Omitted_Cozystack`, `TestContract_Machine_RegistryMirrors_OverridePath_FalseIsEmitted_Cozystack`, `TestContract_Machine_RegistryMirrors_NonBoolOverridePath_Fails_Cozystack`, `TestContract_Machine_RegistryMirrors_NonBoolSkipFallback_Fails_Cozystack`, `TestContract_Machine_RegistryMirrors_MapEndpoint_Fails_Cozystack`, `TestContract_Machine_RegistryMirrors_SkipFallback_Pre19Contract_Fails_Cozystack`. +Regression anchor: `TestContract_Machine_RegistryMirrors_OverridePath_Multidoc_Cozystack`, `TestContract_Machine_RegistryMirrors_OverridePath_Legacy_Cozystack`, `TestContract_Machine_RegistryMirrors_OverridePath_Unset_Omitted_Cozystack`, `TestContract_Machine_RegistryMirrors_OverridePath_FalseIsEmitted_Cozystack`, `TestContract_Machine_RegistryMirrors_SkipFallback_FalseIsEmitted_Cozystack`, `TestContract_Machine_RegistryMirrors_NonBoolOverridePath_Fails_Cozystack`, `TestContract_Machine_RegistryMirrors_NonBoolSkipFallback_Fails_Cozystack`, `TestContract_Machine_RegistryMirrors_MapEndpoint_Fails_Cozystack`, `TestContract_Machine_RegistryMirrors_OverridePath_Pre13Contract_Fails_Cozystack`, `TestContract_Machine_RegistryMirrors_OverridePath_GuardPassingSide_Cozystack`, `TestContract_Machine_RegistryMirrors_SkipFallback_Pre19Contract_Fails_Cozystack`, `TestContract_Machine_RegistryMirrors_SkipFallback_GuardPassingSide_Cozystack`. A self-signed pull-through mirror also needs its TLS posture stated. `registryTLS` is keyed by the endpoint HOST, not the mirror name: diff --git a/pkg/engine/contract_machine_test.go b/pkg/engine/contract_machine_test.go index b8975722..582c384e 100644 --- a/pkg/engine/contract_machine_test.go +++ b/pkg/engine/contract_machine_test.go @@ -1591,10 +1591,70 @@ func TestContract_Machine_RegistryMirrors_MapEndpoint_Fails_Cozystack(t *testing } } +// Contract: overridePath is an unknown key to Talos older than v1.3, and +// the strict node-side decode would reject the whole config for it, so +// the render fails fast on a pinned pre-1.3 contract. +func TestContract_Machine_RegistryMirrors_OverridePath_Pre13Contract_Fails_Cozystack(t *testing.T) { + err := renderCozystackExpectError(t, simpleNicLookup(), map[string]any{ + "advertisedSubnets": []any{testAdvertisedSubnet}, + "registryMirrors": map[string]any{ + "docker.io": map[string]any{ + "endpoints": []any{"https://mirror.example.com/v2/proxy-docker.io"}, + "overridePath": true, + }, + }, + }, "v1.2") + if err == nil { + t.Fatal("expected a fail-fast for overridePath on a pre-1.3 contract") + } + if !strings.Contains(err.Error(), "overridePath") { + t.Errorf("error should name the offending key, got %v", err) + } + if !strings.Contains(err.Error(), "v1.3") { + t.Errorf("error should name the version that introduced the key, got %v", err) + } +} + +// Contract: the overridePath version guard must also stay green on the +// passing side, so widening its boundary or dropping the overridePath-is-set +// term cannot ship silently. These versions take the legacy schema path. +func TestContract_Machine_RegistryMirrors_OverridePath_GuardPassingSide_Cozystack(t *testing.T) { + t.Run("v1.2 default mirrors", func(t *testing.T) { + out := renderCozystackWith(t, simpleNicLookup(), map[string]any{ + "advertisedSubnets": []any{testAdvertisedSubnet}, + "registryMirrors": map[string]any{ + "docker.io": map[string]any{"endpoints": []any{"https://mirror.gcr.io"}}, + }, + }, "v1.2") + assertContains(t, out, "- https://mirror.gcr.io") + assertNotContains(t, out, "overridePath") + }) + for _, version := range []string{"v1.3", "v1.11"} { + t.Run(version+" with overridePath", func(t *testing.T) { + out := renderCozystackWith(t, simpleNicLookup(), map[string]any{ + "advertisedSubnets": []any{testAdvertisedSubnet}, + "registryMirrors": map[string]any{ + "docker.io": map[string]any{ + "endpoints": []any{"https://mirror.example.com/v2/proxy-docker.io"}, + "overridePath": true, + }, + }, + }, version) + doc := machineDoc(t, out) + mirror, ok := nestedMap(doc, "machine", "registries", "mirrors", "docker.io") + if !ok { + t.Fatalf("machine.registries.mirrors[\"docker.io\"] must be a mapping in:\n%s", out) + } + if v, ok := mirror["overridePath"].(bool); !ok || !v { + t.Errorf("overridePath: true must render on a %s contract, got %v", version, mirror["overridePath"]) + } + }) + } +} + // Contract: skipFallback is an unknown key to Talos older than v1.9, and // the strict node-side decode would reject the whole config for it, so -// the render fails fast on a pinned pre-1.9 contract. overridePath needs -// no such guard — it exists in the legacy schema since v1.3. +// the render fails fast on a pinned pre-1.9 contract. func TestContract_Machine_RegistryMirrors_SkipFallback_Pre19Contract_Fails_Cozystack(t *testing.T) { err := renderCozystackExpectError(t, simpleNicLookup(), map[string]any{ "advertisedSubnets": []any{testAdvertisedSubnet},