NEW PROVIDER: Azure Private DNS Zones

[matthewmgamble]

The current Azure DNS provider does not allow you to manage Azure Private DNS Zones with dnscontrol, which is something I'd like to be able to do since the rest of our DNS infrastructure is managed currently with dnscontrol.

It's mostly based on the existing Azure DNS provider, with the required Azure API changes to modify private DNS zones instead of public ones. The only major limitation of Azure Private DNS is that it does not support NS records (which makes sense) but there currently isn't a feature flag for this in dnscontrol so it will fail if someone attempts to add an NS record using this provider.

[tlimoncelli]

Greetings!

Thanks for submitting this! I didn't know private zones were handled differently.

there currently isn't a feature flag for this in dnscontrol

If I can show you how to make a feature flag, would you consider adopting the AZURE_DNS provider to work with Azure Private DNS Zones? Having one provider instead of 2 has benefits.

[matthewmgamble]

The only issue I see with making them one provider is that they are technically different products from Azure with different API end points, different name spaces, and different supported record types (CAA for example is not supported in Azure Private DNS). For example on the different name spaces, the call to nativeToRecordTypeDiff2 would have to be passed a flag to determine if it's a Azure private zone or an Azure public zone since the result set from Azure for the query are two different types. If you think it's still possible to merge them together I'm open to trying it, just don't yet understand enough about the internals of dnscontrol to see how to make that happen without breaking the existing Azure module.

[tlimoncelli]

Understood. Let's give it a try and see if we can make this work.

Basically you will add a field to the azurednsProvider struct that indicates which will be set to true for "private". For example, set azurednsProvider.isPrivateDNSMode to true to enable the new mode. Then recordToNativeDiff2 can look at a.privateDNSmode any time it needs to act differently.

There are 2 ways to pass feature flags to a provider.

(1) In creds.json:

  "azure_main": {
    "ClientID": "redacted",
    "ClientSecret": "redacted",
    "ResourceGroup": "redacted",
    "SubscriptionID": "redacted",
    "TYPE": "AZURE_DNS",
    "TenantID": "redacted"
  },
  "azure_private": {
    "ClientID": "redacted",
    "ClientSecret": "redacted",
    "ResourceGroup": "redacted",
    "SubscriptionID": "redacted",
    "TYPE": "AZURE_DNS",
    "TenantID": "redacted",
    "azure_private_dns": "true"         <<< I ADDED THIS
  },

A good example of this is in providers/azuredns/azureDnsProvider.go where SubscriptionID is configured.

(2) Or in dnsconfig.js itself when you declare the provider:

// Add this to pkg/js/helpers.js:
var AZURE_PRIVATE_DNS = { private_zone: true };

// Here's how you use it in dnsconfig.js:

var DSP_AZURE_PUBLIC = NewDnsProvider("myazure");

var DSP_AZURE_PRIVATE = NewDnsProvider("myazure", AZURE_PRIVATE_DNS);

A good example of this is the cloudflareapi driver where it uses manage_redirects to enable a feature.

---

It's up to you which user experience makes more sense. Would a typical Azure user have different creds for private zones? Another way to look at this decision would be to consider how the documentation would read:

(1) Note: If you are using Azure Private DNS, the entry in creds.json needs "azure_private_dns": "true". If you are using both public DNS and private DNS, you'll need two separate entries in creds.json.
(2) Note: If the domain uses Azure Private DNS, add AZURE_PRIVATE_DNS to the NewDnsProvider statement. (then include an example)

Tom

P.S. Rather than updating nativeToRecordTypeDiff2, it

[matthewmgamble]

I started looking at making them the same and it's really a mess to combine the two. Take the basic structure of the client. For azure you have this:

type azurednsProvider struct {
zonesClient *adns.ZonesClient
recordsClient *adns.RecordSetsClient
zones map[string]*adns.Zone
resourceGroup *string
subscriptionID *string
rawRecords map[string][]*adns.RecordSet
zoneName map[string]string
privateDNS bool
}

Whereas for Azure Private you have this:

type azurePrivateDnsProvider struct {
zonesClient *adns.PrivateZonesClient
recordsClient *adns.RecordSetsClient
zones map[string]*adns.PrivateZone
resourceGroup *string
subscriptionID *string
rawRecords map[string][]*adns.RecordSet
zoneName map[string]string
}

The zonesClient, recordsClient, and zones would all have to be duplicated for private zones, and then everywhere in the code the zones or record client is used an if statement put in to determine which to use. From my point of view given they use two different APIs and record types it's really best to keep them as seperate clients but if after reading this you still think it's worth trying to merge them I can give it a shot.

[tlimoncelli]

Welp, looks like we're going with two separate providers. Thanks for humoring me.

I conferred with my co-workers who pointed out: All the tools that interact with Azure DNS seem to separate the two (az cli, az pwsh, terraform, etxternal-dns). At least we're in good company!

I'll do a code-review soon.

Best,
Tom

[tlimoncelli]

Suggested change

	This provider does not recognize any special metadata fields unique to Azure DNS.
	This provider does not recognize any special metadata fields unique to Azure Private DNS.

[tlimoncelli]

Remove these commented out sections of code. (here and elsewhere)

[tlimoncelli]

It is more "go style" to reverse the logic:

if set.Properties.ARecords == nil { return nil, fmt.Errorf(..) }
for _, rec := range set.Properties.ARecords {
...
}

This creates more readable code and less indenting.

(same for the other cases)

[tlimoncelli]

Please do a global search and replace of recordToNativeDiff2 to recordToNative

[tlimoncelli]

Please remove the commented out code (and the comment).

[tlimoncelli]

This returns nil either way. Shouldn't it create the zone if it doesn't exist?

[tlimoncelli]

A lot has changed on the master banch. I've rebased (using the Github Web UI) so I can review it in light of the new TXT-related helper functions.

[matthewmgamble]

I've committed a new change with the suggested edits to replace .TxtStrings with GetTargetTXTJoined

[tlimoncelli]

Thanks!

I'm sure you've seen the checklist at the end of https://docs.dnscontrol.org/service-providers/providers#providers-with-contributor-support but I want to make sure you saw this:

---

Expectations of maintainers:

• Maintainers are expected to support their provider and/or help find a new maintainer.
• Maintainers should set up test accounts and periodically verify that all tests pass (pkg/js/parse_tests and integrationTest).
• Contributors are encouraged to add new tests and refine old ones. (Test-driven development is encouraged.)
• Bugs will be referred to the maintainer or their designate.
• Maintainers must be responsible to bug reports and PRs. If a maintainer is unresponsive for more than 2 months, we will consider disabling the provider. First we will put out a call for new maintainer. If nobody volunteers, the provider may be disabled.
• Tom needs to know your real email address. Please email tlimoncelli at stack over flow dot com so he has it.

---

(I just added the last bullet. I'll update the official docs soon.)

Once I get the email from you, I'll merge this PR.

Thanks again for contributing this provider! I think it will find a lot of use!

Tom