GetParentReferences should use namespace from RouteContext (#3876)

* GetParentReferences should use namespace from RouteContext Signed-off-by: zirain <zirain2009@gmail.com> * lint Signed-off-by: zirain <zirain2009@gmail.com> * add test Signed-off-by: zirain <zirain2009@gmail.com> * fix test Signed-off-by: zirain <zirain2009@gmail.com> * update Signed-off-by: zirain <zirain2009@gmail.com> * add negative case Signed-off-by: zirain <zirain2009@gmail.com> * address review comment Signed-off-by: zirain <zirain2009@gmail.com> --------- Signed-off-by: zirain <zirain2009@gmail.com>
envoyproxy · Jul 18, 2024 · 09fcfb3 · 09fcfb3
1 parent 6b232f1
commit 09fcfb3
Show file tree

Hide file tree

Showing 7 changed files with 526 additions and 6 deletions.
diff --git a/internal/gatewayapi/helpers.go b/internal/gatewayapi/helpers.go
@@ -109,7 +109,7 @@ func KindDerefOr(kind *gwapiv1.Kind, defaultKind string) string {
 // to a Gateway with the given namespace/name, irrespective of whether a
 // section/listener name has been specified (i.e. a parent ref to a listener
 // on the specified gateway will return "true").
-func IsRefToGateway(parentRef gwapiv1.ParentReference, gateway types.NamespacedName) bool {
+func IsRefToGateway(routeNamespace gwapiv1.Namespace, parentRef gwapiv1.ParentReference, gateway types.NamespacedName) bool {
 	if parentRef.Group != nil && string(*parentRef.Group) != gwapiv1.GroupName {
 		return false
 	}
@@ -118,7 +118,12 @@ func IsRefToGateway(parentRef gwapiv1.ParentReference, gateway types.NamespacedN
 		return false
 	}
 
-	if parentRef.Namespace != nil && string(*parentRef.Namespace) != gateway.Namespace {
+	ns := routeNamespace
+	if parentRef.Namespace != nil {
+		ns = *parentRef.Namespace
+	}
+
+	if string(ns) != gateway.Namespace {
 		return false
 	}
 
@@ -129,11 +134,11 @@ func IsRefToGateway(parentRef gwapiv1.ParentReference, gateway types.NamespacedN
 // in the given list, and if so, a list of the Listeners within that Gateway that
 // are included by the parent ref (either one specific Listener, or all Listeners
 // in the Gateway, depending on whether section name is specified or not).
-func GetReferencedListeners(parentRef gwapiv1.ParentReference, gateways []*GatewayContext) (bool, []*ListenerContext) {
+func GetReferencedListeners(routeNamespace gwapiv1.Namespace, parentRef gwapiv1.ParentReference, gateways []*GatewayContext) (bool, []*ListenerContext) {
 	var referencedListeners []*ListenerContext
 
 	for _, gateway := range gateways {
-		if IsRefToGateway(parentRef, utils.NamespacedName(gateway)) {
+		if IsRefToGateway(routeNamespace, parentRef, utils.NamespacedName(gateway)) {
 			// The parentRef may be to the entire Gateway, or to a specific listener.
 			for _, listener := range gateway.listeners {
 				if (parentRef.SectionName == nil || *parentRef.SectionName == listener.Name) && (parentRef.Port == nil || *parentRef.Port == listener.Port) {

diff --git a/internal/gatewayapi/helpers_test.go b/internal/gatewayapi/helpers_test.go
@@ -17,6 +17,7 @@ import (
 	"github.com/stretchr/testify/require"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/utils/ptr"
 	gwapiv1 "sigs.k8s.io/gateway-api/apis/v1"
 	gwapiv1a2 "sigs.k8s.io/gateway-api/apis/v1alpha2"
@@ -485,3 +486,71 @@ func TestGetPolicyTargetRefs(t *testing.T) {
 		})
 	}
 }
+
+func TestIsRefToGateway(t *testing.T) {
+	cases := []struct {
+		name           string
+		routeNamespace gwapiv1.Namespace
+		parentRef      gwapiv1.ParentReference
+		gatewayNN      types.NamespacedName
+		expected       bool
+	}{
+		{
+			name:           "match without namespace-true",
+			routeNamespace: gwapiv1.Namespace("ns1"),
+			parentRef: gwapiv1.ParentReference{
+				Name: gwapiv1.ObjectName("eg"),
+			},
+			gatewayNN: types.NamespacedName{
+				Name:      "eg",
+				Namespace: "ns1",
+			},
+			expected: true,
+		},
+		{
+			name:           "match without namespace-false",
+			routeNamespace: gwapiv1.Namespace("ns1"),
+			parentRef: gwapiv1.ParentReference{
+				Name: gwapiv1.ObjectName("eg"),
+			},
+			gatewayNN: types.NamespacedName{
+				Name:      "eg",
+				Namespace: "ns2",
+			},
+			expected: false,
+		},
+		{
+			name:           "match with namespace-true",
+			routeNamespace: gwapiv1.Namespace("ns1"),
+			parentRef: gwapiv1.ParentReference{
+				Name:      gwapiv1.ObjectName("eg"),
+				Namespace: NamespacePtr("ns1"),
+			},
+			gatewayNN: types.NamespacedName{
+				Name:      "eg",
+				Namespace: "ns1",
+			},
+			expected: true,
+		},
+		{
+			name:           "match without namespace2-false",
+			routeNamespace: gwapiv1.Namespace("ns1"),
+			parentRef: gwapiv1.ParentReference{
+				Name:      gwapiv1.ObjectName("eg"),
+				Namespace: NamespacePtr("ns2"),
+			},
+			gatewayNN: types.NamespacedName{
+				Name:      "eg",
+				Namespace: "ns1",
+			},
+			expected: false,
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			got := IsRefToGateway(tc.routeNamespace, tc.parentRef, tc.gatewayNN)
+			require.Equal(t, tc.expected, got)
+		})
+	}
+}
diff --git a/internal/gatewayapi/route.go b/internal/gatewayapi/route.go
@@ -1409,9 +1409,9 @@ func inspectAppProtocolByRouteKind(kind gwapiv1.Kind) ir.AppProtocol {
 // attach for each parentRef.
 func (t *Translator) processAllowedListenersForParentRefs(routeContext RouteContext, gateways []*GatewayContext, resources *Resources) bool {
 	var relevantRoute bool
-
+	ns := gwapiv1.Namespace(routeContext.GetNamespace())
 	for _, parentRef := range GetParentReferences(routeContext) {
-		isRelevantParentRef, selectedListeners := GetReferencedListeners(parentRef, gateways)
+		isRelevantParentRef, selectedListeners := GetReferencedListeners(ns, parentRef, gateways)
 
 		// Parent ref is not to a Gateway that we control: skip it
 		if !isRelevantParentRef {

diff --git a/internal/gatewayapi/testdata/httproute-with-multi-gateways-notmatch.in.yaml b/internal/gatewayapi/testdata/httproute-with-multi-gateways-notmatch.in.yaml
@@ -0,0 +1,45 @@
+gateways:
+  - apiVersion: gateway.networking.k8s.io/v1
+    kind: Gateway
+    metadata:
+      namespace: envoy-gateway
+      name: gateway-1
+    spec:
+      gatewayClassName: envoy-gateway-class
+      listeners:
+        - name: http
+          protocol: HTTP
+          port: 80
+          allowedRoutes:
+            namespaces:
+              from: All
+  - apiVersion: gateway.networking.k8s.io/v1
+    kind: Gateway
+    metadata:
+      namespace: envoy-gateway
+      name: gateway-2
+    spec:
+      gatewayClassName: envoy-gateway-class
+      listeners:
+        - name: http
+          protocol: HTTP
+          port: 80
+          allowedRoutes:
+            namespaces:
+              from: All
+httpRoutes:
+  - apiVersion: gateway.networking.k8s.io/v1
+    kind: HTTPRoute
+    metadata:
+      namespace: default
+      name: httproute-1
+    spec:
+      parentRefs:
+        - name: gateway-1
+      rules:
+        - matches:
+            - path:
+                value: "/"
+          backendRefs:
+            - name: service-1
+              port: 8080
diff --git a/internal/gatewayapi/testdata/httproute-with-multi-gateways-notmatch.out.yaml b/internal/gatewayapi/testdata/httproute-with-multi-gateways-notmatch.out.yaml
@@ -0,0 +1,151 @@
+gateways:
+- apiVersion: gateway.networking.k8s.io/v1
+  kind: Gateway
+  metadata:
+    creationTimestamp: null
+    name: gateway-1
+    namespace: envoy-gateway
+  spec:
+    gatewayClassName: envoy-gateway-class
+    listeners:
+    - allowedRoutes:
+        namespaces:
+          from: All
+      name: http
+      port: 80
+      protocol: HTTP
+  status:
+    listeners:
+    - attachedRoutes: 0
+      conditions:
+      - lastTransitionTime: null
+        message: Sending translated listener configuration to the data plane
+        reason: Programmed
+        status: "True"
+        type: Programmed
+      - lastTransitionTime: null
+        message: Listener has been successfully translated
+        reason: Accepted
+        status: "True"
+        type: Accepted
+      - lastTransitionTime: null
+        message: Listener references have been resolved
+        reason: ResolvedRefs
+        status: "True"
+        type: ResolvedRefs
+      name: http
+      supportedKinds:
+      - group: gateway.networking.k8s.io
+        kind: HTTPRoute
+      - group: gateway.networking.k8s.io
+        kind: GRPCRoute
+- apiVersion: gateway.networking.k8s.io/v1
+  kind: Gateway
+  metadata:
+    creationTimestamp: null
+    name: gateway-2
+    namespace: envoy-gateway
+  spec:
+    gatewayClassName: envoy-gateway-class
+    listeners:
+    - allowedRoutes:
+        namespaces:
+          from: All
+      name: http
+      port: 80
+      protocol: HTTP
+  status:
+    listeners:
+    - attachedRoutes: 0
+      conditions:
+      - lastTransitionTime: null
+        message: Sending translated listener configuration to the data plane
+        reason: Programmed
+        status: "True"
+        type: Programmed
+      - lastTransitionTime: null
+        message: Listener has been successfully translated
+        reason: Accepted
+        status: "True"
+        type: Accepted
+      - lastTransitionTime: null
+        message: Listener references have been resolved
+        reason: ResolvedRefs
+        status: "True"
+        type: ResolvedRefs
+      name: http
+      supportedKinds:
+      - group: gateway.networking.k8s.io
+        kind: HTTPRoute
+      - group: gateway.networking.k8s.io
+        kind: GRPCRoute
+infraIR:
+  envoy-gateway/gateway-1:
+    proxy:
+      listeners:
+      - address: null
+        name: envoy-gateway/gateway-1/http
+        ports:
+        - containerPort: 10080
+          name: http-80
+          protocol: HTTP
+          servicePort: 80
+      metadata:
+        labels:
+          gateway.envoyproxy.io/owning-gateway-name: gateway-1
+          gateway.envoyproxy.io/owning-gateway-namespace: envoy-gateway
+      name: envoy-gateway/gateway-1
+  envoy-gateway/gateway-2:
+    proxy:
+      listeners:
+      - address: null
+        name: envoy-gateway/gateway-2/http
+        ports:
+        - containerPort: 10080
+          name: http-80
+          protocol: HTTP
+          servicePort: 80
+      metadata:
+        labels:
+          gateway.envoyproxy.io/owning-gateway-name: gateway-2
+          gateway.envoyproxy.io/owning-gateway-namespace: envoy-gateway
+      name: envoy-gateway/gateway-2
+xdsIR:
+  envoy-gateway/gateway-1:
+    accessLog:
+      text:
+      - path: /dev/stdout
+    http:
+    - address: 0.0.0.0
+      hostnames:
+      - '*'
+      isHTTP2: false
+      metadata:
+        kind: Gateway
+        name: gateway-1
+        namespace: envoy-gateway
+        sectionName: http
+      name: envoy-gateway/gateway-1/http
+      path:
+        escapedSlashesAction: UnescapeAndRedirect
+        mergeSlashes: true
+      port: 10080
+  envoy-gateway/gateway-2:
+    accessLog:
+      text:
+      - path: /dev/stdout
+    http:
+    - address: 0.0.0.0
+      hostnames:
+      - '*'
+      isHTTP2: false
+      metadata:
+        kind: Gateway
+        name: gateway-2
+        namespace: envoy-gateway
+        sectionName: http
+      name: envoy-gateway/gateway-2/http
+      path:
+        escapedSlashesAction: UnescapeAndRedirect
+        mergeSlashes: true
+      port: 10080
diff --git a/internal/gatewayapi/testdata/httproute-with-multi-gateways-with-same-name.in.yaml b/internal/gatewayapi/testdata/httproute-with-multi-gateways-with-same-name.in.yaml
@@ -0,0 +1,45 @@
+gateways:
+  - apiVersion: gateway.networking.k8s.io/v1
+    kind: Gateway
+    metadata:
+      namespace: envoy-gateway
+      name: gateway-1
+    spec:
+      gatewayClassName: envoy-gateway-class
+      listeners:
+        - name: http
+          protocol: HTTP
+          port: 80
+          allowedRoutes:
+            namespaces:
+              from: All
+  - apiVersion: gateway.networking.k8s.io/v1
+    kind: Gateway
+    metadata:
+      namespace: default
+      name: gateway-1
+    spec:
+      gatewayClassName: envoy-gateway-class
+      listeners:
+        - name: http
+          protocol: HTTP
+          port: 80
+          allowedRoutes:
+            namespaces:
+              from: All
+httpRoutes:
+  - apiVersion: gateway.networking.k8s.io/v1
+    kind: HTTPRoute
+    metadata:
+      namespace: default
+      name: httproute-1
+    spec:
+      parentRefs:
+        - name: gateway-1
+      rules:
+        - matches:
+            - path:
+                value: "/"
+          backendRefs:
+            - name: service-1
+              port: 8080