Skip to content

Commit

Permalink
feat(s3): add skip destination validation property
Browse files Browse the repository at this point in the history
Closes aws#30914
  • Loading branch information
yerzhan7 committed Jul 22, 2024
1 parent 7621439 commit 89a5d9b
Show file tree
Hide file tree
Showing 13 changed files with 164 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,8 @@
}
]
},
"Managed": true
"Managed": true,
"SkipDestinationValidation": false
},
"DependsOn": [
"BAllowBucketNotificationsTolambdaeventsources3F741608059EF9F709",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@
}
]
},
"Managed": true
"Managed": true,
"SkipDestinationValidation": false
},
"DependsOn": [
"TopicPolicyA1747468",
Expand Down Expand Up @@ -305,7 +306,8 @@
}
]
},
"Managed": true
"Managed": true,
"SkipDestinationValidation": false
},
"DependsOn": [
"Topic3Policy49BDDFBD",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,8 @@
}
]
},
"Managed": true
"Managed": true,
"SkipDestinationValidation": false
},
"DependsOn": [
"YourBucketAllowBucketNotificationsTocdkinteglambdabuckets3notificationsMyFunction47013F39EAF10051"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,8 @@
}
]
},
"Managed": true
"Managed": true,
"SkipDestinationValidation": false
},
"DependsOn": [
"ObjectCreatedTopicPolicyA938ECFC",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@
}
]
},
"Managed": true
"Managed": true,
"SkipDestinationValidation": false
},
"DependsOn": [
"EncryptedQueueKey6F4FD304",
Expand Down Expand Up @@ -322,7 +323,8 @@
}
]
},
"Managed": true
"Managed": true,
"SkipDestinationValidation": false
},
"DependsOn": [
"Bucket2Policy945B22E3",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,8 @@
"NotificationConfiguration": {
"EventBridgeConfiguration": {}
},
"Managed": true
"Managed": true,
"SkipDestinationValidation": false
},
"DependsOn": [
"MyEventBridgeBucketPolicy8F5346E3"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,14 @@ def handler(event: dict, context):
props = event["ResourceProperties"]
notification_configuration = props["NotificationConfiguration"]
managed = props.get('Managed', 'true').lower() == 'true'
skipDestinationValidation = props.get('SkipDestinationValidation', 'false').lower() == 'true'
stack_id = event['StackId']
old = event.get("OldResourceProperties", {}).get("NotificationConfiguration", {})
if managed:
config = handle_managed(event["RequestType"], notification_configuration)
else:
config = handle_unmanaged(props["BucketName"], stack_id, event["RequestType"], notification_configuration, old)
s3.put_bucket_notification_configuration(Bucket=props["BucketName"], NotificationConfiguration=config)
s3.put_bucket_notification_configuration(Bucket=props["BucketName"], NotificationConfiguration=config, SkipDestinationValidation=skipDestinationValidation)
except Exception as e:
logging.exception("Failed to put bucket notification configuration")
response_status = "FAILED"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
FROM public.ecr.aws/lambda/python:3.7
FROM public.ecr.aws/lambda/python:3.11

ADD . /opt/lambda
WORKDIR /opt/lambda
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ def make_event(request_type: str, managed: bool):
"Managed": str(managed),
"BucketName": "BucketName",
"NotificationConfiguration": make_notification_configuration(),
"SkipDestinationValidation": str(False),
},
}

Expand All @@ -43,6 +44,19 @@ def make_event_with_eventbridge(request_type: str, managed: bool):
"Managed": str(managed),
"BucketName": "BucketName",
"NotificationConfiguration": make_notification_configuration_with_eventbridge(),
"SkipDestinationValidation": str(False),
},
}

def make_event_with_skip_destination_validation(request_type: str, managed: bool):
return {
"StackId": "StackId",
"RequestType": request_type,
"ResourceProperties": {
"Managed": str(managed),
"BucketName": "BucketName",
"NotificationConfiguration": make_notification_configuration(),
"SkipDestinationValidation": str(True),
},
}

Expand Down Expand Up @@ -96,6 +110,7 @@ def test_create(self, _, mock_s3: MagicMock):
mock_s3.put_bucket_notification_configuration.assert_called_once_with(
Bucket=event["ResourceProperties"]["BucketName"],
NotificationConfiguration=event["ResourceProperties"]["NotificationConfiguration"],
SkipDestinationValidation=False,
)

@patch('index.s3')
Expand All @@ -109,6 +124,7 @@ def test_update(self, _, mock_s3: MagicMock):
mock_s3.put_bucket_notification_configuration.assert_called_once_with(
Bucket=event["ResourceProperties"]["BucketName"],
NotificationConfiguration=event["ResourceProperties"]["NotificationConfiguration"],
SkipDestinationValidation=False,
)

@patch('index.s3')
Expand All @@ -119,7 +135,25 @@ def test_delete(self, _, mock_s3: MagicMock):

index.handler(event, {})

mock_s3.put_bucket_notification_configuration.assert_called_once_with(Bucket=event["ResourceProperties"]["BucketName"], NotificationConfiguration={})
mock_s3.put_bucket_notification_configuration.assert_called_once_with(
Bucket=event["ResourceProperties"]["BucketName"],
NotificationConfiguration={},
SkipDestinationValidation=False,
)

@patch('index.s3')
@patch("index.submit_response")
def test_skip_destination_validation(self, _, mock_s3: MagicMock):

event = make_event_with_skip_destination_validation("Create", True)

index.handler(event, {})

mock_s3.put_bucket_notification_configuration.assert_called_once_with(
Bucket=event["ResourceProperties"]["BucketName"],
NotificationConfiguration=event["ResourceProperties"]["NotificationConfiguration"],
SkipDestinationValidation=True,
)


class UnmanagedCleanBucketTest(unittest.TestCase):
Expand All @@ -136,6 +170,7 @@ def test_create(self, _, mock_s3: MagicMock):
mock_s3.put_bucket_notification_configuration.assert_called_once_with(
Bucket=event["ResourceProperties"]["BucketName"],
NotificationConfiguration=event["ResourceProperties"]["NotificationConfiguration"],
SkipDestinationValidation=False,
)

@patch('index.s3')
Expand All @@ -151,6 +186,7 @@ def test_create_with_eventbridge(self, _, mock_s3: MagicMock):
mock_s3.put_bucket_notification_configuration.assert_called_once_with(
Bucket=event["ResourceProperties"]["BucketName"],
NotificationConfiguration=event["ResourceProperties"]["NotificationConfiguration"],
SkipDestinationValidation=False,
)

@patch('index.s3')
Expand All @@ -171,6 +207,7 @@ def test_update(self, _, mock_s3: MagicMock):
current_notifications,
event["ResourceProperties"]["NotificationConfiguration"]
),
SkipDestinationValidation=False,
)


Expand All @@ -192,6 +229,7 @@ def test_delete_existing_s3_notifications(self, _, mock_s3: MagicMock):
current_notifications,
event["ResourceProperties"]["NotificationConfiguration"]
),
SkipDestinationValidation=False,
)

@patch('index.s3')
Expand All @@ -212,6 +250,7 @@ def test_update_with_eventbridge(self, _, mock_s3: MagicMock):
current_notifications,
event["ResourceProperties"]["NotificationConfiguration"]
),
SkipDestinationValidation=False,
)


Expand All @@ -233,6 +272,7 @@ def test_update_with_existing_eventbridge(self, _, mock_s3: MagicMock):
current_notifications,
event["ResourceProperties"]["NotificationConfiguration"],
),
SkipDestinationValidation=False,
)

@patch('index.s3')
Expand All @@ -250,6 +290,7 @@ def test_delete(self, _, mock_s3: MagicMock):
mock_s3.put_bucket_notification_configuration.assert_called_once_with(
Bucket=event["ResourceProperties"]["BucketName"],
NotificationConfiguration=make_empty_notification_configuration(),
SkipDestinationValidation=False,
)

@patch('index.s3')
Expand All @@ -267,6 +308,7 @@ def test_delete_with_eventbridge_should_not_remove_eventbridge(self, _, mock_s3:
mock_s3.put_bucket_notification_configuration.assert_called_once_with(
Bucket=event["ResourceProperties"]["BucketName"],
NotificationConfiguration=make_empty_notification_configuration_with_eventbridge(),
SkipDestinationValidation=False,
)


Expand All @@ -289,6 +331,7 @@ def test_create(self, _, mock_s3: MagicMock):
current_notifications,
event["ResourceProperties"]["NotificationConfiguration"],
),
SkipDestinationValidation=False,
)

@patch('index.s3')
Expand All @@ -309,6 +352,7 @@ def test_create_with_eventbridge(self, _, mock_s3: MagicMock):
current_notifications,
event["ResourceProperties"]["NotificationConfiguration"],
),
SkipDestinationValidation=False,
)

@patch('index.s3')
Expand All @@ -329,6 +373,7 @@ def test_create_with_existing_eventbridge(self, _, mock_s3: MagicMock):
current_notifications,
event["ResourceProperties"]["NotificationConfiguration"],
),
SkipDestinationValidation=False,
)

@patch('index.s3')
Expand All @@ -349,6 +394,7 @@ def test_update(self, _, mock_s3: MagicMock):
current_notifications,
event["ResourceProperties"]["NotificationConfiguration"],
),
SkipDestinationValidation=False,
)

@patch('index.s3')
Expand All @@ -369,6 +415,7 @@ def test_update_with_eventbridge(self, _, mock_s3: MagicMock):
current_notifications,
event["ResourceProperties"]["NotificationConfiguration"],
),
SkipDestinationValidation=False,
)

@patch('index.s3')
Expand All @@ -387,6 +434,7 @@ def test_update_without_eventbridge_should_not_remove_existing_eventbridge(self,
current_notifications,
event["ResourceProperties"]["NotificationConfiguration"],
),
SkipDestinationValidation=False,
)

@patch('index.s3')
Expand All @@ -404,6 +452,7 @@ def test_delete(self, _, mock_s3: MagicMock):
mock_s3.put_bucket_notification_configuration.assert_called_once_with(
Bucket=event["ResourceProperties"]["BucketName"],
NotificationConfiguration=current_notifications,
SkipDestinationValidation=False,
)

@patch('index.s3')
Expand All @@ -421,6 +470,7 @@ def test_delete_with_eventbridge_should_not_remove_eventbridge(self, _, mock_s3:
mock_s3.put_bucket_notification_configuration.assert_called_once_with(
Bucket=event["ResourceProperties"]["BucketName"],
NotificationConfiguration=current_notifications,
SkipDestinationValidation=False,
)


Expand Down
10 changes: 10 additions & 0 deletions packages/aws-cdk-lib/aws-s3/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,16 @@ const bucket = s3.Bucket.fromBucketAttributes(this, 'ImportedBucket', {
bucket.addEventNotification(s3.EventType.OBJECT_CREATED, new s3n.SnsDestination(topic));
```

If you do not want for S3 to validate permissions of Amazon SQS, Amazon SNS, and Lambda destinations you can use this flag:

```ts
declare const myQueue: sqs.Queue;
const bucket = new s3.Bucket(this, 'MyBucket', {
notificationsSkipDestinationValidation: true,
});
bucket.addEventNotification(s3.EventType.OBJECT_REMOVED, new s3n.SqsDestination(myQueue));
```

When you add an event notification to a bucket, a custom resource is created to
manage the notifications. By default, a new role is created for the Lambda
function that implements this feature. If you want to use your own role instead,
Expand Down
11 changes: 11 additions & 0 deletions packages/aws-cdk-lib/aws-s3/lib/bucket.ts
Original file line number Diff line number Diff line change
Expand Up @@ -537,6 +537,8 @@ export abstract class BucketBase extends Resource implements IBucket {

protected notificationsHandlerRole?: iam.IRole;

protected notificationsSkipDestinationValidation?: boolean;

protected objectOwnership?: ObjectOwnership;

constructor(scope: Construct, id: string, props: ResourceProps = {}) {
Expand Down Expand Up @@ -890,6 +892,7 @@ export abstract class BucketBase extends Resource implements IBucket {
this.notifications = new BucketNotifications(this, 'Notifications', {
bucket: this,
handlerRole: this.notificationsHandlerRole,
skipDestinationValidation: this.notificationsSkipDestinationValidation ?? false,
});
}
cb(this.notifications);
Expand Down Expand Up @@ -1652,6 +1655,13 @@ export interface BucketProps {
*/
readonly notificationsHandlerRole?: iam.IRole;

/**
* Skips notification validation of Amazon SQS, Amazon SNS, and Lambda destinations.
*
* @default false
*/
readonly notificationsSkipDestinationValidation?: boolean;

/**
* Inteligent Tiering Configurations
*
Expand Down Expand Up @@ -1911,6 +1921,7 @@ export class Bucket extends BucketBase {
});

this.notificationsHandlerRole = props.notificationsHandlerRole;
this.notificationsSkipDestinationValidation = props.notificationsSkipDestinationValidation;

const { bucketEncryption, encryptionKey } = this.parseEncryption(props);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,11 @@ interface NotificationsProps {
* The role to be used by the lambda handler
*/
handlerRole?: iam.IRole;

/**
* Skips notification validation of Amazon SQS, Amazon SNS, and Lambda destinations.
*/
skipDestinationValidation: boolean;
}

/**
Expand All @@ -40,11 +45,13 @@ export class BucketNotifications extends Construct {
private resource?: cdk.CfnResource;
private readonly bucket: IBucket;
private readonly handlerRole?: iam.IRole;
private readonly skipDestinationValidation: boolean;

constructor(scope: Construct, id: string, props: NotificationsProps) {
super(scope, id);
this.bucket = props.bucket;
this.handlerRole = props.handlerRole;
this.skipDestinationValidation = props.skipDestinationValidation;
}

/**
Expand Down Expand Up @@ -133,6 +140,7 @@ export class BucketNotifications extends Construct {
BucketName: this.bucket.bucketName,
NotificationConfiguration: cdk.Lazy.any({ produce: () => this.renderNotificationConfiguration() }),
Managed: managed,
SkipDestinationValidation: this.skipDestinationValidation ?? false,
},
});

Expand Down
Loading

0 comments on commit 89a5d9b

Please sign in to comment.