azurerm_app_service_certificate - support for hosting_environment_profile_id

[wmdave]

No description provided.

[katbyte]

Thanks for the enhancement PR @wmdave - overall this looks good but could we update the docs with the new property and add this to a test? thanks!

[dhdanie]

I added a test for certificate creation in the ASE test cases (figured that was better than adding an ASE to certificate test cases). There's now a test failure on CI. Locally, I was relying on ARM_SUBSCRIPTION_ID to be set to help calculate the appropriate hosting_environment_profile for the certificate but apparently it's not for CI. I think I may just re-factor the change to require something like this:

hosting_environment {
  resource_group_name = "<ASE RG Name>"
  environment_name = "<ASE Name>"
}

I'm relatively sure that the ASE in question would have to be in the same subscription, so doing this makes the configuration a little cleaner. Only thing I don't like is that the original design leveraged hosting_environment_profile_id which is the name that's used in the Azure RM, so it made it a little easier infer how the configuration was being applied in Azure.

[katbyte]

Thats the place for the test! however i'm not entirely sure why you moved away from the single hosting_environment_id property - the RG and ASE name would be a part of is as would the subscription ID?

[wmdave]

The hosting_environment_id was of the format /subscriptions/<subid>/resourceGroups/<rg>/... and the subscription seemed superfluous (plus I didn't see an easy way to determine subid inside the test cases, though I may just not have looked in the right place). I liked the single property to begin with was because the name matched an actual property in the Azure resource manager, so you could literally copy and paste the value from Azure to your tf file. Once I decided to get rid of subscription, it didn't seem as intuitive (no longer copy/paste) as a single property since you for sure need both ASE RG (likely different than cert RG) and ASE name.

Open to changing back to a single property, if you think that's best.

[katbyte]

@wmdave - I do, you'd then be able to just us azurerm_app_service_environment.test.id, parse it's id with an ASE parse id function and have everything you need

[dhdanie]

Okay, switched back and I think updated docs and test case. Passed locally.

[katbyte]

thanks @wmdave! LGTM now

[ghost]

This has been released in version 2.15.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.15.0"
}
# ... other configuration ...

[ghost]

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 👉 hashibot-feedback@hashicorp.com. Thanks!