aws / aws-cdk

The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code
https://aws.amazon.com/cdk
Apache License 2.0
11.66k stars 3.92k forks source link

(cdk): diff ignores --role-arn #17150

Open amirfireeye opened 3 years ago

amirfireeye commented 3 years ago

What is the problem?

cdk --role-arn arn:aws:iam::123:role/xxx diff still tries to assume the default cdk-hnb659fds-deploy-role-123-us-xxx role. We want to use custom limited roles and this limitation means we are forced to give diff users full access to target accounts with the deploy role.

I believe the problem is with this line:

https://github.com/aws/aws-cdk/blob/74776f393462f7e7d23cb1953ef786a823adc896/packages/aws-cdk/lib/cdk-toolkit.ts#L104

It needs to pass along args.roleArn to prepareSdkFor() just like bootstrap and deploy commands do.

Reproduction Steps

Setup a profile that cannot assume the deploy role, but can assume another role that has access to read stacks (xxx in this example). Use:

cdk --role-arn arn:aws:iam::123:role/xxx diff

What did you expect to happen?

I would expect CDK to assume the role I asked it to assume and successfully print a diff.

What actually happened?

Could not assume role in target account using current credentials (which are for account 123) User: arn:aws:sts::123:assumed-role/instance-role/i-123 is not authorized to perform: sts:AssumeRole on resource: arn:aws:iam::123:role/cdk-hnb659fds-deploy-role-123-us-west-2 . Please make sure that this role exists in the account. If it doesn't exist, (re)-bootstrap the environment with the right '--trust', using the latest version of the CDK CLI.

CDK CLI Version

1.129.0

Framework Version

1.121.0

Node.js Version

16.12.0

OS

macOS Big Sur

Language

Python

Language Version

3.8

Other information

Also reported on StackOverflow https://stackoverflow.com/questions/68422581/cdk-diff-with-read-only-permissions-what-is-a-good-way

igilham commented 2 years ago

I'm finding this problem to also exist for the deploy command e.g. cdk -r arn:aws:iam:::role/my-cfn-role deploy MyStack.

Using CDK 2.5.0 NodeJS on MacOS.

mek88 commented 2 years ago

I believe fixing this would ultimately require a change to what --role-arn means, since this is the role that gets used when the deploy role interacts with cloudformation and is not the role that the cdk assumes when interacting with the a destination account for a CDK diff or deploy.

With that said, we have the same fundamental problem outlined in this issue. Out of the box, we cannot give users or CI/CD systems the ability to run a diff without also giving them the ability to do a deploy. I'd like to see a new role, less empowered, that allows for diffs but not deploys/deletes. The PR tied to this issue appears to do that but was closed.

igilham commented 2 years ago

That's useful context - thanks, @mek88.

I can confirm that CDK commands work if the --role-arn you assume has sufficient permissions to perform the operations that CDK invokes, but it will still try to assume the bootstrapped roles anyway. The CLI warns the user when this fails. So it works but the CLI output is messy and a little disingenuous.

Maybe the CLI messaging and related documentation could be clearer to help us to know what to expect.

Calling for more granularity of roles may be best to split into another issue.

corymhall commented 2 years ago

A couple of things have been changed/added recently that should address this. cdk diff now assumes the lookup bootstrap role which has read only permissions. In addition if you do not want to use the bootstrap roles at all you can use the CliCredentialsStackSynthesizer which uses the current credentials and does not attempt to assume any roles.

https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib-readme.html#stack-synthesizers

github-actions[bot] commented 2 years ago

This issue has not received a response in a while. If you want to keep this issue open, please leave a comment below and auto-close will be canceled.

peterwoodworth commented 2 years ago

This issue is still relevant because the documentation implies that you should be able to set the role arn when calling diff which is not the case. We should either let you set the role to use, or update documentation to be more clear

ntachukwu commented 2 years ago

hi @peterwoodworth , could you point me to the documentation you mentioned above?

peterwoodworth commented 2 years ago

There's a couple places where our documentation is found related to CLI options.

One place is using the --help option. cdk diff --help will show --role-arn as an option. This is because --role-arn is considered a "global" option based on its placement in the CLI options here https://github.com/aws/aws-cdk/blob/e280dfe7c5062c76a2be2ed2ad5f623e57fdb188/packages/aws-cdk/lib/cli.ts#L77

Since this is a global option, it shows up as an option for all commands - which isn't accurate. You can improve this in one of two ways:

  1. Remove the command as a global option, and include it as an option for ALL commands that currently make use of it
  2. Clarify the description of the option better. "Invoking CloudFormation" is vague enough to potentially include any commands that make a CloudFormation API call, like cdk diff does. I believe that this option is just specific to creating, updating, or destroying stacks.

Another place documentation on this topic is found is in our devguide. Our devguide doesn't make note of the --role-arn option, however, no documentation exists on what set of permissions cdk diff will assume. I think we should clarify this in the devguide

gabrielenosso commented 2 years ago

Hi everyone, I still don't get it how can we run "cdk diff" without deployment permissions. In my case, I would like to run "cdk diff" directly from GitHub Actions (so the keys would be saved as GitHub Secrets). How can I be sure that I only give Read permissions to those?