Revert "feat: actionable error on non-ARN policy (#3132)" (#3153)

aws · May 10, 2023 · 0b875f6 · 0b875f6
1 parent 7b5de1a
commit 0b875f6
Show file tree

Hide file tree

Showing 11 changed files with 24 additions and 168 deletions.
diff --git a/samtranslator/model/role_utils/role_constructor.py b/samtranslator/model/role_utils/role_constructor.py
@@ -13,7 +13,7 @@ def _get_managed_policy_arn(
     name: str,
     managed_policy_map: Optional[Dict[str, str]],
     get_managed_policy_map: Optional[GetManagedPolicyMap],
-) -> Optional[str]:
+) -> str:
     """
     Get the ARN of a AWS managed policy name. Used in Policies property of
     AWS::Serverless::Function and AWS::Serverless::StateMachine.
@@ -57,7 +57,7 @@ def _get_managed_policy_arn(
             if arn:
                 return arn
 
-    return None
+    return name
 
 
 def construct_role_for_resource(  # type: ignore[no-untyped-def] # noqa: too-many-arguments
@@ -146,11 +146,6 @@ def construct_role_for_resource(  # type: ignore[no-untyped-def] # noqa: too-man
                     managed_policy_map,
                     get_managed_policy_map,
                 )
-                if not policy_arn:
-                    raise InvalidResourceException(
-                        resource_logical_id,
-                        f"Invalid policy '{policy_entry.data}'; make sure the value is a valid AWS managed policy ARN.",
-                    )
 
             # De-Duplicate managed policy arns before inserting. Mainly useful
             # when customer specifies a managed policy which is already inserted

diff --git a/tests/translator/input/error_managed_policies_function.yaml b/tests/translator/input/error_managed_policies_function.yaml
diff --git a/tests/translator/input/error_managed_policies_state_machine.yaml b/tests/translator/input/error_managed_policies_state_machine.yaml
diff --git a/tests/translator/input/managed_policies_minimal.yaml b/tests/translator/input/managed_policies_minimal.yaml
@@ -6,19 +6,13 @@ Resources:
       Handler: foo
       InlineCode: bar
       Policies:
-      - arn:aws:iam::aws:policy/AmazonSQSFullAccess
-      - arn:aws-cn:iam::aws:policy/AmazonRedshiftFullAccess
-      - arn:looks:like::an/arn
+      - AnyNonOfficialManagedPolicy
       - AmazonS3FullAccess
-      - AmazonAPIGatewayPushToCloudWatchLogs
 
   MyStateMachine:
     Type: AWS::Serverless::StateMachine
     Properties:
       DefinitionUri: s3://foo/bar
       Policies:
-      - arn:aws:iam::aws:policy/AmazonSQSFullAccess
-      - arn:aws-cn:iam::aws:policy/AmazonRedshiftFullAccess
-      - arn:looks:like::an/arn
+      - AnyNonOfficialManagedPolicy
       - AmazonS3FullAccess
-      - AmazonAPIGatewayPushToCloudWatchLogs
diff --git a/tests/translator/output/aws-cn/managed_policies_minimal.json b/tests/translator/output/aws-cn/managed_policies_minimal.json
@@ -42,11 +42,8 @@
         },
         "ManagedPolicyArns": [
           "arn:aws-cn:iam::aws:policy/service-role/AWSLambdaBasicExecutionRole",
-          "arn:aws:iam::aws:policy/AmazonSQSFullAccess",
-          "arn:aws-cn:iam::aws:policy/AmazonRedshiftFullAccess",
-          "arn:looks:like::an/arn",
-          "arn:aws-cn:iam::aws:policy/AmazonS3FullAccess",
-          "arn:aws-cn:iam::aws:policy/service-role/AmazonAPIGatewayPushToCloudWatchLogs"
+          "AnyNonOfficialManagedPolicy",
+          "arn:aws-cn:iam::aws:policy/AmazonS3FullAccess"
         ],
         "Tags": [
           {
@@ -97,11 +94,8 @@
           "Version": "2012-10-17"
         },
         "ManagedPolicyArns": [
-          "arn:aws:iam::aws:policy/AmazonSQSFullAccess",
-          "arn:aws-cn:iam::aws:policy/AmazonRedshiftFullAccess",
-          "arn:looks:like::an/arn",
-          "arn:aws-cn:iam::aws:policy/AmazonS3FullAccess",
-          "arn:aws-cn:iam::aws:policy/service-role/AmazonAPIGatewayPushToCloudWatchLogs"
+          "AnyNonOfficialManagedPolicy",
+          "arn:aws-cn:iam::aws:policy/AmazonS3FullAccess"
         ],
         "Tags": [
           {

diff --git a/tests/translator/output/aws-us-gov/managed_policies_minimal.json b/tests/translator/output/aws-us-gov/managed_policies_minimal.json
@@ -42,11 +42,8 @@
         },
         "ManagedPolicyArns": [
           "arn:aws-us-gov:iam::aws:policy/service-role/AWSLambdaBasicExecutionRole",
-          "arn:aws:iam::aws:policy/AmazonSQSFullAccess",
-          "arn:aws-cn:iam::aws:policy/AmazonRedshiftFullAccess",
-          "arn:looks:like::an/arn",
-          "arn:aws-us-gov:iam::aws:policy/AmazonS3FullAccess-mock-from-fallback-policy-loader",
-          "arn:aws-us-gov:iam::aws:policy/service-role/AmazonAPIGatewayPushToCloudWatchLogs-mock-from-fallback-policy-loader"
+          "AnyNonOfficialManagedPolicy",
+          "AmazonS3FullAccess"
         ],
         "Tags": [
           {
@@ -97,11 +94,8 @@
           "Version": "2012-10-17"
         },
         "ManagedPolicyArns": [
-          "arn:aws:iam::aws:policy/AmazonSQSFullAccess",
-          "arn:aws-cn:iam::aws:policy/AmazonRedshiftFullAccess",
-          "arn:looks:like::an/arn",
-          "arn:aws-us-gov:iam::aws:policy/AmazonS3FullAccess-mock-from-fallback-policy-loader",
-          "arn:aws-us-gov:iam::aws:policy/service-role/AmazonAPIGatewayPushToCloudWatchLogs-mock-from-fallback-policy-loader"
+          "AnyNonOfficialManagedPolicy",
+          "AmazonS3FullAccess"
         ],
         "Tags": [
           {

diff --git a/tests/translator/output/aws-us-gov/schema_validation_1.json b/tests/translator/output/aws-us-gov/schema_validation_1.json
@@ -60,7 +60,7 @@
           "Version": "2012-10-17"
         },
         "ManagedPolicyArns": [
-          "arn:aws-us-gov:iam::aws:policy/AWSXrayWriteOnlyAccess-mock-from-fallback-policy-loader",
+          "AWSXrayWriteOnlyAccess",
           "arn:aws-us-gov:iam::aws:policy/AWSXRayDaemonWriteAccess",
           "arn:aws-us-gov:iam::aws:policy/service-role/AWSLambdaBasicExecutionRole"
         ],
@@ -170,7 +170,7 @@
           "Version": "2012-10-17"
         },
         "ManagedPolicyArns": [
-          "arn:aws-us-gov:iam::aws:policy/AWSXrayWriteOnlyAccess-mock-from-fallback-policy-loader",
+          "AWSXrayWriteOnlyAccess",
           "arn:aws-us-gov:iam::aws:policy/AWSXRayDaemonWriteAccess"
         ],
         "Policies": [

diff --git a/tests/translator/output/error_managed_policies_function.json b/tests/translator/output/error_managed_policies_function.json
diff --git a/tests/translator/output/error_managed_policies_state_machine.json b/tests/translator/output/error_managed_policies_state_machine.json
diff --git a/tests/translator/output/managed_policies_minimal.json b/tests/translator/output/managed_policies_minimal.json
@@ -42,11 +42,8 @@
         },
         "ManagedPolicyArns": [
           "arn:aws:iam::aws:policy/service-role/AWSLambdaBasicExecutionRole",
-          "arn:aws:iam::aws:policy/AmazonSQSFullAccess",
-          "arn:aws-cn:iam::aws:policy/AmazonRedshiftFullAccess",
-          "arn:looks:like::an/arn",
-          "arn:aws:iam::aws:policy/AmazonS3FullAccess",
-          "arn:aws:iam::aws:policy/service-role/AmazonAPIGatewayPushToCloudWatchLogs"
+          "AnyNonOfficialManagedPolicy",
+          "arn:aws:iam::aws:policy/AmazonS3FullAccess"
         ],
         "Tags": [
           {
@@ -97,11 +94,8 @@
           "Version": "2012-10-17"
         },
         "ManagedPolicyArns": [
-          "arn:aws:iam::aws:policy/AmazonSQSFullAccess",
-          "arn:aws-cn:iam::aws:policy/AmazonRedshiftFullAccess",
-          "arn:looks:like::an/arn",
-          "arn:aws:iam::aws:policy/AmazonS3FullAccess",
-          "arn:aws:iam::aws:policy/service-role/AmazonAPIGatewayPushToCloudWatchLogs"
+          "AnyNonOfficialManagedPolicy",
+          "arn:aws:iam::aws:policy/AmazonS3FullAccess"
         ],
         "Tags": [
           {

diff --git a/tests/translator/test_translator.py b/tests/translator/test_translator.py
@@ -175,21 +175,6 @@ def _compare_transform(self, manifest, expected, partition, region):
                     "AWSXRayDaemonWriteAccess"
                 ] = f"arn:{partition}:iam::aws:policy/AWSXRayDaemonWriteAccess"
 
-            # For the managed_policies_minimal.yaml transform test.
-            # For aws and aws-cn, these policies are cached (see https://github.com/aws/serverless-application-model/pull/2839),
-            # however we don't bundle managed policies in aws-us-gov, so instead we simulate passing the
-            # fallback policy loader
-            if partition == "aws-us-gov":
-                mock_policy_loader.load.return_value[
-                    "AmazonS3FullAccess"
-                ] = "arn:aws-us-gov:iam::aws:policy/AmazonS3FullAccess-mock-from-fallback-policy-loader"
-                mock_policy_loader.load.return_value[
-                    "AWSXrayWriteOnlyAccess"
-                ] = "arn:aws-us-gov:iam::aws:policy/AWSXrayWriteOnlyAccess-mock-from-fallback-policy-loader"
-                mock_policy_loader.load.return_value[
-                    "AmazonAPIGatewayPushToCloudWatchLogs"
-                ] = "arn:aws-us-gov:iam::aws:policy/service-role/AmazonAPIGatewayPushToCloudWatchLogs-mock-from-fallback-policy-loader"
-
             output_fragment = transform(manifest, parameter_values, mock_policy_loader)
 
         print(json.dumps(output_fragment, indent=2))
@@ -500,7 +485,6 @@ def test_transform_invalid_document(testcase):
     expected = json.load(open(os.path.join(OUTPUT_FOLDER, testcase + ".json")))
 
     mock_policy_loader = MagicMock()
-    mock_policy_loader.load.return_value = None
     parameter_values = get_template_parameter_values()
 
     with pytest.raises(InvalidDocumentException) as e:
@@ -775,6 +759,7 @@ class TestTemplateValidation(TestCase):
                     "InlineCode": "bar",
                     "Policies": [
                         "foo",
+                        "bar",
                     ],
                 },
             },
@@ -784,6 +769,7 @@ class TestTemplateValidation(TestCase):
                     "DefinitionUri": "s3://foo/bar",
                     "Policies": [
                         "foo",
+                        "bar",
                     ],
                 },
             },
@@ -800,6 +786,7 @@ class TestTemplateValidation(TestCase):
             (None, None, {"foo": "a3"}, "a3"),
             (None, {"foo": "a2"}, None, "a2"),
             ({"foo": "a1"}, None, None, "a1"),
+            (None, None, None, "foo"),
         ]
     )
     @patch("boto3.session.Session.region_name", "ap-southeast-1")
@@ -840,35 +827,6 @@ def get_managed_policy_map():
         self.assertEqual(function_arn, expected_arn)
         self.assertEqual(sfn_arn, expected_arn)
 
-    @parameterized.expand(
-        [
-            (None, None, None, "foo"),
-        ]
-    )
-    @patch("boto3.session.Session.region_name", "ap-southeast-1")
-    @patch("botocore.client.ClientEndpointBridge._check_default_region", mock_get_region)
-    def test_managed_policies_translator_translate_no_match(
-        self,
-        managed_policy_map,
-        bundled_managed_policy_map,
-        get_managed_policy_map_value,
-        expected_arn,
-    ):
-        def get_managed_policy_map():
-            return get_managed_policy_map_value
-
-        with patch(
-            "samtranslator.internal.managed_policies._BUNDLED_MANAGED_POLICIES",
-            {"aws": bundled_managed_policy_map},
-        ):
-            parameters = {}
-            with self.assertRaises(InvalidDocumentException):
-                Translator(managed_policy_map, Parser()).translate(
-                    self._MANAGED_POLICIES_TEMPLATE,
-                    parameters,
-                    get_managed_policy_map=get_managed_policy_map,
-                )
-
     # test to make sure with arn it doesnt load, with non-arn it does
     @parameterized.expand(
         [
@@ -892,13 +850,7 @@ def __init__(self):
 
             def load(self):
                 self.call_count += 1
-                return {
-                    "": "arn:",
-                    "SomeNonArnThing": "arn:SomeNonArnThing",
-                    "AnotherNonArnThing": "arn:AnotherNonArnThing",
-                    "aws:looks:like:an:ARN:but-not-really": "arn:aws:looks:like:an:ARN:but-not-really",
-                    "Mixing_things_v2": "arn:Mixing_things_v2",
-                }
+                return {}
 
         managed_policy_loader = ManagedPolicyLoader()
 
@@ -936,6 +888,7 @@ def load(self):
             ({"foo": "a1"}, {"foo": "a2"}, "a2"),
             ({"foo": "a1"}, None, "a1"),
             (None, {"foo": "a2"}, "a2"),
+            (None, None, "foo"),
         ]
     )
     @patch("boto3.session.Session.region_name", "ap-southeast-1")
@@ -972,35 +925,6 @@ def load(self):
         self.assertEqual(function_arn, expected_arn)
         self.assertEqual(sfn_arn, expected_arn)
 
-    @parameterized.expand(
-        [
-            (None, None, "foo"),
-        ]
-    )
-    @patch("boto3.session.Session.region_name", "ap-southeast-1")
-    @patch("botocore.client.ClientEndpointBridge._check_default_region", mock_get_region)
-    def test_managed_policies_transform_no_match(
-        self,
-        managed_policy_map,
-        bundled_managed_policy_map,
-        expected_arn,
-    ):
-        class ManagedPolicyLoader:
-            def load(self):
-                return managed_policy_map
-
-        with patch(
-            "samtranslator.internal.managed_policies._BUNDLED_MANAGED_POLICIES",
-            {"aws": bundled_managed_policy_map},
-        ):
-            parameters = {}
-            with self.assertRaises(InvalidDocumentException):
-                transform(
-                    self._MANAGED_POLICIES_TEMPLATE,
-                    parameters,
-                    ManagedPolicyLoader(),
-                )
-
     @patch("boto3.session.Session.region_name", "ap-southeast-1")
     @patch("botocore.client.ClientEndpointBridge._check_default_region", mock_get_region)
     def test_managed_policies_transform_policies_loaded_once(self):
@@ -1014,12 +938,7 @@ def __init__(self):
 
             def load(self):
                 self.call_count += 1
-                return {
-                    "foo": "arn:foo",
-                    "bar": "arn:bar",
-                    "egg": "arn:egg",
-                    "baz": "arn:baz",
-                }
+                return {}
 
         managed_policy_loader = ManagedPolicyLoader()