Add toggle to control invalid character substitution in group names (#52748)

* make add_group return proper name
* ensure central transform/check
* added 'silent' option to avoid spamming current users
  those already using the plugins were used to the transformations, so no need to alert them
* centralized valid var names
* dont display dupes
* comment on regex
* added regex tests
  ini and script will now warn about deprecation
* more complete errormsg
This commit is contained in:
Brian Coca 2019-03-06 11:49:40 -05:00 committed by GitHub
commit d241794daa
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
14 changed files with 107 additions and 41 deletions

View file

@ -21,10 +21,10 @@ __metaclass__ = type
import hashlib
import os
import re
import string
from ansible.errors import AnsibleError, AnsibleParserError
from ansible.inventory.group import to_safe_group_name as original_safe
from ansible.parsing.utils.addresses import parse_address
from ansible.plugins import AnsiblePlugin
from ansible.plugins.cache import InventoryFileCacheModule
@ -37,13 +37,11 @@ from ansible.utils.display import Display
display = Display()
_SAFE_GROUP = re.compile("[^A-Za-z0-9_]")
# Helper methods
def to_safe_group_name(name):
''' Converts 'bad' characters in a string to underscores so they can be used as Ansible hosts or groups '''
return _SAFE_GROUP.sub("_", name)
# placeholder for backwards compat
return original_safe(name, force=True, silent=True)
def detect_range(line=None):
@ -319,6 +317,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)
try:
result = boolean(self.templar.template(conditional))
except Exception as e:
@ -327,8 +326,8 @@ class Constructable(object):
continue
if result:
# ensure group exists
self.inventory.add_group(group_name)
# ensure group exists, use sanatized name
group_name = self.inventory.add_group(group_name)
# add host to group
self.inventory.add_child(group_name, host)

View file

@ -446,7 +446,7 @@ class InventoryModule(BaseInventoryPlugin, Constructable, Cacheable):
def _populate(self, groups, hostnames):
for group in groups:
self.inventory.add_group(group)
group = self.inventory.add_group(group)
self._add_hosts(hosts=groups[group], group=group, hostnames=hostnames)
self.inventory.add_child('all', group)

View file

@ -60,14 +60,12 @@ password: secure
validate_certs: False
'''
import re
from distutils.version import LooseVersion
from ansible.errors import AnsibleError
from ansible.module_utils._text import to_bytes, to_native
from ansible.module_utils.common._collections_compat import MutableMapping
from ansible.plugins.inventory import BaseInventoryPlugin, Cacheable
from ansible.plugins.inventory import BaseInventoryPlugin, Cacheable, to_safe_group_name
# 3rd party imports
try:
@ -185,14 +183,6 @@ class InventoryModule(BaseInventoryPlugin, Cacheable):
raise ValueError("More than one set of facts returned for '%s'" % host)
return facts
def to_safe(self, word):
'''Converts 'bad' characters in a string to underscores so they can be used as Ansible groups
#> ForemanInventory.to_safe("foo-bar baz")
'foo_barbaz'
'''
regex = r"[^A-Za-z0-9\_]"
return re.sub(regex, "_", word.replace(" ", ""))
def _populate(self):
for host in self._get_hosts():
@ -203,8 +193,8 @@ class InventoryModule(BaseInventoryPlugin, Cacheable):
# create directly mapped groups
group_name = host.get('hostgroup_title', host.get('hostgroup_name'))
if group_name:
group_name = self.to_safe('%s%s' % (self.get_option('group_prefix'), group_name.lower()))
self.inventory.add_group(group_name)
group_name = to_safe_group_name('%s%s' % (self.get_option('group_prefix'), group_name.lower().replace(" ", "")))
group_name = self.inventory.add_group(group_name)
self.inventory.add_child(group_name, host['name'])
# set host vars from host info
@ -224,7 +214,8 @@ class InventoryModule(BaseInventoryPlugin, Cacheable):
try:
self.inventory.set_variable(host['name'], p['name'], p['value'])
except ValueError as e:
self.display.warning("Could not set parameter hostvar for %s, skipping %s: %s" % (host, p['name'], to_native(p['value'])))
self.display.warning("Could not set hostvar %s to '%s' for the '%s' host, skipping: %s" %
(p['name'], to_native(p['value']), host, to_native(e)))
# set host vars from facts
if self.get_option('want_facts'):

View file

@ -77,6 +77,7 @@ EXAMPLES = '''
import ast
import re
from ansible.inventory.group import to_safe_group_name
from ansible.plugins.inventory import BaseFileInventoryPlugin
from ansible.errors import AnsibleError, AnsibleParserError
@ -171,6 +172,8 @@ class InventoryModule(BaseFileInventoryPlugin):
if m:
(groupname, state) = m.groups()
groupname = to_safe_group_name(groupname)
state = state or 'hosts'
if state not in ['hosts', 'children', 'vars']:
title = ":".join(m.groups())

View file

@ -153,7 +153,7 @@ class InventoryModule(BaseInventoryPlugin, Cacheable):
try:
got = data_from_meta.get(host, {})
except AttributeError as e:
raise AnsibleError("Improperly formatted host information for %s: %s" % (host, to_native(e)))
raise AnsibleError("Improperly formatted host information for %s: %s" % (host, to_native(e)), orig_exc=e)
self._populate_host_vars([host], got)
@ -162,7 +162,7 @@ class InventoryModule(BaseInventoryPlugin, Cacheable):
def _parse_group(self, group, data):
self.inventory.add_group(group)
group = self.inventory.add_group(group)
if not isinstance(data, dict):
data = {'hosts': data}
@ -187,7 +187,7 @@ class InventoryModule(BaseInventoryPlugin, Cacheable):
if group != '_meta' and isinstance(data, dict) and 'children' in data:
for child_name in data['children']:
self.inventory.add_group(child_name)
child_name = self.inventory.add_group(child_name)
self.inventory.add_child(group, child_name)
def get_host_variables(self, path, host):

View file

@ -163,7 +163,7 @@ class InventoryModule(BaseFileInventoryPlugin):
self.display.warning("Skipping '%s' as this is not a valid group definition" % group)
return
self.inventory.add_group(group)
group = self.inventory.add_group(group)
if group_data is None:
return

View file

@ -107,7 +107,7 @@ class InventoryModule(BaseInventoryPlugin, Constructable, Cacheable):
if group == 'all':
continue
else:
self.inventory.add_group(group)
group = self.inventory.add_group(group)
hosts = source_data[group].get('hosts', [])
for host in hosts:
self._populate_host_vars([host], hostvars.get(host, {}), group)
@ -162,10 +162,10 @@ class InventoryModule(BaseInventoryPlugin, Constructable, Cacheable):
elif k == 'Groups':
for group in v.split('/'):
if group:
group = self.inventory.add_group(group)
self.inventory.add_child(group, current_host)
if group not in cacheable_results:
cacheable_results[group] = {'hosts': []}
self.inventory.add_group(group)
self.inventory.add_child(group, current_host)
cacheable_results[group]['hosts'].append(current_host)
continue

View file

@ -124,7 +124,7 @@ class InventoryModule(BaseFileInventoryPlugin):
if isinstance(group_data, (MutableMapping, NoneType)):
try:
self.inventory.add_group(group)
group = self.inventory.add_group(group)
except AnsibleError as e:
raise AnsibleParserError("Unable to add group %s: %s" % (group, to_text(e)))