Merge branch 'master' into dependabot/pip/lxml-4.5.1

eerkunt · web-flow · commit 7856a5c33289 · 2020-05-25T11:21:02.000+01:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,12 @@
 # CHANGELOG
 
+## 1.2.3 (2020-05-25)
+* Fixed a crash where some module outputs could not be processed. ([#275](https://github.com/eerkunt/terraform-compliance/issues/275))
+
+## 1.2.2 (2020-05-24)
+* Improved resource mounting where some terraform providers were creating inconsistent plan output and omitted some parameters that are referenced to a dynamic resource. ([#260](https://github.com/eerkunt/terraform-compliance/issues/260))
+* Fixed an issue where regular expression usage on CIDR steps was causing a problem. ([#265](https://github.com/eerkunt/terraform-compliance/issues/265))
+
 ## 1.2.1 (2020-05-19)
 * Fixed a problem where properties having a space character were not recognised.
 * Optimised key/value (property) definitions on all steps, where all keys or values can also have space characters encapsulated within "". ([#270](https://github.com/eerkunt/terraform-compliance/issues/270))
diff --git a/terraform_compliance/common/helper.py b/terraform_compliance/common/helper.py
@@ -28,33 +28,26 @@ def flatten(items):
 
 
 def check_if_cidr(value):
-    regex = r'(1[0-9][0-9]|2[0-4][0-9]|25[0-5]|[0-9][0-9]|[0-9])' \
-            r'\.' \
-            r'(1[0-9][0-9]|2[0-4][0-9]|25[0-5]|[0-9][0-9]|[0-9])' \
-            r'\.' \
-            r'(1[0-9][0-9]|2[0-4][0-9]|25[0-5]|[0-9][0-9]|[0-9])' \
-            r'\.' \
-            r'(1[0-9][0-9]|2[0-4][0-9]|25[0-5]|[0-9][0-9]|[0-9])' \
-            r'\/' \
-            r'(3[0-2]|2[0-9]|1[0-9]|[0-9])'
-    matches = re.match(regex, value)
-
-    if matches is not None:
-        return True
-
-    # Check if the CIDR is a regex
-    is_ip_regex = r'^([0-9\.]+)$'
-    if re.match(is_ip_regex, value) is not None:
+    try:
+        re.compile(value)
         return True
-
-    return False
+    except TypeError as e:
+        return False
 
 
 def is_ip_in_cidr(ip_cidr, cidr):
-    is_ip_regex = r'^([0-9\.]+)$'
+    is_ip_regex = r'(1[0-9][0-9]|2[0-4][0-9]|25[0-5]|[0-9][0-9]|[0-9])' \
+                  r'\.' \
+                  r'(1[0-9][0-9]|2[0-4][0-9]|25[0-5]|[0-9][0-9]|[0-9])' \
+                  r'\.' \
+                  r'(1[0-9][0-9]|2[0-4][0-9]|25[0-5]|[0-9][0-9]|[0-9])' \
+                  r'\.' \
+                  r'(1[0-9][0-9]|2[0-4][0-9]|25[0-5]|[0-9][0-9]|[0-9])' \
+                  r'\/' \
+                  r'(3[0-2]|2[0-9]|1[0-9]|[0-9])'
 
     # IP is a not a regex string
-    if re.match(is_ip_regex, ip_cidr) is None:
+    if re.match(is_ip_regex, ip_cidr) is not None:
         for ip_network in cidr:
             if check_if_cidr(ip_cidr) and check_if_cidr(ip_network) and IPNetwork(ip_cidr) in IPNetwork(ip_network):
                 return True
diff --git a/terraform_compliance/extensions/ext_radish_bdd.py b/terraform_compliance/extensions/ext_radish_bdd.py
@@ -44,7 +44,7 @@ def step_condition(step):
     return current_condition
 
 
-@custom_type("ANY", r"[\"'\.\/_\-A-Za-z0-9\s:]+")
+@custom_type("ANY", r".+")
 def custom_type_any(text):
     return text.replace('"', '').replace('\'', '')
 
@@ -54,9 +54,9 @@ def custom_type_any(text):
 def custom_type_prop(text):
     return text.replace('"', '').replace('\'', '')
 
-@custom_type("PROPERTY_COMPAT", r"([\*\.\/_\-A-Za-z0-9:\(\)\[\]\']+\s[\*\.\/_\-A-Za-z0-9:\(\)\[\]\']+)|"
-                                r"(\"[\s\*\.\/_\-A-Za-z0-9:\(\)\[\]\']+\")|"
-                                r"([\*\.\/_\-A-Za-z0-9:\(\)\[\]\']+)")
+@custom_type("PROPERTY_COMPAT", r"([\*\.\/_\-A-Za-z0-9:\(\)\[\]\']+\s[\*\.\/_\-A-Za-z0-9:\(\)\[\]\']+$)|"
+                                r"(\"[\s\*\.\/_\-A-Za-z0-9:\(\)\[\]\']+\"$)|"
+                                r"([\*\.\/_\-A-Za-z0-9:\(\)\[\]\']+$)")
 def custom_type_prop(text):
     return text.replace('"', '').replace('\'', '')
 
diff --git a/terraform_compliance/extensions/terraform.py b/terraform_compliance/extensions/terraform.py
@@ -237,7 +237,7 @@ def _mount_resources(self, source, target, ref_type):
                         self.resources[target_resource][Defaults.r_mount_addr_ptr_list].extend(source)
 
                     if parameter not in self.resources[source_resource]['values']:
-                        self.resources[source_resource]['values'][parameter] = resource
+                        self.resources[source_resource]['values'][parameter] = target_resource
 
     def _find_resource_from_name(self, resource_name):
         '''
@@ -258,7 +258,9 @@ def _find_resource_from_name(self, resource_name):
             module = self.raw['configuration']['root_module'].get('module_calls', {}).get(module_name, {})
 
             output_value = module.get('module', {}).get('outputs', {}).get(output_id, {})
-            resources = output_value.get('expression', {}).get('references') if 'expression' in output_value else output_value.get('value', [])
+
+            resources = output_value.get('expression', {}).get('references', []) if 'expression' in output_value else output_value.get('value', [])
+
             resources = ['{}.{}.{}'.format(resource_type, module_name, res) for res in resources]
 
             if resources:
diff --git a/terraform_compliance/steps/steps.py b/terraform_compliance/steps/steps.py
@@ -104,13 +104,19 @@ def wrapper(_step_obj, reference_address):
     return it_must_have_reference_address_referenced(_step_obj, reference_address)
 
 
+@then(u'it {condition:ANY} have {proto} protocol and port {port} for {cidr:ANY}')
+def wrapper(_step_obj, condition, proto, port, cidr):
+    return it_condition_have_proto_protocol_and_port_port_for_cidr(_step_obj, condition, proto, port, cidr)
+
+
 @then(u'it must contain {something:PROPERTY_COMPAT}')
 @then(u'it must have {something:PROPERTY_COMPAT}')
 @then(u'they must contain {something:PROPERTY_COMPAT}')
 @then(u'they must have {something:PROPERTY_COMPAT}')
 def wrapper(_step_obj, something, inherited_values=Null):
     return it_must_contain_something(_step_obj, something, inherited_values=Null)
 
+
 @then(u'it must not contain {something:PROPERTY_COMPAT}')
 @then(u'they must not contain {something:PROPERTY_COMPAT}')
 @then(u'it must not have {something:PROPERTY_COMPAT}')
@@ -125,11 +131,6 @@ def wrapper(_step_obj, something):
     return property_is_enabled(_step_obj, something)
 
 
-@then(u'it {condition:ANY} have {proto:ANY} protocol and port {port} for {cidr:ANY}')
-def wrapper(_step_obj, condition, proto, port, cidr):
-    return it_condition_have_proto_protocol_and_port_port_for_cidr(_step_obj, condition, proto, port, cidr)
-
-
 @when(u'I {action_type:PROPERTY} it')
 @when(u'I {action_type:PROPERTY} them')
 @when(u'I {action_type:PROPERTY} the value')
diff --git a/tests/functional/test_issue-216/.expected b/tests/functional/test_issue-216/.expected
@@ -1 +1 @@
-Failure: tcp\/443 port is not defined within 192\. network in aws_security_group_rule\.ecs_task_egress_to_internet_443\.
+Failure: tcp\/443 port is not defined within 192\.\* network in aws_security_group_rule\.ecs_task_egress_to_internet_443\.
diff --git a/tests/functional/test_issue-260/test.feature b/tests/functional/test_issue-260/test.feature
@@ -2,4 +2,5 @@ Feature: test
 
   Scenario: This is for constant values
     Given I have aws_sqs_queue resource configured
-    Then it must contain kms_master_key_id
+    Then it must contain kms_master_key_id
+    And its value must not be null
diff --git a/tests/functional/test_issue-275/plan.out.json b/tests/functional/test_issue-275/plan.out.json
@@ -0,0 +1 @@
+{"format_version":"0.1","terraform_version":"0.12.13","planned_values":{"root_module":{"resources":[{"address":"aws_security_group.security_group","mode":"managed","type":"aws_security_group","name":"security_group","provider_name":"aws","schema_version":1,"values":{"description":"Test Security Group","name":"security_group","name_prefix":null,"revoke_rules_on_delete":false,"tags":null,"timeouts":null,"vpc_id":"vpc-123456"}},{"address":"aws_security_group_rule.ingress_rule","mode":"managed","type":"aws_security_group_rule","name":"ingress_rule","provider_name":"aws","schema_version":2,"values":{"cidr_blocks":["10.123.0.0/16"],"description":null,"from_port":443,"ipv6_cidr_blocks":null,"prefix_list_ids":null,"protocol":"tcp","self":false,"to_port":443,"type":"ingress"}}]}},"resource_changes":[{"address":"aws_security_group.security_group","mode":"managed","type":"aws_security_group","name":"security_group","provider_name":"aws","change":{"actions":["create"],"before":null,"after":{"description":"Test Security Group","name":"security_group","name_prefix":null,"revoke_rules_on_delete":false,"tags":null,"timeouts":null,"vpc_id":"vpc-123456"},"after_unknown":{"arn":true,"egress":true,"id":true,"ingress":true,"owner_id":true}}},{"address":"aws_security_group_rule.ingress_rule","mode":"managed","type":"aws_security_group_rule","name":"ingress_rule","provider_name":"aws","change":{"actions":["create"],"before":null,"after":{"cidr_blocks":["10.123.0.0/16"],"description":null,"from_port":443,"ipv6_cidr_blocks":null,"prefix_list_ids":null,"protocol":"tcp","self":false,"to_port":443,"type":"ingress"},"after_unknown":{"cidr_blocks":[false],"id":true,"security_group_id":true,"source_security_group_id":true}}}],"configuration":{"provider_config":{"aws":{"name":"aws","version_constraint":"\u003e= 2.58","expressions":{"max_retries":{"constant_value":"20"},"profile":{"constant_value":"lab"},"region":{"constant_value":"us-east-1"},"skip_credentials_validation":{"constant_value":true},"skip_get_ec2_platforms":{"constant_value":true},"skip_metadata_api_check":{"constant_value":true},"skip_region_validation":{"constant_value":true},"skip_requesting_account_id":{"constant_value":true}}},"flywheel":{"name":"flywheel","version_constraint":"\u003e= 0.1.8"}},"root_module":{"resources":[{"address":"aws_security_group.security_group","mode":"managed","type":"aws_security_group","name":"security_group","provider_config_key":"aws","expressions":{"description":{"constant_value":"Test Security Group"},"name":{"constant_value":"security_group"},"vpc_id":{"constant_value":"vpc-123456"}},"schema_version":1},{"address":"aws_security_group_rule.ingress_rule","mode":"managed","type":"aws_security_group_rule","name":"ingress_rule","provider_config_key":"aws","expressions":{"cidr_blocks":{"references":["module.cidrtest.cidr"]},"from_port":{"constant_value":"443"},"protocol":{"constant_value":"tcp"},"security_group_id":{"references":["aws_security_group.security_group"]},"to_port":{"constant_value":"443"},"type":{"constant_value":"ingress"}},"schema_version":2}],"module_calls":{"cidrtest":{"source":"../../module-test","module":{"outputs":{"cidr":{"expression":{"constant_value":["10.123.0.0/16"]},"description":"List of cidr(s)"}}}}}}}}
diff --git a/tests/functional/test_issue-275/test.feature b/tests/functional/test_issue-275/test.feature
@@ -0,0 +1,7 @@
+Feature: The scenarios defined below
+	 require an MR review if any 
+	 of the resources below exist 
+	 in a plan 
+
+Scenario: Changes to Security Group
+    Given I have aws_security_group defined
diff --git a/tests/terraform_compliance/common/test_helper.py b/tests/terraform_compliance/common/test_helper.py
@@ -45,11 +45,10 @@ def test_check_if_cidr_success(self):
         self.assertTrue(check_if_cidr('10.0.0.7/32'))
 
     def test_check_if_cidr_failure(self):
-        self.assertFalse(check_if_cidr('256.0.0.0/8'))
-        self.assertFalse(check_if_cidr('10.256.0.0/16'))
-        self.assertFalse(check_if_cidr('10.0.256.0/24'))
-        self.assertFalse(check_if_cidr('10.0.0.256/32'))
-        self.assertFalse(check_if_cidr('10.0.0.256/33'))
+        self.assertFalse(check_if_cidr(123))
+        self.assertFalse(check_if_cidr(False))
+        self.assertFalse(check_if_cidr([]))
+        self.assertFalse(check_if_cidr({}))
 
     def test_is_ip_in_cidr_success(self):
         self.assertTrue(is_ip_in_cidr('10.0.0.0/8', ['0.0.0.0/0']))
diff --git a/tests/terraform_compliance/extensions/test_terraform.py b/tests/terraform_compliance/extensions/test_terraform.py
@@ -337,6 +337,57 @@ def test_find_resource_from_name_resource_name_in_child_resources(self, *args):
         }
         self.assertEqual(obj._find_resource_from_name('resource_type.resource_id'), ['a'])
 
+    @patch.object(TerraformParser, '_read_file', return_value={})
+    def test_find_resource_from_name_resource_name_in_module_outputs_expression_references(self, *args):
+        obj = TerraformParser('somefile', parse_it=False)
+        obj.raw['configuration'] = {
+            "root_module": {
+            "module_calls": {
+                "module_name": {
+                    "source": "../../module-test",
+                    "module": {
+                        "outputs": {
+                            "my_expression": {
+                                "expression": {
+                                    "references": [
+                                        "var.example"
+                                    ]
+                                }
+                            }
+                        }
+                    }
+                }
+            }
+            }
+        }
+        self.assertEqual(obj._find_resource_from_name('module.module_name.my_expression'), ['module.module_name.var.example'])
+
+    @patch.object(TerraformParser, '_read_file', return_value={})
+    def test_find_resource_from_name_resource_name_in_module_outputs_expression(self, *args):
+        obj = TerraformParser('somefile', parse_it=False)
+        obj.raw['configuration'] = {
+            "root_module": {
+            "module_calls": {
+                "module_name": {
+                    "source": "../../module-test",
+                    "module": {
+                        "outputs": {
+                            "my_expression": {
+                                "expression": {
+                                    "constant_value": [
+                                        "10.123.0.0/16"
+                                    ]
+                                },
+                                "description": "List of cidr(s)"
+                            }
+                        }
+                    }
+                }
+            }
+            }
+        }
+        self.assertEqual(obj._find_resource_from_name('module.module_name.my_expression'), [])
+
     @patch.object(TerraformParser, '_read_file', return_value={})
     def test_distribute_providers(self, *args):
         obj = TerraformParser('somefile', parse_it=False)

Original file line number	Diff line number	Diff line change
`@@ -1 +1 @@`
`1`		`-Failure: tcp\/443 port is not defined within 192\. network in aws_security_group_rule\.ecs_task_egress_to_internet_443\.`
	`1`	`+Failure: tcp\/443 port is not defined within 192\.\* network in aws_security_group_rule\.ecs_task_egress_to_internet_443\.`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	+{"format_version":"0.1","terraform_version":"0.12.13","planned_values":{"root_module":{"resources":[{"address":"aws_security_group.security_group","mode":"managed","type":"aws_security_group","name":"security_group","provider_name":"aws","schema_version":1,"values":{"description":"Test Security Group","name":"security_group","name_prefix":null,"revoke_rules_on_delete":false,"tags":null,"timeouts":null,"vpc_id":"vpc-123456"}},{"address":"aws_security_group_rule.ingress_rule","mode":"managed","type":"aws_security_group_rule","name":"ingress_rule","provider_name":"aws","schema_version":2,"values":{"cidr_blocks":["10.123.0.0/16"],"description":null,"from_port":443,"ipv6_cidr_blocks":null,"prefix_list_ids":null,"protocol":"tcp","self":false,"to_port":443,"type":"ingress"}}]}},"resource_changes":[{"address":"aws_security_group.security_group","mode":"managed","type":"aws_security_group","name":"security_group","provider_name":"aws","change":{"actions":["create"],"before":null,"after":{"description":"Test Security Group","name":"security_group","name_prefix":null,"revoke_rules_on_delete":false,"tags":null,"timeouts":null,"vpc_id":"vpc-123456"},"after_unknown":{"arn":true,"egress":true,"id":true,"ingress":true,"owner_id":true}}},{"address":"aws_security_group_rule.ingress_rule","mode":"managed","type":"aws_security_group_rule","name":"ingress_rule","provider_name":"aws","change":{"actions":["create"],"before":null,"after":{"cidr_blocks":["10.123.0.0/16"],"description":null,"from_port":443,"ipv6_cidr_blocks":null,"prefix_list_ids":null,"protocol":"tcp","self":false,"to_port":443,"type":"ingress"},"after_unknown":{"cidr_blocks":[false],"id":true,"security_group_id":true,"source_security_group_id":true}}}],"configuration":{"provider_config":{"aws":{"name":"aws","version_constraint":"\u003e= 2.58","expressions":{"max_retries":{"constant_value":"20"},"profile":{"constant_value":"lab"},"region":{"constant_value":"us-east-1"},"skip_credentials_validation":{"constant_value":true},"skip_get_ec2_platforms":{"constant_value":true},"skip_metadata_api_check":{"constant_value":true},"skip_region_validation":{"constant_value":true},"skip_requesting_account_id":{"constant_value":true}}},"flywheel":{"name":"flywheel","version_constraint":"\u003e= 0.1.8"}},"root_module":{"resources":[{"address":"aws_security_group.security_group","mode":"managed","type":"aws_security_group","name":"security_group","provider_config_key":"aws","expressions":{"description":{"constant_value":"Test Security Group"},"name":{"constant_value":"security_group"},"vpc_id":{"constant_value":"vpc-123456"}},"schema_version":1},{"address":"aws_security_group_rule.ingress_rule","mode":"managed","type":"aws_security_group_rule","name":"ingress_rule","provider_config_key":"aws","expressions":{"cidr_blocks":{"references":["module.cidrtest.cidr"]},"from_port":{"constant_value":"443"},"protocol":{"constant_value":"tcp"},"security_group_id":{"references":["aws_security_group.security_group"]},"to_port":{"constant_value":"443"},"type":{"constant_value":"ingress"}},"schema_version":2}],"module_calls":{"cidrtest":{"source":"../../module-test","module":{"outputs":{"cidr":{"expression":{"constant_value":["10.123.0.0/16"]},"description":"List of cidr(s)"}}}}}}}}