[libvirt-ci PATCH 04/13] lcitool: Use a temporary JSON file to pass extra variables

[Andrea Bolognani]

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 at redhat.com>

-- 
Andrea Bolognani / Red Hat / Virtualization