Skip to content

Commit 07d436c

Browse files
authored
Add CompatibilityReporter for backward compatibility change tracking (opensearch-project#349)
* Add CompatibilityReporter for backward compatibility change tracking Signed-off-by: xil <fridalu66@gmail.com> * change back to main repository Signed-off-by: xil <fridalu66@gmail.com> * reformat type change Signed-off-by: xil <fridalu66@gmail.com> * Add more details to report Signed-off-by: xil <fridalu66@gmail.com> * update Signed-off-by: xil <fridalu66@gmail.com> * Add field number Signed-off-by: xil <fridalu66@gmail.com> * Change Enum type name from REMOVED to DEPRECATED Signed-off-by: xil <fridalu66@gmail.com> * field version start from 2 Signed-off-by: xil <fridalu66@gmail.com> * Revert test file Signed-off-by: xil <fridalu66@gmail.com> --------- Signed-off-by: xil <fridalu66@gmail.com>
1 parent 011902a commit 07d436c

11 files changed

Lines changed: 945 additions & 205 deletions
Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,133 @@
1+
name: Backward Compatible Report
2+
3+
on:
4+
workflow_call:
5+
inputs:
6+
opensearch_version:
7+
description: 'OpenSearch version. Leave empty to fetch latest.'
8+
required: false
9+
type: string
10+
spec_artifact_name:
11+
description: 'Name of the uploaded artifact containing opensearch-openapi.yaml'
12+
required: false
13+
type: string
14+
default: 'openapi-spec'
15+
outputs:
16+
report:
17+
description: 'The compatibility report in markdown format'
18+
value: ${{ jobs.generate-report.outputs.report }}
19+
20+
workflow_dispatch:
21+
inputs:
22+
opensearch_version:
23+
description: 'OpenSearch version. Leave empty to fetch latest.'
24+
required: false
25+
type: string
26+
27+
jobs:
28+
generate-report:
29+
runs-on: ubuntu-latest
30+
outputs:
31+
report: ${{ steps.merge_report.outputs.report }}
32+
steps:
33+
- name: Checkout Repository
34+
uses: actions/checkout@v4
35+
with:
36+
repository: ${{ github.repository_owner }}/opensearch-protobufs
37+
ref: main
38+
39+
- name: Setup Node.js
40+
uses: actions/setup-node@v3
41+
with:
42+
node-version: 20
43+
44+
- name: Setup Java
45+
uses: actions/setup-java@v3
46+
with:
47+
distribution: temurin
48+
java-version: 17
49+
50+
- name: Download OpenAPI spec artifact
51+
if: ${{ inputs.spec_artifact_name != '' }}
52+
uses: actions/download-artifact@v4
53+
with:
54+
name: ${{ inputs.spec_artifact_name }}
55+
path: .
56+
57+
- name: Get Latest OpenSearch Core Version
58+
id: get_opensearch_version
59+
uses: actions/github-script@v6
60+
with:
61+
github-token: ${{ secrets.GITHUB_TOKEN }}
62+
script: |
63+
// Check if version was provided as input
64+
const inputVersion = "${{ inputs.opensearch_version }}";
65+
if (inputVersion && inputVersion.trim() !== "") {
66+
core.setOutput("version", inputVersion.trim());
67+
console.log("Using provided OpenSearch version: " + inputVersion.trim());
68+
return;
69+
}
70+
71+
// Otherwise fetch version from buildSrc/version.properties on main branch
72+
try {
73+
const response = await github.request(
74+
'GET /repos/{owner}/{repo}/contents/{path}',
75+
{
76+
owner: 'opensearch-project',
77+
repo: 'OpenSearch',
78+
path: 'buildSrc/version.properties',
79+
ref: 'main'
80+
}
81+
);
82+
83+
// Decode the file content
84+
const content = Buffer.from(response.data.content, 'base64').toString('utf-8');
85+
86+
// Extract opensearch version (e.g., "opensearch = 3.4.0")
87+
const match = content.match(/opensearch\s*=\s*([^\s]+)/);
88+
89+
if (match && match[1]) {
90+
const version = match[1].trim();
91+
core.setOutput("version", version);
92+
console.log("Fetched OpenSearch version from source: " + version);
93+
} else {
94+
core.setFailed("Could not find opensearch version in version.properties");
95+
}
96+
} catch (error) {
97+
core.setFailed("Could not fetch OpenSearch version: " + error.message);
98+
}
99+
100+
- name: Run Proto Conversion
101+
env:
102+
OPENSEARCH_VERSION: ${{ steps.get_opensearch_version.outputs.version }}
103+
run: npm ci && npm run preprocessing -- --opensearch-version "$OPENSEARCH_VERSION"
104+
105+
- name: Clone Protobuf Generator Repository
106+
run: |
107+
git clone https://github.com/OpenAPITools/openapi-generator cloned-repo
108+
cd cloned-repo
109+
git checkout 6699ecd9d2f4e0868f23bb36566ea03cd1230e6a
110+
111+
- name: Build Protobuf Generator Tool
112+
run: |
113+
cd cloned-repo
114+
./mvnw clean package
115+
116+
- name: Convert protobuf
117+
run: |
118+
java -jar cloned-repo/modules/openapi-generator-cli/target/openapi-generator-cli.jar generate -c tools/proto-convert/src/config/protobuf-generator-config.yaml
119+
120+
- name: Post Process Protobuf (dry-run for report)
121+
id: merge_report
122+
run: |
123+
npm run postprocessing:dry-run
124+
REPORT_PATH="/tmp/merge-report.md"
125+
126+
echo "=== Compatibility Report ==="
127+
cat "$REPORT_PATH"
128+
echo "============================="
129+
130+
REPORT=$(cat "$REPORT_PATH")
131+
echo "report<<EOF" >> $GITHUB_OUTPUT
132+
echo "$REPORT" >> $GITHUB_OUTPUT
133+
echo "EOF" >> $GITHUB_OUTPUT

.github/workflows/convert-proto.yml

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@ on:
77
description: 'OpenSearch version. Leave empty to fetch latest.'
88
required: false
99
type: string
10+
repository_dispatch:
11+
types: [spec-updated]
12+
1013
jobs:
1114
auto-proto-convert:
1215
runs-on: ubuntu-latest
@@ -26,11 +29,6 @@ jobs:
2629
distribution: temurin
2730
java-version: 17
2831

29-
- name: Install buf
30-
run: |
31-
npm install -g @bufbuild/buf
32-
buf --version
33-
3432
- name: Download Release Assets
3533
uses: robinraju/release-downloader@v1
3634
with:
@@ -121,12 +119,19 @@ jobs:
121119
run: |
122120
java -jar cloned-repo/modules/openapi-generator-cli/target/openapi-generator-cli.jar generate -c tools/proto-convert/src/config/protobuf-generator-config.yaml
123121
124-
- name: Reformat proto files
125-
run: |
126-
buf format -w protos/generated
127-
128122
- name: Post Process Protobuf
129-
run: npm run postprocessing
123+
id: merge_report
124+
run: |
125+
npm run postprocessing
126+
REPORT_PATH="/tmp/merge-report.md"
127+
if [ -f "$REPORT_PATH" ]; then
128+
REPORT=$(cat "$REPORT_PATH")
129+
echo "report<<EOF" >> $GITHUB_OUTPUT
130+
echo "$REPORT" >> $GITHUB_OUTPUT
131+
echo "EOF" >> $GITHUB_OUTPUT
132+
else
133+
echo "report=No changes detected." >> $GITHUB_OUTPUT
134+
fi
130135
131136
- name: Configure Git User
132137
run: |
@@ -161,3 +166,7 @@ jobs:
161166
162167
**OpenSearch Version**: ${{ steps.get_opensearch_version.outputs.version }}
163168
**API Spec Commit**: ${{ steps.get_commit.outputs.latest_commit }}
169+
170+
---
171+
172+
${{ steps.merge_report.outputs.report }}

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
66
### Added
77
- Add unit test workflow with coverage reporting ([#346](https://github.com/opensearch-project/opensearch-protobufs/pull/346))
88
- parsing service definition to support auto-detect root messages for cleanup ([#347](https://github.com/opensearch-project/opensearch-protobufs/pull/347))
9+
- Add CompatibilityReporter for backward compatibility change tracking ([#349](https://github.com/opensearch-project/opensearch-protobufs/pull/349))
910

1011
### Changed
1112

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
"backward-compat": "ts-node tools/proto-convert/src/postprocessing/BackwardCompatibleWriter.ts",
1010
"cleanup-unused": "ts-node tools/proto-convert/src/postprocessing/CleanupUnusedMessages.ts -i protos/schemas/common.proto",
1111
"postprocessing": "npm run backward-compat && npm run cleanup-unused",
12+
"postprocessing:dry-run": "npm run backward-compat -- --dry-run",
1213
"test": "npx jest --no-watchman"
1314
},
1415
"dependencies": {

tools/proto-convert/src/postprocessing/BackwardCompatibleWriter.ts

Lines changed: 32 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,19 @@
11
import { existsSync } from 'fs';
22
import { Command, Option } from '@commander-js/extra-typings';
3-
import {
4-
ProtoMessage,
5-
ProtoEnum,
6-
BackwardCompatibilityError
7-
} from './types';
3+
import { ProtoMessage, ProtoEnum } from './types';
84
import { parseProtoFile } from './parser';
95
import { mergeMessage, mergeEnum } from './CompatibilityMerger';
106
import { writeProtoFile, CUSTOM_MESSAGE_NAMES, CUSTOM_ENUM_NAMES } from './writer';
7+
import { CompatibilityReporter } from './CompatibilityReporter';
118
import logger from '../utils/logger';
129

1310
export class BackwardCompatibleWriter {
1411
private existingMessages: ProtoMessage[];
1512
private existingEnums: ProtoEnum[];
1613
private incomingMessageMap: Map<string, ProtoMessage> = new Map();
1714
private incomingEnumMap: Map<string, ProtoEnum> = new Map();
18-
private errors: string[] = [];
1915
private outputPath: string;
16+
private reporter: CompatibilityReporter = new CompatibilityReporter();
2017

2118
constructor(existingPath: string, incomingPaths: string[], outputPath: string) {
2219
this.outputPath = outputPath;
@@ -46,7 +43,7 @@ export class BackwardCompatibleWriter {
4643
}
4744
}
4845

49-
process(): void {
46+
process(dryRun: boolean = false): void {
5047
const finalMessages: ProtoMessage[] = [];
5148
const finalEnums: ProtoEnum[] = [];
5249

@@ -59,7 +56,7 @@ export class BackwardCompatibleWriter {
5956

6057
const incomingMsg = this.incomingMessageMap.get(existingMsg.name);
6158
if (incomingMsg) {
62-
finalMessages.push(mergeMessage(existingMsg, incomingMsg, this.errors));
59+
finalMessages.push(mergeMessage(existingMsg, incomingMsg, this.reporter));
6360
this.incomingMessageMap.delete(existingMsg.name);
6461
} else {
6562
finalMessages.push(existingMsg);
@@ -75,7 +72,7 @@ export class BackwardCompatibleWriter {
7572

7673
const incomingEnum = this.incomingEnumMap.get(existingEnum.name);
7774
if (incomingEnum) {
78-
finalEnums.push(mergeEnum(existingEnum, incomingEnum));
75+
finalEnums.push(mergeEnum(existingEnum, incomingEnum, this.reporter));
7976
this.incomingEnumMap.delete(existingEnum.name);
8077
} else {
8178
finalEnums.push(existingEnum);
@@ -96,20 +93,20 @@ export class BackwardCompatibleWriter {
9693
}
9794
}
9895

99-
// Check for errors before writing
100-
if (this.errors.length > 0) {
101-
logger.error('Backward compatibility errors:');
102-
for (const error of this.errors) {
103-
logger.error(` ${error}`);
104-
}
105-
throw new BackwardCompatibilityError(
106-
`Found ${this.errors.length} backward compatibility violation(s).`
107-
);
96+
// Write output
97+
if (dryRun) {
98+
logger.info(`Dry run: would update ${this.outputPath}`);
99+
} else {
100+
writeProtoFile(finalMessages, finalEnums, this.outputPath);
101+
logger.info(`Updated: ${this.outputPath}`);
108102
}
103+
}
109104

110-
// Write output using shared function
111-
writeProtoFile(finalMessages, finalEnums, this.outputPath);
112-
logger.info(`Updated: ${this.outputPath}`);
105+
/**
106+
* Get the merge reporter for accessing change reports.
107+
*/
108+
getReporter(): CompatibilityReporter {
109+
return this.reporter;
113110
}
114111
}
115112

@@ -124,13 +121,15 @@ if (require.main === module) {
124121
.argParser((val: string) => val.split(',').map(s => s.trim()))
125122
.default(['protos/generated/models/aggregated_models.proto', 'protos/generated/services/default_service.proto']))
126123
.addOption(new Option('-o, --output <path>', 'output proto file').default('protos/schemas/common.proto'))
124+
.addOption(new Option('-d, --dry-run', 'preview changes without writing output file').default(false))
127125
.allowExcessArguments(false)
128126
.parse();
129127

130128
type BackwardCompatOpts = {
131129
existing: string;
132130
incoming: string[];
133131
output: string;
132+
dryRun: boolean;
134133
};
135134

136135
const opts = command.opts() as BackwardCompatOpts;
@@ -146,17 +145,16 @@ if (require.main === module) {
146145
process.exit(1);
147146
}
148147

149-
try {
150-
const writer = new BackwardCompatibleWriter(
151-
opts.existing,
152-
opts.incoming,
153-
opts.output
154-
);
155-
writer.process();
156-
} catch (error) {
157-
if (error instanceof BackwardCompatibilityError) {
158-
process.exit(1);
159-
}
160-
throw error;
161-
}
148+
const writer = new BackwardCompatibleWriter(
149+
opts.existing,
150+
opts.incoming,
151+
opts.output
152+
);
153+
154+
// Process and merge
155+
writer.process(opts.dryRun);
156+
157+
// Write report to temp directory
158+
const reportPath = writer.getReporter().writeToFile();
159+
logger.info(`Report written: ${reportPath}`);
162160
}

0 commit comments

Comments
 (0)