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

[custom-resources] allow for no policy to be specified #13232

Closed
mh-love opened this issue Feb 23, 2021 · 9 comments · Fixed by #20966
Closed

[custom-resources] allow for no policy to be specified #13232

mh-love opened this issue Feb 23, 2021 · 9 comments · Fixed by #20966
Labels
@aws-cdk/custom-resources Related to AWS CDK Custom Resources effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. good first issue Related to contributions. See CONTRIBUTING.md p2

Comments

@mh-love
Copy link

mh-love commented Feb 23, 2021

I do not have the ability to modify roles and the CDK assumes I do for an AwsCustomResource. I can specify the execution role for the custom resource Lambda, but I have to specify a policy, and cdk deploy fails for me because I don't have access to modify the execution role with the new policy. I would expect for policy to be optional if role is specified. I am in a corporate setting where permissions are "locked down" and roles exist but can not be modified.

Reproduction Steps

export class ExistingS3BucketEventSource extends CDK.Construct {
  constructor(scope: CDK.Construct, id: string, props: S3NotificationLambdaProps) {
    super(scope, id);

    new CR.AwsCustomResource(scope, id + 'CustomResource', {
      onCreate: {
        ...
      },
      onDelete: {
        ...
      },
      policy: CR.AwsCustomResourcePolicy.fromStatements([]), // I don't want this! Also specifying no statements doesn't work!
      role: props.role // I want permissions from here!
    });

    props.lambda.addPermission('AllowS3Invocation', {
      action: 'lambda:InvokeFunction',
      principal: new IAM.ServicePrincipal('s3.amazonaws.com'),
      sourceArn: props.bucket.bucketArn
    });
  }
}

interface S3NotificationLambdaProps {
  role: IAM.IRole;
  bucket: S3.IBucket;
  lambda: Lambda.IFunction;
  events: string[];
  prefix: string;
}

What did you expect to happen?

I do not want to modify the execution role.

What actually happened?

The execution role is modified.

Environment

  • CDK CLI Version : 1.90.0 (build 7edba31)
  • Framework Version: 1.90.0
  • Node.js Version: v12.18.3
  • OS : Catalina 10.15.7
  • Language (Version): TypeScript (3.8.3)

Other


This is 🐛 Bug Report

@mh-love mh-love added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Feb 23, 2021
@github-actions github-actions bot added the @aws-cdk/custom-resources Related to AWS CDK Custom Resources label Feb 23, 2021
@rix0rrr
Copy link
Contributor

rix0rrr commented Feb 24, 2021

I guess I don't mind that field being optional.

In the mean time, you can use role.withoutPolicyUpdates() (or call Role.fromRoleName(..., { mutable: false })) to prevent your existing role from being attempted to be updated.

@rix0rrr rix0rrr added effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. good first issue Related to contributions. See CONTRIBUTING.md p2 and removed bug This issue is a bug. labels Feb 24, 2021
@rix0rrr rix0rrr changed the title [custom-resources] Use policy associated with execution role, allow for no policy to be specified [custom-resources] allow for no policy to be specified Feb 24, 2021
@mainframenzo
Copy link

@rix0rrr Very useful tip. Thank you!

@BLasan
Copy link
Contributor

BLasan commented Mar 1, 2021

@rix0rrr Still opened?

@BLasan
Copy link
Contributor

BLasan commented Mar 1, 2021

@rix0rrr Can I give a try for this?

@jacobsnapp
Copy link

jacobsnapp commented May 17, 2021

I guess I don't mind that field being optional.

In the mean time, you can use role.withoutPolicyUpdates() (or call Role.fromRoleName(..., { mutable: false })) to prevent your existing role from being attempted to be updated.

@rix0rrr - I have used Role.from_role_arn( ..., mutable=False) and role.without_policy_updates() with no luck (in Python). AwsCustomResource does not seem to respect the immutable role and always adds duplicate inline policies.

@ryparker ryparker removed the needs-triage This issue or PR still needs to be triaged. label Jun 1, 2021
@rix0rrr rix0rrr removed their assignment Jun 3, 2021
@mattiamatrix
Copy link
Contributor

Hello, I am also having the same issue with CDK 2.17.0. Did anyone find a workaround for this? I currently have a role with 60+ replicated inline policies and almost reached the characters limit.

@mattiamatrix
Copy link
Contributor

mattiamatrix commented Apr 3, 2022

#19114 might partially help

Update: tested this morning and the AwsCustomResource policy is not minified.

@haslam22
Copy link
Contributor

Experiencing the same problem. I need to provide an immutable role to AwsCustomResource which already has the correct permissions to do what I want. I'm not allowed to update an IAM role with a new policy in my corporate environment.

No workaround available as far as I can see, it always tries to attach a policy if role is defined: aws-custom-resource.ts#L409

@mergify mergify bot closed this as completed in #20966 Aug 5, 2022
mergify bot pushed a commit that referenced this issue Aug 5, 2022
…tes immutable roles (#20966)

This fixes #13232 by making the `policy` property optional in `AwsCustomResource` as long as `role` is specified.

Previously, if an immutable role was provided to `AwsCustomResource`, an IAM policy was still created with a reference to the provided role. This resulted in a call to `iam:PutRolePolicy` to update the immutable role when deploying the stack.

The underlying motivation for using an immutable role here is to support a restricted corporate environment where IAM role changes are not allowed.

----

### All Submissions:

* [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*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 Aug 5, 2022

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

josephedward pushed a commit to josephedward/aws-cdk that referenced this issue Aug 30, 2022
…tes immutable roles (aws#20966)

This fixes aws#13232 by making the `policy` property optional in `AwsCustomResource` as long as `role` is specified.

Previously, if an immutable role was provided to `AwsCustomResource`, an IAM policy was still created with a reference to the provided role. This resulted in a call to `iam:PutRolePolicy` to update the immutable role when deploying the stack.

The underlying motivation for using an immutable role here is to support a restricted corporate environment where IAM role changes are not allowed.

----

### All Submissions:

* [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/custom-resources Related to AWS CDK Custom Resources effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. good first issue Related to contributions. See CONTRIBUTING.md p2
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants