From a03caea549c033796de8720e5ed45dd8384a12f2 Mon Sep 17 00:00:00 2001 From: diginc Date: Mon, 10 Oct 2016 23:14:39 -0500 Subject: [PATCH 1/9] setupVar tests passing for debian & centos --- .gitignore | 4 ++ automated install/basic-install.sh | 4 +- autotest | 1 + test/__init__.py | 0 test/centos.Dockerfile | 14 ++++++ test/conftest.py | 46 ++++++++++++++++++ test/debian.Dockerfile | 15 ++++++ test/test_000_build_containers.py | 18 +++++++ test/test_automated_install.py | 77 ++++++++++++++++++++++++++++++ 9 files changed, 178 insertions(+), 1 deletion(-) create mode 100755 autotest create mode 100644 test/__init__.py create mode 100644 test/centos.Dockerfile create mode 100644 test/conftest.py create mode 100644 test/debian.Dockerfile create mode 100644 test/test_000_build_containers.py create mode 100644 test/test_automated_install.py diff --git a/.gitignore b/.gitignore index e43b0f98..0e0d4b99 100644 --- a/.gitignore +++ b/.gitignore @@ -1 +1,5 @@ .DS_Store +*.pyc +*.swp +__pycache__ +.cache diff --git a/automated install/basic-install.sh b/automated install/basic-install.sh index 945fa1e1..13e546ba 100755 --- a/automated install/basic-install.sh +++ b/automated install/basic-install.sh @@ -971,4 +971,6 @@ echo "::: The install log is located at: /etc/pihole/install.log" echo "::: View the web interface at http://pi.hole/admin or http://${IPv4_address%/*}/admin" } -main "$@" +if [[ -z "$PHTEST" ]] ; then + main "$@" +fi diff --git a/autotest b/autotest new file mode 100755 index 00000000..3747cc0b --- /dev/null +++ b/autotest @@ -0,0 +1 @@ +py.test -v -f test/ diff --git a/test/__init__.py b/test/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/test/centos.Dockerfile b/test/centos.Dockerfile new file mode 100644 index 00000000..9af7eb4d --- /dev/null +++ b/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 && \ diff --git a/test/conftest.py b/test/conftest.py new file mode 100644 index 00000000..407d00dc --- /dev/null +++ b/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' diff --git a/test/debian.Dockerfile b/test/debian.Dockerfile new file mode 100644 index 00000000..b80d6155 --- /dev/null +++ b/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 && \ diff --git a/test/test_000_build_containers.py b/test/test_000_build_containers.py new file mode 100644 index 00000000..c617f3ae --- /dev/null +++ b/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 diff --git a/test/test_automated_install.py b/test/test_automated_install.py new file mode 100644 index 00000000..1605fdb6 --- /dev/null +++ b/test/test_automated_install.py @@ -0,0 +1,77 @@ +import pytest +from textwrap import dedent + +SETUPVARS = { + 'piholeInterface' : 'eth99', + 'IPv4_address' : '1.1.1.1', + 'IPv6_address' : '2:2:2:2:2:2', + 'piholeDNS1' : '4.2.2.1', + 'piholeDNS2' : '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 < /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 "piholeInterface=\${piholeInterface}" + echo "IPv4_address=\${IPv4_address}" + echo "IPv6_address=\${IPv6_address}" + echo "piholeDNS1=\${piholeDNS1}" + echo "piholeDNS2=\${piholeDNS2}" + } + 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 < {file}\n{script}\nEOF'.format(file=file, script=script)) + Pihole.run('chmod +x {}'.format(file)) From 97c44042e19858137b7e010800263d7a5490e714 Mon Sep 17 00:00:00 2001 From: diginc Date: Mon, 10 Oct 2016 23:18:05 -0500 Subject: [PATCH 2/9] Adding failing shellcheck test and results Should be useful for showing others why other changes were made in the next commit. --- test/shellcheck_failing_output.txt | 140 +++++++++++++++++++++++++++++ test/test_shellcheck.py | 13 +++ 2 files changed, 153 insertions(+) create mode 100644 test/shellcheck_failing_output.txt create mode 100644 test/test_shellcheck.py diff --git a/test/shellcheck_failing_output.txt b/test/shellcheck_failing_output.txt new file mode 100644 index 00000000..65b79560 --- /dev/null +++ b/test/shellcheck_failing_output.txt @@ -0,0 +1,140 @@ +============================= 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 'basic-install.sh' | while read file; do shellcheck \"$file\"; done;" + results = run_local(shellcheck) + print results.stdout +> assert '' == results.stdout +E assert '' == '\nIn ./automated install/bas...C may run when A is true.\n\n' +E + +E + In ./automated install/basic-install.sh line 79: +E + INSTALLER_DEPS=( apt-utils whiptail git dhcpcd5) +E + ^-- SC2034: INSTALLER_DEPS appears unused. Verify it or export it. +E + +E + +E + In ./automated install/basic-install.sh line 80: +E + PIHOLE_DEPS=( dnsutils bc dnsmasq lighttpd ${phpVer}-common ${phpVer}-cgi curl unzip wget sudo netcat cron iproute2 ) +E + ^-- SC2034: PIHOLE_DEPS appears unused. Verify it or export it. +E Detailed information truncated (91 more lines), use "-vv" to show + +test/test_shellcheck.py:13: AssertionError +----------------------------- Captured stdout call ----------------------------- + +In ./automated install/basic-install.sh line 79: + INSTALLER_DEPS=( apt-utils whiptail git dhcpcd5) + ^-- SC2034: INSTALLER_DEPS appears unused. Verify it or export it. + + +In ./automated install/basic-install.sh line 80: + PIHOLE_DEPS=( dnsutils bc dnsmasq lighttpd ${phpVer}-common ${phpVer}-cgi curl unzip wget sudo netcat cron iproute2 ) + ^-- SC2034: PIHOLE_DEPS appears unused. Verify it or export it. + + +In ./automated install/basic-install.sh line 86: + dpkg-query -W -f='${Status}' "$1" 2>/dev/null | grep -c "ok installed" || ${PKG_INSTALL} "$1" + ^-- SC2016: Expressions don't expand in single quotes, use double quotes for that. + + +In ./automated install/basic-install.sh line 100: + INSTALLER_DEPS=( iproute net-tools procps-ng newt git ) + ^-- SC2034: INSTALLER_DEPS appears unused. Verify it or export it. + + +In ./automated install/basic-install.sh line 101: + PIHOLE_DEPS=( epel-release bind-utils bc dnsmasq lighttpd lighttpd-fastcgi php-common php-cli php curl unzip wget findutils cronie sudo nmap-ncat ) + ^-- SC2034: PIHOLE_DEPS appears unused. Verify it or export it. + + +In ./automated install/basic-install.sh line 120: + while [ "$(ps a | awk '{print $1}' | grep "$pid")" ]; do + ^-- SC2143: Instead of [ -n $(foo | grep bar) ], use foo | grep -q bar . + + +In ./automated install/basic-install.sh line 214: + chooseInterfaceOptions=$("${chooseInterfaceCmd[@]}" "${interfacesArray[@]}" 2>&1 >/dev/tty) + ^-- SC2069: The order of the 2>&1 and the redirect matters. The 2>&1 has to be last. + + +In ./automated install/basic-install.sh line 241: + choices=$("${cmd[@]}" "${options[@]}" 2>&1 >/dev/tty) + ^-- SC2069: The order of the 2>&1 and the redirect matters. The 2>&1 has to be last. + + +In ./automated install/basic-install.sh line 354: + cp "${IFCFG_FILE}" "${IFCFG_FILE}".backup-"$(date +%Y-%m-%d-%H%M%S)" + ^-- SC2140: The double quotes around this do nothing. Remove or escape them. + + +In ./automated install/basic-install.sh line 408: + DNSchoices=$("${DNSChooseCmd[@]}" "${DNSChooseOptions[@]}" 2>&1 >/dev/tty) + ^-- SC2069: The order of the 2>&1 and the redirect matters. The 2>&1 has to be last. + + +In ./automated install/basic-install.sh line 585: + systemctl stop "${1}" &> /dev/null & spinner $! || true + ^-- SC2086: Double quote to prevent globbing and word splitting. + + +In ./automated install/basic-install.sh line 587: + service "${1}" stop &> /dev/null & spinner $! || true + ^-- SC2086: Double quote to prevent globbing and word splitting. + + +In ./automated install/basic-install.sh line 598: + systemctl restart "${1}" &> /dev/null & spinner $! + ^-- SC2086: Double quote to prevent globbing and word splitting. + + +In ./automated install/basic-install.sh line 600: + service "${1}" restart &> /dev/null & spinner $! + ^-- SC2086: Double quote to prevent globbing and word splitting. + + +In ./automated install/basic-install.sh line 610: + systemctl enable "${1}" &> /dev/null & spinner $! + ^-- SC2086: Double quote to prevent globbing and word splitting. + + +In ./automated install/basic-install.sh line 612: + update-rc.d "${1}" defaults &> /dev/null & spinner $! + ^-- SC2086: Double quote to prevent globbing and word splitting. + + +In ./automated install/basic-install.sh line 631: + ${UPDATE_PKG_CACHE} &> /dev/null & spinner $! + ^-- SC2086: Double quote to prevent globbing and word splitting. + + +In ./automated install/basic-install.sh line 688: + git clone -q --depth 1 "${2}" "${1}" > /dev/null & spinner $! + ^-- SC2086: Double quote to prevent globbing and word splitting. + + +In ./automated install/basic-install.sh line 696: + git pull -q > /dev/null & spinner $! + ^-- SC2086: Double quote to prevent globbing and word splitting. + + +In ./automated install/basic-install.sh line 761: + id -u pihole &> /dev/null && echo "::: User 'pihole' already exists" || (echo "::: User 'pihole' doesn't exist. Creating..." && useradd -r -s /usr/sbin/nologin pihole) + ^-- SC2015: Note that A && B || C is not if-then-else. C may run when A is true. + + +===================== 1 failed, 6 passed in 20.79 seconds ====================== diff --git a/test/test_shellcheck.py b/test/test_shellcheck.py new file mode 100644 index 00000000..7f777caf --- /dev/null +++ b/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 'basic-install.sh' | while read file; do shellcheck \"$file\"; done;" + results = run_local(shellcheck) + print results.stdout + assert '' == results.stdout From 26789f9b3670a1390e0bcd948900f4c1e6fcb243 Mon Sep 17 00:00:00 2001 From: diginc Date: Tue, 1 Nov 2016 23:53:11 -0500 Subject: [PATCH 3/9] add travis and python requirements --- .travis.yml | 10 ++++++++++ requirements.txt | 5 +++++ 2 files changed, 15 insertions(+) create mode 100644 .travis.yml create mode 100644 requirements.txt diff --git a/.travis.yml b/.travis.yml new file mode 100644 index 00000000..2ca3b2d2 --- /dev/null +++ b/.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 diff --git a/requirements.txt b/requirements.txt new file mode 100644 index 00000000..53737ca5 --- /dev/null +++ b/requirements.txt @@ -0,0 +1,5 @@ +docker-compose +pytest +pytest-xdist +pytest-cov +testinfra From 7548d9a8fe935320a564da5e3ae77b4147edc3a5 Mon Sep 17 00:00:00 2001 From: diginc Date: Tue, 1 Nov 2016 23:56:46 -0500 Subject: [PATCH 4/9] point shellcheck at update.sh instead --- test/shellcheck_failing_output.txt | 128 ++++++++--------------------- test/test_shellcheck.py | 2 +- 2 files changed, 33 insertions(+), 97 deletions(-) diff --git a/test/shellcheck_failing_output.txt b/test/shellcheck_failing_output.txt index 65b79560..c741a8e9 100644 --- a/test/shellcheck_failing_output.txt +++ b/test/shellcheck_failing_output.txt @@ -18,123 +18,59 @@ _________________________ test_scripts_pass_shellcheck _________________________ def test_scripts_pass_shellcheck(): ''' Make sure shellcheck does not find anything wrong with our shell scripts ''' - shellcheck = "find . -name 'basic-install.sh' | while read file; do shellcheck \"$file\"; done;" + 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 ./automated install/bas...C may run when A is true.\n\n' +E assert '' == '\nIn ./advanced/Scripts/upda...vent glob interpretation.\n\n' E + -E + In ./automated install/basic-install.sh line 79: -E + INSTALLER_DEPS=( apt-utils whiptail git dhcpcd5) -E + ^-- SC2034: INSTALLER_DEPS appears unused. Verify it or export it. +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 ./automated install/basic-install.sh line 80: -E + PIHOLE_DEPS=( dnsutils bc dnsmasq lighttpd ${phpVer}-common ${phpVer}-cgi curl unzip wget sudo netcat cron iproute2 ) -E + ^-- SC2034: PIHOLE_DEPS appears unused. Verify it or export it. -E Detailed information truncated (91 more lines), use "-vv" to show +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 ./automated install/basic-install.sh line 79: - INSTALLER_DEPS=( apt-utils whiptail git dhcpcd5) - ^-- SC2034: INSTALLER_DEPS appears unused. Verify it or export it. +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 ./automated install/basic-install.sh line 80: - PIHOLE_DEPS=( dnsutils bc dnsmasq lighttpd ${phpVer}-common ${phpVer}-cgi curl unzip wget sudo netcat cron iproute2 ) - ^-- SC2034: PIHOLE_DEPS appears unused. Verify it or export it. +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 ./automated install/basic-install.sh line 86: - dpkg-query -W -f='${Status}' "$1" 2>/dev/null | grep -c "ok installed" || ${PKG_INSTALL} "$1" - ^-- SC2016: Expressions don't expand in single quotes, use double quotes for that. +In ./advanced/Scripts/update.sh line 65: + git stash -q > /dev/null & spinner $! + ^-- SC2086: Double quote to prevent globbing and word splitting. -In ./automated install/basic-install.sh line 100: - INSTALLER_DEPS=( iproute net-tools procps-ng newt git ) - ^-- SC2034: INSTALLER_DEPS appears unused. Verify it or export it. +In ./advanced/Scripts/update.sh line 66: + git pull -q > /dev/null & spinner $! + ^-- SC2086: Double quote to prevent globbing and word splitting. -In ./automated install/basic-install.sh line 101: - PIHOLE_DEPS=( epel-release bind-utils bc dnsmasq lighttpd lighttpd-fastcgi php-common php-cli php curl unzip wget findutils cronie sudo nmap-ncat ) - ^-- SC2034: PIHOLE_DEPS appears unused. Verify it or export it. +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 ./automated install/basic-install.sh line 120: - 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 112: +elif [[ ${piholeVersion} == ${piholeVersionLatest} && ${webVersion} != ${webVersionLatest} ]]; then + ^-- SC2053: Quote the rhs of = in [[ ]] to prevent glob interpretation. -In ./automated install/basic-install.sh line 214: - chooseInterfaceOptions=$("${chooseInterfaceCmd[@]}" "${interfacesArray[@]}" 2>&1 >/dev/tty) - ^-- SC2069: The order of the 2>&1 and the redirect matters. The 2>&1 has to be last. +In ./advanced/Scripts/update.sh line 120: +elif [[ ${piholeVersion} != ${piholeVersionLatest} && ${webVersion} == ${webVersionLatest} ]]; then + ^-- SC2053: Quote the rhs of = in [[ ]] to prevent glob interpretation. -In ./automated install/basic-install.sh line 241: - choices=$("${cmd[@]}" "${options[@]}" 2>&1 >/dev/tty) - ^-- SC2069: The order of the 2>&1 and the redirect matters. The 2>&1 has to be last. - - -In ./automated install/basic-install.sh line 354: - cp "${IFCFG_FILE}" "${IFCFG_FILE}".backup-"$(date +%Y-%m-%d-%H%M%S)" - ^-- SC2140: The double quotes around this do nothing. Remove or escape them. - - -In ./automated install/basic-install.sh line 408: - DNSchoices=$("${DNSChooseCmd[@]}" "${DNSChooseOptions[@]}" 2>&1 >/dev/tty) - ^-- SC2069: The order of the 2>&1 and the redirect matters. The 2>&1 has to be last. - - -In ./automated install/basic-install.sh line 585: - systemctl stop "${1}" &> /dev/null & spinner $! || true - ^-- SC2086: Double quote to prevent globbing and word splitting. - - -In ./automated install/basic-install.sh line 587: - service "${1}" stop &> /dev/null & spinner $! || true - ^-- SC2086: Double quote to prevent globbing and word splitting. - - -In ./automated install/basic-install.sh line 598: - systemctl restart "${1}" &> /dev/null & spinner $! - ^-- SC2086: Double quote to prevent globbing and word splitting. - - -In ./automated install/basic-install.sh line 600: - service "${1}" restart &> /dev/null & spinner $! - ^-- SC2086: Double quote to prevent globbing and word splitting. - - -In ./automated install/basic-install.sh line 610: - systemctl enable "${1}" &> /dev/null & spinner $! - ^-- SC2086: Double quote to prevent globbing and word splitting. - - -In ./automated install/basic-install.sh line 612: - update-rc.d "${1}" defaults &> /dev/null & spinner $! - ^-- SC2086: Double quote to prevent globbing and word splitting. - - -In ./automated install/basic-install.sh line 631: - ${UPDATE_PKG_CACHE} &> /dev/null & spinner $! - ^-- SC2086: Double quote to prevent globbing and word splitting. - - -In ./automated install/basic-install.sh line 688: - git clone -q --depth 1 "${2}" "${1}" > /dev/null & spinner $! - ^-- SC2086: Double quote to prevent globbing and word splitting. - - -In ./automated install/basic-install.sh line 696: - git pull -q > /dev/null & spinner $! - ^-- SC2086: Double quote to prevent globbing and word splitting. - - -In ./automated install/basic-install.sh line 761: - id -u pihole &> /dev/null && echo "::: User 'pihole' already exists" || (echo "::: User 'pihole' doesn't exist. Creating..." && useradd -r -s /usr/sbin/nologin pihole) - ^-- SC2015: Note that A && B || C is not if-then-else. C may run when A is true. - - -===================== 1 failed, 6 passed in 20.79 seconds ====================== +===================== 1 failed, 6 passed in 24.01 seconds ====================== diff --git a/test/test_shellcheck.py b/test/test_shellcheck.py index 7f777caf..43b27164 100644 --- a/test/test_shellcheck.py +++ b/test/test_shellcheck.py @@ -7,7 +7,7 @@ run_local = testinfra.get_backend( def test_scripts_pass_shellcheck(): ''' Make sure shellcheck does not find anything wrong with our shell scripts ''' - shellcheck = "find . -name 'basic-install.sh' | while read file; do shellcheck \"$file\"; done;" + shellcheck = "find . -name 'update.sh' | while read file; do shellcheck \"$file\"; done;" results = run_local(shellcheck) print results.stdout assert '' == results.stdout From ee37c37cab37a5faa9f53aeb09eb5fb6fff168a9 Mon Sep 17 00:00:00 2001 From: Adam Warner Date: Wed, 2 Nov 2016 14:29:20 +0000 Subject: [PATCH 5/9] fix update output logic. version number variables should not be read only! --- advanced/Scripts/update.sh | 40 ++++++++++++++++++++++++++------------ 1 file changed, 28 insertions(+), 12 deletions(-) diff --git a/advanced/Scripts/update.sh b/advanced/Scripts/update.sh index 7d05efd8..1a12dfc1 100644 --- a/advanced/Scripts/update.sh +++ b/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}" - echo "::: If you had made any changes in '/etc/.pihole/', they have been stashed using 'git stash'" + + if [[ "${web_updated}" == true ]]; then 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'" - echo "" - exit 0 + 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 From fef9ab674e2d853b970bb73200d69c0286fef9e5 Mon Sep 17 00:00:00 2001 From: Dan Schaper Date: Wed, 2 Nov 2016 07:53:02 -0700 Subject: [PATCH 6/9] Pi-hole CORE installer variables are being changed to standardize scripts. --- test/test_automated_install.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/test_automated_install.py b/test/test_automated_install.py index 1605fdb6..b09eabd6 100644 --- a/test/test_automated_install.py +++ b/test/test_automated_install.py @@ -23,11 +23,11 @@ def test_setupVars_are_sourced_to_global_scope(Pihole): printSetupVars() { # Currently debug test function only echo "Outputting sourced variables" - echo "piholeInterface=\${piholeInterface}" - echo "IPv4_address=\${IPv4_address}" - echo "IPv6_address=\${IPv6_address}" - echo "piholeDNS1=\${piholeDNS1}" - echo "piholeDNS2=\${piholeDNS2}" + echo "PIHOLE_INTERFAC=\${piholeInterface}" + echo "IPv4_ADDRESS=\${IPv4_address}" + echo "IPv6_ADDRESS=\${IPv6_address}" + echo "PIHOLE_DNS1=\${piholeDNS1}" + echo "PIHOLE_DNS2=\${piholeDNS2}" } update_dialogs() { . /etc/pihole/setupVars.conf From e99ef9c093666434898eb641129685c97ed4793a Mon Sep 17 00:00:00 2001 From: Dan Schaper Date: Wed, 2 Nov 2016 08:22:45 -0700 Subject: [PATCH 7/9] Cap the variables and echo out the proper environment. --- test/test_automated_install.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/test/test_automated_install.py b/test/test_automated_install.py index b09eabd6..44bdacb2 100644 --- a/test/test_automated_install.py +++ b/test/test_automated_install.py @@ -2,11 +2,11 @@ import pytest from textwrap import dedent SETUPVARS = { - 'piholeInterface' : 'eth99', - 'IPv4_address' : '1.1.1.1', - 'IPv6_address' : '2:2:2:2:2:2', - 'piholeDNS1' : '4.2.2.1', - 'piholeDNS2' : '4.2.2.2' + 'PIHOLE_INTERFACE' : 'eth99', + 'IPV4_ADDRESS' : '1.1.1.1', + 'IPV6_ADDRESS' : 'FE80::240:D0FF:FE48:4672', + 'PIHOLE_DNS1' : '4.2.2.1', + 'PIHOLE_DNS2' : '4.2.2.2' } def test_setupVars_are_sourced_to_global_scope(Pihole): @@ -23,11 +23,11 @@ def test_setupVars_are_sourced_to_global_scope(Pihole): printSetupVars() { # Currently debug test function only echo "Outputting sourced variables" - echo "PIHOLE_INTERFAC=\${piholeInterface}" - echo "IPv4_ADDRESS=\${IPv4_address}" - echo "IPv6_ADDRESS=\${IPv6_address}" - echo "PIHOLE_DNS1=\${piholeDNS1}" - echo "PIHOLE_DNS2=\${piholeDNS2}" + echo "PIHOLE_INTERFACE=\${PIHOLE_INTERFACE}" + echo "IPv4_ADDRESS=\${IPv4_ADDRESS}" + echo "IPv6_ADDRESS=\${IPv6_ADDRESS}" + echo "PIHOLE_DNS1=\${PIHOLE_DNS1}" + echo "PIHOLE_DNS2=\${PIHOLE_DNS2}" } update_dialogs() { . /etc/pihole/setupVars.conf From 07029f93e36c0a03ccefa6b1b5f352e3d72bce00 Mon Sep 17 00:00:00 2001 From: Dan Schaper Date: Wed, 2 Nov 2016 08:34:56 -0700 Subject: [PATCH 8/9] Match team convention in naming. --- test/test_automated_install.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/test_automated_install.py b/test/test_automated_install.py index 44bdacb2..b9b05708 100644 --- a/test/test_automated_install.py +++ b/test/test_automated_install.py @@ -24,8 +24,8 @@ def test_setupVars_are_sourced_to_global_scope(Pihole): # 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 "IPV4_ADDRESS=\${IPV4_ADDRESS}" + echo "IPV6_ADDRESS=\${IPV6_ADDRESS}" echo "PIHOLE_DNS1=\${PIHOLE_DNS1}" echo "PIHOLE_DNS2=\${PIHOLE_DNS2}" } From a1a9a7fa9e0cfe49a4e8e1cec28b40573204b9aa Mon Sep 17 00:00:00 2001 From: Dan Schaper Date: Wed, 2 Nov 2016 08:52:23 -0700 Subject: [PATCH 9/9] Clarify which DNS entry we are modifying. --- test/test_automated_install.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/test_automated_install.py b/test/test_automated_install.py index b9b05708..458536eb 100644 --- a/test/test_automated_install.py +++ b/test/test_automated_install.py @@ -5,8 +5,8 @@ SETUPVARS = { 'PIHOLE_INTERFACE' : 'eth99', 'IPV4_ADDRESS' : '1.1.1.1', 'IPV6_ADDRESS' : 'FE80::240:D0FF:FE48:4672', - 'PIHOLE_DNS1' : '4.2.2.1', - 'PIHOLE_DNS2' : '4.2.2.2' + 'PIHOLE_DNS_1' : '4.2.2.1', + 'PIHOLE_DNS_2' : '4.2.2.2' } def test_setupVars_are_sourced_to_global_scope(Pihole): @@ -26,8 +26,8 @@ def test_setupVars_are_sourced_to_global_scope(Pihole): echo "PIHOLE_INTERFACE=\${PIHOLE_INTERFACE}" echo "IPV4_ADDRESS=\${IPV4_ADDRESS}" echo "IPV6_ADDRESS=\${IPV6_ADDRESS}" - echo "PIHOLE_DNS1=\${PIHOLE_DNS1}" - echo "PIHOLE_DNS2=\${PIHOLE_DNS2}" + echo "PIHOLE_DNS_1=\${PIHOLE_DNS_1}" + echo "PIHOLE_DNS_2=\${PIHOLE_DNS_2}" } update_dialogs() { . /etc/pihole/setupVars.conf