diff --git a/api/v1alpha1/ext_auth_types.go b/api/v1alpha1/ext_auth_types.go index 0670ed4b676..faa0897e300 100644 --- a/api/v1alpha1/ext_auth_types.go +++ b/api/v1alpha1/ext_auth_types.go @@ -56,7 +56,6 @@ type ExtAuth struct { // The authorization request message is defined in // https://www.envoyproxy.io/docs/envoy/latest/api-v3/service/auth/v3/external_auth.proto // +kubebuilder:validation:XValidation:message="backendRef or backendRefs needs to be set",rule="has(self.backendRef) || self.backendRefs.size() > 0" -// +kubebuilder:validation:XValidation:message="BackendRefs must be used, backendRef is not supported.",rule="!has(self.backendRef)" // +kubebuilder:validation:XValidation:message="BackendRefs only supports Service and Backend kind.",rule="has(self.backendRefs) ? self.backendRefs.all(f, f.kind == 'Service' || f.kind == 'Backend') : true" // +kubebuilder:validation:XValidation:message="BackendRefs only supports Core and gateway.envoyproxy.io group.",rule="has(self.backendRefs) ? (self.backendRefs.all(f, f.group == \"\" || f.group == 'gateway.envoyproxy.io')) : true" type GRPCExtAuthService struct { @@ -67,7 +66,6 @@ type GRPCExtAuthService struct { // HTTPExtAuthService defines the HTTP External Authorization service // // +kubebuilder:validation:XValidation:message="backendRef or backendRefs needs to be set",rule="has(self.backendRef) || self.backendRefs.size() > 0" -// +kubebuilder:validation:XValidation:message="BackendRefs must be used, backendRef is not supported.",rule="!has(self.backendRef)" // +kubebuilder:validation:XValidation:message="BackendRefs only supports Service and Backend kind.",rule="has(self.backendRefs) ? self.backendRefs.all(f, f.kind == 'Service' || f.kind == 'Backend') : true" // +kubebuilder:validation:XValidation:message="BackendRefs only supports Core and gateway.envoyproxy.io group.",rule="has(self.backendRefs) ? (self.backendRefs.all(f, f.group == \"\" || f.group == 'gateway.envoyproxy.io')) : true" type HTTPExtAuthService struct { diff --git a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml index b6a040f8c42..840c8d59d30 100644 --- a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml +++ b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml @@ -1204,8 +1204,6 @@ spec: x-kubernetes-validations: - message: backendRef or backendRefs needs to be set rule: has(self.backendRef) || self.backendRefs.size() > 0 - - message: BackendRefs must be used, backendRef is not supported. - rule: '!has(self.backendRef)' - message: BackendRefs only supports Service and Backend kind. rule: 'has(self.backendRefs) ? self.backendRefs.all(f, f.kind == ''Service'' || f.kind == ''Backend'') : true' @@ -2103,8 +2101,6 @@ spec: x-kubernetes-validations: - message: backendRef or backendRefs needs to be set rule: has(self.backendRef) || self.backendRefs.size() > 0 - - message: BackendRefs must be used, backendRef is not supported. - rule: '!has(self.backendRef)' - message: BackendRefs only supports Service and Backend kind. rule: 'has(self.backendRefs) ? self.backendRefs.all(f, f.kind == ''Service'' || f.kind == ''Backend'') : true' diff --git a/internal/gatewayapi/securitypolicy.go b/internal/gatewayapi/securitypolicy.go index 3c2d2af31ed..8635d216457 100644 --- a/internal/gatewayapi/securitypolicy.go +++ b/internal/gatewayapi/securitypolicy.go @@ -841,14 +841,15 @@ func (t *Translator) buildBasicAuth( func (t *Translator) buildExtAuth(policy *egv1a1.SecurityPolicy, resources *resource.Resources, envoyProxy *egv1a1.EnvoyProxy) (*ir.ExtAuth, error) { var ( - http = policy.Spec.ExtAuth.HTTP - grpc = policy.Spec.ExtAuth.GRPC - backends *egv1a1.BackendCluster - protocol ir.AppProtocol - rd *ir.RouteDestination - authority string - err error - traffic *ir.TrafficFeatures + http = policy.Spec.ExtAuth.HTTP + grpc = policy.Spec.ExtAuth.GRPC + backendRefs []egv1a1.BackendRef + backendSettings *egv1a1.ClusterSettings + protocol ir.AppProtocol + rd *ir.RouteDestination + authority string + err error + traffic *ir.TrafficFeatures ) // These are sanity checks, they should never happen because the API server @@ -861,18 +862,42 @@ func (t *Translator) buildExtAuth(policy *egv1a1.SecurityPolicy, resources *reso switch { case http != nil: - backends = &http.BackendCluster protocol = ir.HTTP + switch { + case len(http.BackendRefs) > 0: + backendRefs = http.BackendCluster.BackendRefs + case http.BackendRef != nil: + backendRefs = []egv1a1.BackendRef{ + { + BackendObjectReference: *http.BackendRef, + }, + } + default: + // This is a sanity check, it should never happen because the API server should have caught it + return nil, errors.New("http backend refs must be specified") + } case grpc != nil: - backends = &grpc.BackendCluster protocol = ir.GRPC + switch { + case len(grpc.BackendCluster.BackendRefs) > 0: + backendRefs = grpc.BackendRefs + case grpc.BackendRef != nil: + backendRefs = []egv1a1.BackendRef{ + { + BackendObjectReference: *grpc.BackendRef, + }, + } + default: + // This is a sanity check, it should never happen because the API server should have caught it + return nil, errors.New("grpc backend refs must be specified") + } } - if rd, err = t.translateExtServiceBackendRefs(policy, backends.BackendRefs, protocol, resources, envoyProxy, 0); err != nil { + if rd, err = t.translateExtServiceBackendRefs(policy, backendRefs, protocol, resources, envoyProxy, 0); err != nil { return nil, err } - for _, backendRef := range backends.BackendRefs { + for _, backendRef := range backendRefs { // Authority is the calculated hostname that will be used as the Authority header. // If there are multiple backend referenced, simply use the first one - there are no good answers here. // When translated to XDS, the authority is used on the filter level not on the cluster level. @@ -882,7 +907,7 @@ func (t *Translator) buildExtAuth(policy *egv1a1.SecurityPolicy, resources *reso } } - if traffic, err = translateTrafficFeatures(backends.BackendSettings); err != nil { + if traffic, err = translateTrafficFeatures(backendSettings); err != nil { return nil, err } extAuth := &ir.ExtAuth{ diff --git a/internal/gatewayapi/testdata/securitypolicy-with-extauth-backend.in.yaml b/internal/gatewayapi/testdata/securitypolicy-with-extauth-backend.in.yaml index 78529bf6d73..5d756b3b981 100644 --- a/internal/gatewayapi/testdata/securitypolicy-with-extauth-backend.in.yaml +++ b/internal/gatewayapi/testdata/securitypolicy-with-extauth-backend.in.yaml @@ -58,6 +58,44 @@ httpRoutes: backendRefs: - name: service-3 port: 8080 + - apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + namespace: default + name: httproute-3 + spec: + hostnames: + - www.baz.com + parentRefs: + - namespace: default + name: gateway-1 + sectionName: http + rules: + - matches: + - path: + value: /baz + backendRefs: + - name: service-4 + port: 8080 + - apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + namespace: default + name: httproute-4 + spec: + hostnames: + - www.qux.com + parentRefs: + - namespace: default + name: gateway-1 + sectionName: http + rules: + - matches: + - path: + value: /qux + backendRefs: + - name: service-5 + port: 8080 backends: - apiVersion: gateway.envoyproxy.io/v1alpha1 kind: Backend @@ -108,3 +146,40 @@ securityPolicies: kind: Backend group: gateway.envoyproxy.io port: 3000 + - apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: SecurityPolicy + metadata: + namespace: default + name: policy-for-http-route-3--grpc-backendref + spec: + targetRef: + group: gateway.networking.k8s.io + kind: HTTPRoute + name: httproute-3 + extAuth: + failOpen: true + headersToExtAuth: + - header3 + - header4 + grpc: + backendRef: + name: service-2 + kind: Service + port: 8080 + - apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: SecurityPolicy + metadata: + namespace: default + name: policy-for-http-route-3-http-backendref + spec: + targetRef: + group: gateway.networking.k8s.io + kind: HTTPRoute + name: httproute-4 + extAuth: + http: + backendRef: + name: backend-fqdn + kind: Backend + group: gateway.envoyproxy.io + port: 3000 diff --git a/internal/gatewayapi/testdata/securitypolicy-with-extauth-backend.out.yaml b/internal/gatewayapi/testdata/securitypolicy-with-extauth-backend.out.yaml index 05086bae4c8..d304f6c13eb 100644 --- a/internal/gatewayapi/testdata/securitypolicy-with-extauth-backend.out.yaml +++ b/internal/gatewayapi/testdata/securitypolicy-with-extauth-backend.out.yaml @@ -35,7 +35,7 @@ gateways: protocol: HTTP status: listeners: - - attachedRoutes: 2 + - attachedRoutes: 4 conditions: - lastTransitionTime: null message: Sending translated listener configuration to the data plane @@ -141,6 +141,82 @@ httpRoutes: name: gateway-1 namespace: default sectionName: http +- apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + creationTimestamp: null + name: httproute-3 + namespace: default + spec: + hostnames: + - www.baz.com + parentRefs: + - name: gateway-1 + namespace: default + sectionName: http + rules: + - backendRefs: + - name: service-4 + port: 8080 + matches: + - path: + value: /baz + 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-4 + namespace: default + spec: + hostnames: + - www.qux.com + parentRefs: + - name: gateway-1 + namespace: default + sectionName: http + rules: + - backendRefs: + - name: service-5 + port: 8080 + matches: + - path: + value: /qux + status: + parents: + - conditions: + - lastTransitionTime: null + message: Route is accepted + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: Service default/service-5 not found + reason: BackendNotFound + status: "False" + type: ResolvedRefs + controllerName: gateway.envoyproxy.io/gatewayclass-controller + parentRef: + name: gateway-1 + namespace: default + sectionName: http infraIR: default/gateway-1: proxy: @@ -198,6 +274,75 @@ securityPolicies: status: "True" type: Accepted controllerName: gateway.envoyproxy.io/gatewayclass-controller +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: SecurityPolicy + metadata: + creationTimestamp: null + name: policy-for-http-route-3--grpc-backendref + namespace: default + spec: + extAuth: + failOpen: true + grpc: + backendRef: + kind: Service + name: service-2 + port: 8080 + headersToExtAuth: + - header3 + - header4 + targetRef: + group: gateway.networking.k8s.io + kind: HTTPRoute + name: httproute-3 + status: + ancestors: + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: default + sectionName: http + 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: SecurityPolicy + metadata: + creationTimestamp: null + name: policy-for-http-route-3-http-backendref + namespace: default + spec: + extAuth: + http: + backendRef: + group: gateway.envoyproxy.io + kind: Backend + name: backend-fqdn + port: 3000 + targetRef: + group: gateway.networking.k8s.io + kind: HTTPRoute + name: httproute-4 + status: + ancestors: + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: default + sectionName: http + conditions: + - lastTransitionTime: null + message: Policy has been accepted. + reason: Accepted + status: "True" + type: Accepted + controllerName: gateway.envoyproxy.io/gatewayclass-controller xdsIR: default/gateway-1: accessLog: @@ -327,3 +472,73 @@ xdsIR: distinct: false name: "" prefix: /bar + - destination: + name: httproute/default/httproute-3/rule/0 + settings: + - addressType: IP + endpoints: + - host: 7.7.7.7 + port: 8080 + protocol: HTTP + weight: 1 + hostname: www.baz.com + isHTTP2: false + metadata: + kind: HTTPRoute + name: httproute-3 + namespace: default + name: httproute/default/httproute-3/rule/0/match/0/www_baz_com + pathMatch: + distinct: false + name: "" + prefix: /baz + security: + extAuth: + failOpen: true + grpc: + authority: service-2.default:8080 + destination: + name: securitypolicy/default/policy-for-http-route-3--grpc-backendref/0 + settings: + - addressType: IP + endpoints: + - host: 7.7.7.7 + port: 8080 + protocol: GRPC + weight: 1 + headersToExtAuth: + - header3 + - header4 + name: securitypolicy/default/policy-for-http-route-3--grpc-backendref + - destination: + name: httproute/default/httproute-4/rule/0 + settings: + - weight: 1 + directResponse: + statusCode: 500 + hostname: www.qux.com + isHTTP2: false + metadata: + kind: HTTPRoute + name: httproute-4 + namespace: default + name: httproute/default/httproute-4/rule/0/match/0/www_qux_com + pathMatch: + distinct: false + name: "" + prefix: /qux + security: + extAuth: + http: + authority: primary.foo.com:3000 + destination: + name: securitypolicy/default/policy-for-http-route-3-http-backendref/0 + settings: + - addressType: FQDN + endpoints: + - host: primary.foo.com + port: 3000 + protocol: HTTP + weight: 1 + path: "" + name: securitypolicy/default/policy-for-http-route-3-http-backendref diff --git a/release-notes/current.yaml b/release-notes/current.yaml index 39e8a900c47..2e2df4724ab 100644 --- a/release-notes/current.yaml +++ b/release-notes/current.yaml @@ -15,6 +15,7 @@ new features: | # Fixes for bugs identified in previous versions. bug fixes: | Add a bug fix here + Fixed failed to update SecurityPolicy resources with the `backendRef` field specified # Enhancements that improve performance. performance improvements: | diff --git a/test/cel-validation/securitypolicy_test.go b/test/cel-validation/securitypolicy_test.go index f00ee84260c..033726f2b56 100644 --- a/test/cel-validation/securitypolicy_test.go +++ b/test/cel-validation/securitypolicy_test.go @@ -566,6 +566,26 @@ func TestSecurityPolicyTarget(t *testing.T) { }, wantErrors: []string{}, }, + { + desc: "empty HTTP external auth service", + mutate: func(sp *egv1a1.SecurityPolicy) { + sp.Spec = egv1a1.SecurityPolicySpec{ + ExtAuth: &egv1a1.ExtAuth{ + HTTP: &egv1a1.HTTPExtAuthService{}, + }, + PolicyTargetReferences: egv1a1.PolicyTargetReferences{ + TargetRef: &gwapiv1a2.LocalPolicyTargetReferenceWithSectionName{ + LocalPolicyTargetReference: gwapiv1a2.LocalPolicyTargetReference{ + Group: "gateway.networking.k8s.io", + Kind: "Gateway", + Name: "eg", + }, + }, + }, + } + }, + wantErrors: []string{" backendRef or backendRefs needs to be set"}, + }, { desc: "no extAuth", mutate: func(sp *egv1a1.SecurityPolicy) { @@ -657,36 +677,6 @@ func TestSecurityPolicyTarget(t *testing.T) { " BackendRefs only supports Core and gateway.envoyproxy.io group.", }, }, - { - desc: "http extAuth service invalid Kind", - mutate: func(sp *egv1a1.SecurityPolicy) { - sp.Spec = egv1a1.SecurityPolicySpec{ - ExtAuth: &egv1a1.ExtAuth{ - HTTP: &egv1a1.HTTPExtAuthService{ - BackendCluster: egv1a1.BackendCluster{ - BackendRef: &gwapiv1.BackendObjectReference{ - Kind: ptr.To(gwapiv1.Kind("unsupported")), - Name: "http-auth-service", - Port: ptr.To(gwapiv1.PortNumber(15001)), - }, - }, - }, - }, - PolicyTargetReferences: egv1a1.PolicyTargetReferences{ - TargetRef: &gwapiv1a2.LocalPolicyTargetReferenceWithSectionName{ - LocalPolicyTargetReference: gwapiv1a2.LocalPolicyTargetReference{ - Group: "gateway.networking.k8s.io", - Kind: "Gateway", - Name: "eg", - }, - }, - }, - } - }, - wantErrors: []string{ - "BackendRefs must be used, backendRef is not supported.", - }, - }, { desc: "http extAuth service backendRefs invalid Kind", mutate: func(sp *egv1a1.SecurityPolicy) { @@ -753,36 +743,6 @@ func TestSecurityPolicyTarget(t *testing.T) { "BackendRefs only supports Core and gateway.envoyproxy.io group.", }, }, - { - desc: "grpc extAuth service invalid Kind", - mutate: func(sp *egv1a1.SecurityPolicy) { - sp.Spec = egv1a1.SecurityPolicySpec{ - ExtAuth: &egv1a1.ExtAuth{ - GRPC: &egv1a1.GRPCExtAuthService{ - BackendCluster: egv1a1.BackendCluster{ - BackendRef: &gwapiv1.BackendObjectReference{ - Kind: ptr.To(gwapiv1.Kind("unsupported")), - Name: "http-auth-service", - Port: ptr.To(gwapiv1.PortNumber(15001)), - }, - }, - }, - }, - PolicyTargetReferences: egv1a1.PolicyTargetReferences{ - TargetRef: &gwapiv1a2.LocalPolicyTargetReferenceWithSectionName{ - LocalPolicyTargetReference: gwapiv1a2.LocalPolicyTargetReference{ - Group: "gateway.networking.k8s.io", - Kind: "Gateway", - Name: "eg", - }, - }, - }, - } - }, - wantErrors: []string{ - "BackendRefs must be used, backendRef is not supported.", - }, - }, { desc: "grpc extAuth service backendRefs invalid Kind", mutate: func(sp *egv1a1.SecurityPolicy) { diff --git a/test/e2e/testdata/ext-auth-http-securitypolicy.yaml b/test/e2e/testdata/ext-auth-http-securitypolicy.yaml index c6a1e73c6a6..f79bbaf8745 100644 --- a/test/e2e/testdata/ext-auth-http-securitypolicy.yaml +++ b/test/e2e/testdata/ext-auth-http-securitypolicy.yaml @@ -47,8 +47,8 @@ spec: name: http-with-ext-auth extAuth: http: - backendRefs: - - name: http-ext-auth + backendRef: + name: http-ext-auth namespace: gateway-conformance-infra port: 9002 headersToBackend: ["x-current-user"]