diff --git a/.github/actions/ci-test-notify/README.md b/.github/actions/ci-test-notify/README.md index 38875bc..da88580 100644 --- a/.github/actions/ci-test-notify/README.md +++ b/.github/actions/ci-test-notify/README.md @@ -11,7 +11,7 @@ Replaces the nightly-specific `ci-notify-nightly-tests` action with a generic in | INPUT | TYPE | REQUIRED | DEFAULT | DESCRIPTION | |-------------|--------|----------|---------|---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| | details | string | false | | Markdown text appended after the build
URL (test results, versions, artifact links, etc.) | -| status | string | true | | Test status: success, failure, cancelled, or
skipped | +| status | string | true | | Run status, typically `needs..result` or `job.status`.
`success` and `failure` notify; `cancelled` and
`skipped` are treated as no-ops and
send nothing. | | test-name | string | true | | Test suite name for the header
(e.g. "E2E Ginkgo Nightly Tests"). Keep under ~130 chars —
Slack header blocks have a 150-char
limit and the status suffix takes
~15 chars. | | webhook-url | string | true | | Slack incoming webhook URL | @@ -73,6 +73,17 @@ Build URL: webhook-url: ${{ secrets.SLACK_WEBHOOK_URL_DEV_VCLUSTER }} ``` +## Notification gating + +The action only notifies on actionable outcomes. A `status` of `cancelled` or +`skipped` is treated as a no-op: the action logs a notice and sends nothing. +This means callers can pass `needs..result` or `job.status` straight +through without a guard. A cancelled run (aborted by a human or superseded) or a +skipped job never produces a Slack alert; only `success` and `failure` do. + +An empty `webhook-url` (fork PRs, where secrets are unavailable) also suppresses +the notification. + ## Permissions No special GitHub permissions required. The `webhook-url` must be supplied via a repository secret. diff --git a/.github/actions/ci-test-notify/action.yml b/.github/actions/ci-test-notify/action.yml index be4f4f2..4f40dd6 100644 --- a/.github/actions/ci-test-notify/action.yml +++ b/.github/actions/ci-test-notify/action.yml @@ -8,7 +8,7 @@ inputs: description: 'Test suite name for the header (e.g. "E2E Ginkgo Nightly Tests"). Keep under ~130 chars — Slack header blocks have a 150-char limit and the status suffix takes ~15 chars.' required: true status: - description: 'Test status: success, failure, cancelled, or skipped' + description: 'Run status, typically `needs..result` or `job.status`. `success` and `failure` notify; `cancelled` and `skipped` are treated as no-ops and send nothing.' required: true details: description: 'Markdown text appended after the build URL (test results, versions, artifact links, etc.)' @@ -21,14 +21,16 @@ inputs: runs: using: "composite" steps: - - name: Check webhook URL - if: inputs.webhook-url == '' + - name: Decide whether to notify + id: gate shell: bash - run: | - echo "::warning::webhook-url is empty (expected on fork PRs where secrets are unavailable), skipping notification" + env: + STATUS: ${{ inputs.status }} + WEBHOOK_URL: ${{ inputs.webhook-url }} + run: ${{ github.action_path }}/should-notify.sh - name: Build Slack payload - if: inputs.webhook-url != '' + if: steps.gate.outputs.notify == 'true' shell: bash env: TEST_NAME: ${{ inputs.test-name }} @@ -41,7 +43,7 @@ runs: run: ${{ github.action_path }}/build-payload.sh - name: Send Slack notification - if: inputs.webhook-url != '' + if: steps.gate.outputs.notify == 'true' uses: slackapi/slack-github-action@45a88b9581bfab2566dc881e2cd66d334e621e2c # v3.0.3 with: errors: true diff --git a/.github/actions/ci-test-notify/should-notify.sh b/.github/actions/ci-test-notify/should-notify.sh new file mode 100755 index 0000000..a229af8 --- /dev/null +++ b/.github/actions/ci-test-notify/should-notify.sh @@ -0,0 +1,29 @@ +#!/usr/bin/env bash +set -euo pipefail + +# Decides whether ci-test-notify should send a Slack message, writing +# `notify=true|false` to $GITHUB_OUTPUT for the composite action to gate on. +# +# Callers pass the run conclusion straight from `needs..result` or +# `job.status`, which can be success, failure, cancelled, or skipped. Only +# success and failure are actionable: a cancelled run was aborted by a human +# (or superseded), and a skipped job never executed. Neither warrants a Slack +# alert, so both are silenced here rather than in every caller. +# +# An empty webhook (fork PRs, where secrets are unavailable) also suppresses +# the notification, same as before. +# +# Required env vars: STATUS, GITHUB_OUTPUT +# Optional env vars: WEBHOOK_URL + +notify=true + +if [[ -z "${WEBHOOK_URL:-}" ]]; then + echo "::warning::webhook-url is empty (expected on fork PRs where secrets are unavailable), skipping notification" + notify=false +elif [[ "${STATUS:?STATUS is required}" == "cancelled" || "$STATUS" == "skipped" ]]; then + echo "::notice::status is '$STATUS' — only success and failure notify, skipping Slack notification" + notify=false +fi + +echo "notify=$notify" >> "${GITHUB_OUTPUT:?GITHUB_OUTPUT is required}" diff --git a/.github/actions/ci-test-notify/test/should-notify.bats b/.github/actions/ci-test-notify/test/should-notify.bats new file mode 100644 index 0000000..a306861 --- /dev/null +++ b/.github/actions/ci-test-notify/test/should-notify.bats @@ -0,0 +1,94 @@ +#!/usr/bin/env bats +# Tests for should-notify.sh — the gate that decides whether a Slack +# notification is sent. Cancelled and skipped runs must stay silent. + +SCRIPT="$BATS_TEST_DIRNAME/../should-notify.sh" + +setup() { + MOCK_DIR=$(mktemp -d) + export GITHUB_OUTPUT="$MOCK_DIR/output" + : > "$GITHUB_OUTPUT" + + # Sensible defaults; individual tests override STATUS / WEBHOOK_URL. + export STATUS="success" + export WEBHOOK_URL="https://hooks.slack.com/services/T000/B000/xxx" +} + +teardown() { + rm -rf "$MOCK_DIR" +} + +# Helper: read the notify value written to GITHUB_OUTPUT +notify_value() { + sed -n 's/^notify=//p' "$GITHUB_OUTPUT" +} + +# --- Statuses that should notify --- + +@test "success notifies" { + STATUS="success" run bash "$SCRIPT" + [ "$status" -eq 0 ] + [ "$(notify_value)" = "true" ] +} + +@test "failure notifies" { + STATUS="failure" run bash "$SCRIPT" + [ "$status" -eq 0 ] + [ "$(notify_value)" = "true" ] +} + +# --- Statuses that must stay silent (the bug this fixes) --- + +@test "cancelled does not notify" { + STATUS="cancelled" run bash "$SCRIPT" + [ "$status" -eq 0 ] + [ "$(notify_value)" = "false" ] +} + +@test "skipped does not notify" { + STATUS="skipped" run bash "$SCRIPT" + [ "$status" -eq 0 ] + [ "$(notify_value)" = "false" ] +} + +@test "cancelled emits an explanatory notice" { + STATUS="cancelled" run bash "$SCRIPT" + [ "$status" -eq 0 ] + [[ "$output" == *"::notice::"* ]] + [[ "$output" == *"cancelled"* ]] +} + +# --- Empty webhook (fork PRs) wins over an otherwise-notifying status --- + +@test "empty webhook does not notify even on failure" { + STATUS="failure" WEBHOOK_URL="" run bash "$SCRIPT" + [ "$status" -eq 0 ] + [ "$(notify_value)" = "false" ] + [[ "$output" == *"webhook-url is empty"* ]] +} + +@test "empty webhook does not notify on success" { + STATUS="success" WEBHOOK_URL="" run bash "$SCRIPT" + [ "$status" -eq 0 ] + [ "$(notify_value)" = "false" ] +} + +# --- Error handling --- + +@test "fails when STATUS is unset" { + unset STATUS + run bash "$SCRIPT" + [ "$status" -ne 0 ] +} + +@test "fails when GITHUB_OUTPUT is unset" { + unset GITHUB_OUTPUT + run bash "$SCRIPT" + [ "$status" -ne 0 ] +} + +@test "exactly one notify line is written" { + STATUS="success" run bash "$SCRIPT" + [ "$status" -eq 0 ] + [ "$(grep -c '^notify=' "$GITHUB_OUTPUT")" -eq 1 ] +} diff --git a/Makefile b/Makefile index 9717582..2e7f060 100644 --- a/Makefile +++ b/Makefile @@ -123,7 +123,7 @@ test-ai-step: ## run ai-step bats tests bats $(ACTIONS_DIR)/ai-step/test/*.bats test-ci-test-notify: ## run ci-test-notify bats tests - bats $(ACTIONS_DIR)/ci-test-notify/test/build-payload.bats + bats $(ACTIONS_DIR)/ci-test-notify/test test-publish-helm-chart: ## run publish-helm-chart bats tests (requires mikefarah/yq on PATH) bats $(ACTIONS_DIR)/publish-helm-chart/test/run.bats