fix(chart): discovery-based defaults for subnets; required endpoint and VIP

[lexfrei]

Summary

Close the loop on the 192.168.100.* placeholder problem: talm template on a node in any subnet other than 192.168.100.0/24 silently produced machine configs pointing at network IPs no node actually owns.

Closes #25

What changes

1. Subnet fields derive from discovery

machine.kubelet.nodeIP.validSubnets and cluster.etcd.advertisedSubnets now fall back to the CIDR of the node's default-gateway-bearing link when values.yaml leaves advertisedSubnets empty. The chart pipes the discovered address (192.168.201.10/24) through a new cidrNetwork template function — net/netip's ParsePrefix().Masked() in 6 lines — so the emitted YAML is the canonical network form (192.168.201.0/24) rather than the host form. Multiple addresses on the same link are deduped via sprig's uniq.

Override precedence is preserved: if the operator sets advertisedSubnets explicitly in values.yaml, that wins.

When both paths fail (operator didn't set it, discovery returned nothing), the chart fails loudly with a required() error that explains both remediations (set it explicitly, or fix the node's default route) — not silently emit an empty list.

2. Cluster-wide fields are now required instead of placeholder-defaulted

Three fields shipped with 192.168.100.* placeholder defaults that silently produced broken configs for anyone not on that subnet:

• endpoint — what every node's kubelet and kube-proxy dials.
• floatingIP (cozystack preset) — drives the Layer-2 VIP document.
• Implicitly the coupling between them: they must match for a VIP setup.

All three are now blank by default. endpoint triggers a required() guard with a verbose message that explains why auto-derivation isn't possible (talm template runs once per node and has no way to reconcile per-node IPs into a single shared endpoint) and enumerates the viable options (cozystack VIP, external LB, single-node routable IP). floatingIP emits no Layer2VIPConfig document when blank, which is the correct behavior for single-node and external-LB topologies.

3. Documentation matches reality

• charts/cozystack/values.yaml and charts/generic/values.yaml — rewritten comments explain the decision tree (cozystack VIP vs single-node vs external LB) and the endpoint/floatingIP coupling.
• README.md "Getting Started" — adds the required values.yaml edit step between talm init and talm template. Example values match the tutorial's node IP so a reader walking the example doesn't see placeholder-vs-tutorial drift.

4. Test coverage

pkg/engine/render_test.go gains tests for:

• validSubnets and advertisedSubnets discovery fallback (controlplane + worker + generic preset).
• Override precedence: operator values.yaml wins.
• Empty-discovery required() guard fires with both the field name and the remediation phrase in the error.
• Shipped-defaults regression guard: fresh chrt.Values render fails, catching any future commit that re-introduces a placeholder.
• VIP-when-set: Layer2VIPConfig emitted only when operator sets floatingIP.
• No-VIP on fresh defaults.
• Dedupe: two addresses in the same subnet on one link collapse to one entry per consumer.

pkg/engine/helm/engine_test.go gains TestCidrNetworkTemplateFunc — direct IPv4/IPv6 unit test of the new template function (7 cases including already-canonical, narrow prefix, malformed inputs).

Migration notes for existing users

• Anyone who talm init'd before and left values.yaml defaults now sees a required error on next talm template. Remediation is in the error message.
• Anyone who explicitly set endpoint and advertisedSubnets in values.yaml is unaffected.
• Anyone who relied on the cozystack floatingIP: 192.168.100.10 default (unlikely, given it was network-incorrect for most users) now needs to set it explicitly.

Verification

• go test ./... on macOS — pass
• GOOS=windows GOARCH=amd64 go build ./... — clean
• golangci-lint run ./... — 0 issues

Summary by CodeRabbit

Release Notes

• Documentation

  • Enhanced README with detailed configuration guidance for control-plane endpoint and VIP setup across deployment scenarios.

• Configuration Changes

  • Removed default values for control-plane endpoint, floating IP, and subnet configuration; explicit values now required.

• Improvements

  • Added automatic subnet discovery from network interfaces when not explicitly configured.
  • Strengthened validation with clearer error messages for required settings.

[coderabbitai]

Warning

Rate limit exceeded

@lexfrei has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 53 minutes and 16 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 53 minutes and 16 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: d9588a28-4114-483d-9a45-f0c4cf68a541

📥 Commits

Reviewing files that changed from the base of the PR and between 6295e02 and d67d0d1.

📒 Files selected for processing (1)

• README.md

📝 Walkthrough

Walkthrough

The pull request removes hardcoded cluster networking defaults from Helm charts and implements discovery-based subnet derivation with explicit endpoint validation. Users must now configure control-plane endpoints and floating IP values, while subnet defaults are automatically computed from discovered network interfaces when not explicitly provided.

Changes

Cohort / File(s)	Summary
Documentation README.md	Updated setup guidance with scenario-specific rules for configuring endpoint and floatingIP across VIP, single-node, and multi-node topologies; includes concrete YAML example.
Chart Default Values charts/cozystack/values.yaml, charts/generic/values.yaml	Removed baked-in control-plane endpoint, floating IP, and subnet defaults; changed to empty strings/lists with explanatory comments instructing users on required configuration relationships and discovery-time derivation.
Cozystack Template Logic charts/cozystack/templates/_helpers.tpl	Added fallback subnet derivation using talm.discovered.default_addresses_by_gateway and cidrNetwork masking when advertisedSubnets is empty; wraps cluster.controlPlane.endpoint in required() validation with explicit error message.
Generic Template Logic charts/generic/templates/_helpers.tpl	Implements conditional subnet resolution for both machine.kubelet.nodeIP.validSubnets and cluster.etcd.advertisedSubnets: prefers user-provided values, falls back to discovery-derived masked subnet CIDRs with deduplication, hard-fails when discovery yields no addresses and no override provided; enforces cluster.controlPlane.endpoint via required().
Helm Engine Function pkg/engine/helm/engine.go	Adds new cidrNetwork template function that parses CIDR strings, masks to canonical network address, and returns normalized form; returns wrapped error on invalid input.
Helm Engine Tests pkg/engine/helm/engine_test.go	Introduces table-driven test suite for cidrNetwork function covering IPv4/IPv6 normalization, edge cases (already-canonical forms), and error conditions (missing prefix, invalid CIDR).
Render Integration Tests pkg/engine/render_test.go	Adds comprehensive test fixtures, helpers, and regression tests validating: discovery fallback behavior for subnet derivation, user-provided subnet precedence, endpoint validation enforcement, floating IP gating, deduplication, discovery failure scenarios, and behavior across both cozystack and worker templates.

Sequence Diagram

sequenceDiagram
    participant User as User (values.yaml)
    participant Template as Helm Template
    participant Discovery as Discovery Data
    participant Config as Talos Config

    User->>Template: endpoint (required), advertisedSubnets (optional)
    
    alt User provides advertisedSubnets
        Template->>Config: Use provided advertisedSubnets
    else advertisedSubnets empty/unset
        Template->>Discovery: Fetch talm.discovered.default_addresses_by_gateway
        alt Discovery has default-gateway addresses
            Discovery-->>Template: Return gateway addresses
            Template->>Template: Mask each address via cidrNetwork
            Template->>Template: Deduplicate CIDR results
            Template->>Config: Use derived subnet CIDRs
        else No default-gateway addresses
            Discovery-->>Template: Empty result
            Template->>Template: Hard fail with error
        end
    end
    
    Template->>Template: Validate endpoint via required()
    alt endpoint provided
        Template->>Config: Inject endpoint
    else endpoint empty
        Template->>Template: Fail validation with error
    end

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

The changes span multiple interrelated layers—chart values, template logic with fallback subnet derivation and validation, new Helm engine functions, and extensive test coverage (572 additional test lines)—with heterogeneous logic across discovery integration, CIDR masking, and error handling that each requires separate reasoning.

Poem

🐰 No more baked defaults in sight—
Discovery whispers subnets into the night,
Endpoints must be set with care,
Templates validate what's there,
Flexibility and rigor take flight! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 70.37% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly captures the main changes: discovery-based subnet defaults and making endpoint/VIP required—the core fix for placeholder network values being silently emitted.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/values-discovery-defaults

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[gemini-code-assist]

Code Review

This pull request implements automatic discovery for network subnets in the cozystack and generic Helm charts, falling back to the node's default gateway when advertisedSubnets is not explicitly defined. It also enforces mandatory configuration for the cluster control-plane endpoint by removing default placeholders and utilizing Helm's required function. To support these changes, a new cidrNetwork template function was added to the engine along with comprehensive test cases. Review feedback suggests using the fail function for more idiomatic error reporting in templates and recommends using deep copies for chart values in tests to avoid side effects from shallow copying.

[gemini-code-assist]

While using required with an empty string works to throw an error, it is more idiomatic in Helm templates to use the fail function for unconditional errors with custom messages. Since fail is already implemented in the engine and used elsewhere, consider using it here for better clarity.

        {{- fail "values.yaml: `advertisedSubnets` was left empty and talm could not derive a default from discovery. No default-gateway-bearing link was found on the node. This field is a cluster-wide subnet selector fed to kubelet and etcd; `talm template` is invoked once per node and cannot merge per-node values into one cluster value. Either set advertisedSubnets explicitly in values.yaml, or ensure the node has a default route before running `talm template`." }}

[lexfrei]

Good call — swapped to fail in 42967b7. required "" was a hack to express unconditional failure, fail says the same thing directly and matches how the Helm engine already registers both helpers side by side.

[gemini-code-assist]

Similar to the cozystack chart, consider using the fail function instead of required with an empty string for throwing this discovery error. This is more idiomatic for unconditional failures in Helm templates.

        {{- fail "values.yaml: `advertisedSubnets` was left empty and talm could not derive a default from discovery. No default-gateway-bearing link was found on the node. This field is a cluster-wide subnet selector fed to kubelet and etcd; `talm template` is invoked once per node and cannot merge per-node values into one cluster value. Either set advertisedSubnets explicitly in values.yaml, or ensure the node has a default route before running `talm template`." }}

[lexfrei]

Fixed symmetrically in 3a8c0a5 — the generic preset now uses fail too, keeping the two presets identical.

[gemini-code-assist]

The use of maps.Copy performs a shallow copy of the chart values. If any test modifies a nested map or slice within the values map, it will inadvertently modify the original chrt.Values, potentially leading to flaky tests or side effects in subsequent test cases. While the current tests only seem to modify top-level keys, using a deep copy mechanism would be more robust for future-proofing the test suite.

[lexfrei]

Fair point for future-proofing — switched in 6295e02. Added a small cloneValues helper that recursively deep-copies maps and slices (primitives are immutable so they can stay shared) and routed renderCozystackWith, renderGenericWith, and renderChartTemplate through it. The remaining maps.Copy is only for merging caller-supplied overrides, and those maps are built fresh per test so they're already safe.

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@pkg/engine/render_test.go`:
- Around line 1403-1433: The test fixture is inconsistent: addressesList returns
two addresses but the nodeaddress/default response only contains one, so update
the resource-returning closure in render_test.go (the anonymous function that
switches on resource) to make the "nodeaddress"/"default" case return a
spec.addresses list containing both "192.168.201.10/24" and "192.168.201.11/24"
(matching addressesList) so talm.discovered.default_addresses_by_gateway sees
duplicate input; ensure you modify the map returned in the nodeaddress/default
branch rather than the addressesList variable itself.

In `@README.md`:
- Around line 75-81: The README's rendered config sample still shows the old
placeholder endpoint "https://192.168.0.1:6443" which contradicts the new
values.yaml example using "https://1.2.3.4:6443"; update the generated config
examples in the README (the sample that renders the endpoint and the later block
around lines referred to as 132-135) to use "https://1.2.3.4:6443" (or switch
both to a consistent placeholder like "https://<NODE_IP>:6443") so the endpoint
in the config sample matches the values.yaml example and the walkthrough; search
for the literal "https://192.168.0.1:6443" in README and replace it with the
consistent address/placeholder.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 7649c732-ca1f-4842-816f-d751c547bc33

📥 Commits

Reviewing files that changed from the base of the PR and between 00358fd and 6295e02.

📒 Files selected for processing (8)

• README.md
• charts/cozystack/templates/_helpers.tpl
• charts/cozystack/values.yaml
• charts/generic/templates/_helpers.tpl
• charts/generic/values.yaml
• pkg/engine/helm/engine.go
• pkg/engine/helm/engine_test.go
• pkg/engine/render_test.go

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Make the duplicate-subnet fixture consistent across discovery resources.

This test publishes two addresses through the addresses lookup, but nodeaddress/default still exposes only one. If talm.discovered.default_addresses_by_gateway reads from nodeaddress/default, the dedupe regression test can pass without exercising duplicate input.

🧪 Proposed fixture fix

 			case "nodeaddress":
 				if id == "default" {
-					return map[string]any{"spec": map[string]any{"addresses": []any{"192.168.201.10/24"}}}, nil
+					return map[string]any{"spec": map[string]any{"addresses": []any{
+						"192.168.201.10/24",
+						"192.168.201.11/24",
+					}}}, nil
 				}

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	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.11/24",
	"family": "inet4", "scope": "global",
	}},
	},
	}
	return func(resource, namespace, id string) (map[string]any, error) {
	switch resource {
	case "routes":
	return routesList, nil
	case "links":
	if id == "eth0" {
	return eth0, nil
	}
	if id == "" {
	return map[string]any{"apiVersion": "v1", "kind": "List", "items": []any{eth0}}, nil
	}
	case "addresses":
	return addressesList, nil
	case "nodeaddress":
	if id == "default" {
	return map[string]any{"spec": map[string]any{"addresses": []any{"192.168.201.10/24"}}}, nil
	}
	case "addresses":
	return addressesList, nil
	case "nodeaddress":
	if id == "default" {
	return map[string]any{"spec": map[string]any{"addresses": []any{
	"192.168.201.10/24",
	"192.168.201.11/24",
	}}}, nil
	}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/engine/render_test.go` around lines 1403 - 1433, The test fixture is
inconsistent: addressesList returns two addresses but the nodeaddress/default
response only contains one, so update the resource-returning closure in
render_test.go (the anonymous function that switches on resource) to make the
"nodeaddress"/"default" case return a spec.addresses list containing both
"192.168.201.10/24" and "192.168.201.11/24" (matching addressesList) so
talm.discovered.default_addresses_by_gateway sees duplicate input; ensure you
modify the map returned in the nodeaddress/default branch rather than the
addressesList variable itself.

[lexfrei]

The dedupe path here runs through talm.discovered.default_addresses_by_gateway, which reads the addresses resource (see charts/talm/templates/_helpers.tpl:75) — not nodeaddress/default. The addresses fixture already publishes two entries on eth0 in the same /24, so the duplicate input is exercised as intended. nodeaddress/default feeds talm.discovered.default_addresses, a separate helper that is not on the subnet-derivation path, so the single-address entry there does not weaken the regression test.

[coderabbitai]

@lexfrei, thanks for the clear explanation — you're right, and my comment was incorrect. Since talm.discovered.default_addresses_by_gateway reads from the addresses resource (not nodeaddress/default), the two-entry addressesList fixture already exercises the duplicate-input path. The inconsistency in nodeaddress/default is intentional and harmless because that resource feeds a completely separate helper. The dedupe regression test is correctly structured as-is.

---

✏️ Learnings added

Learnt from: lexfrei
Repo: cozystack/talm PR: 130
File: pkg/engine/render_test.go:1403-1433
Timestamp: 2026-04-24T10:57:14.996Z
Learning: In the talm chart helpers (`charts/talm/templates/_helpers.tpl`), `talm.discovered.default_addresses_by_gateway` reads from the `addresses` lookup resource (around line 75), NOT from `nodeaddress/default`. The `nodeaddress/default` resource feeds `talm.discovered.default_addresses`, a separate helper that is not on the subnet-derivation/dedupe path used by `kubelet.validSubnets` and `etcd.advertisedSubnets`. These two helpers are distinct and should not be conflated when reviewing test fixtures for subnet-derivation or deduplication behavior.

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: 116
File: charts/cozystack/templates/_helpers.tpl:127-131
Timestamp: 2026-04-14T15:32:01.453Z
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.

[myasnikovdaniil]

Here is my draft review.

---

Draft review for PR #130 — fix(chart): discovery-based defaults for subnets; required endpoint and VIP

Summary

This PR closes a real foot-gun: charts shipping with 192.168.100.* placeholder defaults silently produced broken machine configs for everyone not on that subnet. The fix is in three layers — empty-by-default required values for cluster-wide fields (endpoint, floatingIP), a discovery fallback wired through a new cidrNetwork template function for advertisedSubnets, and rewritten user-facing docs that explain the cozystack-VIP / single-node / external-LB decision tree. Test coverage is thorough (override precedence, empty-discovery guard, dedupe, fresh-defaults regression guard, IPv4 + IPv6 cidrNetwork unit tests, worker path, generic path). The breaking change for existing users is acknowledged and the error messages enumerate remediation, so the migration story is reasonable. A few minor cleanups noted below; nothing blocking.

Migration safety check — schema validation as a nice-to-have

This isn't a blocker, but since endpoint is now hard-required and the failure mode is "render-time error with a long message," adding a values.schema.json to each chart that marks endpoint as required would surface the issue at helm lint time too — earlier in the workflow than a render error. Out of scope for this PR; flagging only because the PR is the natural place to do it next.

Recommendation

Comment — the change is sound and well-tested. The findings above are nits and small defensive-programming suggestions; none of them block merge. I'd particularly encourage fixing the stale required()-vs-fail comment (5-second change) and adding the duplicate empty-discovery guard to the etcd block (5-line change) before merge. The CIDR canonicalization consistency and the schema-validation suggestion can land later.

---

Stopping here — this is a draft for your review, not posted anywhere.

cloneValues / deepClone justification reads well (pkg/engine/render_test.go)

The deep-copy helper is well-justified by the comment ("maps.Copy is a shallow copy — mutating a nested map or slice in a test would leak into chrt.Values and corrupt subsequent renders"). Nice.

[myasnikovdaniil]

stale "required()" wording in comment

The most recent commit (3a8c0a5) switched from required to fail for the empty-discovery path, but the comment in the etcd block still references the old guard name. Same wording lives in charts/generic/templates/_helpers.tpl:71.

      {{- /* Fall back to the subnet of the node's default-gateway-bearing
             link; cidrNetwork masks host bits to emit canonical network
             form. Dedupe handled the same way as validSubnets above.
             Empty discovery already errored via validSubnets' fail
             guard, so we reach this block only when at least one address
             was resolved. */ -}}

[myasnikovdaniil]

etcd block's empty-discovery handling is implicit

The etcd advertisedSubnets block deliberately omits the empty-discovery fail because validSubnets renders first and errors out earlier. That's true today, but fragile: any future refactor that splits talos.config.machine.common and talos.config.cluster into different render passes (or skips the machine block for some path) silently emits an empty advertisedSubnets: list — exactly the silent-fail mode this PR is trying to eliminate.

A defensive duplicate of the same if not $addrs / fail check costs five lines and removes the cross-block dependency. Same applies to charts/generic/templates/_helpers.tpl:65-82.

      {{- else }}
      {{- $addrs := fromJsonArray (include "talm.discovered.default_addresses_by_gateway" .) }}
      {{- if not $addrs }}
      {{- fail "values.yaml: `advertisedSubnets` was left empty and talm could not derive a default from discovery. ..." }}
      {{- end }}
      {{- $subnets := list }}
      {{- range $addrs }}
      {{- $subnets = append $subnets (. | cidrNetwork) }}
      {{- end }}
      {{- range uniq $subnets }}
      - {{ . }}
      {{- end }}
      {{- end }}

[myasnikovdaniil]

operator-override path skips canonicalization

When the operator sets advertisedSubnets: ["192.168.201.10/24"] (host form) in values.yaml, the chart emits it verbatim via toYaml, while the fallback path runs every entry through cidrNetwork so the YAML is in canonical network form. That inconsistency means two operators with the same underlying intent — one relying on discovery, one with an explicit override — get differently-formatted machine configs, which is noisy in kubectl diff/talm diff output and surprising in code review. Consider canonicalizing in both branches:

      validSubnets:
        {{- $sources := .Values.advertisedSubnets }}
        {{- if not $sources }}
        {{- $sources = fromJsonArray (include "talm.discovered.default_addresses_by_gateway" .) }}
        {{- if not $sources }}
        {{- fail "values.yaml: `advertisedSubnets` was left empty and talm could not derive a default from discovery. ..." }}
        {{- end }}
        {{- end }}
        {{- $subnets := list }}
        {{- range $sources }}
        {{- $subnets = append $subnets (. | cidrNetwork) }}
        {{- end }}
        {{- range uniq $subnets }}
        - {{ . }}
        {{- end }}

This also de-duplicates the validSubnets and etcd.advertisedSubnets blocks down to a shared talos.discovered.subnet_list define, which is a sympathetic refactor for the comment in the etcd block that already says "handled the same way as validSubnets above." If you'd rather keep the operator-override path passthrough (e.g., to preserve operator-supplied formatting verbatim), call this out in a comment so future readers don't try to "fix" the inconsistency.

[myasnikovdaniil]

fail message field-name disambiguation

The error fires from inside the validSubnets: rendering block but the message references advertisedSubnets. The values.yaml key is advertisedSubnets, so the message is correct, but a user staring at the rendered (broken) output sees validSubnets: and may chase the wrong field. One word in the message would close the gap:

"values.yaml: `advertisedSubnets` (which feeds both kubelet validSubnets and etcd advertisedSubnets) was left empty and talm could not derive a default from discovery. ..."

Same in the generic copy.

[myasnikovdaniil]

cidrNetwork is fine; consider also exposing the bare network address

Not a request, just a flag for future iterations: callers may eventually want the network address without the prefix length (192.168.201.0 rather than 192.168.201.0/24) — for example when constructing CIDR list entries with explicit prefix logic. If a cidrNetworkAddress companion is ever needed, the netip.Prefix.Masked().Addr().String() form is one line. Out of scope for this PR.

[myasnikovdaniil]

coupling note in floatingIP comment is great, suggest explicit cross-link

The floatingIP comment says "MUST equal the host portion of endpoint above" but doesn't show the symmetric note in the endpoint comment (it does mention "set endpoint AND floatingIP below to the SAME IP"). The two comments together tell the right story, but a reader who hits one comment first without scrolling may miss the coupling. Consider a one-line cross-reference at the top of floatingIP:

# See `endpoint` above — these two values are coupled and MUST match
# for cozystack VIP setups.