Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tweak/debug improvements #1585

Merged
merged 7 commits into from Jul 14, 2017

Conversation

@jacobsalmela
Copy link
Member

jacobsalmela commented Jul 6, 2017

By submitting this pull request, I confirm the following (please check boxes, eg [X]) Failure to fill the template will close your PR:

Please submit all pull requests against the development branch. Failure to do so will delay or deny your request

  • I have read and understood the contributors guide.
  • I have checked that another pull request for this purpose does not exist.
  • I have considered, and confirmed that this submission will be valuable to others.
  • I accept that this submission may not be used, and the pull request closed at the will of the maintainer.
  • I give this submission freely, and claim no ownership to its content.

How familiar are you with the codebase?:

5

Improvements to the new debug script:

  • added a disclaimer to the beginning of the script
  • the automated mode wasn't working via the dashboard; this is fixed
  • highlight bad address entries in pihole.log and reference a corresponding FAQ
  • show the header output if the X-Pi-hole header does not match what we expect it to be
  • added an undocumented flag that will obfuscate domains before uploading them to our server (we'll need a new flag for pihole -d to accommodate this

This template was created based on the work of udemy-dl.

jacobsalmela added some commits Jul 5, 2017

fix automated mode when running from the dashboard. It would previous…
…ly not automatically upload and generate a token.
undocumented feature for now: obfuscate domains in pihole.log so they…
… are not visible when sent to the Pi-hole developers. We need to make an additonal flag for this in the pihole command. if the variable OBFUSCATE has a value, it will replace the domain in the log with a placeholder value
@@ -159,6 +162,17 @@ ${PIHOLE_FTL_LOG}
${PIHOLE_WEB_SERVER_ACCESS_LOG_FILE}
${PIHOLE_WEB_SERVER_ERROR_LOG_FILE})

DISCLAIMER="This process collects information from your Pi-hole, and optionally uploads it to a unique and random directory on tricorder.pi-hole.net.
The intent of this script is to allow users to self-diagnose their installations. This is accomplished by running tests against our software and providing the user with links to FAQ articles when a problem is detected. Since we are a small team and Pi-hole has been growing steadily, it is our hope that this will help use spend more time on development.

This comment has been minimized.

@Mcat12

Mcat12 Jul 6, 2017

Member

it is our hope that this will help use spend more time on development.

use -> us

@@ -457,7 +472,7 @@ does_ip_match_setup_vars() {
# If it's an IPv6 address
if [[ "${protocol}" == "6" ]]; then
# Strip off the / (CIDR notation)
if [[ "${ip_address%/*}" == "${setup_vars_ip}" ]]; then
if [[ "${ip_address%/*}" == "${setup_vars_ip%/*}" ]]; then

This comment has been minimized.

@PromoFaux

PromoFaux Jul 14, 2017

Member

I assume this has no effect if ${setup_vars_ip} has no CIDR? We pushed a fix into development to stop that from happening completely. The initial fix just removed it when gravity.sh processed it, but a later fix then stopped it from being put into setupvars with CIDR

This comment has been minimized.

@PromoFaux

PromoFaux Jul 14, 2017

Member

Caused no issues in test

This comment has been minimized.

@PromoFaux

PromoFaux Jul 14, 2017

Member

Commenting to leave github review, too.

@PromoFaux

This comment has been minimized.

Copy link
Member

PromoFaux commented Jul 14, 2017

Approved

Approved with PullApprove

@@ -457,7 +472,7 @@ does_ip_match_setup_vars() {
# If it's an IPv6 address
if [[ "${protocol}" == "6" ]]; then
# Strip off the / (CIDR notation)
if [[ "${ip_address%/*}" == "${setup_vars_ip}" ]]; then
if [[ "${ip_address%/*}" == "${setup_vars_ip%/*}" ]]; then

This comment has been minimized.

@PromoFaux

PromoFaux Jul 14, 2017

Member

Commenting to leave github review, too.

@PromoFaux PromoFaux merged commit e1f818f into development Jul 14, 2017

5 checks passed

codacy/pr Good work! A positive pull request.
Details
code-review/pullapprove Approved by jacobsalmela, PromoFaux
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
license/cla Contributor License Agreement is signed.
Details

@PromoFaux PromoFaux deleted the tweak/debug-improvements branch Jul 14, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.