From 7fa8cdd03ee1884b61add34d923d0741da8a6a3a Mon Sep 17 00:00:00 2001 From: Adam Warner Date: Wed, 16 Mar 2022 20:46:15 +0000 Subject: [PATCH] Address: - Review Comments - Stickler Complaints --- advanced/Scripts/utils.sh | 22 ++++++++-------------- advanced/Scripts/webpage.sh | 12 +++++++----- pihole | 8 ++++---- test/test_any_utils.py | 19 ++++++++++--------- 4 files changed, 29 insertions(+), 32 deletions(-) diff --git a/advanced/Scripts/utils.sh b/advanced/Scripts/utils.sh index 86a7e0b4..0906ce49 100755 --- a/advanced/Scripts/utils.sh +++ b/advanced/Scripts/utils.sh @@ -1,4 +1,6 @@ #!/usr/bin/env sh +# shellcheck disable=SC3043 #https://github.com/koalaman/shellcheck/wiki/SC3043#exceptions + # Pi-hole: A black hole for Internet advertisements # (c) 2017 Pi-hole, LLC (https://pi-hole.net) # Network-wide ad blocking via your own hardware. @@ -16,28 +18,20 @@ ####################### # Takes either -# - Three arguments: key, value, and file. -# - Two arguments: key, and file +# - Three arguments: file, key, and value. +# - Two arguments: file, and key. # # Checks the target file for the existence of the key # - If it exists, it changes the value # - If it does not exist, it adds the value # # Example usage: -# addOrEditKeyValuePair "BLOCKING_ENABLED" "true" "/etc/pihole/setupVars.conf" +# addOrEditKeyValuePair "/etc/pihole/setupVars.conf" "BLOCKING_ENABLED" "true" ####################### addOrEditKeyValPair() { - local key="${1}" - local value - local file - - # If two arguments have been passed, then the second one is the file - there is no value - if [ $# -lt 3 ]; then - file="${2}" - else - value="${2}" - file="${3}" - fi + local file="${1}" + local key="${2}" + local value="${3}" if [ "${value}" != "" ]; then # value has a value, so it is a key pair diff --git a/advanced/Scripts/webpage.sh b/advanced/Scripts/webpage.sh index 14cf5999..f63fd0ca 100755 --- a/advanced/Scripts/webpage.sh +++ b/advanced/Scripts/webpage.sh @@ -1,5 +1,7 @@ #!/usr/bin/env bash # shellcheck disable=SC1090 +# shellcheck disable=SC2154 + # Pi-hole: A black hole for Internet advertisements # (c) 2017 Pi-hole, LLC (https://pi-hole.net) @@ -54,7 +56,7 @@ Options: } add_setting() { - addOrEditKeyValPair "${1}" "${2}" "${setupVars}" + addOrEditKeyValPair "${setupVars}" "${1}" "${2}" } delete_setting() { @@ -62,11 +64,11 @@ delete_setting() { } change_setting() { - addOrEditKeyValPair "${1}" "${2}" "${setupVars}" + addOrEditKeyValPair "${setupVars}" "${1}" "${2}" } addFTLsetting() { - addOrEditKeyValPair "${1}" "${2}" "${FTLconf}" + addOrEditKeyValPair "${FTLconf}" "${1}" "${2}" } deleteFTLsetting() { @@ -74,11 +76,11 @@ deleteFTLsetting() { } changeFTLsetting() { - addOrEditKeyValPair "${1}" "${2}" "${FTLconf}" + addOrEditKeyValPair "${FTLconf}" "${1}" "${2}" } add_dnsmasq_setting() { - addOrEditKeyValPair "${1}" "${2}" "${dnsmasqconfig}" + addOrEditKeyValPair "${dnsmasqconfig}" "${1}" "${2}" } delete_dnsmasq_setting() { diff --git a/pihole b/pihole index d73fd5aa..bdce6663 100755 --- a/pihole +++ b/pihole @@ -226,7 +226,7 @@ Time: fi local str="Pi-hole Disabled" - addOrEditKeyValPair "BLOCKING_ENABLED" "false" "${setupVars}" + addOrEditKeyValPair "${setupVars}" "BLOCKING_ENABLED" "false" fi else # Enable Pi-hole @@ -238,7 +238,7 @@ Time: echo -e " ${INFO} Enabling blocking" local str="Pi-hole Enabled" - addOrEditKeyValPair "BLOCKING_ENABLED" "true" "${setupVars}" + addOrEditKeyValPair "${setupVars}" "BLOCKING_ENABLED" "true" fi restartDNS reload-lists @@ -261,7 +261,7 @@ Options: elif [[ "${1}" == "off" ]]; then # Disable logging sed -i 's/^log-queries/#log-queries/' /etc/dnsmasq.d/01-pihole.conf - addOrEditKeyValPair "QUERY_LOGGING" "false" "${setupVars}" + addOrEditKeyValPair "${setupVars}" "QUERY_LOGGING" "false" if [[ "${2}" != "noflush" ]]; then # Flush logs "${PI_HOLE_BIN_DIR}"/pihole -f @@ -271,7 +271,7 @@ Options: elif [[ "${1}" == "on" ]]; then # Enable logging sed -i 's/^#log-queries/log-queries/' /etc/dnsmasq.d/01-pihole.conf - addOrEditKeyValPair "QUERY_LOGGING" "true" "${setupVars}" + addOrEditKeyValPair "${setupVars}" "QUERY_LOGGING" "true" echo -e " ${INFO} Enabling logging..." local str="Logging has been enabled!" else diff --git a/test/test_any_utils.py b/test/test_any_utils.py index f73cc1b2..1c8f9531 100644 --- a/test/test_any_utils.py +++ b/test/test_any_utils.py @@ -2,12 +2,12 @@ def test_key_val_replacement_works(host): ''' Confirms addOrEditKeyValPair provides the expected output ''' host.run(''' source /opt/pihole/utils.sh - addOrEditKeyValPair "KEY_ONE" "value1" "./testoutput" - addOrEditKeyValPair "KEY_TWO" "value2" "./testoutput" - addOrEditKeyValPair "KEY_ONE" "value3" "./testoutput" - addOrEditKeyValPair "KEY_FOUR" "value4" "./testoutput" - addOrEditKeyValPair "KEY_FIVE_NO_VALUE" "./testoutput" - addOrEditKeyValPair "KEY_FIVE_NO_VALUE" "./testoutput" + addOrEditKeyValPair "./testoutput" "KEY_ONE" "value1" + addOrEditKeyValPair "./testoutput" "KEY_TWO" "value2" + addOrEditKeyValPair "./testoutput" "KEY_ONE" "value3" + addOrEditKeyValPair "./testoutput" "KEY_FOUR" "value4" + addOrEditKeyValPair "./testoutput" "KEY_FIVE_NO_VALUE" + addOrEditKeyValPair "./testoutput" "KEY_FIVE_NO_VALUE" ''') output = host.run(''' cat ./testoutput @@ -15,13 +15,14 @@ def test_key_val_replacement_works(host): expected_stdout = 'KEY_ONE=value3\nKEY_TWO=value2\nKEY_FOUR=value4\nKEY_FIVE_NO_VALUE\n' assert expected_stdout == output.stdout + def test_key_val_removal_works(host): ''' Confirms addOrEditKeyValPair provides the expected output ''' host.run(''' source /opt/pihole/utils.sh - addOrEditKeyValPair "KEY_ONE" "value1" "./testoutput" - addOrEditKeyValPair "KEY_TWO" "value2" "./testoutput" - addOrEditKeyValPair "KEY_THREE" "value3" "./testoutput" + addOrEditKeyValPair "./testoutput" "KEY_ONE" "value1" + addOrEditKeyValPair "./testoutput" "KEY_TWO" "value2" + addOrEditKeyValPair "./testoutput" "KEY_THREE" "value3" removeKey "KEY_TWO" "./testoutput" ''') output = host.run('''