-
-
Notifications
You must be signed in to change notification settings - Fork 131
local DNS server via dnsmasq #2168
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
Conversation
…m domains - dnsmasq docker image - dnsmasq network bridge - point *.sndev to 127.0.0.1 - set-dnsmasq script - -- add/remove/list dns records in dnsmasq.conf - add 'domains' to sndev - 'sndev domains dns' referencing set-dnsmasq script
Awesome! I'll give it a gander soon (probably monday). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just a quick glance and tried to run sndev
but got an error, see comment
docker-compose.yml
Outdated
driver: bridge | ||
ipam: | ||
config: | ||
- subnet: 172.20.0.0/24 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
172.20.0.0/24 was already taken by a docker interface on my machine:
$ sndev start
[+] Building 3.7s (27/27) FINISHED docker:default
=> [db internal] load build definition from Dockerfile 0.0s
=> => transferring dockerfile: 244B 0.0s
=> [db internal] load metadata for docker.io/library/postgres:16.3 0.9s
=> [db internal] load .dockerignore 0.0s
=> => transferring context: 2B 0.0s
=> [db 1/2] FROM docker.io/library/postgres:16.3@sha256:d0f363f8366fbc3f52d172c6e76bc27151c3d643b870e1062b4e8bfe65baf609 0.0s
=> CACHED [db 2/2] RUN apt-get update && apt-get install --no-install-recommends -y postgresql-16-ip4r && rm -rf /var/lib/apt/lists/* 0.0s
=> [db] exporting to image 0.0s
=> => exporting layers 0.0s
=> => writing image sha256:5e15736b8a66c1bf089b903b682c4fc20a06ee20ec7495fb9ea0dca12b2e2372 0.0s
=> => naming to docker.io/library/stackernews-db 0.0s
=> [db] resolving provenance for metadata file 0.0s
=> [app internal] load build definition from Dockerfile 0.0s
=> => transferring dockerfile: 458B 0.0s
=> [worker] resolve image config for docker-image://docker.io/docker/dockerfile:1 1.4s
=> CACHED [worker] docker-image://docker.io/docker/dockerfile:1@sha256:9857836c9ee4268391bb5b09f9f157f3c91bb15821bb77969642813b0d00518d 0.0s
=> [app internal] load build definition from Dockerfile 0.0s
=> [worker internal] load metadata for docker.io/library/node:18.20.4-bullseye 1.1s
=> [app internal] load .dockerignore 0.0s
=> => transferring context: 2B 0.0s
=> [worker 1/4] FROM docker.io/library/node:18.20.4-bullseye@sha256:2ab5a84870fbe448d216fd421d5721a4bf3928cdeef70d8966c8de4460fa2edc 0.0s
=> [app internal] load build context 0.0s
=> => transferring context: 72B 0.0s
=> CACHED [worker 2/4] RUN groupadd -fg "100" apprunner 0.0s
=> CACHED [worker 3/4] RUN useradd -om -u "1000" -g "100" apprunner 0.0s
=> CACHED [worker 4/4] WORKDIR /app 0.0s
=> CACHED [app 5/6] COPY package.json package-lock.json ./ 0.0s
=> CACHED [app 6/6] RUN npm ci --legacy-peer-deps --loglevel verbose 0.0s
=> [app] exporting to image 0.0s
=> => exporting layers 0.0s
=> => writing image sha256:2277f37468659e46935a774f74f6756d7c757f73745348bdeff2397f1d5e1bf9 0.0s
=> => naming to docker.io/library/stackernews-app 0.0s
=> [app] resolving provenance for metadata file 0.0s
=> [worker internal] load build definition from Dockerfile 0.0s
=> => transferring dockerfile: 247B 0.0s
=> [worker internal] load build definition from Dockerfile 0.0s
=> [worker internal] load .dockerignore 0.0s
=> => transferring context: 2B 0.0s
=> [worker] exporting to image 0.0s
=> => exporting layers 0.0s
=> => writing image sha256:d9f4aa1a5e316bd39a5dcde7f0ba651f0da3540e11c0ab9b098d62839b3c3bf0 0.0s
=> => naming to docker.io/library/stackernews-worker 0.0s
=> [worker] resolving provenance for metadata file 0.0s
[+] Running 2/1
✔ Network stackernews_default Created 0.1s
✘ Network domains-network Error 0.0s
failed to create network domains-network: Error response from daemon: invalid pool request: Pool overlaps with other one on this address space
It worked when I changed this to 172.21.0.0/24 and 172.20.0.2 to 172.21.0.2.
But do we need to hard code a subnet here? Is it not possible to let docker manage that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm, I wanted to be safer than sorry by declaring a dedicated IP for dnsmasq. IIRC at first I was using dnsmasq as global DNS server for worker and app, which on docker-compose should require an IP to point at.
Since I'm not doing that anymore because it would've been stupid, I can actually use the container's hostname as DNS server for the DNS verification job and avoid a subnet! Thanks ^^
edit: ok no, an hostname is not RFC 5952 compliant. Now that I remember, I ultimately used a dedicated subnet because node:dns
requires an IP and my previous assumption that 127.0.0.1 would work is wrong as it would point at the worker
container's localhost.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Most of my comments are nits, but this does not edit my /etc/hosts
file even though it says it will.
If we can't edit the file reliably, it might be better to just give instructions. At least, if there's an error adding it, we should say so. AFAICT it just exits without saying anything.
[keyan stackernews]🍏 sndev domains dns add cname bitcoin.sndev sn.sndev
Adding record: cname=bitcoin.sndev,sn.sndev
While sndev will use dnsmasq DNS server, your system won't use it by default.
You can either manually point DNS to 127.0.0.1:5353 to access it system-wide,
or add this record to /etc/hosts to access it via browser.
[sudo] Do you want to add '127.0.0.1 bitcoin.sndev' to /etc/hosts? [y/N] y
I could not get the script to modify my /etc/hosts
even with sudo. Do you know what could be wrong?
If I manually add the entry to my /etc/hosts
it works.
scripts/set-dnsmasq
Outdated
@@ -0,0 +1,244 @@ | |||
#!/usr/bin/env bash |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We try to make sure sndev
scripts conform to POSIX shell so that we don't have to assume which shell a contributor has.
This isn't a dealbreaker, but some folks will probably have issues running these dns commands.
You can test your POSIX compliance with shellcheck.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! Turned out that the sudo problem you were facing was because of SC2024, didn't notice it on my config, sorry for the overlook ^^
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm going to retest this. Also, because the docker/dnsmasq/dnsmasq.conf
file is written to by sndev
sometimes, and it's checked in, it's going to be really easy to accidentally commit changes unintentially.
Is there a way to provide a default configuration, then .gitignore
what sndev
modifies? ... or something like that. Kind of like how we have docker-compose.override.yml
or .env.local
for personal changes that we don't want committed.
Yes and actually really needed, I was having an hard time myself. edit: on a different topic, I'm going to do more POSIX compliance |
Cool. I just tested again, and it's good enough to merge so we can unblock review of the important stuff. I'll let you wrench on it until tomorrow though. |
The |
Also
|
…ust TXT type only on list
Fixed, sorry about it. I forgot to test it iterations after iterations.
Oh the odyssey of BSD/GNU differences... BSD |
All good. Merging to begin QA/review of #2145. |
Description
In support of #2145
It adds dnsmasq to docker-compose, allowing local DNS management to mock DNS behavior for custom domain verification.
set-dnsmasq
composes a line based on the chosen type [CNAME/TXT] that results in:cname=www.pizza.sndev,sn.sndev
txt-record=www.pizza.sndev,"EXAMPLETXT"
When you add a
cname
, this script will give directions to use dnsmasq as DNS server or ask if you want to add a rule to /etc/hosts to access the virtual host via browser.The user can avoid getting asked to do that by appending the
--no-hosts
parameter to add/remove commands.cli
sndev
sndev domains
sndev domains dns
sndev domains dns add cname pizzapoint.sndev sn.sndev
Additional Context
It would've been more elegant to use dnsmasq as dns server, avoiding edits to /etc/hosts, but while on macOS there's a single edit involved (
/etc/resolver/domain
), on Linux there's not a preferred method.Therefore we request the user to set it manually if they'd like to do so.
Checklist
Are your changes backwards compatible? Please answer below:
Yes, it retains the default docker network
On a scale of 1-10 how well and how have you QA'd this change and any features it might affect? Please answer below:
6, add/remove/list work correctly, DNS gets picked up correctly by the domainVerification worker in custom domains PR.
For frontend changes: Tested on mobile, light and dark mode? Please answer below:
n/a
Did you introduce any new environment variables? If so, call them out explicitly here:
n/a