fix: default disk encryption key type to RSA - #25
Conversation
There was a problem hiding this comment.
Code Review
This pull request changes the default key_type from RSA-HSM to RSA and updates the documentation to clarify that HSM key types require a premium or HSM-capable Key Vault. The review feedback suggests adding a validation block to the key_type variable and removing references to EC and EC-HSM from the descriptions, as the current implementation only supports RSA types.
| default = "RSA" | ||
| description = "The Key Type for Key Vault. Possible values: EC, EC-HSM, RSA, RSA-HSM. Use an HSM key type only with a premium or HSM-capable Key Vault." |
There was a problem hiding this comment.
While changing the default to RSA improves compatibility with standard Key Vaults, there are two improvement opportunities:
- Validation: Adding a validation block ensures that only valid values are provided, preventing late-stage deployment failures.
- EC Support Discrepancy: The description lists
ECandEC-HSMas possible values, but the module's implementation inmain.tf(lines 101-109) does not currently support them (it lacks the requiredcurveparameter and unconditionally passeskey_size). It is recommended to remove these from the description until support is implemented.
default = "RSA"
description = "The Key Type for Key Vault. Possible values: RSA, RSA-HSM. Use an HSM key type only with a premium or HSM-capable Key Vault."
validation {
condition = contains(["RSA", "RSA-HSM"], var.key_type)
error_message = "The key_type must be one of: RSA, RSA-HSM."
}
| | key\_permissions | List of Key Vault key permissions | `list(string)` | <pre>[<br> "Create",<br> "Delete",<br> "Get",<br> "Purge",<br> "Recover",<br> "Update",<br> "WrapKey",<br> "UnwrapKey",<br> "List",<br> "Decrypt",<br> "Sign"<br>]</pre> | no | | ||
| | key\_size | Size of the RSA key in bytes (e.g., 1024, 2048). | `number` | `2048` | no | | ||
| | key\_type | The Key Type for Key Vault. Possible values: EC, EC-HSM, RSA, RSA-HSM. | `string` | `"RSA-HSM"` | no | | ||
| | key\_type | The Key Type for Key Vault. Possible values: EC, EC-HSM, RSA, RSA-HSM. Use an HSM key type only with a premium or HSM-capable Key Vault. | `string` | `"RSA"` | no | |
There was a problem hiding this comment.
The description lists EC and EC-HSM as possible values, but the module's implementation currently only supports RSA types. Consider removing them to accurately reflect the module's capabilities.
| | key\_type | The Key Type for Key Vault. Possible values: EC, EC-HSM, RSA, RSA-HSM. Use an HSM key type only with a premium or HSM-capable Key Vault. | `string` | `"RSA"` | no | | |
| | key\_type | The Key Type for Key Vault. Possible values: RSA, RSA-HSM. Use an HSM key type only with a premium or HSM-capable Key Vault. | `string` | `"RSA"` | no | |
Summary
Why
The current module default creates an HSM-backed key when . That fails against a standard Key Vault with . Defaulting to keeps the module compatible with standard Key Vault deployments while still allowing callers to opt into explicitly when they use a premium vault.
Validation