remove unneeded steps in inventory import

Delete some cases that directly loads scripts due
to ansible-inventory group_vars problem (now fixed)

Delete intermediate method that was a go-between the
command and the loader class

Change return type of loader from MemInventory to
a simple python dict

remove backport script and star imports

awx/main/management/commands/inventory_import.py

@@ -15,12 +15,20 @@ import shutil
 # Django
 from django.conf import settings
 from django.core.management.base import BaseCommand, CommandError
-from django.core.exceptions import ImproperlyConfigured
 from django.db import connection, transaction
 from django.utils.encoding import smart_text
 
-# AWX
+# AWX inventory imports
-from awx.main.models import * # noqa
+from awx.main.models.inventory import (
+    Inventory,
+    InventorySource,
+    InventoryUpdate,
+    Host
+)
+from awx.main.utils.mem_inventory import MemInventory, dict_to_mem_data
+
+# other AWX imports
+from awx.main.models.rbac import batch_role_ancestor_rebuilding
 from awx.main.utils import (
     ignore_inventory_computed_fields,
     check_proot_installed,
@@ -28,7 +36,6 @@ from awx.main.utils import (
     build_proot_temp_dir,
     get_licenser
 )
-from awx.main.utils.mem_inventory import MemInventory, dict_to_mem_data
 from awx.main.signals import disable_activity_stream
 from awx.main.constants import STANDARD_INVENTORY_UPDATE_ENV
 
@@ -63,21 +70,14 @@ class AnsibleInventoryLoader(object):
     use the ansible-inventory CLI utility to convert it into in-memory
     representational objects. Example:
         /usr/bin/ansible/ansible-inventory -i hosts --list
-    If it fails to find this, it uses the backported script instead
     '''
 
-    def __init__(self, source, group_filter_re=None, host_filter_re=None, is_custom=False):
+    def __init__(self, source, is_custom=False):
         self.source = source
         self.source_dir = functioning_dir(self.source)
         self.is_custom = is_custom
         self.tmp_private_dir = None
         self.method = 'ansible-inventory'
-        self.group_filter_re = group_filter_re
-        self.host_filter_re = host_filter_re
-
-        self.is_vendored_source = False
-        if self.source_dir == os.path.join(settings.BASE_DIR, 'plugins', 'inventory'):
-            self.is_vendored_source = True
 
     def build_env(self):
         env = dict(os.environ.items())
@@ -95,28 +95,10 @@ class AnsibleInventoryLoader(object):
 
     def get_base_args(self):
         # get ansible-inventory absolute path for running in bubblewrap/proot, in Popen
-        for path in os.environ["PATH"].split(os.pathsep):
+        abs_ansible_inventory = shutil.which('ansible-inventory')
-            potential_path = os.path.join(path.strip('"'), 'ansible-inventory')
+        bargs= [abs_ansible_inventory, '-i', self.source]
-            if os.path.isfile(potential_path) and os.access(potential_path, os.X_OK):
+        logger.debug('Using base command: {}'.format(' '.join(bargs)))
-                logger.debug('Using system install of ansible-inventory CLI: {}'.format(potential_path))
+        return bargs
-                return [potential_path, '-i', self.source]
-
-        # Stopgap solution for group_vars, do not use backported module for official
-        # vendored cloud modules or custom scripts TODO: remove after Ansible 2.3 deprecation
-        if self.is_vendored_source or self.is_custom:
-            self.method = 'inventory script invocation'
-            return [self.source]
-
-        # ansible-inventory was not found, look for backported module TODO: remove after Ansible 2.3 deprecation
-        abs_module_path = os.path.abspath(os.path.join(
-            os.path.dirname(__file__), '..', '..', '..', 'plugins',
-            'ansible_inventory', 'backport.py'))
-        self.method = 'ansible-inventory backport'
-
-        if not os.path.exists(abs_module_path):
-            raise ImproperlyConfigured('Cannot find inventory module')
-        logger.debug('Using backported ansible-inventory module: {}'.format(abs_module_path))
-        return [abs_module_path, '-i', self.source]
 
     def get_proot_args(self, cmd, env):
         cwd = os.getcwd()
@@ -179,80 +161,7 @@ class AnsibleInventoryLoader(object):
         base_args = self.get_base_args()
         logger.info('Reading Ansible inventory source: %s', self.source)
 
-        data = self.command_to_json(base_args + ['--list'])
+        return self.command_to_json(base_args + ['--list'])
-
-        # TODO: remove after we run custom scripts through ansible-inventory
-        if self.is_custom and '_meta' not in data or 'hostvars' not in data['_meta']:
-            # Invoke the executable once for each host name we've built up
-            # to set their variables
-            data.setdefault('_meta', {})
-            data['_meta'].setdefault('hostvars', {})
-            logger.warning('Re-calling script for hostvars individually.')
-            for group_name, group_data in list(data.items()):
-                if group_name == '_meta':
-                    continue
-
-                if isinstance(group_data, dict):
-                    group_host_list = group_data.get('hosts', [])
-                elif isinstance(group_data, list):
-                    group_host_list = group_data
-                else:
-                    logger.warning('Group data for "%s" is not a dict or list',
-                                   group_name)
-                    group_host_list = []
-
-                for hostname in group_host_list:
-                    logger.debug('Obtaining hostvars for %s' % hostname.encode('utf-8'))
-                    hostdata = self.command_to_json(
-                        base_args + ['--host', hostname.encode("utf-8")]
-                    )
-                    if isinstance(hostdata, dict):
-                        data['_meta']['hostvars'][hostname] = hostdata
-                    else:
-                        logger.warning(
-                            'Expected dict of vars for host "%s" when '
-                            'calling with `--host`, got %s instead',
-                            k, str(type(data))
-                        )
-
-        logger.info('Processing JSON output...')
-        inventory = MemInventory(
-            group_filter_re=self.group_filter_re, host_filter_re=self.host_filter_re)
-        inventory = dict_to_mem_data(data, inventory=inventory)
-
-        return inventory
-
-
-def load_inventory_source(source, group_filter_re=None,
-                          host_filter_re=None, exclude_empty_groups=False,
-                          is_custom=False):
-    '''
-    Load inventory from given source directory or file.
-    '''
-    # Sanity check: We sanitize these module names for our API but Ansible proper doesn't follow
-    # good naming conventions
-    source = source.replace('rhv.py', 'ovirt4.py')
-    source = source.replace('satellite6.py', 'foreman.py')
-    source = source.replace('vmware.py', 'vmware_inventory.py')
-    if not os.path.exists(source):
-        raise IOError('Source does not exist: %s' % source)
-    source = os.path.join(os.getcwd(), os.path.dirname(source),
-                          os.path.basename(source))
-    source = os.path.normpath(os.path.abspath(source))
-
-    inventory = AnsibleInventoryLoader(
-        source=source,
-        group_filter_re=group_filter_re,
-        host_filter_re=host_filter_re,
-        is_custom=is_custom).load()
-
-    logger.debug('Finished loading from source: %s', source)
-    # Exclude groups that are completely empty.
-    if exclude_empty_groups:
-        inventory.delete_empty_groups()
-    logger.info('Loaded %d groups, %d hosts', len(inventory.all_group.all_groups),
-                len(inventory.all_group.all_hosts))
-    return inventory.all_group
 
 
 class Command(BaseCommand):
@@ -359,6 +268,19 @@ class Command(BaseCommand):
         else:
             raise NotImplementedError('Value of enabled {} not understood.'.format(enabled))
 
+    def get_source_absolute_path(self, source):
+        # Sanity check: We sanitize these module names for our API but Ansible proper doesn't follow
+        # good naming conventions
+        source = source.replace('rhv.py', 'ovirt4.py')
+        source = source.replace('satellite6.py', 'foreman.py')
+        source = source.replace('vmware.py', 'vmware_inventory.py')
+        if not os.path.exists(source):
+            raise IOError('Source does not exist: %s' % source)
+        source = os.path.join(os.getcwd(), os.path.dirname(source),
+                              os.path.basename(source))
+        source = os.path.normpath(os.path.abspath(source))
+        return source
+
     def load_inventory_from_database(self):
         '''
         Load inventory and related objects from the database.
@@ -988,12 +910,26 @@ class Command(BaseCommand):
                         self.inventory_update.status = 'running'
                         self.inventory_update.save()
 
-            # Load inventory from source.
+            source = self.get_source_absolute_path(self.source)
-            self.all_group = load_inventory_source(self.source,
+
-                                                   self.group_filter_re,
+            data = AnsibleInventoryLoader(source=source, is_custom=self.is_custom).load()
-                                                   self.host_filter_re,
+
-                                                   self.exclude_empty_groups,
+            logger.debug('Finished loading from source: %s', source)
-                                                   self.is_custom)
+            logger.info('Processing JSON output...')
+            inventory = MemInventory(
+                group_filter_re=self.group_filter_re, host_filter_re=self.host_filter_re)
+            inventory = dict_to_mem_data(data, inventory=inventory)
+
+            del data  # forget dict from import, could be large
+
+            logger.info('Loaded %d groups, %d hosts', len(inventory.all_group.all_groups),
+                        len(inventory.all_group.all_hosts))
+
+            if self.exclude_empty_groups:
+                inventory.delete_empty_groups()
+
+            self.all_group = inventory.all_group
+
             if settings.DEBUG:
                 # depending on inventory source, this output can be
                 # *exceedingly* verbose - crawling a deeply nested

awx/main/tests/functional/commands/test_inventory_import.py

@@ -11,7 +11,6 @@ from django.core.management.base import CommandError
 # AWX
 from awx.main.management.commands import inventory_import
 from awx.main.models import Inventory, Host, Group
-from awx.main.utils.mem_inventory import dict_to_mem_data
 
 
 TEST_INVENTORY_CONTENT = {
@@ -73,7 +72,13 @@ TEST_INVENTORY_CONTENT = {
 }
 
 
-TEST_MEM_OBJECTS = dict_to_mem_data(TEST_INVENTORY_CONTENT)
+class MockLoader:
+
+    def __init__(self, *args, **kwargs):
+        pass
+
+    def load(self):
+        return self._data
 
 
 def mock_logging(self):
@@ -86,7 +91,6 @@ def mock_logging(self):
 @mock.patch.object(inventory_import.Command, 'set_logging_level', mock_logging)
 class TestInvalidOptionsFunctional:
 
-    @mock.patch.object(inventory_import.InstanceGroup.objects, 'get', new=mock.MagicMock(return_value=None))
     def test_invalid_options_invalid_source(self, inventory):
         # Give invalid file to the command
         cmd = inventory_import.Command()
@@ -114,13 +118,13 @@ class TestInvalidOptionsFunctional:
 
 @pytest.mark.django_db
 @pytest.mark.inventory_import
-@mock.patch.object(inventory_import.InstanceGroup.objects, 'get', new=mock.MagicMock(return_value=None))
 @mock.patch.object(inventory_import.Command, 'check_license', new=mock.MagicMock())
 @mock.patch.object(inventory_import.Command, 'set_logging_level', new=mock_logging)
 class TestINIImports:
 
-    @mock.patch.object(inventory_import.AnsibleInventoryLoader, 'load', mock.MagicMock(return_value=TEST_MEM_OBJECTS))
+    @mock.patch.object(inventory_import, 'AnsibleInventoryLoader', MockLoader)
     def test_inventory_single_ini_import(self, inventory, capsys):
+        inventory_import.AnsibleInventoryLoader._data = TEST_INVENTORY_CONTENT
         cmd = inventory_import.Command()
         r = cmd.handle(
             inventory_id=inventory.pk, source=__file__,
@@ -174,52 +178,44 @@ class TestINIImports:
         assert reloaded_inv.inventory_sources.count() == 1
         assert reloaded_inv.inventory_sources.all()[0].source == 'file'
 
-    @mock.patch.object(
+    @mock.patch.object(inventory_import, 'AnsibleInventoryLoader', MockLoader)
-        inventory_import, 'load_inventory_source', mock.MagicMock(
-            return_value=dict_to_mem_data(
-                {
-                    "_meta": {
-                        "hostvars": {"foo": {"some_hostvar": "foobar"}}
-                    },
-                    "all": {
-                        "children": ["ungrouped"]
-                    },
-                    "ungrouped": {
-                        "hosts": ["foo"]
-                    }
-                }).all_group
-        )
-    )
     def test_hostvars_are_saved(self, inventory):
+        inventory_import.AnsibleInventoryLoader._data = {
+            "_meta": {
+                "hostvars": {"foo": {"some_hostvar": "foobar"}}
+            },
+            "all": {
+                "children": ["ungrouped"]
+            },
+            "ungrouped": {
+                "hosts": ["foo"]
+            }
+        }
         cmd = inventory_import.Command()
-        cmd.handle(inventory_id=inventory.pk, source='doesnt matter')
+        cmd.handle(inventory_id=inventory.pk, source=__file__)
         assert inventory.hosts.count() == 1
         h = inventory.hosts.all()[0]
         assert h.name == 'foo'
         assert h.variables_dict == {"some_hostvar": "foobar"}
 
-    @mock.patch.object(
+    @mock.patch.object(inventory_import, 'AnsibleInventoryLoader', MockLoader)
-        inventory_import, 'load_inventory_source', mock.MagicMock(
-            return_value=dict_to_mem_data(
-                {
-                    "_meta": {
-                        "hostvars": {}
-                    },
-                    "all": {
-                        "children": ["fooland", "barland"]
-                    },
-                    "fooland": {
-                        "children": ["barland"]
-                    },
-                    "barland": {
-                        "children": ["fooland"]
-                    }
-                }).all_group
-        )
-    )
     def test_recursive_group_error(self, inventory):
+        inventory_import.AnsibleInventoryLoader._data = {
+            "_meta": {
+                "hostvars": {}
+            },
+            "all": {
+                "children": ["fooland", "barland"]
+            },
+            "fooland": {
+                "children": ["barland"]
+            },
+            "barland": {
+                "children": ["fooland"]
+            }
+        }
         cmd = inventory_import.Command()
-        cmd.handle(inventory_id=inventory.pk, source='doesnt matter')
+        cmd.handle(inventory_id=inventory.pk, source=__file__)
 
 
 @pytest.mark.django_db

awx/plugins/ansible_inventory/backport.py

@@ -1,326 +0,0 @@
-#!/usr/bin/env python
-
-# (c) 2017, Brian Coca <bcoca@ansible.com>
-#
-# Ansible is free software: you can redistribute it and/or modify
-# it under the terms of the GNU General Public License as published by
-# the Free Software Foundation, either version 3 of the License, or
-# (at your option) any later version.
-#
-# Ansible is distributed in the hope that it will be useful,
-# but WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-# GNU General Public License for more details.
-#
-# You should have received a copy of the GNU General Public License
-# along with Ansible.  If not, see <http://www.gnu.org/licenses/>.
-#
-
-from __future__ import (absolute_import, division, print_function)
-__metaclass__ = type
-
-import distutils.spawn
-import optparse
-from operator import attrgetter
-
-from ansible.cli import CLI
-from ansible.errors import AnsibleOptionsError
-from ansible.parsing.dataloader import DataLoader
-
-try:
-    from __main__ import display
-except ImportError:
-    from ansible.utils.display import Display
-    display = Display()
-
-INTERNAL_VARS = frozenset([ 'ansible_facts',
-                            'ansible_version',
-                            'ansible_playbook_python',
-                            'inventory_dir',
-                            'inventory_file',
-                            'inventory_hostname',
-                            'inventory_hostname_short',
-                            'groups',
-                            'group_names',
-                            'omit',
-                            'playbook_dir',
-                            ])
-
-
-class InventoryCLI(CLI):
-    ''' used to display or dump the configured inventory as Ansible sees it '''
-
-    ARGUMENTS = { 'host': 'The name of a host to match in the inventory, relevant when using --list',
-                  'group': 'The name of a group in the inventory, relevant when using --graph',
-                  }
-
-    def __init__(self, args):
-
-        super(InventoryCLI, self).__init__(args)
-        self.args = args
-        self.vm = None
-        self.loader = None
-        self.inventory = None
-
-        self._new_api = True
-
-    def parse(self):
-
-        self.parser = CLI.base_parser(
-            usage='usage: %prog [options] [host|group]',
-            epilog='Show Ansible inventory information, by default it uses the inventory script JSON format',
-            inventory_opts=True,
-            vault_opts=True
-        )
-        self.parser.add_option("--optimize", action="store_true", default=False, dest='optimize',
-                               help='Output variables on the group or host where they are defined')
-
-        # Actions
-        action_group = optparse.OptionGroup(self.parser, "Actions", "One of following must be used on invocation, ONLY ONE!")
-        action_group.add_option("--list", action="store_true", default=False, dest='list', help='Output all hosts info, works as inventory script')
-        action_group.add_option("--host", action="store", default=None, dest='host', help='Output specific host info, works as inventory script')
-        action_group.add_option("--graph", action="store_true", default=False, dest='graph',
-                                help='create inventory graph, if supplying pattern it must be a valid group name')
-        self.parser.add_option_group(action_group)
-
-        # Options
-        self.parser.add_option("-y", "--yaml", action="store_true", default=False, dest='yaml',
-                               help='Use YAML format instead of default JSON, ignored for --graph')
-        self.parser.add_option("--vars", action="store_true", default=False, dest='show_vars',
-                               help='Add vars to graph display, ignored unless used with --graph')
-
-        try:
-            super(InventoryCLI, self).parse()
-        except Exception as e:
-            if 'Need to implement!' not in e.args[0]:
-                raise
-            # --- Start of 2.3+ super(InventoryCLI, self).parse() ---
-            self.options, self.args = self.parser.parse_args(self.args[1:])
-            # --- End of 2.3+ super(InventoryCLI, self).parse() ---
-
-        display.verbosity = self.options.verbosity
-
-        self.validate_conflicts(vault_opts=True)
-
-        # there can be only one! and, at least, one!
-        used = 0
-        for opt in (self.options.list, self.options.host, self.options.graph):
-            if opt:
-                used += 1
-        if used == 0:
-            raise AnsibleOptionsError("No action selected, at least one of --host, --graph or --list needs to be specified.")
-        elif used > 1:
-            raise AnsibleOptionsError("Conflicting options used, only one of --host, --graph or --list can be used at the same time.")
-
-        # set host pattern to default if not supplied
-        if len(self.args) > 0:
-            self.options.pattern = self.args[0]
-        else:
-            self.options.pattern = 'all'
-
-    def run(self):
-
-        results = None
-
-        super(InventoryCLI, self).run()
-
-        # Initialize needed objects
-        if getattr(self, '_play_prereqs', False):
-            self.loader, self.inventory, self.vm = self._play_prereqs(self.options)
-        else:
-            # fallback to pre 2.4 way of initialzing
-            from ansible.vars import VariableManager
-            from ansible.inventory import Inventory
-
-            self._new_api = False
-            self.loader = DataLoader()
-            self.vm = VariableManager()
-
-            # use vault if needed
-            if self.options.vault_password_file:
-                vault_pass = CLI.read_vault_password_file(self.options.vault_password_file, loader=self.loader)
-            elif self.options.ask_vault_pass:
-                vault_pass = self.ask_vault_passwords()
-            else:
-                vault_pass = None
-
-            if vault_pass:
-                self.loader.set_vault_password(vault_pass)
-                # actually get inventory and vars
-
-            self.inventory = Inventory(loader=self.loader, variable_manager=self.vm, host_list=self.options.inventory)
-            self.vm.set_inventory(self.inventory)
-
-        if self.options.host:
-            hosts = self.inventory.get_hosts(self.options.host)
-            if len(hosts) != 1:
-                raise AnsibleOptionsError("You must pass a single valid host to --hosts parameter")
-
-            myvars = self._get_host_variables(host=hosts[0])
-            self._remove_internal(myvars)
-
-            # FIXME: should we template first?
-            results = self.dump(myvars)
-
-        elif self.options.graph:
-            results = self.inventory_graph()
-        elif self.options.list:
-            top = self._get_group('all')
-            if self.options.yaml:
-                results = self.yaml_inventory(top)
-            else:
-                results = self.json_inventory(top)
-            results = self.dump(results)
-
-        if results:
-            # FIXME: pager?
-            display.display(results)
-            exit(0)
-
-        exit(1)
-
-    def dump(self, stuff):
-
-        if self.options.yaml:
-            import yaml
-            from ansible.parsing.yaml.dumper import AnsibleDumper
-            results = yaml.dump(stuff, Dumper=AnsibleDumper, default_flow_style=False)
-        else:
-            import json
-            results = json.dumps(stuff, sort_keys=True, indent=4)
-
-        return results
-
-    def _get_host_variables(self, host):
-        if self._new_api:
-            hostvars = self.vm.get_vars(host=host)
-        else:
-            hostvars = self.vm.get_vars(self.loader, host=host)
-        return hostvars
-
-    def _get_group(self, gname):
-        if self._new_api:
-            group = self.inventory.groups.get(gname)
-        else:
-            group = self.inventory.get_group(gname)
-        return group
-
-    def _remove_internal(self, dump):
-
-        for internal in INTERNAL_VARS:
-            if internal in dump:
-                del dump[internal]
-
-    def _remove_empty(self, dump):
-        # remove empty keys
-        for x in ('hosts', 'vars', 'children'):
-            if x in dump and not dump[x]:
-                del dump[x]
-
-    def _show_vars(self, dump, depth):
-        result = []
-        self._remove_internal(dump)
-        if self.options.show_vars:
-            for (name, val) in sorted(dump.items()):
-                result.append(self._graph_name('{%s = %s}' % (name, val), depth + 1))
-        return result
-
-    def _graph_name(self, name, depth=0):
-        if depth:
-            name = "  |" * (depth) + "--%s" % name
-        return name
-
-    def _graph_group(self, group, depth=0):
-
-        result = [self._graph_name('@%s:' % group.name, depth)]
-        depth = depth + 1
-        for kid in sorted(group.child_groups, key=attrgetter('name')):
-            result.extend(self._graph_group(kid, depth))
-
-        if group.name != 'all':
-            for host in sorted(group.hosts, key=attrgetter('name')):
-                result.append(self._graph_name(host.name, depth))
-                result.extend(self._show_vars(host.get_vars(), depth))
-
-        result.extend(self._show_vars(group.get_vars(), depth))
-
-        return result
-
-    def inventory_graph(self):
-
-        start_at = self._get_group(self.options.pattern)
-        if start_at:
-            return '\n'.join(self._graph_group(start_at))
-        else:
-            raise AnsibleOptionsError("Pattern must be valid group name when using --graph")
-
-    def json_inventory(self, top):
-
-        def format_group(group):
-            results = {}
-            results[group.name] = {}
-            if group.name != 'all':
-                results[group.name]['hosts'] = [h.name for h in sorted(group.hosts, key=attrgetter('name'))]
-            results[group.name]['vars'] = group.get_vars()
-            results[group.name]['children'] = []
-            for subgroup in sorted(group.child_groups, key=attrgetter('name')):
-                results[group.name]['children'].append(subgroup.name)
-                results.update(format_group(subgroup))
-
-            self._remove_empty(results[group.name])
-            return results
-
-        results = format_group(top)
-
-        # populate meta
-        results['_meta'] = {'hostvars': {}}
-        hosts = self.inventory.get_hosts()
-        for host in hosts:
-            results['_meta']['hostvars'][host.name] = self._get_host_variables(host=host)
-            self._remove_internal(results['_meta']['hostvars'][host.name])
-
-        return results
-
-    def yaml_inventory(self, top):
-
-        seen = []
-
-        def format_group(group):
-            results = {}
-
-            # initialize group + vars
-            results[group.name] = {}
-            results[group.name]['vars'] = group.get_vars()
-
-            # subgroups
-            results[group.name]['children'] = {}
-            for subgroup in sorted(group.child_groups, key=attrgetter('name')):
-                if subgroup.name != 'all':
-                    results[group.name]['children'].update(format_group(subgroup))
-
-            # hosts for group
-            results[group.name]['hosts'] = {}
-            if group.name != 'all':
-                for h in sorted(group.hosts, key=attrgetter('name')):
-                    myvars = {}
-                    if h.name not in seen:  # avoid defining host vars more than once
-                        seen.append(h.name)
-                        myvars = self._get_host_variables(host=h)
-                        self._remove_internal(myvars)
-                    results[group.name]['hosts'][h.name] = myvars
-
-            self._remove_empty(results[group.name])
-            return results
-
-        return format_group(top)
-
-
-if __name__ == '__main__':
-    import imp
-    import sys
-    with open(__file__) as f:
-        imp.load_source('ansible.cli.inventory', __file__ + '.py', f)
-    ansible_path = distutils.spawn.find_executable('ansible')
-    sys.argv[0] = 'ansible-inventory'
-    with open(ansible_path) as in_file:
-        exec(in_file.read())