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: improve usability of --cloudformation-execution-policies #8571

Closed
zxkane opened this issue Jun 16, 2020 · 12 comments · Fixed by #9867
Closed

cli: improve usability of --cloudformation-execution-policies #8571

zxkane opened this issue Jun 16, 2020 · 12 comments · Fixed by #9867
Assignees
Labels
@aws-cdk/core Related to core CDK functionality @aws-cdk/pipelines CDK Pipelines library effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p1

Comments

@zxkane
Copy link
Contributor

zxkane commented Jun 16, 2020

Deploy an app via new asset system "@aws-cdk/core:newStyleStackSynthesis": true,, got below errors,

6/22 | 5:39:27 PM | CREATE_FAILED        | AWS::IAM::Role                            | Nexus3-Execution-Role (Nexus3ExecutionRole7D7DCE03) API: iam:CreateRole User: arn:aws-cn:sts::034068960621:assumed-role/cdk-hnb659fds-cfn-exec-role-034068960621-cn-northwest-1/AWSCloudFormation is not authorized to perform: iam:CreateRole on resource: arn:aws-cn:iam::034068960621:role/EcsSonatypeNexus3Stack-Nexus3ExecutionRole7D7DCE03-PPD1G31B45J9
8/22 | 5:39:27 PM | CREATE_FAILED        | AWS::ECS::Cluster                         | Nexus3Cluster (Nexus3ClusterE4F27F99) User: arn:aws-cn:sts::034068960621:assumed-role/cdk-hnb659fds-cfn-exec-role-034068960621-cn-northwest-1/AWSCloudFormation is not authorized to perform: ecs:DescribeClusters on resource: arn:aws-cn:ecs:cn-northwest-1:034068960621:cluster/EcsSonatypeNexus3Stack-Nexus3ClusterE4F27F99-0FoUr3S2HsAt (Service: Ecs, Status Code: 400, Request ID: 204508a4-29cd-425c-9ec6-527464a04f9d)
9/22 | 5:39:40 PM | CREATE_FAILED        | AWS::EFS::FileSystem                      | Nexus3FileSystem (Nexus3FileSystemEFB61BFC) User: arn:aws-cn:sts::034068960621:assumed-role/cdk-hnb659fds-cfn-exec-role-034068960621-cn-northwest-1/AWSCloudFormation is not authorized to perform: elasticfilesystem:CreateFileSystem on the specified resource (Service: Efs, Status Code: 403, Request ID: a2f803d1-c166-45c2-84d6-d2e15c905025, Extended Request ID: null)

Reproduction Steps

  1. Specify the current aws profile via below env,
export AWS_DEFAULT_PROFILE=
  1. enable new asset system in cdk.json
{
  "context": {
    "@aws-cdk/core:newStyleStackSynthesis": true
  }
}
  1. Deploy the app npx cdk deploy

Error Log

See above

Environment

  • CLI Version : 1.45.0
  • Framework Version:
  • Node.js Version:
  • OS :
  • Language (Version):

Other


This is 🐛 Bug Report

@zxkane zxkane added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jun 16, 2020
@SomayaB SomayaB added the @aws-cdk/core Related to core CDK functionality label Jun 17, 2020
@eladb eladb assigned rix0rrr and unassigned eladb Jun 18, 2020
@rix0rrr
Copy link
Contributor

rix0rrr commented Jun 18, 2020

Did you bootstrap and passed --cloudformation-execution-policies arn:aws:iam::aws:policy/AdministratorAccess to give sufficient permissions to the CloudFormation execution role?

@zxkane
Copy link
Contributor Author

zxkane commented Jun 18, 2020

Did you bootstrap and passed --cloudformation-execution-policies arn:aws:iam::aws:policy/AdministratorAccess to give sufficient permissions to the CloudFormation execution role?

Yes. I did reran the bootstrap with new asset system via below command.

env CDK_NEW_BOOTSTRAP=1 npx cdk bootstrap \
  --cloudformation-execution-policies arn:aws:iam::aws:policy/AdministratorAccess

I find the permission issue only happening when updating the existing stack CDKToolkit, the CloudFormationExecutionRole role cdk-xxx-cfn-exec-role-<accountid>-ap-southeast-1 is not associated with the given policy arn:aws:iam::aws:policy/AdministratorAccess. I deleted the existing stack CDKToolkit, then bootstraping from scratching it works.

Also the bootstrap runs without error if I specify a non-existing policy arn:aws:iam::aws:policy/AdministratorAccess when bootstraping in China region. The ARN of administrator policy is arn:aws-cn:iam::aws:policy/AdministratorAccess in China region, it would be helpful printing out error message if the given policy does not exist or the permission of policy is insufficient for deploying cloudformation stack.

@SomayaB SomayaB removed the needs-triage This issue or PR still needs to be triaged. label Jun 18, 2020
@rix0rrr
Copy link
Contributor

rix0rrr commented Jun 19, 2020

Your last 2 concerns are hard to work around. IAM should validate whether the referenced policy exists, but apparently it doesn't.

Also, we have no way to know beforehand whether the given policy is going to give enough permissions, because we won't know what kind of permissions you need.

In some other environments you may legitimately want to use a narrower policy than AdministratorAccess.

@rix0rrr
Copy link
Contributor

rix0rrr commented Jun 19, 2020

I find the permission issue only happening when updating the existing stack CDKToolkit, the CloudFormationExecutionRole role cdk-xxx-cfn-exec-role-<accountid>-ap-southeast-1 is not associated with the given policy arn:aws:iam::aws:policy/AdministratorAccess. I deleted the existing stack CDKToolkit, then bootstraping from scratching it works.

This is concerning to me, and goes against everything I thought I knew.

Can you reproduce this? If you bootstrap using the old stack and then upgrade, using the --cloudformation-execution-policies flag, and the policy is not attached? I'm asking because we have a test that does that and it works, so there must be something else going on in your environment.

@zxkane
Copy link
Contributor Author

zxkane commented Jun 22, 2020

I can reproduce it if bootstrapping with enabling new asset system without specifying the arn of execution role in first time.

Then the CDKToolkit won't be updated after bootstrapping with specifying another execution role.

env CDK_NEW_BOOTSTRAP=1 npx cdk bootstrap
 ⏳  Bootstrapping environment aws://034068960xxx/cn-north-1...
CDKToolkit: creating CloudFormation changeset...
  0/11 | 10:11:52 AM | CREATE_IN_PROGRESS   | AWS::IAM::Role        | FilePublishingRole
  0/11 | 10:11:52 AM | CREATE_IN_PROGRESS   | AWS::ECR::Repository  | ContainerAssetsRepository
  0/11 | 10:11:52 AM | CREATE_IN_PROGRESS   | AWS::IAM::Role        | CloudFormationExecutionRole
  0/11 | 10:11:52 AM | CREATE_IN_PROGRESS   | AWS::IAM::Role        | ImagePublishingRole
  0/11 | 10:11:52 AM | CREATE_IN_PROGRESS   | AWS::ECR::Repository  | ContainerAssetsRepository Resource creation Initiated
  0/11 | 10:11:52 AM | CREATE_IN_PROGRESS   | AWS::IAM::Role        | FilePublishingRole Resource creation Initiated
  0/11 | 10:11:53 AM | CREATE_IN_PROGRESS   | AWS::IAM::Role        | CloudFormationExecutionRole Resource creation Initiated
  1/11 | 10:11:53 AM | CREATE_COMPLETE      | AWS::ECR::Repository  | ContainerAssetsRepository
  1/11 | 10:11:53 AM | CREATE_IN_PROGRESS   | AWS::IAM::Role        | ImagePublishingRole Resource creation Initiated
  2/11 | 10:12:02 AM | CREATE_COMPLETE      | AWS::IAM::Role        | FilePublishingRole
  3/11 | 10:12:02 AM | CREATE_COMPLETE      | AWS::IAM::Role        | CloudFormationExecutionRole
  4/11 | 10:12:02 AM | CREATE_COMPLETE      | AWS::IAM::Role        | ImagePublishingRole
  4/11 | 10:12:04 AM | CREATE_IN_PROGRESS   | AWS::KMS::Key         | FileAssetsBucketEncryptionKey
  4/11 | 10:12:04 AM | CREATE_IN_PROGRESS   | AWS::IAM::Role        | DeploymentActionRole
  4/11 | 10:12:04 AM | CREATE_IN_PROGRESS   | AWS::KMS::Key         | FileAssetsBucketEncryptionKey Resource creation Initiated
  4/11 | 10:12:04 AM | CREATE_IN_PROGRESS   | AWS::IAM::Role        | DeploymentActionRole Resource creation Initiated
  4/11 | 10:12:04 AM | CREATE_IN_PROGRESS   | AWS::IAM::Policy      | ImagePublishingRoleDefaultPolicy
  4/11 | 10:12:05 AM | CREATE_IN_PROGRESS   | AWS::IAM::Policy      | ImagePublishingRoleDefaultPolicy Resource creation Initiated
  5/11 | 10:12:14 AM | CREATE_COMPLETE      | AWS::IAM::Policy      | ImagePublishingRoleDefaultPolicy
  6/11 | 10:12:14 AM | CREATE_COMPLETE      | AWS::IAM::Role        | DeploymentActionRole
 6/11 Currently in progress: FileAssetsBucketEncryptionKey
  7/11 | 10:13:04 AM | CREATE_COMPLETE      | AWS::KMS::Key         | FileAssetsBucketEncryptionKey
  7/11 | 10:13:09 AM | UPDATE_IN_PROGRESS   | AWS::S3::Bucket       | StagingBucket Requested update requires the creation of a new physical resource; hence creating one.
  7/11 | 10:13:09 AM | UPDATE_IN_PROGRESS   | AWS::S3::Bucket       | StagingBucket Resource creation Initiated
  8/11 | 10:13:31 AM | UPDATE_COMPLETE      | AWS::S3::Bucket       | StagingBucket
  8/11 | 10:13:33 AM | CREATE_IN_PROGRESS   | AWS::IAM::Policy      | FilePublishingRoleDefaultPolicy
  8/11 | 10:13:34 AM | CREATE_IN_PROGRESS   | AWS::IAM::Policy      | FilePublishingRoleDefaultPolicy Resource creation Initiated
  8/11 | 10:13:35 AM | UPDATE_IN_PROGRESS   | AWS::S3::BucketPolicy | StagingBucketPolicy Requested update requires the creation of a new physical resource; hence creating one.
  8/11 | 10:13:36 AM | UPDATE_IN_PROGRESS   | AWS::S3::BucketPolicy | StagingBucketPolicy Resource creation Initiated
  9/11 | 10:13:36 AM | UPDATE_COMPLETE      | AWS::S3::BucketPolicy | StagingBucketPolicy
 10/11 | 10:13:42 AM | CREATE_COMPLETE      | AWS::IAM::Policy      | FilePublishingRoleDefaultPolicy
 10/11 | 10:13:44 AM | UPDATE_COMPLETE_CLEA | AWS::CloudFormation::Stack | CDKToolkit
 10/11 | 10:13:45 AM | DELETE_IN_PROGRESS   | AWS::S3::BucketPolicy | StagingBucketPolicy
 ✅  Environment aws://034068960xxx/cn-north-1 bootstrapped.

(base) base ❯ env CDK_NEW_BOOTSTRAP=1 npx cdk bootstrap \
  --cloudformation-execution-policies arn:aws-cn:iam::aws:policy/AdministratorAccess
 ⏳  Bootstrapping environment aws://034068960xxx/cn-north-1...
 ✅  Environment aws://034068960xxx/cn-north-1 bootstrapped (no changes).

@zxkane zxkane changed the title asset: Failed on lots of no permission when deploying with new asset system asset: can not update cfn-execution-role of CDKToolkit stack though specifying new arn of execution role Jun 22, 2020
@zxkane
Copy link
Contributor Author

zxkane commented Jun 22, 2020

Your last 2 concerns are hard to work around. IAM should validate whether the referenced policy exists, but apparently it doesn't.

Also, we have no way to know beforehand whether the given policy is going to give enough permissions, because we won't know what kind of permissions you need.

In some other environments you may legitimately want to use a narrower policy than AdministratorAccess.

How about checking the existence of given execution role in synthesis phase? It should be doable via IAM API.

And how about using the managed AdministratorAccess policy as default permission of execution role? It does not make sense creating an execution role without any permission.

@rix0rrr
Copy link
Contributor

rix0rrr commented Jun 22, 2020

I know, but this is a requirement from our security department. YOU need to be the person who opts in to giving a role potentially dangerous permissions. We cannot do that for you.

@hoegertn
Copy link
Contributor

Could this opt-in be in the form of an interactive confirmation?

rix0rrr added a commit that referenced this issue Jun 22, 2020
Our nested stack deployment optimizer only used to look at template
differences in order to be able to skip deployments. This has been
enhanced over time, but stack parameters were still not included
in the evaluation.

The new bootstrapping experience relies heavily on parameters to
configure important aspects such as policies and trust. However,
due to the stack deployment skipping, you would not be able to
reconfigure trust or policies after the initial deployment.

Now takes parameters into account as well.

Relates to #8571, relates to #6582, fixes #6581.
@rix0rrr
Copy link
Contributor

rix0rrr commented Jun 22, 2020

Could this opt-in be in the form of an interactive confirmation?

That is something we'll pursue but due to organizational reasons it may be a couple of weeks before we can get an answer to that question.

@hoegertn
Copy link
Contributor

Ok, can you then make the cli argument mandatory to prevent users from creating exec roles without permissions?

mergify bot pushed a commit that referenced this issue Jun 24, 2020
)

Our nested stack deployment optimizer only used to look at template
differences in order to be able to skip deployments. This has been
enhanced over time, but stack parameters were still not included
in the evaluation.

The new bootstrapping experience relies heavily on parameters to
configure important aspects such as policies and trust. However,
due to the stack deployment skipping, you would not be able to
reconfigure trust or policies after the initial deployment.

Now takes parameters into account as well.

Relates to #8571, relates to #6582, fixes #6581.


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@rix0rrr rix0rrr added p1 @aws-cdk/pipelines CDK Pipelines library and removed bug This issue is a bug. labels Jul 8, 2020
@rix0rrr rix0rrr changed the title asset: can not update cfn-execution-role of CDKToolkit stack though specifying new arn of execution role cli: improve usability of --cloudformation-execution-policies Jul 8, 2020
@rix0rrr rix0rrr added effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. labels Jul 8, 2020
@rix0rrr rix0rrr added this to the [CDK Pipelines] Soon milestone Aug 12, 2020
@rix0rrr rix0rrr added effort/small Small work item – less than a day of effort and removed effort/medium Medium work item – several days of effort labels Aug 12, 2020
@rix0rrr
Copy link
Contributor

rix0rrr commented Aug 12, 2020

It was supposed to default to "admin" if you didn't establish any --trusts, we need to investigate why that doesn't work.

rix0rrr added a commit that referenced this issue Aug 20, 2020
The current bootstrapping experience has sharp edges. It requires you
to pass `--cloudformation-execution-policies`. If you don't pass any,
it will successfully deploy a bootstrap stack that doesn't actually
work.

In effect, you are forced to constantly look up and copy/paste the ARN
for `AdministratorAccess` (because that's effectively what people
need to use anyway). We don't want to default to this policy for them,
because it equates to handing over the keys to your account to another
account, and you need to be fully aware that you're doing that.

In the case of bootstrapping an account just for use "by itself",
however (which is the 90% use case for bootstrapping),
the trust boundary is very clear and there's no risk of privilege
escalation between accounts (there might still be risk of privilege
escalation between different people using different IAM users in
the same AWS account, but that is not a recommended setup).

Add an optimization where in the case of a "simple", non-cross account
bootstrap, we'll default to the AdministratorAccess ARN for you.

Once you establish `--trust` with another account, we'll still
force you to spell out the execution policy you'll want to use though.

Fixes #8571.

Also in this PR:

- The `blockPublicAccessConfiguration` would be reset on every
  re-bootstrap, instead of reusing the previous setting.
- The `enableTerminationProtection` would be reset on every
  re-bootstrap, instead of reusing the previous setting.
@mergify mergify bot closed this as completed in #9867 Oct 28, 2020
mergify bot pushed a commit that referenced this issue Oct 28, 2020
…cy ARNs (#9867)

The current bootstrapping experience has sharp edges. It requires you
to pass `--cloudformation-execution-policies` in all cases, even if
you just want a "simple", same-account bootstrap stack. If you bootstrap
your own accounts, you probably don't have a centralized SecOps
team that is limiting you, and you're probably intending to deploy
everything using the CDK, which means you're looking for AdministratorAccess.

In effect, you are forced to constantly Google and copy/paste the ARN
for `AdministratorAccess` (because who can remember that)
and it's a bad experience.

In the case of bootstrapping an account just for use "by itself",
however (which is the 90% use case for bootstrapping),
the trust boundary is very clear and there's no risk of privilege
escalation between accounts.

Add an optimization where in the case of a "simple", non-cross account
bootstrap, we'll default to the AdministratorAccess ARN for you.

Once you establish `--trust` with another account, we'll still
force you to spell out the execution policy you'll want to use though.

Fixes #8571.

----

*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

⚠️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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/core Related to core CDK functionality @aws-cdk/pipelines CDK Pipelines library effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants