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

pkg/redirectpolicy: Add missing namespace check in pod update handler #19193

Merged

Conversation

aditighag
Copy link
Member

@aditighag aditighag commented Mar 18, 2022

pkg/redirectpolicy: Add missing namespace check

Local Redirect Policy (LRP) namespace needs to match
with the backend pods selected by the LRP.

This check was missing in the case where backend
pods are deployed after an LRP that selects them
was applied.

Reported-By: Joe Stringer joe@covalent.io

Release note

Fix a bug where a backend pod can be selected by a local redirect policy deployed in a different namespace if the local redirect policy was deployed first. 

@aditighag aditighag requested review from joestringer, a team and borkmann March 18, 2022 19:15
@maintainer-s-little-helper
Copy link

Commit eb71afb9c3caa488563b96de9eceb5281e1d545a does not contain "Signed-off-by".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@maintainer-s-little-helper maintainer-s-little-helper bot added dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Mar 18, 2022
@aditighag aditighag force-pushed the pr/aditighag/lrp-fix-namespace-check branch from eb71afb to af85f46 Compare March 18, 2022 19:57
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Mar 18, 2022
@aditighag aditighag added the release-note/bug This PR fixes an issue in a previous release of Cilium. label Mar 18, 2022
@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, 2022
@aditighag
Copy link
Member Author

/test-only --focus="K8sLRPTests" --kernel_version=net-next

@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.10.9 Mar 18, 2022
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.11.3 Mar 18, 2022
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.9.14 Mar 18, 2022
@aditighag aditighag force-pushed the pr/aditighag/lrp-fix-namespace-check branch from af85f46 to 03dd99e Compare March 18, 2022 20:05
@aditighag
Copy link
Member Author

aditighag commented Mar 18, 2022

/test-only --focus="K8sLRPTests" --kernel_version=net-next

Edit : Passed - https://jenkins.cilium.io/job/Cilium-PR-Tests-Kernel-Focus/425/.

Local Redirect Policy (LRP) namespace needs to match
with the backend pods selected by the LRP.

This check was missing in the case where backend
pods are deployed after an LRP that selects them
was applied.

Added unit tests.

Reported-by: Joe Stringer <joe@covalent.io>
Signed-off-by: Aditi Ghag <aditi@cilium.io>
@aditighag aditighag force-pushed the pr/aditighag/lrp-fix-namespace-check branch from 03dd99e to 0ad534a Compare March 18, 2022 22:27
@aditighag aditighag requested a review from a team as a code owner March 18, 2022 22:27
@aditighag
Copy link
Member Author

/test-only --focus="K8sLRPTests" --kernel_version=net-next

@aditighag
Copy link
Member Author

Latest commits only include a documentation change.

Copy link
Contributor

@ldelossa ldelossa left a comment

Choose a reason for hiding this comment

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

noice 👍

@aanm aanm added this to Needs backport from master in 1.9.15 Mar 26, 2022
@aanm aanm removed this from Needs backport from master in 1.9.14 Mar 26, 2022
@aanm aanm added this to Needs backport from master in 1.10.10 Mar 26, 2022
@aanm aanm removed this from Needs backport from master in 1.10.9 Mar 26, 2022
@aanm aanm added this to Needs backport from master in 1.11.4 Mar 26, 2022
@aanm aanm removed this from Needs backport from master in 1.11.3 Mar 26, 2022
@joestringer joestringer merged commit 2efbdd6 into cilium:master Mar 28, 2022
@qmonnet qmonnet mentioned this pull request Mar 29, 2022
10 tasks
@qmonnet qmonnet mentioned this pull request Mar 30, 2022
18 tasks
@pchaigno pchaigno added backport-done/1.11 The backport for Cilium 1.11.x for this PR is done. and removed backport-pending/1.11 labels Apr 4, 2022
@joestringer joestringer moved this from Needs backport from master to Backport done to v1.9 in 1.9.15 Apr 15, 2022
@joestringer joestringer moved this from Needs backport from master to Backport done to v1.10 in 1.10.10 Apr 15, 2022
@joestringer joestringer moved this from Needs backport from master to Backport done to v1.11 in 1.11.4 Apr 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-done/1.11 The backport for Cilium 1.11.x for this PR is done. release-note/bug This PR fixes an issue in a previous release of Cilium.
Projects
No open projects
1.10.10
Backport done to v1.10
1.11.4
Backport done to v1.11
1.9.15
Backport done to v1.9
Development

Successfully merging this pull request may close these issues.

None yet

5 participants