Added the ability to specify --accountid parameter on the command line
Which issue(s) does this change fix?
#2325
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?
It adds the ability to specify a command line option( --accountid ). This parameter is then used during the substitution of parameters in the CloudFormation template to replace ${AWS::AccountId} rather than using a hardcoded default (123456789012)
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?)
- [x] Write/update unit tests
- [n/a] Write/update integration tests
- [n/a] Write/update functional tests if needed
- [x]
make prpasses - [n/a]
make update-reproducible-reqsif dependencies were changed - [x] Write documentation - parameter description added to 'AWS Credential Options' section for --help
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
Hi @defenderkev, thanks for opening a PR. I consulted with the team and we agreed to read the account-id from the credentials file and resolve it rather than adding a new flag to all the commands. Would you be open to update your PR with those changes?
Certainly. Once I commit the update, will that notify you or do I need to bump the PR somehow?
You can notify us once the PR is ready for review and we can take a look.
Can you confirm which file you're referring to? The only place I can see where the AWS CLI stores the account id is in the config file when using Identity Center SSO - the standard ~/.aws/credentials file does not store the account id.
The page I referenced here is https://docs.aws.amazon.com/cli/latest/userguide/cli-configure-files.html
Hi @defenderkev, apologies for the late reply. You are right, account_id is not defined in the config file except when using SSO credentials. I was thinking in the sense that the credentials in that file could be used to create a sts boto3 client to retrieve the account_id. An alternate solution could also be to define the account_id as a parameter and read the provided AWS::AccountId as --parameter-overrides value to parse in SAM CLI as @Bradleywboggs mentions here.
Hi @defenderkev, just checking in if the above suggestions make sense to you. Are you still willing to continue contributions on this PR?
Sorry, work has been crazy recently. I'm still willing to continue with this, but I just haven't had time to review your suggestions yet. I hope to get some time in the next couple of weeks
On Thu, Feb 29, 2024 at 6:25 AM Haresh Nasit @.***> wrote:
Hi @defenderkev https://github.com/defenderkev, just checking in if the above suggestions make sense to you. Are you still willing to continue contributions on this PR?
— Reply to this email directly, view it on GitHub https://github.com/aws/aws-sam-cli/pull/6568#issuecomment-1970488111, or unsubscribe https://github.com/notifications/unsubscribe-auth/BAMIXGDB7YO4RTGLUQAZGCDYWANMJAVCNFSM6AAAAABB66I3KOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNZQGQ4DQMJRGE . You are receiving this because you were mentioned.Message ID: @.***>
I'm going to convert this PR to draft for now since it is still being worked on, but not actively. Let us know if there is anything you need on our end to help push this forward.
Hey @defenderkev, closing your PR for now since there's no activity. You can still work on the PR to address the comments. And feel free to re-open the PR once it's ready for review. Thanks!