Registry configuration package#4138
Conversation
Add `remotes/certutil` functions for loading `ca.crt`, `client.cert`, and `client.key` into `tls.Config` from a directory like `/etc/docker/certs.d/<hostname>. See https://docs.docker.com/engine/security/certificates/ . Client applications including CRI plugin are expected to configure the resolver using these functions. As an example, the `ctr` tool is extended to support `ctr images pull --certs-dir=/etc/docker/certs.d example.com/foo/bar:baz`. Tested with Harbor 1.8. Signed-off-by: Akihiro Suda <[email protected]>
|
Build succeeded.
|
bb775e3 to
8f5a61b
Compare
|
Build succeeded.
|
8f5a61b to
7c4229d
Compare
|
Build succeeded.
|
Add configuration toml file format and configuration function to configure registry hosts from a directory based configuration. Compatible with Docker registry certificate loading. Signed-off-by: Derek McGowan <[email protected]>
Moved registry host configuration to the config package and allows support of loading configurations from a directory when the hosts are being resolved. Signed-off-by: Derek McGowan <[email protected]>
7c4229d to
547301c
Compare
|
Build succeeded.
|
Codecov Report
@@ Coverage Diff @@
## master #4138 +/- ##
==========================================
- Coverage 42.42% 42.09% -0.33%
==========================================
Files 130 133 +3
Lines 14884 15209 +325
==========================================
+ Hits 6314 6402 +88
- Misses 7651 7878 +227
- Partials 919 929 +10
Continue to review full report at Codecov.
|
| "github.com/pkg/errors" | ||
| ) | ||
|
|
||
| type hostConfig struct { |
There was a problem hiding this comment.
Hi, if cri plugin want to use some tls configuration, cri plugin can not reach here.
I think maybe cri plugin also need this feature, and plugin just use WithSkipVerify(args) or WithHostDir(args) to finish
There was a problem hiding this comment.
The CRI plugin already has its own way of turning its configuration into a registry host list. Instead of trying to expand the CRI configuration to support everything this does, I would rather allow CRI plugin to specify a hosts directory so containerd doesn't require a restart to pick up registry configuration changes. If we decide it is best to share this code in CRI configuration, we can export it then, that might not be the best route though.
There was a problem hiding this comment.
With users moving from dockershim I think this would be pretty desirable. Has there been any follow-up on this?
| tlsConfig.Certificates = append(tlsConfig.Certificates, cert) | ||
| } | ||
| } | ||
| tr := defaultTransport.Clone() |
There was a problem hiding this comment.
Looks like this makes it break on Go < 1.13;
go get golang.org/dl/go1.12.17 && go1.12.17 download
go1.12.17 run cmd/gen-manpages/main.go ctr man/
# github.com/containerd/containerd/remotes/docker/config
../../go/src/github.com/containerd/containerd/remotes/docker/config/hosts.go:193:27: defaultTransport.Clone undefined (type *http.Transport has no field or method Clone)
Finally finished this one up, please review the configuration file format. This builds on @AkihiroSuda 's PR which adds support for Docker certificate directory configuration. In addition to that, a
hosts.tomlfile can exist in that directory to full specify all the registry hosts, including host specific capabilities and certificates. See the unit test for an example of the format. I can write up some documentation once there is consensus on the format for it.