From 50b40c47df031ef48d34006f70c03972005632a8 Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Tue, 29 Jan 2019 15:59:38 -0500 Subject: [PATCH] aws_ec2 Implement the missing 'region discovery' (#51333) * aws_ec2 Implement the missing 'region discovery' fixes #45288 tries to use api as documented (which seems to fail in latest boto3 versions) and fallback to boto3 'hardcoded' list of regions * fixes and cleanup, add error for worst case scenario * fix tests, remove more unused code * add load_name * acually load the plugin * set plugin as required * reverted test changes, removed options tests * fixes as per feedback and cleanup --- .../fragments/allow_regions_aws_invp.yml | 2 + lib/ansible/plugins/inventory/aws_ec2.py | 83 ++++++++----------- test/units/plugins/inventory/test_aws_ec2.py | 36 +------- 3 files changed, 39 insertions(+), 82 deletions(-) create mode 100644 changelogs/fragments/allow_regions_aws_invp.yml diff --git a/changelogs/fragments/allow_regions_aws_invp.yml b/changelogs/fragments/allow_regions_aws_invp.yml new file mode 100644 index 0000000000..efeaa35549 --- /dev/null +++ b/changelogs/fragments/allow_regions_aws_invp.yml @@ -0,0 +1,2 @@ +bugfixes: + - Fix aws_ec2 inventory plugin code to automatically populate regions when missing as documentation states, also leverage config system vs self default/type validation diff --git a/lib/ansible/plugins/inventory/aws_ec2.py b/lib/ansible/plugins/inventory/aws_ec2.py index 977d94e044..7fa4c98804 100644 --- a/lib/ansible/plugins/inventory/aws_ec2.py +++ b/lib/ansible/plugins/inventory/aws_ec2.py @@ -48,18 +48,27 @@ DOCUMENTATION = ''' - name: AWS_SESSION_TOKEN - name: EC2_SECURITY_TOKEN regions: - description: A list of regions in which to describe EC2 instances. By default this is all regions except us-gov-west-1 - and cn-north-1. + description: + - A list of regions in which to describe EC2 instances. + - If empty (the default) default this will include all regions, except possibly restricted ones like us-gov-west-1 and cn-north-1. + type: list + default: [] hostnames: description: A list in order of precedence for hostname variables. You can use the options specified in U(http://docs.aws.amazon.com/cli/latest/reference/ec2/describe-instances.html#options). To use tags as hostnames use the syntax tag:Name=Value to use the hostname Name_Value, or tag:Name to use the value of the Name tag. + type: list + default: [] filters: description: A dictionary of filter value pairs. Available filters are listed here U(http://docs.aws.amazon.com/cli/latest/reference/ec2/describe-instances.html#options) + type: dict + default: {} strict_permissions: description: By default if a 403 (Forbidden) is encountered this plugin will fail. You can set strict_permissions to False in the inventory config file which will allow 403 errors to be gracefully skipped. + type: bool + default: True ''' EXAMPLES = ''' @@ -127,9 +136,8 @@ compose: ansible_host: private_ip_address ''' -from ansible.errors import AnsibleError, AnsibleParserError +from ansible.errors import AnsibleError from ansible.module_utils._text import to_native, to_text -from ansible.module_utils.six import string_types from ansible.module_utils.ec2 import ansible_dict_to_boto3_filter_list, boto3_tag_list_to_ansible_dict from ansible.module_utils.ec2 import camel_dict_to_snake_dict from ansible.plugins.inventory import BaseInventoryPlugin, Constructable, Cacheable, to_safe_group_name @@ -315,6 +323,25 @@ class InventoryModule(BaseInventoryPlugin, Constructable, Cacheable): Generator that yields a boto3 client and the region ''' + if not regions: + try: + # as per https://boto3.amazonaws.com/v1/documentation/api/latest/guide/ec2-example-regions-avail-zones.html + client = boto3.client('ec2') + resp = client.describe_regions() + regions = [x['RegionName'] for x in resp.get('Regions', [])] + except botocore.exceptions.NoRegionError: + # above seems to fail depending on boto3 version, ignore and lets try something else + pass + + # fallback to local list hardcoded in boto3 if still no regions + if not regions: + session = boto3.Session() + regions = session.get_available_regions('ec2') + + # I give up, now you MUST give me regions + if not regions: + raise AnsibleError('Unable to get regions list from available methods, you must specify the "regions" option to continue.') + credentials = self._get_credentials() for region in regions: @@ -486,49 +513,6 @@ class InventoryModule(BaseInventoryPlugin, Constructable, Cacheable): display.debug("aws_ec2 inventory filename must end with 'aws_ec2.yml' or 'aws_ec2.yaml'") return False - def _get_query_options(self, config_data): - ''' - :param config_data: contents of the inventory config file - :return A list of regions to query, - a list of boto3 filter dicts, - a list of possible hostnames in order of preference - a boolean to indicate whether to fail on permission errors - ''' - options = {'regions': {'type_to_be': list, 'value': config_data.get('regions', [])}, - 'filters': {'type_to_be': dict, 'value': config_data.get('filters', {})}, - 'hostnames': {'type_to_be': list, 'value': config_data.get('hostnames', [])}, - 'strict_permissions': {'type_to_be': bool, 'value': config_data.get('strict_permissions', True)}} - - # validate the options - for name in options: - options[name]['value'] = self._validate_option(name, options[name]['type_to_be'], options[name]['value']) - - regions = options['regions']['value'] - filters = ansible_dict_to_boto3_filter_list(options['filters']['value']) - hostnames = options['hostnames']['value'] - strict_permissions = options['strict_permissions']['value'] - - return regions, filters, hostnames, strict_permissions - - def _validate_option(self, name, desired_type, option_value): - ''' - :param name: the option name - :param desired_type: the class the option needs to be - :param option: the value the user has provided - :return The option of the correct class - ''' - - 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 parse(self, inventory, loader, path, cache=True): super(InventoryModule, self).parse(inventory, loader, path) @@ -536,7 +520,10 @@ class InventoryModule(BaseInventoryPlugin, Constructable, Cacheable): self._set_credentials() # get user specifications - regions, filters, hostnames, strict_permissions = self._get_query_options(config_data) + regions = self.get_option('regions') + filters = ansible_dict_to_boto3_filter_list(self.get_option('filters')) + hostnames = self.get_option('hostnames') + strict_permissions = self.get_option('strict_permissions') cache_key = self.get_cache_key(path) # false when refresh_cache or --flush-cache is used diff --git a/test/units/plugins/inventory/test_aws_ec2.py b/test/units/plugins/inventory/test_aws_ec2.py index 682f111ee9..dfed6684d4 100644 --- a/test/units/plugins/inventory/test_aws_ec2.py +++ b/test/units/plugins/inventory/test_aws_ec2.py @@ -28,9 +28,8 @@ import datetime boto3 = pytest.importorskip('boto3') botocore = pytest.importorskip('botocore') -from ansible.errors import AnsibleError, AnsibleParserError -from ansible.plugins.inventory.aws_ec2 import InventoryModule -from ansible.plugins.inventory.aws_ec2 import instance_data_filter_to_boto_attr +from ansible.errors import AnsibleError +from ansible.plugins.inventory.aws_ec2 import InventoryModule, instance_data_filter_to_boto_attr instances = { u'Instances': [ @@ -176,36 +175,5 @@ def test_insufficient_credentials(inventory): assert "Insufficient boto credentials found" in error_message -def test_validate_option(inventory): - assert ['us-east-1'] == inventory._validate_option('regions', list, 'us-east-1') - assert ['us-east-1'] == inventory._validate_option('regions', list, ['us-east-1']) - - -def test_illegal_option(inventory): - bad_filters = [{'tag:Environment': 'dev'}] - with pytest.raises(AnsibleParserError) as error_message: - inventory._validate_option('filters', dict, bad_filters) - assert "The option filters ([{'tag:Environment': 'dev'}]) must be a " == error_message - - -def test_empty_config_query_options(inventory): - regions, filters, hostnames, strict_permissions = inventory._get_query_options({}) - assert regions == filters == hostnames == [] - assert strict_permissions is True - - -def test_conig_query_options(inventory): - regions, filters, hostnames, strict_permissions = inventory._get_query_options( - {'regions': ['us-east-1', 'us-east-2'], - 'filters': {'tag:Environment': ['dev', 'prod']}, - 'hostnames': 'ip-address', - 'strict_permissions': False} - ) - assert regions == ['us-east-1', 'us-east-2'] - assert filters == [{'Name': 'tag:Environment', 'Values': ['dev', 'prod']}] - assert hostnames == ['ip-address'] - assert strict_permissions is False - - def test_verify_file_bad_config(inventory): assert inventory.verify_file('not_aws_config.yml') is False