From beeeccb5b72699591962af32a0eb1804cc8eaf14 Mon Sep 17 00:00:00 2001 From: Mark Chappell Date: Tue, 22 Nov 2022 14:46:21 +0100 Subject: [PATCH] Refactor inventory plugin connection handling (#1271) Refactor inventory plugin connection handling SUMMARY Further refactors inventory plugin connection handling: Expands AWSPluginBase client() and resource() to allow overriding parameters renames _boto3_conn to all_clients to better describe what it's doing (_boto3_conn is used elswhere for singluar clients) assumes the IAM role once updates plugin parameters to match other plugins/modules updates RDS description wrapper to avoid reordering arguments avoids maintaining a full inventory scoped rewrite of client/resource code avoids maintaining a full inventory scoped handling of boto3/botocore version handling ISSUE TYPE Feature Pull Request COMPONENT NAME plugins/inventory/aws_ec2.py plugins/inventory/aws_rds.py plugins/plugin_utils/base.py plugins/plugin_utils/inventory.py ADDITIONAL INFORMATION Reviewed-by: Alina Buzachis Reviewed-by: Mark Chappell --- .../fragments/1271-inventory-connections.yml | 2 + plugins/doc_fragments/assume_role.py | 24 ++ plugins/inventory/aws_ec2.py | 53 ++-- plugins/inventory/aws_rds.py | 47 ++-- plugins/plugin_utils/base.py | 16 +- plugins/plugin_utils/inventory.py | 257 +++++++++--------- tests/unit/plugin_utils/base/test_plugin.py | 17 ++ .../plugin_utils/inventory/test_inventory.py | 143 ---------- .../inventory/test_inventory_base.py | 31 +++ .../inventory/test_inventory_clients.py | 112 ++++++++ tests/unit/plugins/inventory/test_aws_ec2.py | 12 +- tests/unit/plugins/inventory/test_aws_rds.py | 38 +-- 12 files changed, 374 insertions(+), 378 deletions(-) create mode 100644 changelogs/fragments/1271-inventory-connections.yml create mode 100644 plugins/doc_fragments/assume_role.py delete mode 100644 tests/unit/plugin_utils/inventory/test_inventory.py create mode 100644 tests/unit/plugin_utils/inventory/test_inventory_base.py create mode 100644 tests/unit/plugin_utils/inventory/test_inventory_clients.py diff --git a/changelogs/fragments/1271-inventory-connections.yml b/changelogs/fragments/1271-inventory-connections.yml new file mode 100644 index 00000000000..2b45a2e8b2e --- /dev/null +++ b/changelogs/fragments/1271-inventory-connections.yml @@ -0,0 +1,2 @@ +minor_changes: +- amazon.aws inventory plugins - additional refactorization of inventory plugin connection handling (https://github.com/ansible-collections/amazon.aws/pull/1271). diff --git a/plugins/doc_fragments/assume_role.py b/plugins/doc_fragments/assume_role.py new file mode 100644 index 00000000000..88b3ab7cd86 --- /dev/null +++ b/plugins/doc_fragments/assume_role.py @@ -0,0 +1,24 @@ +# -*- coding: utf-8 -*- +# Copyright: (c) 2022, Ansible, Inc +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + + +class ModuleDocFragment: + # Note: If you're updating MODULES, PLUGINS probably needs updating too. + + # Formatted for Modules + # - modules don't support 'env' + MODULES = r""" +options: {} +""" + + # Formatted for non-module plugins + # - modules don't support 'env' + PLUGINS = r""" +options: + assume_role_arn: + description: + - The ARN of the IAM role to assume to perform the lookup. + - You should still provide AWS credentials with enough privilege to perform the AssumeRole action. + aliases: ["iam_role_arn"] +""" diff --git a/plugins/inventory/aws_ec2.py b/plugins/inventory/aws_ec2.py index 7fbc29ff991..39cf3482cd9 100644 --- a/plugins/inventory/aws_ec2.py +++ b/plugins/inventory/aws_ec2.py @@ -8,7 +8,9 @@ - inventory_cache - constructed - amazon.aws.boto3 - - amazon.aws.aws_credentials + - amazon.aws.common.plugins + - amazon.aws.region.plugins + - amazon.aws.assume_role.plugins description: - Get inventory hosts from Amazon Web Services EC2. - Uses a YAML configuration file that ends with C(aws_ec2.{yml|yaml}). @@ -18,14 +20,6 @@ author: - Sloane Hertel (@s-hertel) options: - plugin: - description: Token that ensures this is a source file for the plugin. - required: True - choices: ['aws_ec2', 'amazon.aws.aws_ec2'] - iam_role_arn: - description: - - The ARN of the IAM role to assume to perform the inventory lookup. You should still provide AWS - credentials with enough privilege to perform the AssumeRole action. regions: description: - A list of regions in which to describe EC2 instances. @@ -266,15 +260,13 @@ except ImportError: pass # will be captured by imported HAS_BOTO3 -from ansible.module_utils._text import to_native from ansible.module_utils._text import to_text -from ansible.module_utils.basic import missing_required_lib +from ansible.module_utils.common.dict_transformations import camel_dict_to_snake_dict + -from ansible.template import Templar -from ansible_collections.amazon.aws.plugins.module_utils.ec2 import HAS_BOTO3 -from ansible_collections.amazon.aws.plugins.module_utils.ec2 import ansible_dict_to_boto3_filter_list -from ansible_collections.amazon.aws.plugins.module_utils.ec2 import boto3_tag_list_to_ansible_dict -from ansible_collections.amazon.aws.plugins.module_utils.ec2 import camel_dict_to_snake_dict +from ansible_collections.amazon.aws.plugins.module_utils.core import is_boto3_error_code +from ansible_collections.amazon.aws.plugins.module_utils.transformation import ansible_dict_to_boto3_filter_list +from ansible_collections.amazon.aws.plugins.module_utils.tagging import boto3_tag_list_to_ansible_dict from ansible_collections.amazon.aws.plugins.plugin_utils.inventory import AWSInventoryBase @@ -475,7 +467,7 @@ class InventoryModule(AWSInventoryBase): def __init__(self): - super(InventoryModule, self).__init__() + super().__init__() self.group_prefix = 'aws_ec2_' @@ -491,7 +483,7 @@ def _get_instances_by_region(self, regions, filters, strict_permissions): if not any(f['Name'] == 'instance-state-name' for f in filters): filters.append({'Name': 'instance-state-name', 'Values': ['running', 'pending', 'stopping', 'stopped']}) - for connection, _region in self._boto3_conn(regions, "ec2"): + for connection, _region in self.all_clients("ec2"): try: reservations = _describe_ec2_instances(connection, filters).get('Reservations') instances = [] @@ -505,13 +497,12 @@ def _get_instances_by_region(self, regions, filters, strict_permissions): for instance in new_instances: instance.update(reservation_details) instances.extend(new_instances) - except botocore.exceptions.ClientError as e: - if e.response['ResponseMetadata']['HTTPStatusCode'] == 403 and not strict_permissions: - instances = [] - else: - self.fail_aws("Failed to describe instances: %s" % to_native(e)) - except botocore.exceptions.BotoCoreError as e: - self.fail_aws("Failed to describe instances: %s" % to_native(e)) + except is_boto3_error_code('UnauthorizedOperation') as e: + if not strict_permissions: + continue + self.fail_aws("Failed to describe instances", exception=e) + except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: + self.fail_aws("Failed to describe instances", exception=e) all_instances.extend(instances) @@ -694,7 +685,7 @@ def verify_file(self, path): :return the contents of the config file ''' inventory_file_suffix = ('aws_ec2.yml', 'aws_ec2.yaml') - if super(InventoryModule, self).verify_file(path): + if super().verify_file(path): if path.endswith(inventory_file_suffix): return True self.display.debug(f"aws_ec2 inventory filename must end with {inventory_file_suffix}") @@ -707,19 +698,11 @@ def build_include_filters(self): return result or [{}] def parse(self, inventory, loader, path, cache=True): - - super(InventoryModule, self).parse(inventory, loader, path) - - if not HAS_BOTO3: - self.fail_aws(missing_required_lib('botocore and boto3')) - - self._read_config_data(path) + super().parse(inventory, loader, path, cache=cache) if self.get_option('use_contrib_script_compatible_sanitization'): self._sanitize_group_name = self._legacy_script_compatible_group_sanitization - self._set_credentials(loader) - # get user specifications regions = self.get_option('regions') include_filters = self.build_include_filters() diff --git a/plugins/inventory/aws_rds.py b/plugins/inventory/aws_rds.py index a3e4dafb0b5..bf667b2fd8d 100644 --- a/plugins/inventory/aws_rds.py +++ b/plugins/inventory/aws_rds.py @@ -36,10 +36,6 @@ default: - creating - available - iam_role_arn: - description: - - The ARN of the IAM role to assume to perform the inventory lookup. You should still provide - AWS credentials with enough privilege to perform the AssumeRole action. hostvars_prefix: description: - The prefix for host variables names coming from AWS. @@ -56,7 +52,9 @@ - inventory_cache - constructed - amazon.aws.boto3 - - amazon.aws.aws_credentials + - amazon.aws.common.plugins + - amazon.aws.region.plugins + - amazon.aws.assume_role.plugins author: - Sloane Hertel (@s-hertel) ''' @@ -84,15 +82,13 @@ from ansible.errors import AnsibleError from ansible.module_utils._text import to_native -from ansible.module_utils.basic import missing_required_lib -from ansible_collections.amazon.aws.plugins.plugin_utils.inventory import AWSInventoryBase +from ansible.module_utils.common.dict_transformations import camel_dict_to_snake_dict -from ansible.template import Templar from ansible_collections.amazon.aws.plugins.module_utils.core import is_boto3_error_code -from ansible_collections.amazon.aws.plugins.module_utils.ec2 import HAS_BOTO3 -from ansible_collections.amazon.aws.plugins.module_utils.ec2 import ansible_dict_to_boto3_filter_list -from ansible_collections.amazon.aws.plugins.module_utils.ec2 import boto3_tag_list_to_ansible_dict -from ansible_collections.amazon.aws.plugins.module_utils.ec2 import camel_dict_to_snake_dict +from ansible_collections.amazon.aws.plugins.module_utils.transformation import ansible_dict_to_boto3_filter_list +from ansible_collections.amazon.aws.plugins.module_utils.tagging import boto3_tag_list_to_ansible_dict + +from ansible_collections.amazon.aws.plugins.plugin_utils.inventory import AWSInventoryBase def _find_hosts_with_valid_statuses(hosts, statuses): @@ -133,7 +129,7 @@ def _add_tags_for_rds_hosts(connection, hosts, strict): def describe_resource_with_tags(func): - def describe_wrapper(connection, strict, filters): + def describe_wrapper(connection, filters, strict=False): try: results = func(connection=connection, filters=filters) if 'DBInstances' in results: @@ -143,11 +139,11 @@ def describe_wrapper(connection, strict, filters): _add_tags_for_rds_hosts(connection, results, strict) except is_boto3_error_code('AccessDenied') as e: # pylint: disable=duplicate-except if not strict: - results = [] - else: - raise AnsibleError("Failed to query RDS: {0}".format(to_native(e))) + return [] + raise AnsibleError("Failed to query RDS: {0}".format(to_native(e))) except (botocore.exceptions.BotoCoreError, botocore.exceptions.ClientError) as e: # pylint: disable=duplicate-except raise AnsibleError("Failed to query RDS: {0}".format(to_native(e))) + return results return describe_wrapper @@ -169,7 +165,7 @@ class InventoryModule(AWSInventoryBase): NAME = 'amazon.aws.aws_rds' def __init__(self): - super(InventoryModule, self).__init__() + super().__init__() self.credentials = {} def _populate(self, hosts): @@ -245,7 +241,7 @@ def verify_file(self, path): :param path: the path to the inventory config file :return the contents of the config file ''' - if super(InventoryModule, self).verify_file(path): + if super().verify_file(path): if path.endswith(('aws_rds.yml', 'aws_rds.yaml')): return True return False @@ -262,10 +258,10 @@ def _get_all_db_hosts(self, regions, instance_filters, cluster_filters, strict, all_instances = [] all_clusters = [] - for connection, _region in self._boto3_conn(regions, "rds"): - all_instances += _describe_db_instances(connection, strict, instance_filters) + for connection, _region in self.all_clients("rds"): + all_instances += _describe_db_instances(connection, instance_filters, strict=strict) if gather_clusters: - all_clusters += _describe_db_clusters(connection, strict, cluster_filters) + all_clusters += _describe_db_clusters(connection, cluster_filters, strict=strict) sorted_hosts = list( sorted(all_instances, key=lambda x: x['DBInstanceIdentifier']) + sorted(all_clusters, key=lambda x: x['DBClusterIdentifier']) @@ -273,14 +269,7 @@ def _get_all_db_hosts(self, regions, instance_filters, cluster_filters, strict, return _find_hosts_with_valid_statuses(sorted_hosts, statuses) def parse(self, inventory, loader, path, cache=True): - super(InventoryModule, self).parse(inventory, loader, path) - - if not HAS_BOTO3: - self.fail_aws(missing_required_lib('botocore and boto3')) - - self._read_config_data(path) - - self._set_credentials(loader) + super().parse(inventory, loader, path, cache=cache) # get user specifications regions = self.get_option('regions') diff --git a/plugins/plugin_utils/base.py b/plugins/plugin_utils/base.py index 8491c085883..057548a3f29 100644 --- a/plugins/plugin_utils/base.py +++ b/plugins/plugin_utils/base.py @@ -34,18 +34,20 @@ def _do_fail(self, message): def fail_aws(self, message, exception=None): if not exception: self._do_fail(to_native(message)) - self._do_fail("{0}: {1}".format(message, to_native(exception))) + self._do_fail(f"{message}: {to_native(exception)}") - def client(self, service, retry_decorator=None): + def client(self, service, retry_decorator=None, **params): region, endpoint_url, aws_connect_kwargs = get_aws_connection_info(self) - conn = boto3_conn(self, conn_type='client', resource=service, - region=region, endpoint=endpoint_url, **aws_connect_kwargs) + kw_args = dict(region=region, endpoint=endpoint_url, **aws_connect_kwargs) + kw_args.update(params) + conn = boto3_conn(self, conn_type='client', resource=service, **kw_args) return conn if retry_decorator is None else RetryingBotoClientWrapper(conn, retry_decorator) - def resource(self, service): + def resource(self, service, **params): region, endpoint_url, aws_connect_kwargs = get_aws_connection_info(self) - return boto3_conn(self, conn_type='resource', resource=service, - region=region, endpoint=endpoint_url, **aws_connect_kwargs) + kw_args = dict(region=region, endpoint=endpoint_url, **aws_connect_kwargs) + kw_args.update(params) + return boto3_conn(self, conn_type='resource', resource=service, **kw_args) @property def region(self): diff --git a/plugins/plugin_utils/inventory.py b/plugins/plugin_utils/inventory.py index a31db8077cf..a5a96e51315 100644 --- a/plugins/plugin_utils/inventory.py +++ b/plugins/plugin_utils/inventory.py @@ -4,20 +4,18 @@ from __future__ import (absolute_import, division, print_function) __metaclass__ = type - try: import boto3 import botocore except ImportError: pass # will be captured by imported HAS_BOTO3 -from ansible_collections.amazon.aws.plugins.module_utils.core import is_boto3_error_code -from ansible.module_utils._text import to_native -from ansible.template import Templar from ansible.plugins.inventory import BaseInventoryPlugin from ansible.plugins.inventory import Cacheable from ansible.plugins.inventory import Constructable -from ansible_collections.amazon.aws.plugins.plugin_utils.botocore import boto3_conn + +from ansible_collections.amazon.aws.plugins.module_utils.core import is_boto3_error_code +from ansible_collections.amazon.aws.plugins.plugin_utils.botocore import AnsibleBotocoreError from ansible_collections.amazon.aws.plugins.plugin_utils.base import AWSPluginBase @@ -29,148 +27,145 @@ def _boto3_session(profile_name=None): class AWSInventoryBase(BaseInventoryPlugin, Constructable, Cacheable, AWSPluginBase): + class TemplatedOptions: + # When someone looks up the TEMPLATABLE_OPTIONS using get() any templates + # will be templated using the loader passed to parse. + TEMPLATABLE_OPTIONS = ( + "access_key", "secret_key", "session_token", "profile", "iam_role_name", + ) + + def __init__(self, templar, options): + self.original_options = options + self.templar = templar + + def __getitem__(self, *args): + return self.original_options.__getitem__(self, *args) + + def __setitem__(self, *args): + return self.original_options.__setitem__(self, *args) + + def get(self, *args): + value = self.original_options.get(*args) + if not value: + return value + if args[0] not in self.TEMPLATABLE_OPTIONS: + return value + if not self.templar.is_template(value): + return value + + return self.templar.template(variable=value, disable_lookups=False) + + def get_options(self, *args): + original_options = super().get_options(*args) + if not self.templar: + return original_options + return self.TemplatedOptions(self.templar, original_options) + def __init__(self): + super().__init__() + self._frozen_credentials = {} + + def parse(self, inventory, loader, path, cache=True, botocore_version=None, boto3_version=None): + super().parse(inventory, loader, path) + self.require_aws_sdk(botocore_version=botocore_version, boto3_version=boto3_version) + self._read_config_data(path) + self._set_frozen_credentials() + + def client(self, *args, **kwargs): + kw_args = dict(self._frozen_credentials) + kw_args.update(kwargs) + return super().client(*args, **kw_args) + + def resource(self, *args, **kwargs): + kw_args = dict(self._frozen_credentials) + kw_args.update(kwargs) + return super().resource(*args, **kw_args) + + def _freeze_iam_role(self, iam_role_arn): + if hasattr(self, "ansible_name"): + role_session_name = f"ansible_aws_{self.ansible_name}_dynamic_inventory" + else: + role_session_name = "ansible_aws_dynamic_inventory" + assume_params = {"RoleArn": iam_role_arn, "RoleSessionName": role_session_name} - super(AWSInventoryBase, self).__init__() - - self.boto_profile = None - self.aws_secret_access_key = None - self.aws_access_key_id = None - self.aws_security_token = None - self.iam_role_arn = None - - def _get_credentials(self): - ''' - :return A dictionary of boto client credentials - ''' - boto_params = { - 'aws_access_key_id': self.aws_access_key_id, - 'aws_secret_access_key': self.aws_secret_access_key, - 'aws_session_token': self.aws_security_token, + try: + sts = self.client("sts") + assumed_role = sts.assume_role(**assume_params) + except AnsibleBotocoreError as e: + self.fail_aws(f"Unable to assume role {iam_role_arn}", exception=e) + + credentials = assumed_role.get("Credentials") + if not credentials: + self.fail_aws(f"Unable to assume role {iam_role_arn}") + + self._frozen_credentials = { + "profile_name": None, + "aws_access_key_id": credentials.get("AccessKeyId"), + "aws_secret_access_key": credentials.get("SecretAccessKey"), + "aws_session_token": credentials.get("SessionToken"), } - return {k: v for k, v in boto_params.items() if v} - - def _set_credentials(self, loader): - ''' - :param config_data: contents of the inventory config file - ''' - - templar = Templar(loader=loader) - credentials = {} - - for credential_type in ('aws_profile', 'aws_access_key', 'aws_secret_key', 'aws_security_token', 'iam_role_arn'): - if templar.is_template(self.get_option(credential_type)): - credentials[credential_type] = templar.template(variable=self.get_option(credential_type), disable_lookups=False) - else: - credentials[credential_type] = self.get_option(credential_type) - - self.boto_profile = credentials['aws_profile'] - self.aws_access_key_id = credentials['aws_access_key'] - self.aws_secret_access_key = credentials['aws_secret_key'] - self.aws_security_token = credentials['aws_security_token'] - self.iam_role_arn = credentials['iam_role_arn'] - - if not self.boto_profile and not (self.aws_access_key_id and self.aws_secret_access_key): - session = botocore.session.get_session() - try: - credentials = session.get_credentials().get_frozen_credentials() - except AttributeError: - pass - else: - self.aws_access_key_id = credentials.access_key - self.aws_secret_access_key = credentials.secret_key - self.aws_security_token = credentials.token - - if not self.boto_profile and not (self.aws_access_key_id and self.aws_secret_access_key): - self.fail_aws("Insufficient boto credentials found. Please provide them in your " - "inventory configuration file or set them as environment variables.") - - def _get_connection(self, credentials, resource, region='us-east-1'): - return boto3_conn(self, conn_type='client', resource=resource, region=region, **credentials) - - def _boto3_assume_role(self, credentials, resource, iam_role_arn, profile_name, region=None): - """ - Assume an IAM role passed by iam_role_arn parameter - :return: a dict containing the credentials of the assumed role - """ + def _set_frozen_credentials(self): + options = self.get_options() + iam_role_arn = options.get("assume_role_arn") + if iam_role_arn: + self._freeze_iam_role(iam_role_arn) + def _describe_regions(self, service): + # Try pulling a list of regions from the service try: - sts_connection = _boto3_session(profile_name=profile_name).client('sts', region, **credentials) - role_session_name = f"ansible_aws_{resource}_dynamic_inventory" - sts_session = sts_connection.assume_role(RoleArn=iam_role_arn, RoleSessionName=role_session_name) - return dict( - aws_access_key_id=sts_session['Credentials']['AccessKeyId'], - aws_secret_access_key=sts_session['Credentials']['SecretAccessKey'], - aws_session_token=sts_session['Credentials']['SessionToken'] - ) - except botocore.exceptions.ClientError as e: - self.fail_aws("Unable to assume IAM role: %s" % to_native(e)) - - def _boto3_regions(self, credentials, iam_role_arn, resource): - - regions = [] - if iam_role_arn is not None: - try: - # Describe regions assuming arn role - assumed_credentials = self._boto3_assume_role(credentials, resource, iam_role_arn, self.boto_profile) - client = self._get_connection(credentials=assumed_credentials, resource='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 + client = self.client(service) + resp = client.describe_regions() + except AttributeError: + # Not all clients support describe + pass + except is_boto3_error_code("UnauthorizedOperation"): + self.warn(f"UnauthorizedOperation when trying to list {service} regions") + except botocore.exceptions.NoRegionError: + self.warn(f"NoRegionError when trying to list {service} regions") + except (botocore.exceptions.BotoCoreError, botocore.exceptions.ClientError) as e: + self.warn(f"Unexpected error while trying to list {service} regions: {e}") else: - try: - # as per https://boto3.amazonaws.com/v1/documentation/api/latest/guide/ec2-example-regions-avail-zones.html - client = self._get_connection(credentials=credentials, resource='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 - except is_boto3_error_code('UnauthorizedOperation') as e: # pylint: disable=duplicate-except - self.fail_aws("Unauthorized operation: %s" % to_native(e)) + regions = [x["RegionName"] for x in resp.get("Regions", [])] + if regions: + return regions + return None + + def _boto3_regions(self, service): + options = self.get_options() + + if options.get("regions"): + return options.get("regions") + + # boto3 has hard coded lists of available regions for resources, however this does bit-rot + # As such we try to query the service, and fall back to ec2 for a list of regions + for resource_type in list({service, "ec2"}): + regions = self._describe_regions(resource_type) + if regions: + return regions # fallback to local list hardcoded in boto3 if still no regions + session = _boto3_session(options.get('profile')) + regions = session.get_available_regions(service) + if not regions: - session = _boto3_session() - regions = session.get_available_regions(resource) + # I give up, now you MUST give me regions + self.fail_aws('Unable to get regions list from available methods, you must specify the "regions" option to continue.') return regions - def _get_boto3_connection(self, credentials, iam_role_arn, resource, region): - try: - assumed_credentials = credentials - if iam_role_arn is not None: - assumed_credentials = self._boto3_assume_role(credentials, resource, iam_role_arn, self.boto_profile, region) - return _boto3_session(profile_name=self.boto_profile).client(resource, region, **assumed_credentials) - except (botocore.exceptions.ProfileNotFound, botocore.exceptions.PartialCredentialsError) as e: - if self.boto_profile: - try: - return _boto3_session(profile_name=self.boto_profile).client(resource, region) - except (botocore.exceptions.ProfileNotFound, botocore.exceptions.PartialCredentialsError) as e: - self.fail_aws("Insufficient credentials found: %s" % to_native(e)) - else: - self.fail_aws("Insufficient credentials found: %s" % to_native(e)) - - def _boto3_conn(self, regions, resource): - ''' - :param regions: A list of regions to create a boto3 client - + def all_clients(self, service): + """ Generator that yields a boto3 client and the region - ''' - credentials = self._get_credentials() - iam_role_arn = self.iam_role_arn - if not regions: - # list regions as none was provided - regions = self._boto3_regions(credentials, iam_role_arn, resource) + :param service: The boto3 service to connect to. - # I give up, now you MUST give me regions - if not regions: - self.fail_aws('Unable to get regions list from available methods, you must specify the "regions" option to continue.') + Note: For services which don't support 'DescribeRegions' this may include bad + endpoints, and as such EndpointConnectionError should be cleanly handled as a non-fatal + error. + """ + regions = self._boto3_regions(service=service) for region in regions: - connection = self._get_boto3_connection(credentials, iam_role_arn, resource, region) + connection = self.client(service, region=region) yield connection, region diff --git a/tests/unit/plugin_utils/base/test_plugin.py b/tests/unit/plugin_utils/base/test_plugin.py index f168cb4deff..d7acb652471 100644 --- a/tests/unit/plugin_utils/base/test_plugin.py +++ b/tests/unit/plugin_utils/base/test_plugin.py @@ -120,6 +120,16 @@ def test_client_wrapper(monkeypatch): region=sentinel.CONN_REGION, endpoint=sentinel.CONN_URL,) + # Check that we can override parameters + wrapped_conn = base_plugin.client(sentinel.PARAM_SERVICE, sentinel.PARAM_WRAPPER, region=sentinel.PARAM_REGION) + assert wrapped_conn.client is sentinel.BOTO3_CONN + assert wrapped_conn.retry is sentinel.PARAM_WRAPPER + assert get_aws_connection_info.call_args == call(base_plugin) + assert boto3_conn.call_args == call(base_plugin, conn_type='client', + resource=sentinel.PARAM_SERVICE, + region=sentinel.PARAM_REGION, + endpoint=sentinel.CONN_URL,) + def test_resource(monkeypatch): get_aws_connection_info = MagicMock(name='get_aws_connection_info') @@ -137,3 +147,10 @@ def test_resource(monkeypatch): resource=sentinel.PARAM_SERVICE, region=sentinel.CONN_REGION, endpoint=sentinel.CONN_URL,) + + assert base_plugin.resource(sentinel.PARAM_SERVICE, region=sentinel.PARAM_REGION) is sentinel.BOTO3_CONN + assert get_aws_connection_info.call_args == call(base_plugin) + assert boto3_conn.call_args == call(base_plugin, conn_type='resource', + resource=sentinel.PARAM_SERVICE, + region=sentinel.PARAM_REGION, + endpoint=sentinel.CONN_URL,) diff --git a/tests/unit/plugin_utils/inventory/test_inventory.py b/tests/unit/plugin_utils/inventory/test_inventory.py deleted file mode 100644 index 5c76c9e3b83..00000000000 --- a/tests/unit/plugin_utils/inventory/test_inventory.py +++ /dev/null @@ -1,143 +0,0 @@ -# -*- coding: utf-8 -*- - -# Copyright 2017 Sloane Hertel -# -# This file is part of Ansible -# -# Ansible is free software: you can redistribute it and/or modify -# it under the terms of the GNU General Public License as published by -# the Free Software Foundation, either version 3 of the License, or -# (at your option) any later version. -# -# Ansible is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with Ansible. If not, see . - -# Make coding more python3-ish -import pytest -from unittest.mock import MagicMock, patch, call - -from ansible.errors import AnsibleError -from ansible.parsing.dataloader import DataLoader -from ansible_collections.amazon.aws.plugins.plugin_utils.inventory import AWSInventoryBase - - -@pytest.fixture() -def inventory(): - inventory = AWSInventoryBase() - inventory._options = { - "aws_profile": "first_precedence", - "aws_access_key": "test_access_key", - "aws_secret_key": "test_secret_key", - "aws_security_token": "test_security_token", - "iam_role_arn": None, - "use_contrib_script_compatible_ec2_tag_keys": False, - "hostvars_prefix": "", - "hostvars_suffix": "", - "strict": True, - "compose": {}, - "groups": {}, - "keyed_groups": [], - "regions": ["us-east-1"], - "filters": [], - "include_filters": [], - "exclude_filters": [], - "hostnames": [], - "strict_permissions": False, - "allow_duplicated_hosts": False, - "cache": False, - "include_extra_api_calls": False, - "use_contrib_script_compatible_sanitization": False, - } - inventory.inventory = MagicMock() - return inventory - - -@pytest.fixture() -def loader(): - return DataLoader() - - -def test_boto3_conn(inventory, loader): - inventory._options = {"aws_profile": "first_precedence", - "aws_access_key": "test_access_key", - "aws_secret_key": "test_secret_key", - "aws_security_token": "test_security_token", - "iam_role_arn": None} - inventory._set_credentials(loader) - with pytest.raises(AnsibleError) as error_message: - for _connection, _region in inventory._boto3_conn(regions=['us-east-1'], resource="test"): - assert "Insufficient credentials found" in error_message - - -def test_set_credentials(inventory, loader): - inventory._options = {'aws_access_key': 'test_access_key', - 'aws_secret_key': 'test_secret_key', - 'aws_security_token': 'test_security_token', - 'aws_profile': 'test_profile', - 'iam_role_arn': 'arn:aws:iam::123456789012:role/test-role'} - inventory._set_credentials(loader) - - assert inventory.boto_profile == "test_profile" - assert inventory.aws_access_key_id == "test_access_key" - assert inventory.aws_secret_access_key == "test_secret_key" - assert inventory.aws_security_token == "test_security_token" - assert inventory.iam_role_arn == "arn:aws:iam::123456789012:role/test-role" - - -def test_insufficient_credentials(inventory, loader): - inventory._options = { - 'aws_access_key': None, - 'aws_secret_key': None, - 'aws_security_token': None, - 'aws_profile': None, - 'iam_role_arn': None - } - with pytest.raises(AnsibleError) as error_message: - inventory._set_credentials(loader) - assert "Insufficient credentials found" in error_message - - -@pytest.mark.skip(reason="skipping for now, will be reactivated later") -@pytest.mark.parametrize("hasregions", [False]) -def test_boto3_conn(inventory, hasregions): - - credentials = MagicMock() - iam_role_arn = MagicMock() - resource = MagicMock() - - inventory._get_credentials = MagicMock() - inventory._get_credentials.return_value = credentials - inventory.iam_role_arn = iam_role_arn - - regions = [] - if hasregions: - regions = ["us-east-1", "us-west-1", "eu-east-1"] - - inventory._boto3_regions = MagicMock() - inventory._boto3_regions.return_value = regions - - inventory.fail_aws = MagicMock() - inventory.fail_aws.side_effect = SystemExit(1) - - connections = [MagicMock(name="connection_%d" % c) for c in range(len(regions))] - - inventory._get_boto3_connection = MagicMock() - inventory._get_boto3_connection.side_effect = connections - - if hasregions: - assert list(inventory._boto3_conn(regions, resource)) == [(connections[i], regions[i]) for i in range(len(regions))] - else: - result = inventory._boto3_conn(regions, resource) - - inventory._get_credentials.assert_called_once() - inventory._boto3_regions.assert_called_with(credentials, iam_role_arn, resource) - if hasregions: - inventory._get_boto3_connection.assert_has_calls( - [call(credentials, iam_role_arn, resource, region) for region in regions], - any_order=True - ) diff --git a/tests/unit/plugin_utils/inventory/test_inventory_base.py b/tests/unit/plugin_utils/inventory/test_inventory_base.py new file mode 100644 index 00000000000..14bde838775 --- /dev/null +++ b/tests/unit/plugin_utils/inventory/test_inventory_base.py @@ -0,0 +1,31 @@ +# (c) 2022 Red Hat Inc. +# +# This file is part of Ansible +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + +from unittest.mock import call +from unittest.mock import MagicMock +from unittest.mock import patch +from unittest.mock import sentinel + +import ansible_collections.amazon.aws.plugins.plugin_utils.inventory as utils_inventory + + +@patch("ansible.plugins.inventory.BaseInventoryPlugin.parse", MagicMock) +def test_parse(monkeypatch): + require_aws_sdk = MagicMock(name='require_aws_sdk') + require_aws_sdk.return_value = sentinel.RETURNED_SDK + config_data = MagicMock(name='_read_config_data') + config_data.return_value = sentinel.RETURNED_OPTIONS + frozen_credentials = MagicMock(name='_set_frozen_credentials') + frozen_credentials.return_value = sentinel.RETURNED_CREDENTIALS + + inventory_plugin = utils_inventory.AWSInventoryBase() + monkeypatch.setattr(inventory_plugin, 'require_aws_sdk', require_aws_sdk) + monkeypatch.setattr(inventory_plugin, '_read_config_data', config_data) + monkeypatch.setattr(inventory_plugin, '_set_frozen_credentials', frozen_credentials) + + inventory_plugin.parse(sentinel.PARAM_INVENTORY, sentinel.PARAM_LOADER, sentinel.PARAM_PATH) + assert require_aws_sdk.call_args == call(botocore_version=None, boto3_version=None) + assert config_data.call_args == call(sentinel.PARAM_PATH) + assert frozen_credentials.call_args == call() diff --git a/tests/unit/plugin_utils/inventory/test_inventory_clients.py b/tests/unit/plugin_utils/inventory/test_inventory_clients.py new file mode 100644 index 00000000000..0aac20808d7 --- /dev/null +++ b/tests/unit/plugin_utils/inventory/test_inventory_clients.py @@ -0,0 +1,112 @@ +# (c) 2022 Red Hat Inc. +# +# This file is part of Ansible +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + +from unittest.mock import call +from unittest.mock import MagicMock +from unittest.mock import sentinel + +import ansible_collections.amazon.aws.plugins.plugin_utils.inventory as utils_inventory +import ansible_collections.amazon.aws.plugins.plugin_utils.base as utils_base +# import ansible_collections.amazon.aws.plugins.module_utils. + + +def test_client(monkeypatch): + super_client = MagicMock(name="client") + super_client.return_value = sentinel.SUPER_CLIENT + monkeypatch.setattr(utils_base.AWSPluginBase, "client", super_client) + inventory_plugin = utils_inventory.AWSInventoryBase() + + client = inventory_plugin.client(sentinel.SERVICE_NAME) + assert super_client.call_args == call(sentinel.SERVICE_NAME) + assert client is sentinel.SUPER_CLIENT + + client = inventory_plugin.client(sentinel.SERVICE_NAME, extra_arg=sentinel.EXTRA_ARG) + assert super_client.call_args == call(sentinel.SERVICE_NAME, extra_arg=sentinel.EXTRA_ARG) + assert client is sentinel.SUPER_CLIENT + + frozen_creds = {"credential_one": sentinel.CREDENTIAL_ONE} + inventory_plugin._frozen_credentials = frozen_creds + + client = inventory_plugin.client(sentinel.SERVICE_NAME) + assert super_client.call_args == call( + sentinel.SERVICE_NAME, + credential_one=sentinel.CREDENTIAL_ONE + ) + assert client is sentinel.SUPER_CLIENT + + client = inventory_plugin.client(sentinel.SERVICE_NAME, extra_arg=sentinel.EXTRA_ARG) + assert super_client.call_args == call( + sentinel.SERVICE_NAME, + credential_one=sentinel.CREDENTIAL_ONE, + extra_arg=sentinel.EXTRA_ARG + ) + assert client is sentinel.SUPER_CLIENT + + client = inventory_plugin.client(sentinel.SERVICE_NAME, credential_one=sentinel.CREDENTIAL_ARG) + assert super_client.call_args == call( + sentinel.SERVICE_NAME, + credential_one=sentinel.CREDENTIAL_ARG, + ) + assert client is sentinel.SUPER_CLIENT + + +def test_resource(monkeypatch): + super_resource = MagicMock(name="resource") + super_resource.return_value = sentinel.SUPER_RESOURCE + monkeypatch.setattr(utils_base.AWSPluginBase, "resource", super_resource) + inventory_plugin = utils_inventory.AWSInventoryBase() + + resource = inventory_plugin.resource(sentinel.SERVICE_NAME) + assert super_resource.call_args == call(sentinel.SERVICE_NAME) + assert resource is sentinel.SUPER_RESOURCE + + resource = inventory_plugin.resource(sentinel.SERVICE_NAME, extra_arg=sentinel.EXTRA_ARG) + assert super_resource.call_args == call(sentinel.SERVICE_NAME, extra_arg=sentinel.EXTRA_ARG) + assert resource is sentinel.SUPER_RESOURCE + + frozen_creds = {"credential_one": sentinel.CREDENTIAL_ONE} + inventory_plugin._frozen_credentials = frozen_creds + + resource = inventory_plugin.resource(sentinel.SERVICE_NAME) + assert super_resource.call_args == call( + sentinel.SERVICE_NAME, + credential_one=sentinel.CREDENTIAL_ONE + ) + assert resource is sentinel.SUPER_RESOURCE + + resource = inventory_plugin.resource(sentinel.SERVICE_NAME, extra_arg=sentinel.EXTRA_ARG) + assert super_resource.call_args == call( + sentinel.SERVICE_NAME, + credential_one=sentinel.CREDENTIAL_ONE, + extra_arg=sentinel.EXTRA_ARG + ) + assert resource is sentinel.SUPER_RESOURCE + + resource = inventory_plugin.resource(sentinel.SERVICE_NAME, credential_one=sentinel.CREDENTIAL_ARG) + assert super_resource.call_args == call( + sentinel.SERVICE_NAME, + credential_one=sentinel.CREDENTIAL_ARG, + ) + assert resource is sentinel.SUPER_RESOURCE + + +def test_all_clients(monkeypatch): + test_regions = ['us-east-1', 'us-east-2'] + inventory_plugin = utils_inventory.AWSInventoryBase() + mock_client = MagicMock(name="client") + mock_client.return_value = sentinel.RETURN_CLIENT + monkeypatch.setattr(inventory_plugin, "client", mock_client) + boto3_regions = MagicMock(name="_boto3_regions") + boto3_regions.return_value = test_regions + monkeypatch.setattr(inventory_plugin, "_boto3_regions", boto3_regions) + + regions = [] + for (client, region) in inventory_plugin.all_clients(sentinel.ARG_SERVICE): + assert boto3_regions.call_args == call(service=sentinel.ARG_SERVICE) + assert mock_client.call_args == call(sentinel.ARG_SERVICE, region=region) + assert client is sentinel.RETURN_CLIENT + regions.append(region) + + assert set(regions) == set(test_regions) diff --git a/tests/unit/plugins/inventory/test_aws_ec2.py b/tests/unit/plugins/inventory/test_aws_ec2.py index f1daa144ef0..cc39f95bb18 100644 --- a/tests/unit/plugins/inventory/test_aws_ec2.py +++ b/tests/unit/plugins/inventory/test_aws_ec2.py @@ -473,8 +473,8 @@ def test_inventory_get_instances_by_region(m_describe_ec2_instances, inventory, (MagicMock(), "us-east-1"), (MagicMock(), "us-east-2") ] - inventory._boto3_conn = MagicMock() - inventory._boto3_conn.return_value = boto3_conn + inventory.all_clients = MagicMock() + inventory.all_clients.return_value = boto3_conn m_describe_ec2_instances.side_effect = [ { @@ -526,7 +526,7 @@ def test_inventory_get_instances_by_region(m_describe_ec2_instances, inventory, regions = ["us-east-2", "us-east-4"] assert inventory._get_instances_by_region(regions, filters, False) == expected - inventory._boto3_conn.assert_called_with(regions, "ec2") + inventory.all_clients.assert_called_with("ec2") if any((f['Name'] == 'instance-state-name' for f in filters)): filters.append(default_filter) @@ -551,7 +551,7 @@ def test_inventory_get_instances_by_region(m_describe_ec2_instances, inventory, ), botocore.exceptions.ClientError( { - 'Error': {'Code': 1, 'Message': 'Something went wrong'}, + 'Error': {'Code': 'UnauthorizedOperation', 'Message': 'Something went wrong'}, 'ResponseMetadata': { 'HTTPStatusCode': 403 } @@ -564,8 +564,8 @@ def test_inventory_get_instances_by_region(m_describe_ec2_instances, inventory, @patch("ansible_collections.amazon.aws.plugins.inventory.aws_ec2._describe_ec2_instances") def test_inventory_get_instances_by_region_failures(m_describe_ec2_instances, inventory, strict, error): - inventory._boto3_conn = MagicMock() - inventory._boto3_conn.return_value = [(MagicMock(), "us-west-2")] + inventory.all_clients = MagicMock() + inventory.all_clients.return_value = [(MagicMock(), "us-west-2")] inventory.fail_aws = MagicMock() inventory.fail_aws.side_effect = SystemExit(1) diff --git a/tests/unit/plugins/inventory/test_aws_rds.py b/tests/unit/plugins/inventory/test_aws_rds.py index 2a2d0fb8b5b..a915df5c050 100644 --- a/tests/unit/plugins/inventory/test_aws_rds.py +++ b/tests/unit/plugins/inventory/test_aws_rds.py @@ -17,11 +17,13 @@ # You should have received a copy of the GNU General Public License # along with Ansible. If not, see . +import copy import pytest -from unittest.mock import MagicMock, patch, call, ANY import random import string -import copy +from unittest.mock import MagicMock +from unittest.mock import call +from unittest.mock import patch try: import botocore @@ -30,6 +32,7 @@ pass from ansible.errors import AnsibleError +from ansible_collections.amazon.aws.plugins.module_utils.botocore import HAS_BOTO3 from ansible_collections.amazon.aws.plugins.inventory.aws_rds import ( InventoryModule, _find_hosts_with_valid_statuses, @@ -37,7 +40,6 @@ _add_tags_for_rds_hosts, _describe_db_clusters, _describe_db_instances, - HAS_BOTO3, ansible_dict_to_boto3_filter_list, ) @@ -66,7 +68,7 @@ def inventory(): inventory.inventory = MagicMock() inventory._populate_host_vars = MagicMock() - inventory._boto3_conn = MagicMock() + inventory.all_clients = MagicMock() inventory.get_option = MagicMock() inventory._set_composite_vars = MagicMock() @@ -449,7 +451,7 @@ def test_inventory_get_all_db_hosts(m_find_hosts, m_describe_db_clusters, m_desc connections = [MagicMock() for i in range(regions)] - inventory._boto3_conn.return_value = [ + inventory.all_clients.return_value = [ (connections[i], "us-east-%d" % i) for i in range(regions) ] @@ -471,14 +473,14 @@ def test_inventory_get_all_db_hosts(m_find_hosts, m_describe_db_clusters, m_desc m_find_hosts.return_value = result assert result == inventory._get_all_db_hosts(**params) - inventory._boto3_conn.assert_called_with(params["regions"], "rds") + inventory.all_clients.assert_called_with("rds") m_describe_db_instances.assert_has_calls( - [call(connections[i], params["strict"], params["instance_filters"]) for i in range(regions)] + [call(connections[i], params["instance_filters"], strict=params["strict"]) for i in range(regions)] ) if gather_clusters: m_describe_db_clusters.assert_has_calls( - [call(connections[i], params["strict"], params["cluster_filters"]) for i in range(regions)] + [call(connections[i], params["cluster_filters"], strict=params["strict"]) for i in range(regions)] ) m_find_hosts.assert_called_with(result, params["statuses"]) @@ -625,23 +627,7 @@ def _get_option_side_effect(x): ) -MOCK_HAS_BOTO3 = "ansible_collections.amazon.aws.plugins.inventory.aws_rds.HAS_BOTO3" BASE_INVENTORY_PARSE = "ansible_collections.amazon.aws.plugins.inventory.aws_rds.AWSInventoryBase.parse" -MISSING_REQUIRED_LIB = "ansible_collections.amazon.aws.plugins.inventory.aws_rds.missing_required_lib" - - -@patch(MISSING_REQUIRED_LIB) -@patch(BASE_INVENTORY_PARSE) -@patch(MOCK_HAS_BOTO3, False) -def test_inventory_parse_with_missing_boto3(m_parse, m_missing_required_lib, inventory): - - loader = MagicMock() - path = generate_random_string(with_punctuation=False, with_digits=False) - m_missing_required_lib.side_effect = lambda x: f"Failed to import the required Python library ({x})" - - with pytest.raises(AnsibleError) as err: - inventory.parse(MagicMock(), loader, path) - assert "Failed to import the required Python library (botocore and boto3)" in err @pytest.mark.parametrize("include_clusters", [True, False]) @@ -701,9 +687,7 @@ def get_option_side_effect(v): inventory.parse(inventory_data, loader, path, cache) - m_parse.assert_called_with(inventory_data, loader, path) - inventory._read_config_data.assert_called_with(path) - inventory._set_credentials.assert_called_with(ANY) + m_parse.assert_called_with(inventory_data, loader, path, cache=cache) boto3_instance_filters = ansible_dict_to_boto3_filter_list(options["filters"]) boto3_cluster_filters = []