It would be awesome to support passing pgp_keys, recovery_pgp_keys, and root_token_pgp_key attributes to vaultoperator_init, which would help to alleviate the potential issues that come with having unseal/recovery keys and root tokens in plaintext in the Terraform state.
I'm creating this issue mostly for tracking purposes, since I've already implemented this feature in my fork and would be very happy to have a pull request merged for it.
For now I've opted to only support base64-encoded binary PGP public keys as the HTTP API does, and highlight this in the attribute descriptions, but support for binary, armored, and keybase usernames might be a nice-to-have in the future.
Together, these changes make it possible to run multiple isolated Vault servers concurrently, one for each acceptance test, although Vault does not output the random port chosen by the kernel, so I had to instead parse the automatically chosen cluster address and port from Vault's output, which should be one port higher than the randomly chosen Vault API port, although I don't know that this is guaranteed to always hold true.
Using this helper I've added a new acceptance test that exercises the new attributes, rather than modifying the current acceptance test, to prevent any regressions. This required adding gopenpgp as a dependency though, and as this is my first time writing Go I'm not sure if there's an idiomatic way to specify test-only dependencies, or if that's necessary.
I don't know if you necessarily want to incorporate these testing changes, but if not they should be pretty easy to revert (or I can do so if you don't have time).
I'm not sure if it would make sense to mark the pgp_keys, recovery_pgp_keys, and root_token_pgp_key attributes as sensitive, but I chose not to since they're public keys and shouldn't be considered secret anyhow.
Backwards compatibility
In order to allow users to match the returned encrypted unseal/recovery keys to their corresponding PGP public keys, the ordering of the elements of the keys, keys_base64, recovery_keys, and recovery_keys_base64 attributes MUST match their ordering as returned by the Vault HTTP API.
I initially thought the change of type might require the implementation of a state migration, but I manually tested the upgrade scenario and didn't have any issues:
Hello,
It would be awesome to support passing
pgp_keys,recovery_pgp_keys, androot_token_pgp_keyattributes tovaultoperator_init, which would help to alleviate the potential issues that come with having unseal/recovery keys and root tokens in plaintext in the Terraform state.I'm creating this issue mostly for tracking purposes, since I've already implemented this feature in my fork and would be very happy to have a pull request merged for it.
Considerations
Key format
The
vault operator initcommand takes a list of PGP public key filenames (which can be either binary or armored) or keybase usernames in the formatkeybase:<username>, but the Vault HTTP API only accepts base64-encoded binary PGP public keys (unarmored), which could cause confusion for users who are accustomed to the formats accepted byvault operator init.For now I've opted to only support base64-encoded binary PGP public keys as the HTTP API does, and highlight this in the attribute descriptions, but support for binary, armored, and keybase usernames might be a nice-to-have in the future.
Testing
The current acceptance tests start a local Vault server before running and stop it after all the tests have run, meaning that Vault can only be initialized once during acceptance tests.
I've added a helper to start and stop a Vault server before and after each acceptance test, and modified the
vault.hclconfiguration file to use in-memory storage, so it doesn't have to be cleaned up after each test run. I have also modifiedvault.hclto use a random port for each Vault server by specifying a port number of0.Together, these changes make it possible to run multiple isolated Vault servers concurrently, one for each acceptance test, although Vault does not output the random port chosen by the kernel, so I had to instead parse the automatically chosen cluster address and port from Vault's output, which should be one port higher than the randomly chosen Vault API port, although I don't know that this is guaranteed to always hold true.
Using this helper I've added a new acceptance test that exercises the new attributes, rather than modifying the current acceptance test, to prevent any regressions. This required adding
gopenpgpas a dependency though, and as this is my first time writing Go I'm not sure if there's an idiomatic way to specify test-only dependencies, or if that's necessary.I've also switched the GitHub
test.ymlworkflow from usinginnovationnorway/setup-vaulttoeLco/setup-vault, since it was failing intermittently during my testing and appears to be archived.I don't know if you necessarily want to incorporate these testing changes, but if not they should be pretty easy to revert (or I can do so if you don't have time).
Your testing setup is great, and made it super easy to test my changes successfully on my fork!
Sensitive attributes
I'm not sure if it would make sense to mark the
pgp_keys,recovery_pgp_keys, androot_token_pgp_keyattributes as sensitive, but I chose not to since they're public keys and shouldn't be considered secret anyhow.Backwards compatibility
In order to allow users to match the returned encrypted unseal/recovery keys to their corresponding PGP public keys, the ordering of the elements of the
keys,keys_base64,recovery_keys, andrecovery_keys_base64attributes MUST match their ordering as returned by the Vault HTTP API.Currently, these attributes are of
Type: schema.TypeSet, which does not preserve the ordering of the items returned by the Vault HTTP API, making this impossible.I've changed them to
Type: schema.TypeList, but this is considered a breaking change by Terraform perChanging attribute type where the new type is functionally incompatible (including but not limited to changing TypeSet to TypeList and TypeList to TypeSet).Versioning
Accordingly, the version should be bumped to
0.2.0(although theCHANGELOG.mdcontradicts the current tag) or to1.0.0if you prefer.State migration
I initially thought the change of type might require the implementation of a state migration, but I manually tested the upgrade scenario and didn't have any issues:
Provider upgrade testing steps
``` git checkout pgp mkdir -p test cat > test/provider.tf <