azurerm_cdn_endpoint_custom_domain - add supports for HTTPS

[magodo]

For the https case, there are two cases - CDN managed and user managed. For both cases, the https setting doesn't allow in-place update. That is because when updateing an https setting by disabling and enabling. The enabling request after the endpoint turns into disabled state will result into following error:

Custom HTTPS cannot be enabled at the moment because it takes 8 hours to
clean up your previous enablement request for the same custom domain.
Please try again later.

So we only allow toggling update, i.e. enable or disable.

Besides, both CDN managed and user managed test case might take a very long time. Only a disable/enable operation will take up to 8 hours to finish (hence I set the timeout accordingly). Though some configuration runs fast (e.g. for the user managed test case with the Standard_Microsoft sku, it only requires ~5 min to enable https).

Addtionally, for the user managed case, two more environment variables ARM_TEST_DNS_CERTIFICATE and ARM_TEST_DNS_SUBDOMAIN_NAME are needed, which contains the CA signed certificate for that domain and the corresponding subdomain name (i.e. the subdomain in the subject CN).

This should fix #398.

Test Result

💤 export ARM_TEST_DNS_ZONE_RESOURCE_GROUP_NAME=zhwen-domain \
export ARM_TEST_DNS_ZONE_NAME=e-moran.com \
export ARM_TEST_DNS_CERTIFICATE=/home/magodo/github/tf-config/acme/cert.pfx \
export ARM_TEST_DNS_SUBDOMAIN_NAME=sub

💤 💢 TF_ACC=1 go test -v -timeout=20h ./internal/services/cdn -run="TestAccCdnEndpointCustomDomain_httpsUserManagedBasic"
=== RUN   TestAccCdnEndpointCustomDomain_httpsUserManagedBasic
=== PAUSE TestAccCdnEndpointCustomDomain_httpsUserManagedBasic
=== CONT  TestAccCdnEndpointCustomDomain_httpsUserManagedBasic
--- PASS: TestAccCdnEndpointCustomDomain_httpsUserManagedBasic (794.12s)
PASS
ok      github.com/hashicorp/terraform-provider-azurerm/internal/services/cdn   794.148s

[katbyte]

looks like we have a build failure:

./cdn_endpoint_custom_domain_resource.go:249:11: props.CustomHTTPSParameters undefined (type *cdn.CustomDomainProperties has no field or method CustomHTTPSParameters)
[18:11:47]	[Step 4/5] ./cdn_endpoint_custom_domain_resource.go:275:11: props.CustomHTTPSParameters undefined (type *cdn.CustomDomainProperties has no field or method CustomHTTPSParameters)
[18:11:47]	[Step 4/5] ./cdn_endpoint_custom_domain_resource.go:321:25: props.CustomHTTPSParameters undefined (type *cdn.CustomDomainProperties has no field or method CustomHTTPSParameters)
[18:11:47]	[Step 4/5] ./cdn_endpoint_custom_domain_resource.go:370:3: cannot use cdn.None (type cdn.ResponseBasedDetectedErrorTypes) as type cdn.MinimumTLSVersion in field value
[18:11:47]	[Step 4/5] ./cdn_endpoint_custom_domain_resource.go:401:3: cannot use cdn.None (type cdn.ResponseBasedDetectedErrorTypes) as type cdn.MinimumTLSVersion in field value
[18:11:47]	[Step 4/5] ./cdn_endpoint_custom_domain_resource.go:465:31: undefined: cdn.Enabling
[18:11:47]	[Step 4/5] ./cdn_endpoint_custom_domain_resource.go:495:31: undefined: cdn.Disabling

[magodo]

@katbyte It is because of #13282, which updates the api version of CDN from 2019-04-15 to 2021-09-01.

In the Azure/azure-rest-api-specs, there is a commit: Add missing customHttpsParameters field of CustomDomainProperties. The issue here is that in the currently used Go SDK, the model generated for 2019-04-15 has the HTTPS parameters when getting the custom domain, while the model generated for 2021-09-01 doesn't.

[magodo]

The SDK issue is tracked by Azure/azure-sdk-for-go#15528

[magodo]

@katbyte The build issue is now fixed, please kindly take another look!

[alex-bes]

Hi guys! thanks for your efforts on this! @katbyte, is there a chance this will be merged/released soon?

[wilsonnm02]

@katbyte Any eta on merge/release for this PR?

[tombuildsstuff]

hey @magodo

Thanks for pushing those changes - I've taken a look through and left some other comments in addition to @jackofallops comments, if we can fix those up then this should otherwise be good to go 👍

Thanks!

[tombuildsstuff]

kind of outside the scope of this PR, but I've been wondering for a long-time if we should split the CDN endpoints by type, since the Verizon one only supports half of the features (the rest need to be managed in a separate portal) - I have a feeling we're nearing the point where we should make a call on that

[tombuildsstuff]

this'll crash if it's an array containing a computed item? so we'll need to length-check this?

[magodo]

FWIK, in case the properties inside the block are not all optional (i.e. at least one required), then if the returned value from GetOk is not the default value, then it is guaranteed to contain at least one element.

Anyway, it is always safe to length check for this :)

[magodo]

@tombuildsstuff I've resolved all the comments above and added a new testcase for the multi-step update. Please take another look, thanks!

💤  TF_ACC=1 go test -v -timeout=20h ./internal/services/cdn -run='TestAccCdnEndpointCustomDomain_httpsUpdate'
=== RUN   TestAccCdnEndpointCustomDomain_httpsUpdate
=== PAUSE TestAccCdnEndpointCustomDomain_httpsUpdate
=== CONT  TestAccCdnEndpointCustomDomain_httpsUpdate
--- PASS: TestAccCdnEndpointCustomDomain_httpsUpdate (3806.46s)
PASS
ok      github.com/hashicorp/terraform-provider-azurerm/internal/services/cdn   3806.476s

[jackofallops]

Thanks @magodo
I've run as much of the testing as I can in our test environment, and this looks good to go now.

addressed

[github-actions]

This functionality has been released in v2.91.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!

[wasfree]

Maybe it should be mentioned this is a breaking change, if it was manually enabled and is not reflected in terraform configuration https settings will be disabled. Also if the certificate is managed by Azure you have to wait 8 hours to reneable it. @tombuildsstuff

[tombuildsstuff]

@wasfree adding a new field to the Provider isn't a breaking change, this would have been highlighted in the Terraform Plan if it's configured outside of Terraform and not in the config - are you saying that this wasn't reflected in the Terraform Plan?

[wasfree]

With breaking change I mean https configuration was not possible to be set with terraform for a long time. So looking into issues people worked around it with azure cli or manually enable https settings for cdn custom domains. So if that is the case after updating to version 2.91.0 will disable https settings completely, potentially this will cause some outage. But I also agree with you it's possible to see it in the plan that https settings changed to null (so whatever the default is).

[magodo]

@wasfree Also noted that the Standard_Microsoft with following https configuration takes less than 1 hour to propagate.

    certificate_type = "Dedicated"
    protocol_type    = "ServerNameIndication"

[github-actions]

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.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.