diff --git a/samtranslator/model/role_utils/role_constructor.py b/samtranslator/model/role_utils/role_constructor.py index 6b558d477..be53f1b71 100644 --- 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], -) -> str: +) -> Optional[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 name + return None def construct_role_for_resource( # type: ignore[no-untyped-def] # noqa: too-many-arguments @@ -146,6 +146,11 @@ 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 new file mode 100644 index 000000000..198dde59f --- /dev/null +++ b/tests/translator/input/error_managed_policies_function.yaml @@ -0,0 +1,9 @@ +Resources: + MyFunction: + Type: AWS::Serverless::Function + Properties: + Runtime: python3.8 + Handler: foo + InlineCode: bar + Policies: + - AnyNonOfficialManagedPolicy diff --git a/tests/translator/input/error_managed_policies_state_machine.yaml b/tests/translator/input/error_managed_policies_state_machine.yaml new file mode 100644 index 000000000..01806f3b6 --- /dev/null +++ b/tests/translator/input/error_managed_policies_state_machine.yaml @@ -0,0 +1,7 @@ +Resources: + MyStateMachine: + Type: AWS::Serverless::StateMachine + Properties: + DefinitionUri: s3://foo/bar + Policies: + - AnyNonOfficialManagedPolicy diff --git a/tests/translator/input/managed_policies_minimal.yaml b/tests/translator/input/managed_policies_minimal.yaml index 8f82c6cc2..d8174fff7 100644 --- a/tests/translator/input/managed_policies_minimal.yaml +++ b/tests/translator/input/managed_policies_minimal.yaml @@ -6,13 +6,19 @@ Resources: Handler: foo InlineCode: bar Policies: - - AnyNonOfficialManagedPolicy + - arn:aws:iam::aws:policy/AmazonSQSFullAccess + - arn:aws-cn:iam::aws:policy/AmazonRedshiftFullAccess + - arn:looks:like::an/arn - AmazonS3FullAccess + - AmazonAPIGatewayPushToCloudWatchLogs MyStateMachine: Type: AWS::Serverless::StateMachine Properties: DefinitionUri: s3://foo/bar Policies: - - AnyNonOfficialManagedPolicy + - arn:aws:iam::aws:policy/AmazonSQSFullAccess + - arn:aws-cn:iam::aws:policy/AmazonRedshiftFullAccess + - arn:looks:like::an/arn - AmazonS3FullAccess + - AmazonAPIGatewayPushToCloudWatchLogs diff --git a/tests/translator/output/aws-cn/managed_policies_minimal.json b/tests/translator/output/aws-cn/managed_policies_minimal.json index 79c1a514b..8a0086d2b 100644 --- a/tests/translator/output/aws-cn/managed_policies_minimal.json +++ b/tests/translator/output/aws-cn/managed_policies_minimal.json @@ -42,8 +42,11 @@ }, "ManagedPolicyArns": [ "arn:aws-cn:iam::aws:policy/service-role/AWSLambdaBasicExecutionRole", - "AnyNonOfficialManagedPolicy", - "arn:aws-cn:iam::aws:policy/AmazonS3FullAccess" + "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" ], "Tags": [ { @@ -94,8 +97,11 @@ "Version": "2012-10-17" }, "ManagedPolicyArns": [ - "AnyNonOfficialManagedPolicy", - "arn:aws-cn:iam::aws:policy/AmazonS3FullAccess" + "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" ], "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 index e68361f12..56674433c 100644 --- a/tests/translator/output/aws-us-gov/managed_policies_minimal.json +++ b/tests/translator/output/aws-us-gov/managed_policies_minimal.json @@ -42,8 +42,11 @@ }, "ManagedPolicyArns": [ "arn:aws-us-gov:iam::aws:policy/service-role/AWSLambdaBasicExecutionRole", - "AnyNonOfficialManagedPolicy", - "AmazonS3FullAccess" + "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" ], "Tags": [ { @@ -94,8 +97,11 @@ "Version": "2012-10-17" }, "ManagedPolicyArns": [ - "AnyNonOfficialManagedPolicy", - "AmazonS3FullAccess" + "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" ], "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 index bdfeb13ad..5eb366239 100644 --- 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": [ - "AWSXrayWriteOnlyAccess", + "arn:aws-us-gov:iam::aws:policy/AWSXrayWriteOnlyAccess-mock-from-fallback-policy-loader", "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": [ - "AWSXrayWriteOnlyAccess", + "arn:aws-us-gov:iam::aws:policy/AWSXrayWriteOnlyAccess-mock-from-fallback-policy-loader", "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 new file mode 100644 index 000000000..5c073c23a --- /dev/null +++ b/tests/translator/output/error_managed_policies_function.json @@ -0,0 +1,9 @@ +{ + "_autoGeneratedBreakdownErrorMessage": [ + "Invalid Serverless Application Specification document. ", + "Number of errors found: 1. ", + "Resource with id [MyFunction] is invalid. ", + "Invalid policy 'AnyNonOfficialManagedPolicy'; make sure the value is a valid AWS managed policy ARN." + ], + "errorMessage": "Invalid Serverless Application Specification document. Number of errors found: 1. Resource with id [MyFunction] is invalid. Invalid policy 'AnyNonOfficialManagedPolicy'; make sure the value is a valid AWS managed policy ARN." +} diff --git a/tests/translator/output/error_managed_policies_state_machine.json b/tests/translator/output/error_managed_policies_state_machine.json new file mode 100644 index 000000000..c76661318 --- /dev/null +++ b/tests/translator/output/error_managed_policies_state_machine.json @@ -0,0 +1,9 @@ +{ + "_autoGeneratedBreakdownErrorMessage": [ + "Invalid Serverless Application Specification document. ", + "Number of errors found: 1. ", + "Resource with id [MyStateMachine] is invalid. ", + "Invalid policy 'AnyNonOfficialManagedPolicy'; make sure the value is a valid AWS managed policy ARN." + ], + "errorMessage": "Invalid Serverless Application Specification document. Number of errors found: 1. Resource with id [MyStateMachine] is invalid. Invalid policy 'AnyNonOfficialManagedPolicy'; make sure the value is a valid AWS managed policy ARN." +} diff --git a/tests/translator/output/managed_policies_minimal.json b/tests/translator/output/managed_policies_minimal.json index 87a39ca3c..7d7d2ef3f 100644 --- a/tests/translator/output/managed_policies_minimal.json +++ b/tests/translator/output/managed_policies_minimal.json @@ -42,8 +42,11 @@ }, "ManagedPolicyArns": [ "arn:aws:iam::aws:policy/service-role/AWSLambdaBasicExecutionRole", - "AnyNonOfficialManagedPolicy", - "arn:aws:iam::aws:policy/AmazonS3FullAccess" + "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" ], "Tags": [ { @@ -94,8 +97,11 @@ "Version": "2012-10-17" }, "ManagedPolicyArns": [ - "AnyNonOfficialManagedPolicy", - "arn:aws:iam::aws:policy/AmazonS3FullAccess" + "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" ], "Tags": [ { diff --git a/tests/translator/test_translator.py b/tests/translator/test_translator.py index 8bbf06fab..ab2457677 100644 --- a/tests/translator/test_translator.py +++ b/tests/translator/test_translator.py @@ -175,6 +175,21 @@ 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)) @@ -485,6 +500,7 @@ 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: @@ -759,7 +775,6 @@ class TestTemplateValidation(TestCase): "InlineCode": "bar", "Policies": [ "foo", - "bar", ], }, }, @@ -769,7 +784,6 @@ class TestTemplateValidation(TestCase): "DefinitionUri": "s3://foo/bar", "Policies": [ "foo", - "bar", ], }, }, @@ -786,7 +800,6 @@ 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") @@ -827,6 +840,35 @@ 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( [ @@ -850,7 +892,13 @@ def __init__(self): def load(self): self.call_count += 1 - return {} + 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", + } managed_policy_loader = ManagedPolicyLoader() @@ -888,7 +936,6 @@ 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") @@ -925,6 +972,35 @@ 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): @@ -938,7 +1014,12 @@ def __init__(self): def load(self): self.call_count += 1 - return {} + return { + "foo": "arn:foo", + "bar": "arn:bar", + "egg": "arn:egg", + "baz": "arn:baz", + } managed_policy_loader = ManagedPolicyLoader()