feat(hetzner): floating-IP config surface + stable API endpoint wiring for Talos × Hetzner#5719
Merged
Merged
Conversation
…g for Talos × Hetzner Adds the opt-in config surface for the stable-API-endpoint lane (platform#2120 option A): floatingIPEnabled + floatingIPLocation on OptionsHetzner. When enabled, cluster create ensures the cluster-owned floating IP (EnsureFloatingIP, #5699), attaches it to the first control-plane server, and renders it as the cluster endpoint with the floating IP plus every control-plane node IP in the certificate SANs (node IPs must stay verifiable — readiness checks and break-glass access dial nodes directly). Disabled (default) renders byte-identical configs. Node-side ownership handover (Talos VIP block) ships separately (#5718); until then the address must be claimed via a user Talos patch. Fixes #5714
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds Hetzner floating IP configuration, defaulting, Talos endpoint/SAN rendering, tests, and matching schema/docs updates. ChangesHetzner Floating IP Endpoint Feature
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant Provisioner
participant ensureFloatingIPEndpoint
participant HetznerProvider
participant TalosConfigs
Provisioner->>Provisioner: updateConfigsWithEndpoint(ctx, hzProvider, clusterName, controlPlaneServers)
alt FloatingIPEnabled
Provisioner->>ensureFloatingIPEndpoint: ensure and attach floating IP
ensureFloatingIPEndpoint->>HetznerProvider: create/find floating IP
HetznerProvider-->>ensureFloatingIPEndpoint: floating IP address
ensureFloatingIPEndpoint->>HetznerProvider: attach floating IP to control-plane server
ensureFloatingIPEndpoint-->>Provisioner: floating IP + SAN list
Provisioner->>TalosConfigs: regenerate configs with floating IP endpoint and SANs
else FloatingIPDisabled
Provisioner->>TalosConfigs: regenerate configs with first control-plane IP as endpoint
end
Possibly related issues
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
✅MegaLinter analysis: Success✅ Linters with no issuesactionlint, bash-exec, git_diff, hadolint, jscpd, jsonlint, lychee, markdown-table-formatter, markdownlint, prettier, prettier, shellcheck, shfmt, stylelint, syft, trivy-sbom, trufflehog, v8r, v8r, yamllint Notices📣 MegaLinter 9.5.0 is out! Discover the new features and security recommendations in the release announcement. (Skip this info by defining See detailed reports in MegaLinter artifacts
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
pkg/svc/provisioner/cluster/talos/provisioner_hetzner_floating_ip_test.go (1)
166-226: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd error-path coverage for
ensureFloatingIPEndpoint.Current tests cover disabled/enabled happy paths and the no-control-plane guard, but not
EnsureFloatingIPfailing (e.g. an existing non-ksail-owned floating IP, or ownership rejection) norAttachFloatingIPToServerfailing (e.g. the assign action returning an error). Both are realistic failure modes of this new opt-in path and aren't exercised.Want me to draft these two additional test cases using the existing
floatingIPEndpointTestServer-style httptest fixtures?🤖 Prompt for 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. In `@pkg/svc/provisioner/cluster/talos/provisioner_hetzner_floating_ip_test.go` around lines 166 - 226, Add error-path test coverage for ensureFloatingIPEndpoint in the floating IP opt-in flow. Extend the existing floatingIPEndpointTestServer-style fixture and the TestUpdateConfigsWithEndpoint_FloatingIPEnabled area to cover two failures: EnsureFloatingIP returning an error for a non-owned or rejected floating IP, and AttachFloatingIPToServer failing when the assign action errors. Use UpdateConfigsWithEndpointForTest and the Hetzner provider path to assert these errors are surfaced.pkg/svc/provisioner/cluster/talos/provisioner_hetzner.go (1)
159-178: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant recomputation of the first control-plane node's address.
firstCPIPis computed at Line 159 and then discarded wheneverFloatingIPEnabledis true (overwritten at Line 176), andensureFloatingIPEndpoint's loop overcontrolPlaneServers(Lines 235-242) recomputeshetznerNodeTalosAddressforcontrolPlaneServers[0]a second time. Not a correctness issue, just avoidable duplicate work.♻️ Possible simplification
- certSANs := make([]string, 0, len(controlPlaneServers)+1) - certSANs = append(certSANs, endpointIP) - - for _, server := range controlPlaneServers { - nodeIP, nodeAddrErr := hetznerNodeTalosAddress(server) - if nodeAddrErr != nil { - return "", nil, nodeAddrErr - } - - certSANs = append(certSANs, nodeIP) - } + certSANs := make([]string, 0, len(controlPlaneServers)+1) + certSANs = append(certSANs, endpointIP, firstCPIP) + + for _, server := range controlPlaneServers[1:] { + nodeIP, nodeAddrErr := hetznerNodeTalosAddress(server) + if nodeAddrErr != nil { + return "", nil, nodeAddrErr + } + + certSANs = append(certSANs, nodeIP) + }(requires threading
firstCPIPintoensureFloatingIPEndpoint's parameters)Also applies to: 232-242
🤖 Prompt for 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. In `@pkg/svc/provisioner/cluster/talos/provisioner_hetzner.go` around lines 159 - 178, Avoid recomputing the first control-plane address in provisioner_hetzner.go: the initial hetznerNodeTalosAddress result is already available as firstCPIP in the main provisioning flow and is being recalculated again inside ensureFloatingIPEndpoint. Thread firstCPIP into ensureFloatingIPEndpoint and reuse it for the first server instead of calling hetznerNodeTalosAddress(controlPlaneServers[0]) again, while keeping the endpointIP assignment logic in the caller unchanged.
🤖 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.
Nitpick comments:
In `@pkg/svc/provisioner/cluster/talos/provisioner_hetzner_floating_ip_test.go`:
- Around line 166-226: Add error-path test coverage for ensureFloatingIPEndpoint
in the floating IP opt-in flow. Extend the existing
floatingIPEndpointTestServer-style fixture and the
TestUpdateConfigsWithEndpoint_FloatingIPEnabled area to cover two failures:
EnsureFloatingIP returning an error for a non-owned or rejected floating IP, and
AttachFloatingIPToServer failing when the assign action errors. Use
UpdateConfigsWithEndpointForTest and the Hetzner provider path to assert these
errors are surfaced.
In `@pkg/svc/provisioner/cluster/talos/provisioner_hetzner.go`:
- Around line 159-178: Avoid recomputing the first control-plane address in
provisioner_hetzner.go: the initial hetznerNodeTalosAddress result is already
available as firstCPIP in the main provisioning flow and is being recalculated
again inside ensureFloatingIPEndpoint. Thread firstCPIP into
ensureFloatingIPEndpoint and reuse it for the first server instead of calling
hetznerNodeTalosAddress(controlPlaneServers[0]) again, while keeping the
endpointIP assignment logic in the caller unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: de3a93c8-20cf-4f14-b3ef-9d3d24e569da
📒 Files selected for processing (8)
docs/src/content/docs/configuration/declarative-configuration.mdxpkg/apis/cluster/v1alpha1/envvar_drift_test.gopkg/apis/cluster/v1alpha1/options.gopkg/svc/provisioner/cluster/talos/export_test.gopkg/svc/provisioner/cluster/talos/factory.gopkg/svc/provisioner/cluster/talos/provisioner_hetzner.gopkg/svc/provisioner/cluster/talos/provisioner_hetzner_floating_ip_test.goschemas/ksail-config.schema.json
Contributor
Code Coverage OverviewLanguages: Go Go / code-coverage/goThe overall coverage in the branch remains at 64%, unchanged from the branch. Show a code coverage summary of the most impacted files.
Code Coverage is in Public Preview. Learn more and provide us with your feedback. |
This was referenced Jul 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #5714 — slice 2 of the stable-API-endpoint lane (devantler-tech/platform#2120, maintainer-confirmed option A; slice 1 = the provider primitives, merged via #5699).
What
OptionsHetzner, flat fields per the PlacementGroup* precedent):floatingIPEnabled(default false) +floatingIPLocation(defaults to the cluster'slocation, applied inapplyHetznerDefaults).updateConfigsWithEndpointensures the cluster-owned floating IP, attaches it to the first control-plane server, and renders it as the cluster endpoint via the existing PKI-preservingWithEndpoint+WithCertSANsseams. The SAN set = floating IP plus every control-plane node IP — the node IPs must stay verifiable because KSail's own readiness checks (and break-glass access) dial nodes directly.schemas/ksail-config.schema.json(gen_schema) +declarative-configuration.mdx(go generate ./docs/...) — regenerated, not hand-edited.location).Flags for steering
machine.networkVIP patch; the schema description and field doc say so explicitly. Opt-in + default-off keeps this safe.floatingIPEnabled/floatingIPLocation(notfloatingIp…) to match the siblingworkerPublicIPv4/controlPlanePublicIPv4fields; tagliatelle silenced with a reasoned nolint per thematchOIDCIdentityprecedent.Validation
go build ./...clean; 1317 tests green across the touched packages (apis, provisioner/talos, configmanager/talos, schemas), including 7 new tests: defaults resolution (empty → cluster location, custom preserved), disabled-path no-op (endpoint = CP IP, no SAN patch, no API calls), enabled-path via thehttptestfake Hetzner API from thefloating_ip_test.gopattern (attach called once, endpoint = floating IP, SANs = FIP + both CP IPs), and the no-control-planes guard.golangci-lint run --new-from-rev origin/main: no issues.Follow-ups after this slice: #5718 (Talos VIP generation), then the platform-side Talos patch +
ksail.prod.yaml+ runbook Scenario 9 (HIGH blast-radius CP roll, maintainer-sequenced per #2120).