diff --git a/.github/actions/ext-ci/README.md b/.github/actions/ext-ci/README.md index d472c8db42d..836c4ba9697 100644 --- a/.github/actions/ext-ci/README.md +++ b/.github/actions/ext-ci/README.md @@ -18,12 +18,23 @@ also runs for merge groups and on manual dispatch (`repos`, `pr_number` or `fpri | `repos[].workflow` | workflow file in that repository; must accept the inputs below | | `repos[].default_ref` | branch to dispatch when neither `pr-` nor a branch named like the F´ base branch exists there | | `repos[].timeout_minutes` | remote running time allowed, and separately the time it may wait in queue; the remote run is cancelled on timeout. Keep `2 * timeout_minutes` under the job's `timeout-minutes` in ext-ci.yml | -| `repos[].failure_policy` | `block`: a failure is a red check and fails the orchestrator job; `notify`: neutral (non-blocking) check | -| `repos[].on_pr` | `false`: never dispatched for a PR head (merge queue and manual `fprime_ref` runs only) | +| `repos[].failure_policy` | `block`: a failure is a red check and fails the orchestrator job; `notify`: the check still shows the real result (red on failure) but the orchestrator job stays green; do not make it a required check | +| `repos[].on_pr` | `false`: never dispatched for a PR head (merge queue and manual `fprime_ref` runs only), except once on the PR that adds the entry (see below) | Orchestrator-side faults (dispatch error, run never started, polling lost the run) fail the job whatever the policy. A re-run of the orchestrator re-dispatches only the repos whose check is not green. +### Which manifest.json is used + +The one at the F´ revision under test: the PR head, the merge-queue commit or the manual `fprime_ref`. +It is read as data through this repository's contents API (PR head commits are reachable here, fork or +not); the workflow and the script come from the default branch, whose `manifest.json` only serves the +`upstream_repository` preflight. So a PR that adds or edits an entry exercises it on its own checks, and +entries the default branch does not have ignore `on_pr`. The external repository must already be in the +External CI App installation or the check reports `dispatch failed ... 403`. A remote run held by a +protected environment shows as "Waiting for approval"; both timeout clocks restart when it is released +and the orchestrator job's `timeout-minutes` bounds the wait. + ## Contract for an external `ci.yml` - `on: workflow_dispatch` with inputs `fprime_repository`, `fprime_ref` (exact SHA to use as the F´ @@ -31,7 +42,9 @@ whatever the policy. A re-run of the orchestrator re-dispatches only the repos w - `permissions: contents: read` (or `{}`), no `${{ secrets.* }}`, no Actions cache writes on dispatched runs: the dispatched run executes untrusted F´ PR code in the external repository's own context. - Repositories with self-hosted or hardware runners: `on_pr: false` in the manifest, ephemeral runners, - SHA-pinned actions, and a job-level guard on `inputs.fprime_repository` in the workflow. + SHA-pinned actions, a protected environment with required reviewers on the hardware job, and a job-level + guard on `inputs.fprime_repository` in the workflow. `on_pr` alone is not the gate: the PR that adds the + entry dispatches it once. ## Credentials diff --git a/.github/actions/ext-ci/orchestrator.py b/.github/actions/ext-ci/orchestrator.py index 3d8bf300307..47710c2aeaa 100644 --- a/.github/actions/ext-ci/orchestrator.py +++ b/.github/actions/ext-ci/orchestrator.py @@ -19,6 +19,7 @@ remote runs """ +import base64 import json import os import re @@ -318,7 +319,8 @@ def relevant_change(files: list[str] | None, manifest: dict) -> bool: def select_repos(manifest: dict, target: dict) -> list[dict]: """Return the manifest entries to run: all of them, narrowed to the `repos` input ids on a manual run (unknown id raises RuntimeError) and to entries with `on_pr` true (the default) when a PR is tested, - manual runs included.""" + manual runs included. Entries the PR itself adds (`new`, see head_manifest) ignore `on_pr`: the PR + introducing an entry is the one run it must get.""" repos = manifest["repos"] wanted = (env.get("INPUT_REPOS") or "all").strip() if EVENT_NAME == "workflow_dispatch" and wanted and wanted != "all": @@ -328,10 +330,46 @@ def select_repos(manifest: dict, target: dict) -> list[dict]: raise RuntimeError("unknown manifest ids: " + ", ".join(unknown)) repos = [entry for entry in repos if entry["id"] in ids] if target["pr"]: - repos = [entry for entry in repos if entry.get("on_pr", True)] + repos = [ + entry for entry in repos if entry.get("on_pr", True) or entry.get("new") + ] return repos +def head_manifest(default: dict, target: dict, files: list[str] | None) -> dict: + """The manifest to run from: the one at the F´ revision under test (PR head, merge-queue commit or + manual ref), so a PR that adds or edits an entry exercises it on its own checks. It is read as + data through the contents API of this repo (PR head commits are reachable here, fork or not); the + workflow and this script stay default-branch. When the PR edits manifest.json (`files`), entries + whose id the default branch does not have are marked `new`; a stale branch that merely predates a + removal gets no such mark. Raises when the file is missing, malformed or has duplicate ids. + """ + try: + blob = own.get( + f"/repos/{REPO}/contents/{MANIFEST_PATH}?ref={target['head_sha']}" + ) + except ApiError as e: + if e.status == 404: + raise RuntimeError( + f"no {MANIFEST_PATH} at {fprime_rev(target)} (predates external CI?)" + ) from e + raise + head = json.loads(base64.b64decode(blob["content"])) + ids = [entry["id"] for entry in head["repos"]] + if len(ids) != len(set(ids)): + raise RuntimeError(f"duplicate ids in {MANIFEST_PATH} at {fprime_rev(target)}") + known = {entry["id"] for entry in default["repos"]} + edited = files is None or MANIFEST_PATH in files + for entry in head["repos"]: + entry["new"] = edited and entry["id"] not in known + new = [entry["id"] for entry in head["repos"] if entry["new"]] + log( + f"manifest.json from {fprime_rev(target)}: {ids}" + + (f", new: {new}" if new else "") + ) + return head + + def resolve_ext_ref(entry: dict, target: dict) -> str: """Return the external repo branch to dispatch: its `pr-` branch if it exists, else a branch named like the F´ base branch (`devel`, `release/x`), else the entry's `default_ref` (`main`). @@ -450,7 +488,15 @@ def dispatch_one(manifest: dict, entry: dict, target: dict, state: dict) -> None except ( Exception ) as e: # noqa: BLE001 - one repo's failure must not stop the fan-out - s["error"], s["remote_conclusion"] = f"dispatch failed: {e}", "dispatch_failed" + hint = ( + " (no such workflow, or the repository is not in the External CI App installation)" + if isinstance(e, ApiError) and e.status in (403, 404) + else "" + ) + s["error"], s["remote_conclusion"] = ( + f"dispatch failed: {e}{hint}", + "dispatch_failed", + ) log(f"::error::[{entry['id']}] {s['error']}") save_state(state) @@ -463,6 +509,24 @@ def cancel_run(s: dict) -> None: log(f"::warning::[{s['id']}] cancel: {e}") +def report_waiting(s: dict) -> None: + """Retitle one repo's Check Run while its remote run waits for an environment approval.""" + log(f"[{s['id']}] waiting for approval in {s['repository']}: {s['remote_run_url']}") + try: + own.patch( + f"/repos/{REPO}/check-runs/{s['check_run_id']}", + { + "output": { + "title": f"Waiting for approval in {s['repository']}", + "summary": f"The remote run is held by a protected environment and needs a reviewer " + f"of {code(s['repository'])}: {s['remote_run_url']}\n\nOrchestrator: {ORCHESTRATOR_URL}", + }, + }, + ) + except ApiError as e: # cosmetic + log(f"::warning::[{s['id']}] check update: {e}") + + def poll_all(state: dict) -> None: """Poll every dispatched run every POLL_S seconds until each has a `remote_conclusion`. @@ -484,8 +548,20 @@ def active() -> list[dict]: run = ext.get( f"/repos/{s['repository']}/actions/runs/{s['remote_run_id']}" ) - if run.get("run_started_at") and not s["remote_started_at"]: - s["remote_started_at"] = run["run_started_at"] + if run["status"] == "waiting": + # Held for a protected-environment approval in the external repo (possibly after + # earlier jobs ran): both clocks restart when it is released; the orchestrator + # job's own timeout bounds the wait (finalize cancels it). + s["remote_started_at"] = None + s["dispatched_at"] = now().strftime("%Y-%m-%dT%H:%M:%SZ") + if not s.get("waiting_reported"): + s["waiting_reported"] = True + report_waiting(s) + continue + if run["status"] == "in_progress" and not s["remote_started_at"]: + # Observed start, not `run_started_at`: GitHub stamps that at creation, approval + # wait included. + s["remote_started_at"] = now().strftime("%Y-%m-%dT%H:%M:%SZ") timeout = timedelta(minutes=s["timeout_minutes"]) if run["status"] == "completed": s["remote_conclusion"] = run["conclusion"] @@ -546,13 +622,12 @@ def failure_details(s: dict) -> str: def check_conclusion(s: dict) -> str: - """Map the remote result to the Check Run conclusion: success, neutral (any failure on a - `failure_policy: notify` repo), cancelled, timed_out or failure.""" + """Map the remote result to the Check Run conclusion: success, cancelled, timed_out or failure. + `failure_policy` does not soften it (GitHub folds neutral checks away; a non-blocking failure + must still show red): it only decides whether this job fails, see run().""" rc = s["remote_conclusion"] if rc == "success": return "success" - if s["failure_policy"] == "notify": - return "neutral" # visible, but does not block the PR if rc == "cancelled": return "cancelled" if rc in ("timed_out", "queue_timeout"): @@ -579,7 +654,7 @@ def finalize_one(target: dict, s: dict) -> None: if s["failure_policy"] == "notify" and s["remote_conclusion"] != "success": lines += [ "", - f"This repo is {code('failure_policy: notify')}: the failure does not block the PR.", + f"This repo is {code('failure_policy: notify')}: informational, the failure does not block merging.", ] lines += ["", f"Orchestrator run: {ORCHESTRATOR_URL}"] output = { @@ -676,11 +751,12 @@ def write_summary(target: dict, state: dict) -> None: # ---------------------------------------------------------------- modes def run() -> None: - """The `run` mode: resolve target, load manifest, fork preflight, select repos, dispatch, poll, - close checks, write summary. Exits 1 when a `failure_policy: block` repo did not succeed or the - orchestrator itself failed on any repo.""" + """The `run` mode: resolve target, fork preflight, load the target's manifest, select repos, + dispatch, poll, close checks, write summary. Exits 1 when a `failure_policy: block` repo did not + succeed or the orchestrator itself failed on any repo.""" target = resolve_target() log(f"target: {json.dumps(target)}") + # The default branch's copy: trusted, used for the preflight below and as the `new` baseline. with open(MANIFEST_PATH) as f: manifest = json.load(f) # Fork preflight: a fork of F´ that copies this workflow must not dispatch into the ext repos. @@ -691,6 +767,8 @@ def run() -> None: "Use workflow_dispatch to run it here deliberately." ) return + files = changed_files(target) + manifest = head_manifest(manifest, target, files) repos = select_repos(manifest, target) if ATTEMPT > 1: # re-run: only the repos whose check is not green yet repos = [e for e in repos if not check_succeeded(manifest, e, target)] @@ -699,9 +777,7 @@ def run() -> None: ) state = {"target": target, "attempt": ATTEMPT, "repos": {}} save_state(state) - if EVENT_NAME != "workflow_dispatch" and not relevant_change( - changed_files(target), manifest - ): + if EVENT_NAME != "workflow_dispatch" and not relevant_change(files, manifest): log("no relevant files changed; marking all repos as skipped") skip_all(manifest, target, repos, state) write_summary(target, state) @@ -760,7 +836,7 @@ def finalize() -> None: except ApiError: pass if cancelled: - conclusion = "neutral" if s["failure_policy"] == "notify" else "cancelled" + conclusion = "cancelled" title, why = ( "Cancelled by orchestrator", "The orchestrator run was cancelled", @@ -774,7 +850,7 @@ def finalize() -> None: ) why = "Closed by the finalize step after the check update failed" else: - conclusion = "neutral" if s["failure_policy"] == "notify" else "failure" + conclusion = "failure" title = "Orchestrator error" why = f"The orchestrator run ended before this repo was reported ({code(s.get('error') or 'orchestrator error')})" try: