From c07686b889a96eb10a33bf4c2067a0badf0ca6e7 Mon Sep 17 00:00:00 2001
From: David Lundgren <dlundgren@syberisle.net>
Date: Fri, 6 Nov 2020 18:47:17 -0600
Subject: [PATCH 1/7] Adds the ability to use bypath to aws_secrets lookup

---
 plugins/lookup/aws_secret.py                 | 125 +++++++++++++------
 tests/unit/plugins/lookup/test_aws_secret.py |  97 +++++++++++---
 2 files changed, 166 insertions(+), 56 deletions(-)

diff --git a/plugins/lookup/aws_secret.py b/plugins/lookup/aws_secret.py
index 1bc1f1968f7..4cc0c5232aa 100644
--- a/plugins/lookup/aws_secret.py
+++ b/plugins/lookup/aws_secret.py
@@ -25,6 +25,10 @@
   _terms:
     description: Name of the secret to look up in AWS Secrets Manager.
     required: True
+  bypath:
+    description: A boolean to indicate whether the parameter is provided as a hierarchy.
+    default: false
+    type: boolean
   version_id:
     description: Version of the secret(s).
     required: False
@@ -35,6 +39,7 @@
     description:
         - Join two or more entries to form an extended secret.
         - This is useful for overcoming the 4096 character limit imposed by AWS.
+        - No effect when used with `bypath`
     type: boolean
     default: false
   on_missing:
@@ -58,6 +63,9 @@
 '''
 
 EXAMPLES = r"""
+ - name: lookup secretsmanager secret in the current region
+   debug: msg="{{ lookup('aws_secret', '/path/to/secrets', bypath=true) }}"
+
  - name: Create RDS instance with aws_secret lookup for password param
    rds:
      command: create
@@ -96,6 +104,8 @@
 from ansible.module_utils._text import to_native
 from ansible_collections.amazon.aws.plugins.module_utils.core import is_boto3_error_code
 
+from ..module_utils.ec2 import HAS_BOTO3
+
 
 def _boto3_conn(region, credentials):
     boto_profile = credentials.pop('aws_profile', None)
@@ -114,24 +124,26 @@ def _boto3_conn(region, credentials):
 
 
 class LookupModule(LookupBase):
-    def _get_credentials(self):
-        credentials = {}
-        credentials['aws_profile'] = self.get_option('aws_profile')
-        credentials['aws_secret_access_key'] = self.get_option('aws_secret_key')
-        credentials['aws_access_key_id'] = self.get_option('aws_access_key')
-        credentials['aws_session_token'] = self.get_option('aws_security_token')
-
-        # fallback to IAM role credentials
-        if not credentials['aws_profile'] and not (credentials['aws_access_key_id'] and credentials['aws_secret_access_key']):
-            session = botocore.session.get_session()
-            if session.get_credentials() is not None:
-                credentials['aws_access_key_id'] = session.get_credentials().access_key
-                credentials['aws_secret_access_key'] = session.get_credentials().secret_key
-                credentials['aws_session_token'] = session.get_credentials().token
-
-        return credentials
-
-    def run(self, terms, variables, **kwargs):
+    def run(self, terms, variables=None, boto_profile=None, aws_profile=None,
+            aws_secret_key=None, aws_access_key=None, aws_security_token=None, region=None,
+            bypath=False, join=False,version_stage=None, version_id=None, on_missing=None,
+            on_denied=None):
+        '''
+           :arg terms: a list of lookups to run.
+               e.g. ['parameter_name', 'parameter_name_too' ]
+           :kwarg variables: ansible variables active at the time of the lookup
+           :kwarg aws_secret_key: identity of the AWS key to use
+           :kwarg aws_access_key: AWS secret key (matching identity)
+           :kwarg aws_security_token: AWS session key if using STS
+           :kwarg decrypt: Set to True to get decrypted parameters
+           :kwarg region: AWS region in which to do the lookup
+           :kwarg bypath: Set to True to do a lookup of variables under a path
+           :kwarg recursive: Set to True to recurse below the path (requires bypath=True)
+           :returns: A list of parameter values or a list of dictionaries if bypath=True.
+       '''
+
+        if not HAS_BOTO3:
+            raise AnsibleError('botocore and boto3 are required for aws_ssm lookup.')
 
         missing = kwargs.get('on_missing', 'error').lower()
         if not isinstance(missing, string_types) or missing not in ['error', 'warn', 'skip']:
@@ -141,21 +153,67 @@ def run(self, terms, variables, **kwargs):
         if not isinstance(denied, string_types) or denied not in ['error', 'warn', 'skip']:
             raise AnsibleError('"on_denied" must be a string and one of "error", "warn" or "skip", not %s' % denied)
 
-        self.set_options(var_options=variables, direct=kwargs)
-        boto_credentials = self._get_credentials()
+        credentials = {}
+        if aws_profile:
+            credentials['aws_profile'] = aws_profile
+        else:
+            credentials['aws_profile'] = boto_profile
+        credentials['aws_secret_access_key'] = aws_secret_key
+        credentials['aws_access_key_id'] = aws_access_key
+        credentials['aws_session_token'] = aws_security_token
+
+        # fallback to IAM role credentials
+        if not credentials['aws_profile'] and not (
+                credentials['aws_access_key_id'] and credentials['aws_secret_access_key']):
+            session = botocore.session.get_session()
+            if session.get_credentials() is not None:
+                credentials['aws_access_key_id'] = session.get_credentials().access_key
+                credentials['aws_secret_access_key'] = session.get_credentials().secret_key
+                credentials['aws_session_token'] = session.get_credentials().token
 
-        region = self.get_option('region')
-        client = _boto3_conn(region, boto_credentials)
+        client = _boto3_conn(region, credentials)
 
-        secrets = []
-        for term in terms:
-            params = {}
-            params['SecretId'] = term
-            if kwargs.get('version_id'):
-                params['VersionId'] = kwargs.get('version_id')
-            if kwargs.get('version_stage'):
-                params['VersionStage'] = kwargs.get('version_stage')
+        if bypath:
+            secrets = {}
+            for term in terms:
+                try:
+                    response = client.list_secrets(Filters=[{'Key': 'name','Values': [term]}])
 
+                    if 'SecretList' in response:
+                        for secret in response['SecretList']:
+                            secrets.update({secret['Name'] : self.get_secret_value(secret['Name'], client)})
+                except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e:
+                    raise AnsibleError("Failed to retrieve secret: %s" % to_native(e))
+            secrets = [secrets]
+        else:
+            secrets = []
+            for term in terms:
+                value = self.get_secret_value(term, client, version_stage=version_stage, version_id=version_id)
+                if value:
+                    secrets.append(value)
+            if join:
+                joined_secret = []
+                joined_secret.append(''.join(secrets))
+                return joined_secret
+
+        return secrets
+
+    def get_secret_value(self, term, client, version_stage=None, version_id=None):
+        params = {}
+        params['SecretId'] = term
+        if version_id:
+            params['VersionId'] = version_id
+        if version_stage:
+            params['VersionStage'] = version_stage
+
+        try:
+            response = client.get_secret_value(**params)
+            if 'SecretBinary' in response:
+                return response['SecretBinary']
+            if 'SecretString' in response:
+                return response['SecretString']
+        except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e:
+            raise AnsibleError("Failed to retrieve secret: %s" % to_native(e))
             try:
                 response = client.get_secret_value(**params)
                 if 'SecretBinary' in response:
@@ -175,9 +233,4 @@ def run(self, terms, variables, **kwargs):
             except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e:  # pylint: disable=duplicate-except
                 raise AnsibleError("Failed to retrieve secret: %s" % to_native(e))
 
-        if kwargs.get('join'):
-            joined_secret = []
-            joined_secret.append(''.join(secrets))
-            return joined_secret
-        else:
-            return secrets
+        return None
diff --git a/tests/unit/plugins/lookup/test_aws_secret.py b/tests/unit/plugins/lookup/test_aws_secret.py
index dd27bbf60af..30fbd7b3153 100644
--- a/tests/unit/plugins/lookup/test_aws_secret.py
+++ b/tests/unit/plugins/lookup/test_aws_secret.py
@@ -26,6 +26,8 @@
 from ansible.errors import AnsibleError
 from ansible.plugins.loader import lookup_loader
 
+from ansible_collections.amazon.aws.plugins.lookup import aws_secret
+
 try:
     import boto3
     from botocore.exceptions import ClientError
@@ -43,31 +45,30 @@ def dummy_credentials():
     dummy_credentials['region'] = 'eu-west-1'
     return dummy_credentials
 
+simple_variable_success_response = {
+    'Name': 'secret',
+    'VersionId': 'cafe8168-e6ce-4e59-8830-5b143faf6c52',
+    'SecretString': '{"secret":"simplesecret"}',
+    'VersionStages': ['AWSCURRENT'],
+    'ResponseMetadata': {
+        'RequestId': '21099462-597c-490a-800f-8b7a41e5151c',
+        'HTTPStatusCode': 200,
+        'HTTPHeaders': {
+            'date': 'Thu, 04 Apr 2019 10:43:12 GMT',
+            'content-type': 'application/x-amz-json-1.1',
+            'content-length': '252',
+            'connection': 'keep-alive',
+            'x-amzn-requestid': '21099462-597c-490a-800f-8b7a41e5151c'
+        },
+        'RetryAttempts': 0
+    }
+}
 
 def test_lookup_variable(mocker, dummy_credentials):
     dateutil_tz = pytest.importorskip("dateutil.tz")
-    simple_variable_success_response = {
-        'Name': 'secret',
-        'VersionId': 'cafe8168-e6ce-4e59-8830-5b143faf6c52',
-        'SecretString': '{"secret":"simplesecret"}',
-        'VersionStages': ['AWSCURRENT'],
-        'CreatedDate': datetime.datetime(2019, 4, 4, 11, 41, 0, 878000, tzinfo=dateutil_tz.tzlocal()),
-        'ResponseMetadata': {
-            'RequestId': '21099462-597c-490a-800f-8b7a41e5151c',
-            'HTTPStatusCode': 200,
-            'HTTPHeaders': {
-                'date': 'Thu, 04 Apr 2019 10:43:12 GMT',
-                'content-type': 'application/x-amz-json-1.1',
-                'content-length': '252',
-                'connection': 'keep-alive',
-                'x-amzn-requestid': '21099462-597c-490a-800f-8b7a41e5151c'
-            },
-            'RetryAttempts': 0
-        }
-    }
     lookup = lookup_loader.get('amazon.aws.aws_secret')
     boto3_double = mocker.MagicMock()
-    boto3_double.Session.return_value.client.return_value.get_secret_value.return_value = simple_variable_success_response
+    boto3_double.Session.return_value.client.return_value.get_secret_value.return_value = copy(simple_variable_success_response)
     boto3_client_double = boto3_double.Session.return_value.client
 
     mocker.patch.object(boto3, 'session', boto3_double)
@@ -122,3 +123,59 @@ def test_on_denied_option(mocker, dummy_credentials):
     args["on_denied"] = 'warn'
     retval = lookup_loader.get('amazon.aws.aws_secret').run(["denied_secret"], None, **args)
     assert(retval == [])
+
+
+def test_path_lookup_variable(mocker, dummy_credentials):
+    lookup = aws_secret.LookupModule()
+    lookup._load_name = "aws_secret"
+
+    path_list_secrets_success_response = {
+        'SecretList': [
+            {
+                'Name' : '/testpath/too',
+            },
+            {
+                'Name': '/testpath/won',
+            }
+        ],
+        'ResponseMetadata': {
+            'RequestId': '21099462-597c-490a-800f-8b7a41e5151c',
+            'HTTPStatusCode': 200,
+            'HTTPHeaders': {
+                'date': 'Thu, 04 Apr 2019 10:43:12 GMT',
+                'content-type': 'application/x-amz-json-1.1',
+                'content-length': '252',
+                'connection': 'keep-alive',
+                'x-amzn-requestid': '21099462-597c-490a-800f-8b7a41e5151c'
+            },
+            'RetryAttempts': 0
+        }
+    }
+
+    boto3_double = mocker.MagicMock()
+    list_secrets_fn = boto3_double.Session.return_value.client.return_value.list_secrets
+    list_secrets_fn.return_value = path_list_secrets_success_response
+
+    get_secret_value_fn = boto3_double.Session.return_value.client.return_value.get_secret_value
+    first_path = copy(simple_variable_success_response)
+    first_path['SecretString'] = 'simple_value_too'
+    second_path = copy(simple_variable_success_response)
+    second_path['SecretString'] = 'simple_value_won'
+    get_secret_value_fn.side_effect = [
+        first_path,
+        second_path
+    ]
+
+    boto3_client_double = boto3_double.Session.return_value.client
+
+    mocker.patch.object(boto3, 'session', boto3_double)
+    dummy_credentials["bypath"] = 'true'
+    dummy_credentials["boto_profile"] = 'test'
+    dummy_credentials["aws_profile"] = 'test'
+    retval = lookup.run(["/testpath"], {}, **dummy_credentials)
+    print(retval[0])
+    assert(retval[0]["/testpath/won"] == "simple_value_won")
+    assert(retval[0]["/testpath/too"] == "simple_value_too")
+    boto3_client_double.assert_called_with('secretsmanager', 'eu-west-1', aws_access_key_id='notakey',
+                                           aws_secret_access_key="notasecret", aws_session_token=None)
+    list_secrets_fn.assert_called_with(Filters=[{'Key': 'name','Values': ['/testpath']}])
\ No newline at end of file

From 4069fce542fbea0b8e06e6e99236e89ea1a8f374 Mon Sep 17 00:00:00 2001
From: David Lundgren <dlundgren@syberisle.net>
Date: Fri, 6 Nov 2020 18:56:05 -0600
Subject: [PATCH 2/7] Fix up some linting format

---
 plugins/lookup/aws_secret.py                 | 43 ++++++++++++--------
 tests/unit/plugins/lookup/test_aws_secret.py | 17 ++++----
 2 files changed, 35 insertions(+), 25 deletions(-)

diff --git a/plugins/lookup/aws_secret.py b/plugins/lookup/aws_secret.py
index 4cc0c5232aa..61d277977d9 100644
--- a/plugins/lookup/aws_secret.py
+++ b/plugins/lookup/aws_secret.py
@@ -126,22 +126,25 @@ def _boto3_conn(region, credentials):
 class LookupModule(LookupBase):
     def run(self, terms, variables=None, boto_profile=None, aws_profile=None,
             aws_secret_key=None, aws_access_key=None, aws_security_token=None, region=None,
-            bypath=False, join=False,version_stage=None, version_id=None, on_missing=None,
+            bypath=False, join=False, version_stage=None, version_id=None, on_missing=None,
             on_denied=None):
         '''
-           :arg terms: a list of lookups to run.
-               e.g. ['parameter_name', 'parameter_name_too' ]
-           :kwarg variables: ansible variables active at the time of the lookup
-           :kwarg aws_secret_key: identity of the AWS key to use
-           :kwarg aws_access_key: AWS secret key (matching identity)
-           :kwarg aws_security_token: AWS session key if using STS
-           :kwarg decrypt: Set to True to get decrypted parameters
-           :kwarg region: AWS region in which to do the lookup
-           :kwarg bypath: Set to True to do a lookup of variables under a path
-           :kwarg recursive: Set to True to recurse below the path (requires bypath=True)
-           :returns: A list of parameter values or a list of dictionaries if bypath=True.
-       '''
-
+                   :arg terms: a list of lookups to run.
+                       e.g. ['parameter_name', 'parameter_name_too' ]
+                   :kwarg variables: ansible variables active at the time of the lookup
+                   :kwarg aws_secret_key: identity of the AWS key to use
+                   :kwarg aws_access_key: AWS secret key (matching identity)
+                   :kwarg aws_security_token: AWS session key if using STS
+                   :kwarg decrypt: Set to True to get decrypted parameters
+                   :kwarg region: AWS region in which to do the lookup
+                   :kwarg bypath: Set to True to do a lookup of variables under a path
+                   :kwarg join: Join two or more entries to form an extended secret
+                   :kwarg version_stage: Stage of the secret version
+                   :kwarg version_id: Version of the secret(s)
+                   :kwarg on_missing: Action to take if the secret is missing
+                   :kwarg on_denied: Action to take if access to the secret is denied
+                   :returns: A list of parameter values or a list of dictionaries if bypath=True.
+               '''
         if not HAS_BOTO3:
             raise AnsibleError('botocore and boto3 are required for aws_ssm lookup.')
 
@@ -177,18 +180,22 @@ def run(self, terms, variables=None, boto_profile=None, aws_profile=None,
             secrets = {}
             for term in terms:
                 try:
-                    response = client.list_secrets(Filters=[{'Key': 'name','Values': [term]}])
+                    response = client.list_secrets(Filters=[{'Key': 'name', 'Values': [term]}])
 
                     if 'SecretList' in response:
                         for secret in response['SecretList']:
-                            secrets.update({secret['Name'] : self.get_secret_value(secret['Name'], client)})
+                            secrets.update({secret['Name']: self.get_secret_value(secret['Name'], client,
+                                                                                  on_missing=on_missing,
+                                                                                  on_denied=on_denied)})
                 except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e:
                     raise AnsibleError("Failed to retrieve secret: %s" % to_native(e))
             secrets = [secrets]
         else:
             secrets = []
             for term in terms:
-                value = self.get_secret_value(term, client, version_stage=version_stage, version_id=version_id)
+                value = self.get_secret_value(term, client,
+                                              version_stage=version_stage, version_id=version_id,
+                                              on_missing=on_missing, on_denied=on_denied)
                 if value:
                     secrets.append(value)
             if join:
@@ -198,7 +205,7 @@ def run(self, terms, variables=None, boto_profile=None, aws_profile=None,
 
         return secrets
 
-    def get_secret_value(self, term, client, version_stage=None, version_id=None):
+    def get_secret_value(self, term, client, version_stage=None, version_id=None, on_missing=None, on_denied=None):
         params = {}
         params['SecretId'] = term
         if version_id:
diff --git a/tests/unit/plugins/lookup/test_aws_secret.py b/tests/unit/plugins/lookup/test_aws_secret.py
index 30fbd7b3153..6abac2f3078 100644
--- a/tests/unit/plugins/lookup/test_aws_secret.py
+++ b/tests/unit/plugins/lookup/test_aws_secret.py
@@ -17,6 +17,7 @@
 
 # Make coding more python3-ish
 from __future__ import (absolute_import, division, print_function)
+
 __metaclass__ = type
 
 import pytest
@@ -45,6 +46,7 @@ def dummy_credentials():
     dummy_credentials['region'] = 'eu-west-1'
     return dummy_credentials
 
+
 simple_variable_success_response = {
     'Name': 'secret',
     'VersionId': 'cafe8168-e6ce-4e59-8830-5b143faf6c52',
@@ -64,16 +66,18 @@ def dummy_credentials():
     }
 }
 
+
 def test_lookup_variable(mocker, dummy_credentials):
     dateutil_tz = pytest.importorskip("dateutil.tz")
     lookup = lookup_loader.get('amazon.aws.aws_secret')
     boto3_double = mocker.MagicMock()
-    boto3_double.Session.return_value.client.return_value.get_secret_value.return_value = copy(simple_variable_success_response)
+    boto3_double.Session.return_value.client.return_value.get_secret_value.return_value = copy(
+        simple_variable_success_response)
     boto3_client_double = boto3_double.Session.return_value.client
 
     mocker.patch.object(boto3, 'session', boto3_double)
     retval = lookup.run(["simple_variable"], None, **dummy_credentials)
-    assert(retval[0] == '{"secret":"simplesecret"}')
+    assert (retval[0] == '{"secret":"simplesecret"}')
     boto3_client_double.assert_called_with('secretsmanager', 'eu-west-1', aws_access_key_id='notakey',
                                            aws_secret_access_key="notasecret", aws_session_token=None)
 
@@ -132,7 +136,7 @@ def test_path_lookup_variable(mocker, dummy_credentials):
     path_list_secrets_success_response = {
         'SecretList': [
             {
-                'Name' : '/testpath/too',
+                'Name': '/testpath/too',
             },
             {
                 'Name': '/testpath/won',
@@ -173,9 +177,8 @@ def test_path_lookup_variable(mocker, dummy_credentials):
     dummy_credentials["boto_profile"] = 'test'
     dummy_credentials["aws_profile"] = 'test'
     retval = lookup.run(["/testpath"], {}, **dummy_credentials)
-    print(retval[0])
-    assert(retval[0]["/testpath/won"] == "simple_value_won")
-    assert(retval[0]["/testpath/too"] == "simple_value_too")
+    assert (retval[0]["/testpath/won"] == "simple_value_won")
+    assert (retval[0]["/testpath/too"] == "simple_value_too")
     boto3_client_double.assert_called_with('secretsmanager', 'eu-west-1', aws_access_key_id='notakey',
                                            aws_secret_access_key="notasecret", aws_session_token=None)
-    list_secrets_fn.assert_called_with(Filters=[{'Key': 'name','Values': ['/testpath']}])
\ No newline at end of file
+    list_secrets_fn.assert_called_with(Filters=[{'Key': 'name', 'Values': ['/testpath']}])

From 2f0518c01e099ef786c7b64004ae5e712bcc2e8e Mon Sep 17 00:00:00 2001
From: David Lundgren <dlundgren@syberisle.net>
Date: Sat, 19 Dec 2020 15:07:39 -0600
Subject: [PATCH 3/7] Fixes from rebase

---
 plugins/lookup/aws_secret.py | 44 +++++++++++++++---------------------
 1 file changed, 18 insertions(+), 26 deletions(-)

diff --git a/plugins/lookup/aws_secret.py b/plugins/lookup/aws_secret.py
index 61d277977d9..8db3f02fc44 100644
--- a/plugins/lookup/aws_secret.py
+++ b/plugins/lookup/aws_secret.py
@@ -126,8 +126,8 @@ def _boto3_conn(region, credentials):
 class LookupModule(LookupBase):
     def run(self, terms, variables=None, boto_profile=None, aws_profile=None,
             aws_secret_key=None, aws_access_key=None, aws_security_token=None, region=None,
-            bypath=False, join=False, version_stage=None, version_id=None, on_missing=None,
-            on_denied=None):
+            bypath=False, join=False, version_stage=None, version_id=None, on_missing='error',
+            on_denied='error'):
         '''
                    :arg terms: a list of lookups to run.
                        e.g. ['parameter_name', 'parameter_name_too' ]
@@ -148,11 +148,11 @@ def run(self, terms, variables=None, boto_profile=None, aws_profile=None,
         if not HAS_BOTO3:
             raise AnsibleError('botocore and boto3 are required for aws_ssm lookup.')
 
-        missing = kwargs.get('on_missing', 'error').lower()
+        missing = on_missing.lower()
         if not isinstance(missing, string_types) or missing not in ['error', 'warn', 'skip']:
             raise AnsibleError('"on_missing" must be a string and one of "error", "warn" or "skip", not %s' % missing)
 
-        denied = kwargs.get('on_denied', 'error').lower()
+        denied = on_denied.lower()
         if not isinstance(denied, string_types) or denied not in ['error', 'warn', 'skip']:
             raise AnsibleError('"on_denied" must be a string and one of "error", "warn" or "skip", not %s' % denied)
 
@@ -185,8 +185,8 @@ def run(self, terms, variables=None, boto_profile=None, aws_profile=None,
                     if 'SecretList' in response:
                         for secret in response['SecretList']:
                             secrets.update({secret['Name']: self.get_secret_value(secret['Name'], client,
-                                                                                  on_missing=on_missing,
-                                                                                  on_denied=on_denied)})
+                                                                                  on_missing=missing,
+                                                                                  on_denied=denied)})
                 except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e:
                     raise AnsibleError("Failed to retrieve secret: %s" % to_native(e))
             secrets = [secrets]
@@ -195,7 +195,7 @@ def run(self, terms, variables=None, boto_profile=None, aws_profile=None,
             for term in terms:
                 value = self.get_secret_value(term, client,
                                               version_stage=version_stage, version_id=version_id,
-                                              on_missing=on_missing, on_denied=on_denied)
+                                              on_missing=missing, on_denied=denied)
                 if value:
                     secrets.append(value)
             if join:
@@ -219,25 +219,17 @@ def get_secret_value(self, term, client, version_stage=None, version_id=None, on
                 return response['SecretBinary']
             if 'SecretString' in response:
                 return response['SecretString']
-        except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e:
+        except is_boto3_error_code('ResourceNotFoundException'):
+            if on_missing == 'error':
+                raise AnsibleError("Failed to find secret %s (ResourceNotFound)" % term)
+            elif on_missing == 'warn':
+                self._display.warning('Skipping, did not find secret %s' % term)
+        except is_boto3_error_code('AccessDeniedException'):  # pylint: disable=duplicate-except
+            if on_denied == 'error':
+                raise AnsibleError("Failed to access secret %s (AccessDenied)" % term)
+            elif on_denied == 'warn':
+                self._display.warning('Skipping, access denied for secret %s' % term)
+        except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e:  # pylint: disable=duplicate-except
             raise AnsibleError("Failed to retrieve secret: %s" % to_native(e))
-            try:
-                response = client.get_secret_value(**params)
-                if 'SecretBinary' in response:
-                    secrets.append(response['SecretBinary'])
-                if 'SecretString' in response:
-                    secrets.append(response['SecretString'])
-            except is_boto3_error_code('ResourceNotFoundException'):
-                if missing == 'error':
-                    raise AnsibleError("Failed to find secret %s (ResourceNotFound)" % term)
-                elif missing == 'warn':
-                    self._display.warning('Skipping, did not find secret %s' % term)
-            except is_boto3_error_code('AccessDeniedException'):  # pylint: disable=duplicate-except
-                if denied == 'error':
-                    raise AnsibleError("Failed to access secret %s (AccessDenied)" % term)
-                elif denied == 'warn':
-                    self._display.warning('Skipping, access denied for secret %s' % term)
-            except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e:  # pylint: disable=duplicate-except
-                raise AnsibleError("Failed to retrieve secret: %s" % to_native(e))
 
         return None

From 501ed3ec3b53dcfbe01785f969d47a264afa65fb Mon Sep 17 00:00:00 2001
From: David Lundgren <dlundgren@syberisle.net>
Date: Mon, 21 Dec 2020 13:09:56 -0600
Subject: [PATCH 4/7] Updated per code review

---
 plugins/lookup/aws_secret.py | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/plugins/lookup/aws_secret.py b/plugins/lookup/aws_secret.py
index 8db3f02fc44..ca2e331ff9e 100644
--- a/plugins/lookup/aws_secret.py
+++ b/plugins/lookup/aws_secret.py
@@ -19,8 +19,8 @@
 description:
   - Look up secrets stored in AWS Secrets Manager provided the caller
     has the appropriate permissions to read the secret.
-  - Lookup is based on the secret's `Name` value.
-  - Optional parameters can be passed into this lookup; `version_id` and `version_stage`
+  - Lookup is based on the secret's I(Name) value.
+  - Optional parameters can be passed into this lookup; I(version_id) and I(version_stage)
 options:
   _terms:
     description: Name of the secret to look up in AWS Secrets Manager.
@@ -28,7 +28,7 @@
   bypath:
     description: A boolean to indicate whether the parameter is provided as a hierarchy.
     default: false
-    type: boolean
+    type: bool
   version_id:
     description: Version of the secret(s).
     required: False
@@ -39,8 +39,8 @@
     description:
         - Join two or more entries to form an extended secret.
         - This is useful for overcoming the 4096 character limit imposed by AWS.
-        - No effect when used with `bypath`
-    type: boolean
+        - No effect when used with I(bypath)
+    type: bool
     default: false
   on_missing:
     description:
@@ -103,8 +103,7 @@
 from ansible.plugins.lookup import LookupBase
 from ansible.module_utils._text import to_native
 from ansible_collections.amazon.aws.plugins.module_utils.core import is_boto3_error_code
-
-from ..module_utils.ec2 import HAS_BOTO3
+from ansible_collections.amazon.aws.plugins.module_utils.ec2 import HAS_BOTO3
 
 
 def _boto3_conn(region, credentials):

From 91db991e0f1ccc51b6ff23e3796001d4b41459f4 Mon Sep 17 00:00:00 2001
From: David Lundgren <dlundgren@syberisle.net>
Date: Thu, 7 Jan 2021 21:54:51 -0600
Subject: [PATCH 5/7] Add changelog fragment

---
 changelogs/fragments/192-aws_secret-bypath-option.yaml | 2 ++
 1 file changed, 2 insertions(+)
 create mode 100644 changelogs/fragments/192-aws_secret-bypath-option.yaml

diff --git a/changelogs/fragments/192-aws_secret-bypath-option.yaml b/changelogs/fragments/192-aws_secret-bypath-option.yaml
new file mode 100644
index 00000000000..c302d74cffc
--- /dev/null
+++ b/changelogs/fragments/192-aws_secret-bypath-option.yaml
@@ -0,0 +1,2 @@
+minor_changes:
+- aws_secret - Add bypath functionality

From 5cf0ebf513e919618516306fb37951d0627dafe4 Mon Sep 17 00:00:00 2001
From: David Lundgren <dlundgren@syberisle.net>
Date: Thu, 7 Jan 2021 22:08:35 -0600
Subject: [PATCH 6/7] Add the PR link to the changelog

---
 changelogs/fragments/192-aws_secret-bypath-option.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/changelogs/fragments/192-aws_secret-bypath-option.yaml b/changelogs/fragments/192-aws_secret-bypath-option.yaml
index c302d74cffc..858d07aadf1 100644
--- a/changelogs/fragments/192-aws_secret-bypath-option.yaml
+++ b/changelogs/fragments/192-aws_secret-bypath-option.yaml
@@ -1,2 +1,2 @@
 minor_changes:
-- aws_secret - Add bypath functionality
+- aws_secret - Add bypath functionality (https://github.com/ansible-collections/amazon.aws/pull/192).

From 61420f07fd650803629be6604d3170d09ae738f9 Mon Sep 17 00:00:00 2001
From: David Lundgren <dlundgren@syberisle.net>
Date: Fri, 8 Jan 2021 15:15:20 -0600
Subject: [PATCH 7/7] Fix documentation

---
 .../fragments/192-aws_secret-bypath-option.yaml   |  2 +-
 plugins/lookup/aws_secret.py                      | 15 ++++++++-------
 2 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/changelogs/fragments/192-aws_secret-bypath-option.yaml b/changelogs/fragments/192-aws_secret-bypath-option.yaml
index 858d07aadf1..14e1c25eab2 100644
--- a/changelogs/fragments/192-aws_secret-bypath-option.yaml
+++ b/changelogs/fragments/192-aws_secret-bypath-option.yaml
@@ -1,2 +1,2 @@
 minor_changes:
-- aws_secret - Add bypath functionality (https://github.com/ansible-collections/amazon.aws/pull/192).
+- aws_secret - add ``bypath`` functionality (https://github.com/ansible-collections/amazon.aws/pull/192).
diff --git a/plugins/lookup/aws_secret.py b/plugins/lookup/aws_secret.py
index ca2e331ff9e..f5884ee1f9f 100644
--- a/plugins/lookup/aws_secret.py
+++ b/plugins/lookup/aws_secret.py
@@ -28,7 +28,8 @@
   bypath:
     description: A boolean to indicate whether the parameter is provided as a hierarchy.
     default: false
-    type: bool
+    type: boolean
+    version_added: 1.4.0
   version_id:
     description: Version of the secret(s).
     required: False
@@ -39,8 +40,8 @@
     description:
         - Join two or more entries to form an extended secret.
         - This is useful for overcoming the 4096 character limit imposed by AWS.
-        - No effect when used with I(bypath)
-    type: bool
+        - No effect when used with I(bypath).
+    type: boolean
     default: false
   on_missing:
     description:
@@ -64,7 +65,7 @@
 
 EXAMPLES = r"""
  - name: lookup secretsmanager secret in the current region
-   debug: msg="{{ lookup('aws_secret', '/path/to/secrets', bypath=true) }}"
+   debug: msg="{{ lookup('amazon.aws.aws_secret', '/path/to/secrets', bypath=true) }}"
 
  - name: Create RDS instance with aws_secret lookup for password param
    rds:
@@ -74,15 +75,15 @@
      size: 10
      instance_type: db.m1.small
      username: dbadmin
-     password: "{{ lookup('aws_secret', 'DbSecret') }}"
+     password: "{{ lookup('amazon.aws.aws_secret', 'DbSecret') }}"
      tags:
        Environment: staging
 
  - name: skip if secret does not exist
-   debug: msg="{{ lookup('aws_secret', 'secret-not-exist', on_missing='skip')}}"
+   debug: msg="{{ lookup('amazon.aws.aws_secret', 'secret-not-exist', on_missing='skip')}}"
 
  - name: warn if access to the secret is denied
-   debug: msg="{{ lookup('aws_secret', 'secret-denied', on_denied='warn')}}"
+   debug: msg="{{ lookup('amazon.aws.aws_secret', 'secret-denied', on_denied='warn')}}"
 """
 
 RETURN = r"""