azurerm_keyvault_key: udpate client cache logic to support managedHSM keys

[wuxu92]

The current version's cache logic only supports key vault keys. This PR refactors the cache to accommodate various vault IDs, such as manage HSM. Presently, existing locks seem fail to prevent simultaneous read/write operations on the keyVaultCache map by merely locking a specific cacheKey.

[tombuildsstuff]

@wuxu92 FWIW Managed HSMs shouldn't be using the same cache as Key Vaults as they're different products (and not 100% interchangeable, since not every service which supports Key Vault supports Managed HSM - and there's behavioural differences between them) - so can we split this cache into one for Managed HSM and another for Key Vault to allow for this differentiation?

[wuxu92]

@tombuildsstuff Sure, that makes sense! I'll update it this way.

[wuxu92]

Hi @tombuildsstuff

I have updated the PR to utilize a distinct cache instance for keyvault. I have kept the signature of the cache-related methods unchanged to avoid breaking them. Additionally, I will include a managedHSM cache instance in the pull request for the managed HSM key resource.

[tombuildsstuff]

@wuxu92 I've spend some more time digging into Key Vault and Managed HSM this morning as a part of determining what's needed to move Key Vault to the new base layer.

As a part of that, I've opened #24737 which splits ManagedHSM out into it's own Service Package - this allows us to both separate the logic for these two (since there's a number of behavioural differences already) - but also ensures that when importing packages for parsing/validation purposes (or as in the case of this PR, adding caching) - that these changes are being scoped only to the appropriate product.

Since these are now going to be entirely isolated Service Packages, rather than try to combine this into the existing cache for Key Vaults - it'd be better to duplicate the Key Vault cache that we've got for Managed HSMs, as such would you mind updating this? Would you also mind elaborating on the use-case for this cache?

Thanks!

[wuxu92]

@tombuildsstuff I'm currently developing key resources for managedHSM, and it requires the same cache logic as the keyvault cache. Since managedHSM will be separate from the keyvault package, I will update this PR once #24737 is merged.

[katbyte]

@wuxu92 - now that #24737 is merged whats the timeline to get this PR updated?

[wuxu92]

@katbyte I apologize for the delayed response. As the managedHSM is now in a separate package, I will close this pull request since the cache logic is completely different from what was initially proposed.

[github-actions]

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.