Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .github/PULL_REQUEST_TEMPLATE.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
- [ ] Write/update tests
- [ ] `make pr` passes
- [ ] Update documentation
- [ ] Verify transformed template deploys and application functions as expected
- [ ] Add/update example to `examples/2016-10-31`


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
45 changes: 42 additions & 3 deletions samtranslator/model/function_policies.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@

from six import string_types

from samtranslator.model.intrinsics import is_instrinsic
from samtranslator.model.intrinsics import is_instrinsic, is_intrinsic_if, is_intrinsic_no_value
from samtranslator.model.exceptions import InvalidTemplateException

PolicyEntry = namedtuple("PolicyEntry", "data type")

Expand Down Expand Up @@ -114,8 +115,16 @@ def _get_type(self, policy):

# Must handle intrinsic functions. Policy could be a primitive type or an intrinsic function

# Managed policies are either string or an intrinsic function that resolves to a string
if isinstance(policy, string_types) or is_instrinsic(policy):
# Managed policies are of type string
if isinstance(policy, string_types):
return PolicyTypes.MANAGED_POLICY

# Handle the special case for 'if' intrinsic function
if is_intrinsic_if(policy):
return self._get_type_from_intrinsic_if(policy)

# Intrinsic functions are treated as managed policies by default
if is_instrinsic(policy):
return PolicyTypes.MANAGED_POLICY

# Policy statement is a dictionary with the key "Statement" in it
Expand Down Expand Up @@ -143,6 +152,36 @@ def _is_policy_template(self, policy):
len(policy) == 1 and \
self._policy_template_processor.has(list(policy.keys())[0]) is True

def _get_type_from_intrinsic_if(self, policy):
"""
Returns the type of the given policy assuming that it is an intrinsic if function

:param policy: Input value to get type from
:return: PolicyTypes: Type of the given policy. PolicyTypes.UNKNOWN, if type could not be inferred
"""
intrinsic_if_value = policy["Fn::If"]

if not len(intrinsic_if_value) == 3:
raise InvalidTemplateException("Fn::If requires 3 arguments")

if_data = intrinsic_if_value[1]
else_data = intrinsic_if_value[2]

if_data_type = self._get_type(if_data)
else_data_type = self._get_type(else_data)

if if_data_type == else_data_type:
return if_data_type

if is_intrinsic_no_value(if_data):
return else_data_type

if is_intrinsic_no_value(else_data):
return if_data_type

raise InvalidTemplateException("Different policy types within the same Fn::If statement is unsupported. "
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a great limitation, makes this feature easier and cuts down on edge cases. Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great! Can you merge it in? Looks like its still blocked for me on some "code changes requested".

"Separate different policy types into different Fn::If statements")


class PolicyTypes(Enum):
"""
Expand Down
32 changes: 32 additions & 0 deletions samtranslator/model/intrinsics.py
Original file line number Diff line number Diff line change
Expand Up @@ -138,3 +138,35 @@ def is_instrinsic(input):
return key == "Ref" or key == "Condition" or key.startswith("Fn::")

return False


def is_intrinsic_if(input):
"""
Is the given input an intrinsic if? Intrinsic function 'if' is a dictionary with single
key - if

:param input: Input value to check if it is an intrinsic if
:return: True, if yes
"""

if not is_instrinsic(input):
return False

key = list(input.keys())[0]
return key == "Fn::If"


def is_intrinsic_no_value(input):
"""
Is the given input an intrinsic Ref: AWS::NoValue? Intrinsic function is a dictionary with single
key - Ref and value - AWS::NoValue

:param input: Input value to check if it is an intrinsic if
:return: True, if yes
"""

if not is_instrinsic(input):
return False

key = list(input.keys())[0]
return key == "Ref" and input["Ref"] == "AWS::NoValue"
33 changes: 29 additions & 4 deletions samtranslator/model/sam_resources.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
from samtranslator.model.types import dict_of, is_str, is_type, list_of, one_of, any_type
from samtranslator.translator import logical_id_generator
from samtranslator.translator.arn_generator import ArnGenerator
from samtranslator.model.intrinsics import is_intrinsic_if, is_intrinsic_no_value


class SamFunction(SamResourceMacro):
Expand Down Expand Up @@ -212,10 +213,34 @@ def _construct_role(self, managed_policy_map):

if policy_entry.type is PolicyTypes.POLICY_STATEMENT:

policy_documents.append({
'PolicyName': execution_role.logical_id + 'Policy' + str(index),
'PolicyDocument': policy_entry.data
})
if is_intrinsic_if(policy_entry.data):

intrinsic_if = policy_entry.data
then_statement = intrinsic_if["Fn::If"][1]
else_statement = intrinsic_if["Fn::If"][2]

if not is_intrinsic_no_value(then_statement):
then_statement = {
'PolicyName': execution_role.logical_id + 'Policy' + str(index),
'PolicyDocument': then_statement
}
intrinsic_if["Fn::If"][1] = then_statement

if not is_intrinsic_no_value(else_statement):
else_statement = {
'PolicyName': execution_role.logical_id + 'Policy' + str(index),
'PolicyDocument': else_statement
}
intrinsic_if["Fn::If"][2] = else_statement

policy_documents.append(intrinsic_if)

else:
policy_documents.append({
'PolicyName': execution_role.logical_id + 'Policy' + str(index),
'PolicyDocument': policy_entry.data
})

elif policy_entry.type is PolicyTypes.MANAGED_POLICY:

# There are three options:
Expand Down
67 changes: 50 additions & 17 deletions samtranslator/plugins/policies/policy_templates_plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
from samtranslator.model.function_policies import FunctionPolicies, PolicyTypes
from samtranslator.model.exceptions import InvalidResourceException
from samtranslator.policy_template_processor.exceptions import InsufficientParameterValues, InvalidParameterValues
from samtranslator.model.intrinsics import is_intrinsic_if, is_intrinsic_no_value


class PolicyTemplatesForFunctionPlugin(BasePlugin):
Expand Down Expand Up @@ -55,28 +56,60 @@ def on_before_transform_resource(self, logical_id, resource_type, resource_prope
result.append(policy_entry.data)
continue

# We are processing policy templates. We know they have a particular structure:
# {"templateName": { parameter_values_dict }}
template_data = policy_entry.data
template_name = list(template_data.keys())[0]
template_parameters = list(template_data.values())[0]

try:

# 'convert' will return a list of policy statements
result.append(self._policy_template_processor.convert(template_name, template_parameters))
if is_intrinsic_if(policy_entry.data):
# If policy is an intrinsic if, we need to process each sub-statement separately
processed_intrinsic_if = self._process_intrinsic_if_policy_template(logical_id, policy_entry)
result.append(processed_intrinsic_if)
continue

except InsufficientParameterValues as ex:
# Exception's message will give lot of specific details
raise InvalidResourceException(logical_id, str(ex))
except InvalidParameterValues:
raise InvalidResourceException(logical_id,
"Must specify valid parameter values for policy template '{}'"
.format(template_name))
converted_policy = self._process_policy_template(logical_id, policy_entry.data)
result.append(converted_policy)

# Save the modified policies list to the input
resource_properties[FunctionPolicies.POLICIES_PROPERTY_NAME] = result

def _process_intrinsic_if_policy_template(self, logical_id, policy_entry):
intrinsic_if = policy_entry.data
then_statement = intrinsic_if["Fn::If"][1]
else_statement = intrinsic_if["Fn::If"][2]

processed_then_statement = then_statement \
if is_intrinsic_no_value(then_statement) \
else self._process_policy_template(logical_id, then_statement)

processed_else_statement = else_statement \
if is_intrinsic_no_value(else_statement) \
else self._process_policy_template(logical_id, else_statement)

processed_intrinsic_if = {
"Fn::If": [
policy_entry.data["Fn::If"][0],
processed_then_statement,
processed_else_statement
]
}

return processed_intrinsic_if

def _process_policy_template(self, logical_id, template_data):

# We are processing policy templates. We know they have a particular structure:
# {"templateName": { parameter_values_dict }}
template_name = list(template_data.keys())[0]
template_parameters = list(template_data.values())[0]
try:

# 'convert' will return a list of policy statements
return self._policy_template_processor.convert(template_name, template_parameters)

except InsufficientParameterValues as ex:
# Exception's message will give lot of specific details
raise InvalidResourceException(logical_id, str(ex))
except InvalidParameterValues:
raise InvalidResourceException(logical_id,
"Must specify valid parameter values for policy template '{}'"
.format(template_name))

def _is_supported(self, resource_type):
"""
Is this resource supported by this plugin?
Expand Down
137 changes: 137 additions & 0 deletions tests/model/test_function_policies.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
from unittest import TestCase

from samtranslator.model.function_policies import FunctionPolicies, PolicyTypes, PolicyEntry
from samtranslator.model.exceptions import InvalidTemplateException
from samtranslator.model.intrinsics import is_intrinsic_if, is_intrinsic_no_value

class TestFunctionPolicies(TestCase):

Expand Down Expand Up @@ -333,3 +335,138 @@ def test_is_policy_template_must_return_false_without_the_processor(self):

self.assertFalse(function_policies_obj._is_policy_template(policy))
self.policy_template_processor_mock.has.assert_not_called()

def test_is_intrinsic_if_must_return_true_for_if(self):
policy = {
"Fn::If": "some value"
}

self.assertTrue(is_intrinsic_if(policy))

def test_is_intrinsic_if_must_return_false_for_others(self):
too_many_keys = {
"Fn::If": "some value",
"Fn::And": "other value"
}

not_if = {
"Fn::Or": "some value"
}

self.assertFalse(is_intrinsic_if(too_many_keys))
self.assertFalse(is_intrinsic_if(not_if))
self.assertFalse(is_intrinsic_if(None))

def test_is_intrinsic_no_value_must_return_true_for_no_value(self):
policy = {
"Ref": "AWS::NoValue"
}

self.assertTrue(is_intrinsic_no_value(policy))

def test_is_intrinsic_no_value_must_return_false_for_other_value(self):
bad_key = {
"sRefs": "AWS::NoValue"
}

bad_value = {
"Ref": "SWA::NoValue"
}

too_many_keys = {
"Ref": "AWS::NoValue",
"feR": "SWA::NoValue"
}

self.assertFalse(is_intrinsic_no_value(bad_key))
self.assertFalse(is_intrinsic_no_value(bad_value))
self.assertFalse(is_intrinsic_no_value(None))
self.assertFalse(is_intrinsic_no_value(too_many_keys))

def test_get_type_with_intrinsic_if_must_return_managed_policy_type(self):
managed_policy = {
"Fn::If": ["SomeCondition", "some managed policy arn", "other managed policy arn"]
}

no_value_if = {
"Fn::If": ["SomeCondition", {"Ref": "AWS::NoValue"}, "other managed policy arn"]
}

no_value_else = {
"Fn::If": ["SomeCondition", "other managed policy arn", {"Ref": "AWS::NoValue"}]
}

expected_managed_policy = PolicyTypes.MANAGED_POLICY

self.assertTrue(expected_managed_policy, self.function_policies._get_type(managed_policy))
self.assertTrue(expected_managed_policy, self.function_policies._get_type(no_value_if))
self.assertTrue(expected_managed_policy, self.function_policies._get_type(no_value_else))

def test_get_type_with_intrinsic_if_must_return_policy_statement_type(self):
policy_statement = {
"Fn::If": ["SomeCondition", {"Statement": "then statement"}, {"Statement": "else statement"}]
}

no_value_if = {
"Fn::If": ["SomeCondition", {"Ref": "AWS::NoValue"}, {"Statement": "else statement"}]
}

no_value_else = {
"Fn::If": ["SomeCondition", {"Statement": "then statement"}, {"Ref": "AWS::NoValue"}]
}
expected_managed_policy = PolicyTypes.POLICY_STATEMENT

self.assertTrue(expected_managed_policy, self.function_policies._get_type(policy_statement))
self.assertTrue(expected_managed_policy, self.function_policies._get_type(no_value_if))
self.assertTrue(expected_managed_policy, self.function_policies._get_type(no_value_else))

def test_get_type_with_intrinsic_if_must_return_policy_template_type(self):
policy_template = {
"Fn::If": [ "SomeCondition",
{"template_name_one": { "Param1": "foo"}},
{"template_name_one": { "Param1": "foo"}}
]
}
no_value_if = {
"Fn::If": [ "SomeCondition",
{"Ref": "AWS::NoValue"},
{"template_name_one": { "Param1": "foo"}}
]
}
no_value_else = {
"Fn::If": [ "SomeCondition",
{"template_name_one": { "Param1": "foo"}},
{"Ref": "AWS::NoValue"}
]
}

expected_managed_policy = PolicyTypes.POLICY_TEMPLATE
self.policy_template_processor_mock.has.return_value = True
function_policies = FunctionPolicies({}, self.policy_template_processor_mock)

self.assertTrue(expected_managed_policy, function_policies._get_type(policy_template))
self.assertTrue(expected_managed_policy, function_policies._get_type(no_value_if))
self.assertTrue(expected_managed_policy, function_policies._get_type(no_value_else))

def test_get_type_with_intrinsic_if_must_raise_exception_for_bad_policy(self):
policy_too_few_values = {
"Fn::If": ["condition", "then"]
}

policy_too_many_values = {
"Fn::If": ["condition", "then", "else", "extra"]
}

self.assertRaises(InvalidTemplateException, self.function_policies._get_type, policy_too_few_values)
self.assertRaises(InvalidTemplateException, self.function_policies._get_type, policy_too_many_values)

def test_get_type_with_intrinsic_if_must_raise_exception_for_different_policy_types(self):
policy_one = {
"Fn::If": ["condition", "then", {"Statement": "else"}]
}
policy_two = {
"Fn::If": ["condition", {"Statement": "then"}, "else"]
}

self.assertRaises(InvalidTemplateException, self.function_policies._get_type, policy_one)
self.assertRaises(InvalidTemplateException, self.function_policies._get_type, policy_two)
Loading