From 4900614963ea36dfd59596dde38fb625deb15a83 Mon Sep 17 00:00:00 2001 From: Gavin Zhang Date: Fri, 14 Oct 2022 13:30:39 -0700 Subject: [PATCH 1/3] Enable more pylint rules and fixes pylint issues --- .pylintrc | 10 +- samtranslator/intrinsics/resolver.py | 2 +- samtranslator/model/__init__.py | 5 +- samtranslator/model/api/api_generator.py | 2 + samtranslator/model/apigatewayv2.py | 2 +- samtranslator/model/eventsources/push.py | 1 + samtranslator/model/function_policies.py | 205 -------------- samtranslator/plugins/globals/globals.py | 2 +- tests/model/test_function_policies.py | 332 ----------------------- 9 files changed, 13 insertions(+), 548 deletions(-) delete mode 100644 samtranslator/model/function_policies.py delete mode 100644 tests/model/test_function_policies.py diff --git a/.pylintrc b/.pylintrc index f0d5e0ccb2..23283cbb15 100644 --- a/.pylintrc +++ b/.pylintrc @@ -97,12 +97,12 @@ disable= # Constant name style warnings. We will remove them one by one. C0103, # Class constant name "%s" doesn't conform to UPPER_CASE naming style ('([^\\W\\da-z][^\\Wa-z]*|__.*__)$' pattern) (invalid-name) # New recommendations, disable for now to avoid introducing behaviour changes. - R1729, # Use a generator instead '%s' (use-a-generator) + # R1729, # Use a generator instead '%s' (use-a-generator) # R1732, # Consider using 'with' for resource-allocating operations (consider-using-with) # This applies to CPython only. # I1101, # Module 'math' has no 'pow' member, but source is unavailable. Consider adding this module to extension-pkg-allow-list if you want to perform analysis based on run-time introspection of living objects. (c-extension-no-member) # Added to allow linting. Need to work on removing over time - R0801, # dupilcated code + # R0801, # dupilcated code R0401, # Cyclic import C0411, # import ordering W9015, # missing parameter in doc strings @@ -155,12 +155,12 @@ disable= # E0611, # no-name-in-module W9013, # missing-yield-doc W9014, # missing-yield-type-doc - C0201, # consider-iterating-dictionary + # C0201, # consider-iterating-dictionary # W0237, # arguments-renamed # R1718, # consider-using-set-comprehension # R1723, # no-else-break - E1133, # not-an-iterable - E1135, # unsupported-membership-test + # E1133, # not-an-iterable + # E1135, # unsupported-membership-test # R1705, # no-else-return diff --git a/samtranslator/intrinsics/resolver.py b/samtranslator/intrinsics/resolver.py index f6513c4746..7524952456 100644 --- a/samtranslator/intrinsics/resolver.py +++ b/samtranslator/intrinsics/resolver.py @@ -25,7 +25,7 @@ def __init__(self, parameters, supported_intrinsics=None): ) if not isinstance(supported_intrinsics, dict) or not all( - [isinstance(value, Action) for value in supported_intrinsics.values()] + (isinstance(value, Action) for value in supported_intrinsics.values()) ): raise TypeError("supported_intrinsics argument must be intrinsic names to corresponding Action classes") diff --git a/samtranslator/model/__init__.py b/samtranslator/model/__init__.py index d4437a31aa..2f2d0186cc 100644 --- a/samtranslator/model/__init__.py +++ b/samtranslator/model/__init__.py @@ -229,7 +229,7 @@ def _generate_resource_dict(self): resource_dict.update(self.resource_attributes) properties_dict = {} - for name in self.property_types: + for name in self.property_types.keys(): value = getattr(self, name) if value is not None: properties_dict[name] = value @@ -246,7 +246,7 @@ def __setattr__(self, name, value): :param value: the value of the attribute to be set :raises InvalidResourceException: if an invalid property is provided """ - if name in self._keywords or name in self.property_types: + if name in self._keywords or name in self.property_types.keys(): return super(Resource, self).__setattr__(name, value) raise InvalidResourceException( @@ -376,7 +376,6 @@ def to_cloudformation(self, **kwargs): """ raise NotImplementedError("Method to_cloudformation() must be implemented in a subclass of ResourceMacro") - class SamResourceMacro(ResourceMacro): """ResourceMacro that specifically refers to SAM (AWS::Serverless::*) resources.""" diff --git a/samtranslator/model/api/api_generator.py b/samtranslator/model/api/api_generator.py index c8a2ef89eb..5b4f98bae3 100644 --- a/samtranslator/model/api/api_generator.py +++ b/samtranslator/model/api/api_generator.py @@ -418,6 +418,7 @@ def _construct_stage(self, deployment, swagger, redeploy_restapi_parameters): return stage def _construct_api_domain(self, rest_api, route53_record_set_groups): + # pylint: disable=R0801 """ Constructs and returns the ApiGateway Domain and BasepathMapping """ @@ -574,6 +575,7 @@ def _construct_record_sets_for_domain(self, domain): return recordset_list def _construct_alias_target(self, domain): + # pylint: disable=R0801 alias_target = {} route53 = domain.get("Route53") target_health = route53.get("EvaluateTargetHealth") diff --git a/samtranslator/model/apigatewayv2.py b/samtranslator/model/apigatewayv2.py index ece8166d3a..fb9b2f3bfd 100644 --- a/samtranslator/model/apigatewayv2.py +++ b/samtranslator/model/apigatewayv2.py @@ -179,7 +179,7 @@ def _validate_lambda_authorizer(self): ) headers = self.identity.get("Headers") if headers: - if not isinstance(headers, list) or any([not isinstance(header, str) for header in headers]): + if not isinstance(headers, list) or any((not isinstance(header, str) for header in headers)): raise InvalidResourceException( self.api_logical_id, self.name + " Lambda Authorizer property identity's 'Headers' is of invalid type.", diff --git a/samtranslator/model/eventsources/push.py b/samtranslator/model/eventsources/push.py index c38f072a18..f75c21f63a 100644 --- a/samtranslator/model/eventsources/push.py +++ b/samtranslator/model/eventsources/push.py @@ -684,6 +684,7 @@ def _get_permission(self, resources_to_link, stage, suffix): return self._construct_permission(resources_to_link["function"], source_arn=source_arn, suffix=suffix) def _add_swagger_integration(self, api, function, intrinsics_resolver): + # pylint: disable=R0801 """Adds the path and method for this Api event source to the Swagger body for the provided RestApi. :param model.apigateway.ApiGatewayRestApi rest_api: the RestApi to which the path and method should be added. diff --git a/samtranslator/model/function_policies.py b/samtranslator/model/function_policies.py deleted file mode 100644 index 0991cd59be..0000000000 --- a/samtranslator/model/function_policies.py +++ /dev/null @@ -1,205 +0,0 @@ -from enum import Enum -from collections import namedtuple - -from samtranslator.model.intrinsics import ( - is_intrinsic, - is_intrinsic_if, - is_intrinsic_no_value, - validate_intrinsic_if_items, -) -from samtranslator.model.exceptions import InvalidTemplateException - -PolicyEntry = namedtuple("PolicyEntry", "data type") - - -class FunctionPolicies(object): - """ - Class encapsulating the policies property of AWS::Serverless::Function. This class strictly encapsulates the data - and does not take opinions on how to handle them. - - There are three types of policies: - - Policy Statements - - AWS or Custom Managed Policy names/arns - - Policy Templates - - This class is capable of parsing and detecting the type of the policy. Optionally, if policy template information - is provided to this class, it will detect Policy Templates too. - """ - - POLICIES_PROPERTY_NAME = "Policies" - - def __init__(self, resource_properties, policy_template_processor=None): - """ - Initialize with policies data from resource's properties - - :param dict resource_properties: Dictionary containing properties of this resource - :param policy_template_processor: Optional Instance of PolicyTemplateProcessor that can conclusively detect - if a given policy is a template or not. If not provided, then this class will not detect policy templates. - """ - - # This variable is required to get policies - self._policy_template_processor = policy_template_processor - - # Build the list of policies upon construction. - self.policies = self._get_policies(resource_properties) - - def get(self): - """ - Iterator method that "yields" the next policy entry on subsequent calls to this method. - - :yields namedtuple("data", "type"): Yields a named tuple containing the policy data and its type - """ - - for policy_tuple in self.policies: - yield policy_tuple - - def __len__(self): - return len(self.policies) - - def _get_policies(self, resource_properties): - """ - Returns a list of policies from the resource properties. This method knows how to interpret and handle - polymorphic nature of the policies property. - - Policies can be one of the following: - - * Managed policy name: string - * List of managed policy names: list of strings - * IAM Policy document: dict containing Statement key - * List of IAM Policy documents: list of IAM Policy Document - * Policy Template: dict with only one key where key is in list of supported policy template names - * List of Policy Templates: list of Policy Template - - - :param dict resource_properties: Dictionary of resource properties containing the policies property. - It is assumed that this is already a dictionary and contains policies key. - :return list of PolicyEntry: List of policies, where each item is an instance of named tuple `PolicyEntry` - """ - - policies = None - - if self._contains_policies(resource_properties): - policies = resource_properties[self.POLICIES_PROPERTY_NAME] - - if not policies: - # Policies is None or empty - return [] - - if not isinstance(policies, list): - # Just a single entry. Make it into a list of convenience - policies = [policies] - - result = [] - for policy in policies: - policy_type = self._get_type(policy) - entry = PolicyEntry(data=policy, type=policy_type) - result.append(entry) - - return result - - def _contains_policies(self, resource_properties): - """ - Is there policies data in this resource? - - :param dict resource_properties: Properties of the resource - :return: True if we can process this resource. False, otherwise - """ - return ( - resource_properties is not None - and isinstance(resource_properties, dict) - and self.POLICIES_PROPERTY_NAME in resource_properties - ) - - def _get_type(self, policy): - """ - Returns the type of the given policy - - :param string or dict policy: Policy data - :return PolicyTypes: Type of the given policy. None, if type could not be inferred - """ - - # Must handle intrinsic functions. Policy could be a primitive type or an intrinsic function - - # Managed policies are of type string - if isinstance(policy, str): - 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_intrinsic(policy): - return PolicyTypes.MANAGED_POLICY - - # Policy statement is a dictionary with the key "Statement" in it - if isinstance(policy, dict) and "Statement" in policy: - return PolicyTypes.POLICY_STATEMENT - - # This could be a policy template then. - if self._is_policy_template(policy): - return PolicyTypes.POLICY_TEMPLATE - - # Nothing matches. Don't take opinions on how to handle it. Instead just set the appropriate type. - return PolicyTypes.UNKNOWN - - def _is_policy_template(self, policy): - """ - Is the given policy data a policy template? Policy templates is a dictionary with one key which is the name - of the template. - - :param dict policy: Policy data - :return: True, if this is a policy template. False if it is not - """ - - return ( - self._policy_template_processor is not None - and isinstance(policy, dict) - and 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"] - - try: - validate_intrinsic_if_items(intrinsic_if_value) - except ValueError as e: - raise InvalidTemplateException(e) - - 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. " - "Separate different policy types into different Fn::If statements" - ) - - -class PolicyTypes(Enum): - """ - Enum of different policy types supported by SAM & this plugin - """ - - MANAGED_POLICY = "managed_policy" - POLICY_STATEMENT = "policy_statement" - POLICY_TEMPLATE = "policy_template" - UNKNOWN = "unknown" diff --git a/samtranslator/plugins/globals/globals.py b/samtranslator/plugins/globals/globals.py index 56fee8c6f7..a1acfd896d 100644 --- a/samtranslator/plugins/globals/globals.py +++ b/samtranslator/plugins/globals/globals.py @@ -93,7 +93,7 @@ def __init__(self, template): :param dict template: SAM template to be parsed """ self.supported_resource_section_names = [ - x.replace(self._RESOURCE_PREFIX, "") for x in self.supported_properties.keys() + x.replace(self._RESOURCE_PREFIX, "") for x in self.supported_properties ] # Sort the names for stability in list ordering self.supported_resource_section_names.sort() diff --git a/tests/model/test_function_policies.py b/tests/model/test_function_policies.py deleted file mode 100644 index d2405284c6..0000000000 --- a/tests/model/test_function_policies.py +++ /dev/null @@ -1,332 +0,0 @@ -from unittest.mock import Mock, patch -from unittest import TestCase - -from samtranslator.model.function_policies import FunctionPolicies, PolicyTypes, PolicyEntry -from samtranslator.model.exceptions import InvalidTemplateException - - -class TestFunctionPolicies(TestCase): - def setUp(self): - self.policy_template_processor_mock = Mock() - self.is_policy_template_mock = Mock() - - self.function_policies = FunctionPolicies({}, self.policy_template_processor_mock) - self.function_policies._is_policy_template = self.is_policy_template_mock - - @patch.object(FunctionPolicies, "_get_policies") - def test_initialization_must_ingest_policies_from_resource_properties(self, get_policies_mock): - resource_properties = {} - dummy_policy_results = ["some", "policy", "statements"] - expected_length = 3 - - get_policies_mock.return_value = dummy_policy_results - function_policies = FunctionPolicies(resource_properties, self.policy_template_processor_mock) - - get_policies_mock.assert_called_once_with(resource_properties) - self.assertEqual(expected_length, len(function_policies)) - - @patch.object(FunctionPolicies, "_get_policies") - def test_get_must_yield_results_on_every_call(self, get_policies_mock): - resource_properties = {} # Just some input - dummy_policy_results = ["some", "policy", "statements"] - expected_results = ["some", "policy", "statements"] - - # Setup _get_policies to return these dummy values for testing - get_policies_mock.return_value = dummy_policy_results - function_policies = FunctionPolicies(resource_properties, self.policy_template_processor_mock) - - # `list()` will implicitly call the `get()` repeatedly because it is a generator - self.assertEqual(list(function_policies.get()), expected_results) - - @patch.object(FunctionPolicies, "_get_policies") - def test_get_must_yield_no_results_with_no_policies(self, get_policies_mock): - resource_properties = {} # Just some input - dummy_policy_results = [] - expected_result = [] - - # Setup _get_policies to return these dummy values for testing - get_policies_mock.return_value = dummy_policy_results - function_policies = FunctionPolicies(resource_properties, self.policy_template_processor_mock) - - # `list()` will implicitly call the `get()` repeatedly because it is a generator - self.assertEqual(list(function_policies.get()), expected_result) - - def test_contains_policies_must_work_for_valid_input(self): - resource_properties = {"Policies": "some managed policy"} - - self.assertTrue(self.function_policies._contains_policies(resource_properties)) - - def test_contains_policies_must_ignore_resources_without_policies(self): - resource_properties = {"some key": "value"} - - self.assertFalse(self.function_policies._contains_policies(resource_properties)) - - def test_contains_policies_must_ignore_non_dict_resources(self): - resource_properties = "some value" - - self.assertFalse(self.function_policies._contains_policies(resource_properties)) - - def test_contains_policies_must_ignore_none_resources(self): - resource_properties = None - - self.assertFalse(self.function_policies._contains_policies(resource_properties)) - - def test_contains_policies_must_ignore_lowercase_property_name(self): - # Property names are case sensitive - resource_properties = {"policies": "some managed policy"} - - self.assertFalse(self.function_policies._contains_policies(resource_properties)) - - def test_get_type_must_work_for_managed_policy(self): - policy = "managed policy is a string" - expected = PolicyTypes.MANAGED_POLICY - - result = self.function_policies._get_type(policy) - self.assertEqual(result, expected) - - @patch("samtranslator.model.function_policies.is_intrinsic") - def test_get_type_must_work_for_managed_policy_with_intrinsics(self, is_intrinsic_mock): - policy = {"Ref": "somevalue"} - expected = PolicyTypes.MANAGED_POLICY - is_intrinsic_mock.return_value = True - - result = self.function_policies._get_type(policy) - self.assertEqual(result, expected) - - def test_get_type_must_work_for_policy_statements(self): - policy = {"Statement": "policy statements have a 'Statement' key"} - expected = PolicyTypes.POLICY_STATEMENT - - result = self.function_policies._get_type(policy) - self.assertEqual(result, expected) - - def test_get_type_must_work_for_policy_templates(self): - policy = {"PolicyTemplate": "some template"} - self.is_policy_template_mock.return_value = True - expected = PolicyTypes.POLICY_TEMPLATE - - result = self.function_policies._get_type(policy) - self.assertEqual(result, expected) - - def test_get_type_must_ignore_invalid_policy(self): - policy = {"not-sure-what-this-is": "value"} - # This is also not a policy template - self.is_policy_template_mock.return_value = False - expected = PolicyTypes.UNKNOWN - - result = self.function_policies._get_type(policy) - self.assertEqual(result, expected) - - def test_get_type_must_ignore_invalid_policy_value_list(self): - policy = ["invalid", "policy"] - expected = PolicyTypes.UNKNOWN - - self.is_policy_template_mock.return_value = False - - result = self.function_policies._get_type(policy) - self.assertEqual(result, expected) - self.is_policy_template_mock.assert_called_once_with(policy) - - def test_get_policies_must_return_all_policies(self): - policies = [ - "managed policy 1", - {"Ref": "some managed policy"}, - {"Statement": "policy statement"}, - {"PolicyTemplate": "some value"}, - ["unknown", "policy"], - ] - resource_properties = {"Policies": policies} - self.is_policy_template_mock.side_effect = [True, False] # Return True for policy template, False for the list - - expected = [ - PolicyEntry(data="managed policy 1", type=PolicyTypes.MANAGED_POLICY), - PolicyEntry(data={"Ref": "some managed policy"}, type=PolicyTypes.MANAGED_POLICY), - PolicyEntry(data={"Statement": "policy statement"}, type=PolicyTypes.POLICY_STATEMENT), - PolicyEntry(data={"PolicyTemplate": "some value"}, type=PolicyTypes.POLICY_TEMPLATE), - PolicyEntry(data=["unknown", "policy"], type=PolicyTypes.UNKNOWN), - ] - - result = self.function_policies._get_policies(resource_properties) - self.assertEqual(result, expected) - - def test_get_policies_must_ignore_if_resource_does_not_contain_policy(self): - resource_properties = {} - expected = [] - - result = self.function_policies._get_policies(resource_properties) - self.assertEqual(result, expected) - - def test_get_policies_must_ignore_if_policies_is_empty(self): - resource_properties = {"Policies": []} - expected = [] - - result = self.function_policies._get_policies(resource_properties) - self.assertEqual(result, expected) - - def test_get_policies_must_work_for_single_policy_string(self): - resource_properties = {"Policies": "single managed policy"} - expected = [PolicyEntry(data="single managed policy", type=PolicyTypes.MANAGED_POLICY)] - - result = self.function_policies._get_policies(resource_properties) - self.assertEqual(result, expected) - - def test_get_policies_must_work_for_single_dict_with_managed_policy_intrinsic(self): - resource_properties = {"Policies": {"Ref": "some managed policy"}} - expected = [PolicyEntry(data={"Ref": "some managed policy"}, type=PolicyTypes.MANAGED_POLICY)] - - result = self.function_policies._get_policies(resource_properties) - self.assertEqual(result, expected) - - def test_get_policies_must_work_for_single_dict_with_policy_statement(self): - resource_properties = {"Policies": {"Statement": "some policy statement"}} - expected = [PolicyEntry(data={"Statement": "some policy statement"}, type=PolicyTypes.POLICY_STATEMENT)] - - result = self.function_policies._get_policies(resource_properties) - self.assertEqual(result, expected) - - def test_get_policies_must_work_for_single_dict_of_policy_template(self): - resource_properties = {"Policies": {"PolicyTemplate": "some template"}} - self.is_policy_template_mock.return_value = True - expected = [PolicyEntry(data={"PolicyTemplate": "some template"}, type=PolicyTypes.POLICY_TEMPLATE)] - - result = self.function_policies._get_policies(resource_properties) - self.assertEqual(result, expected) - self.is_policy_template_mock.assert_called_once_with(resource_properties["Policies"]) - - def test_get_policies_must_work_for_single_dict_of_invalid_policy_template(self): - resource_properties = {"Policies": {"InvalidPolicyTemplate": "some template"}} - self.is_policy_template_mock.return_value = False # Invalid policy template - expected = [PolicyEntry(data={"InvalidPolicyTemplate": "some template"}, type=PolicyTypes.UNKNOWN)] - - result = self.function_policies._get_policies(resource_properties) - self.assertEqual(result, expected) - self.is_policy_template_mock.assert_called_once_with({"InvalidPolicyTemplate": "some template"}) - - def test_get_policies_must_work_for_unknown_policy_types(self): - resource_properties = {"Policies": [1, 2, 3]} - expected = [ - PolicyEntry(data=1, type=PolicyTypes.UNKNOWN), - PolicyEntry(data=2, type=PolicyTypes.UNKNOWN), - PolicyEntry(data=3, type=PolicyTypes.UNKNOWN), - ] - - self.is_policy_template_mock.return_value = False - - result = self.function_policies._get_policies(resource_properties) - self.assertEqual(result, expected) - - def test_is_policy_template_must_detect_valid_policy_templates(self): - template_name = "template_name" - policy = {template_name: {"Param1": "foo"}} - - self.policy_template_processor_mock.has.return_value = True - function_policies = FunctionPolicies({}, self.policy_template_processor_mock) - - self.assertTrue(function_policies._is_policy_template(policy)) - - self.policy_template_processor_mock.has.assert_called_once_with(template_name) - - def test_is_policy_template_must_ignore_non_dict_policies(self): - policy = [1, 2, 3] - - self.policy_template_processor_mock.has.return_value = True - function_policies = FunctionPolicies({}, self.policy_template_processor_mock) - - self.assertFalse(function_policies._is_policy_template(policy)) - - self.policy_template_processor_mock.has.assert_not_called() - - def test_is_policy_template_must_ignore_none_policies(self): - policy = None - - function_policies = FunctionPolicies({}, self.policy_template_processor_mock) - self.assertFalse(function_policies._is_policy_template(policy)) - - def test_is_policy_template_must_ignore_dict_with_two_keys(self): - template_name = "template_name" - policy = {template_name: {"param1": "foo"}, "A": "B"} - - self.policy_template_processor_mock.has.return_value = True - - function_policies = FunctionPolicies({}, self.policy_template_processor_mock) - self.assertFalse(function_policies._is_policy_template(policy)) - - def test_is_policy_template_must_ignore_non_policy_templates(self): - template_name = "template_name" - policy = {template_name: {"param1": "foo"}} - - self.policy_template_processor_mock.has.return_value = False - - function_policies = FunctionPolicies({}, self.policy_template_processor_mock) - self.assertFalse(function_policies._is_policy_template(policy)) - - self.policy_template_processor_mock.has.assert_called_once_with(template_name) - - def test_is_policy_template_must_return_false_without_the_processor(self): - policy = {"template_name": {"param1": "foo"}} - - function_policies_obj = FunctionPolicies({}, None) # No policy template processor - - self.assertFalse(function_policies_obj._is_policy_template(policy)) - self.policy_template_processor_mock.has.assert_not_called() - - 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) From e5ad2e6624529ec288b24a52dc91ca7ad8973348 Mon Sep 17 00:00:00 2001 From: Gavin Zhang Date: Fri, 14 Oct 2022 13:34:24 -0700 Subject: [PATCH 2/3] Remove enabled rules --- .pylintrc | 45 --------------------------------------------- 1 file changed, 45 deletions(-) diff --git a/.pylintrc b/.pylintrc index 23283cbb15..f1370f491e 100644 --- a/.pylintrc +++ b/.pylintrc @@ -69,7 +69,6 @@ disable= W0613, # Unused argument %r W0640, # Cell variable %s defined in loop A variable used in a closure is defined in a loop R0902, # Too many instance attributes (%s/%s) - # R0903, # Too few public methods (%s/%s) R0913, # Too many arguments (%s/%s) W0703, # Catching too general exception %s R0904, # Too many public methods (%s/%s) @@ -77,36 +76,14 @@ disable= R0915, # Too many statements C0415, # Import outside toplevel (%s) Used when an import statement is used anywhere other than the module toplevel. Move this import to the top of the file. C0115, # missing-class-docstring - # below are docstring lint rules, in order to incrementally improve our docstring while - # without introduce too much effort at a time, we exclude most of the docstring lint rules. - # We will remove them one by one. - # W9005, # "%s" has constructor parameters documented in class and __init__ W9006, # "%s" not documented as being raised - # W9008, # Redundant returns documentation - # W9010, # Redundant yields documentation W9011, # Missing return documentation W9012, # Missing return type documentation - # W9013, # Missing yield documentation - # W9014, # Missing yield type documentation - # W9015, # "%s" missing in parameter documentation W9016, # "%s" missing in parameter type documentation - # W9017, # "%s" differing in parameter documentation - # W9018, # "%s" differing in parameter type documentation - # W9019, # "%s" useless ignored parameter documentation - # W9020, # "%s" useless ignored parameter type documentation - # Constant name style warnings. We will remove them one by one. C0103, # Class constant name "%s" doesn't conform to UPPER_CASE naming style ('([^\\W\\da-z][^\\Wa-z]*|__.*__)$' pattern) (invalid-name) - # New recommendations, disable for now to avoid introducing behaviour changes. - # R1729, # Use a generator instead '%s' (use-a-generator) - # R1732, # Consider using 'with' for resource-allocating operations (consider-using-with) - # This applies to CPython only. - # I1101, # Module 'math' has no 'pow' member, but source is unavailable. Consider adding this module to extension-pkg-allow-list if you want to perform analysis based on run-time introspection of living objects. (c-extension-no-member) - # Added to allow linting. Need to work on removing over time - # R0801, # dupilcated code R0401, # Cyclic import C0411, # import ordering W9015, # missing parameter in doc strings - # W0612, # unused variable R0205, # useless-object-inheritanc C0301, # line to long R0201, # no self use, method could be a function @@ -114,8 +91,6 @@ disable= W1202, # Use lazy % formatting in logging functions (logging-format-interpolation) E1101, # No member W0622, # Redefining built-in 'property' (redefined-builtin) - # W0611, # unused imports - # W0231, # super not called W0212, # protected-access W0201, # attribute-defined-outside-init R1725, # Consider using Python 3 style super() without arguments (super-with-arguments) @@ -126,8 +101,6 @@ disable= W0223, # abstract-method W0107, # unnecessary-pass W0707, # raise-missing-from - # R1720, # no-else-raise - # W0621, # redefined-outer-name E0203, # access-member-before-definition W0221, # arguments-differ R1710, # inconsistent-return-statements @@ -136,32 +109,14 @@ disable= W0105, # String statement has no effect (pointless-string-statement) C0206, # Consider iterating with .items() (consider-using-dict-items) W9008, # Redundant returns documentation (redundant-returns-doc) - # E0602, # undefined-variable C0112, # empty-docstring C0116, # missing-function-docstring - # C0200, # consider-using-enumerate - # R5501, # Consider using "elif" instead of "else if" (else-if-used) W9017, # differing-param-doc W9018, # differing-type-doc W0511, # fixme - # C0325, # superfluous-parens - # R1701, # consider-merging-isinstance C0302, # too-many-lines - # C0303, # trailing-whitespace - # R1721, # unnecessary-comprehension - # C0121, # singleton-comparison - # E1305, # too-many-format-args - # R1724, # no-else-continue - # E0611, # no-name-in-module W9013, # missing-yield-doc W9014, # missing-yield-type-doc - # C0201, # consider-iterating-dictionary - # W0237, # arguments-renamed - # R1718, # consider-using-set-comprehension - # R1723, # no-else-break - # E1133, # not-an-iterable - # E1135, # unsupported-membership-test - # R1705, # no-else-return [REPORTS] From 8405a2e41d7a0df974b1f39e1aa63d742673e2ab Mon Sep 17 00:00:00 2001 From: Gavin Zhang Date: Fri, 14 Oct 2022 14:18:47 -0700 Subject: [PATCH 3/3] Address Feedback --- samtranslator/model/__init__.py | 1 + samtranslator/model/api/api_generator.py | 4 ++-- samtranslator/model/eventsources/push.py | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/samtranslator/model/__init__.py b/samtranslator/model/__init__.py index 2f2d0186cc..cbc00c6dec 100644 --- a/samtranslator/model/__init__.py +++ b/samtranslator/model/__init__.py @@ -376,6 +376,7 @@ def to_cloudformation(self, **kwargs): """ raise NotImplementedError("Method to_cloudformation() must be implemented in a subclass of ResourceMacro") + class SamResourceMacro(ResourceMacro): """ResourceMacro that specifically refers to SAM (AWS::Serverless::*) resources.""" diff --git a/samtranslator/model/api/api_generator.py b/samtranslator/model/api/api_generator.py index 5b4f98bae3..86217c0765 100644 --- a/samtranslator/model/api/api_generator.py +++ b/samtranslator/model/api/api_generator.py @@ -418,7 +418,7 @@ def _construct_stage(self, deployment, swagger, redeploy_restapi_parameters): return stage def _construct_api_domain(self, rest_api, route53_record_set_groups): - # pylint: disable=R0801 + # pylint: disable=duplicate-code """ Constructs and returns the ApiGateway Domain and BasepathMapping """ @@ -575,7 +575,7 @@ def _construct_record_sets_for_domain(self, domain): return recordset_list def _construct_alias_target(self, domain): - # pylint: disable=R0801 + # pylint: disable=duplicate-code alias_target = {} route53 = domain.get("Route53") target_health = route53.get("EvaluateTargetHealth") diff --git a/samtranslator/model/eventsources/push.py b/samtranslator/model/eventsources/push.py index f75c21f63a..ccc8db3b69 100644 --- a/samtranslator/model/eventsources/push.py +++ b/samtranslator/model/eventsources/push.py @@ -684,7 +684,7 @@ def _get_permission(self, resources_to_link, stage, suffix): return self._construct_permission(resources_to_link["function"], source_arn=source_arn, suffix=suffix) def _add_swagger_integration(self, api, function, intrinsics_resolver): - # pylint: disable=R0801 + # pylint: disable=duplicate-code """Adds the path and method for this Api event source to the Swagger body for the provided RestApi. :param model.apigateway.ApiGatewayRestApi rest_api: the RestApi to which the path and method should be added.