Add sidecar injection webhook proposal

[tombentley]

Assisted-by: Claude Opus 4.6 noreply@anthropic.com

[k-wall]

Discussed at Community Call 23rd April. Tom is looking for feedback.

[k-wall]

Sounds very sensible

[tombentley]

Not really. Because the proxy configuration is built from the KroxyliciousSidecarConfig at injection time it means that any subsequent changes to the KroxyliciousSidecarConfig are ignored until the next time the pod get created. That creates a lot of problems:

• The user who has RBAC on the KroxyliciousSidecarConfig might not have RBAC on the Pod, Deployment etc. So it assumes some kind of communication between those users. Such communication might not be possible in practice.
• Erroneous changes to the KroxyliciousSidecarConfig are only discovered at some unbounded time later.

The right way to solve this would be for the proxy to phone home to obtain a configuration from a control plane, and to receive updates to that configuration without the need for Pod restarts. But I really want to avoid boiling the ocean here by blocking this alpha sidecar feature on having such a control plane. I think the control plane piece is something we could add later (also as an alpha feature) and then maybe they would become stable features together later on.

[k-wall]

Thanks @tombentley, looks like a solid proposal. I've left a few comments to be considered.

[tombentley]

Thanks @k-wall @robobario, I think I've addresses your comments, though with a watering down of some of the initial ambition wrt trust boundaries. PTAL.

[tombentley]

Not really. Because the proxy configuration is built from the KroxyliciousSidecarConfig at injection time it means that any subsequent changes to the KroxyliciousSidecarConfig are ignored until the next time the pod get created. That creates a lot of problems:

• The user who has RBAC on the KroxyliciousSidecarConfig might not have RBAC on the Pod, Deployment etc. So it assumes some kind of communication between those users. Such communication might not be possible in practice.
• Erroneous changes to the KroxyliciousSidecarConfig are only discovered at some unbounded time later.

The right way to solve this would be for the proxy to phone home to obtain a configuration from a control plane, and to receive updates to that configuration without the need for Pod restarts. But I really want to avoid boiling the ocean here by blocking this alpha sidecar feature on having such a control plane. I think the control plane piece is something we could add later (also as an alpha feature) and then maybe they would become stable features together later on.

[k-wall]

Hi! We've updated the proposal numbering system to use PR numbers as proposal identifiers.

Action required: Please rebase your PR on main.

Once you rebase, you'll need to rename your proposal file and update the title:

git mv proposals/100-sidecar-injection-webhook.md proposals/100-injection-webhook.md
# Update title: remove any old number prefix and add PR number
sed -i.bak '0,/^# /{s/^# \([0-9]\{3\}\|xxx\|nnn\|000\) - /# 100 - /; t; s/^# /# 100 - /}' proposals/100-injection-webhook.md && rm proposals/100-injection-webhook.md.bak
git add proposals/100-injection-webhook.md
git commit -m "Rename proposal to use PR number"
git push

The GitHub workflow will automatically check your proposal file naming after you push and update this PR description if any corrections are still needed.

See proposals/README.md for the updated workflow.

[k-wall]

Correction: The previous notification had an incorrect filename. Here are the correct commands:

git mv proposals/100-sidecar-injection-webhook.md proposals/100-sidecar-injection-webhook.md
# Update title: remove any old number prefix and add PR number
sed -i.bak '0,/^# /{s/^# \([0-9]\{3\}\|xxx\|nnn\|000\) - /# 100 - /; t; s/^# /# 100 - /}' proposals/100-sidecar-injection-webhook.md && rm proposals/100-sidecar-injection-webhook.md.bak
git add proposals/100-sidecar-injection-webhook.md
git commit -m "Rename proposal to use PR number"
git push

Sorry for the confusion!

[tombentley]

@robobario @SamBarker @k-wall I've tried to address your previous comments, and also bring this into live with the implementation PR. Please take another look.

[robobario]

should we mention that initially we will only support PEMs with PKCS8 RSA/EC private keys, and whatever the cert format is we can parse.

[SamBarker]

secretMounts[].secretName is a cross-resource reference — without validating
that the referenced Secret exists in the pod's namespace, a
KroxyliciousSidecarConfig with a bad reference passes admission silently and
only fails when Kubernetes tries to mount the volume at scheduling time. This
validation needs to be part of the alpha implementation, not deferred. It is also
exactly the case where Ready=False would be most valuable — see separate
comment on § Status.

The operator has validation logic for LocalObjectReference fields that handles
this; the webhook should follow the same pattern.

[tombentley]

secretMounts[].secretName is a cross-resource reference — without validating that the referenced Secret exists in the pod's namespace, a KroxyliciousSidecarConfig with a bad reference passes admission silently and only fails when Kubernetes tries to mount the volume at scheduling time.

True.

This validation needs to be part of the alpha implementation, not deferred.

I don't think that's necessary -- if we were to add this later it would not impact the API or be a breaking change. So I think 'need' is too strong.

It is also exactly the case where Ready=False would be most valuable — see separate comment on § Status.

The operator has validation logic for LocalObjectReference fields that handles this; the webhook should follow the same pattern.

While I agree the concern is valid, but let's consider the implications of doing this:

1. The webhook currently needs zero Secret access. Adding validation means get on Secrets in every namespace where a sidecar config might exist. A webhook service account that can read arbitrary Secrets is a high-value target, and turns a compromise of the webhook into a credential exfiltration vector. This directly undermines the "minimal RBAC" design the proposal currently has. So Expecting a kube admin to grant this unknown new webhook access to all the secrets in all the namespaces seems to be asking a heck of a lot.
2. While it provides a way to notify the person configuring the sidecar config at the time the sidecar config is created, there's nothing stopping the secret from being deleted afterwards. It's at the point when the pod's created that the secret needs to exist. But under the two actor design there's no guarantee that the side car admin is paying attention at the point the pod is created by the app owner.
3. The operator analogy is misleading. The operator has a controller that continuously reconciles, so it can re-check references and update status. The webhook fires once at pod admission. If it sets Ready=False because a Secret is missing, when does it set Ready=True? Only options: watch all Secrets (enormous scope, back to the RBAC problem) or only update on the next pod admission (leaving the status stale and misleading for potentially hours or days).
4. A pod referencing a nonexistent Secret via a volume mount won't schedule. kubectl describe pod shows exactly what's missing. This is a standard failure mode every Kubernetes operator knows how to debug. Adding a second layer of validation doesn't prevent the problem, it just moves the error message to a different place, while adding complexity and RBAC surface.
5. If you validate secretMounts, logical consistency demands validating trustAnchorSecretRef too, and any future secret references. This sets a precedent where the webhook must validate all cross-resource references, compounding the RBAC and complexity costs.
6. In progressive delivery workflows (Argo CD, Flux), the KroxyliciousSidecarConfig and the Secret may be in different apps or kustomizations and deploy in arbitrary order. Rejecting the config because the Secret doesn't exist yet breaks these workflows.

[SamBarker]

Thanks for the detailed response. The RBAC concern and the progressive delivery point are well taken — I accept that validating at config-creation time is genuinely problematic.

But I want to push back on the framing that kubectl describe pod is an adequate UX story here. This isn't injection being skipped — the sidecar gets injected, the pod fails to start, and the only signal is a volume mount error buried in pod events. The KroxyliciousSidecarConfig shows Ready=True. There's no trail back to the config.

This is also the case you named as the prime example of where Ready=False would be valuable ("e.g. referencing a non-existent Secret"). If that case can't be covered, the Ready=False story is narrower than it appeared.

I'm not insisting on fixing this in alpha — the reconciliation problem is real. But I think the proposal should explicitly acknowledge this as a known UX gap rather than leaving it implicit, and note that a future reconciliation controller is the right fix.

[SamBarker]

Sketching a future enhancement that addresses the UX gap without changing the webhook's minimal-RBAC design:

Operator sidecar reconciler (future)

The operator already has the RBAC, reconciliation loop, and CRD machinery that the webhook deliberately avoids. A future operator enhancement could add a reconciler that watches KroxyliciousSidecarConfig resources cluster-wide and provides what the webhook can't:

1. Referential integrity: watch referenced Secrets and TLS trust anchor Secrets. Set Ready=False (reason: MissingSecret, with the name) when a reference is broken; flip back to Ready=True when resolved. This handles both the initial-creation case and the "Secret deleted after the fact" case that a one-shot webhook check can't catch.

2. Drift surfacing: watch pods with sidecar.kroxylicious.io/config-generation annotations. Compare against the current KroxyliciousSidecarConfig generation and surface an UpToDate condition on the CRD status — "3 pods running stale config" — without requiring manual jq queries.

3. Optional auto-roll (opt-in): if KroxyliciousSidecarConfig carries a sidecar.kroxylicious.io/auto-roll: enabled annotation, the reconciler triggers a rolling restart of stale pods by patching the owning Deployment/StatefulSet pod template annotation (the standard kubectl rollout restart mechanism). Opt-in so teams that manage their own rollouts aren't surprised.

The webhook stays unchanged — inject, set Ready=True, done. The operator adds observability and optionally remediation on top. Neither component needs to compromise its design to support the other.

This is the right vehicle for the "future reconciler" the proposal already gestures at in § Configuration drift detection.

[tombentley]

the sidecar gets injected, the pod fails to start, and the only signal is a volume mount error buried in pod events

But that is something that anyone familiar with kube will know how to deal with.

There's no trail back to the config.

That's not really true. If you describe the pod you'll see the secret volume, the volume mount, that the volume mount is in an injected sidecar. I think that's enough for a competant person or AI to figure out that they need to involve the kube administrator (if they don't directly know about the kroxylicious sidecar already). It's not exemplary UX, but it's enough.

If that case can't be covered, the Ready=False story is narrower than it appeared.

I get your disappointment. I'd also like to have a better UX here. But that just doesn't seem to fit within the role of a webhook, which is what this proposal is about.

But I think the proposal should explicitly acknowledge this as a known UX gap rather than leaving it implicit,

I'm happy to do that.

note that a future reconciliation controller is the right fix.

I don't think either of us really know the extent to which this is a real problem for people in practice. So I'd prefer to avoid making something that looks like a commitment to future work in this proposal.

Operator sidecar reconciler (future)

We've been following the path that the admission webhook is strictly orthogonal to the current operator. That's because :

• we think there will be people who are interested in one but not the other, because
• they're fundamentally different deployment styles for using a proxy, with different pros and cons.

So I think deciding to extend the scope of the existing operator to include the sidecar CR is premature.

Sure, it's one option. Another option is to have a dedicated operator for sidecar configs. Another option is to do nothing. How we weigh these options depends on feedback we get from users.

[SamBarker]

Really like the forward-looking list structure here. Active/passive DR is a
compelling use case — the sidecar exposes both live and DR clusters, and failover
is a config change plus pod restart. Could be a great showcase for the project.

[SamBarker]

Two permissions are missing from this list:

• get (or list/watch via informer) on Secrets — needed to validate secretMounts[].secretName references at config-observation time.
• update on KroxyliciousSidecarConfig/status — needed to write the Ready condition described in § Status.