Fix missing ansible cli extra_vars (#154)

Author: asalajan

README.md

@@ -42,6 +42,7 @@ It can be used to add a layer of templating (using jinja2) on top of Terraform f
             * [Terraform landscape](#terraform-landscape)
          * [SSH](#ssh)
             * [SSHPass](#sshpass)
+            * [Balabit SCB](#scb)
          * [Play](#play)
          * [Run command](#run-command)
          * [Sync files](#sync-files)

src/ops/cli/config.py

@@ -8,21 +8,22 @@
 # OF ANY KIND, either express or implied. See the License for the specific language
 # governing permissions and limitations under the License.
 
-import collections
 import os
-
 import yaml
 
 from ansible.module_utils.common.collections import ImmutableDict
 from ansible.parsing.dataloader import DataLoader
-from ansible.plugins.loader import PluginLoader
 from ansible.template import Templar
-from ansible.utils.vars import load_extra_vars
+from ansible.utils.vars import combine_vars
 from ansible.vars.manager import VariableManager
 from ops.cli import display
 from ansible import constants as C
 from ansible import context
 import logging
+from ansible.errors import AnsibleOptionsError
+from ansible.module_utils._text import to_text
+from ansible.parsing.splitter import parse_kv
+from collections.abc import MutableMapping
 
 logger = logging.getLogger(__name__)
 
@@ -34,6 +35,38 @@ def get_cluster_name(cluster_config_path):
         '/')[-1].replace('.yaml', '').replace('.yml', '')
 
 
+def load_extra_vars(loader):
+    """
+    Overriding Ansible function using version before slight var loading optimization
+    in order to avoid caching issues https://github.com/ansible/ansible/pull/78835/files
+    """
+
+    extra_vars = {}
+    for extra_vars_opt in context.CLIARGS.get('extra_vars', tuple()):
+        data = None
+        extra_vars_opt = to_text(extra_vars_opt, errors='surrogate_or_strict')
+        if extra_vars_opt is None or not extra_vars_opt:
+            continue
+
+        if extra_vars_opt.startswith(u"@"):
+            # Argument is a YAML file (JSON is a subset of YAML)
+            data = loader.load_from_file(extra_vars_opt[1:])
+        elif extra_vars_opt[0] in [u'/', u'.']:
+            raise AnsibleOptionsError("Please prepend extra_vars filename '%s' with '@'" % extra_vars_opt)
+        elif extra_vars_opt[0] in [u'[', u'{']:
+            # Arguments as YAML
+            data = loader.load(extra_vars_opt)
+        else:
+            # Arguments as Key-value
+            data = parse_kv(extra_vars_opt)
+
+        if isinstance(data, MutableMapping):
+            extra_vars = combine_vars(extra_vars, data)
+        else:
+            raise AnsibleOptionsError("Invalid extra vars data supplied. '%s' could not be made into a dictionary" % extra_vars_opt)
+    return extra_vars
+
+
 class ClusterConfig(object):
     def __init__(self, cluster_config_generator,
                  ops_config, cluster_config_path):
@@ -119,7 +152,6 @@ def get(self):
         context_cliargs['extra_vars'] = tuple(extra_vars)
 
         context.CLIARGS = ImmutableDict(context_cliargs)
-        setattr(load_extra_vars, 'extra_vars', {})
         variable_manager._extra_vars = load_extra_vars(
             loader=data_loader)
 
@@ -159,7 +191,6 @@ def get(self):
         context_cliargs['extra_vars'] = tuple(extra_vars)
 
         context.CLIARGS = ImmutableDict(context_cliargs)
-        setattr(load_extra_vars, 'extra_vars', {})
         variable_manager._extra_vars = load_extra_vars(
             loader=data_loader)
 

src/ops/cli/ssh.py

@@ -286,7 +286,7 @@ def run(self, args, extra_args):
         if args.proxy:
             if scb_enabled:
                 proxy_port = args.local or SshConfigGenerator.generate_ssh_scb_proxy_port(
-                    self.ansible_inventory.generated_path.rstrip("/inventory"),
+                    self.ansible_inventory.generated_path.removesuffix("/inventory"),
                     args.auto_scb_port,
                     scb_proxy_port
                 )

src/ops/inventory/sshconfig.py

@@ -90,7 +90,7 @@ def generate_ssh_scb_config(ssh_config_tpl_path, scb_proxy_port):
         ssh_config_content = ssh_config_template.format(
             scb_proxy_port=scb_proxy_port
         )
-        ssh_config_path = ssh_config_tpl_path.rstrip("_tpl")
+        ssh_config_path = ssh_config_tpl_path.removesuffix("_tpl")
         with open(ssh_config_path, 'w') as f:
             f.write(ssh_config_content)
             os.fchmod(f.fileno(), 0o644)

tests/e2e/fixture/ansible/playbooks/play_module.yaml

@@ -1,8 +1,13 @@
 - hosts: web
   gather_facts: no
+  vars:
+    test_cmd_var: false
+    test_cmd_bool_var: True
   tasks:
     - debug: msg="{{ 'filter_this' | my_filter }}"
     - my_module:
         set_facts:
           the_module_works: yep
-    - debug: var=the_module_works
+    - debug: var=the_module_works
+    - debug: msg="test_cmd_var = {{ test_cmd_var | bool }}"
+    - debug: msg="test_cmd_bool_var = {{ test_cmd_bool_var }}"

tests/e2e/test_playbook.py

@@ -31,20 +31,26 @@ def _app(args):
 def test_loading_of_modules_and_extensions(capsys, app):
     root_dir = current_dir + '/fixture/ansible'
     container = app(['-vv', '--root-dir', root_dir, 'clusters/test.yaml', 'play',
-                       'playbooks/play_module.yaml'])
+                       'playbooks/play_module.yaml', '--', '-e', 'test_cmd_var=true',
+                       '-e', '\'{"test_cmd_bool_var": false}\''])
     command = container.run()
     code = container.execute(command, pass_trough=False)
     out, err = capsys.readouterr()
     display(out, color='gray')
     display(err, color='red')
     assert code is 0
-
     # the filter plugins work
     assert '"msg": "filtered: filter_this"' in out
 
     # custom modules are interpreted
     assert '"the_module_works": "yep"' in out
 
+    # cmd extra_vars override playbook vars
+    assert '"test_cmd_var = True"' in out
+
+    # cmd extra_vars bool var
+    assert '"test_cmd_bool_var = False"' in out
+
     # cluster is present as a variable in the command line
     assert '-e cluster=test' in command['command']