Skip to content

ci: fail closed on API compatibility checks #837

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
ryanduguid wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

ci: fail closed on API compatibility checks #837

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
baa1d98
ci: fail closed on API compatibility checks
ryanduguid Aug 12, 2026
8d62cda
test: cover unset Actions environment
ryanduguid Aug 12, 2026
3311e7b
docs: describe the pinned oasdiff image
ryanduguid Aug 12, 2026
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
19 changes: 5 additions & 14 deletions .github/workflows/api-diff.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,32 +12,23 @@ concurrency:
cancel-in-progress: true

jobs:
test-conventional-commit-logic:
runs-on: ubuntu-latest
permissions:
contents: read
steps:
- name: Checkout code
uses: actions/checkout@v4

- name: Run conventional commit logic unit tests
run: ./scripts/api-diff/api-diff.test.sh

api-diff:
runs-on: ubuntu-latest
needs: test-conventional-commit-logic
timeout-minutes: 20
permissions:
contents: read
pull-requests: write

steps:
- name: Checkout code
uses: actions/checkout@v4
uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.2.2
with:
fetch-depth: 0

- name: Make script executable
run: chmod +x scripts/api-diff/api-diff.sh

- name: Test fail-closed behaviour
run: bash scripts/api-diff/api-diff.test.sh

- name: Run API diff check
run: ./scripts/api-diff/api-diff.sh
4 changes: 3 additions & 1 deletion scripts/api-diff/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,11 @@ Main script that compares OpenAPI specifications against the master branch.
```

**Environment Variables:**
- `OASDIFF_DOCKER_IMAGE` - Docker image to use (default: `tufin/oasdiff:latest`)
- `OASDIFF_DOCKER_IMAGE` - Docker image to use (default: oasdiff 1.28.0 pinned by image digest in `api-diff.sh`)
- `BASE_BRANCH` - Branch to compare against (default: `origin/master`)

When updating oasdiff, verify the release tag and the image manifest digest from the publisher, then update both the tag and digest together. Do not replace the default with a mutable tag such as `latest`.

### `api-diff.test.sh`
Unit tests for conventional commit breaking marker detection used in GitHub Actions.

Expand Down
117 changes: 29 additions & 88 deletions scripts/api-diff/api-diff.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,97 +1,37 @@
#!/bin/bash

# Script to check API diffs using oasdiff
# Usage: ./scripts/api-diff/api-diff.sh [--fail-on-breaking] [filename.yaml]
# Usage: ./scripts/api-diff/api-diff.sh [filename.yaml]
# Assumes you have Docker installed and the repo is checked out with master branch available

set -e # Exit on error
set -o pipefail # Catch errors in pipes
set -euo pipefail

# Change to repo root
cd "$(dirname "$0")/../.."

# Configuration
DOCKER_IMAGE="${OASDIFF_DOCKER_IMAGE:-tufin/oasdiff:latest}"
DOCKER_IMAGE="${OASDIFF_DOCKER_IMAGE:-tufin/oasdiff:v1.28.0@sha256:86830f988eaafcf589acb2794ee5ab78e3300ded071d6517bf085469300cbf36}"

# Detect base branch from GitHub Actions environment or fallback to local defaults
if [ -n "$GITHUB_BASE_REF" ]; then
if [ -n "${GITHUB_BASE_REF:-}" ]; then
# In GitHub Actions PR, use the base ref (e.g., "master")
BASE_BRANCH="${BASE_BRANCH:-origin/$GITHUB_BASE_REF}"
else
# Local development: default to origin/master
BASE_BRANCH="${BASE_BRANCH:-origin/master}"
fi

FAIL_ON_BREAKING=false
TARGET_FILE=""
DRY_RUN=false

detect_breaking_commit_marker() {
local commits="$1"

# Conventional Commits breaking indicators:
# 1) An exclamation mark in the type/scope header, e.g. feat!: ... or feat(api)!: ...
# 2) A BREAKING CHANGE footer in the commit body
if echo "$commits" | grep -Eiq '^[[:space:]]*[a-z]+(\([^)]+\))?!:'; then
return 0
fi

if echo "$commits" | grep -Eiq 'BREAKING[ -]CHANGE:'; then
return 0
fi

return 1
}

get_commit_messages() {
# For tests and local overrides
if [ -n "$COMMIT_MESSAGES" ]; then
echo "$COMMIT_MESSAGES"
return 0
fi

# In GitHub Actions PRs, scan all commit subjects + bodies in the PR range.
if [ -n "$GITHUB_BASE_REF" ]; then
git log --format='%s%n%b%n----' "origin/$GITHUB_BASE_REF..HEAD" 2>/dev/null || true
return 0
fi

# Local default: inspect HEAD commit only.
git log -1 --format='%s%n%b' 2>/dev/null || true
}

# Parse arguments
for arg in "$@"; do
if [ "$arg" = "--fail-on-breaking" ]; then
FAIL_ON_BREAKING=true
elif [ "$arg" = "--dry-run" ]; then
DRY_RUN=true
elif [[ "$arg" == *.yaml ]]; then
if [[ "$arg" == *.yaml ]] && [ -z "$TARGET_FILE" ]; then
TARGET_FILE="$arg"
fi
done

# If --fail-on-breaking not explicitly set, determine based on conventional commit markers.
if [ "$FAIL_ON_BREAKING" = false ]; then
COMMIT_TEXT=$(get_commit_messages)
if detect_breaking_commit_marker "$COMMIT_TEXT"; then
echo "Detected conventional commit breaking marker ('!' in header or 'BREAKING CHANGE:' footer), allowing breaking changes"
FAIL_ON_BREAKING=false
else
echo "No conventional commit breaking marker found, failing on breaking changes"
FAIL_ON_BREAKING=true
echo "Error: unsupported argument '$arg'" >&2
exit 2
fi
fi

if [ "$DRY_RUN" = true ]; then
if [ "$FAIL_ON_BREAKING" = true ]; then
echo "Mode: Failing on breaking changes"
else
echo "Mode: Allowing breaking changes"
fi
echo "Dry run mode, exiting after commit message check"
exit 0
fi
done

echo "Starting API diff check..."

Expand All @@ -101,12 +41,14 @@ if [ ! -f "xero_accounting.yaml" ]; then
exit 1
fi

# Fetch master if not already done
git fetch "${BASE_BRANCH%%/*}" "${BASE_BRANCH##*/}" 2>/dev/null || echo "Warning: Could not fetch ${BASE_BRANCH}"
# Refresh and verify the exact base. A missing or stale base must not turn this
# compatibility check into a pass.
git fetch "${BASE_BRANCH%%/*}" "${BASE_BRANCH##*/}"
git rev-parse --verify "${BASE_BRANCH}^{commit}" >/dev/null

# Create temp directory for master branch files (outside repo to avoid overlap with /current mount)
TEMP_DIR=$(mktemp -d)
trap "rm -rf $TEMP_DIR" EXIT
trap 'rm -rf "$TEMP_DIR"' EXIT

# Get list of xero*.yaml files (excluding any master_*.yaml files)
if [ -n "$TARGET_FILE" ]; then
Expand All @@ -127,6 +69,7 @@ else
fi

BREAKING_CHANGES_FOUND=false
EXECUTION_FAILED=false
FILES_WITH_BREAKING_CHANGES=()
TOTAL_FILES=0
PROCESSED_FILES=0
Expand All @@ -143,16 +86,13 @@ for file in $files; do
echo "========== $file =========="

# Get the file from master branch
if ! git show "$BASE_BRANCH:$file" > "$TEMP_DIR/$file" 2>/dev/null; then
if ! git cat-file -e "$BASE_BRANCH:$file" 2>/dev/null; then
echo "ℹ️ New file (does not exist in master branch)"
PROCESSED_FILES=$((PROCESSED_FILES + 1))
continue
fi

# Verify the temp file was created
if [ ! -f "$TEMP_DIR/$file" ]; then
echo "❌ Failed to create temp file"
continue
fi
git show "$BASE_BRANCH:$file" > "$TEMP_DIR/$file"

# Note: oasdiff has some non-deterministic behavior in change counts due to
# unordered map iteration in Go. Error counts are consistent, but warning
Expand All @@ -170,7 +110,8 @@ for file in $files; do
if [ $CHANGELOG_EXIT -eq 0 ]; then
echo "✓ Changelog generated successfully"
else
echo "⚠ Could not generate changelog (exit code: $CHANGELOG_EXIT)"
echo "❌ Could not generate changelog (exit code: $CHANGELOG_EXIT)"
EXECUTION_FAILED=true
fi

# Run breaking changes check
Expand Down Expand Up @@ -201,26 +142,26 @@ echo "Processed: $PROCESSED_FILES/$TOTAL_FILES files"
echo "========================================"

# Summary
if [ "$EXECUTION_FAILED" = true ]; then
echo "❌ API diff execution failed"
exit 1
fi

if [ "$BREAKING_CHANGES_FOUND" = true ]; then
echo ""
echo "❌ Breaking changes detected in the following files:"
for file in "${FILES_WITH_BREAKING_CHANGES[@]}"; do
echo " - $file"
# Output GitHub Actions annotation
if [ -n "$GITHUB_ACTIONS" ]; then
if [ -n "${GITHUB_ACTIONS:-}" ]; then
echo "::warning file=${file}::Breaking changes detected in this API spec file"
fi
done

if [ "$FAIL_ON_BREAKING" = true ]; then
echo ""
echo "Exiting with error due to breaking changes"
exit 1
else
echo ""
echo "Note: Not failing build (use --fail-on-breaking to fail on breaking changes)"
fi
echo ""
echo "Exiting with error due to breaking changes"
exit 1
else
echo ""
echo "✓ No breaking changes detected across all files"
fi
fi
125 changes: 42 additions & 83 deletions scripts/api-diff/api-diff.test.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,91 +1,50 @@
#!/bin/bash
#!/usr/bin/env bash

# Unit test for api-diff.sh conventional commit logic
# Tests commit message detection and FAIL_ON_BREAKING setting
set -euo pipefail

set -e
cd "$(dirname "$0")/../.."

echo "=== Unit Test: api-diff.sh Conventional Commit Logic ==="
echo
FAKE_BIN=$(mktemp -d)
trap 'rm -rf "$FAKE_BIN"' EXIT

TESTS_PASSED=0
TESTS_FAILED=0

SCRIPT_PATH="scripts/api-diff/api-diff.sh"

# Helper function to test script with commit messages
test_commit_messages() {
local commit_messages="$1"
local expected_mode="$2" # "allow-breaking-changes" or "block-breaking-changes"
local test_name="$3"

echo "Testing: $test_name (commit: $commit_messages)"

# Run the script in dry-run mode with COMMIT_MESSAGES set
local output
output=$(COMMIT_MESSAGES="$commit_messages" "$SCRIPT_PATH" --dry-run 2>&1)

if [[ "$expected_mode" == "allow-breaking-changes" ]]; then
if echo "$output" | grep -q "Mode: Allowing breaking changes"; then
echo " ✓ PASS: Correctly allows breaking changes"
TESTS_PASSED=$((TESTS_PASSED + 1))
else
echo " ✗ FAIL: Expected to allow breaking changes, but output was:"
echo "$output"
TESTS_FAILED=$((TESTS_FAILED + 1))
fi
elif [[ "$expected_mode" == "block-breaking-changes" ]]; then
if echo "$output" | grep -q "Mode: Failing on breaking changes"; then
echo " ✓ PASS: Correctly fails on breaking changes"
TESTS_PASSED=$((TESTS_PASSED + 1))
else
echo " ✗ FAIL: Expected to fail on breaking changes, but output was:"
echo "$output"
TESTS_FAILED=$((TESTS_FAILED + 1))
fi
cat > "$FAKE_BIN/docker" <<'EOF'
#!/usr/bin/env bash
if [[ " $* " == *" changelog "* ]] && [ "${FAKE_DOCKER_MODE:-pass}" = "changelog-error" ]; then
echo "simulated changelog error" >&2
exit 2
fi
if [[ " $* " == *" breaking "* ]] && [ "${FAKE_DOCKER_MODE:-pass}" = "breaking" ]; then
echo "simulated breaking change" >&2
exit 1
fi
exit 0
EOF
chmod +x "$FAKE_BIN/docker"

run_check() {
local expected_exit=$1
local mode=$2
shift 2

set +e
env -u GITHUB_ACTIONS \
PATH="$FAKE_BIN:$PATH" \
BASE_BRANCH=origin/master \
FAKE_DOCKER_MODE="$mode" \
OASDIFF_DOCKER_IMAGE=test-image \
bash scripts/api-diff/api-diff.sh "$@" >/dev/null 2>&1
local actual_exit=$?
set -e

if [ "$actual_exit" -ne "$expected_exit" ]; then
echo "Expected exit $expected_exit for mode '$mode', got $actual_exit" >&2
exit 1
fi
echo
}

# Test cases: test_commit_messages "commit_messages" "expected_mode" "test_name"
# expected_mode: "allow-breaking-changes" = allows breaking changes, "block-breaking-changes" = fails on breaking
run_check 0 pass xero_accounting.yaml
run_check 1 breaking xero_accounting.yaml
run_check 1 changelog-error xero_accounting.yaml
run_check 2 pass --unsupported

echo "--- Commit messages that SHOULD allow breaking changes ---"
test_commit_messages "feat!: remove deprecated endpoint" "allow-breaking-changes" "Header with ! marker"
test_commit_messages "feat(api)!: remove deprecated endpoint" "allow-breaking-changes" "Header with scope and ! marker"
test_commit_messages "feat: refactor API\n\nBREAKING CHANGE: response schema updated" "allow-breaking-changes" "BREAKING CHANGE footer"
test_commit_messages "fix: patch bug\n\nSome details\nBREAKING CHANGE: removed old field" "allow-breaking-changes" "BREAKING CHANGE footer after body"

echo "--- Commit messages that SHOULD fail on breaking changes ---"
test_commit_messages "feat: add optional field" "block-breaking-changes" "Normal feature commit"
test_commit_messages "fix: resolve null issue" "block-breaking-changes" "Normal fix commit"
test_commit_messages "chore: update docs" "block-breaking-changes" "Chore commit"
test_commit_messages "docs: mention breaking behaviour in description" "block-breaking-changes" "Contains word breaking but no marker"

echo "--- Test override with --fail-on-breaking ---"
# Test that --fail-on-breaking overrides commit message logic
echo "Testing override: commit with breaking marker and --fail-on-breaking"
output=$(COMMIT_MESSAGES="feat!: breaking api update" "$SCRIPT_PATH" --dry-run --fail-on-breaking 2>&1)
if echo "$output" | grep -q "Mode: Failing on breaking changes"; then
echo " ✓ PASS: --fail-on-breaking overrides commit logic"
TESTS_PASSED=$((TESTS_PASSED + 1))
else
echo " ✗ FAIL: --fail-on-breaking did not override, output:"
echo "$output"
TESTS_FAILED=$((TESTS_FAILED + 1))
fi
echo

echo "========================================"
echo "Test Results:"
echo " Passed: $TESTS_PASSED"
echo " Failed: $TESTS_FAILED"
echo "========================================"

if [ $TESTS_FAILED -gt 0 ]; then
echo "❌ Some tests failed!"
exit 1
else
echo "✅ All tests passed!"
exit 0
fi
echo "API diff fail-closed tests passed"
Loading