-
Notifications
You must be signed in to change notification settings - Fork 34
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
Add conformance for Gress
rules
#112
Add conformance for Gress
rules
#112
Conversation
✅ Deploy Preview for kubernetes-sigs-network-policy-api ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
conformance/tests/baseline-admin-network-policy-core-gress-rules.go
Outdated
Show resolved
Hide resolved
Manifests: []string{"tests/admin-network-policy-core-gress-rules_base.yaml"}, | ||
Test: func(t *testing.T, s *suite.ConformanceTestSuite) { | ||
|
||
t.Run("Should support an 'allow-gress' policy across different protocols", func(t *testing.T) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is really hard to review and I'm sure I'm missing things.... Maybe putting the explicit rule yaml each section is testing in a comment before it might help? (we can do this later)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like I need to see the yaml next to the test case
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yea i am going to move the yamls to the base folder like we discussed in previous PR and I think once this PR and next one lands I will spend some time writing docs that outline the design of these tests.
Is this ready to go @tssurya? |
This commit adds conformance tests for mix of ingress and egress rules in same CRD, which mixes up protocols and ports in same rules. They should behave in an idempotent manner with regards to each other. Signed-off-by: Surya Seetharaman <suryaseetharaman.9@gmail.com>
102d293
to
9fe2b7b
Compare
@astoycos : yes this is ready to be merged! |
This commit adds conformance tests for mix of ingress and egress rules in same CRD, which mixes up protocols and ports in same rules. They should behave in an idempotent manner with regards to each other. Signed-off-by: Surya Seetharaman <suryaseetharaman.9@gmail.com>
9fe2b7b
to
3d592ba
Compare
/lgtm Thanks! |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: astoycos, tssurya The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
So far we have been adding modular testing for "ingress" and "egress" rules separately over TCP/UDP/SCTP test suites. What happens when we have a mix of them all in the same CRD? Let's ensure we have coverage for that for both BANP & ANP.