Skip to content

Commit 35ed5c6

Browse files
authored
fix(iam): override Role.applyRemovalPolicy for customizeRoles (#31652)
### Issue # (if applicable) Closes #31651 ### Reason for this change `Role.customizeRoles` throws an Error if there is a construct that calls `applyRemovalPolicy` internally. This means users cannot use with some constructs like `RestApi`. ``` Error: Cannot apply RemovalPolicy: no child or not a CfnResource. Apply the removal policy on the CfnResource directly. ``` This can be reproduced with: ```typescript const app = new App(); Role.customizeRoles(app); const stack = new Stack(app, 'Stack'); new RestApi(stack, 'RestApi'); ``` Or explicitly: ```typescript const app = new App(); Role.customizeRoles(app); const stack = new Stack(app, 'Stack'); const role = new Role(stack, 'Role', { assumedBy: new ServicePrincipal('sns.amazonaws.com') }); role.applyRemovalPolicy(RemovalPolicy.DESTROY); ``` ### Description of changes While it might be possible to fix `RestApi`, there could be other constructs within aws-cdk-lib that also call `Role.applyRemovalPolicy`. Moreover, it's nearly impossible to make library users aware of this. Since `Role` implements the `IResource` interface, it has the responsibility to respond to the `applyRemovalPolicy` call. Therefore, I think it would be good to override `applyRemovalPolicy` in the `Role` class. ### Description of how you validated changes Fixed the existing unit test to change behavior. ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 81b47b7 commit 35ed5c6

13 files changed

+850
-2
lines changed

Diff for: packages/@aws-cdk-testing/framework-integ/test/aws-iam/test/integ.customize-roles-restapi.js.snapshot/IntegTestDefaultTestDeployAssertE3E7D2A4.assets.json

+19
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Diff for: packages/@aws-cdk-testing/framework-integ/test/aws-iam/test/integ.customize-roles-restapi.js.snapshot/IntegTestDefaultTestDeployAssertE3E7D2A4.template.json

+36
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Diff for: packages/@aws-cdk-testing/framework-integ/test/aws-iam/test/integ.customize-roles-restapi.js.snapshot/cdk.out

+1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Diff for: packages/@aws-cdk-testing/framework-integ/test/aws-iam/test/integ.customize-roles-restapi.js.snapshot/iam-policy-report.json

+40
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Diff for: packages/@aws-cdk-testing/framework-integ/test/aws-iam/test/integ.customize-roles-restapi.js.snapshot/iam-policy-report.txt

+43
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Diff for: packages/@aws-cdk-testing/framework-integ/test/aws-iam/test/integ.customize-roles-restapi.js.snapshot/integ-customize-roles-restapi.assets.json

+19
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,144 @@
1+
{
2+
"Resources": {
3+
"RestApi0C43BF4B": {
4+
"Type": "AWS::ApiGateway::RestApi",
5+
"Properties": {
6+
"Name": "RestApi"
7+
}
8+
},
9+
"RestApiAccount7C83CF5A": {
10+
"Type": "AWS::ApiGateway::Account",
11+
"Properties": {
12+
"CloudWatchRoleArn": {
13+
"Fn::Join": [
14+
"",
15+
[
16+
"arn:",
17+
{
18+
"Ref": "AWS::Partition"
19+
},
20+
":iam::",
21+
{
22+
"Ref": "AWS::AccountId"
23+
},
24+
":role/precreated-role-api"
25+
]
26+
]
27+
}
28+
},
29+
"DependsOn": [
30+
"RestApi0C43BF4B"
31+
],
32+
"UpdateReplacePolicy": "Retain",
33+
"DeletionPolicy": "Retain"
34+
},
35+
"RestApiDeployment180EC503f7fb2c75a4565f0d9e459f179f2a7481": {
36+
"Type": "AWS::ApiGateway::Deployment",
37+
"Properties": {
38+
"Description": "Automatically created by the RestApi construct",
39+
"RestApiId": {
40+
"Ref": "RestApi0C43BF4B"
41+
}
42+
},
43+
"DependsOn": [
44+
"RestApiGET0F59260B"
45+
]
46+
},
47+
"RestApiDeploymentStageprod3855DE66": {
48+
"Type": "AWS::ApiGateway::Stage",
49+
"Properties": {
50+
"DeploymentId": {
51+
"Ref": "RestApiDeployment180EC503f7fb2c75a4565f0d9e459f179f2a7481"
52+
},
53+
"RestApiId": {
54+
"Ref": "RestApi0C43BF4B"
55+
},
56+
"StageName": "prod"
57+
},
58+
"DependsOn": [
59+
"RestApiAccount7C83CF5A"
60+
]
61+
},
62+
"RestApiGET0F59260B": {
63+
"Type": "AWS::ApiGateway::Method",
64+
"Properties": {
65+
"AuthorizationType": "NONE",
66+
"HttpMethod": "GET",
67+
"Integration": {
68+
"Type": "MOCK"
69+
},
70+
"ResourceId": {
71+
"Fn::GetAtt": [
72+
"RestApi0C43BF4B",
73+
"RootResourceId"
74+
]
75+
},
76+
"RestApiId": {
77+
"Ref": "RestApi0C43BF4B"
78+
}
79+
}
80+
}
81+
},
82+
"Outputs": {
83+
"RestApiEndpoint0551178A": {
84+
"Value": {
85+
"Fn::Join": [
86+
"",
87+
[
88+
"https://",
89+
{
90+
"Ref": "RestApi0C43BF4B"
91+
},
92+
".execute-api.",
93+
{
94+
"Ref": "AWS::Region"
95+
},
96+
".",
97+
{
98+
"Ref": "AWS::URLSuffix"
99+
},
100+
"/",
101+
{
102+
"Ref": "RestApiDeploymentStageprod3855DE66"
103+
},
104+
"/"
105+
]
106+
]
107+
}
108+
}
109+
},
110+
"Parameters": {
111+
"BootstrapVersion": {
112+
"Type": "AWS::SSM::Parameter::Value<String>",
113+
"Default": "/cdk-bootstrap/hnb659fds/version",
114+
"Description": "Version of the CDK Bootstrap resources in this environment, automatically retrieved from SSM Parameter Store. [cdk:skip]"
115+
}
116+
},
117+
"Rules": {
118+
"CheckBootstrapVersion": {
119+
"Assertions": [
120+
{
121+
"Assert": {
122+
"Fn::Not": [
123+
{
124+
"Fn::Contains": [
125+
[
126+
"1",
127+
"2",
128+
"3",
129+
"4",
130+
"5"
131+
],
132+
{
133+
"Ref": "BootstrapVersion"
134+
}
135+
]
136+
}
137+
]
138+
},
139+
"AssertDescription": "CDK bootstrap stack version 6 required. Please run 'cdk bootstrap' with a recent version of the CDK CLI."
140+
}
141+
]
142+
}
143+
}
144+
}

Diff for: packages/@aws-cdk-testing/framework-integ/test/aws-iam/test/integ.customize-roles-restapi.js.snapshot/integ.json

+20
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)