Skip to content

feat(ci): post-deploy verification for managed and enterprise orgs#1672

Open
joelorzet wants to merge 7 commits into
stagingfrom
feat/keep-856-post-deploy-verification
Open

feat(ci): post-deploy verification for managed and enterprise orgs#1672
joelorzet wants to merge 7 commits into
stagingfrom
feat/keep-856-post-deploy-verification

Conversation

@joelorzet

@joelorzet joelorzet commented Jun 26, 2026

Copy link
Copy Markdown

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

  • Triggered via workflow_run on 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.
  • Runs inside the cluster (the internal routes are HMAC authed and WAF blocked on the public hostname): it signs the request like deploy/scripts/digest-cron.sh and curls the in-pod service URL, polling once a minute for at most 5 minutes and stopping early on the first flagged org.
  • The endpoint GET /api/internal/post-deploy-verification resolves 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, any system_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)

  • The hardcoded slug list is gone. 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).
  • Remaining client names were scrubbed from the backfill script, unit-test fixtures, and code comments.

Alerting (PagerDuty, per client)

  • Replaces the Discord notification. On a flagged org the endpoint pages that org's own service via a per-client routing key (falling back to DEFAULT_PAGERDUTY_ROUTING_KEY for enterprise orgs without a dedicated service), deduped to one incident per org per deploy. Slugs and routing keys never enter the CI job.
  • Severity: no executions and system_error are P2; a user-error-rate spike is P3. "No executions" is concluded on the loop's final poll.
  • The CI job now only fails red on problems (aggregate counts); PagerDuty does the paging, and per-org detail stays in internal logs.

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

  • Runtime env only (not NEXT_PUBLIC), so no Docker build arg is needed. Added to the keeperhub-stack values (prod + staging): MANAGED_ORGS_CONFIG + DEFAULT_PAGERDUTY_ROUTING_KEY on the app, and MANAGED_ORGS_CONFIG on the metrics-collector. The legacy standalone values under deploy/keeperhub/ and deploy/metrics-collector/ are not used by the stack deploy and were left untouched.
  • Before merge/deploy: create the managed-orgs and default-pagerduty-routing-key Parameter Store entries per env, and confirm the app/metrics IRSA SSM policy covers the path prefix. The pipeline needs no new AWS permissions.
  • Follow-up (infra repo): point the Grafana managed_org_slugs_regex at the same Parameter Store value so the cohort has one source across app and alerting.

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
@joelorzet joelorzet requested review from a team, OleksandrUA, eskp and suisuss and removed request for a team June 26, 2026 15:49
@joelorzet joelorzet added the metrics-db-reviewed Reviewer sign-off: metrics aggregate queries optimised + tables indexed (KEEP-680) label Jun 26, 2026
…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 OleksandrUA left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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-orgs and default-pagerduty-routing-key created 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 nextRunAt in-window and the final-poll total === 0 P2 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, nextRunAt moves out of the window, the org leaves the cohort, and its stats are never fetched. A system_error on 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 [] when MANAGED_ORGS_CONFIG is 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-true result, 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".
  • parseNumberParam accepts 0 and negatives, so minExecutions=0 would 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

metrics-db-reviewed Reviewer sign-off: metrics aggregate queries optimised + tables indexed (KEEP-680)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants