-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
azurerm_api_management_custom_domain: resource gets updated every time #10636
Conversation
filter proxy default hostname in custom_domain resource remove default proxy hostname from test cases
As the original author of the resource, I'm pleased to see you found a solution to this quirk 😄 How will existing configurations be affected? If they include the default hostname today, will it appear twice in the config (thus becoming invalid) or will it be filtered out then, too? |
@sirlatrom I just tried that. It will just work. You will get a diff in If you delete the default hostname there will be no change in |
@patst That is excellent! Thanks again for the improvement! |
@sirlatrom sorry my test had a little error. I updated my post above. |
@patst Well that's the second-best possible outcome and is still fine, as it will help current users see they don't need to add it anymore. Maybe it should be added to the website docs for the resource, in order to help people understand what is happening if they have been copying the current example from the docs? |
@sirlatrom thanks for the feedback, good point. I added a note in the resource documentation to not include the default hostname |
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 for this @patst, this is looking good and I think it's a reasonable compromise to have a persistent plan in pursuit of removing this boilerplate, as long as its nondestructive. I still need to test this myself, but I have a suggestion about simplifying the flatten function in the meantime.
azurerm/internal/services/apimanagement/api_management_custom_domain_resource.go
Show resolved
Hide resolved
azurerm/internal/services/apimanagement/api_management_custom_domain_resource.go
Show resolved
Hide resolved
Awaiting acceptance test results |
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.
tests passed, LGTM 👍
This has been released in version 2.51.0 of the provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. As an example: provider "azurerm" {
version = "~> 2.51.0"
}
# ... other configuration ... |
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks! |
A possible solution for #10253 .
As described in the issue it is unclear if the default hostname for a proxy (
<apim-name>.azure-api.net
) needs to be included in theazurerm_api_management_custom_domain
resource. The test cases include the default hostname, the example in the docs don't.In both cases there are problems using the
azurerm_api_management_custom_domain
resource at the moment becausetf plan
will try to apply changes on subsequent runs (see issue).My solution filters out the default hostname (as the
azurerm_api_management
resource does as well, see https://github.com/terraform-providers/terraform-provider-azurerm/blob/master/azurerm/internal/services/apimanagement/api_management_resource.go#L986 ).The test cases are succeeding and my local tests are successful with this behaviour: