Refacterization of code to use 'fallback' instead of using os.environ (…

…ansible-collections#1174) Refacterization of code to use 'fallback' instead of using os.environ SUMMARY When the "ec2" module was first added, it included support for pulling credentials out of the environment variables. 3.5 years later, in Ansible 2.1, support for configuring a 'fallback' directly in the argument specification was added, but we never got around to using it. This PR finally cleans up the code and lets the argument parser directly handle fallback to environment variables. ISSUE TYPE Feature Pull Request COMPONENT NAME plugins/module_utils/botocore.py plugins/module_utils/modules.py ADDITIONAL INFORMATION Reviewed-by: Gonéri Le Bouder <[email protected]>
abikouo · Oct 17, 2022 · 9401932 · 9401932
1 parent e08acb8
commit 9401932
Show file tree

Hide file tree

Showing 4 changed files with 36 additions and 75 deletions.
diff --git a/changelogs/fragments/1174-module_params.yml b/changelogs/fragments/1174-module_params.yml
@@ -0,0 +1,2 @@
+minor_changes:
+- amazon.aws collection - refacterization of code to use argument specification ``fallback`` when falling back to environment variables for security credentials and AWS connection details (https://github.com/ansible-collections/amazon.aws/pull/1174).
diff --git a/plugins/doc_fragments/aws.py b/plugins/doc_fragments/aws.py
@@ -120,9 +120,11 @@ class ModuleDocFragment(object):
     type: bool
     default: false
 notes:
-  - B(Caution:) Environment variables and configuration files are read from the
-    Ansible 'host' context and not the 'controller' context.  Files may need to
-    be explicitly copied to the 'host'.
+  - B(Caution:) For modules, environment variables and configuration files are
+    read from the Ansible 'host' context and not the 'controller' context.
+    As such, files may need to be explicitly copied to the 'host'.  For lookup
+    and connection plugins, environment variables and configuration files are
+    read from the Ansible 'controller' context and not the 'host' context.
   - The AWS SDK (boto3) that Ansible uses may also read defaults for credentials
     and other settings, such as the region, from its configuration files in the
     Ansible 'host' context (typically C(~/.aws/credentials)).

diff --git a/plugins/module_utils/botocore.py b/plugins/module_utils/botocore.py
@@ -146,16 +146,11 @@ def get_aws_region(module, boto3=None):
     if not HAS_BOTO3:
         module.fail_json(msg=missing_required_lib('boto3'), exception=BOTO3_IMP_ERR)
 
-    if 'AWS_REGION' in os.environ:
-        return os.environ['AWS_REGION']
-    if 'AWS_DEFAULT_REGION' in os.environ:
-        return os.environ['AWS_DEFAULT_REGION']
-    if 'EC2_REGION' in os.environ:
-        return os.environ['EC2_REGION']
-
     # here we don't need to make an additional call, will default to 'us-east-1' if the below evaluates to None.
     try:
-        profile_name = module.params.get('profile')
+        # Botocore doesn't like empty strings, make sure we default to None in the case of an empty
+        # string.
+        profile_name = module.params.get('profile') or None
         return botocore.session.Session(profile=profile_name).get_config_variable('region')
     except botocore.exceptions.ProfileNotFound:
         return None
@@ -176,76 +171,31 @@ def get_aws_connection_info(module, boto3=None):
     ca_bundle = module.params.get('aws_ca_bundle')
     config = module.params.get('aws_config')
 
-    # Only read the profile environment variables if we've *not* been passed
-    # any credentials as parameters.
-    if not profile_name and not access_key and not secret_key:
-        if os.environ.get('AWS_PROFILE'):
-            profile_name = os.environ.get('AWS_PROFILE')
-        if os.environ.get('AWS_DEFAULT_PROFILE'):
-            profile_name = os.environ.get('AWS_DEFAULT_PROFILE')
-
     if profile_name and (access_key or secret_key or session_token):
         module.fail_json(msg="Passing both a profile and access tokens is not supported.")
 
-    if not endpoint_url:
-        if 'AWS_URL' in os.environ:
-            endpoint_url = os.environ['AWS_URL']
-        elif 'EC2_URL' in os.environ:
-            endpoint_url = os.environ['EC2_URL']
-
+    # Botocore doesn't like empty strings, make sure we default to None in the case of an empty
+    # string.
     if not access_key:
-        # AWS_ACCESS_KEY_ID is the one supported by the AWS CLI
-        # AWS_ACCESS_KEY is to match up with our parameter name
-        if os.environ.get('AWS_ACCESS_KEY_ID'):
-            access_key = os.environ['AWS_ACCESS_KEY_ID']
-        elif os.environ.get('AWS_ACCESS_KEY'):
-            access_key = os.environ['AWS_ACCESS_KEY']
-        # Deprecated - 'EC2' implies just EC2, but is global
-        elif os.environ.get('EC2_ACCESS_KEY'):
-            access_key = os.environ['EC2_ACCESS_KEY']
-        else:
-            # in case access_key came in as empty string
-            access_key = None
-
+        access_key = None
     if not secret_key:
-        # AWS_SECRET_ACCESS_KEY is the one supported by the AWS CLI
-        # AWS_SECRET_KEY is to match up with our parameter name
-        if os.environ.get('AWS_SECRET_ACCESS_KEY'):
-            secret_key = os.environ['AWS_SECRET_ACCESS_KEY']
-        elif os.environ.get('AWS_SECRET_KEY'):
-            secret_key = os.environ['AWS_SECRET_KEY']
-        # Deprecated - 'EC2' implies just EC2, but is global
-        elif os.environ.get('EC2_SECRET_KEY'):
-            secret_key = os.environ['EC2_SECRET_KEY']
-        else:
-            # in case secret_key came in as empty string
-            secret_key = None
-
+        secret_key = None
     if not session_token:
-        # AWS_SESSION_TOKEN is supported by the AWS CLI
-        if os.environ.get('AWS_SESSION_TOKEN'):
-            session_token = os.environ['AWS_SESSION_TOKEN']
-        # Deprecated - boto
-        elif os.environ.get('AWS_SECURITY_TOKEN'):
-            session_token = os.environ['AWS_SECURITY_TOKEN']
-        # Deprecated - 'EC2' implies just EC2, but is global
-        elif os.environ.get('EC2_SECURITY_TOKEN'):
-            session_token = os.environ['EC2_SECURITY_TOKEN']
-        else:
-            # in case secret_token came in as empty string
-            session_token = None
-
-    if not ca_bundle:
-        if os.environ.get('AWS_CA_BUNDLE'):
-            ca_bundle = os.environ.get('AWS_CA_BUNDLE')
-
-    boto_params = dict(aws_access_key_id=access_key,
-                       aws_secret_access_key=secret_key,
-                       aws_session_token=session_token)
+        session_token = None
 
     if profile_name:
-        boto_params = dict(aws_access_key_id=None, aws_secret_access_key=None, aws_session_token=None)
-        boto_params['profile_name'] = profile_name
+        boto_params = dict(
+            aws_access_key_id=None,
+            aws_secret_access_key=None,
+            aws_session_token=None,
+            profile_name=profile_name,
+        )
+    else:
+        boto_params = dict(
+            aws_access_key_id=access_key,
+            aws_secret_access_key=secret_key,
+            aws_session_token=session_token,
+        )
 
     if validate_certs and ca_bundle:
         boto_params['verify'] = ca_bundle

diff --git a/plugins/module_utils/modules.py b/plugins/module_utils/modules.py
@@ -380,13 +380,15 @@ def _aws_common_argument_spec():
             deprecated_aliases=[
                 dict(name='ec2_access_key', date='2024-12-01', collection_name='amazon.aws'),
             ],
+            fallback=(env_fallback, ['AWS_ACCESS_KEY_ID', 'AWS_ACCESS_KEY', 'EC2_ACCESS_KEY']),
             no_log=False,
         ),
         secret_key=dict(
             aliases=['aws_secret_access_key', 'aws_secret_key', 'ec2_secret_key'],
             deprecated_aliases=[
                 dict(name='ec2_secret_key', date='2024-12-01', collection_name='amazon.aws'),
             ],
+            fallback=(env_fallback, ['AWS_SECRET_ACCESS_KEY', 'AWS_SECRET_KEY', 'EC2_SECRET_KEY']),
             no_log=True,
         ),
         session_token=dict(
@@ -396,10 +398,12 @@ def _aws_common_argument_spec():
                 dict(name='security_token', date='2024-12-01', collection_name='amazon.aws'),
                 dict(name='aws_security_token', date='2024-12-01', collection_name='amazon.aws'),
             ],
+            fallback=(env_fallback, ['AWS_SESSION_TOKEN', 'AWS_SECURITY_TOKEN', 'EC2_SECURITY_TOKEN']),
             no_log=True,
         ),
         profile=dict(
             aliases=['aws_profile'],
+            fallback=(env_fallback, ['AWS_PROFILE', 'AWS_DEFAULT_PROFILE']),
         ),
 
         endpoint_url=dict(
@@ -408,16 +412,18 @@ def _aws_common_argument_spec():
                 dict(name='ec2_url', date='2024-12-01', collection_name='amazon.aws'),
                 dict(name='s3_url', date='2024-12-01', collection_name='amazon.aws'),
             ],
+            fallback=(env_fallback, ['AWS_URL', 'EC2_URL', 'S3_URL']),
         ),
         validate_certs=dict(
             type='bool',
             default=True,
         ),
         aws_ca_bundle=dict(
-            type='path'
+            type='path',
+            fallback=(env_fallback, ['AWS_CA_BUNDLE']),
         ),
         aws_config=dict(
-            type='dict'
+            type='dict',
         ),
         debug_botocore_endpoint_logs=dict(
             type='bool',
@@ -437,6 +443,7 @@ def aws_argument_spec():
             deprecated_aliases=[
                 dict(name='ec2_region', date='2024-12-01', collection_name='amazon.aws'),
             ],
+            fallback=(env_fallback, ['AWS_REGION', 'AWS_DEFAULT_REGION', 'EC2_REGION']),
         ),
     )
     spec = _aws_common_argument_spec()
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		minor_changes:
		- amazon.aws collection - refacterization of code to use argument specification ``fallback`` when falling back to environment variables for security credentials and AWS connection details (https://github.com/ansible-collections/amazon.aws/pull/1174).