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

27 Apr 2020


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

-- 
Andrea Bolognani / Red Hat / Virtualization

    