diff --git a/.github/workflows/README.md b/.github/workflows/README.md index 72476b6d..7762f6aa 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -40,11 +40,16 @@ container and executes: - `make vet` - `make lint` (golangci-lint, built from source to match the runner toolchain) - `make vuln` (govulncheck) -- a single `go test -race -json -timeout 600s -p 4 ./...` run: the race detector - plus the full integration suite against PostgreSQL, emitting the JSON that - `specter` ingests. This is one pass, not two. It replaced the former separate - `make test-race` + non-race `go test -json` runs (which walked the DB-bound - suite twice) +- the `go test -race -json` run: the race detector plus the full integration + suite against PostgreSQL, emitting the JSON that `specter` ingests. Every + package runs exactly once, in two invocations: everything except + `internal/server` under the shared 900 s per-package budget, then + `internal/server` alone under its own 1800 s budget, so the database-heavy suite + does not compete with sibling packages for the PostgreSQL service and a + hang elsewhere still fails at the shared budget. Both exit statuses and + both JSON streams are kept. It replaced the former separate `make + test-race` + non-race `go test -json` runs (which walked the DB-bound suite + twice) - frontend `vitest` (JUnit), also ingested by `specter` for spec AC coverage - `specter sync` to enforce coverage thresholds diff --git a/.github/workflows/go-ci.yml b/.github/workflows/go-ci.yml index 8c79796b..5c3b7d58 100644 --- a/.github/workflows/go-ci.yml +++ b/.github/workflows/go-ci.yml @@ -1,9 +1,10 @@ # Pre-merge gates for OpenWatch (repo root). # # Runs: make vet, make lint (staticcheck + gosec + ...), make vuln -# (govulncheck), a single `go test -race -json` run (data races + full +# (govulncheck), the `go test -race -json` run (data races + full # integration suite against a Postgres service container, AND the JSON -# specter ingests — one pass, not two), and `specter sync` (spec +# specter ingests; every package once, in two invocations so that +# internal/server runs alone, CP bugs/OW-066), and `specter sync` (spec # validation + strict AC coverage). # # Spec: specs/release/ci-gates.spec.yaml. @@ -29,6 +30,13 @@ jobs: gates: name: Quality + security gates runs-on: ubuntu-latest + # Explicit budget (CP bugs/OW-066), not the 360-minute platform default: + # the shared test phase can run to its 900 s per-package ceiling, the + # internal/server phase to its own ceiling, and the rest of the job + # (build, vet, lint, vuln, specter, vitest, upload) has measured at 5 to + # 8 minutes. 60 minutes covers both ceilings plus that with margin and + # still fails a job that has wedged. + timeout-minutes: 60 services: postgres: @@ -288,21 +296,47 @@ jobs: # ships inside the kensa module, so api-system-scan-config/AC-08 runs # here instead of skipping (100% outcome coverage requires it). export OPENWATCH_KENSA_RULES_DIR="$(go list -m -f '{{.Dir}}' github.com/Hanalyx/kensa)/rules" - # -timeout is per package. internal/server is the heavy suite (~470s - # without -race) and rides near the old 600s limit under -race + -p 4 - # DB contention, so it intermittently times out (e.g. main run #976). - # 900s gives headroom without masking a genuine hang. - go test -race -json -timeout 900s -p 4 ./... > /tmp/go-test.json - EXIT=$? - if [ "$EXIT" -ne 0 ]; then - echo "::group::Failed tests" - jq -r 'select(.Action=="fail" and .Test != null) | "FAIL \(.Package) \(.Test)"' /tmp/go-test.json | sort -u + # -timeout is per package. Two invocations, every package exactly + # once (CP bugs/OW-066). internal/server is the database-heavy suite: + # on the hosted runner it ran 794 s on main and timed out at 900 s on + # two PRs with no failing test, the same tests simply running 27% to + # 48% slower while sharing the PostgreSQL service with three sibling + # packages under -p 4. Phase 1 runs everything else under the shared + # 900 s ceiling, so a hang anywhere else still fails at 15 minutes. + # Phase 2 runs internal/server alone under 1800 s. Isolation on its + # own measured 885 s (run 35635263290): the same tests at the same + # per-test speed as the contended base, so the budget is what closes + # the margin. 1800 s is sized from evidence, base 794 s times the + # worst observed slowdown 1.63 is 1294 s, and doubles the time to + # detect a hang in this one package, which the founder accepted. + # Both exit statuses are kept: + # a green phase 2 never masks a red phase 1. Both JSON streams are + # kept for specter ingest and the artifact upload. + report() { + echo "::group::Failed tests ($1)" + jq -r 'select(.Action=="fail" and .Test != null) | "FAIL \(.Package) \(.Test)"' "$2" | sort -u echo "::endgroup::" - echo "::group::Failure output (last 80 lines)" - jq -r 'select(.Action=="output" and (.Output | ascii_downcase | test("--- fail|panic:|fatal error|error:"))) | "\(.Package): \(.Output)"' /tmp/go-test.json | tail -80 + echo "::group::Failure output ($1, last 80 lines)" + jq -r 'select(.Action=="output" and (.Output | ascii_downcase | test("--- fail|panic:|fatal error|error:"))) | "\(.Package): \(.Output)"' "$2" | tail -80 echo "::endgroup::" + } + ALL=$(go list ./...) + OTHERS=$(printf '%s\n' "$ALL" | grep -v '^github.com/Hanalyx/openwatch/internal/server$') + # The arithmetic has to close, or a package could be tested twice + # or not at all without anyone noticing. + if [ "$(printf '%s\n' "$ALL" | wc -l)" -ne $(( $(printf '%s\n' "$OTHERS" | wc -l) + 1 )) ]; then + echo "::error::package split does not close: internal/server must be exactly one package of $(printf '%s\n' "$ALL" | wc -l)" + exit 1 fi - exit $EXIT + # shellcheck disable=SC2086 + go test -race -json -timeout 900s -p 4 $OTHERS > /tmp/go-test.json + EXIT1=$? + [ "$EXIT1" -ne 0 ] && report "phase 1: every package except internal/server" /tmp/go-test.json + go test -race -json -timeout 1800s ./internal/server/ > /tmp/go-test-server.json + EXIT2=$? + [ "$EXIT2" -ne 0 ] && report "phase 2: internal/server" /tmp/go-test-server.json + echo "phase 1 exit $EXIT1, phase 2 exit $EXIT2" + [ "$EXIT1" -eq 0 ] && [ "$EXIT2" -eq 0 ] # Frontend Vitest run for specter ingest. The specs/frontend/ specs # reference tests in frontend/tests/pages/*. Without these JUnit @@ -342,7 +376,7 @@ jobs: - name: specter ingest if: steps.paths.outputs.go == 'true' - run: specter ingest --go-test /tmp/go-test.json --junit /tmp/vitest-junit.xml + run: specter ingest --go-test /tmp/go-test.json --go-test /tmp/go-test-server.json --junit /tmp/vitest-junit.xml - name: specter sync (AC coverage thresholds) if: steps.paths.outputs.go == 'true' @@ -378,6 +412,7 @@ jobs: path: | .specter-results.json /tmp/go-test.json + /tmp/go-test-server.json /tmp/vitest-junit.xml retention-days: 7 diff --git a/.secrets.baseline b/.secrets.baseline index ebd9373f..7e759472 100644 --- a/.secrets.baseline +++ b/.secrets.baseline @@ -133,14 +133,14 @@ "filename": ".github/workflows/go-ci.yml", "hashed_secret": "0e4f7f719374de1c33da2c0e5c0db5bcdc2e15d4", "is_verified": false, - "line_number": 38 + "line_number": 46 }, { "type": "Basic Auth Credentials", "filename": ".github/workflows/go-ci.yml", "hashed_secret": "0e4f7f719374de1c33da2c0e5c0db5bcdc2e15d4", "is_verified": false, - "line_number": 53 + "line_number": 61 } ], "api/openapi.yaml": [ @@ -818,5 +818,5 @@ } ] }, - "generated_at": "2026-09-19T22:42:43Z" + "generated_at": "2026-09-21T17:55:15Z" } diff --git a/packaging/tests/ci_gates_test.go b/packaging/tests/ci_gates_test.go index ea7f4e25..6db3ff53 100644 --- a/packaging/tests/ci_gates_test.go +++ b/packaging/tests/ci_gates_test.go @@ -206,11 +206,30 @@ func TestCIGates_WorkflowRunsAllGates(t *testing.T) { t.Errorf("workflow missing step that runs %q", g) } } - // The single race+coverage run must still detect data races AND - // emit JSON for specter ingest. + // The race+coverage run must still detect data races AND emit JSON + // for specter ingest. if !strings.Contains(wf, "go test -race") || !strings.Contains(wf, "-json") { t.Error("workflow missing the race+JSON test run (`go test -race ... -json`) — race detection must still gate") } + // v1.18.1: two invocations, every package once. The first excludes + // internal/server, the second runs it alone with its own budget, + // both streams are ingested and uploaded, and the job budget is + // explicit rather than the platform default. + if !strings.Contains(wf, "grep -v '^github.com/Hanalyx/openwatch/internal/server$'") { + t.Error("first go test invocation must exclude internal/server so the package runs once, in its own phase") + } + if !regexp.MustCompile(`go test -race -json -timeout \d+s ./internal/server/`).MatchString(wf) { + t.Error("internal/server must run in its own go test invocation with its own -timeout") + } + for _, want := range []string{ + "--go-test /tmp/go-test.json --go-test /tmp/go-test-server.json", + "/tmp/go-test-server.json\n", + "timeout-minutes:", + } { + if !strings.Contains(wf, want) { + t.Errorf("workflow missing %q: both JSON streams must be ingested and uploaded, and the job budget must be explicit", want) + } + } }) } diff --git a/specs/release/ci-gates.spec.yaml b/specs/release/ci-gates.spec.yaml index a29a10a3..dea36952 100644 --- a/specs/release/ci-gates.spec.yaml +++ b/specs/release/ci-gates.spec.yaml @@ -1,7 +1,7 @@ spec: id: release-ci-gates title: CI quality + security gates - version: "1.18.0" + version: "1.18.1" status: approved tier: 1 @@ -388,7 +388,22 @@ spec: priority: critical references_constraints: [C-06] - id: AC-09 - description: The workflow executes (in order) make vet, make lint, make vuln, a single race-detector + JSON test run (`go test -race -json`, which both detects data races and feeds specter ingest — replacing the former separate `make test-race` + non-race json passes), and specter sync — each as its own step so a failure pinpoints which gate broke. + description: > + The workflow executes (in order) make vet, make lint, make vuln, the + race-detector + JSON test run, and specter sync — each as its own step + so a failure pinpoints which gate broke. The test run is `go test -race + -json` (it both detects data races and feeds specter ingest, replacing + the former separate `make test-race` + non-race json passes). v1.18.1 + (CP bugs/OW-066) — it is two invocations inside one step, not one, so + that every package is still tested exactly once under -race: the first + covers every package except internal/server under the shared per-package + budget; the second runs internal/server alone under its own, so the + database-heavy suite no longer competes with sibling packages for the + PostgreSQL service and a hang elsewhere still fails at the shared + budget. Both exit statuses are preserved (a later success never masks an + earlier failure), both JSON streams are kept and uploaded on every + outcome, specter ingests both, and the job declares an explicit timeout- + minutes that covers both phases plus the artifact upload. priority: critical references_constraints: [C-06] - id: AC-10