fix(agent): trust a registry CA given with --ca-file (MK8S-431) - #28
ezekiel-alexrod wants to merge 6 commits into
Conversation
A registry signed by a private CA was unreachable: the puller used the default transport, so only the image's system CAs were trusted and the pull failed with "x509: certificate signed by unknown authority". The manager takes a --ca-file PEM bundle and adds it to the system pool rather than replacing it, so a registry behind a public certificate stays reachable. The file is read once at startup, and an unusable one stops the agent there instead of failing every pull on every node. Relates-to: MK8S-431
The command pulls through the same puller as the agent, so it could not reach a registry signed by a private CA either. import now takes --ca-file too. The CA is loaded before the cache state is read, so a file that cannot be used is reported on every run, including one that finds the resource already complete. An archive source reaches no registry and never reads it. Relates-to: MK8S-431
The agent README says what --ca-file does and how to hand the CA to the DaemonSet, the root README covers the same flag on imagecachectl, and the design records that the CA extends the system pool and is read once. The kustomize patch mounts the ca.crt key of a registry-ca ConfigMap and points --ca-file at it. It ships commented out, like the other optional patches of config/default. Relates-to: MK8S-431
ezekiel-alexrod
added
P2
There was a problem hiding this comment.
It can be difficult to integrate into MetalK8S deployment, don't know exactly how the Static-OCI-Registry CA Certificate is deployed
There was a problem hiding this comment.
The registry operator can give us this CA. It has a MirrorConfig resource for pods that pull images themselves, like this agent. When you create a MirrorConfig, the operator writes a ConfigMap with the same name, in the same namespace. That ConfigMap holds the registry CA under ca.crt.
So in MetalK8s, the integration (MK8S-166) only needs to create an empty MirrorConfig named registry-ca in the agent's namespace, then mount the ConfigMap the way the patch does. Nobody has to copy the CA by hand.
I tested it on kind with the real stack (operator 298d4d1, static-oci-registry v0.1.0-beta.2, node-agent v0.0.1-alpha.11):
- The empty
MirrorConfigbecomesReady, and theregistry-caConfigMap contains the registry CA. - With the patch, the agent pulls from
metalk8s-registry-server.metalk8s-registry.svc:5000and the node becomessynced. - Without the patch, the node stays
pendingwithx509: certificate signed by unknown authority, the error from the ticket. imagecachectl import --ca-filealso works in a pod that mounts the same ConfigMap.
I also made the comment clearer in 222683a. It now gives the ConfigMap name, the key and the namespace.
There was a problem hiding this comment.
Does it mean that:
- imagecache-operator is deployed in the same Namespace as Registry stack (Not surprising) ?
- When a node first joins the cluster, the imagecachectl will be executed in a Pod ?
This solution seems to me a bit weird
There was a problem hiding this comment.
Can it be useful to add --insecure flag for testing ?
There was a problem hiding this comment.
Good idea for a test cluster. Added in dd489c1 as --insecure-skip-tls-verify, like the kubectl flag. It exists on the agent and on imagecachectl import.
- It is off by default, and no manifest in the repo turns it on.
- The agent logs a warning at startup, and the command prints one before it pulls.
- You can't use it with
--ca-file: the two settings contradict each other, so the agent stops at startup instead of picking one.
On the same kind cluster, the agent pulls fine with it and logs the warning.
There was a problem hiding this comment.
Can it be useful to add --insecure flag for testing ?
There was a problem hiding this comment.
Same flag here, see my reply on main.go. The agent and the command build their registry client with the same function, puller.NewRemote.
The flag showed up under Options, but the usage line above it still read as if --name and --cache-path were the only ones. Relates-to: MK8S-431
The comment said to uncomment "the following line" above three lines, and left the reader to guess that "it" was the CA and that the ConfigMap belongs in the agent's namespace. It isn't a kustomize resource, so neither namespace nor namePrefix is applied to it. Relates-to: MK8S-431
Setting up a test cluster by hand means fetching the registry CA before the agent can pull anything. The manager and imagecachectl import now take --insecure-skip-tls-verify, which accepts any certificate. It is off by default and no manifest sets it. The agent logs a warning at startup and the command prints one before it pulls. It excludes --ca-file: given both, the agent stops at startup and the command exits 2, so neither setting wins in silence. Relates-to: MK8S-431
There was a problem hiding this comment.
Does it mean that:
- imagecache-operator is deployed in the same Namespace as Registry stack (Not surprising) ?
- When a node first joins the cluster, the imagecachectl will be executed in a Pod ?
This solution seems to me a bit weird
| // newRemote builds on base; tests hand it a transport that dials their own | ||
| // server. | ||
| func newRemote(cfg TLS, base *http.Transport) (Remote, error) { | ||
| t := base.Clone() |
There was a problem hiding this comment.
nit: why cloning the given base ?
2 participants
Component
agent, docs
Problem
The agent can't pull from a registry signed by a private CA. The puller used go-containerregistry's default transport, so it only trusted the system CAs of the distroless image, and there was no way to hand it another one. The pull fails with
x509: certificate signed by unknown authorityand the node stayspending.imagecachectlgoes through the samepuller.Remote, so it has the same problem. That's why this branch sits on top of #25.Fix
The manager and
imagecachectl importboth take--ca-file, a PEM bundle.puller.NewRemoteadds it to the system pool rather than replacing it, so a registry behind a public certificate stays reachable next to the private one (agent/internal/puller/puller.go).imagecachectlloads the CA before it reads the cache state. A bad file gets reported on every run, even one that finds the resource complete, and not only on the run that finally needs the registry. An archive source never reads it.config/default/manager_registry_ca_patch.yamlmounts theca.crtkey of aregistry-caConfigMap and points--ca-fileat it. It ships commented out inconfig/default/kustomization.yaml, like the other optional patches there.namePrefixdoesn't touch the ConfigMap name, because the ConfigMap isn't a kustomize resource.I rejected a CA field on the
ImageCacheresource. It would mean a CRD change, RBAC on Secrets and one more CEL rule, for a setup with one registry per cluster. The flag followsCACertFilePathin metalk8s-registry-node-agent.Test
make -C agent test,make -C agent lintandmake -C agent test-e2eare green (Go 1.26.0, lint cache cleared, 2 of 2 e2e specs).x509.UnknownAuthorityError, which is the ticket's error. With it they pull. A missing file or a non-PEM file givesErrCAand names the path. The reference usesexample.comand not the loopback address, because go-containerregistry talks plain HTTP to a loopback registry and a test there would never check a certificate.registry:2with TLS signed by a throwaway CA: without the patch the node is labelledpendingand the logs show the x509 error. With the patch and the ConfigMap it's labelledsyncedandpause.taris on the node. A ConfigMap holding garbage puts the pod in CrashLoopBackOff withunusable registry CA: no PEM certificate in the file: path='/etc/image-cache/registry-ca/ca.crt'.imagecachectl importagainst the same registry exits 1 on the x509 error without--ca-fileand 0 with it.Out of scope
ImageCache, registry auth, client certificates, reloading the CA without a restart.README.md,agent/README.md,DESIGN.md,agent/DESIGN.md,CONTRIBUTING.md, whichever owns it.names, sentinel, permissions) lands in both halves and in
agent/DESIGN.md. Not applicable to most pull requests.Relates-to: MK8S-431