Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ingress: Avoid potential nil pointer during cleanup #24444

Merged
merged 1 commit into from Mar 22, 2023

Conversation

sayboras
Copy link
Member

@sayboras sayboras commented Mar 18, 2023

Description

As part of garbage collected steps in default Ingress class changes, the garbage collection process kicks off based on our translation logic. However, depending on which load-balancer mode is used, the translated service or endpoint might be nil. This commit is to make sure that we only performs GC if applicable. Ideally, only Service and Endpoint objs should be checked, but for readability, nil check for CEC object is also done.

Fixes: 85671ad
Fixes: #24347

Notes

This is only happening in master branch but not 1.13 branch, so backport is not required.

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Mar 18, 2023
@sayboras sayboras added the release-note/bug This PR fixes an issue in a previous release of Cilium. label Mar 18, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Mar 18, 2023
@sayboras sayboras marked this pull request as ready for review March 18, 2023 09:31
@sayboras sayboras requested a review from a team as a code owner March 18, 2023 09:31
@sayboras sayboras requested a review from youngnick March 18, 2023 09:31
As part of garbage collected steps in default Ingress class changes, the
garbage collection process kicks off based on our translation logic.
However, depending on which load-balancer mode is used, the translated
service or endpoint might be nil. This commit is to make sure that we
only performs GC if applicable. Ideally, only Service and Endpoint objs
should be checked, but for readability, nil check for CEC object is also
done.

Fixes: 85671ad
Fixes: cilium#24347
Signed-off-by: Tam Mach <tam.mach@cilium.io>
@sayboras
Copy link
Member Author

sayboras commented Mar 21, 2023

/test

Job 'Cilium-PR-K8s-1.26-kernel-net-next' failed:

Click to show.

Test Name

K8sUpdates Tests upgrade and downgrade from a Cilium stable image to master

Failure Output

FAIL: Expected

If it is a flake and a GitHub issue doesn't already exist to track it, comment /mlh new-flake Cilium-PR-K8s-1.26-kernel-net-next so I can create one.

@sayboras
Copy link
Member Author

/test-1.26-net-next

Copy link
Member

@meyskens meyskens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@sayboras sayboras merged commit a17f306 into cilium:master Mar 22, 2023
42 checks passed
@sayboras sayboras deleted the tam/ingress-cleanup branch March 22, 2023 08:11
@christarazi christarazi added kind/bug This is a bug in the Cilium logic. release-note/misc This PR makes changes that have no direct user impact. area/servicemesh GH issues or PRs regarding servicemesh and removed release-note/bug This PR fixes an issue in a previous release of Cilium. labels Mar 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/servicemesh GH issues or PRs regarding servicemesh kind/bug This is a bug in the Cilium logic. release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Potential nil dereference while handling ingress events
4 participants