Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,10 @@ cluster:
endpoint: https://192.168.0.1: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`, `ResolverConfig` and a network interface document (`LinkConfig`, `BondConfig`, or `VLANConfig` — depending on topology) are always emitted; `Layer2VIPConfig` appears on controlplane nodes when `floatingIP` is set; `RegistryMirrorConfig` is emitted only by the cozystack chart.

Apply config:
```bash
talm apply -f nodes/node1.yaml -i
Expand Down
253 changes: 200 additions & 53 deletions charts/cozystack/templates/_helpers.tpl
Original file line number Diff line number Diff line change
@@ -1,4 +1,13 @@
{{- define "talos.config" }}
{{- if and .TalosVersion (not (semverCompare "<1.12.0-0" .TalosVersion)) }}
{{- include "talos.config.multidoc" . }}
{{- else }}
{{- include "talos.config.legacy" . }}
{{- end }}
{{- end }}

{{- /* Shared machine section: type, nodeLabels (controlplane), kubelet, sysctls, kernel, certSANs, files, install */ -}}
{{- define "talos.config.machine.common" }}
machine:
{{- if eq .MachineType "controlplane" }}
nodeLabels:
Expand All @@ -14,8 +23,8 @@ machine:
cpuManagerPolicy: static
maxPods: 512
sysctls:
{{- with .Values.nr_hugepages }}
vm.nr_hugepages: {{ .Values.nr_hugepages | quote }}
{{- with $.Values.nr_hugepages }}
vm.nr_hugepages: {{ . | quote }}
{{- end }}
net.ipv4.neigh.default.gc_thresh1: "4096"
net.ipv4.neigh.default.gc_thresh2: "8192"
Expand All @@ -35,11 +44,6 @@ machine:
{{- with .Values.certSANs }}
{{- toYaml . | nindent 2 }}
{{- end }}
registries:
mirrors:
docker.io:
endpoints:
- https://mirror.gcr.io
files:
- content: |
[plugins]
Expand All @@ -66,53 +70,10 @@ machine:
{{- end }}
{{- (include "talm.discovered.disks_info" .) | nindent 4 }}
disk: {{ include "talm.discovered.system_disk_name" . | quote }}
network:
hostname: {{ include "talm.discovered.hostname" . | quote }}
nameservers: {{ include "talm.discovered.default_resolvers" . }}
{{- (include "talm.discovered.physical_links_info" .) | nindent 4 }}
interfaces:
{{- $existingInterfacesConfiguration := include "talm.discovered.existing_interfaces_configuration" . }}
{{- if $existingInterfacesConfiguration }}
{{- $existingInterfacesConfiguration | nindent 4 }}
{{- else }}
{{- $defaultLinkName := include "talm.discovered.default_link_name_by_gateway" . }}
{{- $isVlan := include "talm.discovered.is_vlan" $defaultLinkName }}
{{- $parentLinkName := "" }}
{{- if $isVlan }}
{{- $parentLinkName = include "talm.discovered.parent_link_name" $defaultLinkName }}
{{- end }}
{{- $interfaceName := $defaultLinkName }}
{{- if and $isVlan $parentLinkName }}
{{- $interfaceName = $parentLinkName }}
{{- end }}
- interface: {{ $interfaceName }}
{{- $bondConfig := include "talm.discovered.bond_config" $interfaceName }}
{{- if $bondConfig }}
{{- $bondConfig | nindent 6 }}
{{- end }}
{{- if $isVlan }}
vlans:
- vlanId: {{ include "talm.discovered.vlan_id" $defaultLinkName }}
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") }}
vip:
ip: {{ .Values.floatingIP }}
{{- 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") }}
vip:
ip: {{ .Values.floatingIP }}
{{- end }}
{{- end }}
{{- end }}
{{- end }}

{{- /* Shared cluster section */ -}}
{{- define "talos.config.cluster" }}
cluster:
network:
cni:
Expand Down Expand Up @@ -161,3 +122,189 @@ cluster:
{{- toYaml .Values.advertisedSubnets | nindent 6 }}
{{- end }}
{{- end }}

{{- /* Shared network document generation for v1.12+ multi-doc format */ -}}
{{- define "talos.config.network.multidoc" }}
{{- /* Multi-doc format always reconstructs network config from discovery resources.
existing_interfaces_configuration is not used here because v1.12 nodes store
network config in separate documents (LinkConfig, BondConfig, etc.), not in
the legacy machine.network.interfaces field. */ -}}
Comment on lines +127 to +131
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Preserve existing_interfaces_configuration in the multi-doc branch.

The legacy path consumes talm.discovered.existing_interfaces_configuration, but the v1.12 path ignores it and always rebuilds from the default-route interface. That silently drops any non-default interface state as soon as TalosVersion flips to v1.12+. Please translate that helper output into named docs here, or fail fast when it is present.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@charts/cozystack/templates/_helpers.tpl` around lines 127 - 131, The
multi-doc helper "talos.config.network.multidoc" currently reconstructs network
config for v1.12+ and ignores talm.discovered.existing_interfaces_configuration;
update the helper to preserve that value by emitting it as a named document
(e.g., "existing_interfaces_configuration") alongside the LinkConfig/BondConfig
docs when talm.discovered.existing_interfaces_configuration is present, or
alternatively add an explicit fast-fail validation at the top of
talos.config.network.multidoc that returns an error if
talm.discovered.existing_interfaces_configuration is set (to avoid silent data
loss when TalosVersion becomes v1.12+). Ensure the change references the helper
"talos.config.network.multidoc" and the variable
talm.discovered.existing_interfaces_configuration so callers get the preserved
data or a clear failure.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Intentional. talm.discovered.existing_interfaces_configuration emits a legacy machine.network.interfaces YAML fragment, which has no safe 1:1 translation to the v1.12 multi-doc resources (LinkConfig / BondConfig / VLANConfig / Layer2VIPConfig). Rather than silently producing a partial or incorrect translation, the multidoc path rebuilds the required documents from the discovery resources directly; the comment at the top of talos.config.network.multidoc documents this. Users who rely on custom legacy interface state should keep machine.talosVersion on v1.11.x until they migrate their overrides to the new resources.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lexfrei, understood — that's a sound design rationale. Silently emitting a partial or incorrectly translated fragment would be far more dangerous than a clean rebuild from discovery resources, and the existing comment in the template documents the deliberate omission clearly. I'll note this for future reviews.

(ʕ •ᴥ•ʔ)≡


✏️ Learnings added
Learnt from: lexfrei
Repo: cozystack/talm PR: 116
File: charts/cozystack/templates/_helpers.tpl:127-131
Timestamp: 2026-04-14T15:31:55.570Z
Learning: In `charts/cozystack/templates/_helpers.tpl` and `charts/generic/templates/_helpers.tpl`, the `talos.config.network.multidoc` template intentionally ignores `talm.discovered.existing_interfaces_configuration`. That helper emits a legacy `machine.network.interfaces` YAML fragment that has no safe 1:1 translation to Talos v1.12 multi-doc resources (LinkConfig / BondConfig / VLANConfig / Layer2VIPConfig). The multidoc path rebuilds all required network documents directly from discovery resources instead. Users relying on custom legacy interface state must stay on Talos v1.11.x until they migrate their overrides to the new v1.12 resource types.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


🧠 Learnings used
Learnt from: lexfrei
Repo: cozystack/talm PR: 113
File: pkg/commands/apply.go:116-136
Timestamp: 2026-04-02T08:59:42.636Z
Learning: In the talm project (pkg/commands/apply.go and pkg/commands/template.go), when a node file (e.g. nodes/node0.yaml) is passed via `-f`, it serves only as a modeline container providing metadata (nodes, endpoints, template paths). The actual cluster configuration is produced entirely by Helm templates + values.yaml. The configFile is intentionally NOT passed as a patch to engine.Render. This matches templateWithFiles behavior in template.go and is equivalent to the two-step `talm template -f node.yaml | talm apply -f -` pattern.

{{- (include "talm.discovered.physical_links_info" .) }}
---
apiVersion: v1alpha1
kind: HostnameConfig
hostname: {{ include "talm.discovered.hostname" . | quote }}
---
apiVersion: v1alpha1
kind: ResolverConfig
nameservers:
{{- $resolvers := include "talm.discovered.default_resolvers" . }}
{{- if $resolvers }}
{{- range fromJsonArray $resolvers }}
- address: {{ . | quote }}
{{- end }}
{{- else }}
[]
{{- end }}
Comment on lines +140 to +148
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

In Talos v1.12+, the ResolverConfig resource expects nameservers to be a list of strings (IP addresses), not a list of objects with an address key.

nameservers:
{{- $resolvers := include "talm.discovered.default_resolvers" . }}
{{- if $resolvers }}
{{- range fromJsonArray $resolvers }}
  - {{ . | quote }}
{{- end }}
{{- else }}
  []
{{- end }}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current format is correct per the Go type definitions.

ResolverConfigV1Alpha1 has ResolverNameservers []NameserverConfig with yaml tag nameservers,omitempty, where NameserverConfig has Address Addr with yaml tag address.

Source: resolver.go:63 and resolver.go:81

The official test data (testdata/resolverconfig.yaml) confirms the format:

nameservers:
  - address: 10.0.0.1
  - address: 2001:4860:4860::8888

Comment on lines +140 to +148
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Minor edge case: empty nameservers output.

If talm.discovered.default_resolvers returns an empty JSON array [], the condition if $resolvers is true (non-empty string), but range fromJsonArray $resolvers produces nothing, resulting in:

nameservers:

This is consistent with the generic template and likely acceptable, but verify Talos tolerates an empty nameservers: key without a value.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@charts/cozystack/templates/_helpers.tpl` around lines 140 - 148, The template
currently treats a non-empty JSON string "[]" as truthy and ranges over
fromJsonArray $resolvers which yields nothing, leaving an empty "nameservers:"
key; change the logic to parse the JSON into an array first and check its length
before rendering: call fromJsonArray once into a variable (e.g. $arr :=
fromJsonArray $resolvers), then if gt (len $arr) 0 range $arr to render each "-
address: {{ . | quote }}", else render an explicit empty YAML list "  []".
Ensure you reference the include "talm.discovered.default_resolvers", the
$resolvers variable, and the fromJsonArray result variable (e.g. $arr) in the
updated condition.

{{- $defaultLinkName := include "talm.discovered.default_link_name_by_gateway" . }}
{{- $isVlan := include "talm.discovered.is_vlan" $defaultLinkName }}
{{- $parentLinkName := "" }}
{{- if $isVlan }}
{{- $parentLinkName = include "talm.discovered.parent_link_name" $defaultLinkName }}
{{- end }}
{{- $interfaceName := $defaultLinkName }}
{{- if and $isVlan $parentLinkName }}
{{- $interfaceName = $parentLinkName }}
{{- end }}
{{- $isBondInterface := include "talm.discovered.is_bond" $interfaceName }}
{{- if $isBondInterface }}
{{- $link := lookup "links" "" $interfaceName }}
{{- if $link }}
{{- $bondMaster := $link.spec.bondMaster }}
{{- $slaves := fromJsonArray (include "talm.discovered.bond_slaves" $link.spec.index) }}
---
apiVersion: v1alpha1
kind: BondConfig
name: {{ $interfaceName }}
links:
{{- range $slaves }}
- {{ . }}
{{- end }}
bondMode: {{ $bondMaster.mode }}
{{- if $bondMaster.xmitHashPolicy }}
xmitHashPolicy: {{ $bondMaster.xmitHashPolicy }}
{{- end }}
{{- if $bondMaster.lacpRate }}
lacpRate: {{ $bondMaster.lacpRate }}
{{- end }}
{{- if $bondMaster.miimon }}
miimon: {{ $bondMaster.miimon }}
{{- end }}
{{- if $bondMaster.updelay }}
updelay: {{ $bondMaster.updelay }}
{{- end }}
{{- if $bondMaster.downdelay }}
downdelay: {{ $bondMaster.downdelay }}
{{- end }}
{{- if not $isVlan }}
addresses:
{{- range fromJsonArray (include "talm.discovered.default_addresses_by_gateway" .) }}
- address: {{ . }}
{{- end }}
routes:
- gateway: {{ include "talm.discovered.default_gateway" . }}
{{- end }}
{{- end }}
Comment on lines +160 to +197
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

There are several schema and safety issues in the BondConfig generation:

  1. The lookup function can return an empty map if the resource is not found. Accessing $link.spec without checking if it exists will cause a template rendering error.
  2. The field for bond mode is mode, not bondMode.
  3. The addresses field expects a list of strings.
  4. The routes field entries require a network field (e.g., 0.0.0.0/0).
{{- if $isBondInterface }}
{{- $link := lookup "links" "" $interfaceName }}
{{- if $link }}
{{- $bondMaster := $link.spec.bondMaster }}
{{- $slaves := fromJsonArray (include "talm.discovered.bond_slaves" $link.spec.index) }}
---
apiVersion: v1alpha1
kind: BondConfig
name: {{ $interfaceName }}
links:
{{- range $slaves }}
  - {{ . }}
{{- end }}
mode: {{ $bondMaster.mode }}
{{- if $bondMaster.xmitHashPolicy }}
xmitHashPolicy: {{ $bondMaster.xmitHashPolicy }}
{{- end }}
{{- if $bondMaster.lacpRate }}
lacpRate: {{ $bondMaster.lacpRate }}
{{- end }}
{{- if $bondMaster.miimon }}
miimon: {{ $bondMaster.miimon }}
{{- end }}
{{- if $bondMaster.updelay }}
updelay: {{ $bondMaster.updelay }}
{{- end }}
{{- if $bondMaster.downdelay }}
downdelay: {{ $bondMaster.downdelay }}
{{- end }}
{{- if not $isVlan }}
addresses:
{{- range fromJsonArray (include "talm.discovered.default_addresses_by_gateway" .) }}
  - {{ . }}
{{- end }}
routes:
  - network: 0.0.0.0/0
    gateway: {{ include "talm.discovered.default_gateway" . }}
{{- end }}
{{- end }}
{{- end }}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Schema claims are incorrect, but the lookup safety point is valid.

bondMode is correctBondConfigV1Alpha1 has BondMode *nethelpers.BondMode with yaml tag bondMode,omitempty (bond.go:96). Test data confirms: bondMode: 802.3ad.

addresses format is correctAddressConfig has AddressAddress netip.Prefix with yaml tag address (link.go:122). Format is - address: IP/prefix, not bare strings.

routes without destination is validRouteDestination has yaml tag destination,omitempty (NOT network). The field is optional: "If not specified, a default route will be created for the address family of the gateway." (link.go:133-135)

Lookup safety$isBondInterface calls talm.discovered.is_bond which already performs lookup "links" "" $interfaceName and checks $link.spec.kind == "bond". If it returned true, the resource is guaranteed to exist. Nevertheless, adding a guard is reasonable defensive coding. Will fix.

{{- end }}
{{- if $isVlan }}
---
apiVersion: v1alpha1
kind: VLANConfig
name: {{ $defaultLinkName }}
vlanID: {{ include "talm.discovered.vlan_id" $defaultLinkName }}
parent: {{ $interfaceName }}
addresses:
{{- range fromJsonArray (include "talm.discovered.default_addresses_by_gateway" .) }}
- address: {{ . }}
{{- end }}
routes:
- gateway: {{ include "talm.discovered.default_gateway" . }}
Comment on lines +201 to +211
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The VLANConfig resource has schema mismatches:

  1. The field name is vlanId, not vlanID.
  2. addresses should be a list of strings.
  3. routes entries require a network field.
kind: VLANConfig
name: {{ $defaultLinkName }}
vlanId: {{ include "talm.discovered.vlan_id" $defaultLinkName }}
parent: {{ $interfaceName }}
addresses:
{{- range fromJsonArray (include "talm.discovered.default_addresses_by_gateway" .) }}
  - {{ . }}
{{- end }}
routes:
  - network: 0.0.0.0/0
    gateway: {{ include "talm.discovered.default_gateway" . }}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All three claims are incorrect.

vlanID is correctVLANConfigV1Alpha1 has VLANIDConfig uint16 with yaml tag vlanID,omitempty (vlan.go:69). Test data confirms: vlanID: 2.

addresses format is correct — uses AddressConfig with address key, same as LinkConfig/BondConfig.

routes without destination is validRouteDestination is omitempty, not required.

{{- else if not $isBondInterface }}
---
apiVersion: v1alpha1
kind: LinkConfig
name: {{ $interfaceName }}
addresses:
{{- range fromJsonArray (include "talm.discovered.default_addresses_by_gateway" .) }}
- address: {{ . }}
{{- end }}
routes:
- gateway: {{ include "talm.discovered.default_gateway" . }}
Comment on lines +215 to +222
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The LinkConfig resource expects addresses to be a list of strings and routes to include a network field.

kind: LinkConfig
name: {{ $interfaceName }}
addresses:
{{- range fromJsonArray (include "talm.discovered.default_addresses_by_gateway" .) }}
  - {{ . }}
{{- end }}
routes:
  - network: 0.0.0.0/0
    gateway: {{ include "talm.discovered.default_gateway" . }}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both claims are incorrect.

addresses uses AddressConfig with yaml tag address — format is - address: IP/prefix, not bare strings (link.go:98,122).

routesRouteDestination has yaml tag destination,omitempty. The destination field is optional, not required. There is no network field in RouteConfig. A route with only gateway creates a default route.

{{- end }}
{{- $vipLinkName := $interfaceName }}
{{- if $isVlan }}
{{- $vipLinkName = $defaultLinkName }}
{{- end }}
{{- if and .Values.floatingIP (eq .MachineType "controlplane") }}
---
apiVersion: v1alpha1
kind: Layer2VIPConfig
name: {{ .Values.floatingIP | quote }}
link: {{ $vipLinkName }}
{{- end }}
{{- end }}

{{- /* Shared legacy network section for machine.network */ -}}
{{- define "talos.config.network.legacy" }}
network:
hostname: {{ include "talm.discovered.hostname" . | quote }}
nameservers: {{ include "talm.discovered.default_resolvers" . }}
{{- (include "talm.discovered.physical_links_info" .) | nindent 4 }}
interfaces:
{{- $existingInterfacesConfiguration := include "talm.discovered.existing_interfaces_configuration" . }}
{{- if $existingInterfacesConfiguration }}
{{- $existingInterfacesConfiguration | nindent 4 }}
{{- else }}
{{- $defaultLinkName := include "talm.discovered.default_link_name_by_gateway" . }}
{{- $isVlan := include "talm.discovered.is_vlan" $defaultLinkName }}
{{- $parentLinkName := "" }}
{{- if $isVlan }}
{{- $parentLinkName = include "talm.discovered.parent_link_name" $defaultLinkName }}
{{- end }}
{{- $interfaceName := $defaultLinkName }}
{{- if and $isVlan $parentLinkName }}
{{- $interfaceName = $parentLinkName }}
{{- end }}
- interface: {{ $interfaceName }}
{{- $bondConfig := include "talm.discovered.bond_config" $interfaceName }}
{{- if $bondConfig }}
{{- $bondConfig | nindent 6 }}
{{- end }}
{{- if $isVlan }}
vlans:
- vlanId: {{ include "talm.discovered.vlan_id" $defaultLinkName }}
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") }}
vip:
ip: {{ .Values.floatingIP }}
{{- 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") }}
vip:
ip: {{ .Values.floatingIP }}
{{- end }}
{{- end }}
{{- end }}
{{- end }}

{{- define "talos.config.legacy" }}
{{- include "talos.config.machine.common" . }}
registries:
mirrors:
docker.io:
endpoints:
- https://mirror.gcr.io
{{- include "talos.config.network.legacy" . }}

{{- include "talos.config.cluster" . }}
{{- end }}

{{- define "talos.config.multidoc" }}
{{- include "talos.config.machine.common" . }}

{{- include "talos.config.cluster" . }}
---
apiVersion: v1alpha1
kind: RegistryMirrorConfig
name: docker.io
endpoints:
- url: https://mirror.gcr.io
Comment on lines +307 to +308
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The RegistryMirrorConfig resource expects endpoints to be a list of strings (URLs), not a list of objects with a url key.

endpoints:
  - https://mirror.gcr.io

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incorrect. Endpoints are objects, not strings.

RegistryMirrorConfigV1Alpha1 has RegistryEndpoints []RegistryEndpoint with yaml tag endpoints,omitempty. RegistryEndpoint has EndpointURL meta.URL with yaml tag url AND EndpointOverridePath *bool with yaml tag overridePath,omitempty (registry_mirror.go:69,87,90).

Endpoints must be objects because they support overridePath. Test data confirms:

endpoints:
  - url: https://my-private-registry.local:5000
  - url: http://my-harbor/v2/registry-k8s.io/
    overridePath: true

{{- include "talos.config.network.multidoc" . }}
{{- end }}
Loading