Skip to content

Commit

Permalink
fix(custom-resources): AwsCustomResource requires a policy which upda…
Browse files Browse the repository at this point in the history
…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*
  • Loading branch information
haslam22 authored Aug 5, 2022
1 parent 82a597a commit a02ef9c
Show file tree
Hide file tree
Showing 10 changed files with 716 additions and 412 deletions.
5 changes: 3 additions & 2 deletions packages/@aws-cdk/custom-resources/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -421,8 +421,9 @@ the `installLatestAwsSdk` prop to `false`.

### Custom Resource Execution Policy

You must provide the `policy` property defining the IAM Policy that will be applied to the API calls.
The library provides two factory methods to quickly configure this:
The `policy` property defines the IAM Policy that will be applied to the API calls. This must be provided
if an existing `role` is not specified and is optional otherwise. The library provides two factory methods
to quickly configure this:

* **`AwsCustomResourcePolicy.fromSdkCalls`** - Use this to auto-generate IAM
Policy statements based on the configured SDK calls. Keep two things in mind
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -260,10 +260,16 @@ export interface AwsCustomResourceProps {
* to note the that function's role will eventually accumulate the
* permissions/grants from all resources.
*
* Note that a policy must be specified if `role` is not provided, as
* by default a new role is created which requires policy changes to access
* resources.
*
* @default - no policy added
*
* @see Policy.fromStatements
* @see Policy.fromSdkCalls
*/
readonly policy: AwsCustomResourcePolicy;
readonly policy?: AwsCustomResourcePolicy;

/**
* The execution role for the singleton Lambda function implementing this custom
Expand Down Expand Up @@ -344,6 +350,10 @@ export class AwsCustomResource extends Construct implements iam.IGrantable {
throw new Error('At least `onCreate`, `onUpdate` or `onDelete` must be specified.');
}

if (!props.role && !props.policy) {
throw new Error('At least one of `policy` or `role` (or both) must be specified.');
}

for (const call of [props.onCreate, props.onUpdate]) {
if (call && !call.physicalResourceId) {
throw new Error('`physicalResourceId` must be specified for onCreate and onUpdate calls.');
Expand Down Expand Up @@ -377,37 +387,6 @@ export class AwsCustomResource extends Construct implements iam.IGrantable {
});
this.grantPrincipal = provider.grantPrincipal;

// Create the policy statements for the custom resource function role, or use the user-provided ones
const statements = [];
if (props.policy.statements.length !== 0) {
// Use custom statements provided by the user
for (const statement of props.policy.statements) {
statements.push(statement);
}
} else {
// Derive statements from AWS SDK calls
for (const call of [props.onCreate, props.onUpdate, props.onDelete]) {
if (call && call.assumedRoleArn == null) {
const statement = new iam.PolicyStatement({
actions: [awsSdkToIamAction(call.service, call.action)],
resources: props.policy.resources,
});
statements.push(statement);
} else if (call && call.assumedRoleArn != null) {
const statement = new iam.PolicyStatement({
actions: ['sts:AssumeRole'],
resources: [call.assumedRoleArn],
});
statements.push(statement);
}
}
}
const policy = new iam.Policy(this, 'CustomResourcePolicy', {
statements: statements,
});
if (provider.role !== undefined) {
policy.attachToRole(provider.role);
}
const create = props.onCreate || props.onUpdate;
this.customResource = new cdk.CustomResource(this, 'Resource', {
resourceType: props.resourceType || 'Custom::AWS',
Expand All @@ -421,9 +400,43 @@ export class AwsCustomResource extends Construct implements iam.IGrantable {
},
});

// If the policy was deleted first, then the function might lose permissions to delete the custom resource
// This is here so that the policy doesn't get removed before onDelete is called
this.customResource.node.addDependency(policy);
// Create the policy statements for the custom resource function role, or use the user-provided ones
if (props.policy) {
const statements = [];
if (props.policy.statements.length !== 0) {
// Use custom statements provided by the user
for (const statement of props.policy.statements) {
statements.push(statement);
}
} else {
// Derive statements from AWS SDK calls
for (const call of [props.onCreate, props.onUpdate, props.onDelete]) {
if (call && call.assumedRoleArn == null) {
const statement = new iam.PolicyStatement({
actions: [awsSdkToIamAction(call.service, call.action)],
resources: props.policy.resources,
});
statements.push(statement);
} else if (call && call.assumedRoleArn != null) {
const statement = new iam.PolicyStatement({
actions: ['sts:AssumeRole'],
resources: [call.assumedRoleArn],
});
statements.push(statement);
}
}
}
const policy = new iam.Policy(this, 'CustomResourcePolicy', {
statements: statements,
});
if (provider.role !== undefined) {
policy.attachToRole(provider.role);
}

// If the policy was deleted first, then the function might lose permissions to delete the custom resource
// This is here so that the policy doesn't get removed before onDelete is called
this.customResource.node.addDependency(policy);
}
}

/**
Expand Down

Large diffs are not rendered by default.

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,28 +1,28 @@
{
"version": "20.0.0",
"files": {
"9d784cf317cead201dfe56ed0404d6d23eba6d499ca7354138230c2267f2fe90": {
"105b4f39ae68785e705640aa91919e412fcba2dd454aca53412747be8d955286": {
"source": {
"path": "asset.9d784cf317cead201dfe56ed0404d6d23eba6d499ca7354138230c2267f2fe90",
"path": "asset.105b4f39ae68785e705640aa91919e412fcba2dd454aca53412747be8d955286",
"packaging": "zip"
},
"destinations": {
"current_account-current_region": {
"bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}",
"objectKey": "9d784cf317cead201dfe56ed0404d6d23eba6d499ca7354138230c2267f2fe90.zip",
"objectKey": "105b4f39ae68785e705640aa91919e412fcba2dd454aca53412747be8d955286.zip",
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}"
}
}
},
"e2171113a71c90af51e1d81ceac438f6d9e02bcc88b94a6a3a91685f4fa3dc59": {
"8f6951ad9bc39810d6e4a8eb249d085b8796a74fce45b0fc13a6fb609c1632ff": {
"source": {
"path": "aws-cdk-sdk-js.template.json",
"packaging": "file"
},
"destinations": {
"current_account-current_region": {
"bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}",
"objectKey": "e2171113a71c90af51e1d81ceac438f6d9e02bcc88b94a6a3a91685f4fa3dc59.json",
"objectKey": "8f6951ad9bc39810d6e4a8eb249d085b8796a74fce45b0fc13a6fb609c1632ff.json",
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}"
}
}
Expand Down
Loading

0 comments on commit a02ef9c

Please sign in to comment.