fix(cli): can't use credential providers for stacks with assets #7022
Conversation
Stacks with assets were broken for environments that completely used credential providers (and no other sources of AWS credentials). Not because the wrong credentials were being used (as you would expect it to be broken). Instead, an error was being thrown because the "current" account lookup was incorrectly forwarding to the "default" account lookup (fix: we know what the current account is. Just return that instead). What's worse, this value wasn't even being used! It was being looked up so that if something was wrong finding the target bucket, we could format a nice error message containing the account id. Even happy paths were failing due to the premature lookup though (fix: only look up when we actually need the value). Fixes #7005.
| // don't have "default" credentials, (such as when we're using a | ||
| // CredentialProvider), then we'll just guess the partition to be 'aws'. Our | ||
| // credential provider protocol currently doesn't provide for partition | ||
| // information. | ||
| const account = await this.aws.defaultAccount(); |
There was a problem hiding this comment.
Shouldn't that be based on this.targetEnv.account instead of defaultAccount?
There was a problem hiding this comment.
It should. I was making a simplification with a plan to fix it later. But we could just take the current configuration to look up the partition anyway.
It'll be less efficient but more correct.
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
1 similar comment
|
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
1 similar comment
|
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
Stacks with assets were broken for environments that completely
used credential providers (and no other sources of AWS credentials).
Not because the wrong credentials were being used (as you would expect
it to be broken). Instead, an error was being thrown because the
"current" account lookup was incorrectly forwarding to the "default"
account lookup (fix: we know what the current account is. Just return
that instead).
What's worse, this value wasn't even being used! It was being looked up
so that if something was wrong finding the target bucket, we could
format a nice error message containing the account id. Even happy
paths were failing due to the premature lookup though (fix: only
look up when we actually need the value).
Fixes #7005.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license