New Resource: `azurerm_key_vault_managed_hardware_security_module_key` #25935

mbfrahry · 2024-05-10T22:50:53Z

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

This PR adds support for HSM Keys.

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”

Changes to existing Resource / Data Source

I have added an explanation of what my changes do and why I'd like you to include them (This may be covered by linking to an issue above, but may benefit from additional explanation).
I have written new tests for my resource or datasource changes & updated any relevent documentation.
I have successfully run tests with my changes locally. If not, please provide details on testing challenges that prevented you running the tests.

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)

Change Log

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

New Resource: azurerm_key_vault_managed_hardware_security_module_key [GH-00000]

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.

catriona-m

Left a couple of minor comments, but otherwise LGTM!

catriona-m · 2024-05-16T15:44:40Z

internal/services/managedhsm/key_vault_managed_hardware_security_module_key_resource.go

+		Timeout: 30 * time.Minute,
+		Func: func(ctx context.Context, metadata sdk.ResourceMetaData) error {
+			client := metadata.Client.ManagedHSMs.DataPlaneKeysClient
+			// client := metadata.Client.ManagedHSMs.ManagedHsmKeyClient


should this be removed?

Good shout! Ty ty

catriona-m · 2024-05-16T15:58:01Z

internal/services/managedhsm/key_vault_managed_hardware_security_module_key_resource.go

+			}
+
+			if resp, err := client.CreateKey(ctx, endpoint.BaseURI(), config.Name, parameters); err != nil {
+				if metadata.Client.Features.KeyVault.RecoverSoftDeletedKeys && utils.ResponseWasConflict(resp.Response) {


should this be checking for HSM keys?

Suggested change

if metadata.Client.Features.KeyVault.RecoverSoftDeletedKeys && utils.ResponseWasConflict(resp.Response) {

if metadata.Client.Features.KeyVault.RecoverSoftDeletedHSMKeys && utils.ResponseWasConflict(resp.Response) {

Ohhhh that coulda been a big bug. Ty ty

tombuildsstuff

Left a few comments inline but this otherwise LGTM 👍

tombuildsstuff · 2024-05-16T16:01:23Z

internal/features/defaults.go

+			PurgeSoftDeletedHSMKeysOnDestroy: true,
 			RecoverSoftDeletedKeyVaults:      true,
 			RecoverSoftDeletedKeys:           true,
 			RecoverSoftDeletedCerts:          true,
 			RecoverSoftDeletedSecrets:        true,
+			RecoverSoftDeletedHSMKeys:        true,


Since we're supporting Key Vault Keys and Managed HSM Keys in the Provider - it'd probably be worth putting the Managed HSM stuff within managed_hsm - so whilst I realise we have the existing feature-flag here - can we add a TODO to fix this in 4.0?

tombuildsstuff · 2024-05-16T16:51:21Z

internal/services/managedhsm/internal.go

+
+		conn, err := client.Get(secretUri)
+		if err != nil {
+			log.Printf("[DEBUG] Didn't find KeyVault secret at %q", secretUri)


Suggested change

log.Printf("[DEBUG] Didn't find KeyVault secret at %q", secretUri)

log.Printf("[DEBUG] Didn't find Managed HSM Key at %q", secretUri)

tombuildsstuff · 2024-05-16T16:51:42Z

internal/services/managedhsm/internal.go

+func keyVaultChildItemRefreshFunc(secretUri string) pluginsdk.StateRefreshFunc {
+	return func() (interface{}, string, error) {
+		log.Printf("[DEBUG] Checking to see if KeyVault Secret %q is available..", secretUri)
+
+		PTransport := &http.Transport{Proxy: http.ProxyFromEnvironment}


Suggested change

func keyVaultChildItemRefreshFunc(secretUri string) pluginsdk.StateRefreshFunc {

return func() (interface{}, string, error) {

log.Printf("[DEBUG] Checking to see if KeyVault Secret %q is available..", secretUri)

PTransport := &http.Transport{Proxy: http.ProxyFromEnvironment}

func keyVaultChildItemRefreshFunc(keyUri string) pluginsdk.StateRefreshFunc {

return func() (interface{}, string, error) {

log.Printf("[DEBUG] Checking to see if Managed HSM Key %q is available..", keyUri)

PTransport := &http.Transport{

Proxy: http.ProxyFromEnvironment,

}

tombuildsstuff · 2024-05-16T16:52:01Z

internal/services/managedhsm/internal.go

+
+		defer conn.Body.Close()
+
+		log.Printf("[DEBUG] Found KeyVault Secret %q", secretUri)


Suggested change

log.Printf("[DEBUG] Found KeyVault Secret %q", secretUri)

log.Printf("[DEBUG] Found Managed HSM Key %q", keyUri)

tombuildsstuff · 2024-05-16T16:57:51Z

internal/services/managedhsm/key_vault_managed_hardware_security_module_key_resource.go

+			locks.ByName(managedHsmId.ID(), "azurerm_key_vault_managed_hardware_security_module")
+			defer locks.UnlockByName(managedHsmId.ID(), "azurerm_key_vault_managed_hardware_security_module")


we should do this above the RequiresImport check, else we're potentially provisioning 2 of these

tombuildsstuff · 2024-05-16T16:58:40Z

internal/services/managedhsm/key_vault_managed_hardware_security_module_key_resource.go

+				return fmt.Errorf("determining Resource Manager ID for %q: %+v", id, err)
+			}
+			if resourceManagerId == nil {
+				return fmt.Errorf("unable to determine the Resource Manager ID for %s", id)


This could also mean it's gone:

Suggested change

return fmt.Errorf("unable to determine the Resource Manager ID for %s", id)

return metadata.MarkAsGone(*id)

tombuildsstuff · 2024-05-16T17:00:14Z

internal/services/managedhsm/key_vault_managed_hardware_security_module_key_resource.go

+				return fmt.Errorf("determining Resource Manager ID for %q: %+v", id, err)
+			}
+			if resourceManagerId == nil {
+				return fmt.Errorf("unable to determine the Resource Manager ID for %s", id)


(note: this should stay as it is, and not become return nil, since we'd have passed by the Read prior to calling Delete, so this'd be an inconsistency error)

<Actions> <action id="f410411e63aff4bb73a81c2aec1d373cf8a903e63b30dee2006b0030d8a94cc8"> <h3>Bump Terraform `azurerm` provider version</h3> <details id="1d9343c012f5434ac9fe8a98135bae3667b399259be16d9b14302ea3bd424a24"> <summary>Update Terraform lock file</summary> <p>changes detected:
	"hashicorp/azurerm" updated from "3.103.1" to "3.104.0" in file ".terraform.lock.hcl"</p> <details> <summary>3.104.0</summary> <pre>Changelog retrieved from:
	https://github.com/hashicorp/terraform-provider-azurerm/releases/tag/v3.104.0
FEATURES:

* New Data Source: `azurerm_elastic_san` ([#25719](https://github.com/hashicorp/terraform-provider-azurerm/issues/25719))

ENHANCEMENTS:

* New Resource - `azurerm_key_vault_managed_hardware_security_module_key` ([#25935](hashicorp/terraform-provider-azurerm#25935 Data Source - `azurerm_kubernetes_service_version` - support for the `default_version` property ([#25953](hashicorp/terraform-provider-azurerm#25953 `network/applicationgateways` - update to use `hashicorp/go-azure-sdk` ([#25844](hashicorp/terraform-provider-azurerm#25844 `dataprotection` - update API version to `2024-04-01` ([#25882](hashicorp/terraform-provider-azurerm#25882 `databasemigration` - update API version to `2021-06-30` ([#25997](hashicorp/terraform-provider-azurerm#25997 `network/ips` - update to use `hashicorp/go-azure-sdk` ([#25905](hashicorp/terraform-provider-azurerm#25905 `network/localnetworkgateway` - update to use `hashicorp/go-azure-sdk` ([#25905](hashicorp/terraform-provider-azurerm#25905 `network/natgateway` - update to use `hashicorp/go-azure-sdk` ([#25905](hashicorp/terraform-provider-azurerm#25905 `network/networksecuritygroup` - update to use `hashicorp/go-azure-sdk` ([#25971](hashicorp/terraform-provider-azurerm#25971 `network/publicips` - update to use `hashicorp/go-azure-sdk` ([#25971](hashicorp/terraform-provider-azurerm#25971 `network/virtualwan` - update to use `hashicorp/go-azure-sdk` ([#25971](hashicorp/terraform-provider-azurerm#25971 `network/vpn` - update to use `hashicorp/go-azure-sdk` ([#25971](hashicorp/terraform-provider-azurerm#25971 `azurerm_databricks_workspace` - support for the `default_storage_firewall_enabled` property ([#25919](hashicorp/terraform-provider-azurerm#25919 `azurerm_key_vault` - allow previously existing key vaults to continue to manage the `contact` field prior to the `v3.93.0` conditional polling change ([#25777](hashicorp/terraform-provider-azurerm#25777 `azurerm_linux_function_app` - support for the PowerShell `7.4` ([#25980](hashicorp/terraform-provider-azurerm#25980 `azurerm_log_analytics_cluster` - support for the value `UserAssigned` in the `identity.type` property ([#25940](hashicorp/terraform-provider-azurerm#25940 `azurerm_pim_active_role_assignment` - remove hard dependency on the `roleAssignmentScheduleRequests` API, so that role assignments will not become unmanageable over time ([#25956](hashicorp/terraform-provider-azurerm#25956 `azurerm_pim_eligible_role_assignment` - remove hard dependency on the `roleEligibilityScheduleRequests` API, so that role assignments will not become unmanageable over time ([#25956](hashicorp/terraform-provider-azurerm#25956 `azurerm_windows_function_app` - support for the PowerShell `7.4` ([#25980](https://github.com/hashicorp/terraform-provider-azurerm/issues/25980))

BUG FIXES:

* `azurerm_container_app_job` - Allow `event_trigger_config.scale.min_executions` to be `0` ([#25931](hashicorp/terraform-provider-azurerm#25931 `azurerm_container_app_job` - update validation to allow the `replica_retry_limit` property to be set to `0` ([#25984](hashicorp/terraform-provider-azurerm#25984 `azurerm_data_factory_trigger_custom_event` - one of `subject_begins_with` and `subject_ends_with` no longer need to be set ([#25932](hashicorp/terraform-provider-azurerm#25932 `azurerm_kubernetes_cluster_node_pool` - prevent race condition by checking the virtual network status when creating a node pool with a subnet ID ([#25888](hashicorp/terraform-provider-azurerm#25888 `azurerm_postgresql_flexible_server` - fix for default `storage_tier` value when `storage_mb` field has been changed ([#25947](hashicorp/terraform-provider-azurerm#25947 `azurerm_pim_active_role_assignment` - resolve a number of potential crashes ([#25956](hashicorp/terraform-provider-azurerm#25956 `azurerm_pim_eligible_role_assignment` - resolve a number of potential crashes ([#25956](hashicorp/terraform-provider-azurerm#25956 `azurerm_redis_enterprise_cluster_location_zone_support` - add `Central India` zones support ([#26000](hashicorp/terraform-provider-azurerm#26000 `azurerm_sentinel_alert_rule_scheduled` - the `alert_rule_template_version` property is no longer `ForceNew` ([#25688](hashicorp/terraform-provider-azurerm#25688 `azurerm_storage_sync_server_endpoint` - preventing a crashed due to `initial_upload_policy` ([#25968](https://github.com/hashicorp/terraform-provider-azurerm/issues/25968))


</pre> </details> </details> <a href="https://infra.ci.jenkins.io/job/updatecli/job/azure/job/main/185/">Jenkins pipeline link</a> </action> </Actions> --- <table> <tr> <td width="77"> <img src="https://www.updatecli.io/images/updatecli.png" alt="Updatecli logo" width="50" height="50"> </td> <td> <p> Created automatically by <a href="https://www.updatecli.io/">Updatecli</a> </p> <details><summary>Options:</summary> <br /> <p>Most of Updatecli configuration is done via <a href="https://www.updatecli.io/docs/prologue/quick-start/">its manifest(s)</a>.</p> <ul> <li>If you close this pull request, Updatecli will automatically reopen it, the next time it runs.</li> <li>If you close this pull request and delete the base branch, Updatecli will automatically recreate it, erasing all previous commits made.</li> </ul> <p> Feel free to report any issues at <a href="https://github.com/updatecli/updatecli/issues">github.com/updatecli/updatecli</a>.<br /> If you find this tool useful, do not hesitate to star <a href="https://github.com/updatecli/updatecli/stargazers">our GitHub repository</a> as a sign of appreciation, and/or to tell us directly on our <a href="https://matrix.to/#/#Updatecli_community:gitter.im">chat</a>! </p> </details> </td> </tr> </table> Co-authored-by: Jenkins Infra Bot (updatecli) <60776566+jenkins-infra-bot@users.noreply.github.com>

github-actions · 2024-06-16T02:09:46Z

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.

mbfrahry added 2 commits May 10, 2024 15:48

go mod vendor

1f09cef

New Resource: azurerm_key_vault_managed_hardware_security_module_key

d0ccffa

github-actions bot added documentation dependencies service/managed-hsm size/XL waiting-response labels May 10, 2024

Lint

53d63fa

github-actions bot added waiting-response and removed waiting-response labels May 14, 2024

Add recover from soft delete

71a8f04

github-actions bot removed the waiting-response label May 14, 2024

go mod tidy

b2381c5

github-actions bot added the waiting-response label May 14, 2024

mbfrahry changed the title ~~[WIP] New Resource: azurerm_key_vault_managed_hardware_security_module_key~~ New Resource: azurerm_key_vault_managed_hardware_security_module_key May 14, 2024

Update docs and tests

0dba7d4

github-actions bot removed the waiting-response label May 14, 2024

mbfrahry marked this pull request as ready for review May 14, 2024 23:36

Add storage account hsm key test

cdd4595

catriona-m approved these changes May 16, 2024

View reviewed changes

tombuildsstuff reviewed May 16, 2024

View reviewed changes

github-actions bot added the waiting-response label May 16, 2024

Remove KeyVault from comments

1efc906

github-actions bot added service/storage waiting-response and removed waiting-response labels May 16, 2024

Address reviews

801997e

github-actions bot removed the waiting-response label May 16, 2024

mbfrahry added this to the v3.104.0 milestone May 16, 2024

mbfrahry merged commit 02b4634 into main May 16, 2024
32 checks passed

mbfrahry deleted the f-managedhsm-key branch May 16, 2024 18:38

mbfrahry added a commit that referenced this pull request May 16, 2024

Update CHANGELOG.md for #25935

0c5f987

mbfrahry mentioned this pull request May 16, 2024

Support for creating keys in a managed HSM #13654

Closed

github-actions bot locked as resolved and limited conversation to collaborators Jun 16, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New Resource: `azurerm_key_vault_managed_hardware_security_module_key` #25935

New Resource: `azurerm_key_vault_managed_hardware_security_module_key` #25935

mbfrahry commented May 10, 2024 •

edited

Loading

catriona-m left a comment

catriona-m May 16, 2024

mbfrahry May 16, 2024

catriona-m May 16, 2024

mbfrahry May 16, 2024

tombuildsstuff left a comment

tombuildsstuff May 16, 2024

mbfrahry May 16, 2024

tombuildsstuff May 16, 2024

tombuildsstuff May 16, 2024

tombuildsstuff May 16, 2024

tombuildsstuff May 16, 2024

tombuildsstuff May 16, 2024

tombuildsstuff May 16, 2024

github-actions bot commented Jun 16, 2024

	if metadata.Client.Features.KeyVault.RecoverSoftDeletedKeys && utils.ResponseWasConflict(resp.Response) {
	if metadata.Client.Features.KeyVault.RecoverSoftDeletedHSMKeys && utils.ResponseWasConflict(resp.Response) {

	log.Printf("[DEBUG] Didn't find KeyVault secret at %q", secretUri)
	log.Printf("[DEBUG] Didn't find Managed HSM Key at %q", secretUri)


		defer conn.Body.Close()

		log.Printf("[DEBUG] Found KeyVault Secret %q", secretUri)

	log.Printf("[DEBUG] Found KeyVault Secret %q", secretUri)
	log.Printf("[DEBUG] Found Managed HSM Key %q", keyUri)

		locks.ByName(managedHsmId.ID(), "azurerm_key_vault_managed_hardware_security_module")
		defer locks.UnlockByName(managedHsmId.ID(), "azurerm_key_vault_managed_hardware_security_module")

	return fmt.Errorf("unable to determine the Resource Manager ID for %s", id)
	return metadata.MarkAsGone(*id)

New Resource: azurerm_key_vault_managed_hardware_security_module_key #25935

New Resource: azurerm_key_vault_managed_hardware_security_module_key #25935

Conversation

mbfrahry commented May 10, 2024 • edited Loading

Community Note

Description

PR Checklist

Changes to existing Resource / Data Source

Testing

Change Log

Related Issue(s)

catriona-m left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tombuildsstuff left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jun 16, 2024

New Resource: `azurerm_key_vault_managed_hardware_security_module_key` #25935

New Resource: `azurerm_key_vault_managed_hardware_security_module_key` #25935

mbfrahry commented May 10, 2024 •

edited

Loading