Skip to content

Conversation

@RanVaknin
Copy link
Contributor

Following the release of a breaking change where a newer version of a service client was trying to hit a non-existent code path on an older version of core, we have identified a number of core classes that are considered "risky" to change.

When changed / added to, a "risky" core class can have a codepath that breaks customers using mixed SDK versions. For example, when newer service clients expect methods or functionality that older core versions don't provide, it causes runtime failures for customers who haven't upgraded all their SDK modules together.

This workflow detects changes to the following base classes that generated service code implements:
AwsRequest
AwsResponse
AwsResponseMetadata
AwsServiceException
SdkPojo

The workflow uses git diff to detect any modifications to these specific base class files. If changes are found, it searches for new public method additions using pattern matching, and (attempts to) filter out obvious false positives like comments, string literals, and javadoc examples. When "risky" changes are detected, the workflow blocks the PR from merging until the team adds a mixed-version-compatibility-reviewed label.

Testing:
tested on personal fork

Considerations:
This is not supposed to be the "end all be all" tool for catching these potential runtime violations between mixed versions. It's supposed to be a first line of defense for flagging changes with potentially risky elements. Essentially is a very blunt instrument that will require us to do some manual validation after it flags potential violations.

We can always disable this if its more annoying than helpful.

- 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
- 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
Add filtering for block comments that start with /*
This should catch patterns like: /* Example usage: public void method() */
@RanVaknin RanVaknin requested a review from a team as a code owner October 15, 2025 18:24
- 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
@sonarqubecloud
Copy link

@RanVaknin RanVaknin added this pull request to the merge queue Oct 16, 2025
Merged via the queue into master with commit c2fb40c Oct 16, 2025
37 checks passed
@github-actions
Copy link

This pull request has been closed and the conversation has been locked. Comments on closed PRs are hard for our team to see. If you need more assistance, please open a new issue that references this one.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 16, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants