Commit Graph

6 Commits

Author SHA1 Message Date
shankar0123 4bb7a748ac fix(deploy/test) + ci(guard): unblock deploy-vendor-e2e — encryption-key length
Two-part complete-path fix for the deploy-vendor-e2e failure that has
been firing since the ci-pipeline-cleanup Phase 5 matrix collapse
started actually booting the certctl-test-server:

    Failed to load configuration:
    CERTCTL_CONFIG_ENCRYPTION_KEY too short (29 bytes; minimum 32).

Surfaced via the diagnostic-dump step landed in commit 69266c8 — the
server panicked on startup, Docker restarted it endlessly, compose
reported the dependency-chain symptom ("container certctl-test-server
is unhealthy"), but the actual cause was invisible in the previous
CI output. With the dump in place, the next failing run named the
problem in one line.

Root cause. The H-1 audit-closure master commit 6cb4414
("feat(security): bodyLimit on noAuth + security headers + encryption-
key validation (H-1 master)") added internal/config/config.go's
minEncryptionKeyLength = 32 byte floor + 5 unit tests that pin it.
The closure was incomplete: it never enforced the rule against the
literal CERTCTL_CONFIG_ENCRYPTION_KEY values certctl's own
deploy/docker-compose*.yml files pass. Pre-Phase-5 the test stack
didn't fully exercise the validator (the per-vendor matrix didn't
boot certctl-test-server in every job), so the gap was silent.
deploy/docker-compose.test.yml's literal value
`test-encryption-key-32chars!!` was 29 bytes — the name claimed 32
but the author miscounted (4+1+10+1+3+1+2+5+2 = 29). Pattern matches
every fix in this CI-stabilization sequence: pre-existing latent bug
that the old CI structurally hid.

Part 1 — direct fix (deploy/docker-compose.test.yml):

  Replace the 29-byte literal with a clearly test-only,
  self-documenting 49-byte value (`test-encryption-key-deterministic-
  32-byte-fixture`). 17 bytes of safety margin so a future tightening
  of the floor (32 → 33+) doesn't break this fixture again. Inline
  comment block explains the byte-budget contract + points at the
  H-1 closure commit. Production deploy/docker-compose.yml's default
  (`change-me-32-char-encryption-key`) is exactly 32 bytes — passes
  by 1 byte but on the edge; not touched here because operators are
  already told to override it via env (`${VAR:-default}`).

Part 2 — structural fix (scripts/ci-guards/H-1-encryption-key-min-
length.sh):

  New regression guard. Scans every deploy/docker-compose*.yml for
  literal CERTCTL_CONFIG_ENCRYPTION_KEY values + values inside
  ${VAR:-default} expansions, checks each against the 32-byte floor,
  fails CI with `::error::` annotation pointing at the offending
  file:line if any literal regresses. Bare ${VAR} env references with
  no default are skipped — those are operator-supplied at runtime
  and the validator handles them at boot.

  Verified manually:
    - Clean repo: `H-1-encryption-key-min-length: clean.` (exit 0)
    - 5-byte regression: emits proper ::error:: annotation, exit 1
    - Restore: clean again (exit 0)

  CI auto-picks up the new guard via the `for g in
  scripts/ci-guards/*.sh; do bash "$g"; done` loop in ci.yml's
  Regression guards step (no ci.yml change required).

  scripts/ci-guards/README.md updated: 20 → 21 guards, new row
  explaining the closure rationale.

The structural piece is the more important half of this fix. The
direct fix unblocks today's CI; the guard prevents the same class of
drift from ever recurring silently. Future audit closures that add
new validation rules to internal/config/config.go now have a working
template for the matching CI guard — drop a sibling .sh in the
ci-guards directory.

Bonus — what the diagnostic-dump step (69266c8) bought us. Before
that step landed, the same failure looked like an opaque "container
unhealthy" with no actionable signal. With it, the actual error
message + the offending env var + the exact byte count came out in
one CI run. The diagnostic infrastructure paid for itself within one
push.
2026-05-01 00:57:43 +00:00
shankar0123 3f4f8bbe36 refactor(scripts): move CI helpers out of scripts/ci-guards/
The 'Regression guards' loop step in ci.yml runs:
    for g in scripts/ci-guards/*.sh; do bash "$g"; done

Per the directory's own contract (scripts/ci-guards/README.md), every
script there MUST be runnable bare with no args / no env. Three files
violated that contract — they're helpers consumed by specific CI job
steps with arguments, not regression guards. They were misplaced.

Moved (git mv):
  scripts/ci-guards/vendor-e2e-skip-check.sh         → scripts/
  scripts/ci-guards/vendor-e2e-skip-allowlist.txt    → scripts/
  scripts/ci-guards/coverage-pr-comment.sh           → scripts/

Updated ci.yml call sites:
  - deploy-vendor-e2e job: bash scripts/vendor-e2e-skip-check.sh $LOG
  - go-build-and-test job: bash scripts/coverage-pr-comment.sh

Tightened scripts/vendor-e2e-skip-check.sh arg parse from a silent
default ('LOG=${1:-test-output.log}') to a mandatory-arg form
('LOG=${1:?usage: ...}') so misuse fails loud at parse time rather
than at the missing-file check.

Updated scripts/ci-guards/README.md contract to spell out the
guard-vs-helper distinction explicitly; lists current helpers under
scripts/ for future-author guidance.

Verified locally: 'for g in scripts/ci-guards/*.sh; do bash $g; done'
returns clean (22 guards pass) on HEAD post-move.

Closes the regression-guards-loop failure that surfaced in CI run
25192163943 (job 73864471346 'Frontend Build').
2026-04-30 22:37:12 +00:00
shankar0123 3a69600c2c ci-pipeline-cleanup Phase 10: coverage PR-comment action
Bundle: ci-pipeline-cleanup, Phase 10 / frozen decision 0.9.

Self-hosted alternative to Codecov / Coveralls. Posts a per-package
coverage delta as a PR comment on every PR; updates the same comment
in place on subsequent pushes (avoids duplicate noise).

scripts/ci-guards/coverage-pr-comment.sh:
- Reads coverage.out from the prior Go Test step
- Builds per-package coverage table (mirrors check-coverage-thresholds
  averaging logic)
- Searches existing PR comments for the '**Coverage report' marker
  and PATCHes the existing one if found, else POSTs a new one
- No-op on non-PR builds (push to master, scheduled, etc.)

Wired into go-build-and-test job after 'Upload Coverage Report' step
with if: github.event_name == 'pull_request' guard.

Operator can swap to Codecov/Coveralls later by replacing this script
+ step with a third-party action — the YAML manifest at
.github/coverage-thresholds.yml stays unchanged either way.
2026-04-30 20:51:48 +00:00
shankar0123 19a5e438f2 ci-pipeline-cleanup Phases 7-9: image-and-supply-chain job
Bundle: ci-pipeline-cleanup, Phases 7-9 / frozen decisions 0.8 + 0.10 + 0.11.

NEW image-and-supply-chain job (Ubuntu, ~3 min). Three steps:

PHASE 7 — Digest validity
scripts/ci-guards/digest-validity.sh resolves every @sha256:<digest>
ref in deploy/**/*.{yml,Dockerfile*} against its registry. Closes the
H-001 lying-field gap that Bundle II hit (11 fabricated digests passed
H-001's regex-only check and failed docker pull in CI).
Sandbox verification: 16/16 digests in deploy/* + Dockerfiles all
return HTTP 200 from registry-1.docker.io / ghcr.io / mcr.microsoft.com.

PHASE 8 — Docker build smoke (all 4 Dockerfiles)
Per frozen decision 0.10: build Dockerfile, Dockerfile.agent,
deploy/test/f5-mock-icontrol/Dockerfile, deploy/test/libest/Dockerfile.
Catches syntax errors + COPY path drift before tag-time release.yml.
The test-sidecar Dockerfiles are load-bearing for vendor-e2e — a
syntax error there silently breaks the e2e suite.

PHASE 9 — OpenAPI ↔ handler operationId parity
scripts/ci-guards/openapi-handler-parity.sh extracts router routes
(r.mux.Handle / r.Register "METHOD /path" syntax — Go 1.22+ ServeMux),
extracts OpenAPI operations (paths × HTTP methods), and fails if any
router route has no operationId AND is not documented in the new
api/openapi-handler-exceptions.yaml.

Verified gap at HEAD 1de61e91 (root-caused):
  142 router routes, 136 OpenAPI operations
  6 router-only routes — all SCEP wire-protocol endpoints (RFC-shaped,
    not REST). Documented in api/openapi-handler-exceptions.yaml with
    one-line why: justifications.
  0 OpenAPI-only operations.

Going forward: any new gap fails the build unless documented.

Status checks per push: now 7 (was 8 after Phase 5+6 dropped windows;
this Phase adds 1 = +1 net). Final acceptance gate target.

ci.yml: 383 → 432 lines (+49 for the new job + steps).
2026-04-30 20:50:52 +00:00
shankar0123 6f6de639a0 ci-pipeline-cleanup Phase 5+6: collapse vendor matrix; delete Windows matrix
Bundle: ci-pipeline-cleanup, Phases 5+6 / frozen decisions 0.4 + 0.5
+ 0.6. Revises Bundle II decisions 0.4 (Windows matrix) and 0.9 (per-
vendor granularity).

PHASE 5 — Linux vendor matrix collapsed (12 jobs → 1):

The previous per-vendor matrix produced 12 status-check rows for
~1 real assertion (115/116 vendor-edge tests are t.Log placeholders
per Bundle II Phase 2-13 design). Granularity was fake signal.

Single-job version: brings up all 11 sidecars at once via
docker compose --profile deploy-e2e up -d, runs go test -run
'VendorEdge_' once, tears down once.

Critical caveat: requireSidecar() in deploy/test/vendor_e2e_helpers.go
uses t.Skipf() when a sidecar isn't reachable — silent test skip,
not CI failure. The new Skip-count enforcement step
(scripts/ci-guards/vendor-e2e-skip-check.sh) counts SKIP lines and
fails the build if it exceeds the allowlist at
scripts/ci-guards/vendor-e2e-skip-allowlist.txt (15 windows-iis-
requiring tests legitimately skip on Linux per Phase 6).

PHASE 6 — Windows matrix deleted entirely:

The deploy-vendor-e2e-windows job removed. Two reasons:
1. Can't physically work on windows-latest today (Docker not started
   in Windows-containers mode by default; bridge network driver
   missing on Windows Docker — see CI run 25183374742 failure logs).
2. Even fixed, validates nothing — all 16 IIS + WinCertStore tests
   are t.Log placeholders that exercise no IIS-specific behavior.

Per Bundle II frozen decision 0.14, the third criterion for
"verified" status in the vendor matrix is operator manual smoke
against a real instance. IIS + WinCertStore now satisfy that via
the playbook (Phase 6 follow-up adds docs/connector-iis.md::
Operator validation playbook).

The windows-iis-test sidecar STAYS in deploy/docker-compose.test.yml
under profiles: [deploy-e2e-windows] for operator local use. Linux
CI never activates this profile.

Operator-required action before merge: RAM headroom verification on
prototype branch (per frozen decision 0.14). If peak RSS > 12 GB on
ubuntu-latest with all 11 sidecars up, fall back to bucketed matrix
per cowork/ci-pipeline-cleanup/decisions-revised.md.

ci.yml: 417 → 383 lines (-34 net; -1105 cumulative since baseline 1488).
Status checks per push: 19 → 7 (collapse 12 vendor + 2 windows = -14;
add image-and-supply-chain in Phase 7-9 = +1; net 19-12-2+1 = ~7).

Operator action for Phase 13: update GitHub branch protection rules
(required-checks list 19 → 7 entries). Documented in cowork/
ci-pipeline-cleanup/decisions-revised.md.
2026-04-30 20:46:05 +00:00
shankar0123 5b7a022767 ci-pipeline-cleanup Phase 1: extract 20 regression guards to scripts/ci-guards/
Bundle: ci-pipeline-cleanup, Phase 1.

Pure relocation — no behavior change. Each guard's bash logic is
byte-identical to the prior inline version; the only changes are:
(a) the guard becomes a sibling script under scripts/ci-guards/<id>.sh,
(b) ci.yml's per-guard step is replaced by a single loop step that
iterates all scripts.

20 scripts extracted (alphabetized):
  B-1-orphan-crud.sh, D-1-D-2-statusbadge-phantom.sh,
  G-1-jwt-auth-literal.sh, G-2-api-key-hash-json.sh,
  G-3-env-docs-drift.sh, H-001-bare-from.sh, H-009-readme-jwt.sh,
  L-001-insecure-skip-verify.sh, L-1-bulk-action-loop.sh,
  M-012-no-root-user.sh, P-1-documented-orphan-fns.sh,
  S-1-hardcoded-source-counts.sh, S-2-strings-contains-err.sh,
  T-1-frontend-page-coverage.sh, U-2-plaintext-healthcheck.sh,
  U-3-migration-mount.sh, bundle-8-L-015-target-blank-rel-noopener.sh,
  bundle-8-L-019-dangerously-set-inner-html.sh,
  bundle-8-M-009-bare-usemutation.sh, test-naming-convention.sh

Plus scripts/ci-guards/README.md documenting the contract:
- Each script must exit 0 on clean repo, non-zero with ::error::
  prefix on regression
- Runnable from repo root via 'bash scripts/ci-guards/<id>.sh'
- Adding a new guard: drop a new <id>.sh; CI auto-picks it up

ci.yml dropped 1488 → 557 lines (-931, -63%).

Single CI loop step now collects ALL guard failures before failing
the build instead of fail-fast — UX win for regressions that hit
two guards at once.

Two guards (QA-doc Part-count + seed-count, ci.yml lines 868-917)
deliberately NOT extracted — they move to 'make verify-docs' in
Phase 11 because they protect docs-the-operator-reads, not the
product itself.

Verification (sandbox):
- All 20 scripts pass against HEAD (chmod +x; for g in scripts/ci-guards/*.sh; do bash $g; done)
- New ci.yml YAML-parses cleanly
- Job boundaries preserved: go-build-and-test, frontend-build,
  helm-lint, deploy-vendor-e2e, deploy-vendor-e2e-windows
- Loop step appears twice (once at end of go-build-and-test, once
  at end of frontend-build) so both jobs continue running their
  set of guards
2026-04-30 20:36:26 +00:00