From 9401932f98ef7c6bc412be0487c8e8c054f934eb Mon Sep 17 00:00:00 2001 From: Mark Chappell Date: Mon, 17 Oct 2022 17:02:22 +0200 Subject: [PATCH] Refacterization of code to use 'fallback' instead of using os.environ (#1174) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- changelogs/fragments/1174-module_params.yml | 2 + plugins/doc_fragments/aws.py | 8 +- plugins/module_utils/botocore.py | 90 +++++---------------- plugins/module_utils/modules.py | 11 ++- 4 files changed, 36 insertions(+), 75 deletions(-) create mode 100644 changelogs/fragments/1174-module_params.yml diff --git a/changelogs/fragments/1174-module_params.yml b/changelogs/fragments/1174-module_params.yml new file mode 100644 index 00000000000..841175ac9c1 --- /dev/null +++ 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 index ab9b8ee1650..1a3833ffb20 100644 --- 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 index b063edbe107..0f368e65e75 100644 --- 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 index 490f91833c1..39a207a993c 100644 --- a/plugins/module_utils/modules.py +++ b/plugins/module_utils/modules.py @@ -380,6 +380,7 @@ 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( @@ -387,6 +388,7 @@ def _aws_common_argument_spec(): 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()