WIP Add a matchName selection option to the NetworkPolicy namespace selector API #2113

jayunit100 · 2020-10-23T13:56:20Z

keps/sig-network/20201014-NamespaceAsName.md

andrewsykim · 2020-10-23T14:04:16Z

keps/sig-network/20201014-NamespaceAsName.md

+  ingress:
+  - from:
+    - namespaceSelector:
+         matchName:


Adding a matchName field to namespaceSelector is potentially a breaking change because we would have to create a new internal struct for namespaceSelector to something like:

type NamespaceSelector struct { metav1.LabelSelector matchNames []string }

While maybe we can get away with not breaking json/yaml serialization, I think we still break clients creating NetworkPolicy types off of the new version? If this is a breaking change, at worse we should add a new field outside namespaceSelector. But breaking changes aside, I like matchNames under namespaceSelector much better.

An alternative to changing the syntax would be to add a "virtual label" with some name that is not a valid label name (eg, virtual/name which is not valid because the prefix part isn't a DNS name). Then you could just do:

- namespaceSelector: matchLabels: virtual/name: my-frontend

but also you could do

- namespaceSelector: matchExpressions: - key: virtual/name operator: NotIn values: - kube-system

ive never understood the virtual labels thing... are there any examples of how people hack stuff up to use these ?

Let's not invent ad hoc syntax. Please :)

Adding a field to metav1.LabelSelector is dangerous at best. I find it confusing - it's not a selector in the usual kubernetes sense. Also breaks clients which should not be taken lightly.

Virtual labels are interesting but if we want to go that route, we'd need to pause and define that whole mechanism. Does it apply to all selectors and all resources? Is it just for "name" or does it have other uses? Does it show up when I GET an object? This is a KEP of its own.

The simplest option here is to add namespaceNames []string to NetworkPolicyPeer.

You still have to define what consumers should do if they don't support this field. They would see a NetworkPolicyPeer with no fields set which fails validation. So to make this work we would need to specifically loosen that validation. Define that to mean "allow none". That, of course has to be alpha-gated. We'd really need to work with the myriad implementations to verify that they either do the right thing already or change them to treat that safely.

So (for example) the very best case: 1.20 could allow empty peers and add support for namespaceNames[] if and only if the gate was enabled. 1.21 could flip the gate to default-on for both (beta or GA, TBD). If it takes a while to get implementations doing the right thing, we may have to leave it in alpha for longer.

You still have to define what consumers should do if they don't support this field.

If it takes a while to get implementations doing the right thing, we may have to leave it in alpha for longer.

The feature gate should be tracking "are we satisfied with the API?". That's separate from "does everyone implement the API?" though clearly related.

#2137 is an attempt to deal with the problem of NetworkPolicy feature versioning more generically (though of course, if it merges in the same release as this feature does then it doesn't really solve the problem for this feature since plugins that didn't implement namespace names probably wouldn't implementing versioning either)

keps/sig-network/20201014-NamespaceAsName.md

rikatz · 2020-10-27T12:18:21Z

keps/sig-network/20201014-NamespaceAsName.md

+  - from:
+    - namespaceSelector:
+         matchName:
+         -  *


As you said supporting matching of 'wildcard namespaces' is a non goal, don't you think this syntax (with *) will lead to some confusion? Just thinking here.

yeah, it seems like it would be clearer to do matchNameExcept: or something

yeah this * is kinda sneaky, restricted regex, will mislead people.
matchNameExcept seems equally weird though to me... ill bring this up on the call today

keps/sig-network/20201014-NamespaceAsName.md

danwinship · 2020-10-28T18:49:21Z

keps/sig-network/20201014-NamespaceAsName.md

+### Goals
+
+- Add a `matchName` option (which is additive to the current `matchLabels` selector).
+- Add an additional semantic mechanism to "allow all" namespaces while excluding a finite, specific, list of namespaces (canonically, this might be `kube-system`, as this is an obvious security boundary which most clusters would prefer).


The second goal here doesn't relate to anything in the Summary and Motivation discussion...

Also it's not clear what "this might be kube-system" means. Are you saying people might want to allow connections to/from all namespaces except kube-system or that kube-system might want to allow connections to/from all but a finite set of namespaces? (And either way, if you are not proposing a change to the default behavior then I don't think the word "canonically" belongs there.)

updated, pushing shortly...

danwinship · 2020-10-28T18:57:37Z

keps/sig-network/20201014-NamespaceAsName.md

+
+#### Story 1
+
+As an administrator I want to block all users from accessing the kube-system namespace as a fundamental default policy.


If you want to block "all users" you can do that without using a namespaceSelector at all. Be clearer about what you want to do and why you can't do it with the existing system.

updated, pushing shortly

keps/sig-network/20201014-NamespaceAsName.md

danwinship · 2020-10-28T19:03:27Z

keps/sig-network/20201014-NamespaceAsName.md

+### Risks and Mitigations
+
+- CNIs may not choose, initially to support this construct, and dilligent communication with CNI providers will be needed to make sure its widely adopted.
+- Fractionating the Kubernetes API idiom by introducing a separate way to select objects.  We accept this cost because security is a fundamentally important paradigm that justifies breaking other paradigms, at times. 


There are other APIs that select objects by name in some cases. I don't think this is a "risk". It's a choice that needs to be defended, but it's not a risk.

danwinship · 2020-10-28T19:12:55Z

keps/sig-network/20201014-NamespaceAsName.md

+         matchName:
+         -  *
+         exclude:
+         - kube-system


This policy says "allow connections from all namespaces except kube-system" which is the opposite of the thing you said you wanted to support and is therefore confusing.

Yeah, i guess as an egress rule this makes more sense, but better yet i think removing this for now, discussing it and re-adding it later is better given ricardos comment above ... the user story for the exclusion rules is a little messy ATM. fixing...

danwinship · 2020-10-28T19:13:53Z

keps/sig-network/20201014-NamespaceAsName.md

+### Graduation Criteria
+
+#### Alpha 
+- Add a feature gated new field to NetworkPolicy


In the past we have not used feature gates for new NetworkPolicy features.

sure, im happy to modify this or remove it, ill bring this up at the mtng today

@andrewsykim can you please provide the api guide discussed in the meeting? Thanks!

I believe feature gates are now required for new fields to stable APIs to ensure safe rollback https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api_changes.md#alpha-field-in-existing-api-version

danwinship · 2020-10-28T19:16:14Z

keps/sig-network/20201014-NamespaceAsName.md

+
+#### GA Graduation
+
+- At least two CNIs support the The name selector field


I think "at least one" and "at least two" are too small for an NP feature. We don't currently have good support for being able to express that the CNI plugin doesn't support a NetworkPolicy feature that exists in the API. Therefore, it is bad when the feature is there but plugins don't support it. So we should aim for reasonably broad support before we start encouraging users to think it's safe to use.

broadened to include 4 - am assuming that might be something like ovn, calico, cillium, antrea.

still says 2 here?

fixed pushing shortly

danwinship · 2020-10-28T19:18:03Z

keps/sig-network/20201014-NamespaceAsName.md

+- At least two CNIs support the The name selector field
+- The name selector has been enabled by default for at least 1 minor release
+
+#### Removing a Deprecated Flag


(this section is irrelevant to this KEP and can be removed)

danwinship · 2020-10-28T19:21:28Z

keps/sig-network/20201014-NamespaceAsName.md

+
+### Notes/Constraints/Caveats (Optional)
+
+### Risks and Mitigations


We need to consider how policies using the new feature will be interpreted by plugins that don't yet support the new feature. eg:

causes the plugin to crash due to unexpectedly invalid data

causes any NetworkPolicyPeer using the new feature to look like it says "allow all"

causes any NetworkPolicyPeer using the new feature to look like it says "allow none"

Some of these possibilities are better than others...

fixed pushing shortly

This is probably my prime concern with this proposal

updated to include these below , pushing shortly

kikisdeliveryservice

So you need to use the issue number to correctly format this as follows:
main dir:
keps/sig-network/2112-NamespaceAsName/

(though I'm not sure that's a descriptive enough title tbh)

20201014-NamespaceAsName.md ->../2112-NamespaceAsName/README.md

add kep.yaml

See: https://github.com/kubernetes/enhancements/tree/master/keps/NNNN-kep-template

Update README.md to have title KEP-2112:

k8s-ci-robot · 2020-10-30T22:02:00Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: jayunit100
To complete the pull request process, please assign thockin after the PR has been reviewed.
You can assign the PR to them by writing /assign @thockin in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

keps/sig-network/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Co-authored-by: Dan Winship <[email protected]>

thockin · 2020-11-02T22:05:44Z

keps/sig-network/20201014-NamespaceAsName.md

+### Risks and Mitigations
+
+- CNIs may not choose, initially, to support this construct, and diligent communication with CNI providers will be needed to make sure it's widely adopted.
+- CNIs may not choose, initially to support this construct, and dilligent communication with CNI providers will be needed to make sure its widely adopted.


why is this line duplicated?

fixed, pushing

thockin · 2020-11-02T22:24:46Z

keps/sig-network/20201014-NamespaceAsName.md

+  ingress:
+  - from:
+    - namespaceSelector:
+         matchName:


Let's not invent ad hoc syntax. Please :)

Adding a field to metav1.LabelSelector is dangerous at best. I find it confusing - it's not a selector in the usual kubernetes sense. Also breaks clients which should not be taken lightly.

Virtual labels are interesting but if we want to go that route, we'd need to pause and define that whole mechanism. Does it apply to all selectors and all resources? Is it just for "name" or does it have other uses? Does it show up when I GET an object? This is a KEP of its own.

The simplest option here is to add namespaceNames []string to NetworkPolicyPeer.

You still have to define what consumers should do if they don't support this field. They would see a NetworkPolicyPeer with no fields set which fails validation. So to make this work we would need to specifically loosen that validation. Define that to mean "allow none". That, of course has to be alpha-gated. We'd really need to work with the myriad implementations to verify that they either do the right thing already or change them to treat that safely.

So (for example) the very best case: 1.20 could allow empty peers and add support for namespaceNames[] if and only if the gate was enabled. 1.21 could flip the gate to default-on for both (beta or GA, TBD). If it takes a while to get implementations doing the right thing, we may have to leave it in alpha for longer.

thockin · 2020-11-02T22:25:40Z

keps/sig-network/20201014-NamespaceAsName.md

+
+### Graduation Criteria
+
+Note that we may not have explicity feature gates for this KEP, because NetworkPolicies typically haven't been implemented with gates.  We keep this section for the time being in any case as it organizes our thinking around the general progression of the feature over time.


All new field introductions need gates for at least one release.

thockin · 2020-11-02T22:26:23Z

keps/sig-network/20201014-NamespaceAsName.md

+
+#### Beta
+- The name selector has been supported for at least 1 minor release
+- Four commonly used CNI providers implement the new field, with generally positive feedback on its usage.


Why is this written in terms of CNI providers instead of NetPol providers?

thockin · 2020-11-02T22:27:52Z

keps/sig-network/20201014-NamespaceAsName.md

+
+#### GA Graduation
+
+- At least two CNIs support the The name selector field


still says 2 here?

Fixes and reworked proposal to modify networkpolicyPeer matchName yaml format change add feature gates note add feature gates note

keps/sig-network/20201014-NamespaceAsName.md

rikatz · 2020-11-04T20:44:55Z

keps/sig-network/20201014-NamespaceAsName.md

+      app: mysql
+  ingress:
+  - from:
+      matchName:


Probably here is namespaceNames

but reading loud, namespaceNames as a field doesn't sounds strange? wouldn't be better to be only namespaces here?

updated to namespaceSelectorAllow , because now that were putting this in NetworkPolicyPeer, I figured having uniform naming to align it with namespaceSelector (i.e. it extends the namespaceSelector rather then being a logical union with it, as we do w/ podSelectors and so on)

rikatz · 2020-11-04T21:26:54Z

So you need to use the issue number to correctly format this as follows:
main dir:
keps/sig-network/2112-NamespaceAsName/

(though I'm not sure that's a descriptive enough title tbh)

20201014-NamespaceAsName.md ->../2112-NamespaceAsName/README.md

add kep.yaml

See: https://github.com/kubernetes/enhancements/tree/master/keps/NNNN-kep-template

Update README.md to have title KEP-2112:

@kikisdeliveryservice updated Jay's PR here

Signed-off-by: Ricardo Pchevuzinske Katz <[email protected]>

andrewsykim · 2020-11-04T21:52:47Z

keps/sig-network/2112-netpol-namespace-name-selector/README.md

+## Design Details
+
+- Add a new selector to the network policy peer data structure which can 
+switch between allowing a "namespaceNames" or "matchLabels" implementation, 


This seems to imply that a user has to set either namespaceNames or namespaceSelector but above in the goals it mentions:

- Add a `namespaceNames` option (which is additive to the current `matchLabels` selector).

Which implies we are taking the union of namespaceNames and namespaceSelector. Might be good to clarify this a bit.

updating this now, I actually thought more about this today

andrewsykim · 2020-11-04T21:54:20Z

keps/sig-network/2112-netpol-namespace-name-selector/README.md

+```
+type NetworkPolicyPeer struct {
+  // new field
+  namespaceNames []string


Can we add the API docs here?

I would like to see a draft of what the API docs would say.

jayunit100 · 2020-11-05T11:54:30Z

pushed some updates, mainly to the impl details.

so, after thinking more about it... now that we're modding NetworkPolicyPeer, it seems that naming the field so that it aligns with the namespaceSelector more, made sense, so I updated the design details to reflect that iteration.

I still want to comb through this and make sure its logically consistent all the way through again, after getting away from it for a day or so.

Thanks for all the feedback so far, I think its starting to take shape.

as a side note: I'd like to explore virtual labels again at some point (maybe on Monday's meeting) , before we go too far with this, as that might still be an alternative which is more robust long term, but don't understand the idea well enough yet. Just want to make sure we don't completely brush it under the rug ..

danwinship · 2020-11-10T17:45:48Z

keps/sig-network/2112-netpol-namespace-name-selector/README.md

@@ -0,0 +1,371 @@
+<!-- toc -->


you lost the title

danwinship · 2020-11-10T17:52:39Z

keps/sig-network/2112-netpol-namespace-name-selector/README.md

+
+## Proposal
+
+In NetworkPolicy specification, inside `NetworkPolicyPeer` specify a new `namespaceSelectorAllow` field.  


It's always sort of confusing what goes where in a KEP, but this seems like it interrupts the flow at this point (coming between the Motivation and the User Stories) and so would probably go better under Design Details

danwinship · 2020-11-10T18:00:42Z

keps/sig-network/2112-netpol-namespace-name-selector/README.md

+#### Story 1
+
+As an administrator i want to allow access from monitoring pods in kube-system 
+namespace, into any pod in my cluster, as a fundamental policy. 


NetworkPolicy is not intended to solve administrator problems, it's intended to solve user/developer problems. Thus, administrator-focused user stories do not provide justification for NetworkPolicy features.

And anyway, the administrator can configure the cluster in such a way that they are the only one who can manipulate Namespace labels, and thus they can make it so they can securely use namespaceSelector. In fact, they should use namespaceSelector, because at some point they're going to realize that they should move the monitoring pods into their own namespace, and if they used labels to identify the monitoring namespace then all they have to do is properly label the new namespace and they're done, while if they had added explicit "allow from kube-system" policies everywhere, then they have to replace all of those policies with new "allow from monitoring" ones.

removing section

danwinship · 2020-11-10T18:03:06Z

keps/sig-network/2112-netpol-namespace-name-selector/README.md

+#### Story 2
+
+As an user I want to "just add" a namespace to my allow list without having to 
+manage the labels which might get added/removed over time from said namespace.


This isn't a user story, it's just a restatement of the feature proposal.

You already spent a bunch of time justifying the feature in the Motivation section. You don't need user stories as well if you made your case there.

ok, got rid of this section, its also redundant to the motivation which is pretty data driven

danwinship · 2020-11-10T18:10:53Z

keps/sig-network/2112-netpol-namespace-name-selector/README.md

+
+- NetworkPolicy providers may **opt-out**, initially to support this construct, and dilligent communication with CNI providers will be needed to make sure its widely adopted, thus, this feature needs to be backwards compatible with the existing v1 api.
+- We thus need to make sure that hidden defaults don't break the meaning of existing policys, for example:
+  - if `namespaceSelectorAllow` field is retrieved by an api client which doesn't yet support it, the client doesnt crash (and the plugin doesnt crash, either).


I talked about this only vaguely before because it wasn't clear at that point what the API was going to look like. At this point you can get more concrete. Specifically, when an "old" plugin sees a policy using namespaceSelectorAllow, it will think that it has an empty NetworkPolicyPeer, which is something it may not be able to deal with, since that wouldn't have passed validation before...

nested the bullets to make this more clear and readable, added that old distinction

danwinship · 2020-11-10T18:13:46Z

keps/sig-network/2112-netpol-namespace-name-selector/README.md

+```
+type NetworkPolicyPeer struct {
+  // new field
+  namespaceSelectorAllow []string


having namespaceSelector and namespaceSelectorAllow implies that the two fields differ in something having to do with "allowing", which is not accurate at all. The new field should have some name that makes it clear that it's about namespace names. (It does not need to specify "allow" because everything in NetworkPolicyPeer specifies something to allow.)

yeah, namespaceSelectorAllow was a horrible choice :) , changing to namespaceNames

danwinship · 2020-11-10T18:14:35Z

keps/sig-network/2112-netpol-namespace-name-selector/README.md

+
+- Add a new selector to the network policy peer data structure which can switch between allowing a `namespaceSelectorAllow`, supporting a policy that is expressed like this:
+
+- A list of conventional namespaces


what are "conventional" namespaces?

clarifying/ replacing the sentence below as well

danwinship · 2020-11-10T18:17:08Z

keps/sig-network/2112-netpol-namespace-name-selector/README.md

+      -  my-frontend-2
+```
+
+- As a more sophisticated example: The following would allow all traffic from `podName:xyz` living in `my-frontend` AND `my-frontend-2`, but NOT trafic from `my-frontend-3`.  This is because the presence of EITHER a namespaceSelectorAllow rule or a namespaceSelector, makes the podSelector subject to an **AND** filter operation.


I don't think there's any good argument for allowing both namespaceSelector and namespaceSelectorAllow in the same peer.

podSelector && ( namespaceNames || namespaceSelector ) is what i meant there, updated the text.

does that make sense , or you think thats bad to allow using both (imo namespaceNames just appends to the ALLOW rules so...

... theres an obvious use case where

you have one policy

you have 5 namespaces selected by label and the 6th namespace you select by name

danwinship · 2020-11-10T18:17:30Z

keps/sig-network/2112-netpol-namespace-name-selector/README.md

+      namespaceSelector:
+        matchLabels:
+          app: my-frontend-2
+      ipBlock:


You are definitely not allowed to specify ipBlock with any other NetworkPolicyPeer field

danwinship · 2020-11-10T18:18:17Z

keps/sig-network/2112-netpol-namespace-name-selector/README.md

+
+## Alternatives
+
+A network policy operator could be created which translated a CRD into many networkpolicys on the fly, by watching namespaces and updating labels dynamically.  This would be a privileged container in a cluster and likely would not gain much adoption.


I'm not at all sure what you're suggesting here

added a small example and clarified it.... its that weird policy meta operator idea i keep shoping around :).

andrewsykim · 2020-11-10T18:43:14Z

keps/sig-network/2112-netpol-namespace-name-selector/README.md

+
+### Goals
+
+- Add a `namespaceSelectorAllow` option (which is additive to the current `matchLabels` selector).


This name is pretty confusing IMO. First, I think the term Allow is redundant cause you only specify namespaces you are trying to allow. Second the term Selector implies this selects against labels (or most users will think this at least). Prefer namespaces or namespaceNames`.

yeah i dont like this name either. just wanted something other then namespaceSelector since that was taken.

I'll go to namespaceNames

rikatz · 2020-11-12T20:00:32Z

keps/sig-network/2112-netpol-namespace-name-selector/README.md

+```
+type NetworkPolicyPeer struct {
+  // new field
+  namespaceNames []string


Probably per api design, this needs to be upper case first letter, right? (also because this field needs to be exported)

thockin · 2020-11-13T22:56:09Z

keps/sig-network/2112-netpol-namespace-name-selector/README.md

+```
+type NetworkPolicyPeer struct {
+  // new field
+  namespaceNames []string


thockin · 2020-11-13T22:56:39Z

keps/sig-network/2112-netpol-namespace-name-selector/README.md

+```
+type NetworkPolicyPeer struct {
+  // new field
+  namespaceNames []string


I would like to see a draft of what the API docs would say.

thockin · 2020-11-13T23:00:39Z

keps/sig-network/2112-netpol-namespace-name-selector/README.md

+      app: mysql
+  ingress:
+  - from:
+    - namespaceNames:


This doesn't gel - NamespaceNames and NamespaceSelector should be mutually exclusive, shouldn't they? They only exception we offer is that you can define NamespaceSelector and PodSelector, so I assume NamespaceNames and PodSelector would be legit.

ah missing bullet, fixing

thockin · 2020-11-13T23:21:25Z

keps/sig-network/2112-netpol-namespace-name-selector/README.md

+
+- NetworkPolicy providers may **opt-out**, initially to support this construct, and dilligent communication with CNI providers will be needed to make sure its widely adopted, thus, this feature needs to be backwards compatible with the existing v1 api.
+- We thus need to make sure that hidden defaults don't break the meaning of existing policys, for example:
+  - if `namespaceNames` field is retrieved by an older client which **doesn't** yet support it:


This is a huge liability.

If you have policy that says:

from: namespaceNames: - foo podSelector: app: myapp

That explicitly means to allow traffic from namespace "foo" AND NOT from this namespace. If you have an old client who doesn't know about the new field you will deserialize as:

from: podSelector: app: myapp

That explicitly means to allow traffic from this namespace. That's a VERY different meaning.

I'm not sure what the best way to mitigate this is. This is a general API evolution problem. We're adding a new option to a one-of and we need to accommodate clients that can't see it. In this case, we really want to fail-closed.

As proposed, this API doesn't fail-closed.

One option is to add a new sub-struct that contains BOTH the list of namespace names and the pod selector. This is redundant BUT it dodges the failure mode. Now the above example becomes:

from: namespaceList: namespaceNames: - foo podSelector: app: myapp

Which the old clients read as:

from: {}

I am very open to alternate ideas, but I think "old client fails closed" is probably a requirement.

curiously, virtual labels do not suffer this failure mode...

Ahhhh so, now that we're adding a new way to select things, if an old thing ignores that selector, a new policy will appear to be extremely promiscuous, because that new selector will be perceived as nil.

1

by virtual label, do we roughly mean "put junk in the selector that always mismatches unless a client is aware of the secret syntax" ? i.e.

namespaceSelector: ^virtualNamespaceName: "foo"

if so , yeah your right thats pretty clean . HOWEVER, i think the "definition" of wether or weve defined an objective "failure mode is debatable."

2.

Just to make sure we get to the bottom of the assumptions here (strongly leaning towards synth. labels...)... but to complete this thought of your workaround...

QUESTION: just making sure im correct, by from: {} did you really mean

from: [] ### Deny all .. reasonable interpretation if the goal is for old clients to be less open then the policies they cannot interpret

OR

from: - {}. ### Allow all , but.. i think this is invalid bc every peer must specify something?

(from is a list, afaik).

... In any case, that from: [] thingy... thats a DENY ALL... right ? (Theres no peers, so theres nothing to match an allow against). So, its still quite far from the original intent of allowing specific traffic from a ns/pod combination, albeit, its more restrictive rather then less.

Like, normally a NetworkPolicy says "isolate these pods (implicitly) and then allow access from X, Y, and Z". If you "hide" spec.ingress then the policy ends up saying "isolate these pods, and that's it"

If perfectly non surprising backward compatibility with other v1 CNIs is a must

It's unlikely that there is a perfect solution here. Only "acceptable", which
(IMO) means "fail closed".

Virtual labels is the closest to ideal FOR THIS SPECIFIC CASE, but it's a bit
of a detour to the objective here.

are we saying that we're deprecating fields in a stable API

Deprecating fields is allowed, just not REMOVING them. A v2 API would allow us
to clean up the API a little, but not really much. We still need fidelity in
round-trip. There's no panacea.

per #2137, the apiserver sees namespaceNames and so adds spec.minVersion: 1.21 at creation time.

I'll go read that KEP, but I will say that micro-versioning of APIs has been
discussed, but it comes with its own set of non-trivial costs. As you
describe, SOMEONE needs to taste the incoming objects and set the API versions,
including across update operations. Absent a general mechanism to do this, I
have little faith it will work.

Or we make the user set it, and deal with all sorts of error handling when they
get it wrong.

so it would still see the namespaceSelector rule that it understands, but it wouldn't see the peer containing the namespaceNames rule at all. (So it would end up implementing a more restrictive policy rather than a less restrictive one

We don't need micro-versioning to achieve this, though, just thoughful API
design?

Agree lack of a way to match on name is a friction point. The Calico team have some experience here; (after user demand) when we convert a namespace to our datamodel, we add a label in our namespace so that the user can match on the name. That seems to work fine for our users. We police user labels and disallow labels that would conflict.

There's plenty of precedent in the k8s API for auto-added labels (such as all the ones on the Node resource). Seems like the established norm is that those are just added to the datamodel as "real" labels that can be read as normal with kubectl. If the label was added as a "real" label like that, support would just fall out in all the CNI plugins and users would be able to do more advanced matches where they want to do so.

Node objects are more-or-less owned by Kubernetes itself though, so the fact that Kubernetes adds labels to Nodes isn't an example of "Kubernetes adding labels to other people's objects", it's just a slightly weird case of "owners adding labels to their own objects".

So #2137 would solve this if everyone already implemented #2137...

after some discussion there, #2137 solves the problem of "the apiserver knows about this feature but the network plugin doesn't" (user tries to apply a policy using this feature to a 1.22 cluster running a 1.20-era network plugin) but it doesn't solve the problem when the apiserver doesn't know about the feature (user tries to apply a policy using this feature to a 1.20 cluster) unless the user manually specifies minVersion on the policy, which is ugly.

k8s-ci-robot · 2020-11-15T16:47:49Z

@jayunit100: The following test failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
pull-enhancements-verify	`8ed8882`	link	`/test pull-enhancements-verify`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

jayunit100 · 2020-11-20T20:27:55Z

per discussion with @thockin and @liggitt "virtual labels" are potentially a thing, and were going to explore that avenue as a solution to this

jayunit100 · 2020-11-30T21:32:22Z

likely closing this in lieue of #2162 !

thockin · 2020-12-09T00:07:58Z

Preemptive close to get it off my radar :)

network policy namespace as name

64f8264

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 23, 2020

k8s-ci-robot requested review from dcbw and thockin October 23, 2020 13:56

k8s-ci-robot added kind/kep Categorizes KEP tracking issues and PRs modifying the KEP directory sig/network Categorizes an issue or PR as relevant to SIG Network. labels Oct 23, 2020

jayunit100 changed the title ~~Add a matchName selection option to the NetworkPolicy namespace selector API~~ WIP Add a matchName selection option to the NetworkPolicy namespace selector API Oct 23, 2020

k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 23, 2020

andrewsykim reviewed Oct 23, 2020

View reviewed changes

jayunit100 added 3 commits October 26, 2020 16:15

Update 20201014-NamespaceAsName.md

1af940b

Update 20201014-NamespaceAsName.md

a8aed65

Update 20201014-NamespaceAsName.md

1360bae

rikatz reviewed Oct 27, 2020

View reviewed changes

keps/sig-network/20201014-NamespaceAsName.md Outdated Show resolved Hide resolved

rikatz reviewed Oct 27, 2020

View reviewed changes

danwinship reviewed Oct 28, 2020

View reviewed changes

thockin self-assigned this Oct 28, 2020

kikisdeliveryservice requested changes Oct 30, 2020

View reviewed changes

jayunit100 and others added 5 commits November 2, 2020 04:44

Update keps/sig-network/20201014-NamespaceAsName.md

e0ceaf8

Co-authored-by: Dan Winship <[email protected]>

Update keps/sig-network/20201014-NamespaceAsName.md

e76b4ab

Co-authored-by: Dan Winship <[email protected]>

initial comment revisions

0ab230f

original issue link

a0aa2f3

remove f g

f6830ed

thockin reviewed Nov 2, 2020

View reviewed changes

jayunit100 mentioned this pull request Nov 3, 2020

kubernetes-sigs/network-policy-apis kubernetes/org#2239

Closed

backpeddle on the exclude rule and discuss w the broader group

fb32a49

Fixes and reworked proposal to modify networkpolicyPeer matchName yaml format change add feature gates note add feature gates note

jayunit100 force-pushed the namespace-name branch from 60f4a74 to fb32a49 Compare November 4, 2020 20:15

rikatz reviewed Nov 4, 2020

View reviewed changes

keps/sig-network/20201014-NamespaceAsName.md Outdated Show resolved Hide resolved

rikatz reviewed Nov 4, 2020

View reviewed changes

rikatz force-pushed the namespace-name branch from af5201a to d59de4d Compare November 4, 2020 21:25

rikatz force-pushed the namespace-name branch from d59de4d to 273398a Compare November 4, 2020 21:27

Fix some typos and formatted the Kep properly

08f7e6c

Signed-off-by: Ricardo Pchevuzinske Katz <[email protected]>

rikatz force-pushed the namespace-name branch from 273398a to 08f7e6c Compare November 4, 2020 21:32

andrewsykim reviewed Nov 4, 2020

View reviewed changes

NamespaceSelectorAllow as an add on rather then standalone

faf53e7

NamespaceSelectorAllow as an add on rather then standalone 2

80c9572

rikatz mentioned this pull request Nov 5, 2020

KEP 2079 - Add PortRange KEP #2090

Merged

danwinship mentioned this pull request Nov 9, 2020

KEP-2136: NetworkPolicy Versioning and Status #2137

Closed

danwinship reviewed Nov 10, 2020

View reviewed changes

andrewsykim reviewed Nov 10, 2020

View reviewed changes

jayunit100 added 6 commits November 11, 2020 13:48

s/namespaceSelectorAllow/namespaceNames

c7a89c5

move

2d5eade

fix yaml

5a41f49

clarify alternative operator impl

8101fb8

update TOC

e00fa66

bullets

7026bfd

rikatz reviewed Nov 12, 2020

View reviewed changes

thockin reviewed Nov 13, 2020

View reviewed changes

jayunit100 added 2 commits November 15, 2020 11:44

add table definitions of policy application, fix missing bullet

a1e4716

add table definitions of policy application, fix missing bullet

8ed8882

thockin closed this Dec 9, 2020


		#### Story 1

		As an administrator I want to block all users from accessing the kube-system namespace as a fundamental default policy.


		#### GA Graduation

		- At least two CNIs support the The name selector field


		### Notes/Constraints/Caveats (Optional)

		### Risks and Mitigations


		### Graduation Criteria

		Note that we may not have explicity feature gates for this KEP, because NetworkPolicies typically haven't been implemented with gates. We keep this section for the time being in any case as it organizes our thinking around the general progression of the feature over time.


		## Proposal

		In NetworkPolicy specification, inside `NetworkPolicyPeer` specify a new `namespaceSelectorAllow` field.


		- Add a new selector to the network policy peer data structure which can switch between allowing a `namespaceSelectorAllow`, supporting a policy that is expressed like this:

		- A list of conventional namespaces


		## Alternatives

		A network policy operator could be created which translated a CRD into many networkpolicys on the fly, by watching namespaces and updating labels dynamically. This would be a privileged container in a cluster and likely would not gain much adoption.


		### Goals

		- Add a `namespaceSelectorAllow` option (which is additive to the current `matchLabels` selector).

WIP Add a matchName selection option to the NetworkPolicy namespace selector API #2113

WIP Add a matchName selection option to the NetworkPolicy namespace selector API #2113

Conversation

jayunit100 commented Oct 23, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thockin Nov 2, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrewsykim Nov 2, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kikisdeliveryservice left a comment • edited Loading

Choose a reason for hiding this comment

k8s-ci-robot commented Oct 30, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thockin Nov 2, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rikatz commented Nov 4, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jayunit100 commented Nov 5, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jayunit100 Nov 11, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jayunit100 Nov 11, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jayunit100 commented Oct 23, 2020 •

edited

Loading

thockin Nov 2, 2020 •

edited

Loading

andrewsykim Nov 2, 2020 •

edited

Loading

kikisdeliveryservice left a comment •

edited

Loading

thockin Nov 2, 2020 •

edited

Loading

jayunit100 commented Nov 5, 2020 •

edited

Loading

jayunit100 Nov 11, 2020 •

edited

Loading

jayunit100 Nov 11, 2020 •

edited

Loading