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

daemon: Remove encrypt key from syncHostIPs() #25252

Conversation

christarazi
Copy link
Member

There's no need for the encrypt key to be set on host IPs. Additionally,
this code was also setting the encrypt key for the 0.0.0.0/0 world
entry as well, which isn't necessary.

Encryption is done for node-to-node traffic therefore it's not necessary
for encrypt key to be set on local host IPs.

Fixes: b698972 (cilium: ipsec, support rolling updates)

Signed-off-by: Chris Tarazi chris@isovalent.com

There's no need for the encrypt key to be set on host IPs. Additionally,
this code was also setting the encrypt key for the `0.0.0.0/0` `world`
entry as well, which isn't necessary.

Encryption is done for node-to-node traffic therefore it's not necessary
for encrypt key to be set on local host IPs.

Fixes: b698972 (cilium: ipsec, support rolling updates)

Signed-off-by: Chris Tarazi <chris@isovalent.com>
@christarazi christarazi added kind/enhancement This would improve or streamline existing functionality. area/daemon Impacts operation of the Cilium daemon. sig/policy Impacts whether traffic is allowed or denied based on user-defined policies. area/encryption Impacts encryption support such as IPSec, WireGuard, or kTLS. release-note/misc This PR makes changes that have no direct user impact. labels May 3, 2023
@christarazi christarazi marked this pull request as ready for review May 3, 2023 20:53
@christarazi christarazi requested a review from a team as a code owner May 3, 2023 20:53
@christarazi christarazi requested review from gentoo-root, a team and gandro and removed request for a team May 3, 2023 20:53
Copy link
Member

@pchaigno pchaigno left a comment

Choose a reason for hiding this comment

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

Thanks!

@christarazi
Copy link
Member Author

/test-runtime

@christarazi
Copy link
Member Author

/test-1.25-4.19

Copy link
Contributor

@gentoo-root gentoo-root left a comment

Choose a reason for hiding this comment

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

I'm not an expert in this code area, I hope it doesn't remove encryption between nodes :D

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label May 4, 2023
@michi-covalent
Copy link
Contributor

is the button green? yes.

@michi-covalent michi-covalent merged commit 7973477 into cilium:main May 4, 2023
57 checks passed
@christarazi christarazi deleted the pr/christarazi/encrypt-key-syncHostIPs branch May 4, 2023 17:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/daemon Impacts operation of the Cilium daemon. area/encryption Impacts encryption support such as IPSec, WireGuard, or kTLS. kind/enhancement This would improve or streamline existing functionality. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact. sig/policy Impacts whether traffic is allowed or denied based on user-defined policies.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants