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

clean-up: remove check for permissive CCNPs #27690

Merged
merged 1 commit into from Aug 25, 2023

Conversation

shawnh2
Copy link
Contributor

@shawnh2 shawnh2 commented Aug 25, 2023

Please ensure your pull request adheres to the following guidelines:

  • For first time contributors, read Submitting a pull request
  • All code is covered by unit and/or runtime tests where feasible.
  • All commits contain a well written commit description including a title,
    description and a Fixes: #XXX line if the commit addresses a particular
    GitHub issue.
  • If your commit description contains a Fixes: <commit-id> tag, then
    please add the commit author[s] as reviewer[s] to this issue.
  • All commits are signed off. See the section Developer’s Certificate of Origin
  • Provide a title or release-note blurb suitable for the release notes.
  • Are you a user of Cilium? Please add yourself to the Users doc
  • Thanks for contributing!

This check was designed to inform users of CCNP that the behaviour
changed around the timeframe of v1.9. However, the actual check is
complaining about policy that is valid. At this point there is no need
to warn users during preflight checks that they are using empty endpoint
selectors in CCNPs, because this can validly be used to select all Pods
within the cluster. Remove the check.

Fixes: #27689

@shawnh2 shawnh2 requested review from a team as code owners August 25, 2023 04:46
@shawnh2 shawnh2 requested a review from aanm August 25, 2023 04:46
@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 Aug 25, 2023
@github-actions github-actions bot added the kind/community-contribution This was a contribution made by a community member. label Aug 25, 2023
@joestringer joestringer added the release-note/misc This PR makes changes that have no direct user impact. label Aug 25, 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 Aug 25, 2023
@aanm
Copy link
Member

aanm commented Aug 25, 2023

/test

Copy link
Member

@joestringer joestringer left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

This check was designed to inform users of CCNP that the behaviour
changed around the timeframe of v1.9. However, the actual check is
complaining about policy that is valid. At this point there is no need
to warn users during preflight checks that they are using empty endpoint
selectors in CCNPs, because this can validly be used to select all Pods
within the cluster. Remove the check.

Signed-off-by: sh2 <shawnhxh@outlook.com>
Signed-off-by: Joe Stringer <joe@cilium.io>
@joestringer
Copy link
Member

I rebased & squashed the commits together and provided more context in the commit message. Running CI now. If that passes, this PR should be good to merge.

@joestringer joestringer added the needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch label Aug 25, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from main in 1.14.2 Aug 25, 2023
@joestringer
Copy link
Member

I think that this could benefit users upgrading to v1.14, since it's currently causing preflight checks to fail for valid policy statements. I expect low risk for backporting. Marked for backport to v1.14.

@joestringer
Copy link
Member

/test

@joestringer joestringer merged commit e46ade2 into cilium:main Aug 25, 2023
60 checks passed
@shawnh2 shawnh2 deleted the remove-permissive-ccnps-check branch August 26, 2023 02:53
@pippolo84 pippolo84 mentioned this pull request Aug 28, 2023
9 tasks
@pippolo84 pippolo84 added backport-pending/1.14 The backport for Cilium 1.14.x for this PR is in progress. and removed needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch labels Aug 28, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from main to Backport pending to v1.14 in 1.14.2 Aug 28, 2023
@michi-covalent michi-covalent added backport-done/1.14 The backport for Cilium 1.14.x for this PR is done. and removed backport-pending/1.14 The backport for Cilium 1.14.x for this PR is in progress. labels Sep 9, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.14 to Backport done to v1.14 in 1.14.2 Sep 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-done/1.14 The backport for Cilium 1.14.x for this PR is done. kind/community-contribution This was a contribution made by a community member. release-note/misc This PR makes changes that have no direct user impact.
Projects
No open projects
1.14.2
Backport done to v1.14
Development

Successfully merging this pull request may close these issues.

Remove check for permissive CCNPs
5 participants