accept wildcard in NetworkPolicy RBAC apiGroups and resources #1742
Conversation
Align the title with standard naming convention. This commit changes the title to NetworkPolicy RBAC present in OLM bundle Signed-off-by: Yashvardhan Nanavati <yashn@bu.edu>
Operators granting broad RBAC via wildcard "*" in apiGroups or resources already cover networking.k8s.io/networkpolicies. Update the policy to recognize this and rename the rule title to describe the desired state. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR extends the OLM NetworkPolicy RBAC policy rule to accept wildcard matches in API groups and resources, refactors the matching logic using new helper predicates, adds comprehensive test coverage, and synchronizes documentation to reflect the changes. ChangesNetworkPolicy RBAC Wildcard Policy Enhancement
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
🤖 Review · Started 4:12 PM UTC |
PR Summary by QodoAccept wildcard RBAC for NetworkPolicy rule and align rule title/docs WalkthroughsDescription• Treat wildcard "*" in RBAC apiGroups/resources as satisfying NetworkPolicy permissions. • Rename the rule title to describe the desired state (“present”) and update docs/index links. • Add regression tests covering wildcard and negative matching scenarios. Diagramgraph TD
CSV["CSV RBAC rules"] --> Policy["olm.rego: NetworkPolicy RBAC"] --> Result["deny result"]
Policy --> Helpers["Wildcard match helpers"]
Tests["olm_test.rego"] --> Policy
Index["Release policy index"] --> Docs["Antora rule docs"] --> Policy
High-Level AssessmentThe following are alternative approaches to this PR: 1. Generalize list matching into a single helper
Recommendation: Current approach is solid: small, explicit helpers make the wildcard intent clear and are easy to test. Consider a generalized matcher only if additional RBAC rules need the same wildcard behavior. File ChangesBug fix (1)
Tests (1)
Documentation (3)
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
policy/release/olm/olm_test.rego (1)
1156-1156: 💤 Low valueInconsistent argument order in assertion.
This test uses
assert_equal_results(expected, olm.deny), while all other tests in this file useassert_equal_results(olm.deny, expected). While functionally equivalent, maintaining consistency improves readability.🔄 Suggested fix for consistency
- assertions.assert_equal_results(expected, olm.deny) with input.image.files as {"manifests/csv.yaml": manifest_wrong_group} + assertions.assert_equal_results(olm.deny, expected) with input.image.files as {"manifests/csv.yaml": manifest_wrong_group}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@policy/release/olm/olm_test.rego` at line 1156, The assertion call uses inconsistent argument order: change assertions.assert_equal_results(expected, olm.deny) to assertions.assert_equal_results(olm.deny, expected) so it matches the other tests; update the call that uses with input.image.files as {"manifests/csv.yaml": manifest_wrong_group} and ensure the symbols assertions.assert_equal_results, olm.deny and expected are used in that order for consistency.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@policy/release/olm/olm_test.rego`:
- Line 1156: The assertion call uses inconsistent argument order: change
assertions.assert_equal_results(expected, olm.deny) to
assertions.assert_equal_results(olm.deny, expected) so it matches the other
tests; update the call that uses with input.image.files as
{"manifests/csv.yaml": manifest_wrong_group} and ensure the symbols
assertions.assert_equal_results, olm.deny and expected are used in that order
for consistency.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: 05f647c7-987f-4165-9ae9-080a9c744932
📒 Files selected for processing (5)
antora/docs/modules/ROOT/pages/packages/release_olm.adocantora/docs/modules/ROOT/pages/release_policy.adocantora/docs/modules/ROOT/partials/release_policy_nav.adocpolicy/release/olm/olm.regopolicy/release/olm/olm_test.rego
Codecov Report✅ All modified and coverable lines are covered by tests.
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
/lgtm |
ReviewFindingsLow
Info
|
|
🤖 Finished Review · ✅ Success · Started 4:12 PM UTC · Completed 4:22 PM UTC |
Refers to CLOUDDST-CLOUDDST-32704
Operators granting broad RBAC via wildcard "*" in apiGroups or resources
already cover networking.k8s.io/networkpolicies. Update the policy to
recognize this and rename the rule title to describe the desired state.