shoc-backend/QUALITY_GATES.md

158 lines
8.3 KiB
Markdown
Raw Normal View History

# Quality Gates (canonical)
> **Status: canonical.** This document owns *how* the architecture and
> code-quality rules are enforced: the gate list, the local command, the CI
> mapping, and the pass/fail/skip reporting semantics. The *meaning* of each
> rule lives in `ARCHITECTURE_AND_CODE_QUALITY.md`.
>
> **CI and local run the same check.** Every gate below is executed by
> `scripts/governance-check.sh`. The `architecture-quality` workflow calls that
> script so a green run means the same thing locally and in CI. Do not add a
> check to CI that is not also runnable locally through the script (and vice
> versa).
## Gate inventory
| # | Gate | Protects (architecture §) | Local command | CI step |
|---|------|---------------------------|---------------|---------|
| G1 | Restore | build integrity | `dotnet restore SeaHavenIndustries.sln` | `architecture-quality` → `governance-check.sh` |
| G2 | Architecture boundary tests | §1, §3 (dependency direction) | `dotnet test Api.SeaHavenIndustries.Tests ... --filter FullyQualifiedName~ArchitectureTests --no-restore` | same script |
| G3 | Changed-file formatting | §1 (conventions) | `dotnet format SeaHavenIndustries.sln --no-restore --verify-no-changes --include <changed .cs>` | same script |
| G4 | Release build | compile correctness | `dotnet build SeaHavenIndustries.sln -c Release --no-restore` | `architecture-quality` and `ci` |
| G5 | Full test suite | behavior | `dotnet test SeaHavenIndustries.sln -c Release --no-build` | `architecture-quality` and `ci` |
| G6 | Migration gates | §7 | see §"Migration gates" below | on-demand / release |
| G7 | Cancellation forwarding | §6 | behavior tests on changed I/O paths + analyzer | review-enforced on changed paths |
| G8 | Error disclosure | §5 | `SanitizedErrorsTests` (part of G5) | `ci` |
| G9 | Board-backed regression | review framework | `REVIEW_AND_PR_FRAMEWORK.md` inventory | review-enforced |
| G10 | Terraform import plan safety | live infrastructure adoption | `python scripts/test-terraform-import-plan-check.py` | `architecture-quality` → `governance-check.sh` |
| G11 | Terraform static validation | import configuration integrity | commands below | `ci-terraform` on PRs to `main` or `dev` |
| G13 | App/Terraform isolation | separate delivery lanes | `python3 scripts/check_app_terraform_isolation.py` | `architecture-quality` → `governance-check.sh` |
## How to run locally
```bash
# Complete local repository gate (mirrors the architecture-quality workflow):
bash scripts/governance-check.sh
# By default it compares against the default branch for changed-file formatting.
# Override the comparison point:
BASE_REF=origin/main bash scripts/governance-check.sh
BASE_REF=main bash scripts/governance-check.sh
```
The script:
1. `dotnet restore` (G1)
2. runs the `ArchitectureTests` filter with `--no-restore` (G2)
3. computes changed `.cs` files vs `BASE_REF` (default `origin/main`) and runs
`dotnet format --verify-no-changes --include ...` (G3). When there are no
changed C# files it skips G3 with an explicit "skipped: no changed C#" line.
4. builds the complete solution in Release with no restore (G4).
5. runs the complete solution test suite in Release with no rebuild (G5).
6. verifies that the Terraform plan guard rejects create, delete, replacement,
unmanaged resource types, and updates not allowlisted by exact address (G10).
7. runs the release-tag, commit-check, and isolation unit tests.
8. rejects a diff that contains both `terraform/` and deployable application
files (G13).
G10 permits only exact approved resource address/type pairs for the
environment-owned boundary: Elastic
Beanstalk environment, IAM role/inline policy/managed-policy attachment/
instance profile, Secrets Manager secret metadata, and Route 53 record.
SSM `/shoc-backend/<env>/deploy/*` parameters are created after adoption and
are not part of the import allowlist. Initial mode permits no update.
Controlled mode requires one
`--allow-update-address` argument per reviewed in-place update. Every invocation
also requires `--environment dev` or `--environment staging`; an empty or
incomplete environment plan fails.
G11 runs `terraform fmt -check -recursive terraform`, `terraform init -backend=false`,
and `terraform validate` for both `live/dev` and `live/staging` on every PR to
`main` or `dev`. Org-baseline CloudFormation owns the HCP role substrate, and Terraform
owns the environment GitHub deploy roles, so no backend CDK or bootstrap root
remains in the matrix. HCP plan/apply roles stay in org-baseline; this
repository never manages `hcptf-*` roles.
The former G12 version-only HCP apply guard is not part of the repository gate.
G13 fails when the same diff contains both `terraform/` and deployable
application files. Workflow, documentation, and gate-script changes may share
a PR with either side.
## Migration gates (G6)
Provider-real, not reflection-derived. Run against the configured provider:
- **Actual-provider discovery** — confirm the runtime provider matches the
configured connection (e.g., SQL Server / Npgsql) from configuration, not
from package guesses.
- **Snapshot consistency** — `dotnet ef migrations script --idempotent` (or a
model diff) shows no phantom operations; `ModelSnapshot` matches the model.
- **Empty-db apply** — `dotnet ef database update` against a fresh empty
database applies the full history cleanly.
- **Supported-prior upgrade** — upgrade from each still-supported prior
migration applies cleanly.
- **Rollback** — report **not guaranteed** unless a real downgrade was
exercised against the provider. Never claim "EF can roll it back" untested.
Migration gates are run on-demand and gated for releases; they are not in the
per-PR fast path because they require a database. A PR that adds/changes a
migration must document the G6 evidence in the PR description.
## Cancellation forwarding (G7)
For changed async I/O paths, the requirement (§6) is **verified forwarding**,
not a signature checkbox. Acceptable evidence, any one of:
- a behavior test asserting the `CancellationToken` reaches the data service /
EF call; or
- an analyzer diagnostic confirming propagation on the changed method.
A parameter that exists but is swallowed/`default`-ed is a **fail**. If an
analyzer is unavailable for a path, a behavior test is required; silence is not
acceptable. Review enforces this on changed paths.
## Architecture discovery must fail on zero (G2 invariants)
Each reflection invariant in `ArchitectureTests` asserts the discovered set is
non-empty. A future namespace move that makes reflection return zero types will
**fail the gate** rather than silently pass. The current invariants:
1. Controllers inject neither `DbContext`, concrete data services, nor concrete
service implementations.
2. Business services inject neither `DbContext` nor `IConfiguration`.
3. Business services depend on interfaces (not concrete service/data-service
implementations) for their data and persistence dependencies.
## What this does not claim
- Reflection over constructors proves **wiring shape**, not runtime behavior.
Runtime/semantic properties (cancellation actually cancelling, a query
executing in SQL, a transaction actually committing atomically) are proven by
behavior tests or provider evidence, never by reflection.
- A passing script proves G1–G5 ran in sequence. It does not prove the
provider-backed or review-owned gates G6–G9.
## Pass / fail / skip / not-run reporting
Every gate result is reported as exactly one of:
- **pass** — ran and succeeded, with evidence.
- **fail** — ran and failed; cite the failing test/command and output.
- **skipped** — intentionally not run for this change (e.g., no changed C# for
G3); name the gate and the reason.
- **not-run** — could not run (environment/dependency missing); name the
blocker.
A PR is not "green" if any applicable gate is fail, skipped-without-justification,
or not-run. Never infer a pass from silence.
## Adding or changing a gate
- Any new executable check goes into `scripts/governance-check.sh` **and** the
`architecture-quality` workflow together (or into the full CI for G4/G5-type
checks). One source of truth for local and CI.
- A new rule documents its meaning in `ARCHITECTURE_AND_CODE_QUALITY.md` and its
command/mapping here.
- Explicit, documented exceptions only — no wildcard suppressions (see
`ARCHITECTURE_AND_CODE_QUALITY.md` §10).