Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

azure: zone name as environment variable #1433

Merged
merged 14 commits into from
Aug 7, 2021
Merged

Conversation

fibbs
Copy link
Contributor

@fibbs fibbs commented Jun 19, 2021

This pull request solves issue #1432 (feature request).

In some cases you might want to use acme to fetch certificates while the server doing so does not have access to (public) DNS and therefore cannot fetch the SOA record of the domain. This happens to me with a subdomain. I have this subdomain in the authoritative DNS server of another provider, forwarding one sub-domain to an azure DNS hosted zone. This NS Record is not set at my company's internal DNS server, because I do not want to have the records inside the sub-domain to be visible from the internet.

For this to work, I need to supply the zone name within which lego should create the TXT validation records instead of letting lego gather this from DNS. This is what this pull request solves by adding an additional (optional) environment variable AZURE_ZONE_NAME.

Copy link
Member

@dmke dmke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sounds like a useful feature to have, not only for the Azure provider, but for all DNS providers. I believe lego --dns.providers=... should be able to this already, though (if not, we should move the logic of dns01.FindZoneByFqdn for a more generic solution).

I've left some remarks on this PR, for the case we don't want to change dns01.FindZoneByFqdn.

providers/dns/azure/azure.toml Outdated Show resolved Hide resolved
providers/dns/azure/azure.go Outdated Show resolved Hide resolved
@fibbs fibbs force-pushed the azure_zone_name branch from dee5ac7 to 0178373 Compare July 16, 2021 17:46
@fibbs
Copy link
Contributor Author

fibbs commented Jul 16, 2021

This sounds like a useful feature to have, not only for the Azure provider, but for all DNS providers. I believe lego --dns.providers=... should be able to this already, though (if not, we should move the logic of dns01.FindZoneByFqdn for a more generic solution).

To be honest, I have not checked how exactly other of the providers work exactly. I was supposing that this "get the actual zone within which the TXT record is to be added" was something specific to how the structure is in Azure DNS was, and that probably other provider's code could be working completely different. So, I do believe that fixing this is good enough only for Azure, as I don't know whether the function dns01.FindZoneByFqdn might be used by something completely different though.

Anyway, that's of course something you as the developers of this project have much more insights into. I would be happy with either solution.

I've left some remarks on this PR, for the case we don't want to change dns01.FindZoneByFqdn.

Fixed :)

@ldez
Copy link
Member

ldez commented Jul 16, 2021

@fibbs the message of @dmke is for me 😉

providers/dns/azure/azure.go Outdated Show resolved Hide resolved
providers/dns/azure/azure.go Outdated Show resolved Hide resolved
fibbs and others added 2 commits July 17, 2021 12:03
Co-authored-by: Dominik Menke <git@dmke.org>
@ldez ldez changed the title Azure zone name as environment variable azure: zone name as environment variable Aug 7, 2021
@ldez ldez added this to the v4.5 milestone Aug 7, 2021
@ldez ldez enabled auto-merge (squash) August 7, 2021 10:00
@sergiomcalzada

This comment was marked as off-topic.

@ldez

This comment was marked as off-topic.

@sergiomcalzada

This comment was marked as off-topic.

@ldez

This comment was marked as off-topic.

@ldez

This comment was marked as off-topic.

@sergiomcalzada

This comment was marked as off-topic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

5 participants