klinux / klinux/gocdnext

hardening: rerun/rollback definition-drift safety (snapshot vs current, fingerprint fence)

Open
#209 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Go
Stars
7
Forks
3
Avg merge
2h 12m
Merged PRs (30d)
30

Description

Problem

A manual rerun or rollback can execute a mix of the run's original snapshot definition and the
current definition, and can slip an environment freeze.

  • GetRunForDispatch selects p.definition (the current pipeline definition), not
    runs.definition (the run's snapshot) — so after ApplyProject, an old run can execute
    image/tasks/variables from the current def, not the snapshot it was created with (a pre-existing drift
    bug).
  • On rerun, BuildAssignment reads only the snapshot, so a job's declared environment: (needed for
    freeze) may not reach the scheduler for a revived job; a freeze created before dispatch can slip.
  • RerunJob reopens the clicked job + downstream system-canceled jobs + later gates; if the current def
    changed (env retargeted, job removed/renamed, cluster/script changed), reviving that set is unsafe.

Proposed direction

  • Execution uses the SNAPSHOT (runs.definition), not the current def. The hot path returns the
    snapshot + a definition_version/fingerprint; the current def is fetched only at the manual-rerun
    boundary.
  • Persist the admission decision on the attempt: job_runs.admission_environment +
    job_runs.admission_fingerprint — where the fingerprint is a canonical hash over the revival-set
    jobs + their GovernedFreezeEnvs governance
    , NOT the raw definition_version (any unrelated edit
    bumps the raw version and would cause false drift). Effective-env precedence at dispatch: a revived
    job (admission_fingerprint IS NOT NULL) uses admission_environment (even NULL); a never-revived
    job uses snapshot.TargetEnvironment().
  • Close the rerun→dispatch TOCTOU atomically: the fence lives inside the admission tx
    (Pipeline … FOR SHARE + fingerprint compare → Freeze lock → freeze check → job CAS), lock order
    Lane → Pipeline → Freeze → …. Unchanged ⇒ admit; changed ⇒ fail closed (config-error terminalise).
  • Validate the whole revival set at the top of RerunJob, atomically: the clicked job may differ
    from the current def only in Environment (inherit as overlay); every revived downstream/gate must be
    exactly equivalent, else return 409 ErrHistoricalRerunUnsafe. Freeze the set by IDs+states (SELECT … FOR UPDATE, revive with WHERE id = ANY(validated_ids) AND status = expected + rows-affected
    check). Propagate ErrHistoricalRerunUnsafe → 409/422 through the rollback endpoint
    (environments.go maps only ErrJobRunActive today → others become a misleading 500).
  • Native takeover also needs the fence: tryNativeDeploy acquires Lane + TakeOver before the
    plugin helper, so a reran native deploy would skip the fence — add the version/fingerprint check
    inside StartNativeDeploy/StartNativeDeployDeclared's tx; a mismatch terminalises the job
    (config-error) rather than logging + retrying forever.
  • A drifted rearmed gate is reconciled to terminal, not left immortal: if approve 409s AND expiry
    skips forever, the gate hangs awaiting_approval and the run + services: leak — reconcile it
    (canceled with reason superseded_by_definition_change, firing the normal Check-close + cleanup),
    surfaced as an actionable state on the gate.
  • extends merge bug (related): mergeJobDef starts out := base, so a child's own
    Deploy/Cluster/(new) Environment override is dropped — a deploy child extending a base loses its
    deploy:; cluster: null/"" silently inherits the template's prod cluster. Needs a presence-aware
    JobDef.UnmarshalYAML recording presence/tag for Deploy/Cluster/Environment (preserving
    KnownFields).

Baseline: origin/main. This is the "definition-drift safety" epic split out of the freeze/cancel work
so it can be attacked in isolation. Verify line numbers against current origin/main.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Use current origin/main as the baseline and trace GetRunForDispatch, BuildAssignment, and RerunJob through the admission transaction. Then inspect environments.go, tryNativeDeploy, StartNativeDeploy/StartNativeDeployDeclared, and mergeJobDef with JobDef.UnmarshalYAML. Done means snapshot execution, fingerprint fencing, safe revival and gate reconciliation, error propagation, and presence-aware definition merging are covered without unsafe drift.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
backend, ci-cd, database
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.