New resource azurerm_postgresql_server_key - Add CMK support

[ArcturusZhang]

Adding customer managed key support for postgreSQL Server
Fixes #7811

[tombuildsstuff]

hey @ArcturusZhang

Thanks for this PR - I've taken a look through and left some comments online which mostly mirror the MySQL PR. If we can fix those up then we should be able to take another look 👍

Thanks!

[tombuildsstuff]

Should this also support the "versionless" secrets? If so we'll likely need to look up the current version to be able to use that as a name?

[ArcturusZhang]

By saying key_vault_key_id, it means some key URL WITH version like this:

https://YourVaultName.vault.azure.net/keys/YourKeyName/01234567890123456789012345678901>

Do you suggest we change this schema to align with the CMK for storage? i.e, like this:

key_vault_id = 
key_name = 
key_version = 

to emphasize that we are requiring a versioned key here?

[tombuildsstuff]

right however most Azure API's which support a versioned secret also take a "versionless" version too (since the 'version' gets parsed as empty, which the Key Vault API's return as "latest") - so we should test that

No, this makes sense to leave as key_vault_key_id - however we need to confirm if this validation needs to accept both a versioned and versionless Key ID, which we'll identify through testing that?

[ArcturusZhang]

Sure I will have a try with this

[ArcturusZhang]

Just have a try if we pass a key URL without version, and get an internalServerError.
And per their document page, they should be expecting the key URL with version

[katbyte]

Then should we be validing that the id has a version then?

[ArcturusZhang]

This function ValidateKeyVaultChildId is expecting this ID to have a version. We have a ValidateKeyVaultChildIdVersionOptional for the version-optional scenario

[ArcturusZhang]

Hi @tombuildsstuff I have made some changes, please have a look.

About the version of keys, this resource is requiring a key URL with version. Therefore it is possible that we could change the schema a little bit to align with the CMK in storage account (details in review comments). What do you think?

[katbyte]

Thanks @ArcturusZhang - however it looks like multiple tests are failing with:

 Error: creating PostgreSQL Server "acctest-psql-server-200919181404659776" (Resource Group "acctestRG-psql-200919181404659776"): postgresql.ServersClient#Create: Failure sending request: StatusCode=405 -- Original Error: Code="FeatureSwitchNotEnabled" Message="Requested feature is not enabled"

Is there something we need to enable? and how can we add this new feature without breaking existing users?

[katbyte]

LGTM as the test failure is unrelated 👍

[ghost]

This has been released in version 2.29.0 of the provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. As an example:

provider "azurerm" {
    version = "~> 2.29.0"
}
# ... other configuration ...

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks!