Merge pull request #24 from eerkunt/sonarcloud_integration

SonarCloud integration and general code optimisation.

.travis.yml

@@ -1,15 +1,25 @@
 language: python
 python:
   - '2.7'
+services:
+  - docker
 cache:
   pip: true
-  custom_install: true
 jobs:
   include:
-    - stage: Test
+    - stage: Tests
       provider: script
       script: py.test -v
 
+    - stage: Code Quality
+      addons:
+        sonarcloud:
+          organization: "eerkunt-github"
+          token: $SONAR_LOGIN
+      script:
+        - "echo 'Scanning source code with SonarCloud'"
+        - sonar-scanner -Dsonar.projectKey=terraform-compliance -Dsonar.organization=eerkunt-github -Dsonar.sources=. -Dsonar.host.url=https://sonarcloud.io -Dsonar.login=$SONAR_LOGIN
+
     - stage: Build & Deploy (PYPI)
       script: "echo 'PYPI Build & Deploy'"
       if: branch = master
@@ -33,7 +43,7 @@ jobs:
         - echo "Release version identified as <$RELEASE_VERSION>"
         - cat /home/travis/build/eerkunt/terraform-compliance/terraform_compliance/main.py | grep -e "__version__ = \".*\"" | cut -d " " -f3 | tr -d "\""
         - if [ -z "$RELEASE_VERSION" ]; then echo "Can not identify the version!"; travis_terminate 1; fi
-        - sed s/__VERSION__/"$RELEASE_VERSION"/ Dockerfile.template > Dockerfile || travis_terminate 1
+        - sed s/__VERSION__/"$RELEASE_VERSION"/g Dockerfile.template > Dockerfile || travis_terminate 1
         - docker build --compress --no-cache -t "$IMAGE_NAME" . || travis_terminate 1
         - docker images || travis_terminate 1
         - docker login -u "$DOCKER_HUB_USER" -p "$DOCKER_HUB_PASS" || travis_terminate 1
@@ -61,4 +71,4 @@ jobs:
           name: $RELEASE_VERSION
           on:
             branch: master
-            fork: false
+            fork: false

Dockerfile.template

@@ -6,7 +6,7 @@ LABEL source="https://github.com/eerkunt/terraform-compliance"
 
 RUN  apt-get update && \
      apt-get install -y git && \
-     pip install terraform-compliance && \
+     pip install 'terraform-compliance==__VERSION__' && \
      pip uninstall -y radish radish-bdd && \
      pip install radish radish-bdd && \
      rm -rf /var/lib/apt/lists/* && \

setup.py

@@ -32,14 +32,7 @@
         ],
     },
     classifiers=[
-        # As from http://pypi.python.org/pypi?%3Aaction=list_classifiers
-        # 'Development Status :: 1 - Planning',
-        # 'Development Status :: 2 - Pre-Alpha',
-        # 'Development Status :: 3 - Alpha',
         'Development Status :: 4 - Beta',
-        # 'Development Status :: 5 - Production/Stable',
-        # 'Development Status :: 6 - Mature',
-        # 'Development Status :: 7 - Inactive',
         'Environment :: Console',
         'Intended Audience :: Developers',
         'License :: OSI Approved :: MIT License',
@@ -49,9 +42,6 @@
         'Operating System :: Microsoft :: Windows',
         'Programming Language :: Python',
         'Programming Language :: Python :: 2.7',
-        # 'Programming Language :: Python :: 3.4',
-        # 'Programming Language :: Python :: 3.5',
-        # 'Programming Language :: Python :: 3.6',
         'Topic :: Software Development :: Libraries :: Python Modules',
     ]
 )

terraform_compliance/common/helper.py

@@ -48,31 +48,16 @@ def check_if_cidr( value ):
     return False
 
 
-def is_ip_in_cidr( ip_cidr, cidr ):
-    return_value = False
-    cidr = eval(cidr)
-    if type(cidr) is list:
-        for ip_network in cidr:
-            if IPNetwork(ip_cidr) in IPNetwork(ip_network):
-                return_value = True
-    else:
-        if IPNetwork(ip_cidr) in IPNetwork(cidr):
-            return_value = True
+def is_ip_in_cidr(ip_cidr, cidr):
+    for ip_network in cidr:
+        if IPNetwork(ip_cidr) in IPNetwork(ip_network):
+            return True
 
-    return return_value
+    return False
 
 
 # A helper function that compares port related data with given dictionary
-def check_port_cidr_ranges(tf_conf, security_group, proto, port, cidr):
-    protocol = ''
-    from_port = 0
-    to_port = 0
-    cidr_blocks = None
-    giveError = False
-
-    # This is because of resource_mounting
-    #if 'referenced_name' in security_group:
-    #    return
+def check_sg_rules(tf_conf, security_group, proto, port, cidr):
 
     if 'cidr_blocks' in security_group:
         if type(security_group['cidr_blocks']) is list:
@@ -83,42 +68,44 @@ def check_port_cidr_ranges(tf_conf, security_group, proto, port, cidr):
             if not check_if_cidr(security_group['cidr_blocks']):
                 security_group['cidr_blocks'] = expand_variable(tf_conf, security_group['cidr_blocks'])['default']
 
-    #TODO: Add custom protocol support where HCL has numbers instead of protocol names like tcp, udp
-    #TODO: Add IP Range/Netmask check with the given CIDR.
-    for y in security_group:
-        if y == 'protocol':
-            protocol = [security_group[y]]
-            if protocol[0] == '-1':
-                protocol = ['tcp', 'udp']
 
-        if y == 'from_port' and security_group[y] > 0:
-            from_port = int(security_group[y])
+    validate_sg_rule(proto=proto, port=port, cidr=cidr, params=assign_sg_params(security_group))
+
+def assign_sg_params(rule):
+    from_port = int(rule.get('from_port', 0))
+    to_port = int(rule.get('to_port', 0))
+
+    protocol = [proto for proto in [rule.get('protocol', '-1')]]
 
-        if y == 'to_port' and security_group[y] > 0:
-            to_port = int(security_group[y])
+    # TODO: Make IANA Protocol numbers matching here.
+    # http://www.iana.org/assignments/protocol-numbers/protocol-numbers.xhtml
+    if protocol[0] == '-1' or type(protocol[0]) is int:
+        protocol = ['tcp', 'udp']
 
-        if y == 'cidr_blocks':
-            if type(security_group[y] is list):
-                cidr_blocks = str(security_group[y])
+    protocol[0] = protocol[0].lower()
 
-    if int(to_port) == 0 and int(from_port) == 0:
+    cidr_blocks = rule.get('cidr_blocks', [])
+
+    if type(cidr_blocks) is not list:
+        cidr_blocks = [cidr_blocks]
+
+    if to_port == 0 and from_port == 0:
         to_port = 65535
 
-    if int(to_port) > int(from_port):
-        if int(from_port) <= port <= int(to_port) and proto in protocol and is_ip_in_cidr(cidr, cidr_blocks):
-            giveError = True
-    elif int(from_port) > int(to_port):
-        if int(to_port) <= port <= int(from_port) or proto in protocol and is_ip_in_cidr(cidr, cidr_blocks):
-            giveError = True
-    elif int(from_port) == int(to_port):
-        if int(from_port) == port and proto in protocol and is_ip_in_cidr(cidr, cidr_blocks):
-            giveError = True
+    if from_port > to_port:
+        raise AssertionError('Invalid configuration from_port can not be bigger than to_port. {} > {} {} in {}'.format(from_port,
+                                                                                                                 to_port,
+                                                                                                                 protocol,
+                                                                                                                 cidr_blocks))
 
-    if giveError:
-        raise AssertionError('Found {}/{} in {}/{}-{} for {}'.format(proto, port, protocol, from_port,
-                                                                     to_port, cidr_blocks))
+    return dict(protocol=protocol, from_port=from_port, to_port=to_port, cidr_blocks=cidr_blocks)
 
 
+def validate_sg_rule(proto, port, cidr, params):
+    port = int(port)
+    if port >= params['from_port'] and port <= params['to_port'] and proto in params['protocol'] and is_ip_in_cidr(cidr, params['cidr_blocks']):
+        raise AssertionError('Found {}/{} in {}/{}-{} for {}'.format(proto, port, params['protocol'], params['from_port'], params['to_port'], params['cidr_blocks']))
+
 def change_value_in_dict(target_dictionary, path_to_change, value_to_change):
     if type(path_to_change) is str:
         path_to_change = path_to_change.split('.')
@@ -127,9 +114,6 @@ def change_value_in_dict(target_dictionary, path_to_change, value_to_change):
         return False
 
     path_to_adjust = '["{}"]'.format('"]["'.join(path_to_change))
-    path_to_check  = '["{}"]["type"]'.format('"]["'.join(path_to_change))
-    path_to_add    = '["{}"]'.format('"]["'.join(path_to_change[:-1]))
-
 
     try:
         target = eval('target_dictionary{}'.format(path_to_adjust))
@@ -142,8 +126,7 @@ def change_value_in_dict(target_dictionary, path_to_change, value_to_change):
 
                 if type_key not in target:
                     target[type_key] = list()
-                elif type_key in target:
-                    if type(target[type_key]) is not list:
+                elif type_key in target and type(target[type_key]) is not list:
                         target[type_key] = [target[type_key]]
 
                 target[type_key].append(source)

terraform_compliance/extensions/terraform_validate.py

@@ -32,17 +32,15 @@ def enable_resource_mounting(tf_conf, processing_resource=None, resource=None):
             if type(sub_value) is str or type(sub_value) is unicode:
                 matches = re.match(regex, sub_value)
 
-                if matches is not None:
-                    if not matches.group(1).startswith(('var', 'data', 'module')) and '(' not in matches.group(1):
-                        target = generate_target_resource(matches.group(1))
-                        change_value_in_dict(tf_conf, target, {source: processing_resource})
+                if matches is not None and not matches.group(1).startswith(('var', 'data', 'module')) and '(' not in matches.group(1):
+                    target = generate_target_resource(matches.group(1))
+                    change_value_in_dict(tf_conf, target, {source: processing_resource})
 
             elif type(sub_value) is list:
                 for value in sub_value:
                     if type(value) is str or type(value) is unicode:
                         matches = re.match(regex, value)
-                        if matches is not None:
-                            if not matches.group(1).startswith(('var', 'data', 'module')) and '(' not in matches.group(1):
+                        if matches is not None and not matches.group(1).startswith(('var', 'data', 'module')) and '(' not in matches.group(1):
                                 target = generate_target_resource(matches.group(1))
                                 change_value_in_dict(tf_conf, target, {source: processing_resource})
 

terraform_compliance/main.py

@@ -7,7 +7,7 @@
 from git import Repo
 
 __app_name__ = "terraform-compliance"
-__version__ = "0.3.5"
+__version__ = "0.3.6"
 
 
 class ArgHandling(object):
@@ -16,7 +16,7 @@ class ArgHandling(object):
 #TODO: Handle all directory/protocol handling via a better class structure here.
 #TODO: Extend git: (on features or tf files argument) into native URLs instead of using a prefix here.
 
-class readable_dir(Action):
+class ReadableDir(Action):
     def __call__(self, parser, namespace, values, option_string=None):
         prospective_dir = values
 
@@ -44,9 +44,9 @@ def cli():
     argument = ArgHandling()
     parser = ArgumentParser(prog=__app_name__,
                             description="BDD Test Framework for Hashicorp terraform")
-    parser.add_argument("--features", "-f", dest="features", metavar='feature_directory', action=readable_dir,
+    parser.add_argument("--features", "-f", dest="features", metavar='feature_directory', action=ReadableDir,
                         help="Directory consists of BDD features", required=True)
-    parser.add_argument("--tfdir", "-t", dest="tf_dir", metavar='terraform_directory', action=readable_dir,
+    parser.add_argument("--tfdir", "-t", dest="tf_dir", metavar='terraform_directory', action=ReadableDir,
                         help="Directory (or git repository with 'git:' prefix) consists of Terraform Files", required=True)
     parser.add_argument("--version", "-v", action="version", version=__version__)
 

terraform_compliance/steps/steps.py

@@ -2,7 +2,7 @@
 
 from radish import step, world, custom_type, then, when, given
 from terraform_compliance.steps import untaggable_resources, regex, resource_name, encryption_property
-from terraform_compliance.common.helper import check_port_cidr_ranges
+from terraform_compliance.common.helper import check_sg_rules
 from terraform_compliance.extensions.terraform_validate import normalise_tag_values
 
 
@@ -101,16 +101,13 @@ def func(step):
 
 @step(u'it must not have {proto} protocol and port {port:d} for {cidr:ANY}')
 def func(step, proto, port, cidr):
-    if not step.context.resources.resource_list:
-        return
-
     proto = str(proto)
     port = int(port)
     cidr = str(cidr)
 
     for item in step.context.resources.properties:
         if type(item.property_value) is list:
             for security_group in item.property_value:
-                check_port_cidr_ranges(world.config.terraform.terraform_config, security_group, proto, port, cidr)
+                check_sg_rules(world.config.terraform.terraform_config, security_group, proto, port, cidr)
         else:
-            check_port_cidr_ranges(world.config.terraform.terraform_config, item.property_value, proto, port, cidr)
+            check_sg_rules(world.config.terraform.terraform_config, item.property_value, proto, port, cidr)

tests/mocks.py

@@ -219,5 +219,17 @@ class MockedData(object):
         }
     }
 
-    # check_port_cidr_ranges() tests
-    plain_security_group = ''
+    # security_groups
+    sg_ssh_with_2_cidrs = {u'to_port': 22, u'cidr_blocks': [u'213.86.221.35/32', u'195.99.231.117/32'], u'from_port': 22, u'protocol': u'tcp'}
+    sg_ssh_with_2_cidrs_any_proto = {u'to_port': 22, u'cidr_blocks': [u'213.86.221.35/32', u'195.99.231.117/32'], u'from_port': 22, u'protocol': u'-1'}
+    sg_ssh_with_all_ips = {u'to_port': 22, u'cidr_blocks': [u'0.0.0.0/0'], u'from_port': 22, u'protocol': u'tcp'}
+    sg_all_port_all_ip = {u'to_port': 0, u'cidr_blocks': [u'0.0.0.0/0'], u'from_port': 0, u'protocol': u'tcp'}
+    sg_all_port_no_ip = {}
+    sg_invalid = {u'to_port': 1, u'from_port': 2}
+
+
+    # refined sg_params
+    sg_params_ssh_with_2_cidrs = dict(protocol=['tcp'], from_port=22, to_port=22, cidr_blocks=['213.86.221.35/32', '195.99.231.117/32'])
+    sg_params_ssh_with_2_cidrs_any_proto = dict(protocol=['tcp', 'udp'], from_port=22, to_port=22, cidr_blocks=['213.86.221.35/32', '195.99.231.117/32'])
+    sg_params_all_port_all_ip = dict(protocol=['tcp'], from_port=0, to_port=65535, cidr_blocks=['0.0.0.0/0'])
+    sg_params_all_port_no_ip = dict(protocol=['tcp', 'udp'], from_port=0, to_port=65535, cidr_blocks=[])

tests/terraform_compliance/common/test_helper.py

@@ -1,5 +1,9 @@
 from unittest import TestCase
-from terraform_compliance.common.helper import flatten_list, check_port_cidr_ranges
+from terraform_compliance.common.helper import (
+    flatten_list,
+    assign_sg_params,
+    validate_sg_rule
+)
 from tests.mocks import MockedData
 from copy import deepcopy
 
@@ -24,17 +28,47 @@ def test_flatten_multi_dimensional_nested_list(self):
 
         self.assertEqual(flatten_list(a), b)
 
-    def test_check_port_cidr_ranges_port_fail(self):
+    def test_assign_sg_params_one_port_with_two_cidrs(self):
+        self.assertEqual(MockedData.sg_params_ssh_with_2_cidrs, assign_sg_params(MockedData.sg_ssh_with_2_cidrs))
+
+    def test_assign_sg_params_one_port_two_cidrs_any_proto(self):
+        self.assertEqual(MockedData.sg_params_ssh_with_2_cidrs_any_proto, assign_sg_params(MockedData.sg_ssh_with_2_cidrs_any_proto))
+
+    def test_assign_sg_params_all_ports_with_all_ips(self):
+        self.assertEqual(MockedData.sg_params_all_port_all_ip, assign_sg_params(MockedData.sg_all_port_all_ip))
+
+    def test_assign_sg_params_no_data_given_in_rules(self):
+        self.assertEqual(MockedData.sg_params_all_port_no_ip, assign_sg_params(MockedData.sg_all_port_no_ip))
+
+    def test_assign_sg_params_from_port_bigger_than_to_port(self):
+        with self.assertRaises(AssertionError) as context:
+            assign_sg_params(MockedData.sg_invalid)
+
+            self.assertTrue('Invalid configuration from_port can not be bigger than to_port.' in context.exception)
+
+    def test_validate_sg_rule_port_found_in_cidr(self):
+        with self.assertRaises(AssertionError) as context:
+            validate_sg_rule('tcp', '22', '0.0.0.0/0', MockedData.sg_params_all_port_all_ip)
+
+            self.assertTrue('Found' in context.exception)
+
+    def test_validate_sg_rule_port_found_but_cidr_is_different(self):
+        pass
+
+    def test_validate_sg_rule_port_found_but_proto_is_different(self):
+        pass
+
+    def test_check_sg_rules_fail(self):
         pass
 
-    def test_check_port_cidr_ranges_port_passed_because_of_different_protocol(self):
+    def test_check_sg_rules_passed_because_of_different_protocol(self):
         pass
 
-    def test_check_port_cidr_ranges_port_fail_and_protocol_number_is_used(self):
+    def test_check_sg_rules_fail_and_protocol_number_is_used(self):
         pass
 
-    def test_check_port_cidr_ranges_port_not_fail_because_of_cidr(self):
+    def test_check_sg_rules_not_fail_because_of_cidr(self):
         pass
 
-    def test_check_port_cidr_ranges_port_fail_because_of_given_ip_is_a_member_of_cidr(self):
+    def test_check_sg_rules_fail_because_of_given_ip_is_a_member_of_cidr(self):
         pass