Skip to content

Commit 33f4a89

Browse files
eerkunteerkunt
authored
Merge branch 'master' into dependabot/pip/ddt-1.4.1
2 parents 8b969af + 270c96a commit 33f4a89

File tree

12 files changed

+102
-40
lines changed

12 files changed

+102
-40
lines changed

‎CHANGELOG.md

+7
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,12 @@
11
# CHANGELOG
22

3+
## 1.2.3 (2020-05-25)
4+
* Fixed a crash where some module outputs could not be processed. ([#275](https://github.com/eerkunt/terraform-compliance/issues/275))
5+
6+
## 1.2.2 (2020-05-24)
7+
* 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))
8+
* Fixed an issue where regular expression usage on CIDR steps was causing a problem. ([#265](https://github.com/eerkunt/terraform-compliance/issues/265))
9+
310
## 1.2.1 (2020-05-19)
411
* Fixed a problem where properties having a space character were not recognised.
512
* 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))

‎requirements.txt

+1-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ colorful==0.5.4
66
filetype==1.0.7
77
junit-xml==1.9
88
emoji==0.5.4
9-
lxml==4.5.0
9+
lxml==4.5.1
1010
ddt==1.4.1
1111
pytest==5.4.2
1212
nose==1.3.7

‎terraform_compliance/common/helper.py

+14-21
Original file line numberDiff line numberDiff line change
@@ -28,33 +28,26 @@ def flatten(items):
2828

2929

3030
def check_if_cidr(value):
31-
regex = r'(1[0-9][0-9]|2[0-4][0-9]|25[0-5]|[0-9][0-9]|[0-9])' \
32-
r'\.' \
33-
r'(1[0-9][0-9]|2[0-4][0-9]|25[0-5]|[0-9][0-9]|[0-9])' \
34-
r'\.' \
35-
r'(1[0-9][0-9]|2[0-4][0-9]|25[0-5]|[0-9][0-9]|[0-9])' \
36-
r'\.' \
37-
r'(1[0-9][0-9]|2[0-4][0-9]|25[0-5]|[0-9][0-9]|[0-9])' \
38-
r'\/' \
39-
r'(3[0-2]|2[0-9]|1[0-9]|[0-9])'
40-
matches = re.match(regex, value)
41-
42-
if matches is not None:
43-
return True
44-
45-
# Check if the CIDR is a regex
46-
is_ip_regex = r'^([0-9\.]+)$'
47-
if re.match(is_ip_regex, value) is not None:
31+
try:
32+
re.compile(value)
4833
return True
49-
50-
return False
34+
except TypeError as e:
35+
return False
5136

5237

5338
def is_ip_in_cidr(ip_cidr, cidr):
54-
is_ip_regex = r'^([0-9\.]+)$'
39+
is_ip_regex = r'(1[0-9][0-9]|2[0-4][0-9]|25[0-5]|[0-9][0-9]|[0-9])' \
40+
r'\.' \
41+
r'(1[0-9][0-9]|2[0-4][0-9]|25[0-5]|[0-9][0-9]|[0-9])' \
42+
r'\.' \
43+
r'(1[0-9][0-9]|2[0-4][0-9]|25[0-5]|[0-9][0-9]|[0-9])' \
44+
r'\.' \
45+
r'(1[0-9][0-9]|2[0-4][0-9]|25[0-5]|[0-9][0-9]|[0-9])' \
46+
r'\/' \
47+
r'(3[0-2]|2[0-9]|1[0-9]|[0-9])'
5548

5649
# IP is a not a regex string
57-
if re.match(is_ip_regex, ip_cidr) is None:
50+
if re.match(is_ip_regex, ip_cidr) is not None:
5851
for ip_network in cidr:
5952
if check_if_cidr(ip_cidr) and check_if_cidr(ip_network) and IPNetwork(ip_cidr) in IPNetwork(ip_network):
6053
return True

‎terraform_compliance/extensions/ext_radish_bdd.py

+4-4
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ def step_condition(step):
4444
return current_condition
4545

4646

47-
@custom_type("ANY", r"[\"'\.\/_\-A-Za-z0-9\s:]+")
47+
@custom_type("ANY", r".+")
4848
def custom_type_any(text):
4949
return text.replace('"', '').replace('\'', '')
5050

@@ -54,9 +54,9 @@ def custom_type_any(text):
5454
def custom_type_prop(text):
5555
return text.replace('"', '').replace('\'', '')
5656

57-
@custom_type("PROPERTY_COMPAT", r"([\*\.\/_\-A-Za-z0-9:\(\)\[\]\']+\s[\*\.\/_\-A-Za-z0-9:\(\)\[\]\']+)|"
58-
r"(\"[\s\*\.\/_\-A-Za-z0-9:\(\)\[\]\']+\")|"
59-
r"([\*\.\/_\-A-Za-z0-9:\(\)\[\]\']+)")
57+
@custom_type("PROPERTY_COMPAT", r"([\*\.\/_\-A-Za-z0-9:\(\)\[\]\']+\s[\*\.\/_\-A-Za-z0-9:\(\)\[\]\']+$)|"
58+
r"(\"[\s\*\.\/_\-A-Za-z0-9:\(\)\[\]\']+\"$)|"
59+
r"([\*\.\/_\-A-Za-z0-9:\(\)\[\]\']+$)")
6060
def custom_type_prop(text):
6161
return text.replace('"', '').replace('\'', '')
6262

‎terraform_compliance/extensions/terraform.py

+4-2
Original file line numberDiff line numberDiff line change
@@ -237,7 +237,7 @@ def _mount_resources(self, source, target, ref_type):
237237
self.resources[target_resource][Defaults.r_mount_addr_ptr_list].extend(source)
238238

239239
if parameter not in self.resources[source_resource]['values']:
240-
self.resources[source_resource]['values'][parameter] = resource
240+
self.resources[source_resource]['values'][parameter] = target_resource
241241

242242
def _find_resource_from_name(self, resource_name):
243243
'''
@@ -258,7 +258,9 @@ def _find_resource_from_name(self, resource_name):
258258
module = self.raw['configuration']['root_module'].get('module_calls', {}).get(module_name, {})
259259

260260
output_value = module.get('module', {}).get('outputs', {}).get(output_id, {})
261-
resources = output_value.get('expression', {}).get('references') if 'expression' in output_value else output_value.get('value', [])
261+
262+
resources = output_value.get('expression', {}).get('references', []) if 'expression' in output_value else output_value.get('value', [])
263+
262264
resources = ['{}.{}.{}'.format(resource_type, module_name, res) for res in resources]
263265

264266
if resources:

‎terraform_compliance/steps/steps.py

+6-5
Original file line numberDiff line numberDiff line change
@@ -104,13 +104,19 @@ def wrapper(_step_obj, reference_address):
104104
return it_must_have_reference_address_referenced(_step_obj, reference_address)
105105

106106

107+
@then(u'it {condition:ANY} have {proto} protocol and port {port} for {cidr:ANY}')
108+
def wrapper(_step_obj, condition, proto, port, cidr):
109+
return it_condition_have_proto_protocol_and_port_port_for_cidr(_step_obj, condition, proto, port, cidr)
110+
111+
107112
@then(u'it must contain {something:PROPERTY_COMPAT}')
108113
@then(u'it must have {something:PROPERTY_COMPAT}')
109114
@then(u'they must contain {something:PROPERTY_COMPAT}')
110115
@then(u'they must have {something:PROPERTY_COMPAT}')
111116
def wrapper(_step_obj, something, inherited_values=Null):
112117
return it_must_contain_something(_step_obj, something, inherited_values=Null)
113118

119+
114120
@then(u'it must not contain {something:PROPERTY_COMPAT}')
115121
@then(u'they must not contain {something:PROPERTY_COMPAT}')
116122
@then(u'it must not have {something:PROPERTY_COMPAT}')
@@ -125,11 +131,6 @@ def wrapper(_step_obj, something):
125131
return property_is_enabled(_step_obj, something)
126132

127133

128-
@then(u'it {condition:ANY} have {proto:ANY} protocol and port {port} for {cidr:ANY}')
129-
def wrapper(_step_obj, condition, proto, port, cidr):
130-
return it_condition_have_proto_protocol_and_port_port_for_cidr(_step_obj, condition, proto, port, cidr)
131-
132-
133134
@when(u'I {action_type:PROPERTY} it')
134135
@when(u'I {action_type:PROPERTY} them')
135136
@when(u'I {action_type:PROPERTY} the value')

‎tests/functional/test_issue-216/.expected

+1-1
Original file line numberDiff line numberDiff 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\.

‎tests/functional/test_issue-260/test.feature

+2-1
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,5 @@ Feature: test
22

33
Scenario: This is for constant values
44
Given I have aws_sqs_queue resource configured
5-
Then it must contain kms_master_key_id
5+
Then it must contain kms_master_key_id
6+
And its value must not be null

‎tests/functional/test_issue-275/plan.out.json

+1
Original file line numberDiff line numberDiff 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)"}}}}}}}}

‎tests/functional/test_issue-275/test.feature

+7
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
Feature: The scenarios defined below
2+
require an MR review if any
3+
of the resources below exist
4+
in a plan
5+
6+
Scenario: Changes to Security Group
7+
Given I have aws_security_group defined

‎tests/terraform_compliance/common/test_helper.py

+4-5
Original file line numberDiff line numberDiff line change
@@ -45,11 +45,10 @@ def test_check_if_cidr_success(self):
4545
self.assertTrue(check_if_cidr('10.0.0.7/32'))
4646

4747
def test_check_if_cidr_failure(self):
48-
self.assertFalse(check_if_cidr('256.0.0.0/8'))
49-
self.assertFalse(check_if_cidr('10.256.0.0/16'))
50-
self.assertFalse(check_if_cidr('10.0.256.0/24'))
51-
self.assertFalse(check_if_cidr('10.0.0.256/32'))
52-
self.assertFalse(check_if_cidr('10.0.0.256/33'))
48+
self.assertFalse(check_if_cidr(123))
49+
self.assertFalse(check_if_cidr(False))
50+
self.assertFalse(check_if_cidr([]))
51+
self.assertFalse(check_if_cidr({}))
5352

5453
def test_is_ip_in_cidr_success(self):
5554
self.assertTrue(is_ip_in_cidr('10.0.0.0/8', ['0.0.0.0/0']))

‎tests/terraform_compliance/extensions/test_terraform.py

+51
Original file line numberDiff line numberDiff line change
@@ -337,6 +337,57 @@ def test_find_resource_from_name_resource_name_in_child_resources(self, *args):
337337
}
338338
self.assertEqual(obj._find_resource_from_name('resource_type.resource_id'), ['a'])
339339

340+
@patch.object(TerraformParser, '_read_file', return_value={})
341+
def test_find_resource_from_name_resource_name_in_module_outputs_expression_references(self, *args):
342+
obj = TerraformParser('somefile', parse_it=False)
343+
obj.raw['configuration'] = {
344+
"root_module": {
345+
"module_calls": {
346+
"module_name": {
347+
"source": "../../module-test",
348+
"module": {
349+
"outputs": {
350+
"my_expression": {
351+
"expression": {
352+
"references": [
353+
"var.example"
354+
]
355+
}
356+
}
357+
}
358+
}
359+
}
360+
}
361+
}
362+
}
363+
self.assertEqual(obj._find_resource_from_name('module.module_name.my_expression'), ['module.module_name.var.example'])
364+
365+
@patch.object(TerraformParser, '_read_file', return_value={})
366+
def test_find_resource_from_name_resource_name_in_module_outputs_expression(self, *args):
367+
obj = TerraformParser('somefile', parse_it=False)
368+
obj.raw['configuration'] = {
369+
"root_module": {
370+
"module_calls": {
371+
"module_name": {
372+
"source": "../../module-test",
373+
"module": {
374+
"outputs": {
375+
"my_expression": {
376+
"expression": {
377+
"constant_value": [
378+
"10.123.0.0/16"
379+
]
380+
},
381+
"description": "List of cidr(s)"
382+
}
383+
}
384+
}
385+
}
386+
}
387+
}
388+
}
389+
self.assertEqual(obj._find_resource_from_name('module.module_name.my_expression'), [])
390+
340391
@patch.object(TerraformParser, '_read_file', return_value={})
341392
def test_distribute_providers(self, *args):
342393
obj = TerraformParser('somefile', parse_it=False)

0 commit comments

Comments
 (0)