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

[s3-deployment] BucketDeployment not reading role from Role.fromRoleArn #9989

Closed
mukeshchauhan opened this issue Aug 26, 2020 · 9 comments
Closed
Labels
@aws-cdk/aws-s3-deployment bug This issue is a bug. effort/medium Medium work item – several days of effort p1

Comments

@mukeshchauhan
Copy link

mukeshchauhan commented Aug 26, 2020

Role.fromRoleArn have no affect when provided to BucketDeployment via props

Issue

Instantiating BucketDeployment with provided Role.fromRoleArn have no affect, approval for updating the policy is presented with the same permissions that are in the existing role. Due to corporate IAM limitations, we cannot approve the IAM changes on fly and have to use existing roles.

Environment

  • CDK CLI Version: 1.60.0
  • Module Version: 1.60.0
  • Node.js Version: v12.18.2
  • OS: osx 10.15.6
  • Language: Typescript

Code Snippet

        let destinationBucket = Bucket.fromBucketName(this, 'myexistingbucket', 'test-dev-us-east-1');
        let irole = Role.fromRoleArn(this, 'exising_role', 'arn:aws:iam::12345:role/MyRole');
        let dep = new BucketDeployment(this, 'MyFiles', {
            sources: [Source.asset(path.join(__dirname, '../../dist/test'))],
            destinationBucket: destinationBucket,
            role: irole,
            serverSideEncryption: ServerSideEncryption.AWS_KMS,
            serverSideEncryptionAwsKmsKeyId: 'arn:aws:kms:us-east-1:12345:key/123-123-1233'
        });
@mukeshchauhan mukeshchauhan changed the title BucketDeployment not reading Role.fromRoleArn BucketDeployment not reading role from Role.fromRoleArn Aug 26, 2020
@SomayaB SomayaB changed the title BucketDeployment not reading role from Role.fromRoleArn [s3-deployment] BucketDeployment not reading role from Role.fromRoleArn Aug 31, 2020
@SomayaB SomayaB added needs-triage This issue or PR still needs to be triaged. bug This issue is a bug. @aws-cdk/aws-s3-deployment and removed @aws-cdk/aws-s3-deployment labels Aug 31, 2020
@iliapolo
Copy link
Contributor

iliapolo commented Sep 1, 2020

@mukeshchauhan

Instantiating BucketDeployment with provided Role.fromRoleArn have no affect

What do you mean have no affect? The way it should work is by adding an AWS::IAM::Policy resource that grants your role the necessary permissions to download from the staging bucket and upload to the destination bucket.

Is that policy not being created? Are you getting errors during deployment?

@iliapolo iliapolo added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Sep 1, 2020
@SomayaB SomayaB removed the needs-triage This issue or PR still needs to be triaged. label Sep 1, 2020
@github-actions
Copy link

github-actions bot commented Sep 9, 2020

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.

@github-actions github-actions bot added the closing-soon This issue will automatically close in 4 days unless further comments are made. label Sep 9, 2020
@mukeshchauhan
Copy link
Author

Because of corporate policies, creating roles at runtime is not allowed (which cdk is doing ) All roles are reviewed and approved before they can be used. For now we found a work around mentioned here. #3684

@github-actions github-actions bot removed closing-soon This issue will automatically close in 4 days unless further comments are made. response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. labels Sep 10, 2020
@iliapolo iliapolo added the needs-triage This issue or PR still needs to be triaged. label Sep 15, 2020
@iliapolo iliapolo added effort/medium Medium work item – several days of effort p1 and removed needs-triage This issue or PR still needs to be triaged. labels Sep 27, 2020
@iliapolo iliapolo removed their assignment Jun 27, 2021
@otaviomacedo
Copy link
Contributor

@mukeshchauhan, just to clarify a couple of things first: when you provide a role to BucketDeployment, the CDK will respect that and assign that role to the Lambda function. And, as @iliapolo pointed out, it will create a policy with the right permissions (s3:GetObject*, s3:GetBucket*, etc) and attach that policy to the function's role, regardless of where that role came from.

Now, to your problem: I get it. It would make more sense if, when presented with a custom role, it didn't attach any policy to it. As part of its contract, BucketDeployment should assume that the role has all the right permissions already. But if we change that now, it will remove some permissions that users are (implicitly or explicitly) relying on.

@Georift
Copy link

Georift commented Dec 2, 2021

@otaviomacedo we noticed this today at work. I also feel like providing a custom role should prevent a new one being made. I've been thinking but I can't think of a use case where adding extra permissions would make sense for a S3 Deployment. Given we're not in control of the code, what more permission could it need than the S3 access it's already making?

FWIW we worked around it with some disgusting patching of the bucket passed in to make grantReadWrite have no effect:

Bucket.fromBucketArn(this, "nulledReadWrite", filesBucket.bucketArn).grantReadWrite = (() => {
	console.trace('nulled out grantReadWrite() will have no effect');
}) as any;

@otaviomacedo
Copy link
Contributor

Ok, what I said above is not entirely correct. I wasn't aware that there is a boolean property, mutable that you can pass to Role.fromRoleArn(). If you set that property to false, the role will be left untouched and no additional policy or role will be created. I believe this solves the problem.

Feel free to reopen if you have additional questions.

@github-actions
Copy link

github-actions bot commented Dec 2, 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.

@Georift
Copy link

Georift commented Dec 2, 2021

Perfect thank you!

@BillyWitrock
Copy link

I also found, that if you are passing in a role, you can use the withoutPolicyUpdates() function to get a copy of the role that ignores any grants it is given, and pass that in instead.

https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_iam.Role.html#withoutwbrpolicywbrupdatesoptions

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-s3-deployment bug This issue is a bug. effort/medium Medium work item – several days of effort p1
Projects
None yet
Development

No branches or pull requests

6 participants