api: add "Listeners" field to /info, to help discover how the API is exposed#43459
api: add "Listeners" field to /info, to help discover how the API is exposed#43459thaJeztah wants to merge 1 commit intomoby:masterfrom
Conversation
daemon/info.go
Outdated
| continue | ||
| } | ||
| if addr.Scheme != "tcp" { | ||
| v.Listeners = append(v.Listeners, systemtypes.ListenerInfo{Address: addr.String()}) |
There was a problem hiding this comment.
In this case, Insecure should be nil (with omitempty) rather than false?
I'd except Insecure to be false only when mTLS is fully configured.
Or maybe change the property name to MTLS bool?
There was a problem hiding this comment.
Yes, so I was a bit struggling with this part. Perhaps it should be changed indeed (could use input from you and others though);
- I wanted to have this information per listener (as a combination of "secure" and "insecure" listeners is possible)
- Using
Insecure boolmakes it "easier" to present this on the cli side without having to implement complicated logic on the client side (we can print an(insecure)after the host (e.g.), and - I picked
Insecureto allow it to be a bit more "flexible / generic", as there may be various reasons for a listener to be "insecure" (e.g./var/run/docker.sockhaving "world-readable" permissions), and with "adding other kind of listeners to the list" in the back of my mind.
However, as you pointed out perhaps it's too generic? I considered a Warning(s) field per Listener, which could be useful to provide arbitrary "additional" information that may not make sense to include in a fully structured approach. But the info struct already has a global Warnings field, which can include such warnings, so adding another Warnings field felt like it may be "too much" at this time (but perhaps could be considered?)
While looking at this code, I also saw that daemon.loadListeners() uses a slightly more complete check to determine if a listener is "secure", so there's some inconsistency. We should probably have a single approach for all of these (and if needed "go the extra mile" to make sure it's actually secure (MTLS properly configured, not just "flags are set"));
Line 658 in 011e1c7
There was a problem hiding this comment.
/cc @cpuguy83 @tianon @tonistiigi - could use more eyes/input on this. I think this information (Listeners) can be quite useful, but could use input on the design / implementation (see above)
| // Insecure indicates if the address is insecure, which could be either | ||
| // if the daemon is configured without TLS client verification, or if | ||
| // TLS verify is disabled. | ||
| Insecure bool |
There was a problem hiding this comment.
Maybe break down Address into a Type (tcp, unix-socket, named-pipe) and an address.
Then add an extra field to show the config like:
type ListenerInfo {
Type string
Address string
TCPConfig *ListenerTCPConfig `json:"tcp-config,omitempty"`
}
type ListenerTCPConfig {
Encrypted bool
Authenticated bool
}
…exposed
The daemon can be configured to listen on multiple endpoints, which can make
it difficult to discover how the API is exposed (e.g. if the daemon is listening
both on `/var/run/docker.sock` and through tcp).
This patch adds a `Listeners` field to the `/info` response, listing on what
addresses (and/or sockets) the API is exposed on to make it more discoverable.
Currently only API listeners are included, but as a follow-up, we can include
other listeners (metrics socket, SwarmKit control and data-plane).
With this patch:
```bash
dockerd
curl -s --unix-socket /var/run/docker.sock http://localhost/info | jq .Hosts
[
{
"Address": "unix:///var/run/docker.sock",
"Insecure": false
}
]
dockerd -H localhost:2375 -H unix:///var/run/docker.sock
curl -s --unix-socket /var/run/docker.sock http://localhost/info | jq .Hosts
[
{
"Address": "http://localhost:2375",
"Insecure": true
},
{
"Address": "unix:///var/run/docker.sock",
"Insecure": false
}
]
```
Signed-off-by: Sebastiaan van Stijn <[email protected]>
thaJeztah
force-pushed
the
daemon_info_add_listeners
branch
from
a195085 to
8f1bfcc
Compare
|
(only rebased, haven't looked at updating the implementation yet) |
4 participants
The daemon can be configured to listen on multiple endpoints, which can make
it difficult to discover how the API is exposed (e.g. if the daemon is listening
both on
/var/run/docker.sockand through tcp).This patch adds a
Listenersfield to the/inforesponse, listing on whataddresses (and/or sockets) the API is exposed on to make it more discoverable.
Currently only API listeners are included, but as a follow-up, we can include
other listeners (metrics socket, SwarmKit control and data-plane).
With this patch:
- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)