diff --git a/README.md b/README.md index f2f5ab78..fc3ac6fb 100644 --- a/README.md +++ b/README.md @@ -61,7 +61,24 @@ talm init -p cozystack -N myawesomecluster --image factory.talos.dev/installer/< `--image` rewrites the top-level `image:` field in the preset's `values.yaml` before write. The flag is honored on initial `init` only — for an existing project, edit `values.yaml` directly. The `cozystack` preset declares `image:`; the `generic` preset does not, so `--image --preset generic` is rejected up front. -Edit `values.yaml` to set your cluster's control-plane endpoint. This is the URL every node's kubelet and kube-proxy will dial. The chart leaves it empty on purpose so a missed override fails loudly instead of silently embedding a placeholder. For cozystack VIP setups set `endpoint` and `floatingIP` together (same IP, single shared VIP); for single-node clusters use that node's routable IP and leave `floatingIP` blank; for multi-node with an external load balancer use the LB URL and leave `floatingIP` blank. When the VIP must sit on a link that does not yet exist on the live system at first apply (typically a VLAN sub-interface), set `vipLink` to that link name — the chart pins `Layer2VIPConfig.link` to it instead of the default-gateway link that discovery would otherwise pick, and emits the document even on a totally fresh node where no default-gateway link has been discovered yet. The chart does not auto-emit a `LinkConfig` or `VLANConfig` for the override link; the operator is responsible for ensuring the link comes up, typically by adding a `LinkConfig` or `VLANConfig` for that link to the per-node body overlay alongside `vipLink`. Subnet-selector fields (`kubelet.validSubnets`, `etcd.advertisedSubnets`) are derived automatically from the node's default-gateway-bearing link, so no override is needed unless you have a multi-homed node that requires a specific subnet pinned. +Edit `values.yaml` to set your cluster's control-plane endpoint. This is the URL every node's kubelet and kube-proxy will dial. The chart leaves it empty on purpose so a missed override fails loudly instead of silently embedding a placeholder. + +Endpoint / floatingIP combinations: + +- **cozystack VIP setup**: set `endpoint` and `floatingIP` together to the same IP — single shared VIP. +- **single-node cluster**: set `endpoint` to the node's routable IP and leave `floatingIP` blank. +- **multi-node with external load balancer**: set `endpoint` to the LB URL and leave `floatingIP` blank. + +When `vipLink` is left empty the chart picks the link automatically using a two-step rule: + +1. **Longest-prefix match across configurable links.** If `floatingIP` falls inside the CIDR of any address on a configurable link (physical NIC, bond, VLAN, bridge), the most specific subnet wins. This handles the Hetzner-style topology where a public NIC carries the default route and a VLAN child carries the private cluster subnet — the VIP lands on the VLAN child. +2. **Fallback to the IPv4-default-gateway-bearing link.** Used when no configurable link's CIDR contains the `floatingIP` — typical for upstream-routable VIPs that arrive via the default route. + +Addresses on links the chart does not emit a per-link document for (Wireguard, kernel-managed loopback, slave NICs of a bond, anything outside the configurable set) are skipped — a VIP pinned there would have no surrounding network document. + +Set `vipLink` explicitly when the target link does not yet exist on the live system at first apply (typically a VLAN sub-interface). The chart pins `Layer2VIPConfig.link` to it directly and emits the document even on a fresh node where discovery has not yet populated the addresses table. The chart does not auto-emit a `LinkConfig` or `VLANConfig` for the override link; the operator is responsible for ensuring the link comes up, typically by adding a `LinkConfig` or `VLANConfig` for that link to the per-node body overlay alongside `vipLink`. + +Subnet-selector fields (`kubelet.validSubnets`, `etcd.advertisedSubnets`) are derived automatically from the node's default-gateway-bearing link, so no override is needed unless you have a multi-homed node that requires a specific subnet pinned. Boot Talos Linux node, let's say it has address `192.0.2.4`. Then: @@ -126,7 +143,30 @@ cluster: endpoint: https://192.0.2.4:6443 ``` -> **Note:** The output format depends on the Talos version configured in `Chart.yaml` (`templateOptions.talosVersion`) or via the `--talos-version` CLI flag. For Talos < v1.12, the output is a single YAML document with `machine.network` and `machine.registries` sections (as shown above). For Talos >= v1.12, the output uses the multi-document format with separate typed documents instead of the deprecated monolithic fields. `HostnameConfig` and `ResolverConfig` are always emitted; one network interface document is emitted per configurable link on the node (`LinkConfig` for physical NICs, `BondConfig` for bond masters, `VLANConfig` for VLAN sub-interfaces) — multi-NIC nodes therefore produce one document per NIC, not one document total. The link carrying the IPv4 default route gets the gateway entry on its document; every other link is emitted gateway-less. Both IPv4 and IPv6 global-scope addresses on a link are surfaced in its document. Bond slaves are filtered out so they do not collide with the master's `BondConfig`. Bridges are deliberately not auto-emitted as `BridgeConfig` yet — a non-gateway bridge is skipped (declare it via a per-node body overlay if needed); a bridge that carries the default route fails the render with a clear migration hint. The operator-declared `floatingIP` is stripped from per-link addresses so the VIP currently held by a leader does not leak into the static `LinkConfig`. `Layer2VIPConfig` appears on controlplane nodes when `floatingIP` is set; `RegistryMirrorConfig` is emitted only by the cozystack chart. +> **Note: output format depends on Talos version.** +> +> Selected via `Chart.yaml` (`templateOptions.talosVersion`) or `--talos-version`: +> +> - **Talos < v1.12** — single YAML document with `machine.network` and `machine.registries` sections (as shown above). +> - **Talos >= v1.12** — multi-document format with separate typed documents instead of the deprecated monolithic fields. +> +> For v1.12+ multi-doc output, one document is emitted per configurable link on the node, plus a fixed pair on every render: +> +> - `HostnameConfig` and `ResolverConfig` — always emitted. +> - `LinkConfig` — physical NICs. +> - `BondConfig` — bond masters. Bond slaves are filtered out so they do not collide with the master's document. +> - `VLANConfig` — VLAN sub-interfaces. +> - `BridgeConfig` — bridges, symmetric to `BondConfig` for bonds. Ports discovered via `spec.slaveKind == "bridge"` + `spec.masterIndex`; STP / VLAN-filtering settings reach the output when the bridge controller reports them on `spec.bridgeMaster`. +> - `Layer2VIPConfig` — controlplane nodes when `floatingIP` is set. +> - `RegistryMirrorConfig` — cozystack chart only. +> +> Per-link emission rules: +> +> - The link carrying the IPv4 default route gets the `routes.gateway` entry on its document; every other link is emitted gateway-less. Applies uniformly to `LinkConfig`, `BondConfig`, `VLANConfig`, `BridgeConfig`. +> - Both IPv4 and IPv6 global-scope addresses on a link are surfaced. +> - The operator-declared `floatingIP` is stripped from per-link addresses so the VIP currently held by a leader does not leak into the static document. +> +> Multi-NIC nodes therefore produce one document per NIC, not one document total. > **Version compatibility (`templateOptions.talosVersion` / `--talos-version`).** This setting must match the **Talos version actually running on the target node** — i.e. the maintenance ISO/PXE the node booted from for `apply -i`, or the installed Talos for an authenticated apply. It is **not** the same as `install.image`, which only controls what gets written to disk after a successful apply. When the configured contract is newer than the running binary, machinery injects fields (e.g. `machine.install.grubUseUKICmdline` from v1.12) that the running parser does not know, and the apply fails on the node side with `failed to parse config: unknown keys found during decoding: ...`. `talm apply` runs a best-effort pre-flight check against the running version and prints a `warning: pre-flight: ...` line with a hint when it detects this mismatch; if the warning is missed, the same hint is appended to the apply error. Either reboot the node into a maintenance image that matches the configured contract, or lower `templateOptions.talosVersion` / `--talos-version` to match what is running. diff --git a/charts/cozystack/templates/_helpers.tpl b/charts/cozystack/templates/_helpers.tpl index f68e3ebb..d4bc548b 100644 --- a/charts/cozystack/templates/_helpers.tpl +++ b/charts/cozystack/templates/_helpers.tpl @@ -199,17 +199,37 @@ nameservers: {{- else }} [] {{- end }} +{{- /* Coerce .Values.floatingIP to its string form once at the + top of the multi-doc body and reuse the result everywhere + a downstream lookup or formatter needs it. The per-link + addresses_by_link strip on every link emission below + depends on the same stringified value — a worker render + with `floatingIP: 192168` would otherwise feed printf + "%s/" an int, producing `%!s(int=192168)/` that never + matches a CIDR. The coercion isolates that trap to one + place and lets the rest of the template treat the value + uniformly. + + "" is Sprig's serialisation of nil and "" is the + unset string; both mean "operator did not supply a + value". The shared talm.validate_floatingIP partial below + handles the actual fail-fast — invoke it here AND in the + legacy define so a malformed value fails at render time + regardless of the rendered Talos version. */}} +{{- $fipStr := .Values.floatingIP | toString }} +{{- $fipIsSet := and (ne $fipStr "") (ne $fipStr "") }} +{{- include "talm.validate_floatingIP" . }} {{- /* Operator-declared vipLink override: emit Layer2VIPConfig regardless of discovery state. Useful when the target link does not yet exist on the live system at first apply (typical case: a VLAN sub-interface this template is about to bring up). The discovery-derived block below skips its own Layer2VIPConfig when this branch fires, so we never emit duplicates. */}} -{{- if and .Values.floatingIP .Values.vipLink (eq .MachineType "controlplane") }} +{{- if and $fipIsSet .Values.vipLink (eq .MachineType "controlplane") }} --- apiVersion: v1alpha1 kind: Layer2VIPConfig -name: {{ .Values.floatingIP | quote }} +name: {{ $fipStr | quote }} link: {{ .Values.vipLink }} {{- end }} {{- $defaultLinkName := include "talm.discovered.default_link_name_by_gateway" . }} @@ -230,7 +250,13 @@ link: {{ .Values.vipLink }} and follower configs out of sync. */}} {{- $addresses := list }} {{- range $rawAddresses }} -{{- if not (and $.Values.floatingIP (hasPrefix (printf "%s/" $.Values.floatingIP) .)) }} +{{- /* Use the hoisted $fipStr/$fipIsSet from the top of the + define so the strip honours the same coerced value the + validation block above used. Going through `printf "%s/" + $.Values.floatingIP` directly would emit + `%!s(int=192168)/` for a numeric YAML scalar on a worker + render (controlplane was caught by the fail-fast). */ -}} +{{- if not (and $fipIsSet (hasPrefix (printf "%s/" $fipStr) .)) }} {{- $addresses = append $addresses . }} {{- end }} {{- end }} @@ -239,17 +265,58 @@ link: {{ .Values.vipLink }} {{- $linkGateway = include "talm.discovered.gateway_by_link" $linkName }} {{- end }} {{- if eq $kind "bridge" }} -{{- /* BridgeConfig is a separate v1alpha1 typed document the chart - does not yet emit. Skipping a non-gateway bridge leaves the - rendered config without a bridge document and the operator is - responsible for declaring it via a per-node body. A bridge - carrying the IPv4 default route, however, cannot be silently - skipped: that would drop every network document for the - gateway link and the rendered config would describe a node - with no working uplink. Surface a fail with the offending - link and the migration path. */ -}} -{{- if $isGatewayLink }} -{{- fail (printf "talm: discovered bridge %q is the IPv4-default link, but BridgeConfig emission is not yet implemented in the chart. Move the bridge declaration into a per-node body overlay (kind: BridgeConfig), or set Values.vipLink to a different link until bridge support lands." $linkName) }} +{{- /* BridgeConfig emission. Discovers bridge ports (members) via + talm.discovered.bridge_slaves and emits a typed v1.12+ + BridgeConfig document with the same address / route / mtu + shape as the other branches. STP and VLAN filtering are + opt-in: they are emitted only when the bridge controller + reported a non-nil spec.bridgeMaster.stp / spec.bridgeMaster + value, so a default-state bridge stays minimal. */ -}} +{{- $bridgeMaster := $link.spec.bridgeMaster }} +{{- $bridgePorts := fromJsonArray (include "talm.discovered.bridge_slaves" $link.spec.index) }} +--- +apiVersion: v1alpha1 +kind: BridgeConfig +name: {{ $linkName }} +{{- if $bridgePorts }} +links: +{{- range $bridgePorts }} + - {{ . }} +{{- end }} +{{- end }} +{{- if $bridgeMaster }} +{{- if $bridgeMaster.stp }} +{{- if hasKey $bridgeMaster.stp "enabled" }} +stp: + enabled: {{ $bridgeMaster.stp.enabled }} +{{- end }} +{{- end }} +{{- /* COSI's BridgeVLANSpec serialises FilteringEnabled as + yaml:"filteringEnabled" (verified against + siderolabs/talos pkg/machinery/resources/network/link.go). + The output-side BridgeConfig schema uses the shorter + yaml:"filtering,omitempty" key — so we read the long form + from discovery and emit the short form into the rendered + document. */ -}} +{{- if $bridgeMaster.vlan }} +{{- if hasKey $bridgeMaster.vlan "filteringEnabled" }} +vlan: + filtering: {{ $bridgeMaster.vlan.filteringEnabled }} +{{- end }} +{{- end }} +{{- end }} +{{- if $addresses }} +addresses: +{{- range $addresses }} + - address: {{ . }} +{{- end }} +{{- end }} +{{- if $linkGateway }} +routes: + - gateway: {{ $linkGateway }} +{{- end }} +{{- if $link.spec.mtu }} +mtu: {{ $link.spec.mtu }} {{- end }} {{- else if eq $kind "bond" }} {{- $bondMaster := $link.spec.bondMaster }} @@ -354,18 +421,52 @@ mtu: {{ $link.spec.mtu }} {{- /* Discovery-derived Layer2VIPConfig: skipped when the operator has set .Values.vipLink, since the override-path block above has already emitted the document with the operator's chosen - link. */}} -{{- if and .Values.floatingIP (not .Values.vipLink) (eq .MachineType "controlplane") $defaultLinkName }} + link. + + Link selection prefers the link whose discovered addresses + contain the floatingIP (talm.discovered.link_name_for_address), + so a VIP in a private subnet hosted on a VLAN child lands on + that VLAN — not on the IPv4-default-route NIC. The + default-gateway link stays as the fallback for topologies + where the VIP isn't on any discovered subnet (typical for + upstream-routable VIPs that arrive via the default-route + link). When neither resolves a link, no Layer2VIPConfig is + emitted, matching the prior behaviour. */}} +{{- if and $fipIsSet (not .Values.vipLink) (eq .MachineType "controlplane") }} +{{- $vipLink := include "talm.discovered.link_name_for_address" $fipStr }} +{{- /* Default-gateway fallback must also point at a configurable + link — otherwise an unmanaged default-route NIC (Wireguard, + a slave NIC of a bond, anything outside the configurable + set) would silently win selection and the rendered + Layer2VIPConfig would dangle on a link the chart never + emits a per-link document for. Mirror the same + configurable-link gate link_name_for_address applies inside + its own iteration. */ -}} +{{- if not $vipLink }} +{{- if has $defaultLinkName $configurableLinks }} +{{- $vipLink = $defaultLinkName }} +{{- end }} +{{- end }} +{{- if $vipLink }} --- apiVersion: v1alpha1 kind: Layer2VIPConfig -name: {{ .Values.floatingIP | quote }} -link: {{ $defaultLinkName }} +name: {{ $fipStr | quote }} +link: {{ $vipLink }} +{{- end }} {{- end }} {{- end }} {{- /* Shared legacy network section for machine.network */ -}} {{- define "talos.config.network.legacy" }} +{{- /* Coerce floatingIP through toString and call the shared + talm.validate_floatingIP partial so legacy renders fail at + template time on a malformed value, same as the multi-doc + path. $fipStr / $fipIsSet are reused below in place of every + direct .Values.floatingIP reference. */ -}} +{{- $fipStr := .Values.floatingIP | toString }} +{{- $fipIsSet := and (ne $fipStr "") (ne $fipStr "") }} +{{- include "talm.validate_floatingIP" . }} network: hostname: {{ include "talm.discovered.hostname" . | quote }} nameservers: {{ include "talm.discovered.default_resolvers" . }} @@ -377,7 +478,7 @@ link: {{ $defaultLinkName }} top-level interfaces[] entry that carries only the vip block. When vipLink == $defaultLinkName the inline vip below already lands on the right link, so no override entry is needed. */}} - {{- $vipOverride := and .Values.floatingIP .Values.vipLink (eq .MachineType "controlplane") (ne .Values.vipLink $defaultLinkName) }} + {{- $vipOverride := and $fipIsSet .Values.vipLink (eq .MachineType "controlplane") (ne .Values.vipLink $defaultLinkName) }} {{- /* Suppress the inline (discovery-derived) vip when the operator has redirected it to a different link; otherwise the VIP would be pinned twice on different interfaces. */}} @@ -408,25 +509,25 @@ link: {{ $defaultLinkName }} routes: - network: 0.0.0.0/0 gateway: {{ include "talm.discovered.default_gateway" . }} - {{- if and .Values.floatingIP (eq .MachineType "controlplane") (not $suppressInlineVip) }} + {{- if and $fipIsSet (eq .MachineType "controlplane") (not $suppressInlineVip) }} vip: - ip: {{ .Values.floatingIP }} + ip: {{ $fipStr }} {{- end }} {{- else }} addresses: {{ include "talm.discovered.default_addresses_by_gateway" . }} routes: - network: 0.0.0.0/0 gateway: {{ include "talm.discovered.default_gateway" . }} - {{- if and .Values.floatingIP (eq .MachineType "controlplane") (not $suppressInlineVip) }} + {{- if and $fipIsSet (eq .MachineType "controlplane") (not $suppressInlineVip) }} vip: - ip: {{ .Values.floatingIP }} + ip: {{ $fipStr }} {{- end }} {{- end }} {{- end }} {{- if $vipOverride }} - interface: {{ .Values.vipLink }} vip: - ip: {{ .Values.floatingIP }} + ip: {{ $fipStr }} {{- end }} {{- end }} {{- end }} diff --git a/charts/cozystack/values.yaml b/charts/cozystack/values.yaml index 4e73283c..33b7be82 100644 --- a/charts/cozystack/values.yaml +++ b/charts/cozystack/values.yaml @@ -37,14 +37,36 @@ clusterDomain: cozy.local floatingIP: "" # Optional override for the link Layer2VIPConfig is pinned to. When -# left empty the chart picks the IPv4-default-gateway-bearing link -# the node already has (the VLAN sub-interface if one carries the -# default route, otherwise the physical NIC). Set this when the -# target link does not yet exist on the live system at first apply -# -- typically a VLAN sub-interface that the same template is about -# to bring up. Without an override the chart would derive the link -# from discovery on the bare-metal NIC, pin the VIP there, and the -# VIP would land on the wrong link once the VLAN comes up. +# left empty the chart picks the link automatically using a two-step +# discovery rule, with longest-prefix-match across configurable +# links: +# +# 1. If the floatingIP falls inside the CIDR of any address on a +# configurable link (physical NIC, bond, vlan, bridge — same +# set the multi-doc renderer emits LinkConfig for), the link +# with the most-specific (longest-prefix) match wins. This is +# what handles the Hetzner-style topology: a public NIC +# carrying the IPv4 default route plus a VLAN child carrying +# the private cluster network — the VIP lands on the VLAN +# child where the cluster subnet lives. +# +# 2. Otherwise, fall back to the IPv4-default-gateway-bearing +# link. This handles upstream-routable VIPs that arrive via +# the default route and have no matching subnet on the node. +# +# Addresses on links the chart does not emit a per-link document +# for (Wireguard, kernel-managed loopback, slave NICs of a bond, +# anything outside the {physical NIC, bond, vlan, bridge} set +# above) cannot win selection. A VIP pinned there would have no +# surrounding network document and would race the link's own +# address management on apply. +# +# Set vipLink explicitly when the target link does not yet exist on +# the live system at first apply -- typically a VLAN sub-interface +# that the same template is about to bring up. Without the override, +# fresh-node renders before discovery has populated the addresses +# table would fall back to the default-gateway link (or skip +# emission entirely if discovery has not yet resolved one). # When set, Layer2VIPConfig is emitted unconditionally, even on a # totally fresh node where discovery has not yet resolved a default- # gateway link. The chart does NOT auto-emit a LinkConfig/VLANConfig diff --git a/charts/generic/templates/_helpers.tpl b/charts/generic/templates/_helpers.tpl index d67fecd2..cc22a2ca 100644 --- a/charts/generic/templates/_helpers.tpl +++ b/charts/generic/templates/_helpers.tpl @@ -123,17 +123,37 @@ nameservers: {{- else }} [] {{- end }} +{{- /* Coerce .Values.floatingIP to its string form once at the + top of the multi-doc body and reuse the result everywhere + a downstream lookup or formatter needs it. The per-link + addresses_by_link strip on every link emission below + depends on the same stringified value — a worker render + with `floatingIP: 192168` would otherwise feed printf + "%s/" an int, producing `%!s(int=192168)/` that never + matches a CIDR. The coercion isolates that trap to one + place and lets the rest of the template treat the value + uniformly. + + "" is Sprig's serialisation of nil and "" is the + unset string; both mean "operator did not supply a + value". The shared talm.validate_floatingIP partial below + handles the actual fail-fast — invoke it here AND in the + legacy define so a malformed value fails at render time + regardless of the rendered Talos version. */}} +{{- $fipStr := .Values.floatingIP | toString }} +{{- $fipIsSet := and (ne $fipStr "") (ne $fipStr "") }} +{{- include "talm.validate_floatingIP" . }} {{- /* Operator-declared vipLink override: emit Layer2VIPConfig regardless of discovery state. Useful when the target link does not yet exist on the live system at first apply (typical case: a VLAN sub-interface this template is about to bring up). The discovery-derived block below skips its own Layer2VIPConfig when this branch fires, so we never emit duplicates. */}} -{{- if and .Values.floatingIP .Values.vipLink (eq .MachineType "controlplane") }} +{{- if and $fipIsSet .Values.vipLink (eq .MachineType "controlplane") }} --- apiVersion: v1alpha1 kind: Layer2VIPConfig -name: {{ .Values.floatingIP | quote }} +name: {{ $fipStr | quote }} link: {{ .Values.vipLink }} {{- end }} {{- $defaultLinkName := include "talm.discovered.default_link_name_by_gateway" . }} @@ -154,7 +174,13 @@ link: {{ .Values.vipLink }} and follower configs out of sync. */}} {{- $addresses := list }} {{- range $rawAddresses }} -{{- if not (and $.Values.floatingIP (hasPrefix (printf "%s/" $.Values.floatingIP) .)) }} +{{- /* Use the hoisted $fipStr/$fipIsSet from the top of the + define so the strip honours the same coerced value the + validation block above used. Going through `printf "%s/" + $.Values.floatingIP` directly would emit + `%!s(int=192168)/` for a numeric YAML scalar on a worker + render (controlplane was caught by the fail-fast). */ -}} +{{- if not (and $fipIsSet (hasPrefix (printf "%s/" $fipStr) .)) }} {{- $addresses = append $addresses . }} {{- end }} {{- end }} @@ -163,17 +189,58 @@ link: {{ .Values.vipLink }} {{- $linkGateway = include "talm.discovered.gateway_by_link" $linkName }} {{- end }} {{- if eq $kind "bridge" }} -{{- /* BridgeConfig is a separate v1alpha1 typed document the chart - does not yet emit. Skipping a non-gateway bridge leaves the - rendered config without a bridge document and the operator is - responsible for declaring it via a per-node body. A bridge - carrying the IPv4 default route, however, cannot be silently - skipped: that would drop every network document for the - gateway link and the rendered config would describe a node - with no working uplink. Surface a fail with the offending - link and the migration path. */ -}} -{{- if $isGatewayLink }} -{{- fail (printf "talm: discovered bridge %q is the IPv4-default link, but BridgeConfig emission is not yet implemented in the chart. Move the bridge declaration into a per-node body overlay (kind: BridgeConfig), or set Values.vipLink to a different link until bridge support lands." $linkName) }} +{{- /* BridgeConfig emission. Discovers bridge ports (members) via + talm.discovered.bridge_slaves and emits a typed v1.12+ + BridgeConfig document with the same address / route / mtu + shape as the other branches. STP and VLAN filtering are + opt-in: they are emitted only when the bridge controller + reported a non-nil spec.bridgeMaster.stp / spec.bridgeMaster + value, so a default-state bridge stays minimal. */ -}} +{{- $bridgeMaster := $link.spec.bridgeMaster }} +{{- $bridgePorts := fromJsonArray (include "talm.discovered.bridge_slaves" $link.spec.index) }} +--- +apiVersion: v1alpha1 +kind: BridgeConfig +name: {{ $linkName }} +{{- if $bridgePorts }} +links: +{{- range $bridgePorts }} + - {{ . }} +{{- end }} +{{- end }} +{{- if $bridgeMaster }} +{{- if $bridgeMaster.stp }} +{{- if hasKey $bridgeMaster.stp "enabled" }} +stp: + enabled: {{ $bridgeMaster.stp.enabled }} +{{- end }} +{{- end }} +{{- /* COSI's BridgeVLANSpec serialises FilteringEnabled as + yaml:"filteringEnabled" (verified against + siderolabs/talos pkg/machinery/resources/network/link.go). + The output-side BridgeConfig schema uses the shorter + yaml:"filtering,omitempty" key — so we read the long form + from discovery and emit the short form into the rendered + document. */ -}} +{{- if $bridgeMaster.vlan }} +{{- if hasKey $bridgeMaster.vlan "filteringEnabled" }} +vlan: + filtering: {{ $bridgeMaster.vlan.filteringEnabled }} +{{- end }} +{{- end }} +{{- end }} +{{- if $addresses }} +addresses: +{{- range $addresses }} + - address: {{ . }} +{{- end }} +{{- end }} +{{- if $linkGateway }} +routes: + - gateway: {{ $linkGateway }} +{{- end }} +{{- if $link.spec.mtu }} +mtu: {{ $link.spec.mtu }} {{- end }} {{- else if eq $kind "bond" }} {{- $bondMaster := $link.spec.bondMaster }} @@ -278,18 +345,50 @@ mtu: {{ $link.spec.mtu }} {{- /* Discovery-derived Layer2VIPConfig: skipped when the operator has set .Values.vipLink, since the override-path block above has already emitted the document with the operator's chosen - link. */}} -{{- if and .Values.floatingIP (not .Values.vipLink) (eq .MachineType "controlplane") $defaultLinkName }} + link. + + Link selection prefers the link whose discovered addresses + contain the floatingIP (talm.discovered.link_name_for_address), + so a VIP in a private subnet hosted on a VLAN child lands on + that VLAN — not on the IPv4-default-route NIC. The + default-gateway link stays as the fallback for topologies + where the VIP isn't on any discovered subnet (typical for + upstream-routable VIPs that arrive via the default-route + link). When neither resolves a link, no Layer2VIPConfig is + emitted, matching the prior behaviour. */}} +{{- if and $fipIsSet (not .Values.vipLink) (eq .MachineType "controlplane") }} +{{- $vipLink := include "talm.discovered.link_name_for_address" $fipStr }} +{{- /* Default-gateway fallback must also point at a configurable + link — otherwise an unmanaged default-route NIC (Wireguard, + a slave link) would silently win selection and the rendered + Layer2VIPConfig would dangle on a link the chart never emits + a per-link document for. Mirror the same configurable-link + gate link_name_for_address applies inside its own iteration. */ -}} +{{- if not $vipLink }} +{{- if has $defaultLinkName $configurableLinks }} +{{- $vipLink = $defaultLinkName }} +{{- end }} +{{- end }} +{{- if $vipLink }} --- apiVersion: v1alpha1 kind: Layer2VIPConfig -name: {{ .Values.floatingIP | quote }} -link: {{ $defaultLinkName }} +name: {{ $fipStr | quote }} +link: {{ $vipLink }} +{{- end }} {{- end }} {{- end }} {{- /* Shared legacy network section for machine.network */ -}} {{- define "talos.config.network.legacy" }} +{{- /* Coerce floatingIP through toString and call the shared + talm.validate_floatingIP partial so legacy renders fail at + template time on a malformed value, same as the multi-doc + path. $fipStr / $fipIsSet are reused below in place of every + direct .Values.floatingIP reference. */ -}} +{{- $fipStr := .Values.floatingIP | toString }} +{{- $fipIsSet := and (ne $fipStr "") (ne $fipStr "") }} +{{- include "talm.validate_floatingIP" . }} network: hostname: {{ include "talm.discovered.hostname" . | quote }} nameservers: {{ include "talm.discovered.default_resolvers" . }} @@ -301,7 +400,7 @@ link: {{ $defaultLinkName }} top-level interfaces[] entry that carries only the vip block. When vipLink == $defaultLinkName the inline vip below already lands on the right link, so no override entry is needed. */}} - {{- $vipOverride := and .Values.floatingIP .Values.vipLink (eq .MachineType "controlplane") (ne .Values.vipLink $defaultLinkName) }} + {{- $vipOverride := and $fipIsSet .Values.vipLink (eq .MachineType "controlplane") (ne .Values.vipLink $defaultLinkName) }} {{- /* Suppress the inline (discovery-derived) vip when the operator has redirected it to a different link; otherwise the VIP would be pinned twice on different interfaces. */}} @@ -332,25 +431,25 @@ link: {{ $defaultLinkName }} routes: - network: 0.0.0.0/0 gateway: {{ include "talm.discovered.default_gateway" . }} - {{- if and .Values.floatingIP (eq .MachineType "controlplane") (not $suppressInlineVip) }} + {{- if and $fipIsSet (eq .MachineType "controlplane") (not $suppressInlineVip) }} vip: - ip: {{ .Values.floatingIP }} + ip: {{ $fipStr }} {{- end }} {{- else }} addresses: {{ include "talm.discovered.default_addresses_by_gateway" . }} routes: - network: 0.0.0.0/0 gateway: {{ include "talm.discovered.default_gateway" . }} - {{- if and .Values.floatingIP (eq .MachineType "controlplane") (not $suppressInlineVip) }} + {{- if and $fipIsSet (eq .MachineType "controlplane") (not $suppressInlineVip) }} vip: - ip: {{ .Values.floatingIP }} + ip: {{ $fipStr }} {{- end }} {{- end }} {{- end }} {{- if $vipOverride }} - interface: {{ .Values.vipLink }} vip: - ip: {{ .Values.floatingIP }} + ip: {{ $fipStr }} {{- end }} {{- end }} {{- end }} diff --git a/charts/generic/values.yaml b/charts/generic/values.yaml index 141a01f6..2a9d5458 100644 --- a/charts/generic/values.yaml +++ b/charts/generic/values.yaml @@ -19,14 +19,36 @@ endpoint: "" floatingIP: "" # Optional override for the link Layer2VIPConfig is pinned to. When -# left empty the chart picks the IPv4-default-gateway-bearing link -# the node already has (the VLAN sub-interface if one carries the -# default route, otherwise the physical NIC). Set this when the -# target link does not yet exist on the live system at first apply -# — typically a VLAN sub-interface that the same template is about -# to bring up. Without an override the chart would derive the link -# from discovery on the bare-metal NIC, pin the VIP there, and the -# VIP would land on the wrong link once the VLAN comes up. +# left empty the chart picks the link automatically using a two-step +# discovery rule, with longest-prefix-match across configurable +# links: +# +# 1. If the floatingIP falls inside the CIDR of any address on a +# configurable link (physical NIC, bond, vlan, bridge — same +# set the multi-doc renderer emits LinkConfig for), the link +# with the most-specific (longest-prefix) match wins. This is +# what handles the Hetzner-style topology: a public NIC +# carrying the IPv4 default route plus a VLAN child carrying +# the private cluster network — the VIP lands on the VLAN +# child where the cluster subnet lives. +# +# 2. Otherwise, fall back to the IPv4-default-gateway-bearing +# link. This handles upstream-routable VIPs that arrive via +# the default route and have no matching subnet on the node. +# +# Addresses on links the chart does not emit a per-link document +# for (Wireguard, kernel-managed loopback, slave NICs of a bond, +# anything outside the {physical NIC, bond, vlan, bridge} set +# above) cannot win selection. A VIP pinned there would have no +# surrounding network document and would race the link's own +# address management on apply. +# +# Set vipLink explicitly when the target link does not yet exist on +# the live system at first apply — typically a VLAN sub-interface +# that the same template is about to bring up. Without the override, +# fresh-node renders before discovery has populated the addresses +# table would fall back to the default-gateway link (or skip +# emission entirely if discovery has not yet resolved one). # When set, Layer2VIPConfig is emitted unconditionally, even on a # totally fresh node where discovery has not yet resolved a default- # gateway link. The chart does NOT auto-emit a LinkConfig/VLANConfig diff --git a/charts/talm/templates/_helpers.tpl b/charts/talm/templates/_helpers.tpl index 1e47da7b..57178a2d 100644 --- a/charts/talm/templates/_helpers.tpl +++ b/charts/talm/templates/_helpers.tpl @@ -1,3 +1,27 @@ +{{- /* Validate .Values.floatingIP is a parseable IPv4 / IPv6 literal + on controlplane renders. Shared by both the v1.12 multi-doc + and the v1.11 legacy network defines so the same fail-fast + happens regardless of the rendered Talos version. Calling + template must pass the chart context as the dot — the partial + reads .Values.floatingIP and .MachineType. + + The Sprig serialisation of nil is the literal string "", + and an unset string is "". Both mean "operator did not supply + a value" and skip the check. Numeric YAML scalars (operator + writes `floatingIP: 192168` without quotes), bool false / + numeric 0, and any other shape stringifies via toString and + reaches ipIsValid — the friendly fail names the bad value + with %q. */ -}} +{{- define "talm.validate_floatingIP" -}} +{{- $fipStr := .Values.floatingIP | toString -}} +{{- $fipIsSet := and (ne $fipStr "") (ne $fipStr "") -}} +{{- if and $fipIsSet (eq .MachineType "controlplane") -}} +{{- if not (ipIsValid $fipStr) -}} +{{- fail (printf "talm: floatingIP %q is not a valid IPv4 / IPv6 literal. Edit values.yaml and re-run." $fipStr) -}} +{{- end -}} +{{- end -}} +{{- end -}} + {{- define "talm.discovered.system_disk_name" }} {{- $systemDisk := (lookup "systemdisk" "" "system-disk") }} {{- if $systemDisk }} @@ -109,11 +133,25 @@ {{- break }} {{- end }} {{- end }} +{{- /* Coerce .Values.floatingIP through toString before the prefix + compare so an unquoted numeric YAML scalar (operator writes + `floatingIP: 192168`) does not emit `%!s(int=192168)/` that + never matches a real CIDR. Same trap the v1.12 multi-doc + path guards against; legacy schema needs the same treatment. + toString'd nil renders as "" which is also harmless — + it cannot match a real CIDR prefix. */ -}} +{{- $fipStr := $.Values.floatingIP | toString }} {{- $addresses := list }} {{- range (lookup "addresses" "" "").items }} -{{- if and (eq .spec.linkName $linkName) (eq .spec.family $family) (not (eq .spec.scope "host")) }} -{{- if not (hasPrefix (printf "%s/" $.Values.floatingIP) .spec.address) }} -{{- $addresses = append $addresses .spec.address }} +{{- /* Filter malformed or future-format entries the same way + addresses_by_link does (cidrPrefixLen >= 0). A corrupt entry + in the COSI addresses table must not leak verbatim into the + legacy v1.11 machine.network.interfaces[].addresses block. */ -}} +{{- $address := .spec.address | toString }} +{{- $validCidr := ge (int (cidrPrefixLen $address)) 0 }} +{{- if and (eq .spec.linkName $linkName) (eq .spec.family $family) (not (eq .spec.scope "host")) $validCidr }} +{{- if not (hasPrefix (printf "%s/" $fipStr) $address) }} +{{- $addresses = append $addresses $address }} {{- end }} {{- end }} {{- end }} @@ -248,6 +286,22 @@ busPath: {{ .spec.busPath }} {{- toJson $slaves -}} {{- end -}} +{{- /* Get bridge member interfaces (ports) for a given bridge index. + Discovered via spec.slaveKind=="bridge" + spec.masterIndex + matching — symmetric to bond_slaves above. Returns the JSON list + so the multi-doc renderer can iterate without reaching back into + the links collection. */ -}} +{{- define "talm.discovered.bridge_slaves" -}} +{{- $bridgeIndex := . -}} +{{- $slaves := list -}} +{{- range (lookup "links" "" "").items -}} +{{- if and (eq .spec.slaveKind "bridge") (eq (int .spec.masterIndex) (int $bridgeIndex)) -}} +{{- $slaves = append $slaves .metadata.id -}} +{{- end -}} +{{- end -}} +{{- toJson $slaves -}} +{{- end -}} + {{- /* Generate bond configuration from bondMaster spec */ -}} {{- define "talm.discovered.bond_config" -}} {{- $linkName := . -}} @@ -288,6 +342,103 @@ true {{- end -}} {{- end -}} +{{- /* Returns the link name whose discovered addresses contain a CIDR + encompassing the supplied IP literal. Used by the multi-doc + Layer2VIPConfig path to pin the VIP onto the link carrying the + relevant subnet, rather than the IPv4 default-route link (which + is wrong whenever the floatingIP lives on a non-default-route + NIC, e.g. a private VLAN child on a Hetzner-style topology). + Returns the empty string when no link's addresses contain the + IP — the caller (multi-doc Layer2VIPConfig block in the + cozystack and generic charts) falls back to + default_link_name_by_gateway in that case. + + Selection rules: + + 1. The link must be in the configurable-link set + (talm.discovered.configurable_link_names) — addresses on + links the chart does not emit a per-link document for + (Wireguard, kernel-managed loopback, slave NICs of a + bond, anything outside the {physical NIC, bond, vlan, + bridge} set) must not steal the VIP. A VIP pinned there + would have no surrounding network document. + + 2. The address must be globally scoped — host-local, + link-local, and nowhere-scoped addresses are skipped, the + same filter addresses_by_link applies before emitting + LinkConfig.addresses. + + 3. Longest-prefix wins. When a node has overlapping subnets + (e.g. a /16 on a physical NIC and a /24 on a VLAN child + both containing the floatingIP), the more specific subnet + is the right answer — same rule the kernel uses for route + decisions. Without this, iteration order would silently + decide. + + CIDR-membership is computed by the engine-registered + cidrContains helper (net/netip-backed); chart templates do not + have to do per-family bit math, and IPv4 / IPv6 are handled + uniformly. cidrContains is lenient on parse failures (returns + false), so a single corrupt or future-format entry in the + address table cannot crash the entire render. */ -}} +{{- define "talm.discovered.link_name_for_address" -}} +{{- $target := . -}} +{{- /* configurable_link_names ignores its dot input — it walks the + links collection via lookup. Any value is safe; passing + $target rather than synthesizing a new dot here keeps the + define body uncluttered. */ -}} +{{- $configurable := fromJsonArray (include "talm.discovered.configurable_link_names" $target) -}} +{{- /* Hoist the scope skip-list out of the range body so it is + built once per call rather than once per address-table + entry. */ -}} +{{- $skipScopes := list "host" "link" "nowhere" -}} +{{- /* Track best match across iterations. dict-mutation via Sprig + set is the established pattern for cross-iteration state in + Go templates; range introduces a new scope per iteration so + a plain $var = ... reassignment does not propagate. */ -}} +{{- $best := dict "link" "" "prefixLen" -1 -}} +{{- range (lookup "addresses" "" "").items -}} +{{- $address := .spec.address | toString -}} +{{- $linkName := .spec.linkName | toString -}} +{{- if and $address $linkName -}} +{{- /* Filter 1: link must be configurable. */ -}} +{{- if has $linkName $configurable -}} +{{- /* Filter 2: scope must be set and not host/link/nowhere. + Match the addresses_by_link rule exactly: truthy check on + the raw .spec.scope field (rejects nil) AND non-empty + string check on its toString'd value AND not in the + skip-list. The looser variant (only the toString'd check) + would let a nil-scope entry through as the literal + "" string, which is neither "" nor in the skip set. + Real Talos COSI always sets scope, so this is a latent + guardrail rather than a hot path. */ -}} +{{- if and .spec.scope (ne (.spec.scope | toString) "") (not (has (.spec.scope | toString) $skipScopes)) -}} +{{- /* Filter 3: CIDR must contain the target. */ -}} +{{- if cidrContains $address $target -}} +{{- /* Filter 4: longest-prefix match. cidrPrefixLen is the + engine-registered helper that wraps net/netip.Prefix.Bits; + it returns -1 on parse failure, which loses to any valid + prefix length under `gt`. The prior shape split the CIDR on + "/" and atoi-d the second part — masked the failure mode + where a /0 default-route entry mixed into the address table + would tie at 0 and let iteration order pick the winner. + Ties at the same prefix length resolve by COSI's emission + order for the addresses resource — rare in practice (two + configurable links with identically-sized CIDRs both + containing the floatingIP). */ -}} +{{- $prefixLen := cidrPrefixLen $address -}} +{{- if gt $prefixLen (get $best "prefixLen") -}} +{{- $_ := set $best "link" $linkName -}} +{{- $_ := set $best "prefixLen" $prefixLen -}} +{{- end -}} +{{- end -}} +{{- end -}} +{{- end -}} +{{- end -}} +{{- end -}} +{{- get $best "link" -}} +{{- end -}} + {{- /* Check if a link is a vlan interface */ -}} {{- define "talm.discovered.is_vlan" -}} {{- $linkName := . -}} @@ -393,11 +544,26 @@ vlans: {{- define "talm.discovered.addresses_by_link" -}} {{- $linkName := . -}} {{- $addresses := list -}} +{{- /* Hoist the scope skip-list out of the range body so it is + built once per call rather than once per address-table + entry. Symmetric to the hoisted skip-list inside + link_name_for_address. */ -}} +{{- $skipScopes := list "host" "link" "nowhere" -}} {{- range (lookup "addresses" "" "").items -}} {{- $hasScope := and .spec.scope (ne (.spec.scope | toString) "") -}} -{{- $skip := has (.spec.scope | toString) (list "host" "link" "nowhere") -}} -{{- if and (eq .spec.linkName $linkName) $hasScope (not $skip) -}} -{{- $addresses = append $addresses .spec.address -}} +{{- $skip := has (.spec.scope | toString) $skipScopes -}} +{{- /* Filter out corrupt or future-format entries whose address + does not parse as a CIDR. cidrPrefixLen returns -1 on parse + failure, which we treat as "skip" the same way + link_name_for_address does — a single bad entry in COSI + must not propagate into LinkConfig / VLANConfig / BridgeConfig + addresses where it would produce a config Talos rejects on + apply with a less-informative error than the chart could + give. */ -}} +{{- $address := .spec.address | toString -}} +{{- $validCidr := ge (int (cidrPrefixLen $address)) 0 -}} +{{- if and (eq .spec.linkName $linkName) $hasScope (not $skip) $validCidr -}} +{{- $addresses = append $addresses $address -}} {{- end -}} {{- end -}} {{- toJson $addresses -}} diff --git a/pkg/engine/contract_errors_test.go b/pkg/engine/contract_errors_test.go index 0642d892..6e0f5f5b 100644 --- a/pkg/engine/contract_errors_test.go +++ b/pkg/engine/contract_errors_test.go @@ -272,35 +272,24 @@ func bridgeAsGatewayLookup() func(string, string, string) (map[string]any, error } } -// Contract: multi-doc renderer aborts when a bridge carries the IPv4 -// default route. BridgeConfig emission is not yet implemented in the -// chart; the renderer must not silently skip the gateway link (which -// would produce a config describing a node with no working uplink). -// The fail names the offending link, the missing feature -// (BridgeConfig), and the recourse (per-node body overlay, or move -// the VIP via vipLink). -func TestContract_Errors_MultidocBridgeAsGateway(t *testing.T) { +// Contract: multi-doc renderer no longer aborts when a bridge +// carries the IPv4 default route — it now emits a typed +// BridgeConfig document with the gateway. The previous shape +// hard-failed here on the premise that BridgeConfig emission was +// unimplemented, but the typed-document branch handles bridges +// symmetrically to bonds today. The per-chart cross-product is +// pinned in TestContract_NetworkMultidoc_BridgeConfigEmitted and +// TestMultiDocEmitsBridgeConfigWhenBridgeCarriesDefaultRoute; this +// test stays as the negative pin against the previous fail-fast. +func TestContract_Errors_MultidocBridgeAsGateway_NoLongerFails(t *testing.T) { for _, chartPath := range []string{cozystackChartPath, genericChartPath} { t.Run(chartPath, func(t *testing.T) { err := renderExpectingError(t, chartPath, multidocTalos, bridgeAsGatewayLookup(), map[string]any{ "endpoint": testEndpoint, "advertisedSubnets": []any{testAdvertisedSubnet}, }) - if err == nil { - t.Fatalf("expected bridge-as-gateway fail, got nil") - } - msg := err.Error() - if !strings.Contains(msg, "talm:") { - t.Errorf("error must use 'talm:' prefix, got: %s", msg) - } - if !strings.Contains(msg, `"br0"`) { - t.Errorf("error must name the offending link 'br0', got: %s", msg) - } - if !strings.Contains(msg, "BridgeConfig") { - t.Errorf("error must mention BridgeConfig as the missing feature, got: %s", msg) - } - if !strings.Contains(msg, "vipLink") { - t.Errorf("error must suggest vipLink as a workaround, got: %s", msg) + if err != nil { + t.Errorf("bridge-as-gateway must no longer fail (BridgeConfig is emitted now), got: %v", err) } }) } diff --git a/pkg/engine/contract_network_multidoc_test.go b/pkg/engine/contract_network_multidoc_test.go index ff9f52f8..cacedb35 100644 --- a/pkg/engine/contract_network_multidoc_test.go +++ b/pkg/engine/contract_network_multidoc_test.go @@ -237,19 +237,77 @@ func TestContract_NetworkMultidoc_VLANOnBondRendersVLANConfig(t *testing.T) { // === Bridge non-gateway: silent skip === -// Contract: a bridge link that is NOT the IPv4 default route is -// skipped silently — no BridgeConfig is emitted (chart does not yet -// support BridgeConfig output), and no LinkConfig is emitted (it is -// not a physical NIC). The expectation is that operators who run -// bridges declare them via per-node body overlays. The non-gateway -// case is the silent path; the gateway case is a hard fail (pinned -// in contract_errors_test.go). -func TestContract_NetworkMultidoc_NonGatewayBridgeSkipped(t *testing.T) { +// Contract: a bridge link discovered on a node is rendered as a +// typed BridgeConfig document, symmetric to BondConfig for bonds. +// The non-gateway case lands BridgeConfig with addresses and mtu +// but no routes entry (no default gateway to emit). The gateway +// case adds the routes.gateway entry — pinned by +// TestMultiDocEmitsBridgeConfigWhenBridgeCarriesDefaultRoute. +// +// Prior to BridgeConfig support landing, a non-gateway bridge was +// silently skipped (no document emitted) on the premise that the +// feature was unimplemented; this contract pins the current +// "always emit" shape. +func TestContract_NetworkMultidoc_BridgeConfigEmitted(t *testing.T) { out := renderCozystackWith(t, bridgeLookup(), map[string]any{ "advertisedSubnets": []any{testAdvertisedSubnet}, }) - // No BridgeConfig — feature unimplemented. - assertNotContains(t, out, "kind: BridgeConfig") + assertContains(t, out, "kind: BridgeConfig") + assertContains(t, out, "name: br0") +} + +// Contract: a controlplane floatingIP that lives inside the subnet +// configured on a bridge link now legitimately lands on the bridge — +// the bridge is fully rendered as a typed BridgeConfig document, so +// pinning the VIP there no longer leaves it dangling without a +// surrounding network document. Symmetric to the VLAN-child case +// pinned in HetznerTopology_VIPOnPrivateVLAN. +// +// Fixture: bridgeWithClusterSubnetLookup has br0 carrying +// 10.5.0.10/24 (global scope) and the IPv4 default route. floatingIP +// 10.5.0.99 is inside that subnet, so link_name_for_address resolves +// to br0; the discovery-derived Layer2VIPConfig pins link=br0. +// Without BridgeConfig emission (the prior shape), this would have +// been a "VIP on undocumented link" symptom; now BridgeConfig +// documents the link explicitly and the chart also emits STP +// settings carried in spec.bridgeMaster. +func TestContract_NetworkMultidoc_VIPOnBridge(t *testing.T) { + out := renderCozystackWith(t, bridgeWithClusterSubnetLookup(), map[string]any{ + "floatingIP": "10.5.0.99", + "advertisedSubnets": []any{"10.5.0.0/24"}, + }) + + // BridgeConfig must be emitted alongside Layer2VIPConfig — that is + // the whole reason landing the VIP on a bridge is now safe. + assertContains(t, out, "kind: BridgeConfig") + assertContains(t, out, "name: br0") + assertContains(t, out, "- address: 10.5.0.10/24") + assertContains(t, out, "gateway: 10.5.0.1") + // STP setting from spec.bridgeMaster.stp.enabled must surface. + assertContains(t, out, "stp:") + assertContains(t, out, "enabled: true") + // VLAN filtering from spec.bridgeMaster.vlan.filteringEnabled + // must surface as the shorter yaml key (filtering) the + // BridgeConfig output schema uses. + assertContains(t, out, "vlan:") + assertContains(t, out, "filtering: true") + // Bridge port discovered via spec.slaveKind=="bridge" must be + // listed under the BridgeConfig.links. + assertContains(t, out, "links:") + assertContains(t, out, " - eth0") + assertContains(t, out, "kind: Layer2VIPConfig") + assertContains(t, out, `name: "10.5.0.99"`) + assertContains(t, out, "link: br0") + // Guard against an alternate implementation that emits Layer2VIPConfig + // on the wrong link (the physical NIC slaved under the bridge): the + // physical port eth0 is in configurable_link_names too, so a regression + // in the link-selection helper could land the VIP there instead. + if strings.Contains(out, "link: eth0\n") { + t.Errorf("Layer2VIPConfig points at eth0 (bridge port) instead of br0:\n%s", out) + } + if got := strings.Count(out, "kind: Layer2VIPConfig"); got != 1 { + t.Errorf("expected exactly 1 Layer2VIPConfig document, got %d:\n%s", got, out) + } } // === Layer2VIPConfig: discovery-derived === @@ -346,3 +404,863 @@ func TestContract_NetworkMultidoc_FloatingIPStrippedFromLinkAddresses(t *testing // LinkConfig.addresses must NOT contain the VIP CIDR. assertNotContains(t, out, "- address: 192.168.201.10/24") } + +// === Hetzner-style topology: public NIC + private VLAN child === + +// Contract: a controlplane floatingIP that lives in a private subnet +// hosted on a VLAN sub-interface — while the IPv4 default route goes +// out the parent (public) NIC — must be pinned to the VLAN child in +// Layer2VIPConfig, NOT to the default-route link. The relevant +// real-world topology is a deployment where a single physical NIC +// carries the public-internet default gateway and a VLAN child +// carries the private cluster network where the VIP belongs. +// +// Why this needs a dedicated pin: the discovery-derived +// Layer2VIPConfig.link selection in the multi-doc chart must not +// short-circuit to the IPv4-default-route link. Doing so puts the +// VIP on the public NIC; Talos installs the VIP there and the +// cluster never sees the leader on the private subnet. +// +// The test pins three independent contracts: +// +// 1. VLANConfig for the private VLAN child is emitted with the +// correct parent, vlanID, and addresses. If discovery state has +// classified the link correctly and configurable_link_names +// enumerates it, the existing VLAN branch in the multi-doc +// template fires. Surfacing this assertion as part of the +// same test means a regression in either the iteration filter +// or the kind classification surfaces immediately, not via a +// separate downstream symptom. +// +// 2. Layer2VIPConfig is emitted with link=enp0s31f6.4000. +// Helper talm.discovered.link_name_for_address picks the link +// whose CIDR encompasses the floatingIP. +// +// 3. There is no Layer2VIPConfig with link=enp0s31f6 — guards +// against an alternate implementation that emits both +// documents, leaving the operator's apply with a duplicate VIP +// target. +func TestContract_NetworkMultidoc_HetznerTopology_VIPOnPrivateVLAN(t *testing.T) { + out := renderCozystackWith(t, hetznerPublicNICWithPrivateVLANLookup(), map[string]any{ + "floatingIP": "192.168.100.10", + "advertisedSubnets": []any{"192.168.100.0/24"}, + }) + + // Assertion 1: VLANConfig for the private VLAN child is emitted. + assertContains(t, out, "kind: VLANConfig") + assertContains(t, out, "name: enp0s31f6.4000") + assertContains(t, out, "vlanID: 4000") + assertContains(t, out, "parent: enp0s31f6") + assertContains(t, out, "- address: 192.168.100.4/24") + + // Assertion 2: Layer2VIPConfig pins the VIP to the VLAN child. + assertContains(t, out, "kind: Layer2VIPConfig") + assertContains(t, out, `name: "192.168.100.10"`) + assertContains(t, out, "link: enp0s31f6.4000") + + // Assertion 3: no Layer2VIPConfig with the wrong link, and exactly one document total. + if strings.Contains(out, "link: enp0s31f6\n") { + t.Errorf("Layer2VIPConfig points at the public default-route NIC; should be the VLAN child:\n%s", out) + } + if got := strings.Count(out, "kind: Layer2VIPConfig"); got != 1 { + t.Errorf("expected exactly 1 Layer2VIPConfig document, got %d:\n%s", got, out) + } + + // Assertion 4: the public NIC retains its address and default + // route — the fix must not collaterally drop public uplink config + // while moving the VIP. + assertContains(t, out, "name: enp0s31f6\n") + assertContains(t, out, "- address: 88.99.210.37/26") + assertContains(t, out, "gateway: 88.99.210.1") +} + +// Contract: the IPv6 counterpart of the Hetzner-topology test. +// Same shape — public NIC carries the IPv4 default route, VLAN +// child carries the private cluster network — but the private +// subnet and the floatingIP are IPv6 (ULA /64 and a host inside +// it). net/netip's Prefix.Contains is family-agnostic, so the +// VIP-link helper resolves the same way as for IPv4. Without this +// pin, a future swap of cidrContains for a per-family +// implementation could silently regress only the IPv6 path. +// +// Real-world relevance: dual-stack Hetzner / colo deployments where +// IPv4 reaches the upstream gateway and the cluster speaks IPv6 +// internally over a private VLAN. The cluster VIP must land on the +// VLAN's IPv6 ULA, not on the public NIC. +func TestContract_NetworkMultidoc_HetznerTopology_IPv6VIPOnPrivateVLAN(t *testing.T) { + out := renderCozystackWith(t, hetznerPublicNICWithPrivateIPv6VLANLookup(), map[string]any{ + "floatingIP": "2001:db8:cafe::10", + "advertisedSubnets": []any{"2001:db8:cafe::/64"}, + }) + + // VLANConfig for the private VLAN child must still be emitted + // with the IPv6 ULA as a global-scope address. + assertContains(t, out, "kind: VLANConfig") + assertContains(t, out, "name: enp0s31f6.4000") + assertContains(t, out, "vlanID: 4000") + assertContains(t, out, "parent: enp0s31f6") + assertContains(t, out, "- address: 2001:db8:cafe::4/64") + + // Layer2VIPConfig pinned to the VLAN child via subnet membership. + assertContains(t, out, "kind: Layer2VIPConfig") + assertContains(t, out, `name: "2001:db8:cafe::10"`) + assertContains(t, out, "link: enp0s31f6.4000") + + // Must not pin the IPv6 VIP to the public NIC; exactly one + // Layer2VIPConfig document. + if strings.Contains(out, "link: enp0s31f6\n") { + t.Errorf("Layer2VIPConfig points at the public IPv4-default-route NIC; should be the VLAN child:\n%s", out) + } + if got := strings.Count(out, "kind: Layer2VIPConfig"); got != 1 { + t.Errorf("expected exactly 1 Layer2VIPConfig document, got %d:\n%s", got, out) + } + + // Public NIC keeps its IPv4 address and default route — the IPv6 + // fix must not collaterally drop the IPv4 uplink. + assertContains(t, out, "name: enp0s31f6\n") + assertContains(t, out, "- address: 88.99.210.37/26") + assertContains(t, out, "gateway: 88.99.210.1") +} + +// Contract: a non-configurable link (Wireguard, kernel-managed +// loopback, or other interface the chart does not emit a per-link +// document for) cannot win VIP-link selection even if its +// discovered address CIDR contains the floatingIP. +// configurable_link_names is the gate — addresses on links outside +// that set are skipped before CIDR membership is even checked. The +// chart does not emit LinkConfig for non-configurable links, so a +// VIP pinned to one would have no surrounding network document and +// would race the link's own address management on apply. +// +// Fixture: Wireguard subnet 10.244.0.0/16 on wg0 (kind=ether, no +// busPath, not in configurable_link_names). floatingIP 10.244.0.5 +// is INSIDE wg0's subnet. The helper must skip wg0 and fall back +// to $defaultLinkName (the IPv4-default-route physical NIC). +func TestContract_NetworkMultidoc_VIPSkipsNonConfigurableLink(t *testing.T) { + out := renderCozystackWith(t, hetznerWithWireguardLookup(), map[string]any{ + "floatingIP": "10.244.0.5", + "advertisedSubnets": []any{"192.168.100.0/24"}, + }) + + assertContains(t, out, "kind: Layer2VIPConfig") + assertContains(t, out, `name: "10.244.0.5"`) + // Falls back to the IPv4-default-route NIC because wg0 is not + // in configurable_link_names — even though its /16 contains the + // VIP. + assertContains(t, out, "link: enp0s31f6\n") + if strings.Contains(out, "link: wg0") { + t.Errorf("Layer2VIPConfig stole link=wg0 (non-configurable Wireguard); must skip non-configurable links:\n%s", out) + } + if got := strings.Count(out, "kind: Layer2VIPConfig"); got != 1 { + t.Errorf("expected exactly 1 Layer2VIPConfig, got %d:\n%s", got, out) + } +} + +// Contract: when two configurable links both carry addresses whose +// CIDR contains the floatingIP, the link with the more specific +// (longer) prefix wins. Mirrors the kernel's longest-prefix rule for +// route decisions. Without this, iteration order silently picks the +// "winning" link. +// +// Fixture: enp0s31f6 has 192.168.0.10/16 (broad) listed first, +// enp0s31f6.4000 has 192.168.100.4/24 (narrow) listed second. Both +// CIDRs contain floatingIP 192.168.100.10. The /24 must win. +func TestContract_NetworkMultidoc_VIPLinkLongestPrefixMatch(t *testing.T) { + out := renderCozystackWith(t, overlappingSubnetsLookup(), map[string]any{ + "floatingIP": "192.168.100.10", + "advertisedSubnets": []any{"192.168.100.0/24"}, + }) + + assertContains(t, out, "kind: Layer2VIPConfig") + assertContains(t, out, `name: "192.168.100.10"`) + assertContains(t, out, "link: enp0s31f6.4000") + if strings.Contains(out, "link: enp0s31f6\n") { + t.Errorf("Layer2VIPConfig picked the broader /16 instead of the more specific /24 — longest-prefix-match regressed:\n%s", out) + } + if got := strings.Count(out, "kind: Layer2VIPConfig"); got != 1 { + t.Errorf("expected exactly 1 Layer2VIPConfig, got %d:\n%s", got, out) + } +} + +// Contract: BridgeConfig emits the vlan sub-block even when +// spec.bridgeMaster carries no stp setting. Pins the independence +// of the two BridgeConfig sub-blocks (stp / vlan) against a future +// refactor that accidentally nests one inside the other or +// conditions one on the other. +func TestContract_NetworkMultidoc_BridgeConfig_VLANOnlyNoStp(t *testing.T) { + out := renderCozystackWith(t, bridgeWithVLANOnlyLookup(), map[string]any{ + "advertisedSubnets": []any{"10.5.0.0/24"}, + }) + assertContains(t, out, "kind: BridgeConfig") + assertContains(t, out, "vlan:") + assertContains(t, out, "filtering: true") + if strings.Contains(out, "stp:") { + t.Errorf("BridgeConfig emits stp: block when spec.bridgeMaster.stp is unset; sub-blocks must be independent:\n%s", out) + } +} + +// Contract: BridgeConfig emits the stp sub-block even when +// spec.bridgeMaster carries no vlan setting. Mirror of the +// VLAN-only contract above. +func TestContract_NetworkMultidoc_BridgeConfig_StpOnlyNoVlan(t *testing.T) { + out := renderCozystackWith(t, bridgeWithSTPOnlyLookup(), map[string]any{ + "advertisedSubnets": []any{"10.5.0.0/24"}, + }) + assertContains(t, out, "kind: BridgeConfig") + assertContains(t, out, "stp:") + assertContains(t, out, "enabled: true") + if strings.Contains(out, "vlan:") { + t.Errorf("BridgeConfig emits vlan: block when spec.bridgeMaster.vlan is unset; sub-blocks must be independent:\n%s", out) + } +} + +// Contract: malformed entries in COSI's addresses table do not +// propagate into the rendered LinkConfig / VLANConfig / BridgeConfig +// `addresses` blocks. The chart's addresses_by_link helper filters +// out entries whose `.spec.address` fails to parse as a CIDR +// (cidrPrefixLen returns -1), so a corrupt or future-format entry +// stays inside discovery and never reaches a typed document Talos +// would reject on apply. +// +// Fixture: malformedAddressEntryLookup carries +// "definitely-not-a-cidr" on enp0s31f6.4000 sandwiched between two +// well-formed entries. The test asserts the bad value is absent +// from any `- address:` line and the valid sibling +// 192.168.100.4/24 IS present. +func TestContract_NetworkMultidoc_LinkAddressesFilterMalformedCidr(t *testing.T) { + out := renderCozystackWith(t, malformedAddressEntryLookup(), map[string]any{ + "advertisedSubnets": []any{"192.168.100.0/24"}, + }) + if strings.Contains(out, "definitely-not-a-cidr") { + t.Errorf("malformed CIDR leaked into LinkConfig/VLANConfig.addresses; corrupt COSI entries must be filtered at addresses_by_link:\n%s", out) + } + if !strings.Contains(out, "- address: 192.168.100.4/24") { + t.Errorf("valid sibling CIDR 192.168.100.4/24 missing from VLANConfig.addresses; filter must not drop well-formed entries:\n%s", out) + } +} + +// Contract: a malformed address entry in COSI's addresses table does +// not crash the chart render. cidrContains is lenient on parse +// failures (returns false), so the helper skips the bad entry and +// continues iterating; the rest of the addresses table is processed +// normally and the VIP-link still resolves correctly. +// +// Fixture: a "definitely-not-a-cidr" entry sandwiched between two +// valid ones. The render must produce the same Layer2VIPConfig as +// the well-formed Hetzner topology. +func TestContract_NetworkMultidoc_VIPLinkSurvivesMalformedAddressEntry(t *testing.T) { + out := renderCozystackWith(t, malformedAddressEntryLookup(), map[string]any{ + "floatingIP": "192.168.100.10", + "advertisedSubnets": []any{"192.168.100.0/24"}, + }) + + assertContains(t, out, "kind: Layer2VIPConfig") + assertContains(t, out, `name: "192.168.100.10"`) + assertContains(t, out, "link: enp0s31f6.4000") +} + +// Contract: a link-scoped address (scope=link, RFC 3927-style +// 169.254/16 link-local) on a configurable link must NOT win VIP- +// link selection even when its CIDR is the most specific match. +// Filter 2 in link_name_for_address skips host/link/nowhere-scoped +// addresses, mirroring the filter addresses_by_link applies before +// emitting LinkConfig.addresses. Without this filter, link-local +// noise on a configurable VLAN could trump the operator's intent. +// +// Fixture: VLAN child carries 192.168.100.4/24 (global) AND +// 169.254.0.1/16 (link-local). floatingIP 169.254.0.5 is INSIDE +// the link-local /16. The helper must skip that entry; the global +// /24 doesn't match the VIP, so the helper returns "" and the +// caller falls back to the default-route link (enp0s31f6). +func TestContract_NetworkMultidoc_VIPSkipsLinkScopedAddress(t *testing.T) { + out := renderCozystackWith(t, hetznerWithLinkScopedAddressLookup(), map[string]any{ + "floatingIP": "169.254.0.5", + "advertisedSubnets": []any{"192.168.100.0/24"}, + }) + + assertContains(t, out, "kind: Layer2VIPConfig") + assertContains(t, out, `name: "169.254.0.5"`) + // Falls back to the default-route NIC because the only link + // whose subnet contains the VIP is link-scoped, which is + // filtered out before the longest-prefix comparison runs. + assertContains(t, out, "link: enp0s31f6\n") + if got := strings.Count(out, "kind: Layer2VIPConfig"); got != 1 { + t.Errorf("expected exactly 1 Layer2VIPConfig, got %d:\n%s", got, out) + } +} + +// Contract: a malformed floatingIP fails the chart render at +// template time with a clear hint that names the bad value. The +// previous shape silently fell through cidrContains (lenient on +// parse failure) into the default-link fallback, shipping a +// Layer2VIPConfig with a nonsense `name:` value that surfaced only +// when Talos rejected it on apply. Render-time fail is much cheaper +// to debug. +// +// Fixture: simple Hetzner topology with floatingIP "10.0.0.300" +// (octet > 255). Render must fail; the error message must include +// the bad literal so the operator can correlate. +func TestContract_NetworkMultidoc_VIPFailsOnInvalidFloatingIP(t *testing.T) { + err := renderCozystackExpectError(t, hetznerPublicNICWithPrivateVLANLookup(), map[string]any{ + "floatingIP": "10.0.0.300", + "advertisedSubnets": []any{testAdvertisedSubnet}, + }) + + if err == nil { + t.Fatal("expected render to fail on malformed floatingIP, got nil error") + } + if !strings.Contains(err.Error(), "10.0.0.300") { + t.Errorf("error must echo the bad floatingIP literal so the operator can correlate; got: %v", err) + } + if !strings.Contains(err.Error(), "floatingIP") { + t.Errorf("error must mention the offending field name; got: %v", err) + } +} + +// Contract: Filter 2 in link_name_for_address (raw-truthy +// .spec.scope check before any toString coercion) rejects +// entries whose scope field is absent / nil. Without the +// raw-truthy guard, the looser variant would let a nil-scope +// entry through as the literal "" string (neither empty +// nor in the skip-list) and the address would propagate into +// the longest-prefix-match candidate set. Real Talos COSI +// always sets scope, so this is a latent guardrail rather than +// a hot path — but pinning the contract here means a future +// refactor that drops the raw-truthy guard surfaces. +// +// Fixture: VLAN child carries the global /24 (real cluster +// subnet) plus a nil-scope /16 covering the same floatingIP. +// The /16 has a LONGER prefix span — without Filter 2 the +// nil-scope entry would still lose to the /24 by longest- +// prefix-match (24 > 16), so iteration order alone would not +// surface the regression. Both addresses are on the SAME link +// (enp0s31f6.4000), so the test instead asserts on the +// LinkConfig.addresses emission: the nil-scope CIDR must NOT +// appear there even though it sits on a configurable link. +func TestContract_NetworkMultidoc_LinkAddressesSkipsNilScope(t *testing.T) { + out := renderCozystackWith(t, hetznerWithNilScopeAddressLookup(), map[string]any{ + "floatingIP": "192.168.100.10", + "advertisedSubnets": []any{"192.168.100.0/24"}, + }) + // VIP still resolves to the VLAN child via the global-scope /24. + assertContains(t, out, "link: enp0s31f6.4000") + // Nil-scope /16 must be filtered out of addresses_by_link + // (Filter 2 is shared between link_name_for_address and + // addresses_by_link), so it must not appear in any + // `- address:` line of the rendered output. + if strings.Contains(out, "192.168.0.5/16") { + t.Errorf("nil-scope CIDR 192.168.0.5/16 leaked into rendered addresses; Filter 2's raw-truthy scope check regressed:\n%s", out) + } +} + +// Contract: when two configurable links both carry CIDRs of +// the same prefix length and both contain the floatingIP, +// link_name_for_address picks the FIRST match in iteration +// order. Strict-gt comparator (`gt $prefixLen +// $best.prefixLen`) means a later equal-prefix entry cannot +// overwrite the best-so-far. This is the documented behaviour +// — "ties resolve by iteration order" — and the pin here +// catches a future refactor that flips the comparator to ge +// (later wins) or sorts the addresses table differently. +// +// Fixture: eth0 and eth1 both carry 192.168.100.x/24 covering +// floatingIP 192.168.100.10. eth0 is listed FIRST in the +// addresses table, so eth0 wins. +func TestContract_NetworkMultidoc_VIPLinkTieBreakByIterationOrder(t *testing.T) { + out := renderCozystackWith(t, twoConfigurableLinksSamePrefixLookup(), map[string]any{ + "floatingIP": "192.168.100.10", + "advertisedSubnets": []any{"192.168.100.0/24"}, + }) + assertContains(t, out, "kind: Layer2VIPConfig") + assertContains(t, out, `name: "192.168.100.10"`) + assertContains(t, out, "link: eth0") + if strings.Contains(out, "link: eth1") { + t.Errorf("tie-break regressed — equal-prefix later entry won over earlier one; iteration-order contract documented in link_name_for_address comment:\n%s", out) + } +} + +// === Legacy v1.11 fail-fast contracts === +// +// The shared talm.validate_floatingIP partial is included from +// both the v1.12 multi-doc define and the v1.11 legacy define, +// so render-time fail-fast on a malformed floatingIP works +// regardless of templateOptions.talosVersion. These contracts +// pin the legacy path so a future refactor that drops the +// include silently regresses only one schema. + +// Contract: legacy v1.11 render path fails fast on a malformed +// floatingIP literal — same shape as the multi-doc contract +// pinned by TestContract_NetworkMultidoc_VIPFailsOnInvalidFloatingIP. +func TestContract_NetworkLegacy_VIPFailsOnInvalidFloatingIP(t *testing.T) { + err := renderCozystackExpectError(t, hetznerPublicNICWithPrivateVLANLookup(), map[string]any{ + "floatingIP": "10.0.0.300", + "advertisedSubnets": []any{testAdvertisedSubnet}, + }, "v1.11") + + if err == nil { + t.Fatal("expected render to fail on malformed floatingIP on v1.11, got nil error") + } + if !strings.Contains(err.Error(), "10.0.0.300") { + t.Errorf("error must echo the bad floatingIP literal; got: %v", err) + } + if !strings.Contains(err.Error(), "floatingIP") { + t.Errorf("error must mention the offending field name; got: %v", err) + } +} + +// Contract: legacy v1.11 render path coerces numeric floatingIP +// through toString before predicate so the Go-template +// "wrong type for value; expected string; got int" panic does +// not fire. Mirror of +// TestContract_NetworkMultidoc_VIPFailsOnNumericFloatingIP. +func TestContract_NetworkLegacy_VIPFailsOnNumericFloatingIP(t *testing.T) { + err := renderCozystackExpectError(t, hetznerPublicNICWithPrivateVLANLookup(), map[string]any{ + "floatingIP": 192168, + "advertisedSubnets": []any{testAdvertisedSubnet}, + }, "v1.11") + + if err == nil { + t.Fatal("expected render to fail on numeric floatingIP on v1.11, got nil error") + } + if strings.Contains(err.Error(), "wrong type for value") { + t.Errorf("got Go-template type-mismatch panic; the toString coercion must run before the predicate on v1.11 too: %v", err) + } + if !strings.Contains(err.Error(), "192168") { + t.Errorf("error must echo the bad value (stringified); got: %v", err) + } +} + +// Contract: legacy v1.11 render path treats nil floatingIP as +// "unset" — no fail, no Layer2VIP, no "" leak. Mirror of +// TestContract_NetworkMultidoc_VIPGracefulWhenFloatingIPNil. +func TestContract_NetworkLegacy_VIPGracefulWhenFloatingIPNil(t *testing.T) { + err := renderCozystackExpectError(t, hetznerPublicNICWithPrivateVLANLookup(), map[string]any{ + "floatingIP": nil, + "advertisedSubnets": []any{testAdvertisedSubnet}, + }, "v1.11") + if err != nil { + t.Fatalf("nil floatingIP must render cleanly on v1.11, got error: %v", err) + } +} + +// Contract: legacy v1.11 render path fails fast on explicitly +// falsy non-string floatingIP (numeric 0, bool false). Same +// raw-truthy bypass the multi-doc path closed. +func TestContract_NetworkLegacy_VIPFailsOnFalsyNonStringFloatingIP(t *testing.T) { + tests := []struct { + name string + input any + wantInMsg string + }{ + {"numeric zero", 0, "0"}, + {"bool false", false, "false"}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + err := renderCozystackExpectError(t, hetznerPublicNICWithPrivateVLANLookup(), map[string]any{ + "floatingIP": tc.input, + "advertisedSubnets": []any{testAdvertisedSubnet}, + }, "v1.11") + if err == nil { + t.Fatalf("expected render to fail on floatingIP=%v on v1.11, got nil error", tc.input) + } + if !strings.Contains(err.Error(), "floatingIP") { + t.Errorf("error must mention the offending field; got: %v", err) + } + if !strings.Contains(err.Error(), tc.wantInMsg) { + t.Errorf("error must echo the bad value %q; got: %v", tc.wantInMsg, err) + } + }) + } +} + +// Contract: legacy v1.11 default_addresses_by_gateway filters +// malformed entries from the COSI addresses table the same way +// addresses_by_link does — cidrPrefixLen < 0 entries do not leak +// into the rendered machine.network.interfaces[].addresses block. +// Mirror of TestContract_NetworkMultidoc_LinkAddressesFilterMalformedCidr +// against the legacy schema. +func TestContract_NetworkLegacy_DefaultAddressesFilterMalformedCidr(t *testing.T) { + out := renderChartTemplateWithLookup(t, cozystackChartPath, controlplaneTpl, malformedAddressEntryLookup(), "v1.11") + if strings.Contains(out, "definitely-not-a-cidr") { + t.Errorf("legacy v1.11: malformed CIDR leaked into machine.network.interfaces[].addresses; default_addresses_by_gateway filter regressed:\n%s", out) + } +} + +// Generic-chart mirrors of the four legacy fail-fast contracts above. + +func TestContract_NetworkLegacy_Generic_VIPFailsOnInvalidFloatingIP(t *testing.T) { + err := renderGenericExpectError(t, hetznerPublicNICWithPrivateVLANLookup(), map[string]any{ + "floatingIP": "10.0.0.300", + "advertisedSubnets": []any{testAdvertisedSubnet}, + }, "v1.11") + if err == nil || !strings.Contains(err.Error(), "10.0.0.300") { + t.Errorf("generic v1.11: expected fail-fast naming the bad floatingIP, got: %v", err) + } +} + +func TestContract_NetworkLegacy_Generic_VIPFailsOnNumericFloatingIP(t *testing.T) { + err := renderGenericExpectError(t, hetznerPublicNICWithPrivateVLANLookup(), map[string]any{ + "floatingIP": 192168, + "advertisedSubnets": []any{testAdvertisedSubnet}, + }, "v1.11") + if err == nil || strings.Contains(err.Error(), "wrong type for value") { + t.Errorf("generic v1.11: numeric floatingIP must produce friendly fail (not Go-template type panic); got: %v", err) + } +} + +func TestContract_NetworkLegacy_Generic_VIPGracefulWhenFloatingIPNil(t *testing.T) { + err := renderGenericExpectError(t, hetznerPublicNICWithPrivateVLANLookup(), map[string]any{ + "floatingIP": nil, + "advertisedSubnets": []any{testAdvertisedSubnet}, + }, "v1.11") + if err != nil { + t.Errorf("generic v1.11: nil floatingIP must render cleanly, got: %v", err) + } +} + +// Contract: when the default-route-link fallback resolves to a +// non-configurable link (Wireguard, slave NIC, anything outside the +// {physical, bond, vlan, bridge} set), the chart MUST NOT emit +// Layer2VIPConfig — the chart does not emit a per-link document +// for such links, so the VIP would dangle on a link the chart +// never configures. The fallback path mirrors the configurable- +// link gate that link_name_for_address applies inside its own +// iteration; matched-link selection and fallback-link selection +// have to honour the same renderable-link set. +// +// Fixture: IPv4 default route on wg0 (Wireguard, not configurable). +// floatingIP 10.99.99.99 falls outside every discovered subnet, so +// link_name_for_address returns empty. The fallback would have +// picked wg0 before the guard landed; with the guard it skips and +// no Layer2VIPConfig is emitted at all. +func TestContract_NetworkMultidoc_VIPSkipsNonConfigurableDefaultRouteLink(t *testing.T) { + out := renderCozystackWith(t, defaultRouteOnNonConfigurableLinkLookup(), map[string]any{ + "floatingIP": "10.99.99.99", + "advertisedSubnets": []any{testAdvertisedSubnet}, + }) + if strings.Contains(out, "kind: Layer2VIPConfig") { + t.Errorf("Layer2VIPConfig must not emit when the only resolvable link is non-configurable; got:\n%s", out) + } + if strings.Contains(out, "link: wg0") { + t.Errorf("VIP pinned to non-configurable wg0 — fallback must honour the configurable-link gate; got:\n%s", out) + } +} + +// Generic-chart mirror of TestContract_NetworkMultidoc_VIPSkipsNonConfigurableDefaultRouteLink. +func TestContract_NetworkMultidoc_Generic_VIPSkipsNonConfigurableDefaultRouteLink(t *testing.T) { + out := renderGenericWith(t, defaultRouteOnNonConfigurableLinkLookup(), map[string]any{ + "floatingIP": "10.99.99.99", + "advertisedSubnets": []any{testAdvertisedSubnet}, + }) + if strings.Contains(out, "kind: Layer2VIPConfig") { + t.Errorf("generic chart: Layer2VIPConfig must not emit when fallback target is non-configurable; got:\n%s", out) + } + if strings.Contains(out, "link: wg0") { + t.Errorf("generic chart: VIP pinned to non-configurable wg0:\n%s", out) + } +} + +// Contract: discovery has populated the addresses table with a +// configurable link whose subnet contains the floatingIP, but the +// routes table has no IPv4 default route yet (fresh-boot before +// the gateway is reachable, or policy-routing-only topology). +// Layer2VIPConfig must still emit pinned to the subnet-matching +// link — a successful link_name_for_address resolution does not +// depend on $defaultLinkName being non-empty. +// +// Pinning this case catches a future refactor that reintroduces an +// outer $defaultLinkName guard around the whole VIP-emission block +// and silently regresses the fresh-boot / policy-routing path. +func TestContract_NetworkMultidoc_VIPEmitsWithMatchingSubnetEvenWithoutDefaultRoute(t *testing.T) { + out := renderCozystackWith(t, noDefaultRouteWithSubnetMatchLookup(), map[string]any{ + "floatingIP": "192.168.100.10", + "advertisedSubnets": []any{"192.168.100.0/24"}, + }) + + assertContains(t, out, "kind: Layer2VIPConfig") + assertContains(t, out, `name: "192.168.100.10"`) + assertContains(t, out, "link: enp0s31f6.4000") + if got := strings.Count(out, "kind: Layer2VIPConfig"); got != 1 { + t.Errorf("expected exactly 1 Layer2VIPConfig, got %d:\n%s", got, out) + } +} + +// Contract: a nil / unset floatingIP on a controlplane node +// renders without error and emits no Layer2VIPConfig. The +// validation block must gate on the RAW .Values.floatingIP +// truthiness before any toString coercion — Sprig's +// `nil | toString` returns the literal string "", which is +// truthy and not a valid IP, so a naive predicate on the +// toString'd value would fail-fast on every controlplane render +// where the operator left floatingIP unset (single-node +// clusters, LB-fronted multi-node, anything Helm coalesces out +// of the values table). +func TestContract_NetworkMultidoc_VIPGracefulWhenFloatingIPNil(t *testing.T) { + out := renderCozystackWith(t, hetznerPublicNICWithPrivateVLANLookup(), map[string]any{ + "floatingIP": nil, + "advertisedSubnets": []any{testAdvertisedSubnet}, + }) + if strings.Contains(out, "kind: Layer2VIPConfig") { + t.Errorf("Layer2VIPConfig must not emit when floatingIP is nil; got:\n%s", out) + } + if strings.Contains(out, "") { + t.Errorf("rendered output leaks the Sprig literal — fail-fast misfired on nil floatingIP:\n%s", out) + } +} + +// Generic-chart mirror of the nil-safe contract above. +func TestContract_NetworkMultidoc_Generic_VIPGracefulWhenFloatingIPNil(t *testing.T) { + out := renderGenericWith(t, hetznerPublicNICWithPrivateVLANLookup(), map[string]any{ + "floatingIP": nil, + "advertisedSubnets": []any{testAdvertisedSubnet}, + }) + if strings.Contains(out, "kind: Layer2VIPConfig") { + t.Errorf("generic chart: Layer2VIPConfig must not emit when floatingIP is nil; got:\n%s", out) + } + if strings.Contains(out, "") { + t.Errorf("generic chart: rendered output leaks the Sprig literal:\n%s", out) + } +} + +// Contract: explicit falsy non-string floatingIP values (numeric 0, +// bool false) must trip the fail-fast rather than be silently +// treated as "operator unset the field". A raw-truthiness gate +// would skip the predicate entirely and ship a config with no +// Layer2VIPConfig where the operator clearly intended one (just +// with a malformed value). Coercing through toString first +// distinguishes "did not supply" (nil, "") from "supplied a bad +// shape" (0, false, 192168, "10.0.0.300"). +func TestContract_NetworkMultidoc_VIPFailsOnFalsyNonStringFloatingIP(t *testing.T) { + tests := []struct { + name string + input any + wantInMsg string + }{ + {"numeric zero", 0, "0"}, + {"bool false", false, "false"}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + err := renderCozystackExpectError(t, hetznerPublicNICWithPrivateVLANLookup(), map[string]any{ + "floatingIP": tc.input, + "advertisedSubnets": []any{testAdvertisedSubnet}, + }) + if err == nil { + t.Fatalf("expected render to fail on floatingIP=%v, got nil error", tc.input) + } + if !strings.Contains(err.Error(), "floatingIP") { + t.Errorf("error must mention the offending field; got: %v", err) + } + if !strings.Contains(err.Error(), tc.wantInMsg) { + t.Errorf("error must echo the bad value %q; got: %v", tc.wantInMsg, err) + } + }) + } +} + +// Contract: a numeric (non-string) floatingIP — typed without +// quotes in values.yaml so YAML parses it as int — must produce +// the friendly fail-fast error, NOT a Go-template +// "wrong type for value; expected string; got int" panic. +// +// The ipIsValid template function is registered with a string +// parameter; passing an int through Go text/template raises a +// type-mismatch panic that surfaces as a stack trace at the line +// of the `if` predicate, defeating the whole point of the +// validation block. The chart guards against this by coercing +// .Values.floatingIP through `toString` before the predicate. +// +// Pinning this here ensures a future refactor that drops the +// toString coercion does not silently regress into the panic — +// which is exactly the kind of latent failure mode the CLAUDE.md +// "Helm/Go template numeric scalar" rule flags as a recurring +// trap. +func TestContract_NetworkMultidoc_VIPFailsOnNumericFloatingIP(t *testing.T) { + err := renderCozystackExpectError(t, hetznerPublicNICWithPrivateVLANLookup(), map[string]any{ + "floatingIP": 192168, // int, not string + "advertisedSubnets": []any{testAdvertisedSubnet}, + }) + + if err == nil { + t.Fatal("expected render to fail on numeric floatingIP, got nil error") + } + if strings.Contains(err.Error(), "wrong type for value") { + t.Errorf("got Go-template type-mismatch panic instead of friendly fail; the toString coercion must run before the predicate: %v", err) + } + if !strings.Contains(err.Error(), "floatingIP") { + t.Errorf("error must mention the offending field name; got: %v", err) + } + if !strings.Contains(err.Error(), "192168") { + t.Errorf("error must echo the bad value (stringified); got: %v", err) + } +} + +// Contract: the malformed-floatingIP fail-fast block runs BEFORE +// either VIP-emission branch, so even an operator who set vipLink +// (which would normally suppress the discovery branch entirely) +// still gets a clear render-time error rather than a Layer2VIPConfig +// document with a nonsense `name:` value. Pin the validation order +// here — the override block is at the top of the multi-doc network +// section, so a future refactor that moves the validation BELOW the +// override would silently regress this case. +func TestContract_NetworkMultidoc_VIPFailsOnInvalidFloatingIPEvenWithVipLinkOverride(t *testing.T) { + err := renderCozystackExpectError(t, hetznerPublicNICWithPrivateVLANLookup(), map[string]any{ + "floatingIP": "192.168.300.10", + "vipLink": "enp0s31f6.4000", + "advertisedSubnets": []any{testAdvertisedSubnet}, + }) + + if err == nil { + t.Fatal("expected render to fail on malformed floatingIP, got nil error (vipLink override must NOT bypass validation)") + } + if !strings.Contains(err.Error(), "192.168.300.10") { + t.Errorf("error must echo the bad floatingIP literal so the operator can correlate; got: %v", err) + } + if !strings.Contains(err.Error(), "floatingIP") { + t.Errorf("error must mention the offending field name; got: %v", err) + } +} + +// === Generic-chart mirror tests === +// +// The generic chart carries a verbatim copy of the multi-doc +// Layer2VIPConfig selection block from the cozystack chart, but +// every cozystack contract test routes through renderCozystackWith. +// These mirrors exist so a regression in only the generic chart's +// helper hunk surfaces immediately. If the generic chart ever +// drifts from cozystack on this contract, the symmetric pair fails +// and the gap is obvious. + +// Generic-chart mirror of TestContract_NetworkMultidoc_HetznerTopology_VIPOnPrivateVLAN. +func TestContract_NetworkMultidoc_Generic_HetznerTopology_VIPOnPrivateVLAN(t *testing.T) { + out := renderGenericWith(t, hetznerPublicNICWithPrivateVLANLookup(), map[string]any{ + "floatingIP": "192.168.100.10", + "advertisedSubnets": []any{"192.168.100.0/24"}, + }) + assertContains(t, out, "kind: VLANConfig") + assertContains(t, out, "kind: Layer2VIPConfig") + assertContains(t, out, `name: "192.168.100.10"`) + assertContains(t, out, "link: enp0s31f6.4000") + if strings.Contains(out, "link: enp0s31f6\n") { + t.Errorf("generic chart: Layer2VIPConfig points at the public NIC instead of the VLAN child:\n%s", out) + } +} + +// Generic-chart mirror of TestContract_NetworkMultidoc_HetznerTopology_IPv6VIPOnPrivateVLAN. +func TestContract_NetworkMultidoc_Generic_HetznerTopology_IPv6VIPOnPrivateVLAN(t *testing.T) { + out := renderGenericWith(t, hetznerPublicNICWithPrivateIPv6VLANLookup(), map[string]any{ + "floatingIP": "2001:db8:cafe::10", + "advertisedSubnets": []any{"2001:db8:cafe::/64"}, + }) + assertContains(t, out, "kind: Layer2VIPConfig") + assertContains(t, out, `name: "2001:db8:cafe::10"`) + assertContains(t, out, "link: enp0s31f6.4000") +} + +// Generic-chart mirror of TestContract_NetworkMultidoc_VIPLinkLongestPrefixMatch. +func TestContract_NetworkMultidoc_Generic_VIPLinkLongestPrefixMatch(t *testing.T) { + out := renderGenericWith(t, overlappingSubnetsLookup(), map[string]any{ + "floatingIP": "192.168.100.10", + "advertisedSubnets": []any{"192.168.100.0/24"}, + }) + assertContains(t, out, "link: enp0s31f6.4000") + if strings.Contains(out, "link: enp0s31f6\n") { + t.Errorf("generic chart: longest-prefix match regressed:\n%s", out) + } +} + +// Generic-chart mirror of TestContract_NetworkMultidoc_FloatingIPNotInDiscoveredSubnetFallsBackToGateway. +func TestContract_NetworkMultidoc_Generic_FloatingIPNotInDiscoveredSubnetFallsBackToGateway(t *testing.T) { + out := renderGenericWith(t, simpleNicLookup(), map[string]any{ + "floatingIP": "10.99.99.99", + "advertisedSubnets": []any{testAdvertisedSubnet}, + }) + assertContains(t, out, "kind: Layer2VIPConfig") + assertContains(t, out, "link: eth0") +} + +// Generic-chart mirror of TestContract_NetworkMultidoc_LinkAddressesFilterMalformedCidr. +func TestContract_NetworkMultidoc_Generic_LinkAddressesFilterMalformedCidr(t *testing.T) { + out := renderGenericWith(t, malformedAddressEntryLookup(), map[string]any{ + "advertisedSubnets": []any{"192.168.100.0/24"}, + }) + if strings.Contains(out, "definitely-not-a-cidr") { + t.Errorf("generic chart: malformed CIDR leaked into rendered addresses:\n%s", out) + } + if !strings.Contains(out, "- address: 192.168.100.4/24") { + t.Errorf("generic chart: valid sibling CIDR dropped by the filter:\n%s", out) + } +} + +// Generic-chart mirror of TestContract_NetworkMultidoc_LinkAddressesSkipsNilScope. +func TestContract_NetworkMultidoc_Generic_LinkAddressesSkipsNilScope(t *testing.T) { + out := renderGenericWith(t, hetznerWithNilScopeAddressLookup(), map[string]any{ + "floatingIP": "192.168.100.10", + "advertisedSubnets": []any{"192.168.100.0/24"}, + }) + assertContains(t, out, "link: enp0s31f6.4000") + if strings.Contains(out, "192.168.0.5/16") { + t.Errorf("generic chart: nil-scope CIDR leaked into rendered addresses:\n%s", out) + } +} + +// Generic-chart mirror of TestContract_NetworkMultidoc_VIPLinkTieBreakByIterationOrder. +func TestContract_NetworkMultidoc_Generic_VIPLinkTieBreakByIterationOrder(t *testing.T) { + out := renderGenericWith(t, twoConfigurableLinksSamePrefixLookup(), map[string]any{ + "floatingIP": "192.168.100.10", + "advertisedSubnets": []any{"192.168.100.0/24"}, + }) + assertContains(t, out, "link: eth0") + if strings.Contains(out, "link: eth1") { + t.Errorf("generic chart: tie-break regressed:\n%s", out) + } +} + +// Generic-chart mirror of TestContract_NetworkMultidoc_BridgeConfig_VLANOnlyNoStp. +func TestContract_NetworkMultidoc_Generic_BridgeConfig_VLANOnlyNoStp(t *testing.T) { + out := renderGenericWith(t, bridgeWithVLANOnlyLookup(), map[string]any{ + "advertisedSubnets": []any{"10.5.0.0/24"}, + }) + assertContains(t, out, "kind: BridgeConfig") + assertContains(t, out, "vlan:") + assertContains(t, out, "filtering: true") + if strings.Contains(out, "stp:") { + t.Errorf("generic chart: BridgeConfig emits stp: block when bridgeMaster.stp unset:\n%s", out) + } +} + +// Generic-chart mirror of TestContract_NetworkMultidoc_BridgeConfig_StpOnlyNoVlan. +func TestContract_NetworkMultidoc_Generic_BridgeConfig_StpOnlyNoVlan(t *testing.T) { + out := renderGenericWith(t, bridgeWithSTPOnlyLookup(), map[string]any{ + "advertisedSubnets": []any{"10.5.0.0/24"}, + }) + assertContains(t, out, "kind: BridgeConfig") + assertContains(t, out, "stp:") + assertContains(t, out, "enabled: true") + if strings.Contains(out, "vlan:") { + t.Errorf("generic chart: BridgeConfig emits vlan: block when bridgeMaster.vlan unset:\n%s", out) + } +} + +// Contract: when the floatingIP isn't on any discovered subnet (e.g. +// an upstream-routable VIP that arrives via the default-route link, +// or an operator typo), Layer2VIPConfig falls back to the +// IPv4-default-route link rather than silently skipping or failing. +// Pre-fix behaviour was always-fall-back; the fix prefers +// subnet-membership but preserves the fallback for topologies where +// the new helper has nothing to match on. simpleNicLookup carries a +// gateway on eth0 with addresses 192.168.201.10/24; we set a +// floatingIP outside that subnet and assert it lands on eth0. +func TestContract_NetworkMultidoc_FloatingIPNotInDiscoveredSubnetFallsBackToGateway(t *testing.T) { + out := renderCozystackWith(t, simpleNicLookup(), map[string]any{ + "floatingIP": "10.99.99.99", + "advertisedSubnets": []any{testAdvertisedSubnet}, + }) + assertContains(t, out, "kind: Layer2VIPConfig") + assertContains(t, out, `name: "10.99.99.99"`) + assertContains(t, out, "link: eth0") + if got := strings.Count(out, "kind: Layer2VIPConfig"); got != 1 { + t.Errorf("expected exactly 1 Layer2VIPConfig (fallback path), got %d:\n%s", got, out) + } +} diff --git a/pkg/engine/helm/engine.go b/pkg/engine/helm/engine.go index d064efbd..c83fee5c 100644 --- a/pkg/engine/helm/engine.go +++ b/pkg/engine/helm/engine.go @@ -260,21 +260,89 @@ func (e Engine) initFunMap(tmpl *template.Template) { } } - // cidrNetwork canonicalizes a CIDR string to its network form - // ("192.168.201.10/24" -> "192.168.201.0/24"), matching what - // operators see in Talos docs and upstream examples. Sprig ships - // no equivalent; net/netip's ParsePrefix + Masked handles both - // IPv4 and IPv6 without any host-bit arithmetic in the template. - funcMap["cidrNetwork"] = func(cidr string) (string, error) { - p, err := netip.ParsePrefix(cidr) - if err != nil { - return "", fmt.Errorf("cidrNetwork: %w", err) - } + funcMap["cidrNetwork"] = cidrNetwork + funcMap["cidrContains"] = cidrContains + funcMap["cidrPrefixLen"] = cidrPrefixLen + funcMap["ipIsValid"] = ipIsValid + + tmpl.Funcs(funcMap) +} - return p.Masked().String(), nil +// cidrNetwork returns the network portion of a CIDR (host bits zeroed). The +// canonical "/" form is what operators see in Talos docs and +// upstream examples. Sprig ships no equivalent; net/netip's ParsePrefix + +// Masked handles both IPv4 and IPv6 without any host-bit arithmetic in the +// template. +func cidrNetwork(cidr string) (string, error) { + prefix, err := netip.ParsePrefix(cidr) + if err != nil { + return "", fmt.Errorf("cidrNetwork: %w", err) } - tmpl.Funcs(funcMap) + return prefix.Masked().String(), nil +} + +// cidrPrefixLen returns the prefix length (in bits) of the given CIDR. -1 +// signals an unparseable input rather than an error so the chart-side +// longest-prefix-match comparator can simply check `gt prefixLen bestSoFar` +// without having to thread error handling through every iteration. Mirrors +// the lenient parse behaviour of cidrContains for the same reason: a corrupt +// or future-format entry in the COSI addresses table must not crash the +// entire chart render. +func cidrPrefixLen(cidr string) (int, error) { + prefix, err := netip.ParsePrefix(cidr) + if err != nil { + //nolint:nilerr // parse-failure is deliberately a sentinel value, see docstring + return -1, nil + } + + return prefix.Bits(), nil +} + +// ipIsValid reports whether the given string parses as an IP address literal. +// Used by the multi-doc Layer2VIPConfig block to fail-fast at render time +// when an operator-supplied floatingIP is malformed — a render-time error +// with the exact bad value is much cheaper to debug than an apply-time +// rejection from the Talos config controller. +func ipIsValid(addrStr string) (bool, error) { + _, err := netip.ParseAddr(addrStr) + if err != nil { + //nolint:nilerr // parse-failure is the "false" outcome of this predicate + return false, nil + } + + return true, nil +} + +// cidrContains reports whether the given IP literal falls inside the given +// CIDR. Used by the multi-doc Layer2VIPConfig discovery path to pick the link +// whose subnet hosts the operator-declared floatingIP — net/netip handles +// IPv4 and IPv6 uniformly so chart templates do not have to do per-family bit +// math. +// +// Parse failures on either input return (false, nil) rather than an error. +// Set-membership semantics: an undefined CIDR cannot contain anything; an +// undefined IP is not in any defined set. The chart-side helper that drives +// the membership search runs over every entry in the addresses COSI +// resource, so a single corrupt or future-format entry must not crash the +// entire render. The operator-typoed floatingIP case is handled separately +// by the chart layer: cozystack and generic call ipIsValid up-front and +// fail the render with a clear hint that names the bad value, so a typoed +// floatingIP never reaches cidrContains. +func cidrContains(cidr, addrStr string) (bool, error) { + prefix, err := netip.ParsePrefix(cidr) + if err != nil { + //nolint:nilerr // parse-failure is deliberately the "no match" case; see docstring + return false, nil + } + + addr, err := netip.ParseAddr(addrStr) + if err != nil { + //nolint:nilerr // parse-failure is deliberately the "no match" case; see docstring + return false, nil + } + + return prefix.Contains(addr), nil } // render takes a map of templates/values and renders them. The err return is diff --git a/pkg/engine/helm/engine_test.go b/pkg/engine/helm/engine_test.go index f01e02d8..ba59c47f 100644 --- a/pkg/engine/helm/engine_test.go +++ b/pkg/engine/helm/engine_test.go @@ -1268,3 +1268,176 @@ func TestCidrNetworkTemplateFunc(t *testing.T) { }) } } + +// TestCidrContainsTemplateFunc exercises the cidrContains template +// function directly. Used by talm.discovered.link_name_for_address +// to pick the link whose subnet hosts a floatingIP. Both IPv4 and +// IPv6 paths are exercised so a future swap of net/netip for +// per-family bit math surfaces here, not through chart symptoms. +func TestCidrContainsTemplateFunc(t *testing.T) { + renderExpr := func(expr string) (string, error) { + chrt := &chart.Chart{ + Metadata: &chart.Metadata{Name: "cidrtest"}, + Templates: []*chart.File{{Name: "templates/out.yaml", Data: []byte(expr)}}, + Values: map[string]any{}, + } + var eng Engine + out, err := eng.Render(chrt, chartutil.Values{helmKeyValues: map[string]any{}}) + if err != nil { + return "", err + } + return out["cidrtest/templates/out.yaml"], nil + } + + tests := []struct { + name string + cidr string + ip string + want string + wantErr bool + }{ + {"ipv4 host inside /24", "192.168.100.0/24", "192.168.100.10", "true", false}, + {"ipv4 host outside /24", "192.168.100.0/24", "192.168.101.10", "false", false}, + {"ipv4 host on boundary /24", "192.168.100.0/24", "192.168.100.0", "true", false}, + {"ipv4 broadcast in /24", "192.168.100.0/24", "192.168.100.255", "true", false}, + {"ipv4 inside /26 first quarter", "88.99.210.0/26", "88.99.210.37", "true", false}, + {"ipv4 outside /26 first quarter", "88.99.210.0/26", "88.99.210.64", "false", false}, + {"ipv4 /32 self-match", "10.0.0.1/32", "10.0.0.1", "true", false}, + {"ipv4 /32 other-host", "10.0.0.1/32", "10.0.0.2", "false", false}, + {"ipv6 inside /64", "2001:db8::/64", "2001:db8::1", "true", false}, + {"ipv6 outside /64", "2001:db8::/64", "2001:db9::1", "false", false}, + {"hetzner case: VIP in private VLAN /24", "192.168.100.4/24", "192.168.100.10", "true", false}, + {"hetzner case: VIP NOT in public /26", "88.99.210.37/26", "192.168.100.10", "false", false}, + // Parse failures fall through to "no match" rather than an + // error so the chart-side helper that iterates over every + // address in the COSI table doesn't crash the entire render + // on a single corrupt or future-format entry. An operator- + // typoed floatingIP likewise produces a "no match" outcome, + // which routes through the default-link fallback — Talos + // rejects the malformed IP literal on apply with a clearer + // error than the chart could produce. + {"malformed cidr returns false", "not-a-cidr", "10.0.0.1", "false", false}, + {"malformed ip returns false", "10.0.0.0/24", "not-an-ip", "false", false}, + {"empty cidr returns false", "", "10.0.0.1", "false", false}, + {"empty ip returns false", "10.0.0.0/24", "", "false", false}, + {"both empty returns false", "", "", "false", false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := renderExpr(fmt.Sprintf(`{{ cidrContains %q %q }}`, tt.cidr, tt.ip)) + if tt.wantErr { + if err == nil { + t.Errorf("expected error for cidr=%q ip=%q, got output %q", tt.cidr, tt.ip, got) + } + return + } + if err != nil { + t.Fatalf("unexpected error for cidr=%q ip=%q: %v", tt.cidr, tt.ip, err) + } + if got != tt.want { + t.Errorf("cidrContains(%q, %q) = %q, want %q", tt.cidr, tt.ip, got, tt.want) + } + }) + } +} + +// TestCidrPrefixLenTemplateFunc exercises the cidrPrefixLen helper +// the chart template uses for longest-prefix-match comparison +// inside the VIP-link selection loop. -1 on parse failure is a +// load-bearing sentinel: it loses to any valid prefix length under +// `gt`, so a corrupt entry in the addresses table cannot tie at 0 +// with a /0 default-route entry and let iteration order pick the +// winner. +func TestCidrPrefixLenTemplateFunc(t *testing.T) { + renderExpr := func(expr string) (string, error) { + chrt := &chart.Chart{ + Metadata: &chart.Metadata{Name: "cidrtest"}, + Templates: []*chart.File{{Name: "templates/out.yaml", Data: []byte(expr)}}, + Values: map[string]any{}, + } + var eng Engine + out, err := eng.Render(chrt, chartutil.Values{helmKeyValues: map[string]any{}}) + if err != nil { + return "", err + } + return out["cidrtest/templates/out.yaml"], nil + } + + tests := []struct { + name string + input string + want string + }{ + {"ipv4 /24", "192.168.100.0/24", "24"}, + {"ipv4 /16", "10.0.0.0/16", "16"}, + {"ipv4 /32 host", "10.0.0.1/32", "32"}, + {"ipv4 /0 default", "0.0.0.0/0", "0"}, + {"ipv6 /64", "2001:db8::/64", "64"}, + {"ipv6 /128 host", "2001:db8::1/128", "128"}, + {"malformed cidr returns -1", "not-a-cidr", "-1"}, + {"missing prefix returns -1", "192.168.0.1", "-1"}, + {"empty returns -1", "", "-1"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := renderExpr(fmt.Sprintf(`{{ cidrPrefixLen %q }}`, tt.input)) + if err != nil { + t.Fatalf("unexpected error for %q: %v", tt.input, err) + } + if got != tt.want { + t.Errorf("cidrPrefixLen(%q) = %q, want %q", tt.input, got, tt.want) + } + }) + } +} + +// TestIPIsValidTemplateFunc exercises the ipIsValid predicate the +// multi-doc chart uses to fail-fast on a typoed floatingIP. +// Returns Go bool; Helm's renderExpr stringifies bool as +// "true"/"false" so the chart can compose via `if not +// (ipIsValid ...)`. The fail-fast at render time is much cheaper +// to debug than a Talos apply-time rejection. +func TestIPIsValidTemplateFunc(t *testing.T) { + renderExpr := func(expr string) (string, error) { + chrt := &chart.Chart{ + Metadata: &chart.Metadata{Name: "iptest"}, + Templates: []*chart.File{{Name: "templates/out.yaml", Data: []byte(expr)}}, + Values: map[string]any{}, + } + var eng Engine + out, err := eng.Render(chrt, chartutil.Values{helmKeyValues: map[string]any{}}) + if err != nil { + return "", err + } + return out["iptest/templates/out.yaml"], nil + } + + tests := []struct { + name string + input string + want string + }{ + {"ipv4 valid", "192.168.100.10", "true"}, + {"ipv4 host CIDR not a bare ip", "192.168.100.10/24", "false"}, + {"ipv4 octet > 255", "10.0.0.300", "false"}, + {"ipv4 with junk suffix", "192.168.100.10x", "false"}, + {"ipv6 valid", "2001:db8::1", "true"}, + {"ipv6 with zone", "fe80::1%eth0", "true"}, + {"empty returns false", "", "false"}, + {"hostname not an ip", "controlplane.example.com", "false"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := renderExpr(fmt.Sprintf(`{{ ipIsValid %q }}`, tt.input)) + if err != nil { + t.Fatalf("unexpected error for %q: %v", tt.input, err) + } + if got != tt.want { + t.Errorf("ipIsValid(%q) = %q, want %q", tt.input, got, tt.want) + } + }) + } +} diff --git a/pkg/engine/render_test.go b/pkg/engine/render_test.go index e799d6df..b4499f8f 100644 --- a/pkg/engine/render_test.go +++ b/pkg/engine/render_test.go @@ -1726,41 +1726,54 @@ func TestMultiDocLinkConfigStripsFloatingIPFromAddresses(t *testing.T) { } } -// TestMultiDocFailsWhenBridgeCarriesDefaultRoute pins the guardrail -// for the case where a discovered bridge is the IPv4-default link. -// The bridge branch of the renderer skips emission (BridgeConfig is -// not yet implemented), so without an explicit fail the rendered -// config would carry no document for the gateway-bearing link at -// all — silent drop of the entire network configuration. The -// guardrail surfaces the missing branch as a clear error pointing -// the operator at the per-node body workaround. -func TestMultiDocFailsWhenBridgeCarriesDefaultRoute(t *testing.T) { - origLookup := helmEngine.LookupFunc - t.Cleanup(func() { helmEngine.LookupFunc = origLookup }) - helmEngine.LookupFunc = bridgeWithGatewayLookup() - - chrt, err := loader.LoadDir("../../charts/cozystack") - if err != nil { - t.Fatalf("load chart: %v", err) +// TestMultiDocEmitsBridgeConfigWhenBridgeCarriesDefaultRoute pins +// that a discovered bridge that is also the IPv4-default link is +// rendered as a typed BridgeConfig document with the gateway entry +// attached. Earlier shapes of the renderer hard-failed here on the +// premise that BridgeConfig emission was unimplemented; the typed- +// document branch now handles bridges symmetrically to bonds, so +// the operator no longer has to declare the bridge via a per-node +// body overlay just to keep the default-route link alive. +func TestMultiDocEmitsBridgeConfigWhenBridgeCarriesDefaultRoute(t *testing.T) { + output := renderChartTemplateWithLookup(t, "../../charts/cozystack", "templates/controlplane.yaml", bridgeWithGatewayLookup(), "v1.12") + + // Find the br0 document and verify it is the BridgeConfig with the + // gateway entry attached. Asserting against split documents + // (rather than "some BridgeConfig" / "some gateway:" globally) + // guards against an alternate shape where the renderer emits a + // stray LinkConfig for br0 alongside the BridgeConfig, or where + // the gateway lands on the wrong document entirely. + var ( + br0Doc string + br0Kinds []string + linkConfigs int + ) + for doc := range strings.SplitSeq(output, "\n---\n") { + if !strings.Contains(doc, "name: br0\n") && !strings.HasSuffix(strings.TrimSpace(doc), "name: br0") { + continue + } + br0Doc = doc + for line := range strings.SplitSeq(doc, "\n") { + if kind, ok := strings.CutPrefix(line, "kind:"); ok { + br0Kinds = append(br0Kinds, strings.TrimSpace(kind)) + if strings.Contains(line, "LinkConfig") { + linkConfigs++ + } + } + } } - values := cloneValues(chrt.Values) - if v, _ := values["endpoint"].(string); v == "" { - values["endpoint"] = testEndpoint + + if br0Doc == "" { + t.Fatalf("no document named br0 in rendered output:\n%s", output) } - if arr, ok := values["advertisedSubnets"].([]any); !ok || len(arr) == 0 { - values["advertisedSubnets"] = []any{testAdvertisedSubnet} + if linkConfigs > 0 { + t.Errorf("br0 was emitted as a LinkConfig — wrong kind for a bridge link:\n%s", output) } - - eng := helmEngine.Engine{} - _, err = eng.Render(chrt, chartutil.Values{ - "Values": values, - "TalosVersion": "v1.12", - }) - if err == nil { - t.Fatal("expected render to fail when a bridge carries the IPv4 default route — silent drop of every network document for the gateway link is the regression this guardrail prevents") + if !strings.Contains(br0Doc, "kind: BridgeConfig") { + t.Errorf("br0 document is not a BridgeConfig (kinds seen: %v):\n%s", br0Kinds, br0Doc) } - if !strings.Contains(err.Error(), "bridge") { - t.Errorf("fail message must name the bridge so the operator can locate the offending link, got: %v", err) + if !strings.Contains(br0Doc, "gateway:") { + t.Errorf("br0 BridgeConfig is missing the routes.gateway entry for the IPv4 default route:\n%s", br0Doc) } } @@ -1898,18 +1911,18 @@ func TestMultiDocBondSlavesNotEmittedAsLinkConfig(t *testing.T) { } } -// TestMultiDocBridgeSkipsLinkConfigBranch pins that a discovered -// bridge link does NOT fall through to the LinkConfig branch. The -// chart does not yet emit BridgeConfig, so the bridge must be -// skipped (rather than rendered as a wrong-kind LinkConfig that -// Talos would attach to the wrong interface semantics). Once a -// future change adds a BridgeConfig branch, the test gets updated -// to assert the new emission. -func TestMultiDocBridgeSkipsLinkConfigBranch(t *testing.T) { +// TestMultiDocBridgeDoesNotFallThroughToLinkConfig pins that a +// discovered bridge link is NOT rendered through the LinkConfig +// fallback branch. Bridges now have their own BridgeConfig +// emission path, so a bridge must surface as BridgeConfig (not +// LinkConfig). Catching a wrong-kind LinkConfig here prevents a +// future refactor that drops the dedicated bridge branch and +// silently routes bridges through LinkConfig. +func TestMultiDocBridgeDoesNotFallThroughToLinkConfig(t *testing.T) { output := renderChartTemplateWithLookup(t, "../../charts/cozystack", "templates/controlplane.yaml", bridgeLookup(), "v1.12") if strings.Contains(output, "kind: LinkConfig\nname: br0") { - t.Errorf("bridge br0 emitted as a LinkConfig — wrong document kind. Should be skipped until BridgeConfig support lands:\n%s", output) + t.Errorf("bridge br0 emitted as a LinkConfig — wrong document kind for a bridge:\n%s", output) } // Routed physical NIC (eth0) still emits its own LinkConfig. if !strings.Contains(output, "name: eth0") { @@ -4423,23 +4436,43 @@ func multiNicWithVLANLookup() func(string, string, string) (map[string]any, erro } } -// legacyInterfacesInRunningConfigLookup returns a lookup fixture -// shaped like a node that was originally bootstrapped on a legacy -// chart (talosVersion v1.11) and carries non-empty -// machine.network.interfaces[] in its running MachineConfig. The -// multi-doc renderer must detect this and refuse to render rather -// than silently dropping the legacy interface block — otherwise an -// upgrade from chart v0.23 to v0.24+ would silently lose every -// user-declared address, route, and VLAN that lived under the -// legacy schema. -func legacyInterfacesInRunningConfigLookup() func(string, string, string) (map[string]any, error) { - eth0 := map[string]any{ - "metadata": map[string]any{"id": "eth0"}, +// hetznerPublicNICWithPrivateVLANLookup returns a lookup fixture +// that mirrors a Hetzner-style topology: a public NIC carrying the +// IPv4 default route and a VLAN sub-interface carrying the private +// cluster network. Distinct from multiNicWithVLANLookup, where the +// VLAN itself owns the default route. +// +// Topology pinned by this fixture: +// - enp0s31f6: physical, public address 88.99.210.37/26, IPv4 +// default route 0.0.0.0/0 via 88.99.210.1. +// - enp0s31f6.4000: VLAN child of enp0s31f6 (linkIndex=1, vlanID=4000), +// private address 192.168.100.4/24, NO default route. +// +// Use case: a controlplane floatingIP in the private cluster subnet +// (e.g. 192.168.100.10) must be hosted on the VLAN sub-interface, not +// on the public default-route NIC. Today the multi-doc renderer +// hardcodes Layer2VIPConfig.link to the IPv4-default-route link, which +// puts the VIP on enp0s31f6 — wrong for this topology. This fixture is +// the reproduction case for that bug. +func hetznerPublicNICWithPrivateVLANLookup() func(string, string, string) (map[string]any, error) { + publicNIC := map[string]any{ + "metadata": map[string]any{"id": "enp0s31f6"}, "spec": map[string]any{ "kind": "physical", "index": 1, - "hardwareAddr": "aa:bb:cc:00:00:01", - "busPath": "pci-0000:00:1f.0", + "hardwareAddr": "aa:bb:cc:00:01:01", + "busPath": "pci-0000:00:1f.6", + "mtu": 1500, + }, + } + privateVLAN := map[string]any{ + "metadata": map[string]any{"id": "enp0s31f6.4000"}, + "spec": map[string]any{ + "kind": "vlan", + "index": 2, + "linkIndex": 1, + "vlan": map[string]any{"vlanID": 4000}, + "mtu": 1500, }, } routesList := map[string]any{ @@ -4449,10 +4482,11 @@ func legacyInterfacesInRunningConfigLookup() func(string, string, string) (map[s map[string]any{ "spec": map[string]any{ "dst": "", - "gateway": "192.168.201.1", - "outLinkName": "eth0", + "gateway": "88.99.210.1", + "outLinkName": "enp0s31f6", "family": "inet4", "table": "main", + "priority": 100, }, }, }, @@ -4460,35 +4494,19 @@ func legacyInterfacesInRunningConfigLookup() func(string, string, string) (map[s linksList := map[string]any{ "apiVersion": "v1", "kind": "List", - "items": []any{eth0}, + "items": []any{publicNIC, privateVLAN}, } addressesList := map[string]any{ "apiVersion": "v1", "kind": "List", "items": []any{ - map[string]any{"spec": map[string]any{"linkName": "eth0", "address": "192.168.201.10/24", "family": "inet4", "scope": "global"}}, - }, - } - machineConfig := map[string]any{ - "spec": map[string]any{ - "machine": map[string]any{ - "network": map[string]any{ - "interfaces": []any{ - map[string]any{ - "interface": "eth0", - "mtu": 9000, - "vlans": []any{ - map[string]any{"vlanId": 4000, "addresses": []any{"192.168.100.2/24"}}, - }, - }, - }, - }, - }, + map[string]any{"spec": map[string]any{"linkName": "enp0s31f6", "address": "88.99.210.37/26", "family": "inet4", "scope": "global"}}, + map[string]any{"spec": map[string]any{"linkName": "enp0s31f6.4000", "address": "192.168.100.4/24", "family": "inet4", "scope": "global"}}, }, } nodeDefault := map[string]any{ "spec": map[string]any{ - "addresses": []any{"192.168.201.10/24"}, + "addresses": []any{"192.168.100.4/24"}, }, } resolvers := map[string]any{ @@ -4496,17 +4514,21 @@ func legacyInterfacesInRunningConfigLookup() func(string, string, string) (map[s "dnsServers": []any{"8.8.8.8"}, }, } + return func(resource, _, id string) (map[string]any, error) { switch resource { case "routes": return routesList, nil case "links": - if id == "eth0" { - return eth0, nil - } - if id == "" { + switch id { + case "enp0s31f6": + return publicNIC, nil + case "enp0s31f6.4000": + return privateVLAN, nil + case "": return linksList, nil } + return map[string]any{}, nil case "addresses": return addressesList, nil @@ -4518,60 +4540,42 @@ func legacyInterfacesInRunningConfigLookup() func(string, string, string) (map[s if id == "resolvers" { return resolvers, nil } - case "machineconfig": - if id == "v1alpha1" { - return machineConfig, nil - } } + return map[string]any{}, nil } } -// bondWithSlavesLookup returns a lookup fixture for a node where two -// physical NICs (eth0 + eth1) are enrolled into a bond master bond0. -// Mirrors the Talos representation: the slaves expose their busPath -// (so the regex matches them as "physical") AND have spec.slaveKind -// set ("bond"), which configurable_link_names uses to filter them -// out of the iteration. Without that filter the renderer would emit -// LinkConfig for each slave alongside the master's BondConfig and -// Talos would reject the conflicting declarations. -func bondWithSlavesLookup() func(string, string, string) (map[string]any, error) { - eth0 := map[string]any{ - "metadata": map[string]any{"id": "eth0"}, +// hetznerWithNilScopeAddressLookup mirrors hetznerPublicNICWithPrivateVLANLookup +// but adds a configurable-link entry whose .spec.scope is nil +// (no scope field) AND whose CIDR happens to contain the +// operator-declared floatingIP. Filter 2 in +// link_name_for_address claims to skip these entries; this +// fixture pins the guardrail. Real Talos COSI always sets +// scope, so the entry is synthetic — but the looser variant +// (only toString'd check) would let a nil-scope entry through +// as the literal "" string, which is neither "" nor in the +// skip-list. A future refactor that drops the raw-truthy guard +// must surface here. +func hetznerWithNilScopeAddressLookup() func(string, string, string) (map[string]any, error) { + publicNIC := map[string]any{ + "metadata": map[string]any{"id": "enp0s31f6"}, "spec": map[string]any{ "kind": "physical", "index": 1, - "hardwareAddr": "aa:bb:cc:00:00:01", - "busPath": "pci-0000:00:1f.0", - "slaveKind": "bond", - "masterIndex": 3, - "mtu": 9000, - }, - } - eth1 := map[string]any{ - "metadata": map[string]any{"id": "eth1"}, - "spec": map[string]any{ - "kind": "physical", - "index": 2, - "hardwareAddr": "aa:bb:cc:00:00:02", - "busPath": "pci-0000:00:1f.1", - "slaveKind": "bond", - "masterIndex": 3, - "mtu": 9000, + "hardwareAddr": "aa:bb:cc:00:01:01", + "busPath": "pci-0000:00:1f.6", + "mtu": 1500, }, } - bond0 := map[string]any{ - "metadata": map[string]any{"id": "bond0"}, + privateVLAN := map[string]any{ + "metadata": map[string]any{"id": "enp0s31f6.4000"}, "spec": map[string]any{ - "kind": "bond", - "index": 3, - "hardwareAddr": "aa:bb:cc:00:00:01", - "mtu": 9000, - "bondMaster": map[string]any{ - "mode": "802.3ad", - "xmitHashPolicy": "layer2+3", - "miimon": 100, - }, + "kind": "vlan", + "index": 2, + "linkIndex": 1, + "vlan": map[string]any{"vlanID": 4000}, + "mtu": 1500, }, } routesList := map[string]any{ @@ -4581,11 +4585,10 @@ func bondWithSlavesLookup() func(string, string, string) (map[string]any, error) map[string]any{ "spec": map[string]any{ "dst": "", - "gateway": "192.168.201.1", - "outLinkName": "bond0", + "gateway": "88.99.210.1", + "outLinkName": "enp0s31f6", "family": "inet4", "table": "main", - "priority": 100, }, }, }, @@ -4593,18 +4596,30 @@ func bondWithSlavesLookup() func(string, string, string) (map[string]any, error) linksList := map[string]any{ "apiVersion": "v1", "kind": "List", - "items": []any{eth0, eth1, bond0}, + "items": []any{publicNIC, privateVLAN}, } addressesList := map[string]any{ "apiVersion": "v1", "kind": "List", "items": []any{ - map[string]any{"spec": map[string]any{"linkName": "bond0", "address": "192.168.201.10/24", "family": "inet4", "scope": "global"}}, + map[string]any{"spec": map[string]any{"linkName": "enp0s31f6", "address": "88.99.210.37/26", "family": "inet4", "scope": "global"}}, + // VLAN child has TWO addresses on it: a global-scope + // /24 (cluster network) and a NIL-scope /16 covering + // the same target. Filter 2 must skip the nil-scope + // entry — otherwise longest-prefix-match would still + // land on enp0s31f6.4000 (same link), but a different + // fixture where the nil-scope CIDR sits on a less- + // specific link could let it win selection by virtue + // of being matched at all. + map[string]any{"spec": map[string]any{"linkName": "enp0s31f6.4000", "address": "192.168.100.4/24", "family": "inet4", "scope": "global"}}, + // scope key absent entirely — Filter 2's raw-truthy + // check on .spec.scope rejects this entry. + map[string]any{"spec": map[string]any{"linkName": "enp0s31f6.4000", "address": "192.168.0.5/16", "family": "inet4"}}, }, } nodeDefault := map[string]any{ "spec": map[string]any{ - "addresses": []any{"192.168.201.10/24"}, + "addresses": []any{"192.168.100.4/24"}, }, } resolvers := map[string]any{ @@ -4612,21 +4627,21 @@ func bondWithSlavesLookup() func(string, string, string) (map[string]any, error) "dnsServers": []any{"8.8.8.8"}, }, } + return func(resource, _, id string) (map[string]any, error) { switch resource { case "routes": return routesList, nil case "links": switch id { - case "eth0": - return eth0, nil - case "eth1": - return eth1, nil - case "bond0": - return bond0, nil + case "enp0s31f6": + return publicNIC, nil + case "enp0s31f6.4000": + return privateVLAN, nil case "": return linksList, nil } + return map[string]any{}, nil case "addresses": return addressesList, nil @@ -4639,18 +4654,22 @@ func bondWithSlavesLookup() func(string, string, string) (map[string]any, error) return resolvers, nil } } + return map[string]any{}, nil } } -// bondWithoutBondMasterLookup returns a lookup fixture for a bond -// link where the bondMaster sub-resource is missing or partial -// (real Talos sometimes returns this on freshly-created bonds where -// the master controller hasn't filled the spec yet). The renderer -// must gate every BondMaster field on its presence so the rendered -// BondConfig stays valid YAML — without the gate, missing fields -// surfaced as `bondMode: ` and broke the parse. -func bondWithoutBondMasterLookup() func(string, string, string) (map[string]any, error) { +// twoConfigurableLinksSamePrefixLookup pins tie-break behavior +// for link_name_for_address when two configurable links both +// carry CIDRs of the same prefix length and both contain the +// floatingIP. Current contract: iteration order over the COSI +// addresses list decides — strict-gt comparator means the first +// match in iteration order wins. This fixture lists eth0's +// 192.168.100.0/24 first and eth1's 192.168.100.0/24 second. +// A future refactor that flips the comparator to ge (or sorts +// the addresses table differently) silently changes behavior; +// pinning the current iteration-order contract catches it. +func twoConfigurableLinksSamePrefixLookup() func(string, string, string) (map[string]any, error) { eth0 := map[string]any{ "metadata": map[string]any{"id": "eth0"}, "spec": map[string]any{ @@ -4658,16 +4677,17 @@ func bondWithoutBondMasterLookup() func(string, string, string) (map[string]any, "index": 1, "hardwareAddr": "aa:bb:cc:00:00:01", "busPath": "pci-0000:00:1f.0", - "slaveKind": "bond", - "masterIndex": 2, + "mtu": 1500, }, } - bond0 := map[string]any{ - "metadata": map[string]any{"id": "bond0"}, + eth1 := map[string]any{ + "metadata": map[string]any{"id": "eth1"}, "spec": map[string]any{ - "kind": "bond", + "kind": "physical", "index": 2, - "hardwareAddr": "aa:bb:cc:00:00:01", + "hardwareAddr": "aa:bb:cc:00:00:02", + "busPath": "pci-0000:00:1f.1", + "mtu": 1500, }, } routesList := map[string]any{ @@ -4677,8 +4697,8 @@ func bondWithoutBondMasterLookup() func(string, string, string) (map[string]any, map[string]any{ "spec": map[string]any{ "dst": "", - "gateway": "192.168.201.1", - "outLinkName": "bond0", + "gateway": "192.168.100.1", + "outLinkName": "eth0", "family": "inet4", "table": "main", }, @@ -4688,18 +4708,24 @@ func bondWithoutBondMasterLookup() func(string, string, string) (map[string]any, linksList := map[string]any{ "apiVersion": "v1", "kind": "List", - "items": []any{eth0, bond0}, + "items": []any{eth0, eth1}, } addressesList := map[string]any{ "apiVersion": "v1", "kind": "List", "items": []any{ - map[string]any{"spec": map[string]any{"linkName": "bond0", "address": "192.168.201.10/24", "family": "inet4", "scope": "global"}}, + // Both links carry the SAME /24 and both contain the + // floatingIP. eth0 listed FIRST so the strict-gt + // comparator never overwrites the best-so-far with + // eth1's equal-prefix entry — eth0 wins by iteration + // order. + map[string]any{"spec": map[string]any{"linkName": "eth0", "address": "192.168.100.10/24", "family": "inet4", "scope": "global"}}, + map[string]any{"spec": map[string]any{"linkName": "eth1", "address": "192.168.100.11/24", "family": "inet4", "scope": "global"}}, }, } nodeDefault := map[string]any{ "spec": map[string]any{ - "addresses": []any{"192.168.201.10/24"}, + "addresses": []any{"192.168.100.10/24"}, }, } resolvers := map[string]any{ @@ -4707,6 +4733,7 @@ func bondWithoutBondMasterLookup() func(string, string, string) (map[string]any, "dnsServers": []any{"8.8.8.8"}, }, } + return func(resource, _, id string) (map[string]any, error) { switch resource { case "routes": @@ -4715,11 +4742,12 @@ func bondWithoutBondMasterLookup() func(string, string, string) (map[string]any, switch id { case "eth0": return eth0, nil - case "bond0": - return bond0, nil + case "eth1": + return eth1, nil case "": return linksList, nil } + return map[string]any{}, nil case "addresses": return addressesList, nil @@ -4732,30 +4760,42 @@ func bondWithoutBondMasterLookup() func(string, string, string) (map[string]any, return resolvers, nil } } + return map[string]any{}, nil } } -// bridgeLookup returns a lookup fixture for a node with a routed -// physical NIC eth0 plus a bridge br0. The renderer must emit -// LinkConfig for eth0 and SKIP the bridge entirely (until -// BridgeConfig support lands) rather than emit a wrong-kind -// LinkConfig name: br0. -func bridgeLookup() func(string, string, string) (map[string]any, error) { - eth0 := map[string]any{ - "metadata": map[string]any{"id": "eth0"}, +// hetznerWithLinkScopedAddressLookup pins that Filter 2 in +// link_name_for_address (scope must be set and not host/link/nowhere) +// actually fires. Without the filter, a link-scoped /16 covering the +// floatingIP would win over the global-scope /24 by longest-prefix +// match. Configurable VLAN child carries both: +// - 192.168.100.4/24 scope=global (the real cluster subnet) +// - 169.254.0.1/16 scope=link (link-local, larger CIDR) +// +// floatingIP is 169.254.0.5 — inside the link-local /16, NOT inside +// the global /24. With Filter 2 the helper skips the link-scoped +// entry and the helper returns "" so the caller falls back to the +// default-route link. +func hetznerWithLinkScopedAddressLookup() func(string, string, string) (map[string]any, error) { + publicNIC := map[string]any{ + "metadata": map[string]any{"id": "enp0s31f6"}, "spec": map[string]any{ "kind": "physical", "index": 1, - "hardwareAddr": "aa:bb:cc:00:00:01", - "busPath": "pci-0000:00:1f.0", + "hardwareAddr": "aa:bb:cc:00:01:01", + "busPath": "pci-0000:00:1f.6", + "mtu": 1500, }, } - br0 := map[string]any{ - "metadata": map[string]any{"id": "br0"}, + privateVLAN := map[string]any{ + "metadata": map[string]any{"id": "enp0s31f6.4000"}, "spec": map[string]any{ - "kind": "bridge", - "index": 2, + "kind": "vlan", + "index": 2, + "linkIndex": 1, + "vlan": map[string]any{"vlanID": 4000}, + "mtu": 1500, }, } routesList := map[string]any{ @@ -4765,10 +4805,11 @@ func bridgeLookup() func(string, string, string) (map[string]any, error) { map[string]any{ "spec": map[string]any{ "dst": "", - "gateway": "192.168.201.1", - "outLinkName": "eth0", + "gateway": "88.99.210.1", + "outLinkName": "enp0s31f6", "family": "inet4", "table": "main", + "priority": 100, }, }, }, @@ -4776,18 +4817,23 @@ func bridgeLookup() func(string, string, string) (map[string]any, error) { linksList := map[string]any{ "apiVersion": "v1", "kind": "List", - "items": []any{eth0, br0}, + "items": []any{publicNIC, privateVLAN}, } addressesList := map[string]any{ "apiVersion": "v1", "kind": "List", "items": []any{ - map[string]any{"spec": map[string]any{"linkName": "eth0", "address": "192.168.201.10/24", "family": "inet4", "scope": "global"}}, + map[string]any{"spec": map[string]any{"linkName": "enp0s31f6", "address": "88.99.210.37/26", "family": "inet4", "scope": "global"}}, + map[string]any{"spec": map[string]any{"linkName": "enp0s31f6.4000", "address": "192.168.100.4/24", "family": "inet4", "scope": "global"}}, + // Link-local /16 on the same configurable VLAN. Filter 2 + // must skip this entry; otherwise the longest-prefix + // comparator would pick it for a 169.254.0.x VIP. + map[string]any{"spec": map[string]any{"linkName": "enp0s31f6.4000", "address": "169.254.0.1/16", "family": "inet4", "scope": "link"}}, }, } nodeDefault := map[string]any{ "spec": map[string]any{ - "addresses": []any{"192.168.201.10/24"}, + "addresses": []any{"192.168.100.4/24"}, }, } resolvers := map[string]any{ @@ -4795,19 +4841,21 @@ func bridgeLookup() func(string, string, string) (map[string]any, error) { "dnsServers": []any{"8.8.8.8"}, }, } + return func(resource, _, id string) (map[string]any, error) { switch resource { case "routes": return routesList, nil case "links": switch id { - case "eth0": - return eth0, nil - case "br0": - return br0, nil + case "enp0s31f6": + return publicNIC, nil + case "enp0s31f6.4000": + return privateVLAN, nil case "": return linksList, nil } + return map[string]any{}, nil case "addresses": return addressesList, nil @@ -4820,57 +4868,73 @@ func bridgeLookup() func(string, string, string) (map[string]any, error) { return resolvers, nil } } + return map[string]any{}, nil } } -// vipActiveOnLinkLookup returns a lookup fixture for a node where -// the configured floatingIP is currently active on eth0 — discovery -// reports two global-scope addresses on the link: the permanent -// address and the VIP. The Talos VIP operator does not mark the VIP -// address with any distinguishing field, so the chart must filter -// it out by matching against the operator-declared floatingIP. -func vipActiveOnLinkLookup() func(string, string, string) (map[string]any, error) { - eth0 := map[string]any{ - "metadata": map[string]any{"id": "eth0"}, +// noDefaultRouteWithSubnetMatchLookup pins the contract that the +// discovery-derived Layer2VIPConfig path emits even when discovery +// has not yet resolved an IPv4 default route, as long as +// link_name_for_address resolves a configurable link whose subnet +// contains the floatingIP. The fallback to $defaultLinkName fires +// only when the subnet-match returns empty; a successful match +// must not be gated on the default route being known. +// +// Two real-world reasons this matters: a fresh-boot node before the +// gateway is reachable but with addresses already plumbed on a +// VLAN child, and a policy-routing-only topology where there is no +// "default" route at all but each network has its own scoped route +// table. In both, the cluster subnet is reachable on a +// configurable link and the VIP belongs there. +// +// Topology: physical NIC carries a public address with no default +// route, VLAN child carries 192.168.100.4/24. floatingIP +// 192.168.100.10. Expected: Layer2VIPConfig.link=enp0s31f6.4000. +func noDefaultRouteWithSubnetMatchLookup() func(string, string, string) (map[string]any, error) { + publicNIC := map[string]any{ + "metadata": map[string]any{"id": "enp0s31f6"}, "spec": map[string]any{ "kind": "physical", "index": 1, - "hardwareAddr": "aa:bb:cc:00:00:01", - "busPath": "pci-0000:00:1f.0", + "hardwareAddr": "aa:bb:cc:00:01:01", + "busPath": "pci-0000:00:1f.6", + "mtu": 1500, + }, + } + privateVLAN := map[string]any{ + "metadata": map[string]any{"id": "enp0s31f6.4000"}, + "spec": map[string]any{ + "kind": "vlan", + "index": 2, + "linkIndex": 1, + "vlan": map[string]any{"vlanID": 4000}, + "mtu": 1500, }, } + // Empty routes list — no default route, no policy routes. The + // addresses table below carries the cluster subnet anyway. routesList := map[string]any{ "apiVersion": "v1", "kind": "List", - "items": []any{ - map[string]any{ - "spec": map[string]any{ - "dst": "", - "gateway": "192.168.201.1", - "outLinkName": "eth0", - "family": "inet4", - "table": "main", - }, - }, - }, + "items": []any{}, } linksList := map[string]any{ "apiVersion": "v1", "kind": "List", - "items": []any{eth0}, + "items": []any{publicNIC, privateVLAN}, } addressesList := map[string]any{ "apiVersion": "v1", "kind": "List", "items": []any{ - map[string]any{"spec": map[string]any{"linkName": "eth0", "address": "192.168.201.10/24", "family": "inet4", "scope": "global"}}, - map[string]any{"spec": map[string]any{"linkName": "eth0", "address": "192.168.201.5/32", "family": "inet4", "scope": "global"}}, + map[string]any{"spec": map[string]any{"linkName": "enp0s31f6", "address": "88.99.210.37/26", "family": "inet4", "scope": "global"}}, + map[string]any{"spec": map[string]any{"linkName": "enp0s31f6.4000", "address": "192.168.100.4/24", "family": "inet4", "scope": "global"}}, }, } nodeDefault := map[string]any{ "spec": map[string]any{ - "addresses": []any{"192.168.201.10/24"}, + "addresses": []any{"192.168.100.4/24"}, }, } resolvers := map[string]any{ @@ -4878,15 +4942,1266 @@ func vipActiveOnLinkLookup() func(string, string, string) (map[string]any, error "dnsServers": []any{"8.8.8.8"}, }, } + return func(resource, _, id string) (map[string]any, error) { switch resource { case "routes": return routesList, nil case "links": - if id == "eth0" { - return eth0, nil - } - if id == "" { + switch id { + case "enp0s31f6": + return publicNIC, nil + case "enp0s31f6.4000": + return privateVLAN, nil + case "": + return linksList, nil + } + + return map[string]any{}, nil + case "addresses": + return addressesList, nil + case "nodeaddress": + if id == "default" { + return nodeDefault, nil + } + case "resolvers": + if id == "resolvers" { + return resolvers, nil + } + } + + return map[string]any{}, nil + } +} + +// bridgeWithClusterSubnetLookup is the BridgeConfig analogue of +// hetznerPublicNICWithPrivateVLANLookup: a node where the cluster +// network is configured on a bridge link (e.g. a node that bridges +// physical NICs into one logical "uplink" carrying the cluster +// subnet) and the floatingIP belongs inside that bridge's subnet. +// +// With BridgeConfig emission landed the bridge is rendered as a +// typed document and the discovery-derived Layer2VIPConfig can +// safely pin link=br0. Without BridgeConfig (the prior shape) the +// VIP would have dangled on a link with no surrounding network +// document. +// +// Topology: eth0 (physical, slaveKind=bridge, masterIndex=2) + +// br0 (bridge, index=2) carrying the cluster subnet +// 10.5.0.10/24 and the IPv4 default route via 10.5.0.1. +func bridgeWithClusterSubnetLookup() func(string, string, string) (map[string]any, error) { + eth0 := map[string]any{ + "metadata": map[string]any{"id": "eth0"}, + "spec": map[string]any{ + "kind": "physical", + "index": 1, + "hardwareAddr": "aa:bb:cc:00:00:01", + "busPath": "pci-0000:00:1f.0", + "slaveKind": "bridge", + "masterIndex": 2, + }, + } + br0 := map[string]any{ + "metadata": map[string]any{"id": "br0"}, + "spec": map[string]any{ + "kind": "bridge", + "index": 2, + "bridgeMaster": map[string]any{ + "stp": map[string]any{"enabled": true}, + "vlan": map[string]any{"filteringEnabled": true}, + }, + }, + } + routesList := map[string]any{ + "apiVersion": "v1", + "kind": "List", + "items": []any{ + map[string]any{ + "spec": map[string]any{ + "dst": "", + "gateway": "10.5.0.1", + "outLinkName": "br0", + "family": "inet4", + "table": "main", + }, + }, + }, + } + linksList := map[string]any{ + "apiVersion": "v1", + "kind": "List", + "items": []any{eth0, br0}, + } + addressesList := map[string]any{ + "apiVersion": "v1", + "kind": "List", + "items": []any{ + map[string]any{"spec": map[string]any{"linkName": "br0", "address": "10.5.0.10/24", "family": "inet4", "scope": "global"}}, + }, + } + nodeDefault := map[string]any{ + "spec": map[string]any{ + "addresses": []any{"10.5.0.10/24"}, + }, + } + resolvers := map[string]any{ + "spec": map[string]any{ + "dnsServers": []any{"8.8.8.8"}, + }, + } + + return func(resource, _, id string) (map[string]any, error) { + switch resource { + case "routes": + return routesList, nil + case "links": + switch id { + case "eth0": + return eth0, nil + case "br0": + return br0, nil + case "": + return linksList, nil + } + + return map[string]any{}, nil + case "addresses": + return addressesList, nil + case "nodeaddress": + if id == "default" { + return nodeDefault, nil + } + case "resolvers": + if id == "resolvers" { + return resolvers, nil + } + } + + return map[string]any{}, nil + } +} + +// bridgeWithVLANOnlyLookup is a slim BridgeConfig fixture where +// spec.bridgeMaster carries ONLY a vlan.filteringEnabled setting +// (no stp). The contract under test: the chart emits the +// BridgeConfig.vlan block even when stp is unset, and does NOT +// emit a stp block at all. Together with bridgeWithSTPOnlyLookup +// this pins the independence of the two sub-blocks against a +// future refactor that accidentally nests one inside the other. +func bridgeWithVLANOnlyLookup() func(string, string, string) (map[string]any, error) { + br0 := map[string]any{ + "metadata": map[string]any{"id": "br0"}, + "spec": map[string]any{ + "kind": "bridge", + "index": 1, + "bridgeMaster": map[string]any{ + "vlan": map[string]any{"filteringEnabled": true}, + }, + }, + } + + return bridgeOnlyLookup(br0) +} + +// bridgeWithSTPOnlyLookup is the stp-only counterpart of +// bridgeWithVLANOnlyLookup. Pins that the BridgeConfig.stp block +// emits even when vlan is unset. +func bridgeWithSTPOnlyLookup() func(string, string, string) (map[string]any, error) { + br0 := map[string]any{ + "metadata": map[string]any{"id": "br0"}, + "spec": map[string]any{ + "kind": "bridge", + "index": 1, + "bridgeMaster": map[string]any{ + "stp": map[string]any{"enabled": true}, + }, + }, + } + + return bridgeOnlyLookup(br0) +} + +// bridgeOnlyLookup builds a single-bridge lookup fixture given a +// pre-shaped br0 link map. The bridge owns the IPv4 default route +// at 10.5.0.1 via address 10.5.0.10/24. No physical NIC, no +// slaves — keeps the fixture tight around the BridgeConfig +// sub-block contracts the callers exercise. +func bridgeOnlyLookup(br0 map[string]any) func(string, string, string) (map[string]any, error) { + routesList := map[string]any{ + "apiVersion": "v1", + "kind": "List", + "items": []any{ + map[string]any{ + "spec": map[string]any{ + "dst": "", + "gateway": "10.5.0.1", + "outLinkName": "br0", + "family": "inet4", + "table": "main", + }, + }, + }, + } + linksList := map[string]any{ + "apiVersion": "v1", + "kind": "List", + "items": []any{br0}, + } + addressesList := map[string]any{ + "apiVersion": "v1", + "kind": "List", + "items": []any{ + map[string]any{"spec": map[string]any{"linkName": "br0", "address": "10.5.0.10/24", "family": "inet4", "scope": "global"}}, + }, + } + nodeDefault := map[string]any{ + "spec": map[string]any{ + "addresses": []any{"10.5.0.10/24"}, + }, + } + resolvers := map[string]any{ + "spec": map[string]any{ + "dnsServers": []any{"8.8.8.8"}, + }, + } + + return func(resource, _, id string) (map[string]any, error) { + switch resource { + case "routes": + return routesList, nil + case "links": + switch id { + case "br0": + return br0, nil + case "": + return linksList, nil + } + + return map[string]any{}, nil + case "addresses": + return addressesList, nil + case "nodeaddress": + if id == "default" { + return nodeDefault, nil + } + case "resolvers": + if id == "resolvers" { + return resolvers, nil + } + } + + return map[string]any{}, nil + } +} + +// defaultRouteOnNonConfigurableLinkLookup pins the contract that +// the default-route-link fallback in the discovery-derived +// Layer2VIPConfig path must also pass the configurable-link gate. +// A node whose IPv4 default route sits on a Wireguard / slave / +// other unmanaged link must NOT pin the VIP there — the chart +// emits no per-link document for such links, so the VIP would +// dangle on a link the chart never configures. +// +// Topology: physical NIC enp0s31f6 with a private address but no +// default route; Wireguard wg0 carries the IPv4 default route. +// floatingIP 10.99.99.99 falls outside every discovered subnet, so +// link_name_for_address returns empty and the chart falls back — +// but the fallback target wg0 is non-configurable, so the chart +// MUST NOT emit Layer2VIPConfig at all (matches the prior "no +// match, no emit" silent-skip path). +func defaultRouteOnNonConfigurableLinkLookup() func(string, string, string) (map[string]any, error) { + publicNIC := map[string]any{ + "metadata": map[string]any{"id": "enp0s31f6"}, + "spec": map[string]any{ + "kind": "physical", + "index": 1, + "hardwareAddr": "aa:bb:cc:00:01:01", + "busPath": "pci-0000:00:1f.6", + "mtu": 1500, + }, + } + wireguard := map[string]any{ + "metadata": map[string]any{"id": "wg0"}, + "spec": map[string]any{ + "kind": "ether", + "index": 3, + "mtu": 1420, + }, + } + routesList := map[string]any{ + "apiVersion": "v1", + "kind": "List", + "items": []any{ + map[string]any{ + "spec": map[string]any{ + "dst": "", + "gateway": "10.244.0.1", + "outLinkName": "wg0", + "family": "inet4", + "table": "main", + "priority": 100, + }, + }, + }, + } + linksList := map[string]any{ + "apiVersion": "v1", + "kind": "List", + "items": []any{publicNIC, wireguard}, + } + addressesList := map[string]any{ + "apiVersion": "v1", + "kind": "List", + "items": []any{ + map[string]any{"spec": map[string]any{"linkName": "enp0s31f6", "address": "192.168.100.4/24", "family": "inet4", "scope": "global"}}, + map[string]any{"spec": map[string]any{"linkName": "wg0", "address": "10.244.0.5/24", "family": "inet4", "scope": "global"}}, + }, + } + nodeDefault := map[string]any{ + "spec": map[string]any{ + "addresses": []any{"192.168.100.4/24"}, + }, + } + resolvers := map[string]any{ + "spec": map[string]any{ + "dnsServers": []any{"8.8.8.8"}, + }, + } + + return func(resource, _, id string) (map[string]any, error) { + switch resource { + case "routes": + return routesList, nil + case "links": + switch id { + case "enp0s31f6": + return publicNIC, nil + case "wg0": + return wireguard, nil + case "": + return linksList, nil + } + + return map[string]any{}, nil + case "addresses": + return addressesList, nil + case "nodeaddress": + if id == "default" { + return nodeDefault, nil + } + case "resolvers": + if id == "resolvers" { + return resolvers, nil + } + } + + return map[string]any{}, nil + } +} + +// hetznerWithWireguardLookup mirrors hetznerPublicNICWithPrivateVLANLookup +// but adds a Wireguard interface (wg0) carrying a private subnet that +// happens to encompass the floatingIP. Pins that an address on a +// non-configurable link cannot win VIP-link selection — the multi-doc +// loop only emits LinkConfig for links in +// talm.discovered.configurable_link_names, so a VIP pinned to wg0 +// would have no surrounding LinkConfig and would race the link's +// own address management on apply. +// +// Wireguard is the canonical "real-world non-configurable link" because +// it has no busPath (kernel-managed, no PCI) and reports a kind that +// is neither bond/vlan/bridge. CNI bridges in production typically +// report kind="bridge" and ARE in configurable_link_names today — +// using Wireguard here keeps the fixture honest about what the filter +// actually catches. Operators putting a VIP inside a pod CIDR is +// already operationally wrong regardless of chart behavior. +// +// Selection logic must filter on the configurable-link set BEFORE +// running CIDR-membership; this fixture's floatingIP (10.244.0.5) is +// inside wg0's subnet but wg0 isn't configurable, so the helper must +// skip wg0 entirely and fall back to the default-route link +// (enp0s31f6). +func hetznerWithWireguardLookup() func(string, string, string) (map[string]any, error) { + publicNIC := map[string]any{ + "metadata": map[string]any{"id": "enp0s31f6"}, + "spec": map[string]any{ + "kind": "physical", + "index": 1, + "hardwareAddr": "aa:bb:cc:00:01:01", + "busPath": "pci-0000:00:1f.6", + "mtu": 1500, + }, + } + // Wireguard interface: no busPath (kernel-managed, no PCI), kind + // is "ether" (Wireguard typically reports this on Linux), name + // does not match the physical-NIC regex. Neither isPhysical nor + // isVirtual fires in configurable_link_names, so wg0 is excluded. + wireguard := map[string]any{ + "metadata": map[string]any{"id": "wg0"}, + "spec": map[string]any{ + "kind": "ether", + "index": 3, + "mtu": 1420, + }, + } + privateVLAN := map[string]any{ + "metadata": map[string]any{"id": "enp0s31f6.4000"}, + "spec": map[string]any{ + "kind": "vlan", + "index": 2, + "linkIndex": 1, + "vlan": map[string]any{"vlanID": 4000}, + "mtu": 1500, + }, + } + routesList := map[string]any{ + "apiVersion": "v1", + "kind": "List", + "items": []any{ + map[string]any{ + "spec": map[string]any{ + "dst": "", + "gateway": "88.99.210.1", + "outLinkName": "enp0s31f6", + "family": "inet4", + "table": "main", + "priority": 100, + }, + }, + }, + } + linksList := map[string]any{ + "apiVersion": "v1", + "kind": "List", + "items": []any{publicNIC, privateVLAN, wireguard}, + } + addressesList := map[string]any{ + "apiVersion": "v1", + "kind": "List", + "items": []any{ + map[string]any{"spec": map[string]any{"linkName": "enp0s31f6", "address": "88.99.210.37/26", "family": "inet4", "scope": "global"}}, + map[string]any{"spec": map[string]any{"linkName": "enp0s31f6.4000", "address": "192.168.100.4/24", "family": "inet4", "scope": "global"}}, + // Wireguard subnet on wg0. The floatingIP (10.244.0.5 in + // the test) is inside this CIDR — but wg0 is not in + // configurable_link_names, so the helper must NOT pick it. + map[string]any{"spec": map[string]any{"linkName": "wg0", "address": "10.244.0.1/16", "family": "inet4", "scope": "global"}}, + }, + } + nodeDefault := map[string]any{ + "spec": map[string]any{ + "addresses": []any{"192.168.100.4/24"}, + }, + } + resolvers := map[string]any{ + "spec": map[string]any{ + "dnsServers": []any{"8.8.8.8"}, + }, + } + + return func(resource, _, id string) (map[string]any, error) { + switch resource { + case "routes": + return routesList, nil + case "links": + switch id { + case "enp0s31f6": + return publicNIC, nil + case "enp0s31f6.4000": + return privateVLAN, nil + case "wg0": + return wireguard, nil + case "": + return linksList, nil + } + + return map[string]any{}, nil + case "addresses": + return addressesList, nil + case "nodeaddress": + if id == "default" { + return nodeDefault, nil + } + case "resolvers": + if id == "resolvers" { + return resolvers, nil + } + } + + return map[string]any{}, nil + } +} + +// overlappingSubnetsLookup pins the longest-prefix-match contract. +// Two configurable links carry overlapping subnets that both contain +// the floatingIP: enp0s31f6 with a /16 (192.168.0.0/16) and +// enp0s31f6.4000 with a /24 (192.168.100.0/24). The VIP must land on +// the more specific subnet — otherwise iteration order silently +// decides, which is whatever discovery happens to list first. +func overlappingSubnetsLookup() func(string, string, string) (map[string]any, error) { + publicNIC := map[string]any{ + "metadata": map[string]any{"id": "enp0s31f6"}, + "spec": map[string]any{ + "kind": "physical", + "index": 1, + "hardwareAddr": "aa:bb:cc:00:01:01", + "busPath": "pci-0000:00:1f.6", + "mtu": 1500, + }, + } + privateVLAN := map[string]any{ + "metadata": map[string]any{"id": "enp0s31f6.4000"}, + "spec": map[string]any{ + "kind": "vlan", + "index": 2, + "linkIndex": 1, + "vlan": map[string]any{"vlanID": 4000}, + "mtu": 1500, + }, + } + routesList := map[string]any{ + "apiVersion": "v1", + "kind": "List", + "items": []any{ + map[string]any{ + "spec": map[string]any{ + "dst": "", + "gateway": "192.168.0.1", + "outLinkName": "enp0s31f6", + "family": "inet4", + "table": "main", + "priority": 100, + }, + }, + }, + } + linksList := map[string]any{ + "apiVersion": "v1", + "kind": "List", + "items": []any{publicNIC, privateVLAN}, + } + addressesList := map[string]any{ + "apiVersion": "v1", + "kind": "List", + "items": []any{ + // Broad /16 listed FIRST — without longest-prefix logic + // the helper would return enp0s31f6 by iteration order. + map[string]any{"spec": map[string]any{"linkName": "enp0s31f6", "address": "192.168.0.10/16", "family": "inet4", "scope": "global"}}, + map[string]any{"spec": map[string]any{"linkName": "enp0s31f6.4000", "address": "192.168.100.4/24", "family": "inet4", "scope": "global"}}, + }, + } + nodeDefault := map[string]any{ + "spec": map[string]any{ + "addresses": []any{"192.168.0.10/16"}, + }, + } + resolvers := map[string]any{ + "spec": map[string]any{ + "dnsServers": []any{"8.8.8.8"}, + }, + } + + return func(resource, _, id string) (map[string]any, error) { + switch resource { + case "routes": + return routesList, nil + case "links": + switch id { + case "enp0s31f6": + return publicNIC, nil + case "enp0s31f6.4000": + return privateVLAN, nil + case "": + return linksList, nil + } + + return map[string]any{}, nil + case "addresses": + return addressesList, nil + case "nodeaddress": + if id == "default" { + return nodeDefault, nil + } + case "resolvers": + if id == "resolvers" { + return resolvers, nil + } + } + + return map[string]any{}, nil + } +} + +// malformedAddressEntryLookup is the IPv4 Hetzner fixture with one +// extra "garbage" address entry — used to pin that a corrupt or +// future-format CIDR in the addresses table does not crash the +// entire chart render. cidrContains is lenient on parse failures +// (returns false) so the helper skips the bad entry and continues +// iterating. +func malformedAddressEntryLookup() func(string, string, string) (map[string]any, error) { + publicNIC := map[string]any{ + "metadata": map[string]any{"id": "enp0s31f6"}, + "spec": map[string]any{ + "kind": "physical", + "index": 1, + "hardwareAddr": "aa:bb:cc:00:01:01", + "busPath": "pci-0000:00:1f.6", + "mtu": 1500, + }, + } + privateVLAN := map[string]any{ + "metadata": map[string]any{"id": "enp0s31f6.4000"}, + "spec": map[string]any{ + "kind": "vlan", + "index": 2, + "linkIndex": 1, + "vlan": map[string]any{"vlanID": 4000}, + "mtu": 1500, + }, + } + routesList := map[string]any{ + "apiVersion": "v1", + "kind": "List", + "items": []any{ + map[string]any{ + "spec": map[string]any{ + "dst": "", + "gateway": "88.99.210.1", + "outLinkName": "enp0s31f6", + "family": "inet4", + "table": "main", + "priority": 100, + }, + }, + }, + } + linksList := map[string]any{ + "apiVersion": "v1", + "kind": "List", + "items": []any{publicNIC, privateVLAN}, + } + addressesList := map[string]any{ + "apiVersion": "v1", + "kind": "List", + "items": []any{ + map[string]any{"spec": map[string]any{"linkName": "enp0s31f6", "address": "88.99.210.37/26", "family": "inet4", "scope": "global"}}, + // Garbage entry sandwiched between valid ones — corrupt + // COSI state or a future Talos version emitting a + // different format must not abort the whole render. + map[string]any{"spec": map[string]any{"linkName": "enp0s31f6.4000", "address": "definitely-not-a-cidr", "family": "inet4", "scope": "global"}}, + map[string]any{"spec": map[string]any{"linkName": "enp0s31f6.4000", "address": "192.168.100.4/24", "family": "inet4", "scope": "global"}}, + }, + } + nodeDefault := map[string]any{ + "spec": map[string]any{ + "addresses": []any{"192.168.100.4/24"}, + }, + } + resolvers := map[string]any{ + "spec": map[string]any{ + "dnsServers": []any{"8.8.8.8"}, + }, + } + + return func(resource, _, id string) (map[string]any, error) { + switch resource { + case "routes": + return routesList, nil + case "links": + switch id { + case "enp0s31f6": + return publicNIC, nil + case "enp0s31f6.4000": + return privateVLAN, nil + case "": + return linksList, nil + } + + return map[string]any{}, nil + case "addresses": + return addressesList, nil + case "nodeaddress": + if id == "default" { + return nodeDefault, nil + } + case "resolvers": + if id == "resolvers" { + return resolvers, nil + } + } + + return map[string]any{}, nil + } +} + +// hetznerPublicNICWithPrivateIPv6VLANLookup is the IPv6-equivalent of +// hetznerPublicNICWithPrivateVLANLookup. The same physical / VLAN +// topology, but the private subnet is a /64 ULA and the VIP is an +// IPv6 literal. Pins that the VIP-link selection helper handles +// IPv6 just as it does IPv4 — net/netip.Prefix.Contains is family- +// agnostic, so the chart side has no per-family branches; this +// fixture exists to surface a regression that ever introduces one. +// +// IPv4 default-route stays on the public NIC (matching real-world +// dual-stack: IPv4 default goes upstream, the IPv6 ULA never has a +// default route — operators run IPv6 only between cluster nodes). +func hetznerPublicNICWithPrivateIPv6VLANLookup() func(string, string, string) (map[string]any, error) { + publicNIC := map[string]any{ + "metadata": map[string]any{"id": "enp0s31f6"}, + "spec": map[string]any{ + "kind": "physical", + "index": 1, + "hardwareAddr": "aa:bb:cc:00:01:01", + "busPath": "pci-0000:00:1f.6", + "mtu": 1500, + }, + } + privateVLAN := map[string]any{ + "metadata": map[string]any{"id": "enp0s31f6.4000"}, + "spec": map[string]any{ + "kind": "vlan", + "index": 2, + "linkIndex": 1, + "vlan": map[string]any{"vlanID": 4000}, + "mtu": 1500, + }, + } + routesList := map[string]any{ + "apiVersion": "v1", + "kind": "List", + "items": []any{ + map[string]any{ + "spec": map[string]any{ + "dst": "", + "gateway": "88.99.210.1", + "outLinkName": "enp0s31f6", + "family": "inet4", + "table": "main", + "priority": 100, + }, + }, + }, + } + linksList := map[string]any{ + "apiVersion": "v1", + "kind": "List", + "items": []any{publicNIC, privateVLAN}, + } + addressesList := map[string]any{ + "apiVersion": "v1", + "kind": "List", + "items": []any{ + map[string]any{"spec": map[string]any{"linkName": "enp0s31f6", "address": "88.99.210.37/26", "family": "inet4", "scope": "global"}}, + map[string]any{"spec": map[string]any{"linkName": "enp0s31f6.4000", "address": "2001:db8:cafe::4/64", "family": "inet6", "scope": "global"}}, + }, + } + nodeDefault := map[string]any{ + "spec": map[string]any{ + "addresses": []any{"2001:db8:cafe::4/64"}, + }, + } + resolvers := map[string]any{ + "spec": map[string]any{ + "dnsServers": []any{"8.8.8.8"}, + }, + } + + return func(resource, _, id string) (map[string]any, error) { + switch resource { + case "routes": + return routesList, nil + case "links": + switch id { + case "enp0s31f6": + return publicNIC, nil + case "enp0s31f6.4000": + return privateVLAN, nil + case "": + return linksList, nil + } + + return map[string]any{}, nil + case "addresses": + return addressesList, nil + case "nodeaddress": + if id == "default" { + return nodeDefault, nil + } + case "resolvers": + if id == "resolvers" { + return resolvers, nil + } + } + + return map[string]any{}, nil + } +} + +// legacyInterfacesInRunningConfigLookup returns a lookup fixture +// shaped like a node that was originally bootstrapped on a legacy +// chart (talosVersion v1.11) and carries non-empty +// machine.network.interfaces[] in its running MachineConfig. The +// multi-doc renderer must detect this and refuse to render rather +// than silently dropping the legacy interface block — otherwise an +// upgrade from chart v0.23 to v0.24+ would silently lose every +// user-declared address, route, and VLAN that lived under the +// legacy schema. +func legacyInterfacesInRunningConfigLookup() func(string, string, string) (map[string]any, error) { + eth0 := map[string]any{ + "metadata": map[string]any{"id": "eth0"}, + "spec": map[string]any{ + "kind": "physical", + "index": 1, + "hardwareAddr": "aa:bb:cc:00:00:01", + "busPath": "pci-0000:00:1f.0", + }, + } + routesList := map[string]any{ + "apiVersion": "v1", + "kind": "List", + "items": []any{ + map[string]any{ + "spec": map[string]any{ + "dst": "", + "gateway": "192.168.201.1", + "outLinkName": "eth0", + "family": "inet4", + "table": "main", + }, + }, + }, + } + linksList := map[string]any{ + "apiVersion": "v1", + "kind": "List", + "items": []any{eth0}, + } + addressesList := map[string]any{ + "apiVersion": "v1", + "kind": "List", + "items": []any{ + map[string]any{"spec": map[string]any{"linkName": "eth0", "address": "192.168.201.10/24", "family": "inet4", "scope": "global"}}, + }, + } + machineConfig := map[string]any{ + "spec": map[string]any{ + "machine": map[string]any{ + "network": map[string]any{ + "interfaces": []any{ + map[string]any{ + "interface": "eth0", + "mtu": 9000, + "vlans": []any{ + map[string]any{"vlanId": 4000, "addresses": []any{"192.168.100.2/24"}}, + }, + }, + }, + }, + }, + }, + } + nodeDefault := map[string]any{ + "spec": map[string]any{ + "addresses": []any{"192.168.201.10/24"}, + }, + } + resolvers := map[string]any{ + "spec": map[string]any{ + "dnsServers": []any{"8.8.8.8"}, + }, + } + return func(resource, _, id string) (map[string]any, error) { + switch resource { + case "routes": + return routesList, nil + case "links": + if id == "eth0" { + return eth0, nil + } + if id == "" { + return linksList, nil + } + return map[string]any{}, nil + case "addresses": + return addressesList, nil + case "nodeaddress": + if id == "default" { + return nodeDefault, nil + } + case "resolvers": + if id == "resolvers" { + return resolvers, nil + } + case "machineconfig": + if id == "v1alpha1" { + return machineConfig, nil + } + } + return map[string]any{}, nil + } +} + +// bondWithSlavesLookup returns a lookup fixture for a node where two +// physical NICs (eth0 + eth1) are enrolled into a bond master bond0. +// Mirrors the Talos representation: the slaves expose their busPath +// (so the regex matches them as "physical") AND have spec.slaveKind +// set ("bond"), which configurable_link_names uses to filter them +// out of the iteration. Without that filter the renderer would emit +// LinkConfig for each slave alongside the master's BondConfig and +// Talos would reject the conflicting declarations. +func bondWithSlavesLookup() func(string, string, string) (map[string]any, error) { + eth0 := map[string]any{ + "metadata": map[string]any{"id": "eth0"}, + "spec": map[string]any{ + "kind": "physical", + "index": 1, + "hardwareAddr": "aa:bb:cc:00:00:01", + "busPath": "pci-0000:00:1f.0", + "slaveKind": "bond", + "masterIndex": 3, + "mtu": 9000, + }, + } + eth1 := map[string]any{ + "metadata": map[string]any{"id": "eth1"}, + "spec": map[string]any{ + "kind": "physical", + "index": 2, + "hardwareAddr": "aa:bb:cc:00:00:02", + "busPath": "pci-0000:00:1f.1", + "slaveKind": "bond", + "masterIndex": 3, + "mtu": 9000, + }, + } + bond0 := map[string]any{ + "metadata": map[string]any{"id": "bond0"}, + "spec": map[string]any{ + "kind": "bond", + "index": 3, + "hardwareAddr": "aa:bb:cc:00:00:01", + "mtu": 9000, + "bondMaster": map[string]any{ + "mode": "802.3ad", + "xmitHashPolicy": "layer2+3", + "miimon": 100, + }, + }, + } + routesList := map[string]any{ + "apiVersion": "v1", + "kind": "List", + "items": []any{ + map[string]any{ + "spec": map[string]any{ + "dst": "", + "gateway": "192.168.201.1", + "outLinkName": "bond0", + "family": "inet4", + "table": "main", + "priority": 100, + }, + }, + }, + } + linksList := map[string]any{ + "apiVersion": "v1", + "kind": "List", + "items": []any{eth0, eth1, bond0}, + } + addressesList := map[string]any{ + "apiVersion": "v1", + "kind": "List", + "items": []any{ + map[string]any{"spec": map[string]any{"linkName": "bond0", "address": "192.168.201.10/24", "family": "inet4", "scope": "global"}}, + }, + } + nodeDefault := map[string]any{ + "spec": map[string]any{ + "addresses": []any{"192.168.201.10/24"}, + }, + } + resolvers := map[string]any{ + "spec": map[string]any{ + "dnsServers": []any{"8.8.8.8"}, + }, + } + return func(resource, _, id string) (map[string]any, error) { + switch resource { + case "routes": + return routesList, nil + case "links": + switch id { + case "eth0": + return eth0, nil + case "eth1": + return eth1, nil + case "bond0": + return bond0, nil + case "": + return linksList, nil + } + return map[string]any{}, nil + case "addresses": + return addressesList, nil + case "nodeaddress": + if id == "default" { + return nodeDefault, nil + } + case "resolvers": + if id == "resolvers" { + return resolvers, nil + } + } + return map[string]any{}, nil + } +} + +// bondWithoutBondMasterLookup returns a lookup fixture for a bond +// link where the bondMaster sub-resource is missing or partial +// (real Talos sometimes returns this on freshly-created bonds where +// the master controller hasn't filled the spec yet). The renderer +// must gate every BondMaster field on its presence so the rendered +// BondConfig stays valid YAML — without the gate, missing fields +// surfaced as `bondMode: ` and broke the parse. +func bondWithoutBondMasterLookup() func(string, string, string) (map[string]any, error) { + eth0 := map[string]any{ + "metadata": map[string]any{"id": "eth0"}, + "spec": map[string]any{ + "kind": "physical", + "index": 1, + "hardwareAddr": "aa:bb:cc:00:00:01", + "busPath": "pci-0000:00:1f.0", + "slaveKind": "bond", + "masterIndex": 2, + }, + } + bond0 := map[string]any{ + "metadata": map[string]any{"id": "bond0"}, + "spec": map[string]any{ + "kind": "bond", + "index": 2, + "hardwareAddr": "aa:bb:cc:00:00:01", + }, + } + routesList := map[string]any{ + "apiVersion": "v1", + "kind": "List", + "items": []any{ + map[string]any{ + "spec": map[string]any{ + "dst": "", + "gateway": "192.168.201.1", + "outLinkName": "bond0", + "family": "inet4", + "table": "main", + }, + }, + }, + } + linksList := map[string]any{ + "apiVersion": "v1", + "kind": "List", + "items": []any{eth0, bond0}, + } + addressesList := map[string]any{ + "apiVersion": "v1", + "kind": "List", + "items": []any{ + map[string]any{"spec": map[string]any{"linkName": "bond0", "address": "192.168.201.10/24", "family": "inet4", "scope": "global"}}, + }, + } + nodeDefault := map[string]any{ + "spec": map[string]any{ + "addresses": []any{"192.168.201.10/24"}, + }, + } + resolvers := map[string]any{ + "spec": map[string]any{ + "dnsServers": []any{"8.8.8.8"}, + }, + } + return func(resource, _, id string) (map[string]any, error) { + switch resource { + case "routes": + return routesList, nil + case "links": + switch id { + case "eth0": + return eth0, nil + case "bond0": + return bond0, nil + case "": + return linksList, nil + } + return map[string]any{}, nil + case "addresses": + return addressesList, nil + case "nodeaddress": + if id == "default" { + return nodeDefault, nil + } + case "resolvers": + if id == "resolvers" { + return resolvers, nil + } + } + return map[string]any{}, nil + } +} + +// bridgeLookup returns a lookup fixture for a node with a routed +// physical NIC eth0 plus a bridge br0 that carries no addresses +// of its own. The renderer must emit LinkConfig for eth0 and a +// minimal BridgeConfig for br0 (no addresses, no routes, no STP +// / vlan blocks since spec.bridgeMaster is unset on this +// fixture). The wrong-kind regression to guard against is a +// LinkConfig named br0. +func bridgeLookup() func(string, string, string) (map[string]any, error) { + eth0 := map[string]any{ + "metadata": map[string]any{"id": "eth0"}, + "spec": map[string]any{ + "kind": "physical", + "index": 1, + "hardwareAddr": "aa:bb:cc:00:00:01", + "busPath": "pci-0000:00:1f.0", + }, + } + br0 := map[string]any{ + "metadata": map[string]any{"id": "br0"}, + "spec": map[string]any{ + "kind": "bridge", + "index": 2, + }, + } + routesList := map[string]any{ + "apiVersion": "v1", + "kind": "List", + "items": []any{ + map[string]any{ + "spec": map[string]any{ + "dst": "", + "gateway": "192.168.201.1", + "outLinkName": "eth0", + "family": "inet4", + "table": "main", + }, + }, + }, + } + linksList := map[string]any{ + "apiVersion": "v1", + "kind": "List", + "items": []any{eth0, br0}, + } + addressesList := map[string]any{ + "apiVersion": "v1", + "kind": "List", + "items": []any{ + map[string]any{"spec": map[string]any{"linkName": "eth0", "address": "192.168.201.10/24", "family": "inet4", "scope": "global"}}, + }, + } + nodeDefault := map[string]any{ + "spec": map[string]any{ + "addresses": []any{"192.168.201.10/24"}, + }, + } + resolvers := map[string]any{ + "spec": map[string]any{ + "dnsServers": []any{"8.8.8.8"}, + }, + } + return func(resource, _, id string) (map[string]any, error) { + switch resource { + case "routes": + return routesList, nil + case "links": + switch id { + case "eth0": + return eth0, nil + case "br0": + return br0, nil + case "": + return linksList, nil + } + return map[string]any{}, nil + case "addresses": + return addressesList, nil + case "nodeaddress": + if id == "default" { + return nodeDefault, nil + } + case "resolvers": + if id == "resolvers" { + return resolvers, nil + } + } + return map[string]any{}, nil + } +} + +// vipActiveOnLinkLookup returns a lookup fixture for a node where +// the configured floatingIP is currently active on eth0 — discovery +// reports two global-scope addresses on the link: the permanent +// address and the VIP. The Talos VIP operator does not mark the VIP +// address with any distinguishing field, so the chart must filter +// it out by matching against the operator-declared floatingIP. +func vipActiveOnLinkLookup() func(string, string, string) (map[string]any, error) { + eth0 := map[string]any{ + "metadata": map[string]any{"id": "eth0"}, + "spec": map[string]any{ + "kind": "physical", + "index": 1, + "hardwareAddr": "aa:bb:cc:00:00:01", + "busPath": "pci-0000:00:1f.0", + }, + } + routesList := map[string]any{ + "apiVersion": "v1", + "kind": "List", + "items": []any{ + map[string]any{ + "spec": map[string]any{ + "dst": "", + "gateway": "192.168.201.1", + "outLinkName": "eth0", + "family": "inet4", + "table": "main", + }, + }, + }, + } + linksList := map[string]any{ + "apiVersion": "v1", + "kind": "List", + "items": []any{eth0}, + } + addressesList := map[string]any{ + "apiVersion": "v1", + "kind": "List", + "items": []any{ + map[string]any{"spec": map[string]any{"linkName": "eth0", "address": "192.168.201.10/24", "family": "inet4", "scope": "global"}}, + map[string]any{"spec": map[string]any{"linkName": "eth0", "address": "192.168.201.5/32", "family": "inet4", "scope": "global"}}, + }, + } + nodeDefault := map[string]any{ + "spec": map[string]any{ + "addresses": []any{"192.168.201.10/24"}, + }, + } + resolvers := map[string]any{ + "spec": map[string]any{ + "dnsServers": []any{"8.8.8.8"}, + }, + } + return func(resource, _, id string) (map[string]any, error) { + switch resource { + case "routes": + return routesList, nil + case "links": + if id == "eth0" { + return eth0, nil + } + if id == "" { return linksList, nil } return map[string]any{}, nil @@ -4908,9 +6223,9 @@ func vipActiveOnLinkLookup() func(string, string, string) (map[string]any, error // bridgeWithGatewayLookup returns a lookup fixture where a discovered // bridge br0 carries the IPv4 default route (typical shape: VMs sit // behind br0, the bridge gets the host's address). The renderer -// cannot emit BridgeConfig today, so it must surface a fail rather -// than silently drop every network document for the gateway-bearing -// link. +// emits a typed BridgeConfig document with the gateway entry — the +// bridge branch handles the gateway-bearing case the same way the +// non-gateway path does, just with routes.gateway populated. func bridgeWithGatewayLookup() func(string, string, string) (map[string]any, error) { br0 := map[string]any{ "metadata": map[string]any{"id": "br0"}, @@ -5328,6 +6643,89 @@ func renderCozystackWith(t *testing.T, lookup func(string, string, string) (map[ return out["cozystack/templates/controlplane.yaml"] } +// renderCozystackExpectError mirrors renderCozystackWith but returns +// the render error to the caller instead of t.Fatal'ing on it. Used +// by tests that pin error-message contracts (fail-fast on malformed +// floatingIP, etc.). On unexpected success the returned error is +// nil and the caller is responsible for t.Fatal-ing. +// +// Variadic talosVersion: defaults to v1.12 (multi-doc path); pass +// "v1.11" or earlier to exercise the legacy network define. +func renderCozystackExpectError(t *testing.T, lookup func(string, string, string) (map[string]any, error), overrides map[string]any, talosVersion ...string) error { + t.Helper() + origLookup := helmEngine.LookupFunc + t.Cleanup(func() { helmEngine.LookupFunc = origLookup }) + helmEngine.LookupFunc = lookup + + chrt, err := loader.LoadDir("../../charts/cozystack") + if err != nil { + t.Fatalf("load chart: %v", err) + } + values := cloneValues(chrt.Values) + if v, _ := values["endpoint"].(string); v == "" { + values["endpoint"] = testEndpoint + } + // Seed advertisedSubnets the same way renderChartTemplateWithLookup + // does — otherwise an error test that targets a deeper validation + // (e.g. malformed floatingIP) trips the empty-discovery required() + // guard on advertisedSubnets first and surfaces the wrong error. + // Callers that want to exercise the required() guard itself + // override the field explicitly via the `overrides` argument. + if arr, ok := values["advertisedSubnets"].([]any); !ok || len(arr) == 0 { + values["advertisedSubnets"] = []any{testAdvertisedSubnet} + } + maps.Copy(values, overrides) + + version := "v1.12" + if len(talosVersion) > 0 && talosVersion[0] != "" { + version = talosVersion[0] + } + + eng := helmEngine.Engine{} + _, err = eng.Render(chrt, chartutil.Values{ + "Values": values, + "TalosVersion": version, + }) + + return err //nolint:wrapcheck // surfacing the render error verbatim is the whole point of this helper +} + +// renderGenericExpectError is the generic-preset counterpart of +// renderCozystackExpectError. Used by mirror tests that pin the +// same error-message contract on the generic chart. +func renderGenericExpectError(t *testing.T, lookup func(string, string, string) (map[string]any, error), overrides map[string]any, talosVersion ...string) error { + t.Helper() + origLookup := helmEngine.LookupFunc + t.Cleanup(func() { helmEngine.LookupFunc = origLookup }) + helmEngine.LookupFunc = lookup + + chrt, err := loader.LoadDir("../../charts/generic") + if err != nil { + t.Fatalf("load chart: %v", err) + } + values := cloneValues(chrt.Values) + if v, _ := values["endpoint"].(string); v == "" { + values["endpoint"] = testEndpoint + } + if arr, ok := values["advertisedSubnets"].([]any); !ok || len(arr) == 0 { + values["advertisedSubnets"] = []any{testAdvertisedSubnet} + } + maps.Copy(values, overrides) + + version := "v1.12" + if len(talosVersion) > 0 && talosVersion[0] != "" { + version = talosVersion[0] + } + + eng := helmEngine.Engine{} + _, err = eng.Render(chrt, chartutil.Values{ + "Values": values, + "TalosVersion": version, + }) + + return err //nolint:wrapcheck // surfacing the render error verbatim is the whole point of this helper +} + // renderGenericWith is the generic-preset counterpart of renderCozystackWith. func renderGenericWith(t *testing.T, lookup func(string, string, string) (map[string]any, error), overrides map[string]any) string { t.Helper()