✨OPRUN-3873: Add e2e tests for NetworkPolicies #2013
Conversation
✅ Deploy Preview for olmv1 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@joelanford incorporated your suggestions..also fyi had to change the struct definitions a bit because I realized the policies are defined as a single ingress/egress policy with multiple ports, instead of multiple ingress/egress policies, for each NetworkPolicy object. PTAL, thanks! |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2013 +/- ##
==========================================
+ Coverage 73.78% 73.80% +0.01%
==========================================
Files 80 80
Lines 7146 7146
==========================================
+ Hits 5273 5274 +1
+ Misses 1550 1549 -1
Partials 323 323
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
5496cd9 to
ec22c0a
Compare
7506ef0 to
b61ee05
Compare
|
@joelanford @perdasilva regarding the discussion about possibly switching this to a unit test, I see the Quickstart target in the Makefile, and ack the discussion about using steps from that target (separated out) to generate the manifests and before starting the unit tests, and then consuming the generated manifests from the unit test. I don't see a natural home for the unit test though. Maybe that's actually a good signal that this should be a e2e test instead. Thoughts? |
|
Hi @anik120 I see that you asked for their input here as well; #2013 (comment), so we can wait for their thoughts However, IHMO:
Yes — it would indeed be better to write the tests by checking the YAML files directly, as we discussed yesterday. That said, the current implementation is already acceptable. Also, considering our earlier conversation, we’ll likely have the YAML merged in the repo in the future, so an update would be necessary at that point anyway if we want to move toward the ideal approach. So, IHMO: it’s really up to you. If you’re able to make the change now, that would be great 🥇 — but if not, I think this PR still accomplishes its goal effectively. From my side, LGTM. |
|
@camilamacedo86 PR #2025 gives me the manifests locally that I can consume and write the test with. However the question of "which package is a natural home for that test, and if we cannot think of a natural home package for that test is that a strong signal that this is better suited as an e2e test", still remains. Maybe we wait a little to see if anyone has a better idea, if not I'm happy with this PR and we can merge it in 👍🏽 |
|
Make sure you rebase... |
|
@anik120 I've pushed up a rebase to try and get this down if its green =D |
|
/lgtm |
|
@anik120 I'll wait to approve until you give the final go ahead. I don't know if you want to make some changes still... |
Description
This PR introduces a new e2e test to continuously validate the NetworkPolicy objects for the catalogd and operator-controller components.
The new test,
TestNetworkPolicyJustifications, achieves the following:Documents NetworkPolicies deployed: A registry of all allowed network policies (
allowedNetworkPolicies) is defined using the RFC: OLMv1 Static Network Policies. Every rule within these policies is explicitly registered in the registry with a clear, written justification of a minimum length. This ensures that every rule's purpose is documented and understood.Prevents Regressions: The test validates that the NetworkPolicy objects deployed in the cluster exactly match the definitions in a registry (
allowedNetworkPolicies). This prevents undocumented or accidental changes to our network rules.Improves Security Posture: Enforces thoughtful consideration for any change to existing network rules, improving overall security posture. Also ensures that other, unrelated network policies won't inadvertently break existing functionalities.
Reviewer Checklist