Allow sam local invoke to retrieve account id from current logged in session
#7013
Conversation
github-actions
bot
added
pr/external
stage/needs-triage
| sts = boto3.client("sts") | ||
| try: | ||
| account_id = sts.get_caller_identity()["Account"] | ||
| if account_id: | ||
| if self._global_parameter_overrides is None: | ||
| self._global_parameter_overrides = {} | ||
| self._global_parameter_overrides["AWS::AccountId"] = account_id | ||
| except TokenRetrievalError: | ||
| LOG.warning( "No current session found, using default AWS::AccountId" ) |
There was a problem hiding this comment.
nit: Can we move this logic into its own function? It should also make it easier to test.
There was a problem hiding this comment.
I've moved the code to its own function. Happy to write unit tests but I would appreciate some pointers on where to start, as the functionality relies on whether the user running the code is/isn't logged into an AWS account. I'm not sure how to mock the response from sts.get_caller_identity
There was a problem hiding this comment.
You'll want to write the tests in this file https://github.com/aws/aws-sam-cli/blob/develop/tests/unit/commands/local/cli_common/test_invoke_context.py. There should be a few examples in our unit tests that demonstrate mocking external calls. Here's on you can use as reference: https://github.com/aws/aws-sam-cli/blob/develop/tests/unit/commands/local/cli_common/test_invoke_context.py#L1355
There was a problem hiding this comment.
I've made the suggested changes (as you probably noticed Python is not my first language!) and added unit tests to make sure the new function works both with and without a caller id being returned by STS
Co-authored-by: Daniel Mil <84205762+mildaniel@users.noreply.github.com>
Co-authored-by: Daniel Mil <84205762+mildaniel@users.noreply.github.com>
Co-authored-by: Daniel Mil <84205762+mildaniel@users.noreply.github.com>
| If there is no current session, the standard parameter override for | ||
| AWS::AccountId is used | ||
| """ | ||
| client_provider = samcli.lib.utils.boto_utils.get_boto_client_provider_with_config() |
There was a problem hiding this comment.
Could we update the import at the top of this file to import that method instead of qualifying it here?
There was a problem hiding this comment.
I couldn't get the test to patch correctly if I used from samcli.lib.utils.boto_utils import get_boto_client_provider_with_config. Happy to update if someone can assist here; Python is not my first language..
There was a problem hiding this comment.
Was the issue that the patch wasn't working when moving the import to the top of the file? You might need to patch it using from the invoke_context file instead. That is, instead of
@patch("samcli.lib.utils.boto_utils.get_boto_client_provider_with_config")
The following patch is used instead
@patch("samcli.commands.local.cli_common.invoke_context.get_boto_client_provider_with_config")
|
It looks like the previous CI check failed for linting reasons, can you run |
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
InvokeContextobject 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
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.