NO-JIRA: Guard nil dereference in CachedConfigMapGetter and CachedSecretGetter - #2378
NO-JIRA: Guard nil dereference in CachedConfigMapGetter and CachedSecretGetter#2378sanchezl wants to merge 1 commit into
Conversation
CachedConfigMapGetter.ConfigMaps() and CachedSecretGetter.Secrets() chain method calls on the return value of InformersFor(), which returns nil for unregistered namespaces. This produces an unhelpful nil-pointer panic with no diagnostic context. Add a nil check before chaining, matching the pattern already used by the lister wrappers in informers.go (configMapLister.ConfigMaps, etc.) which panic with a message identifying the missing namespace.
|
@sanchezl: This pull request explicitly references no jira issue. DetailsIn response to this:
Instructions 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 openshift-eng/jira-lifecycle-plugin repository. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: openshift/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
WalkthroughThe cached ConfigMap and Secret getters now validate namespace informer availability and panic with a formatted missing-namespace message. Tests cover missing and registered namespaces, multiple fake informer configurations, and exact panic-message matching. ChangesCached getter validation
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: sanchezl 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 |
|
@sanchezl: all tests passed! Full PR test history. Your PR dashboard. 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. I understand the commands that are listed here. |
Summary
CachedConfigMapGetter.ConfigMaps()andCachedSecretGetter.Secrets()chain method calls on the return value ofInformersFor(), which returnsnilfor unregistered namespaces. This produces an unhelpful nil-pointer panic with no diagnostic context.informers.go(configMapLister.ConfigMaps,secretLister.Secrets,podLister.Pods) which panic with a message identifying the missing namespace.Test plan
go test ./pkg/operator/v1helpers/...passesmake verifypassesSummary by CodeRabbit
Bug Fixes
Tests