Fix constructable inventory default keyed group (#85002)

Fix the logic for a constructable inventory `keyed_groups` entry to use
the `default_value` if the ``key` expression result is `None` or `omit`
and not just an empty string. This bug was introduced with the changes
in data tagging and goes back to the original behaviour.

Co-authored-by: Sloane Hertel <19572925+s-hertel@users.noreply.github.com>

changelogs/fragments/constructed-default-value.yml

@@ -0,0 +1,4 @@
+bugfixes:
+  - >-
+    constructed inventory - Use the ``default_value`` or ``trailing_separator`` in a ``keyed_groups`` entry if the expression result of ``key``
+    is ``None`` and not just an empty string.

lib/ansible/plugins/doc_fragments/constructed.py

@@ -47,13 +47,13 @@ options:
         - The key from input dictionary used to generate groups.
       default_value:
         description:
-        - The default value when the host variable's value is an empty string.
+        - The default value when the host variable's value is V(None) or an empty string.
         - This option is mutually exclusive with O(keyed_groups[].trailing_separator).
         type: str
         version_added: '2.12'
       trailing_separator:
         description:
-        - Set this option to V(false) to omit the O(keyed_groups[].separator) after the host variable when the value is an empty string.
+        - Set this option to V(false) to omit the O(keyed_groups[].separator) after the host variable when the value is V(None) or an empty string.
         - This option is mutually exclusive with O(keyed_groups[].default_value).
         type: bool
         default: true

lib/ansible/plugins/inventory/__init__.py

@@ -401,6 +401,8 @@ class Constructable(_BaseInventoryPlugin):
 
     def _add_host_to_keyed_groups(self, keys, variables, host, strict=False, fetch_hostvars=True):
         """ helper to create groups for plugins based on variable values and add the corresponding hosts to it"""
+        should_default_value = (None, '')
+
         if keys and isinstance(keys, list):
             for keyed in keys:
                 if keyed and isinstance(keyed, dict):
@@ -417,7 +419,9 @@ class Constructable(_BaseInventoryPlugin):
                     trailing_separator = keyed.get('trailing_separator')
                     if trailing_separator is not None and default_value_name is not None:
                         raise AnsibleParserError("parameters are mutually exclusive for keyed groups: default_value|trailing_separator")
-                    if key or (key == '' and default_value_name is not None):
+
+                    use_default = key in should_default_value and default_value_name is not None
+                    if key or use_default:
                         prefix = keyed.get('prefix', '')
                         sep = keyed.get('separator', '_')
                         raw_parent_name = keyed.get('parent_group', None)
@@ -433,23 +437,21 @@ class Constructable(_BaseInventoryPlugin):
                             continue
 
                         new_raw_group_names = []
-                        if isinstance(key, string_types):
+                        if use_default:
-                            # if key is empty, 'default_value' will be used as group name
+                            new_raw_group_names.append(default_value_name)
-                            if key == '' and default_value_name is not None:
+                        elif isinstance(key, string_types):
-                                new_raw_group_names.append(default_value_name)
+                            new_raw_group_names.append(key)
-                            else:
-                                new_raw_group_names.append(key)
                         elif isinstance(key, list):
                             for name in key:
                                 # if list item is empty, 'default_value' will be used as group name
-                                if name == '' and default_value_name is not None:
+                                if name in should_default_value and default_value_name is not None:
                                     new_raw_group_names.append(default_value_name)
                                 else:
                                     new_raw_group_names.append(name)
                         elif isinstance(key, Mapping):
                             for (gname, gval) in key.items():
                                 bare_name = '%s%s%s' % (gname, sep, gval)
-                                if gval == '':
+                                if gval in should_default_value:
                                     # key's value is empty
                                     if default_value_name is not None:
                                         bare_name = '%s%s%s' % (gname, sep, default_value_name)

test/integration/targets/inventory_constructed/keyed_group_default_value.yml

@@ -3,3 +3,7 @@ keyed_groups:
   - key: tags
     prefix: tag
     default_value: "running"
+
+  - key: tags
+    prefix: without_trailing
+    trailing_separator: false

test/integration/targets/inventory_constructed/keyed_group_list_default_value.yml

@@ -3,3 +3,7 @@ keyed_groups:
   - key: roles
     default_value: storage
     prefix: host
+
+  - key: '[]'
+    default_value: default_value
+    prefix: empty_list_test

test/integration/targets/inventory_constructed/keyed_group_str_default_value.yml

@@ -3,3 +3,11 @@ keyed_groups:
   - key: os
     default_value: "fedora"
     prefix: host
+
+  - key: invalid_var | default(None)
+    prefix: none_test
+    default_value: default_value
+
+  - key: '""'
+    prefix: empty_test
+    default_value: default_value

test/integration/targets/inventory_constructed/runme.sh

@@ -30,6 +30,12 @@ ansible-inventory -i tag_inventory.yml -i keyed_group_default_value.yml --graph
 grep '@tag_name_host0' out.txt
 grep '@tag_environment_test' out.txt
 grep '@tag_status_running' out.txt
+grep '@tag_type_running' out.txt
+
+grep '@without_trailing_name_host0' out.txt
+grep '@without_trailing_environment_test' out.txt
+grep '@without_trailing_status' out.txt
+grep '@without_trailing_type' out.txt
 
 # keyed group with default value for key's value empty (list)
 ansible-inventory -i tag_inventory.yml -i keyed_group_list_default_value.yml --graph | tee out.txt
@@ -37,12 +43,15 @@ ansible-inventory -i tag_inventory.yml -i keyed_group_list_default_value.yml --g
 grep '@host_db' out.txt
 grep '@host_web' out.txt
 grep '@host_storage' out.txt
+grep '@host_None' out.txt && exit 1
+grep '@empty_list_test_default_value' out.txt && exit 1
 
 # keyed group with default value for key's value empty (str)
 ansible-inventory -i tag_inventory.yml -i keyed_group_str_default_value.yml --graph | tee out.txt
 
 grep '@host_fedora' out.txt
-
+grep '@none_test_default_value' out.txt
+grep '@empty_test_default_value' out.txt
 
 # keyed group with 'trailing_separator' set to 'False' for key's value empty
 ansible-inventory -i tag_inventory.yml -i keyed_group_trailing_separator.yml --graph | tee out.txt

test/integration/targets/inventory_constructed/tag_inventory.yml

@@ -5,8 +5,10 @@ all:
         name: "host0"
         environment: "test"
         status: ""
+        type: ~
       os: ""
       roles:
         - db
         - web
         - ""
+        - ~