fix: bind loadbalancer healthcheck endpoint to localhost by default (…#12834
fix: bind loadbalancer healthcheck endpoint to localhost by default (…#12834hadi2f244 wants to merge 1 commit intokubernetes-sigs:masterfrom
Conversation
|
|
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: hadi2f244 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 @hadi2f244. Thanks for your PR. I'm waiting for a github.com member to verify that this patch is reasonable to test. If it is, they should reply with 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. |
There was a problem hiding this comment.
Pull request overview
This PR addresses a security concern by changing the default bind address of the internal load balancer health check endpoint from all interfaces (0.0.0.0/::) to localhost (127.0.0.1/::1) when using NGINX or HAProxy as the internal load balancer for the Kubernetes API server.
Key changes:
- Introduces two new configurable variables for IPv4 and IPv6 health check bind addresses with secure defaults
- Updates NGINX and HAProxy configuration templates to use the new bind address variables
- Maintains backward compatibility by allowing users to override the defaults for external health check access
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| roles/kubernetes/node/defaults/main.yml | Adds new default variables loadbalancer_apiserver_healthcheck_bind_address (127.0.0.1) and loadbalancer_apiserver_healthcheck_bind_address_ipv6 (::1) for controlling health check endpoint binding |
| roles/kubernetes/node/templates/loadbalancer/nginx.conf.j2 | Updates NGINX configuration to use the new bind address variables for both IPv4 and IPv6 health check endpoints |
| roles/kubernetes/node/templates/loadbalancer/haproxy.cfg.j2 | Updates HAProxy configuration to use the new bind address variables for both IPv4 and IPv6 health check endpoints |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| bind {{ loadbalancer_apiserver_healthcheck_bind_address }}:{{ loadbalancer_apiserver_healthcheck_port }} | ||
| {% if ipv6_stack -%} | ||
| bind :::{{ loadbalancer_apiserver_healthcheck_port }} | ||
| bind {{ loadbalancer_apiserver_healthcheck_bind_address_ipv6 }}:{{ loadbalancer_apiserver_healthcheck_port }} |
There was a problem hiding this comment.
The IPv6 address in HAProxy bind configuration requires square brackets. The current syntax {{ loadbalancer_apiserver_healthcheck_bind_address_ipv6 }}:{{ loadbalancer_apiserver_healthcheck_port }} will fail with IPv6 addresses like ::1. HAProxy expects IPv6 addresses to be wrapped in square brackets, similar to the nginx configuration and the existing kube_api_frontend configuration at line 35. The correct syntax should be [{{ loadbalancer_apiserver_healthcheck_bind_address_ipv6 }}]:{{ loadbalancer_apiserver_healthcheck_port }}.
| bind {{ loadbalancer_apiserver_healthcheck_bind_address_ipv6 }}:{{ loadbalancer_apiserver_healthcheck_port }} | |
| bind [{{ loadbalancer_apiserver_healthcheck_bind_address_ipv6 }}]:{{ loadbalancer_apiserver_healthcheck_port }} |
|
Is there an actual use case for external access to those LB ? Because if not, I'll think we should just bind to localhost and not make this configurable. |
|
/ok-to-test |
I've searched and couldn't find any usage for external access. We can just make it expose to localhost. |
|
Let's do this then 👍 |
fix: bind loadbalancer healthcheck endpoint to localhost by default (#12809)
What type of PR is this?
/kind bug
What this PR does / why we need it:
This PR addresses a security concern where the internal NGINX/HAProxy load balancer health check endpoint was bound to all interfaces (0.0.0.0:8081 and [::]:8081) when
loadbalancer_apiserver_localhost: trueis enabled. This unnecessarily exposes an internal endpoint to external networks.The fix introduces two new variables for full control over bind addresses:
loadbalancer_apiserver_healthcheck_bind_address- defaults to127.0.0.1(IPv4 localhost)loadbalancer_apiserver_healthcheck_bind_address_ipv6- defaults to::1(IPv6 localhost)Key improvements:
0.0.0.0/::if external health checks are neededWhich issue(s) this PR fixes:
Fixes #12809
Special notes for your reviewer:
Does this PR introduce a user-facing change?:
The internal load balancer health check endpoint now binds to localhost (127.0.0.1 for IPv4, ::1 for IPv6) by default instead of all interfaces (0.0.0.0, ::) for improved security. Users requiring external health check access can configure
loadbalancer_apiserver_healthcheck_bind_addressandloadbalancer_apiserver_healthcheck_bind_address_ipv6to bind to all interfaces.