From c7ae7f19354d9cbbe389d5a38054eb06d3ca09c6 Mon Sep 17 00:00:00 2001 From: Kyle Martin Date: Mon, 16 May 2022 12:51:57 +1000 Subject: [PATCH] Add Label filltering for Ambassador Host source Currently the `--label-filter` flag can only be used to filter CRDs, Ingress, Service and Openshift Route objects which match the label selector passed through that flag. This change extends the functionality to the Ambassador Host type object. When the flag is not specified the default value is `labels.Everything()` which is an empty string, the same as before. Annotation based filter is inefficient because the filtering has to be done in the controller instead of the API server like with label filtering. The Annotation based filtering has been left in for legacy reasons so the Ambassador Host source can be used inconjunction with the other sources that don't yet support label filltering. It is possible to use label based filltering with annotation based filltering so you can initially filter by label then filter the returned hosts by annotation. This is not recomended --- pkg/apis/externaldns/types.go | 2 +- source/ambassador_host.go | 5 +- source/ambassador_host_test.go | 181 ++++++++++++++++++++++++++++++++- source/store.go | 2 +- 4 files changed, 183 insertions(+), 7 deletions(-) diff --git a/pkg/apis/externaldns/types.go b/pkg/apis/externaldns/types.go index 46d46ad978..f40e23ee7e 100644 --- a/pkg/apis/externaldns/types.go +++ b/pkg/apis/externaldns/types.go @@ -438,7 +438,7 @@ func (cfg *Config) ParseFlags(args []string) error { app.Flag("openshift-router-name", "if source is openshift-route then you can pass the ingress controller name. Based on this name external-dns will select the respective router from the route status and map that routerCanonicalHostname to the route host while creating a CNAME record.").StringVar(&cfg.OCPRouterName) app.Flag("namespace", "Limit resources queried for endpoints to a specific namespace (default: all namespaces)").Default(defaultConfig.Namespace).StringVar(&cfg.Namespace) app.Flag("annotation-filter", "Filter resources queried for endpoints by annotation, using label selector semantics").Default(defaultConfig.AnnotationFilter).StringVar(&cfg.AnnotationFilter) - app.Flag("label-filter", "Filter resources queried for endpoints by label selector; currently supported by source types crd, gateway-httproute, gateway-grpcroute, gateway-tlsroute, gateway-tcproute, gateway-udproute, ingress, node, openshift-route, and service").Default(defaultConfig.LabelFilter).StringVar(&cfg.LabelFilter) + app.Flag("label-filter", "Filter resources queried for endpoints by label selector; currently supported by source types crd, gateway-httproute, gateway-grpcroute, gateway-tlsroute, gateway-tcproute, gateway-udproute, ingress, node, openshift-route, service and ambassador-host").Default(defaultConfig.LabelFilter).StringVar(&cfg.LabelFilter) app.Flag("ingress-class", "Require an Ingress to have this class name (defaults to any class; specify multiple times to allow more than one class)").StringsVar(&cfg.IngressClassNames) app.Flag("fqdn-template", "A templated string that's used to generate DNS names from sources that don't define a hostname themselves, or to add a hostname suffix when paired with the fake source (optional). Accepts comma separated list for multiple global FQDN.").Default(defaultConfig.FQDNTemplate).StringVar(&cfg.FQDNTemplate) app.Flag("combine-fqdn-annotation", "Combine FQDN template and Annotations instead of overwriting").BoolVar(&cfg.CombineFQDNAndAnnotation) diff --git a/source/ambassador_host.go b/source/ambassador_host.go index 864e349e96..9be6bc9da4 100644 --- a/source/ambassador_host.go +++ b/source/ambassador_host.go @@ -60,6 +60,7 @@ type ambassadorHostSource struct { annotationFilter string ambassadorHostInformer informers.GenericInformer unstructuredConverter *unstructuredConverter + labelSelector labels.Selector } // NewAmbassadorHostSource creates a new ambassadorHostSource with the given config. @@ -69,6 +70,7 @@ func NewAmbassadorHostSource( kubeClient kubernetes.Interface, namespace string, annotationFilter string, + labelSelector labels.Selector, ) (Source, error) { var err error @@ -104,13 +106,14 @@ func NewAmbassadorHostSource( annotationFilter: annotationFilter, ambassadorHostInformer: ambassadorHostInformer, unstructuredConverter: uc, + labelSelector: labelSelector, }, nil } // Endpoints returns endpoint objects for each host-target combination that should be processed. // Retrieves all Hosts in the source's namespace(s). func (sc *ambassadorHostSource) Endpoints(ctx context.Context) ([]*endpoint.Endpoint, error) { - hosts, err := sc.ambassadorHostInformer.Lister().ByNamespace(sc.namespace).List(labels.Everything()) + hosts, err := sc.ambassadorHostInformer.Lister().ByNamespace(sc.namespace).List(sc.labelSelector) if err != nil { return nil, err } diff --git a/source/ambassador_host_test.go b/source/ambassador_host_test.go index ce36a8a7a4..a3bc0165b8 100644 --- a/source/ambassador_host_test.go +++ b/source/ambassador_host_test.go @@ -27,6 +27,7 @@ import ( v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" fakeDynamic "k8s.io/client-go/dynamic/fake" "k8s.io/client-go/kubernetes/fake" @@ -66,6 +67,8 @@ func TestAmbassadorHostSource(t *testing.T) { annotationFilter := "" + labelSelector := labels.Everything() + host, err := createAmbassadorHost("test-host", "test-service") if err != nil { t.Fatalf("could not create host resource: %v", err) @@ -78,7 +81,7 @@ func TestAmbassadorHostSource(t *testing.T) { } } - ambassadorSource, err := NewAmbassadorHostSource(ctx, fakeDynamicClient, fakeKubernetesClient, namespace, annotationFilter) + ambassadorSource, err := NewAmbassadorHostSource(ctx, fakeDynamicClient, fakeKubernetesClient, namespace, annotationFilter,labelSelector) if err != nil { t.Fatalf("could not create ambassador source: %v", err) } @@ -120,12 +123,15 @@ func testAmbassadorSourceEndpoints(t *testing.T) { ambassadorHostItems []fakeAmbassadorHost expected []*endpoint.Endpoint expectError bool + labelSelector labels.Selector }{ { title: "no host", + labelSelector: labels.Everything(), }, { - title: "two simple hosts", + title: "two simple hosts", + labelSelector: labels.Everything(), loadBalancer: fakeAmbassadorLoadBalancerService{ ips: []string{"8.8.8.8"}, hostnames: []string{"lb.com"}, @@ -170,7 +176,8 @@ func testAmbassadorSourceEndpoints(t *testing.T) { }, }, { - title: "two simple hosts on different namespaces", + title: "two simple hosts on different namespaces", + labelSelector: labels.Everything(), loadBalancer: fakeAmbassadorLoadBalancerService{ ips: []string{"8.8.8.8"}, hostnames: []string{"lb.com"}, @@ -217,6 +224,7 @@ func testAmbassadorSourceEndpoints(t *testing.T) { { title: "two simple hosts on different namespaces and a target namespace", targetNamespace: "testing1", + labelSelector: labels.Everything(), loadBalancer: fakeAmbassadorLoadBalancerService{ ips: []string{"8.8.8.8"}, hostnames: []string{"lb.com"}, @@ -253,7 +261,8 @@ func testAmbassadorSourceEndpoints(t *testing.T) { }, }, { - title: "invalid non matching host ambassador service annotation", + title: "invalid non matching host ambassador service annotation", + labelSelector: labels.Everything(), loadBalancer: fakeAmbassadorLoadBalancerService{ ips: []string{"8.8.8.8"}, hostnames: []string{"lb.com"}, @@ -275,6 +284,7 @@ func testAmbassadorSourceEndpoints(t *testing.T) { { title: "valid matching annotation filter expression", annotationFilter: "kubernetes.io/ingress.class in (external-ingress)", + labelSelector: labels.Everything(), loadBalancer: fakeAmbassadorLoadBalancerService{ ips: []string{"8.8.8.8"}, hostnames: []string{"lb.com"}, @@ -306,6 +316,7 @@ func testAmbassadorSourceEndpoints(t *testing.T) { { title: "valid non-matching annotation filter expression", annotationFilter: "kubernetes.io/ingress.class in (external-ingress)", + labelSelector: labels.Everything(), loadBalancer: fakeAmbassadorLoadBalancerService{ ips: []string{"8.8.8.8"}, hostnames: []string{"lb.com"}, @@ -328,6 +339,7 @@ func testAmbassadorSourceEndpoints(t *testing.T) { { title: "invalid annotation filter expression", annotationFilter: "kubernetes.io/ingress.class in (external ingress)", + labelSelector: labels.Everything(), loadBalancer: fakeAmbassadorLoadBalancerService{ ips: []string{"8.8.8.8"}, hostnames: []string{"lb.com"}, @@ -351,6 +363,7 @@ func testAmbassadorSourceEndpoints(t *testing.T) { { title: "valid matching annotation filter label", annotationFilter: "kubernetes.io/ingress.class=external-ingress", + labelSelector: labels.Everything(), loadBalancer: fakeAmbassadorLoadBalancerService{ ips: []string{"8.8.8.8"}, hostnames: []string{"lb.com"}, @@ -382,6 +395,53 @@ func testAmbassadorSourceEndpoints(t *testing.T) { { title: "valid non-matching annotation filter label", annotationFilter: "kubernetes.io/ingress.class=external-ingress", + labelSelector: labels.Everything(), + loadBalancer: fakeAmbassadorLoadBalancerService{ + ips: []string{"8.8.8.8"}, + hostnames: []string{"lb.com"}, + name: "emissary", + namespace: "emissary-ingress", + }, + ambassadorHostItems: []fakeAmbassadorHost{ + { + name: "fake1", + namespace: "testing1", + hostname: "fake1.org", + annotations: map[string]string{ + "external-dns.ambassador-service": "emissary-ingress/emissary", + "kubernetes.io/ingress.class": "internal-ingress", + }, + }, + }, + expected: []*endpoint.Endpoint{}, + }, + { + title: "valid non-matching label filter expression", + labelSelector: labels.SelectorFromSet(labels.Set{"kubernetes.io/ingress.class": "external-ingress"}), + loadBalancer: fakeAmbassadorLoadBalancerService{ + ips: []string{"8.8.8.8"}, + hostnames: []string{"lb.com"}, + name: "emissary", + namespace: "emissary-ingress", + }, + ambassadorHostItems: []fakeAmbassadorHost{ + { + name: "fake1", + namespace: "testing1", + hostname: "fake1.org", + annotations: map[string]string{ + "external-dns.ambassador-service": "emissary-ingress/emissary", + }, + labels: map[string]string{ + "kubernetes.io/ingress.class": "internal-ingress", + }, + }, + }, + expected: []*endpoint.Endpoint{}, + }, + { + title: "valid matching label filter expression for single host", + labelSelector: labels.SelectorFromSet(labels.Set{"kubernetes.io/ingress.class": "external-ingress"}), loadBalancer: fakeAmbassadorLoadBalancerService{ ips: []string{"8.8.8.8"}, hostnames: []string{"lb.com"}, @@ -393,10 +453,120 @@ func testAmbassadorSourceEndpoints(t *testing.T) { name: "fake1", namespace: "testing1", hostname: "fake1.org", + annotations: map[string]string{ + "external-dns.ambassador-service": "emissary-ingress/emissary", + }, + labels: map[string]string{ + "kubernetes.io/ingress.class": "external-ingress", + }, + }, + { + name: "fake2", + namespace: "testing2", + hostname: "fake2.org", annotations: map[string]string{ "external-dns.ambassador-service": "emissary-ingress/emissary", "kubernetes.io/ingress.class": "internal-ingress", }, + labels: map[string]string{ + "kubernetes.io/ingress.class": "internal-ingress", + }, + }, + }, + expected: []*endpoint.Endpoint{ + { + DNSName: "fake1.org", + Targets: endpoint.Targets{"8.8.8.8"}, + }, + { + DNSName: "fake1.org", + Targets: endpoint.Targets{"lb.com"}, + }, + }, + }, + { + title: "valid matching label filter expression and matching annotation filter", + annotationFilter: "kubernetes.io/ingress.class in (external-ingress)", + labelSelector: labels.SelectorFromSet(labels.Set{"kubernetes.io/ingress.class": "external-ingress"}), + loadBalancer: fakeAmbassadorLoadBalancerService{ + ips: []string{"8.8.8.8"}, + hostnames: []string{"lb.com"}, + name: "emissary", + namespace: "emissary-ingress", + }, + ambassadorHostItems: []fakeAmbassadorHost{ + { + name: "fake1", + namespace: "testing1", + hostname: "fake1.org", + annotations: map[string]string{ + "external-dns.ambassador-service": "emissary-ingress/emissary", + "kubernetes.io/ingress.class": "external-ingress", + }, + labels: map[string]string{ + "kubernetes.io/ingress.class": "external-ingress", + }, + }, + }, + expected: []*endpoint.Endpoint{ + { + DNSName: "fake1.org", + Targets: endpoint.Targets{"8.8.8.8"}, + }, + { + DNSName: "fake1.org", + Targets: endpoint.Targets{"lb.com"}, + }, + }, + }, + { + title: "valid non matching label filter expression and valid matching annotation filter", + annotationFilter: "kubernetes.io/ingress.class in (external-ingress)", + labelSelector: labels.SelectorFromSet(labels.Set{"kubernetes.io/ingress.class": "external-ingress"}), + loadBalancer: fakeAmbassadorLoadBalancerService{ + ips: []string{"8.8.8.8"}, + hostnames: []string{"lb.com"}, + name: "emissary", + namespace: "emissary-ingress", + }, + ambassadorHostItems: []fakeAmbassadorHost{ + { + name: "fake1", + namespace: "testing1", + hostname: "fake1.org", + annotations: map[string]string{ + "external-dns.ambassador-service": "emissary-ingress/emissary", + "kubernetes.io/ingress.class": "external-ingress", + }, + labels: map[string]string{ + "kubernetes.io/ingress.class": "internal-ingress", + }, + }, + }, + expected: []*endpoint.Endpoint{}, + }, + { + title: "valid matching label filter expression and non matching annotation filter", + annotationFilter: "kubernetes.io/ingress.class in (external-ingress)", + labelSelector: labels.SelectorFromSet(labels.Set{"kubernetes.io/ingress.class": "external-ingress"}), + loadBalancer: fakeAmbassadorLoadBalancerService{ + ips: []string{"8.8.8.8"}, + hostnames: []string{"lb.com"}, + name: "emissary", + namespace: "emissary-ingress", + }, + ambassadorHostItems: []fakeAmbassadorHost{ + { + name: "fake1", + namespace: "testing1", + hostname: "fake1.org", + annotations: map[string]string{ + "external-dns.ambassador-service": "emissary-ingress/emissary", + "kubernetes.io/ingress.class": "internal-ingress", + }, + labels: map[string]string{ + "kubernetes.io/ingress.class": "external-ingress", + }, }, }, expected: []*endpoint.Endpoint{}, @@ -432,6 +602,7 @@ func testAmbassadorSourceEndpoints(t *testing.T) { fakeKubernetesClient, ti.targetNamespace, ti.annotationFilter, + ti.labelSelector, ) require.NoError(t, err) @@ -506,6 +677,7 @@ type fakeAmbassadorHost struct { name string annotations map[string]string hostname string + labels map[string]string } func (ir fakeAmbassadorHost) Host() *ambassador.Host { @@ -518,6 +690,7 @@ func (ir fakeAmbassadorHost) Host() *ambassador.Host { Namespace: ir.namespace, Name: ir.name, Annotations: ir.annotations, + Labels: ir.labels, }, Spec: &spec, } diff --git a/source/store.go b/source/store.go index 7c1eef945d..f67091d315 100644 --- a/source/store.go +++ b/source/store.go @@ -276,7 +276,7 @@ func BuildWithConfig(ctx context.Context, source string, p ClientGenerator, cfg if err != nil { return nil, err } - return NewAmbassadorHostSource(ctx, dynamicClient, kubernetesClient, cfg.Namespace, cfg.AnnotationFilter) + return NewAmbassadorHostSource(ctx, dynamicClient, kubernetesClient, cfg.Namespace, cfg.AnnotationFilter, cfg.LabelFilter) case "contour-httpproxy": dynamicClient, err := p.DynamicKubernetesClient() if err != nil {