feat(ci): post-deploy verification for managed and enterprise orgs#1672
feat(ci): post-deploy verification for managed and enterprise orgs#1672joelorzet wants to merge 7 commits into
Conversation
A separate pipeline that runs after the entire prod CI Pipeline finishes green (workflow_run) and confirms managed and enterprise organizations are executing workflows correctly on the just-shipped build. - new HMAC-authed internal endpoint reads recent execution outcomes for the managed slugs plus enterprise-plan orgs and flags error-rate regressions - runs in-cluster (internal routes are WAF-blocked on the public host), signing like digest-cron.sh and curling the in-pod service URL - per-org detail goes to internal observability only; public CI logs and the chat alert carry aggregate counts so customer data is not exposed - extract the managed-client slug list into a shared dependency-free module
…ardcoded slugs Replace the hardcoded managed-client slug list with getManagedOrgSlugs(), parsed from MANAGED_ORGS_CONFIG (a JSON dict injected at deploy from Parameter Store). This keeps client identities out of the public source; the metrics scraper and the post-deploy verification route now read the same env-sourced list. Empty/unset config yields no managed cohort (bounded gauge, safe default). Also scrub the remaining client names from the backfill script, unit-test fixtures, and code comments.
Replace the Discord notification with server-side PagerDuty paging: each flagged managed/enterprise org pages its own service via a per-client routing key (env sourced), deduped to one incident per org per deploy. Slugs and routing keys never enter the CI job. Add "no executions in the window" detection (P2) concluded on the loop's final poll, alongside system_error (P2) and user-error-rate (P3). The workflow passes final/deployId and now only fails the job red on problems; PagerDuty does the paging.
…meter Store Add MANAGED_ORGS_CONFIG + DEFAULT_PAGERDUTY_ROUTING_KEY to the app env and MANAGED_ORGS_CONFIG to the metrics-collector env in the keeperhub-stack values (prod + staging). Runtime env only -- no Docker build arg needed since these are not NEXT_PUBLIC and are read at request time.
OleksandrUA
left a comment
There was a problem hiding this comment.
Nice work here, and the description made it quite easy to follow. Public-repo hygiene is careful, the HMAC signing matches our internal-service contract exactly (pathname-only signature, empty-body digest, 300s window, scheduler is a whitelisted caller), and moving client identities into Parameter Store is I'd say the right call.
I'd request changes on two deploy dependencies though. As written they can fail a prod deploy or silently no-op the feature. Neither is a problem in the route code itself, both are about ordering and cross-repo prerequisites. There is also one correctness gap in cohort selection I'd like to resolve or at least document before this ships.
Blocking: Parameter Store entries must exist before the chart change deploys
Both values.yaml files add MANAGED_ORGS_CONFIG and DEFAULT_PAGERDUTY_ROUTING_KEY as parameterStore env. In the common chart that renders an ExternalSecret, and the pod reads it via valueFrom.secretKeyRef with no optional: true. Our keeperhub deploy runs helm with atomic: true and a 5m timeout.
So if either SSM parameter does not exist at deploy time, ESO can not populate the Secret, the app container can not start, health gate times out, and atomic: true rolls back the whole keeperhub release on that env. That is a failed prod deploy, not a soft degrade. We already had one incident from an empty env value in a mutating op, so I'm quite careful on this one.
Before merge I'd suggest to have:
managed-orgsanddefault-pagerduty-routing-keycreated in SSM for both staging and prod, confirmed present.- app and metrics-collector IRSA policy confirmed to cover the
/eks/<env>/keeperhub/path prefix for these names.
I'd prefer we not rely on the checklist alone here given the blast radius. If there is a clean way to preflight the params, or make the secret non-fatal on first rollout, that would remove the footgun.
Blocking: the kubectl step depends on the OIDC migration (KEEP-834) applied first
The verify job authenticates with OIDC (role-to-assume: ${{ vars.AWS_ROLE_ARN }}, id-token: write) and then runs kubectl run in the prod keeperhub namespace.
Every existing cluster-mutating deploy authenticates with the static IAM user (secrets.TO_AWS_*), which is the cicd_user access entry on the cluster. Current prod access_entries set has no GitHub OIDC role in it. The OIDC role github-oidc-eks-keeperhub and its prod EKS access entry are introduced by KEEP-834, which is not merged or applied yet.
So until KEEP-834 is applied on prod and AWS_ROLE_ARN is set in the prod GitHub environment, this workflow's kubectl run gets rejected by EKS authz, the loop never gets a signal, RESULT="error", and the job goes red on every prod deploy while verifying nothing.
I'd suggest to make this dependency explicit in the PR description and gate merge on KEEP-834 landing first. One related note :KEEP-834 also carries the Grafana $managed_slugs alert rework, which is the same follow-up this PR lists for pointing managed_org_slugs_regex at Parameter Store. So the two are entangled and I'd say should be sequenced together.
Correctness: cohort selection gated on a moving nextRunAt
resolveTargetOrgs selects orgs by workflowSchedules.nextRunAt BETWEEN since AND until. When a scheduled workflow runs, updateScheduleAfterRun advances nextRunAt to the next occurrence, so for any interval bigger than the 5m window the org drops out of cohort right after it runs.
What this means in practice:
- The silent no-run case works, because a schedule that never fired keeps its
nextRunAtin-window and the final-polltotal === 0P2 fires. That is the main thing this feature is for, so core intent holds. - Error detection on a scheduled run is timing-racy.Once the run fires,
nextRunAtmoves out of the window, the org leaves the cohort, and its stats are never fetched. Asystem_erroron that run can be missed depending which poll lands in the gap - and that is exactly the strongest regression signal. - Enterprise orgs that run only on webhook/event/manual and have no due schedule are never selected at all, even though execution counting claims to cover every trigger type. Counting covers all triggers, but selection is schedule-only.
I'd prefer we select the managed/enterprise set directly and then judge on actual executions, rather than gate membership on a future schedule. If the intent really is "only verify orgs with a schedule due in the window", then let us say that explicitly in the route doc and accept the reduced coverage.
Non-blocking
- The managed-client error gauge (
getWorkflowErrorsByWorkflowFromDb) now returns[]whenMANAGED_ORGS_CONFIGis unset. It was a hardcoded const before, always present. So if the metrics-collector env is missing or empty, the existing managed-client Grafana alert silently loses its series. I'd suggest to wire and verify the collector param as part of this change, not the follow-up. - The final step fails the job on any non-
trueresult, including "no signal at all". Since paging is server-side, a transient cluster or image-pull blip pages nobody but still shows a red deploy verification. Pre-KEEP-834 that red is basically the default outcome. Maybe worth treating "could not reach the cluster" differently from "confirmed a problem". parseNumberParamaccepts0and negatives, sominExecutions=0would make the error-rate branch evaluate on every org. CI sets sane values so it is only a hardening nit.
Overall I like the direction a lot. The two blocking items are really about sequencing with the infra side, so once params are in place and KEEP-834 is applied, most of this clears.
What
A separate pipeline that runs after the whole prod CI Pipeline finishes green and verifies that managed and enterprise organizations are executing workflows correctly on the just-shipped build. If an org regresses, it pages that client's own PagerDuty service. Client identities are sourced from AWS Parameter Store, not hardcoded in this (public) repo.
How it works
workflow_runon the "CI Pipeline" workflow completing with conclusion success on prod. It fires only once the whole run (build, e2e, deploy, release, docs-sync) is green, not after any single step.deploy/scripts/digest-cron.shand curls the in-pod service URL, polling once a minute for at most 5 minutes and stopping early on the first flagged org.GET /api/internal/post-deploy-verificationresolves the cohort (managed slugs plus any enterprise-plan org) gated to orgs with a schedule due in the window, then flags an org that has no executions, anysystem_error, or an error rate over the threshold.Monitoring window
Polls every minute for 5 minutes, anchored at the check start so only executions on the new build count. This was confirmed sufficient for the current managed clients given their execution cadence. The window and thresholds are query-param overridable from the workflow, so tuning does not need a code change.
Managed-client identities (Parameter Store, single source of truth)
MANAGED_ORGS_CONFIG(a JSON dict{"<slug>":{"pagerdutyRoutingKey":"..."}}) is injected at deploy from Parameter Store and read by both the metrics scraper (managed-client error gauge) and the verification route. Empty/unset yields no managed cohort (bounded gauge, safe default).Alerting (PagerDuty, per client)
DEFAULT_PAGERDUTY_ROUTING_KEYfor enterprise orgs without a dedicated service), deduped to one incident per org per deploy. Slugs and routing keys never enter the CI job.system_errorare P2; a user-error-rate spike is P3. "No executions" is concluded on the loop's final poll.Data exposure
This repository is public, so Actions logs are world readable. The pipeline surfaces only aggregate counts. Per-org slugs, names, and sample error text are written to internal observability and PagerDuty only, never to CI logs.
Deploy notes
NEXT_PUBLIC), so no Docker build arg is needed. Added to the keeperhub-stack values (prod + staging):MANAGED_ORGS_CONFIG+DEFAULT_PAGERDUTY_ROUTING_KEYon the app, andMANAGED_ORGS_CONFIGon the metrics-collector. The legacy standalone values underdeploy/keeperhub/anddeploy/metrics-collector/are not used by the stack deploy and were left untouched.managed-orgsanddefault-pagerduty-routing-keyParameter Store entries per env, and confirm the app/metrics IRSA SSM policy covers the path prefix. The pipeline needs no new AWS permissions.managed_org_slugs_regexat the same Parameter Store value so the cohort has one source across app and alerting.