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

added option to disable dns in wireguard conf #168

Merged

Conversation

gchamon
Copy link

@gchamon gchamon commented May 5, 2021

to:
cc: @subspacecommunity/subspace-maintainers
related to:
resolves:

Background

To avoid wrong regional DNS resolution, which would cause international users to try to connect to the wrong servers when using the DNS provided by subspace, an option should exist to disable the DNS entry in wireguard conf.

Changes

  • added env var SUBSPACE_DISABLE_DNS which acts as a toogle and, when set, causes new config files to be created with the DNS entry ommited.
  • disabling dnsmasq if the flag is set
  • updating readme

Testing

Create a new subspace instance with SUBSPACE_DISABLE_DNS=1, create a new config file, check if DNS = in wg0.conf is ommited:

[Interface]
PrivateKey = {REDACTED}
Address = 10.99.97.2/24,fd00::10:97:2/64

[Peer]
PublicKey = XIvA8kM+O9M+FGgR1SvkCv2DDrxWah+wp10tRfYAVzU=

Endpoint = {REDACTED}:51820
AllowedIPs = 0.0.0.0/0, ::/0

@agonbar
Copy link
Collaborator

agonbar commented May 16, 2021

Hi! I tested this, but I think it needs a bit more work.

Right now, the dnsmasq server is still getting launched without taking into consideration the env variable. To avoid unnecesary confusion, the DNS server should only be started if needed.

Also, adding a new var to the config, should be on the Docker-Compose Example section of the readme, or maybe we can refactor the way we explain all this vars to the users.

Well, aslo, this needs a rebase from master to solve some conflicts, I saw what the difference was and it should be easy to integrate while adding what I said.

@gchamon
Copy link
Author

gchamon commented May 17, 2021

@agonbar I think refactoring the readme to make the env vars easier to understand for the users is worth its own separate issue. I will, however update my PR with your suggestions

@gchamon gchamon force-pushed the feature/disable-dns-upstream branch from 76806d1 to 9bc1041 Compare May 17, 2021 01:25
- adding example in both docker and docker-compose commands
- changing disableDNS from string to bool
- removing the DNS line altogether
- disabling dnsmasq service if dns is disabled
@gchamon gchamon force-pushed the feature/disable-dns-upstream branch from 9bc1041 to 08ba9ed Compare May 17, 2021 01:26
@sonarcloud
Copy link

sonarcloud bot commented May 17, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@gchamon gchamon requested a review from a team May 17, 2021 01:28
Copy link
Collaborator

@agonbar agonbar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! I just tried this, it works! Sorry for the delay. Respecting the other PR, a rebase can be made and any extra feature can be introduced there.

@agonbar agonbar merged commit f3f0c8e into subspacecommunity:master Jun 3, 2021
@agonbar
Copy link
Collaborator

agonbar commented Jun 3, 2021

@all-contributors please add @gchamon for code

@allcontributors
Copy link

@agonbar

I've put up a pull request to add @gchamon! 🎉

agonbar pushed a commit that referenced this pull request Jun 3, 2021
commit 63f9ffe
Author: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com>
Date:   Thu Jun 3 09:40:48 2021 +0200

    docs: add gchamon as a contributor (#185)

    * docs: update README.md [skip ci]

    * docs: update .all-contributorsrc [skip ci]

    Co-authored-by: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com>

commit f3f0c8e
Author: Gabriel Chamon Araujo <gchamon@live.com>
Date:   Thu Jun 3 04:30:10 2021 -0300

    added option to disable dns in wireguard conf (#168)

    * added option to disable dns in wireguard conf

    * updated readme

    * adding suggested edits
    - adding example in both docker and docker-compose commands
    - changing disableDNS from string to bool
    - removing the DNS line altogether
    - disabling dnsmasq service if dns is disabled

commit e3cf519
Author: Jack <39212456+jack1902@users.noreply.github.com>
Date:   Thu May 20 14:45:09 2021 +0100

    feature: Add release-drafter to project (#172)

    * Add release-drafter to project

    * Extend release-drafter for features and refactor

commit dda5a7e
Author: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com>
Date:   Thu May 20 14:18:05 2021 +0200

    docs: add Coffeeri as a contributor (#183)

    * docs: update README.md [skip ci]

    * docs: update .all-contributorsrc [skip ci]

    Co-authored-by: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com>

commit fbd481c
Author: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com>
Date:   Thu May 20 14:06:09 2021 +0200

    docs: add Freekers as a contributor (#182)

    * docs: update README.md [skip ci]

    * docs: update .all-contributorsrc [skip ci]

    Co-authored-by: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com>

commit 1ef43bc
Author: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com>
Date:   Thu May 20 14:03:14 2021 +0200

    docs: add dovreshef as a contributor (#181)

    * docs: update README.md [skip ci]

    * docs: update .all-contributorsrc [skip ci]

    Co-authored-by: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com>

commit 6490762
Author: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com>
Date:   Thu May 20 14:02:21 2021 +0200

    docs: add miki725 as a contributor (#180)

    * docs: update README.md [skip ci]

    * docs: update .all-contributorsrc [skip ci]

    Co-authored-by: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com>

commit 1e86175
Author: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com>
Date:   Thu May 20 14:01:28 2021 +0200

    docs: add d3473r as a contributor (#179)

    * docs: update README.md [skip ci]

    * docs: update .all-contributorsrc [skip ci]

    Co-authored-by: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com>

commit 4f221ca
Author: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com>
Date:   Thu May 20 13:59:19 2021 +0200

    docs: add vojta7 as a contributor (#178)

    * docs: update README.md [skip ci]

    * docs: update .all-contributorsrc [skip ci]

    Co-authored-by: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com>

commit 7bf0af9
Author: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com>
Date:   Thu May 20 13:57:51 2021 +0200

    docs: add SGudbrandsson as a contributor (#177)

    * docs: update README.md [skip ci]

    * docs: update .all-contributorsrc [skip ci]

    Co-authored-by: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com>

commit 6570b64
Author: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com>
Date:   Thu May 20 13:48:43 2021 +0200

    docs: add sinanmohd as a contributor (#176)

    * docs: update README.md [skip ci]

    * docs: update .all-contributorsrc [skip ci]

    Co-authored-by: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com>

commit 0fa7706
Author: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com>
Date:   Thu May 20 13:47:27 2021 +0200

    docs: add nhamlh as a contributor (#175)

    * docs: update README.md [skip ci]

    * docs: update .all-contributorsrc [skip ci]

    Co-authored-by: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com>
@agonbar agonbar mentioned this pull request Jun 3, 2021
@gchamon gchamon deleted the feature/disable-dns-upstream branch June 3, 2021 09:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants