Skip to content

operator: bind necessary policy flags #35599

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

Merged
merged 1 commit into from
Oct 31, 2024

Conversation

squeed
Copy link
Contributor

@squeed squeed commented Oct 28, 2024

There was a bug where the operator was reporting policies containing ICMP fields as invalid, even though they were accepted by the daemon. It turns out that because option.Config.EnableICMPRules is populated by vp.GetBool(), it's not enough just to initialize it to the default value. We must bind that command-line argument as well.

Fixes: #35214

Fixes a bug where the operator incorrectly flagged CiliumNetworkPolicies containing ICMP rules as invalid.

@squeed squeed added kind/bug This is a bug in the Cilium logic. sig/policy Impacts whether traffic is allowed or denied based on user-defined policies. needs-backport/1.16 This PR / issue needs backporting to the v1.16 branch labels Oct 28, 2024
@squeed squeed requested review from a team as code owners October 28, 2024 21:05
@squeed squeed requested a review from nathanjsweet October 28, 2024 21:05
@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 Oct 28, 2024
@squeed squeed requested a review from pippolo84 October 28, 2024 21:05
@maintainer-s-little-helper maintainer-s-little-helper bot added dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Oct 28, 2024
@squeed squeed added the release-note/bug This PR fixes an issue in a previous release of Cilium. label Oct 28, 2024
@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 Oct 28, 2024
@squeed
Copy link
Contributor Author

squeed commented Oct 28, 2024

/test

There was a bug where the operator was reporting policies containing
ICMP fields as invalid, even though they were accepted by the daemon. It
turns out that because option.Config.EnableICMPRules is populated by
`vp.GetBool()`, it's not enough just to initialize it to the default
value. We must bind that command-line argument as well.

Fixes: cilium#35214

Signed-off-by: Casey Callendrello <cdc@isovalent.com>
@squeed squeed force-pushed the fix-icmp-policy-warnings branch from f654f90 to 29ab2e0 Compare October 29, 2024 10:09
@squeed
Copy link
Contributor Author

squeed commented Oct 29, 2024

/test

@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 Oct 31, 2024
@julianwiedmann julianwiedmann added this pull request to the merge queue Oct 31, 2024
Merged via the queue into cilium:main with commit 08c311f Oct 31, 2024
64 checks passed
@joamaki joamaki mentioned this pull request Nov 5, 2024
23 tasks
@joamaki joamaki added backport-pending/1.16 The backport for Cilium 1.16.x for this PR is in progress. and removed needs-backport/1.16 This PR / issue needs backporting to the v1.16 branch labels Nov 5, 2024
@github-actions github-actions bot added backport-done/1.16 The backport for Cilium 1.16.x for this PR is done. and removed backport-pending/1.16 The backport for Cilium 1.16.x for this PR is in progress. labels Nov 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-done/1.16 The backport for Cilium 1.16.x for this PR is done. 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/bug This PR fixes an issue in a previous release of Cilium. sig/policy Impacts whether traffic is allowed or denied based on user-defined policies.
Projects
None yet
5 participants