fix(kubernetes-nodes): scope the imageFactoryURL guard to the paths that read it

resolveOsImage assigns $factoryURL before it branches, and the builtin
arm never reassigns it, so the http(s) pattern ran for every pool
including one that clones. A builtin pool builds no source URL at all:
its DataVolume carries source.pvc and no source.http, and the reconcile
Job takes only the schematic and the version. The value was therefore
refused on a path that does not consume it.

The operator this landed on is the one the feature exists for. Somebody
moving to osImage.builtin to stop depending on the Image Factory is
exactly the person likely to blank imageFactoryURL, and the render then
failed with a message naming a source URL that path never builds. The
field is a bare string in values.schema.json and in the cozyrds
openAPISchema -- no pattern, no minLength -- so an empty or malformed
one reaches the render rather than being rejected at admission.

The guard is unchanged for osImage.factory and for the no-osImage
default, which are the two arms that interpolate it. Three cases: a
builtin pool renders with the field blanked and with it malformed, and
the omitted-osImage default still refuses a malformed one, so neither
new case can be read as relaxing the arm a running cluster is already
on. Forcing the new condition open reddens the two builtin cases;
forcing it shut reddens the factory and default ones.

Reviewed as [MINOR] on cozystack/cozystack#3294.

Assisted-By: LLM
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
This commit is contained in:
Myasnikov Daniil committed 2026-09-05 15:21:32 +05:00
1 parent 35d7a3a746
commit 61bbf86755
2 files changed
+63 -2

No files matched your search

@@ -267,8 +267,23 @@ dict the result is written into), groupName (named in every error message).
{{- if not (regexMatch "^v[0-9]+\\.[0-9]+\\.[0-9]+(-[0-9a-z.]+)?$" ($version | toString)) -}}
{{- fail (printf "nodeGroup %q: Talos version %q is not a vMAJOR.MINOR.PATCH release. It is interpolated into the worker DataVolume name and the image URL, so it must carry no whitespace or YAML metacharacters." .groupName ($version | toString)) -}}
{{- end -}}
{{- if not (regexMatch "^https?://[A-Za-z0-9._~:/?#\\[\\]@!&'()*+,;=%-]+$" ($factoryURL | toString)) -}}
{{- fail (printf "nodeGroup %q: imageFactoryURL %q is not a plain http(s) URL. It is interpolated into the worker DataVolume source URL, so it must carry no whitespace, backtick, dollar sign or YAML metacharacters." .groupName ($factoryURL | toString)) -}}
{{- /* Only the paths that build an HTTP source URL are held to this. A builtin
pool clones a golden PVC: its DataVolume carries source.pvc and no
source.http at all, so imageFactoryURL is never interpolated into
anything it renders. Validating it there refused a value the pool does
not consume, and the operator it refused is the one the feature is for --
somebody moving to osImage.builtin to stop depending on the Factory is
exactly the person likely to blank imageFactoryURL, and the message they
got named a source URL this path never builds. The field is a bare string
in values.schema.json and in the cozyrds openAPISchema, so an empty or
malformed one reaches the render rather than being rejected at admission.
Reviewed as [MINOR] on cozystack/cozystack#3294. The check itself stays
exactly as strict for osImage.factory and for the no-osImage default,
which are the two arms that do interpolate it. */ -}}
{{- if not $clone -}}
{{- if not (regexMatch "^https?://[A-Za-z0-9._~:/?#\\[\\]@!&'()*+,;=%-]+$" ($factoryURL | toString)) -}}
{{- fail (printf "nodeGroup %q: imageFactoryURL %q is not a plain http(s) URL. It is interpolated into the worker DataVolume source URL, so it must carry no whitespace, backtick, dollar sign or YAML metacharacters." .groupName ($factoryURL | toString)) -}}
{{- end -}}
{{- end -}}
{{- $_ := set .out "schematicID" $schematicID -}}
{{- $_ := set .out "version" $version -}}
@@ -420,3 +420,49 @@ tests:
path: spec.template.spec.virtualMachineTemplate.spec.dataVolumeTemplates[0].spec.source.http.url
value: https://factory.talos.dev/image/ce4c980550dd2ab1b17bbf2b08801c7eb59418eafe8f279833297925d67c7515/v1.13.6/openstack-amd64.raw.xz
documentSelector: {path: kind, value: KubevirtMachineTemplate}
# A builtin pool never builds a source URL -- its DataVolume carries source.pvc
# and no source.http -- so imageFactoryURL is not an input on that path and is
# not held to the URL pattern. Before this the check ran before the branch and
# refused a field the pool does not consume, which landed on exactly the
# operator moving to builtin to stop depending on the Factory. Reviewed as
# [MINOR] on cozystack/cozystack#3294.
- it: a builtin pool renders with a blanked imageFactoryURL
set:
osImage:
builtin: {}
talos:
imageFactoryURL: ""
asserts:
- equal:
path: spec.template.spec.virtualMachineTemplate.spec.dataVolumeTemplates[0].spec.source.pvc.name
value: talos-worker-ce4c980550dd2ab1b17bbf2b08801c7eb59418eafe8f279833297925d67c7515-v1.13.6
documentSelector: {path: kind, value: KubevirtMachineTemplate}
- notExists:
path: spec.template.spec.virtualMachineTemplate.spec.dataVolumeTemplates[0].spec.source.http
documentSelector: {path: kind, value: KubevirtMachineTemplate}
# Same for a malformed one: the value is unreachable from the clone path, so
# refusing it would be refusing a field this render never reads.
- it: a builtin pool renders with a malformed imageFactoryURL
set:
osImage:
builtin: {}
talos:
imageFactoryURL: "not a url"
asserts:
- equal:
path: spec.template.spec.virtualMachineTemplate.spec.dataVolumeTemplates[0].spec.source.pvc.name
value: talos-worker-ce4c980550dd2ab1b17bbf2b08801c7eb59418eafe8f279833297925d67c7515-v1.13.6
documentSelector: {path: kind, value: KubevirtMachineTemplate}
# The no-osImage default DOES interpolate it, so it stays refused there. This
# is the arm a running cluster is already on, and the sibling cases above must
# not be read as relaxing it.
- it: a pool with osImage omitted still refuses a malformed imageFactoryURL
set:
talos:
imageFactoryURL: "not a url"
asserts:
- failedTemplate:
errorPattern: 'is not a plain http\(s\) URL'