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

fix(custom-resources): deleting custom resource fails when using two or more #10012

Merged
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,10 @@
"InstallLatestAwsSdk": true
},
"UpdateReplacePolicy": "Delete",
"DeletionPolicy": "Delete"
"DeletionPolicy": "Delete",
"DependsOn": [
"UserPoolDomainCloudFrontDomainNameCustomResourcePolicy7DE54188"
]
},
"AWS679f53fac002430cb0da5b7982bd2287ServiceRoleC1EA0FF2": {
"Type": "AWS::IAM::Role",
Expand Down Expand Up @@ -111,25 +114,21 @@
]
}
},
"AWS679f53fac002430cb0da5b7982bd2287ServiceRoleDefaultPolicyD28E1A5E": {
"UserPoolDomainCloudFrontDomainNameCustomResourcePolicy7DE54188": {
"Type": "AWS::IAM::Policy",
"Properties": {
"PolicyDocument": {
"Statement": [
{
"Action": "cognito-idp:DescribeUserPoolDomain",
"Effect": "Allow",
"Action":"cognito-idp:DescribeUserPoolDomain",
"Effect":"Allow",
"Resource": "*"
}
],
"Version": "2012-10-17"
},
"PolicyName": "AWS679f53fac002430cb0da5b7982bd2287ServiceRoleDefaultPolicyD28E1A5E",
"Roles": [
{
"Ref": "AWS679f53fac002430cb0da5b7982bd2287ServiceRoleC1EA0FF2"
}
]
"PolicyName": "UserPoolDomainCloudFrontDomainNameCustomResourcePolicy7DE54188",
"Roles": [{"Ref":"AWS679f53fac002430cb0da5b7982bd2287ServiceRoleC1EA0FF2"}]
}
},
"AWS679f53fac002430cb0da5b7982bd22872D164C4C": {
Expand Down Expand Up @@ -184,7 +183,6 @@
"Timeout": 120
},
"DependsOn": [
"AWS679f53fac002430cb0da5b7982bd2287ServiceRoleDefaultPolicyD28E1A5E",
"AWS679f53fac002430cb0da5b7982bd2287ServiceRoleC1EA0FF2"
]
}
Expand Down
54 changes: 26 additions & 28 deletions packages/@aws-cdk/aws-ecr/test/integ.imagescan.expected.json
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,32 @@
"InstallLatestAwsSdk": true
},
"UpdateReplacePolicy": "Delete",
"DeletionPolicy": "Delete"
"DeletionPolicy": "Delete",
"DependsOn": [
"RepoImageScanOnPushCustomResourcePolicy556E941E"
]
},
"RepoImageScanOnPushCustomResourcePolicy556E941E": {
"Type": "AWS::IAM::Policy",
"Properties": {
"PolicyDocument": {
"Statement": [
{
"Action":"ecr:PutImageScanningConfiguration",
"Effect":"Allow",
"Resource": {
"Fn::GetAtt": [
"Repo02AC86CF",
"Arn"
]
}
}
],
"Version": "2012-10-17"
},
"PolicyName": "RepoImageScanOnPushCustomResourcePolicy556E941E",
"Roles": [{"Ref":"AWS679f53fac002430cb0da5b7982bd2287ServiceRoleC1EA0FF2"}]
}
},
"RepoImageScanComplete7BC71935": {
"Type": "AWS::Events::Rule",
Expand Down Expand Up @@ -134,32 +159,6 @@
]
}
},
"AWS679f53fac002430cb0da5b7982bd2287ServiceRoleDefaultPolicyD28E1A5E": {
"Type": "AWS::IAM::Policy",
"Properties": {
"PolicyDocument": {
"Statement": [
{
"Action": "ecr:PutImageScanningConfiguration",
"Effect": "Allow",
"Resource": {
"Fn::GetAtt": [
"Repo02AC86CF",
"Arn"
]
}
}
],
"Version": "2012-10-17"
},
"PolicyName": "AWS679f53fac002430cb0da5b7982bd2287ServiceRoleDefaultPolicyD28E1A5E",
"Roles": [
{
"Ref": "AWS679f53fac002430cb0da5b7982bd2287ServiceRoleC1EA0FF2"
}
]
}
},
"AWS679f53fac002430cb0da5b7982bd22872D164C4C": {
"Type": "AWS::Lambda::Function",
"Properties": {
Expand Down Expand Up @@ -212,7 +211,6 @@
"Timeout": 120
},
"DependsOn": [
"AWS679f53fac002430cb0da5b7982bd2287ServiceRoleDefaultPolicyD28E1A5E",
"AWS679f53fac002430cb0da5b7982bd2287ServiceRoleC1EA0FF2"
]
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ test('custom resource exists', () => {
InstallLatestAwsSdk: true,
},
DependsOn: [
'GlobalAcceleratorSGCustomResourceCustomResourcePolicyF3294553',
'GroupC77FDACD',
],
}, ResourcePart.CompleteDefinition));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -324,24 +324,31 @@ export class AwsCustomResource extends cdk.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) {
provider.addToRolePolicy(statement);
statements.push(statement);
}
} else {
// Derive statements from AWS SDK calls
for (const call of [props.onCreate, props.onUpdate, props.onDelete]) {
if (call) {
provider.addToRolePolicy(new iam.PolicyStatement({
const statement = new iam.PolicyStatement({
actions: [awsSdkToIamAction(call.service, call.action)],
resources: props.policy.resources,
}));
});
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 @@ -354,6 +361,10 @@ export class AwsCustomResource extends cdk.Construct implements iam.IGrantable {
installLatestAwsSdk: props.installLatestAwsSdk ?? true,
},
});

// 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
Original file line number Diff line number Diff line change
Expand Up @@ -286,11 +286,6 @@ test('implements IGrantable', () => {
expect(stack).toHaveResource('AWS::IAM::Policy', {
PolicyDocument: {
Statement: [
{
Action: 'service:Action',
Effect: 'Allow',
Resource: '*',
},
{
Action: 'iam:PassRole',
Effect: 'Allow',
Expand Down Expand Up @@ -564,3 +559,52 @@ test('can specify function name', () => {
FunctionName: 'my-cool-function',
});
});

test('separate policies per custom resource', () => {
// GIVEN
const stack = new cdk.Stack();

// WHEN
new AwsCustomResource(stack, 'Custom1', {
onCreate: {
service: 'service1',
action: 'action1',
physicalResourceId: PhysicalResourceId.of('id1'),
},
policy: AwsCustomResourcePolicy.fromSdkCalls({ resources: AwsCustomResourcePolicy.ANY_RESOURCE }),
});
new AwsCustomResource(stack, 'Custom2', {
onCreate: {
service: 'service2',
action: 'action2',
physicalResourceId: PhysicalResourceId.of('id2'),
},
policy: AwsCustomResourcePolicy.fromSdkCalls({ resources: AwsCustomResourcePolicy.ANY_RESOURCE }),
});

// THEN
expect(stack).toHaveResource('AWS::IAM::Policy', {
PolicyDocument: {
Statement: [
{
Action: 'service1:Action1',
Effect: 'Allow',
Resource: '*',
},
],
Version: '2012-10-17',
},
});
expect(stack).toHaveResource('AWS::IAM::Policy', {
PolicyDocument: {
Statement: [
{
Action: 'service2:Action2',
Effect: 'Allow',
Resource: '*',
},
],
Version: '2012-10-17',
},
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,10 @@
"InstallLatestAwsSdk": true
},
"UpdateReplacePolicy": "Delete",
"DeletionPolicy": "Delete"
"DeletionPolicy": "Delete",
"DependsOn": [
"PublishCustomResourcePolicyDF696FCA"
]
},
"AWS679f53fac002430cb0da5b7982bd2287ServiceRoleC1EA0FF2": {
"Type": "AWS::IAM::Role",
Expand Down Expand Up @@ -78,37 +81,6 @@
]
}
},
"AWS679f53fac002430cb0da5b7982bd2287ServiceRoleDefaultPolicyD28E1A5E": {
"Type": "AWS::IAM::Policy",
"Properties": {
"PolicyDocument": {
"Statement": [
{
"Action": "sns:Publish",
"Effect": "Allow",
"Resource": "*"
},
{
"Action": "sns:ListTopics",
"Effect": "Allow",
"Resource": "*"
},
{
"Action": "ssm:GetParameter",
"Effect": "Allow",
"Resource": "*"
}
],
"Version": "2012-10-17"
},
"PolicyName": "AWS679f53fac002430cb0da5b7982bd2287ServiceRoleDefaultPolicyD28E1A5E",
"Roles": [
{
"Ref": "AWS679f53fac002430cb0da5b7982bd2287ServiceRoleC1EA0FF2"
}
]
}
},
"AWS679f53fac002430cb0da5b7982bd22872D164C4C": {
"Type": "AWS::Lambda::Function",
"Properties": {
Expand Down Expand Up @@ -161,7 +133,6 @@
"Timeout": 120
},
"DependsOn": [
"AWS679f53fac002430cb0da5b7982bd2287ServiceRoleDefaultPolicyD28E1A5E",
"AWS679f53fac002430cb0da5b7982bd2287ServiceRoleC1EA0FF2"
]
},
Expand Down Expand Up @@ -191,6 +162,7 @@
"InstallLatestAwsSdk": true
},
"DependsOn": [
"ListTopicsCustomResourcePolicy31A8396A",
"TopicBFC7AF6E"
],
"UpdateReplacePolicy": "Delete",
Expand Down Expand Up @@ -241,7 +213,44 @@
"InstallLatestAwsSdk": true
},
"UpdateReplacePolicy": "Delete",
"DeletionPolicy": "Delete"
"DeletionPolicy": "Delete",
"DependsOn": [
"GetParameterCustomResourcePolicyD8E5D455"
]
},
"PublishCustomResourcePolicyDF696FCA": {
"Type": "AWS::IAM::Policy",
"Properties": {
"PolicyDocument": {
"Statement": [{"Action":"sns:Publish","Effect":"Allow","Resource":"*"}],
"Version": "2012-10-17"
},
"PolicyName": "PublishCustomResourcePolicyDF696FCA",
"Roles": [{"Ref":"AWS679f53fac002430cb0da5b7982bd2287ServiceRoleC1EA0FF2"}]
}
},
"ListTopicsCustomResourcePolicy31A8396A": {
"Type": "AWS::IAM::Policy",
"Properties": {
"PolicyDocument": {
"Statement": [{"Action":"sns:ListTopics","Effect":"Allow","Resource":"*"}],
"Version": "2012-10-17"
},
"PolicyName": "ListTopicsCustomResourcePolicy31A8396A",
"Roles": [{"Ref":"AWS679f53fac002430cb0da5b7982bd2287ServiceRoleC1EA0FF2"}]
},
"DependsOn": ["TopicBFC7AF6E"]
},
"GetParameterCustomResourcePolicyD8E5D455": {
"Type": "AWS::IAM::Policy",
"Properties": {
"PolicyDocument": {
"Statement": [{"Action":"ssm:GetParameter","Effect":"Allow","Resource":"*"}],
"Version": "2012-10-17"
},
"PolicyName": "GetParameterCustomResourcePolicyD8E5D455",
"Roles": [{"Ref":"AWS679f53fac002430cb0da5b7982bd2287ServiceRoleC1EA0FF2"}]
}
}
},
"Parameters": {
Expand Down