`managed_hsm_role_*_ids`: use specific resource id to replace generic nested item id #25323

wuxu92 · 2024-03-20T03:04:03Z

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

Use specific resource ids for managed hsm role defitnition and role assignment instead of the origin generic id. It contains a scope field in the middle of the current resource ID, So it's not supported to convert to a New Resource ID Type in this PR.

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.
(For changes that include a state migration only). I have manually tested the migration path between relevant versions of the provider.

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.

azurerm_key_vault_managed_hardware_security_module_role_definition - update resource id with specific definition instead of generic ones
azurerm_key_vault_managed_hardware_security_module_role_assignment - update resource id with specific definition instead of generic ones

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)

rcskosir · 2024-03-25T19:27:49Z

@wuxu92 👋 Thank you for using our new PR template and adding a description! Can you please check the boxes that apply for this PR and also update the changelog portion for this PR? This aids with the review process. Thank you!

wuxu92 · 2024-03-26T05:41:53Z

@rcskosir thank you for reminding me. I have made the necessary updates.

mbfrahry

Hey @wuxu92 , thanks for this PR! I think it mostly looks good but I do have a question around ports. It looks like we're stripping them but I wasn't under the impression that ports were associated with this id.

If ports do need to be stripped, is there a reason we are only doing it on the role assignment resource and not the role definition resource?

mbfrahry · 2024-03-27T23:48:27Z

internal/services/managedhsm/parse/managed_hsm_role_assignment_id.go

+	if err != nil || hsmBaseUrl == "" {
+		return nil, fmt.Errorf("parsing managedHSM nested itemID %q: %+v", hsmBaseUrl, err)
+	}
+	// (@jackofallops) - Log Analytics service adds the port number to the API returns, so we strip it here


Is this relevant here? And if so, is it related to the log analytics service?

The role definition and assignment ID shouldn't relate to the log analytics service. I've removed the stripping operation for both the role definition and assignment.

mbfrahry

Thanks for getting that cleaned up but it looks like the unit tests need to be updated

wuxu92 · 2024-03-29T00:51:14Z

My apologies, @mbfrahry. I missed that unit test, but I've updated it now.

mbfrahry

LGTM! Thanks for getting that fixed up @wuxu92

<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.97.1" to "3.98.0" in file ".terraform.lock.hcl"</p> <details> <summary>3.98.0</summary> <pre>Changelog retrieved from:
	https://github.com/hashicorp/terraform-provider-azurerm/releases/tag/v3.98.0
FEATURES:

* New Resource: `azurerm_static_web_app_function_app_registration` ([#25331](hashicorp/terraform-provider-azurerm#25331 New Resource: `azurerm_system_center_virtual_machine_manager_inventory_items` ([#25110](hashicorp/terraform-provider-azurerm#25110 New Resource: `azurerm_workloads_sap_discovery_virtual_instance` ([#24342](hashicorp/terraform-provider-azurerm#24342 New Resource: `azurerm_redis_cache_policy` ([#25477](hashicorp/terraform-provider-azurerm#25477 New Resource: `azurerm_redis_cache_policy_assignment` ([#25477](https://github.com/hashicorp/terraform-provider-azurerm/issues/25477))

ENHANCEMENTS:

* dependencies: updating to `v0.20240402.1085733` of `github.com/hashicorp/go-azure-sdk` ([#25482](hashicorp/terraform-provider-azurerm#25482 dependencies: updating to `v0.67.0` of `github.com/hashicorp/go-azure-helpers` ([#25446](hashicorp/terraform-provider-azurerm#25446 dependencies: updating to `v0.25.4` of `github.com/tombuildsstuff/giovanni` ([#25404](hashicorp/terraform-provider-azurerm#25404 `alertsmanagement` - updating remaining resources to use `hashicorp/go-azure-sdk` ([#25486](hashicorp/terraform-provider-azurerm#25486 `applicationinsights` - updating remaining resources to use `hashicorp/go-azure-sdk` ([#25376](hashicorp/terraform-provider-azurerm#25376 `compute` - update to API version `2024-03-01` ([#25436](hashicorp/terraform-provider-azurerm#25436 `compute` - update shared image resources and data sources to use `hashicorp/go-azure-sdk` ([#25503](hashicorp/terraform-provider-azurerm#25503 `containerinstance` - update to use the transport layer from `hashicorp/go-azure-sdk` rather than `Azure/go-autorest` ([#25416](hashicorp/terraform-provider-azurerm#25416 `maintenance` - updating to API Version `2023-04-01` ([#25388](hashicorp/terraform-provider-azurerm#25388 `recovery_services` - Add `recovery_service` block to the provider that supports `vm_backup_stop_protection_and_retain_data_on_destroy` and `purge_protected_items_from_vault_on_destroy`([#25515](hashicorp/terraform-provider-azurerm#25515 `storage` - the Storage Account cache is now populated using `hashicorp/go-azure-sdk` ([#25437](hashicorp/terraform-provider-azurerm#25437 `azurerm_bot_service_azure_bot` - support for the `cmk_key_vault_key_url` property ([#23640](hashicorp/terraform-provider-azurerm#23640 `azurerm_capacity_reservation` - update validation for `capacity` ([#25471](hashicorp/terraform-provider-azurerm#25471 `azurerm_container_app` - add support for `key_vault_id` and `identity` properties in the `secret` block ([#24773](hashicorp/terraform-provider-azurerm#24773 `azurerm_databricks_workspace` - expose `managed_services_cmk_key_vault_id` and `managed_disk_cmk_key_vault_id and key_vault_id` to support cross subscription CMK's. ([#25091](hashicorp/terraform-provider-azurerm#25091 `azurerm_databricks_workspace_root_dbfs_customer_managed_key` - expose `key_vault_id` to support cross subscription CMK's. ([#25091](hashicorp/terraform-provider-azurerm#25091 `azurerm_managed_hsm_role_*_ids` - use specific resource id to replace generic nested item id ([#25323](hashicorp/terraform-provider-azurerm#25323 `azurerm_mssql_database` - add support for `secondary_type` ([#25360](hashicorp/terraform-provider-azurerm#25360 `azurerm_monitor_scheduled_query_rules_alert_v2` - support for the `identity` block ([#25365](hashicorp/terraform-provider-azurerm#25365 `azurerm_mssql_server_extended_auditing_policy` - support for `audit_actions_and_groups` and `predicate_expression` ([#25425](hashicorp/terraform-provider-azurerm#25425 `azurerm_netapp_account` - can now be imported ([#25384](hashicorp/terraform-provider-azurerm#25384 `azurerm_netapp_volume` - support for the `kerberos_enabled`, `smb_continuous_availability_enabled`, `kerberos_5_read_only_enabled`, `kerberos_5_read_write_enabled`, `kerberos_5i_read_only_enabled`, `kerberos_5i_read_write_enabled`, `kerberos_5p_read_only_enabled`, and `kerberos_5p_read_write_enabled` properties ([#25385](hashicorp/terraform-provider-azurerm#25385 `azurerm_recovery_services_vault` - upgrading to version `2024-01-01` ([#25325](hashicorp/terraform-provider-azurerm#25325 `azurerm_stack_hci_cluster` - the `client_id` property is now optional ([#25407](hashicorp/terraform-provider-azurerm#25407 `azurerm_storage_encryption_scope` - refactoring to use `hashicorp/go-azure-sdk` rather than `Azure/azure-sdk-for-go` ([#25437](hashicorp/terraform-provider-azurerm#25437 `azurerm_mssql_elasticpool` - the `maintenance_configuration_name` property now supports values `SQL_SouthAfricaNorth_DB_1`, `SQL_SouthAfricaNorth_DB_2`, `SQL_WestUS3_DB_1` and `SQL_WestUS3_DB_2` ([#25500](hashicorp/terraform-provider-azurerm#25500 `azurerm_lighthouse_assignment` - updating API Version from `2019-06-01` to `2022-10-01` ([#25473](https://github.com/hashicorp/terraform-provider-azurerm/issues/25473))

BUG FIXES:

* `network` - updating the `GatewaySubnet` validation to show the Subnet Name when the validation fails ([#25484](hashicorp/terraform-provider-azurerm#25484 `azurerm_function_app_hybrid_connection` - fix an issue during creation when `send_key_name` is specified ([#25379](hashicorp/terraform-provider-azurerm#25379 `azurerm_linux_web_app_slot` - fix a crash when upgrading the provider to v3.88.0 or later ([#25406](hashicorp/terraform-provider-azurerm#25406 `azurerm_mssql_database` - update the behavior of the `enclave_type` field. ([#25508](hashicorp/terraform-provider-azurerm#25508 `azurerm_mssql_elasticpool` - update the behavior of the `enclave_type` field. ([#25508](hashicorp/terraform-provider-azurerm#25508 `azurerm_network_manager_deployment` - add locking ([#25368](hashicorp/terraform-provider-azurerm#25368 `azurerm_resource_group_template_deployment` - changes to `parameters_content` and `template_content` now force `output_content` to be updated in the plan ([#25403](hashicorp/terraform-provider-azurerm#25403 `azurerm_storage_blob` - fix a potential crash when the endpoint is unreachable ([#25404](hashicorp/terraform-provider-azurerm#25404 `azurerm_storage_container` - fix a potential crash when the endpoint is unreachable ([#25404](hashicorp/terraform-provider-azurerm#25404 `azurerm_storage_data_lake_gen2_filesystem` - fix a potential crash when the endpoint is unreachable ([#25404](hashicorp/terraform-provider-azurerm#25404 `azurerm_storage_data_lake_gen2_filesystem_path` - fix a potential crash when the endpoint is unreachable ([#25404](hashicorp/terraform-provider-azurerm#25404 `azurerm_storage_queue` - fix a potential crash when the endpoint is unreachable ([#25404](hashicorp/terraform-provider-azurerm#25404 `azurerm_storage_share` - fix a potential crash when the endpoint is unreachable ([#25404](hashicorp/terraform-provider-azurerm#25404 `azurerm_storage_share_directory` - fix a potential crash when the endpoint is unreachable ([#25404](hashicorp/terraform-provider-azurerm#25404 `azurerm_storage_share_directory` - resolve an issue where directories might fail to destroy ([#25404](hashicorp/terraform-provider-azurerm#25404 `azurerm_storage_share_file` - fix a potential crash when the endpoint is unreachable ([#25404](hashicorp/terraform-provider-azurerm#25404 `azurerm_storage_share_file` - fix several bugs with path handling when creating files in subdirectories ([#25404](hashicorp/terraform-provider-azurerm#25404 `azurerm_web_app_hybrid_connection` - fix an issue during creation when `send_key_name` is specified ([#25379](hashicorp/terraform-provider-azurerm#25379 `azurerm_windows_web_app` - prevent a panic during resource upgrade ([#25509](https://github.com/hashicorp/terraform-provider-azurerm/issues/25509))


</pre> </details> </details> <a href="https://infra.ci.jenkins.io/job/updatecli/job/azure/job/main/88/">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]>

github-actions · 2024-04-29T02:08:17Z

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.

use specific resource id for mhsm role definitionand role assignment

e2b4862

github-actions bot added service/managed-hsm size/XL labels Mar 20, 2024

wuxu92 force-pushed the mhsm/ids branch from e5cc205 to e2b4862 Compare March 20, 2024 03:11

wuxu92 changed the title ~~managed_hsm_*_ids: use specific resource id to replace generic nested item id~~ managed_hsm_role_*_ids: use specific resource id to replace generic nested item id Mar 20, 2024

wuxu92 marked this pull request as draft March 26, 2024 05:13

wuxu92 marked this pull request as ready for review March 26, 2024 05:41

mbfrahry reviewed Mar 27, 2024

View reviewed changes

github-actions bot added the waiting-response label Mar 27, 2024

remove port strip operation from id structure method

fd47baa

github-actions bot added waiting-response and removed waiting-response labels Mar 28, 2024

mbfrahry requested changes Mar 28, 2024

View reviewed changes

fix acctest

fcf705c

github-actions bot removed the waiting-response label Mar 29, 2024

mbfrahry approved these changes Mar 29, 2024

View reviewed changes

mbfrahry merged commit 01d353f into hashicorp:main Mar 29, 2024
30 checks passed

mbfrahry added this to the v3.98.0 milestone Mar 29, 2024

mbfrahry added a commit that referenced this pull request Mar 29, 2024

Update CHANGELOG.md for #25323

548b691

github-actions bot locked as resolved and limited conversation to collaborators Apr 29, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`managed_hsm_role_*_ids`: use specific resource id to replace generic nested item id #25323

`managed_hsm_role_*_ids`: use specific resource id to replace generic nested item id #25323

wuxu92 commented Mar 20, 2024 •

edited

Loading

rcskosir commented Mar 25, 2024 •

edited

Loading

wuxu92 commented Mar 26, 2024

mbfrahry left a comment

mbfrahry Mar 27, 2024

wuxu92 Mar 28, 2024

mbfrahry left a comment

wuxu92 commented Mar 29, 2024

mbfrahry left a comment

github-actions bot commented Apr 29, 2024

managed_hsm_role_*_ids: use specific resource id to replace generic nested item id #25323

managed_hsm_role_*_ids: use specific resource id to replace generic nested item id #25323

Conversation

wuxu92 commented Mar 20, 2024 • edited Loading

Community Note

Description

PR Checklist

Changes to existing Resource / Data Source

Testing

Change Log

Related Issue(s)

rcskosir commented Mar 25, 2024 • edited Loading

wuxu92 commented Mar 26, 2024

mbfrahry left a comment

Choose a reason for hiding this comment

mbfrahry Mar 27, 2024

Choose a reason for hiding this comment

wuxu92 Mar 28, 2024

Choose a reason for hiding this comment

mbfrahry left a comment

Choose a reason for hiding this comment

wuxu92 commented Mar 29, 2024

mbfrahry left a comment

Choose a reason for hiding this comment

github-actions bot commented Apr 29, 2024

`managed_hsm_role_*_ids`: use specific resource id to replace generic nested item id #25323

`managed_hsm_role_*_ids`: use specific resource id to replace generic nested item id #25323

wuxu92 commented Mar 20, 2024 •

edited

Loading

rcskosir commented Mar 25, 2024 •

edited

Loading