feat(hetzner): support per-nodepool Hetzner firewalls#9810
Conversation
Add an optional `firewalls` field to the nodeConfigs cluster-config, listing firewall ids or names attached to that pool's servers in addition to the cluster-wide HCLOUD_FIREWALL. The cluster and per-pool firewalls are merged (deduplicated by id) at server creation, so a pool can carry extra rules without relaxing the firewall on the rest of the cluster. Resolved at node-group build time and fail-fast if a firewall is missing, mirroring the existing placementGroup handling. New-format only; HCLOUD_FIREWALL is unchanged and remains cluster-wide. Signed-off-by: Tuomo <tjorri@gmail.com>
|
|
|
This issue is currently awaiting triage. If SIG Autoscaling contributors determines this is a relevant issue, they will accept it by applying the The DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
Welcome @tjorri! |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: tjorri The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Hi @tjorri. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
Hey @tjorri, thanks for the contribution 🚀 The idea looks good to me. I just noticed, that the API is return an internal server error when the same Firewall ID is specified more than once in the create Server call. I would just like to sort out this issue with the corresponding internal team and then do a review here. |
| placementGroupTotals[placementGroup.Name] += spec.maxSize | ||
| } | ||
|
|
||
| for _, firewallRef := range manager.clusterConfig.NodeConfigs[spec.name].Firewalls { |
There was a problem hiding this comment.
At this stage we could already append the global Firewall and deduplicate the list. Then we don't need to do this for every create Server call.
If we don't use the hcloud.Firewall object anywhere else, we could directly store the hcloud.ServerCreateFirewall object.
| firewall, _, err := manager.client.Firewall.Get(ctx, firewallRef) | ||
|
|
||
| // Check if an error occurred | ||
| if err != nil { |
There was a problem hiding this comment.
| firewall, _, err := manager.client.Firewall.Get(ctx, firewallRef) | |
| // Check if an error occurred | |
| if err != nil { | |
| firewall, _, err := manager.client.Firewall.Get(ctx, firewallRef) | |
| if err != nil { |
What type of PR is this?
/kind feature
What this PR does / why we need it:
Today the Hetzner cloud provider attaches a single, cluster-wide firewall (
HCLOUD_FIREWALL) to every server it creates. There is no way to give one nodepool extra firewall rules without applying them to the entire cluster.This PR adds a
firewallsfield toNodeConfig(nodeConfigsinHCLOUD_CLUSTER_CONFIG) listing additional firewall ids or names to attach to that pool's servers, on top ofHCLOUD_FIREWALL. The cluster firewall and the per-nodepool firewalls are merged (deduplicated by id) at server creation, so a pool can carry extra rules — e.g. a dedicated ingress/edge pool that needs a public port range — without relaxing the firewall on the rest of the cluster.Firewalls are resolved once at provider initialization and fail fast if a referenced firewall does not exist, mirroring the existing
placementGrouphandling.HCLOUD_FIREWALLis unchanged and remains cluster-wide; the new field is optional and additive, so existing deployments behave identically. Per-nodepool firewalls require thenodeConfigsformat.Which issue(s) this PR fixes:
Fixes #9809
Special notes for your reviewer:
HCLOUD_FIREWALLbehaviour is untouched andfirewallsdefaults to empty.placementGrouppattern: resolved inBuildHetzner, stored on the node group, applied increateServer.TestBuildServerCreateFirewalls); README updated.Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: