Skip to content

Commit cd0e9bd

Browse files
jogoldElad Ben-Israel
authored and
Elad Ben-Israel
committed
fix(s3): fail early with bad notification filters (#3397)
Avoid CF deploy time errors when specifying multiple prefixes or suffixes in notification filters. Closes #3347 Possibly something to fix in v2.0 (#3398)
1 parent 820575b commit cd0e9bd

File tree

2 files changed

+83
-1
lines changed

2 files changed

+83
-1
lines changed

packages/@aws-cdk/aws-s3/lib/notifications-resource/notifications-resource.ts

+10
Original file line numberDiff line numberDiff line change
@@ -122,18 +122,28 @@ function renderFilters(filters?: NotificationKeyFilter[]): Filter | undefined {
122122
}
123123

124124
const renderedRules = new Array<FilterRule>();
125+
let hasPrefix = false;
126+
let hasSuffix = false;
125127

126128
for (const rule of filters) {
127129
if (!rule.suffix && !rule.prefix) {
128130
throw new Error('NotificationKeyFilter must specify `prefix` and/or `suffix`');
129131
}
130132

131133
if (rule.suffix) {
134+
if (hasSuffix) {
135+
throw new Error('Cannot specify more than one suffix rule in a filter.');
136+
}
132137
renderedRules.push({ Name: 'suffix', Value: rule.suffix });
138+
hasSuffix = true;
133139
}
134140

135141
if (rule.prefix) {
142+
if (hasPrefix) {
143+
throw new Error('Cannot specify more than one prefix rule in a filter.');
144+
}
136145
renderedRules.push({ Name: 'prefix', Value: rule.prefix });
146+
hasPrefix = true;
137147
}
138148
}
139149

packages/@aws-cdk/aws-s3/test/test.notification.ts

+73-1
Original file line numberDiff line numberDiff line change
@@ -21,4 +21,76 @@ export = {
2121

2222
test.done();
2323
},
24-
};
24+
25+
'can specify prefix and suffix filter rules'(test: Test) {
26+
const stack = new cdk.Stack();
27+
28+
const bucket = new s3.Bucket(stack, 'MyBucket');
29+
30+
bucket.addEventNotification(s3.EventType.OBJECT_CREATED, {
31+
bind: () => ({
32+
arn: 'ARN',
33+
type: s3.BucketNotificationDestinationType.TOPIC
34+
}),
35+
}, { prefix: 'images/', suffix: '.png' });
36+
37+
expect(stack).to(haveResource('Custom::S3BucketNotifications', {
38+
NotificationConfiguration: {
39+
TopicConfigurations: [
40+
{
41+
Events: [
42+
's3:ObjectCreated:*'
43+
],
44+
Filter: {
45+
Key: {
46+
FilterRules: [
47+
{
48+
Name: 'suffix',
49+
Value: '.png'
50+
},
51+
{
52+
Name: 'prefix',
53+
Value: 'images/'
54+
}
55+
]
56+
}
57+
},
58+
TopicArn: 'ARN'
59+
}
60+
]
61+
}
62+
}));
63+
64+
test.done();
65+
},
66+
67+
'throws with multiple prefix rules in a filter'(test: Test) {
68+
const stack = new cdk.Stack();
69+
70+
const bucket = new s3.Bucket(stack, 'MyBucket');
71+
72+
test.throws(() => bucket.addEventNotification(s3.EventType.OBJECT_CREATED, {
73+
bind: () => ({
74+
arn: 'ARN',
75+
type: s3.BucketNotificationDestinationType.TOPIC
76+
}),
77+
}, { prefix: 'images/'}, { prefix: 'archive/' }), /prefix rule/);
78+
79+
test.done();
80+
},
81+
82+
'throws with multiple suffix rules in a filter'(test: Test) {
83+
const stack = new cdk.Stack();
84+
85+
const bucket = new s3.Bucket(stack, 'MyBucket');
86+
87+
test.throws(() => bucket.addEventNotification(s3.EventType.OBJECT_CREATED, {
88+
bind: () => ({
89+
arn: 'ARN',
90+
type: s3.BucketNotificationDestinationType.TOPIC
91+
}),
92+
}, { suffix: '.png'}, { suffix: '.zip' }), /suffix rule/);
93+
94+
test.done();
95+
}
96+
};

0 commit comments

Comments
 (0)