From ec3d1199bf043fc546626f3d9d459c91fc80e686 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=E7=BD=97=E6=B3=BD=E8=BD=A9?= <spacewanderlzx@gmail.com>
Date: Thu, 12 Dec 2024 10:52:45 +0800
Subject: [PATCH] fix: should reject bad FilterSubPolicy (#819)

Signed-off-by: spacewander <spacewanderlzx@gmail.com>
---
 types/apis/v1/validation.go      | 16 +++++-
 types/apis/v1/validation_test.go | 88 ++++++++++++++++++++++++++++++++
 2 files changed, 103 insertions(+), 1 deletion(-)

diff --git a/types/apis/v1/validation.go b/types/apis/v1/validation.go
index 5d675656..879660e0 100644
--- 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
index 197ac7f9..b7160f8b 100644
--- 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"}`),
+									},
+								},
+							},
 						},
 					},
 				},