New Resource: `azurerm_certificate_order_key_vault_store` #25464

xiaxyi · 2024-03-29T02:24:05Z

Community Note

Please vote on this PR by adding a 👍 reaction to the original PR to help the community and maintainers prioritize for review
Please do not leave "+1" or "me too" comments, they generate extra noise for PR followers and do not help prioritize for review

Description

Enable user to bind certificate order to key vault using the api:"/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.CertificateRegistration/certificateOrders/{certificateOrderName}/certificates/{name}"

This PR is depending on the fix of the id of the parent resource azurerm_app_service_certificate_order that included in the pr #25428

PR Checklist

I have followed the guidelines in our Contributing Documentation.
I have checked to ensure there aren't other open Pull Requests for the same update/change.
I have checked if my changes close any open issues. If so please include appropriate closing keywords below.
I have updated/added Documentation as required written in a helpful and kind way to assist users that may be unfamiliar with the resource / data source.
I have used a meaningful PR title to help maintainers and other users understand this change and help prevent duplicate work.
For example: “resource_name_here - description of change e.g. adding property new_property_name_here”

Testing

My submission includes Test coverage as described in the Contribution Guide and the tests pass. (if this is not possible for any reason, please include details of why you did or could not add test coverage)

--- PASS: TestAccAppServiceCertificateOrderCertificate_basic (274.54s)
PASS
ok      github.com/hashicorp/terraform-provider-azurerm/internal/services/appservice    291.518s
--- PASS: TestAccAppServiceCertificateOrderCertificate_updateKeyVaultId (1964.45s)
PASS
ok      github.com/hashicorp/terraform-provider-azurerm/internal/services/appservice  
--- PASS: TestAccAppServiceCertificateOrderCertificate_updateKeyVaultName

Extra information: for online failed acc tests with the error message like "waiting for resource provisioning status to become success", this is expected as the domain verification needs to be performed before binding the cert order to key vault. Tips added in the doc.

Change Log

Below please provide what should go into the changelog (if anything) conforming to the Changelog Format documented here.

azurerm_app_service_certificate_order_certificate - Support key vault binding.

This is a (please select all that apply):

Bug Fix
New Feature (ie adding a service, resource, or data source)
Enhancement
Breaking Change

Related Issue(s)

Fixes #0000

Note

If this PR changes meaningfully during the course of review please update the title and description as required.

stephybun

Thanks @xiaxyi. Would you be able to provide some more detail on what this actually does? I see in the description that it enables binding a certificate order to a key vault, but what does that mean or do in an App Service context?

xiaxyi · 2024-04-23T07:37:39Z

Thanks @stephybun for the review!

The feature is to configure the key vault information for a purchased app service certificate. After user buy an app service certificate using azurerm_app_service_certificate_order, the certificate configuration isn't completed as displayed in bleow screenshots:

API reference:https://learn.microsoft.com/en-us/rest/api/appservice/app-service-certificate-orders/create-or-update-certificate?view=rest-appservice-2023-01-01&tabs=HTTP
Docs" https://learn.microsoft.com/en-us/azure/app-service/configure-ssl-app-service-certificate?tabs=portal

Let me know if there is anything that's still not clear to you.

stephybun

@xiaxyi could you please take a look through the comments and suggestions left in-line. Once those are fixed up we can take another look through this.

stephybun · 2024-05-16T14:07:58Z

internal/services/appservice/app_service_certificate_order_certificate.go

+
+			existing, err := client.GetCertificate(ctx, id)
+			if err != nil && !response.WasNotFound(existing.HttpResponse) {
+				return fmt.Errorf("retreiving %s: %v", id, err)


Suggested change

return fmt.Errorf("retreiving %s: %v", id, err)

return fmt.Errorf("retrieving %s: %v", id, err)

stephybun · 2024-05-16T14:09:00Z

internal/services/appservice/app_service_certificate_order_certificate.go

+
+			existing, err := client.GetCertificate(ctx, *id)
+			if err != nil {
+				return fmt.Errorf("reading %s: %+v", id, err)


Suggested change

return fmt.Errorf("reading %s: %+v", id, err)

return fmt.Errorf("retrieving %s: %+v", id, err)

stephybun · 2024-05-16T14:09:28Z

internal/services/appservice/app_service_certificate_order_certificate.go

+				if response.WasNotFound(certificateOrderCertificate.HttpResponse) {
+					return metadata.MarkAsGone(id)
+				}
+				return fmt.Errorf("reading %s: %+v", id, err)


Suggested change

return fmt.Errorf("reading %s: %+v", id, err)

return fmt.Errorf("retrieving %s: %+v", id, err)

stephybun · 2024-05-16T14:11:34Z

internal/services/appservice/app_service_certificate_order_certificate.go

+}
+
+func (r CertificateOrderCertificateResource) ResourceType() string {
+	return "azurerm_app_service_certificate_order_certificate"


Can we rename this so it's a bit clearer at first glance what this does

Suggested change

return "azurerm_app_service_certificate_order_certificate"

return "azurerm_app_service_certificate_order_key_vault_store"

stephybun · 2024-05-21T07:28:09Z

internal/services/appservice/app_service_certificate_order_certificate.go

+		"key_vault_id": {
+			Type:         pluginsdk.TypeString,
+			Required:     true,
+			ValidateFunc: commonids.ValidateKeyVaultID,
+			// TODO -- remove when issue https://github.com/Azure/azure-rest-api-specs/issues/28498 is addressed
+			DiffSuppressFunc: suppress.CaseDifference,
+		},
+
+		"key_vault_secret_name": {
+			Type:         pluginsdk.TypeString,
+			Required:     true,
+			ValidateFunc: keyVaultValidate.NestedItemName,
+		},


Could these be replaced by key_vault_secret_id?

I'm afraid not, because we are not referring to a key vault secret, instead, we are creating the secret in the key vault with a name specified.

stephybun · 2024-05-21T09:05:51Z