From 7de151f2cbda704ed8db581f6b37d2b2020f16e5 Mon Sep 17 00:00:00 2001 From: RanVaknin <50976344+RanVaknin@users.noreply.github.com> Date: Tue, 14 Oct 2025 15:49:58 -0700 Subject: [PATCH 1/5] Add mixed version compatibility detection workflow - Detects changes to base classes (AwsRequest, AwsResponse, SdkPojo, etc.) - Requires manual review via 'mixed-version-compatibility-reviewed' label - Prevents merge until team approves compatibility impact --- .../mixed-version-compatibility-review.yml | 87 +++++++++++++++++++ 1 file changed, 87 insertions(+) create mode 100644 .github/workflows/mixed-version-compatibility-review.yml diff --git a/.github/workflows/mixed-version-compatibility-review.yml b/.github/workflows/mixed-version-compatibility-review.yml new file mode 100644 index 000000000000..c2426d083f6e --- /dev/null +++ b/.github/workflows/mixed-version-compatibility-review.yml @@ -0,0 +1,87 @@ +name: Mixed Version Compatibility Review + +permissions: + contents: read + pull-requests: read + +on: + merge_group: + pull_request: + types: [ opened, synchronize, reopened, labeled, unlabeled ] + branches: + - master + +jobs: + mixed-version-compatibility-check: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Check for mixed version compatibility risks + id: compatibility-check + run: | + git fetch origin ${{ github.base_ref }} --depth 1 + + # Define the specific base class files that are risky for mixed versions + BASE_CLASS_FILES="core/aws-core/src/main/java/software/amazon/awssdk/awscore/AwsRequest.java + core/aws-core/src/main/java/software/amazon/awssdk/awscore/AwsResponse.java + core/aws-core/src/main/java/software/amazon/awssdk/awscore/AwsResponseMetadata.java + core/aws-core/src/main/java/software/amazon/awssdk/awscore/exception/AwsServiceException.java + core/sdk-core/src/main/java/software/amazon/awssdk/core/SdkPojo.java" + + # Check if any of the base class files were modified + CHANGED_BASE_FILES=$(git diff --name-only remotes/origin/${{ github.base_ref }} -- $BASE_CLASS_FILES || true) + + if [ -z "$CHANGED_BASE_FILES" ]; then + echo "No base class changes detected." + echo "has_risk=false" >> $GITHUB_OUTPUT + exit 0 + fi + + echo "Base class changes detected in:" + echo "$CHANGED_BASE_FILES" + + # Look for new public methods in the changed base class files + NEW_METHODS=$(git diff remotes/origin/${{ github.base_ref }} -- $BASE_CLASS_FILES | grep '^+.*public.*(' || true) + + if [ -n "$NEW_METHODS" ]; then + echo "::warning::New public methods detected in base classes:" + echo "$NEW_METHODS" | while read line; do + echo "::warning::$line" + done + echo "has_risk=true" >> $GITHUB_OUTPUT + echo "risk_type=new_methods" >> $GITHUB_OUTPUT + else + echo "::warning::Base class files modified but no new public methods detected" + echo "has_risk=true" >> $GITHUB_OUTPUT + echo "risk_type=other_changes" >> $GITHUB_OUTPUT + fi + + - name: Fail if compatibility risks found without approval + if: ${{ steps.compatibility-check.outputs.has_risk == 'true' && !contains(github.event.pull_request.labels.*.name, 'mixed-version-compatibility-reviewed') }} + run: | + echo "::error::Mixed version compatibility risk detected!" + echo "::error::Changes were made to base classes that generated service code implements:" + echo "::error::- AwsRequest, AwsResponse, AwsResponseMetadata, AwsServiceException, SdkPojo" + echo "::error::" + echo "::error::This may break customers using mixed SDK versions if:" + echo "::error::- New methods are added with UnsupportedOperationException defaults" + echo "::error::- Core behavior changes invoke existing methods in new ways" + echo "::error::- Interface contracts change in subtle ways" + echo "::error::" + echo "::error::Please review with the team for mixed version impact and add" + echo "::error::'mixed-version-compatibility-reviewed' label after approval." + echo "::error::" + echo "::error::If this introduces compatibility issues, consider:" + echo "::error::- Bumping minor version" + echo "::error::- Documenting compatibility impact in release notes" + echo "::error::- Ensuring older service modules can handle the changes" + exit 1 + + - name: Success message when approved + if: ${{ steps.compatibility-check.outputs.has_risk == 'true' && contains(github.event.pull_request.labels.*.name, 'mixed-version-compatibility-reviewed') }} + run: | + echo "✅ Mixed version compatibility risk detected but approved for merge" + echo "Base class changes have been reviewed and approved by the team" \ No newline at end of file From b4cf90620cba74468882cb72f65cd03005d50495 Mon Sep 17 00:00:00 2001 From: RanVaknin <50976344+RanVaknin@users.noreply.github.com> Date: Tue, 14 Oct 2025 16:12:33 -0700 Subject: [PATCH 2/5] Improve mixed version detection: filter false positives - Filter out comments containing 'public.*(' patterns - Filter out string literals with 'public.*(' patterns - Filter out javadoc examples with 'public.*(' patterns - Reduces noise while maintaining detection accuracy --- .github/workflows/mixed-version-compatibility-review.yml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/.github/workflows/mixed-version-compatibility-review.yml b/.github/workflows/mixed-version-compatibility-review.yml index c2426d083f6e..936b1c814266 100644 --- a/.github/workflows/mixed-version-compatibility-review.yml +++ b/.github/workflows/mixed-version-compatibility-review.yml @@ -44,7 +44,12 @@ jobs: echo "$CHANGED_BASE_FILES" # Look for new public methods in the changed base class files - NEW_METHODS=$(git diff remotes/origin/${{ github.base_ref }} -- $BASE_CLASS_FILES | grep '^+.*public.*(' || true) + # Filter out obvious false positives: comments, string literals, javadoc + NEW_METHODS=$(git diff remotes/origin/${{ github.base_ref }} -- $BASE_CLASS_FILES | \ + grep '^+.*public.*(' | \ + grep -v '^+[[:space:]]*//.*public.*(' | \ + grep -v '^+.*".*public.*(' | \ + grep -v '^+[[:space:]]*\*.*public.*(' || true) if [ -n "$NEW_METHODS" ]; then echo "::warning::New public methods detected in base classes:" From 47d0582730b241e25d8aa0dd9150cb5f246bf50c Mon Sep 17 00:00:00 2001 From: RanVaknin <50976344+RanVaknin@users.noreply.github.com> Date: Tue, 14 Oct 2025 16:19:44 -0700 Subject: [PATCH 3/5] Fix false positive filtering for block comments Add filtering for block comments that start with /* This should catch patterns like: /* Example usage: public void method() */ --- .github/workflows/mixed-version-compatibility-review.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/mixed-version-compatibility-review.yml b/.github/workflows/mixed-version-compatibility-review.yml index 936b1c814266..d57fd6be08eb 100644 --- a/.github/workflows/mixed-version-compatibility-review.yml +++ b/.github/workflows/mixed-version-compatibility-review.yml @@ -49,7 +49,8 @@ jobs: grep '^+.*public.*(' | \ grep -v '^+[[:space:]]*//.*public.*(' | \ grep -v '^+.*".*public.*(' | \ - grep -v '^+[[:space:]]*\*.*public.*(' || true) + grep -v '^+[[:space:]]*\*.*public.*(' | \ + grep -v '^+[[:space:]]*/\*.*public.*(' || true) if [ -n "$NEW_METHODS" ]; then echo "::warning::New public methods detected in base classes:" From 74eeb2a33d055c88ee7f6b28508d90547ee995c6 Mon Sep 17 00:00:00 2001 From: RanVaknin <50976344+RanVaknin@users.noreply.github.com> Date: Wed, 15 Oct 2025 14:48:25 -0700 Subject: [PATCH 4/5] Address feedback: show only changed class names in error message - Extract class names from actually changed files, not all possible files - Makes error message dynamic and specific to detected changes - Addresses maintainability concern from code review --- .../mixed-version-compatibility-review.yml | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/.github/workflows/mixed-version-compatibility-review.yml b/.github/workflows/mixed-version-compatibility-review.yml index d57fd6be08eb..f5f83eac66d8 100644 --- a/.github/workflows/mixed-version-compatibility-review.yml +++ b/.github/workflows/mixed-version-compatibility-review.yml @@ -68,9 +68,20 @@ jobs: - name: Fail if compatibility risks found without approval if: ${{ steps.compatibility-check.outputs.has_risk == 'true' && !contains(github.event.pull_request.labels.*.name, 'mixed-version-compatibility-reviewed') }} run: | + # Define the base class files + BASE_CLASS_FILES="core/aws-core/src/main/java/software/amazon/awssdk/awscore/AwsRequest.java + core/aws-core/src/main/java/software/amazon/awssdk/awscore/AwsResponse.java + core/aws-core/src/main/java/software/amazon/awssdk/awscore/AwsResponseMetadata.java + core/aws-core/src/main/java/software/amazon/awssdk/awscore/exception/AwsServiceException.java + core/sdk-core/src/main/java/software/amazon/awssdk/core/SdkPojo.java" + + # Extract class names from the actually changed files + CHANGED_BASE_FILES=$(git diff --name-only remotes/origin/${{ github.base_ref }} -- $BASE_CLASS_FILES || true) + CHANGED_CLASS_NAMES=$(echo "$CHANGED_BASE_FILES" | sed 's|.*/||' | sed 's|\.java||' | paste -sd ',' - | sed 's/,/, /g') + echo "::error::Mixed version compatibility risk detected!" echo "::error::Changes were made to base classes that generated service code implements:" - echo "::error::- AwsRequest, AwsResponse, AwsResponseMetadata, AwsServiceException, SdkPojo" + echo "::error::- $CHANGED_CLASS_NAMES" echo "::error::" echo "::error::This may break customers using mixed SDK versions if:" echo "::error::- New methods are added with UnsupportedOperationException defaults" From 915f93b9cc6e42ec84f7ba57393ef89ebecdb8a7 Mon Sep 17 00:00:00 2001 From: RanVaknin <50976344+RanVaknin@users.noreply.github.com> Date: Wed, 15 Oct 2025 21:07:21 -0700 Subject: [PATCH 5/5] Add comments, expand exclusion of patterns --- .../workflows/mixed-version-compatibility-review.yml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/workflows/mixed-version-compatibility-review.yml b/.github/workflows/mixed-version-compatibility-review.yml index f5f83eac66d8..548d4b530737 100644 --- a/.github/workflows/mixed-version-compatibility-review.yml +++ b/.github/workflows/mixed-version-compatibility-review.yml @@ -46,11 +46,11 @@ jobs: # Look for new public methods in the changed base class files # Filter out obvious false positives: comments, string literals, javadoc NEW_METHODS=$(git diff remotes/origin/${{ github.base_ref }} -- $BASE_CLASS_FILES | \ - grep '^+.*public.*(' | \ - grep -v '^+[[:space:]]*//.*public.*(' | \ - grep -v '^+.*".*public.*(' | \ - grep -v '^+[[:space:]]*\*.*public.*(' | \ - grep -v '^+[[:space:]]*/\*.*public.*(' || true) + + grep '^+.*public.*(' | \ # Find lines with new public methods + grep -v '^+[[:space:]]*//.*' | \ # Line comments + grep -v '^+[[:space:]]*\*.*' | \ # Javadoc lines + grep -v '^+[[:space:]]*/\*.*' || true # Block comments if [ -n "$NEW_METHODS" ]; then echo "::warning::New public methods detected in base classes:"