Make SecurityPolicy validate correctly.

Signed-off-by: Lior Okman <lior.okman@sap.com>
envoyproxy · Mar 18, 2024 · 383041e · 383041e
1 parent abd4faa
commit 383041e
Show file tree

Hide file tree

Showing 4 changed files with 727 additions and 8 deletions.
diff --git a/internal/gatewayapi/securitypolicy.go b/internal/gatewayapi/securitypolicy.go
@@ -194,7 +194,7 @@ func (t *Translator) ProcessSecurityPolicies(securityPolicies []*egv1a1.Security
 			irKey := t.getIRKey(targetedGateway.Gateway)
 			// Should exist since we've validated this
 			xds := xdsIR[irKey]
-			err := validatePortOverlapForSecurityPolicyGateway(xds)
+			err := validatePortOverlapForSecurityPolicyGateway(xds, targetedGateway)
 			if err == nil {
 				err = t.translateSecurityPolicyForGateway(policy, targetedGateway, resources, xdsIR)
 			}
@@ -522,11 +522,26 @@ func (t *Translator) translateSecurityPolicyForGateway(
 	return errs
 }
 
-func validatePortOverlapForSecurityPolicyGateway(xds *ir.Xds) error {
+// should return error if the policy attaches to listeners that originate from gateways other than the one requested on the policy.
+func validatePortOverlapForSecurityPolicyGateway(xds *ir.Xds, targetedGateway *GatewayContext) error {
+	targetedGwName := irStringKey(targetedGateway.Namespace, targetedGateway.Name)
+	relevantPorts := map[uint32]bool{}
+	for _, listener := range targetedGateway.listeners {
+		containerPort := servicePortToContainerPort(int32(listener.Port))
+		relevantPorts[uint32(containerPort)] = true
+	}
 	affectedListeners := []string{}
 	for _, http := range xds.HTTP {
-		if sameListeners := listenersWithSameHTTPPort(xds, http); len(sameListeners) != 0 {
-			affectedListeners = append(affectedListeners, sameListeners...)
+		if _, found := relevantPorts[http.Port]; !found {
+			continue
+		}
+		// look for listeners on this XDS that aren't from the targetedGateway
+		for _, currListener := range listenersWithSameHTTPPort(xds, http) {
+			listenerName := currListener[0:strings.LastIndex(currListener, "/")]
+			if listenerName != targetedGwName {
+				affectedListeners = append(affectedListeners, currListener)
+			}
+
 		}
 	}
 

diff --git a/internal/gatewayapi/testdata/conflicting-policies.out.yaml b/internal/gatewayapi/testdata/conflicting-policies.out.yaml
@@ -261,7 +261,7 @@ securityPolicies:
         namespace: default
       conditions:
       - lastTransitionTime: null
-        message: 'Affects multiple listeners: default/mfqjpuycbgjrtdww/http, default/gateway-1/http'
+        message: 'Affects multiple listeners: default/gateway-1/http'
         reason: Invalid
         status: "False"
         type: Accepted

diff --git a/internal/gatewayapi/testdata/merge-with-isolated-policies-2.in.yaml b/internal/gatewayapi/testdata/merge-with-isolated-policies-2.in.yaml
@@ -90,6 +90,44 @@ httpRoutes:
           backendRefs:
             - name: service-2
               port: 8080
+  - apiVersion: gateway.networking.k8s.io/v1
+    kind: HTTPRoute
+    metadata:
+      namespace: default
+      name: httproute-3
+    spec:
+      hostnames:
+        - bar.example.com
+      parentRefs:
+        - namespace: default
+          name: gateway-2
+          sectionName: http
+      rules:
+        - matches:
+            - path:
+                value: "/"
+          backendRefs:
+            - name: service-1
+              port: 8080
+  - apiVersion: gateway.networking.k8s.io/v1
+    kind: HTTPRoute
+    metadata:
+      namespace: default
+      name: httproute-4
+    spec:
+      hostnames:
+        - foo.example.com
+      parentRefs:
+        - namespace: default
+          name: gateway-2
+          sectionName: http-2
+      rules:
+        - matches:
+            - path:
+                value: "/"
+          backendRefs:
+            - name: service-2
+              port: 8080
 securityPolicies:
   - apiVersion: gateway.envoyproxy.io/v1alpha1
     kind: SecurityPolicy
@@ -123,10 +161,9 @@ securityPolicies:
     spec:
       targetRef:
         group: gateway.networking.k8s.io
-        kind: Gateway
-        name: gateway-2
+        kind: HTTPRoute
+        name: httproute-3
         namespace: default
-        sectionName: http
       cors:
         allowOrigins:
           - "*"