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 <None>
Reviewed-by: Mark Chappell <None>

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).

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"]
+"""

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()

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,25 +258,18 @@ 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'])
         )
         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')

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):