feat: add annotation and label filters to Ambassador Host Source

[KyleMartin901]

This change makes the Ambassador Host source respect the External-DNS annotation-filter and label-filter allowing for an Ambassador Host resource to specify what External-DNS deployment to use when there are multiple External-DNS deployments within the same cluster. Before this change if you had two External-DNS deployments within the cluster and used the Ambassador Host source the first External-DNS to process the resource will create the record and not the one that was specified in the filter annotation.

Annotation Fillter

I added the filterByAnnotations function so that it matched the same way the other sources have implemented annotation filtering. I didn't add the controller check only because I wanted to keep this change to implementing the annotationFilter.

I added Endpoint tests to validate that the filterByAnnotations function works as expected. Again these tests were based of the Endpoint tests that other sources use. To keep the tests simpler I only allow for a single load balancer to be used.

Example: Create two External-DNS deployments 1 public and 1 private and set the Ambassador Host to use the public External-DNS using the annotation filter.

---
apiVersion: apps/v1
kind: Deployment
metadata:
  name: external-dns-private
spec:
  strategy:
    type: Recreate
  selector:
    matchLabels:
      app: external-dns-private
  template:
    metadata:
      labels:
        app: external-dns-private
      annotations:
        iam.amazonaws.com/role: {ARN} # AWS ARN role
    spec:
      serviceAccountName: external-dns
      containers:
      - name: external-dns
        image: k8s.gcr.io/external-dns/external-dns:latest
        args:
        - --source=ambassador-host
        - --domain-filter=example.net # will make ExternalDNS see only the hosted zones matching provided domain, omit to process all available hosted zones
        - --provider=aws
        - --policy=upsert-only # would prevent ExternalDNS from deleting any records, omit to enable full synchronization
        - --aws-zone-type=private # only look at public hosted zones (valid values are public, private or no value for both)
        - --registry=txt
        - --txt-owner-id= {Hosted Zone ID} # Insert Route53 Hosted Zone ID here
        - --annotation-filter=kubernetes.io/ingress.class in (private)
---
apiVersion: apps/v1
kind: Deployment
metadata:
  name: external-dns-public
spec:
  strategy:
    type: Recreate
  selector:
    matchLabels:
      app: external-dns-public
  template:
    metadata:
      labels:
        app: external-dns-public
      annotations:
        iam.amazonaws.com/role: {ARN} # AWS ARN role
    spec:
      serviceAccountName: external-dns
      containers:
      - name: external-dns
        image: k8s.gcr.io/external-dns/external-dns:latest
        args:
        - --source=ambassador-host
        - --domain-filter=example.net # will make ExternalDNS see only the hosted zones matching provided domain, omit to process all available hosted zones
        - --provider=aws
        - --policy=upsert-only # would prevent ExternalDNS from deleting any records, omit to enable full synchronization
        - --aws-zone-type= # only look at public hosted zones (valid values are public, private or no value for both)
        - --registry=txt
        - --txt-owner-id= {Hosted Zone ID} # Insert Route53 Hosted Zone ID here
        - --annotation-filter=kubernetes.io/ingress.class in (public)
---
apiVersion: getambassador.io/v3alpha1
  kind: Host
  metadata:
    name: your-hostname
    annotations:
      external-dns.ambassador-service: emissary-ingress/emissary
      kubernetes.io/ingress.class: public
  spec:
		acmeProvider:
      authority: none
		hostname: your-hostname.example.com

Fixes kubernetes-sigs/external-dns#2632

Label Filter

Currently, the --label-filter flag can only be used to filter CRDs, Ingress, Service and Openshift Route objects that 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 in conjunction with the other sources that don't yet support label filtering.

It is possible to use label based filtering with annotation based filtering so you can initially filter by label and then filter the returned hosts by annotation. This is not recommended

Fixes kubernetes-sigs/external-dns#2761

Checklist

• [X] Unit tests updated
• [ ] End user documentation updated

[linux-foundation-easycla[bot]]

The committers listed above are authorized under a signed CLA.

• :white_check_mark: login: KyleMartin901 / name: Kyle (9a573f567470d7cc95e66bb6581de897fde54598, c7ae7f19354d9cbbe389d5a38054eb06d3ca09c6, 7da6898a0ded4ec62a7dac29c34b90d0286eacf2, 671157ce097467e4ba9683ec0d38183995f92853)

[k8s-ci-robot]

Welcome @KyleMartin901!

It looks like this is your first PR to kubernetes-sigs/external-dns 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/external-dns has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. :smiley:

[KyleMartin901]

I have also added the label filter to the Ambassador host source as per suggestion by @alebedev87 in https://github.com/kubernetes-sigs/external-dns/pull/2043#issuecomment-1121664222

[KyleMartin901]

@alebedev87 thanks for picking up those issues.

I wasn't sure on the process of adding in the changes if it is preferred for the changes to be added in as new commits or squashed so the PR still only had the two commits for adding annotation filter and label filter. I have just added them as seperate commits so it is easy to squash if that's what is preferred. Let me know if you would prefer me to squash them or happy as is.

[alebedev87]

@KyleMartin901: I'm not aware of any strict rules about the commits. However I don't think that the commits made to address the review remarks really need to be upstream.

[KyleMartin901]

@KyleMartin901: I'm not aware of any strict rules about the commits. However I don't think that the commits made to address the review remarks really need to be upstream.

I have rebased so there are only 2 commits 1 for adding the annotation filtering and 1 for adding the label filtering with all the changes suggested. Sorry for the delay in getting it all updated.

[alebedev87]

/lgtm

[alebedev87]

/assign @Raffo

[k8s-ci-robot]

@alebedev87: changing LGTM is restricted to collaborators

In response to this:

/lgtm

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: KyleMartin901 Once this PR has been reviewed and has the lgtm label, please ask for approval from raffo by writing /assign @raffo in a comment. For more information see:The Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment Approvers can cancel approval by writing /approve cancel in a comment

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue or PR as fresh with /remove-lifecycle stale
• Mark this issue or PR as rotten with /lifecycle rotten
• Close this issue or PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[KyleMartin901]

/remove-lifecycle stale

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

• Mark this PR as fresh with /remove-lifecycle stale
• Close this PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[KyleMartin901]

/remove-lifecycle stale

[mloiseleur]

@KyleMartin901 I understand it has been a long time but do you think you can update the sources part of doc and rebase your PR ? It's looking good.

[mloiseleur]

Since author is not answering, I'm closing this PR. Feel free to re-open it or open a new one if you need it. /close

[k8s-ci-robot]

@mloiseleur: Closed this PR.

In response to this:

Since author is not answering, I'm closing this PR. Feel free to re-open it or open a new one if you need it. /close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[KyleMartin901]

Sorry I am just getting back from 6 weeks holiday where I had my notifications turned off. I’ll fix up the issues this weekend @mloiseleur

[mloiseleur]

sure, no problem /reopen

[k8s-ci-robot]

@mloiseleur: Reopened this PR.

In response to this:

sure, no problem /reopen

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[KyleMartin901]

@mloiseleur thanks for reopening the PR and giving me the time to rebase and update the docs. I have now pushed those changes, let me know if there is anything else you would like me to do.

[mloiseleur]

/ok-to-test /retitle feat: add annotation and label filters to Ambassador Host Source

[KyleMartin901]

@mloiseleur sorry about the failed tests, I have fixed them up by adding the now required record type. Sorry I was lazy and just did the rebase and didn't rerun the tests but I have now run the tests and they are all passing. I added a lazy commit let me know if you want it squashed in or changed.

[mloiseleur]

/lgtm /label tide/merge-method-squash

[szuecs]

/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: szuecs

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• ~~OWNERS~~ [szuecs]

Approvers can indicate their approval by writing /approve in a comment Approvers can cancel approval by writing /approve cancel in a comment

[k8s-ci-robot]

PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[k8s-ci-robot]

@KyleMartin901: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-external-dns-licensecheck	671157ce097467e4ba9683ec0d38183995f92853	link	unknown	/test pull-external-dns-licensecheck
pull-external-dns-unit-test	671157ce097467e4ba9683ec0d38183995f92853	link	unknown	/test pull-external-dns-unit-test

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

[mloiseleur]

@KyleMartin901 This PR has been approved. It means that it just needs a rebase, to pass tests and it will be merged.

[KyleMartin901]

Thanks @mloiseleur i will attempt to get this done this week. Looks like someone got tests merged in before mine so going to refactor to match what has already been merged