stacklok / toolhive / 25107214395

push

github

Detect imagePullSecrets drift on proxy Deployments (#5113)

The MCPServer and MCPRemoteProxy controllers correctly include
spec.resourceOverrides.proxyDeployment.imagePullSecrets in newly built
proxy Deployments, but their deploymentNeedsUpdate drift checks never
compare Spec.Template.Spec.ImagePullSecrets. As a result, editing the
field on a live CR did not roll the existing Deployment: the proxy-
runner ServiceAccount was refreshed via EnsureRBACResources (so kubelet
recovered for new pods through the SA path), but the pod-spec field
on the live Deployment remained stale. In the user-managed-
ServiceAccount branch, where RBAC reconciliation is skipped, the pod-
spec field is the only mechanism — so that path stayed broken.

Add a comparison in both controllers using equality.Semantic.DeepEqual
so nil and empty slices are treated identically (matching Kubernetes'
own serialization semantics) and reorder/add/remove on populated
slices triggers a rollout. MCPRemoteProxy gets a new podSpecNeedsUpdate
sub-helper alongside the existing containerNeedsUpdate /
deploymentMetadataNeedsUpdate / podTemplateMetadataNeedsUpdate trio;
MCPServer gets an inline check that mirrors the surrounding style of
its single deploymentNeedsUpdate function.

Each controller gets a table-driven unit test covering seven cases:
the no-override baseline, the bug repro, the removal flow, the
populated match, the nil-vs-empty asymmetry from both sides, and the
slice-reorder case (which equality.Semantic.DeepEqual treats as
drift). Two envtest integration tests per controller exercise the
full reconcile-update path: create CR, assert Deployment, mutate
imagePullSecrets, assert Deployment rolls.

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>

16 of 16 new or added lines in 2 files covered. (100.0%)

60603 of 94568 relevant lines covered (64.08%)

60.02 hits per line