-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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
new resource: azurerm_private_dns_resolver
#18473
new resource: azurerm_private_dns_resolver
#18473
Conversation
894e66c
to
4037610
Compare
4037610
to
f191943
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you may need to configure what regions to run this service in:
Test Failed
------- Stdout: -------
=== RUN TestAccDNSResolverDnsResolver_requiresImport
=== PAUSE TestAccDNSResolverDnsResolver_requiresImport
=== CONT TestAccDNSResolverDnsResolver_requiresImport
=== CONT TestAccDNSResolverDnsResolver_requiresImport
testcase.go:110: Step 1/2 error: Error running apply: exit status 1
Error: creating Dns Resolver (Subscription: "*******"
Resource Group Name: "acctest-rg-220922165907733233"
Dns Resolver Name: "acctest-dr-220922165907733233"): performing CreateOrUpdate: dnsresolvers.DnsResolversClient#CreateOrUpdate: Failure sending request: StatusCode=0 -- Original Error: Code="BadRequest" Message="The provided location 'westus2' is not available for Microsoft.Network/dnsResolvers and Microsoft.Network/dnsForwardingRulesets resources." Target="" Details=[{}] InnerError={}
with azurerm_dns_resolver.test,
on terraform_plugin_test.tf line 20, in resource "azurerm_dns_resolver" "test":
20: resource "azurerm_dns_resolver" "test" {
creating Dns Resolver (Subscription: "*******"
Resource Group Name: "acctest-rg-220922165907733233"
Dns Resolver Name: "acctest-dr-220922165907733233"): performing
CreateOrUpdate: dnsresolvers.DnsResolversClient#CreateOrUpdate: Failure
sending request: StatusCode=0 -- Original Error: Code="BadRequest"
Message="The provided location 'westus2' is not available for
Microsoft.Network/dnsResolvers and Microsoft.Network/dnsForwardingRulesets
resources." Target="" Details=[{}] InnerError={}
--- FAIL: TestAccDNSResolverDnsResolver_requiresImport (70.39s)
Hi @katbyte , maybe disable the |
No, we can configure this service to only run in certain regions grep for:
|
Hi @katbyte , thanks for the suggestion, I've added it into test configs. Please take another look, thanks! |
The service itself is very specifically called "Azure DNS Private Resolver" (not just DNS resolver). I'd highly recommend to align the name of the terraform resource, too. Or is there a reason to only call it azurerm_dns_resolver? https://learn.microsoft.com/en-us/azure/dns/dns-private-resolver-overview |
Hi @sebader , thanks for the suggestion! I've confirmed with the service's PM, he prefers |
@ms-henglu are there plans for this service to also support the Public DNS resources? If not this'd make more sense as |
@ms-henglu - i agree with @sebader, we should match the correct name and change this to azurerm_dns_private_resolver |
Hi @katbyte , WDYT about
|
@ms-henglu - are there plans to support public DNS with this service? because if not this the name is misleading and conflicts with the existing nomenclature in the provider with azurerm_private_dns_* for private DNS, and azurerm_dns for public DNS |
@katbyte yes, public version will still be under this same service |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks both - with public going into the same service this LGTM now 🙌
Hi @katbyte, I think there is a misunderstanding for this resource. As I mentioned, public dns can resolved from everyone from anywhere. There is no SaaS from Microsoft needed. So I would recommend to use the correct name azure dns private resolver. |
@JannoTjarks It is probably true that there never will be a "public resolver SAAS Service". Still I think the name is fine, because it is "THE" Azure DNS Resolver and it already is named like this in PSH (see for example) and AZ CLI (see for example) documentation from MS. |
@JannoTjarks as product owner of the services mentioned above, I prefer not to disclose any information on this PR related to roadmap and/or directional strategy of Azure DNS. Lets keep consistency with CLI/PSH were possible given that this is the only DNS resolver service from Azure |
Hi @sfiguemsft thanks for the clarification 👍🏻 I understand that the product owner has more insights :D |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on responses here and how the naming is in the portal/docs, the name should not be azurerm_dns_resolver
and instead be azurerm_private_dns_resolver
. We should keep it consistent with the other dns resources in Terraform and not psh and cli.
@mbfrahry I'm asking the name to be azurerm_dns_resolver and this is what I would like customers to see in the TF modules - if this needs to be formalized through other channel please let me know. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sfiguemsft - given this is for private DNS only i also agree this should be azurerm_private_dns_resolver
- our resources names often do not 1:1 match service names and generally are chosen to create the least confusion for our users. Primarily we look to the portal for guidance on what to pick as that is where most people will be using the service/coming to terraform from. Given the existing private dns resources all start azurerm_private_dns_*
and that the portal also calls it Private resolver users are going to expect/look for "azure private resolver" not "azure dns resolver"
Because of this i concur with everyone else here the name should be azurerm_private_dns_resolver
to match the portal's chosen name for these resources and match existing resource in terraform. As long as that is its name in the portal (our primary reference for names - not APIs) i don't see us being able to call it something else.
@ms-henglu can we update the name as requested to at the very least get this in for users? we can always add a resource alias later on for a different name if required
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @ms-henglu - LGTM 🧮
azurerm_dns_resolver
azurerm_private_dns_resolver
This functionality has been released in v3.28.0 of the Terraform Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you! |
Don't see any docs in v3.28.0 for azurerm_private_dns_resolver nor in the release notes. |
Hi @DevopsMercenary , I guess it was labeled v3.28.0 by mistake, but these resources have been released in v3.29.0, please check: https://github.com/hashicorp/terraform-provider-azurerm/releases/tag/v3.29.0 |
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
#16956