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

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:
 

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

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

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
 
 

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