Repository: kubernetes-sigs/external-dns (language: go)

Issue:
Allow to configure a ingressClassName filter
<!-- Please only use this template for submitting enhancement requests. This can be something like a new provider or a new gateway.  -->

**What would you like to be added**:
Currently there is the option to configure a `--annotation-filter` and a `--source` filter [0]. Since Kubernetes 1.19 the Ingress Resource was promoted to GA and got a new ingressClassName field.

I'd like to have a option in External DNS to configure a `--ingressClassName-filter`.


**Why is this needed**:

We use 2 external dns and 2 ingress deployments. One for private DNS zones, One for public DNS zones. We configured one external dns to filter for the internal ingress and the other one to filter for the external ingress, just like its described in the FAQ [0].

Now with the GA of the Ingress API there is a dedicated field to configure your Ingress called `ingressClassName`. And the old annotation is now deprecated. To avoid configuration errors (e.g. setting annotations AND ingressClassName) it would be much better if external dns supports a new kind of filter based on based on `ingressClassNames`

[0] https://github.com/kubernetes-sigs/external-dns/blob/master/docs/faq.md#running-an-internal-and-external-dns-service
[1] https://kubernetes.io/docs/concepts/services-networking/ingress/#deprecated-annotation



Discussion / hints:

Fix patch:
diff --git a/docs/faq.md b/docs/faq.md
index 30af2309ca..10a36422f5 100644
--- a/docs/faq.md
+++ b/docs/faq.md
@@ -258,18 +258,23 @@ Sometimes you need to run an internal and an external dns service.
 The internal one should provision hostnames used on the internal network (perhaps inside a VPC), and the external
 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 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)`.
+To do this with ExternalDNS you can use the `--ingress-class` 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`
+then you can start two ExternalDNS providers one with `--ingress-class=nginx-internal` and one with `--ingress-class=nginx-external`.
 
-If you need to search for multiple values of said annotation, you can provide a comma separated list, like so:
-`--annotation-filter=kubernetes.io/ingress.class in (nginx-internal, alb-ingress-internal)`.
+If you need to search for multiple ingress classes, you can specify the argument multiple times, like so:
+`--ingress-class=nginx-internal --ingress-class=alb-ingress-internal`.
 
-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`.
+The `--ingress-class` argument will check both the `ingressClassName` field as well as the deprecated `kubernetes.io/ingress.class` annotation.
 
-**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.
+Note: the `--ingress-class` argument cannot be used at the same time as a `kubernetes.io/ingress.class` annotation filter; if you do this an error will be raised.
+
+If you use annotations to indicate different ingress classes in your cluster, you can instead use an `--annotation-filter` argument to restrict which objects ExternalDNS considers; for example, `--annotation-filter=kubernetes.io/ingress.class in (public,dmz)`.
+
+However, beware when using annotation filters with multiple sources, e.g. `--source=service --source=ingress`, since `--annotation-filter` will filter every given source objects.
+If you need to use annotation filters against a specific source you have to run a separated external dns service containing only the wanted `--source`  and `--annotation-filter`.
+
+**Note:** Filtering based on annotation or ingress class name 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.
diff --git a/docs/tutorials/public-private-route53.md b/docs/tutorials/public-private-route53.md
index 764a7e7474..b8110ecbd2 100644
--- a/docs/tutorials/public-private-route53.md
+++ b/docs/tutorials/public-private-route53.md
@@ -213,7 +213,7 @@ spec:
 
 Consult [AWS ExternalDNS setup docs](aws.md) for installation guidelines.
 
-In ExternalDNS containers args, make sure to specify `annotation-filter` and `aws-zone-type`:
+In ExternalDNS containers args, make sure to specify `aws-zone-type` and either `ingress-class` or `annotation-filter` (depending on whether your cluster makes use of `ingressClassName`):
 
 ```yaml
 apiVersion: apps/v1beta2
@@ -241,7 +241,9 @@ spec:
         - --provider=aws
         - --registry=txt
         - --txt-owner-id=external-dns
-        - --annotation-filter=kubernetes.io/ingress.class in (external-ingress)
+        - --ingress-class=external-ingress
+        # ... or, if you use annotations for ingress classes
+        # - --annotation-filter=kubernetes.io/ingress.class in (external-ingress)
         - --aws-zone-type=public
         image: k8s.gcr.io/external-dns/external-dns:v0.7.6
         name: external-dns-public
@@ -251,7 +253,7 @@ spec:
 
 Consult [AWS ExternalDNS setup docs](aws.md) for installation guidelines.
 
-In ExternalDNS containers args, make sure to specify `annotation-filter` and `aws-zone-type`:
+In ExternalDNS containers args, make sure to specify `aws-zone-type` and either `ingress-class` or `annotation-filter` (depending on whether your cluster makes use of `ingressClassName`):
 
 ```yaml
 apiVersion: apps/v1beta2
@@ -279,7 +281,9 @@ spec:
         - --provider=aws
         - --registry=txt
         - --txt-owner-id=dev.k8s.nexus
-        - --annotation-filter=kubernetes.io/ingress.class in (internal-ingress)
+        - --ingress-class=internal-ingress
+        # ... or, if you use annotations for ingress classes
+        # - --annotation-filter=kubernetes.io/ingress.class in (internal-ingress)
         - --aws-zone-type=private
         image: k8s.gcr.io/external-dns/external-dns:v0.7.6
         name: external-dns-private
@@ -287,20 +291,23 @@ spec:
 
 ## Create application Service definitions
 
-For this setup to work, you've to create two Service definitions for your application.
+For this setup to work, you need to create two Ingress definitions for your application.
 
-At first, create public Service definition:
+At first, create public Ingress definition (make sure to un-comment either the `annotations` or `ingressClassName` lines):
 
 ```yaml
 apiVersion: networking.k8s.io/v1
 kind: Ingress
 metadata:
-  annotations:
-    kubernetes.io/ingress.class: "external-ingress"
+  # uncomment if you use annotations for ingress classes
+  # annotations:
+  #   kubernetes.io/ingress.class: "external-ingress"
   labels:
     app: app
   name: app-public
 spec:
+  # uncomment if you use ingressClassName
+  # ingressClassName: external-ingress
   rules:
   - host: app.domain.com
     http:
@@ -310,18 +317,21 @@ spec:
           servicePort: 80
 ```
 
-Then create private Service definition:
+Then create private Ingress definition (again, make sure to un-comment either the `annotations` or `ingressClassName` lines):
 
 ```yaml
 apiVersion: networking.k8s.io/v1
 kind: Ingress
 metadata:
-  annotations:
-    kubernetes.io/ingress.class: "internal-ingress"
+  # uncomment if you use annotations for ingress classes
+  # annotations:
+  #   kubernetes.io/ingress.class: "internal-ingress"
   labels:
     app: app
   name: app-private
 spec:
+  # uncomment if you use ingressClassName
+  # ingressClassName: internal-ingress
   rules:
   - host: app.domain.com
     http:
diff --git a/main.go b/main.go
index fd3b48465e..b7501d4c51 100644
--- a/main.go
+++ b/main.go
@@ -109,6 +109,7 @@ func main() {
 		Namespace:                      cfg.Namespace,
 		AnnotationFilter:               cfg.AnnotationFilter,
 		LabelFilter:                    labelSelector,
+		IngressClassNames:              cfg.IngressClassNames,
 		FQDNTemplate:                   cfg.FQDNTemplate,
 		CombineFQDNAndAnnotation:       cfg.CombineFQDNAndAnnotation,
 		IgnoreHostnameAnnotation:       cfg.IgnoreHostnameAnnotation,
diff --git a/pkg/apis/externaldns/types.go b/pkg/apis/externaldns/types.go
index 3b06e3a004..cb20983058 100644
--- a/pkg/apis/externaldns/types.go
+++ b/pkg/apis/externaldns/types.go
@@ -55,6 +55,7 @@ type Config struct {
 	Namespace                         string
 	AnnotationFilter                  string
 	LabelFilter                       string
+	IngressClassNames                 []string
 	FQDNTemplate                      string
 	CombineFQDNAndAnnotation          bool
 	IgnoreHostnameAnnotation          bool
@@ -199,6 +200,7 @@ var defaultConfig = &Config{
 	Namespace:                   "",
 	AnnotationFilter:            "",
 	LabelFilter:                 labels.Everything().String(),
+	IngressClassNames:           nil,
 	FQDNTemplate:                "",
 	CombineFQDNAndAnnotation:    false,
 	IgnoreHostnameAnnotation:    false,
@@ -380,6 +382,7 @@ func (cfg *Config) ParseFlags(args []string) error {
 	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 supported by source types CRD, ingress, service and openshift-route").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)
 	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)
diff --git a/source/ingress.go b/source/ingress.go
index 76d5429e7f..56833e15c1 100644
--- a/source/ingress.go
+++ b/source/ingress.go
@@ -18,6 +18,7 @@ package source
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"sort"
 	"strings"
@@ -26,6 +27,7 @@ import (
 	log "github.com/sirupsen/logrus"
 	networkv1 "k8s.io/api/networking/v1"
 	"k8s.io/apimachinery/pkg/labels"
+	"k8s.io/apimachinery/pkg/selection"
 	kubeinformers "k8s.io/client-go/informers"
 	netinformers "k8s.io/client-go/informers/networking/v1"
 	"k8s.io/client-go/kubernetes"
@@ -53,6 +55,7 @@ type ingressSource struct {
 	client                   kubernetes.Interface
 	namespace                string
 	annotationFilter         string
+	ingressClassNames        []string
 	fqdnTemplate             *template.Template
 	combineFQDNAnnotation    bool
 	ignoreHostnameAnnotation bool
@@ -63,12 +66,27 @@ type ingressSource struct {
 }
 
 // NewIngressSource creates a new ingressSource with the given config.
-func NewIngressSource(ctx context.Context, kubeClient kubernetes.Interface, namespace, annotationFilter string, fqdnTemplate string, combineFqdnAnnotation bool, ignoreHostnameAnnotation bool, ignoreIngressTLSSpec bool, ignoreIngressRulesSpec bool, labelSelector labels.Selector) (Source, error) {
+func NewIngressSource(ctx context.Context, kubeClient kubernetes.Interface, namespace, annotationFilter string, fqdnTemplate string, combineFqdnAnnotation bool, ignoreHostnameAnnotation bool, ignoreIngressTLSSpec bool, ignoreIngressRulesSpec bool, labelSelector labels.Selector, ingressClassNames []string) (Source, error) {
 	tmpl, err := parseTemplate(fqdnTemplate)
 	if err != nil {
 		return nil, err
 	}
 
+	// ensure that ingress class is only set in either the ingressClassNames or
+	// annotationFilter but not both
+	if ingressClassNames != nil && annotationFilter != "" {
+		selector, err := getLabelSelector(annotationFilter)
+		if err != nil {
+			return nil, err
+		}
+
+		requirements, _ := selector.Requirements()
+		for _, requirement := range requirements {
+			if requirement.Key() == "kubernetes.io/ingress.class" {
+				return nil, errors.New("--ingress-class is mutually exclusive with the kubernetes.io/ingress.class annotation filter")
+			}
+		}
+	}
 	// Use shared informer to listen for add/update/delete of ingresses in the specified namespace.
 	// Set resync period to 0, to prevent processing when nothing has changed.
 	informerFactory := kubeinformers.NewSharedInformerFactoryWithOptions(kubeClient, 0, kubeinformers.WithNamespace(namespace))
@@ -93,6 +111,7 @@ func NewIngressSource(ctx context.Context, kubeClient kubernetes.Interface, name
 		client:                   kubeClient,
 		namespace:                namespace,
 		annotationFilter:         annotationFilter,
+		ingressClassNames:        ingressClassNames,
 		fqdnTemplate:             tmpl,
 		combineFQDNAnnotation:    combineFqdnAnnotation,
 		ignoreHostnameAnnotation: ignoreHostnameAnnotation,
@@ -116,6 +135,11 @@ func (sc *ingressSource) Endpoints(ctx context.Context) ([]*endpoint.Endpoint, e
 		return nil, err
 	}
 
+	ingresses, err = sc.filterByIngressClass(ingresses)
+	if err != nil {
+		return nil, err
+	}
+
 	endpoints := []*endpoint.Endpoint{}
 
 	for _, ing := range ingresses {
@@ -210,6 +234,48 @@ func (sc *ingressSource) filterByAnnotations(ingresses []*networkv1.Ingress) ([]
 	return filteredList, nil
 }
 
+// filterByIngressClass filters a list of ingresses based on a required ingress
+// class
+func (sc *ingressSource) filterByIngressClass(ingresses []*networkv1.Ingress) ([]*networkv1.Ingress, error) {
+	// if no class filter is specified then there's nothing to do
+	if sc.ingressClassNames == nil {
+		return ingresses, nil
+	}
+
+	classNameReq, err := labels.NewRequirement("kubernetes.io/ingress.class", selection.In, sc.ingressClassNames)
+	if err != nil {
+		return nil, err
+	}
+
+	selector := labels.NewSelector()
+	selector = selector.Add(*classNameReq)
+
+	filteredList := []*networkv1.Ingress{}
+
+	for _, ingress := range ingresses {
+		var matched = false
+
+		for _, nameFilter := range sc.ingressClassNames {
+			if ingress.Spec.IngressClassName != nil && nameFilter == *ingress.Spec.IngressClassName {
+				matched = true
+			} else if matchLabelSelector(selector, ingress.Annotations) {
+				matched = true
+			}
+
+			if matched {
+				filteredList = append(filteredList, ingress)
+				break
+			}
+		}
+
+		if !matched {
+			log.Debugf("Discarding ingress %s/%s because it does not match required ingress classes %v", ingress.Namespace, ingress.Name, sc.ingressClassNames)
+		}
+	}
+
+	return filteredList, nil
+}
+
 func (sc *ingressSource) setResourceLabel(ingress *networkv1.Ingress, endpoints []*endpoint.Endpoint) {
 	for _, ep := range endpoints {
 		ep.Labels[endpoint.ResourceLabelKey] = fmt.Sprintf("ingress/%s/%s", ingress.Namespace, ingress.Name)
diff --git a/source/store.go b/source/store.go
index 942400aef5..5aff8371c7 100644
--- a/source/store.go
+++ b/source/store.go
@@ -45,6 +45,7 @@ type Config struct {
 	Namespace                      string
 	AnnotationFilter               string
 	LabelFilter                    labels.Selector
+	IngressClassNames              []string
 	FQDNTemplate                   string
 	CombineFQDNAndAnnotation       bool
 	IgnoreHostnameAnnotation       bool
@@ -192,7 +193,7 @@ func BuildWithConfig(ctx context.Context, source string, p ClientGenerator, cfg
 		if err != nil {
 			return nil, err
 		}
-		return NewIngressSource(ctx, client, cfg.Namespace, cfg.AnnotationFilter, cfg.FQDNTemplate, cfg.CombineFQDNAndAnnotation, cfg.IgnoreHostnameAnnotation, cfg.IgnoreIngressTLSSpec, cfg.IgnoreIngressRulesSpec, cfg.LabelFilter)
+		return NewIngressSource(ctx, client, cfg.Namespace, cfg.AnnotationFilter, cfg.FQDNTemplate, cfg.CombineFQDNAndAnnotation, cfg.IgnoreHostnameAnnotation, cfg.IgnoreIngressTLSSpec, cfg.IgnoreIngressRulesSpec, cfg.LabelFilter, cfg.IngressClassNames)
 	case "pod":
 		client, err := p.KubeClient()
 		if err != nil {


Test patch:
diff --git a/source/ingress_test.go b/source/ingress_test.go
index 75a9703566..a3bfc457af 100644
--- a/source/ingress_test.go
+++ b/source/ingress_test.go
@@ -66,6 +66,7 @@ func (suite *IngressSuite) SetupTest() {
 		false,
 		false,
 		labels.Everything(),
+		[]string{},
 	)
 	suite.NoError(err, "should initialize ingress source")
 }
@@ -102,6 +103,7 @@ func TestNewIngressSource(t *testing.T) {
 		fqdnTemplate             string
 		combineFQDNAndAnnotation bool
 		expectError              bool
+		ingressClassNames        []string
 	}{
 		{
 			title:        "invalid template",
@@ -133,6 +135,17 @@ func TestNewIngressSource(t *testing.T) {
 			expectError:      false,
 			annotationFilter: "kubernetes.io/ingress.class=nginx",
 		},
+		{
+			title:            "non-empty ingress class name list",
+			expectError:      false,
+			ingressClassNames: []string{"internal", "external"},
+		},
+		{
+			title:            "ingress class name and annotation filter jointly specified",
+			expectError:      true,
+			ingressClassNames: []string{"internal", "external"},
+			annotationFilter: "kubernetes.io/ingress.class=nginx",
+		},
 	} {
 		ti := ti
 		t.Run(ti.title, func(t *testing.T) {
@@ -149,6 +162,7 @@ func TestNewIngressSource(t *testing.T) {
 				false,
 				false,
 				labels.Everything(),
+				ti.ingressClassNames,
 			)
 			if ti.expectError {
 				assert.Error(t, err)
@@ -364,6 +378,7 @@ func testIngressEndpoints(t *testing.T) {
 		ignoreIngressTLSSpec     bool
 		ignoreIngressRulesSpec   bool
 		ingressLabelSelector     labels.Selector
+		ingressClassNames   []string
 	}{
 		{
 			title:           "no ingress",
@@ -1176,6 +1191,57 @@ func testIngressEndpoints(t *testing.T) {
 			},
 		},
 		{
+			title:                  "ingressClassName filtering",
+			targetNamespace:        "",
+			ingressClassNames:      []string{"public", "dmz"},
+			ingressItems: []fakeIngress{
+				{
+					name:             "fake-public",
+					namespace:        namespace,
+					tlsdnsnames:      [][]string{{"example.org"}},
+					ips:              []string{"1.2.3.4"},
+					ingressClassName: "public",
+				},
+				{
+					name:             "fake-internal",
+					namespace:        namespace,
+					tlsdnsnames:      [][]string{{"int.example.org"}},
+					ips:              []string{"2.3.4.5"},
+					ingressClassName: "internal",
+				},
+				{
+					name:             "fake-dmz",
+					namespace:        namespace,
+					tlsdnsnames:      [][]string{{"dmz.example.org"}},
+					ips:              []string{"3.4.5.6"},
+					ingressClassName: "dmz",
+				},
+				{
+					name:             "annotated-dmz",
+					namespace:        namespace,
+					tlsdnsnames:      [][]string{{"annodmz.example.org"}},
+					ips:              []string{"4.5.6.7"},
+					annotations: map[string]string{
+						"kubernetes.io/ingress.class": "dmz",
+					},
+				},
+			},
+			expected: []*endpoint.Endpoint{
+				{
+					DNSName: "example.org",
+					Targets: endpoint.Targets{"1.2.3.4"},
+				},
+				{
+					DNSName: "dmz.example.org",
+					Targets: endpoint.Targets{"3.4.5.6"},
+				},
+				{
+					DNSName: "annodmz.example.org",
+					Targets: endpoint.Targets{"4.5.6.7"},
+				},
+			},
+                    },
+                    {
 			ingressLabelSelector: labels.SelectorFromSet(labels.Set{"app": "web-external"}),
 			title:                "ingress with matching labels",
 			targetNamespace:      "",
@@ -1237,6 +1303,7 @@ func testIngressEndpoints(t *testing.T) {
 				ti.ignoreIngressTLSSpec,
 				ti.ignoreIngressRulesSpec,
 				ti.ingressLabelSelector,
+				ti.ingressClassNames,
 			)
 			// Informer cache has all of the ingresses. Retrieve and validate their endpoints.
 			res, err := source.Endpoints(context.Background())
@@ -1252,14 +1319,15 @@ func testIngressEndpoints(t *testing.T) {
 
 // ingress specific helper functions
 type fakeIngress struct {
-	dnsnames    []string
-	tlsdnsnames [][]string
-	ips         []string
-	hostnames   []string
-	namespace   string
-	name        string
-	annotations map[string]string
-	labels      map[string]string
+	dnsnames         []string
+	tlsdnsnames      [][]string
+	ips              []string
+	hostnames        []string
+	namespace        string
+	name             string
+	annotations      map[string]string
+	labels           map[string]string
+	ingressClassName string
 }
 
 func (ing fakeIngress) Ingress() *networkv1.Ingress {
@@ -1272,6 +1340,7 @@ func (ing fakeIngress) Ingress() *networkv1.Ingress {
 		},
 		Spec: networkv1.IngressSpec{
 			Rules: []networkv1.IngressRule{},
+			IngressClassName: &ing.ingressClassName,
 		},
 		Status: networkv1.IngressStatus{
 			LoadBalancer: v1.LoadBalancerStatus{