diff --git a/.github/workflows/notify-main-failure.yml b/.github/workflows/notify-main-failure.yml index 08467ae..d583af8 100644 --- a/.github/workflows/notify-main-failure.yml +++ b/.github/workflows/notify-main-failure.yml @@ -88,10 +88,10 @@ on: description: "Silence this caller entirely while the staging freeze window is closed. For staging pipelines, where neither a failure nor its recovery is release-relevant mid-week." type: boolean default: false - ruleset-name: - description: "The freeze ruleset to read. Must match what the repo's release-freeze caller sets, or this reads a name that does not exist and escalates every staging failure forever." + freeze-property: + description: "The freeze custom property to read. Must match what the repo's release-freeze caller sets, or this reads a name that does not exist and escalates every staging failure forever." type: string - default: staging-freeze + default: staging_frozen force-post: description: "Post a 'recovered' message with no prior failure to recover from. Only the smoke-test dispatch sets this; a real pipeline leaves it off, or every green run reports a recovery." type: boolean @@ -173,7 +173,7 @@ jobs: env-name: ${{ inputs.env-name }} status: ${{ inputs.status }} freeze-scoped: ${{ inputs.freeze-scoped }} - ruleset-name: ${{ inputs.ruleset-name }} + freeze-property: ${{ inputs.freeze-property }} freeze-token: ${{ steps.freeze-token.outputs.token }} github-token: ${{ github.token }} force-post: ${{ inputs.force-post }} diff --git a/.github/workflows/notify-startup-failure.yml b/.github/workflows/notify-startup-failure.yml index c5bffce..ac99c96 100644 --- a/.github/workflows/notify-startup-failure.yml +++ b/.github/workflows/notify-startup-failure.yml @@ -154,10 +154,10 @@ on: description: "Which of `branches` is the release candidate, and so is only reported while the freeze window is open" type: string default: staging - ruleset-name: - description: "The freeze ruleset to read. Must match what the repo's release-freeze caller sets." + freeze-property: + description: "The freeze custom property to read. Must match what the repo's release-freeze caller sets." type: string - default: staging-freeze + default: staging_frozen runs-on: description: "Runner label for the sweep jobs" type: string @@ -188,10 +188,10 @@ on: description: "Which branch is the release candidate" type: string default: staging - ruleset-name: - description: "The freeze ruleset to read" + freeze-property: + description: "The freeze custom property to read" type: string - default: staging-freeze + default: staging_frozen lookback-minutes: description: "Lookback window in minutes (widen it to replay a past incident)" type: number @@ -390,10 +390,10 @@ jobs: path: .ci-shared persist-credentials: false - # Reading rulesets needs admin, which the default workflow token does not - # carry, so this reuses the App that owns the freeze toggle. A repo that is - # not on the release train has no App and no ruleset; the read escalates to - # frozen, which for this sweep means staging is swept like any other branch. + # This reuses the App that owns the freeze toggle, so the reader and the + # writer share one credential. A repo that is not on the release train has + # no App installed; the read escalates to frozen, which for this sweep means + # staging is swept like any other branch. - name: Mint release-train App token id: freeze-token continue-on-error: true @@ -407,7 +407,7 @@ jobs: env: GH_TOKEN: ${{ steps.freeze-token.outputs.token }} REPO: ${{ github.repository }} - RULESET_NAME: ${{ inputs.ruleset-name }} + FREEZE_PROPERTY: ${{ inputs.freeze-property }} run: | set -uo pipefail if [ -z "${GH_TOKEN:-}" ]; then @@ -416,7 +416,7 @@ jobs: exit 0 fi python3 .ci-shared/scripts/freeze_state.py read \ - --repo "${REPO}" --ruleset-name "${RULESET_NAME}" --on-error escalate + --repo "${REPO}" --property-name "${FREEZE_PROPERTY}" --on-error escalate - name: Find deploy branches that are still red id: find diff --git a/.github/workflows/release-freeze.yml b/.github/workflows/release-freeze.yml index ae5e353..fe66741 100644 --- a/.github/workflows/release-freeze.yml +++ b/.github/workflows/release-freeze.yml @@ -1,12 +1,12 @@ # Reusable workflow: freeze the staging branch to open the weekly QA window. # -# Freezing flips a *pre-provisioned* repository ruleset (default name -# `staging-freeze`) from `disabled` to `active`. That ruleset carries a single -# `update` rule on the staging branch with the `mindsdb-release-train` GitHub -# App as a bypass actor, so ordinary pushes to staging are blocked while the -# App (and the release sync-back push) can still write. Because rulesets layer -# on top of classic branch protection, toggling enforcement can never clobber -# the underlying protection config. +# Freezing sets this repo's `staging_frozen` custom property to `true`. One +# org-level ruleset (terraform: newprod/global/github) targets the staging +# branch of every repo where that property is `true`, with a single `update` +# rule and the `mindsdb-release-train` GitHub App as a bypass actor, so ordinary +# pushes to staging are blocked while the App (and the release sync-back push) +# can still write. The ruleset itself is never edited here; only the property +# moves, so terraform's plan stays empty across the weekly cycle. # # If staging has no unreleased changes (staging == main), the freeze is skipped. # @@ -33,10 +33,10 @@ on: description: "Production branch staging is compared against" type: string default: main - ruleset-name: - description: "Name of the pre-provisioned freeze ruleset to toggle" + freeze-property: + description: "Custom property that marks this repo frozen. Must match the org ruleset's condition in terraform." type: string - default: staging-freeze + default: staging_frozen # No workflow-level `permissions:` block, deliberately, so this job inherits the # calling job's grant. That is what makes the notify step's recovery lookup work: @@ -75,10 +75,10 @@ jobs: fetch-depth: 0 persist-credentials: false - # The ruleset read-modify-write lives in `scripts/freeze_state.py`, which is - # also what the alerting workflows use to READ the freeze state. One file - # owns the contract, so a repo that renames its ruleset cannot end up frozen - # by one name and alerted on another. + # The property write lives in `scripts/freeze_state.py`, which is also what + # the alerting workflows use to READ the freeze state. One file owns the + # contract, so a repo cannot end up frozen under one property name and + # alerted on another. - name: Check out the shared release-freeze contract uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1 with: @@ -112,14 +112,14 @@ jobs: echo "${STAGING} is ${AHEAD} commit(s) ahead of ${BASE} — freezing." fi - - name: Freeze staging (activate ruleset) + - name: Freeze staging (set the frozen property) if: steps.check.outputs.should_freeze == 'true' env: GH_TOKEN: ${{ steps.app-token.outputs.token }} REPO: ${{ github.repository }} - RULESET_NAME: ${{ inputs.ruleset-name }} + FREEZE_PROPERTY: ${{ inputs.freeze-property }} STAGING: ${{ inputs.staging-branch }} - ENFORCEMENT: active + FROZEN: "true" run: | set -euo pipefail # Fails loudly on purpose, unlike the alerting path's read: a freeze @@ -127,14 +127,14 @@ jobs: # let the window appear to open. python3 .ci-shared/scripts/freeze_state.py set \ --repo "${REPO}" \ - --ruleset-name "${RULESET_NAME}" \ - --enforcement "${ENFORCEMENT}" \ - --body-path "${RUNNER_TEMP}/ruleset.json" + --property-name "${FREEZE_PROPERTY}" \ + --frozen "${FROZEN}" \ + --body-path "${RUNNER_TEMP}/freeze-property.json" echo "${STAGING} is FROZEN." { echo "## Staging branch FROZEN" echo "" - echo "The \`${STAGING}\` branch is **frozen** via ruleset \`${RULESET_NAME}\`." + echo "The \`${STAGING}\` branch is **frozen**: \`${FREEZE_PROPERTY}=true\`." } >> "$GITHUB_STEP_SUMMARY" # The alert lives here rather than in each repo's wrapper, which is what diff --git a/.github/workflows/release-pr.yml b/.github/workflows/release-pr.yml index 2f900dd..eb4bd01 100644 --- a/.github/workflows/release-pr.yml +++ b/.github/workflows/release-pr.yml @@ -50,10 +50,10 @@ on: description: "Target branch for the release PR" type: string default: main - ruleset-name: - description: "The freeze ruleset to read, to decide draft vs ready. Must match what the repo's release-freeze caller sets." + freeze-property: + description: "The freeze custom property to read, to decide draft vs ready. Must match what the repo's release-freeze caller sets." type: string - default: staging-freeze + default: staging_frozen # No workflow-level `permissions:` block, deliberately, so this job inherits the # calling job's grant. That is what makes the notify step's recovery lookup work: @@ -103,11 +103,11 @@ jobs: env: GH_TOKEN: ${{ steps.app-token.outputs.token }} REPO: ${{ github.repository }} - RULESET_NAME: ${{ inputs.ruleset-name }} + FREEZE_PROPERTY: ${{ inputs.freeze-property }} run: | set -uo pipefail if ! python3 .ci-shared/scripts/freeze_state.py read \ - --repo "${REPO}" --ruleset-name "${RULESET_NAME}" --on-error fail; then + --repo "${REPO}" --property-name "${FREEZE_PROPERTY}" --on-error fail; then echo "::warning::Could not read the freeze state. Leaving the release PR as a draft." echo "frozen=false" >> "$GITHUB_OUTPUT" fi diff --git a/.github/workflows/release-unfreeze.yml b/.github/workflows/release-unfreeze.yml index 972a5ed..4fdfd4d 100644 --- a/.github/workflows/release-unfreeze.yml +++ b/.github/workflows/release-unfreeze.yml @@ -1,11 +1,12 @@ # Reusable workflow: unfreeze staging when the weekly release PR merges, then # sync main back into staging. # -# Unfreezing flips the pre-provisioned `staging-freeze` ruleset back to -# `disabled`, reopening staging for normal development the moment the release -# ships — no fixed Monday-morning wait. It is also what turns staging's alerting -# back off: the alert path reads the same ruleset, so an unfrozen staging stops -# paging the channel without anything else being told. +# Unfreezing sets this repo's `staging_frozen` custom property back to `false`, +# which takes the branch out of the org-level freeze ruleset and reopens staging +# for normal development the moment the release ships — no fixed Monday-morning +# wait. It is also what turns staging's alerting back off: the alert path reads +# the same property, so an unfrozen staging stops paging the channel without +# anything else being told. # # Order is load-bearing: unlock first, then push. If the sync-back push fails, # staging is left unlocked, which is the acceptable failure mode. @@ -13,7 +14,7 @@ # The sync-back merge (main -> staging) is load-bearing on squash-merge repos: # without it, every previously shipped commit reappears in the next release # PR's commit list. The push is performed with the `mindsdb-release-train` App -# token; the App is the ruleset bypass actor on staging, so the push lands even +# token; the App is the freeze ruleset's bypass actor, so the push lands even # though staging otherwise requires review. # # Called by a per-repo wrapper, e.g. @@ -39,10 +40,10 @@ on: description: "Production branch to sync back from" type: string default: main - ruleset-name: - description: "Name of the pre-provisioned freeze ruleset to toggle" + freeze-property: + description: "Custom property that marks this repo frozen. Must match the org ruleset's condition in terraform." type: string - default: staging-freeze + default: staging_frozen # No workflow-level `permissions:` block, deliberately, so this job inherits the # calling job's grant. That is what makes the notify step's recovery lookup work: @@ -70,10 +71,10 @@ jobs: client-id: ${{ vars.RELEASE_APP_CLIENT_ID }} private-key: ${{ secrets.RELEASE_APP_PRIVATE_KEY }} - # The ruleset read-modify-write lives in `scripts/freeze_state.py`, which is - # also what the alerting workflows use to READ the freeze state. One file - # owns the contract, so unfreezing and "staging alerts are off again" can - # never disagree about which ruleset they mean. + # The property write lives in `scripts/freeze_state.py`, which is also what + # the alerting workflows use to READ the freeze state. One file owns the + # contract, so unfreezing and "staging alerts are off again" can never + # disagree about which property they mean. # # This has to precede the unlock, because the unlock runs that script. It is # therefore the one step whose failure leaves staging FROZEN, which is the @@ -87,26 +88,26 @@ jobs: path: .ci-shared persist-credentials: false - - name: Unfreeze staging (disable ruleset) + - name: Unfreeze staging (clear the frozen property) env: GH_TOKEN: ${{ steps.app-token.outputs.token }} REPO: ${{ github.repository }} - RULESET_NAME: ${{ inputs.ruleset-name }} + FREEZE_PROPERTY: ${{ inputs.freeze-property }} STAGING: ${{ inputs.staging-branch }} - ENFORCEMENT: disabled + FROZEN: "false" run: | set -euo pipefail python3 .ci-shared/scripts/freeze_state.py set \ --repo "${REPO}" \ - --ruleset-name "${RULESET_NAME}" \ - --enforcement "${ENFORCEMENT}" \ - --body-path "${RUNNER_TEMP}/ruleset.json" + --property-name "${FREEZE_PROPERTY}" \ + --frozen "${FROZEN}" \ + --body-path "${RUNNER_TEMP}/freeze-property.json" PR_NUMBER='${{ github.event.pull_request.number }}' echo "${STAGING} is UNFROZEN." { echo "## Staging branch UNFROZEN" echo "" - echo "The \`${STAGING}\` branch is **unfrozen** via ruleset \`${RULESET_NAME}\` (triggered by PR #${PR_NUMBER:-manual})." + echo "The \`${STAGING}\` branch is **unfrozen**: \`${FREEZE_PROPERTY}=false\` (triggered by PR #${PR_NUMBER:-manual})." } >> "$GITHUB_STEP_SUMMARY" # Order is load-bearing and this is the whole reason for it: unlock first, diff --git a/README.md b/README.md index 499626e..66b8a24 100644 --- a/README.md +++ b/README.md @@ -21,9 +21,9 @@ They live in `.github/workflows/` and are called from ~25-line per-repo wrappers | Reusable workflow | Name (keep identical in callers) | What it does | |---|---|---| -| `release-freeze.yml` | `Staging Freeze` | Activates the `staging-freeze` ruleset to lock staging (skips if staging == main) | +| `release-freeze.yml` | `Staging Freeze` | Sets the repo's `staging_frozen` property, which puts staging under the org freeze ruleset (skips if staging == main) | | `release-pr.yml` | `Create staging to main release PR` | Keeps the `staging → main` PR open and current: a draft that lists what is queued, marked ready for review when the freeze opens | -| `release-unfreeze.yml` | `Staging Unfreeze` | Disables the ruleset when the release PR merges, then syncs `main` back into `staging` | +| `release-unfreeze.yml` | `Staging Unfreeze` | Clears the property when the release PR merges, then syncs `main` back into `staging` | | `sync-main-to-staging.yml` | `Sync main to staging` | Merges `main` into `staging` after **any** push to main, not just the release merge | `release-unfreeze.yml` syncs main back only on the release path, because its wrapper's guard requires the merged PR's head branch to be `staging`. A commit that reaches main any other way (a hotfix PR, a revert, a direct merge) fires nothing, and on a squash-merge repo that leaves the next release PR diffed against a `main` that `staging` does not contain. `sync-main-to-staging.yml` closes that window on `push: main`. @@ -150,7 +150,7 @@ A workflow triggered on **both** main and staging from one notify job must not h freeze-scoped: ${{ github.ref_name == 'staging' }} ``` -Freeze state is read from the `staging-freeze` ruleset itself, not inferred from workflow history — a freeze that skipped itself because staging had nothing unreleased still concludes `success`, and history cannot tell that apart from a real freeze. Reading rulesets needs admin, so this reuses the same App that toggles them (`vars.RELEASE_APP_CLIENT_ID` + `secrets.RELEASE_APP_PRIVATE_KEY`); no extra `permissions:` on the caller. If the App token or the ruleset lookup fails, it escalates to the red alert rather than downgrading, so a lookup problem can never silence a real release-blocking failure. +Freeze state is read from the `staging_frozen` custom property itself, not inferred from workflow history — a freeze that skipped itself because staging had nothing unreleased still concludes `success`, and history cannot tell that apart from a real freeze. This reuses the same App that sets the property (`vars.RELEASE_APP_CLIENT_ID` + `secrets.RELEASE_APP_PRIVATE_KEY`); no extra `permissions:` on the caller. If the App token or the property lookup fails, it escalates to the red alert rather than downgrading, so a lookup problem can never silence a real release-blocking failure. Leave `freeze-scoped` off for prod and freeze/unfreeze callers: a failure there is always worth interrupting for. @@ -256,9 +256,9 @@ Blind spot to know about: it can only read local (`./.github/workflows/...`) cal ## The release-freeze contract -A freeze is one thing: the `enforcement` field of a pre-provisioned repository ruleset, flipped between `active` and `disabled`. Four workflows care — freeze and unfreeze write it, and the two alerting paths read it to decide whether a red staging is worth interrupting anyone for. +A freeze is one thing: the repo's `staging_frozen` custom property, set to `true` or `false`. A single org-level ruleset (terraform, `newprod/global/github`) blocks pushes to `staging` in every repo where it is `true`; the workflows never touch the ruleset. Four workflows care — freeze and unfreeze write it, and the two alerting paths read it to decide whether a red staging is worth interrupting anyone for. -They used to carry three separate copies of that knowledge, and only two of them took the ruleset name as an input; the alerting path hardcoded it. Renaming the ruleset in one repo would therefore have moved the freeze and left the alerting reading a name that no longer existed — silent in the direction that hurts, because a reader that cannot establish the state escalates, so every ordinary mid-week staging red would have paged the channel forever and the cause would have looked like a Slack problem. +They used to carry three separate copies of that knowledge, and only two of them took the name as an input; the alerting path hardcoded it. Renaming it in one repo would therefore have moved the freeze and left the alerting reading a name that no longer existed — silent in the direction that hurts, because a reader that cannot establish the state escalates, so every ordinary mid-week staging red would have paged the channel forever and the cause would have looked like a Slack problem. `scripts/freeze_state.py` now owns it, with two modes that fail in opposite directions on purpose: @@ -266,9 +266,9 @@ They used to carry three separate copies of that knowledge, and only two of them |---|---|---| | `read --on-error escalate` | the alert paths | report frozen, exit 0 — never downgrade a real release-blocking failure, and never redden a green run | | `read --on-error fail` | `release-pr.yml` | leave the PR a draft — the safe direction there is the opposite one, since "ready" invites a merge of an unvalidated branch | -| `set --enforcement …` | freeze and unfreeze | fail loudly — a freeze that did not apply must stop the release train rather than let the window appear to open | +| `set --frozen …` | freeze and unfreeze | fail loudly — a freeze that did not apply must stop the release train rather than let the window appear to open | -The flip is a read-modify-write of the whole ruleset: a partial `PUT` is not guaranteed to preserve the fields it omits, and the omitted fields are the bypass actors and branch conditions, so getting it wrong unlocks the branch it was asked to lock. The body goes to a file and is never echoed, because three of the consuming repos are public and a ruleset body names its bypass actors. +A property the org does not define is a lookup error in both modes, not "thawed": that is provisioning drift, and the reader escalating on it is what makes the drift visible instead of silently opening every repo. The four release-train reusables and both notify reusables check these scripts out at `job.workflow_sha` — the commit of the workflow file that defines the running job, so a consumer that pins the workflow to a SHA gets that SHA's scripts too. Pinning a workflow while its scripts float is not a pin. @@ -481,15 +481,15 @@ whether its author has signed. Four repos learned that the hard way, gating on - **`mindsdb-release-train` GitHub App** with `Administration`, `Contents`, and `Pull requests: write`, installed on each repo, and set as a **bypass actor** - on the `staging` ruleset. Per-job tokens are minted with + on the org `staging-freeze` ruleset. Per-job tokens are minted with `actions/create-github-app-token`. - **`vars.RELEASE_APP_CLIENT_ID`** (org variable) and **`secrets.RELEASE_APP_PRIVATE_KEY`** (org secret) — the private key reaches the reusable workflows via `secrets: inherit` in the caller. -- A **pre-provisioned `staging-freeze` ruleset** in each repo: one `update` rule - targeting `staging`, created `disabled`, with the App as bypass actor. The - workflows only flip its `enforcement` between `active` and `disabled` — they - never touch the underlying branch protection. +- The **`staging_frozen` org custom property** and the **`staging-freeze` org + ruleset** that targets repos where it is `true`, both defined in terraform + (`newprod/global/github/main.tf`). The workflows only set the property — they + never touch the ruleset or the underlying branch protection. ### Caller wrappers diff --git a/notify-pipeline-status/action.yml b/notify-pipeline-status/action.yml index bb123f8..a6856e8 100644 --- a/notify-pipeline-status/action.yml +++ b/notify-pipeline-status/action.yml @@ -54,12 +54,12 @@ inputs: description: "'true' to stay silent entirely while the staging freeze window is closed" required: false default: "false" - ruleset-name: - description: "The freeze ruleset to read" + freeze-property: + description: "The freeze custom property to read" required: false - default: staging-freeze + default: staging_frozen freeze-token: - description: "A release-train App token that can read rulesets. Empty means 'could not mint', which escalates to frozen." + description: "A release-train App token. Empty means 'could not mint', which escalates to frozen." required: false default: "" github-token: @@ -92,14 +92,14 @@ runs: # the prior-run lookup that recovery mode would otherwise spend an API call # on. # - # The signal is the repository ruleset that the freeze/unfreeze workflows - # toggle, not an inference from workflow-run history: a freeze that skipped + # The signal is the `staging_frozen` custom property that the freeze/unfreeze + # workflows set, not an inference from workflow-run history: a freeze that skipped # itself because staging had nothing unreleased still concludes `success`, # and history cannot tell that apart from a real freeze. # # `scripts/freeze_state.py` is also what those workflows use to WRITE it, so - # one file owns the ruleset contract and renaming a ruleset is one edit - # rather than four that agree by luck. + # one file owns the contract and renaming the property is one edit rather + # than four that agree by luck. - name: Check release-freeze state id: freeze if: inputs.freeze-scoped == 'true' @@ -107,7 +107,7 @@ runs: env: GH_TOKEN: ${{ inputs.freeze-token }} REPO: ${{ github.repository }} - RULESET_NAME: ${{ inputs.ruleset-name }} + FREEZE_PROPERTY: ${{ inputs.freeze-property }} SHARED: ${{ inputs.shared-path }} run: | set -uo pipefail @@ -121,7 +121,7 @@ runs: exit 0 fi python3 "${SHARED}/scripts/freeze_state.py" read \ - --repo "${REPO}" --ruleset-name "${RULESET_NAME}" --on-error escalate + --repo "${REPO}" --property-name "${FREEZE_PROPERTY}" --on-error escalate # For the recovery message, find out how the PREVIOUS conclusive run of this # same workflow on this same branch ended. This step only gathers evidence; diff --git a/scripts/freeze_state.py b/scripts/freeze_state.py index 99f4500..2bb34b6 100644 --- a/scripts/freeze_state.py +++ b/scripts/freeze_state.py @@ -1,20 +1,19 @@ """The one place that knows how a repository's release freeze is stored. -A release freeze is a pre-provisioned repository ruleset (by default named -``staging-freeze``) whose ``enforcement`` field is flipped between ``active`` and -``disabled``. Four workflows care about that fact and they used to each carry -their own copy of it: the freeze and unfreeze workflows flipped it, and the two -alerting workflows read it to decide whether a red staging branch is worth -interrupting anyone for. - -Three copies agreed by luck rather than by contract. Only two of them took the -ruleset name as an input, so renaming the ruleset in one repository moved the -freeze and left the alerting reading a name that no longer existed. That failure -is silent in the direction that hurts: the reader escalates when it cannot -establish the state, so every ordinary mid-week staging red would have paged the -channel forever and the cause would have looked like a Slack problem. - -So the contract lives here, once, and the workflows call it. +A release freeze is the repository custom property ``staging_frozen`` (a +``true_false`` property defined once at the org level, in terraform). A single +org ruleset, also in terraform, targets ``refs/heads/staging`` in every repo +whose ``staging_frozen`` is ``true`` and carries one ``update`` rule with the +``mindsdb-release-train`` App as a bypass actor. Freezing a repo is therefore +setting its property to ``true``; unfreezing is setting it to ``false``. The +ruleset itself is never touched, which is what lets terraform own it with an +empty plan while the workflows flip repos in and out of it all week. + +Four workflows care about that fact and they used to each carry their own copy +of it: the freeze and unfreeze workflows flipped it, and the two alerting +workflows read it to decide whether a red staging branch is worth interrupting +anyone for. Three copies agreed by luck rather than by contract, so the contract +lives here, once, and the workflows call it. Two modes, because the two callers want opposite things from a failure. @@ -24,16 +23,13 @@ a real release-blocking failure, and because a notify job must never turn a green pipeline red. -``set`` flips enforcement for the freeze and unfreeze workflows, and fails loudly. -A freeze that could not be applied has to stop the release train rather than let -the window appear to open. +``set`` writes the property for the freeze and unfreeze workflows, and fails +loudly. A freeze that could not be applied has to stop the release train rather +than let the window appear to open. -The flip is read-modify-write against the whole ruleset. A partial ``PUT`` is not -guaranteed to preserve the fields it omits, and the fields being omitted here are -the bypass actors and the branch conditions, so getting that wrong unlocks the -branch it was asked to lock. The body is written to a file and never echoed: -three of the consuming repositories are public, and a ruleset body names its -bypass actors. +A property that the org does not define is a lookup error in both modes, not a +"false": that is provisioning drift, and the reader escalating on it is what +makes the drift visible instead of silently thawing every repo. """ from __future__ import annotations @@ -48,100 +44,80 @@ # A `gh api` invocation, injectable so the tests do not need a network or a token. Runner = Callable[[Sequence[str]], "subprocess.CompletedProcess[str]"] -ENFORCEMENT_ACTIVE = "active" -ENFORCEMENT_DISABLED = "disabled" +DEFAULT_PROPERTY = "staging_frozen" + +# GitHub stores a `true_false` property's value as the string "true" / "false", +# and returns null for a repo the property has never been set on. +FROZEN = "true" +THAWED = "false" class LookupError_(Exception): - """The ruleset could not be read. Carries the message the caller should print.""" + """The property could not be read or written. Carries the message the caller should print.""" def _run(argv: Sequence[str]) -> "subprocess.CompletedProcess[str]": return subprocess.run(list(argv), capture_output=True, text=True, check=False) -def fetch_ruleset(repo: str, name: str, *, runner: Runner = _run) -> dict: - """The ruleset named ``name`` in ``repo``, as a dict. +def _failure(prefix: str, result: "subprocess.CompletedProcess[str]") -> LookupError_: + # Both streams, because `gh` reports an API error body on stdout while + # writing its own diagnostics to stderr, and a release-train-stopping + # error with no cause in it is the worst kind to be paged about. + return LookupError_(f"{prefix}: {result.stderr.strip() or result.stdout.strip()}") + + +def fetch_property(repo: str, name: str, *, runner: Runner = _run) -> str | None: + """The value of custom property ``name`` on ``repo``: "true", "false" or None. - Raises ``LookupError_`` when the API call fails or no ruleset carries that - name. The two are distinct messages on purpose: "the token cannot read - rulesets" and "provisioning has drifted" get fixed by different people. + Raises ``LookupError_`` when the API call fails or the org defines no such + property. The two are distinct messages on purpose: "the token cannot read + properties" and "provisioning has drifted" get fixed by different people. """ - listing = runner(["gh", "api", f"repos/{repo}/rulesets"]) + listing = runner(["gh", "api", f"repos/{repo}/properties/values"]) if listing.returncode != 0: - raise LookupError_(f"Could not read rulesets: {listing.stderr.strip() or listing.stdout.strip()}") + raise _failure("Could not read custom properties", listing) try: - rulesets = json.loads(listing.stdout or "[]") + values = json.loads(listing.stdout or "[]") except json.JSONDecodeError as exc: - raise LookupError_(f"Ruleset listing was not JSON: {exc}") from exc + raise LookupError_(f"Custom property listing was not JSON: {exc}") from exc - for ruleset in rulesets: - if ruleset.get("name") == name: - return ruleset - raise LookupError_(f"Ruleset '{name}' not found in {repo}") + for entry in values: + if entry.get("property_name") == name: + value = entry.get("value") + # Tolerate a real boolean in case the API ever stops stringifying. + if isinstance(value, bool): + return FROZEN if value else THAWED + return value + raise LookupError_(f"Custom property '{name}' is not defined for {repo}") def is_frozen(repo: str, name: str, *, runner: Runner = _run) -> bool: - """True when the freeze ruleset is enforced, i.e. the branch is locked.""" - return fetch_ruleset(repo, name, runner=runner).get("enforcement") == ENFORCEMENT_ACTIVE + """True when the property is "true", i.e. the org freeze ruleset applies here.""" + return fetch_property(repo, name, runner=runner) == FROZEN -def set_enforcement( +def set_frozen( repo: str, name: str, - enforcement: str, + frozen: bool, *, body_path: str, runner: Runner = _run, -) -> int: - """Flip the ruleset's enforcement, preserving every other field. - - Returns the ruleset id so the caller can name it in its log line. - """ - ruleset = fetch_ruleset(repo, name, runner=runner) - ruleset_id = ruleset["id"] - - # Re-read the full ruleset rather than reusing the listing entry: the list - # endpoint returns a summary that omits `rules` and `bypass_actors`, and - # PUTting that summary back would drop them. - detail = runner(["gh", "api", f"repos/{repo}/rulesets/{ruleset_id}"]) - if detail.returncode != 0: - # Both streams, because `gh` reports an API error body on stdout while - # writing its own diagnostics to stderr, and a release-train-stopping - # error with no cause in it is the worst kind to be paged about. - raise LookupError_( - f"Could not read ruleset {ruleset_id}: " - f"{detail.stderr.strip() or detail.stdout.strip()}" - ) - - try: - full = json.loads(detail.stdout) - except json.JSONDecodeError as exc: - # Guarded for the same reason the listing above is: an unparseable body - # has to arrive as the "provisioning has drifted" annotation `main()` - # prints, not as a traceback the operator has to read past. - raise LookupError_(f"Ruleset {ruleset_id} was not JSON: {exc}") from exc - payload = { - "name": full["name"], - "target": full["target"], - "enforcement": enforcement, - "bypass_actors": full.get("bypass_actors", []), - "conditions": full.get("conditions", {}), - "rules": full.get("rules", []), - } +) -> str: + """Write the property. Returns the value written so the caller can log it.""" + value = FROZEN if frozen else THAWED + payload = {"properties": [{"property_name": name, "value": value}]} with open(body_path, "w", encoding="utf-8") as handle: json.dump(payload, handle) - put = runner( - ["gh", "api", "--method", "PUT", f"repos/{repo}/rulesets/{ruleset_id}", "--input", body_path] + patch = runner( + ["gh", "api", "--method", "PATCH", f"repos/{repo}/properties/values", "--input", body_path] ) - if put.returncode != 0: - raise LookupError_( - f"Could not update ruleset {ruleset_id}: " - f"{put.stderr.strip() or put.stdout.strip()}" - ) - return ruleset_id + if patch.returncode != 0: + raise _failure(f"Could not set custom property '{name}'", patch) + return value def emit(key: str, value: str) -> None: @@ -157,24 +133,24 @@ def main(argv: list[str] | None = None, *, runner: Runner = _run) -> int: parser = argparse.ArgumentParser(description=__doc__) parser.add_argument("mode", choices=["read", "set"]) parser.add_argument("--repo", required=True, help="owner/name") - parser.add_argument("--ruleset-name", default="staging-freeze") + parser.add_argument("--property-name", default=DEFAULT_PROPERTY) parser.add_argument( - "--enforcement", - choices=[ENFORCEMENT_ACTIVE, ENFORCEMENT_DISABLED], - help="set mode only: the enforcement to write", + "--frozen", + choices=[FROZEN, THAWED], + help="set mode only: the value to write", ) parser.add_argument( "--on-error", choices=["escalate", "fail"], default="escalate", - help="read mode only: what an unreadable ruleset means", + help="read mode only: what an unreadable property means", ) - parser.add_argument("--body-path", default=None, help="set mode only: where to stage the PUT body") + parser.add_argument("--body-path", default=None, help="set mode only: where to stage the PATCH body") args = parser.parse_args(argv) if args.mode == "read": try: - frozen = is_frozen(args.repo, args.ruleset_name, runner=runner) + frozen = is_frozen(args.repo, args.property_name, runner=runner) except LookupError_ as exc: if args.on_error == "fail": # No ::error:: annotation: every caller of this mode catches the @@ -192,17 +168,17 @@ def main(argv: list[str] | None = None, *, runner: Runner = _run) -> int: emit("frozen", "true" if frozen else "false") return 0 - if not args.enforcement: - parser.error("set mode requires --enforcement") - body_path = args.body_path or os.path.join(os.environ.get("RUNNER_TEMP", "."), "ruleset.json") + if not args.frozen: + parser.error("set mode requires --frozen") + body_path = args.body_path or os.path.join(os.environ.get("RUNNER_TEMP", "."), "freeze-property.json") try: - ruleset_id = set_enforcement( - args.repo, args.ruleset_name, args.enforcement, body_path=body_path, runner=runner + value = set_frozen( + args.repo, args.property_name, args.frozen == FROZEN, body_path=body_path, runner=runner ) except LookupError_ as exc: print(f"::error::{exc} — provisioning has drifted.", file=sys.stderr) return 1 - print(f"Ruleset '{args.ruleset_name}' (#{ruleset_id}) enforcement set to {args.enforcement}.") + print(f"Custom property '{args.property_name}' set to {value} on {args.repo}.") return 0 diff --git a/tests/test_freeze_state.py b/tests/test_freeze_state.py index d72ffe4..ae0348d 100644 --- a/tests/test_freeze_state.py +++ b/tests/test_freeze_state.py @@ -36,62 +36,73 @@ def runner(argv): return runner -RULESET_LIST = json.dumps( - [ - {"id": 7, "name": "staging-freeze", "enforcement": "disabled"}, - {"id": 9, "name": "something-else", "enforcement": "active"}, - ] -) - -RULESET_DETAIL = json.dumps( - { - "id": 7, - "name": "staging-freeze", - "target": "branch", - "enforcement": "disabled", - "bypass_actors": [{"actor_id": 1, "actor_type": "Integration"}], - "conditions": {"ref_name": {"include": ["refs/heads/staging"]}}, - "rules": [{"type": "update"}], - } -) +def values(frozen): + return json.dumps( + [ + {"property_name": "team", "value": "devops"}, + {"property_name": "staging_frozen", "value": frozen}, + ] + ) class TestIsFrozen: - def test_active_enforcement_is_frozen(self): - listing = json.dumps([{"id": 7, "name": "staging-freeze", "enforcement": "active"}]) - assert freeze_state.is_frozen("o/r", "staging-freeze", runner=fake_runner([completed(listing)])) + def test_true_string_is_frozen(self): + assert freeze_state.is_frozen("o/r", "staging_frozen", runner=fake_runner([completed(values("true"))])) - def test_disabled_enforcement_is_not_frozen(self): + def test_false_string_is_not_frozen(self): assert not freeze_state.is_frozen( - "o/r", "staging-freeze", runner=fake_runner([completed(RULESET_LIST)]) + "o/r", "staging_frozen", runner=fake_runner([completed(values("false"))]) ) + def test_never_set_is_not_frozen(self): + """A repo the property was never written on comes back null, not absent.""" + assert not freeze_state.is_frozen( + "o/r", "staging_frozen", runner=fake_runner([completed(values(None))]) + ) + + def test_real_boolean_is_tolerated(self): + assert freeze_state.is_frozen("o/r", "staging_frozen", runner=fake_runner([completed(values(True))])) + def test_matches_on_name_not_position(self): - """A repo carries several rulesets; only the named one decides the freeze.""" - runner = fake_runner([completed(RULESET_LIST)]) - assert freeze_state.is_frozen("o/r", "something-else", runner=runner) + """A repo carries several properties; only the named one decides the freeze.""" + listing = json.dumps( + [ + {"property_name": "staging_frozen", "value": "false"}, + {"property_name": "other_freeze", "value": "true"}, + ] + ) + assert freeze_state.is_frozen("o/r", "other_freeze", runner=fake_runner([completed(listing)])) - def test_missing_ruleset_raises(self): - with pytest.raises(freeze_state.LookupError_, match="not found"): - freeze_state.is_frozen("o/r", "absent", runner=fake_runner([completed(RULESET_LIST)])) + def test_undefined_property_raises(self): + """Absent from the listing means the org does not define it: drift, not thawed.""" + with pytest.raises(freeze_state.LookupError_, match="not defined"): + freeze_state.is_frozen("o/r", "absent", runner=fake_runner([completed(values("true"))])) def test_api_failure_raises(self): runner = fake_runner([completed(stderr="HTTP 403", returncode=1)]) - with pytest.raises(freeze_state.LookupError_, match="Could not read rulesets"): - freeze_state.is_frozen("o/r", "staging-freeze", runner=runner) + with pytest.raises(freeze_state.LookupError_, match="Could not read custom properties"): + freeze_state.is_frozen("o/r", "staging_frozen", runner=runner) + + def test_non_json_listing_is_a_lookup_error_not_a_traceback(self): + runner = fake_runner([completed("502")]) + with pytest.raises(freeze_state.LookupError_, match="was not JSON"): + freeze_state.is_frozen("o/r", "staging_frozen", runner=runner) class TestReadMode: """The alerting path. It must never fail the job and never under-report.""" def test_reports_false_when_thawed(self, capsys): - code = freeze_state.main( - ["read", "--repo", "o/r"], runner=fake_runner([completed(RULESET_LIST)]) - ) + code = freeze_state.main(["read", "--repo", "o/r"], runner=fake_runner([completed(values("false"))])) assert code == 0 assert "frozen=false" in capsys.readouterr().out - def test_unreadable_ruleset_escalates_to_frozen(self, capsys): + def test_reads_the_repo_property_values_endpoint(self): + runner = fake_runner([completed(values("false"))]) + freeze_state.main(["read", "--repo", "o/r"], runner=runner) + assert runner.calls == [["gh", "api", "repos/o/r/properties/values"]] + + def test_unreadable_property_escalates_to_frozen(self, capsys): """A lookup problem must not silence a real release-blocking failure.""" runner = fake_runner([completed(stderr="HTTP 403", returncode=1)]) code = freeze_state.main(["read", "--repo", "o/r"], runner=runner) @@ -100,9 +111,9 @@ def test_unreadable_ruleset_escalates_to_frozen(self, capsys): assert "frozen=true" in out assert "::warning::" in out - def test_missing_ruleset_escalates_to_frozen(self, capsys): - runner = fake_runner([completed(RULESET_LIST)]) - code = freeze_state.main(["read", "--repo", "o/r", "--ruleset-name", "renamed"], runner=runner) + def test_undefined_property_escalates_to_frozen(self, capsys): + runner = fake_runner([completed(values("false"))]) + code = freeze_state.main(["read", "--repo", "o/r", "--property-name", "renamed"], runner=runner) assert code == 0 assert "frozen=true" in capsys.readouterr().out @@ -113,71 +124,52 @@ def test_on_error_fail_is_available_for_non_alerting_callers(self): def test_writes_github_output(self, tmp_path, monkeypatch): out = tmp_path / "gh-output" monkeypatch.setenv("GITHUB_OUTPUT", str(out)) - freeze_state.main(["read", "--repo", "o/r"], runner=fake_runner([completed(RULESET_LIST)])) + freeze_state.main(["read", "--repo", "o/r"], runner=fake_runner([completed(values("false"))])) assert out.read_text().strip() == "frozen=false" class TestSetMode: - """The freeze/unfreeze path. It must preserve the ruleset and fail loudly.""" - - def test_put_preserves_bypass_actors_conditions_and_rules(self, tmp_path): - body = tmp_path / "ruleset.json" - runner = fake_runner([completed(RULESET_LIST), completed(RULESET_DETAIL), completed("{}")]) - freeze_state.set_enforcement( - "o/r", "staging-freeze", "active", body_path=str(body), runner=runner - ) - written = json.loads(body.read_text()) - assert written["enforcement"] == "active" - assert written["bypass_actors"] == [{"actor_id": 1, "actor_type": "Integration"}] - assert written["rules"] == [{"type": "update"}] - assert written["conditions"]["ref_name"]["include"] == ["refs/heads/staging"] - - def test_reads_the_detail_endpoint_not_the_listing(self, tmp_path): - """The listing omits rules and bypass actors; PUTting it back drops them.""" - runner = fake_runner([completed(RULESET_LIST), completed(RULESET_DETAIL), completed("{}")]) - freeze_state.set_enforcement( - "o/r", "staging-freeze", "active", body_path=str(tmp_path / "b.json"), runner=runner - ) - assert runner.calls[1] == ["gh", "api", "repos/o/r/rulesets/7"] - - def test_failed_put_raises(self, tmp_path): - runner = fake_runner( - [completed(RULESET_LIST), completed(RULESET_DETAIL), completed(stderr="HTTP 422", returncode=1)] - ) - with pytest.raises(freeze_state.LookupError_, match="Could not update"): - freeze_state.set_enforcement( - "o/r", "staging-freeze", "active", body_path=str(tmp_path / "b.json"), runner=runner - ) - - def test_a_non_json_detail_body_is_a_lookup_error_not_a_traceback(self, tmp_path): - """The one unguarded parse in the module. An operator paged by a stopped - release train reads `main()`'s "provisioning has drifted" annotation, not a - JSONDecodeError stack.""" - runner = fake_runner([completed(RULESET_LIST), completed("502")]) - with pytest.raises(freeze_state.LookupError_, match="was not JSON"): - freeze_state.set_enforcement( - "o/r", "staging-freeze", "active", body_path=str(tmp_path / "b.json"), runner=runner - ) + """The freeze/unfreeze path. It must write exactly the property and fail loudly.""" + + def test_patches_the_property_as_a_string(self, tmp_path): + body = tmp_path / "prop.json" + runner = fake_runner([completed("")]) + freeze_state.set_frozen("o/r", "staging_frozen", True, body_path=str(body), runner=runner) + assert json.loads(body.read_text()) == { + "properties": [{"property_name": "staging_frozen", "value": "true"}] + } + assert runner.calls == [ + ["gh", "api", "--method", "PATCH", "repos/o/r/properties/values", "--input", str(body)] + ] + + def test_unfreeze_writes_false_not_null(self, tmp_path): + """Null would read back as thawed too, but leaves no trace that a freeze ever ran.""" + body = tmp_path / "prop.json" + freeze_state.set_frozen("o/r", "staging_frozen", False, body_path=str(body), runner=fake_runner([completed("")])) + assert json.loads(body.read_text())["properties"][0]["value"] == "false" + + def test_failed_patch_raises(self, tmp_path): + runner = fake_runner([completed(stderr="HTTP 422", returncode=1)]) + with pytest.raises(freeze_state.LookupError_, match="Could not set"): + freeze_state.set_frozen("o/r", "staging_frozen", True, body_path=str(tmp_path / "b.json"), runner=runner) def test_an_error_body_on_stdout_still_reaches_the_message(self, tmp_path): """`gh` puts the API's error body on stdout and its own noise on stderr, so taking stderr alone can print an error with no cause in it.""" runner = fake_runner( - [ - completed(RULESET_LIST), - completed(RULESET_DETAIL), - completed(stdout='{"message":"Resource not accessible by integration"}', returncode=1), - ] + [completed(stdout='{"message":"Resource not accessible by integration"}', returncode=1)] ) with pytest.raises(freeze_state.LookupError_, match="not accessible by integration"): - freeze_state.set_enforcement( - "o/r", "staging-freeze", "active", body_path=str(tmp_path / "b.json"), runner=runner - ) + freeze_state.set_frozen("o/r", "staging_frozen", True, body_path=str(tmp_path / "b.json"), runner=runner) - def test_missing_ruleset_fails_the_job(self, tmp_path): + def test_failed_set_fails_the_job(self, tmp_path): """Never escalate here: a freeze that did not apply must stop the train.""" code = freeze_state.main( - ["set", "--repo", "o/r", "--enforcement", "active", "--body-path", str(tmp_path / "b.json")], - runner=fake_runner([completed("[]")]), + ["set", "--repo", "o/r", "--frozen", "true", "--body-path", str(tmp_path / "b.json")], + runner=fake_runner([completed(stderr="HTTP 404", returncode=1)]), ) assert code == 1 + + def test_set_requires_a_value(self, tmp_path): + with pytest.raises(SystemExit): + freeze_state.main(["set", "--repo", "o/r"], runner=fake_runner([]))