diff --git a/samtranslator/intrinsics/resolver.py b/samtranslator/intrinsics/resolver.py index 8e2b77c043..d0dfe69b12 100644 --- a/samtranslator/intrinsics/resolver.py +++ b/samtranslator/intrinsics/resolver.py @@ -93,12 +93,12 @@ def resolve_sam_resource_id_refs(self, input, supported_resource_id_refs): # ty """ return self._traverse(input, supported_resource_id_refs, self._try_resolve_sam_resource_id_refs) # type: ignore[no-untyped-call] - def _traverse(self, input, resolution_data, resolver_method): # type: ignore[no-untyped-def] + def _traverse(self, input_value, resolution_data, resolver_method): # type: ignore[no-untyped-def] """ Driver method that performs the actual traversal of input and calls the appropriate `resolver_method` when to perform the resolution. - :param input: Any primitive type (dict, array, string etc) whose value might contain an intrinsic function + :param input_value: Any primitive type (dict, array, string etc) whose value might contain an intrinsic function :param resolution_data: Data that will help with resolution. For example, when resolving parameter references, this object will contain a dictionary of parameter names and their values. :param resolver_method: Method that will be called to actually resolve an intrinsic function. This method @@ -108,7 +108,7 @@ def _traverse(self, input, resolution_data, resolver_method): # type: ignore[no # There is data to help with resolution. Skip the traversal altogether if len(resolution_data) == 0: - return input + return input_value # # Traversal Algorithm: @@ -126,15 +126,14 @@ def _traverse(self, input, resolution_data, resolver_method): # type: ignore[no # to handle nested intrinsics. All of these cases lend well towards a Pre-Order traversal where we try and # process the intrinsic, which results in a modified sub-tree to traverse. # - - input = resolver_method(input, resolution_data) - - if isinstance(input, dict): - return self._traverse_dict(input, resolution_data, resolver_method) # type: ignore[no-untyped-call] - if isinstance(input, list): - return self._traverse_list(input, resolution_data, resolver_method) # type: ignore[no-untyped-call] + input_value = resolver_method(input_value, resolution_data) + if isinstance(input_value, dict): + return self._traverse_dict(input_value, resolution_data, resolver_method) # type: ignore[no-untyped-call] + if isinstance(input_value, list): + return self._traverse_list(input_value, resolution_data, resolver_method) # type: ignore[no-untyped-call] # We can iterate only over dict or list types. Primitive types are terminals - return input + + return input_value def _traverse_dict(self, input_dict, resolution_data, resolver_method): # type: ignore[no-untyped-def] """ diff --git a/samtranslator/policy_template_processor/template.py b/samtranslator/policy_template_processor/template.py index d6a02eb311..35386367bf 100644 --- a/samtranslator/policy_template_processor/template.py +++ b/samtranslator/policy_template_processor/template.py @@ -1,10 +1,13 @@ -import copy +from typing import Any from samtranslator.intrinsics.resolver import IntrinsicsResolver from samtranslator.intrinsics.actions import RefAction from samtranslator.policy_template_processor.exceptions import InsufficientParameterValues, InvalidParameterValues +POLICY_PARAMETER_DISAMBIGUATE_PREFIX = "___SAM_POLICY_PARAMETER_" + + class Template(object): """ Class representing a single policy template. It includes the name, parameters and template dictionary. @@ -50,17 +53,53 @@ def to_statement(self, parameter_values): # type: ignore[no-untyped-def] # injection of values for parameters not intended in the template. This is important because "Ref" resolution # will substitute any references for which a value is provided. necessary_parameter_values = { - name: value for name, value in parameter_values.items() if name in self.parameters + POLICY_PARAMETER_DISAMBIGUATE_PREFIX + name: value + for name, value in parameter_values.items() + if name in self.parameters } # Only "Ref" is supported supported_intrinsics = {RefAction.intrinsic_name: RefAction()} resolver = IntrinsicsResolver(necessary_parameter_values, supported_intrinsics) # type: ignore[no-untyped-call] - definition_copy = copy.deepcopy(self.definition) + definition_copy = self._disambiguate_policy_parameter(self.definition) return resolver.resolve_parameter_refs(definition_copy) + @staticmethod + def _disambiguate_policy_parameter(policy_definition: Any) -> Any: + """ + Return a deepcopy of policy definition where all parameters are + renamed to avoid naming collision of normal CFN parameters. + This is to avoid IntrinsicResolver.resolve_parameter_refs() + will make infinitely recursion on this: + ``` + - DynamoDBCrudPolicy: + TableName: <- this is the policy parameter + Fn::ImportValue: + Fn::Join: + - '-' + - - Ref: TableName <- this is the CFN parameter + - hello + - Ref: EnvironmentType + ``` + Once IntrinsicResolver.resolve_parameter_refs() replace the "Ref: TableName" + with "TableName: .... Ref: TableName - hello ---" + There are "Ref: TableName" in it again (indefinitely). + """ + + def _traverse(node: Any) -> Any: + if isinstance(node, dict): + copy = {key: _traverse(value) for key, value in node.items()} + if "Ref" in copy and isinstance(copy["Ref"], str): + copy["Ref"] = POLICY_PARAMETER_DISAMBIGUATE_PREFIX + copy["Ref"] + return copy + if isinstance(node, list): + return [_traverse(item) for item in node] + return node + + return _traverse(policy_definition) + def missing_parameter_values(self, parameter_values): # type: ignore[no-untyped-def] """ Checks if the given input contains values for all parameters used by this template diff --git a/tests/policy_template_processor/test_template.py b/tests/policy_template_processor/test_template.py index 03b5858cc8..7885af5b42 100644 --- a/tests/policy_template_processor/test_template.py +++ b/tests/policy_template_processor/test_template.py @@ -128,7 +128,7 @@ def test_to_statement_must_work_with_valid_inputs(self, intrinsics_resolver_mock result = template.to_statement(parameter_values) self.assertEqual(expected, result) - intrinsics_resolver_mock.assert_called_once_with(parameter_values, {"Ref": ANY}) + intrinsics_resolver_mock.assert_called_once_with({"___SAM_POLICY_PARAMETER_param1": "b"}, {"Ref": ANY}) resolver_instance_mock.resolve_parameter_refs.assert_called_once_with({"Statement": {"key": "value"}}) @patch("samtranslator.policy_template_processor.template.IntrinsicsResolver") @@ -145,7 +145,7 @@ def test_to_statement_must_exclude_extra_parameter_values(self, intrinsics_resol template.to_statement(parameter_values) # Intrinsics resolver must be called only with the parameters declared in the template - expected_parameter_values = {"param1": "b"} + expected_parameter_values = {"___SAM_POLICY_PARAMETER_param1": "b"} intrinsics_resolver_mock.assert_called_once_with(expected_parameter_values, ANY) @patch("samtranslator.policy_template_processor.template.IntrinsicsResolver") diff --git a/tests/translator/input/policy_template_import_vaule_refs_parameters_with_same_name_as_policy_parameter.yaml b/tests/translator/input/policy_template_import_vaule_refs_parameters_with_same_name_as_policy_parameter.yaml new file mode 100644 index 0000000000..90352c1d37 --- /dev/null +++ b/tests/translator/input/policy_template_import_vaule_refs_parameters_with_same_name_as_policy_parameter.yaml @@ -0,0 +1,19 @@ +Parameters: + TableName: + Type: String + +Resources: + MapFunction: + Type: AWS::Serverless::Function + Properties: + Handler: index.handler + Runtime: nodejs16.x + CodeUri: s3://bucket/key + Policies: + - DynamoDBCrudPolicy: + TableName: + Fn::ImportValue: + Fn::Join: + - '-' + - - Ref: TableName # this is the same as DynamoDBCrudPolicy's parameter name + - hello diff --git a/tests/translator/output/aws-cn/policy_template_import_vaule_refs_parameters_with_same_name_as_policy_parameter.json b/tests/translator/output/aws-cn/policy_template_import_vaule_refs_parameters_with_same_name_as_policy_parameter.json new file mode 100644 index 0000000000..e8bcd4b0ce --- /dev/null +++ b/tests/translator/output/aws-cn/policy_template_import_vaule_refs_parameters_with_same_name_as_policy_parameter.json @@ -0,0 +1,128 @@ +{ + "Parameters": { + "TableName": { + "Type": "String" + } + }, + "Resources": { + "MapFunction": { + "Properties": { + "Code": { + "S3Bucket": "bucket", + "S3Key": "key" + }, + "Handler": "index.handler", + "Role": { + "Fn::GetAtt": [ + "MapFunctionRole", + "Arn" + ] + }, + "Runtime": "nodejs16.x", + "Tags": [ + { + "Key": "lambda:createdBy", + "Value": "SAM" + } + ] + }, + "Type": "AWS::Lambda::Function" + }, + "MapFunctionRole": { + "Properties": { + "AssumeRolePolicyDocument": { + "Statement": [ + { + "Action": [ + "sts:AssumeRole" + ], + "Effect": "Allow", + "Principal": { + "Service": [ + "lambda.amazonaws.com" + ] + } + } + ], + "Version": "2012-10-17" + }, + "ManagedPolicyArns": [ + "arn:aws-cn:iam::aws:policy/service-role/AWSLambdaBasicExecutionRole" + ], + "Policies": [ + { + "PolicyDocument": { + "Statement": [ + { + "Action": [ + "dynamodb:GetItem", + "dynamodb:DeleteItem", + "dynamodb:PutItem", + "dynamodb:Scan", + "dynamodb:Query", + "dynamodb:UpdateItem", + "dynamodb:BatchWriteItem", + "dynamodb:BatchGetItem", + "dynamodb:DescribeTable", + "dynamodb:ConditionCheckItem" + ], + "Effect": "Allow", + "Resource": [ + { + "Fn::Sub": [ + "arn:${AWS::Partition}:dynamodb:${AWS::Region}:${AWS::AccountId}:table/${tableName}", + { + "tableName": { + "Fn::ImportValue": { + "Fn::Join": [ + "-", + [ + { + "Ref": "TableName" + }, + "hello" + ] + ] + } + } + } + ] + }, + { + "Fn::Sub": [ + "arn:${AWS::Partition}:dynamodb:${AWS::Region}:${AWS::AccountId}:table/${tableName}/index/*", + { + "tableName": { + "Fn::ImportValue": { + "Fn::Join": [ + "-", + [ + { + "Ref": "TableName" + }, + "hello" + ] + ] + } + } + } + ] + } + ] + } + ] + }, + "PolicyName": "MapFunctionRolePolicy0" + } + ], + "Tags": [ + { + "Key": "lambda:createdBy", + "Value": "SAM" + } + ] + }, + "Type": "AWS::IAM::Role" + } + } +} diff --git a/tests/translator/output/aws-us-gov/policy_template_import_vaule_refs_parameters_with_same_name_as_policy_parameter.json b/tests/translator/output/aws-us-gov/policy_template_import_vaule_refs_parameters_with_same_name_as_policy_parameter.json new file mode 100644 index 0000000000..5c1fe3473c --- /dev/null +++ b/tests/translator/output/aws-us-gov/policy_template_import_vaule_refs_parameters_with_same_name_as_policy_parameter.json @@ -0,0 +1,128 @@ +{ + "Parameters": { + "TableName": { + "Type": "String" + } + }, + "Resources": { + "MapFunction": { + "Properties": { + "Code": { + "S3Bucket": "bucket", + "S3Key": "key" + }, + "Handler": "index.handler", + "Role": { + "Fn::GetAtt": [ + "MapFunctionRole", + "Arn" + ] + }, + "Runtime": "nodejs16.x", + "Tags": [ + { + "Key": "lambda:createdBy", + "Value": "SAM" + } + ] + }, + "Type": "AWS::Lambda::Function" + }, + "MapFunctionRole": { + "Properties": { + "AssumeRolePolicyDocument": { + "Statement": [ + { + "Action": [ + "sts:AssumeRole" + ], + "Effect": "Allow", + "Principal": { + "Service": [ + "lambda.amazonaws.com" + ] + } + } + ], + "Version": "2012-10-17" + }, + "ManagedPolicyArns": [ + "arn:aws-us-gov:iam::aws:policy/service-role/AWSLambdaBasicExecutionRole" + ], + "Policies": [ + { + "PolicyDocument": { + "Statement": [ + { + "Action": [ + "dynamodb:GetItem", + "dynamodb:DeleteItem", + "dynamodb:PutItem", + "dynamodb:Scan", + "dynamodb:Query", + "dynamodb:UpdateItem", + "dynamodb:BatchWriteItem", + "dynamodb:BatchGetItem", + "dynamodb:DescribeTable", + "dynamodb:ConditionCheckItem" + ], + "Effect": "Allow", + "Resource": [ + { + "Fn::Sub": [ + "arn:${AWS::Partition}:dynamodb:${AWS::Region}:${AWS::AccountId}:table/${tableName}", + { + "tableName": { + "Fn::ImportValue": { + "Fn::Join": [ + "-", + [ + { + "Ref": "TableName" + }, + "hello" + ] + ] + } + } + } + ] + }, + { + "Fn::Sub": [ + "arn:${AWS::Partition}:dynamodb:${AWS::Region}:${AWS::AccountId}:table/${tableName}/index/*", + { + "tableName": { + "Fn::ImportValue": { + "Fn::Join": [ + "-", + [ + { + "Ref": "TableName" + }, + "hello" + ] + ] + } + } + } + ] + } + ] + } + ] + }, + "PolicyName": "MapFunctionRolePolicy0" + } + ], + "Tags": [ + { + "Key": "lambda:createdBy", + "Value": "SAM" + } + ] + }, + "Type": "AWS::IAM::Role" + } + } +} diff --git a/tests/translator/output/policy_template_import_vaule_refs_parameters_with_same_name_as_policy_parameter.json b/tests/translator/output/policy_template_import_vaule_refs_parameters_with_same_name_as_policy_parameter.json new file mode 100644 index 0000000000..b293ab04df --- /dev/null +++ b/tests/translator/output/policy_template_import_vaule_refs_parameters_with_same_name_as_policy_parameter.json @@ -0,0 +1,128 @@ +{ + "Parameters": { + "TableName": { + "Type": "String" + } + }, + "Resources": { + "MapFunction": { + "Properties": { + "Code": { + "S3Bucket": "bucket", + "S3Key": "key" + }, + "Handler": "index.handler", + "Role": { + "Fn::GetAtt": [ + "MapFunctionRole", + "Arn" + ] + }, + "Runtime": "nodejs16.x", + "Tags": [ + { + "Key": "lambda:createdBy", + "Value": "SAM" + } + ] + }, + "Type": "AWS::Lambda::Function" + }, + "MapFunctionRole": { + "Properties": { + "AssumeRolePolicyDocument": { + "Statement": [ + { + "Action": [ + "sts:AssumeRole" + ], + "Effect": "Allow", + "Principal": { + "Service": [ + "lambda.amazonaws.com" + ] + } + } + ], + "Version": "2012-10-17" + }, + "ManagedPolicyArns": [ + "arn:aws:iam::aws:policy/service-role/AWSLambdaBasicExecutionRole" + ], + "Policies": [ + { + "PolicyDocument": { + "Statement": [ + { + "Action": [ + "dynamodb:GetItem", + "dynamodb:DeleteItem", + "dynamodb:PutItem", + "dynamodb:Scan", + "dynamodb:Query", + "dynamodb:UpdateItem", + "dynamodb:BatchWriteItem", + "dynamodb:BatchGetItem", + "dynamodb:DescribeTable", + "dynamodb:ConditionCheckItem" + ], + "Effect": "Allow", + "Resource": [ + { + "Fn::Sub": [ + "arn:${AWS::Partition}:dynamodb:${AWS::Region}:${AWS::AccountId}:table/${tableName}", + { + "tableName": { + "Fn::ImportValue": { + "Fn::Join": [ + "-", + [ + { + "Ref": "TableName" + }, + "hello" + ] + ] + } + } + } + ] + }, + { + "Fn::Sub": [ + "arn:${AWS::Partition}:dynamodb:${AWS::Region}:${AWS::AccountId}:table/${tableName}/index/*", + { + "tableName": { + "Fn::ImportValue": { + "Fn::Join": [ + "-", + [ + { + "Ref": "TableName" + }, + "hello" + ] + ] + } + } + } + ] + } + ] + } + ] + }, + "PolicyName": "MapFunctionRolePolicy0" + } + ], + "Tags": [ + { + "Key": "lambda:createdBy", + "Value": "SAM" + } + ] + }, + "Type": "AWS::IAM::Role" + } + } +}