fix: roll connector deployment when webhook secret is rotated - #74
Conversation
The webhook-receiver pod reads the HMAC secret from the WEBHOOK_SECRET env var, which is only resolved at pod startup. Rotating the connector secret updated the K8s Secret but never rolled the deployment, so the new token kept failing signature validation while the old one stayed valid — making "Generate new" tokens appear unpersisted/invalid. - operator: stamp the pod template with a sha256 hash of the referenced webhook secret and watch secrets (mapped via the ainsel.dev/connector label, with the connector-<id>-webhook-hmac naming convention as fallback) so a rotation triggers a reconcile and rolling restart - hub: label created webhook secrets with ainsel.dev/connector and apply the previously missing requireWrite authz check on rotate-secret
|
@review-agent please have a look |
![ainsel-reviewer[bot]](https://avatars.githubusercontent.com/in/4488506?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review: fix: roll connector deployment when webhook secret is rotated
Verdict: APPROVE (submitted post-merge; review run was blocked earlier by missing write credentials).
Clean, well-scoped change: the operator now hashes the webhook HMAC secret into a pod-template annotation (ainsel.dev/webhook-secret-hash) so secret rotation deterministically rolls the connector Deployment, with the connector back-linked via the ainsel.dev/connector label and covered by new controller + handler tests.
Two minor, non-blocking suggestions inline.
| Labels: labels, | ||
| // Changing the annotation forces a rolling restart so | ||
| // the pod picks up the (possibly rotated) secret. | ||
| Annotations: map[string]string{ |
There was a problem hiding this comment.
Suggestion: this replaces the pod template's entire Annotations map on every reconcile. Fine today (no other annotations), but if a second annotation is ever added to the pod template it would be wiped here. Merging the key into the existing map would make this future-proof. Not a blocker.
| // webhookConnectorLabel links a webhook HMAC secret back to its | ||
| // WebhookConnector. The event-gateway operator uses it to map secret events | ||
| // (e.g. rotations) to the owning connector so the deployment can roll. | ||
| const webhookConnectorLabel = "ainsel.dev/connector" |
There was a problem hiding this comment.
Suggestion: this constant is duplicated in the event-gateway operator (webhookConnectorLabel). If both services already depend on shared/, consider defining forge-agreed label/annotation keys there once. Not a blocker.
1 participant
Problem
The "Generate new" token button on connector detail pages appeared broken: the newly generated webhook token failed signature validation from then on, while the old token kept working.
Root cause: the connector's webhook-receiver pod reads the HMAC secret from the
WEBHOOK_SECRETenvironment variable, which Kubernetes only resolves at pod startup.POST /connectors/{id}/rotate-secretupdated the K8s Secret object but never rolled the connector Deployment — so the running pod kept validating against the old secret indefinitely.Fix
event-gateway operator (
webhookconnector_controller.go)ainsel.dev/webhook-secret-hashannotation (SHA-256 of the referenced secret's value — the hash, not the credential). Any change to the hash alters the pod template, forcing a rolling restart so new pods resolve the rotated secret.ainsel.dev/connectorlabel first, falling back to theconnector-<id>-webhook-hmacnaming convention for pre-existing unlabeled secrets), so a rotation triggers a reconcile automatically.hub (
handlers_connectors.go,secrets.go)ainsel.dev/connector: <id>for robust event mapping.requireWriteauthorization check on therotate-secretendpoint (PUT/DELETE already had it).Tests
Operational note
No chart/RBAC changes needed: the operator already had
get;list;watchon secrets. Existing connectors with unlabeled secrets are covered by the naming-convention fallback.