Label filtering for Ingress, Service, Openshift Route sources

Currently the `--label-filter` flag can only be used to filter CRDs
which match the label selector passed through that flag. This change
extends the functionality to the Ingress, Service and Openshift Route
type objects. 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.

docs/faq.md

@@ -255,7 +255,7 @@ The internal one should provision hostnames used on the internal network (perhap
 one to expose DNS to the internet.
 
 To do this with ExternalDNS you can use the `--annotation-filter` to specifically tie an instance of ExternalDNS to
-an instance of a ingress controller. Let's assume you have two ingress controllers `nginx-internal` and `nginx-external`
+an instance of an ingress controller. Let's assume you have two ingress controllers `nginx-internal` and `nginx-external`
 then you can start two ExternalDNS providers one with `--annotation-filter=kubernetes.io/ingress.class in (nginx-internal)`
 and one with `--annotation-filter=kubernetes.io/ingress.class in (nginx-external)`.
 
@@ -265,6 +265,11 @@ If you need to search for multiple values of said annotation, you can provide a
 Beware when using multiple sources, e.g. `--source=service --source=ingress`, `--annotation-filter` will filter every given source objects.
 If you need to filter only one specific source you have to run a separated external dns service containing only the wanted `--source`  and `--annotation-filter`.
 
+**Note:** Filtering based on annotation means that the external-dns controller will receive all resources of that kind and then filter on the client-side.
+In larger clusters with many resources which change frequently this can cause performance issues. If only some resources need to be managed by an instance
+of external-dns then label filtering can be used instead of annotation filtering. This means that only those resources which match the selector specified
+in `--label-filter` will be passed to the controller.
+
 ### How do I specify that I want the DNS record to point to either the Node's public or private IP when it has both?
 
 If your Nodes have both public and private IP addresses, you might want to write DNS records with one or the other.

main.go

@@ -26,6 +26,7 @@ import (
 
 	"github.com/prometheus/client_golang/prometheus/promhttp"
 	log "github.com/sirupsen/logrus"
+	"k8s.io/apimachinery/pkg/labels"
 	_ "k8s.io/client-go/plugin/pkg/client/auth"
 
 	"sigs.k8s.io/external-dns/controller"
@@ -99,11 +100,14 @@ func main() {
 	go serveMetrics(cfg.MetricsAddress)
 	go handleSigterm(cancel)
 
+	// error is explicitly ignored because the filter is already validated in validation.ValidateConfig
+	labelSelector, _ := labels.Parse(cfg.LabelFilter)
+
 	// Create a source.Config from the flags passed by the user.
 	sourceCfg := &source.Config{
 		Namespace:                      cfg.Namespace,
 		AnnotationFilter:               cfg.AnnotationFilter,
-		LabelFilter:                    cfg.LabelFilter,
+		LabelFilter:                    labelSelector,
 		FQDNTemplate:                   cfg.FQDNTemplate,
 		CombineFQDNAndAnnotation:       cfg.CombineFQDNAndAnnotation,
 		IgnoreHostnameAnnotation:       cfg.IgnoreHostnameAnnotation,

pkg/apis/externaldns/types.go

@@ -23,6 +23,8 @@ import (
 	"strconv"
 	"time"
 
+	"k8s.io/apimachinery/pkg/labels"
+
 	"sigs.k8s.io/external-dns/endpoint"
 
 	"github.com/alecthomas/kingpin"
@@ -185,7 +187,7 @@ var defaultConfig = &Config{
 	Sources:                     nil,
 	Namespace:                   "",
 	AnnotationFilter:            "",
-	LabelFilter:                 "",
+	LabelFilter:                 labels.Everything().String(),
 	FQDNTemplate:                "",
 	CombineFQDNAndAnnotation:    false,
 	IgnoreHostnameAnnotation:    false,
@@ -361,7 +363,7 @@ func (cfg *Config) ParseFlags(args []string) error {
 	app.Flag("source", "The resource types that are queried for endpoints; specify multiple times for multiple sources (required, options: service, ingress, node, fake, connector, istio-gateway, istio-virtualservice, cloudfoundry, contour-ingressroute, contour-httpproxy, gloo-proxy, crd, empty, skipper-routegroup, openshift-route, ambassador-host, kong-tcpingress)").Required().PlaceHolder("source").EnumsVar(&cfg.Sources, "service", "ingress", "node", "pod", "istio-gateway", "istio-virtualservice", "cloudfoundry", "contour-ingressroute", "contour-httpproxy", "gloo-proxy", "fake", "connector", "crd", "empty", "skipper-routegroup", "openshift-route", "ambassador-host", "kong-tcpingress")
 	app.Flag("namespace", "Limit sources of endpoints to a specific namespace (default: all namespaces)").Default(defaultConfig.Namespace).StringVar(&cfg.Namespace)
 	app.Flag("annotation-filter", "Filter sources managed by external-dns via annotation using label selector semantics (default: all sources)").Default(defaultConfig.AnnotationFilter).StringVar(&cfg.AnnotationFilter)
-	app.Flag("label-filter", "Filter sources managed by external-dns via label selector when listing all resources; currently only supported by source CRD").Default(defaultConfig.LabelFilter).StringVar(&cfg.LabelFilter)
+	app.Flag("label-filter", "Filter sources managed by external-dns via label selector when listing all resources; currently supported by source types CRD, ingress, service and openshift-route").Default(defaultConfig.LabelFilter).StringVar(&cfg.LabelFilter)
 	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)
 	app.Flag("ignore-hostname-annotation", "Ignore hostname annotation when generating DNS names, valid only when using fqdn-template is set (optional, default: false)").BoolVar(&cfg.IgnoreHostnameAnnotation)

pkg/apis/externaldns/validation/validation.go

@@ -20,6 +20,8 @@ import (
 	"errors"
 	"fmt"
 
+	"k8s.io/apimachinery/pkg/labels"
+
 	"sigs.k8s.io/external-dns/pkg/apis/externaldns"
 )
 
@@ -110,5 +112,9 @@ func ValidateConfig(cfg *externaldns.Config) error {
 		return errors.New("txt-prefix and txt-suffix are mutual exclusive")
 	}
 
+	_, err := labels.Parse(cfg.LabelFilter)
+	if err != nil {
+		return errors.New("--label-filter does not specify a valid label selector")
+	}
 	return nil
 }

source/crd.go

@@ -43,7 +43,7 @@ type crdSource struct {
 	crdResource      string
 	codec            runtime.ParameterCodec
 	annotationFilter string
-	labelFilter      string
+	labelSelector    labels.Selector
 }
 
 func addKnownTypes(scheme *runtime.Scheme, groupVersion schema.GroupVersion) error {
@@ -103,12 +103,12 @@ func NewCRDClientForAPIVersionKind(client kubernetes.Interface, kubeConfig, apiS
 }
 
 // NewCRDSource creates a new crdSource with the given config.
-func NewCRDSource(crdClient rest.Interface, namespace, kind string, annotationFilter string, labelFilter string, scheme *runtime.Scheme) (Source, error) {
+func NewCRDSource(crdClient rest.Interface, namespace, kind string, annotationFilter string, labelSelector labels.Selector, scheme *runtime.Scheme) (Source, error) {
 	return &crdSource{
 		crdResource:      strings.ToLower(kind) + "s",
 		namespace:        namespace,
 		annotationFilter: annotationFilter,
-		labelFilter:      labelFilter,
+		labelSelector:    labelSelector,
 		crdClient:        crdClient,
 		codec:            runtime.NewParameterCodec(scheme),
 	}, nil
@@ -126,11 +126,7 @@ func (cs *crdSource) Endpoints(ctx context.Context) ([]*endpoint.Endpoint, error
 		err    error
 	)
 
-	if cs.labelFilter != "" {
-		result, err = cs.List(ctx, &metav1.ListOptions{LabelSelector: cs.labelFilter})
-	} else {
-		result, err = cs.List(ctx, &metav1.ListOptions{})
-	}
+	result, err = cs.List(ctx, &metav1.ListOptions{LabelSelector: cs.labelSelector.String()})
 	if err != nil {
 		return nil, err
 	}

source/crd_test.go

@@ -30,6 +30,7 @@ import (
 	"github.com/stretchr/testify/require"
 	"github.com/stretchr/testify/suite"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/runtime/serializer"
@@ -381,9 +382,13 @@ func testCRDSourceEndpoints(t *testing.T) {
 			require.NoError(t, err)
 
 			scheme := runtime.NewScheme()
-			addKnownTypes(scheme, groupVersion)
+			require.NoError(t, addKnownTypes(scheme, groupVersion))
 
-			cs, _ := NewCRDSource(restClient, ti.namespace, ti.kind, ti.annotationFilter, ti.labelFilter, scheme)
+			labelSelector, err := labels.Parse(ti.labelFilter)
+			require.NoError(t, err)
+
+			cs, err := NewCRDSource(restClient, ti.namespace, ti.kind, ti.annotationFilter, labelSelector, scheme)
+			require.NoError(t, err)
 
 			receivedEndpoints, err := cs.Endpoints(context.Background())
 			if ti.expectError {

source/ingress.go

@@ -60,10 +60,11 @@ type ingressSource struct {
 	ingressInformer          netinformers.IngressInformer
 	ignoreIngressTLSSpec     bool
 	ignoreIngressRulesSpec   bool
+	labelSelector            labels.Selector
 }
 
 // NewIngressSource creates a new ingressSource with the given config.
-func NewIngressSource(kubeClient kubernetes.Interface, namespace, annotationFilter string, fqdnTemplate string, combineFqdnAnnotation bool, ignoreHostnameAnnotation bool, ignoreIngressTLSSpec bool, ignoreIngressRulesSpec bool) (Source, error) {
+func NewIngressSource(kubeClient kubernetes.Interface, namespace, annotationFilter string, fqdnTemplate string, combineFqdnAnnotation bool, ignoreHostnameAnnotation bool, ignoreIngressTLSSpec bool, ignoreIngressRulesSpec bool, labelSelector labels.Selector) (Source, error) {
 	tmpl, err := parseTemplate(fqdnTemplate)
 	if err != nil {
 		return nil, err
@@ -100,14 +101,15 @@ func NewIngressSource(kubeClient kubernetes.Interface, namespace, annotationFilt
 		ingressInformer:          ingressInformer,
 		ignoreIngressTLSSpec:     ignoreIngressTLSSpec,
 		ignoreIngressRulesSpec:   ignoreIngressRulesSpec,
+		labelSelector:            labelSelector,
 	}
 	return sc, nil
 }
 
 // Endpoints returns endpoint objects for each host-target combination that should be processed.
 // Retrieves all ingress resources on all namespaces
 func (sc *ingressSource) Endpoints(ctx context.Context) ([]*endpoint.Endpoint, error) {
-	ingresses, err := sc.ingressInformer.Lister().Ingresses(sc.namespace).List(labels.Everything())
+	ingresses, err := sc.ingressInformer.Lister().Ingresses(sc.namespace).List(sc.labelSelector)
 	if err != nil {
 		return nil, err
 	}

source/ingress_test.go

@@ -26,6 +26,7 @@ import (
 	v1 "k8s.io/api/core/v1"
 	networkv1 "k8s.io/api/networking/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/client-go/kubernetes/fake"
 
 	"sigs.k8s.io/external-dns/endpoint"
@@ -63,6 +64,7 @@ func (suite *IngressSuite) SetupTest() {
 		false,
 		false,
 		false,
+		labels.Everything(),
 	)
 	suite.NoError(err, "should initialize ingress source")
 }
@@ -144,6 +146,7 @@ func TestNewIngressSource(t *testing.T) {
 				false,
 				false,
 				false,
+				labels.Everything(),
 			)
 			if ti.expectError {
 				assert.Error(t, err)
@@ -358,6 +361,7 @@ func testIngressEndpoints(t *testing.T) {
 		ignoreHostnameAnnotation bool
 		ignoreIngressTLSSpec     bool
 		ignoreIngressRulesSpec   bool
+		ingressLabelSelector     labels.Selector
 	}{
 		{
 			title:           "no ingress",
@@ -1169,6 +1173,41 @@ func testIngressEndpoints(t *testing.T) {
 				},
 			},
 		},
+		{
+			ingressLabelSelector: labels.SelectorFromSet(labels.Set{"app": "web-external"}),
+			title:                "ingress with matching labels",
+			targetNamespace:      "",
+			ingressItems: []fakeIngress{
+				{
+					name:      "fake1",
+					namespace: namespace,
+					dnsnames:  []string{"example.org"},
+					ips:       []string{"8.8.8.8"},
+					labels:    map[string]string{"app": "web-external", "name": "reverse-proxy"},
+				},
+			},
+			expected: []*endpoint.Endpoint{
+				{
+					DNSName: "example.org",
+					Targets: endpoint.Targets{"8.8.8.8"},
+				},
+			},
+		},
+		{
+			ingressLabelSelector: labels.SelectorFromSet(labels.Set{"app": "web-external"}),
+			title:                "ingress without matching labels",
+			targetNamespace:      "",
+			ingressItems: []fakeIngress{
+				{
+					name:      "fake1",
+					namespace: namespace,
+					dnsnames:  []string{"example.org"},
+					ips:       []string{"8.8.8.8"},
+					labels:    map[string]string{"app": "web-internal", "name": "reverse-proxy"},
+				},
+			},
+			expected: []*endpoint.Endpoint{},
+		},
 	} {
 		ti := ti
 		t.Run(ti.title, func(t *testing.T) {
@@ -1180,6 +1219,11 @@ func testIngressEndpoints(t *testing.T) {
 				_, err := fakeClient.NetworkingV1().Ingresses(ingress.Namespace).Create(context.Background(), ingress, metav1.CreateOptions{})
 				require.NoError(t, err)
 			}
+
+			if ti.ingressLabelSelector == nil {
+				ti.ingressLabelSelector = labels.Everything()
+			}
+
 			source, _ := NewIngressSource(
 				fakeClient,
 				ti.targetNamespace,
@@ -1189,6 +1233,7 @@ func testIngressEndpoints(t *testing.T) {
 				ti.ignoreHostnameAnnotation,
 				ti.ignoreIngressTLSSpec,
 				ti.ignoreIngressRulesSpec,
+				ti.ingressLabelSelector,
 			)
 			// Informer cache has all of the ingresses. Retrieve and validate their endpoints.
 			res, err := source.Endpoints(context.Background())
@@ -1211,6 +1256,7 @@ type fakeIngress struct {
 	namespace   string
 	name        string
 	annotations map[string]string
+	labels      map[string]string
 }
 
 func (ing fakeIngress) Ingress() *networkv1.Ingress {
@@ -1219,6 +1265,7 @@ func (ing fakeIngress) Ingress() *networkv1.Ingress {
 			Namespace:   ing.namespace,
 			Name:        ing.name,
 			Annotations: ing.annotations,
+			Labels:      ing.labels,
 		},
 		Spec: networkv1.IngressSpec{
 			Rules: []networkv1.IngressRule{},

source/openshift_route.go

@@ -48,6 +48,7 @@ type ocpRouteSource struct {
 	combineFQDNAnnotation    bool
 	ignoreHostnameAnnotation bool
 	routeInformer            routeInformer.RouteInformer
+	labelSelector            labels.Selector
 }
 
 // NewOcpRouteSource creates a new ocpRouteSource with the given config.
@@ -58,6 +59,7 @@ func NewOcpRouteSource(
 	fqdnTemplate string,
 	combineFQDNAnnotation bool,
 	ignoreHostnameAnnotation bool,
+	labelSelector labels.Selector,
 ) (Source, error) {
 	tmpl, err := parseTemplate(fqdnTemplate)
 	if err != nil {
@@ -66,11 +68,11 @@ func NewOcpRouteSource(
 
 	// Use a shared informer to listen for add/update/delete of Routes in the specified namespace.
 	// Set resync period to 0, to prevent processing when nothing has changed.
-	informerFactory := extInformers.NewFilteredSharedInformerFactory(ocpClient, 0, namespace, nil)
-	routeInformer := informerFactory.Route().V1().Routes()
+	informerFactory := extInformers.NewSharedInformerFactoryWithOptions(ocpClient, 0, extInformers.WithNamespace(namespace))
+	informer := informerFactory.Route().V1().Routes()
 
 	// Add default resource event handlers to properly initialize informer.
-	routeInformer.Informer().AddEventHandler(
+	informer.Informer().AddEventHandler(
 		cache.ResourceEventHandlerFuncs{
 			AddFunc: func(obj interface{}) {
 			},
@@ -92,7 +94,8 @@ func NewOcpRouteSource(
 		fqdnTemplate:             tmpl,
 		combineFQDNAnnotation:    combineFQDNAnnotation,
 		ignoreHostnameAnnotation: ignoreHostnameAnnotation,
-		routeInformer:            routeInformer,
+		routeInformer:            informer,
+		labelSelector:            labelSelector,
 	}, nil
 }
 
@@ -104,7 +107,7 @@ func (ors *ocpRouteSource) AddEventHandler(ctx context.Context, handler func())
 // Retrieves all OpenShift Route resources on all namespaces, unless an explicit namespace
 // is specified in ocpRouteSource.
 func (ors *ocpRouteSource) Endpoints(ctx context.Context) ([]*endpoint.Endpoint, error) {
-	ocpRoutes, err := ors.routeInformer.Lister().Routes(ors.namespace).List(labels.Everything())
+	ocpRoutes, err := ors.routeInformer.Lister().Routes(ors.namespace).List(ors.labelSelector)
 	if err != nil {
 		return nil, err
 	}