Skip to content

Commit 236fdc8

Browse files
ssenchenkohoffa
ssenchenko
and
hoffa
authored
fix: multiple mq source event policy name (add DynamicPolicyName) (#2953)
Co-authored-by: Christoffer Rehn <[email protected]>
1 parent 653ad56 commit 236fdc8

File tree

9 files changed

+718
-4
lines changed

9 files changed

+718
-4
lines changed

‎samtranslator/internal/schema_source/aws_serverless_function.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -405,6 +405,7 @@ class MSKEvent(BaseModel):
405405
class MQEventProperties(BaseModel):
406406
BatchSize: Optional[PassThroughProp] = mqeventproperties("BatchSize")
407407
Broker: PassThroughProp = mqeventproperties("Broker")
408+
DynamicPolicyName: Optional[bool] # TODO: add docs
408409
Enabled: Optional[PassThroughProp] = mqeventproperties("Enabled")
409410
FilterCriteria: Optional[PassThroughProp] = mqeventproperties("FilterCriteria")
410411
MaximumBatchingWindowInSeconds: Optional[PassThroughProp] = mqeventproperties("MaximumBatchingWindowInSeconds")

‎samtranslator/model/eventsources/pull.py

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33

44
from samtranslator.internal.deprecation_control import deprecated
55
from samtranslator.metrics.method_decorator import cw_timer
6-
from samtranslator.model import PassThroughProperty, PropertyType, ResourceMacro
6+
from samtranslator.model import PassThroughProperty, Property, PropertyType, ResourceMacro
77
from samtranslator.model.eventsources import FUNCTION_EVETSOURCE_METRIC_PREFIX
88
from samtranslator.model.exceptions import InvalidEventException
99
from samtranslator.model.iam import IAMRolePolicies
@@ -424,9 +424,46 @@ class MQ(PullEventSource):
424424
property_types: Dict[str, PropertyType] = {
425425
**PullEventSource.property_types,
426426
"Broker": PassThroughProperty(True),
427+
"DynamicPolicyName": Property(False, is_type(bool)),
427428
}
428429

429430
Broker: PassThrough
431+
DynamicPolicyName: Optional[bool]
432+
433+
@property
434+
def _policy_name(self) -> str:
435+
"""Generate policy name based on DynamicPolicyName flag and MQ logical ID.
436+
437+
Policy name is required though its update is "No interuption".
438+
https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-iam-policy.html#cfn-iam-policy-policyname #noqa
439+
440+
Historically, policy name was hardcoded as `SamAutoGeneratedAMQPolicy` but it led to a policy name clash
441+
and failure to deploy, if a Function had at least 2 MQ event sources.
442+
Since policy is attached to the Lambda execution role,
443+
policy name should be based on MQ logical ID not to clash with policy names of other MQ event sources.
444+
However, to support backwards compatibility, we need to keep policy `SamAutoGeneratedAMQPolicy` by default,
445+
because customers might have code which relys on that policy name consistancy.
446+
447+
To support both old policy name and ability to have more than one MQ event source, we introduce new field
448+
`DynamicPolicyName` which when set to true will use MQ logical ID to generate policy name.
449+
450+
Q: Why to introduce a new field and not to make policy name dynamic by default if there are multiple
451+
MQ event sources?
452+
A: Since a customer could have a single MQ source and rely on it's policy name in their code. If that customer
453+
decides to add a new MQ source, they don't want to change the policy name for the first MQ all over their
454+
code base. But they can opt in using a dynamic policy name for all other MQ sources they add.
455+
456+
Q: Why not use dynamic policy names automatically for all MQ event sources but first?
457+
A: SAM-T doesn't have state and doesn't know what was the CFN resource attribute in a previous transformation.
458+
Hence, trying to "use dynamic policy names automatically for all MQ event sources but first" can rely only
459+
on event source order. If a customer added a new MQ source __before__ an old one, an old one would receive
460+
a dynamic name and would break (potentially) customer's code.
461+
462+
Returns
463+
-------
464+
Name of the policy which will be attached to the Lambda Execution role.
465+
"""
466+
return f"{self.logical_id}AMQPolicy" if self.DynamicPolicyName else "SamAutoGeneratedAMQPolicy"
430467

431468
def get_event_source_arn(self) -> Optional[PassThrough]:
432469
return self.Broker
@@ -438,7 +475,7 @@ def get_policy_statements(self) -> Optional[List[Dict[str, Any]]]:
438475
basic_auth_uri = self._validate_source_access_configurations(["BASIC_AUTH", "VIRTUAL_HOST"], "BASIC_AUTH")
439476

440477
document = {
441-
"PolicyName": "SamAutoGeneratedAMQPolicy",
478+
"PolicyName": self._policy_name,
442479
"PolicyDocument": {
443480
"Statement": [
444481
{

‎samtranslator/schema/schema.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195466,6 +195466,10 @@
195466195466
"markdownDescription": "The Amazon Resource Name \\(ARN\\) of the Amazon MQ broker\\. \n*Type*: String \n*Required*: Yes \n*AWS CloudFormation compatibility*: This property is passed directly to the [`EventSourceArn`](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-lambda-eventsourcemapping.html#cfn-lambda-eventsourcemapping-eventsourcearn) property of an `AWS::Lambda::EventSourceMapping` resource\\.",
195467195467
"title": "Broker"
195468195468
},
195469+
"DynamicPolicyName": {
195470+
"title": "Dynamicpolicyname",
195471+
"type": "boolean"
195472+
},
195469195473
"Enabled": {
195470195474
"allOf": [
195471195475
{

‎schema_source/sam.schema.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1960,6 +1960,10 @@
19601960
"markdownDescription": "The Amazon Resource Name \\(ARN\\) of the Amazon MQ broker\\. \n*Type*: String \n*Required*: Yes \n*AWS CloudFormation compatibility*: This property is passed directly to the [`EventSourceArn`](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-lambda-eventsourcemapping.html#cfn-lambda-eventsourcemapping-eventsourcearn) property of an `AWS::Lambda::EventSourceMapping` resource\\.",
19611961
"title": "Broker"
19621962
},
1963+
"DynamicPolicyName": {
1964+
"title": "Dynamicpolicyname",
1965+
"type": "boolean"
1966+
},
19631967
"Enabled": {
19641968
"allOf": [
19651969
{

‎tests/model/eventsources/test_mq_event_source.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ def test_get_policy_statements(self):
2222
policy_statements = self.mq_event_source.get_policy_statements()
2323
expected_policy_document = [
2424
{
25-
"PolicyName": "SamAutoGeneratedAMQPolicy",
25+
"PolicyName": self.mq_event_source._policy_name,
2626
"PolicyDocument": {
2727
"Statement": [
2828
{
@@ -69,7 +69,7 @@ def test_get_policy_statements_with_secrets_manager_kms_key_id(self):
6969
policy_statements = self.mq_event_source.get_policy_statements()
7070
expected_policy_document = [
7171
{
72-
"PolicyName": "SamAutoGeneratedAMQPolicy",
72+
"PolicyName": self.mq_event_source._policy_name,
7373
"PolicyDocument": {
7474
"Statement": [
7575
{

‎tests/translator/input/function_with_multiple_mq.yaml

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
Resources:
2+
MyFunction:
3+
Type: AWS::Serverless::Function
4+
Properties:
5+
FunctionName: Test
6+
CodeUri: s3://sam-demo-bucket/queues.zip
7+
Handler: app.handler
8+
Runtime: nodejs18.x
9+
MemorySize: 256
10+
Architectures:
11+
- x86_64
12+
Events:
13+
BrokerOne:
14+
Type: MQ
15+
Properties:
16+
Broker: !Sub 'arn:${AWS::Partition}:mq:us-east-2:123456789012:broker:MyBroker:b-1234a5b6-78cd-901e-2fgh-3i45j6k17801'
17+
BatchSize: 1
18+
Queues:
19+
- MyQueue
20+
SourceAccessConfigurations:
21+
- Type: BASIC_AUTH
22+
URI: !Sub 'arn:${AWS::Partition}:secretsmanager:us-west-2:123456789012:secret:my-path/my-secret-name-1a2b01'
23+
BrokerTwo:
24+
Type: MQ
25+
Properties:
26+
Broker: !Sub 'arn:${AWS::Partition}:mq:us-east-2:123456789012:broker:MyBroker:b-1234a5b6-78cd-901e-2fgh-3i45j6k17802'
27+
BatchSize: 1
28+
DynamicPolicyName: true
29+
Queues:
30+
- MyQueue
31+
SourceAccessConfigurations:
32+
- Type: BASIC_AUTH
33+
URI: !Sub 'arn:${AWS::Partition}:secretsmanager:us-west-2:123456789012:secret:my-path/my-secret-name-1a2b02'
34+
BrokerThree:
35+
Type: MQ
36+
Properties:
37+
Broker: !Sub 'arn:${AWS::Partition}:mq:us-east-2:123456789012:broker:MyBroker:b-1234a5b6-78cd-901e-2fgh-3i45j6k17803'
38+
BatchSize: 1
39+
DynamicPolicyName: true
40+
Queues:
41+
- MyQueue
42+
SourceAccessConfigurations:
43+
- Type: BASIC_AUTH
44+
URI: !Sub 'arn:${AWS::Partition}:secretsmanager:us-west-2:123456789012:secret:my-path/my-secret-name-1a2b03'

‎tests/translator/output/aws-cn/function_with_multiple_mq.json

Lines changed: 208 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,208 @@
1+
{
2+
"Resources": {
3+
"MyFunction": {
4+
"Properties": {
5+
"Architectures": [
6+
"x86_64"
7+
],
8+
"Code": {
9+
"S3Bucket": "sam-demo-bucket",
10+
"S3Key": "queues.zip"
11+
},
12+
"FunctionName": "Test",
13+
"Handler": "app.handler",
14+
"MemorySize": 256,
15+
"Role": {
16+
"Fn::GetAtt": [
17+
"MyFunctionRole",
18+
"Arn"
19+
]
20+
},
21+
"Runtime": "nodejs18.x",
22+
"Tags": [
23+
{
24+
"Key": "lambda:createdBy",
25+
"Value": "SAM"
26+
}
27+
]
28+
},
29+
"Type": "AWS::Lambda::Function"
30+
},
31+
"MyFunctionBrokerOne": {
32+
"Properties": {
33+
"BatchSize": 1,
34+
"EventSourceArn": {
35+
"Fn::Sub": "arn:${AWS::Partition}:mq:us-east-2:123456789012:broker:MyBroker:b-1234a5b6-78cd-901e-2fgh-3i45j6k17801"
36+
},
37+
"FunctionName": {
38+
"Ref": "MyFunction"
39+
},
40+
"Queues": [
41+
"MyQueue"
42+
],
43+
"SourceAccessConfigurations": [
44+
{
45+
"Type": "BASIC_AUTH",
46+
"URI": {
47+
"Fn::Sub": "arn:${AWS::Partition}:secretsmanager:us-west-2:123456789012:secret:my-path/my-secret-name-1a2b01"
48+
}
49+
}
50+
]
51+
},
52+
"Type": "AWS::Lambda::EventSourceMapping"
53+
},
54+
"MyFunctionBrokerThree": {
55+
"Properties": {
56+
"BatchSize": 1,
57+
"EventSourceArn": {
58+
"Fn::Sub": "arn:${AWS::Partition}:mq:us-east-2:123456789012:broker:MyBroker:b-1234a5b6-78cd-901e-2fgh-3i45j6k17803"
59+
},
60+
"FunctionName": {
61+
"Ref": "MyFunction"
62+
},
63+
"Queues": [
64+
"MyQueue"
65+
],
66+
"SourceAccessConfigurations": [
67+
{
68+
"Type": "BASIC_AUTH",
69+
"URI": {
70+
"Fn::Sub": "arn:${AWS::Partition}:secretsmanager:us-west-2:123456789012:secret:my-path/my-secret-name-1a2b03"
71+
}
72+
}
73+
]
74+
},
75+
"Type": "AWS::Lambda::EventSourceMapping"
76+
},
77+
"MyFunctionBrokerTwo": {
78+
"Properties": {
79+
"BatchSize": 1,
80+
"EventSourceArn": {
81+
"Fn::Sub": "arn:${AWS::Partition}:mq:us-east-2:123456789012:broker:MyBroker:b-1234a5b6-78cd-901e-2fgh-3i45j6k17802"
82+
},
83+
"FunctionName": {
84+
"Ref": "MyFunction"
85+
},
86+
"Queues": [
87+
"MyQueue"
88+
],
89+
"SourceAccessConfigurations": [
90+
{
91+
"Type": "BASIC_AUTH",
92+
"URI": {
93+
"Fn::Sub": "arn:${AWS::Partition}:secretsmanager:us-west-2:123456789012:secret:my-path/my-secret-name-1a2b02"
94+
}
95+
}
96+
]
97+
},
98+
"Type": "AWS::Lambda::EventSourceMapping"
99+
},
100+
"MyFunctionRole": {
101+
"Properties": {
102+
"AssumeRolePolicyDocument": {
103+
"Statement": [
104+
{
105+
"Action": [
106+
"sts:AssumeRole"
107+
],
108+
"Effect": "Allow",
109+
"Principal": {
110+
"Service": [
111+
"lambda.amazonaws.com"
112+
]
113+
}
114+
}
115+
],
116+
"Version": "2012-10-17"
117+
},
118+
"ManagedPolicyArns": [
119+
"arn:aws-cn:iam::aws:policy/service-role/AWSLambdaBasicExecutionRole"
120+
],
121+
"Policies": [
122+
{
123+
"PolicyDocument": {
124+
"Statement": [
125+
{
126+
"Action": [
127+
"secretsmanager:GetSecretValue"
128+
],
129+
"Effect": "Allow",
130+
"Resource": {
131+
"Fn::Sub": "arn:${AWS::Partition}:secretsmanager:us-west-2:123456789012:secret:my-path/my-secret-name-1a2b01"
132+
}
133+
},
134+
{
135+
"Action": [
136+
"mq:DescribeBroker"
137+
],
138+
"Effect": "Allow",
139+
"Resource": {
140+
"Fn::Sub": "arn:${AWS::Partition}:mq:us-east-2:123456789012:broker:MyBroker:b-1234a5b6-78cd-901e-2fgh-3i45j6k17801"
141+
}
142+
}
143+
]
144+
},
145+
"PolicyName": "SamAutoGeneratedAMQPolicy"
146+
},
147+
{
148+
"PolicyDocument": {
149+
"Statement": [
150+
{
151+
"Action": [
152+
"secretsmanager:GetSecretValue"
153+
],
154+
"Effect": "Allow",
155+
"Resource": {
156+
"Fn::Sub": "arn:${AWS::Partition}:secretsmanager:us-west-2:123456789012:secret:my-path/my-secret-name-1a2b03"
157+
}
158+
},
159+
{
160+
"Action": [
161+
"mq:DescribeBroker"
162+
],
163+
"Effect": "Allow",
164+
"Resource": {
165+
"Fn::Sub": "arn:${AWS::Partition}:mq:us-east-2:123456789012:broker:MyBroker:b-1234a5b6-78cd-901e-2fgh-3i45j6k17803"
166+
}
167+
}
168+
]
169+
},
170+
"PolicyName": "MyFunctionBrokerThreeAMQPolicy"
171+
},
172+
{
173+
"PolicyDocument": {
174+
"Statement": [
175+
{
176+
"Action": [
177+
"secretsmanager:GetSecretValue"
178+
],
179+
"Effect": "Allow",
180+
"Resource": {
181+
"Fn::Sub": "arn:${AWS::Partition}:secretsmanager:us-west-2:123456789012:secret:my-path/my-secret-name-1a2b02"
182+
}
183+
},
184+
{
185+
"Action": [
186+
"mq:DescribeBroker"
187+
],
188+
"Effect": "Allow",
189+
"Resource": {
190+
"Fn::Sub": "arn:${AWS::Partition}:mq:us-east-2:123456789012:broker:MyBroker:b-1234a5b6-78cd-901e-2fgh-3i45j6k17802"
191+
}
192+
}
193+
]
194+
},
195+
"PolicyName": "MyFunctionBrokerTwoAMQPolicy"
196+
}
197+
],
198+
"Tags": [
199+
{
200+
"Key": "lambda:createdBy",
201+
"Value": "SAM"
202+
}
203+
]
204+
},
205+
"Type": "AWS::IAM::Role"
206+
}
207+
}
208+
}

0 commit comments

Comments
 (0)