daemon/config: move proxy settings to "proxies" struct within daemon.json#43448
Conversation
thaJeztah
changed the title
| // ProxyConfig holds the proxy-configuration for the daemon. | ||
| ProxyConfig `json:"proxy-config"` |
There was a problem hiding this comment.
FWIW, I was considering to have an omitempty, but that required the struct to be a pointer, and all the complexity involved with that, so I decided not to go down that road
thaJeztah
force-pushed
the
daemon_config_group_proxies
branch
from
6d59234 to
5e90b9f
Compare
This comment was marked as resolved.
This comment was marked as resolved.
thaJeztah
force-pushed
the
daemon_config_group_proxies
branch
from
5e90b9f to
5e743bc
Compare
|
@tianon @tonistiigi @cpuguy83 PTAL this feature hasn't shipped yet, so if you agree this makes it better, we should try to get this in before the release 👍 |
|
Erring on the side of not making changes, I'm not sure what's the chance this would be overshadowed by something else down the line? |
That's about the Go code changes, or the JSON side? I can drop the second commit if you prefer. For the JSON side; it's not released yet, so we can still modify. The issue was a bit that when pretty-printing the JSON, these settings won't be next to each-other (even more if we'd ever add a "all-proxy" option, e.g.); grouping them together could possibly also make things easier to patch them all together, but perhaps no big difference there. |
thaJeztah
force-pushed
the
daemon_config_group_proxies
branch
from
5e743bc to
97bf8b3
Compare
|
I dropped the second commit (I think that's what you meant) let me know if this LGTY |
| ProxyConfig | ||
|
|
||
| // ProxyConfig holds the proxy-configuration for the daemon. | ||
| ProxyConfig `json:"proxy-config"` |
There was a problem hiding this comment.
How about just proxies?
I know you reverted some go changes (I assume it was giving this an explicit field name).
I think it's good to give it a proper field name.
There was a problem hiding this comment.
Proxies works for me (I was wondering if there would be other proxy-related options that could become relevant); can you think of any?
There was a problem hiding this comment.
👍 let me rename it to proxies
…json This is a follow-up to 427c7cc, which added proxy-configuration options ("http-proxy", "https-proxy", "no-proxy") to the dockerd cli and in `daemon.json`. While working on documentation changes for this feature, I realised that those options won't be "next" to each-other when formatting the daemon.json JSON, for example using `jq` (which sorts the fields alphabetically). As it's possible that additional proxy configuration options are added in future, I considered that grouping these options in a struct within the JSON may help setting these options, as well as discovering related options. This patch introduces a "proxies" field in the JSON, which includes the "http-proxy", "https-proxy", "no-proxy" options. Conflict detection continues to work as before; with this patch applied: mkdir -p /etc/docker/ echo '{"proxies":{"http-proxy":"http-config", "https-proxy":"https-config", "no-proxy": "no-proxy-config"}}' > /etc/docker/daemon.json dockerd --http-proxy=http-flag --https-proxy=https-flag --no-proxy=no-proxy-flag --validate unable to configure the Docker daemon with file /etc/docker/daemon.json: the following directives are specified both as a flag and in the configuration file: http-proxy: (from flag: http-flag, from file: http-config), https-proxy: (from flag: https-flag, from file: https-config), no-proxy: (from flag: no-proxy-flag, from file: no-proxy-config) Signed-off-by: Sebastiaan van Stijn <[email protected]>
thaJeztah
changed the title
thaJeztah
force-pushed
the
daemon_config_group_proxies
branch
from
97bf8b3 to
101dafd
Compare
|
Updated 👍 |
|
thx! |
3 participants
daemon/config: move proxy settings to "proxies" struct within daemon.json
This is a follow-up to 427c7cc (#42835), which added proxy-configuration options ("http-proxy", "https-proxy", "no-proxy") to the dockerd cli and in
daemon.json.While working on documentation changes for this feature, I realised that those options won't be "next" to each-other when formatting the daemon.json JSON, for example using
jq(which sorts the fields alphabetically). As it's possible that additional proxy configuration options are added in future, I considered that grouping these options in a struct within the JSON may help setting these options, as well as discovering related options.This patch introduces a "proxies" field in the JSON, which includes the "http-proxy", "https-proxy", "no-proxy" options.
Conflict detection continues to work as before; with this patch applied:
cat /etc/docker/config.json { "proxies": { "http-proxy": "http-config", "https-proxy": "https-config", "no-proxy": "no-proxy-config" } } dockerd --http-proxy=http-flag --https-proxy=https-flag --no-proxy=no-proxy-flag --validate unable to configure the Docker daemon with file /etc/docker/daemon.json: the following directives are specified both as a flag and in the configuration file: http-proxy: (from flag: http-flag, from file: http-config), https-proxy: (from flag: https-flag, from file: https-config), no-proxy: (from flag: no-proxy-flag, from file: no-proxy-config)- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)