Reformatted to make value changes in to_cloudformation

Author: Sid Madipalli

samtranslator/model/sam_resources.py

@@ -394,7 +394,24 @@ def to_cloudformation(self, **kwargs):  # type: ignore[no-untyped-def] # noqa: P
             intrinsics_resolver,
             get_managed_policy_map,
         )
-        self._make_lambda_role(lambda_function, intrinsics_resolver, execution_role, resources, conditions)
+
+        if lambda_function.Role is None:
+            lambda_function.Role = execution_role.get_runtime_attr("arn")
+            resources.append(execution_role)
+        elif is_intrinsic_if(lambda_function.Role):
+            role_changes = self._make_lambda_role(lambda_function, intrinsics_resolver, execution_role)
+
+            if role_changes["should_append_role"]:
+                resources.append(execution_role)
+
+            if role_changes["lambda_role_value"] is not None:
+                lambda_function.Role = role_changes["lambda_role_value"]
+
+            if role_changes["execution_role_condition"] is not None:
+                execution_role.set_resource_attribute("Condition", role_changes["execution_role_condition"])
+
+            if role_changes["new_condition"] is not None:
+                conditions.update(role_changes["new_condition"])
 
         try:
             resources += self._generate_event_resources(
@@ -417,46 +434,48 @@ def _make_lambda_role(
         lambda_function: LambdaFunction,
         intrinsics_resolver: IntrinsicsResolver,
         execution_role: IAMRole,
-        resources: List[Any],
-        conditions: Dict[str, Any],
-    ) -> None:
-        lambda_role = lambda_function.Role
-
-        if lambda_role is None:
-            resources.append(execution_role)
-            lambda_function.Role = execution_role.get_runtime_attr("arn")
-
-        elif is_intrinsic_if(lambda_role):
-
-            # We need to create and if else condition here
-            role_resolved_value = intrinsics_resolver.resolve_parameter_refs(lambda_role)
-            role_condition, role_if, role_else = role_resolved_value.get("Fn::If")
-
-            is_both_intrinsic_no_values = is_intrinsic_no_value(role_if) and is_intrinsic_no_value(role_else)
+    ) -> Dict[str, Any]:
+        """
+        Analyzes lambda role requirements and returns the changes needed.
 
-            # Create a role only when no value is in either of the conditions
-            if is_intrinsic_no_value(role_if) or is_intrinsic_no_value(role_else):
-                resources.append(execution_role)
+        Returns:
+            Dict containing:
+                - 'should_append_role': bool - whether to append execution_role to resources
+                - 'lambda_role_value': Any - value to set for lambda_function.Role
+                - 'execution_role_condition': str|None - condition to set on execution_role
+                - 'new_condition': Dict|None - new condition to add to conditions dict
+        """
+        lambda_role = lambda_function.Role
+        execution_role_arn = execution_role.get_runtime_attr("arn")
 
-            # both are none values, we always need to create a role
-            if is_both_intrinsic_no_values:
-                lambda_function.Role = execution_role.get_runtime_attr("arn")
+        result = {
+            "should_append_role": False,
+            "lambda_role_value": None,
+            "execution_role_condition": None,
+            "new_condition": None,
+        }
 
-            # first value is none so we should create condition ? create : [2]
-            # create a condition for IAM role to only create on if case
-            elif is_intrinsic_no_value(role_if):
-                lambda_function.Role = make_conditional(
-                    role_condition, execution_role.get_runtime_attr("arn"), role_else
-                )
-                execution_role.set_resource_attribute("Condition", f"{role_condition}")
-
-            # second value is none so we should create condition ? [1] : create
-            # create a condition for IAM role to only create on else case
-            # with top level condition that negates the condition passed
-            elif is_intrinsic_no_value(role_else):
-                lambda_function.Role = make_conditional(role_condition, role_if, execution_role.get_runtime_attr("arn"))
-                execution_role.set_resource_attribute("Condition", f"NOT{role_condition}")
-                conditions[f"NOT{role_condition}"] = make_not_conditional(role_condition)
+        # We need to create and if else condition here
+        role_resolved_value = intrinsics_resolver.resolve_parameter_refs(lambda_role)
+        role_condition, role_if, role_else = role_resolved_value.get("Fn::If")
+
+        # first value is none so we should create condition ? create : [2]
+        # create a condition for IAM role to only create on if case
+        if is_intrinsic_no_value(role_if):
+            result["lambda_role_value"] = make_conditional(role_condition, execution_role_arn, role_else)
+            result["execution_role_condition"] = f"{role_condition}"
+            result["should_append_role"] = True
+
+        # second value is none so we should create condition ? [1] : create
+        # create a condition for IAM role to only create on else case
+        # with top level condition that negates the condition passed
+        elif is_intrinsic_no_value(role_else):
+            result["lambda_role_value"] = make_conditional(role_condition, role_if, execution_role_arn)
+            result["execution_role_condition"] = f"NOT{role_condition}"
+            result["new_condition"] = {f"NOT{role_condition}": make_not_conditional(role_condition)}
+            result["should_append_role"] = True
+
+        return result
 
     def _construct_event_invoke_config(  # noqa: PLR0913
         self,

tests/model/test_sam_resources.py

@@ -751,14 +751,12 @@ def setUp(self):
         self.function.CodeUri = "s3://foobar/foo.zip"
         self.function.Runtime = "foo"
         self.function.Handler = "bar"
-        self.template = {"Conditions": {}}
-
         self.kwargs = {
             "intrinsics_resolver": IntrinsicsResolver({}),
             "event_resources": [],
             "managed_policy_map": {},
             "resource_resolver": ResourceResolver({}),
-            "conditions": self.template.get("Conditions", {}),
+            "conditions": {"Conditions": {}},
         }
 
     def test_role_none_creates_execution_role(self):
@@ -785,9 +783,8 @@ def test_role_fn_if_no_aws_no_value_keeps_original(self):
         }
         self.function.Role = role_conditional
 
-        template = {"Conditions": {"Condition": True}}
         kwargs = dict(self.kwargs)
-        kwargs["conditions"] = template.get("Conditions", {})
+        kwargs["conditions"] = {"Condition": True}
 
         cfn_resources = self.function.to_cloudformation(**self.kwargs)
         generated_roles = [x for x in cfn_resources if isinstance(x, IAMRole)]
@@ -801,9 +798,8 @@ def test_role_fn_if_both_no_value_creates_execution_role(self):
         role_conditional = {"Fn::If": ["Condition", {"Ref": "AWS::NoValue"}, {"Ref": "AWS::NoValue"}]}
         self.function.Role = role_conditional
 
-        template = {"Conditions": {"Condition": True}}
         kwargs = dict(self.kwargs)
-        kwargs["conditions"] = template.get("Conditions", {})
+        kwargs["conditions"] = {"Condition": True}
 
         cfn_resources = self.function.to_cloudformation(**self.kwargs)
         generated_roles = [x for x in cfn_resources if isinstance(x, IAMRole)]
@@ -814,9 +810,8 @@ def test_role_fn_if_first_no_value_creates_conditional_role(self):
         role_conditional = {"Fn::If": ["Condition", {"Ref": "AWS::NoValue"}, {"Ref": "iamRoleArn"}]}
         self.function.Role = role_conditional
 
-        template = {"Conditions": {"Condition": True}}
         kwargs = dict(self.kwargs)
-        kwargs["conditions"] = template.get("Conditions", {})
+        kwargs["conditions"] = {"Condition": True}
 
         cfn_resources = self.function.to_cloudformation(**self.kwargs)
         generated_roles = [x for x in cfn_resources if isinstance(x, IAMRole)]
@@ -831,9 +826,8 @@ def test_role_fn_if_second_no_value_creates_conditional_role(self):
         role_conditional = {"Fn::If": ["Condition", {"Ref": "iamRoleArn"}, {"Ref": "AWS::NoValue"}]}
         self.function.Role = role_conditional
 
-        template = {"Conditions": {"Condition": True}}
         kwargs = dict(self.kwargs)
-        kwargs["conditions"] = template.get("Conditions", {})
+        kwargs["conditions"] = {"Condition": True}
 
         cfn_resources = self.function.to_cloudformation(**self.kwargs)
         generated_roles = [x for x in cfn_resources if isinstance(x, IAMRole)]