Merge pull request #899 from diginc/configureFirewall_unittest

Configure firewall unittest

automated install/basic-install.sh

@@ -1149,6 +1149,6 @@ main() {
 	echo "::: The install log is located at: /etc/pihole/install.log"
 }
 
-if [[ -z "$PHTEST" ]] ; then
+if [[ "${PH_TEST}" != true ]] ; then
     main "$@"
 fi

test/centos.Dockerfile

@@ -11,4 +11,6 @@ ENV PATH /usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:$SCRIPTDIR
 RUN true && \
     chmod +x $SCRIPTDIR/*
 
+ENV PH_TEST true
+
 #sed '/# Start the installer/Q' /opt/pihole/basic-install.sh > /opt/pihole/stub_basic-install.sh && \

test/conftest.py

@@ -7,7 +7,22 @@ check_output = testinfra.get_backend(
 
 @pytest.fixture
 def Pihole(Docker):
-    ''' used to contain some script stubbing, now pretty much an alias '''
+    ''' used to contain some script stubbing, now pretty much an alias.
+    Also provides bash as the default run function shell '''
+    def run_bash(self, command, *args, **kwargs):
+        cmd = self.get_command(command, *args)
+        if self.user is not None:
+            out = self.run_local(
+                "docker exec -u %s %s /bin/bash -c %s",
+                self.user, self.name, cmd)
+        else:
+            out = self.run_local(
+                "docker exec %s /bin/bash -c %s", self.name, cmd)
+        out.command = self.encode(cmd)
+        return out
+
+    funcType = type(Docker.run)
+    Docker.run = funcType(run_bash, Docker, testinfra.backend.docker.DockerBackend)
     return Docker
 
 @pytest.fixture

test/debian.Dockerfile

@@ -8,8 +8,9 @@ ADD . $GITDIR
 RUN cp $GITDIR/advanced/Scripts/*.sh $GITDIR/gravity.sh $GITDIR/pihole $GITDIR/automated\ install/*.sh $SCRIPTDIR/
 ENV PATH /usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:$SCRIPTDIR
 
-
 RUN true && \
     chmod +x $SCRIPTDIR/*
 
+ENV PH_TEST true
+
 #sed '/# Start the installer/Q' /opt/pihole/basic-install.sh > /opt/pihole/stub_basic-install.sh && \

test/shellcheck_failing_output.txt

@@ -1,76 +0,0 @@
-============================= test session starts ==============================
-platform linux2 -- Python 2.7.6, pytest-2.9.2, py-1.4.31, pluggy-0.3.1 -- /usr/bin/python
-cachedir: .cache
-rootdir: /home/a/opensource/pi-hole, inifile: 
-plugins: cov-2.3.0, bdd-2.17.0, xdist-1.14, testinfra-1.4.0
-collecting ... collected 7 items
-
-test/test_000_build_containers.py::test_build_pihole_image[test/debian.Dockerfile-pytest_pihole:debian] PASSED
-test/test_000_build_containers.py::test_build_pihole_image[test/centos.Dockerfile-pytest_pihole:centos] PASSED
-test/test_automated_install.py::test_setupVars_are_sourced_to_global_scope[debian] PASSED
-test/test_automated_install.py::test_setupVars_are_sourced_to_global_scope[centos] PASSED
-test/test_automated_install.py::test_setupVars_saved_to_file[debian] PASSED
-test/test_automated_install.py::test_setupVars_saved_to_file[centos] PASSED
-test/test_shellcheck.py::test_scripts_pass_shellcheck FAILED
-
-=================================== FAILURES ===================================
-_________________________ test_scripts_pass_shellcheck _________________________
-
-    def test_scripts_pass_shellcheck():
-        ''' Make sure shellcheck does not find anything wrong with our shell scripts '''
-        shellcheck = "find . -name 'update.sh' | while read file; do shellcheck \"$file\"; done;"
-        results = run_local(shellcheck)
-        print results.stdout
->       assert '' == results.stdout
-E       assert '' == '\nIn ./advanced/Scripts/upda...vent glob interpretation.\n\n'
-E         + 
-E         + In ./advanced/Scripts/update.sh line 24:
-E         + 	while [ "$(ps a | awk '{print $1}' | grep "${pid}")" ]; do
-E         +                 ^-- SC2143: Instead of [ -n $(foo | grep bar) ], use foo | grep -q bar .
-E         + 
-E         + 
-E         + In ./advanced/Scripts/update.sh line 57:
-E         + 	git clone -q --depth 1 "${2}" "${1}" > /dev/null & spinner $!
-E         +                                                                    ^-- SC2086: Double quote to prevent globbing and word splitting.
-E         Detailed information truncated (27 more lines), use "-vv" to show
-
-test/test_shellcheck.py:13: AssertionError
------------------------------ Captured stdout call -----------------------------
-
-In ./advanced/Scripts/update.sh line 24:
-	while [ "$(ps a | awk '{print $1}' | grep "${pid}")" ]; do
-                ^-- SC2143: Instead of [ -n $(foo | grep bar) ], use foo | grep -q bar .
-
-
-In ./advanced/Scripts/update.sh line 57:
-	git clone -q --depth 1 "${2}" "${1}" > /dev/null & spinner $!
-                                                                   ^-- SC2086: Double quote to prevent globbing and word splitting.
-
-
-In ./advanced/Scripts/update.sh line 65:
-	git stash -q > /dev/null & spinner $!
-                                           ^-- SC2086: Double quote to prevent globbing and word splitting.
-
-
-In ./advanced/Scripts/update.sh line 66:
-	git pull -q > /dev/null & spinner $!
-                                          ^-- SC2086: Double quote to prevent globbing and word splitting.
-
-
-In ./advanced/Scripts/update.sh line 107:
-if [[ ${piholeVersion} == ${piholeVersionLatest} && ${webVersion} == ${webVersionLatest} ]]; then
-                          ^-- SC2053: Quote the rhs of = in [[ ]] to prevent glob interpretation.
-                                                                     ^-- SC2053: Quote the rhs of = in [[ ]] to prevent glob interpretation.
-
-
-In ./advanced/Scripts/update.sh line 112:
-elif [[ ${piholeVersion} == ${piholeVersionLatest} && ${webVersion} != ${webVersionLatest} ]]; then
-                            ^-- SC2053: Quote the rhs of = in [[ ]] to prevent glob interpretation.
-
-
-In ./advanced/Scripts/update.sh line 120:
-elif [[ ${piholeVersion} != ${piholeVersionLatest} && ${webVersion} == ${webVersionLatest} ]]; then
-                                                                       ^-- SC2053: Quote the rhs of = in [[ ]] to prevent glob interpretation.
-
-
-===================== 1 failed, 6 passed in 24.01 seconds ======================

test/test_automated_install.py

@@ -11,7 +11,8 @@ SETUPVARS = {
 
 def test_setupVars_are_sourced_to_global_scope(Pihole):
     ''' currently update_dialogs sources setupVars with a dot,
-    then various other functions use the variables '''
+    then various other functions use the variables.
+    This confirms the sourced variables are in scope between functions '''
     setup_var_file = 'cat <<EOF> /etc/pihole/setupVars.conf\n'
     for k,v in SETUPVARS.iteritems():
         setup_var_file += "{}={}\n".format(k, v)
@@ -19,15 +20,15 @@ def test_setupVars_are_sourced_to_global_scope(Pihole):
     Pihole.run(setup_var_file)
 
     script = dedent('''\
-    #!/bin/bash -e
+    set -e
     printSetupVars() {
         # Currently debug test function only
         echo "Outputting sourced variables"
-        echo "PIHOLE_INTERFACE=\${PIHOLE_INTERFACE}"
+        echo "PIHOLE_INTERFACE=${PIHOLE_INTERFACE}"
-        echo "IPV4_ADDRESS=\${IPV4_ADDRESS}"
+        echo "IPV4_ADDRESS=${IPV4_ADDRESS}"
-        echo "IPV6_ADDRESS=\${IPV6_ADDRESS}"
+        echo "IPV6_ADDRESS=${IPV6_ADDRESS}"
-        echo "PIHOLE_DNS_1=\${PIHOLE_DNS_1}"
+        echo "PIHOLE_DNS_1=${PIHOLE_DNS_1}"
-        echo "PIHOLE_DNS_2=\${PIHOLE_DNS_2}"
+        echo "PIHOLE_DNS_2=${PIHOLE_DNS_2}"
     }
     update_dialogs() {
         . /etc/pihole/setupVars.conf
@@ -49,10 +50,9 @@ def test_setupVars_saved_to_file(Pihole):
     Pihole.run(set_setup_vars).stdout
 
     script = dedent('''\
-    #!/bin/bash -e
+    set -e
     echo start
     TERM=xterm
-    PHTEST=TRUE
     source /opt/pihole/basic-install.sh
     {}
     finalExports
@@ -64,14 +64,39 @@ def test_setupVars_saved_to_file(Pihole):
     for k,v in SETUPVARS.iteritems():
         assert "{}={}".format(k, v) in output
 
-def run_script(Pihole, script, file="/test.sh"):
+def test_configureFirewall_firewalld_no_errors(Pihole):
-    _write_test_script(Pihole, script, file=file)
+    ''' confirms firewalld rules are applied when appopriate '''
-    result = Pihole.run(file)
+    mock_command('firewall-cmd', '0', Pihole)
+    configureFirewall = Pihole.run('''
+    source /opt/pihole/basic-install.sh
+    configureFirewall
+    ''')
+    expected_stdout = '::: Configuring firewalld for httpd and dnsmasq.'
+    assert expected_stdout in configureFirewall.stdout
+    firewall_calls = Pihole.run('cat /var/log/firewall-cmd').stdout
+    assert 'firewall-cmd --state' in firewall_calls
+    assert 'firewall-cmd --permanent --add-port=80/tcp' in firewall_calls
+    assert 'firewall-cmd --permanent --add-port=53/tcp' in firewall_calls
+    assert 'firewall-cmd --permanent --add-port=53/udp' in firewall_calls
+    assert 'firewall-cmd --reload' in firewall_calls
+
+
+# Helper functions
+def mock_command(script, result, container):
+    ''' Allows for setup of commands we don't really want to have to run for real in unit tests '''
+    ''' TODO: support array of results that enable the results to change over multiple executions of a command '''
+    full_script_path = '/usr/local/bin/{}'.format(script)
+    mock_script = dedent('''\
+    #!/bin/bash -e
+    echo "\$0 \$@" >> /var/log/{script}
+    exit {retcode}
+    '''.format(script=script, retcode=result))
+    container.run('''
+    cat <<EOF> {script}\n{content}\nEOF
+    chmod +x {script}
+    '''.format(script=full_script_path, content=mock_script))
+
+def run_script(Pihole, script):
+    result = Pihole.run(script)
     assert result.rc == 0
     return result
-
-def _write_test_script(Pihole, script, file):
-    ''' Running the test script blocks directly can behave differently with regard to global vars '''
-    ''' this is a cheap work around to that until all functions no longer rely on global variables '''
-    Pihole.run('cat <<EOF> {file}\n{script}\nEOF'.format(file=file, script=script))
-    Pihole.run('chmod +x {}'.format(file))