Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(cli): cross account deployments still don't work #12151

Closed
redbaron opened this issue Dec 18, 2020 · 5 comments · Fixed by #12155
Closed

(cli): cross account deployments still don't work #12151

redbaron opened this issue Dec 18, 2020 · 5 comments · Fixed by #12155
Assignees
Labels
bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. p1 package/tools Related to AWS CDK Tools or CLI

Comments

@redbaron
Copy link
Contributor

redbaron commented Dec 18, 2020

Reproduction Steps

CI account: 0147xxxxxxxx
Target account: 1454xxxxxxx

Target account is bootstrapped to trust CI account with --trust 0147xxxxxxxx . IAM user in CI account graned permission iam:AssumeRole to all roles created by bootstrap stack.

Create CDK app with environment in target account and an S3 asset:

...
new Stack(app, 'Stack', { env: {account: "1454xxxxxxx", region: "eu-west-2" }});
...

Do cdk deploy from CI account.

What did you expect to happen?

Successfull deploy

What actually happened?

If we try to deploy it with cdk deploy -v following happens:

OrganizationStack
OrganizationStack: deploying...
Looking up default account ID from STS
Default account ID: 0147xxxxxx
Assuming role 'arn:aws:iam::1454xxxxxx:role/cdk-hnb659fds-deploy-role-1454xxxxx-eu-west-2'.
Waiting for stack CDKToolkit to finish creating or updating...
[0%] start: Publishing 4a3609ad912843e581892f37ae9d6fb0fa1648b547693aaa562b0119452b8956:145469417702-eu-west-2
[50%] fail: Need to perform AWS calls for account 1454xxxxxxx, but the current credentials are for 0147xxxxxx
[50%] start: Publishing 97b2255ceb5577d4534d825ed8407ced52d0f8917ce15f7a76d5c47aba9bd603:145469417702-eu-west-2
[100%] fail: Need to perform AWS calls for account 1454xxxxxxxx, but the current credentials are for 0147xxxxxx

Environment

  • CDK CLI Version : 1.78.-
  • Framework Version:
  • Node.js Version: 14
  • OS : Linux
  • Language (Version): Typescript

Other

Interestingly, cdk-assets uploads just fine. Exact commands when run from CI:

  - yarn add -D [email protected] 
  - node_modules/.bin/cdk synth --verbose
  - node_modules/.bin/cdk-assets publish -p cdk.out/OrganizationStack.assets.json -v
  - node_modules/.bin/cdk deploy --require-approval=never --all --verbose

Produce following output (tail):

$ node_modules/.bin/cdk synth --verbose
Successfully synthesized to /data/build/X-WKy9gs/0/dir/devops/iac/organization/cdk.out
Supply a stack id (OrganizationStack, SingleSignOnStack) to display its template.

$ node_modules/.bin/cdk-assets publish -p cdk.out/OrganizationStack.assets.json -v
verbose: Loaded manifest from cdk.out/OrganizationStack.assets.json: 2 assets found
info   : [0%] start: Publishing 4a3609ad912843e581892f37ae9d6fb0fa1648b547693aaa562b0119452b8956:1454xxxxxxx-eu-west-2
verbose: Assume arn:aws:iam::1454xxxxxxx:role/cdk-hnb659fds-file-publishing-role-1454xxxxxxx-eu-west-2
verbose: [0%] check: Check s3://cdk-hnb659fds-assets-1454xxxxxxx-eu-west-2/4a3609ad912843e581892f37ae9d6fb0fa1648b547693aaa562b0119452b8956.zip
verbose: [0%] found: Found s3://cdk-hnb659fds-assets-1454xxxxxxx-eu-west-2/4a3609ad912843e581892f37ae9d6fb0fa1648b547693aaa562b0119452b8956.zip
info   : [50%] success: Published 4a3609ad912843e581892f37ae9d6fb0fa1648b547693aaa562b0119452b8956:1454xxxxxxx-eu-west-2
info   : [50%] start: Publishing 97b2255ceb5577d4534d825ed8407ced52d0f8917ce15f7a76d5c47aba9bd603:1454xxxxxxx-eu-west-2
verbose: Assume arn:aws:iam::1454xxxxxxx:role/cdk-hnb659fds-file-publishing-role-1454xxxxxxx-eu-west-2
verbose: [50%] check: Check s3://cdk-hnb659fds-assets-1454xxxxxxx-eu-west-2/97b2255ceb5577d4534d825ed8407ced52d0f8917ce15f7a76d5c47aba9bd603
verbose: [50%] upload: Upload s3://cdk-hnb659fds-assets-1454xxxxxxx-eu-west-2/97b2255ceb5577d4534d825ed8407ced52d0f8917ce15f7a76d5c47aba9bd603
info   : [100%] success: Published 97b2255ceb5577d4534d825ed8407ced52d0f8917ce15f7a76d5c47aba9bd603:1454xxxxxxx-eu-west-2


$ node_modules/.bin/cdk deploy --require-approval=never --all --verbose
...
OrganizationStack
OrganizationStack: deploying...
Looking up default account ID from STS
Default account ID: 0147xxxxxxx
Assuming role 'arn:aws:iam::1454xxxxxxx:role/cdk-hnb659fds-deploy-role-1454xxxxxxx-eu-west-2'.
Waiting for stack CDKToolkit to finish creating or updating...
[0%] start: Publishing 4a3609ad912843e581892f37ae9d6fb0fa1648b547693aaa562b0119452b8956:1454xxxxxxx-eu-west-2
[50%] fail: Need to perform AWS calls for account 1454xxxxxxx, but the current credentials are for 0147xxxxxxx
[50%] start: Publishing 97b2255ceb5577d4534d825ed8407ced52d0f8917ce15f7a76d5c47aba9bd603:1454xxxxxxx-eu-west-2
[100%] fail: Need to perform AWS calls for account 1454xxxxxxx, but the current credentials are for 0147xxxxxxx

Looks like fix done in #11966 is incomplete.

/cc @rix0rrr , @scarytom , @polothy


This is 🐛 Bug Report

@redbaron redbaron added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Dec 18, 2020
@github-actions github-actions bot added the package/tools Related to AWS CDK Tools or CLI label Dec 18, 2020
@redbaron
Copy link
Contributor Author

I tracked it down to:

public async discoverCurrentAccount(): Promise<cdk_assets.Account> {
return (await this.sdk({})).currentAccount();
}

cdk_assets.IAws.discoverCurrentAccount() method meant to return id a of a target account. Among other users,it is used by FileAssetHandler from cdk_asset to replace placeholders in asset.destination:

const destination = await replaceAwsPlaceholders(this.asset.destination, this.host.aws);

But asset.destination contains assumeRoleArn (via cloud-assembly-schema.AwsDestination), which can contain placeholders! Problem lies in a circular dependency:

  • To template assumeRoleArn we need to call STS().getCallerIdentity()
  • but getCallerIdentity() need to be called from the assumed role assumeRoleArn to produce target account id!

SdkProvider.ForEnvironment expects either source account to match target account or to be called with fully resolved role to assume in the target account. cdk_assets.IAws.discoverCurrentAccount() doesn't have that role yet, because it is used to resolve that very role.

It would be tricky to resolve. Possibly if placeholder replacement for AwsDestination was done with source (base) credentials only it can fix it. This will break circular dependency without breaking APIs.

@OperationalFallacy
Copy link

Seeing this with CloudFormationCreateUpdateStackAction and CdkPipeline.

I have a question, how the CDK Pipeline is able to deploy across accounts (example here #10166)?
Isn't this broken plugin-based assume creds functionality relies on the same account/roles bootstrapping by cdk?

@polothy
Copy link
Contributor

polothy commented Jan 4, 2021

Just got back from PTO - in my testing, the CDK diff seemed to work cross account, but then yeah, got the same error as you when it tried to deploy and it failed to upload an asset.

Thanks for submitting that PR, hopefully that gets us across the line and I can drop this plugin 🤞

@mergify mergify bot closed this as completed in #12155 Jan 6, 2021
mergify bot pushed a commit that referenced this issue Jan 6, 2021
cdk_asset asset handlers use IAws to make calls to AWS APIs to discover
information about target environment: account id, region, partition.

Each asset is described by its manifest in a Cloud Assembly. This manifest
can contain placeholders to resolve by asset handlers when publishing
assets.

Previously `${Aws::Partition}` placeholder was derived from a code path used
to resolve `${Aws::AccountId}`, which was introducing a cyclic dependency for
cross account deployments:

- to replace partition placeholder it was assuming role in a target account
  to discover partition
- to assume role in a target account it needs to know full role ARN to assume
- role ARN contains partition placeholder

It was working for same account deployments and for non environment aware deployments,
because SdkProvider was always using current default (ambient) credentials without making
`AssumeRole` call, thus it was able to replace placeholders in asset manifest without
introducing a cyclic dependency.

To fix cross account deployments we introduce `IAWS.discoverPartition()` method to return
partition of default (ambient) credentials `cdk deploy` is called with. This works, because
cross partition `AssumeRole` calls are not possible, therefore it's enough to know our
default credentials partition.


Fixes #12151

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

github-actions bot commented Jan 6, 2021

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

@polothy
Copy link
Contributor

polothy commented Jan 13, 2021

Maybe no one will see this comment, but I was just able to deploy cross account with no plugins 🎉

Thanks @redbaron and @rix0rrr for fixing this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. p1 package/tools Related to AWS CDK Tools or CLI
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants