-
Notifications
You must be signed in to change notification settings - Fork 4.9k
new resource azurerm_elastic_san_volume_group
#24166
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
ms-zhenhua
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @teowa,
Thanks for this PR - I've taken a look through and left some comments inline. If we can fix those up, this should be good to go 👍
Thanks!
| "elastic_san_id": { | ||
| Type: pluginsdk.TypeString, | ||
| Required: true, | ||
| ForceNew: true, | ||
| ValidateFunc: volumegroups.ValidateElasticSanID, | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| "elastic_san_id": { | |
| Type: pluginsdk.TypeString, | |
| Required: true, | |
| ForceNew: true, | |
| ValidateFunc: volumegroups.ValidateElasticSanID, | |
| }, | |
| "san_id": commonschema.ResourceIDReferenceForceNew(volumegroups.ElasticSanID{}), |
|
|
||
| * `elastic_san_id` - (Required) Specifies the Elastic SAN ID within which this Elastic SAN Volume Group should exist. Changing this forces a new resource to be created. | ||
|
|
||
| * `encryption_type` - (Optional) Type of encryption. Possible values are `EncryptionAtRestWithCustomerManagedKey` and `EncryptionAtRestWithPlatformKey`. Defaults to `EncryptionAtRestWithPlatformKey`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| * `encryption_type` - (Optional) Type of encryption. Possible values are `EncryptionAtRestWithCustomerManagedKey` and `EncryptionAtRestWithPlatformKey`. Defaults to `EncryptionAtRestWithPlatformKey`. | |
| * `encryption_type` - (Optional) Specifies the type of the key used to encrypt the data of the disk. Possible values are `EncryptionAtRestWithCustomerManagedKey` and `EncryptionAtRestWithPlatformKey`. Defaults to `EncryptionAtRestWithPlatformKey`. |
| "user_assigned_identity_id": { | ||
| Optional: true, | ||
| Type: pluginsdk.TypeString, | ||
| ValidateFunc: commonids.ValidateUserAssignedIdentityID, | ||
| }, | ||
| "key_vault_key_id": { | ||
| Required: true, | ||
| Type: pluginsdk.TypeString, | ||
| ValidateFunc: keyVaultValidate.NestedItemIdWithOptionalVersion, | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
put required ahead of optional
| "user_assigned_identity_id": { | |
| Optional: true, | |
| Type: pluginsdk.TypeString, | |
| ValidateFunc: commonids.ValidateUserAssignedIdentityID, | |
| }, | |
| "key_vault_key_id": { | |
| Required: true, | |
| Type: pluginsdk.TypeString, | |
| ValidateFunc: keyVaultValidate.NestedItemIdWithOptionalVersion, | |
| }, | |
| "key_vault_key_id": { | |
| Required: true, | |
| Type: pluginsdk.TypeString, | |
| ValidateFunc: keyVaultValidate.NestedItemIdWithOptionalVersion, | |
| }, | |
| "user_assigned_identity_id": { | |
| Optional: true, | |
| Type: pluginsdk.TypeString, | |
| ValidateFunc: commonids.ValidateUserAssignedIdentityID, | |
| }, |
|
|
||
| An `identity` block exports the following arguments: | ||
|
|
||
| * `principal_id` - The Principal ID for the System-Assigned Managed Identity assigned to this Elastic SAN Volume Group. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| * `principal_id` - The Principal ID for the System-Assigned Managed Identity assigned to this Elastic SAN Volume Group. | |
| * `principal_id` - The Principal ID associated with the Managed Service Identity assigned to this Elastic SAN Volume Group. |
|
|
||
| * `principal_id` - The Principal ID for the System-Assigned Managed Identity assigned to this Elastic SAN Volume Group. | ||
|
|
||
| * `tenant_id` - The Tenant ID for the System-Assigned Managed Identity assigned to this Elastic SAN Volume Group. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| * `tenant_id` - The Tenant ID for the System-Assigned Managed Identity assigned to this Elastic SAN Volume Group. | |
| * `tenant_id` - The Tenant ID associated with this Managed Service Identity assigned to this Elastic SAN Volume Group. |
|
|
||
| * `current_versioned_key_expiration_timestamp` - Current Versioned Key Expiration Timestamp of the Elastic SAN Volume Group. | ||
|
|
||
| * `current_versioned_key_id` - Current Versioned Key ID. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| * `current_versioned_key_id` - Current Versioned Key ID. | |
| * `current_versioned_key_id` - The ID of the current versioned Key Vault Key in use. |
|
|
||
| * `current_versioned_key_id` - Current Versioned Key ID. | ||
|
|
||
| * `last_key_rotation_timestamp` - Last Key Rotation Timestamp. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| * `last_key_rotation_timestamp` - Last Key Rotation Timestamp. | |
| * `last_key_rotation_timestamp` - The timestamp of the last rotation of the Key Vault Key. |
|
|
||
| An `encryption` block exports the following arguments: | ||
|
|
||
| * `current_versioned_key_expiration_timestamp` - Current Versioned Key Expiration Timestamp of the Elastic SAN Volume Group. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| * `current_versioned_key_expiration_timestamp` - Current Versioned Key Expiration Timestamp of the Elastic SAN Volume Group. | |
| * `current_versioned_key_expiration_timestamp` - The timestamp of the expiration time for the current version of the customer managed key. |
| "san_id": { | ||
| Type: pluginsdk.TypeString, | ||
| Required: true, | ||
| ForceNew: true, | ||
| ValidateFunc: volumegroups.ValidateElasticSanID, | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| "san_id": { | |
| Type: pluginsdk.TypeString, | |
| Required: true, | |
| ForceNew: true, | |
| ValidateFunc: volumegroups.ValidateElasticSanID, | |
| }, | |
| "san_id": commonschema.ResourceIDReferenceForceNew(volumegroups.ElasticSanID{}), |
| Optional: true, | ||
| ValidateFunc: validation.StringInSlice([]string{ | ||
| // None is not exposed | ||
| // None is not exposed, Iscsi is only allowed protocolType to connect to volume group |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| // None is not exposed, Iscsi is only allowed protocolType to connect to volume group | |
| // None is not a valid value and service team will consider removing it in future versions. |
| * `network_rule` - (Optional) One or more `network_rule` blocks as defined below. | ||
|
|
||
| * `protocol_type` - (Optional) Specifies the type of the storage target. The only possible value is `Iscsi`. Defaults to `Iscsi`. | ||
| * `protocol_type` - (Optional) Specifies the type of the storage target. The only possible values are `Iscsi`. Defaults to `Iscsi`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| * `protocol_type` - (Optional) Specifies the type of the storage target. The only possible values are `Iscsi`. Defaults to `Iscsi`. | |
| * `protocol_type` - (Optional) Specifies the type of the storage target. The only possible value is `Iscsi`. Defaults to `Iscsi`. |
| return []ElasticSANVolumeGroupResourceNetworkRuleModel{} | ||
| } | ||
|
|
||
| var networkRules []ElasticSANVolumeGroupResourceNetworkRuleModel |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| var networkRules []ElasticSANVolumeGroupResourceNetworkRuleModel | |
| networkRules := make([]ElasticSANVolumeGroupResourceNetworkRuleModel,0) |
| } | ||
|
|
||
| func (r ElasticSANVolumeGroupTestResource) template(data acceptance.TestData) string { | ||
| // some of the features are supported in limited regions, see https://learn.microsoft.com/en-us/azure/storage/elastic-san/elastic-san-networking-concepts#regional-availability |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| // some of the features are supported in limited regions, see https://learn.microsoft.com/en-us/azure/storage/elastic-san/elastic-san-networking-concepts#regional-availability | |
| // some of the features are supported in limited regions, see https://learn.microsoft.com/azure/storage/elastic-san/elastic-san-networking-concepts#regional-availability |
| ValidateFunc: validate.ElasticSanVolumnGroupName, | ||
| }, | ||
|
|
||
| "san_id": commonschema.ResourceIDReferenceRequiredForceNew(volumegroups.ElasticSanId{}), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| "san_id": commonschema.ResourceIDReferenceRequiredForceNew(volumegroups.ElasticSanId{}), | |
| "elastic_san_id": commonschema.ResourceIDReferenceRequiredForceNew(volumegroups.ElasticSanId{}), |
| KeyVaultKeyId: keyVaultKeyId, | ||
| UserAssignedIdentityId: userAssignedIdentityId, | ||
| CurrentVersionedKeyExpirationTimestamp: pointer.From(input.KeyVaultProperties.CurrentVersionedKeyExpirationTimestamp), | ||
| CurrentVersionedKeyId: pointer.From(input.KeyVaultProperties.CurrentVersionedKeyIdentifier), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what if input.KeyVaultProperties == nil ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nil check added.
|
|
||
| func ExpandVolumeGroupEncryption(input []ElasticSANVolumeGroupResourceEncryptionModel) (*volumegroups.EncryptionProperties, error) { | ||
| if len(input) == 0 { | ||
| return nil, nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
according to the protocol(https://www.rfc-editor.org/rfc/rfc7386.html#page-3), nil properties will be ignored by PATCH which means Encryption cannot be removed. Why TestAccElasticSANVolumeGroup_update can pass?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By my test, if encryptionType is updated from EncryptionAtRestWithCustomerManagedKey to EncryptionAtRestWithPlatformKey, then encryption block will be ignored at backend. In customDiff, we have limited the encryption block can only set with encryptionType=EncryptionAtRestWithCustomerManagedKey
| "regexp" | ||
| ) | ||
|
|
||
| func ElasticSanVolumnGroupName(i interface{}, k string) (warnings []string, errors []error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| func ElasticSanVolumnGroupName(i interface{}, k string) (warnings []string, errors []error) { | |
| func ElasticSanVolumeGroupName(i interface{}, k string) (warnings []string, errors []error) { |
|
|
||
| * `name` - (Required) Specifies the name of this Elastic SAN Volume Group. Changing this forces a new resource to be created. | ||
|
|
||
| * `san_id` - (Required) Specifies the Elastic SAN ID within which this Elastic SAN Volume Group should exist. Changing this forces a new resource to be created. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| * `san_id` - (Required) Specifies the Elastic SAN ID within which this Elastic SAN Volume Group should exist. Changing this forces a new resource to be created. | |
| * `elastic_san_id` - (Required) Specifies the Elastic SAN ID within which this Elastic SAN Volume Group should exist. Changing this forces a new resource to be created. |
|
|
||
| * `encryption` - (Optional) An `encryption` block as defined below. | ||
|
|
||
| **NOTE:** The `encryption` block can only be set when `encryption_type` is set to `EncryptionAtRestWithCustomerManagedKey`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| **NOTE:** The `encryption` block can only be set when `encryption_type` is set to `EncryptionAtRestWithCustomerManagedKey`. | |
| -> **NOTE:** The `encryption` block can only be set when `encryption_type` is set to `EncryptionAtRestWithCustomerManagedKey`. |
|
|
||
| var keyVaultKeyId string | ||
| if kv := input.KeyVaultProperties; kv != nil { | ||
| id, err := keyVaultParse.NewNestedItemID(pointer.From(kv.KeyVaultUri), keyVaultParse.NestedItemTypeKey, pointer.From(kv.KeyName), pointer.From(kv.KeyVersion)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if the key was rotated, which value will be returned by the service end for keyVaultKeyId? The original configured key? Or current versioned key?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From doc, If key_vault_id has no version, the backend will periodically retrieve the latest version, if version specified, we need to manually update version if key is rotated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it mean no matter KeyVersion is set or not, the service end will not change its value in Get API if the keyvault was rotated before? Could you confirm it with the service team or by your test?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, by my test, KeyVersion will not be changed, the versioned Key ID in use is exported through currentVersionedKeyId field of REST API.
| return nil, err | ||
| } | ||
|
|
||
| result := volumegroups.EncryptionProperties{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if the key was rotated and we still take the original configured value to the service end, how will the service end handle it? Will the service end use the configured key to replace the current versioned key?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If version is specified, specific versioned key will be used;
if version is not specified,
Elastic SAN checks the key vault daily for a new version of the key.
stephybun
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @teowa and @ms-zhenhua! LGTM 🦀
|
This functionality has been released in v3.89.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! |
<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.88.0" to "3.89.0" in file
".terraform.lock.hcl"</p>
<details>
<summary>3.89.0</summary>
<pre>Changelog retrieved
from:
	https://github.com/hashicorp/terraform-provider-azurerm/releases/tag/v3.89.0
FEATURES:

*
New Data Source: `azurerm_data_factory_trigger_schedule`
([#24572](hashicorp/terraform-provider-azurerm#24572
New Data Source: `azurerm_data_factory_trigger_schedules`
([#24572](hashicorp/terraform-provider-azurerm#24572
New Data Source: `azurerm_ip_groups`
([#24540](hashicorp/terraform-provider-azurerm#24540
New Data Source: `azurerm_nginx_certificate`
([#24577](hashicorp/terraform-provider-azurerm#24577
New Resource: `azurerm_chaos_studio_target`
([#24580](hashicorp/terraform-provider-azurerm#24580
New Resource: `azurerm_elastic_san_volume_group`
([#24166](hashicorp/terraform-provider-azurerm#24166
New Resource: `azurerm_netapp_account_encryption`
([#23733](hashicorp/terraform-provider-azurerm#23733
New Resource: `azurerm_redhat_openshift_cluster`
([#24375](https://github.com/hashicorp/terraform-provider-azurerm/issues/24375))

ENHANCEMENTS:

*
dependencies: updating to `v0.66.1` of
`github.com/hashicorp/go-azure-helpers`
([#24561](hashicorp/terraform-provider-azurerm#24561
dependencies: updating to `v0.20240124.1115501` of
`github.com/hashicorp/go-azure-sdk`
([#24619](hashicorp/terraform-provider-azurerm#24619
`bot`: updating to API Version `2021-05-01-preview`
([#24555](hashicorp/terraform-provider-azurerm#24555
`containerservice`: the SDK Clients now support logging
([#24564](hashicorp/terraform-provider-azurerm#24564
`cosmosdb`: updating to API Version `2023-04-15`
([#24541](hashicorp/terraform-provider-azurerm#24541
`loadtestservice`: updating to use the base layer from
`hashicorp/go-azure-sdk` rather than `Azure/go-autorest` (and support
logging)
([#24578](hashicorp/terraform-provider-azurerm#24578
`managedidentity`: updating to use the base layer from
`hashicorp/go-azure-sdk` rather than `Azure/go-autorest` (and support
logging)
([#24578](hashicorp/terraform-provider-azurerm#24578
`azurerm_api_management_api` - change the `id` format so specific
`revision`s can be managed by Terraform
([#23031](hashicorp/terraform-provider-azurerm#23031
`azurerm_data_protection_backup_vault` - the `redundancy` propety can
now be set to `ZoneRedundant`
([#24556](hashicorp/terraform-provider-azurerm#24556
`azurerm_data_factory_integration_runtime_azure_ssis` - support for the
`credential_name` property
([#24458](hashicorp/terraform-provider-azurerm#24458
`azurerm_orchestrated_virtual_machine_scale_set` - support
`2022-datacenter-azure-edition-hotpatch` and
`2022-datacenter-azure-edition-hotpatch-smalldisk` hotpatching images
([#23500](hashicorp/terraform-provider-azurerm#23500
`azurerm_stream_analytics_job` - support for the `sku_name` property
([#24554](https://github.com/hashicorp/terraform-provider-azurerm/issues/24554))

BUG
FIXES:

* Data Source: `azurerm_app_service` - parsing the API
Response for `app_service_plan_id` case-insensitively
([#24626](hashicorp/terraform-provider-azurerm#24626
Data Source: `azurerm_function_app` - parsing the API Response for
`app_service_plan_id` case-insensitively
([#24626](hashicorp/terraform-provider-azurerm#24626
`azurerm_app_configuration_key` - the value for the `value` property can
now be removed/emptied
([#24582](https://github.com/hashicorp/terraform-provider-azurerm/issues/24582))

*
`azurerm_app_service` - parsing the API Response for
`app_service_plan_id` case-insensitively
([#24626](hashicorp/terraform-provider-azurerm#24626
`azurerm_app_service_plan` - fix casing in `serverFarms` due to ID
update
([#24562](hashicorp/terraform-provider-azurerm#24562
`azurerm_app_service_slot` - parsing the API Response for
`app_service_plan_id` case-insensitively
([#24626](hashicorp/terraform-provider-azurerm#24626
`azurerm_automation_schedule` - only one `monthly_occurence` block can
now be specified
([#24614](hashicorp/terraform-provider-azurerm#24614
`azurerm_cognitive_deployment` - the `model.version` property is no
longer required
([#24264](hashicorp/terraform-provider-azurerm#24264
`azurerm_container_app` - multiple `custom_scale_rule` can not be
updated
([#24509](hashicorp/terraform-provider-azurerm#24509
`azurerm_container_registry_task_schedule_run_now` - prevent issue where
the incorrect scheduled run in tracked if there have been multiple
([#24592](hashicorp/terraform-provider-azurerm#24592
`azurerm_function_app` - parsing the API Response for
`app_service_plan_id` case-insensitively
([#24626](hashicorp/terraform-provider-azurerm#24626
`azurerm_function_app_slot` - parsing the API Response for
`app_service_plan_id` case-insensitively
([#24626](hashicorp/terraform-provider-azurerm#24626
`azurerm_logic_app_standard` - now will parse the app service ID
insensitively
([#24562](hashicorp/terraform-provider-azurerm#24562
`azurerm_logic_app_workflow` - the `workflow_parameters` will now
correctly handle information specified by `$connections`
([#24141](hashicorp/terraform-provider-azurerm#24141
`azurerm_mssql_managed_instance_security_alert_policy` - can not update
empty storage attributes
([#24553](hashicorp/terraform-provider-azurerm#24553
`azurerm_network_interface` - the `ip_configuration` properties are no
longer added to a Load Balancer Backend if one of those
`ip_configurations` is associated with a backend
([#24470](https://github.com/hashicorp/terraform-provider-azurerm/issues/24470))


</pre>
</details>
</details>
<a
href="https://infra.ci.jenkins.io/job/terraform-jobs/job/azure/job/main/1052/">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) <[email protected]>
Co-authored-by: Damien Duportal <[email protected]>
|
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. |



ref: