Merging in code from my private module #26
Conversation
There was a problem hiding this comment.
There must be a better way to do those templates than all the if not undef. It makes them completely unreadable, and very hard to review for typos.
You may be able to use the * pattern to pass in parameters to the templates, and if you combine that with the delete_undef_values function, you can really just for-loop the templates to be key value expansions. I have not tried if the * pattern works on templates, and if it does not, it would be a shame.
README.md
Outdated
| @@ -36,6 +36,7 @@ Please see [The official documentation](https://www.vaultproject.io/docs/configu | |||
| * `manage_group`: Whether or not the module should create the group. | |||
| * `bin_dir`: Directory the vault executable will be installed in. | |||
| * `config_dir`: Directory the vault configuration will be kept in. | |||
| * `config_output`: What format to output the configuration in (`hcl` or `json`) (defalut: `json`) | |||
There was a problem hiding this comment.
| * `config_output`: What format to output the configuration in (`hcl` or `json`) (defalut: `json`) | |
| * `config_output`: What format to output the configuration in (`hcl` or `json`) (default: `json`) |
manifests/config.pp
Outdated
| extra_config => $vault::extra_config, | ||
| } | ||
| ) | ||
| } | ||
| file { "${vault::config_dir}/config.json": |
There was a problem hiding this comment.
This file path is not really correct anymore, and furthermore there shouldn't be hcl in a file called .json.
Note that the package-provided service has forced the use of a different file for quite some time:
https://github.com/hashicorp/vault/blob/main/.release/linux/package/usr/lib/systemd/system/vault.service#L23
The location of this file should be another parameter, but if the user sets manage repo, changing this parameter would need to be blocked and forced to this path given by hashicorp.
There was a problem hiding this comment.
I think the path should be correct here. It is /etc/vault.d if the user is installing from a repo or /etc/vault if they aren't. If that isn't correct, what should it be?
templates/vault.service.epp
Outdated
| AmbientCapabilities=CAP_IPC_LOCK | ||
| CapabilityBoundingSet=CAP_SYSLOG CAP_IPC_LOCK | ||
| NoNewPrivileges=yes | ||
| ExecStart=<%= $bin_dir %>vault server -config=<%= $config_dir %>vault.hcl |
There was a problem hiding this comment.
This filename will need to match whatever is actually set by the module
|
I am sorry that I completely disappeared after making this PR. I switched jobs and roles and life has been pretty chaotic. To address the question of using the * parameter, I did originally consider that because it seemed like the simplest way to go, however, that pattern doesn't work in this case since data can be different types. I'm open to changing the templates because having a check for every variable sucks, but I can't think of a better way to do it that will still ensure that all of the types are set correctly. |
* Fixes typo in README * Ensures the config file name is correct * Sets the config file extension to reflect the type of config * Updates the service template to use the correct config file
Pull Request (PR) description
This is a large PR and it addresses several (IMO) shortcomings with the existing module. Because this PR is large and contains opinionated code, I am more than happy to change/remove/split any of the proposed changes.
First, it makes a lot of changes to the main class parameters. It reorders most of the parameters, adds data types for all parameters, and adds documentation in the same order as they are defined. It also adds parameters for every possible configuration option currently available for vault.
Second, it adds the capability to output HCL configuration (via the
config_outputparameter). This is handled with a lot of templates. They include and iterate over other templates.And, finally, it adds the ability to upgrade archive installs through the
versionparameter. The code uses an exec to see if the currently installed version of vault is equal to the version defined. If it isn't the binary is deleted so the archive stanza can install the expected version.This Pull Request (PR) fixes the following issues
Fixes #18