Skip to content

Commit

Permalink
fix(firehose): remove unused role during DeliveryStream creation (#26930
Browse files Browse the repository at this point in the history 
)

When a DeliveryStream is created without `sourceStream` or `encryptionKey`,
an extra role is being created that is unused. This PR removes creation of that role. 

I also learned that the role created for `encryptionKey` is used "indirectly" for a grant 
put on the KMS key...interesting.

Closes #26927.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
@msambol
msambol authored and Mike Wrighton committed Sep 14, 2023
1 parent 159ed7e commit 1b8ebf4
Show file tree
Hide file tree
Showing 21 changed files with 519 additions and 397 deletions.
17 changes: 10 additions & 7 deletions ...ns-alpha/test/kinesis-firehose/integ.firehose-put-record-action.js.snapshot/manifest.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
"validateOnSynth": false,
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-deploy-role-${AWS::AccountId}-${AWS::Region}",
"cloudFormationExecutionRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-cfn-exec-role-${AWS::AccountId}-${AWS::Region}",
"stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/cf636658ec15133bceba498f25c92e3b2a42f090f11883a69d8fd68b873600a1.json",
"stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/f75ab8f9b4f9b4569a43902e069684cc217226d66b42e025930c87f6f6dd1cb4.json",
"requiresBootstrapStackVersion": 6,
"bootstrapStackVersionSsmParameter": "/cdk-bootstrap/hnb659fds/version",
"additionalDependencies": [
Expand Down Expand Up @@ -57,12 +57,6 @@
"data": "MyBucketF68F3FF0"
}
],
"/test-stack/MyStream/Service Role/Resource": [
{
"type": "aws:cdk:logicalId",
"data": "MyStreamServiceRole8C50608A"
}
],
"/test-stack/MyStream/S3 Destination Role/Resource": [
{
"type": "aws:cdk:logicalId",
Expand Down Expand Up @@ -110,6 +104,15 @@
"type": "aws:cdk:logicalId",
"data": "CheckBootstrapVersion"
}
],
"MyStreamServiceRole8C50608A": [
{
"type": "aws:cdk:logicalId",
"data": "MyStreamServiceRole8C50608A",
"trace": [
"!!DESTRUCTIVE_CHANGES: WILL_DESTROY"
]
}
]
},
"displayName": "test-stack"
Expand Down
4 changes: 2 additions & 2 deletions ...test/kinesis-firehose/integ.firehose-put-record-action.js.snapshot/test-stack.assets.json
View file
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
{
"version": "34.0.0",
"files": {
"cf636658ec15133bceba498f25c92e3b2a42f090f11883a69d8fd68b873600a1": {
"f75ab8f9b4f9b4569a43902e069684cc217226d66b42e025930c87f6f6dd1cb4": {
"source": {
"path": "test-stack.template.json",
"packaging": "file"
},
"destinations": {
"current_account-current_region": {
"bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}",
"objectKey": "cf636658ec15133bceba498f25c92e3b2a42f090f11883a69d8fd68b873600a1.json",
"objectKey": "f75ab8f9b4f9b4569a43902e069684cc217226d66b42e025930c87f6f6dd1cb4.json",
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}"
}
}
Expand Down
17 changes: 0 additions & 17 deletions ...st/kinesis-firehose/integ.firehose-put-record-action.js.snapshot/test-stack.template.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,23 +77,6 @@
"UpdateReplacePolicy": "Delete",
"DeletionPolicy": "Delete"
},
"MyStreamServiceRole8C50608A": {
"Type": "AWS::IAM::Role",
"Properties": {
"AssumeRolePolicyDocument": {
"Statement": [
{
"Action": "sts:AssumeRole",
"Effect": "Allow",
"Principal": {
"Service": "firehose.amazonaws.com"
}
}
],
"Version": "2012-10-17"
}
}
},
"MyStreamS3DestinationRole5E0BA960": {
"Type": "AWS::IAM::Role",
"Properties": {
Expand Down
141 changes: 49 additions & 92 deletions ...ctions-alpha/test/kinesis-firehose/integ.firehose-put-record-action.js.snapshot/tree.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@
}
},
"constructInfo": {
"fqn": "aws-cdk-lib.aws_iot.CfnTopicRule",
"version": "0.0.0"
"fqn": "constructs.Construct",
"version": "10.2.70"
}
},
"TopicRuleActionRole": {
Expand All @@ -54,8 +54,8 @@
"id": "ImportTopicRuleActionRole",
"path": "test-stack/TopicRule/TopicRuleActionRole/ImportTopicRuleActionRole",
"constructInfo": {
"fqn": "aws-cdk-lib.Resource",
"version": "0.0.0"
"fqn": "constructs.Construct",
"version": "10.2.70"
}
},
"Resource": {
Expand All @@ -79,8 +79,8 @@
}
},
"constructInfo": {
"fqn": "aws-cdk-lib.aws_iam.CfnRole",
"version": "0.0.0"
"fqn": "constructs.Construct",
"version": "10.2.70"
}
},
"DefaultPolicy": {
Expand Down Expand Up @@ -120,20 +120,20 @@
}
},
"constructInfo": {
"fqn": "aws-cdk-lib.aws_iam.CfnPolicy",
"version": "0.0.0"
"fqn": "constructs.Construct",
"version": "10.2.70"
}
}
},
"constructInfo": {
"fqn": "aws-cdk-lib.aws_iam.Policy",
"version": "0.0.0"
"fqn": "constructs.Construct",
"version": "10.2.70"
}
}
},
"constructInfo": {
"fqn": "aws-cdk-lib.aws_iam.Role",
"version": "0.0.0"
"fqn": "constructs.Construct",
"version": "10.2.70"
}
}
},
Expand All @@ -154,63 +154,20 @@
"aws:cdk:cloudformation:props": {}
},
"constructInfo": {
"fqn": "aws-cdk-lib.aws_s3.CfnBucket",
"version": "0.0.0"
"fqn": "constructs.Construct",
"version": "10.2.70"
}
}
},
"constructInfo": {
"fqn": "aws-cdk-lib.aws_s3.Bucket",
"version": "0.0.0"
"fqn": "constructs.Construct",
"version": "10.2.70"
}
},
"MyStream": {
"id": "MyStream",
"path": "test-stack/MyStream",
"children": {
"Service Role": {
"id": "Service Role",
"path": "test-stack/MyStream/Service Role",
"children": {
"ImportService Role": {
"id": "ImportService Role",
"path": "test-stack/MyStream/Service Role/ImportService Role",
"constructInfo": {
"fqn": "aws-cdk-lib.Resource",
"version": "0.0.0"
}
},
"Resource": {
"id": "Resource",
"path": "test-stack/MyStream/Service Role/Resource",
"attributes": {
"aws:cdk:cloudformation:type": "AWS::IAM::Role",
"aws:cdk:cloudformation:props": {
"assumeRolePolicyDocument": {
"Statement": [
{
"Action": "sts:AssumeRole",
"Effect": "Allow",
"Principal": {
"Service": "firehose.amazonaws.com"
}
}
],
"Version": "2012-10-17"
}
}
},
"constructInfo": {
"fqn": "aws-cdk-lib.aws_iam.CfnRole",
"version": "0.0.0"
}
}
},
"constructInfo": {
"fqn": "aws-cdk-lib.aws_iam.Role",
"version": "0.0.0"
}
},
"S3 Destination Role": {
"id": "S3 Destination Role",
"path": "test-stack/MyStream/S3 Destination Role",
Expand All @@ -219,8 +176,8 @@
"id": "ImportS3 Destination Role",
"path": "test-stack/MyStream/S3 Destination Role/ImportS3 Destination Role",
"constructInfo": {
"fqn": "aws-cdk-lib.Resource",
"version": "0.0.0"
"fqn": "constructs.Construct",
"version": "10.2.70"
}
},
"Resource": {
Expand All @@ -244,8 +201,8 @@
}
},
"constructInfo": {
"fqn": "aws-cdk-lib.aws_iam.CfnRole",
"version": "0.0.0"
"fqn": "constructs.Construct",
"version": "10.2.70"
}
},
"DefaultPolicy": {
Expand Down Expand Up @@ -322,20 +279,20 @@
}
},
"constructInfo": {
"fqn": "aws-cdk-lib.aws_iam.CfnPolicy",
"version": "0.0.0"
"fqn": "constructs.Construct",
"version": "10.2.70"
}
}
},
"constructInfo": {
"fqn": "aws-cdk-lib.aws_iam.Policy",
"version": "0.0.0"
"fqn": "constructs.Construct",
"version": "10.2.70"
}
}
},
"constructInfo": {
"fqn": "aws-cdk-lib.aws_iam.Role",
"version": "0.0.0"
"fqn": "constructs.Construct",
"version": "10.2.70"
}
},
"LogGroup": {
Expand All @@ -352,8 +309,8 @@
}
},
"constructInfo": {
"fqn": "aws-cdk-lib.aws_logs.CfnLogGroup",
"version": "0.0.0"
"fqn": "constructs.Construct",
"version": "10.2.70"
}
},
"S3Destination": {
Expand All @@ -372,20 +329,20 @@
}
},
"constructInfo": {
"fqn": "aws-cdk-lib.aws_logs.CfnLogStream",
"version": "0.0.0"
"fqn": "constructs.Construct",
"version": "10.2.70"
}
}
},
"constructInfo": {
"fqn": "aws-cdk-lib.aws_logs.LogStream",
"version": "0.0.0"
"fqn": "constructs.Construct",
"version": "10.2.70"
}
}
},
"constructInfo": {
"fqn": "aws-cdk-lib.aws_logs.LogGroup",
"version": "0.0.0"
"fqn": "constructs.Construct",
"version": "10.2.70"
}
},
"Resource": {
Expand Down Expand Up @@ -421,58 +378,58 @@
}
},
"constructInfo": {
"fqn": "aws-cdk-lib.aws_kinesisfirehose.CfnDeliveryStream",
"version": "0.0.0"
"fqn": "constructs.Construct",
"version": "10.2.70"
}
}
},
"constructInfo": {
"fqn": "@aws-cdk/aws-kinesisfirehose-alpha.DeliveryStream",
"version": "0.0.0"
"fqn": "constructs.Construct",
"version": "10.2.70"
}
},
"@aws-cdk--aws-kinesisfirehose.CidrBlocks": {
"id": "@aws-cdk--aws-kinesisfirehose.CidrBlocks",
"path": "test-stack/@aws-cdk--aws-kinesisfirehose.CidrBlocks",
"constructInfo": {
"fqn": "aws-cdk-lib.CfnMapping",
"version": "0.0.0"
"fqn": "constructs.Construct",
"version": "10.2.70"
}
},
"BootstrapVersion": {
"id": "BootstrapVersion",
"path": "test-stack/BootstrapVersion",
"constructInfo": {
"fqn": "aws-cdk-lib.CfnParameter",
"version": "0.0.0"
"fqn": "constructs.Construct",
"version": "10.2.70"
}
},
"CheckBootstrapVersion": {
"id": "CheckBootstrapVersion",
"path": "test-stack/CheckBootstrapVersion",
"constructInfo": {
"fqn": "aws-cdk-lib.CfnRule",
"version": "0.0.0"
"fqn": "constructs.Construct",
"version": "10.2.70"
}
}
},
"constructInfo": {
"fqn": "aws-cdk-lib.Stack",
"version": "0.0.0"
"fqn": "constructs.Construct",
"version": "10.2.70"
}
},
"Tree": {
"id": "Tree",
"path": "Tree",
"constructInfo": {
"fqn": "constructs.Construct",
"version": "10.2.69"
"version": "10.2.70"
}
}
},
"constructInfo": {
"fqn": "aws-cdk-lib.App",
"version": "0.0.0"
"fqn": "constructs.Construct",
"version": "10.2.70"
}
}
}
16 changes: 9 additions & 7 deletions packages/@aws-cdk/aws-kinesisfirehose-alpha/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -430,13 +430,15 @@ The DeliveryStream class automatically creates IAM service roles with all the mi
necessary permissions for Kinesis Data Firehose to access the resources referenced by your
delivery stream. One service role is created for the delivery stream that allows Kinesis
Data Firehose to read from a Kinesis data stream (if one is configured as the delivery
stream source) and for server-side encryption. Another service role is created for each
destination, which gives Kinesis Data Firehose write access to the destination resource,
as well as the ability to invoke data transformers and read schemas for record format
conversion. If you wish, you may specify your own IAM role for either the delivery stream
or the destination service role, or both. It must have the correct trust policy (it must
allow Kinesis Data Firehose to assume it) or delivery stream creation or data delivery
will fail. Other required permissions to destination resources, encryption keys, etc.,
stream source) and for server-side encryption. Note that if the DeliveryStream is created
without specifying `sourceStream` or `encryptionKey`, this role is not created as it is not needed.

Another service role is created for each destination, which gives Kinesis Data Firehose write
access to the destination resource, as well as the ability to invoke data transformers and
read schemas for record format conversion. If you wish, you may specify your own IAM role for
either the delivery stream or the destination service role, or both. It must have the correct
trust policy (it must allow Kinesis Data Firehose to assume it) or delivery stream creation or
data delivery will fail. Other required permissions to destination resources, encryption keys, etc.,
will be provided automatically.

```ts
Expand Down
Loading

0 comments on commit 1b8ebf4

Please sign in to comment.