dns-proxy-server icon indicating copy to clipboard operation
dns-proxy-server copied to clipboard

Documentation not accurate regarding the use of MG_REGISTER_CONTAINER_NAMES

Open imartinezortiz opened this issue 5 years ago • 1 comments

What is Happening

DPS documentation shows that is possible to use the environment variable MG_REGISTER_CONTAINER_NAMES to configure the registerContainerNames configuration option.

Because the documentation states that the default value is false a user may expect to enable the feature using true for the environment variable, however true (or any other value besides 1) does not work.

What is expected

I expect to pass the param -e MG_REGISTER_CONTAINER_NAMES="true" and DPS will register all running containers

Steps to Reproduce

docker run --name dps -e MG_REGISTER_CONTAINER_NAMES="true" -e MG_DOMAIN="example.org" -p "127.1.1.1:53:53/udp" -v "/var/run/docker.sock:/var/run/docker.sock:ro" -d -p 5380:5380 defreitas/dns-proxy-server:2.19.0
docker run --name whoami -d containous/whoami
dig @127.1.1.1 whoami.example.org
#; <<>> DiG 9.11.3-1ubuntu1.11-Ubuntu <<>> @127.1.1.1 whoami.example.org
#; (1 server found)
#;; global options: +cmd
#;;# Got answer:
#;; ->>HEADER<<- opcode: QUERY, status: NXDOMAIN, id: 59080
#;; flags: qr rd ra; QUERY: 1, ANSWER: 0, AUTHORITY: 1, ADDITIONAL: 0
#
#;; QUESTION SECTION:
#;whoami.example.org.		IN	A
#
#;; AUTHORITY SECTION:
#example.org.		591	IN	SOA	ns.icann.org. noc.dns.icann.org. 2019121386 7200 3600 1209600 3600
#
#;; Query time: 31 msec
#;; SERVER: 127.1.1.1#53(127.1.1.1)
#;; WHEN: Mon May 04 20:43:17 UTC 2020
#;; MSG SIZE  rcvd: 112

# Now with  MG_REGISTER_CONTAINER_NAMES=1
docker stop dps; docker rm dps
docker run --name dps -e MG_REGISTER_CONTAINER_NAMES="1" -e MG_DOMAIN="example.org" -p "127.1.1.1:53:53/udp" -v "/var/run/docker.sock:/var/run/docker.sock:ro" -d -p 5380:5380 defreitas/dns-proxy-server:2.19.0
dig @127.1.1.1 whoami.example.org

#; <<>> DiG 9.11.3-1ubuntu1.11-Ubuntu <<>> @127.1.1.1 whoami.example.org
#; (1 server found)
#;; global options: +cmd
#;; Got answer:
#;; ->>HEADER<<- opcode: QUERY, status: NOERROR, id: 2907
#;; flags: qr rd; QUERY: 1, ANSWER: 1, AUTHORITY: 0, ADDITIONAL: 0
#;; WARNING: recursion requested but not available
#
#;; QUESTION SECTION:
#;whoami.example.org.		IN	A
#
#;; ANSWER SECTION:
#whoami.example.org.	0	IN	A	172.17.0.3
#
#;; Query time: 0 msec
#;; SERVER: 127.1.1.1#53(127.1.1.1)
#;; WHEN: Mon May 04 20:45:12 UTC 2020
#;; MSG SIZE  rcvd: 70

Seems that the behavior is related to https://github.com/mageddo/dns-proxy-server/blob/master/conf/conf.go#L105.

Can I suggest to change the code to?:

if v := os.Getenv(env.MG_REGISTER_CONTAINER_NAMES).ToLower(); len(strings.TrimSpace(v)) > 0 {
    return v != "false" && v != "0"
}

Specs:

  • OS: Ubuntu 18.04
  • Docker Version:
Client: Docker Engine - Community
 Version:           19.03.8
 API version:       1.40
 Go version:        go1.12.17
 Git commit:        afacb8b7f0
 Built:             Wed Mar 11 01:25:46 2020
 OS/Arch:           linux/amd64
 Experimental:      false

Server: Docker Engine - Community
 Engine:
  Version:          19.03.8
  API version:      1.40 (minimum version 1.12)
  Go version:       go1.12.17
  Git commit:       afacb8b7f0
  Built:            Wed Mar 11 01:24:19 2020
  OS/Arch:          linux/amd64
  Experimental:     false
 containerd:
  Version:          1.2.13
  GitCommit:        7ad184331fa3e55e52b890ea95e65ba581ae3429
 runc:
  Version:          1.0.0-rc10
  GitCommit:        dc9208a3303feef5b3839f4323d9beb36df0a9dd
 docker-init:
  Version:          0.18.0
  GitCommit:        fec3683
  • DPS Version: 2.19.0

imartinezortiz avatar May 04 '20 20:05 imartinezortiz

Yep, it's tricky because the code is expecting for 1 and the doc says it expects for true or false.

Sure I think we could accept the true word as a boolean flag, but the code should understand true (case insensitive) or 1 as true, not blabla as true and yes false.

Actually none of the flags supports true keyword so I think we may create a common function and apply to all the flags present on that file

mageddo avatar Jul 09 '20 23:07 mageddo

Sorry I'm late. Now you're able to activate flags by using 1 or true (case insenstive), see the docs. (3.8.0 is being released right now, will be alive in an hour.

mageddo avatar Mar 04 '23 17:03 mageddo