diff --git a/internal/gatewayapi/filters.go b/internal/gatewayapi/filters.go index 97c12bf35d..05ec2772bf 100644 --- a/internal/gatewayapi/filters.go +++ b/internal/gatewayapi/filters.go @@ -119,7 +119,7 @@ func (t *Translator) ProcessGRPCFilters(parentRef *RouteParentContext, route RouteContext, filters []gwapiv1.GRPCRouteFilter, resources *resource.Resources, -) (*HTTPFiltersContext, error) { +) (*HTTPFiltersContext, status.Error) { httpFiltersContext := &HTTPFiltersContext{ ParentRef: parentRef, Route: route, @@ -901,11 +901,11 @@ func (t *Translator) processRequestMirrorFilter( // This sets the status on the HTTPRoute, should the usage be changed so that the status message reflects that the backendRef is from the filter? filterNs := filterContext.Route.GetNamespace() serviceNamespace := NamespaceDerefOr(mirrorBackend.Namespace, filterNs) - err = t.validateBackendRef(mirrorBackendRef, filterContext.ParentRef, filterContext.Route, + err = t.validateBackendRef(mirrorBackendRef, filterContext.Route, resources, serviceNamespace, resource.KindHTTPRoute) if err != nil { return status.NewRouteStatusError( - fmt.Errorf("failed to validate the RequestMirror filter: %w", err), err.Reason()) + fmt.Errorf("failed to validate the RequestMirror filter: %w", err), err.Reason()).WithType(gwapiv1.RouteConditionResolvedRefs) } destName := fmt.Sprintf("%s-mirror-%d", irRouteDestinationName(filterContext.Route, filterContext.RuleIdx), filterIdx) diff --git a/internal/gatewayapi/route.go b/internal/gatewayapi/route.go index 0c0cf73890..0e919c197b 100644 --- a/internal/gatewayapi/route.go +++ b/internal/gatewayapi/route.go @@ -191,10 +191,17 @@ func (t *Translator) processHTTPRouteRules(httpRoute *HTTPRouteContext, parentRe for ruleIdx, rule := range httpRoute.Spec.Rules { httpFiltersContext, err := t.ProcessHTTPFilters(parentRef, httpRoute, rule.Filters, ruleIdx, resources) if err != nil { - errs.Add(status.NewRouteStatusError( - fmt.Errorf("failed to process route rule %d: %w", ruleIdx, err), - status.ConvertToAcceptedReason(err.Reason()), - ).WithType(gwapiv1.RouteConditionAccepted)) + // Some errors should be treated as ResolvedRefs condition type, + // e.g. Failed to resolve the BackendRef in the RequestMirror filter. + // Other errors should be treated as Accepted condition type. + if err.Type() != gwapiv1.RouteConditionResolvedRefs { + errs.Add(status.NewRouteStatusError( + fmt.Errorf("failed to process route rule %d: %w", ruleIdx, err), + status.ConvertToAcceptedReason(err.Reason()), + ).WithType(gwapiv1.RouteConditionAccepted)) + } else { + errs.Add(err) + } continue } @@ -539,12 +546,11 @@ func (t *Translator) processGRPCRouteParentRefs(grpcRoute *GRPCRouteContext, res status.SetRouteStatusCondition(routeStatus, parentRef.routeParentStatusIdx, grpcRoute.GetGeneration(), - gwapiv1.RouteConditionAccepted, + err.Type(), metav1.ConditionFalse, - gwapiv1.RouteReasonUnsupportedValue, // TODO: better reason + err.Reason(), status.Error2ConditionMsg(err), ) - continue } // If no negative condition has been set for ResolvedRefs, set "ResolvedRefs=True" @@ -593,76 +599,99 @@ func (t *Translator) processGRPCRouteParentRefs(grpcRoute *GRPCRouteContext, res } } -func (t *Translator) processGRPCRouteRules(grpcRoute *GRPCRouteContext, parentRef *RouteParentContext, resources *resource.Resources) ([]*ir.HTTPRoute, error) { - var routeRoutes []*ir.HTTPRoute +func (t *Translator) processGRPCRouteRules(grpcRoute *GRPCRouteContext, parentRef *RouteParentContext, resources *resource.Resources) ([]*ir.HTTPRoute, status.Error) { + var ( + irRoutes []*ir.HTTPRoute + errs = &status.MultiStatusError{} + ) // compute matches, filters, backends for ruleIdx, rule := range grpcRoute.Spec.Rules { httpFiltersContext, err := t.ProcessGRPCFilters(parentRef, grpcRoute, rule.Filters, resources) if err != nil { - return nil, err + errs.Add(status.NewRouteStatusError( + fmt.Errorf("failed to process route rule %d: %w", ruleIdx, err), + status.ConvertToAcceptedReason(err.Reason()), + ).WithType(gwapiv1.RouteConditionAccepted)) + continue } // A rule is matched if any one of its matches // is satisfied (i.e. a logical "OR"), so generate // a unique Xds IR HTTPRoute per match. ruleRoutes, err := t.processGRPCRouteRule(grpcRoute, ruleIdx, httpFiltersContext, rule) if err != nil { - return nil, err + errs.Add(status.NewRouteStatusError( + fmt.Errorf("failed to process route rule %d: %w", ruleIdx, err), + status.ConvertToAcceptedReason(err.Reason()), + ).WithType(gwapiv1.RouteConditionAccepted)) + continue } destName := irRouteDestinationName(grpcRoute, ruleIdx) + allDs := []*ir.DestinationSetting{} + failedProcessDestination := false + for i, backendRef := range rule.BackendRefs { settingName := irDestinationSettingName(destName, i) ds, err := t.processDestination(settingName, backendRef, parentRef, grpcRoute, resources) - if ds == nil { + if err != nil { + errs.Add(status.NewRouteStatusError( + fmt.Errorf("failed to process route rule %d backendRef %d: %w", ruleIdx, i, err), + err.Reason(), + )) + failedProcessDestination = true continue } - for _, route := range ruleRoutes { - // If the route already has a direct response or redirect configured, then it was from a filter so skip - // processing any destinations for this route. - if route.DirectResponse != nil || route.Redirect != nil { - continue - } - - // disable associated routes to a backend ref in case some of its config was invalid - if err != nil { - route.DirectResponse = &ir.CustomResponse{ - StatusCode: ptr.To(uint32(500)), - } - } - - if route.Destination == nil { - route.Destination = &ir.RouteDestination{ - Name: destName, - } - } - route.Destination.Settings = append(route.Destination.Settings, ds) + if ds == nil { + continue } + allDs = append(allDs, ds) } - // If the route has no valid backends then just use a direct response and don't fuss with weighted responses - for _, ruleRoute := range ruleRoutes { - noValidBackends := ruleRoute.Destination == nil || ruleRoute.Destination.ToBackendWeights().Valid == 0 - if noValidBackends && ruleRoute.Redirect == nil { - ruleRoute.DirectResponse = &ir.CustomResponse{ + // process each ir route + for _, irRoute := range ruleRoutes { + irRoute.IsHTTP2 = true + destination := &ir.RouteDestination{ + Settings: allDs, + } + + switch { + // If the route already has a direct response or redirect configured, then it was from a filter so skip + // processing any destinations for this route. + case irRoute.DirectResponse != nil || irRoute.Redirect != nil: + // return 500 if any destination setting is invalid + // the error is already added to the error list when processing the destination + case failedProcessDestination: + irRoute.DirectResponse = &ir.CustomResponse{ + StatusCode: ptr.To(uint32(500)), + } + // return 500 if the weight of all the valid destination settings(endpoints list is not empty) is 0 + case destination.ToBackendWeights().Valid == 0: + irRoute.DirectResponse = &ir.CustomResponse{ StatusCode: ptr.To(uint32(500)), } + default: + destination.Name = destName + destination.Settings = allDs + irRoute.Destination = destination } - ruleRoute.IsHTTP2 = true } // TODO handle: // - sum of weights for valid backend refs is 0 // - etc. - routeRoutes = append(routeRoutes, ruleRoutes...) + irRoutes = append(irRoutes, ruleRoutes...) } - return routeRoutes, nil + if errs.Empty() { + return irRoutes, nil + } + return irRoutes, errs } -func (t *Translator) processGRPCRouteRule(grpcRoute *GRPCRouteContext, ruleIdx int, httpFiltersContext *HTTPFiltersContext, rule gwapiv1.GRPCRouteRule) ([]*ir.HTTPRoute, error) { +func (t *Translator) processGRPCRouteRule(grpcRoute *GRPCRouteContext, ruleIdx int, httpFiltersContext *HTTPFiltersContext, rule gwapiv1.GRPCRouteRule) ([]*ir.HTTPRoute, status.Error) { var ruleRoutes []*ir.HTTPRoute // If no matches are specified, the implementation MUST match every gRPC request. @@ -692,7 +721,7 @@ func (t *Translator) processGRPCRouteRule(grpcRoute *GRPCRouteContext, ruleIdx i }) case gwapiv1.GRPCHeaderMatchRegularExpression: if err := regex.Validate(headerMatch.Value); err != nil { - return nil, err + return nil, status.NewRouteStatusError(err, gwapiv1.RouteReasonUnsupportedValue) } irRoute.HeaderMatches = append(irRoute.HeaderMatches, &ir.StringMatch{ Name: string(headerMatch.Name), @@ -709,12 +738,12 @@ func (t *Translator) processGRPCRouteRule(grpcRoute *GRPCRouteContext, ruleIdx i case gwapiv1.GRPCMethodMatchRegularExpression: if match.Method.Service != nil { if err := regex.Validate(*match.Method.Service); err != nil { - return nil, err + return nil, status.NewRouteStatusError(err, gwapiv1.RouteReasonUnsupportedValue) } } if match.Method.Method != nil { if err := regex.Validate(*match.Method.Method); err != nil { - return nil, err + return nil, status.NewRouteStatusError(err, gwapiv1.RouteReasonUnsupportedValue) } } t.processGRPCRouteMethodRegularExpression(match.Method, irRoute) @@ -876,15 +905,21 @@ func (t *Translator) processTLSRouteParentRefs(tlsRoute *TLSRouteContext, resour // Need to compute Route rules within the parentRef loop because // any conditions that come out of it have to go on each RouteParentStatus, // not on the Route as a whole. - var destSettings []*ir.DestinationSetting + var ( + destSettings []*ir.DestinationSetting + resolveErrs = &status.MultiStatusError{} + destName = irRouteDestinationName(tlsRoute, -1 /*rule index*/) + ) - destName := irRouteDestinationName(tlsRoute, -1 /*rule index*/) // compute backends for _, rule := range tlsRoute.Spec.Rules { for i, backendRef := range rule.BackendRefs { settingName := irDestinationSettingName(destName, i) - // not yet handled, requires to align with the conformance test - TLSRouteInvalidReferenceGrant. - ds, _ := t.processDestination(settingName, backendRef, parentRef, tlsRoute, resources) + ds, err := t.processDestination(settingName, backendRef, parentRef, tlsRoute, resources) + if err != nil { + resolveErrs.Add(err) + continue + } if ds != nil { destSettings = append(destSettings, ds) } @@ -897,9 +932,16 @@ func (t *Translator) processTLSRouteParentRefs(tlsRoute *TLSRouteContext, resour // - etc. } - // If no negative condition has been set for ResolvedRefs, set "ResolvedRefs=True" - if !parentRef.HasCondition(tlsRoute, gwapiv1.RouteConditionResolvedRefs, metav1.ConditionFalse) { - routeStatus := GetRouteStatus(tlsRoute) + routeStatus := GetRouteStatus(tlsRoute) + if !resolveErrs.Empty() { + status.SetRouteStatusCondition(routeStatus, + parentRef.routeParentStatusIdx, + tlsRoute.GetGeneration(), + gwapiv1.RouteConditionResolvedRefs, + metav1.ConditionFalse, + resolveErrs.Reason(), + resolveErrs.Error()) + } else { status.SetRouteStatusCondition(routeStatus, parentRef.routeParentStatusIdx, tlsRoute.GetGeneration(), @@ -1004,18 +1046,13 @@ func (t *Translator) ProcessUDPRoutes(udpRoutes []*gwapiv1a2.UDPRoute, gateways func (t *Translator) processUDPRouteParentRefs(udpRoute *UDPRouteContext, resources *resource.Resources, xdsIR resource.XdsIRMap) { for _, parentRef := range udpRoute.ParentRefs { - // Need to compute Route rules within the parentRef loop because - // any conditions that come out of it have to go on each RouteParentStatus, - // not on the Route as a whole. - var destSettings []*ir.DestinationSetting - // compute backends if len(udpRoute.Spec.Rules) != 1 { routeStatus := GetRouteStatus(udpRoute) status.SetRouteStatusCondition(routeStatus, parentRef.routeParentStatusIdx, udpRoute.GetGeneration(), - gwapiv1.RouteConditionResolvedRefs, + gwapiv1.RouteConditionAccepted, metav1.ConditionFalse, "InvalidRule", "One and only one rule is supported", @@ -1023,21 +1060,20 @@ func (t *Translator) processUDPRouteParentRefs(udpRoute *UDPRouteContext, resour continue } - destName := irRouteDestinationName(udpRoute, -1 /*rule index*/) + // Need to compute Route rules within the parentRef loop because + // any conditions that come out of it have to go on each RouteParentStatus, + // not on the Route as a whole. + var ( + destSettings []*ir.DestinationSetting + resolveErrs = &status.MultiStatusError{} + destName = irRouteDestinationName(udpRoute, -1 /*rule index*/) + ) + for i, backendRef := range udpRoute.Spec.Rules[0].BackendRefs { settingName := irDestinationSettingName(destName, i) ds, err := t.processDestination(settingName, backendRef, parentRef, udpRoute, resources) - // skip adding the route and provide the reason via route status. if err != nil { - routeStatus := GetRouteStatus(udpRoute) - status.SetRouteStatusCondition(routeStatus, - parentRef.routeParentStatusIdx, - udpRoute.GetGeneration(), - gwapiv1.RouteConditionAccepted, - metav1.ConditionFalse, - "Failed to process the settings associated with the UDP route.", - err.Error(), - ) + resolveErrs.Add(err) continue } @@ -1047,9 +1083,26 @@ func (t *Translator) processUDPRouteParentRefs(udpRoute *UDPRouteContext, resour } } - // If no negative condition has been set for ResolvedRefs, set "ResolvedRefs=True" - if !parentRef.HasCondition(udpRoute, gwapiv1.RouteConditionResolvedRefs, metav1.ConditionFalse) { - routeStatus := GetRouteStatus(udpRoute) + routeStatus := GetRouteStatus(udpRoute) + if !resolveErrs.Empty() { + status.SetRouteStatusCondition(routeStatus, + parentRef.routeParentStatusIdx, + udpRoute.GetGeneration(), + gwapiv1.RouteConditionResolvedRefs, + metav1.ConditionFalse, + resolveErrs.Reason(), + resolveErrs.Error()) + + // TODO: zhaohuabing should we set the accepted condition to false here? + status.SetRouteStatusCondition(routeStatus, + parentRef.routeParentStatusIdx, + udpRoute.GetGeneration(), + gwapiv1.RouteConditionAccepted, + metav1.ConditionFalse, + "Failed to process the settings associated with the UDP route.", + resolveErrs.Error(), + ) + } else { status.SetRouteStatusCondition(routeStatus, parentRef.routeParentStatusIdx, udpRoute.GetGeneration(), @@ -1152,18 +1205,13 @@ func (t *Translator) ProcessTCPRoutes(tcpRoutes []*gwapiv1a2.TCPRoute, gateways func (t *Translator) processTCPRouteParentRefs(tcpRoute *TCPRouteContext, resources *resource.Resources, xdsIR resource.XdsIRMap) { for _, parentRef := range tcpRoute.ParentRefs { - // Need to compute Route rules within the parentRef loop because - // any conditions that come out of it have to go on each RouteParentStatus, - // not on the Route as a whole. - var destSettings []*ir.DestinationSetting - // compute backends if len(tcpRoute.Spec.Rules) != 1 { routeStatus := GetRouteStatus(tcpRoute) status.SetRouteStatusCondition(routeStatus, parentRef.routeParentStatusIdx, tcpRoute.GetGeneration(), - gwapiv1.RouteConditionResolvedRefs, + gwapiv1.RouteConditionAccepted, metav1.ConditionFalse, "InvalidRule", "One and only one rule is supported", @@ -1171,21 +1219,21 @@ func (t *Translator) processTCPRouteParentRefs(tcpRoute *TCPRouteContext, resour continue } - destName := irRouteDestinationName(tcpRoute, -1 /*rule index*/) + // Need to compute Route rules within the parentRef loop because + // any conditions that come out of it have to go on each RouteParentStatus, + // not on the Route as a whole. + var ( + destSettings []*ir.DestinationSetting + resolveErrs = &status.MultiStatusError{} + destName = irRouteDestinationName(tcpRoute, -1 /*rule index*/) + ) + for i, backendRef := range tcpRoute.Spec.Rules[0].BackendRefs { settingName := irDestinationSettingName(destName, i) ds, err := t.processDestination(settingName, backendRef, parentRef, tcpRoute, resources) // skip adding the route and provide the reason via route status. if err != nil { - routeStatus := GetRouteStatus(tcpRoute) - status.SetRouteStatusCondition(routeStatus, - parentRef.routeParentStatusIdx, - tcpRoute.GetGeneration(), - gwapiv1.RouteConditionAccepted, - metav1.ConditionFalse, - "Failed to process the settings associated with the TCP route.", - err.Error(), - ) + resolveErrs.Add(err) continue } // Skip nil destination settings @@ -1194,9 +1242,16 @@ func (t *Translator) processTCPRouteParentRefs(tcpRoute *TCPRouteContext, resour } } - // If no negative condition has been set for ResolvedRefs, set "ResolvedRefs=True" - if !parentRef.HasCondition(tcpRoute, gwapiv1.RouteConditionResolvedRefs, metav1.ConditionFalse) { - routeStatus := GetRouteStatus(tcpRoute) + routeStatus := GetRouteStatus(tcpRoute) + if !resolveErrs.Empty() { + status.SetRouteStatusCondition(routeStatus, + parentRef.routeParentStatusIdx, + tcpRoute.GetGeneration(), + gwapiv1.RouteConditionResolvedRefs, + metav1.ConditionFalse, + resolveErrs.Reason(), + resolveErrs.Error()) + } else { status.SetRouteStatusCondition(routeStatus, parentRef.routeParentStatusIdx, tcpRoute.GetGeneration(), @@ -1293,7 +1348,7 @@ func (t *Translator) processDestination(name string, backendRefContext BackendRe } backendNamespace := NamespaceDerefOr(backendRef.Namespace, route.GetNamespace()) - err = t.validateBackendRef(backendRefContext, parentRef, route, resources, backendNamespace, routeType) + err = t.validateBackendRef(backendRefContext, route, resources, backendNamespace, routeType) { // return with empty endpoint means the backend is invalid and an error to fail the associated route. if err != nil { @@ -1354,14 +1409,6 @@ func (t *Translator) processDestination(name string, backendRefContext BackendRe } if err := validateDestinationSettings(ds, t.IsEnvoyServiceRouting(envoyProxy), backendRef.Kind); err != nil { - routeStatus := GetRouteStatus(route) - status.SetRouteStatusCondition(routeStatus, - parentRef.routeParentStatusIdx, - route.GetGeneration(), - gwapiv1.RouteConditionResolvedRefs, - metav1.ConditionFalse, - gwapiv1.RouteReasonResolvedRefs, - err.Error()) return nil, err } diff --git a/internal/gatewayapi/testdata/backendtrafficpolicy-with-ratelimit-invalid-distinct-invert.out.yaml b/internal/gatewayapi/testdata/backendtrafficpolicy-with-ratelimit-invalid-distinct-invert.out.yaml index 82857ba43c..a3b43a77f5 100644 --- a/internal/gatewayapi/testdata/backendtrafficpolicy-with-ratelimit-invalid-distinct-invert.out.yaml +++ b/internal/gatewayapi/testdata/backendtrafficpolicy-with-ratelimit-invalid-distinct-invert.out.yaml @@ -103,7 +103,8 @@ grpcRoutes: status: "True" type: Accepted - lastTransitionTime: null - message: Service default/service not found + message: 'Failed to process route rule 0 backendRef 0: service default/service + not found.' reason: BackendNotFound status: "False" type: ResolvedRefs diff --git a/internal/gatewayapi/testdata/envoyproxy-tls-settings-invalid-ns.out.yaml b/internal/gatewayapi/testdata/envoyproxy-tls-settings-invalid-ns.out.yaml index 507259c65b..21e0044078 100644 --- a/internal/gatewayapi/testdata/envoyproxy-tls-settings-invalid-ns.out.yaml +++ b/internal/gatewayapi/testdata/envoyproxy-tls-settings-invalid-ns.out.yaml @@ -210,17 +210,17 @@ tcpRoutes: status: parents: - conditions: + - lastTransitionTime: null + message: Route is accepted + reason: Accepted + status: "True" + type: Accepted - lastTransitionTime: null message: 'ClientCertificateRef Secret is not located in the same namespace as Envoyproxy. Secret namespace: envoy-gateway-user-ns does not match Envoyproxy namespace: envoy-gateway-system' - reason: Failed to process the settings associated with the TCP route. + reason: InvalidBackendTLS status: "False" - 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: @@ -270,6 +270,17 @@ xdsIR: - address: 0.0.0.0 name: envoy-gateway/gateway-tls/ port: 10445 + routes: + - destination: + name: tcproute/envoy-gateway/envoy-gateway/rule/-1 + name: tcproute/envoy-gateway/envoy-gateway + tls: + terminate: + alpnProtocols: [] + certificates: + - name: envoy-gateway/default-cert + privateKey: '[redacted]' + serverCertificate: LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSURKRENDQWd5Z0F3SUJBZ0lVU3JTYktMZjBiTEVHb2dXeC9nQ3cyR0N0dnhFd0RRWUpLb1pJaHZjTkFRRUwKQlFBd0V6RVJNQThHQTFVRUF3d0lWR1Z6ZENCSmJtTXdIaGNOTWpRd01qSTVNRGt6TURFd1doY05NelF3TWpJMgpNRGt6TURFd1dqQVRNUkV3RHdZRFZRUUREQWhVWlhOMElFbHVZekNDQVNJd0RRWUpLb1pJaHZjTkFRRUJCUUFECmdnRVBBRENDQVFvQ2dnRUJBSzFKempQSWlXZzNxb0hTckFkZGtlSmphTVA5aXlNVGkvQlBvOWNKUG9SRThaaTcKV2FwVXJYTC85QTlyK2pITXlHSVpOWk5kY1o1Y1kyWHYwTFA4WnhWeTJsazArM3d0WXpIbnBHWUdWdHlxMnRldApEaEZzaVBsODJZUmpDMG16V2E0UU16NFNYekZITmdJRHBSZGhmcm92bXNldVdHUUU4cFY0VWQ5VUsvU0tpbE1PCnF0QjVKaXJMUDJWczVUMW9XaWNXTFF2ZmJHd3Y3c0ZEZHI5YkcwWHRTUXAxN0hTZ281MFNERTUrQmpTbXB0RncKMVZjS0xscWFoTVhCRERpb3Jnd2hJaEdHS3BFU2VNMFA3YkZoVm1rTTNhc2gyeFNUQnVGVUJEbEU0Sk9haHp3cwpEWHJ1cFVoRGRTMWhkYzJmUHJqaEZBbEpmV0VZWjZCbFpqeXNpVlVDQXdFQUFhTndNRzR3SFFZRFZSME9CQllFCkZCUXVmSzFMaWJ1Vm05VHMvVmpCeDhMM3VpTmVNQjhHQTFVZEl3UVlNQmFBRkJRdWZLMUxpYnVWbTlUcy9WakIKeDhMM3VpTmVNQThHQTFVZEV3RUIvd1FGTUFNQkFmOHdHd1lEVlIwUkJCUXdFb0lCS29JTktpNWxlR0Z0Y0d4bApMbU52YlRBTkJna3Foa2lHOXcwQkFRc0ZBQU9DQVFFQWZQUzQxYWdldldNVjNaWHQwQ09GRzN1WWZQRlhuVnc2ClA0MXA5TzZHa2RZc3VxRnZQZVR5eUgyL2RBSUtLd1N6TS9wdGhnOEtuOExabG1KeUZObkExc3RKeG41WGRiVjEKcFBxajhVdllDQnp5ak1JcW1SeW9peUxpUWxib2hNYTBVZEVCS2NIL1BkTEU5SzhUR0pyWmdvR1hxcTFXbWl0RAozdmNQalNlUEtFaVVKVlM5bENoeVNzMEtZNUIraFVRRDBKajZucEZENFprMHhxZHhoMHJXdWVDcXE3dmpxRVl6CnBqNFB3cnVmbjFQQlRtZnhNdVYvVUpWNWViaWtldVpQMzVrV3pMUjdaV0FMN3d1RGRXcC82bzR5azNRTGFuRFEKQ3dnQ0ZjWCtzcyswVnl1TTNZZXJUT1VVOFFWSkp4NFVaQU5aeDYrNDNwZEpaT2NudFBaNENBPT0KLS0tLS1FTkQgQ0VSVElGSUNBVEUtLS0tLQo= tls: alpnProtocols: [] certificates: diff --git a/internal/gatewayapi/testdata/envoyproxy-tls-settings-invalid.out.yaml b/internal/gatewayapi/testdata/envoyproxy-tls-settings-invalid.out.yaml index 259fa875a4..d38001af09 100644 --- a/internal/gatewayapi/testdata/envoyproxy-tls-settings-invalid.out.yaml +++ b/internal/gatewayapi/testdata/envoyproxy-tls-settings-invalid.out.yaml @@ -209,16 +209,16 @@ tcpRoutes: status: parents: - conditions: + - lastTransitionTime: null + message: Route is accepted + reason: Accepted + status: "True" + type: Accepted - lastTransitionTime: null message: 'failed to locate TLS secret for client auth: envoy-gateway-system/client-auth-not-found specified in EnvoyProxy envoy-gateway-system/test' - reason: Failed to process the settings associated with the TCP route. + reason: InvalidBackendTLS status: "False" - 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: @@ -268,6 +268,17 @@ xdsIR: - address: 0.0.0.0 name: envoy-gateway/gateway-tls/ port: 10445 + routes: + - destination: + name: tcproute/envoy-gateway/envoy-gateway/rule/-1 + name: tcproute/envoy-gateway/envoy-gateway + tls: + terminate: + alpnProtocols: [] + certificates: + - name: envoy-gateway/default-cert + privateKey: '[redacted]' + serverCertificate: LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSURKRENDQWd5Z0F3SUJBZ0lVU3JTYktMZjBiTEVHb2dXeC9nQ3cyR0N0dnhFd0RRWUpLb1pJaHZjTkFRRUwKQlFBd0V6RVJNQThHQTFVRUF3d0lWR1Z6ZENCSmJtTXdIaGNOTWpRd01qSTVNRGt6TURFd1doY05NelF3TWpJMgpNRGt6TURFd1dqQVRNUkV3RHdZRFZRUUREQWhVWlhOMElFbHVZekNDQVNJd0RRWUpLb1pJaHZjTkFRRUJCUUFECmdnRVBBRENDQVFvQ2dnRUJBSzFKempQSWlXZzNxb0hTckFkZGtlSmphTVA5aXlNVGkvQlBvOWNKUG9SRThaaTcKV2FwVXJYTC85QTlyK2pITXlHSVpOWk5kY1o1Y1kyWHYwTFA4WnhWeTJsazArM3d0WXpIbnBHWUdWdHlxMnRldApEaEZzaVBsODJZUmpDMG16V2E0UU16NFNYekZITmdJRHBSZGhmcm92bXNldVdHUUU4cFY0VWQ5VUsvU0tpbE1PCnF0QjVKaXJMUDJWczVUMW9XaWNXTFF2ZmJHd3Y3c0ZEZHI5YkcwWHRTUXAxN0hTZ281MFNERTUrQmpTbXB0RncKMVZjS0xscWFoTVhCRERpb3Jnd2hJaEdHS3BFU2VNMFA3YkZoVm1rTTNhc2gyeFNUQnVGVUJEbEU0Sk9haHp3cwpEWHJ1cFVoRGRTMWhkYzJmUHJqaEZBbEpmV0VZWjZCbFpqeXNpVlVDQXdFQUFhTndNRzR3SFFZRFZSME9CQllFCkZCUXVmSzFMaWJ1Vm05VHMvVmpCeDhMM3VpTmVNQjhHQTFVZEl3UVlNQmFBRkJRdWZLMUxpYnVWbTlUcy9WakIKeDhMM3VpTmVNQThHQTFVZEV3RUIvd1FGTUFNQkFmOHdHd1lEVlIwUkJCUXdFb0lCS29JTktpNWxlR0Z0Y0d4bApMbU52YlRBTkJna3Foa2lHOXcwQkFRc0ZBQU9DQVFFQWZQUzQxYWdldldNVjNaWHQwQ09GRzN1WWZQRlhuVnc2ClA0MXA5TzZHa2RZc3VxRnZQZVR5eUgyL2RBSUtLd1N6TS9wdGhnOEtuOExabG1KeUZObkExc3RKeG41WGRiVjEKcFBxajhVdllDQnp5ak1JcW1SeW9peUxpUWxib2hNYTBVZEVCS2NIL1BkTEU5SzhUR0pyWmdvR1hxcTFXbWl0RAozdmNQalNlUEtFaVVKVlM5bENoeVNzMEtZNUIraFVRRDBKajZucEZENFprMHhxZHhoMHJXdWVDcXE3dmpxRVl6CnBqNFB3cnVmbjFQQlRtZnhNdVYvVUpWNWViaWtldVpQMzVrV3pMUjdaV0FMN3d1RGRXcC82bzR5azNRTGFuRFEKQ3dnQ0ZjWCtzcyswVnl1TTNZZXJUT1VVOFFWSkp4NFVaQU5aeDYrNDNwZEpaT2NudFBaNENBPT0KLS0tLS1FTkQgQ0VSVElGSUNBVEUtLS0tLQo= tls: alpnProtocols: [] certificates: diff --git a/internal/gatewayapi/testdata/gateway-with-listener-with-tcproute-with-mismatch-port-protocol.out.yaml b/internal/gatewayapi/testdata/gateway-with-listener-with-tcproute-with-mismatch-port-protocol.out.yaml index 7977e60505..8c016f7eff 100644 --- a/internal/gatewayapi/testdata/gateway-with-listener-with-tcproute-with-mismatch-port-protocol.out.yaml +++ b/internal/gatewayapi/testdata/gateway-with-listener-with-tcproute-with-mismatch-port-protocol.out.yaml @@ -72,9 +72,9 @@ tcpRoutes: parents: - conditions: - lastTransitionTime: null - message: TCP Port 8081 not found on Service default/service-1 - reason: Failed to process the settings associated with the TCP route. - status: "False" + message: Route is accepted + reason: Accepted + status: "True" type: Accepted - lastTransitionTime: null message: TCP Port 8081 not found on Service default/service-1 @@ -99,3 +99,7 @@ xdsIR: - address: 0.0.0.0 name: envoy-gateway/gateway-1/tcp port: 10162 + routes: + - destination: + name: tcproute/default/tcproute-1/rule/-1 + name: tcproute/default/tcproute-1 diff --git a/internal/gatewayapi/testdata/gateway-with-listener-with-tcproute-with-multiple-rules.out.yaml b/internal/gatewayapi/testdata/gateway-with-listener-with-tcproute-with-multiple-rules.out.yaml index 998a5fa976..1630f34b47 100644 --- a/internal/gatewayapi/testdata/gateway-with-listener-with-tcproute-with-multiple-rules.out.yaml +++ b/internal/gatewayapi/testdata/gateway-with-listener-with-tcproute-with-multiple-rules.out.yaml @@ -76,16 +76,11 @@ tcpRoutes: status: parents: - conditions: - - lastTransitionTime: null - message: Route is accepted - reason: Accepted - status: "True" - type: Accepted - lastTransitionTime: null message: One and only one rule is supported reason: InvalidRule status: "False" - type: ResolvedRefs + type: Accepted controllerName: gateway.envoyproxy.io/gatewayclass-controller parentRef: name: gateway-1 diff --git a/internal/gatewayapi/testdata/gateway-with-listener-with-udproute-with-multiple-rules.out.yaml b/internal/gatewayapi/testdata/gateway-with-listener-with-udproute-with-multiple-rules.out.yaml index 225495e6a3..578d12c6a4 100644 --- a/internal/gatewayapi/testdata/gateway-with-listener-with-udproute-with-multiple-rules.out.yaml +++ b/internal/gatewayapi/testdata/gateway-with-listener-with-udproute-with-multiple-rules.out.yaml @@ -76,16 +76,11 @@ udpRoutes: status: parents: - conditions: - - lastTransitionTime: null - message: Route is accepted - reason: Accepted - status: "True" - type: Accepted - lastTransitionTime: null message: One and only one rule is supported reason: InvalidRule status: "False" - type: ResolvedRefs + type: Accepted controllerName: gateway.envoyproxy.io/gatewayclass-controller parentRef: name: gateway-1 diff --git a/internal/gatewayapi/testdata/httproute-with-invalid-backend-ref-unsupported-filter.out.yaml b/internal/gatewayapi/testdata/httproute-with-invalid-backend-ref-unsupported-filter.out.yaml index a688188f5b..8d7fc7c53f 100644 --- a/internal/gatewayapi/testdata/httproute-with-invalid-backend-ref-unsupported-filter.out.yaml +++ b/internal/gatewayapi/testdata/httproute-with-invalid-backend-ref-unsupported-filter.out.yaml @@ -73,9 +73,9 @@ httpRoutes: status: "True" type: Accepted - lastTransitionTime: null - message: 'Failed to process route rule 0 backendRef 0: specific filter is - not supported within BackendRef, only RequestHeaderModifier and ResponseHeaderModifier - are supported.' + message: 'Failed to process route rule 0 backendRef 0: Specific filter is + not supported within BackendRef, only RequestHeaderModifier, ResponseHeaderModifier + and gateway.envoyproxy.io/HTTPRouteFilter are supported.' reason: UnsupportedRefValue status: "False" type: ResolvedRefs diff --git a/internal/gatewayapi/testdata/httproute-with-mirror-filter-service-no-port.out.yaml b/internal/gatewayapi/testdata/httproute-with-mirror-filter-service-no-port.out.yaml index d2605efc0a..0ebb67ed50 100644 --- a/internal/gatewayapi/testdata/httproute-with-mirror-filter-service-no-port.out.yaml +++ b/internal/gatewayapi/testdata/httproute-with-mirror-filter-service-no-port.out.yaml @@ -71,15 +71,13 @@ httpRoutes: parents: - conditions: - lastTransitionTime: null - message: 'Failed to process route rule 0: failed to validate the RequestMirror - filter: A valid port number corresponding to a port on the Service must - be specified.' - reason: UnsupportedValue - status: "False" + message: Route is accepted + reason: Accepted + status: "True" type: Accepted - lastTransitionTime: null - message: A valid port number corresponding to a port on the Service must be - specified + message: 'Failed to validate the RequestMirror filter: A valid port number + corresponding to a port on the Service must be specified.' reason: PortNotSpecified status: "False" type: ResolvedRefs diff --git a/internal/gatewayapi/testdata/httproute-with-mirror-filter-service-not-found.out.yaml b/internal/gatewayapi/testdata/httproute-with-mirror-filter-service-not-found.out.yaml index 664ffd08f5..6e11835b8c 100644 --- a/internal/gatewayapi/testdata/httproute-with-mirror-filter-service-not-found.out.yaml +++ b/internal/gatewayapi/testdata/httproute-with-mirror-filter-service-not-found.out.yaml @@ -72,13 +72,13 @@ httpRoutes: parents: - conditions: - lastTransitionTime: null - message: 'Failed to process route rule 0: failed to validate the RequestMirror - filter: service default/service-unknown not found.' - reason: UnsupportedValue - status: "False" + message: Route is accepted + reason: Accepted + status: "True" type: Accepted - lastTransitionTime: null - message: Service default/service-unknown not found + message: 'Failed to validate the RequestMirror filter: service default/service-unknown + not found.' reason: BackendNotFound status: "False" type: ResolvedRefs diff --git a/internal/gatewayapi/testdata/securitypolicy-status-conditions.out.yaml b/internal/gatewayapi/testdata/securitypolicy-status-conditions.out.yaml index 2380d0ae7f..748cd2c0e5 100644 --- a/internal/gatewayapi/testdata/securitypolicy-status-conditions.out.yaml +++ b/internal/gatewayapi/testdata/securitypolicy-status-conditions.out.yaml @@ -159,7 +159,8 @@ grpcRoutes: status: "True" type: Accepted - lastTransitionTime: null - message: Service envoy-gateway/service-1 not found + message: 'Failed to process route rule 0 backendRef 0: service envoy-gateway/service-1 + not found.' reason: BackendNotFound status: "False" type: ResolvedRefs diff --git a/internal/gatewayapi/testdata/securitypolicy-with-cors-targetrefs.out.yaml b/internal/gatewayapi/testdata/securitypolicy-with-cors-targetrefs.out.yaml index 708a622f92..5868da9397 100644 --- a/internal/gatewayapi/testdata/securitypolicy-with-cors-targetrefs.out.yaml +++ b/internal/gatewayapi/testdata/securitypolicy-with-cors-targetrefs.out.yaml @@ -144,7 +144,8 @@ grpcRoutes: status: "True" type: Accepted - lastTransitionTime: null - message: Service envoy-gateway/service-1 not found + message: 'Failed to process route rule 0 backendRef 0: service envoy-gateway/service-1 + not found.' reason: BackendNotFound status: "False" type: ResolvedRefs diff --git a/internal/gatewayapi/validate.go b/internal/gatewayapi/validate.go index fc8b34aa2b..0bbf0d1459 100644 --- a/internal/gatewayapi/validate.go +++ b/internal/gatewayapi/validate.go @@ -24,24 +24,24 @@ import ( "github.com/envoyproxy/gateway/internal/gatewayapi/status" ) -func (t *Translator) validateBackendRef(backendRefContext BackendRefContext, parentRef *RouteParentContext, route RouteContext, +func (t *Translator) validateBackendRef(backendRefContext BackendRefContext, route RouteContext, resources *resource.Resources, backendNamespace string, routeKind gwapiv1.Kind, ) status.Error { backendRef := GetBackendRef(backendRefContext) - if err := t.validateBackendRefFilters(backendRefContext, parentRef, route, routeKind); err != nil { + if err := t.validateBackendRefFilters(backendRefContext, routeKind); err != nil { return err } - if err := t.validateBackendRefGroup(backendRef, parentRef, route); err != nil { + if err := t.validateBackendRefGroup(backendRef); err != nil { return err } - if err := t.validateBackendRefKind(backendRef, parentRef, route); err != nil { + if err := t.validateBackendRefKind(backendRef); err != nil { return err } - if err := t.validateBackendNamespace(backendRef, parentRef, route, resources, routeKind); err != nil { + if err := t.validateBackendNamespace(backendRef, route, resources, routeKind); err != nil { return err } - if err := t.validateBackendPort(backendRef, parentRef, route); err != nil { + if err := t.validateBackendPort(backendRef); err != nil { return err } @@ -50,73 +50,26 @@ func (t *Translator) validateBackendRef(backendRefContext BackendRefContext, par protocol = corev1.ProtocolUDP } - // TODO: zhaohuabing remove this and handle status in the out layer backendRefKind := KindDerefOr(backendRef.Kind, resource.KindService) switch backendRefKind { case resource.KindService: if err := validateBackendRefService(backendRef.BackendObjectReference, resources, backendNamespace, protocol); err != nil { - routeStatus := GetRouteStatus(route) - status.SetRouteStatusCondition(routeStatus, - parentRef.routeParentStatusIdx, - route.GetGeneration(), - gwapiv1.RouteConditionResolvedRefs, - metav1.ConditionFalse, - err.Reason(), - errorToMessage(err), - ) return err } case resource.KindServiceImport: if err := t.validateBackendServiceImport(backendRef.BackendObjectReference, resources, backendNamespace, protocol); err != nil { - routeStatus := GetRouteStatus(route) - status.SetRouteStatusCondition(routeStatus, - parentRef.routeParentStatusIdx, - route.GetGeneration(), - gwapiv1.RouteConditionResolvedRefs, - metav1.ConditionFalse, - err.Reason(), - errorToMessage(err), - ) return err } case egv1a1.KindBackend: if err := t.validateBackendRefBackend(backendRef.BackendObjectReference, resources, backendNamespace, false); err != nil { - routeStatus := GetRouteStatus(route) - status.SetRouteStatusCondition(routeStatus, - parentRef.routeParentStatusIdx, - route.GetGeneration(), - gwapiv1.RouteConditionResolvedRefs, - metav1.ConditionFalse, - err.Reason(), - errorToMessage(err), - ) return err } } return nil } -func errorToMessage(err error) string { - if err == nil { - return "" - } - msg := err.Error() - - // Convert first character to upper - return strings.ToUpper(msg[0:1]) + msg[1:] -} - -func (t *Translator) validateBackendRefGroup(backendRef *gwapiv1a2.BackendRef, parentRef *RouteParentContext, route RouteContext) status.Error { +func (t *Translator) validateBackendRefGroup(backendRef *gwapiv1a2.BackendRef) status.Error { if backendRef.Group != nil && *backendRef.Group != "" && *backendRef.Group != GroupMultiClusterService && *backendRef.Group != egv1a1.GroupName { - routeStatus := GetRouteStatus(route) - status.SetRouteStatusCondition(routeStatus, // TODO: zhaohuabing remove this and handle status in the out layer - parentRef.routeParentStatusIdx, - route.GetGeneration(), - gwapiv1.RouteConditionResolvedRefs, - metav1.ConditionFalse, - gwapiv1.RouteReasonInvalidKind, - fmt.Sprintf("Group is invalid, only the core API group (specified by omitting the group field or setting it to an empty string), %s and %s are supported", GroupMultiClusterService, egv1a1.GroupName), - ) return status.NewRouteStatusError( fmt.Errorf("Group is invalid, only the core API group (specified by omitting the group field or setting it to an empty string), %s and %s are supported", GroupMultiClusterService, @@ -126,17 +79,8 @@ func (t *Translator) validateBackendRefGroup(backendRef *gwapiv1a2.BackendRef, p return nil } -func (t *Translator) validateBackendRefKind(backendRef *gwapiv1a2.BackendRef, parentRef *RouteParentContext, route RouteContext) status.Error { +func (t *Translator) validateBackendRefKind(backendRef *gwapiv1a2.BackendRef) status.Error { if backendRef.Kind != nil && *backendRef.Kind != resource.KindService && *backendRef.Kind != resource.KindServiceImport && *backendRef.Kind != egv1a1.KindBackend { - routeStatus := GetRouteStatus(route) - status.SetRouteStatusCondition(routeStatus, // TODO: zhaohuabing remove this and handle status in the out layer - parentRef.routeParentStatusIdx, - route.GetGeneration(), - gwapiv1.RouteConditionResolvedRefs, - metav1.ConditionFalse, - gwapiv1.RouteReasonInvalidKind, - "Kind is invalid, only Service, MCS ServiceImport and Envoy Gateway Backend are supported", - ) return status.NewRouteStatusError( fmt.Errorf("Kind is invalid, only Service, MCS ServiceImport and Envoy Gateway Backend are supported"), gwapiv1.RouteReasonInvalidKind) @@ -144,7 +88,7 @@ func (t *Translator) validateBackendRefKind(backendRef *gwapiv1a2.BackendRef, pa return nil } -func (t *Translator) validateBackendRefFilters(backendRef BackendRefContext, parentRef *RouteParentContext, route RouteContext, routeKind gwapiv1.Kind) status.Error { +func (t *Translator) validateBackendRefFilters(backendRef BackendRefContext, routeKind gwapiv1.Kind) status.Error { filters := GetFilters(backendRef) var unsupportedFilters bool @@ -173,30 +117,20 @@ func (t *Translator) validateBackendRefFilters(backendRef BackendRefContext, par return nil } - // TODO: zhaohuabing remove this and handle status in the out layer if unsupportedFilters { message := "Specific filter is not supported within BackendRef, only RequestHeaderModifier, ResponseHeaderModifier and gateway.envoyproxy.io/HTTPRouteFilter are supported" if routeKind == resource.KindGRPCRoute { message = "Specific filter is not supported within BackendRef, only RequestHeaderModifier and ResponseHeaderModifier are supported" } - routeStatus := GetRouteStatus(route) - status.SetRouteStatusCondition(routeStatus, - parentRef.routeParentStatusIdx, - route.GetGeneration(), - gwapiv1.RouteConditionResolvedRefs, - metav1.ConditionFalse, - status.RouteReasonUnsupportedRefValue, - message, - ) return status.NewRouteStatusError( - errors.New("specific filter is not supported within BackendRef, only RequestHeaderModifier and ResponseHeaderModifier are supported"), + errors.New(message), status.RouteReasonUnsupportedRefValue) } return nil } -func (t *Translator) validateBackendNamespace(backendRef *gwapiv1a2.BackendRef, parentRef *RouteParentContext, route RouteContext, +func (t *Translator) validateBackendNamespace(backendRef *gwapiv1a2.BackendRef, route RouteContext, resources *resource.Resources, routeKind gwapiv1.Kind, ) status.Error { if backendRef.Namespace != nil && string(*backendRef.Namespace) != "" && string(*backendRef.Namespace) != route.GetNamespace() { @@ -214,15 +148,6 @@ func (t *Translator) validateBackendNamespace(backendRef *gwapiv1a2.BackendRef, }, resources.ReferenceGrants, ) { - routeStatus := GetRouteStatus(route) - status.SetRouteStatusCondition(routeStatus, // TODO: zhaohuabing remove this and handle status in the out layer - parentRef.routeParentStatusIdx, - route.GetGeneration(), - gwapiv1.RouteConditionResolvedRefs, - metav1.ConditionFalse, - gwapiv1.RouteReasonRefNotPermitted, - fmt.Sprintf("Backend ref to %s %s/%s not permitted by any ReferenceGrant.", KindDerefOr(backendRef.Kind, resource.KindService), *backendRef.Namespace, backendRef.Name), - ) return status.NewRouteStatusError( fmt.Errorf("Backend ref to %s %s/%s not permitted by any ReferenceGrant.", KindDerefOr(backendRef.Kind, resource.KindService), @@ -234,7 +159,7 @@ func (t *Translator) validateBackendNamespace(backendRef *gwapiv1a2.BackendRef, return nil } -func (t *Translator) validateBackendPort(backendRef *gwapiv1a2.BackendRef, parentRef *RouteParentContext, route RouteContext) status.Error { +func (t *Translator) validateBackendPort(backendRef *gwapiv1a2.BackendRef) status.Error { if backendRef == nil { return nil } @@ -244,16 +169,6 @@ func (t *Translator) validateBackendPort(backendRef *gwapiv1a2.BackendRef, paren } if backendRef.Port == nil { - // TODO: zhaohuabing remove this and handle status in the out layer - routeStatus := GetRouteStatus(route) - status.SetRouteStatusCondition(routeStatus, - parentRef.routeParentStatusIdx, - route.GetGeneration(), - gwapiv1.RouteConditionResolvedRefs, - metav1.ConditionFalse, - status.RouteReasonPortNotSpecified, - "A valid port number corresponding to a port on the Service must be specified", - ) return status.NewRouteStatusError( errors.New("A valid port number corresponding to a port on the Service must be specified"), status.RouteReasonPortNotSpecified)