`r\hpc_cache`: Add support for encryption key #16972

myc2h6o · 2022-05-26T08:29:38Z

This feature internally uses Disk Encryption Set so validations for Key Vault are same as it here

terraform-provider-azurerm/internal/services/compute/disk_encryption_set_resource.go

Line 113 in 82de9ae

keyVaultDetails, err := diskEncryptionSetRetrieveKeyVault(ctx, keyVaultsClient, resourcesClient, keyVaultKeyId)
When updating key_vault_key_id, auto_key_rotation_enabled will be set to false at backend, have confirmed with service team that this is by design, so added validation to enforce auto_key_rotation_enabled to be false when changing key_vault_key_id

tombuildsstuff

Thanks for this PR - I've taken a look through and left a few comments inline, if we can fix those comments up then we should be able to take another look.

Thanks!

tombuildsstuff · 2022-05-26T09:45:51Z

internal/services/hpccache/hpc_cache_resource.go

+		"identity": {
+			Type:     pluginsdk.TypeList,
+			Optional: true,
+			ForceNew: true,
+			MaxItems: 1,
+			Elem: &pluginsdk.Resource{
+				Schema: map[string]*pluginsdk.Schema{
+					"type": {
+						Type:     pluginsdk.TypeString,
+						Required: true,
+						ValidateFunc: validation.StringInSlice([]string{
+							// System-assigned Managed Identity requires manual operation on Portal
+							string(storagecache.CacheIdentityTypeUserAssigned),
+						}, false),
+					},
+					"identity_ids": {
+						Type:     pluginsdk.TypeSet,
+						Optional: true,
+						Elem: &pluginsdk.Schema{
+							Type:         pluginsdk.TypeString,
+							ValidateFunc: msivalidate.UserAssignedIdentityID,
+						},
+					},
+				},
+			},
+		},


this needs to be using Common Schema: https://github.com/hashicorp/go-azure-helpers/blob/main/resourcemanager/commonschema/identity_user.go

have updated to commonschema.UserAssignedIdentityOptionalForceNew()

tombuildsstuff · 2022-05-26T09:47:43Z

website/docs/r/hpc_cache.html.markdown

+
+* `identity` - (Optional) An `identity` block as defined below.
+
+* `key_vault_key_id` - (Optional) Specifies the URL to a Key Vault Key.


is this the Resource Manager or Data Plane Key ID?

this is Resource Managed ID, which is azurerm_key_vault_key.id, have updated the description to be clear

tombuildsstuff · 2022-05-26T09:48:12Z

website/docs/r/hpc_cache.html.markdown

+
+* `key_vault_key_id` - (Optional) Specifies the URL to a Key Vault Key.
+
+~> **NOTE:** `auto_key_rotation_enabled` must be set to `false` when updating `key_vault_key_id`.


that'd be a provider behaviour we're missing, to disable this, then re-enable it during the update?

Yes make sense, I've updated the code to trigger another update when the key is updated and auto key rotation is enabled

tombuildsstuff · 2022-05-26T09:48:30Z

website/docs/r/hpc_cache.html.markdown

@@ -83,7 +83,15 @@ The following arguments are supported:
 * `directory_ldap` - (Optional) A `directory_ldap` block as defined below.

 ~> **Note:** Only one of `directory_active_directory`, `directory_flat_file` and `directory_ldap` can be set.
-
+
+* `identity` - (Optional) An `identity` block as defined below.


this is ForceNew

Added the forceNew description

tombuildsstuff · 2022-05-26T09:48:45Z

website/docs/r/hpc_cache.html.markdown

+* `type` - (Required) Specifies the type of Managed Service Identity that should be configured on this HPC Cache. Possible value is `UserAssigned`.
+
+* `identity_ids` - (Optional) Specifies a list of User Assigned Managed Identity IDs to be assigned to this HPC Cache.


there's common phrasing for these now, can we pull these from another resource

Have updated to use the common phrasing

myc2h6o · 2022-05-27T04:36:38Z

Hi @tombuildsstuff thanks for reviewing the change! I've resolved the comment, could you please take another look?

myc2h6o · 2022-06-01T14:58:15Z

Updated test result: (Failed test TestAccHPCCache_directoryLDAP fails intermittently on main, to be fixed by #17060)

katbyte · 2022-06-02T22:56:05Z

internal/services/hpccache/hpc_cache_resource.go

+			autoKeyRotationEnabled = *props.RotationToLatestKeyVersionEnabled
+		}
+	}
+	d.Set("key_vault_key_id", keyVaultKeyId)
+	d.Set("auto_key_rotation_enabled", autoKeyRotationEnabled)


given the name of the property

Suggested change

autoKeyRotationEnabled = *props.RotationToLatestKeyVersionEnabled

}

}

d.Set("key_vault_key_id", keyVaultKeyId)

d.Set("auto_key_rotation_enabled", autoKeyRotationEnabled)

autoKeyRotationEnabled = *props.RotationToLatestKeyVersionEnabled

}

}

d.Set("key_vault_key_id", keyVaultKeyId)

d.Set("automatically_rotate_key_to_latest_enabled", autoKeyRotationEnabled)

@katbyte I've renamed the property name.

myc2h6o · 2022-06-06T06:13:25Z

Updated test result

katbyte

thanks @myc2h6o - LGTM 🌻

github-actions · 2022-06-10T09:07:20Z

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

github-actions · 2022-07-11T02:35:12Z

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.

r\hpc_cache: Add support for encryption key

8ff034f

github-actions bot added documentation service/hpc-cache size/XL labels May 26, 2022

tombuildsstuff suggested changes May 26, 2022

View reviewed changes

Resolve comments

3ab4c30

myc2h6o added 2 commits May 29, 2022 16:04

use CustomDiffWithAll

60270d6

Move CustomizeDiff to CreateUpdate due to interpolated key id

7803756

katbyte requested changes Jun 2, 2022

View reviewed changes

rename

07626ac

katbyte approved these changes Jun 8, 2022

View reviewed changes

katbyte merged commit 9dbbc27 into hashicorp:main Jun 8, 2022

katbyte added a commit that referenced this pull request Jun 8, 2022

CHANGELOG #16972

c65a26e

github-actions bot added this to the v3.10.0 milestone Jun 8, 2022

myc2h6o deleted the hpccache_encryption branch June 9, 2022 03:14

github-actions bot locked as resolved and limited conversation to collaborators Jul 11, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`r\hpc_cache`: Add support for encryption key #16972

`r\hpc_cache`: Add support for encryption key #16972

myc2h6o commented May 26, 2022

tombuildsstuff left a comment

tombuildsstuff May 26, 2022

myc2h6o May 27, 2022

tombuildsstuff May 26, 2022

myc2h6o May 27, 2022

tombuildsstuff May 26, 2022

myc2h6o May 27, 2022 •

edited

Loading

tombuildsstuff May 26, 2022

myc2h6o May 27, 2022

tombuildsstuff May 26, 2022

myc2h6o May 27, 2022

myc2h6o commented May 27, 2022

myc2h6o commented Jun 1, 2022

katbyte Jun 2, 2022

myc2h6o Jun 6, 2022

myc2h6o commented Jun 6, 2022

katbyte left a comment

github-actions bot commented Jun 10, 2022

github-actions bot commented Jul 11, 2022


		* `identity` - (Optional) An `identity` block as defined below.

		* `key_vault_key_id` - (Optional) Specifies the URL to a Key Vault Key.


		* `key_vault_key_id` - (Optional) Specifies the URL to a Key Vault Key.

		~> NOTE: `auto_key_rotation_enabled` must be set to `false` when updating `key_vault_key_id`.

		* `type` - (Required) Specifies the type of Managed Service Identity that should be configured on this HPC Cache. Possible value is `UserAssigned`.

		* `identity_ids` - (Optional) Specifies a list of User Assigned Managed Identity IDs to be assigned to this HPC Cache.

r\hpc_cache: Add support for encryption key #16972

r\hpc_cache: Add support for encryption key #16972

Conversation

myc2h6o commented May 26, 2022

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

myc2h6o May 27, 2022 • edited Loading

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

myc2h6o commented May 27, 2022

myc2h6o commented Jun 1, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

myc2h6o commented Jun 6, 2022

katbyte left a comment

Choose a reason for hiding this comment

github-actions bot commented Jun 10, 2022

github-actions bot commented Jul 11, 2022

`r\hpc_cache`: Add support for encryption key #16972

`r\hpc_cache`: Add support for encryption key #16972

myc2h6o May 27, 2022 •

edited

Loading