From 9b6721909665813235f2e16615a77459d0853478 Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Mon, 11 Mar 2019 15:12:14 -0400 Subject: [PATCH] Fine tune sanity (#53544) * modify regex to use implicit charsets this should solve issues in py3 and unicode names * fix issue with subgroups in yaml inventory * clarify deprecation message * separated per name warning from deprecation * move noise to verbosity, simplify warnings * fix docs to reflect actual 'good' practice * change toggle to choice list to give users more options --- .../user_guide/playbooks_best_practices.rst | 24 +++++++++---------- lib/ansible/config/base.yml | 10 ++++---- lib/ansible/constants.py | 3 ++- lib/ansible/inventory/group.py | 21 ++++++++++++---- lib/ansible/plugins/inventory/yaml.py | 4 +++- 5 files changed, 40 insertions(+), 22 deletions(-) diff --git a/docs/docsite/rst/user_guide/playbooks_best_practices.rst b/docs/docsite/rst/user_guide/playbooks_best_practices.rst index b6146c7a2ac..1d595b3d3bd 100644 --- a/docs/docsite/rst/user_guide/playbooks_best_practices.rst +++ b/docs/docsite/rst/user_guide/playbooks_best_practices.rst @@ -140,40 +140,40 @@ It is suggested that you define groups based on purpose of the host (roles) and # file: production - [atlanta-webservers] + [atlanta_webservers] www-atl-1.example.com www-atl-2.example.com - [boston-webservers] + [boston_webservers] www-bos-1.example.com www-bos-2.example.com - [atlanta-dbservers] + [atlanta_dbservers] db-atl-1.example.com db-atl-2.example.com - [boston-dbservers] + [boston_dbservers] db-bos-1.example.com # webservers in all geos [webservers:children] - atlanta-webservers - boston-webservers + atlanta_webservers + boston_webservers # dbservers in all geos [dbservers:children] - atlanta-dbservers - boston-dbservers + atlanta_dbservers + boston_dbservers # everything in the atlanta geo [atlanta:children] - atlanta-webservers - atlanta-dbservers + atlanta_webservers + atlanta_dbservers # everything in the boston geo [boston:children] - boston-webservers - boston-dbservers + boston_webservers + boston_dbservers .. _groups_and_hosts: diff --git a/lib/ansible/config/base.yml b/lib/ansible/config/base.yml index a4d302a50cb..983fc42b78f 100644 --- a/lib/ansible/config/base.yml +++ b/lib/ansible/config/base.yml @@ -1467,15 +1467,17 @@ INTERPRETER_PYTHON_FALLBACK: version_added: "2.8" TRANSFORM_INVALID_GROUP_CHARS: name: Transform invalid characters in group names - default: False + default: 'never' description: - Make ansible transform invalid characters in group names supplied by inventory sources. - - If 'false' it will allow for the group name but warn about the issue. - - When 'true' it will replace any invalid charachters with '_' (underscore). + - If 'never' it will allow for the group name but warn about the issue. + - When 'always' it will replace any invalid charachters with '_' (underscore) and warn the user + - When 'silently', it does the same as 'always' sans the warnings. env: [{name: ANSIBLE_TRANSFORM_INVALID_GROUP_CHARS}] ini: - {key: force_valid_group_names, section: defaults} - type: bool + type: string + choices: ['always', 'never', 'silently'] version_added: '2.8' INVALID_TASK_ATTRIBUTE_FAILED: name: Controls whether invalid attributes for a task result in errors instead of warnings diff --git a/lib/ansible/constants.py b/lib/ansible/constants.py index 2b92c3bb5ca..309edbba977 100644 --- a/lib/ansible/constants.py +++ b/lib/ansible/constants.py @@ -119,7 +119,8 @@ VAULT_VERSION_MIN = 1.0 VAULT_VERSION_MAX = 1.0 # This matches a string that cannot be used as a valid python variable name i.e 'not-valid', 'not!valid@either' '1_nor_This' -INVALID_VARIABLE_NAMES = re.compile(r'^[^a-zA-Z_]|[^a-zA-Z0-9_]') +INVALID_VARIABLE_NAMES = re.compile(r'^[\d\W]|[^\w]') + # FIXME: remove once play_context mangling is removed # the magic variable mapping dictionary below is used to translate diff --git a/lib/ansible/inventory/group.py b/lib/ansible/inventory/group.py index 55f8aad74f1..810d57a2714 100644 --- a/lib/ansible/inventory/group.py +++ b/lib/ansible/inventory/group.py @@ -31,16 +31,29 @@ display = Display() def to_safe_group_name(name, replacer="_", force=False, silent=False): # Converts 'bad' characters in a string to underscores (or provided replacer) so they can be used as Ansible hosts or groups + warn = '' if name: # when deserializing we might not have name yet invalid_chars = C.INVALID_VARIABLE_NAMES.findall(name) if invalid_chars: msg = 'invalid character(s) "%s" in group name (%s)' % (to_text(set(invalid_chars)), to_text(name)) - if C.TRANSFORM_INVALID_GROUP_CHARS or force: + if C.TRANSFORM_INVALID_GROUP_CHARS not in ('never', 'ignore') or force: name = C.INVALID_VARIABLE_NAMES.sub(replacer, name) - if not silent: - display.warning('Replacing ' + msg) + if not (silent or C.TRANSFORM_INVALID_GROUP_CHARS == 'silently'): + display.vvvv('Replacing ' + msg) + warn = 'Invalid characters were found in group names and automatically replaced, use -vvvv to see details' else: - display.deprecated('Ignoring ' + msg, version='2.12') + if C.TRANSFORM_INVALID_GROUP_CHARS == 'never': + display.vvvv('Not replacing %s' % msg) + warn = True + warn = 'Invalid characters were found in group names but not replaced, use -vvvv to see details' + + # remove this message after 2.10 AND changing the default to 'always' + display.deprecated('The TRANSFORM_INVALID_GROUP_CHARS settings is set to allow bad characters in group names by default,' + ' this will change, but still be user configurable on deprecation', version='2.10') + + if warn: + display.warning(warn) + return name diff --git a/lib/ansible/plugins/inventory/yaml.py b/lib/ansible/plugins/inventory/yaml.py index 39433989a7d..dc882c6d357 100644 --- a/lib/ansible/plugins/inventory/yaml.py +++ b/lib/ansible/plugins/inventory/yaml.py @@ -153,7 +153,7 @@ class InventoryModule(BaseFileInventoryPlugin): self.inventory.set_variable(group, var, group_data[key][var]) elif key == 'children': for subgroup in group_data[key]: - self._parse_group(subgroup, group_data[key][subgroup]) + subgroup = self._parse_group(subgroup, group_data[key][subgroup]) self.inventory.add_child(group, subgroup) elif key == 'hosts': @@ -166,6 +166,8 @@ class InventoryModule(BaseFileInventoryPlugin): else: self.display.warning("Skipping '%s' as this is not a valid group definition" % group) + return group + def _parse_host(self, host_pattern): ''' Each host key can be a pattern, try to process it and add variables as needed