feat(charts): allow overriding of the cluster's name

[dislogical]

This allows users to override the generated cluster name in the case they don't want it to match the chart name.

Summary by CodeRabbit

• New Features

  • Customizable cluster naming in Helm charts with automatic fallback to the chart name and enforced DNS-1123 validation (invalid names now produce an error).

• Configuration

  • Added optional clusterName chart value to override the default name.

• Tests

  • Rendering tests updated to validate override behavior and enforce quoted clusterName output.

[claude]

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

[coderabbitai]

Warning

Rate limit exceeded

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

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ 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: 3b46981b-dd95-474f-b198-1183feb63be5

📥 Commits

Reviewing files that changed from the base of the PR and between 3b7c174 and c388ee7.

📒 Files selected for processing (6)

• charts/cozystack/templates/_helpers.tpl
• charts/cozystack/values.yaml
• charts/generic/templates/_helpers.tpl
• charts/generic/values.yaml
• pkg/commands/talosconfig.go
• pkg/engine/render_test.go

📝 Walkthrough

Walkthrough

Both Helm charts (cozystack and generic) now accept an optional clusterName configuration value that overrides the chart name. Templates validate and quote the resolved name as a DNS-1123 label; CLI fallback logic and render tests were updated to use and verify this behavior.

Changes

Cluster Name Configurability

Layer / File(s)	Summary
Configuration Schema charts/cozystack/values.yaml, charts/generic/values.yaml	New optional clusterName value added to both charts (clusterName: ""), documented to default to .Chart.Name when empty.
Template Implementation charts/cozystack/templates/_helpers.tpl, charts/generic/templates/_helpers.tpl	talos.config.cluster now resolves clusterName from .Values.clusterName with .Chart.Name fallback, applies regexFind (DNS-1123) + required, then quote.
CLI / Chart Reader pkg/commands/talosconfig.go	getClusterNameFromChart() now prefers values.yaml's clusterName and falls back to Chart.yaml name.
Tests pkg/engine/render_test.go	Render tests strengthened to expect exact quoted clusterName; added multidoc tests validating invalid/valid override behavior.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A name once bound to chart and line,
Now hops free — a string that's mine,
Quoted, checked, and small and neat,
Valid hops on careful feet,
Cluster blooms where names align.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: adding the ability to override the cluster's name in charts, which is the core objective of this pull request.
Docstring Coverage	✅ Passed	Docstring coverage is 81.82% which is sufficient. The required threshold is 80.00%.
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

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

• Generate code and open pull requests
• Plan features and break down work
• Investigate incidents and troubleshoot customer tickets together
• Automate recurring tasks and respond to alerts with triggers
• Summarize progress and report instantly

Built for teams:

• Shared memory across your entire org—no repeating context
• Per-thread sandboxes to safely plan and execute work
• Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started

---

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 introduces the ability to override the cluster name in both the cozystack and generic Helm charts. It adds a clusterName field to values.yaml and updates the _helpers.tpl templates to use this value if provided, defaulting to the chart name. I have no feedback to provide.

[lexfrei]

NOT LGTM — the override is wired only into the chart templates; the talm talosconfig regeneration path still reads the cluster name from Chart.yaml, and the new behaviour ships without test coverage that pins either the legacy default or the override precedence.

Business context: lets users override the cluster name without renaming the chart directory or re-running talm init --name. The previous escape hatch (--name baked into Chart.yaml at init) requires re-init; this PR makes the override mutable post-init via values.yaml.

Blockers

B1: talm talosconfig regenerates with the chart name, not the new override

File: pkg/commands/talosconfig.go:131-137 (interaction with charts/{cozystack,generic}/templates/_helpers.tpl)

When regenerateTalosconfig runs without an existing talosconfig (fresh checkout, deleted file, or first talm talosconfig after bootstrap loss), it falls back to getClusterNameFromChart(), which reads only Chart.yaml.name. The rendered Talos machine config now has cluster.clusterName: <values.clusterName>, but the regenerated talosconfig context is <Chart.Name>.

getClusterNameFromChart() at pkg/commands/talosconfig.go:194-210 parses only Chart.yaml; it never opens values.yaml. The path is reachable via the user-facing talm talosconfig command (talosconfig.go:34-82), which is the documented "regenerate after cert expiry" flow.

Impact: silent cluster-name divergence between client and server config whenever the override is in use and the regenerate path is hit.

Fix: either (a) extend getClusterNameFromChart() to also load values.yaml and prefer clusterName when non-empty, or (b) call out in the PR description and the values.yaml comment that the override is template-only and the user must keep Chart.yaml.name in sync.

B2: Missing test coverage for the new behaviour

File: pkg/engine/render_test.go

Current tests (lines 231, 292, 345, 507) only assert the clusterName: substring is present — none of them pin the rendered value. Both the legacy fallback and the new override are uncovered, and the change touches two charts (cozystack and generic) with independent template files, so a "fixed cozystack, forgot generic" regression would slip through CI. Minimum coverage expected before merge:

Backward compatibility (legacy contract — must not regress)

1. clusterName: "" (explicit empty in values) → rendered output contains clusterName: "<Chart.Name>". Confirms the default pipeline restores the prior literal.
2. clusterName key entirely absent from values → same clusterName: "<Chart.Name>". Guards against a future values.yaml cleanup dropping the key and silently breaking the fallback.

Override precedence

3. clusterName: "my-cluster" → rendered output contains clusterName: "my-cluster", NOT <Chart.Name>. Confirms the override beats the default.

Edge cases that pin current behaviour

4. Special characters / spaces: clusterName: "foo bar" → output contains clusterName: "foo bar" (quoted). Confirms quote actually fires.
5. Override equal to Chart.Name: idempotent — same output as case 1.
6. YAML-ambiguous string values: clusterName: "true" and clusterName: "123" → output keeps them as quoted strings, not booleans/ints. Without quote these would round-trip as native YAML scalars.
7. Whitespace-only: clusterName: " " → output is clusterName: " " (sprig default does NOT treat whitespace as empty). Either pin this behaviour with a comment explaining why, or add a trim in the template — but the test must exist either way.

Both charts

Cases 1–3 must be exercised against BOTH charts/cozystack and charts/generic (the diff touches both _helpers.tpl files independently). A single shared helper executed twice with different chart roots is fine.

The existing pkg/engine/render_test.go fixtures already provide an inline-values pattern; the simplest delivery is to convert one of the existing four assertContains(t, output, "clusterName:") call sites into a table-driven subtest covering the cases above.

Non-blocking follow-ups

1. The values.yaml comment ("Optional override for the cluster's name (defaults to Chart.Name)") understates the implication: cluster.clusterName is baked into PKI (cert SANs) and ETCD identity at bootstrap. Changing it on a live cluster is unsafe. A one-sentence warning in the comment would prevent the typical "I'll just rename my cluster" mistake.
2. No DNS-1123 validation. A non-conforming string (uppercase, spaces, special chars) propagates through and fails downstream in Talos with a less obvious error. An optional Helm fail guard could surface this at template time.
3. README's only mention of clusterName is in a sample output snippet (line 154). Neither the existing --name init flag nor the new override are documented as user-facing knobs. Pre-existing gap, not made worse by this PR.

[dislogical]

Thanks for feedback! I've addressed all comments except non-blocking 3, which seems out of scope for this work.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@pkg/commands/talosconfig.go`:
- Around line 196-204: The clusterName parsing logic is inverted: the
os.ReadFile and yaml.Unmarshal error checks are currently gating the happy path
so values.yaml is never used; update the block that reads valuesYamlPath to
first proceed only when os.ReadFile returns nil (err == nil), then call
yaml.Unmarshal on the read data and, if yaml.Unmarshal returns nil and
valuesData.ClusterName is not empty, return valuesData.ClusterName; reference
the same symbols (valuesYamlPath, valuesData struct, os.ReadFile,
yaml.Unmarshal) and ensure errors are handled/ignored only on failure so the
override behavior from values.yaml works.

🪄 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: 5959e22a-2547-4f2d-9934-9f20c3c2845c

📥 Commits

Reviewing files that changed from the base of the PR and between 06d49fb and 3b7c174.

📒 Files selected for processing (6)

• charts/cozystack/templates/_helpers.tpl
• charts/cozystack/values.yaml
• charts/generic/templates/_helpers.tpl
• charts/generic/values.yaml
• pkg/commands/talosconfig.go
• pkg/engine/render_test.go

🚧 Files skipped from review as they are similar to previous changes (4)

• charts/generic/values.yaml
• charts/cozystack/values.yaml
• charts/cozystack/templates/_helpers.tpl
• charts/generic/templates/_helpers.tpl

[lexfrei]

LGTM — both prior blockers are addressed: getClusterNameFromChart now reads values.yaml.clusterName ahead of Chart.yaml.name, and the test suite pins the override behaviour with both positive and negative cases on each chart.

Business context: lets users override the cluster name without renaming the chart directory or re-running talm init --name.

What changed since the prior review

• pkg/commands/talosconfig.go:194-219 — getClusterNameFromChart now resolves values.yaml.clusterName first, falls back to Chart.yaml.name. Closes the regenerate-path divergence flagged in the previous review.
• pkg/engine/render_test.go — four new tests (TestMultiDoc{Cozystack,Generic}_{Valid,Invalid}ClusterNameOverride) and four existing tests upgraded from substring-only clusterName: to value-pinning clusterName: "<expected>". Closes the test-coverage blocker on the chart side.
• charts/{cozystack,generic}/values.yaml — comment now warns that changing the value on a live cluster is dangerous (PKI / ETCD identity baked at bootstrap).
• charts/{cozystack,generic}/templates/_helpers.tpl — DNS-1123 regex via regexFind ... | required rejects invalid names at render time.

Non-blocking follow-ups

1. No unit test in pkg/commands covers the new values.yaml.clusterName > Chart.yaml.name priority in getClusterNameFromChart. The chart-side tests in render_test.go exercise the template, not the regenerate-talosconfig flow. A short test that creates a tmp dir with values.yaml: clusterName: foo + Chart.yaml: name: bar and asserts the helper returns foo would close the gap.
2. The DNS-1123 regex does not enforce the 63-character label length cap (RFC 1035). A name longer than 63 chars passes the chart validation but Talos / Kubernetes downstream may still reject it. Minor; the regex captures the practical typo cases (uppercase, underscore, leading dash, etc.) which is the high-value fraction.

[dislogical]

Thank you!