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
11 changes: 10 additions & 1 deletion .github/workflows/sonarcloud.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,17 @@ on:

permissions: {}

# Scope the concurrency group to the commit SHA so every pushed commit gets its
# own slot (Fleet Monitor #445; mirrors the ci.yml #380 fix). A per-ref group
# with cancel-in-progress cancels an in-progress run for an earlier commit the
# moment a newer commit lands on the same ref, so quick-succession pushes leave
# earlier commits cancelled — the cancelled runs that inflated this workflow's
# instability metric. Including github.sha gives each commit a distinct slot, so
# no commit is cancelled by a later one. cancel-in-progress: true still cancels
# duplicate runs for the same ref + SHA, but cannot cancel runs for different
# commits because their groups are distinct.
concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
group: ${{ github.workflow }}-${{ github.ref }}-${{ github.sha }}
cancel-in-progress: true

jobs:
Expand Down
48 changes: 48 additions & 0 deletions scripts/test-sonarcloud-workflow.sh
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,54 @@ else
echo "PASS: all third-party actions are SHA-pinned in $WORKFLOW"
fi

# ── Check 5: the concurrency group is scoped to the commit SHA ─────────────
# Root cause of the Fleet Monitor warning (#445): a per-ref concurrency group
# with cancel-in-progress: true cancels an in-progress run for an earlier commit
# the moment a newer commit lands on the same ref, so quick-succession pushes
# leave earlier commits cancelled — inflating the workflow's instability metric.
# ci.yml already fixed this (Fleet Monitor #380) by including github.sha in the
# group, giving each commit its own slot. This check locks the same invariant
# here: the group must be keyed on github.sha and cancel-in-progress must be true
# (safe unconditionally once the group is SHA-scoped — it then only affects
# duplicate runs of the same ref + SHA).
group_sha_regex='\$\{\{[^}]*github\.sha[^}]*\}\}'
group=""
if ! group=$(yq '.concurrency.group' "$WORKFLOW" 2>/dev/null); then
echo "FAIL: yq failed to parse $WORKFLOW. Please check if it is valid YAML."
exit 1
fi
if [[ "$group" == "null" || -z "$group" ]]; then
{
echo "FAIL: no top-level 'concurrency.group' block in $WORKFLOW"
echo " Add a SHA-scoped group so quick-succession commits do not cancel each other."
} >&2
PASS=false
elif [[ ! "$group" =~ $group_sha_regex ]]; then
{
echo "FAIL: concurrency group ($group) is not scoped to github.sha in $WORKFLOW"
echo " A per-ref group with cancel-in-progress cancels in-progress runs for"
echo " earlier commits (Fleet Monitor #445). Include github.sha so each commit"
echo " gets its own slot, e.g.:"
echo " group: \${{ github.workflow }}-\${{ github.ref }}-\${{ github.sha }}"
} >&2
PASS=false
else
echo "PASS: concurrency group is scoped to github.sha in $WORKFLOW"
cancel=""
if ! cancel=$(yq '.concurrency.cancel-in-progress' "$WORKFLOW" 2>/dev/null); then
Comment thread
don-petry marked this conversation as resolved.
echo "FAIL: yq failed to parse $WORKFLOW. Please check if it is valid YAML."
exit 1
fi
if [[ "$cancel" != "true" ]]; then
{
echo "FAIL: 'concurrency.cancel-in-progress' (found: '$cancel') must be 'true' in $WORKFLOW"
} >&2
PASS=false
else
echo "PASS: 'concurrency.cancel-in-progress' is unconditionally true in $WORKFLOW"
fi
fi

echo ""
if [[ "$PASS" == "true" ]]; then
echo "All checks passed."
Expand Down
133 changes: 131 additions & 2 deletions scripts/test-sonarcloud-workflow.test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,10 @@ fi
trap 'rm -rf "$TMP"' EXIT

# ── Fixture fragments ──────────────────────────────────────────────────────
# A valid header shared by every fixture: top-level permissions, per-ref
# concurrency, and a checkout with fetch-depth: 0 (SHA-pinned).
# A valid header shared by every fixture: top-level permissions, SHA-scoped
# concurrency (Fleet Monitor #445 — each commit gets its own slot so
# quick-succession pushes do not cancel each other), and a checkout with
# fetch-depth: 0 (SHA-pinned).
write_header() {
local file="$1"
cat > "$file" <<'YAML'
Expand All @@ -53,6 +55,39 @@ on:
pull_request:
branches: [main]
permissions: {}
concurrency:
group: ${{ github.workflow }}-${{ github.ref }}-${{ github.sha }}
cancel-in-progress: true
jobs:
sonarcloud:
name: SonarCloud
runs-on: ubuntu-latest
permissions:
contents: read
pull-requests: read
env:
SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
steps:
- name: Checkout repository
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
with:
fetch-depth: 0
YAML
}

# A header whose concurrency group is keyed only on github.ref (no github.sha):
# a newer commit on the same ref cancels an in-progress run for an earlier
# commit — the exact cancelled-run flakiness this guard now prevents (#445).
write_header_per_ref() {
local file="$1"
cat > "$file" <<'YAML'
name: SonarCloud Analysis
on:
push:
branches: [main]
pull_request:
branches: [main]
permissions: {}
concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: true
Expand Down Expand Up @@ -218,6 +253,100 @@ else
pass "retry condition referencing an unrelated step ID is rejected"
fi

# ── Case 8: a per-ref-only concurrency group is rejected ───────────────────
perref="${TMP}/sonar-per-ref.yml"
write_header_per_ref "$perref"
append_good_scan "$perref"
if run_guard "$perref"; then
fail "a per-ref-only concurrency group (no github.sha) should be REJECTED"
else
pass "a per-ref-only concurrency group (no github.sha) is rejected"
fi

# ── Case 9: concurrency group with suffixed github.sha is rejected ──────────
# A literal string containing github.sha_suffix (not an expression reference)
# should be rejected because it's not a valid ${{ github.sha }} expression.
write_header_suffixed_sha() {
local file="$1"
cat > "$file" <<'YAML'
name: SonarCloud Analysis
on:
push:
branches: [main]
pull_request:
branches: [main]
permissions: {}
concurrency:
group: ${{ github.workflow }}-${{ github.ref }}-{{ github.sha_suffix }}
cancel-in-progress: true
jobs:
sonarcloud:
name: SonarCloud
runs-on: ubuntu-latest
permissions:
contents: read
pull-requests: read
env:
SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
steps:
- name: Checkout repository
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
with:
fetch-depth: 0
YAML
}

suffixed="${TMP}/sonar-suffixed-sha.yml"
write_header_suffixed_sha "$suffixed"
append_good_scan "$suffixed"
if run_guard "$suffixed"; then
fail "a concurrency group with github.sha_suffix (not github.sha) should be REJECTED"
else
pass "a concurrency group with github.sha_suffix (not github.sha) is rejected"
fi

# ── Case 10: concurrency group with prefixed github.sha is rejected ────────
# A literal string like sonar-github.sha (not an expression reference) should
# be rejected because it's not a valid ${{ github.sha }} expression.
write_header_prefixed_sha() {
local file="$1"
cat > "$file" <<'YAML'
name: SonarCloud Analysis
on:
push:
branches: [main]
pull_request:
branches: [main]
permissions: {}
concurrency:
group: ${{ github.workflow }}-${{ github.ref }}-sonar-github.sha
cancel-in-progress: true
jobs:
sonarcloud:
name: SonarCloud
runs-on: ubuntu-latest
permissions:
contents: read
pull-requests: read
env:
SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
steps:
- name: Checkout repository
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
with:
fetch-depth: 0
YAML
}

prefixed="${TMP}/sonar-prefixed-sha.yml"
write_header_prefixed_sha "$prefixed"
append_good_scan "$prefixed"
if run_guard "$prefixed"; then
fail "a concurrency group with sonar-github.sha (not github.sha expression) should be REJECTED"
else
pass "a concurrency group with sonar-github.sha (not github.sha expression) is rejected"
fi

echo ""
if [[ "$fails" -eq 0 ]]; then
echo "All tests passed."
Expand Down
Loading