Skip to content

Commit 04ab7c8

Browse files
Convert managed policy names in intrinsic if values (#3875)
Co-authored-by: Reed Hamilton <reedham@amazon.com>
1 parent 1ca9db7 commit 04ab7c8

File tree

6 files changed

+346
-19
lines changed

6 files changed

+346
-19
lines changed

samtranslator/model/role_utils/role_constructor.py

Lines changed: 46 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
from typing import Dict, Optional
1+
from typing import Any, Callable, Dict, List, Optional
22

33
from samtranslator.internal.managed_policies import get_bundled_managed_policy_map
44
from samtranslator.internal.types import GetManagedPolicyMap
@@ -60,6 +60,34 @@ def _get_managed_policy_arn(
6060
return name
6161

6262

63+
def _convert_intrinsic_if_values(
64+
intrinsic_if: Dict[str, List[Any]], is_convertible: Callable[[Any], Any], convert: Callable[[Any], Any]
65+
) -> Dict[str, List[Any]]:
66+
"""
67+
Convert the true and false value of the intrinsic if function according to
68+
`convert` function.
69+
70+
:param intrinsic_if: A dict of the form {"Fn::If": [condition, value_if_true, value_if_false]}
71+
:type intrinsic_if: Dict[str, List[Any]]
72+
:param is_convertible: The function used to decide if the value must be converted
73+
:type convert: Callable[[Any], Any]
74+
:param convert: The function used to make the conversion
75+
:type convert: Callable[[Any], Any]
76+
:return: The input dict with values converted
77+
:rtype: Dict[str, List[Any]]
78+
"""
79+
value_if_true = intrinsic_if["Fn::If"][1]
80+
value_if_false = intrinsic_if["Fn::If"][2]
81+
82+
if is_convertible(value_if_true):
83+
intrinsic_if["Fn::If"][1] = convert(value_if_true)
84+
85+
if is_convertible(value_if_false):
86+
intrinsic_if["Fn::If"][2] = convert(value_if_false)
87+
88+
return intrinsic_if
89+
90+
6391
def construct_role_for_resource( # type: ignore[no-untyped-def] # noqa: PLR0913
6492
resource_logical_id,
6593
attributes,
@@ -102,23 +130,16 @@ def construct_role_for_resource( # type: ignore[no-untyped-def] # noqa: PLR0913
102130
for index, policy_entry in enumerate(resource_policies.get()):
103131
if policy_entry.type is PolicyTypes.POLICY_STATEMENT:
104132
if is_intrinsic_if(policy_entry.data):
105-
intrinsic_if = policy_entry.data
106-
then_statement = intrinsic_if["Fn::If"][1]
107-
else_statement = intrinsic_if["Fn::If"][2]
108-
109-
if not is_intrinsic_no_value(then_statement):
110-
then_statement = {
111-
"PolicyName": execution_role.logical_id + "Policy" + str(index),
112-
"PolicyDocument": then_statement,
113-
}
114-
intrinsic_if["Fn::If"][1] = then_statement
115-
116-
if not is_intrinsic_no_value(else_statement):
117-
else_statement = {
118-
"PolicyName": execution_role.logical_id + "Policy" + str(index),
119-
"PolicyDocument": else_statement,
120-
}
121-
intrinsic_if["Fn::If"][2] = else_statement
133+
intrinsic_if = _convert_intrinsic_if_values(
134+
policy_entry.data,
135+
lambda value: not is_intrinsic_no_value(value),
136+
lambda value: (
137+
{
138+
"PolicyName": execution_role.logical_id + "Policy" + str(index), # noqa: B023
139+
"PolicyDocument": value,
140+
}
141+
),
142+
)
122143

123144
policy_documents.append(intrinsic_if)
124145

@@ -134,7 +155,7 @@ def construct_role_for_resource( # type: ignore[no-untyped-def] # noqa: PLR0913
134155
# There are three options:
135156
# Managed Policy Name (string): Try to convert to Managed Policy ARN
136157
# Managed Policy Arn (string): Insert it directly into the list
137-
# Intrinsic Function (dict): Insert it directly into the list
158+
# Intrinsic Function (dict): Try to convert each statement to Managed Policy Arn
138159
#
139160
# When you insert into managed_policy_arns list, de-dupe to prevent same ARN from showing up twice
140161
#
@@ -146,6 +167,12 @@ def construct_role_for_resource( # type: ignore[no-untyped-def] # noqa: PLR0913
146167
managed_policy_map,
147168
get_managed_policy_map,
148169
)
170+
elif is_intrinsic_if(policy_arn):
171+
policy_arn = _convert_intrinsic_if_values(
172+
policy_arn,
173+
lambda value: not is_intrinsic_no_value(value) and isinstance(value, str),
174+
lambda value: _get_managed_policy_arn(value, managed_policy_map, get_managed_policy_map),
175+
)
149176

150177
# De-Duplicate managed policy arns before inserting. Mainly useful
151178
# when customer specifies a managed policy which is already inserted

tests/model/test_sam_resources.py

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -904,3 +904,76 @@ def test_capacity_provider_with_propagate_tags(self):
904904
tags = resource.Tags
905905
self.assertEqual(sorted([tag["Key"] for tag in tags]), ["Environment", "Project", "lambda:createdBy"])
906906
self.assertEqual(sorted([tag["Value"] for tag in tags]), ["Production", "SAM", "ServerlessApp"])
907+
908+
909+
class TestFunctionPolicy(TestCase):
910+
kwargs = {
911+
"intrinsics_resolver": IntrinsicsResolver({}),
912+
"event_resources": [],
913+
"managed_policy_map": {"foo": "bar"},
914+
"resource_resolver": ResourceResolver({}),
915+
}
916+
917+
@patch("boto3.session.Session.region_name", "ap-southeast-1")
918+
def test_managed_policy_name(self):
919+
function = SamFunction("Foo")
920+
function.CodeUri = "s3://foobar/foo.zip"
921+
function.Runtime = "foo"
922+
function.Handler = "bar"
923+
managedPolicyName = "foo"
924+
function.Policies = [managedPolicyName]
925+
926+
cfnResources = function.to_cloudformation(**self.kwargs)
927+
iamRoles = [x for x in cfnResources if isinstance(x, IAMRole)]
928+
self.assertEqual(iamRoles[0].ManagedPolicyArns[1], self.kwargs["managed_policy_map"][managedPolicyName])
929+
930+
@patch("boto3.session.Session.region_name", "ap-southeast-1")
931+
def test_unknown_policy_name(self):
932+
function = SamFunction("Foo")
933+
function.CodeUri = "s3://foobar/foo.zip"
934+
function.Runtime = "foo"
935+
function.Handler = "bar"
936+
unknownPolicyName = "bar"
937+
function.Policies = [unknownPolicyName]
938+
939+
cfnResources = function.to_cloudformation(**self.kwargs)
940+
iamRoles = [x for x in cfnResources if isinstance(x, IAMRole)]
941+
self.assertEqual(iamRoles[0].ManagedPolicyArns[1], unknownPolicyName)
942+
943+
@patch("boto3.session.Session.region_name", "ap-southeast-1")
944+
def test_managed_policy_name_within_intrinsic_if_then(self):
945+
function = SamFunction("Foo")
946+
function.CodeUri = "s3://foobar/foo.zip"
947+
function.Runtime = "foo"
948+
function.Handler = "bar"
949+
managedPolicyName = "foo"
950+
function.Policies = [{"Fn::If": ["Condition", managedPolicyName, {"Fn::Ref": "AWS::NoValue"}]}]
951+
952+
cfnResources = function.to_cloudformation(**self.kwargs)
953+
iamRoles = [x for x in cfnResources if isinstance(x, IAMRole)]
954+
955+
self.assertIn("Fn::If", iamRoles[0].ManagedPolicyArns[1])
956+
self.assertEqual(iamRoles[0].ManagedPolicyArns[1]["Fn::If"][0], "Condition")
957+
self.assertEqual(
958+
iamRoles[0].ManagedPolicyArns[1]["Fn::If"][1], self.kwargs["managed_policy_map"][managedPolicyName]
959+
)
960+
self.assertDictEqual(iamRoles[0].ManagedPolicyArns[1]["Fn::If"][2], {"Fn::Ref": "AWS::NoValue"})
961+
962+
@patch("boto3.session.Session.region_name", "ap-southeast-1")
963+
def test_managed_policy_name_within_intrinsic_if_else(self):
964+
function = SamFunction("Foo")
965+
function.CodeUri = "s3://foobar/foo.zip"
966+
function.Runtime = "foo"
967+
function.Handler = "bar"
968+
managedPolicyName = "foo"
969+
function.Policies = [{"Fn::If": ["Condition", {"Fn::Ref": "AWS::NoValue"}, managedPolicyName]}]
970+
971+
cfnResources = function.to_cloudformation(**self.kwargs)
972+
iamRoles = [x for x in cfnResources if isinstance(x, IAMRole)]
973+
974+
self.assertIn("Fn::If", iamRoles[0].ManagedPolicyArns[1])
975+
self.assertEqual(iamRoles[0].ManagedPolicyArns[1]["Fn::If"][0], "Condition")
976+
self.assertDictEqual(iamRoles[0].ManagedPolicyArns[1]["Fn::If"][1], {"Fn::Ref": "AWS::NoValue"})
977+
self.assertEqual(
978+
iamRoles[0].ManagedPolicyArns[1]["Fn::If"][2], self.kwargs["managed_policy_map"][managedPolicyName]
979+
)
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
AWSTemplateFormatVersion: '2010-09-09'
2+
Transform: AWS::Serverless-2016-10-31
3+
4+
Conditions:
5+
IsTrue: true
6+
7+
Resources:
8+
ExampleFunction:
9+
Type: AWS::Serverless::Function
10+
Properties:
11+
Runtime: python3.12
12+
Handler: handler
13+
InlineCode: >
14+
def handler():
15+
pass
16+
Policies:
17+
- Fn::If:
18+
- IsTrue
19+
- CloudWatchLambdaInsightsExecutionRolePolicy
20+
- !Ref AWS::NoValue
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
{
2+
"AWSTemplateFormatVersion": "2010-09-09",
3+
"Conditions": {
4+
"IsTrue": true
5+
},
6+
"Resources": {
7+
"ExampleFunction": {
8+
"Properties": {
9+
"Code": {
10+
"ZipFile": "def handler():\n pass\n"
11+
},
12+
"Handler": "handler",
13+
"Role": {
14+
"Fn::GetAtt": [
15+
"ExampleFunctionRole",
16+
"Arn"
17+
]
18+
},
19+
"Runtime": "python3.12",
20+
"Tags": [
21+
{
22+
"Key": "lambda:createdBy",
23+
"Value": "SAM"
24+
}
25+
]
26+
},
27+
"Type": "AWS::Lambda::Function"
28+
},
29+
"ExampleFunctionRole": {
30+
"Properties": {
31+
"AssumeRolePolicyDocument": {
32+
"Statement": [
33+
{
34+
"Action": [
35+
"sts:AssumeRole"
36+
],
37+
"Effect": "Allow",
38+
"Principal": {
39+
"Service": [
40+
"lambda.amazonaws.com"
41+
]
42+
}
43+
}
44+
],
45+
"Version": "2012-10-17"
46+
},
47+
"ManagedPolicyArns": [
48+
"arn:aws-cn:iam::aws:policy/service-role/AWSLambdaBasicExecutionRole",
49+
{
50+
"Fn::If": [
51+
"IsTrue",
52+
"arn:aws-cn:iam::aws:policy/CloudWatchLambdaInsightsExecutionRolePolicy",
53+
{
54+
"Ref": "AWS::NoValue"
55+
}
56+
]
57+
}
58+
],
59+
"Tags": [
60+
{
61+
"Key": "lambda:createdBy",
62+
"Value": "SAM"
63+
}
64+
]
65+
},
66+
"Type": "AWS::IAM::Role"
67+
}
68+
}
69+
}
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
{
2+
"AWSTemplateFormatVersion": "2010-09-09",
3+
"Conditions": {
4+
"IsTrue": true
5+
},
6+
"Resources": {
7+
"ExampleFunction": {
8+
"Properties": {
9+
"Code": {
10+
"ZipFile": "def handler():\n pass\n"
11+
},
12+
"Handler": "handler",
13+
"Role": {
14+
"Fn::GetAtt": [
15+
"ExampleFunctionRole",
16+
"Arn"
17+
]
18+
},
19+
"Runtime": "python3.12",
20+
"Tags": [
21+
{
22+
"Key": "lambda:createdBy",
23+
"Value": "SAM"
24+
}
25+
]
26+
},
27+
"Type": "AWS::Lambda::Function"
28+
},
29+
"ExampleFunctionRole": {
30+
"Properties": {
31+
"AssumeRolePolicyDocument": {
32+
"Statement": [
33+
{
34+
"Action": [
35+
"sts:AssumeRole"
36+
],
37+
"Effect": "Allow",
38+
"Principal": {
39+
"Service": [
40+
"lambda.amazonaws.com"
41+
]
42+
}
43+
}
44+
],
45+
"Version": "2012-10-17"
46+
},
47+
"ManagedPolicyArns": [
48+
"arn:aws-us-gov:iam::aws:policy/service-role/AWSLambdaBasicExecutionRole",
49+
{
50+
"Fn::If": [
51+
"IsTrue",
52+
"arn:aws-us-gov:iam::aws:policy/CloudWatchLambdaInsightsExecutionRolePolicy",
53+
{
54+
"Ref": "AWS::NoValue"
55+
}
56+
]
57+
}
58+
],
59+
"Tags": [
60+
{
61+
"Key": "lambda:createdBy",
62+
"Value": "SAM"
63+
}
64+
]
65+
},
66+
"Type": "AWS::IAM::Role"
67+
}
68+
}
69+
}

0 commit comments

Comments
 (0)