📖 pkg/metrics/server: clarify TLS certificate configuration

[ugiordan]

Summary

Clarify how the metrics server selects its TLS certificate.

Behavior

No runtime behavior changes:

• Existing certificate and key files are loaded and watched for changes.
• If either file is absent when the server starts, a self-signed certificate is generated.
• TLSOpts.GetCertificate is the opt-in path for asynchronously provisioned or dynamically supplied certificates.

Tests

• go test ./pkg/metrics/server
• go test ./pkg/certwatcher

[kubernetes-prow]

Welcome @ugiordan!

It looks like this is your first PR to kubernetes-sigs/controller-runtime 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/controller-runtime has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: ugiordan / name: Ugo Giordano (f4c85d0)

[kubernetes-prow]

Hi @ugiordan. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[sbueringer]

Where did you encounter this issue?

This creates a startup race: certificate provisioners (cert-controller, cert-manager, etc.) are themselves manager runnables, so they only write cert files after mgr.Start() is called

I'm wondering how these tools would write the certificates into the manager Pod after manager startup

[ugiordan]

Where did you encounter this issue?

This creates a startup race: certificate provisioners (cert-controller, cert-manager, etc.) are themselves manager runnables, so they only write cert files after mgr.Start() is called

I'm wondering how these tools would write the certificates into the manager Pod after manager startup

The cert files arrive via a Kubernetes Secret mounted as a volume. A provisioner like cert-controller runs as a manager runnable: it generates a keypair, writes the cert data into a k8s Secret, and the kubelet then syncs that Secret to the pod's CertDir volume mount. That sync is asynchronous and can lag significantly behind pod startup.

On first boot, when the Secret exists but has no cert data yet, the sequence is:

1. Pod starts; the volume mount is present, but the Secret is empty, so no cert files on disk yet
2. mgr.Start() fires both the metrics server listener and the cert provisioner runnable simultaneously
3. The metrics server does the one-shot os.Stat: files absent -> permanently falls to self-signed cert
4. The cert provisioner reconciles, generates certs, and writes them to the Secret
5. kubelet syncs the Secret to the volume (cert files now appear on disk)
6. Files exist on disk — but the metrics server already committed to self-signed for the lifetime of this process

On subsequent restarts, the Secret already has cert data, so the kubelet pre-populates the volume before the container process starts, the os.Stat succeeds, and there's no race. This is why it only bites on the very first pod boot and is easy to miss.

Live cluster validation

I built a minimal reproducer to confirm this on a live cluster. The binary is a single main.go + metrics_tls_opt.go, deployed as two pods from the same image, one running the stock createListener path (-patched=false, the default) and one with the lazy GetCertificate fix (-patched=true). Each pod runs cert-controller as a manager runnable against an empty Secret at startup, with that Secret mounted as a volume at CertDir. After certsReady fires, the binary dials its own :8443 and inspects the TLS cert being served.

main.go

func main() {
	patched := flag.Bool("patched", false, "use lazy cert init (the fix)")
	flag.Parse()

	metricsOpts := metricsserver.Options{
		BindAddress:   ":8443",
		SecureServing: true,
		CertDir:       certDir, // "/tmp/k8s-webhook-server/serving-certs"
	}
	if *patched {
		metricsOpts.TLSOpts = append(metricsOpts.TLSOpts, MetricsTLSOpt())
	}

	mgr, _ := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{Metrics: metricsOpts})

	certsReady := make(chan struct{})
	certrotator.AddRotator(mgr, &certrotator.CertRotator{
		SecretKey:             types.NamespacedName{Namespace: namespace, Name: webhookSecretName},
		CertDir:               certDir,
		CAName:                "cert-race-reproducer-ca",
		CAOrganization:        "cert-race-reproducer",
		DNSName:               fmt.Sprintf("%s.%s.svc", webhookServiceName, namespace),
		IsReady:               certsReady,
		Webhooks:              []certrotator.WebhookInfo{},
		RequireLeaderElection: false,
	})

	go func() {
		<-certsReady
		time.Sleep(2 * time.Second)
		conn, _ := tls.DialWithDialer(&net.Dialer{Timeout: 5 * time.Second}, "tcp",
			"127.0.0.1:8443", &tls.Config{InsecureSkipVerify: true})
		cert := conn.ConnectionState().PeerCertificates[0]
		// logs whether it's the self-signed localhost cert or the cert-controller cert
	}()

	mgr.Start(ctrl.SetupSignalHandler())
}

metrics_tls_opt.go

The fix, mirroring what this PR adds to createListener:

func MetricsTLSOpt() func(*tls.Config) {
	var mu sync.Mutex
	var watcher *certwatcher.CertWatcher
	return func(c *tls.Config) {
		c.GetCertificate = func(hello *tls.ClientHelloInfo) (*tls.Certificate, error) {
			mu.Lock()
			if watcher == nil {
				if w, err := certwatcher.New(certDir+"/tls.crt", certDir+"/tls.key"); err == nil {
					go func() { _ = w.Start(context.Background()) }()
					watcher = w
				}
			}
			w := watcher
			mu.Unlock()
			if w == nil {
				return nil, nil
			}
			return w.GetCertificate(hello)
		}
	}
}

Deployment setup

Each variant gets its own empty Secret pre-created, mounted at CertDir:

# pre-create empty Secrets (cert-controller populates them at runtime)
kubectl create secret generic cert-race-unpatched-certs -n cert-race-reproducer
kubectl create secret generic cert-race-patched-certs   -n cert-race-reproducer

# unpatched pod: no -patched flag, mounts cert-race-unpatched-certs
# patched pod:   -patched flag,    mounts cert-race-patched-certs
volumeMounts:
- name: certs
  mountPath: /tmp/k8s-webhook-server/serving-certs
  readOnly: true
volumes:
- name: certs
  secret:
    secretName: cert-race-{variant}-certs

Results

Timestamps are the key part.

Without the fix:

10:53:40  Starting metrics server
10:53:41  cert-rotation: refreshing CA and server certs         ← provisioner starts generating
10:53:45  Serving metrics server bindAddress=:8443 secure=true  ← one-shot os.Stat ran; files absent → self-signed committed permanently
10:53:48  cert-rotation: server certs refreshed                 ← certs written to Secret (3s after the decision was already made)
10:55:28  cert-rotation: certs are ready in /tmp/k8s-webhook-server/serving-certs  ← kubelet syncs volume ~1m40s later
10:55:30  RESULT: RACE LOST — metrics server is using the self-signed localhost fallback cert
          subject=localhost@1786618425  issuer=localhost-ca@1786618421  dnsNames=[localhost]

With the fix:

10:53:40  Starting metrics server
10:53:41  cert-rotation: refreshing CA and server certs         ← provisioner starts generating
          (listener created with lazy GetCertificate; no permanent decision made yet)
10:55:20  cert-rotation: certs are ready in /tmp/k8s-webhook-server/serving-certs
10:55:22  certwatcher: Updated current TLS certificate          ← lazy init picked up the files on first handshake
10:55:22  certwatcher: Starting certificate poll+watcher
10:55:22  RESULT: RACE WON — metrics server is using the cert-controller cert
          subject=cert-race-reproducer.cert-race-reproducer.svc
          issuer=cert-race-reproducer-ca  dnsNames=[cert-race-reproducer.cert-race-reproducer.svc]

The race window in the unpatched case: Serving metrics server fires at 10:53:45, server certs refreshed fires at 10:53:48, the one-shot check committed to self-signed 3 seconds before the certs were even written to the Secret, and the kubelet took another ~1m40s to sync the volume. With the fix, no permanent decision is made at listener creation; the certwatcher is initialized on the first handshake after the files land.

[alvaroaleman]

You can just set GetCertificate and do in there whatever you want:

1. controller-runtime/pkg/metrics/server/server.go

   Lines 282 to 284 in 884d668

   	for _, op := range s.options.TLSOpts {
   	op(cfg)
   	}

2. controller-runtime/pkg/metrics/server/server.go

   Line 312 in 884d668

   if cfg.GetCertificate == nil {

/hold

[ugiordan]

/easycla

[ugiordan]

You can just set GetCertificate and do in there whatever you want:

1. controller-runtime/pkg/metrics/server/server.go

   Lines 282 to 284 in 884d668

   	for _, op := range s.options.TLSOpts {
   	op(cfg)
   	}

2. controller-runtime/pkg/metrics/server/server.go

   Line 312 in 884d668

   if cfg.GetCertificate == nil {

/hold

@alvaroaleman You're right, setting GetCertificate via TLSOpts is a valid way to handle this and avoids the self-signed fallback at L312.

I opened this PR because the one-shot os.Stat in createListener means that if cert files aren't on disk when the listener starts, the server permanently serves a self-signed cert. That race shows up with cert provisioners that write files after mgr.Start().

I'd still lean toward fixing this in controller-runtime. Expecting every operator to discover the TLSOpts workaround for a race that only shows up on first boot (and then silently serves the wrong cert) doesn't scale well. If CertDir is set, retrying until the files appear seems like the right framework behavior.

[alvaroaleman]

Your fix breaks anyone who wants the generated certs. The only way to avoid that would be to add yet another knob which I am opposed to. Feel free to improve the godocs

[ugiordan]

Your fix breaks anyone who wants the generated certs. The only way to avoid that would be to add yet another knob which I am opposed to. Feel free to improve the godocs

Sounds good. I'll drop the behavior change. For the godoc improvements, do you prefer I repurpose this PR or close it and open a follow-up?

[alvaroaleman]

Sounds good. I'll drop the behavior change. For the godoc improvements, do you prefer I repurpose this PR or close it and open a follow-up?

Re-using the same PR is fine but please update the commit message and title

[alvaroaleman]

@ugiordan plase fix the CLA check, co-authored-by: LLM trailers are generally not allowed since the LLM may be helping you, but it can not take responsibility for the result

[ugiordan]

@ugiordan plase fix the CLA check, co-authored-by: LLM trailers are generally not allowed since the LLM may be helping you, but it can not take responsibility for the result

@alvaroaleman you’re right, sorry about that. Thanks for pointing it out.

[alvaroaleman]

thanks!
/hold cancel

[kubernetes-prow]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alvaroaleman, ugiordan

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [alvaroaleman]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[kubernetes-prow]

LGTM label has been added.

Details

Git tree hash: 0c7d161ce3f832c7d94c60b3eb5c4cd97f7ef600