From e1b2a7134f36abaf6dd3b12138aaf80bac93e37a Mon Sep 17 00:00:00 2001 From: Lior Okman Date: Mon, 26 Feb 2024 22:13:58 +0200 Subject: [PATCH] fix: Policies should apply only to gateways they were attached to when mergeGateways is true (#2671) * When policies are attached to a gateway and mergeGateways is set to true, don't apply policies to routes from other gateways. Signed-off-by: Lior Okman * Updated the tests Signed-off-by: Lior Okman * Update test to include all of the policy types Signed-off-by: Lior Okman * Calculate the correct gatewayname as a prefix of the listener's name Signed-off-by: Lior Okman * Make the linter happy Signed-off-by: Lior Okman --------- Signed-off-by: Lior Okman --- internal/gatewayapi/backendtrafficpolicy.go | 8 + internal/gatewayapi/securitypolicy.go | 11 +- .../merge-with-isolated-policies.in.yaml | 132 +++++++ .../merge-with-isolated-policies.out.yaml | 357 ++++++++++++++++++ 4 files changed, 507 insertions(+), 1 deletion(-) create mode 100644 internal/gatewayapi/testdata/merge-with-isolated-policies.in.yaml create mode 100644 internal/gatewayapi/testdata/merge-with-isolated-policies.out.yaml diff --git a/internal/gatewayapi/backendtrafficpolicy.go b/internal/gatewayapi/backendtrafficpolicy.go index 44a07a34151..c3e4cec23d6 100644 --- a/internal/gatewayapi/backendtrafficpolicy.go +++ b/internal/gatewayapi/backendtrafficpolicy.go @@ -352,7 +352,15 @@ func (t *Translator) translateBackendTrafficPolicyForGateway(policy *egv1a1.Back // Should exist since we've validated this ir := xdsIR[irKey] + policyTarget := irStringKey( + string(ptr.Deref(policy.Spec.TargetRef.Namespace, gwv1a2.Namespace(policy.Namespace))), + string(policy.Spec.TargetRef.Name), + ) for _, http := range ir.HTTP { + gatewayName := http.Name[0:strings.LastIndex(http.Name, "/")] + if t.MergeGateways && gatewayName != policyTarget { + continue + } for _, r := range http.Routes { // Apply if not already set if r.RateLimit == nil { diff --git a/internal/gatewayapi/securitypolicy.go b/internal/gatewayapi/securitypolicy.go index abb274e6b08..636b7316e11 100644 --- a/internal/gatewayapi/securitypolicy.go +++ b/internal/gatewayapi/securitypolicy.go @@ -360,7 +360,8 @@ func (t *Translator) translateSecurityPolicyForGateway( } } - // Apply IR to all the routes within the specific Gateway + // Apply IR to all the routes within the specific Gateway that originated + // from the gateway to which this security policy was attached. // If the feature is already set, then skip it, since it must have be // set by a policy attaching to the route // @@ -370,7 +371,15 @@ func (t *Translator) translateSecurityPolicyForGateway( // Should exist since we've validated this ir := xdsIR[irKey] + policyTarget := irStringKey( + string(ptr.Deref(policy.Spec.TargetRef.Namespace, gwv1a2.Namespace(policy.Namespace))), + string(policy.Spec.TargetRef.Name), + ) for _, http := range ir.HTTP { + gatewayName := http.Name[0:strings.LastIndex(http.Name, "/")] + if t.MergeGateways && gatewayName != policyTarget { + continue + } for _, r := range http.Routes { // Apply if not already set if r.CORS == nil { diff --git a/internal/gatewayapi/testdata/merge-with-isolated-policies.in.yaml b/internal/gatewayapi/testdata/merge-with-isolated-policies.in.yaml new file mode 100644 index 00000000000..2ce31b166c6 --- /dev/null +++ b/internal/gatewayapi/testdata/merge-with-isolated-policies.in.yaml @@ -0,0 +1,132 @@ +envoyproxy: + apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: EnvoyProxy + metadata: + namespace: envoy-gateway-system + name: test + spec: + mergeGateways: true +gateways: + - apiVersion: gateway.networking.k8s.io/v1beta1 + kind: Gateway + metadata: + name: gateway-1 + namespace: default + spec: + gatewayClassName: envoy-gateway-class + listeners: + - name: http + port: 80 + protocol: HTTP + allowedRoutes: + namespaces: + from: Same + - apiVersion: gateway.networking.k8s.io/v1beta1 + kind: Gateway + metadata: + name: gateway-2 + namespace: default + spec: + gatewayClassName: envoy-gateway-class + listeners: + - name: http-2 + port: 8888 + protocol: HTTP + allowedRoutes: + namespaces: + from: Same +httpRoutes: + - apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + namespace: default + name: httproute-1 + spec: + hostnames: + - gateway.envoyproxy.io + parentRefs: + - namespace: default + name: gateway-1 + 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-2 + spec: + hostnames: + - gateway.envoyproxy.io + 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 + metadata: + namespace: default + name: policy-for-route-2 + spec: + targetRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: default + cors: + allowOrigins: + - "*" + allowMethods: + - GET + - POST + allowHeaders: + - "x-header-5" + - "x-header-6" + exposeHeaders: + - "x-header-7" + - "x-header-8" + maxAge: 2000s +clientTrafficPolicies: + - apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: ClientTrafficPolicy + metadata: + namespace: default + name: target-gateway + spec: + targetRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-2 + namespace: default + timeout: + http: + requestReceivedTimeout: "5s" +backendTrafficPolicies: + - apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: BackendTrafficPolicy + metadata: + namespace: default + name: policy-for-gateway + spec: + targetRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: default + tcpKeepalive: + probes: 3 + idleTime: 20m + interval: 60s diff --git a/internal/gatewayapi/testdata/merge-with-isolated-policies.out.yaml b/internal/gatewayapi/testdata/merge-with-isolated-policies.out.yaml new file mode 100644 index 00000000000..a23c58be9b6 --- /dev/null +++ b/internal/gatewayapi/testdata/merge-with-isolated-policies.out.yaml @@ -0,0 +1,357 @@ +backendTrafficPolicies: +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: BackendTrafficPolicy + metadata: + creationTimestamp: null + name: policy-for-gateway + namespace: default + spec: + targetRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: default + tcpKeepalive: + idleTime: 20m + interval: 60s + probes: 3 + status: + conditions: + - lastTransitionTime: null + message: BackendTrafficPolicy has been accepted. + reason: Accepted + status: "True" + type: Accepted +clientTrafficPolicies: +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: ClientTrafficPolicy + metadata: + creationTimestamp: null + name: target-gateway + namespace: default + spec: + targetRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-2 + namespace: default + timeout: + http: + requestReceivedTimeout: 5s + status: + conditions: + - lastTransitionTime: null + message: ClientTrafficPolicy has been accepted. + reason: Accepted + status: "True" + type: Accepted +gateways: +- apiVersion: gateway.networking.k8s.io/v1beta1 + kind: Gateway + metadata: + creationTimestamp: null + name: gateway-1 + namespace: default + spec: + gatewayClassName: envoy-gateway-class + listeners: + - allowedRoutes: + namespaces: + from: Same + name: http + port: 80 + protocol: HTTP + status: + listeners: + - attachedRoutes: 1 + 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/v1beta1 + kind: Gateway + metadata: + creationTimestamp: null + name: gateway-2 + namespace: default + spec: + gatewayClassName: envoy-gateway-class + listeners: + - allowedRoutes: + namespaces: + from: Same + name: http-2 + port: 8888 + protocol: HTTP + status: + listeners: + - attachedRoutes: 1 + 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-2 + supportedKinds: + - group: gateway.networking.k8s.io + kind: HTTPRoute + - group: gateway.networking.k8s.io + kind: GRPCRoute +httpRoutes: +- apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + creationTimestamp: null + name: httproute-1 + namespace: default + spec: + hostnames: + - gateway.envoyproxy.io + parentRefs: + - name: gateway-1 + namespace: default + sectionName: http + rules: + - backendRefs: + - name: service-1 + port: 8080 + matches: + - path: + value: / + status: + parents: + - conditions: + - lastTransitionTime: null + message: Route is accepted + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: Resolved all the Object references for the Route + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + controllerName: gateway.envoyproxy.io/gatewayclass-controller + parentRef: + name: gateway-1 + namespace: default + sectionName: http +- apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + creationTimestamp: null + name: httproute-2 + namespace: default + spec: + hostnames: + - gateway.envoyproxy.io + parentRefs: + - name: gateway-2 + namespace: default + sectionName: http-2 + rules: + - backendRefs: + - name: service-2 + port: 8080 + matches: + - path: + value: / + status: + parents: + - conditions: + - lastTransitionTime: null + message: Route is accepted + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: Resolved all the Object references for the Route + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + controllerName: gateway.envoyproxy.io/gatewayclass-controller + parentRef: + name: gateway-2 + namespace: default + sectionName: http-2 +infraIR: + envoy-gateway-class: + proxy: + config: + apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: EnvoyProxy + metadata: + creationTimestamp: null + name: test + namespace: envoy-gateway-system + spec: + logging: {} + mergeGateways: true + status: {} + listeners: + - address: null + name: default/gateway-1/http + ports: + - containerPort: 10080 + name: default/gateway-1/http + protocol: HTTP + servicePort: 80 + - address: null + name: default/gateway-2/http-2 + ports: + - containerPort: 8888 + name: default/gateway-2/http-2 + protocol: HTTP + servicePort: 8888 + metadata: + labels: + gateway.envoyproxy.io/owning-gatewayclass: envoy-gateway-class + name: envoy-gateway-class +securityPolicies: +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: SecurityPolicy + metadata: + creationTimestamp: null + name: policy-for-route-2 + namespace: default + spec: + cors: + allowHeaders: + - x-header-5 + - x-header-6 + allowMethods: + - GET + - POST + allowOrigins: + - '*' + exposeHeaders: + - x-header-7 + - x-header-8 + maxAge: 33m20s + targetRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: default + status: + conditions: + - lastTransitionTime: null + message: SecurityPolicy has been accepted. + reason: Accepted + status: "True" + type: Accepted +xdsIR: + envoy-gateway-class: + accessLog: + text: + - path: /dev/stdout + http: + - address: 0.0.0.0 + hostnames: + - '*' + isHTTP2: false + name: default/gateway-1/http + path: + escapedSlashesAction: UnescapeAndRedirect + mergeSlashes: true + port: 10080 + routes: + - backendWeights: + invalid: 0 + valid: 0 + cors: + allowHeaders: + - x-header-5 + - x-header-6 + allowMethods: + - GET + - POST + allowOrigins: + - distinct: false + name: "" + safeRegex: .* + exposeHeaders: + - x-header-7 + - x-header-8 + maxAge: 33m20s + destination: + name: httproute/default/httproute-1/rule/0 + settings: + - addressType: IP + endpoints: + - host: 7.7.7.7 + port: 8080 + protocol: HTTP + weight: 1 + hostname: gateway.envoyproxy.io + isHTTP2: false + name: httproute/default/httproute-1/rule/0/match/0/gateway_envoyproxy_io + pathMatch: + distinct: false + name: "" + prefix: / + tcpKeepalive: + idleTime: 1200 + interval: 60 + probes: 3 + - address: 0.0.0.0 + hostnames: + - '*' + isHTTP2: false + name: default/gateway-2/http-2 + path: + escapedSlashesAction: UnescapeAndRedirect + mergeSlashes: true + port: 8888 + routes: + - backendWeights: + invalid: 0 + valid: 0 + destination: + name: httproute/default/httproute-2/rule/0 + settings: + - addressType: IP + endpoints: + - host: 7.7.7.7 + port: 8080 + protocol: HTTP + weight: 1 + hostname: gateway.envoyproxy.io + isHTTP2: false + name: httproute/default/httproute-2/rule/0/match/0/gateway_envoyproxy_io + pathMatch: + distinct: false + name: "" + prefix: / + timeout: + http: + requestReceivedTimeout: 5s