fix the validation of the policy's PodSubnet and PodSelector (#942)

Signed-off-by: bzsuni <[email protected]>
spidernet-io · Nov 7, 2023 · b1b2f82 · b1b2f82 · weizhoublue · Nov 7, 2023
1 parent eaf8460
commit b1b2f82
Show file tree

Hide file tree

Showing 3 changed files with 35 additions and 7 deletions.
diff --git a/pkg/controller/webhook/validate.go b/pkg/controller/webhook/validate.go
@@ -86,6 +86,13 @@ func validateEgressPolicy(ctx context.Context, client client.Client, req webhook
 		return webhook.Denied("podSelector and podSubnet cannot be used together")
 	}
 
+	// denied when both PodSelector and PodSubnet are empty
+	if egp.Spec.AppliedTo.PodSubnet == nil || len(egp.Spec.AppliedTo.PodSubnet) == 0 {
+		if egp.Spec.AppliedTo.PodSelector == nil || (len(egp.Spec.AppliedTo.PodSelector.MatchLabels) == 0 && len(egp.Spec.AppliedTo.PodSelector.MatchExpressions) == 0) {
+			return webhook.Denied("invalid EgressPolicy, spec.appliedTo field requires at least one of spec.appliedTo.podSubnet, .spec.appliedTo.podSelector.matchLabels or .spec.appliedTo.podSelector.matchExpressions to be specified.")
+		}
+	}
+
 	if req.Operation == v1.Update {
 		oldEgp := new(egressv1.EgressPolicy)
 		err := json.Unmarshal(req.OldObject.Raw, oldEgp)
@@ -164,6 +171,13 @@ func validateEgressClusterPolicy(ctx context.Context, client client.Client, req
 		return webhook.Denied("podSelector and podSubnet cannot be used together")
 	}
 
+	// denied when both PodSelector and PodSubnet are empty
+	if policy.Spec.AppliedTo.PodSubnet == nil || len(*policy.Spec.AppliedTo.PodSubnet) == 0 {
+		if policy.Spec.AppliedTo.PodSelector == nil || (len(policy.Spec.AppliedTo.PodSelector.MatchLabels) == 0 && len(policy.Spec.AppliedTo.PodSelector.MatchExpressions) == 0) {
+			return webhook.Denied("invalid EgressClusterPolicy, spec.appliedTo field requires at least one of spec.appliedTo.podSubnet, .spec.appliedTo.podSelector.matchLabels or .spec.appliedTo.podSelector.matchExpressions to be specified.")
+		}
+	}
+
 	if req.Operation == v1.Update {
 		oldPolicy := new(egressv1.EgressClusterPolicy)
 		err := json.Unmarshal(req.OldObject.Raw, oldPolicy)

diff --git a/pkg/controller/webhook/validate_test.go b/pkg/controller/webhook/validate_test.go
@@ -226,6 +226,15 @@ func TestValidateEgressPolicy(t *testing.T) {
 			},
 			expAllow: true,
 		},
+		"case6 empty AppliedTo": {
+			existingResources: nil,
+			spec: v1beta1.EgressPolicySpec{
+				EgressGatewayName: "test",
+				AppliedTo:         v1beta1.AppliedTo{},
+				DestSubnet:        []string{},
+			},
+			expAllow: false,
+		},
 	}
 	for name, c := range cases {
 		t.Run(name, func(t *testing.T) {
@@ -641,6 +650,15 @@ func TestValidateEgressClusterPolicy(t *testing.T) {
 			},
 			expAllow: true,
 		},
+		"case5 empty AppliedTo": {
+			existingResources: nil,
+			spec: v1beta1.EgressClusterPolicySpec{
+				EgressGatewayName: "test",
+				AppliedTo:         v1beta1.ClusterAppliedTo{},
+				DestSubnet:        []string{},
+			},
+			expAllow: false,
+		},
 	}
 	for name, c := range cases {
 		t.Run(name, func(t *testing.T) {

diff --git a/test/e2e/egresspolicy/egresspolicy_test.go b/test/e2e/egresspolicy/egresspolicy_test.go
@@ -267,8 +267,7 @@ var _ = Describe("EgressPolicy", Ordered, func() {
 						egp.Spec.EgressIP.IPv6 = "fddd:10::2"
 					}
 				}),
-			// todo @bzsuni waiting for the bug be fixed
-			PEntry("should fail when Spec.AppliedTo is empty", Label("P00005"), true,
+			Entry("should fail when Spec.AppliedTo is empty", Label("P00005"), true,
 				func(egp *egressv1.EgressPolicy) {
 					egp.Spec.EgressGatewayName = egw.Name
 					egp.Spec.AppliedTo = egressv1.AppliedTo{}
@@ -323,15 +322,12 @@ var _ = Describe("EgressPolicy", Ordered, func() {
 						egcp.Spec.EgressIP.IPv6 = "fddd:10::2"
 					}
 				}),
-
-			// todo @bzsuni waiting for the bug be fixed
-			PEntry("should fail when Spec.AppliedTo is empty", Label("P00005"), true,
+			Entry("should fail when Spec.AppliedTo is empty", Label("P00005"), true,
 				func(egcp *egressv1.EgressClusterPolicy) {
 					egcp.Spec.EgressGatewayName = egw.Name
 					egcp.Spec.AppliedTo = egressv1.ClusterAppliedTo{}
 				}),
-			// todo @bzsuni waiting for the bug be fixed
-			PEntry("should fail when the cluster-policy set with both Spec.AppliedTo.PodSubnet and Spec.AppliedTo.PodSelector", Label("P00006"), true,
+			Entry("should fail when the cluster-policy set with both Spec.AppliedTo.PodSubnet and Spec.AppliedTo.PodSelector", Label("P00006"), true,
 				func(egcp *egressv1.EgressClusterPolicy) {
 					egcp.Spec.EgressGatewayName = egw.Name
 					egcp.Spec.AppliedTo.PodSubnet = &[]string{"10.10.0.0/16"}