diff --git a/CHANGELOG.md b/CHANGELOG.md index 847e75c279..7e55680b77 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -73,6 +73,8 @@ [#2781](https://github.com/Kong/kubernetes-ingress-controller/pull/2781) - Added all the Gateway-related conformance tests. [#2777](https://github.com/Kong/kubernetes-ingress-controller/issues/2777) +- Added all the HTTPRoute-related conformance tests. + [#2776](https://github.com/Kong/kubernetes-ingress-controller/issues/2776) #### Fixed @@ -95,6 +97,9 @@ [#2791](https://github.com/Kong/kubernetes-ingress-controller/pull/2791) - Update Listener statuses whenever they change, not just on Gateway creation. [#2797](https://github.com/Kong/kubernetes-ingress-controller/pull/2797) +- StripPath for `HTTPRoute`s is now disabled by default to be conformant with the + Gateway API requirements. + #[#2737](https://github.com/Kong/kubernetes-ingress-controller/pull/2737) #### Under the hood diff --git a/examples/gateway-httproute.yaml b/examples/gateway-httproute.yaml index 3b0cd8e78c..4637c5f4f4 100644 --- a/examples/gateway-httproute.yaml +++ b/examples/gateway-httproute.yaml @@ -99,6 +99,8 @@ apiVersion: gateway.networking.k8s.io/v1beta1 kind: HTTPRoute metadata: name: httproute-testing + annotations: + konghq.com/strip-path: "true" spec: parentRefs: - name: kong @@ -109,8 +111,10 @@ spec: value: /httproute-testing backendRefs: - name: httpbin + kind: Service port: 80 weight: 75 - name: nginx + kind: Service port: 8080 weight: 25 diff --git a/internal/controllers/gateway/gateway_controller_test.go b/internal/controllers/gateway/gateway_controller_test.go index af829e1ea9..b4bc8f9abe 100644 --- a/internal/controllers/gateway/gateway_controller_test.go +++ b/internal/controllers/gateway/gateway_controller_test.go @@ -11,6 +11,7 @@ import ( gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" "github.com/kong/kubernetes-ingress-controller/v2/internal/annotations" + "github.com/kong/kubernetes-ingress-controller/v2/internal/util" ) func Test_readyConditionExistsForObservedGeneration(t *testing.T) { @@ -523,7 +524,7 @@ func Test_getReferenceGrantConditionReason(t *testing.T) { name: "no need for reference", gatewayNamespace: "test", certRef: gatewayv1alpha2.SecretObjectReference{ - Kind: (*gatewayv1alpha2.Kind)(pointer.StringPtr("secret")), + Kind: util.StringToGatewayAPIKindPtr("Secret"), Name: "testSecret", }, expectedReason: string(gatewayv1alpha2.ListenerReasonResolvedRefs), @@ -532,7 +533,7 @@ func Test_getReferenceGrantConditionReason(t *testing.T) { name: "reference not granted", gatewayNamespace: "test", certRef: gatewayv1alpha2.SecretObjectReference{ - Kind: (*gatewayv1alpha2.Kind)(pointer.StringPtr("secret")), + Kind: util.StringToGatewayAPIKindPtr("Secret"), Name: "testSecret", Namespace: (*gatewayv1alpha2.Namespace)(pointer.StringPtr("otherNamespace")), }, @@ -565,7 +566,7 @@ func Test_getReferenceGrantConditionReason(t *testing.T) { name: "reference granted, secret name not specified", gatewayNamespace: "test", certRef: gatewayv1alpha2.SecretObjectReference{ - Kind: (*gatewayv1alpha2.Kind)(pointer.StringPtr("secret")), + Kind: util.StringToGatewayAPIKindPtr("Secret"), Name: "testSecret", Namespace: (*gatewayv1alpha2.Namespace)(pointer.StringPtr("otherNamespace")), }, @@ -604,7 +605,7 @@ func Test_getReferenceGrantConditionReason(t *testing.T) { name: "reference granted, secret name specified", gatewayNamespace: "test", certRef: gatewayv1alpha2.SecretObjectReference{ - Kind: (*gatewayv1alpha2.Kind)(pointer.StringPtr("secret")), + Kind: util.StringToGatewayAPIKindPtr("Secret"), Name: "testSecret", Namespace: (*gatewayv1alpha2.Namespace)(pointer.StringPtr("otherNamespace")), }, diff --git a/internal/controllers/gateway/httproute_controller.go b/internal/controllers/gateway/httproute_controller.go index a31ccf4635..001959f0d6 100644 --- a/internal/controllers/gateway/httproute_controller.go +++ b/internal/controllers/gateway/httproute_controller.go @@ -6,10 +6,12 @@ import ( "reflect" "github.com/go-logr/logr" - "k8s.io/apimachinery/pkg/api/errors" + corev1 "k8s.io/api/core/v1" + k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" + "k8s.io/utils/pointer" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller" @@ -21,6 +23,7 @@ import ( gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" "github.com/kong/kubernetes-ingress-controller/v2/internal/dataplane" + "github.com/kong/kubernetes-ingress-controller/v2/internal/util" ) // ----------------------------------------------------------------------------- @@ -235,7 +238,7 @@ func (r *HTTPRouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( if err := r.Get(ctx, req.NamespacedName, httproute); err != nil { // if the queued object is no longer present in the proxy cache we need // to ensure that if it was ever added to the cache, it gets removed. - if errors.IsNotFound(err) { + if k8serrors.IsNotFound(err) { debug(log, httproute, "object does not exist, ensuring it is not present in the proxy cache") httproute.Namespace = req.Namespace httproute.Name = req.Name @@ -299,25 +302,31 @@ func (r *HTTPRouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( // requeue the object and wait until all supported gateways are ready. debug(log, httproute, "checking if the httproute's gateways are ready") for _, gateway := range gateways { - if !isGatewayReady(gateway) { + if !isGatewayReady(gateway.gateway) { debug(log, httproute, "gateway for route was not ready, waiting") return ctrl.Result{Requeue: true}, nil } } - // if the gateways are ready, and the HTTPRoute is destined for them, ensure that - // the object is pushed to the dataplane. - if err := r.DataplaneClient.UpdateObject(httproute); err != nil { - debug(log, httproute, "failed to update object in data-plane, requeueing") - return ctrl.Result{}, err - } - if r.DataplaneClient.AreKubernetesObjectReportsEnabled() { - // if the dataplane client has reporting enabled (this is the default and is - // tied in with status updates being enabled in the controller manager) then - // we will wait until the object is reported as successfully configured before - // moving on to status updates. - if !r.DataplaneClient.KubernetesObjectIsConfigured(httproute) { - return ctrl.Result{Requeue: true}, nil + // perform operations on the kong store only if the route is in accepted status + if isRouteAccepted(gateways) { + // remove all the hostnames that don't match with at least one Listener's Hostname + filteredHTTPRoute := filterHostnames(gateways, httproute.DeepCopy()) + + // if the gateways are ready, and the HTTPRoute is destined for them, ensure that + // the object is pushed to the dataplane. + if err := r.DataplaneClient.UpdateObject(filteredHTTPRoute); err != nil { + debug(log, httproute, "failed to update object in data-plane, requeueing") + return ctrl.Result{}, err + } + if r.DataplaneClient.AreKubernetesObjectReportsEnabled() { + // if the dataplane client has reporting enabled (this is the default and is + // tied in with status updates being enabled in the controller manager) then + // we will wait until the object is reported as successfully configured before + // moving on to status updates. + if !r.DataplaneClient.KubernetesObjectIsConfigured(httproute) { + return ctrl.Result{Requeue: true}, nil + } } } @@ -352,7 +361,7 @@ var httprouteParentKind = "Gateway" // ensureGatewayReferenceStatus takes any number of Gateways that should be // considered "attached" to a given HTTPRoute and ensures that the status // for the HTTPRoute is updated appropriately. -func (r *HTTPRouteReconciler) ensureGatewayReferenceStatusAdded(ctx context.Context, httproute *gatewayv1alpha2.HTTPRoute, gateways ...*gatewayv1alpha2.Gateway) (bool, error) { +func (r *HTTPRouteReconciler) ensureGatewayReferenceStatusAdded(ctx context.Context, httproute *gatewayv1alpha2.HTTPRoute, gateways ...supportedGatewayWithCondition) (bool, error) { // map the existing parentStatues to avoid duplications parentStatuses := make(map[string]*gatewayv1alpha2.RouteParentStatus) for _, existingParent := range httproute.Status.Parents { @@ -361,7 +370,11 @@ func (r *HTTPRouteReconciler) ensureGatewayReferenceStatusAdded(ctx context.Cont namespace = string(*existingParent.ParentRef.Namespace) } existingParentCopy := existingParent - parentStatuses[namespace+string(existingParent.ParentRef.Name)] = &existingParentCopy + var sectionName string + if existingParent.ParentRef.SectionName != nil { + sectionName = string(*existingParent.ParentRef.SectionName) + } + parentStatuses[fmt.Sprintf("%s/%s/%s", namespace, existingParent.ParentRef.Name, sectionName)] = &existingParentCopy } // overlay the parent ref statuses for all new gateway references @@ -371,23 +384,28 @@ func (r *HTTPRouteReconciler) ensureGatewayReferenceStatusAdded(ctx context.Cont gatewayParentStatus := &gatewayv1alpha2.RouteParentStatus{ ParentRef: gatewayv1alpha2.ParentReference{ Group: (*gatewayv1alpha2.Group)(&gatewayv1alpha2.GroupVersion.Group), - Kind: (*gatewayv1alpha2.Kind)(&httprouteParentKind), - Namespace: (*gatewayv1alpha2.Namespace)(&gateway.Namespace), - Name: gatewayv1alpha2.ObjectName(gateway.Name), + Kind: util.StringToGatewayAPIKindPtr(httprouteParentKind), + Namespace: (*gatewayv1alpha2.Namespace)(&gateway.gateway.Namespace), + Name: gatewayv1alpha2.ObjectName(gateway.gateway.Name), }, ControllerName: ControllerName, Conditions: []metav1.Condition{{ - Type: string(gatewayv1alpha2.RouteConditionAccepted), - Status: metav1.ConditionTrue, + Type: gateway.condition.Type, + Status: gateway.condition.Status, ObservedGeneration: httproute.Generation, LastTransitionTime: metav1.Now(), - Reason: string(gatewayv1alpha2.GatewayReasonReady), + Reason: gateway.condition.Reason, }}, } + if gateway.listenerName != "" { + gatewayParentStatus.ParentRef.SectionName = (*gatewayv1alpha2.SectionName)(pointer.StringPtr(gateway.listenerName)) + } + + key := fmt.Sprintf("%s/%s/%s", gateway.gateway.Namespace, gateway.gateway.Name, gateway.listenerName) // if the reference already exists and doesn't require any changes // then just leave it alone. - if existingGatewayParentStatus, exists := parentStatuses[gateway.Namespace+gateway.Name]; exists { + if existingGatewayParentStatus, exists := parentStatuses[key]; exists { // fake the time of the existing status as this wont be equal for i := range existingGatewayParentStatus.Conditions { existingGatewayParentStatus.Conditions[i].LastTransitionTime = gatewayParentStatus.Conditions[0].LastTransitionTime @@ -400,12 +418,17 @@ func (r *HTTPRouteReconciler) ensureGatewayReferenceStatusAdded(ctx context.Cont } // otherwise overlay the new status on top the list of parentStatuses - parentStatuses[gateway.Namespace+gateway.Name] = gatewayParentStatus + parentStatuses[key] = gatewayParentStatus statusChangesWereMade = true } + parentStatuses, resolvedRefsChanged, err := r.setRouteConditionResolvedRefsCondition(ctx, httproute, parentStatuses) + if err != nil { + return false, err + } + // if we didn't have to actually make any changes, no status update is needed - if !statusChangesWereMade { + if !statusChangesWereMade && !resolvedRefsChanged { return false, nil } @@ -451,3 +474,92 @@ func (r *HTTPRouteReconciler) ensureGatewayReferenceStatusRemoved(ctx context.Co // the status needed an update and it was updated successfully return true, nil } + +// setRouteConditionResolvedRefsCondition sets a condition of type ResolvedRefs on the route status. +func (r *HTTPRouteReconciler) setRouteConditionResolvedRefsCondition(ctx context.Context, httpRoute *gatewayv1alpha2.HTTPRoute, parentStatuses map[string]*gatewayv1alpha2.RouteParentStatus) (map[string]*gatewayv1alpha2.RouteParentStatus, bool, error) { + var changed bool + resolvedRefsStatus := metav1.ConditionFalse + reason, err := r.getHTTPRouteRuleReason(ctx, *httpRoute) + if err != nil { + return nil, false, err + } + if reason == gatewayv1alpha2.RouteReasonResolvedRefs { + resolvedRefsStatus = metav1.ConditionTrue + } + + // iterate over all the parentStatuses conditions, and if no RouteConditionResolvedRefs is found, + // or if the condition is found but has to be changed, update the status and mark it to be updated + for _, parentStatus := range parentStatuses { + var conditionFound bool + for _, cond := range parentStatus.Conditions { + if cond.Type == string(gatewayv1alpha2.RouteConditionResolvedRefs) && + cond.Status == resolvedRefsStatus && + cond.Reason == string(reason) { + conditionFound = true + break + } + } + if !conditionFound { + parentStatus.Conditions = append(parentStatus.Conditions, metav1.Condition{ + Type: string(gatewayv1alpha2.RouteConditionResolvedRefs), + Status: resolvedRefsStatus, + ObservedGeneration: httpRoute.Generation, + LastTransitionTime: metav1.Now(), + Reason: string(reason), + }) + changed = true + } + } + + return parentStatuses, changed, nil +} + +func (r *HTTPRouteReconciler) getHTTPRouteRuleReason(ctx context.Context, httpRoute gatewayv1alpha2.HTTPRoute) (gatewayv1alpha2.RouteConditionReason, error) { + for _, rule := range httpRoute.Spec.Rules { + for _, backendRef := range rule.BackendRefs { + backendNamespace := httpRoute.Namespace + if backendRef.Namespace != nil && *backendRef.Namespace != "" { + backendNamespace = string(*backendRef.Namespace) + } + + // Check if the BackendRef GroupKind is supported + if !util.IsBackendRefGroupKindSupported(backendRef.Group, backendRef.Kind) { + return gatewayv1alpha2.RouteReasonInvalidKind, nil + } + + // Check if all the objects referenced actually exist + // Only services are currently supported as BackendRef objects + service := &corev1.Service{} + err := r.Client.Get(ctx, types.NamespacedName{Namespace: backendNamespace, Name: string(backendRef.Name)}, service) + if err != nil { + if !k8serrors.IsNotFound(err) { + return "", err + } + return gatewayv1alpha2.RouteReasonBackendNotFound, nil + } + + // Check if the object referenced is in another namespace, + // and if there is grant for that reference + if httpRoute.Namespace != backendNamespace { + referenceGrantList := &gatewayv1alpha2.ReferenceGrantList{} + if err := r.Client.List(ctx, referenceGrantList, client.InNamespace(backendNamespace)); err != nil { + return "", err + } + if len(referenceGrantList.Items) == 0 { + return gatewayv1alpha2.RouteReasonRefNotPermitted, nil + } + var isGranted bool + for _, grant := range referenceGrantList.Items { + if isHTTPReferenceGranted(grant.Spec, backendRef, httpRoute.Namespace) { + isGranted = true + break + } + } + if !isGranted { + return gatewayv1alpha2.RouteReasonRefNotPermitted, nil + } + } + } + } + return gatewayv1alpha2.RouteReasonResolvedRefs, nil +} diff --git a/internal/controllers/gateway/route_utils.go b/internal/controllers/gateway/route_utils.go index 9163e75de4..a1edab04fb 100644 --- a/internal/controllers/gateway/route_utils.go +++ b/internal/controllers/gateway/route_utils.go @@ -10,13 +10,25 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" + + "github.com/kong/kubernetes-ingress-controller/v2/internal/util" ) // ----------------------------------------------------------------------------- // Route Utilities // ----------------------------------------------------------------------------- -const unsupportedGW = "no supported Gateway found for route" +const ( + unsupportedGW = "no supported Gateway found for route" +) + +// supportedGatewayWithCondition is a struct that wraps a gateway and some further info +// such as the condition Status condition Accepted of the gateway and the listenerName. +type supportedGatewayWithCondition struct { + gateway *gatewayv1alpha2.Gateway + condition metav1.Condition + listenerName string +} // parentRefsForRoute provides a list of the parentRefs given a Gateway APIs route object // (e.g. HTTPRoute, TCPRoute, e.t.c.) which refer to the Gateway resource(s) which manage it. @@ -39,7 +51,7 @@ func parentRefsForRoute(obj client.Object) ([]gatewayv1alpha2.ParentReference, e // Gateway APIs route object (e.g. HTTPRoute, TCPRoute, e.t.c.) from the provided cached // client if they match this controller. If there are no gateways present for this route // OR the present gateways are references to missing objects, this will return a unsupportedGW error. -func getSupportedGatewayForRoute(ctx context.Context, mgrc client.Client, obj client.Object) ([]*gatewayv1alpha2.Gateway, error) { +func getSupportedGatewayForRoute(ctx context.Context, mgrc client.Client, obj client.Object) ([]supportedGatewayWithCondition, error) { // gather the parentrefs for this route object parentRefs, err := parentRefsForRoute(obj) if err != nil { @@ -47,7 +59,7 @@ func getSupportedGatewayForRoute(ctx context.Context, mgrc client.Client, obj cl } // search each parentRef to see if this controller is one of the supported ones - gateways := make([]*gatewayv1alpha2.Gateway, 0) + gateways := make([]supportedGatewayWithCondition, 0) for _, parentRef := range parentRefs { // gather the namespace/name for the gateway namespace := obj.GetNamespace() @@ -93,14 +105,18 @@ func getSupportedGatewayForRoute(ctx context.Context, mgrc client.Client, obj cl allowedNamespaces := make(map[string]interface{}) // set true if we find any AllowedRoutes. there may be none, in which case any namespace is permitted filtered := false + matchingHostname := metav1.ConditionFalse for _, listener := range gateway.Spec.Listeners { // TODO https://github.com/Kong/kubernetes-ingress-controller/issues/2408 // This currently only performs a baseline filter to ensure that routes cannot match based on namespace // criteria on a listener that cannot possibly handle them (e.g. an HTTPRoute should not be included // based on matching a filter for a UDP listener). This needs to be expanded to an allowedRoutes.kind // implementation with default allowed kinds when there's no user-specified filter. - switch obj.(type) { + var oneHostnameMatch bool + switch obj := obj.(type) { case *gatewayv1alpha2.HTTPRoute: + hostnames := obj.Spec.Hostnames + oneHostnameMatch = listenerHostnameIntersectWithRouteHostnames(listener, hostnames) if !(listener.Protocol == gatewayv1alpha2.HTTPProtocolType || listener.Protocol == gatewayv1alpha2.HTTPSProtocolType) { continue } @@ -113,12 +129,17 @@ func getSupportedGatewayForRoute(ctx context.Context, mgrc client.Client, obj cl continue } case *gatewayv1alpha2.TLSRoute: + hostnames := obj.Spec.Hostnames + oneHostnameMatch = listenerHostnameIntersectWithRouteHostnames(listener, hostnames) if listener.Protocol != gatewayv1alpha2.TLSProtocolType { continue } default: continue } + if oneHostnameMatch { + matchingHostname = metav1.ConditionTrue + } if listener.AllowedRoutes != nil { filtered = true if *listener.AllowedRoutes.Namespaces.From == gatewayv1alpha2.NamespacesFromAll { @@ -148,7 +169,25 @@ func getSupportedGatewayForRoute(ctx context.Context, mgrc client.Client, obj cl _, allowedNamespace := allowedNamespaces[obj.GetNamespace()] if !filtered || allowedNamespace { - gateways = append(gateways, &gateway) + // if there is no matchingHostname, the gateway Status Condition Accepted must be set to False + // with reason NoMatchingListenerHostname + reason := gatewayv1alpha2.RouteReasonAccepted + if matchingHostname == metav1.ConditionFalse { + reason = gatewayv1alpha2.RouteReasonNoMatchingListenerHostname + } + var listenerName string + if parentRef.SectionName != nil && *parentRef.SectionName != "" { + listenerName = string(*parentRef.SectionName) + } + gateways = append(gateways, supportedGatewayWithCondition{ + gateway: &gateway, + listenerName: listenerName, + condition: metav1.Condition{ + Type: string(gatewayv1alpha2.RouteConditionAccepted), + Status: matchingHostname, + Reason: string(reason), + }, + }) } } } @@ -161,3 +200,112 @@ func getSupportedGatewayForRoute(ctx context.Context, mgrc client.Client, obj cl return gateways, nil } + +func listenerHostnameIntersectWithRouteHostnames(listener gatewayv1alpha2.Listener, hostnames []gatewayv1alpha2.Hostname) bool { + // if the listener has no hostname, all hostnames automatically intersect + if listener.Hostname == nil || *listener.Hostname == "" || len(hostnames) == 0 { + return true + } + + // iterate over all the hostnames and check that at least one intersect with the listener hostname + for _, hostname := range hostnames { + if util.HostnamesIntersect(string(*listener.Hostname), string(hostname)) { + return true + } + } + return false +} + +// filterHostnames accepts a HTTPRoute and returns a version of the same object with only a subset of the +// hostnames, the ones matching with the listeners' hostname. +func filterHostnames(gateways []supportedGatewayWithCondition, httpRoute *gatewayv1alpha2.HTTPRoute) *gatewayv1alpha2.HTTPRoute { + filteredHostnames := make([]gatewayv1alpha2.Hostname, 0) + + // if no hostnames are specified in the route spec, get all the hostnames from + // the gateway + if len(httpRoute.Spec.Hostnames) == 0 { + for _, gateway := range gateways { + for _, listener := range gateway.gateway.Spec.Listeners { + if listenerName := gatewayv1alpha2.SectionName(gateway.listenerName); listenerName == "" || listenerName == listener.Name { + if listener.Hostname != nil { + filteredHostnames = append(filteredHostnames, *listener.Hostname) + } + } + } + } + } else { + for _, hostname := range httpRoute.Spec.Hostnames { + if hostnameMatching := getMinimumHostnameIntersection(gateways, hostname); hostnameMatching != "" { + filteredHostnames = append(filteredHostnames, hostnameMatching) + } + } + } + + httpRoute.Spec.Hostnames = filteredHostnames + return httpRoute +} + +// getMinimumHostnameIntersection returns the minimum intersecting hostname, in the sense that: +// +// - if the listener hostname is empty, return the httpRoute hostname +// - if the listener hostname acts as a wildcard for the httpRoute hostname, return the httpRoute hostname +// - if the httpRoute hostname acts as a wildcard for the listener hostname, return the listener hostname +// - if the httpRoute hostname is the same of the listener hostname, return it +// - if none of the above is true, return an empty string. +func getMinimumHostnameIntersection(gateways []supportedGatewayWithCondition, hostname gatewayv1alpha2.Hostname) gatewayv1alpha2.Hostname { + for _, gateway := range gateways { + for _, listener := range gateway.gateway.Spec.Listeners { + // if the listenerName is specified and matches the name of the gateway listener proceed + if gatewayv1alpha2.SectionName(gateway.listenerName) == "" || + gatewayv1alpha2.SectionName(gateway.listenerName) == listener.Name { + if listener.Hostname == nil || *listener.Hostname == "" { + return hostname + } + if util.HostnamesMatch(string(*listener.Hostname), string(hostname)) { + return hostname + } + if util.HostnamesMatch(string(hostname), string(*listener.Hostname)) { + return *listener.Hostname + } + } + } + } + return "" +} + +func isRouteAccepted(gateways []supportedGatewayWithCondition) bool { + for _, gateway := range gateways { + if gateway.condition.Type == string(gatewayv1alpha2.RouteConditionAccepted) && gateway.condition.Status == metav1.ConditionTrue { + return true + } + } + return false +} + +// isHTTPReferenceGranted checks that the backendRef referenced by the HTTPRoute is granted by a ReferenceGrant. +func isHTTPReferenceGranted(grantSpec gatewayv1alpha2.ReferenceGrantSpec, backendRef gatewayv1alpha2.HTTPBackendRef, fromNamespace string) bool { + var backendRefGroup gatewayv1alpha2.Group + var backendRefKind gatewayv1alpha2.Kind + + if backendRef.Group != nil { + backendRefGroup = *backendRef.Group + } + if backendRef.Kind != nil { + backendRefKind = *backendRef.Kind + } + for _, from := range grantSpec.From { + if from.Group != gatewayv1alpha2.GroupName || from.Kind != "HTTPRoute" || fromNamespace != string(from.Namespace) { + continue + } + + for _, to := range grantSpec.To { + if backendRefGroup == to.Group && + backendRefKind == to.Kind && + (to.Name == nil || *to.Name == backendRef.Name) { + return true + } + } + } + + return false +} diff --git a/internal/controllers/gateway/route_utils_test.go b/internal/controllers/gateway/route_utils_test.go new file mode 100644 index 0000000000..133a786a23 --- /dev/null +++ b/internal/controllers/gateway/route_utils_test.go @@ -0,0 +1,166 @@ +package gateway + +import ( + "testing" + + "github.com/stretchr/testify/assert" + gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" + + "github.com/kong/kubernetes-ingress-controller/v2/internal/util" +) + +func Test_filterHostnames(t *testing.T) { + commonGateway := &gatewayv1alpha2.Gateway{ + Spec: gatewayv1alpha2.GatewaySpec{ + Listeners: []gatewayv1alpha2.Listener{ + { + Name: "listener-1", + Hostname: util.StringToGatewayAPIHostnamePtr("very.specific.com"), + }, + { + Name: "listener-2", + Hostname: util.StringToGatewayAPIHostnamePtr("*.wildcard.io"), + }, + { + Name: "listener-3", + Hostname: util.StringToGatewayAPIHostnamePtr("*.anotherwildcard.io"), + }, + }, + }, + } + + testCases := []struct { + name string + gateways []supportedGatewayWithCondition + httpRoute *gatewayv1alpha2.HTTPRoute + expectedHTTPRoute *gatewayv1alpha2.HTTPRoute + }{ + { + name: "listener 1 - specific", + gateways: []supportedGatewayWithCondition{ + { + gateway: commonGateway, + listenerName: "listener-1", + }, + }, + httpRoute: &gatewayv1alpha2.HTTPRoute{ + Spec: gatewayv1alpha2.HTTPRouteSpec{ + Hostnames: []gatewayv1alpha2.Hostname{ + util.StringToGatewayAPIHostname("*.anotherwildcard.io"), + util.StringToGatewayAPIHostname("*.nonmatchingwildcard.io"), + util.StringToGatewayAPIHostname("very.specific.com"), + }, + }, + }, + expectedHTTPRoute: &gatewayv1alpha2.HTTPRoute{ + Spec: gatewayv1alpha2.HTTPRouteSpec{ + Hostnames: []gatewayv1alpha2.Hostname{ + util.StringToGatewayAPIHostname("very.specific.com"), + }, + }, + }, + }, + { + name: "listener 1 - wildcard", + gateways: []supportedGatewayWithCondition{ + { + gateway: commonGateway, + listenerName: "listener-1", + }, + }, + httpRoute: &gatewayv1alpha2.HTTPRoute{ + Spec: gatewayv1alpha2.HTTPRouteSpec{ + Hostnames: []gatewayv1alpha2.Hostname{ + util.StringToGatewayAPIHostname("non.matching.com"), + util.StringToGatewayAPIHostname("*.specific.com"), + }, + }, + }, + expectedHTTPRoute: &gatewayv1alpha2.HTTPRoute{ + Spec: gatewayv1alpha2.HTTPRouteSpec{ + Hostnames: []gatewayv1alpha2.Hostname{ + util.StringToGatewayAPIHostname("very.specific.com"), + }, + }, + }, + }, + { + name: "listener 2", + gateways: []supportedGatewayWithCondition{ + { + gateway: commonGateway, + listenerName: "listener-2", + }, + }, + httpRoute: &gatewayv1alpha2.HTTPRoute{ + Spec: gatewayv1alpha2.HTTPRouteSpec{ + Hostnames: []gatewayv1alpha2.Hostname{ + util.StringToGatewayAPIHostname("non.matching.com"), + util.StringToGatewayAPIHostname("wildcard.io"), + util.StringToGatewayAPIHostname("foo.wildcard.io"), + util.StringToGatewayAPIHostname("bar.wildcard.io"), + util.StringToGatewayAPIHostname("foo.bar.wildcard.io"), + }, + }, + }, + expectedHTTPRoute: &gatewayv1alpha2.HTTPRoute{ + Spec: gatewayv1alpha2.HTTPRouteSpec{ + Hostnames: []gatewayv1alpha2.Hostname{ + util.StringToGatewayAPIHostname("foo.wildcard.io"), + util.StringToGatewayAPIHostname("bar.wildcard.io"), + util.StringToGatewayAPIHostname("foo.bar.wildcard.io"), + }, + }, + }, + }, + { + name: "listener 3 - wildcard", + gateways: []supportedGatewayWithCondition{ + { + gateway: commonGateway, + listenerName: "listener-3", + }, + }, + httpRoute: &gatewayv1alpha2.HTTPRoute{ + Spec: gatewayv1alpha2.HTTPRouteSpec{ + Hostnames: []gatewayv1alpha2.Hostname{ + util.StringToGatewayAPIHostname("*.anotherwildcard.io"), + }, + }, + }, + expectedHTTPRoute: &gatewayv1alpha2.HTTPRoute{ + Spec: gatewayv1alpha2.HTTPRouteSpec{ + Hostnames: []gatewayv1alpha2.Hostname{ + util.StringToGatewayAPIHostname("*.anotherwildcard.io"), + }, + }, + }, + }, + { + name: "no match", + gateways: []supportedGatewayWithCondition{ + { + gateway: commonGateway, + }, + }, + httpRoute: &gatewayv1alpha2.HTTPRoute{ + Spec: gatewayv1alpha2.HTTPRouteSpec{ + Hostnames: []gatewayv1alpha2.Hostname{ + util.StringToGatewayAPIHostname("specific.but.wrong.com"), + util.StringToGatewayAPIHostname("wildcard.io"), + }, + }, + }, + expectedHTTPRoute: &gatewayv1alpha2.HTTPRoute{ + Spec: gatewayv1alpha2.HTTPRouteSpec{ + Hostnames: []gatewayv1alpha2.Hostname{}, + }, + }, + }, + } + + for _, tc := range testCases { + filteredHTTPRoute := filterHostnames(tc.gateways, tc.httpRoute) + assert.Equal(t, tc.expectedHTTPRoute.Spec, filteredHTTPRoute.Spec, tc.name) + } +} diff --git a/internal/controllers/gateway/tcproute_controller.go b/internal/controllers/gateway/tcproute_controller.go index 215e0e4512..f1d6697253 100644 --- a/internal/controllers/gateway/tcproute_controller.go +++ b/internal/controllers/gateway/tcproute_controller.go @@ -21,6 +21,7 @@ import ( gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" "github.com/kong/kubernetes-ingress-controller/v2/internal/dataplane" + "github.com/kong/kubernetes-ingress-controller/v2/internal/util" ) // ----------------------------------------------------------------------------- @@ -299,7 +300,7 @@ func (r *TCPRouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c // requeue the object and wait until all supported gateways are ready. debug(log, tcproute, "checking if the tcproute's gateways are ready") for _, gateway := range gateways { - if !isGatewayReady(gateway) { + if !isGatewayReady(gateway.gateway) { debug(log, tcproute, "gateway for route was not ready, waiting") return ctrl.Result{Requeue: true}, nil } @@ -352,7 +353,7 @@ var tcprouteParentKind = "Gateway" // ensureGatewayReferenceStatus takes any number of Gateways that should be // considered "attached" to a given TCPRoute and ensures that the status // for the TCPRoute is updated appropriately. -func (r *TCPRouteReconciler) ensureGatewayReferenceStatusAdded(ctx context.Context, tcproute *gatewayv1alpha2.TCPRoute, gateways ...*gatewayv1alpha2.Gateway) (bool, error) { +func (r *TCPRouteReconciler) ensureGatewayReferenceStatusAdded(ctx context.Context, tcproute *gatewayv1alpha2.TCPRoute, gateways ...supportedGatewayWithCondition) (bool, error) { // map the existing parentStatues to avoid duplications parentStatuses := make(map[string]*gatewayv1alpha2.RouteParentStatus) for _, existingParent := range tcproute.Status.Parents { @@ -371,9 +372,9 @@ func (r *TCPRouteReconciler) ensureGatewayReferenceStatusAdded(ctx context.Conte gatewayParentStatus := &gatewayv1alpha2.RouteParentStatus{ ParentRef: gatewayv1alpha2.ParentReference{ Group: (*gatewayv1alpha2.Group)(&gatewayv1alpha2.GroupVersion.Group), - Kind: (*gatewayv1alpha2.Kind)(&tcprouteParentKind), - Namespace: (*gatewayv1alpha2.Namespace)(&gateway.Namespace), - Name: gatewayv1alpha2.ObjectName(gateway.Name), + Kind: util.StringToGatewayAPIKindPtr(tcprouteParentKind), + Namespace: (*gatewayv1alpha2.Namespace)(&gateway.gateway.Namespace), + Name: gatewayv1alpha2.ObjectName(gateway.gateway.Name), }, ControllerName: ControllerName, Conditions: []metav1.Condition{{ @@ -381,13 +382,13 @@ func (r *TCPRouteReconciler) ensureGatewayReferenceStatusAdded(ctx context.Conte Status: metav1.ConditionTrue, ObservedGeneration: tcproute.Generation, LastTransitionTime: metav1.Now(), - Reason: string(gatewayv1alpha2.GatewayReasonReady), + Reason: string(gatewayv1alpha2.RouteReasonAccepted), }}, } // if the reference already exists and doesn't require any changes // then just leave it alone. - if existingGatewayParentStatus, exists := parentStatuses[gateway.Namespace+gateway.Name]; exists { + if existingGatewayParentStatus, exists := parentStatuses[gateway.gateway.Namespace+gateway.gateway.Name]; exists { // fake the time of the existing status as this wont be equal for i := range existingGatewayParentStatus.Conditions { existingGatewayParentStatus.Conditions[i].LastTransitionTime = gatewayParentStatus.Conditions[0].LastTransitionTime @@ -400,7 +401,7 @@ func (r *TCPRouteReconciler) ensureGatewayReferenceStatusAdded(ctx context.Conte } // otherwise overlay the new status on top the list of parentStatuses - parentStatuses[gateway.Namespace+gateway.Name] = gatewayParentStatus + parentStatuses[gateway.gateway.Namespace+gateway.gateway.Name] = gatewayParentStatus statusChangesWereMade = true } diff --git a/internal/controllers/gateway/tlsroute_controller.go b/internal/controllers/gateway/tlsroute_controller.go index 225100b0d2..4b16290f47 100644 --- a/internal/controllers/gateway/tlsroute_controller.go +++ b/internal/controllers/gateway/tlsroute_controller.go @@ -21,6 +21,7 @@ import ( gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" "github.com/kong/kubernetes-ingress-controller/v2/internal/dataplane" + "github.com/kong/kubernetes-ingress-controller/v2/internal/util" ) // ----------------------------------------------------------------------------- @@ -299,7 +300,7 @@ func (r *TLSRouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c // requeue the object and wait until all supported gateways are ready. debug(log, tlsroute, "checking if the tlsroute's gateways are ready") for _, gateway := range gateways { - if !isGatewayReady(gateway) { + if !isGatewayReady(gateway.gateway) { debug(log, tlsroute, "gateway for route was not ready, waiting") return ctrl.Result{Requeue: true}, nil } @@ -352,7 +353,7 @@ var tlsrouteParentKind = "Gateway" // ensureGatewayReferenceStatus takes any number of Gateways that should be // considered "attached" to a given TLSRoute and ensures that the status // for the TLSRoute is updated appropriately. -func (r *TLSRouteReconciler) ensureGatewayReferenceStatusAdded(ctx context.Context, tlsroute *gatewayv1alpha2.TLSRoute, gateways ...*gatewayv1alpha2.Gateway) (bool, error) { +func (r *TLSRouteReconciler) ensureGatewayReferenceStatusAdded(ctx context.Context, tlsroute *gatewayv1alpha2.TLSRoute, gateways ...supportedGatewayWithCondition) (bool, error) { // map the existing parentStatues to avoid duplications parentStatuses := make(map[string]*gatewayv1alpha2.RouteParentStatus) for _, existingParent := range tlsroute.Status.Parents { @@ -371,9 +372,9 @@ func (r *TLSRouteReconciler) ensureGatewayReferenceStatusAdded(ctx context.Conte gatewayParentStatus := &gatewayv1alpha2.RouteParentStatus{ ParentRef: gatewayv1alpha2.ParentReference{ Group: (*gatewayv1alpha2.Group)(&gatewayv1alpha2.GroupVersion.Group), - Kind: (*gatewayv1alpha2.Kind)(&tlsrouteParentKind), - Namespace: (*gatewayv1alpha2.Namespace)(&gateway.Namespace), - Name: gatewayv1alpha2.ObjectName(gateway.Name), + Kind: util.StringToGatewayAPIKindPtr(tlsrouteParentKind), + Namespace: (*gatewayv1alpha2.Namespace)(&gateway.gateway.Namespace), + Name: gatewayv1alpha2.ObjectName(gateway.gateway.Name), }, ControllerName: ControllerName, Conditions: []metav1.Condition{{ @@ -381,13 +382,13 @@ func (r *TLSRouteReconciler) ensureGatewayReferenceStatusAdded(ctx context.Conte Status: metav1.ConditionTrue, ObservedGeneration: tlsroute.Generation, LastTransitionTime: metav1.Now(), - Reason: string(gatewayv1alpha2.GatewayReasonReady), + Reason: string(gatewayv1alpha2.RouteReasonAccepted), }}, } // if the reference already exists and doesn't require any changes // then just leave it alone. - if existingGatewayParentStatus, exists := parentStatuses[gateway.Namespace+gateway.Name]; exists { + if existingGatewayParentStatus, exists := parentStatuses[gateway.gateway.Namespace+gateway.gateway.Name]; exists { // fake the time of the existing status as this wont be equal for i := range existingGatewayParentStatus.Conditions { existingGatewayParentStatus.Conditions[i].LastTransitionTime = gatewayParentStatus.Conditions[0].LastTransitionTime @@ -400,7 +401,7 @@ func (r *TLSRouteReconciler) ensureGatewayReferenceStatusAdded(ctx context.Conte } // otherwise overlay the new status on top the list of parentStatuses - parentStatuses[gateway.Namespace+gateway.Name] = gatewayParentStatus + parentStatuses[gateway.gateway.Namespace+gateway.gateway.Name] = gatewayParentStatus statusChangesWereMade = true } diff --git a/internal/controllers/gateway/udproute_controller.go b/internal/controllers/gateway/udproute_controller.go index 5b6c841294..97a1234dc0 100644 --- a/internal/controllers/gateway/udproute_controller.go +++ b/internal/controllers/gateway/udproute_controller.go @@ -21,6 +21,7 @@ import ( gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" "github.com/kong/kubernetes-ingress-controller/v2/internal/dataplane" + "github.com/kong/kubernetes-ingress-controller/v2/internal/util" ) // ----------------------------------------------------------------------------- @@ -299,7 +300,7 @@ func (r *UDPRouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c // requeue the object and wait until all supported gateways are ready. debug(log, udproute, "checking if the udproute's gateways are ready") for _, gateway := range gateways { - if !isGatewayReady(gateway) { + if !isGatewayReady(gateway.gateway) { debug(log, udproute, "gateway for route was not ready, waiting") return ctrl.Result{Requeue: true}, nil } @@ -352,7 +353,7 @@ var udprouteParentKind = "Gateway" // ensureGatewayReferenceStatus takes any number of Gateways that should be // considered "attached" to a given UDPRoute and ensures that the status // for the UDPRoute is updated appropriately. -func (r *UDPRouteReconciler) ensureGatewayReferenceStatusAdded(ctx context.Context, udproute *gatewayv1alpha2.UDPRoute, gateways ...*gatewayv1alpha2.Gateway) (bool, error) { +func (r *UDPRouteReconciler) ensureGatewayReferenceStatusAdded(ctx context.Context, udproute *gatewayv1alpha2.UDPRoute, gateways ...supportedGatewayWithCondition) (bool, error) { // map the existing parentStatues to avoid duplications parentStatuses := make(map[string]*gatewayv1alpha2.RouteParentStatus) for _, existingParent := range udproute.Status.Parents { @@ -371,9 +372,9 @@ func (r *UDPRouteReconciler) ensureGatewayReferenceStatusAdded(ctx context.Conte gatewayParentStatus := &gatewayv1alpha2.RouteParentStatus{ ParentRef: gatewayv1alpha2.ParentReference{ Group: (*gatewayv1alpha2.Group)(&gatewayv1alpha2.GroupVersion.Group), - Kind: (*gatewayv1alpha2.Kind)(&udprouteParentKind), - Namespace: (*gatewayv1alpha2.Namespace)(&gateway.Namespace), - Name: gatewayv1alpha2.ObjectName(gateway.Name), + Kind: util.StringToGatewayAPIKindPtr(udprouteParentKind), + Namespace: (*gatewayv1alpha2.Namespace)(&gateway.gateway.Namespace), + Name: gatewayv1alpha2.ObjectName(gateway.gateway.Name), }, ControllerName: ControllerName, Conditions: []metav1.Condition{{ @@ -381,13 +382,13 @@ func (r *UDPRouteReconciler) ensureGatewayReferenceStatusAdded(ctx context.Conte Status: metav1.ConditionTrue, ObservedGeneration: udproute.Generation, LastTransitionTime: metav1.Now(), - Reason: string(gatewayv1alpha2.GatewayReasonReady), + Reason: string(gatewayv1alpha2.RouteReasonAccepted), }}, } // if the reference already exists and doesn't require any changes // then just leave it alone. - if existingGatewayParentStatus, exists := parentStatuses[gateway.Namespace+gateway.Name]; exists { + if existingGatewayParentStatus, exists := parentStatuses[gateway.gateway.Namespace+gateway.gateway.Name]; exists { // fake the time of the existing status as this wont be equal for i := range existingGatewayParentStatus.Conditions { existingGatewayParentStatus.Conditions[i].LastTransitionTime = gatewayParentStatus.Conditions[0].LastTransitionTime @@ -400,7 +401,7 @@ func (r *UDPRouteReconciler) ensureGatewayReferenceStatusAdded(ctx context.Conte } // otherwise overlay the new status on top the list of parentStatuses - parentStatuses[gateway.Namespace+gateway.Name] = gatewayParentStatus + parentStatuses[gateway.gateway.Namespace+gateway.gateway.Name] = gatewayParentStatus statusChangesWereMade = true } diff --git a/internal/dataplane/parser/parser.go b/internal/dataplane/parser/parser.go index 070f38eccf..effbfe30ca 100644 --- a/internal/dataplane/parser/parser.go +++ b/internal/dataplane/parser/parser.go @@ -61,7 +61,7 @@ func NewParser( // ----------------------------------------------------------------------------- // Build creates a Kong configuration from Ingress and Custom resources -// defined in Kuberentes. +// defined in Kubernetes. // It throws an error if there is an error returned from client-go. func (p *Parser) Build() (*kongstate.KongState, error) { // parse and merge all rules together from all Kubernetes API sources diff --git a/internal/dataplane/parser/translate_httproute.go b/internal/dataplane/parser/translate_httproute.go index dc810edeab..f32d5755fd 100644 --- a/internal/dataplane/parser/translate_httproute.go +++ b/internal/dataplane/parser/translate_httproute.go @@ -47,7 +47,7 @@ func (p *Parser) ingressRulesFromHTTPRoutes() ingressRules { } func (p *Parser) ingressRulesFromHTTPRoute(result *ingressRules, httproute *gatewayv1alpha2.HTTPRoute) error { - // first we grab the spec and gather some metdata about the object + // first we grab the spec and gather some metadata about the object spec := httproute.Spec // validation for HTTPRoutes will happen at a higher layer, but in spite of that we run @@ -125,6 +125,9 @@ func generateKongRoutesFromHTTPRouteRule(httproute *gatewayv1alpha2.HTTPRoute, r // Therefore we treat each match rule as a separate Kong Route, so we iterate through // all matches to determine all the routes that will be needed for the services. var routes []kongstate.Route + + // generate kong plugins from rule.filters + plugins := generatePluginsFromHTTPRouteRuleFilters(rule) if len(rule.Matches) > 0 { for matchNumber, match := range rule.Matches { // determine the name of the route, identify it as a route that belongs @@ -137,7 +140,7 @@ func generateKongRoutesFromHTTPRouteRule(httproute *gatewayv1alpha2.HTTPRoute, r matchNumber, )) - // TODO: implement query param matches + // TODO: implement query param matches (https://github.com/Kong/kubernetes-ingress-controller/issues/2778) if len(match.QueryParams) > 0 { return nil, fmt.Errorf("query param matches are not yet supported") } @@ -185,6 +188,17 @@ func generateKongRoutesFromHTTPRouteRule(httproute *gatewayv1alpha2.HTTPRoute, r r.Route.Headers = headers } + // stripPath needs to be disabled by default to be conformant with the Gateway API + r.StripPath = kong.Bool(false) + + // attach the plugins to be applied to the given route + if len(plugins) != 0 { + if r.Plugins == nil { + r.Plugins = make([]kong.Plugin, 0, len(plugins)) + } + r.Plugins = append(r.Plugins, plugins...) + } + // add the route to the list of routes for the service(s) routes = append(routes, r) } @@ -210,8 +224,79 @@ func generateKongRoutesFromHTTPRouteRule(httproute *gatewayv1alpha2.HTTPRoute, r // otherwise apply the hostnames to the route r.Hosts = append(r.Hosts, hostnames...) + // attach the plugins to be applied to the given route + if len(plugins) != 0 { + if r.Plugins == nil { + r.Plugins = make([]kong.Plugin, 0, len(plugins)) + } + r.Plugins = append(r.Plugins, plugins...) + } routes = append(routes, r) } return routes, nil } + +// generatePluginsFromHTTPRouteRuleFilters accepts a rule as argument and converts +// HttpRouteRule.Filters into Kong filters. +func generatePluginsFromHTTPRouteRuleFilters(rule gatewayv1alpha2.HTTPRouteRule) []kong.Plugin { + kongPlugins := make([]kong.Plugin, 0) + if rule.Filters == nil { + return kongPlugins + } + + for _, filter := range rule.Filters { + if filter.Type == gatewayv1alpha2.HTTPRouteFilterRequestHeaderModifier { + kongPlugins = append(kongPlugins, generateRequestHeaderModifierKongPlugin(filter.RequestHeaderModifier)) + } + // TODO: https://github.com/Kong/kubernetes-ingress-controller/issues/2793 + } + + return kongPlugins +} + +// generateRequestHeaderModifierKongPlugin converts a gatewayv1alpha2.HTTPRequestHeaderFilter into a +// kong.Plugin of type request-transformer. +func generateRequestHeaderModifierKongPlugin(modifier *gatewayv1alpha2.HTTPRequestHeaderFilter) kong.Plugin { + plugin := kong.Plugin{ + Name: kong.String("request-transformer"), + Config: make(kong.Configuration), + } + + // modifier.Set is converted to a pair composed of "replace" and "add" + if modifier.Set != nil { + setModifiers := make([]string, 0, len(modifier.Set)) + for _, s := range modifier.Set { + setModifiers = append(setModifiers, kongHeaderFormatter(s)) + } + plugin.Config["replace"] = map[string][]string{ + "headers": setModifiers, + } + plugin.Config["add"] = map[string][]string{ + "headers": setModifiers, + } + } + + // modifier.Add is converted to "append" + if modifier.Add != nil { + appendModifiers := make([]string, 0, len(modifier.Add)) + for _, a := range modifier.Add { + appendModifiers = append(appendModifiers, kongHeaderFormatter(a)) + } + plugin.Config["append"] = map[string][]string{ + "headers": appendModifiers, + } + } + + if modifier.Remove != nil { + plugin.Config["remove"] = map[string][]string{ + "headers": modifier.Remove, + } + } + + return plugin +} + +func kongHeaderFormatter(header gatewayv1alpha2.HTTPHeader) string { + return fmt.Sprintf("%s:%s", header.Name, header.Value) +} diff --git a/internal/dataplane/parser/translate_httproute_test.go b/internal/dataplane/parser/translate_httproute_test.go index f46459c87e..0a4e7c9221 100644 --- a/internal/dataplane/parser/translate_httproute_test.go +++ b/internal/dataplane/parser/translate_httproute_test.go @@ -10,6 +10,7 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/utils/pointer" gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" "github.com/kong/kubernetes-ingress-controller/v2/internal/dataplane/kongstate" @@ -72,6 +73,7 @@ func Test_ingressRulesFromHTTPRoutes(t *testing.T) { BackendObjectReference: gatewayv1alpha2.BackendObjectReference{ Name: gatewayv1alpha2.ObjectName("fake-service"), Port: &httpPort, + Kind: util.StringToGatewayAPIKindPtr("Service"), }, }, }}, @@ -144,6 +146,7 @@ func Test_ingressRulesFromHTTPRoutes(t *testing.T) { BackendObjectReference: gatewayv1alpha2.BackendObjectReference{ Name: gatewayv1alpha2.ObjectName("fake-service"), Port: &httpPort, + Kind: util.StringToGatewayAPIKindPtr("Service"), }, }, }, @@ -224,6 +227,7 @@ func Test_ingressRulesFromHTTPRoutes(t *testing.T) { BackendObjectReference: gatewayv1alpha2.BackendObjectReference{ Name: gatewayv1alpha2.ObjectName("fake-service"), Port: &httpPort, + Kind: util.StringToGatewayAPIKindPtr("Service"), }, }, }}, @@ -262,6 +266,7 @@ func Test_ingressRulesFromHTTPRoutes(t *testing.T) { kong.String("http"), kong.String("https"), }, + StripPath: pointer.BoolPtr(false), }, Ingress: util.K8sObjectInfo{ Name: "basic-httproute", @@ -299,6 +304,7 @@ func Test_ingressRulesFromHTTPRoutes(t *testing.T) { BackendObjectReference: gatewayv1alpha2.BackendObjectReference{ Name: gatewayv1alpha2.ObjectName("fake-service"), Port: &httpPort, + Kind: util.StringToGatewayAPIKindPtr("Service"), }, }, }, @@ -368,6 +374,7 @@ func Test_ingressRulesFromHTTPRoutes(t *testing.T) { BackendObjectReference: gatewayv1alpha2.BackendObjectReference{ Name: gatewayv1alpha2.ObjectName("fake-service"), Port: &httpPort, + Kind: util.StringToGatewayAPIKindPtr("Service"), }, }, }}, @@ -407,6 +414,7 @@ func Test_ingressRulesFromHTTPRoutes(t *testing.T) { BackendObjectReference: gatewayv1alpha2.BackendObjectReference{ Name: gatewayv1alpha2.ObjectName("fake-service"), Port: &httpPort, + Kind: util.StringToGatewayAPIKindPtr("Service"), }, }, }}, @@ -445,6 +453,7 @@ func Test_ingressRulesFromHTTPRoutes(t *testing.T) { kong.String("http"), kong.String("https"), }, + StripPath: pointer.BoolPtr(false), }, Ingress: util.K8sObjectInfo{ Name: "basic-httproute", @@ -482,6 +491,7 @@ func Test_ingressRulesFromHTTPRoutes(t *testing.T) { BackendObjectReference: gatewayv1alpha2.BackendObjectReference{ Name: gatewayv1alpha2.ObjectName("fake-service"), Port: &httpPort, + Kind: util.StringToGatewayAPIKindPtr("Service"), }, }, }, @@ -527,6 +537,7 @@ func Test_ingressRulesFromHTTPRoutes(t *testing.T) { BackendObjectReference: gatewayv1alpha2.BackendObjectReference{ Name: gatewayv1alpha2.ObjectName("fake-service"), Port: &httpPort, + Kind: util.StringToGatewayAPIKindPtr("Service"), }, }, }}, @@ -565,6 +576,7 @@ func Test_ingressRulesFromHTTPRoutes(t *testing.T) { kong.String("http"), kong.String("https"), }, + StripPath: pointer.BoolPtr(false), }, Ingress: util.K8sObjectInfo{ Name: "basic-httproute", @@ -602,6 +614,7 @@ func Test_ingressRulesFromHTTPRoutes(t *testing.T) { BackendObjectReference: gatewayv1alpha2.BackendObjectReference{ Name: gatewayv1alpha2.ObjectName("fake-service"), Port: &httpPort, + Kind: util.StringToGatewayAPIKindPtr("Service"), }, }, }, diff --git a/internal/dataplane/parser/translate_utils.go b/internal/dataplane/parser/translate_utils.go index c6a2d733b9..c08f5516f5 100644 --- a/internal/dataplane/parser/translate_utils.go +++ b/internal/dataplane/parser/translate_utils.go @@ -131,7 +131,8 @@ func (p *Parser) generateKongServiceFromBackendRef( }, grants) for _, backendRef := range backendRefs { - if isRefAllowedByGrant(backendRef.Namespace, backendRef.Name, backendRef.Group, backendRef.Kind, allowed) { + if util.IsBackendRefGroupKindSupported(backendRef.Group, backendRef.Kind) && + isRefAllowedByGrant(backendRef.Namespace, backendRef.Name, backendRef.Group, backendRef.Kind, allowed) { backend := kongstate.ServiceBackend{ Name: string(backendRef.Name), PortDef: kongstate.PortDef{ @@ -149,17 +150,18 @@ func (p *Parser) generateKongServiceFromBackendRef( // these, we do not want a single impermissible ref to take the entire rule offline. in the case of edits, // failing the entire rule could potentially delete routes that were previously online and in use, and // that remain viable (because they still have some permissible backendRefs) - p.logger.Errorf("%s requested backendRef to %s %s/%s, but no ReferenceGrant permits it, skipping...", - objName, *backendRef.Kind, *backendRef.Namespace, backendRef.Name) + var namespace, kind string = route.GetNamespace(), "" + if backendRef.Namespace != nil { + namespace = string(*backendRef.Namespace) + } + if backendRef.Kind != nil { + kind = string(*backendRef.Kind) + } + p.logger.Errorf("%s requested backendRef to %s %s/%s, but no ReferencePolicy permits it, skipping...", + objName, kind, namespace, backendRef.Name) } } - // however, if there are _no_ permissible backendRefs, the route will not be able to forward any traffic and we - // should reject it - if len(backends) == 0 { - return kongstate.Service{}, fmt.Errorf("%s has no permissible backendRefs, cannot create a Kong service for it", objName) - } - // the service name needs to uniquely identify this service given it's list of // one or more backends. serviceName := fmt.Sprintf("%s.%d", getUniqueKongServiceNameForObject(route), ruleNumber) @@ -187,5 +189,22 @@ func (p *Parser) generateKongServiceFromBackendRef( } } + // In the context of the gateway API conformance tests, if there is no service for the backend, + // the response must have a status code of 500. Since The default behavior of Kong is returning 503 + // if there is no backend for a service, we inject a plugin that terminates all requests with 500 + // as status code + if len(service.Backends) == 0 { + if service.Plugins == nil { + service.Plugins = make([]kong.Plugin, 0) + } + service.Plugins = append(service.Plugins, kong.Plugin{ + Name: kong.String("request-termination"), + Config: kong.Configuration{ + "status_code": 500, + "message": "no existing backendRef provided", + }, + }) + } + return service, nil } diff --git a/internal/dataplane/parser/translate_utils_test.go b/internal/dataplane/parser/translate_utils_test.go index 4f21e44ef8..c0eaeee64b 100644 --- a/internal/dataplane/parser/translate_utils_test.go +++ b/internal/dataplane/parser/translate_utils_test.go @@ -687,8 +687,39 @@ func Test_generateKongServiceFromBackendRef(t *testing.T) { }, }, }, - result: kongstate.Service{}, - wantErr: true, + result: kongstate.Service{ + Service: kong.Service{ + Name: kong.String("tcproute.behbudiy.kitab-ul-atfol.999"), + Host: kong.String("tcproute.behbudiy.kitab-ul-atfol.999"), + Protocol: kong.String(protocol), + ConnectTimeout: kong.Int(DefaultServiceTimeout), + ReadTimeout: kong.Int(DefaultServiceTimeout), + WriteTimeout: kong.Int(DefaultServiceTimeout), + Retries: kong.Int(DefaultRetries), + }, + Namespace: "behbudiy", + Backends: []kongstate.ServiceBackend{}, + Plugins: []kong.Plugin{ + { + Name: kong.String("request-termination"), + Config: kong.Configuration{ + "status_code": 500, + "message": "no existing backendRef provided", + }, + }, + }, + Parent: &gatewayv1alpha2.TCPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "kitab-ul-atfol", + Namespace: "behbudiy", + }, + TypeMeta: metav1.TypeMeta{ + Kind: "TCPRoute", + APIVersion: "gateway.networking.k8s.io/v1alpha2", + }, + }, + }, + wantErr: false, }, { msg: "same and different ns backend", diff --git a/internal/util/conversions.go b/internal/util/conversions.go new file mode 100644 index 0000000000..5d5a394470 --- /dev/null +++ b/internal/util/conversions.go @@ -0,0 +1,30 @@ +package util + +import ( + "k8s.io/utils/pointer" + gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" +) + +// ----------------------------------------------------------------------------- +// Type conversion Utilities +// ----------------------------------------------------------------------------- + +// StringToGatewayAPIHostname converts a string to a gatewayv1alpha2.Hostname. +func StringToGatewayAPIHostname(hostname string) gatewayv1alpha2.Hostname { + return (gatewayv1alpha2.Hostname)(hostname) +} + +// StringToGatewayAPIHostnamePtr converts a string to a *gatewayv1alpha2.Hostname. +func StringToGatewayAPIHostnamePtr(hostname string) *gatewayv1alpha2.Hostname { + return (*gatewayv1alpha2.Hostname)(pointer.StringPtr(hostname)) +} + +// StringToGatewayAPIKind converts a string to a gatewayv1alpha2.Kind. +func StringToGatewayAPIKind(kind string) gatewayv1alpha2.Kind { + return (gatewayv1alpha2.Kind)(kind) +} + +// StringToGatewayAPIKindPtr converts a string to a *gatewayv1alpha2.Kind. +func StringToGatewayAPIKindPtr(kind string) *gatewayv1alpha2.Kind { + return (*gatewayv1alpha2.Kind)(pointer.StringPtr(kind)) +} diff --git a/internal/util/hostname.go b/internal/util/hostname.go new file mode 100644 index 0000000000..78be7043a9 --- /dev/null +++ b/internal/util/hostname.go @@ -0,0 +1,58 @@ +package util + +import "strings" + +// HostnamesIntersect checks if the hostnameA and hostnameB have an intersection. +// To perform this check, the function HostnamesMatch is called twice swapping the +// parameters and using first hostnameA as a mask, then hostnameB. +// If there is at least one match, the hostnames intersect. +func HostnamesIntersect(hostnameA, hostnameB string) bool { + return HostnamesMatch(hostnameA, hostnameB) || HostnamesMatch(hostnameB, hostnameA) +} + +// HostnamesMatch checks that the hostnameB matches the hostnameA. HostnameA is treated as mask +// to be checked against the hostnameB. +func HostnamesMatch(hostnameA, hostnameB string) bool { + // the hostnames are in the form of "foo.bar.com"; split them + // in a slice of substrings + hostnameALabels := strings.Split(hostnameA, ".") + hostnameBLabels := strings.Split(hostnameB, ".") + + var a, b int + var wildcard bool + + // iterate over the parts of both the hostnames + for a, b = 0, 0; a < len(hostnameALabels) && b < len(hostnameBLabels); a, b = a+1, b+1 { + var matchFound bool + + // if the current part of B is a wildcard, we need to find the first + // A part that matches with the following B part + if wildcard { + for ; b < len(hostnameBLabels); b++ { + if hostnameALabels[a] == hostnameBLabels[b] { + matchFound = true + break + } + } + } + + // if no match was found, the hostnames don't match + if wildcard && !matchFound { + return false + } + + // check if at least on of the current parts are a wildcard; if so, continue + if hostnameALabels[a] == "*" { + wildcard = true + continue + } + // reset the wildcard variables + wildcard = false + + // if the current a part is different from the b part, the hostnames are incompatible + if hostnameALabels[a] != hostnameBLabels[b] { + return false + } + } + return len(hostnameBLabels)-b == len(hostnameALabels)-a +} diff --git a/internal/util/hostname_test.go b/internal/util/hostname_test.go new file mode 100644 index 0000000000..7a21d57348 --- /dev/null +++ b/internal/util/hostname_test.go @@ -0,0 +1,101 @@ +package util_test + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/kong/kubernetes-ingress-controller/v2/internal/util" +) + +func TestIntersection(t *testing.T) { + for _, tt := range []struct { + name string + pattern string + value string + expected bool + }{ + { + name: "same hostname", + pattern: "test.com", + value: "test.com", + expected: true, + }, + { + name: "different hostname suffix", + pattern: "test.com", + value: "test.net", + expected: false, + }, + { + name: "different hostname prefix", + pattern: "foo.com", + value: "bar.com", + expected: false, + }, + { + name: "different hostname lengths with only suffix matching", + pattern: "foo.test.com", + value: "test.com", + expected: false, + }, + { + name: "different hostname lengths with only prefix matching", + pattern: "foo.test.com", + value: "foo.net", + expected: false, + }, + { + name: "valid wildcard for one element", + pattern: "*.test.com", + value: "foo.test.com", + expected: true, + }, + { + name: "valid wildcard for many elements", + pattern: "*.example.com", + value: "so.many.names.example.com", + expected: true, + }, + { + name: "not matching wildcard", + pattern: "*.example.com", + value: "example.com", + expected: false, + }, + { + name: "double matching wildcard", + pattern: "*.example.com", + value: "*.example.com", + expected: true, + }, + { + name: "double not matching wildcard", + pattern: "*.example.com", + value: "*.example.net", + expected: false, + }, + { + name: "double matching wildcard with different sizes", + pattern: "*.test.example.com", + value: "*.example.com", + expected: true, + }, + { + name: "double wildcard with different sizes and not matching suffix", + pattern: "*.test.example.com", + value: "*.test.net", + expected: false, + }, + { + name: "double wildcard with different sizes and not matching prefix", + pattern: "*.test.example.com", + value: "*.example.net", + expected: false, + }, + } { + // Test that the functions behave in the same way even swapping the parameters + assert.Equal(t, tt.expected, util.HostnamesIntersect(tt.pattern, tt.value), tt.name) + assert.Equal(t, tt.expected, util.HostnamesIntersect(tt.value, tt.pattern), tt.name) + } +} diff --git a/internal/util/k8s.go b/internal/util/k8s.go index 08c7a8f17c..acfe0fa92b 100644 --- a/internal/util/k8s.go +++ b/internal/util/k8s.go @@ -25,6 +25,7 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" clientset "k8s.io/client-go/kubernetes" + gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" ) // ParseNameNS parses a string searching a namespace and name. @@ -104,3 +105,26 @@ func GetPodDetails(ctx context.Context, kubeClient clientset.Interface) (*PodInf Labels: pod.GetLabels(), }, nil } + +// map of all the supported Group/Kinds for the backend. At the moment, only +// core services are supported, but to provide support to other kinds, it is +// enough to add entries to this map. +var backendRefSupportedGroupKinds = map[string]struct{}{ + "core/Service": {}, +} + +// IsBackendRefGroupKindSupported checks if the GroupKind of the object used as +// BackendRef for the HTTPRoute is supported. +func IsBackendRefGroupKindSupported(gatewayAPIGroup *gatewayv1alpha2.Group, gatewayAPIKind *gatewayv1alpha2.Kind) bool { + if gatewayAPIKind == nil { + return false + } + + group := "core" + if gatewayAPIGroup != nil && *gatewayAPIGroup != "" { + group = string(*gatewayAPIGroup) + } + + _, ok := backendRefSupportedGroupKinds[fmt.Sprintf("%s/%s", group, *gatewayAPIKind)] + return ok +} diff --git a/test/conformance/gateway_conformance_test.go b/test/conformance/gateway_conformance_test.go index 33496926e9..c02f39b79d 100644 --- a/test/conformance/gateway_conformance_test.go +++ b/test/conformance/gateway_conformance_test.go @@ -10,7 +10,6 @@ import ( "github.com/google/uuid" "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/util/sets" "sigs.k8s.io/controller-runtime/pkg/client" gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" "sigs.k8s.io/gateway-api/conformance/tests" @@ -58,6 +57,8 @@ func TestGatewayConformance(t *testing.T) { BaseManifests: conformanceTestsBaseManifests, SupportedFeatures: []suite.SupportedFeature{ suite.SupportReferenceGrant, + // TODO: https://github.com/Kong/kubernetes-ingress-controller/issues/2778 + // suite.SupportHTTPRouteQueryParamMatching, }, }) cSuite.Setup(t) @@ -71,15 +72,7 @@ func TestGatewayConformance(t *testing.T) { t.Log("running gateway conformance tests") for _, tt := range tests.ConformanceTests { - if enabledGatewayConformanceTests.Has(tt.ShortName) { - t.Run(tt.Description, func(t *testing.T) { tt.Run(t, cSuite) }) - } + tt := tt + t.Run(tt.Description, func(t *testing.T) { tt.Run(t, cSuite) }) } } - -var enabledGatewayConformanceTests = sets.NewString( - "GatewaySecretInvalidReferenceGrant", - "GatewaySecretMissingReferenceGrant", - "GatewaySecretReferenceGrantAllInNamespace", - "GatewaySecretReferenceGrantSpecific", -) diff --git a/test/integration/httproute_test.go b/test/integration/httproute_test.go index 0acffd640a..1d670e9d2f 100644 --- a/test/integration/httproute_test.go +++ b/test/integration/httproute_test.go @@ -145,6 +145,7 @@ func TestHTTPRouteEssentials(t *testing.T) { BackendObjectReference: gatewayv1alpha2.BackendObjectReference{ Name: gatewayv1alpha2.ObjectName(service1.Name), Port: &httpPort, + Kind: util.StringToGatewayAPIKindPtr("Service"), }, }, }}, @@ -213,6 +214,7 @@ func TestHTTPRouteEssentials(t *testing.T) { BackendObjectReference: gatewayv1alpha2.BackendObjectReference{ Name: gatewayv1alpha2.ObjectName(service1.Name), Port: &httpPort, + Kind: util.StringToGatewayAPIKindPtr("Service"), }, Weight: &httpbinWeight, }, @@ -222,6 +224,7 @@ func TestHTTPRouteEssentials(t *testing.T) { BackendObjectReference: gatewayv1alpha2.BackendObjectReference{ Name: gatewayv1alpha2.ObjectName(service2.Name), Port: &httpPort, + Kind: util.StringToGatewayAPIKindPtr("Service"), }, Weight: &nginxWeight, },