On Wed, 2020-04-22 at 15:28 +0200, Erik Skultety wrote:
> This patch is a pre-requisite config file consolidation. Currently we've
> got a number of files which serve as a configuration either to the
> lcitool itself or to the ansible playbooks (majority). Once we replace
> these with a single global lcitool config, we'd end up passing tokens
> (potentially some passwords) as ansible extra variables bare naked on
> the cmdline. In order to prevent this security flaw use temporary JSON
> file holding all these extra variables and pass it as follows:
>
> $ ansible-playbook --extra-vars @extra_vars.json playbook.yml
I find it impossible not to point out that, if the configuration
file was in YAML format, then we could pass its contents to Ansible
without having to create a temporary file *or* risk exposing
sensitive data O:-)
> @@ -504,21 +504,26 @@ class Application:
> git_remote = "default"
> git_branch = "master"
>
> + tempdir = tempfile.TemporaryDirectory(prefix='lcitool')
If you want to pass prefix, do the same thing for the call introduced
in the previous commit.
Also, double quotes around strings please.
> ansible_cfg_path = os.path.join(base, "ansible.cfg")
> playbook_base = os.path.join(base, "playbooks", playbook)
> playbook_path = os.path.join(playbook_base, "main.yml")
> + extra_vars_path = os.path.join(tempdir.name, 'extra_vars.json')
Double quotes.
>
> - extra_vars = json.dumps({
> - "base": base,
> - "playbook_base": playbook_base,
> - "root_password_file": root_pass_file,
> - "flavor": flavor,
> - "selected_projects": selected_projects,
> - "git_remote": git_remote,
> - "git_branch": git_branch,
> - "gitlab_url_file": gitlab_url_file,
> - "gitlab_runner_token_file": gitlab_runner_token_file,
> - })
> + with open(extra_vars_path, 'w') as fp:
Double quotes.
> + extra_vars = {
> + "base": base,
> + "playbook_base": playbook_base,
> + "root_password_file": root_pass_file,
> + "flavor": flavor,
> + "selected_projects": selected_projects,
> + "git_remote": git_remote,
> + "git_branch": git_branch,
> + "gitlab_url_file": gitlab_url_file,
> + "gitlab_runner_token_file": gitlab_runner_token_file,
> + }
> + json.dump(extra_vars, fp)
Moving the definition of the dictionary is not needed: just do
extra_vars = {
...
}
with open(...) as fp:
json.dumps(extra_vars, fp)
which keeps the with scope nice and small.
With these few nits fixed,
Reviewed-by: Andrea Bolognani <abologna@xxxxxxxxxx>
--
Andrea Bolognani / Red Hat / Virtualization
