Pin click to at least version 8 and update black to 22.3.0 (#220)

* Pin click to at least version 8 and update black to 22.3.0, running make format as well * Add setup.cfg for isort parameters, to prevent conflict between isort and black Co-authored-by: Oliver Crawford <oliver.crawford@skyscanner.net>
Skyscanner · Apr 5, 2022 · ade94e2 · ade94e2
1 parent d442689
commit ade94e2
Show file tree

Hide file tree

Showing 18 changed files with 94 additions and 32 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,6 +1,11 @@
 # Changelog
 All notable changes to this project will be documented in this file.
 
+## [1.8.0]
+### Improvements
+- Pin `click` to at least version `8.0.0`.
+- Update `black` to `22.3.0`, and run `make format` with this new version of `black`.
+
 ## [1.7.1]
 ### Fixes
 - `EBSVolumeHasSSERule` can now understand `encrypted_status` if modelled as a `bool`.

diff --git a/cfripper/__version__.py b/cfripper/__version__.py
@@ -1,3 +1,3 @@
-VERSION = (1, 7, 1)
+VERSION = (1, 8, 0)
 
 __version__ = ".".join(map(str, VERSION))
diff --git a/cfripper/cli.py b/cfripper/cli.py
@@ -184,7 +184,9 @@ def validate_aws_principals(ctx: click.Context, param: str, value: str) -> Optio
     show_default=True,
 )
 @click.option(
-    "--rules-config-file", type=click.File("r"), help="Loads rules configuration file (type: [.py, .pyc])",
+    "--rules-config-file",
+    type=click.File("r"),
+    help="Loads rules configuration file (type: [.py, .pyc])",
 )
 @click.option(
     "--rules-filters-folder",

diff --git a/cfripper/rules/__init__.py b/cfripper/rules/__init__.py
@@ -98,7 +98,12 @@
 
 BASE_CLASSES = {
     rule.__name__: rule
-    for rule in (BaseDangerousPolicyActions, CrossAccountCheckingRule, PrincipalCheckingRule, ResourceSpecificRule,)
+    for rule in (
+        BaseDangerousPolicyActions,
+        CrossAccountCheckingRule,
+        PrincipalCheckingRule,
+        ResourceSpecificRule,
+    )
 }
 
 

diff --git a/cfripper/rules/ec2_security_group.py b/cfripper/rules/ec2_security_group.py
@@ -50,7 +50,11 @@ def analyse_ingress(
 
                 self.add_failure_to_result(
                     result,
-                    self.REASON.format(self.get_open_ports_wording(non_allowed_open_ports), ip_range, logical_id,),
+                    self.REASON.format(
+                        self.get_open_ports_wording(non_allowed_open_ports),
+                        ip_range,
+                        logical_id,
+                    ),
                     resource_ids={logical_id},
                     resource_types={resource_type},
                     context=filters_available_context,

diff --git a/pyproject.toml b/pyproject.toml
@@ -1,9 +1,8 @@
 [tool.black]
 line-length = 120
-target_version = ['py37']
-[tool.isort]
-multi_line_output = 3
-include_trailing_comma = true
-force_grid_wrap = 0
-use_parentheses = true
-line_length = 120
+exclude = '''
+/(
+  | \.venv
+  | venv
+)/
+'''
diff --git a/requirements.txt b/requirements.txt
@@ -7,7 +7,7 @@
 boto3==1.21.31
 botocore==1.24.31
 cfn-flip==1.3.0
-click==7.1.2
+click==8.1.2
 jmespath==1.0.0
 pluggy==0.13.1
 pycfmodel==0.18.0

diff --git a/setup.cfg b/setup.cfg
@@ -0,0 +1,7 @@
+[isort]
+multi_line_output = 3
+include_trailing_comma = True
+force_grid_wrap = 0
+combine_as_imports = True
+line_length = 120
+skip = .venv,venv
diff --git a/setup.py b/setup.py
@@ -9,15 +9,15 @@
 install_requires = [
     "boto3>=1.4.7,<2",
     "cfn_flip>=1.2.0",
-    "click~=7.1.1",
+    "click>=8.0.0",
     "pluggy~=0.13.1",
     "pycfmodel>=0.18.0",
     "pydash~=4.7.6",
     "PyYAML>=4.2b1",
 ]
 
 dev_requires = [
-    "black==19.10b0",
+    "black==22.3.0",
     "flake8>=3.3.0",
     "isort==4.3.21",
     "pytest>=3.6",

diff --git a/tests/model/test_principal_checking_rule.py b/tests/model/test_principal_checking_rule.py
@@ -173,7 +173,11 @@ def invoke(self, cfmodel: CFModel, extras: Optional[Dict] = None) -> Result:
                 "b14d6a125bdf69854ed8ef2e71d8a20b7c490f252229b806e514966e490b8d83",
             },
         ),
-        (FakePrincipalCheckingRule(config=Config(aws_service_accounts={"A": ["a", "b", "c"]})), None, {"a", "b", "c"},),
+        (
+            FakePrincipalCheckingRule(config=Config(aws_service_accounts={"A": ["a", "b", "c"]})),
+            None,
+            {"a", "b", "c"},
+        ),
         (
             FakePrincipalCheckingRule(config=Config(aws_service_accounts={"A": ["a", "b", "c"], "B": ["d", "e", "f"]})),
             None,

diff --git a/tests/rules/test_CrossAccountTrustRule.py b/tests/rules/test_CrossAccountTrustRule.py
@@ -153,7 +153,11 @@ def test_filter_works_as_expected(template_two_roles_dict, expected_result_two_r
 def test_filter_works_as_expected_with_rules_config_file(
     template_two_roles_dict, expected_result_two_roles, test_files_location
 ):
-    config = Config(rules=["CrossAccountTrustRule"], aws_account_id="123456789", stack_name="mockstack",)
+    config = Config(
+        rules=["CrossAccountTrustRule"],
+        aws_account_id="123456789",
+        stack_name="mockstack",
+    )
     config.load_rules_config_file(open(f"{test_files_location}/config/rules_config_CrossAccountTrustRule.py"))
     config.add_filters_from_dir(f"{test_files_location}/filters")
     rules = [DEFAULT_RULES.get(rule)(config) for rule in config.rules]
@@ -257,7 +261,8 @@ def test_kms_cross_account_failure(principal):
 
 
 @pytest.mark.parametrize(
-    "principal", ["arn:aws:iam::123456789:root", "arn:aws:iam::123456789:not-root", "arn:aws:iam::123456789:not-root*"],
+    "principal",
+    ["arn:aws:iam::123456789:root", "arn:aws:iam::123456789:not-root", "arn:aws:iam::123456789:not-root*"],
 )
 def test_kms_cross_account_success(principal):
     rule = KMSKeyCrossAccountTrustRule(Config(aws_account_id="123456789", aws_principals=["999999999"]))
@@ -332,7 +337,8 @@ def test_es_domain_cross_account_failure(principal):
 
 
 @pytest.mark.parametrize(
-    "principal", ["arn:aws:iam::123456789:root", "arn:aws:iam::123456789:not-root", "arn:aws:iam::123456789:not-root*"],
+    "principal",
+    ["arn:aws:iam::123456789:root", "arn:aws:iam::123456789:not-root", "arn:aws:iam::123456789:not-root*"],
 )
 def test_es_domain_cross_account_success(principal):
     rule = ElasticsearchDomainCrossAccountTrustRule(Config(aws_account_id="123456789", aws_principals=["999999999"]))
@@ -410,7 +416,8 @@ def test_opensearch_domain_cross_account_failure(principal):
 
 
 @pytest.mark.parametrize(
-    "principal", ["arn:aws:iam::123456789:root", "arn:aws:iam::123456789:not-root", "arn:aws:iam::123456789:not-root*"],
+    "principal",
+    ["arn:aws:iam::123456789:root", "arn:aws:iam::123456789:not-root", "arn:aws:iam::123456789:not-root*"],
 )
 def test_opensearch_domain_cross_account_success(principal):
     rule = OpenSearchDomainCrossAccountTrustRule(Config(aws_account_id="123456789", aws_principals=["999999999"]))

diff --git a/tests/rules/test_EBSVolumeHasSSERule.py b/tests/rules/test_EBSVolumeHasSSERule.py
@@ -26,7 +26,8 @@ def test_no_failures_are_raised(good_template):
 
 
 @pytest.mark.parametrize(
-    "template_path", ["rules/EBSVolumeHasSSERule/bad_template.json", "rules/EBSVolumeHasSSERule/bad_template.yaml"],
+    "template_path",
+    ["rules/EBSVolumeHasSSERule/bad_template.json", "rules/EBSVolumeHasSSERule/bad_template.yaml"],
 )
 def test_failures_are_raised(template_path):
     rule = EBSVolumeHasSSERule(Config(aws_account_id="123456789"))

diff --git a/tests/rules/test_EC2SecurityGroupIngressOpenToWorld.py b/tests/rules/test_EC2SecurityGroupIngressOpenToWorld.py
@@ -82,7 +82,9 @@ def test_filter_do_not_report_anything(filter_eval_object, bad_template):
         stack_name="mockstack",
         rules_filters=[
             Filter(
-                rule_mode=RuleMode.ALLOWED, eval=filter_eval_object, rules={"EC2SecurityGroupIngressOpenToWorldRule"},
+                rule_mode=RuleMode.ALLOWED,
+                eval=filter_eval_object,
+                rules={"EC2SecurityGroupIngressOpenToWorldRule"},
             )
         ],
     )
@@ -109,10 +111,18 @@ def test_filter_context_set_correctly(mock_failure_to_result, bad_template):
     assert mock_failure_to_result.mock_calls[0][2]["context"]["ingress_ip"] == "11.0.0.0/8"
     assert mock_failure_to_result.mock_calls[1][2]["context"]["ingress_ip"] == "::/0"
     assert mock_failure_to_result.mock_calls[0][2]["context"]["ingress_obj"] == SecurityGroupIngressProperties(
-        CidrIp=IPv4Network("11.0.0.0/8"), FromPort=46, IpProtocol="tcp", ToPort=46, GroupId="sg-12341234",
+        CidrIp=IPv4Network("11.0.0.0/8"),
+        FromPort=46,
+        IpProtocol="tcp",
+        ToPort=46,
+        GroupId="sg-12341234",
     )
     assert mock_failure_to_result.mock_calls[1][2]["context"]["ingress_obj"] == SecurityGroupIngressProperties(
-        CidrIpv6=IPv6Network("::/0"), FromPort=46, IpProtocol="tcp", ToPort=46, GroupId="sg-12341234",
+        CidrIpv6=IPv6Network("::/0"),
+        FromPort=46,
+        IpProtocol="tcp",
+        ToPort=46,
+        GroupId="sg-12341234",
     )
 
 

diff --git a/tests/rules/test_EC2SecurityGroupOpenToWorldRule.py b/tests/rules/test_EC2SecurityGroupOpenToWorldRule.py
@@ -152,7 +152,11 @@ def test_invalid_security_group_port78_81(invalid_security_group_port78_81):
 
 def test_invalid_security_group_no_ports_defined(invalid_security_group_no_ports_defined):
     rule = EC2SecurityGroupOpenToWorldRule(
-        Config(rules=["EC2SecurityGroupOpenToWorldRule"], aws_account_id="123456789", stack_name="mockstack",)
+        Config(
+            rules=["EC2SecurityGroupOpenToWorldRule"],
+            aws_account_id="123456789",
+            stack_name="mockstack",
+        )
     )
     result = rule.invoke(invalid_security_group_no_ports_defined)
 

diff --git a/tests/rules/test_GenericCrossAccountTrustRule.py b/tests/rules/test_GenericCrossAccountTrustRule.py
@@ -203,7 +203,8 @@ def test_generic_rule_supports_filter_config(s3_bucket_cross_account_and_normal,
 
 
 @pytest.mark.parametrize(
-    "principal", ["arn:aws:iam::123456789:root", "arn:aws:iam::123456789:not-root", "arn:aws:iam::123456789:not-root*"],
+    "principal",
+    ["arn:aws:iam::123456789:root", "arn:aws:iam::123456789:not-root", "arn:aws:iam::123456789:not-root*"],
 )
 def test_generic_cross_account_for_opensearch_domain_with_principal_params(principal):
     rule = GenericCrossAccountTrustRule(Config(aws_account_id="123456789", aws_principals=["999999999"]))
@@ -436,7 +437,8 @@ def test_generic_cross_account_rule_for_resources_with_set_principals(template,
 
 
 @pytest.mark.parametrize(
-    "principal", ["arn:aws:iam::123456789:root", "arn:aws:iam::123456789:not-root", "arn:aws:iam::123456789:not-root*"],
+    "principal",
+    ["arn:aws:iam::123456789:root", "arn:aws:iam::123456789:not-root", "arn:aws:iam::123456789:not-root*"],
 )
 def test_generic_cross_account_es_domain_cross_account_success(principal):
     rule = GenericCrossAccountTrustRule(Config(aws_account_id="123456789", aws_principals=["999999999"]))
@@ -484,7 +486,8 @@ def test_generic_cross_account_rule_es_domain_cross_account_failure(principal):
 
 
 @pytest.mark.parametrize(
-    "principal", ["arn:aws:iam::123456789:root", "arn:aws:iam::123456789:not-root", "arn:aws:iam::123456789:not-root*"],
+    "principal",
+    ["arn:aws:iam::123456789:root", "arn:aws:iam::123456789:not-root", "arn:aws:iam::123456789:not-root*"],
 )
 def test_generic_cross_account_with_kms_key_success(principal):
     rule = GenericCrossAccountTrustRule(Config(aws_account_id="123456789", aws_principals=["999999999"]))

diff --git a/tests/rules/test_GenericResourceWildcardPolicyRule.py b/tests/rules/test_GenericResourceWildcardPolicyRule.py
@@ -150,7 +150,10 @@ def test_generic_rule_with_already_mapped_resources(template, is_valid, failures
     rule = GenericResourceWildcardPolicyRule(None)
     result = rule.invoke(template)
     assert result.valid == is_valid
-    assert compare_lists_of_failures(result.failures, failures,)
+    assert compare_lists_of_failures(
+        result.failures,
+        failures,
+    )
 
 
 def test_rule_supports_filter_config(sns_topic_with_wildcards_fixture, default_allow_all_config):

diff --git a/tests/test_boto3_client.py b/tests/test_boto3_client.py
@@ -248,7 +248,12 @@ def test_urlencoded_url(s3_bucket, boto3_client):
 @pytest.mark.parametrize(
     "aws_responses, expected_exports, mocked_warning_logs, mocked_exceptions",
     [
-        ([[{"Name": "A", "Value": "a"}]], {"A": "a"}, [], [],),
+        (
+            [[{"Name": "A", "Value": "a"}]],
+            {"A": "a"},
+            [],
+            [],
+        ),
         (
             [[{"Foo": "Bar"}], [{"Name": "A", "Value": "a"}]],
             {"A": "a"},

diff --git a/tests/test_cli.py b/tests/test_cli.py
@@ -9,10 +9,12 @@
 
 
 @pytest.mark.parametrize(
-    "aws_account_id_arg, validation_result", [(None, None), ("", None), ("123456789012", "123456789012")],
+    "aws_account_id_arg, validation_result",
+    [(None, None), ("", None), ("123456789012", "123456789012")],
 )
 def test_validate_aws_account_id(
-    aws_account_id_arg, validation_result,
+    aws_account_id_arg,
+    validation_result,
 ):
     fake_command = click.Command("fake_command")
     fake_context = click.Context(fake_command)
@@ -42,7 +44,8 @@ def test_validate_aws_account_id_with_malformed_arg():
     ],
 )
 def test_validate_aws_principals(
-    aws_principals_arg, validation_result,
+    aws_principals_arg,
+    validation_result,
 ):
     fake_command = click.Command("fake_command")
     fake_context = click.Context(fake_command)