diff --git a/packages/apps/kubernetes-nodes/templates/_helpers.tpl b/packages/apps/kubernetes-nodes/templates/_helpers.tpl index cfeb2dd65..b653405d3 100644 --- a/packages/apps/kubernetes-nodes/templates/_helpers.tpl +++ b/packages/apps/kubernetes-nodes/templates/_helpers.tpl @@ -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 -}} diff --git a/packages/apps/kubernetes-nodes/tests/worker_image_source_test.yaml b/packages/apps/kubernetes-nodes/tests/worker_image_source_test.yaml index 680724ed0..f38f77988 100644 --- a/packages/apps/kubernetes-nodes/tests/worker_image_source_test.yaml +++ b/packages/apps/kubernetes-nodes/tests/worker_image_source_test.yaml @@ -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'