Add config/environment/cli options to configure docker socket URL
The main objective of this PR is to add a couple configuration options to specify the URL to connect to the docker daemon and to specify the docker API version to use. The idea is to use tecnativa/docker-socket-proxy to restrict and control the access of DPS to the docker demon.
I also created some tests, updated the documentation and provided an example docker compose that shows how to use the new feature.
Besides the PR also does:
- [X] Update DPS to use go 1.14
- [X] Migrate DPS govendor modules to go native modules
- [X] Migrate the deprecated docker cli / types dependencies to use the current ones.
If you prefer / think that these additional changes require a separate PR let me know.
I just updated the PR, to enable the ability use a hostname / container_name to in MG_DOCKER_HOST that is resolved before launching dns-proxy-server instead of of using an IP
@mageddo any update on this ?
Well, these are a lot of changes to be done at one shot, not sure if they are dependent. I think we may try isolate the Add config/environment/cli options to configure docker socket URL feature, then we can merge it ASAP.
Do you wanna do it by yourself or prefer me to do the job @imartinezortiz ?
Let me do a test and I'll get back to you. If I remember correctly the main reason of all these changes were that the docket docket proxy requires at least docker API v 1.27 and the dependency that DPS is currently using it is compatible up to v1.21 because it is deprecated.
On Fri, Jul 10, 2020, 01:12 Elvis Souza [email protected] wrote:
Well, these are a lot of changes to be done at one shot, not sure if they are dependent. I think we may try isolate the Add config/environment/cli options to configure docker socket URL feature, then we can merge it ASAP.
Do you wanna do it by yourself or prefer me to do the job @imartinezortiz https://github.com/imartinezortiz ?
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mageddo/dns-proxy-server/pull/195#issuecomment-656394473, or unsubscribe https://github.com/notifications/unsubscribe-auth/AALF7P2NU5ZURGPKF2YRYHDR2ZFETANCNFSM4MZBSXOQ .
I've done some tests just adding the configuration options and seems to work at least for my use case. I'm getting some panics from time to time and that is the reason I tried to update the API, but to be honest, I also got some panics after doing the update (golang and docker API dependency).
Reviewing both DPS and tecnativa/docker-socket-proxy, maybe the problem is related due to the fact docker-socket-proxy return a HTML response if the request it is forbidden and DPS panics.
To sum up:
-
I can change the PR just to include the changes to add the two command line options.
-
Seems that at least it is needed to change
mustInspectContainerto not panic, but I do not know the impact that may have this change. Seems that there is a design decision in DPS about panicking and exit instead of handling issues in other ways.Besides, what about the other changes: go update, migration to google modules, etc. ? should I open a different PR(s).
# This is the panic I got today
dns-proxy-server_1 | panic: status=inspect-error, id=: json: cannot unmarshal array into Go value of type types.ContainerJSON
dns-proxy-server_1 |
dns-proxy-server_1 | goroutine 19 [running]:
dns-proxy-server_1 | github.com/mageddo/dns-proxy-server/events/docker.mustInspectContainer(0x8fff60, 0xc00034cf00, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0)
dns-proxy-server_1 | /app/src/github.com/mageddo/dns-proxy-server/events/docker/docker.go:116 +0x24b
dns-proxy-server_1 | github.com/mageddo/dns-proxy-server/events/docker.HandleDockerEvents()
dns-proxy-server_1 | /app/src/github.com/mageddo/dns-proxy-server/events/docker/docker.go:90 +0xd40
dns-proxy-server_1 | created by main.main
dns-proxy-server_1 | /app/src/github.com/mageddo/dns-proxy-server/dns.go:126 +0xe6
# This is the panic I got during the last two months using all the changes included in this PR.
panic: status=inspect-error, id=7134e8ae0488f877d11d1b2ac48846db920d4a6612bb4e828075907bee961d7d: Error: No such container: 7134e8ae0488f877d11d1b2ac48846db920d4a6612bb4e828075907bee961d7d
goroutine 18 [running]:
github.com/mageddo/dns-proxy-server/events/docker.mustInspectContainer(0x918500, 0xc00010e090, 0xc00011c040, 0x40, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0)
/app/src/github.com/mageddo/dns-proxy-server/events/docker/docker.go:113 +0x255
github.com/mageddo/dns-proxy-server/events/docker.HandleDockerEvents()
/app/src/github.com/mageddo/dns-proxy-server/events/docker/docker.go:82 +0xce3
created by main.main
/app/src/github.com/mageddo/dns-proxy-server/dns.go:137 +0x175
Hey, I was stale for some time, below some updates:
- DPS was refactored on 3.x.x so this PR isn't compatible anymore , see #267
- This feature was implemented at #379
So I'm closing this one, thanks for the help, sorry we didn't solve it earlier.