Sceptre / sceptre

Build better AWS infrastructure
https://docs.sceptre-project.org
Other
1.49k stars 313 forks source link

When running sceptre diff, we get an `UnboundLocalError` due to lack of permissions, can we make the error clearer to the user? #1519

Open jak-sdk opened 1 month ago

jak-sdk commented 1 month ago

Subject of the issue

When running sceptre diff some/stack.yaml we get an UnboundLocalError, caused by lack of permissions, can we make the error clearer to the user?

  File "/home/jak/.pyenv/versions/3.12.3/envs/env1/lib/python3.12/site-packages/sceptre/plan/actions.py", line 1077, in diff
    return stack_differ.diff(self)
           ^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/jak/.pyenv/versions/3.12.3/envs/env1/lib/python3.12/site-packages/sceptre/diffing/stack_differ.py", line 126, in diff
    deployed_config = self._create_deployed_stack_config(stack_actions)
                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/jak/.pyenv/versions/3.12.3/envs/env1/lib/python3.12/site-packages/sceptre/diffing/stack_differ.py", line 206, in _create_deployed_stack_config
    print(description)
          ^^^^^^^^^^^
UnboundLocalError: cannot access local variable 'description' where it is not associated with a value

Your environment

Steps to reproduce

We use AWS SSO to manage access to accounts, and obtain temporary access to AWS via SCIM integration with Azure.

We encounter this error when running sceptre diff some/stack.yaml, when the temporary access is not active. i.e. We do not currently have AWS permissions

$ aws --profile dev sts get-caller-identity
An error occurred (ForbiddenException) when calling the GetRoleCredentials operation: No access

Expected behaviour

We expect sceptre to fail because we don't have access, but we also expect it to make it clear the reason is because of receiving 403 Forbidden when interacting with the AWS API's.

Actual behaviour

We see an UnboundLocalError, which is unhelpful and caused us to dig into the sceptre source code to see what was wrong.

Cause of issue

In _create_deployed_stack_config of sceptre/diffing/stack_differ.py there is a try/except block, but the except only checks for err.response["Error"]["Message"].endswith("does not exist")

When the error is a permission error, err is instead:

{'Error': {'Message': 'No access', 'Code': 'ForbiddenException'}, 'ResponseMetadata': {'RequestId': '0bc1*******', 'HTTPStatusCode': 403, 'HTTPHeaders': {'date': 'Wed, 02 Oct 2024 10:35:33 GMT', 'content-type': 'application/json', 'content-length': '86', 'connection': 'keep-alive', 'access-control-expose-headers': 'RequestId, x-amzn-RequestId', 'requestid': '0bc********', 'server': 'AWS SSO', 'x-amzn-requestid': '0bc*********'}, 'RetryAttempts': 0}}

In this case, the code continues on to line 206

stacks = description["Stacks"]

However description was never set and so we get UnboundLocalError

Suggested Fix

I'm happy to raise a PR, if you approve of the following suggestion: We can add another check for when err.response["Error"]["Code"] == "ForbiddenException" and raise an error in this case, or perhaps we should just allow err to be thrown since the code can not continue from here? I.e.

def _create_deployed_stack_config(
    self, stack_actions: StackActions
) -> Optional[StackConfiguration]:
    try:
        description = stack_actions.describe()
    except ClientError as err:
        # This means the stack has not been deployed yet
        if err.response["Error"]["Message"].endswith("does not exist"):
            return None
        else:
            raise err
  stacks = description["Stacks"]

Let me know how you'd prefer the fix to function, but I think it would be good to get the root issue (403 Forbidden / No access) up to the user.

Thanks, Jak

jak-sdk commented 1 month ago

Just to add, with the suggested fix of raising the err, behaviour is now the following

sceptre diff some/stack.yaml
"An error occurred (ForbiddenException) when calling the GetRoleCredentials operation: No access"
alex-harvey-z3q commented 1 month ago

@jak-sdk I have taken over this issue, do you mind reviewing or if possible testing this PR for us? https://github.com/Sceptre/sceptre/pull/1530