NPEP-248: Add match all to ingress/egress peers#339
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. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
cncf-cla: yes
k8s-ci-robot
added
the
size/L
fasaxc
changed the title
| 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.
Have you thought about how the API could evolve to cover the "deny all UDP traffic" case?
There was a problem hiding this comment.
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.
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.
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.
danwinship
left a comment
danwinship
left a comment
There was a problem hiding this comment.
This looks like it matches our consensus at the time it was written, but hasn't been updated after the discussions at KubeCon, #248 (comment).
Especially, the NPEP needs to do something about kubelet probes (either by deciding that "deny all" doesn't deny them (🤮) or by figuring out some way administrators can explicitly re-allow them).
| **Note:** Since we're explicitly using the empty struct here, we won't be able | ||
| 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. |
There was a problem hiding this comment.
I think we've decided that we don't have to fall into that trap. We know how to create an Informer that will warn you if it sees any fields that it doesn't know how to deserialize.
|
The Kubernetes project currently lacks enough contributors to adequately respond to all PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle stale |
k8s-ci-robot
added
the
lifecycle/stale
5 participants
Attempt to tackle #248 by adding an explicit match all operator to peer objects.