diff --git a/.github/workflows/api-diff.yml b/.github/workflows/api-diff.yml new file mode 100644 index 000000000..8ceb59842 --- /dev/null +++ b/.github/workflows/api-diff.yml @@ -0,0 +1,43 @@ +name: OpenAPI Spec Diff Check + +on: + pull_request: + push: + branches: + - master + - main + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +jobs: + test-branch-logic: + runs-on: ubuntu-latest + permissions: + contents: read + steps: + - name: Checkout code + uses: actions/checkout@v4 + + - name: Run branch logic unit tests + run: ./scripts/api-diff/api-diff.test.sh + + api-diff: + runs-on: ubuntu-latest + needs: test-branch-logic + permissions: + contents: read + pull-requests: write + + steps: + - name: Checkout code + uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Make script executable + run: chmod +x scripts/api-diff/api-diff.sh + + - name: Run API diff check + run: ./scripts/api-diff/api-diff.sh \ No newline at end of file diff --git a/.gitignore b/.gitignore index 16dcdb1fa..6ed2b29fd 100644 --- a/.gitignore +++ b/.gitignore @@ -3,4 +3,7 @@ node_modules # JetBrains generated files -.idea \ No newline at end of file +.idea + +# Temporary files for API diff checks +tmp/ \ No newline at end of file diff --git a/README.md b/README.md index 780ed30a7..5ac149356 100644 --- a/README.md +++ b/README.md @@ -29,6 +29,26 @@ Once you sign up or login, you can create a new API under your account and impor ## Updates If you find something missing or incorrect please [open an issue](https://github.com/XeroAPI/Xero-OpenAPI/issues/new) or send us a pull request. +## API Diff Checking +This repository includes automated API diff checking using [oasdiff](https://github.com/oasdiff/oasdiff) to detect breaking changes and modifications to the OpenAPI specifications. + +### Quick Start +```bash +# Check all xero*.yaml files against master branch +./scripts/api-diff/api-diff.sh + +# Check a single file +./scripts/api-diff/api-diff.sh xero_accounting.yaml +``` + +### Branch Naming Convention +Branches containing `breaking` anywhere in the name will allow breaking changes without failing the build. All other branches will fail if breaking changes are detected. + +**Examples:** `breaking-api-v2`, `feature-breaking-change`, `api-breaking-update` + +### Full Documentation +For detailed usage, configuration options, environment variables, and integration details, see [scripts/api-diff/README.md](scripts/api-diff/README.md). + ## License This software is published under the [MIT License](http://en.wikipedia.org/wiki/MIT_License). diff --git a/scripts/api-diff/README.md b/scripts/api-diff/README.md new file mode 100644 index 000000000..374ed418f --- /dev/null +++ b/scripts/api-diff/README.md @@ -0,0 +1,69 @@ +# API Diff Scripts + +This directory contains scripts for detecting and reporting API changes using [oasdiff](https://github.com/oasdiff/oasdiff). + +## Files + +### `api-diff.sh` +Main script that compares OpenAPI specifications against the master branch. + +**Usage:** +```bash +# From the repo root +./scripts/api-diff/api-diff.sh [--fail-on-breaking] [filename.yaml] + +# Check all xero*.yaml files +./scripts/api-diff/api-diff.sh + +# Check a single file +./scripts/api-diff/api-diff.sh xero_accounting.yaml + +# Fail on breaking changes (CI mode) +./scripts/api-diff/api-diff.sh --fail-on-breaking +``` + +**Environment Variables:** +- `OASDIFF_DOCKER_IMAGE` - Docker image to use (default: `tufin/oasdiff:latest`) +- `BASE_BRANCH` - Branch to compare against (default: `origin/master`) + +### `api-diff.test.sh` +Unit tests for the branch logic pattern matching used in GitHub Actions. + +**Usage:** +```bash +./scripts/api-diff/api-diff.test.sh +``` + +Tests validate that: +- Branches containing `breaking` anywhere in the name are correctly identified +- Other branches are handled with breaking change enforcement + +## Integration + +These scripts are integrated into the GitHub Actions workflow at `.github/workflows/api-diff.yml`: +- **test-branch-logic** job - Runs unit tests +- **api-diff** job - Runs API diff checks with conditional breaking change enforcement + +### Branch Naming Convention +The GitHub Actions workflow automatically adjusts its behavior based on branch names: + +**Allow Breaking Changes:** +- Any branch containing `breaking` in the name +- Examples: `breaking-api-v2`, `feature-breaking-change`, `api-breaking-update` +- The `--fail-on-breaking` flag is NOT passed to the script + +**Fail on Breaking Changes:** +- All other branches (main, master, develop, feature branches, etc.) +- The `--fail-on-breaking` flag IS passed to the script +- Build will fail if breaking changes are detected + +This allows developers to explicitly signal when they're working on breaking changes by including `breaking` in their branch name. + +## Known Limitations + +The oasdiff tool has some non-deterministic behavior due to unordered map iteration in Go: +- **Error counts** (breaking changes) are consistent and reliable +- **Warning counts** may vary by ~2-3% between runs on identical inputs +- This is acceptable for CI purposes as breaking change detection remains accurate + +For more details, see the [oasdiff documentation](https://github.com/oasdiff/oasdiff). diff --git a/scripts/api-diff/api-diff.sh b/scripts/api-diff/api-diff.sh new file mode 100755 index 000000000..dccc0f467 --- /dev/null +++ b/scripts/api-diff/api-diff.sh @@ -0,0 +1,184 @@ +#!/bin/bash + +# Script to check API diffs using oasdiff +# Usage: ./scripts/api-diff/api-diff.sh [--fail-on-breaking] [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 + +# Change to repo root +cd "$(dirname "$0")/../.." + +# Configuration +DOCKER_IMAGE="${OASDIFF_DOCKER_IMAGE:-tufin/oasdiff:latest}" +BASE_BRANCH="${BASE_BRANCH:-origin/master}" + +FAIL_ON_BREAKING=false +TARGET_FILE="" +DRY_RUN=false + +# 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 + TARGET_FILE="$arg" + fi +done + +# If --fail-on-breaking not explicitly set, determine based on branch name +if [ "$FAIL_ON_BREAKING" = false ]; then + CURRENT_BRANCH=${CURRENT_BRANCH:-$(git branch --show-current 2>/dev/null || echo "")} + if [[ "$CURRENT_BRANCH" == *breaking* ]]; then + echo "Branch '$CURRENT_BRANCH' contains 'breaking', allowing breaking changes" + FAIL_ON_BREAKING=false + else + echo "Branch '$CURRENT_BRANCH' does not contain 'breaking', failing on breaking changes" + FAIL_ON_BREAKING=true + 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 branch check" + exit 0 +fi + +echo "Starting API diff check..." + +# Ensure we're in the repo root +if [ ! -f "xero_accounting.yaml" ]; then + echo "Error: Not in repo root or xero_accounting.yaml not found" + 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}" + +# 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 + +# Get list of xero*.yaml files (excluding any master_*.yaml files) +if [ -n "$TARGET_FILE" ]; then + # Single file specified + if [ ! -f "$TARGET_FILE" ]; then + echo "Error: File '$TARGET_FILE' not found" + exit 1 + fi + files="$TARGET_FILE" + echo "Running diff for single file: $TARGET_FILE" +else + # All xero*.yaml files + files=$(ls xero*.yaml 2>/dev/null | grep -v "^master_") + if [ -z "$files" ]; then + echo "No xero*.yaml files found" + exit 1 + fi +fi + +BREAKING_CHANGES_FOUND=false +FILES_WITH_BREAKING_CHANGES=() +TOTAL_FILES=0 +PROCESSED_FILES=0 + +echo "========================================" +echo "API Diff Summary" +echo "Using Docker image: $DOCKER_IMAGE" +echo "Base branch: $BASE_BRANCH" +echo "========================================" + +for file in $files; do + TOTAL_FILES=$((TOTAL_FILES + 1)) + echo "" + echo "========== $file ==========" + + # Get the file from master branch + if ! git show "$BASE_BRANCH:$file" > "$TEMP_DIR/$file" 2>/dev/null; then + echo "ℹ️ New file (does not exist in master branch)" + continue + fi + + # Verify the temp file was created + if [ ! -f "$TEMP_DIR/$file" ]; then + echo "❌ Failed to create temp file" + continue + fi + + # Note: oasdiff has some non-deterministic behavior in change counts due to + # unordered map iteration in Go. Error counts are consistent, but warning + # counts may vary by ~2-3% between runs. This is a known limitation. + + # Run oasdiff changelog + echo "--- Changelog ---" + set +e + CHANGELOG_OUTPUT=$(docker run --rm -v "$(pwd)":/current -v "$TEMP_DIR":/base "$DOCKER_IMAGE" changelog --include-path-params /base/"$file" /current/"$file" 2>&1) + CHANGELOG_EXIT=$? + set -e + + echo "$CHANGELOG_OUTPUT" + + if [ $CHANGELOG_EXIT -eq 0 ]; then + echo "✓ Changelog generated successfully" + else + echo "⚠ Could not generate changelog (exit code: $CHANGELOG_EXIT)" + fi + + # Run breaking changes check + echo "" + echo "--- Breaking changes check ---" + set +e + BREAKING_OUTPUT=$(docker run --rm -v "$(pwd)":/current -v "$TEMP_DIR":/base "$DOCKER_IMAGE" breaking --fail-on ERR --include-path-params /base/"$file" /current/"$file" 2>&1) + BREAKING_EXIT=$? + set -e + + echo "$BREAKING_OUTPUT" + + if [ $BREAKING_EXIT -eq 0 ]; then + echo "✓ No breaking changes detected" + else + echo "⚠ Breaking changes detected (exit code: $BREAKING_EXIT)" + BREAKING_CHANGES_FOUND=true + FILES_WITH_BREAKING_CHANGES+=("$file") + fi + + PROCESSED_FILES=$((PROCESSED_FILES + 1)) +done + +echo "" +echo "========================================" +echo "API Diff check completed" +echo "Processed: $PROCESSED_FILES/$TOTAL_FILES files" +echo "========================================" + +# Summary +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 + 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 +else + echo "" + echo "✓ No breaking changes detected across all files" +fi \ No newline at end of file diff --git a/scripts/api-diff/api-diff.test.sh b/scripts/api-diff/api-diff.test.sh new file mode 100755 index 000000000..edd9cdbc8 --- /dev/null +++ b/scripts/api-diff/api-diff.test.sh @@ -0,0 +1,96 @@ +#!/bin/bash + +# Unit test for api-diff.sh branch logic +# Tests the script's branch detection and FAIL_ON_BREAKING setting + +set -e + +echo "=== Unit Test: api-diff.sh Branch Logic ===" +echo + +TESTS_PASSED=0 +TESTS_FAILED=0 + +SCRIPT_PATH="scripts/api-diff/api-diff.sh" + +# Helper function to test script with branch +test_branch() { + local branch_name="$1" + local expected_mode="$2" # "allow" or "fail" + local test_name="$3" + + echo "Testing: $test_name (branch: $branch_name)" + + # Run the script in dry-run mode with CURRENT_BRANCH set + local output + output=$(CURRENT_BRANCH="$branch_name" "$SCRIPT_PATH" --dry-run 2>&1) + + # Check the output for the expected message + if [[ "$expected_mode" == "allow" ]]; 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" == "fail" ]]; 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 + fi + echo +} + +# Test cases: test_branch "branch-name" "expected-mode" "description" +# expected-mode: "allow" = allows breaking changes, "fail" = fails on breaking + +echo "--- Branches that SHOULD allow breaking changes ---" +test_branch "breaking-api-changes" "allow" "Branch with 'breaking' at start" +test_branch "feature-breaking-change" "allow" "Branch with 'breaking' in middle" +test_branch "fix-breaking-bug" "allow" "Branch with 'breaking' in middle" +test_branch "api-breaking-changes" "allow" "Branch with 'breaking' in middle" +test_branch "update-breaking-endpoint" "allow" "Branch with 'breaking' in middle" + +echo "--- Branches that SHOULD fail on breaking changes ---" +test_branch "feature-new-endpoint" "fail" "Normal feature branch" +test_branch "main" "fail" "Main branch" +test_branch "master" "fail" "Master branch" +test_branch "develop" "fail" "Develop branch" +test_branch "add-openapi-diff-tool" "fail" "Current branch name" +test_branch "fix-api-bug" "fail" "Bug fix branch" +test_branch "feature-v2" "fail" "Version feature branch" + +echo "--- Test override with --fail-on-breaking ---" +# Test that --fail-on-breaking overrides branch logic +echo "Testing override: breaking branch with --fail-on-breaking" +output=$(CURRENT_BRANCH="breaking-test" "$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 branch 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