mirror of
https://github.com/ansible-collections/community.general.git
synced 2024-09-14 20:13:21 +02:00
Fix linode inventory filtering (#4336)
* Fix linode inventory filtering There was a regression introduced in the addition of caching. The `_consume_options` method was added and provided the `config_data` dictionary. This `pop`s every entry, resulting in an empty `config_data` dict, which was then reused and expected to be populated. After reviewing, `_consume_data` doesn't need to be called. Also, once the ``_read_config_data` method has been called, we no longer need the config_data dict, and can instead use the `get_option` method throughout. Once those were removed, the filtering function seemed a bit odd, since we were no longer using the file. I used that opportunity to move the filter calls into the populate function directly. * Remove tests that target removed methods This removes tests that targeted some custom methods on configuration file handling. These are no longer necessary since they are now handled by the BaseInventoryPlugin `_read_config_data` method. * Add changelog entry for linode inventory bugfix * Revert filters back to their own method This moves filters back into their own method, but now uses the get_option calls to pull filter configuration items.
This commit is contained in:
parent
c40684db58
commit
386bb4bdd5
3 changed files with 11 additions and 83 deletions
|
@ -0,0 +1,2 @@
|
|||
bugfixes:
|
||||
- linode inventory plugin - fix configuration handling relating to inventory filtering (https://github.com/ansible-collections/community.general/pull/4336).
|
|
@ -181,20 +181,23 @@ class InventoryModule(BaseInventoryPlugin, Constructable, Cacheable):
|
|||
for linode_group in self.linode_groups:
|
||||
self.inventory.add_group(linode_group)
|
||||
|
||||
def _filter_by_config(self, regions, types, tags):
|
||||
def _filter_by_config(self):
|
||||
"""Filter instances by user specified configuration."""
|
||||
regions = self.get_option('regions')
|
||||
if regions:
|
||||
self.instances = [
|
||||
instance for instance in self.instances
|
||||
if instance.region.id in regions
|
||||
]
|
||||
|
||||
types = self.get_option('types')
|
||||
if types:
|
||||
self.instances = [
|
||||
instance for instance in self.instances
|
||||
if instance.type.id in types
|
||||
]
|
||||
|
||||
tags = self.get_option('tags')
|
||||
if tags:
|
||||
self.instances = [
|
||||
instance for instance in self.instances
|
||||
|
@ -247,60 +250,13 @@ class InventoryModule(BaseInventoryPlugin, Constructable, Cacheable):
|
|||
)
|
||||
return data
|
||||
|
||||
def _validate_option(self, name, desired_type, option_value):
|
||||
"""Validate user specified configuration data against types."""
|
||||
if isinstance(option_value, string_types) and desired_type == list:
|
||||
option_value = [option_value]
|
||||
|
||||
if option_value is None:
|
||||
option_value = desired_type()
|
||||
|
||||
if not isinstance(option_value, desired_type):
|
||||
raise AnsibleParserError(
|
||||
'The option %s (%s) must be a %s' % (
|
||||
name, option_value, desired_type
|
||||
)
|
||||
)
|
||||
|
||||
return option_value
|
||||
|
||||
def _get_query_options(self, config_data):
|
||||
"""Get user specified query options from the configuration."""
|
||||
options = {
|
||||
'regions': {
|
||||
'type_to_be': list,
|
||||
'value': config_data.get('regions', [])
|
||||
},
|
||||
'types': {
|
||||
'type_to_be': list,
|
||||
'value': config_data.get('types', [])
|
||||
},
|
||||
'tags': {
|
||||
'type_to_be': list,
|
||||
'value': config_data.get('tags', [])
|
||||
},
|
||||
}
|
||||
|
||||
for name in options:
|
||||
options[name]['value'] = self._validate_option(
|
||||
name,
|
||||
options[name]['type_to_be'],
|
||||
options[name]['value']
|
||||
)
|
||||
|
||||
regions = options['regions']['value']
|
||||
types = options['types']['value']
|
||||
tags = options['tags']['value']
|
||||
|
||||
return regions, types, tags
|
||||
|
||||
def _cacheable_inventory(self):
|
||||
return [i._raw_json for i in self.instances]
|
||||
|
||||
def populate(self, config_data):
|
||||
def populate(self):
|
||||
strict = self.get_option('strict')
|
||||
regions, types, tags = self._get_query_options(config_data)
|
||||
self._filter_by_config(regions, types, tags)
|
||||
|
||||
self._filter_by_config()
|
||||
|
||||
self._add_groups()
|
||||
self._add_instances_to_groups()
|
||||
|
@ -339,8 +295,7 @@ class InventoryModule(BaseInventoryPlugin, Constructable, Cacheable):
|
|||
if not HAS_LINODE:
|
||||
raise AnsibleError('the Linode dynamic inventory plugin requires linode_api4.')
|
||||
|
||||
config_data = self._read_config_data(path)
|
||||
self._consume_options(config_data)
|
||||
self._read_config_data(path)
|
||||
|
||||
cache_key = self.get_cache_key(path)
|
||||
|
||||
|
@ -363,4 +318,4 @@ class InventoryModule(BaseInventoryPlugin, Constructable, Cacheable):
|
|||
if update_cache:
|
||||
self._cache[cache_key] = self._cacheable_inventory()
|
||||
|
||||
self.populate(config_data)
|
||||
self.populate()
|
||||
|
|
|
@ -48,35 +48,6 @@ def test_missing_access_token_lookup(inventory):
|
|||
assert 'Could not retrieve Linode access token' in error_message
|
||||
|
||||
|
||||
def test_validate_option(inventory):
|
||||
assert ['eu-west'] == inventory._validate_option('regions', list, 'eu-west')
|
||||
assert ['eu-west'] == inventory._validate_option('regions', list, ['eu-west'])
|
||||
assert 'api' == inventory._validate_option('ip_style', str, 'api')
|
||||
|
||||
|
||||
def test_validation_option_bad_option(inventory):
|
||||
with pytest.raises(AnsibleParserError) as error_message:
|
||||
inventory._validate_option('regions', dict, [])
|
||||
assert "The option filters ([]) must be a <class 'dict'>" == error_message
|
||||
|
||||
|
||||
def test_empty_config_query_options(inventory):
|
||||
regions, types, tags = inventory._get_query_options({})
|
||||
assert regions == types == tags == []
|
||||
|
||||
|
||||
def test_config_query_options(inventory):
|
||||
regions, types, tags = inventory._get_query_options({
|
||||
'regions': ['eu-west', 'us-east'],
|
||||
'types': ['g5-standard-2', 'g6-standard-2'],
|
||||
'tags': ['web-server'],
|
||||
})
|
||||
|
||||
assert regions == ['eu-west', 'us-east']
|
||||
assert types == ['g5-standard-2', 'g6-standard-2']
|
||||
assert tags == ['web-server']
|
||||
|
||||
|
||||
def test_verify_file(tmp_path, inventory):
|
||||
file = tmp_path / "foobar.linode.yml"
|
||||
file.touch()
|
||||
|
|
Loading…
Reference in a new issue