[stable-5] lookup plugins - raise correct error (#1534)

[stable-5] lookup plugins - raise correct error

SUMMARY
Lookup plugins are currently raising "AnsibleError" this is in turn leading to a surprising error.  Our tests didn't catch this because we expected an error (just a clean error) and our error message was still in there.
fixes: #1528
ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
aws_ssm
ADDITIONAL INFORMATION
TASK [lookup_aws_ssm : lookup a missing key (error)] ***************************
task path: /root/ansible_collections/amazon/aws/tests/output/.tmp/integration/lookup_aws_ssm-uhhi4kie-ÅÑŚÌβŁÈ/tests/integration/targets/lookup_aws_ssm/tasks/main.yml:43
AWS_ssm name lookup term: ['/ansible-test-32065799-mchappel/Simple']
exception during Jinja2 execution: Traceback (most recent call last):
  File "/root/ansible_collections/amazon/aws/plugins/lookup/aws_ssm.py", line 272, in get_parameter_value
    response = client.get_parameter(**ssm_dict)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/botocore/client.py", line 386, in _api_call
    return self._make_api_call(operation_name, kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/botocore/client.py", line 705, in _make_api_call
    raise error_class(parsed_response, operation_name)
botocore.errorfactory.ParameterNotFound: An error occurred (ParameterNotFound) when calling the GetParameter operation: 

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/root/ansible/lib/ansible/template/__init__.py", line 831, in _lookup
    ran = instance.run(loop_terms, variables=self._available_variables, **kwargs)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/root/ansible_collections/amazon/aws/plugins/lookup/aws_ssm.py", line 241, in run
    ret.append(self.get_parameter_value(client, ssm_dict, term, on_missing.lower(), on_denied.lower()))
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/root/ansible_collections/amazon/aws/plugins/lookup/aws_ssm.py", line 276, in get_parameter_value
    raise AnsibleError("Failed to find SSM parameter %s (ResourceNotFound)" % term)
ansible.errors.AnsibleError: Failed to find SSM parameter /ansible-test-32065799-mchappel/Simple (ResourceNotFound)
fatal: [testhost]: FAILED! => {
    "msg": "An unhandled exception occurred while running the lookup plugin 'amazon.aws.aws_ssm'. Error was a <class 'ansible.errors.AnsibleError'>, original message: Failed to find SSM parameter /ansible-test-32065799-mchappel/Simple (ResourceNotFound). Failed to find SSM parameter /ansible-test-32065799-mchappel/Simple (ResourceNotFound)"
}
...ignoring

Direct to stable-5 as the issue was fixed during the refactor work  in main.

Reviewed-by: Alina Buzachis

changelogs/fragments/1528-lookup-error.yml

@@ -0,0 +1,5 @@
+bugfixes:
+- aws_account_attribute - raise correct ``AnsibleLookupError`` rather than ``AnsibleError`` (https://github.com/ansible-collections/amazon.aws/issues/1528).
+- aws_secret -  raise correct ``AnsibleLookupError`` rather than ``AnsibleError`` (https://github.com/ansible-collections/amazon.aws/issues/1528).
+- aws_service_ip_ranges raise correct ``AnsibleLookupError`` rather than ``AnsibleError`` (https://github.com/ansible-collections/amazon.aws/issues/1528).
+- aws_ssm - raise correct ``AnsibleLookupError`` rather than ``AnsibleError`` (https://github.com/ansible-collections/amazon.aws/issues/1528).

plugins/lookup/aws_account_attribute.py

@@ -55,7 +55,7 @@
 except ImportError:
     pass  # will be captured by imported HAS_BOTO3
 
-from ansible.errors import AnsibleError
+from ansible.errors import AnsibleLookupError
 from ansible.module_utils._text import to_native
 from ansible.module_utils.basic import missing_required_lib
 from ansible.plugins.lookup import LookupBase
@@ -74,9 +74,9 @@ def _boto3_conn(region, credentials):
             try:
                 connection = boto3.session.Session(profile_name=boto_profile).client('ec2', region)
             except (botocore.exceptions.ProfileNotFound, botocore.exceptions.PartialCredentialsError):
-                raise AnsibleError("Insufficient credentials found.")
+                raise AnsibleLookupError("Insufficient credentials found.")
         else:
-            raise AnsibleError("Insufficient credentials found.")
+            raise AnsibleLookupError("Insufficient credentials found.")
     return connection
 
 
@@ -100,7 +100,7 @@ class LookupModule(LookupBase):
     def run(self, terms, variables, **kwargs):
 
         if not HAS_BOTO3:
-            raise AnsibleError(missing_required_lib('botocore and boto3'))
+            raise AnsibleLookupError(missing_required_lib('botocore and boto3'))
 
         self.set_options(var_options=variables, direct=kwargs)
         boto_credentials = _get_credentials(self._options)
@@ -120,7 +120,7 @@ def run(self, terms, variables, **kwargs):
         try:
             response = _describe_account_attributes(client, **params)['AccountAttributes']
         except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e:
-            raise AnsibleError("Failed to describe account attributes: %s" % to_native(e))
+            raise AnsibleLookupError("Failed to describe account attributes: %s" % to_native(e))
 
         if check_ec2_classic:
             attr = response[0]

plugins/lookup/aws_secret.py

@@ -126,7 +126,7 @@
 except ImportError:
     pass  # will be captured by imported HAS_BOTO3
 
-from ansible.errors import AnsibleError
+from ansible.errors import AnsibleLookupError
 from ansible.module_utils.six import string_types
 from ansible.module_utils._text import to_native
 from ansible.module_utils.basic import missing_required_lib
@@ -147,9 +147,9 @@ def _boto3_conn(region, credentials):
             try:
                 connection = boto3.session.Session(profile_name=boto_profile).client('secretsmanager', region)
             except (botocore.exceptions.ProfileNotFound, botocore.exceptions.PartialCredentialsError):
-                raise AnsibleError("Insufficient credentials found.")
+                raise AnsibleLookupError("Insufficient credentials found.")
         else:
-            raise AnsibleError("Insufficient credentials found.")
+            raise AnsibleLookupError("Insufficient credentials found.")
     return connection
 
 
@@ -178,19 +178,19 @@ def run(self, terms, variables=None, boto_profile=None, aws_profile=None,
                    :returns: A list of parameter values or a list of dictionaries if bypath=True.
                '''
         if not HAS_BOTO3:
-            raise AnsibleError(missing_required_lib('botocore and boto3'))
+            raise AnsibleLookupError(missing_required_lib('botocore and boto3'))
 
         deleted = on_deleted.lower()
         if not isinstance(deleted, string_types) or deleted not in ['error', 'warn', 'skip']:
-            raise AnsibleError('"on_deleted" must be a string and one of "error", "warn" or "skip", not %s' % deleted)
+            raise AnsibleLookupError('"on_deleted" must be a string and one of "error", "warn" or "skip", not %s' % deleted)
 
         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)
+            raise AnsibleLookupError('"on_missing" must be a string and one of "error", "warn" or "skip", not %s' % missing)
 
         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)
+            raise AnsibleLookupError('"on_denied" must be a string and one of "error", "warn" or "skip", not %s' % denied)
 
         credentials = {}
         if aws_profile:
@@ -227,7 +227,7 @@ def run(self, terms, variables=None, boto_profile=None, aws_profile=None,
                     secrets = [secrets]
 
                 except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e:
-                    raise AnsibleError("Failed to retrieve secret: %s" % to_native(e))
+                    raise AnsibleLookupError("Failed to retrieve secret: %s" % to_native(e))
         else:
             secrets = []
             for term in terms:
@@ -253,7 +253,7 @@ def get_secret_value(self, term, client, version_stage=None, version_id=None, on
             params['VersionStage'] = version_stage
         if nested:
             if len(term.split('.')) < 2:
-                raise AnsibleError("Nested query must use the following syntax: `aws_secret_name.<key_name>.<key_name>")
+                raise AnsibleLookupError("Nested query must use the following syntax: `aws_secret_name.<key_name>.<key_name>")
             secret_name = term.split('.')[0]
             params['SecretId'] = secret_name
 
@@ -270,26 +270,26 @@ def get_secret_value(self, term, client, version_stage=None, version_id=None, on
                         if key in ret_val:
                             ret_val = ret_val[key]
                         else:
-                            raise AnsibleError("Successfully retrieved secret but there exists no key {0} in the secret".format(key))
+                            raise AnsibleLookupError("Successfully retrieved secret but there exists no key {0} in the secret".format(key))
                     return str(ret_val)
                 else:
                     return response['SecretString']
         except is_boto3_error_message('marked for deletion'):
             if on_deleted == 'error':
-                raise AnsibleError("Failed to find secret %s (marked for deletion)" % term)
+                raise AnsibleLookupError("Failed to find secret %s (marked for deletion)" % term)
             elif on_deleted == 'warn':
                 self._display.warning('Skipping, did not find secret (marked for deletion) %s' % term)
         except is_boto3_error_code('ResourceNotFoundException'):  # pylint: disable=duplicate-except
             if on_missing == 'error':
-                raise AnsibleError("Failed to find secret %s (ResourceNotFound)" % term)
+                raise AnsibleLookupError("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)
+                raise AnsibleLookupError("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))
+            raise AnsibleLookupError("Failed to retrieve secret: %s" % to_native(e))
 
         return None

plugins/lookup/aws_service_ip_ranges.py

@@ -45,7 +45,7 @@
 
 import json
 
-from ansible.errors import AnsibleError
+from ansible.errors import AnsibleLookupError
 from ansible.module_utils.six.moves.urllib.error import HTTPError
 from ansible.module_utils.six.moves.urllib.error import URLError
 from ansible.module_utils._text import to_native
@@ -70,15 +70,15 @@ def run(self, terms, variables, **kwargs):
         except getattr(json.decoder, 'JSONDecodeError', ValueError) as e:
             # on Python 3+, json.decoder.JSONDecodeError is raised for bad
             # JSON. On 2.x it's a ValueError
-            raise AnsibleError("Could not decode AWS IP ranges: %s" % to_native(e))
+            raise AnsibleLookupError("Could not decode AWS IP ranges: %s" % to_native(e))
         except HTTPError as e:
-            raise AnsibleError("Received HTTP error while pulling IP ranges: %s" % to_native(e))
+            raise AnsibleLookupError("Received HTTP error while pulling IP ranges: %s" % to_native(e))
         except SSLValidationError as e:
-            raise AnsibleError("Error validating the server's certificate for: %s" % to_native(e))
+            raise AnsibleLookupError("Error validating the server's certificate for: %s" % to_native(e))
         except URLError as e:
-            raise AnsibleError("Failed look up IP range service: %s" % to_native(e))
+            raise AnsibleLookupError("Failed look up IP range service: %s" % to_native(e))
         except ConnectionError as e:
-            raise AnsibleError("Error connecting to IP range service: %s" % to_native(e))
+            raise AnsibleLookupError("Error connecting to IP range service: %s" % to_native(e))
 
         if 'region' in kwargs:
             region = kwargs['region']

plugins/lookup/aws_ssm.py

@@ -132,7 +132,7 @@
 except ImportError:
     pass  # will be captured by imported HAS_BOTO3
 
-from ansible.errors import AnsibleError
+from ansible.errors import AnsibleLookupError
 from ansible.module_utils._text import to_native
 from ansible.plugins.lookup import LookupBase
 from ansible.utils.display import Display
@@ -171,13 +171,13 @@ def run(self, terms, variables=None, boto_profile=None, aws_profile=None,
         '''
 
         if not HAS_BOTO3:
-            raise AnsibleError(missing_required_lib('botocore and boto3'))
+            raise AnsibleLookupError(missing_required_lib('botocore and boto3'))
 
         # validate arguments 'on_missing' and 'on_denied'
         if on_missing is not None and (not isinstance(on_missing, string_types) or on_missing.lower() not in ['error', 'warn', 'skip']):
-            raise AnsibleError('"on_missing" must be a string and one of "error", "warn" or "skip", not %s' % on_missing)
+            raise AnsibleLookupError('"on_missing" must be a string and one of "error", "warn" or "skip", not %s' % on_missing)
         if on_denied is not None and (not isinstance(on_denied, string_types) or on_denied.lower() not in ['error', 'warn', 'skip']):
-            raise AnsibleError('"on_denied" must be a string and one of "error", "warn" or "skip", not %s' % on_denied)
+            raise AnsibleLookupError('"on_denied" must be a string and one of "error", "warn" or "skip", not %s' % on_denied)
 
         ret = []
         ssm_dict = {}
@@ -249,18 +249,18 @@ def get_path_parameters(self, client, ssm_dict, term, on_missing, on_denied):
             paramlist = paginator.paginate(**ssm_dict).build_full_result()['Parameters']
         except is_boto3_error_code('AccessDeniedException'):
             if on_denied == 'error':
-                raise AnsibleError("Failed to access SSM parameter path %s (AccessDenied)" % term)
+                raise AnsibleLookupError("Failed to access SSM parameter path %s (AccessDenied)" % term)
             elif on_denied == 'warn':
                 self._display.warning('Skipping, access denied for SSM parameter path %s' % term)
                 paramlist = [{}]
             elif on_denied == 'skip':
                 paramlist = [{}]
         except botocore.exceptions.ClientError as e:  # pylint: disable=duplicate-except
-            raise AnsibleError("SSM lookup exception: {0}".format(to_native(e)))
+            raise AnsibleLookupError("SSM lookup exception: {0}".format(to_native(e)))
 
         if not len(paramlist):
             if on_missing == "error":
-                raise AnsibleError("Failed to find SSM parameter path %s (ResourceNotFound)" % term)
+                raise AnsibleLookupError("Failed to find SSM parameter path %s (ResourceNotFound)" % term)
             elif on_missing == "warn":
                 self._display.warning('Skipping, did not find SSM parameter path %s' % term)
 
@@ -273,14 +273,14 @@ def get_parameter_value(self, client, ssm_dict, term, on_missing, on_denied):
             return response['Parameter']['Value']
         except is_boto3_error_code('ParameterNotFound'):
             if on_missing == 'error':
-                raise AnsibleError("Failed to find SSM parameter %s (ResourceNotFound)" % term)
+                raise AnsibleLookupError("Failed to find SSM parameter %s (ResourceNotFound)" % term)
             elif on_missing == 'warn':
                 self._display.warning('Skipping, did not find SSM parameter %s' % term)
         except is_boto3_error_code('AccessDeniedException'):  # pylint: disable=duplicate-except
             if on_denied == 'error':
-                raise AnsibleError("Failed to access SSM parameter %s (AccessDenied)" % term)
+                raise AnsibleLookupError("Failed to access SSM parameter %s (AccessDenied)" % term)
             elif on_denied == 'warn':
                 self._display.warning('Skipping, access denied for SSM parameter %s' % term)
         except botocore.exceptions.ClientError as e:  # pylint: disable=duplicate-except
-            raise AnsibleError("SSM lookup exception: {0}".format(to_native(e)))
+            raise AnsibleLookupError("SSM lookup exception: {0}".format(to_native(e)))
         return None