Adding `NS` support for Azure DNS

[sebader]

Description

This adds support for managing NS records in Azure DNS as well. Not sure if there was a particular reason why this wasn't already there. Required in k8gb https://github.com/k8gb-io/k8gb/pull/912

Fixes #2826

I did update the unit tests but I do need some help on updating the one for deletion. I am not sure what is still missing but it's still failing:

--- FAIL: TestAzureApplyChangesZoneName (0.00s)
    azure_test.go:271: 
                Error Trace:    azure_test.go:271
                                                        azure_test.go:512
                Error:          Should be true
                Test:           TestAzureApplyChangesZoneName
                Messages:       expected and actual endpoints don't match. [deleted.foo.example.com 0 IN A   [] deletedcname.foo.example.com 0 IN CNAME   [] old.foo.example.com 0 IN A   [] oldcname.foo.example.com 0 IN CNAME   []]:[old.foo.example.com 0 IN A   [] oldcname.foo.example.com 0 IN CNAME   [] ns-old.nope.example.com 0 IN NS   [] deleted.foo.example.com 0 IN A   [] deletedcname.foo.example.com 0 IN CNAME   []]
FAIL

Checklist

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

[linux-foundation-easycla[bot]]

The committers listed above are authorized under a signed CLA.

• :white_check_mark: login: sebader / name: Sebastian (a9481faad7079cc440fbe770a0e3c47ccfacbac9, a4c607d367302b01a4ef19f7c7987804ac10cff2)

[k8s-ci-robot]

Welcome @sebader!

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:

[sebader]

@JAORMX as I wrote, this is not yet ready to be approved. One test is failing....

@njuettner @Raffo could you maybe please take a look at this and help me out so we can get this forward? thanks!

[Ziphone]

@sebader: Anything remaining but the LGTMs?

[sebader]

@Ziphone there is still the failing tests which I need help with

[Ziphone]

/test ?

[k8s-ci-robot]

@Ziphone: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/test ?

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.

[Ziphone]

@sebader: The test fails because it expects this record to get deleted: https://github.com/sebader/external-dns/blob/feature/sebader-azure-ns/provider/azure/azure_test.go#L508

What blocks the deletion is the domain filter of the test provider: https://github.com/sebader/external-dns/blob/feature/sebader-azure-ns/provider/azure/azure_test.go#L546

The test will pass if you replace the occurrences of ns-old.nope.example.com with e.g ns-old.foo.example.com (in file provider/azure/azure_test.go). Guess subdomain nope.example.com was introduced to ensure that External DNS will not tamper with records outside the scope of the domain filter.

Please involve me if you don't have time to get this change through. Very grateful for your initiative.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: JAORMX, sebader Once this PR has been reviewed and has the lgtm label, please assign szuecs for approval by writing /assign @szuecs 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

[sebader]

@Ziphone awesome, thanks so much for this! Indeed it fixed the failing test

ready for review :)

[Ziphone]

Any thoughts on when we can get a review here, @Raffo?

[ytsarev]

/assign @Raffo

[sebader]

@sebader, thanks a lot for this work! Could you please squash the commits?

done

[sebader]

PR looks good, can you add information to the docs that NS records will not work if the managed-record-type is not configured to also use NS records?

sorry, @Raffo I'm not sure what you refer to?

[Raffo]

@sebader without that flag ( see https://github.com/kubernetes-sigs/external-dns/blob/8e058caf83711370c3d8b1b3667ade5f471f0893/pkg/apis/externaldns/types.go#L426 ) the NS records will not work. We don't want to enable NS records by default because ExternalDNS was never made to manage all DNS records, rather to enable simpler PaaS like workflows in which apps want to expose DNS records. To make this feature usable by users, we need to add that information to the tutorial.

I would also ask if you end-to-end tested this with azure because we don't really have infra to test on azure at this time, I'd be great if you can verify that everything works.

[sebader]

ah got it @Raffo. I added a note to the docs, is that ok?

Yes, I have tested the new NS records through k8gb - I guess there the managed-record-types is already set to enable NS under the hood, which is why I wasnt aware of this.

[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

[ytsarev]

/remove-lifecycle stale

[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.

[mloiseleur]

@sebader Do you think you can rebase this PR ?

[sebader]

@sebader Sebastian Bader FTE Do you think you can rebase this PR ?

puh... I haven't touched this in ages and not even a working local setup for this at the moment. But if anybody else could pick this up, as you can see, the actual change was pretty minimal to re-create that from scratch.

[mloiseleur]

Ok. I'll close this PR then. If/when anyone needs it, a new PR can be opened. /close

[k8s-ci-robot]

@mloiseleur: Closed this PR.

In response to this:

Ok. I'll close this PR then. If/when anyone needs it, a new PR can be opened. /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.

[ytsarev]

/reopen

I would love to get this PR merged and avoid running a fork with NS support in https://github.com/k8gb-io/external-dns

[k8s-ci-robot]

@ytsarev: Reopened this PR.

In response to this:

/reopen

I would love to get this PR merged and avoid running a fork with NS support in https://github.com/k8gb-io/external-dns

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: JAORMX Once this PR has been reviewed and has the lgtm label, please ask for approval from raffo. 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

[mloiseleur]

@ytsarev Sure. Do you think you can make your own or rebase this PR, then ?

[ytsarev]

@mloiseleur yes, I will be also able to test it e2e

[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

[ytsarev]

/remove-lifecycle stale