Commit ea226b5
authored
fix(kinesisfirehose): can't call grantPrincipal multiple times (#34682)
### Issue # (if applicable)
Closes #<issue number here>.
### Reason for this change
DeliveryStream creates a new iam.Role every reference grantPrincipal, so we get the error `There is already a Construct with name 'Service Role' in DeliveryStream [Delivery Stream Multiple]`. This is easy to reproduce if you use deliveryStream with multiple `grantXXX` methods.
This is test result before fix codes.
```
yarn test aws-kinesisfirehose/test/delivery-stream.test.ts -t
"multiple calls to grantPrincipal should return the same instance of IAM role"
yarn run v1.22.22
$ jest aws-kinesisfirehose/test/delivery-stream.test.ts -t 'multiple calls to grantPrincipal should return the same instance of IAM role'
ts-jest[config] (WARN)
The "ts-jest" config option "isolatedModules" is deprecated and will be removed in v30.0.0. Please use "isolatedModules: true" in /workspaces/aws-cdk/packages/aws-cdk-lib/tsconfig.json instead, see https://www.typescriptlang.org/tsconfig/#isolatedModules
FAIL aws-kinesisfirehose/test/delivery-stream.test.ts
delivery stream
✕ multiple calls to grantPrincipal should return the same instance of IAM role (45 ms)
○ other tests...
● delivery stream › multiple calls to grantPrincipal should return the same instance of IAM role
expect(received).not.toThrow()
Error name: "Error"
Error message: "There is already a Construct with name 'Service Role' in DeliveryStream [Delivery Stream Multiple]"
155 |
156 | constructor(scope: Construct, id: string, props: ResourceProps = {}) {
> 157 | super(scope, id);
| ^
158 |
159 | if ((props.account !== undefined || props.region !== undefined) && props.environmentFromArn !== undefined) {
160 | throw new ValidationError(`Supply at most one of 'account'/'region' (${props.account}/${props.region}) and 'environmentFromArn' (${props.environmentFromArn})`, this);
at Node.addChild (../../node_modules/constructs/src/construct.ts:430:13)
at new Node (../../node_modules/constructs/src/construct.ts:71:17)
at new Construct (../../node_modules/constructs/src/construct.ts:482:17)
at new Resource (core/lib/resource.ts:157:5)
at new Role (aws-iam/lib/role.ts:472:5)
at new Role (core/lib/prop-injectable.ts:36:7)
at WrappedClass.get grantPrincipal [as grantPrincipal] (aws-kinesisfirehose/lib/delivery-stream.ts:320:26)
at aws-kinesisfirehose/test/delivery-stream.test.ts:410:33
at Object.<anonymous> (../../node_modules/expect/build/toThrowMatchers.js:74:11)
at Object.throwingMatcher [as toThrow] (../../node_modules/expect/build/index.js:320:21)
at Object.<anonymous> (aws-kinesisfirehose/test/delivery-stream.test.ts:410:53)
408 | });
409 | const principal = deliveryStream.grantPrincipal;
> 410 | expect(() => deliveryStream.grantPrincipal).not.toThrow();
| ^
411 | expect(deliveryStream.grantPrincipal).toBe(principal);
412 | });
413 |
at Object.<anonymous> (aws-kinesisfirehose/test/delivery-stream.test.ts:410:53)
```
### Description of changes
I changed grantPrincipal implementation from create a new iam.Role every reference to create a new iam.Role only at first reference.
### Describe any new or updated permissions being added
None.
### Description of how you validated changes
Create the new unit test.
### 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 3774f78 commit ea226b5
File tree
2 files changed
+12
-5
lines changed- packages/aws-cdk-lib/aws-kinesisfirehose
- lib
- test
2 files changed
+12
-5
lines changedLines changed: 3 additions & 5 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
311 | 311 | | |
312 | 312 | | |
313 | 313 | | |
314 | | - | |
315 | | - | |
316 | | - | |
317 | | - | |
318 | | - | |
| 314 | + | |
| 315 | + | |
319 | 316 | | |
320 | 317 | | |
| 318 | + | |
321 | 319 | | |
322 | 320 | | |
323 | 321 | | |
| |||
Lines changed: 9 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
402 | 402 | | |
403 | 403 | | |
404 | 404 | | |
| 405 | + | |
| 406 | + | |
| 407 | + | |
| 408 | + | |
| 409 | + | |
| 410 | + | |
| 411 | + | |
| 412 | + | |
| 413 | + | |
405 | 414 | | |
406 | 415 | | |
407 | 416 | | |
| |||
0 commit comments