Skip to content

feat(aws-s3-stepfunctions): Changed escape hatch to eventBridgeEnabled prop #666

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Apr 22, 2022
Merged

feat(aws-s3-stepfunctions): Changed escape hatch to eventBridgeEnabled prop #666

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8023dbe
changed escape hatch to eventBridgeEnabled prop
mickychetta Apr 21, 2022
487ea23
suppres cfn nag notification handler warnings
mickychetta Apr 22, 2022
b05dc8c
suppress notification handler cfn nag warnings
mickychetta Apr 22, 2022
d82f194
suppress notification handler cfn nag warnings
mickychetta Apr 22, 2022
a3ed44c
added comments of required eventBridgeEnabled property
mickychetta Apr 22, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
125 changes: 120 additions & 5 deletions ...ws-solutions-constructs/aws-s3-step-function/test/integ.customLoggingBucket.expected.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,11 +114,6 @@
"Ref": "tests3stepfunctiontests3stepfunctionWS3LoggingBucketB716417C"
}
},
"NotificationConfiguration": {
"EventBridgeConfiguration": {
"EventBridgeEnabled": true
}
},
"PublicAccessBlockConfiguration": {
"BlockPublicAcls": true,
"BlockPublicPolicy": true,
Expand Down Expand Up @@ -179,6 +174,24 @@
}
}
},
"tests3stepfunctiontests3stepfunctionWS3BucketNotificationsC4380C1D": {
"Type": "Custom::S3BucketNotifications",
"Properties": {
"ServiceToken": {
"Fn::GetAtt": [
"BucketNotificationsHandler050a0587b7544547bf325f094a3db8347ECC3691",
"Arn"
]
},
"BucketName": {
"Ref": "tests3stepfunctiontests3stepfunctionWS3Bucket9BE64924"
},
"NotificationConfiguration": {
"EventBridgeConfiguration": {}
},
"Managed": true
}
},
"tests3stepfunctiontests3stepfunctionWtests3stepfunctionWeventrulestepfunctionconstructStateMachineLogGroupE83EECDD": {
"Type": "AWS::Logs::LogGroup",
"Properties": {
Expand Down Expand Up @@ -461,6 +474,108 @@
"Statistic": "Maximum",
"Threshold": 1
}
},
"BucketNotificationsHandler050a0587b7544547bf325f094a3db834RoleB6FB88EC": {
"Type": "AWS::IAM::Role",
"Properties": {
"AssumeRolePolicyDocument": {
"Statement": [
{
"Action": "sts:AssumeRole",
"Effect": "Allow",
"Principal": {
"Service": "lambda.amazonaws.com"
}
}
],
"Version": "2012-10-17"
},
"ManagedPolicyArns": [
{
"Fn::Join": [
"",
[
"arn:",
{
"Ref": "AWS::Partition"
},
":iam::aws:policy/service-role/AWSLambdaBasicExecutionRole"
]
]
}
]
}
},
"BucketNotificationsHandler050a0587b7544547bf325f094a3db834RoleDefaultPolicy2CF63D36": {
"Type": "AWS::IAM::Policy",
"Properties": {
"PolicyDocument": {
"Statement": [
{
"Action": "s3:PutBucketNotification",
"Effect": "Allow",
"Resource": "*"
}
],
"Version": "2012-10-17"
},
"PolicyName": "BucketNotificationsHandler050a0587b7544547bf325f094a3db834RoleDefaultPolicy2CF63D36",
"Roles": [
{
"Ref": "BucketNotificationsHandler050a0587b7544547bf325f094a3db834RoleB6FB88EC"
}
]
},
"Metadata": {
"cfn_nag": {
"rules_to_suppress": [
{
"id": "W12",
"reason": "Bucket resource is '*' due to circular dependency with bucket and role creation at the same time"
}
]
}
}
},
"BucketNotificationsHandler050a0587b7544547bf325f094a3db8347ECC3691": {
"Type": "AWS::Lambda::Function",
"Properties": {
"Description": "AWS CloudFormation handler for \"Custom::S3BucketNotifications\" resources (@aws-cdk/aws-s3)",
"Code": {
"ZipFile": "import boto3 # type: ignore\nimport json\nimport logging\nimport urllib.request\n\ns3 = boto3.client(\"s3\")\n\nEVENTBRIDGE_CONFIGURATION = 'EventBridgeConfiguration'\n\nCONFIGURATION_TYPES = [\"TopicConfigurations\", \"QueueConfigurations\", \"LambdaFunctionConfigurations\"]\n\ndef handler(event: dict, context):\n response_status = \"SUCCESS\"\n error_message = \"\"\n try:\n props = event[\"ResourceProperties\"]\n bucket = props[\"BucketName\"]\n notification_configuration = props[\"NotificationConfiguration\"]\n request_type = event[\"RequestType\"]\n managed = props.get('Managed', 'true').lower() == 'true'\n stack_id = event['StackId']\n\n if managed:\n config = handle_managed(request_type, notification_configuration)\n else:\n config = handle_unmanaged(bucket, stack_id, request_type, notification_configuration)\n\n put_bucket_notification_configuration(bucket, config)\n except Exception as e:\n logging.exception(\"Failed to put bucket notification configuration\")\n response_status = \"FAILED\"\n error_message = f\"Error: {str(e)}. \"\n finally:\n submit_response(event, context, response_status, error_message)\n\ndef handle_managed(request_type, notification_configuration):\n if request_type == 'Delete':\n return {}\n return notification_configuration\n\ndef handle_unmanaged(bucket, stack_id, request_type, notification_configuration):\n external_notifications = find_external_notifications(bucket, stack_id)\n\n if request_type == 'Delete':\n return external_notifications\n\n def with_id(notification):\n notification['Id'] = f\"{stack_id}-{hash(json.dumps(notification, sort_keys=True))}\"\n return notification\n\n notifications = {}\n for t in CONFIGURATION_TYPES:\n external = external_notifications.get(t, [])\n incoming = [with_id(n) for n in notification_configuration.get(t, [])]\n notifications[t] = external + incoming\n\n if EVENTBRIDGE_CONFIGURATION in notification_configuration:\n notifications[EVENTBRIDGE_CONFIGURATION] = notification_configuration[EVENTBRIDGE_CONFIGURATION]\n elif EVENTBRIDGE_CONFIGURATION in external_notifications:\n notifications[EVENTBRIDGE_CONFIGURATION] = external_notifications[EVENTBRIDGE_CONFIGURATION]\n\n return notifications\n\ndef find_external_notifications(bucket, stack_id):\n existing_notifications = get_bucket_notification_configuration(bucket)\n external_notifications = {}\n for t in CONFIGURATION_TYPES:\n external_notifications[t] = [n for n in existing_notifications.get(t, []) if not n['Id'].startswith(f\"{stack_id}-\")]\n\n if EVENTBRIDGE_CONFIGURATION in existing_notifications:\n external_notifications[EVENTBRIDGE_CONFIGURATION] = existing_notifications[EVENTBRIDGE_CONFIGURATION]\n\n return external_notifications\n\ndef get_bucket_notification_configuration(bucket):\n return s3.get_bucket_notification_configuration(Bucket=bucket)\n\ndef put_bucket_notification_configuration(bucket, notification_configuration):\n s3.put_bucket_notification_configuration(Bucket=bucket, NotificationConfiguration=notification_configuration)\n\ndef submit_response(event: dict, context, response_status: str, error_message: str):\n response_body = json.dumps(\n {\n \"Status\": response_status,\n \"Reason\": f\"{error_message}See the details in CloudWatch Log Stream: {context.log_stream_name}\",\n \"PhysicalResourceId\": event.get(\"PhysicalResourceId\") or event[\"LogicalResourceId\"],\n \"StackId\": event[\"StackId\"],\n \"RequestId\": event[\"RequestId\"],\n \"LogicalResourceId\": event[\"LogicalResourceId\"],\n \"NoEcho\": False,\n }\n ).encode(\"utf-8\")\n headers = {\"content-type\": \"\", \"content-length\": str(len(response_body))}\n try:\n req = urllib.request.Request(url=event[\"ResponseURL\"], headers=headers, data=response_body, method=\"PUT\")\n with urllib.request.urlopen(req) as response:\n print(response.read().decode(\"utf-8\"))\n print(\"Status code: \" + response.reason)\n except Exception as e:\n print(\"send(..) failed executing request.urlopen(..): \" + str(e))\n"
},
"Handler": "index.handler",
"Role": {
"Fn::GetAtt": [
"BucketNotificationsHandler050a0587b7544547bf325f094a3db834RoleB6FB88EC",
"Arn"
]
},
"Runtime": "python3.7",
"Timeout": 300
},
"DependsOn": [
"BucketNotificationsHandler050a0587b7544547bf325f094a3db834RoleDefaultPolicy2CF63D36",
"BucketNotificationsHandler050a0587b7544547bf325f094a3db834RoleB6FB88EC"
],
"Metadata": {
"cfn_nag": {
"rules_to_suppress": [
{
"id": "W58",
"reason": "Lambda functions has the required permission to write CloudWatch Logs. It uses custom policy instead of arn:aws:iam::aws:policy/service-role/AWSLambdaBasicExecutionRole with tighter permissions."
},
{
"id": "W89",
"reason": "This is not a rule for the general case, just for specific use cases/industries"
},
{
"id": "W92",
"reason": "Impossible for us to define the correct concurrency for clients"
}
]
}
}
}
},
"Mappings": {
Expand Down
125 changes: 120 additions & 5 deletions ...ws-solutions-constructs/aws-s3-step-function/test/integ.pre-existing-bucket.expected.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,6 @@
}
]
},
"NotificationConfiguration": {
"EventBridgeConfiguration": {
"EventBridgeEnabled": true
}
},
"VersioningConfiguration": {
"Status": "Enabled"
}
Expand All @@ -42,6 +37,126 @@
}
}
},
"existingScriptLocationNotificationsC550EA17": {
"Type": "Custom::S3BucketNotifications",
"Properties": {
"ServiceToken": {
"Fn::GetAtt": [
"BucketNotificationsHandler050a0587b7544547bf325f094a3db8347ECC3691",
"Arn"
]
},
"BucketName": {
"Ref": "existingScriptLocation845F3C51"
},
"NotificationConfiguration": {
"EventBridgeConfiguration": {}
},
"Managed": true
}
},
"BucketNotificationsHandler050a0587b7544547bf325f094a3db834RoleB6FB88EC": {
"Type": "AWS::IAM::Role",
"Properties": {
"AssumeRolePolicyDocument": {
"Statement": [
{
"Action": "sts:AssumeRole",
"Effect": "Allow",
"Principal": {
"Service": "lambda.amazonaws.com"
}
}
],
"Version": "2012-10-17"
},
"ManagedPolicyArns": [
{
"Fn::Join": [
"",
[
"arn:",
{
"Ref": "AWS::Partition"
},
":iam::aws:policy/service-role/AWSLambdaBasicExecutionRole"
]
]
}
]
}
},
"BucketNotificationsHandler050a0587b7544547bf325f094a3db834RoleDefaultPolicy2CF63D36": {
"Type": "AWS::IAM::Policy",
"Properties": {
"PolicyDocument": {
"Statement": [
{
"Action": "s3:PutBucketNotification",
"Effect": "Allow",
"Resource": "*"
}
],
"Version": "2012-10-17"
},
"PolicyName": "BucketNotificationsHandler050a0587b7544547bf325f094a3db834RoleDefaultPolicy2CF63D36",
"Roles": [
{
"Ref": "BucketNotificationsHandler050a0587b7544547bf325f094a3db834RoleB6FB88EC"
}
]
},
"Metadata": {
"cfn_nag": {
"rules_to_suppress": [
{
"id": "W12",
"reason": "Bucket resource is '*' due to circular dependency with bucket and role creation at the same time"
}
]
}
}
},
"BucketNotificationsHandler050a0587b7544547bf325f094a3db8347ECC3691": {
"Type": "AWS::Lambda::Function",
"Properties": {
"Description": "AWS CloudFormation handler for \"Custom::S3BucketNotifications\" resources (@aws-cdk/aws-s3)",
"Code": {
"ZipFile": "import boto3 # type: ignore\nimport json\nimport logging\nimport urllib.request\n\ns3 = boto3.client(\"s3\")\n\nEVENTBRIDGE_CONFIGURATION = 'EventBridgeConfiguration'\n\nCONFIGURATION_TYPES = [\"TopicConfigurations\", \"QueueConfigurations\", \"LambdaFunctionConfigurations\"]\n\ndef handler(event: dict, context):\n response_status = \"SUCCESS\"\n error_message = \"\"\n try:\n props = event[\"ResourceProperties\"]\n bucket = props[\"BucketName\"]\n notification_configuration = props[\"NotificationConfiguration\"]\n request_type = event[\"RequestType\"]\n managed = props.get('Managed', 'true').lower() == 'true'\n stack_id = event['StackId']\n\n if managed:\n config = handle_managed(request_type, notification_configuration)\n else:\n config = handle_unmanaged(bucket, stack_id, request_type, notification_configuration)\n\n put_bucket_notification_configuration(bucket, config)\n except Exception as e:\n logging.exception(\"Failed to put bucket notification configuration\")\n response_status = \"FAILED\"\n error_message = f\"Error: {str(e)}. \"\n finally:\n submit_response(event, context, response_status, error_message)\n\ndef handle_managed(request_type, notification_configuration):\n if request_type == 'Delete':\n return {}\n return notification_configuration\n\ndef handle_unmanaged(bucket, stack_id, request_type, notification_configuration):\n external_notifications = find_external_notifications(bucket, stack_id)\n\n if request_type == 'Delete':\n return external_notifications\n\n def with_id(notification):\n notification['Id'] = f\"{stack_id}-{hash(json.dumps(notification, sort_keys=True))}\"\n return notification\n\n notifications = {}\n for t in CONFIGURATION_TYPES:\n external = external_notifications.get(t, [])\n incoming = [with_id(n) for n in notification_configuration.get(t, [])]\n notifications[t] = external + incoming\n\n if EVENTBRIDGE_CONFIGURATION in notification_configuration:\n notifications[EVENTBRIDGE_CONFIGURATION] = notification_configuration[EVENTBRIDGE_CONFIGURATION]\n elif EVENTBRIDGE_CONFIGURATION in external_notifications:\n notifications[EVENTBRIDGE_CONFIGURATION] = external_notifications[EVENTBRIDGE_CONFIGURATION]\n\n return notifications\n\ndef find_external_notifications(bucket, stack_id):\n existing_notifications = get_bucket_notification_configuration(bucket)\n external_notifications = {}\n for t in CONFIGURATION_TYPES:\n external_notifications[t] = [n for n in existing_notifications.get(t, []) if not n['Id'].startswith(f\"{stack_id}-\")]\n\n if EVENTBRIDGE_CONFIGURATION in existing_notifications:\n external_notifications[EVENTBRIDGE_CONFIGURATION] = existing_notifications[EVENTBRIDGE_CONFIGURATION]\n\n return external_notifications\n\ndef get_bucket_notification_configuration(bucket):\n return s3.get_bucket_notification_configuration(Bucket=bucket)\n\ndef put_bucket_notification_configuration(bucket, notification_configuration):\n s3.put_bucket_notification_configuration(Bucket=bucket, NotificationConfiguration=notification_configuration)\n\ndef submit_response(event: dict, context, response_status: str, error_message: str):\n response_body = json.dumps(\n {\n \"Status\": response_status,\n \"Reason\": f\"{error_message}See the details in CloudWatch Log Stream: {context.log_stream_name}\",\n \"PhysicalResourceId\": event.get(\"PhysicalResourceId\") or event[\"LogicalResourceId\"],\n \"StackId\": event[\"StackId\"],\n \"RequestId\": event[\"RequestId\"],\n \"LogicalResourceId\": event[\"LogicalResourceId\"],\n \"NoEcho\": False,\n }\n ).encode(\"utf-8\")\n headers = {\"content-type\": \"\", \"content-length\": str(len(response_body))}\n try:\n req = urllib.request.Request(url=event[\"ResponseURL\"], headers=headers, data=response_body, method=\"PUT\")\n with urllib.request.urlopen(req) as response:\n print(response.read().decode(\"utf-8\"))\n print(\"Status code: \" + response.reason)\n except Exception as e:\n print(\"send(..) failed executing request.urlopen(..): \" + str(e))\n"
},
"Handler": "index.handler",
"Role": {
"Fn::GetAtt": [
"BucketNotificationsHandler050a0587b7544547bf325f094a3db834RoleB6FB88EC",
"Arn"
]
},
"Runtime": "python3.7",
"Timeout": 300
},
"DependsOn": [
"BucketNotificationsHandler050a0587b7544547bf325f094a3db834RoleDefaultPolicy2CF63D36",
"BucketNotificationsHandler050a0587b7544547bf325f094a3db834RoleB6FB88EC"
],
"Metadata": {
"cfn_nag": {
"rules_to_suppress": [
{
"id": "W58",
"reason": "Lambda functions has the required permission to write CloudWatch Logs. It uses custom policy instead of arn:aws:iam::aws:policy/service-role/AWSLambdaBasicExecutionRole with tighter permissions."
},
{
"id": "W89",
"reason": "This is not a rule for the general case, just for specific use cases/industries"
},
{
"id": "W92",
"reason": "Impossible for us to define the correct concurrency for clients"
}
]
}
}
},
"tests3stepfunctionpreexistingbucketconstructtests3stepfunctionpreexistingbucketconstructWtests3stepfunctionpreexistingbucketconstructWeventrulestepfunctionconstructStateMachineLogGroupF215071C": {
"Type": "AWS::Logs::LogGroup",
"Properties": {
Expand Down
12 changes: 6 additions & 6 deletions ...patterns/@aws-solutions-constructs/aws-s3-step-function/test/integ.pre-existing-bucket.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,14 @@
import { App, Stack, RemovalPolicy } from "@aws-cdk/core";
import { S3ToStepFunction, S3ToStepFunctionProps } from "../lib";
import * as stepfunctions from '@aws-cdk/aws-stepfunctions';
import * as s3 from '@aws-cdk/aws-s3';
import { CreateScrapBucket } from '@aws-solutions-constructs/core';
import { generateIntegStackName } from '@aws-solutions-constructs/core';
import { CreateScrapBucket, generateIntegStackName, addCfnNagS3BucketNotificationRulesToSuppress } from '@aws-solutions-constructs/core';

const app = new App();
const stack = new Stack(app, generateIntegStackName(__filename));

const existingBucket = CreateScrapBucket(stack, {});
const cfnBucket = existingBucket.node.defaultChild as s3.CfnBucket;
cfnBucket.addPropertyOverride('NotificationConfiguration.EventBridgeConfiguration.EventBridgeEnabled', true);
const existingBucket = CreateScrapBucket(stack, {
eventBridgeEnabled: true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there now a requirement that if an existing bucket is provided that eventBridgeEnabled is set to true? We should document that in the README. Better yet, can we turn it on ourselves? (I'm guessing that if this accepts an IBucket then we are unable to do so...)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, we are unable to turn it on ourselves

});

const startState = new stepfunctions.Pass(stack, 'StartState');

Expand All @@ -40,4 +38,6 @@ const props: S3ToStepFunctionProps = {

new S3ToStepFunction(stack, 'test-s3-step-function-pre-existing-bucket-construct', props);

addCfnNagS3BucketNotificationRulesToSuppress(stack, 'BucketNotificationsHandler050a0587b7544547bf325f094a3db834');

app.synth();
Loading