Adding `NS` support for Azure DNS #2835

sebader · 2022-06-21T15:51:04Z

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 k8gb-io/k8gb#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

Unit tests updated
End user documentation updated

linux-foundation-easycla · 2022-06-21T15:51:07Z

The committers listed above are authorized under a signed CLA.

✅ login: sebader / name: Sebastian (a9481fa, a4c607d)

k8s-ci-robot · 2022-06-21T15:51:12Z

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

sebader · 2022-06-24T08:36:06Z

@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 · 2022-10-18T10:50:00Z

@sebader: Anything remaining but the LGTMs?

sebader · 2022-10-18T10:51:38Z

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

Ziphone · 2022-10-18T14:00:51Z

/test ?

k8s-ci-robot · 2022-10-18T14:01:06Z

@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 · 2022-10-30T21:51:15Z

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

sebader · 2022-10-31T10:55:39Z

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

ready for review :)

Ziphone · 2022-12-01T09:27:32Z

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

ytsarev · 2023-01-26T23:56:54Z

/assign @Raffo

ytsarev

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

sebader · 2023-01-30T16:10:32Z

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

done

Raffo

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?

sebader · 2023-02-04T19:02:52Z

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 · 2023-02-05T10:11:47Z

@sebader without that flag ( see

external-dns/pkg/apis/externaldns/types.go

Line 426 in 8e058ca

    
           app.Flag("managed-record-types", "Comma separated list of record types to manage (default: A, CNAME) (supported records: CNAME, A, NS").Default("A", "CNAME").StringsVar(&cfg.ManagedDNSRecordTypes)

) 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 · 2023-02-06T17:24:51Z

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.

ytsarev · 2023-05-08T21:29:02Z

/remove-lifecycle stale

k8s-ci-robot · 2023-05-16T05:37:16Z

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 · 2023-12-20T15:44:14Z

@sebader Do you think you can rebase this PR ?

sebader · 2023-12-20T16:00:02Z

@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 · 2023-12-22T12:58:56Z

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

k8s-ci-robot · 2023-12-22T12:59:01Z

@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 · 2024-01-02T15:11:36Z

/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 · 2024-01-02T15:11:41Z

@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 · 2024-01-02T15:11:47Z

[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 · 2024-01-02T15:33:33Z

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

ytsarev · 2024-01-02T16:24:26Z

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

k8s-triage-robot · 2024-04-01T16:31:07Z

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 · 2024-04-01T19:37:20Z

/remove-lifecycle stale

k8s-triage-robot · 2024-05-01T20:36:44Z

The Kubernetes project currently lacks enough active 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 rotten
Close this PR with /close
Offer to help out with Issue Triage

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

/lifecycle rotten

ytsarev · 2024-05-02T03:01:49Z

/remove-lifecycle stale

k8s-triage-robot · 2024-06-01T03:59:40Z

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and 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:

Reopen this PR with /reopen
Mark this PR as fresh with /remove-lifecycle rotten
Offer to help out with Issue Triage

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

/close

k8s-ci-robot · 2024-06-01T03:59:44Z

@k8s-triage-robot: Closed this PR.

In response to this:

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and 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:

Reopen this PR with /reopen

Mark this PR as fresh with /remove-lifecycle rotten

Offer to help out with Issue Triage

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

/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-sigs/prow repository.

ytsarev · 2024-06-01T09:51:44Z

we will switch to new webhook based implementation and eventually contribute NS support to this one

Azure supports NS records on public DNS zones: https://learn.microsoft.com/en-us/rest/api/dns/record-sets/create-or-update?view=rest-dns-2018-05-01&tabs=HTTP#create-ns-recordset This PR adds this support to external dns. The implementation is similar to the other records. Besides the unit tests this PR was also tested by creating a DNSEndpoint with the following spec: ``` endpoints: - dnsName: cloud.k8gb.io recordTTL: 5 recordType: NS targets: - gslb-ns-eu-cloud.k8gb.io - gslb-ns-us-cloud.k8gb.io ``` The creation of the NS record in Azure was successful: ``` az network dns record-set ns list --resource-group rg-k8gb --zone-name "$EDGE_DNS_ZONE" --output json [ {...}, { "NSRecords": [ { "nsdname": "gslb-ns-eu-cloud.k8gb.io" }, { "nsdname": "gslb-ns-us-cloud.k8gb.io" } ], "TTL": 5, "etag": "97a7199f-3be9-47bd-ab00-37013b775180", "fqdn": "cloud.k8gb.io.", "id": "/subscriptions/<redacted>/resourceGroups/rg-k8gb/providers/Microsoft.Network/dnszones/k8gb.io/NS/cloud", "name": "cloud", "provisioningState": "Succeeded", "resourceGroup": "rg-k8gb", "targetResource": {}, "trafficManagementProfile": {}, "type": "Microsoft.Network/dnszones/NS" } ] ``` This change was already attempted in kubernetes-sigs#2835, but it was never merged due to inactivity. Signed-off-by: Andre Aguas <andre.aguas@protonmail.com>

k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label Jun 21, 2022

k8s-ci-robot requested review from njuettner and Raffo June 21, 2022 15:51

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Jun 21, 2022

sebader mentioned this pull request Jun 21, 2022

[WIP] Support for Azure DNS on AKS k8gb-io/k8gb#912

Closed

4 tasks

JAORMX approved these changes Jun 22, 2022

View reviewed changes

sebader mentioned this pull request Nov 17, 2022

Add support for NS records for Azure DNS #2826

Closed

k8s-ci-robot assigned Raffo Jan 26, 2023

ytsarev reviewed Jan 26, 2023

View reviewed changes

squash

a9481fa

sebader force-pushed the feature/sebader-azure-ns branch from b1ad23d to a9481fa Compare January 30, 2023 16:10

Raffo reviewed Feb 4, 2023

View reviewed changes

Mention managed-record-types

a4c607d

k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label May 7, 2023

k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label May 8, 2023

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 16, 2023

ytsarev mentioned this pull request Sep 23, 2023

Cloudflare support k8gb-io/k8gb#1278

Merged

k8s-ci-robot closed this Dec 22, 2023

k8s-ci-robot reopened this Jan 2, 2024

k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 1, 2024

k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels May 1, 2024

k8s-ci-robot closed this Jun 1, 2024

abaguas mentioned this pull request Nov 4, 2024

feat(Azure DNS): add NS record support #4846

Merged

2 tasks

Adding NS support for Azure DNS #2835

Adding NS support for Azure DNS #2835

Conversation

sebader commented Jun 21, 2022 • edited Loading

linux-foundation-easycla bot commented Jun 21, 2022 • edited Loading

k8s-ci-robot commented Jun 21, 2022

sebader commented Jun 24, 2022

Ziphone commented Oct 18, 2022

sebader commented Oct 18, 2022

Ziphone commented Oct 18, 2022

k8s-ci-robot commented Oct 18, 2022

Ziphone commented Oct 30, 2022 • edited Loading

sebader commented Oct 31, 2022

Ziphone commented Dec 1, 2022 • edited Loading

ytsarev commented Jan 26, 2023

ytsarev left a comment

Choose a reason for hiding this comment

sebader commented Jan 30, 2023

Raffo left a comment

Choose a reason for hiding this comment

sebader commented Feb 4, 2023

Raffo commented Feb 5, 2023

sebader commented Feb 6, 2023

ytsarev commented May 8, 2023

k8s-ci-robot commented May 16, 2023

mloiseleur commented Dec 20, 2023 • edited Loading

sebader commented Dec 20, 2023

mloiseleur commented Dec 22, 2023

k8s-ci-robot commented Dec 22, 2023

ytsarev commented Jan 2, 2024

k8s-ci-robot commented Jan 2, 2024

k8s-ci-robot commented Jan 2, 2024

mloiseleur commented Jan 2, 2024

ytsarev commented Jan 2, 2024

k8s-triage-robot commented Apr 1, 2024

ytsarev commented Apr 1, 2024

k8s-triage-robot commented May 1, 2024

ytsarev commented May 2, 2024

k8s-triage-robot commented Jun 1, 2024

k8s-ci-robot commented Jun 1, 2024

ytsarev commented Jun 1, 2024

Adding `NS` support for Azure DNS #2835

Adding `NS` support for Azure DNS #2835

sebader commented Jun 21, 2022 •

edited

Loading

linux-foundation-easycla bot commented Jun 21, 2022 •

edited

Loading

Ziphone commented Oct 30, 2022 •

edited

Loading

Ziphone commented Dec 1, 2022 •

edited

Loading

mloiseleur commented Dec 20, 2023 •

edited

Loading