mirror of
https://github.com/shankar0123/certctl.git
synced 2026-06-09 05:08:55 +00:00
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').
This commit is contained in:
shankar0123
@@ -16,8 +16,11 @@ Every script in this directory MUST:
|
||||
1. Be exit-code 0 on a clean repo (no regression present).
|
||||
2. Be exit-code non-zero on regression, with a `::error::` annotation
|
||||
prefix so PR reviewers see the failing line in the GitHub Actions UI.
|
||||
3. Be runnable from repo root via `bash scripts/ci-guards/<id>.sh` —
|
||||
no implicit `cd` requirement, no env-var requirement.
|
||||
3. **Be runnable from repo root via `bash scripts/ci-guards/<id>.sh`
|
||||
with NO arguments and NO env-var requirements.** The CI loop step
|
||||
(`for g in scripts/ci-guards/*.sh; do bash "$g"; done`) iterates
|
||||
every `.sh` here without args; any script that requires an arg or
|
||||
env var WILL fail in that loop.
|
||||
4. Carry a head-comment block matching the in-source justification
|
||||
from the original ci.yml entry: the audit-finding reference, the
|
||||
closure rationale, the exempt-surface list (if any).
|
||||
@@ -25,6 +28,22 @@ Every script in this directory MUST:
|
||||
6. Produce no output on the happy path beyond a final
|
||||
`echo "<id>: clean."` confirmation line.
|
||||
|
||||
### Helpers vs guards
|
||||
|
||||
Scripts that consume input artifacts (a test-output log, a
|
||||
`coverage.out` file) or env vars (`PR_NUMBER`, `GH_TOKEN`) are
|
||||
HELPERS, not guards. They live in `scripts/`, NOT `scripts/ci-guards/`.
|
||||
|
||||
Current helpers:
|
||||
- `scripts/vendor-e2e-skip-check.sh` — consumes `test-output.log`
|
||||
arg from the deploy-vendor-e2e job
|
||||
- `scripts/coverage-pr-comment.sh` — consumes `coverage.out` +
|
||||
`PR_NUMBER` + `GH_TOKEN` env from the go-build-and-test job
|
||||
- `scripts/check-coverage-thresholds.sh` — consumes `coverage.out`
|
||||
+ `.github/coverage-thresholds.yml`
|
||||
- `scripts/qa-doc-part-count.sh` + `scripts/qa-doc-seed-count.sh` —
|
||||
invoked via `make verify-docs` pre-tag, not in CI
|
||||
|
||||
## Adding a new guard
|
||||
|
||||
1. Drop a new `<id>.sh` in this directory with the head-comment block
|
||||
|
||||
@@ -1,90 +0,0 @@
|
||||
#!/usr/bin/env bash
|
||||
# scripts/ci-guards/coverage-pr-comment.sh
|
||||
#
|
||||
# Post a per-package coverage table as a PR comment on every PR.
|
||||
# Self-hosted alternative to Codecov / Coveralls (per ci-pipeline-cleanup
|
||||
# bundle Phase 10 / frozen decision 0.9).
|
||||
#
|
||||
# Reads coverage.out from the Go Test step. Updates an existing comment
|
||||
# in place if one already exists (avoids duplicate noise on subsequent
|
||||
# pushes to the same PR).
|
||||
#
|
||||
# Required env:
|
||||
# GH_TOKEN — secrets.GITHUB_TOKEN
|
||||
# PR_NUMBER — github.event.number
|
||||
# GITHUB_REPOSITORY — github.repository (owner/name)
|
||||
|
||||
set -e
|
||||
|
||||
if [ -z "$PR_NUMBER" ]; then
|
||||
echo "PR_NUMBER not set — not a PR build, skipping coverage comment."
|
||||
exit 0
|
||||
fi
|
||||
if [ -z "$GH_TOKEN" ]; then
|
||||
echo "::warning::GH_TOKEN not set — cannot post coverage comment"
|
||||
exit 0
|
||||
fi
|
||||
if [ ! -f coverage.out ]; then
|
||||
echo "::warning::coverage.out not found — skipping coverage comment"
|
||||
exit 0
|
||||
fi
|
||||
|
||||
# Build per-package summary table (mirrors check-coverage-thresholds.sh logic).
|
||||
table=$(go tool cover -func=coverage.out | awk '
|
||||
/internal\// {
|
||||
pkg = $1
|
||||
sub(/\/[^\/]+\.go:.*$/, "", pkg)
|
||||
cov = $NF
|
||||
sub(/%/, "", cov)
|
||||
sum[pkg] += cov + 0
|
||||
n[pkg]++
|
||||
}
|
||||
END {
|
||||
for (pkg in sum) printf "| `%s` | %.1f%% |\n", pkg, sum[pkg] / n[pkg]
|
||||
}
|
||||
' | sort)
|
||||
|
||||
total=$(go tool cover -func=coverage.out | tail -1 | awk '{print $NF}')
|
||||
|
||||
body="**Coverage report (HEAD)**
|
||||
|
||||
| package | coverage |
|
||||
|---|---:|
|
||||
| **TOTAL** | **${total}** |
|
||||
${table}
|
||||
|
||||
_Per-package floors enforced by \`scripts/check-coverage-thresholds.sh\`._
|
||||
_Generated by \`scripts/ci-guards/coverage-pr-comment.sh\` (ci-pipeline-cleanup Phase 10)._"
|
||||
|
||||
# Find existing comment created by this script (starts with the marker).
|
||||
api="https://api.github.com/repos/${GITHUB_REPOSITORY}"
|
||||
existing_id=$(curl -sS \
|
||||
-H "Authorization: Bearer $GH_TOKEN" \
|
||||
-H "Accept: application/vnd.github+json" \
|
||||
"$api/issues/$PR_NUMBER/comments?per_page=100" \
|
||||
| python3 -c "
|
||||
import sys, json
|
||||
comments = json.load(sys.stdin)
|
||||
for c in comments:
|
||||
if c['body'].startswith('**Coverage report'):
|
||||
print(c['id'])
|
||||
break
|
||||
")
|
||||
|
||||
if [ -n "$existing_id" ]; then
|
||||
curl -sS -X PATCH \
|
||||
-H "Authorization: Bearer $GH_TOKEN" \
|
||||
-H "Accept: application/vnd.github+json" \
|
||||
"$api/issues/comments/$existing_id" \
|
||||
-d "$(python3 -c "import json,sys; print(json.dumps({'body': open('/dev/stdin').read()}))" <<< "$body")" \
|
||||
> /dev/null
|
||||
echo "Updated existing coverage comment #$existing_id on PR #$PR_NUMBER"
|
||||
else
|
||||
curl -sS -X POST \
|
||||
-H "Authorization: Bearer $GH_TOKEN" \
|
||||
-H "Accept: application/vnd.github+json" \
|
||||
"$api/issues/$PR_NUMBER/comments" \
|
||||
-d "$(python3 -c "import json,sys; print(json.dumps({'body': open('/dev/stdin').read()}))" <<< "$body")" \
|
||||
> /dev/null
|
||||
echo "Created new coverage comment on PR #$PR_NUMBER"
|
||||
fi
|
||||
@@ -1,40 +0,0 @@
|
||||
# scripts/ci-guards/vendor-e2e-skip-allowlist.txt
|
||||
#
|
||||
# Test names that are EXPECTED to skip on Linux ubuntu-latest CI runners.
|
||||
# Each entry: one Go test function name per line. Lines starting with `#`
|
||||
# are comments / ignored. Blank lines ignored.
|
||||
#
|
||||
# Per ci-pipeline-cleanup bundle Phase 5 / frozen decision 0.6.
|
||||
# The skip-detection guard (in the deploy-vendor-e2e job) counts
|
||||
# `^--- SKIP:` lines from the test output and fails the build if it
|
||||
# exceeds the count of unique entries in this allowlist.
|
||||
#
|
||||
# When a sidecar fails to start, the affected tests' requireSidecar() call
|
||||
# triggers t.Skipf() — those skips are NOT in this allowlist and surface
|
||||
# as a build failure.
|
||||
|
||||
# Windows-only tests that legitimately skip on Linux because the
|
||||
# windows-iis-test sidecar is gated by `profiles: [deploy-e2e-windows]`
|
||||
# and CI runs only the `deploy-e2e` profile (per ci-pipeline-cleanup
|
||||
# Phase 6 frozen decision 0.5 — Windows matrix deletion). Operators
|
||||
# validate these via `docs/connector-iis.md::Operator validation playbook`
|
||||
# on a real Windows host.
|
||||
|
||||
# IIS connector (10 tests; require windows-iis sidecar)
|
||||
TestVendorEdge_IIS_ARRReverseProxyCertRotation_E2E
|
||||
TestVendorEdge_IIS_AppPoolRecycle_OptInForCertChange_E2E
|
||||
TestVendorEdge_IIS_BindingTypeHttpsValidated_E2E
|
||||
TestVendorEdge_IIS_CCSCentralizedCertStoreVariant_DeployToSharedStore_E2E
|
||||
TestVendorEdge_IIS_FriendlyNameUpdatedOnRotation_E2E
|
||||
TestVendorEdge_IIS_HTTP2ALPNPreserved_E2E
|
||||
TestVendorEdge_IIS_RemovePreviousBindingOnRotate_E2E
|
||||
TestVendorEdge_IIS_SNIMultiBindingPerSite_DeployUpdatesCorrectBinding_E2E
|
||||
TestVendorEdge_IIS_WinRMRemotePath_vs_LocalPowerShellPath_BothWork_E2E
|
||||
|
||||
# WinCertStore connector (6 tests; require windows-iis sidecar)
|
||||
TestVendorEdge_WinCertStore_CertStoreACL_IISIUSRSAccess_E2E
|
||||
TestVendorEdge_WinCertStore_CertStoreACL_NetworkServiceAccess_E2E
|
||||
TestVendorEdge_WinCertStore_PrivateKeyExportableFlag_E2E
|
||||
TestVendorEdge_WinCertStore_RemovePreviousThumbprintOnRotate_E2E
|
||||
TestVendorEdge_WinCertStore_StoreLocationLocalMachineVsCurrentUser_E2E
|
||||
TestVendorEdge_WinCertStore_ThumbprintBindingVsFriendlyNameBinding_E2E
|
||||
@@ -1,65 +0,0 @@
|
||||
#!/usr/bin/env bash
|
||||
# scripts/ci-guards/vendor-e2e-skip-check.sh
|
||||
#
|
||||
# Counts `^--- SKIP:` lines in the vendor-e2e test output and fails
|
||||
# the build if any test skipped that's NOT in the allowlist at
|
||||
# scripts/ci-guards/vendor-e2e-skip-allowlist.txt.
|
||||
#
|
||||
# Per ci-pipeline-cleanup bundle Phase 5 / frozen decision 0.6.
|
||||
# requireSidecar() in deploy/test/vendor_e2e_helpers.go uses
|
||||
# t.Skipf() when a sidecar isn't reachable. The collapsed
|
||||
# deploy-vendor-e2e job brings up all 11 sidecars at once — if
|
||||
# one fails to start, the affected tests skip silently. This
|
||||
# guard catches that.
|
||||
#
|
||||
# Usage: bash scripts/ci-guards/vendor-e2e-skip-check.sh <test-output.log>
|
||||
|
||||
set -e
|
||||
|
||||
LOG="${1:-test-output.log}"
|
||||
ALLOWLIST="scripts/ci-guards/vendor-e2e-skip-allowlist.txt"
|
||||
|
||||
if [ ! -f "$LOG" ]; then
|
||||
echo "::error::test output log not found: $LOG"
|
||||
exit 1
|
||||
fi
|
||||
|
||||
if [ ! -f "$ALLOWLIST" ]; then
|
||||
echo "::error::skip allowlist not found: $ALLOWLIST"
|
||||
exit 1
|
||||
fi
|
||||
|
||||
# Build the set of allowed-skip test names (strip comments + blanks).
|
||||
allowed=$(grep -vE '^\s*(#|$)' "$ALLOWLIST" | sort -u)
|
||||
allowed_count=$(echo "$allowed" | grep -c .)
|
||||
|
||||
# Extract skipped test names from `--- SKIP: TestName (0.00s)` style lines.
|
||||
skipped=$(grep -E '^--- SKIP: ' "$LOG" | awk '{print $3}' | sort -u || true)
|
||||
skipped_count=$(echo "$skipped" | grep -c . || true)
|
||||
|
||||
echo "Vendor-e2e skip-check:"
|
||||
echo " allowlist size: $allowed_count"
|
||||
echo " observed skips: $skipped_count"
|
||||
|
||||
# Find skips not in allowlist.
|
||||
unexpected=$(comm -23 <(echo "$skipped") <(echo "$allowed") || true)
|
||||
if [ -n "$unexpected" ]; then
|
||||
echo "::error::Unexpected test skips — a sidecar likely failed to start"
|
||||
echo "Unexpected skipped tests (not in $ALLOWLIST):"
|
||||
echo "$unexpected" | sed 's/^/ - /'
|
||||
echo ""
|
||||
echo "Either:"
|
||||
echo " (a) Fix the sidecar / network / docker-compose issue causing the skip, OR"
|
||||
echo " (b) If the skip is legitimate (e.g., a new Windows-only test added),"
|
||||
echo " add the test name to $ALLOWLIST with a one-line justification comment."
|
||||
exit 1
|
||||
fi
|
||||
|
||||
# Also flag skips beyond the allowlist count (defensive — comm -23 catches
|
||||
# this already but the explicit count check makes the error message clearer).
|
||||
if [ "$skipped_count" -gt "$allowed_count" ]; then
|
||||
echo "::error::Skip count $skipped_count exceeds allowlist size $allowed_count"
|
||||
exit 1
|
||||
fi
|
||||
|
||||
echo "vendor-e2e-skip-check: clean ($skipped_count skips ≤ $allowed_count allowed)."
|
||||
Reference in New Issue
Block a user