From 234a77624484dcb5db1abea2539397b6d9fa45dc Mon Sep 17 00:00:00 2001 From: Mark Chappell Date: Tue, 9 May 2023 13:04:42 +0200 Subject: [PATCH] [stable-5] lookup plugins - raise correct error (#1534) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit [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 , 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 | 5 ++++ plugins/lookup/aws_account_attribute.py | 10 ++++---- plugins/lookup/aws_secret.py | 28 +++++++++++----------- plugins/lookup/aws_service_ip_ranges.py | 12 +++++----- plugins/lookup/aws_ssm.py | 20 ++++++++-------- 5 files changed, 40 insertions(+), 35 deletions(-) create mode 100644 changelogs/fragments/1528-lookup-error.yml diff --git a/changelogs/fragments/1528-lookup-error.yml b/changelogs/fragments/1528-lookup-error.yml new file mode 100644 index 00000000000..232986bec0f --- /dev/null +++ b/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). diff --git a/plugins/lookup/aws_account_attribute.py b/plugins/lookup/aws_account_attribute.py index b04731f1096..415b76d75c4 100644 --- a/plugins/lookup/aws_account_attribute.py +++ b/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] diff --git a/plugins/lookup/aws_secret.py b/plugins/lookup/aws_secret.py index 7cfd5b51432..0f694cfa0be 100644 --- a/plugins/lookup/aws_secret.py +++ b/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..") + raise AnsibleLookupError("Nested query must use the following syntax: `aws_secret_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 diff --git a/plugins/lookup/aws_service_ip_ranges.py b/plugins/lookup/aws_service_ip_ranges.py index bd34d1bd131..251debf40f5 100644 --- a/plugins/lookup/aws_service_ip_ranges.py +++ b/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'] diff --git a/plugins/lookup/aws_ssm.py b/plugins/lookup/aws_ssm.py index 81e4cbb9b83..e7180856093 100644 --- a/plugins/lookup/aws_ssm.py +++ b/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