feat(api): add healthCheckNodePort to EnvoyProxy service spec#8843
feat(api): add healthCheckNodePort to EnvoyProxy service spec#8843electricjesus wants to merge 2 commits intoenvoyproxy:mainfrom
Conversation
Allows specifying a fixed health-check NodePort on the Envoy Service when type is LoadBalancer and externalTrafficPolicy is Local. Without this, kube-apiserver auto-allocates the port from the cluster NodePort range, making it hard to allowlist in network policies or firewall rules. A CEL validation rule restricts the field to type: LoadBalancer with externalTrafficPolicy: Local, matching Kubernetes' own rules. When unset, behavior is unchanged and Kubernetes continues to auto-allocate. Fixes envoyproxy#8842 Signed-off-by: Seth Malaki <seth@tigera.io>
✅ Deploy Preview for cerulean-figolla-1f9435 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: adb0e10cd6
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| // +kubebuilder:validation:XValidation:message="allocateLoadBalancerNodePorts can only be set for LoadBalancer type",rule="!has(self.allocateLoadBalancerNodePorts) || self.type == 'LoadBalancer'" | ||
| // +kubebuilder:validation:XValidation:message="loadBalancerSourceRanges can only be set for LoadBalancer type",rule="!has(self.loadBalancerSourceRanges) || self.type == 'LoadBalancer'" | ||
| // +kubebuilder:validation:XValidation:message="loadBalancerIP can only be set for LoadBalancer type",rule="!has(self.loadBalancerIP) || self.type == 'LoadBalancer'" | ||
| // +kubebuilder:validation:XValidation:message="healthCheckNodePort can only be set for LoadBalancer type with Local externalTrafficPolicy",rule="!has(self.healthCheckNodePort) || (self.type == 'LoadBalancer' && (!has(self.externalTrafficPolicy) || self.externalTrafficPolicy == 'Local'))" |
There was a problem hiding this comment.
Mirror healthCheckNodePort checks in non-CEL validation
This adds a CEL rule for healthCheckNodePort, but the non-CEL validator (api/v1alpha1/validation/envoyproxy_validate.go in validateService) was not updated even though it is still used by internal/gatewayapi/translator.go and egctl paths. In those paths, an EnvoyProxy can pass validation and still produce a Service spec with healthCheckNodePort in unsupported combinations (for example after config merges), which Kubernetes then rejects during reconciliation. Please add equivalent checks in validateService so all validation paths enforce the same constraints.
Useful? React with 👍 / 👎.
Matches the pattern established for loadBalancerIP and loadBalancerSourceRanges: a Go-level check in validateService to catch invalid combinations in offline/CLI paths, unit tests for that check, and CEL admission tests covering both the pass (LoadBalancer with default or explicit Local externalTrafficPolicy) and fail (ClusterIP, or LoadBalancer with Cluster externalTrafficPolicy) cases. Signed-off-by: Seth Malaki <seth@tigera.io>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8843 +/- ##
==========================================
+ Coverage 74.39% 74.42% +0.03%
==========================================
Files 246 246
Lines 39221 39223 +2
==========================================
+ Hits 29177 29192 +15
+ Misses 8017 8008 -9
+ Partials 2027 2023 -4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
What type of PR is this?
feat (api)
What this PR does / why we need it:
Adds a new
HealthCheckNodePort *int32field toKubernetesServiceSpec, forwarded tocorev1.ServiceSpec.HealthCheckNodePortin the generated Envoy Service. This lets users pin the health-check NodePort to a known value so it can be allowlisted in network policies or firewall rules — a common requirement when running Envoy Gateway with cloud or on-prem load balancers that probe nodes via NodePort (bare-metal, MetalLB, kube-vip, etc.).Without this, Kubernetes auto-allocates
healthCheckNodePortfrom the cluster NodePort range (default 30000–32767) whenevertype: LoadBalancer+externalTrafficPolicy: Local. The allocated port is unpredictable across deployments and restarts, so operators end up either opening the entire NodePort range or resorting to theEnvoyService.Patchescape hatch. SettingallocateLoadBalancerNodePorts: falsedoesn't help, since it only controls the per-service-port NodePorts — not the health-check NodePort (k8s tracks those in separate allocators).A CEL validation rule restricts the new field to
type: LoadBalancerwithexternalTrafficPolicy: Local(or unset — the default isLocal), matching Kubernetes' own validation rules. When the field is unset, behavior is unchanged.Changes:
api/v1alpha1/shared_types.go: add field, CEL rule, range validation (1–65535)internal/infrastructure/kubernetes/resource/resource.go: forward the field inExpectedServiceSpeczz_generated.deepcopy.go, CRD manifests, helm-template fixtures, and API docsWhich issue(s) this PR fixes:
Fixes #8842
Release Notes: Yes