From 3e52a6a693f33552766f45b3736728d3d8fe2da3 Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Tue, 12 Mar 2019 12:03:20 -0400 Subject: [PATCH] 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 | 3 ++ lib/ansible/plugins/inventory/__init__.py | 9 +++--- lib/ansible/plugins/inventory/aws_ec2.py | 34 ++++++++++++++++++-- lib/ansible/plugins/inventory/azure_rm.py | 30 +++++++++++++++-- lib/ansible/plugins/inventory/gcp_compute.py | 24 ++++++++++++-- 5 files changed, 88 insertions(+), 12 deletions(-) create mode 100644 changelogs/fragments/allow_bad_things.yml diff --git a/changelogs/fragments/allow_bad_things.yml b/changelogs/fragments/allow_bad_things.yml new file mode 100644 index 00000000000..3bdbc50896f --- /dev/null +++ b/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 diff --git a/lib/ansible/plugins/inventory/__init__.py b/lib/ansible/plugins/inventory/__init__.py index 45a8917c93b..393263c9998 100644 --- a/lib/ansible/plugins/inventory/__init__.py +++ b/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) diff --git a/lib/ansible/plugins/inventory/aws_ec2.py b/lib/ansible/plugins/inventory/aws_ec2.py index 933d5a4b6e5..d0645bdc87e 100644 --- a/lib/ansible/plugins/inventory/aws_ec2.py +++ b/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) diff --git a/lib/ansible/plugins/inventory/azure_rm.py b/lib/ansible/plugins/inventory/azure_rm.py index a971012488c..40755e366c0 100644 --- a/lib/ansible/plugins/inventory/azure_rm.py +++ b/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.+)$') diff --git a/lib/ansible/plugins/inventory/gcp_compute.py b/lib/ansible/plugins/inventory/gcp_compute.py index b9d648cecb4..b947e4e1001 100644 --- a/lib/ansible/plugins/inventory/gcp_compute.py +++ b/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