The filtered Secret lister silently turns a LIST into one API GET per Secret in the namespace
secretNamespaceLister.List performs a live GET against the API server for every Secret the metadata-only cache returns. A selector that matches unlabelled Secrets therefore costs one request per Secret in the namespace, on every call. Nothing warns you at the call site, and the code states that this is not expected to happen:
We currently do not LIST unlabelled Secrets. This log line is here in case we do it sometime in the future at which point we can see whether the metadata functionality is performant enough.
This is that issue. It happened on #9224.
What happened
A commit on that branch widened the keymanager's Secret LIST selector from cert-manager.io/next-private-key=true to labels.Everything(), moving the label test into a predicate applied after the LIST. The reasoning was that client-go's ListAllByNamespace indexes only on namespace and then calls selector.Matches per item, so a selector and a predicate do the same work and cost the same.
That is true of client-go's lister. The keymanager does not use client-go's lister when SecretsFilteredCaching is enabled, which is the default. It uses this one. The metadata cache holds every Secret without the controller.cert-manager.io/fao label, and partialMetadataRemoveAll strips labels to nil, so a selector requiring any label matches nothing there and labels.Everything() matches all of it.
The result was one GET per unrelated Secret in the namespace, per reconcile, for every Certificate. No Certificate in the end-to-end clusters reached Ready, and the job watchdog reported no output for 270 seconds. It reproduced on two different nodes before we found it.
Why it is worth changing
Whether a Secret LIST is cheap or floods the API server depends on a property of the selector — that it requires a label — which is invisible at the call site, three packages away from the lister that cares. The comment quoted above is the only statement of the rule, and it is written as an observation about current callers rather than as a constraint on future ones.
This area has produced bugs before: #8994 and #6494.
Suggestions
Any one of these would have turned a cluster-wide slowdown into an obvious failure:
- Make the log line a signal. It sits at
V(InfoLevel)insideif len(metadataSecrets) > 0, beside a comment saying the branch should not be reached. Log it at warning level with the selector and the count, so it is visible in a normal install. - Better, reject the case. If the selector matches an empty label set, return an error telling the caller to narrow it. The lister knows the rule; the caller does not.
- Failing that, say so at the call site. #9224 now carries a comment on the keymanager selector explaining that the label test must stay in the selector, and a test that fails if it is widened again.
I am happy to send a PR for whichever of these you prefer.
[Claude Opus 5]
Source: cert-manager/cert-manager