Skip to content

Commit

Permalink
fix 'return false' from parse
Browse files Browse the repository at this point in the history
this was abandoned early on the manger side  but seems like we left behind on plugin side.
more flexible extensions with yaml plugin
validate data correctly for yaml/constructed
fixed issue with only adding one child to keyed, the group only got the host that forced it's creation

fixes ansible#31382
fixes ansible#31365
  • Loading branch information
bcoca committed Oct 6, 2017
1 parent f2ade09 commit e4c61ea
Show file tree
Hide file tree
Showing 5 changed files with 29 additions and 20 deletions.
9 changes: 6 additions & 3 deletions lib/ansible/plugins/inventory/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,10 +142,13 @@ def _add_host_to_keyed_groups(self, keys, variables, host, strict=False):
continue
if isinstance(groups, string_types):
groups = [groups]
for group_name in groups:
if group_name not in self.inventory.groups:
self.inventory.add_group(group_name)
if isinstance(groups, list):
for group_name in groups:
if group_name not in self.inventory.groups:
self.inventory.add_group(group_name)
self.inventory.add_child(group_name, host)
else:
raise AnsibleOptionsError("Invalid group name format, expected string or list of strings, got: %s" % type(groups))
else:
raise AnsibleOptionsError("No key supplied, invalid entry")
else:
Expand Down
9 changes: 7 additions & 2 deletions lib/ansible/plugins/inventory/constructed.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
version_added: "2.4"
short_description: Uses Jinja2 to construct vars and groups based on existing inventory.
description:
- Uses a YAML configuration file with a ``.config`` extension to define var expresisions and group conditionals
- Uses a YAML configuration file with a valid YAML or ``.config`` extension to define var expressions and group conditionals
- The Jinja2 conditionals that qualify a host for membership.
- The JInja2 exprpessions are calculated and assigned to the variables
- Only variables already available from previous inventories or the fact cache can be used for templating.
Expand Down Expand Up @@ -54,6 +54,9 @@

import os

from collections import MutableMapping

from ansible import constants as C
from ansible.errors import AnsibleParserError
from ansible.plugins.inventory import BaseInventoryPlugin
from ansible.module_utils._text import to_native
Expand All @@ -75,7 +78,7 @@ def verify_file(self, path):
if super(InventoryModule, self).verify_file(path):
file_name, ext = os.path.splitext(path)

if not ext or ext == '.config':
if not ext or ext in ['.config'] + C.YAML_FILENAME_EXTENSIONS:
valid = True

return valid
Expand All @@ -92,6 +95,8 @@ def parse(self, inventory, loader, path, cache=False):

if not data:
raise AnsibleParserError("%s is empty" % (to_native(path)))
elif not isinstance(data, MutableMapping):
raise AnsibleParserError('inventory source has invalid structure, it should be a dictionary, got: %s' % type(data))
elif data.get('plugin') != self.NAME:
raise AnsibleParserError("%s is not a constructed groups config file, plugin entry must be 'constructed'" % (to_native(path)))

Expand Down
23 changes: 10 additions & 13 deletions lib/ansible/plugins/inventory/openstack.py
Original file line number Diff line number Diff line change
Expand Up @@ -129,21 +129,18 @@ def parse(self, inventory, loader, path, cache=True):
except Exception as e:
raise AnsibleParserError(e)

msg = ''
if not self._config_data:
# empty. this is not my config file
return False
if 'plugin' in self._config_data and self._config_data['plugin'] != self.NAME:
# plugin config file, but not for us
return False
msg = 'File empty. this is not my config file'
elif 'plugin' in self._config_data and self._config_data['plugin'] != self.NAME:
msg = 'plugin config file, but not for us: %s' % self._config_data['plugin']
elif 'plugin' not in self._config_data and 'clouds' not in self._config_data:
# it's not a clouds.yaml file either
return False

if not HAS_SHADE:
self.display.warning(
'shade is required for the OpenStack inventory plugin.'
' OpenStack inventory sources will be skipped.')
return False
msg = "it's not a plugin configuration nor a clouds.yaml file"
elif not HAS_SHADE:
msg = "shade is required for the OpenStack inventory plugin. OpenStack inventory sources will be skipped."

if msg:
raise AnsibleParserError(msg)

# The user has pointed us at a clouds.yaml file. Use defaults for
# everything.
Expand Down
2 changes: 1 addition & 1 deletion lib/ansible/plugins/inventory/virtualbox.py
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ def parse(self, inventory, loader, path, cache=True):

if not config_data or config_data.get('plugin') != self.NAME:
# this is not my config file
return False
raise AnsibleParserError("Incorrect plugin name in file: %s" % config_data.get('plugin', 'none found'))

source_data = None
if cache and cache_key in inventory.cache:
Expand Down
6 changes: 5 additions & 1 deletion lib/ansible/plugins/inventory/yaml.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,11 @@ def parse(self, inventory, loader, path, cache=True):
raise AnsibleParserError(e)

if not data:
return False
raise AnsibleParserError('Parsed empty YAML file')
elif not isinstance(data, MutableMapping):
raise AnsibleParserError('YAML inventory has invalid structure, it should be a dictionary, got: %s' % type(data))
elif data.get('plugin'):
raise AnsibleParserError('Plugin configuration YAML file, not YAML inventory')

# We expect top level keys to correspond to groups, iterate over them
# to get host, vars and subgroups (which we iterate over recursivelly)
Expand Down

0 comments on commit e4c61ea

Please sign in to comment.