This repository has been archived by the owner on Jan 25, 2023. It is now read-only.
Enhancement to support ACL bootstrapping #221
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… dependencies with the new modular structure, as well as instructions on how to run tests without building an AMI each time.
… passing in an ACL token. This will allow the methods to be used for testing an ACL enabled cluster as well as the default setup.
…r SSM parameter read / write access if that option is selected at deployment time.
…if the cluster should be aset up to allow reading/writing ACL tokens to permitted stores (currently only SSM).
…er with ACL enabled, and an example added with appropriate options set. Tests still fail since this functionality is not implemented.
…mmons' Gruntwork library.
…to run Consul, as well as a function to calculate the name of the SSM parameter to store the bootstrap token in.
…h the run-consul file now sources. This makes testing functions easier and also follows the pattern of other repositories, such as the terraform-aws-couchbase one.
…er nodes to read / write appropriate SSM parameters.
…pe' parameter when writing the token to SSM by way of the 'aws ssm put-parameter' call.
…n '--enable-acl' is set.
…n-consul script with the '--enable-acl' flag set.
…nsul-commons.sh' file when installing Consul.
…d persists it to SSM storage.
… exists before attempting to create it.
…nt to use them upon start-up.
…if the '--server' flag is set.
…ation, since agents currently pass through their token uand so will fail the test for empty members.
…ave been updated.
The steps to download and install the 'bash-commons' library have been moved to the 'install-consul' script so that it is not dependent upon the Packer configuration to include this required step.
mr-miles
reviewed
May 1, 2021
mr-miles
reviewed
May 3, 2021
The method used by the run-consul script to generate the agent token has been enhanced to use a newer method when the version of Consul is compatible with it.
- The run-consul script will now source a specific script file based on the value given by the new '--acl-storage-type' command line argument. This allows for easier addition of new methods of storing ACL tokens based on a set of common interface functions (write_acl_token, read_acl_token). - An additional check to ensure that the token generated for the agent is not empty has been added, so that the script will properly throw an error and exit if the token cannot be generated. - The install-consul script has been updated to include copying all '*.sh' files in the 'modules/run-consul' directory, so as to include any new common script files added from here on out. - The example deployment with ACLs has been updated with the newly added command line arguments for the user data scripts. - The test for ACL enabled clusters has been corrected so that it expects an empty response to 'consul members' when not passing in an ACL token.
- The function that generates the agent ACL token policy has been updated to remove un-necessary read permissions. It now matches the example given in the official Hashicorp tutorial (https://learn.hashicorp.com/tutorials/consul/access-control-setup-production\#create-the-agent-policy).
|
I think I've incorporated everything requested, please let me know if there's anything else required here? Thanks! |
|
Howdy, just wanted to ping here since I still have this PR outstanding. Was there anything else I need to do to get it closed out? |
|
|
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #119
Functional Changes
consul-iam-policiesmodule now includes permissions for reading / writing SSM parameters when a new variable namedacl_store_typeis set to "ssm".consul-clustermodule now includes an input variable namedacl_store_type, which is used to indicate where created ACL tokens should be stored. Currently this only supports the value "ssm", but could support other methods in the future. This value is only used to pass on to theconsul-iam-policiesmodule.bash-commonslibrary, which is used in cases where ACLs are enabled.run-consulscript now accepts a switch parameter--enable-acl, which if set will cause the script to generate a root ACL token upon cluster provision, and store it as a specially named SSM parameter. The bootstrap command is executed on only one of the server instances; other server instances and client instances will wait for the SSM parameter to become available, then use it to create their own agent tokens with minimal rights. (Note: while this currently only supports SSM, it was written internally in a way that future expansion to other stores would be possible.)run-consulscript now accepts an argument--acl-storage-type, which is required when the--enable-aclflag is set. This allows the user to select how the bootstrap process stores generated ACL tokens. Currently the only supported value is 'ssm'.Testing / Refactoring related changes
consul-commons.shfile so as to make testing them independent of running therun-consulscript easier.Known Issues
Test results: https://gist.github.com/yardbirdsax/e08efec4799c7e286ce093b5dfee89c3