diff --git a/internal/gatewayapi/backendtrafficpolicy.go b/internal/gatewayapi/backendtrafficpolicy.go index bda586ec09..4e96f1a8af 100644 --- a/internal/gatewayapi/backendtrafficpolicy.go +++ b/internal/gatewayapi/backendtrafficpolicy.go @@ -671,7 +671,7 @@ func (t *Translator) translateBackendTrafficPolicyForRouteWithMerge( policyTargetGatewayNN types.NamespacedName, policyTargetListener *gwapiv1.SectionName, route RouteContext, xdsIR resource.XdsIRMap, ) error { - mergedPolicy, err := mergeBackendTrafficPolicy(policy, parentPolicy) + mergedPolicy, err := t.mergeBackendTrafficPolicy(policy, parentPolicy) if err != nil { return fmt.Errorf("error merging policies: %w", err) } @@ -842,14 +842,24 @@ func (t *Translator) applyTrafficFeatureToRoute(route RouteContext, } } -func mergeBackendTrafficPolicy(routePolicy, gwPolicy *egv1a1.BackendTrafficPolicy) (*egv1a1.BackendTrafficPolicy, error) { +// mergeBackendTrafficPolicy merges route policy into gateway policy. +func (t *Translator) mergeBackendTrafficPolicy(routePolicy, gwPolicy *egv1a1.BackendTrafficPolicy) (*egv1a1.BackendTrafficPolicy, error) { if routePolicy.Spec.MergeType == nil || gwPolicy == nil { return routePolicy, nil } + // Resolve LocalObjectReferences to inline content in the policies before merge so the merge operates on concrete values. + if err := t.resolveLocalObjectRefsInPolicy(gwPolicy); err != nil { + return nil, err + } + if err := t.resolveLocalObjectRefsInPolicy(routePolicy); err != nil { + return nil, err + } + return utils.Merge(gwPolicy, routePolicy, *routePolicy.Spec.MergeType) } +// buildTrafficFeatures builds IR traffic features from a BackendTrafficPolicy. func (t *Translator) buildTrafficFeatures(policy *egv1a1.BackendTrafficPolicy) (*ir.TrafficFeatures, error) { var ( rl *ir.RateLimit @@ -1684,10 +1694,10 @@ func (t *Translator) getCustomResponseBody( return binData, nil } default: - return nil, fmt.Errorf("can't find the key %s in the referenced configmap %s", ResponseBodyConfigMapKey, body.ValueRef.Name) + return nil, fmt.Errorf("can't find the key %s in the referenced configmap %s/%s", ResponseBodyConfigMapKey, policyNs, body.ValueRef.Name) } } else { - return nil, fmt.Errorf("can't find the referenced configmap %s", body.ValueRef.Name) + return nil, fmt.Errorf("can't find the referenced configmap %s/%s", policyNs, body.ValueRef.Name) } } else if body.Inline != nil { inlineValue := []byte(*body.Inline) @@ -1697,6 +1707,47 @@ func (t *Translator) getCustomResponseBody( return nil, nil } +// resolveCustomResponseBodyRefToInline resolves a ValueRef in body to inline content using the given namespace. +// It mutates body in place: replaces Type and ValueRef with Inline content. No-op if body is nil or already Inline. +func (t *Translator) resolveCustomResponseBodyRefToInline(body *egv1a1.CustomResponseBody, policyNs string) error { + if body == nil { + return nil + } + if body.Type == nil || *body.Type != egv1a1.ResponseValueTypeValueRef || body.ValueRef == nil { + return nil + } + data, err := t.getCustomResponseBody(body, policyNs) + if err != nil { + return err + } + inlineStr := string(data) + t.Logger.Info("resolved custom response body ref to inline before merge", + "namespace", policyNs, + "ref", body.ValueRef.Name, + ) + body.Type = ptr.To(egv1a1.ResponseValueTypeInline) + body.Inline = &inlineStr + body.ValueRef = nil + return nil +} + +// resolveLocalObjectRefsInPolicy resolves LocalObjectReferences to inline content in the given policy (mutates in place). +// Currently handles ResponseOverride body ValueRefs; may be extended for other refs BackendTrafficPolicy supports. +func (t *Translator) resolveLocalObjectRefsInPolicy(policy *egv1a1.BackendTrafficPolicy) error { + if policy == nil || len(policy.Spec.ResponseOverride) == 0 { + return nil + } + policyNs := policy.Namespace + for _, ro := range policy.Spec.ResponseOverride { + if ro != nil && ro.Response != nil && ro.Response.Body != nil { + if err := t.resolveCustomResponseBodyRefToInline(ro.Response.Body, policyNs); err != nil { + return err + } + } + } + return nil +} + func defaultResponseOverrideRuleName(policy *egv1a1.BackendTrafficPolicy, index int) string { return fmt.Sprintf( "%s/responseoverride/rule/%s", diff --git a/internal/gatewayapi/filters.go b/internal/gatewayapi/filters.go index a642b37a0a..b02eff9315 100644 --- a/internal/gatewayapi/filters.go +++ b/internal/gatewayapi/filters.go @@ -893,9 +893,8 @@ func (t *Translator) processExtensionRefHTTPFilter(extFilter *gwapiv1.LocalObjec if hrf.Spec.DirectResponse != nil { dr := &ir.CustomResponse{} if hrf.Spec.DirectResponse.Body != nil { - body := hrf.Spec.DirectResponse.Body var err error - if dr.Body, err = t.getCustomResponseBody(body, filterNs); err != nil { + if dr.Body, err = t.getCustomResponseBody(hrf.Spec.DirectResponse.Body, filterNs); err != nil { return t.processInvalidHTTPFilter(string(extFilter.Kind), filterContext, err) } } diff --git a/internal/gatewayapi/testdata/backendtrafficpolicy-with-response-override-invalid-valueref.out.yaml b/internal/gatewayapi/testdata/backendtrafficpolicy-with-response-override-invalid-valueref.out.yaml index e2aad9097e..ee1660c69b 100644 --- a/internal/gatewayapi/testdata/backendtrafficpolicy-with-response-override-invalid-valueref.out.yaml +++ b/internal/gatewayapi/testdata/backendtrafficpolicy-with-response-override-invalid-valueref.out.yaml @@ -45,7 +45,7 @@ backendTrafficPolicies: sectionName: http conditions: - lastTransitionTime: null - message: 'ResponseOverride: can''t find the referenced configmap response-override-config-1.' + message: 'ResponseOverride: can''t find the referenced configmap default/response-override-config-1.' reason: Invalid status: "False" type: Accepted @@ -101,7 +101,7 @@ backendTrafficPolicies: conditions: - lastTransitionTime: null message: 'ResponseOverride: can''t find the key response.body in the referenced - configmap response-override-config.' + configmap default/response-override-config.' reason: Invalid status: "False" type: Accepted diff --git a/internal/gatewayapi/testdata/backendtrafficpolicy-with-response-override-merged-diff-namespace.in.yaml b/internal/gatewayapi/testdata/backendtrafficpolicy-with-response-override-merged-diff-namespace.in.yaml new file mode 100644 index 0000000000..d149d18bcb --- /dev/null +++ b/internal/gatewayapi/testdata/backendtrafficpolicy-with-response-override-merged-diff-namespace.in.yaml @@ -0,0 +1,100 @@ +# Tests merged BackendTrafficPolicies when ResponseOverride ConfigMap lives in the +# gateway (parent) policy's namespace. Gateway BTP is in envoy-gateway-system with +# a ConfigMap reference; route BTP is in default with mergeType. After merge, the +# ConfigMap must be resolved in the parent policy's namespace. +gateways: + - apiVersion: gateway.networking.k8s.io/v1 + kind: Gateway + metadata: + namespace: envoy-gateway-system + name: my-gateway + spec: + gatewayClassName: envoy-gateway-class + listeners: + - name: http + protocol: HTTP + port: 80 + allowedRoutes: + namespaces: + from: All +configMaps: + - apiVersion: v1 + kind: ConfigMap + metadata: + name: error-page + namespace: envoy-gateway-system + data: + response.body: | + error page contents here +backendTrafficPolicies: + - apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: BackendTrafficPolicy + metadata: + name: my-gateway-error-response + namespace: envoy-gateway-system + spec: + targetRefs: + - group: gateway.networking.k8s.io + kind: Gateway + name: my-gateway + responseOverride: + - match: + statusCodes: + - type: Range + range: + start: 502 + end: 504 + response: + contentType: "text/html" + body: + type: ValueRef + valueRef: + group: "" + kind: ConfigMap + name: error-page + - apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: BackendTrafficPolicy + metadata: + name: my-app-rate-limit + namespace: default + spec: + targetRefs: + - group: gateway.networking.k8s.io + kind: HTTPRoute + name: my-app + mergeType: StrategicMerge + rateLimit: + local: + rules: + - clientSelectors: + - sourceCIDR: + type: Distinct + value: 0.0.0.0/0 + limit: + requests: 200 + unit: Minute +httpRoutes: + - apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + name: my-app + namespace: default + spec: + hostnames: + - myapp.example.com + parentRefs: + - group: gateway.networking.k8s.io + kind: Gateway + name: my-gateway + namespace: envoy-gateway-system + rules: + - backendRefs: + - group: "" + kind: Service + name: service-1 + port: 8080 + weight: 1 + matches: + - path: + type: PathPrefix + value: / diff --git a/internal/gatewayapi/testdata/backendtrafficpolicy-with-response-override-merged-diff-namespace.out.yaml b/internal/gatewayapi/testdata/backendtrafficpolicy-with-response-override-merged-diff-namespace.out.yaml new file mode 100644 index 0000000000..0394ddcb8a --- /dev/null +++ b/internal/gatewayapi/testdata/backendtrafficpolicy-with-response-override-merged-diff-namespace.out.yaml @@ -0,0 +1,307 @@ +backendTrafficPolicies: +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: BackendTrafficPolicy + metadata: + name: my-app-rate-limit + namespace: default + spec: + mergeType: StrategicMerge + rateLimit: + local: + rules: + - clientSelectors: + - sourceCIDR: + type: Distinct + value: 0.0.0.0/0 + limit: + requests: 200 + unit: Minute + targetRefs: + - group: gateway.networking.k8s.io + kind: HTTPRoute + name: my-app + status: + ancestors: + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: my-gateway + namespace: envoy-gateway-system + sectionName: http + conditions: + - lastTransitionTime: null + message: Merged with policy envoy-gateway-system/my-gateway-error-response + reason: Merged + status: "True" + type: Merged + controllerName: gateway.envoyproxy.io/gatewayclass-controller + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: my-gateway + namespace: envoy-gateway-system + conditions: + - lastTransitionTime: null + message: Policy has been accepted. + reason: Accepted + status: "True" + type: Accepted + controllerName: gateway.envoyproxy.io/gatewayclass-controller +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: BackendTrafficPolicy + metadata: + name: my-gateway-error-response + namespace: envoy-gateway-system + spec: + responseOverride: + - match: + statusCodes: + - range: + end: 504 + start: 502 + type: Range + response: + body: + inline: | + error page contents here + type: Inline + contentType: text/html + targetRefs: + - group: gateway.networking.k8s.io + kind: Gateway + name: my-gateway + status: + ancestors: + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: my-gateway + namespace: envoy-gateway-system + conditions: + - lastTransitionTime: null + message: Policy has been accepted. + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: 'This policy is being merged by other backendTrafficPolicies for + these routes: [default/my-app]' + reason: Merged + status: "True" + type: Merged + controllerName: gateway.envoyproxy.io/gatewayclass-controller +gateways: +- apiVersion: gateway.networking.k8s.io/v1 + kind: Gateway + metadata: + name: my-gateway + namespace: envoy-gateway-system + spec: + gatewayClassName: envoy-gateway-class + listeners: + - allowedRoutes: + namespaces: + from: All + 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 +httpRoutes: +- apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + name: my-app + namespace: default + spec: + hostnames: + - myapp.example.com + parentRefs: + - group: gateway.networking.k8s.io + kind: Gateway + name: my-gateway + namespace: envoy-gateway-system + rules: + - backendRefs: + - group: "" + kind: Service + name: service-1 + port: 8080 + weight: 1 + matches: + - path: + type: PathPrefix + 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: + group: gateway.networking.k8s.io + kind: Gateway + name: my-gateway + namespace: envoy-gateway-system +infraIR: + envoy-gateway-system/my-gateway: + proxy: + listeners: + - name: envoy-gateway-system/my-gateway/http + ports: + - containerPort: 10080 + name: http-80 + protocol: HTTP + servicePort: 80 + metadata: + labels: + gateway.envoyproxy.io/owning-gateway-name: my-gateway + gateway.envoyproxy.io/owning-gateway-namespace: envoy-gateway-system + ownerReference: + kind: GatewayClass + name: envoy-gateway-class + name: envoy-gateway-system/my-gateway + namespace: envoy-gateway-system +xdsIR: + envoy-gateway-system/my-gateway: + accessLog: + json: + - path: /dev/stdout + globalResources: + proxyServiceCluster: + metadata: + kind: Service + name: envoy-envoy-gateway-system-my-gateway-1d35c6f5 + namespace: envoy-gateway-system + sectionName: "8080" + name: envoy-gateway-system/my-gateway + settings: + - addressType: IP + endpoints: + - host: 7.6.5.4 + port: 8080 + zone: zone1 + metadata: + kind: Service + name: envoy-envoy-gateway-system-my-gateway-1d35c6f5 + namespace: envoy-gateway-system + sectionName: "8080" + name: envoy-gateway-system/my-gateway + protocol: TCP + http: + - address: 0.0.0.0 + externalPort: 80 + hostnames: + - '*' + isHTTP2: false + metadata: + kind: Gateway + name: my-gateway + namespace: envoy-gateway-system + sectionName: http + name: envoy-gateway-system/my-gateway/http + path: + escapedSlashesAction: UnescapeAndRedirect + mergeSlashes: true + port: 10080 + routes: + - destination: + metadata: + kind: HTTPRoute + name: my-app + namespace: default + name: httproute/default/my-app/rule/0 + settings: + - addressType: IP + endpoints: + - host: 7.7.7.7 + port: 8080 + metadata: + kind: Service + name: service-1 + namespace: default + sectionName: "8080" + name: httproute/default/my-app/rule/0/backend/0 + protocol: HTTP + weight: 1 + hostname: myapp.example.com + isHTTP2: false + metadata: + kind: HTTPRoute + name: my-app + namespace: default + policies: + - kind: BackendTrafficPolicy + name: my-app-rate-limit + namespace: default + name: httproute/default/my-app/rule/0/match/0/myapp_example_com + pathMatch: + distinct: false + name: "" + prefix: / + traffic: + rateLimit: + local: + default: + requests: 4294967295 + unit: Second + rules: + - cidrMatch: + cidr: 0.0.0.0/0 + distinct: true + ip: 0.0.0.0 + isIPv6: false + maskLen: 0 + headerMatches: [] + limit: + requests: 200 + unit: Minute + name: default/my-app-rate-limit/rule/0 + responseOverride: + name: backendtrafficpolicy/default/my-app-rate-limit + rules: + - match: + statusCodes: + - range: + end: 504 + start: 502 + name: backendtrafficpolicy/default/my-app-rate-limit/responseoverride/rule/0 + response: + body: ZXJyb3IgcGFnZSBjb250ZW50cyBoZXJlCg== + contentType: text/html + readyListener: + address: 0.0.0.0 + ipFamily: IPv4 + path: /ready + port: 19003 diff --git a/release-notes/current.yaml b/release-notes/current.yaml index c88e8bf3a2..8d3934d170 100644 --- a/release-notes/current.yaml +++ b/release-notes/current.yaml @@ -21,6 +21,7 @@ bug fixes: | Fixed validation of XListenerSet certificateRefs Fixed XListenerSet not allowing xRoutes from the same namespace when configured to allow them Fixed API key authentication dropping non-first client IDs when credential Secrets contain multiple keys. + Fixed local object reference resolution from parent policy in merged BackendTrafficPolicies. # Enhancements that improve performance. performance improvements: |