Merge branch 'feature/defensiveTweaks' of https://github.com/pi-hole/pi-hole into feature/defensiveTweaks

.gitignore

@@ -1 +1,5 @@
 .DS_Store
+*.pyc
+*.swp
+__pycache__
+.cache

.travis.yml

@@ -0,0 +1,10 @@
+sudo: required
+services:
+  - docker
+language: python
+python:
+  - "2.7"
+install:
+  - pip install -r requirements.txt
+
+script: py.test -vv

advanced/Scripts/update.sh

@@ -78,14 +78,14 @@ main() {
 
   echo "::: Checking for updates..."
   # Checks Pi-hole version > pihole only > current local git repo version : returns string in format vX.X.X
-  local -r pihole_version_current="$(/usr/local/bin/pihole -v -p -c)"
+  local pihole_version_current="$(/usr/local/bin/pihole -v -p -c)"
   # Checks Pi-hole version > pihole only > remote upstream repo version : returns string in format vX.X.X
-  local -r pihole_version_latest="$(/usr/local/bin/pihole -v -p -l)"
+  local pihole_version_latest="$(/usr/local/bin/pihole -v -p -l)"
 
   # Checks Pi-hole version > admin only > current local git repo version : returns string in format vX.X.X
-  local -r web_version_current="$(/usr/local/bin/pihole -v -a -c)"
+  local web_version_current="$(/usr/local/bin/pihole -v -a -c)"
   # Checks Pi-hole version > admin only > remote upstream repo version : returns string in format vX.X.X
-  local -r web_version_latest="$(/usr/local/bin/pihole -v -a -l)"
+  local web_version_latest="$(/usr/local/bin/pihole -v -a -l)"
 
   # Logic
   # If latest versions are blank - we've probably hit Github rate limit (stop running `pihole -up so often!):
@@ -100,21 +100,27 @@ main() {
   #            pull pihole repo run install --unattended
 
   if [[ "${pihole_version_current}" == "${pihole_version_latest}" ]] && [[ "${web_version_current}" == "${web_version_latest}" ]]; then
+    echo ":::"
+    echo "::: Pi-hole version is $pihole_version_current"
+    echo "::: Web Admin version is $web_version_current"
+    echo ":::"
     echo "::: Everything is up to date!"
+    exit 0
 
   elif [[ "${pihole_version_current}" == "${pihole_version_latest}" ]] && [[ "${web_version_current}" != "${web_version_latest}" ]]; then
+    echo ":::"
     echo "::: Pi-hole Web Admin files out of date"
     getGitFiles "${ADMIN_INTERFACE_DIR}" "${ADMIN_INTERFACE_GIT_URL}"
-    echo ":::"
+
     web_version_current="$(/usr/local/bin/pihole -v -a -c)"
+    web_updated=true
 
   elif [[ "${pihole_version_current}" != "${pihole_version_latest}" ]] && [[ "${web_version_current}" == "${web_version_latest}" ]]; then
     echo "::: Pi-hole core files out of date"
     getGitFiles "${PI_HOLE_FILES_DIR}" "${PI_HOLE_GIT_URL}"
     /etc/.pihole/automated\ install/basic-install.sh --reconfigure --unattended || echo "Unable to complete update, contact Pi-hole" && exit 1
-    echo ":::"
     pihole_version_current="$(/usr/local/bin/pihole -v -p -c)"
-
+    core_updated=true
 
   elif [[ "${pihole_version_current}" != "${pihole_version_latest}" ]] && [[ "${web_version_current}" != "${web_version_latest}" ]]; then
     echo "::: Updating Everything"
@@ -124,15 +130,25 @@ main() {
     web_version_current="$(/usr/local/bin/pihole -v -a -c)"
     # Checks Pi-hole version > admin only > current local git repo version : returns string in format vX.X.X
     pihole_version_current="$(/usr/local/bin/pihole -v -p -c)"
+    web_updated=true
+    core_updated=true
   fi
-    echo ":::"
+
-    echo "::: Pi-hole version is now at ${pihole_version_current}"
+  if [[ "${web_updated}" == true ]]; then
-    echo "::: If you had made any changes in '/etc/.pihole/', they have been stashed using 'git stash'"
     echo ":::"
     echo "::: Web Admin version is now at ${web_version_current}"
     echo "::: If you had made any changes in '/var/www/html/admin/', they have been stashed using 'git stash'"
+  fi
+
+  if [[ "${core_updated}" == true ]]; then
+    echo ":::"
+    echo "::: Pi-hole version is now at ${pihole_version_current}"
+    echo "::: If you had made any changes in '/etc/.pihole/', they have been stashed using 'git stash'"
+  fi
+
   echo ""
   exit 0
+
 }
 
 main

automated install/basic-install.sh

@@ -1133,4 +1133,6 @@ main() {
 	echo "::: View the web interface at http://pi.hole/admin or http://${IPV4_ADDRESS%/*}/admin"
 }
 
+if [[ -z "$PHTEST" ]] ; then
     main "$@"
+fi

autotest

@@ -0,0 +1 @@
+py.test -v -f test/

requirements.txt

@@ -0,0 +1,5 @@
+docker-compose
+pytest
+pytest-xdist
+pytest-cov
+testinfra

test/centos.Dockerfile

@@ -0,0 +1,14 @@
+FROM centos:7
+
+ENV GITDIR /etc/.pihole
+ENV SCRIPTDIR /opt/pihole
+
+RUN mkdir -p $GITDIR $SCRIPTDIR /etc/pihole
+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/*
+
+#sed '/# Start the installer/Q' /opt/pihole/basic-install.sh > /opt/pihole/stub_basic-install.sh && \

test/conftest.py

@@ -0,0 +1,46 @@
+import pytest
+import testinfra
+
+check_output = testinfra.get_backend(
+    "local://"
+).get_module("Command").check_output
+
+@pytest.fixture
+def Pihole(Docker):
+    ''' used to contain some script stubbing, now pretty much an alias '''
+    return Docker
+
+@pytest.fixture
+def Docker(request, args, image, cmd):
+    ''' combine our fixtures into a docker run command and setup finalizer to cleanup '''
+    assert 'docker' in check_output('id'), "Are you in the docker group?"
+    docker_run = "docker run {} {} {}".format(args, image, cmd)
+    docker_id = check_output(docker_run)
+
+    def teardown():
+        check_output("docker rm -f %s", docker_id)
+    request.addfinalizer(teardown)
+
+    docker_container = testinfra.get_backend("docker://" + docker_id)
+    docker_container.id = docker_id
+    return docker_container
+
+@pytest.fixture
+def args(request):
+    ''' -t became required when tput began being used '''
+    return '-t -d'
+
+@pytest.fixture(params=['debian', 'centos'])
+def tag(request):
+    ''' consumed by image to make the test matrix '''
+    return request.param
+
+@pytest.fixture()
+def image(request, tag):
+    ''' built by test_000_build_containers.py '''
+    return 'pytest_pihole:{}'.format(tag)
+
+@pytest.fixture()
+def cmd(request):
+    ''' default to doing nothing by tailing null, but don't exit '''
+    return 'tail -f /dev/null'

test/debian.Dockerfile

@@ -0,0 +1,15 @@
+FROM debian:jessie
+
+ENV GITDIR /etc/.pihole
+ENV SCRIPTDIR /opt/pihole
+
+RUN mkdir -p $GITDIR $SCRIPTDIR /etc/pihole
+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/*
+
+#sed '/# Start the installer/Q' /opt/pihole/basic-install.sh > /opt/pihole/stub_basic-install.sh && \

test/shellcheck_failing_output.txt

@@ -0,0 +1,76 @@
+============================= 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_000_build_containers.py

@@ -0,0 +1,18 @@
+''' This file starts with 000 to make it run first '''
+import pytest
+import testinfra
+
+run_local = testinfra.get_backend(
+    "local://"
+).get_module("Command").run
+
+@pytest.mark.parametrize("image,tag", [
+    ( 'test/debian.Dockerfile', 'pytest_pihole:debian' ),
+    ( 'test/centos.Dockerfile', 'pytest_pihole:centos' ),
+])
+def test_build_pihole_image(image, tag):
+    build_cmd = run_local('docker build -f {} -t {} .'.format(image, tag))
+    if build_cmd.rc != 0:
+        print build_cmd.stdout
+        print build_cmd.stderr
+    assert build_cmd.rc == 0

test/test_automated_install.py

@@ -0,0 +1,77 @@
+import pytest
+from textwrap import dedent
+
+SETUPVARS = {
+    'PIHOLE_INTERFACE' : 'eth99',
+    'IPV4_ADDRESS' : '1.1.1.1',
+    'IPV6_ADDRESS' : 'FE80::240:D0FF:FE48:4672',
+    'PIHOLE_DNS_1' : '4.2.2.1',
+    'PIHOLE_DNS_2' : '4.2.2.2'
+}
+
+def test_setupVars_are_sourced_to_global_scope(Pihole):
+    ''' currently update_dialogs sources setupVars with a dot, 
+    then various other functions use the variables '''
+    setup_var_file = 'cat <<EOF> /etc/pihole/setupVars.conf\n'
+    for k,v in SETUPVARS.iteritems():
+        setup_var_file += "{}={}\n".format(k, v)
+    setup_var_file += "EOF\n"
+    Pihole.run(setup_var_file)
+
+    script = dedent('''\
+    #!/bin/bash -e
+    printSetupVars() {
+        # Currently debug test function only
+        echo "Outputting sourced variables"
+        echo "PIHOLE_INTERFACE=\${PIHOLE_INTERFACE}"
+        echo "IPV4_ADDRESS=\${IPV4_ADDRESS}"
+        echo "IPV6_ADDRESS=\${IPV6_ADDRESS}"
+        echo "PIHOLE_DNS_1=\${PIHOLE_DNS_1}"
+        echo "PIHOLE_DNS_2=\${PIHOLE_DNS_2}"
+    }
+    update_dialogs() {
+        . /etc/pihole/setupVars.conf
+    }
+    update_dialogs
+    printSetupVars
+    ''')
+
+    output = run_script(Pihole, script).stdout
+
+    for k,v in SETUPVARS.iteritems():
+        assert "{}={}".format(k, v) in output
+
+def test_setupVars_saved_to_file(Pihole):
+    ''' confirm saved settings are written to a file for future updates to re-use '''
+    set_setup_vars = '\n'  # dedent works better with this and padding matching script below
+    for k,v in SETUPVARS.iteritems():
+        set_setup_vars += "    {}={}\n".format(k, v)
+    Pihole.run(set_setup_vars).stdout
+
+    script = dedent('''\
+    #!/bin/bash -e
+    echo start
+    TERM=xterm
+    PHTEST=TRUE
+    source /opt/pihole/basic-install.sh
+    {}
+    finalExports
+    cat /etc/pihole/setupVars.conf
+    '''.format(set_setup_vars))
+
+    output = run_script(Pihole, script).stdout
+
+    for k,v in SETUPVARS.iteritems():
+        assert "{}={}".format(k, v) in output
+
+def run_script(Pihole, script, file="/test.sh"):
+    _write_test_script(Pihole, script, file=file)
+    result = Pihole.run(file)
+    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))

test/test_shellcheck.py

@@ -0,0 +1,13 @@
+import pytest
+import testinfra
+
+run_local = testinfra.get_backend(
+    "local://"
+).get_module("Command").run
+
+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