Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
114 changes: 111 additions & 3 deletions .github/workflows/go-pr-analysis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,14 @@ on:
description: 'Number of times to run tests for determinism check'
type: number
default: 3
enable_custom_checks:
description: 'Enable arbitrary caller-owned checks beyond the named gates above (runs each target in custom_checks). Mirrors the same input in frontend-pr-analysis.yml.'
type: boolean
default: false
custom_checks:
description: 'Newline-separated Makefile targets to run as additional checks (e.g. repo-specific static guards). Each runs independently via `make <target>`; a non-zero exit on any of them fails the job. Use for gates the named jobs do not cover, so a repo does not need its own parallel workflow file.'
type: string
default: ''
system_packages:
description: 'Space-separated list of apt packages to install before running Go commands (e.g., "libxml2-dev pkg-config"). Required for CGO repositories that depend on native system libraries.'
type: string
Expand Down Expand Up @@ -907,6 +915,106 @@ jobs:
working-directory: ${{ matrix.app.working_dir }}
run: ${{ inputs.integration_test_command }}

# ============================================
# CUSTOM CHECKS
# ============================================
custom-checks:
name: Custom Checks (${{ matrix.app.name }})
needs: detect-changes
# Deliberately NOT also gated on `custom_checks != ''`. A caller that enables
# the gate but leaves the list empty is misconfigured, and skipping the job
# silently would report coverage that never happened — the run step warns
# instead.
if: needs.detect-changes.outputs.has_changes == 'true' && inputs.enable_custom_checks
runs-on: ${{ vars.GENERAL_RUNNERS || inputs.runner_type }}
# This is the only job that runs caller-supplied commands, so it is the one
# place an unbounded hang is plausible. Caps the 6h GitHub default while
# staying generous enough for a heavy guard.
timeout-minutes: 30
strategy:
fail-fast: false
matrix:
app: ${{ fromJson(needs.detect-changes.outputs.matrix) }}
steps:
- name: Checkout code
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v6
with:
# Caller-supplied make targets run in this workspace; do not leave a
# usable git credential behind for them. Private-module access comes
# from the explicit git config step below, not from checkout.
persist-credentials: false

- name: Install system dependencies
if: inputs.system_packages != ''
env:
SYSTEM_PACKAGES: ${{ inputs.system_packages }}
run: |
mapfile -t PACKAGES < <(printf '%s' "$SYSTEM_PACKAGES" | tr -s '[:space:]' '\n' | grep -v '^[[:space:]]*$')
if [[ ${#PACKAGES[@]} -eq 0 ]]; then
echo "::notice::No valid package names in system_packages; skipping."
exit 0
fi
for pkg in "${PACKAGES[@]}"; do
if [[ "$pkg" == -* ]] || [[ ! "$pkg" =~ ^[A-Za-z0-9][A-Za-z0-9.+:-]*$ ]]; then
echo "::error::Invalid apt package token: '$pkg'"
exit 1
fi
done
sudo DEBIAN_FRONTEND=noninteractive apt-get update -qq
sudo DEBIAN_FRONTEND=noninteractive apt-get install -y --no-install-recommends "${PACKAGES[@]}"

- name: Setup Go
uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v6
with:
go-version: ${{ inputs.go_version }}
cache: true

# MANAGE_TOKEN goes through env rather than being interpolated into the
# script text: an interpolated secret is not masked if the shell traces the
# command, and it is what CodeQL/zizmor flag as template-injection.
- name: Configure private Go modules
if: inputs.go_private_modules != ''
run: |
git config --global url."https://${MANAGE_TOKEN}@github.com/".insteadOf "https://github.com/"
env:
GOPRIVATE: ${{ inputs.go_private_modules }}
MANAGE_TOKEN: ${{ secrets.MANAGE_TOKEN }}

# Every target runs even if an earlier one fails, so one PR surfaces every
# broken guard instead of only the first. CUSTOM_CHECKS is passed via env
# (not interpolated into the script) to avoid code injection from inputs.
- name: Run custom checks
working-directory: ${{ matrix.app.working_dir }}
env:
CUSTOM_CHECKS: ${{ inputs.custom_checks }}
run: |
if [ -z "${CUSTOM_CHECKS//[[:space:]]/}" ]; then
echo "::warning::enable_custom_checks is true but custom_checks is empty — no checks ran"
exit 0
fi
set +e
failures=0
while IFS= read -r target; do
[ -z "$target" ] && continue
if [[ ! "$target" =~ ^[A-Za-z0-9][A-Za-z0-9._-]*$ ]]; then
echo "::error::Invalid Makefile target name: '$target'"
failures=$((failures + 1))
continue
fi
echo "::group::make $target"
make "$target"
status=$?
echo "::endgroup::"
if [ "$status" -ne 0 ]; then
echo "::error::Custom check '$target' failed (exit $status)"
failures=$((failures + 1))
fi
done <<< "$CUSTOM_CHECKS"
if [ "$failures" -ne 0 ]; then
echo "::error::$failures custom check(s) failed"
exit 1
fi

Comment thread
coderabbitai[bot] marked this conversation as resolved.
# ============================================
# TEST DETERMINISM
# ============================================
Expand Down Expand Up @@ -996,12 +1104,12 @@ jobs:
# ============================================
notify:
name: Notify
needs: [detect-changes, lint, security, tests, build, integration-tests, test-determinism]
needs: [detect-changes, lint, security, tests, build, integration-tests, custom-checks, test-determinism]
if: always() && needs.detect-changes.outputs.has_changes == 'true'
uses: ./.github/workflows/slack-notify.yml
with:
status: ${{ (needs.lint.result == 'failure' || needs.security.result == 'failure' || needs.tests.result == 'failure' || needs.build.result == 'failure' || needs.integration-tests.result == 'failure' || needs.test-determinism.result == 'failure') && 'failure' || 'success' }}
status: ${{ (needs.lint.result == 'failure' || needs.security.result == 'failure' || needs.tests.result == 'failure' || needs.build.result == 'failure' || needs.integration-tests.result == 'failure' || needs.custom-checks.result == 'failure' || needs.test-determinism.result == 'failure') && 'failure' || 'success' }}
workflow_name: "Go PR Analysis"
failed_jobs: ${{ needs.lint.result == 'failure' && 'Lint, ' || '' }}${{ needs.security.result == 'failure' && 'Security, ' || '' }}${{ needs.tests.result == 'failure' && 'Tests, ' || '' }}${{ needs.build.result == 'failure' && 'Build, ' || '' }}${{ needs.integration-tests.result == 'failure' && 'Integration Tests, ' || '' }}${{ needs.test-determinism.result == 'failure' && 'Test Determinism' || '' }}
failed_jobs: ${{ needs.lint.result == 'failure' && 'Lint, ' || '' }}${{ needs.security.result == 'failure' && 'Security, ' || '' }}${{ needs.tests.result == 'failure' && 'Tests, ' || '' }}${{ needs.build.result == 'failure' && 'Build, ' || '' }}${{ needs.integration-tests.result == 'failure' && 'Integration Tests, ' || '' }}${{ needs.custom-checks.result == 'failure' && 'Custom Checks, ' || '' }}${{ needs.test-determinism.result == 'failure' && 'Test Determinism' || '' }}
secrets:
SLACK_WEBHOOK_URL: ${{ secrets.SLACK_WEBHOOK_URL }}
37 changes: 36 additions & 1 deletion .github/workflows/go-pr-validation.yml
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,26 @@ on:
description: 'Enable integration tests (requires make test-integration target)'
type: boolean
default: false
integration_test_command:
description: 'Command to run integration tests. Empty = go-pr-analysis default (`make test-integration`).'
type: string
default: ''
enable_test_determinism:
description: 'Enable the test determinism check (runs tests multiple times with shuffle)'
type: boolean
default: false
test_determinism_runs:
description: 'Number of times to run tests for the determinism check'
type: number
default: 3
enable_custom_checks:
description: 'Enable arbitrary caller-owned checks beyond the named gates (runs each target in custom_checks)'
type: boolean
default: false
custom_checks:
description: 'Newline-separated Makefile targets to run as additional checks. Each runs via `make <target>`; a non-zero exit on any of them fails the job. Use for repo-specific guards the named jobs do not cover.'
type: string
default: ''
system_packages:
description: 'Space-separated apt packages to install before Go commands (CGO repos)'
type: string
Expand Down Expand Up @@ -217,9 +237,13 @@ jobs:
secrets: inherit

# ----------------- Change Detection (gate) -----------------
# `edited` must stay in this list. When this job is skipped, the *-gate
# aggregators still run (if: always()) and receive result == 'skipped', which
# result-gate treats as a pass — so a run that evaluated nothing reports the
# required checks green. Matches js-pr-validation.yml, which already includes it.
changes:
name: Detect non-doc changes
if: contains(fromJSON('["opened","synchronize","reopened","ready_for_review"]'), github.event.action)
if: contains(fromJSON('["opened","edited","synchronize","reopened","ready_for_review"]'), github.event.action)
runs-on: ${{ vars.GENERAL_RUNNERS || inputs.runner_type }}
permissions:
contents: read
Expand Down Expand Up @@ -252,6 +276,17 @@ jobs:
fail_on_coverage_threshold: ${{ inputs.fail_on_coverage_threshold }}
go_private_modules: ${{ inputs.go_private_modules }}
enable_integration_tests: ${{ inputs.enable_integration_tests }}
# Intentional duplicate of go-pr-analysis's own default — KEEP IN SYNC.
# A reusable-workflow default applies only when the input is omitted, and a
# `with:` mapping cannot conditionally omit a key: passing the empty string
# through would reach go-pr-analysis as '' and produce an empty `run:` in its
# integration-tests job, which is a workflow validation error. So the literal
# has to be repeated here rather than deferred to.
integration_test_command: ${{ inputs.integration_test_command || 'make test-integration' }}
enable_test_determinism: ${{ inputs.enable_test_determinism }}
test_determinism_runs: ${{ inputs.test_determinism_runs }}
enable_custom_checks: ${{ inputs.enable_custom_checks }}
custom_checks: ${{ inputs.custom_checks }}
system_packages: ${{ inputs.system_packages }}
shared_paths: ${{ inputs.shared_paths }}
secrets: inherit
Expand Down
7 changes: 7 additions & 0 deletions docs/go-pr-validation.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ Umbrella reusable workflow for Go service repositories. A caller references this

The `go-analysis`, `security` and `lib-version` pipelines each have a `*-gate` aggregator job that exposes a single stable status-check name (`Go Analysis`, `Security`, `Lib Version`) for branch protection, regardless of the internal job names. All three are gated by the change detector, so documentation-only PRs skip them (and the aggregators still report success). If the change detector (`changes`) job itself fails, the aggregators propagate that failure instead of passing — so broken change detection cannot let the required checks go green.

> The aggregators run with `if: always()`, so they report on every run of this workflow. `result-gate` treats `skipped` as a pass, which is correct for the docs-only case (the detector ran and found nothing to analyse) but **not** for a run where the detector never ran at all — that would report the required checks green having evaluated nothing. The `changes` job must therefore stay reachable on every `pull_request` action type the caller listens to, `edited` included. A caller that adds an action type to its own `types:` list without it being covered here reintroduces the hole.

## Inputs

| Input | Description | Type | Default |
Expand Down Expand Up @@ -46,6 +48,11 @@ The `go-analysis`, `security` and `lib-version` pipelines each have a `*-gate` a
| `fail_on_coverage_threshold` | Fail when coverage is below threshold | boolean | `true` |
| `go_private_modules` | GOPRIVATE pattern for private modules | string | `''` |
| `enable_integration_tests` | Enable integration tests | boolean | `false` |
| `integration_test_command` | Command for the integration lane. Empty → `make test-integration` | string | `''` |
| `enable_test_determinism` | Enable the test determinism check (repeat runs with shuffle) | boolean | `false` |
| `test_determinism_runs` | Number of repeat runs for the determinism check | number | `3` |
| `enable_custom_checks` | Run arbitrary caller-owned Makefile targets as an extra gate | boolean | `false` |
| `custom_checks` | Newline-separated Makefile targets; each runs via `make <target>`, any non-zero exit fails the job | string | `''` |
| `system_packages` | apt packages to install for CGO repos | string | `''` |
| `ignore_file` | Path to Trivy ignore file | string | `''` |
| `enable_docker_scan` | Build and scan a Docker image with Trivy; set `false` for repos without a root Dockerfile (monorepos with Dockerfiles under `components/`/`cmd/`) | boolean | `true` |
Expand Down
Loading