Merge pull request kubernetes#47123 from danwinship/networkpolicy-update

Automatic merge from submit-queue

Allow NetworkPolicy.spec updates

ValidateNetworkPolicyUpdate currently prohibits changes to `spec` in an existing NetworkPolicy. We were going to fix this for 1.7 but I forgot to submit this PR after the main PR merged. Too late for 1.7? @thockin @caseydavenport @cmluciano 

This only changes networking.NetworkPolicy validation at the moment... Should I change extensions.NetworkPolicy validation too?

Fixes kubernetes#35911

We should add a test to the e2e NetworkPolicy test for this too if this is going to merge.

**Release note**:
```release-note
As part of the NetworkPolicy "v1" changes, it is also now
possible to update the spec field of an existing
NetworkPolicy. (Previously you had to delete and recreate a
NetworkPolicy if you wanted to change it.)
```

Author: Kubernetes Submit Queue

pkg/apis/networking/validation/validation.go

@@ -17,8 +17,6 @@ limitations under the License.
 package validation
 
 import (
-	"reflect"
-
 	unversionedvalidation "k8s.io/apimachinery/pkg/apis/meta/v1/validation"
 	"k8s.io/apimachinery/pkg/util/intstr"
 	"k8s.io/apimachinery/pkg/util/validation"
@@ -92,8 +90,6 @@ func ValidateNetworkPolicy(np *networking.NetworkPolicy) field.ErrorList {
 func ValidateNetworkPolicyUpdate(update, old *networking.NetworkPolicy) field.ErrorList {
 	allErrs := field.ErrorList{}
 	allErrs = append(allErrs, apivalidation.ValidateObjectMetaUpdate(&update.ObjectMeta, &old.ObjectMeta, field.NewPath("metadata"))...)
-	if !reflect.DeepEqual(update.Spec, old.Spec) {
-		allErrs = append(allErrs, field.Forbidden(field.NewPath("spec"), "updates to networkpolicy spec are forbidden."))
-	}
+	allErrs = append(allErrs, ValidateNetworkPolicySpec(&update.Spec, field.NewPath("spec"))...)
 	return allErrs
 }

pkg/apis/networking/validation/validation_test.go

@@ -271,8 +271,8 @@ func TestValidateNetworkPolicyUpdate(t *testing.T) {
 		old    networking.NetworkPolicy
 		update networking.NetworkPolicy
 	}
-	successCases := []npUpdateTest{
-		{
+	successCases := map[string]npUpdateTest{
+		"no change": {
 			old: networking.NetworkPolicy{
 				ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "bar"},
 				Spec: networking.NetworkPolicySpec{
@@ -292,17 +292,7 @@ func TestValidateNetworkPolicyUpdate(t *testing.T) {
 				},
 			},
 		},
-	}
-
-	for _, successCase := range successCases {
-		successCase.old.ObjectMeta.ResourceVersion = "1"
-		successCase.update.ObjectMeta.ResourceVersion = "1"
-		if errs := ValidateNetworkPolicyUpdate(&successCase.update, &successCase.old); len(errs) != 0 {
-			t.Errorf("expected success: %v", errs)
-		}
-	}
-	errorCases := map[string]npUpdateTest{
-		"change name": {
+		"change spec": {
 			old: networking.NetworkPolicy{
 				ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "bar"},
 				Spec: networking.NetworkPolicySpec{
@@ -311,14 +301,27 @@ func TestValidateNetworkPolicyUpdate(t *testing.T) {
 				},
 			},
 			update: networking.NetworkPolicy{
-				ObjectMeta: metav1.ObjectMeta{Name: "baz", Namespace: "bar"},
+				ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "bar"},
 				Spec: networking.NetworkPolicySpec{
-					PodSelector: metav1.LabelSelector{},
-					Ingress:     []networking.NetworkPolicyIngressRule{},
+					PodSelector: metav1.LabelSelector{
+						MatchLabels: map[string]string{"a": "b"},
+					},
+					Ingress: []networking.NetworkPolicyIngressRule{},
 				},
 			},
 		},
-		"change spec": {
+	}
+
+	for testName, successCase := range successCases {
+		successCase.old.ObjectMeta.ResourceVersion = "1"
+		successCase.update.ObjectMeta.ResourceVersion = "1"
+		if errs := ValidateNetworkPolicyUpdate(&successCase.update, &successCase.old); len(errs) != 0 {
+			t.Errorf("expected success (%s): %v", testName, errs)
+		}
+	}
+
+	errorCases := map[string]npUpdateTest{
+		"change name": {
 			old: networking.NetworkPolicy{
 				ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "bar"},
 				Spec: networking.NetworkPolicySpec{
@@ -327,18 +330,18 @@ func TestValidateNetworkPolicyUpdate(t *testing.T) {
 				},
 			},
 			update: networking.NetworkPolicy{
-				ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "bar"},
+				ObjectMeta: metav1.ObjectMeta{Name: "baz", Namespace: "bar"},
 				Spec: networking.NetworkPolicySpec{
-					PodSelector: metav1.LabelSelector{
-						MatchLabels: map[string]string{"a": "b"},
-					},
-					Ingress: []networking.NetworkPolicyIngressRule{},
+					PodSelector: metav1.LabelSelector{},
+					Ingress:     []networking.NetworkPolicyIngressRule{},
 				},
 			},
 		},
 	}
 
 	for testName, errorCase := range errorCases {
+		errorCase.old.ObjectMeta.ResourceVersion = "1"
+		errorCase.update.ObjectMeta.ResourceVersion = "1"
 		if errs := ValidateNetworkPolicyUpdate(&errorCase.update, &errorCase.old); len(errs) == 0 {
 			t.Errorf("expected failure: %s", testName)
 		}