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

auth: temporarily disable node-based auth gc #26073

Merged

Conversation

mhofstetter
Copy link
Member

@mhofstetter mhofstetter commented Jun 9, 2023

Currently, auth map garbage collection based on deleted node events provokes issues in combination with enabled encryption.

The actual problem is that there are combinations where garbage collection unintentionally allocates node ids when trying to resolve the node IDs for a given node IP. This results in multiple node ids for the same ip, which breaks encryption.

Therefore, this commit temporarily disables auth map garbage collection based on deleted node events.

Fixes: #25964

@mhofstetter mhofstetter added the release-note/misc This PR makes changes that have no direct user impact. label Jun 9, 2023
@mhofstetter
Copy link
Member Author

/test

@mhofstetter
Copy link
Member Author

/test

@mhofstetter mhofstetter force-pushed the pr/mhofstetter/test-disable-auth branch from 26f6017 to 33ee6c3 Compare June 12, 2023 15:33
@mhofstetter
Copy link
Member Author

/test

1 similar comment
@mhofstetter
Copy link
Member Author

/test

@mhofstetter mhofstetter force-pushed the pr/mhofstetter/test-disable-auth branch from 33ee6c3 to cfdab63 Compare June 12, 2023 21:42
@mhofstetter mhofstetter changed the title auth: disable GC auth: temporary disable node-based auth gc Jun 12, 2023
Currently, auth map garbage collection based on deleted node events
provokes issues in combination with enabled encryption.

The actual problem is that there are combinations where garbage
collection unintentionally allocates node ids when trying to resolve the
node IDs for a given node IP. This results in multiple node ids for the
same ip, which breaks encryption.

Therefore, this commit temporarily disables auth map garbage collection based on
deleted node events.

Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
@mhofstetter mhofstetter force-pushed the pr/mhofstetter/test-disable-auth branch from cfdab63 to f0b4282 Compare June 12, 2023 21:43
@mhofstetter mhofstetter added kind/bug This is a bug in the Cilium logic. area/servicemesh GH issues or PRs regarding servicemesh labels Jun 12, 2023
@mhofstetter
Copy link
Member Author

/test

@mhofstetter mhofstetter marked this pull request as ready for review June 12, 2023 22:20
@mhofstetter mhofstetter requested a review from a team as a code owner June 12, 2023 22:20
@mhofstetter mhofstetter changed the title auth: temporary disable node-based auth gc auth: temporarily disable node-based auth gc Jun 12, 2023
@mhofstetter
Copy link
Member Author

/ci-multicluster

@mhofstetter
Copy link
Member Author

/ci-external-workloads

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

@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 Jun 13, 2023
@jrajahalme jrajahalme merged commit c8d5e8b into cilium:main Jun 13, 2023
64 checks passed
@mhofstetter mhofstetter deleted the pr/mhofstetter/test-disable-auth branch June 13, 2023 07:36
mhofstetter added a commit to mhofstetter/cilium that referenced this pull request Jun 15, 2023
This commit re-enables the node-based auth map garbage collection which
have been temporarily disabled with cilium#26073.

Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
joestringer pushed a commit that referenced this pull request Jun 15, 2023
This commit re-enables the node-based auth map garbage collection which
have been temporarily disabled with #26073.

Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
romanspb80 pushed a commit to romanspb80/cilium that referenced this pull request Jun 22, 2023
This commit re-enables the node-based auth map garbage collection which
have been temporarily disabled with cilium#26073.

Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
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. 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.
Projects
None yet
3 participants