fix: should reject bad FilterSubPolicy (#819)

Signed-off-by: spacewander <[email protected]>
mosn · Dec 12, 2024 · ec3d119 · ec3d119
1 parent 429af5a
commit ec3d119
Show file tree

Hide file tree

Showing 2 changed files with 103 additions and 1 deletion.
diff --git a/types/apis/v1/validation.go b/types/apis/v1/validation.go
@@ -228,7 +228,21 @@ func validateFilterPolicy(policy *FilterPolicy, strict bool) error {
 		}
 	}
 
-	for _, policy := range policy.Spec.SubPolicies {
+	names := map[string]struct{}{}
+	for i, policy := range policy.Spec.SubPolicies {
+		if policy.SectionName == "" {
+			return fmt.Errorf("sectionName in SubPolicies[%d] is required", i)
+		}
+		if len(policy.Filters) == 0 {
+			return fmt.Errorf("filters in SubPolicies[%d] is required", i)
+		}
+
+		if _, ok := names[string(policy.SectionName)]; ok {
+			return fmt.Errorf("multiple SubPolicies should not have same sectionName %s", policy.SectionName)
+		}
+
+		names[string(policy.SectionName)] = struct{}{}
+
 		for name, filter := range policy.Filters {
 			err := validateFilter(name, filter, strict, targetGateway)
 			if err != nil {

diff --git a/types/apis/v1/validation_test.go b/types/apis/v1/validation_test.go
@@ -15,6 +15,7 @@
 package v1
 
 import (
+	"fmt"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -269,6 +270,86 @@ func TestValidateFilterPolicy(t *testing.T) {
 			},
 			strictErr: "unknown http filter: property",
 		},
+		{
+			name: "bad subPolicy (no filters)",
+			policy: &FilterPolicy{
+				Spec: FilterPolicySpec{
+					TargetRef: &gwapiv1a2.PolicyTargetReferenceWithSectionName{
+						PolicyTargetReference: gwapiv1a2.PolicyTargetReference{
+							Group: "networking.istio.io",
+							Kind:  "VirtualService",
+						},
+					},
+					SubPolicies: []FilterSubPolicy{
+						{
+							SectionName: sectionName,
+						},
+					},
+				},
+			},
+			err: "filters in SubPolicies[0] is required",
+		},
+		{
+			name: "bad subPolicy (no sectionName)",
+			policy: &FilterPolicy{
+				Spec: FilterPolicySpec{
+					TargetRef: &gwapiv1a2.PolicyTargetReferenceWithSectionName{
+						PolicyTargetReference: gwapiv1a2.PolicyTargetReference{
+							Group: "networking.istio.io",
+							Kind:  "VirtualService",
+						},
+					},
+					SubPolicies: []FilterSubPolicy{
+						{
+							Filters: map[string]Plugin{
+								"animal": {
+									Config: runtime.RawExtension{
+										Raw: []byte(`{"pet":"cat"}`),
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+			err: "sectionName in SubPolicies[0] is required",
+		},
+		{
+			name: "bad subPolicy (repeated sectionName)",
+			policy: &FilterPolicy{
+				Spec: FilterPolicySpec{
+					TargetRef: &gwapiv1a2.PolicyTargetReferenceWithSectionName{
+						PolicyTargetReference: gwapiv1a2.PolicyTargetReference{
+							Group: "networking.istio.io",
+							Kind:  "VirtualService",
+						},
+					},
+					SubPolicies: []FilterSubPolicy{
+						{
+							SectionName: sectionName,
+							Filters: map[string]Plugin{
+								"animal": {
+									Config: runtime.RawExtension{
+										Raw: []byte(`{"pet":"cat"}`),
+									},
+								},
+							},
+						},
+						{
+							SectionName: sectionName,
+							Filters: map[string]Plugin{
+								"animal": {
+									Config: runtime.RawExtension{
+										Raw: []byte(`{"pet":"dog"}`),
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+			err: fmt.Sprintf("multiple SubPolicies should not have same sectionName %s", sectionName),
+		},
 		{
 			name: "targetRef.SectionName and SubPolicies can not be used together",
 			policy: &FilterPolicy{
@@ -283,6 +364,13 @@ func TestValidateFilterPolicy(t *testing.T) {
 					SubPolicies: []FilterSubPolicy{
 						{
 							SectionName: sectionName,
+							Filters: map[string]Plugin{
+								"animal": {
+									Config: runtime.RawExtension{
+										Raw: []byte(`{"pet":"cat"}`),
+									},
+								},
+							},
 						},
 					},
 				},