ansible-inventory prevent duplicating host entries (#80059)

this happened after implementing limits we introduced a bug that a host would be duplicated if it existed in a group's children

changelogs/fragments/ainv_limit_fix.yml

@@ -0,0 +1,2 @@
+bugfixes:
+  - ansible-inventory will no longer duplicate host entries if they were part of a group's childrens tree.

lib/ansible/cli/inventory.py

@@ -301,19 +301,19 @@ class InventoryCLI(CLI):
 
     def json_inventory(self, top):
 
-        seen = set()
+        seen_groups = set()
 
-        def format_group(group):
+        def format_group(group, available_hosts):
             results = {}
             results[group.name] = {}
             if group.name != 'all':
-                results[group.name]['hosts'] = [h.name for h in self.inventory.get_hosts(group.name)]
+                results[group.name]['hosts'] = [h.name for h in group.hosts if h.name in available_hosts]
             results[group.name]['children'] = []
             for subgroup in group.child_groups:
                 results[group.name]['children'].append(subgroup.name)
-                if subgroup.name not in seen:
-                    results.update(format_group(subgroup))
-                    seen.add(subgroup.name)
+                if subgroup.name not in seen_groups:
+                    results.update(format_group(subgroup, available_hosts))
+                    seen_groups.add(subgroup.name)
             if context.CLIARGS['export']:
                 results[group.name]['vars'] = self._get_group_variables(group)
 
@@ -324,11 +324,11 @@ class InventoryCLI(CLI):
 
             return results
 
-        results = format_group(top)
+        hosts = self.inventory.get_hosts(top.name)
+        results = format_group(top, [h.name for h in hosts])
 
         # populate meta
         results['_meta'] = {'hostvars': {}}
-        hosts = self.inventory.get_hosts()
         for host in hosts:
             hvars = self._get_host_variables(host)
             if hvars:
@@ -338,9 +338,10 @@ class InventoryCLI(CLI):
 
     def yaml_inventory(self, top):
 
-        seen = []
+        seen_hosts = set()
+        seen_groups = set()
 
-        def format_group(group):
+        def format_group(group, available_hosts):
             results = {}
 
             # initialize group + vars
@@ -350,15 +351,21 @@ class InventoryCLI(CLI):
             results[group.name]['children'] = {}
             for subgroup in group.child_groups:
                 if subgroup.name != 'all':
-                    results[group.name]['children'].update(format_group(subgroup))
+                    if subgroup.name in seen_groups:
+                        results[group.name]['children'].update({subgroup.name: {}})
+                    else:
+                        results[group.name]['children'].update(format_group(subgroup, available_hosts))
+                        seen_groups.add(subgroup.name)
 
             # hosts for group
             results[group.name]['hosts'] = {}
             if group.name != 'all':
-                for h in self.inventory.get_hosts(group.name):
+                for h in group.hosts:
+                    if h.name not in available_hosts:
+                        continue  # observe limit
                     myvars = {}
-                    if h.name not in seen:  # avoid defining host vars more than once
-                        seen.append(h.name)
+                    if h.name not in seen_hosts:  # avoid defining host vars more than once
+                        seen_hosts.add(h.name)
                         myvars = self._get_host_variables(host=h)
                     results[group.name]['hosts'][h.name] = myvars
 
@@ -374,13 +381,15 @@ class InventoryCLI(CLI):
 
             return results
 
-        return format_group(top)
+        available_hosts = [h.name for h in self.inventory.get_hosts(top.name)]
+        return format_group(top, available_hosts)
 
     def toml_inventory(self, top):
-        seen = set()
+        seen_hosts = set()
+        seen_hosts = set()
         has_ungrouped = bool(next(g.hosts for g in top.child_groups if g.name == 'ungrouped'))
 
-        def format_group(group):
+        def format_group(group, available_hosts):
             results = {}
             results[group.name] = {}
 
@@ -390,12 +399,14 @@ class InventoryCLI(CLI):
                     continue
                 if group.name != 'all':
                     results[group.name]['children'].append(subgroup.name)
-                results.update(format_group(subgroup))
+                results.update(format_group(subgroup, available_hosts))
 
             if group.name != 'all':
-                for host in self.inventory.get_hosts(group.name):
-                    if host.name not in seen:
-                        seen.add(host.name)
+                for host in group.hosts:
+                    if host.name not in available_hosts:
+                        continue
+                    if host.name not in seen_hosts:
+                        seen_hosts.add(host.name)
                         host_vars = self._get_host_variables(host=host)
                     else:
                         host_vars = {}
@@ -414,7 +425,8 @@ class InventoryCLI(CLI):
 
             return results
 
-        results = format_group(top)
+        available_hosts = [h.name for h in self.inventory.get_hosts(top.name)]
+        results = format_group(top, available_hosts)
 
         return results
 

test/integration/targets/ansible-inventory/files/complex.ini

@@ -0,0 +1,35 @@
+ihavenogroup
+
+[all]
+hostinall
+
+[all:vars]
+ansible_connection=local
+
+[test_group1]
+test1 myvar=something
+test2 myvar=something2
+test3
+
+[test_group2]
+test1
+test4
+test5
+
+[test_group3]
+test2 othervar=stuff
+test3
+test6
+
+[parent_1:children]
+test_group1
+
+[parent_2:children]
+test_group1
+
+[parent_3:children]
+test_group2
+test_group3
+
+[parent_3]
+test2

test/integration/targets/ansible-inventory/filter_plugins/toml.py

@@ -0,0 +1,50 @@
+# (c) 2017, Matt Martz <matt@sivel.net>
+# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
+
+# Make coding more python3-ish
+from __future__ import (absolute_import, division, print_function)
+__metaclass__ = type
+
+import functools
+
+from ansible.plugins.inventory.toml import HAS_TOML, toml_dumps
+try:
+    from ansible.plugins.inventory.toml import toml
+except ImportError:
+    pass
+
+from ansible.errors import AnsibleFilterError
+from ansible.module_utils._text import to_text
+from ansible.module_utils.common._collections_compat import MutableMapping
+from ansible.module_utils.six import string_types
+
+
+def _check_toml(func):
+    @functools.wraps(func)
+    def inner(o):
+        if not HAS_TOML:
+            raise AnsibleFilterError('The %s filter plugin requires the python "toml" library' % func.__name__)
+        return func(o)
+    return inner
+
+
+@_check_toml
+def from_toml(o):
+    if not isinstance(o, string_types):
+        raise AnsibleFilterError('from_toml requires a string, got %s' % type(o))
+    return toml.loads(to_text(o, errors='surrogate_or_strict'))
+
+
+@_check_toml
+def to_toml(o):
+    if not isinstance(o, MutableMapping):
+        raise AnsibleFilterError('to_toml requires a dict, got %s' % type(o))
+    return to_text(toml_dumps(o), errors='surrogate_or_strict')
+
+
+class FilterModule(object):
+    def filters(self):
+        return {
+            'to_toml': to_toml,
+            'from_toml': from_toml
+        }

test/integration/targets/ansible-inventory/tasks/json_output.yml

@@ -0,0 +1,33 @@
+- block:
+  - name: check baseline
+    command: ansible-inventory -i '{{ role_path }}/files/valid_sample.yml' --list
+    register: limited
+
+  - name: ensure non empty host list
+    assert:
+      that:
+        - "'something' in inv['_meta']['hostvars']"
+
+  - name: check that limit removes host
+    command: ansible-inventory -i '{{ role_path }}/files/valid_sample.yml'  --limit '!something' --list
+    register: limited
+
+  - name: ensure empty host list
+    assert:
+      that:
+        - "'something' not in inv['_meta']['hostvars']"
+
+  - name: check dupes
+    command: ansible-inventory -i '{{ role_path }}/files/complex.ini' --list
+    register: limited
+
+  - name: ensure host only appears on directly assigned
+    assert:
+      that:
+        - "'hosts' not in inv['parent_1']"
+        - "'hosts' not in inv['parent_2']"
+        - "'hosts' in inv['parent_3']"
+        - "'test1' in inv['test_group1']['hosts']"
+  vars:
+    inv: '{{limited.stdout|from_json }}'
+  delegate_to: localhost

test/integration/targets/ansible-inventory/tasks/main.yml

@@ -146,24 +146,9 @@
     loop_var: toml_package
   when: toml_package is not contains 'tomllib' or (toml_package is contains 'tomllib' and ansible_facts.python.version_info >= [3, 11])
 
-- name: check baseline
-  command: ansible-inventory -i '{{ role_path }}/files/valid_sample.yml' --list
-  register: limited
 
-- name: ensure empty host list
-  assert:
-    that:
-      - "'something' in inv['_meta']['hostvars']"
-  vars:
-    inv: '{{ limited.stdout|from_json}}'
-
-- name: check that limit removes host
-  command: ansible-inventory -i '{{ role_path }}/files/valid_sample.yml'  --limit '!something' --list
-  register: limited
-
-- name: ensure empty host list
-  assert:
-    that:
-      - "'something' not in inv['_meta']['hostvars']"
-  vars:
-    inv: '{{ limited.stdout|from_json}}'
+- include_tasks: "{{item}}_output.yml"
+  loop:
+    - json
+    - yaml
+    - toml

test/integration/targets/ansible-inventory/tasks/toml_output.yml

@@ -0,0 +1,43 @@
+- name: only test if have toml in python
+  command: "{{ansible_playbook_python}} -c 'import toml'"
+  ignore_errors: true
+  delegate_to: localhost
+  register: has_toml
+
+- block:
+  - name: check baseline
+    command: ansible-inventory -i '{{ role_path }}/files/valid_sample.yml' --list --toml
+    register: limited
+
+  - name: ensure non empty host list
+    assert:
+      that:
+        - "'something' in inv['somegroup']['hosts']"
+
+  - name: check that limit removes host
+    command: ansible-inventory -i '{{ role_path }}/files/valid_sample.yml'  --limit '!something' --list --toml
+    register: limited
+    ignore_errors: true
+
+  - name: ensure empty host list
+    assert:
+      that:
+        - limited is failed
+
+  - name: check dupes
+    command: ansible-inventory -i '{{ role_path }}/files/complex.ini' --list --toml
+    register: limited
+
+  - debug: var=inv
+
+  - name: ensure host only appears on directly assigned
+    assert:
+      that:
+        - "'hosts' not in inv['parent_1']"
+        - "'hosts' not in inv['parent_2']"
+        - "'hosts' in inv['parent_3']"
+        - "'test1' in inv['test_group1']['hosts']"
+  vars:
+    inv: '{{limited.stdout|from_toml}}'
+  when: has_toml is success
+  delegate_to: localhost

test/integration/targets/ansible-inventory/tasks/yaml_output.yml

@@ -0,0 +1,34 @@
+- block:
+  - name: check baseline
+    command: ansible-inventory -i '{{ role_path }}/files/valid_sample.yml' --list --yaml
+    register: limited
+
+  - name: ensure something in host list
+    assert:
+      that:
+        - "'something' in inv['all']['children']['somegroup']['hosts']"
+
+  - name: check that limit removes host
+    command: ansible-inventory -i '{{ role_path }}/files/valid_sample.yml'  --limit '!something' --list --yaml
+    register: limited
+
+  - name: ensure empty host list
+    assert:
+      that:
+        - not inv
+
+  - name: check dupes
+    command: ansible-inventory -i '{{ role_path }}/files/complex.ini' --list --yaml
+    register: limited
+
+  - name: ensure host only appears on directly assigned
+    assert:
+      that:
+        - "'hosts' not in inv['all']['children']['parent_1']"
+        - "'hosts' not in inv['all']['children']['parent_2']"
+        - "'hosts' in inv['all']['children']['parent_3']"
+        - "'test1' in inv['all']['children']['parent_1']['children']['test_group1']['hosts']"
+        - "'hosts' not in inv['all']['children']['parent_2']['children']['test_group1']"
+  vars:
+    inv: '{{limited.stdout|from_yaml}}'
+  delegate_to: localhost