Overridable safety (#53458)

* create overridable sanitation function
* now used in aws, gce and azure plugins
* added new option to these plugins to work in conjunction with general toggle to make it easier
  to emulate inventory script behavior.

changelogs/fragments/allow_bad_things.yml

@@ -0,0 +1,3 @@
+minor_changes:
+    - Embed an overridable static sanitization method into base inventory plugin class to allow individual plugins to optionally override
+      Add override implementation to inital set of cloud plugins

lib/ansible/plugins/inventory/__init__.py

@@ -150,6 +150,7 @@ class BaseInventoryPlugin(AnsiblePlugin):
     """ Parses an Inventory Source"""
 
     TYPE = 'generator'
+    _sanitize_group_name = staticmethod(to_safe_group_name)
 
     def __init__(self):
 
@@ -371,7 +372,7 @@ class Constructable(object):
             self.templar.set_available_variables(variables)
             for group_name in groups:
                 conditional = "{%% if %s %%} True {%% else %%} False {%% endif %%}" % groups[group_name]
-                group_name = to_safe_group_name(group_name)
+                group_name = self._sanitize_group_name(group_name)
                 try:
                     result = boolean(self.templar.template(conditional))
                 except Exception as e:
@@ -380,7 +381,7 @@ class Constructable(object):
                     continue
 
                 if result:
-                    # ensure group exists, use sanatized name
+                    # ensure group exists, use sanitized name
                     group_name = self.inventory.add_group(group_name)
                     # add host to group
                     self.inventory.add_child(group_name, host)
@@ -418,12 +419,12 @@ class Constructable(object):
                             raise AnsibleParserError("Invalid group name format, expected a string or a list of them or dictionary, got: %s" % type(key))
 
                         for bare_name in new_raw_group_names:
-                            gname = to_safe_group_name('%s%s%s' % (prefix, sep, bare_name))
+                            gname = self._sanitize_group_name('%s%s%s' % (prefix, sep, bare_name))
                             self.inventory.add_group(gname)
                             self.inventory.add_child(gname, host)
 
                             if raw_parent_name:
-                                parent_name = to_safe_group_name(raw_parent_name)
+                                parent_name = self._sanitize_group_name(raw_parent_name)
                                 self.inventory.add_group(parent_name)
                                 self.inventory.add_child(parent_name, gname)
 

lib/ansible/plugins/inventory/aws_ec2.py

@@ -69,6 +69,20 @@ DOCUMENTATION = '''
               False in the inventory config file which will allow 403 errors to be gracefully skipped.
           type: bool
           default: True
+        use_contrib_script_compatible_sanitization:
+          description:
+            - By default this plugin is using a general group name sanitization to create safe and usable group names for use in Ansible.
+              This option allows you to override that, in efforts to allow migration from the old inventory script and
+              matches the sanitization of groups when the script's ``replace_dash_in_groups`` option is set to ``False``.
+              To replicate behavior of ``replace_dash_in_groups = True`` with constructed groups,
+              you will need to replace hyphens with underscores via the regex_replace filter for those entries.
+            - For this to work you should also turn off the TRANSFORM_INVALID_GROUP_CHARS setting,
+              otherwise the core engine will just use the standard sanitization on top.
+            - This is not the default as such names break certain functionality as not all characters are valid Python identifiers
+              which group names end up being used as.
+          type: bool
+          default: False
+          version_added: '2.8'
 '''
 
 EXAMPLES = '''
@@ -135,12 +149,13 @@ compose:
   # (note: this does not modify inventory_hostname, which is set via I(hostnames))
   ansible_host: private_ip_address
 '''
+import re
 
 from ansible.errors import AnsibleError
 from ansible.module_utils._text import to_native, to_text
 from ansible.module_utils.ec2 import ansible_dict_to_boto3_filter_list, boto3_tag_list_to_ansible_dict
 from ansible.module_utils.ec2 import camel_dict_to_snake_dict
-from ansible.plugins.inventory import BaseInventoryPlugin, Constructable, Cacheable, to_safe_group_name
+from ansible.plugins.inventory import BaseInventoryPlugin, Constructable, Cacheable
 from ansible.utils.display import Display
 
 try:
@@ -431,7 +446,7 @@ class InventoryModule(BaseInventoryPlugin, Constructable, Cacheable):
                 break
         if hostname:
             if ':' in to_text(hostname):
-                return to_safe_group_name(to_text(hostname))
+                return self._sanitize_group_name((to_text(hostname)))
             else:
                 return to_text(hostname)
 
@@ -518,9 +533,14 @@ class InventoryModule(BaseInventoryPlugin, Constructable, Cacheable):
         return False
 
     def parse(self, inventory, loader, path, cache=True):
+
         super(InventoryModule, self).parse(inventory, loader, path)
 
-        config_data = self._read_config_data(path)
+        self._read_config_data(path)
+
+        if self.get_option('use_contrib_script_compatible_sanitization'):
+            self._sanitize_group_name = self._legacy_script_compatible_group_sanitization
+
         self._set_credentials()
 
         # get user specifications
@@ -553,3 +573,11 @@ class InventoryModule(BaseInventoryPlugin, Constructable, Cacheable):
         # when the user is using caching, update the cached inventory
         if cache_needs_update or (not cache and self.get_option('cache')):
             self.cache.set(cache_key, results)
+
+    @staticmethod
+    def _legacy_script_compatible_group_sanitization(name):
+
+        # note that while this mirrors what the script used to do, it has many issues with unicode and usability in python
+        regex = re.compile(r"[^A-Za-z0-9\_\-]")
+
+        return regex.sub('_', name)

lib/ansible/plugins/inventory/azure_rm.py

@@ -60,6 +60,20 @@ DOCUMENTATION = r'''
                 C(exclude_host_filters) to exclude powered-off and not-fully-provisioned hosts. Set this to a different
                 value or empty list if you need to include hosts in these states.
             default: ['powerstate != "running"', 'provisioning_state != "succeeded"']
+        use_contrib_script_compatible_sanitization:
+          description:
+            - By default this plugin is using a general group name sanitization to create safe and usable group names for use in Ansible.
+              This option allows you to override that, in efforts to allow migration from the old inventory script and
+              matches the sanitization of groups when the script's ``replace_dash_in_groups`` option is set to ``False``.
+              To replicate behavior of ``replace_dash_in_groups = True`` with constructed groups,
+              you will need to replace hyphens with underscores via the regex_replace filter for those entries.
+            - For this to work you should also turn off the TRANSFORM_INVALID_GROUP_CHARS setting,
+              otherwise the core engine will just use the standard sanitization on top.
+            - This is not the default as such names break certain functionality as not all characters are valid Python identifiers
+              which group names end up being used as.
+          type: bool
+          default: False
+          version_added: '2.8'
 '''
 
 EXAMPLES = '''
@@ -152,7 +166,7 @@ except ImportError:
 
 from collections import namedtuple
 from ansible import release
-from ansible.plugins.inventory import BaseInventoryPlugin, Constructable, Cacheable
+from ansible.plugins.inventory import BaseInventoryPlugin, Constructable
 from ansible.module_utils.six import iteritems
 from ansible.module_utils.azure_rm_common import AzureRMAuth
 from ansible.errors import AnsibleParserError, AnsibleError
@@ -227,6 +241,10 @@ class InventoryModule(BaseInventoryPlugin, Constructable):
         super(InventoryModule, self).parse(inventory, loader, path)
 
         self._read_config_data(path)
+
+        if self.get_option('use_contrib_script_compatible_sanitization'):
+            self._sanitize_group_name = self._legacy_script_compatible_group_sanitization
+
         self._batch_fetch = self.get_option('batch_fetch')
 
         self._filters = self.get_option('exclude_host_filters') + self.get_option('default_host_filters')
@@ -234,7 +252,7 @@ class InventoryModule(BaseInventoryPlugin, Constructable):
         try:
             self._credential_setup()
             self._get_hosts()
-        except Exception as ex:
+        except Exception:
             raise
 
     def _credential_setup(self):
@@ -446,10 +464,18 @@ class InventoryModule(BaseInventoryPlugin, Constructable):
 
         return json.loads(content)
 
+    @staticmethod
+    def _legacy_script_compatible_group_sanitization(name):
+
+        # note that while this mirrors what the script used to do, it has many issues with unicode and usability in python
+        regex = re.compile(r"[^A-Za-z0-9\_\-]")
+
+        return regex.sub('_', name)
 
 # VM list (all, N resource groups): VM -> InstanceView, N NICs, N PublicIPAddress)
 # VMSS VMs (all SS, N specific SS, N resource groups?): SS -> VM -> InstanceView, N NICs, N PublicIPAddress)
 
+
 class AzureHost(object):
     _powerstate_regex = re.compile('^PowerState/(?P<powerstate>.+)$')
 

lib/ansible/plugins/inventory/gcp_compute.py

@@ -51,6 +51,17 @@ DOCUMENTATION = '''
         vars_prefix:
             description: prefix to apply to host variables, does not include facts nor params
             default: ''
+        use_contrib_script_compatible_sanitization:
+          description:
+            - By default this plugin is using a general group name sanitization to create safe and usable group names for use in Ansible.
+              This option allows you to override that, in efforts to allow migration from the old inventory script.
+            - For this to work you should also turn off the TRANSFORM_INVALID_GROUP_CHARS setting,
+              otherwise the core engine will just use the standard sanitization on top.
+            - This is not the default as such names break certain functionality as not all characters are valid Python identifiers
+              which group names end up being used as.
+          type: bool
+          default: False
+          version_added: '2.8'
 '''
 
 EXAMPLES = '''
@@ -81,10 +92,9 @@ compose:
 '''
 
 from ansible.errors import AnsibleError, AnsibleParserError
-from ansible.module_utils._text import to_native, to_text
-from ansible.module_utils.six import string_types
+from ansible.module_utils._text import to_native
 from ansible.module_utils.gcp_utils import GcpSession, navigate_hash, GcpRequestException
-from ansible.plugins.inventory import BaseInventoryPlugin, Constructable, Cacheable, to_safe_group_name
+from ansible.plugins.inventory import BaseInventoryPlugin, Constructable, Cacheable
 import json
 
 
@@ -325,6 +335,9 @@ class InventoryModule(BaseInventoryPlugin, Constructable, Cacheable):
         config_data = {}
         config_data = self._read_config_data(path)
 
+        if self.get_option('use_contrib_script_compatible_sanitization'):
+            self._sanitize_group_name = self._legacy_script_compatible_group_sanitization
+
         # get user specifications
         if 'zones' in config_data:
             if not isinstance(config_data['zones'], list):
@@ -380,3 +393,8 @@ class InventoryModule(BaseInventoryPlugin, Constructable, Cacheable):
 
         if cache_needs_update:
             self.cache.set(cache_key, cached_data)
+
+    @staticmethod
+    def _legacy_script_compatible_group_sanitization(name):
+
+        return name