fix: return early in service reconciler when lb is nil#4725
fix: return early in service reconciler when lb is nil#4725joey-jonko-paypal wants to merge 1 commit into
Conversation
|
|
Welcome @joey-jonko-paypal! |
|
Hi @joey-jonko-paypal. Thanks for your PR. I'm waiting for a kubernetes-sigs 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. |
When buildModel returns lb == nil (e.g. for a ClusterIP service that should not have a load balancer, such as one created by Istio Gateway API with networking.istio.io/service-type: ClusterIP), the reconciler correctly runs cleanupLoadBalancerResources but then falls through to reconcileLoadBalancerResources with lb still nil. This causes a nil pointer dereference panic at lb.DNSName().Resolve(ctx). Add a return nil after successful cleanup when lb == nil so the reconciler exits cleanly without entering the LB provisioning path. Related: kubernetes-sigs#4724 Signed-off-by: Joey Jonko <joey.jonko@happyreturns.com>
77d7545 to
b299b3d
Compare
|
Do you want to fix EasyCLA ? Else, I can submit a change for this. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4725 +/- ##
==========================================
+ Coverage 56.09% 56.13% +0.03%
==========================================
Files 388 388
Lines 30932 30948 +16
==========================================
+ Hits 17352 17373 +21
+ Misses 12566 12564 -2
+ Partials 1014 1011 -3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: joey-jonko-paypal, sandeep-rajan-glean 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 |
What does this PR do?
Fixes a nil pointer dereference panic in the service reconciler that fires when
buildModelreturnslb == nil— for example, when aClusterIP-typed Service is reconciled (such as one created by Istio Gateway API vianetworking.istio.io/service-type: ClusterIP).Root cause
In
reconcile(), whenlb == nil, the reconciler correctly enters the cleanup branch and callscleanupLoadBalancerResources. However, on successful cleanup it falls through toreconcileLoadBalancerResources(ctx, svc, stack, lb, backendSGRequired)withlbstillnil. This causes a nil pointer dereference insidereconcileLoadBalancerResourcesat:The panic is recovered by
runtime.HandleReconcileError, so it manifests as repeated error log noise rather than a crash. Load balancer provisioning is unaffected.Fix
Add
return nilafter the successful cleanup path whenlb == nil, so the reconciler exits cleanly:How to reproduce
networking.istio.io/service-type: ClusterIP(used to suppress NLB provisioning when an external ALB is managed separately via a Kubernetes Ingress resource).<gateway-name>-istioClusterIP service in the controller's namespace.Testing
Existing unit tests pass. The code path is straightforward: the only change is the addition of
return nilto prevent the fall-through.Closes #4724
Release note: