diff --git a/internal/resources/k8s_gateway_resource_test.go b/internal/resources/k8s_gateway_resource_test.go index 61d6548..215367b 100644 --- a/internal/resources/k8s_gateway_resource_test.go +++ b/internal/resources/k8s_gateway_resource_test.go @@ -32,6 +32,7 @@ var _ = BeforeSuite(func() { runtimeScheme = runtime.NewScheme() Expect(scheme.AddToScheme(runtimeScheme)).To(Succeed()) Expect(kamajiv1alpha1.AddToScheme(runtimeScheme)).To(Succeed()) + Expect(gatewayv1.Install(runtimeScheme)).To(Succeed()) Expect(gatewayv1alpha2.Install(runtimeScheme)).To(Succeed()) }) @@ -267,4 +268,242 @@ var _ = Describe("KubernetesGatewayResource", func() { Expect(listener.Port).To(Equal(gatewayv1.PortNumber(80))) }) }) + + Describe("BuildGatewayAccessPointsStatus", func() { + var ( + gwNamespace = "gateway-system" + gwName = "test-gateway" + gateway *gatewayv1.Gateway + route *gatewayv1alpha2.TLSRoute + fakeClient client.Client + ) + + // Builds a RouteStatus with a single Accepted parent using the + // supplied ParentReference. + buildRouteStatus := func(ref gatewayv1.ParentReference) gatewayv1alpha2.RouteStatus { + return gatewayv1alpha2.RouteStatus{ + Parents: []gatewayv1.RouteParentStatus{{ + ParentRef: ref, + Conditions: []metav1.Condition{{ + Type: string(gatewayv1.RouteConditionAccepted), + Status: metav1.ConditionTrue, + Reason: "Accepted", + LastTransitionTime: metav1.Now(), + }}, + }}, + } + } + + BeforeEach(func() { + gateway = &gatewayv1.Gateway{ + ObjectMeta: metav1.ObjectMeta{Name: gwName, Namespace: gwNamespace}, + Spec: gatewayv1.GatewaySpec{ + Listeners: []gatewayv1.Listener{ + {Name: "kube-apiserver", Port: 31443, Protocol: gatewayv1.TLSProtocolType}, + {Name: "konnectivity-server", Port: 32132, Protocol: gatewayv1.TLSProtocolType}, + // Non-TLS listener on the same Gateway: it must not be turned + // into a TLSRoute access point. + {Name: "http-noise", Port: 8080, Protocol: gatewayv1.HTTPProtocolType}, + }, + }, + Status: gatewayv1.GatewayStatus{ + Conditions: []metav1.Condition{{ + Type: string(gatewayv1.GatewayConditionProgrammed), + Status: metav1.ConditionTrue, + Reason: "Programmed", + LastTransitionTime: metav1.Now(), + }}, + }, + } + + route = &gatewayv1alpha2.TLSRoute{ + ObjectMeta: metav1.ObjectMeta{Name: "tcp", Namespace: "tenant-ns"}, + Spec: gatewayv1alpha2.TLSRouteSpec{ + Hostnames: []gatewayv1.Hostname{"tcp.example.com"}, + }, + } + + fakeClient = fake.NewClientBuilder(). + WithScheme(runtimeScheme). + WithObjects(gateway). + WithIndex(&gatewayv1.Gateway{}, kamajiv1alpha1.GatewayListenerNameKey, (&kamajiv1alpha1.GatewayListener{}).ExtractValue()). + Build() + }) + + It("builds a single access point when the parentRef specifies a sectionName", func() { + section := gatewayv1.SectionName("konnectivity-server") + ns := gatewayv1.Namespace(gwNamespace) + + statuses := buildRouteStatus(gatewayv1.ParentReference{ + Name: gatewayv1.ObjectName(gwName), + Namespace: &ns, + SectionName: §ion, + }) + + aps, err := resources.BuildGatewayAccessPointsStatus(ctx, fakeClient, route, statuses) + Expect(err).NotTo(HaveOccurred()) + Expect(aps).To(HaveLen(1)) + Expect(aps[0].Port).To(Equal(gatewayv1.PortNumber(32132))) + Expect(aps[0].Value).To(Equal("https://tcp.example.com:32132")) + }) + + It("ignores a non-TLS listener even when explicitly selected via sectionName", func() { + section := gatewayv1.SectionName("http-noise") + ns := gatewayv1.Namespace(gwNamespace) + + statuses := buildRouteStatus(gatewayv1.ParentReference{ + Name: gatewayv1.ObjectName(gwName), + Namespace: &ns, + SectionName: §ion, + }) + + aps, err := resources.BuildGatewayAccessPointsStatus(ctx, fakeClient, route, statuses) + Expect(err).NotTo(HaveOccurred()) + Expect(aps).To(BeEmpty()) + }) + + It("ignores a TLS listener whose port disagrees with parentRef.Port", func() { + section := gatewayv1.SectionName("kube-apiserver") + ns := gatewayv1.Namespace(gwNamespace) + mismatch := gatewayv1.PortNumber(9999) + + statuses := buildRouteStatus(gatewayv1.ParentReference{ + Name: gatewayv1.ObjectName(gwName), + Namespace: &ns, + SectionName: §ion, + Port: &mismatch, + }) + + aps, err := resources.BuildGatewayAccessPointsStatus(ctx, fakeClient, route, statuses) + Expect(err).NotTo(HaveOccurred()) + Expect(aps).To(BeEmpty()) + }) + + It("builds one access point per listener when sectionName is unset", func() { + ns := gatewayv1.Namespace(gwNamespace) + + statuses := buildRouteStatus(gatewayv1.ParentReference{ + Name: gatewayv1.ObjectName(gwName), + Namespace: &ns, + }) + + aps, err := resources.BuildGatewayAccessPointsStatus(ctx, fakeClient, route, statuses) + Expect(err).NotTo(HaveOccurred()) + Expect(aps).To(HaveLen(2)) + ports := []gatewayv1.PortNumber{aps[0].Port, aps[1].Port} + Expect(ports).To(ConsistOf(gatewayv1.PortNumber(31443), gatewayv1.PortNumber(32132))) + }) + + It("filters listeners by port when sectionName is unset but port is specified", func() { + ns := gatewayv1.Namespace(gwNamespace) + port := gatewayv1.PortNumber(31443) + + statuses := buildRouteStatus(gatewayv1.ParentReference{ + Name: gatewayv1.ObjectName(gwName), + Namespace: &ns, + Port: &port, + }) + + aps, err := resources.BuildGatewayAccessPointsStatus(ctx, fakeClient, route, statuses) + Expect(err).NotTo(HaveOccurred()) + Expect(aps).To(HaveLen(1)) + Expect(aps[0].Port).To(Equal(port)) + }) + + It("defaults the parent namespace to the route namespace when unset", func() { + // Gateway is in the route's namespace so the nil-namespace default kicks in. + colocated := gateway.DeepCopy() + colocated.Namespace = route.Namespace + + c := fake.NewClientBuilder(). + WithScheme(runtimeScheme). + WithObjects(colocated). + WithIndex(&gatewayv1.Gateway{}, kamajiv1alpha1.GatewayListenerNameKey, (&kamajiv1alpha1.GatewayListener{}).ExtractValue()). + Build() + + statuses := buildRouteStatus(gatewayv1.ParentReference{ + Name: gatewayv1.ObjectName(gwName), + }) + + aps, err := resources.BuildGatewayAccessPointsStatus(ctx, c, route, statuses) + Expect(err).NotTo(HaveOccurred()) + Expect(aps).To(HaveLen(2)) + }) + + It("skips silently when the referenced Gateway is missing", func() { + ns := gatewayv1.Namespace("does-not-exist") + + statuses := buildRouteStatus(gatewayv1.ParentReference{ + Name: gatewayv1.ObjectName("ghost-gateway"), + Namespace: &ns, + }) + + aps, err := resources.BuildGatewayAccessPointsStatus(ctx, fakeClient, route, statuses) + Expect(err).NotTo(HaveOccurred()) + Expect(aps).To(BeEmpty()) + }) + + It("skips Gateways that are not Programmed", func() { + notProgrammed := gateway.DeepCopy() + notProgrammed.Status.Conditions = nil + + c := fake.NewClientBuilder(). + WithScheme(runtimeScheme). + WithObjects(notProgrammed). + WithIndex(&gatewayv1.Gateway{}, kamajiv1alpha1.GatewayListenerNameKey, (&kamajiv1alpha1.GatewayListener{}).ExtractValue()). + Build() + + ns := gatewayv1.Namespace(gwNamespace) + + statuses := buildRouteStatus(gatewayv1.ParentReference{ + Name: gatewayv1.ObjectName(gwName), + Namespace: &ns, + }) + + aps, err := resources.BuildGatewayAccessPointsStatus(ctx, c, route, statuses) + Expect(err).NotTo(HaveOccurred()) + Expect(aps).To(BeEmpty()) + }) + + It("ignores non-TLS listeners when sectionName is unset", func() { + ns := gatewayv1.Namespace(gwNamespace) + + statuses := buildRouteStatus(gatewayv1.ParentReference{ + Name: gatewayv1.ObjectName(gwName), + Namespace: &ns, + }) + + aps, err := resources.BuildGatewayAccessPointsStatus(ctx, fakeClient, route, statuses) + Expect(err).NotTo(HaveOccurred()) + // Only the two TLS listeners may produce https:// access points. + Expect(aps).To(HaveLen(2)) + ports := []gatewayv1.PortNumber{aps[0].Port, aps[1].Port} + Expect(ports).To(ConsistOf(gatewayv1.PortNumber(31443), gatewayv1.PortNumber(32132))) + Expect(ports).NotTo(ContainElement(gatewayv1.PortNumber(8080))) + }) + + It("propagates indexer failures from the sectionName fast path", func() { + // Fault-injection; build a client *without* the listener-name, i.e. + // WithIndex(&gatewayv1.Gateway{}, GatewayListenerNameKey, …). + // An error is expected to be propagated + c := fake.NewClientBuilder(). + WithScheme(runtimeScheme). + WithObjects(gateway). + Build() + + section := gatewayv1.SectionName("konnectivity-server") + ns := gatewayv1.Namespace(gwNamespace) + + statuses := buildRouteStatus(gatewayv1.ParentReference{ + Name: gatewayv1.ObjectName(gwName), + Namespace: &ns, + SectionName: §ion, + }) + + _, err := resources.BuildGatewayAccessPointsStatus(ctx, c, route, statuses) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("could not resolve gateway listeners for parentRef")) + Expect(err.Error()).To(ContainSubstring("failed to fetch gateway for listener")) + }) + }) }) diff --git a/internal/resources/k8s_gateway_utils.go b/internal/resources/k8s_gateway_utils.go index 6707ecd..84ff1da 100644 --- a/internal/resources/k8s_gateway_utils.go +++ b/internal/resources/k8s_gateway_utils.go @@ -5,6 +5,7 @@ package resources import ( "context" + "errors" "fmt" "net/url" @@ -12,6 +13,7 @@ import ( "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/fields" + k8stypes "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" @@ -19,6 +21,10 @@ import ( kamajiv1alpha1 "github.com/clastix/kamaji/api/v1alpha1" ) +// errGatewayListenerNotFound is returned when the indexer yields no Gateway +// for the requested (namespace/name/listener) tuple. +var errGatewayListenerNotFound = errors.New("no gateway found for listener") + // fetchGatewayByListener uses the indexer to efficiently find a gateway with a specific listener. // This avoids the need to iterate through all listeners in a gateway. func fetchGatewayByListener(ctx context.Context, c client.Client, ref gatewayv1.ParentReference) (*gatewayv1.Gateway, error) { @@ -38,7 +44,7 @@ func fetchGatewayByListener(ctx context.Context, c client.Client, ref gatewayv1. } if len(gatewayList.Items) == 0 { - return nil, fmt.Errorf("no gateway found with listener '%s'", *ref.SectionName) + return nil, fmt.Errorf("%w: %s", errGatewayListenerNotFound, *ref.SectionName) } // Since we're using a composite key with namespace/name/listener, we should get exactly one result @@ -167,6 +173,20 @@ func CleanupTLSRoute(ctx context.Context, c client.Client, routeName, routeNames } // BuildGatewayAccessPointsStatus builds access points from route statuses. +// +// Per the Gateway API specification, ParentReference.SectionName is optional: +// when unset (or empty), the Route attaches to every listener of the referenced +// Gateway that accepts it (typically via port/protocol matching). We support +// both cases: +// +// - SectionName is set: resolve the Gateway via the listener-name indexer and +// build a single access point for that listener. +// - SectionName is nil/empty: resolve the Gateway by namespace/name and build +// an access point for each listener, optionally filtered by ParentRef.Port. +// +// Unresolvable or unprogrammed Gateways are skipped rather than returned as +// errors, so that a single mis-attached parentRef does not block the whole +// status update for the Route. func BuildGatewayAccessPointsStatus(ctx context.Context, c client.Client, route *gatewayv1alpha2.TLSRoute, routeStatuses gatewayv1alpha2.RouteStatus) ([]kamajiv1alpha1.GatewayAccessPoint, error) { accessPoints := []kamajiv1alpha1.GatewayAccessPoint{} routeNamespace := gatewayv1.Namespace(route.Namespace) @@ -185,44 +205,123 @@ func BuildGatewayAccessPointsStatus(ctx context.Context, c client.Client, route routeStatus.ParentRef.Namespace = &routeNamespace } - // Use the indexer to efficiently find the gateway with the specific listener - gateway, err := fetchGatewayByListener(ctx, c, routeStatus.ParentRef) + listeners, err := resolveMatchingListeners(ctx, c, routeStatus.ParentRef) if err != nil { - return nil, fmt.Errorf("could not fetch gateway with listener '%v': %w", - routeStatus.ParentRef.SectionName, err) - } - gatewayProgrammed := meta.IsStatusConditionTrue( - gateway.Status.Conditions, - string(gatewayv1.GatewayConditionProgrammed), - ) - if !gatewayProgrammed { - continue + return nil, fmt.Errorf("could not resolve gateway listeners for parentRef '%s/%s': %w", + *routeStatus.ParentRef.Namespace, routeStatus.ParentRef.Name, err) } - // Since we fetched the gateway using the indexer, we know the listener exists - // but we still need to get its details from the gateway spec - listener, err := FindMatchingListener( - gateway.Spec.Listeners, routeStatus.ParentRef, - ) - if err != nil { - return nil, fmt.Errorf("failed to match listener: %w", err) - } + for _, listener := range listeners { + for _, hostname := range route.Spec.Hostnames { + rawURL := fmt.Sprintf("https://%s:%d", hostname, listener.Port) + parsedURL, err := url.Parse(rawURL) + if err != nil { + return nil, fmt.Errorf("invalid url: %w", err) + } - for _, hostname := range route.Spec.Hostnames { - rawURL := fmt.Sprintf("https://%s:%d", hostname, listener.Port) - parsedURL, err := url.Parse(rawURL) - if err != nil { - return nil, fmt.Errorf("invalid url: %w", err) + hostnameAddressType := gatewayv1.HostnameAddressType + accessPoints = append(accessPoints, kamajiv1alpha1.GatewayAccessPoint{ + Type: &hostnameAddressType, + Value: parsedURL.String(), + Port: listener.Port, + }) } - - hostnameAddressType := gatewayv1.HostnameAddressType - accessPoints = append(accessPoints, kamajiv1alpha1.GatewayAccessPoint{ - Type: &hostnameAddressType, - Value: parsedURL.String(), - Port: listener.Port, - }) } } return accessPoints, nil } + +// resolveMatchingListeners returns the listeners of the Gateway referenced by +// ref that should contribute access points for the enclosing Route. +// +// When ref.SectionName is set, exactly one listener is returned (looked up via +// the name indexer for efficiency). When SectionName is nil or empty, every +// listener of the Gateway is considered and, if ref.Port is set, further +// filtered to listeners exposing that port. +// +// If the referenced Gateway is not found, is not Programmed, or has no +// matching listener, an empty slice is returned with a nil error: a single +// unresolvable parentRef must not fail the whole status update. +func resolveMatchingListeners(ctx context.Context, c client.Client, ref gatewayv1.ParentReference) ([]gatewayv1.Listener, error) { + hasSectionName := ref.SectionName != nil && *ref.SectionName != "" + + // Fast path: look the Gateway up by its specific listener name. + if hasSectionName { + gateway, err := fetchGatewayByListener(ctx, c, ref) + if err != nil { + // Only the "no gateway with that listener" case is benign and + // must not block the status update. Failures are propagated. + if errors.Is(err, errGatewayListenerNotFound) { + return nil, nil + } + + return nil, fmt.Errorf("failed to fetch gateway for listener: %w", err) + } + + if !isGatewayProgrammed(gateway) { + return nil, nil + } + + listener, err := FindMatchingListener(gateway.Spec.Listeners, ref) + if err != nil { + return nil, fmt.Errorf("failed to match listener: %w", err) + } + + if !isEligibleListener(listener, ref) { + return nil, nil + } + + return []gatewayv1.Listener{listener}, nil + } + + // SectionName unset: resolve the Gateway by namespace/name. + gateway := &gatewayv1.Gateway{} + key := k8stypes.NamespacedName{Namespace: string(*ref.Namespace), Name: string(ref.Name)} + if err := c.Get(ctx, key, gateway); err != nil { + if k8serrors.IsNotFound(err) { + return nil, nil + } + + return nil, fmt.Errorf("failed to fetch gateway %s: %w", key, err) + } + + if !isGatewayProgrammed(gateway) { + return nil, nil + } + + matching := make([]gatewayv1.Listener, 0, len(gateway.Spec.Listeners)) + for _, listener := range gateway.Spec.Listeners { + if !isEligibleListener(listener, ref) { + continue + } + + matching = append(matching, listener) + } + + return matching, nil +} + +func isGatewayProgrammed(gateway *gatewayv1.Gateway) bool { + return meta.IsStatusConditionTrue( + gateway.Status.Conditions, + string(gatewayv1.GatewayConditionProgrammed), + ) +} + +// isEligibleListener reports whether a Gateway listener can contribute an +// access point for the TLSRoute attachment, ie.: +// - its protocol is TLS (BuildGatewayAccessPointsStatus is invoked for +// TLSRoutes only, so non-TLS listeners can never legally attach); +// - and, if ref.Port is set, its port matches ref.Port. +func isEligibleListener(listener gatewayv1.Listener, ref gatewayv1.ParentReference) bool { + if listener.Protocol != gatewayv1.TLSProtocolType { + return false + } + + if ref.Port != nil && listener.Port != *ref.Port { + return false + } + + return true +}