Allow sam local invoke to retrieve account id from current logged in session

[defenderkev]

Which issue(s) does this change fix?
#2325
Replaces Pull Request #6568 after feedback from team

Why is this change necessary?
Because currently if you have !Sub xxx${AWS::AccountId}xxx in, for example, a layer definition, SAM uses a default substitution which doesn't reference the correct Account ID

How does it address the issue?

During instantiation of the InvokeContext object an attempt is made to retrieve the caller identity from STS. Assuming this succeeds, the account id is put into _global_parameter_overrides. If a token retrieval error occurs, a warning message is printed and the existing default value is used.

What side effects does this change have?
None that I can see

Mandatory Checklist
PRs will only be reviewed after checklist is complete

[n/a] Add input/output type hints to new functions/methods
[n/a ] Write design document if needed (Do I need to write a design document?)
Write/update unit tests
[n/a] Write/update integration tests
[n/a] Write/update functional tests if needed
make pr passes
[n/a] make update-reproducible-reqs if dependencies were changed
Write documentation

• Not sure where this should be documented. Happy to do so if pointed in the right direction
  By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[mildaniel]

Thanks for the contribution. This looks good to me overall. I left one small nit-picky comment. Can you also add some unit testing?

[mildaniel]

Thanks @defenderkev, a few more minor comments.

[hawflau]

Thanks for your contributions!
Please check my feedback

[lucashuy]

Thanks for maintaining this PR thus far, left some pretty minor comments

[lucashuy]

It looks like the previous CI check failed for linting reasons, can you run make format to automatically fix them?

[jysheng123]

If this function is ok to error, can you also add a generic exception handling right now? If theres no exception handling here, if an error occurs outside of the 2 defined errors, there is potential for this function to panic the execution

[defenderkev]

I caatch these two exceptions because they can be the consequences of

• Not having a ~/.aws/credentials or ~/.aws/config file
• Valid configuration but token is expired
  If anything else causes an error, we don't know what's caused it and in my opinion the execution should panic because we don't know how to handle it.
  Thoughts? I mean, I'm happy to put in a generic except and continue but I'm not sure if that's what we should do here

[lucashuy]

We might want to consider catching the ClientError exception here too. Using expired credentials, botocore returns:

botocore.exceptions.ClientError: An error occurred (ExpiredToken) when calling the GetCallerIdentity operation: The security token included in the request is expired

You had mentioned that other tests start failing once the ClientError was caught, maybe we could look at fixing those?

[defenderkev]

@lucashuy What I've done is add catching ClientError to the new method, and patching the new method for the existing tests so it's just a stub; they don't need a live connection so inserting a live account ID is irrelevant. All tests are passing now
I also put back in some definitions of aws_profile I had taken out of some of those tests. Now the only differences to upstream are the patching of _add_account_id_to_global and the extra tests for the new code.

[jysheng123]

Left one comment about error handling, but otherwise this PR looks good to me