From b632a81c5664e29ac7be753fc1941897b883662a Mon Sep 17 00:00:00 2001 From: Peter Stokes Date: Tue, 12 Nov 2024 13:17:41 +0000 Subject: [PATCH] Honour annotations specified on Gateway resources. --- source/gateway.go | 106 ++++++++++++------ source/gateway_httproute_test.go | 187 ++++++++++++++++++++++++++++++- source/gateway_test.go | 45 -------- 3 files changed, 257 insertions(+), 81 deletions(-) diff --git a/source/gateway.go b/source/gateway.go index 668a7d8ad4..470562a781 100644 --- a/source/gateway.go +++ b/source/gateway.go @@ -18,8 +18,8 @@ package source import ( "context" - "fmt" "net/netip" + "slices" "sort" "strings" "text/template" @@ -206,30 +206,25 @@ func (src *gatewayRouteSource) Endpoints(ctx context.Context) ([]*endpoint.Endpo if err != nil { return nil, err } - kind := strings.ToLower(src.rtKind) resolver := newGatewayRouteResolver(src, gateways, namespaces) for _, rt := range routes { // Filter by annotations. meta := rt.Metadata() - annots := meta.Annotations - if !src.rtAnnotations.Matches(labels.Set(annots)) { + if !src.rtAnnotations.Matches(labels.Set(meta.Annotations)) { continue } - // Check controller annotation to see if we are responsible. - if v, ok := annots[controllerAnnotationKey]; ok && v != controllerAnnotationValue { + if v, ok := meta.Annotations[controllerAnnotationKey]; ok && v != controllerAnnotationValue { log.Debugf("Skipping %s %s/%s because controller value does not match, found: %s, required: %s", src.rtKind, meta.Namespace, meta.Name, v, controllerAnnotationValue) continue } - // Get Gateway Listeners associated with Route. gwListeners := resolver.resolve(rt) if len(gwListeners) == 0 { log.Debugf("No endpoints could be generated from %s %s/%s", src.rtKind, meta.Namespace, meta.Name) continue } - // Create endpoints for Route and associated Gateway Listeners rtHosts := rt.Hostnames() if len(rtHosts) == 0 { @@ -237,10 +232,16 @@ func (src *gatewayRouteSource) Endpoints(ctx context.Context) ([]*endpoint.Endpo // attached Gateway Listeners. rtHosts = []v1.Hostname{""} } - - hostTargets := make(map[string]endpoint.Targets) + resource := strings.Join([]string{strings.ToLower(src.rtKind), meta.Namespace, meta.Name}, "/") + hostGateways := map[string][]*v1beta1.Gateway{} + ttl := getTTLFromAnnotations(meta.Annotations, resource) + dualstack := false + if v, ok := meta.Annotations[gatewayAPIDualstackAnnotationKey]; ok && v == gatewayAPIDualstackAnnotationValue { + dualstack = true + } + providerSpecific, setIdentifier := getProviderSpecificAnnotations(meta.Annotations) for gateway, listeners := range gwListeners { - var hosts []string + hosts := map[string]struct{}{} for _, listener := range listeners { // Find all overlapping hostnames between the Route and Listener. gwHost := getVal(listener.Hostname, "") @@ -249,13 +250,18 @@ func (src *gatewayRouteSource) Endpoints(ctx context.Context) ([]*endpoint.Endpo if !ok || host == "" { continue } - hosts = append(hosts, host) + hosts[host] = struct{}{} } } // TODO: The ignore-hostname-annotation flag help says "valid only when using fqdn-template" // but other sources don't check if fqdn-template is set. Which should it be? if !src.ignoreHostnameAnnotation { - hosts = append(hosts, getHostnamesFromAnnotations(annots)...) + for _, host := range getHostnamesFromAnnotations(gateway.Annotations) { + hosts[host] = struct{}{} + } + for _, host := range getHostnamesFromAnnotations(meta.Annotations) { + hosts[host] = struct{}{} + } } // TODO: The combine-fqdn-annotation flag is similarly vague. if src.fqdnTemplate != nil && (len(hosts) == 0 || src.combineFQDNAnnotation) { @@ -263,26 +269,66 @@ func (src *gatewayRouteSource) Endpoints(ctx context.Context) ([]*endpoint.Endpo if err != nil { return nil, err } - hosts = append(hosts, templated...) + for _, host := range templated { + hosts[host] = struct{}{} + } + } + if len(hosts) == 0 { + continue + } + for host, _ := range hosts { + hostGateways[host] = append(hostGateways[host], gateway) + } + // Merge Gateway annotations + gwTTL := getTTLFromAnnotations(gateway.Annotations, strings.Join([]string{strings.ToLower(gateway.Kind), gateway.Namespace, gateway.Name}, "/")) + if gwTTL.IsConfigured() { + if !ttl.IsConfigured() || ttl > gwTTL { + ttl = gwTTL + } + } + if v, ok := gateway.Annotations[gatewayAPIDualstackAnnotationKey]; ok && v == gatewayAPIDualstackAnnotationValue { + dualstack = true + } + gwProviderSpecific, gwSetIdentifier := getProviderSpecificAnnotations(gateway.Annotations) + for _, gwProperty := range gwProviderSpecific { + present := false + for _, property := range providerSpecific { + if property.Name == gwProperty.Name { + present = true + break + } + } + if !present { + providerSpecific = append(providerSpecific, gwProperty) + } } - for _, host := range hosts { + if setIdentifier == "" { + setIdentifier = gwSetIdentifier + } + } + for host, gateways := range hostGateways { + var targets endpoint.Targets + for _, gateway := range gateways { override := getTargetsFromTargetAnnotation(gateway.Annotations) - hostTargets[host] = append(hostTargets[host], override...) + targets = append(targets, override...) if len(override) == 0 { for _, addr := range gateway.Status.Addresses { - hostTargets[host] = append(hostTargets[host], addr.Value) + targets = append(targets, addr.Value) } } } + origin := resource + if len(gateways) == 1 && !src.ignoreHostnameAnnotation && slices.Contains(getHostnamesFromAnnotations(gateways[0].Annotations), host) { + // Annotated hostnames from a single Gateway are attributed to the Gateway rather than the Route + origin = strings.Join([]string{strings.ToLower(gateways[0].Kind), gateways[0].Namespace, gateways[0].Name}, "/") + } + for _, ep := range endpointsForHostname(host, targets, ttl, providerSpecific, setIdentifier, origin) { + if dualstack { + ep.Labels[endpoint.DualstackLabelKey] = "true" + } + endpoints = append(endpoints, ep) + } } - - resource := fmt.Sprintf("%s/%s/%s", kind, meta.Namespace, meta.Name) - providerSpecific, setIdentifier := getProviderSpecificAnnotations(annots) - ttl := getTTLFromAnnotations(annots, resource) - for host, targets := range hostTargets { - endpoints = append(endpoints, endpointsForHostname(host, targets, ttl, providerSpecific, setIdentifier, resource)...) - } - setDualstackLabel(rt, endpoints) log.Debugf("Endpoints generated from %s %s/%s: %v", src.rtKind, meta.Namespace, meta.Name, endpoints) } return endpoints, nil @@ -585,13 +631,3 @@ func selectorsEqual(a, b labels.Selector) bool { } return true } - -func setDualstackLabel(rt gatewayRoute, endpoints []*endpoint.Endpoint) { - val, ok := rt.Metadata().Annotations[gatewayAPIDualstackAnnotationKey] - if ok && val == gatewayAPIDualstackAnnotationValue { - log.Debugf("Adding dualstack label to GatewayRoute %s/%s.", rt.Metadata().Namespace, rt.Metadata().Name) - for _, ep := range endpoints { - ep.Labels[endpoint.DualstackLabelKey] = "true" - } - } -} diff --git a/source/gateway_httproute_test.go b/source/gateway_httproute_test.go index 1f8e7175ac..ff092040d6 100644 --- a/source/gateway_httproute_test.go +++ b/source/gateway_httproute_test.go @@ -677,7 +677,16 @@ func TestGatewayHTTPRouteSourceEndpoints(t *testing.T) { config: Config{}, namespaces: namespaces("default"), gateways: []*v1beta1.Gateway{{ - ObjectMeta: objectMeta("default", "test"), + TypeMeta: metav1.TypeMeta{ + Kind: "Gateway", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + Annotations: map[string]string{ + hostnameAnnotationKey: "annotation.gateway.internal", + }, + }, Spec: v1.GatewaySpec{ Listeners: []v1.Listener{{Protocol: v1.HTTPProtocolType}}, }, @@ -712,6 +721,10 @@ func TestGatewayHTTPRouteSourceEndpoints(t *testing.T) { }, }, endpoints: []*endpoint.Endpoint{ + newTestEndpoint("annotation.gateway.internal", "A", "1.2.3.4"). + WithLabel(endpoint.ResourceLabelKey, "gateway/default/test"), + newTestEndpoint("annotation.gateway.internal", "A", "1.2.3.4"). + WithLabel(endpoint.ResourceLabelKey, "gateway/default/test"), newTestEndpoint("annotation.without-hostname.internal", "A", "1.2.3.4"). WithLabel(endpoint.ResourceLabelKey, "httproute/default/without-hostname"), newTestEndpoint("annotation.with-hostname.internal", "A", "1.2.3.4"). @@ -861,6 +874,64 @@ func TestGatewayHTTPRouteSourceEndpoints(t *testing.T) { WithLabel(endpoint.ResourceLabelKey, "httproute/default/valid-ttl"), }, }, + { + title: "TTLGateway", + config: Config{}, + namespaces: namespaces("default"), + gateways: []*v1beta1.Gateway{{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + Annotations: map[string]string{ttlAnnotationKey: "15s"}, + }, + Spec: v1.GatewaySpec{ + Listeners: []v1.Listener{{Protocol: v1.HTTPProtocolType}}, + }, + Status: gatewayStatus("1.2.3.4"), + }}, + routes: []*v1beta1.HTTPRoute{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "no-ttl", + Namespace: "default", + }, + Spec: v1.HTTPRouteSpec{ + Hostnames: hostnames("no-ttl.internal"), + }, + Status: httpRouteStatus(gwParentRef("default", "test")), + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "longer-ttl", + Namespace: "default", + Annotations: map[string]string{ttlAnnotationKey: "20s"}, + }, + Spec: v1.HTTPRouteSpec{ + Hostnames: hostnames("longer-ttl.internal"), + }, + Status: httpRouteStatus(gwParentRef("default", "test")), + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "shorter-ttl", + Namespace: "default", + Annotations: map[string]string{ttlAnnotationKey: "5s"}, + }, + Spec: v1.HTTPRouteSpec{ + Hostnames: hostnames("shorter-ttl.internal"), + }, + Status: httpRouteStatus(gwParentRef("default", "test")), + }, + }, + endpoints: []*endpoint.Endpoint{ + newTestEndpointWithTTL("no-ttl.internal", "A", 15, "1.2.3.4"). + WithLabel(endpoint.ResourceLabelKey, "httproute/default/no-ttl"), + newTestEndpointWithTTL("longer-ttl.internal", "A", 15, "1.2.3.4"). + WithLabel(endpoint.ResourceLabelKey, "httproute/default/longer-ttl"), + newTestEndpointWithTTL("shorter-ttl.internal", "A", 5, "1.2.3.4"). + WithLabel(endpoint.ResourceLabelKey, "httproute/default/shorter-ttl"), + }, + }, { title: "ProviderAnnotations", config: Config{}, @@ -893,6 +964,61 @@ func TestGatewayHTTPRouteSourceEndpoints(t *testing.T) { WithSetIdentifier("test-set-identifier"), }, }, + { + title: "ProviderAnnotationsGateway", + config: Config{}, + namespaces: namespaces("default"), + gateways: []*v1beta1.Gateway{{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + Annotations: map[string]string{ + SetIdentifierKey: "gateway", + "external-dns.alpha.kubernetes.io/webhook-property": "gateway", + }, + }, + Spec: v1.GatewaySpec{ + Listeners: []v1.Listener{{Protocol: v1.HTTPProtocolType}}, + }, + Status: gatewayStatus("1.2.3.4"), + }}, + routes: []*v1beta1.HTTPRoute{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "with-provider-annotations", + Namespace: "default", + Annotations: map[string]string{ + SetIdentifierKey: "route", + "external-dns.alpha.kubernetes.io/webhook-property": "route", + }, + }, + Spec: v1.HTTPRouteSpec{ + Hostnames: hostnames("with-provider-annotations.internal"), + }, + Status: httpRouteStatus(gwParentRef("default", "test")), + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "without-provider-annotations", + Namespace: "default", + }, + Spec: v1.HTTPRouteSpec{ + Hostnames: hostnames("without-provider-annotations.internal"), + }, + Status: httpRouteStatus(gwParentRef("default", "test")), + }, + }, + endpoints: []*endpoint.Endpoint{ + newTestEndpoint("with-provider-annotations.internal", "A", "1.2.3.4"). + WithLabel(endpoint.ResourceLabelKey, "httproute/default/with-provider-annotations"). + WithProviderSpecific("webhook/property", "route"). + WithSetIdentifier("route"), + newTestEndpoint("without-provider-annotations.internal", "A", "1.2.3.4"). + WithLabel(endpoint.ResourceLabelKey, "httproute/default/without-provider-annotations"). + WithProviderSpecific("webhook/property", "gateway"). + WithSetIdentifier("gateway"), + }, + }, { title: "DifferentHostnameDifferentGateway", config: Config{}, @@ -1153,6 +1279,65 @@ func TestGatewayHTTPRouteSourceEndpoints(t *testing.T) { WithLabel(endpoint.ResourceLabelKey, "httproute/route-namespace/test"), }, }, + { + title: "DualstackAnnotation", + config: Config{}, + namespaces: namespaces("default"), + gateways: []*v1beta1.Gateway{{ + ObjectMeta: objectMeta("default", "test"), + Spec: v1.GatewaySpec{ + Listeners: []v1.Listener{{Protocol: v1.HTTPProtocolType}}, + }, + Status: gatewayStatus("1.2.3.4"), + }}, + routes: []*v1beta1.HTTPRoute{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "invalid-dualstack-annotation", + Namespace: "default", + Annotations: map[string]string{ + gatewayAPIDualstackAnnotationKey: "invalid", + }, + }, + Spec: v1.HTTPRouteSpec{ + Hostnames: hostnames("invalid-dualstack-annotation.internal"), + }, + Status: httpRouteStatus(gwParentRef("default", "test")), + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "with-dualstack-annotation", + Namespace: "default", + Annotations: map[string]string{ + gatewayAPIDualstackAnnotationKey: gatewayAPIDualstackAnnotationValue, + }, + }, + Spec: v1.HTTPRouteSpec{ + Hostnames: hostnames("with-dualstack-annotation.internal"), + }, + Status: httpRouteStatus(gwParentRef("default", "test")), + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "without-dualstack-annotation", + Namespace: "default", + }, + Spec: v1.HTTPRouteSpec{ + Hostnames: hostnames("without-dualstack-annotation.internal"), + }, + Status: httpRouteStatus(gwParentRef("default", "test")), + }, + }, + endpoints: []*endpoint.Endpoint{ + newTestEndpoint("invalid-dualstack-annotation.internal", "A", "1.2.3.4"). + WithLabel(endpoint.ResourceLabelKey, "httproute/default/invalid-dualstack-annotation"), + newTestEndpoint("with-dualstack-annotation.internal", "A", "1.2.3.4"). + WithLabel(endpoint.ResourceLabelKey, "httproute/default/with-dualstack-annotation"). + WithLabel(endpoint.DualstackLabelKey, "true"), + newTestEndpoint("without-dualstack-annotation.internal", "A", "1.2.3.4"). + WithLabel(endpoint.ResourceLabelKey, "httproute/default/without-dualstack-annotation"), + }, + }, } for _, tt := range tests { tt := tt diff --git a/source/gateway_test.go b/source/gateway_test.go index 19d603a24b..940622e958 100644 --- a/source/gateway_test.go +++ b/source/gateway_test.go @@ -20,7 +20,6 @@ import ( "strings" "testing" - "sigs.k8s.io/external-dns/endpoint" v1 "sigs.k8s.io/gateway-api/apis/v1" ) @@ -246,47 +245,3 @@ func TestIsDNS1123Domain(t *testing.T) { }) } } - -func TestDualStackLabel(t *testing.T) { - tests := []struct { - desc string - in map[string](string) - setsLabel bool - }{ - { - desc: "empty-annotation", - setsLabel: false, - }, - { - desc: "correct-annotation-key-and-value", - in: map[string]string{gatewayAPIDualstackAnnotationKey: gatewayAPIDualstackAnnotationValue}, - setsLabel: true, - }, - { - desc: "correct-annotation-key-incorrect-value", - in: map[string]string{gatewayAPIDualstackAnnotationKey: "foo"}, - setsLabel: false, - }, - { - desc: "incorrect-annotation-key-correct-value", - in: map[string]string{"FOO": gatewayAPIDualstackAnnotationValue}, - setsLabel: false, - }, - } - for _, tt := range tests { - t.Run(tt.desc, func(t *testing.T) { - endpoints := make([]*endpoint.Endpoint, 0) - endpoints = append(endpoints, endpoint.NewEndpoint("www.example.com", endpoint.RecordTypeA, "10.0.0.2", "10.0.0.3")) - - rt := &gatewayHTTPRoute{} - rt.Metadata().Annotations = tt.in - - setDualstackLabel(rt, endpoints) - got := endpoints[0].Labels[endpoint.DualstackLabelKey] == "true" - - if got != tt.setsLabel { - t.Errorf("setDualstackLabel(%q); got: %v; want: %v", tt.in, got, tt.setsLabel) - } - }) - } -}