-
Notifications
You must be signed in to change notification settings - Fork 39
NPEP-248: Add match all to ingress/egress peers #339
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
base: main
Are you sure you want to change the base?
Conversation
Attempt to tackly kubernetes-sigs#248 by adding an explicit match all operator to peer objects.
✅ Deploy Preview for kubernetes-sigs-network-policy-api 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: fasaxc 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 |
| to add new fields inside the struct later without a breaking change. "Old" | ||
| implementations won't see the new fields, and they'd act like they weren't present. | ||
|
|
||
| I think this is OK in this unique case because "all" is naturally a unique "top" |
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.
Have you thought about how the API could evolve to cover the "deny all UDP traffic" 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.
deny all UDP traffic
Can already be expressed by saying "deny UDP ports 1-65535". In general, you can already pick out the parts of the Venn diagram, but what was missing was a way to say "just match it all, don't restrict"
| - all: {} | ||
| ``` | ||
|
|
||
| Then, via to-be-written controller, we auto-generate a higher precedence policy per unique tenant |
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.
Are the implementations/end-users expected to write such a controller or do we plan to include something in this repo?
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.
We've discussed writing such a controller as a way to prove out tenancy. Then, either that becomes the answer or the templating mechanism gets absorbed into the vendor implementations once we're happy with it. This section was mainly to say "this feature fits in with what we discussed about tenancy", not to commit us to that tenancy path.
Attempt to tackle #248 by adding an explicit match all operator to peer objects.