This change is 

Here's an initial attempt at moving NetworkPolicy to v1... this still needs work.

(Just to clarify, that work is blocked on decisions being made about the questions I asked above)

Sorry for radio silence I was OOO and am still catching up. asking in the api-machinery slack channel is probably your best bet if you have open api machinery questions.

Before declaring v1 done we should probably land #35660 (e2e NetworkPolicy tests), extend it to cover all features that are going to be part of v1, and make sure existing implementations actually implement all those features. And if most/all of them don't implement a feature, maybe we should consider dropping it for v1.

eg, as far as I can tell, none of Calico, Romana, or Weave implement string-valued Ports (qv #39769)

How does migration between the v1beta1 net.beta.kubernetes.io/network-policy Namespace annotation and the v1 Namespace.NetworkPolicy field work

This message to kubernetes-dev says they're not maintaining any backward compatibility with alpha annotations for scheduling features so maybe we can do the same thing? (Although that's alpha->beta, not beta->v1...)

Do the v1beta1 comment changes correctly describe the way that existing NetworkPolicy implementations work?

Yep, I think those comments are accurate now.

What should the unversioned→v1beta1 conversion do with policies containing an "allow no ports" or "allow no peers" rule? Currently it tries to represent that using a nil-vs-[] distinction, but that's obviously wrong since we're not documenting any such distinction any more (and the conversion to/from json won't respect that distinction). Should it return an error instead? (Presumably that would mean v1beta1 clients would get errors if they tried to fetch such policies?)

Well, a "match no ports" or a "match no peers" rule can be represented in the v1beta1 API by just removing that rule altogether, right? That rule is essentially useless I think.

Should NetworkPolicy stay in pkg/apis/extensions? If not, where should it move? (Does the infrastructure support moving APIs between versions? Eg, conversions.)

Not sure (and I don't think I have a very strong opinion).

How does migration between the v1beta1 net.beta.kubernetes.io/network-policy Namespace annotation and the v1 Namespace.NetworkPolicy field work, given that there is not actually a version number change for Namespace?

Can probably respect the annotation if it is present and the Namespace field is not, but prioritize the Namespace field if it is present? I don't think any special migration code is necessary - up to the implementations what they want to do, but I suspect that they'll want to support both for some period of time.

Do we want to simplify NamespaceNetworkPolicy at all? eg, it currently distinguishes between "default network policy" ({ "networkPolicy": nil }) "default ingress network policy (but possibly non-default egress policy)" ({ "networkPolicy": { "ingress": nil } }), and "default ingress isolation policy (but possibly non-default policy for other aspects of ingress)" ({ "networkPolicy": { "ingress": { "isolation": nil } } }). Do we still think NetworkPolicy will eventually describe things other than traffic isolation?

I'd opt for keeping it as is since we've seen no reason for changing it during it's time in beta. I'm really keen to not change things from beta -> GA unless we've seen a compelling reason to do so.

Where do we move docs/proposals/network-policy.md now that it's no longer a "proposal"? Also, presumably we update it to refer only to the v1 API? (Do we need to worry about documenting the v1beta1 Namespace annotation somewhere since it won't show up anywhere in the generated API docs?)

I imagine that can stay under proposals, right? I don't think the intention is that those evolve as the API evolves.

All-in-all, I'm still a little bit uneasy about the empty vs nil change. It feels like extra work for implementors that doesn't buy us anything. I keep asking myself "when did we ever get feedback from a user asking for this distinction? when did it confuse a user of the API? when did it make things harder for implementations?" and the answers always come back "never".

eg, as far as I can tell, none of Calico, Romana, or Weave implement string-valued Ports (qv #39769)

Speaking for Weave, that is indeed not presently implemented. It complicates matters, but does not seem insurmountable.

eg, as far as I can tell, none of Calico, Romana, or Weave implement string-valued Ports

Yep, Calico doesn't implement this yet either. It's not impossible but it's not a one-liner to add support.

I've not heard any user demand for this.

Well, a "match no ports" or a "match no peers" rule can be represented in the v1beta1 API by just removing that rule altogether, right? That rule is essentially useless I think.

Ah, indeed

Where do we move docs/proposals/network-policy.md now that it's no longer a "proposal"?

I imagine that can stay under proposals, right? I don't think the intention is that those evolve as the API evolves.

Well, OK, but we need to document NetworkPolicy somewhere that isn't just a "proposal". But I guess that's an easier question: just add the new docs wherever they fit in best.

All-in-all, I'm still a little bit uneasy about the empty vs nil change. It feels like extra work for implementors that doesn't buy us anything. I keep asking myself "when did we ever get feedback from a user asking for this distinction? when did it confuse a user of the API? when did it make things harder for implementations?" and the answers always come back "never".

Yeah, I don't think anyone actually wanted the distinction, it's just that the way that v1beta1 ended up being implemented (empty Ports means "all ports", empty From means "from anywhere") makes us inconsistent with other APIs (and in particular, it makes Ports/From inconsistent with PodSelector/NamespaceSelector, meaning NetworkPolicy isn't even internally self-consistent).

But yeah, the "fix" is ugly. I'd be fine sticking with the v1beta1 behavior. (But there were objections to that when it was discussed in the SIG.)

Yeah, I don't think anyone actually wanted the distinction, it's just that the way that v1beta1 ended up being implemented (empty Ports means "all ports", empty From means "from anywhere") makes us inconsistent with other APIs (and in particular, it makes Ports/From inconsistent with PodSelector/NamespaceSelector, meaning NetworkPolicy isn't even internally self-consistent).

I noticed while poking around the ClusterRole specification that it seems there are other APIs using an empty slice to indicate "all" rather than "none".

// ResourceNames is an optional white list of names that the rule applies to.  An empty set means that everything is allowed.
ResourceNames []string

@danwinship: The following test(s) failed:

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.

@thockin: fixed defaults, rebased, regenerated

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: danwinship, thockin

Automatic merge from submit-queue

If we delete the extensions.NetworkPolicy - who would register the v1beta1 endpoint? Ugh, I don't understand the apiserver anymore....

@thockin you could register the types from the other package. For example, pkg/apis/apps registers the pkg/apis/extensions types:

// Adds the list of known types to api.Scheme.
func addKnownTypes(scheme *runtime.Scheme) error {
	// TODO this will get cleaned up with the scheme types are fixed
	scheme.AddKnownTypes(SchemeGroupVersion,
		&extensions.Deployment{},
		&extensions.DeploymentList{},
		&extensions.DeploymentRollback{},
		&extensions.Scale{},
		&StatefulSet{},
		&StatefulSetList{},
	)
	return nil
}

One question here is if I upgrade NetworkPolicy to v1, do I also need to upgrade the calico? Currently, I was using calico node image as 1.2.1 and calico policy controller image as 0.6.0. @danwinship

I filed an issue for calico https://github.com/projectcalico/k8s-policy/issues/108 , the NetworkPolicy of net.beta.kubernetes.io/network-policy still works in Kubernetes 1.7, but the CHANGELOG mentioned this has been removed in 1.7, is it a bug? @danwinship

NetworkPolicy has been promoted from extensions/v1beta1 to the new networking.k8s.io/v1 API group. The structure remains unchanged from the v1beta1 API. The net.beta.kubernetes.io/network-policy annotation on Namespaces (used to opt in to isolation) has been removed.