-
Notifications
You must be signed in to change notification settings - Fork 466
Add example of applying enterprise license on auto-unseal example #99
Conversation
Etiene
commented
Oct 9, 2018
- Added example of applying enterprise license
- Updated from variables for kms key to data source through key alias
examples/vault-auto-unseal/main.tf
Outdated
aws_region = "${data.aws_region.current.name}" | ||
kms_key_id = "${data.aws_kms_alias.vault-example.target_key_id}" | ||
aws_region = "${data.aws_region.current.name}" | ||
vault_enterprise_license_key = "${var.vault_enterprise_license_key}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the license key itself considered a secret? i.e., do we need to tell users to encrypt it for production use cases? Of course, this is a bit of a chicken-and-egg, as Vault itself is what we'd generally use to manage secrets...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Following discussion with Josh we decided to treat it with moderate secrecy and not encrypt it
# will only succeed in the first one that runs it, the others will fail with "the server has | ||
# already been initialized" and the script will be interrupted. This is not an | ||
# issue, and, for the purpose of this example, it is simpler than reorganizing the | ||
# code in such a way that the nodes receive different user data files. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm... but what about production use cases? Should we tell users to run init
manually against one node? Or perhaps better, should we detect the "already initialized" error and ignore it based on the assumption one of the other nodes already did the initialization? I'd rather avoid the script exiting with an error, as that will get very confusing to debug (e.g., if later on we add something after this line that we expect to run, but now doesn't on 2 out of 3 nodes).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Following discussion with Josh, we decided to move this bit from the user data to the go tests since it should be manually done by the operator one time. This would then eliminate this issue. But I will leave it as a comment in the user data.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should it still be a manual operation even if you have auto-unseal and could therefore boot up a cluster completely automatically?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is a fair point. But then we have to make an informed decision to suggest on how to handle the root token in production, dont we? Any opinions on that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I totally forgot that's when you get a root token. Yea, never mind, best that's handled manually.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok 👍