Skip to content

Commit 3eaeea1

Browse files
committed
chore: updated rules based logic for migration check , added regex pattern for changes migration parsing, improved step failure detection for easier POF detection
1 parent 2484588 commit 3eaeea1

4 files changed

Lines changed: 221 additions & 36 deletions

File tree

.github/oasdiff/.oasdiff-severity-levels.yaml renamed to .github/api-migration-compatibility/.oasdiff-severity-levels.yaml

File renamed without changes.
File renamed without changes.
Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
# Migration Rules Configuration
2+
# This file defines SQL patterns that are considered breaking changes or warnings
3+
# for database migrations, with folder-specific rule application.
4+
5+
# Define reusable rule patterns
6+
rule_patterns:
7+
drop_table:
8+
pattern: 'DROP\s+TABLE\s+'
9+
description: "Dropping a table removes data and breaks existing queries"
10+
11+
drop_column:
12+
pattern: 'DROP\s+COLUMN\s+'
13+
description: "Dropping a column breaks applications expecting it"
14+
15+
delete_data:
16+
pattern: 'DELETE\s+FROM\s+'
17+
description: "Deleting data can break application logic"
18+
19+
truncate_table:
20+
pattern: 'TRUNCATE\s+'
21+
description: "Truncating removes all data from a table"
22+
23+
rename_table:
24+
pattern: 'RENAME\s+TO\s+'
25+
description: "Renaming a table breaks existing queries and code references"
26+
27+
rename_column:
28+
pattern: 'RENAME\s+COLUMN\s+'
29+
description: "Renaming a column breaks applications using the old name"
30+
31+
set_not_null:
32+
pattern: 'ALTER\s+COLUMN\s+.*\s+SET\s+NOT\s+NULL'
33+
description: "Making an existing nullable column NOT NULL can fail with existing NULL data"
34+
35+
add_column_not_null_no_default:
36+
pattern: 'ADD\s+COLUMN\s+(?!.*DEFAULT).*NOT\s+NULL'
37+
description: "Adding a NOT NULL column without a DEFAULT will fail on existing rows"
38+
39+
alter_column_type:
40+
pattern: '(ALTER\s+COLUMN\s+.*\s+TYPE|ALTER\s+COLUMN\s+.*\s+SET\s+DATA\s+TYPE)'
41+
description: "Changing column type may cause data loss or conversion issues"
42+
43+
drop_index:
44+
pattern: 'DROP\s+INDEX\s+'
45+
description: "Dropping an index can significantly impact query performance"
46+
47+
drop_constraint:
48+
pattern: 'DROP\s+CONSTRAINT\s+'
49+
description: "Dropping a constraint may allow invalid data"
50+
51+
alter_default:
52+
pattern: '(SET\s+DEFAULT|DROP\s+DEFAULT)'
53+
description: "Changing defaults affects new rows only, may cause inconsistency"
54+
55+
# Apply rules per migration folder
56+
# Each folder can have its own set of breaking and warning rules
57+
folder_rules:
58+
# V1 migrations
59+
migrations:
60+
breaking:
61+
- drop_table
62+
- drop_column
63+
- delete_data
64+
- truncate_table
65+
- rename_table
66+
- rename_column
67+
- set_not_null
68+
- add_column_not_null_no_default
69+
warnings:
70+
- alter_column_type
71+
- drop_index
72+
- drop_constraint
73+
- alter_default
74+
75+
# V2 migrations
76+
v2_migrations:
77+
breaking:
78+
- drop_table
79+
- drop_column
80+
- delete_data
81+
- truncate_table
82+
- rename_table
83+
- rename_column
84+
- set_not_null
85+
- add_column_not_null_no_default
86+
warnings:
87+
- alter_column_type
88+
- drop_index
89+
- drop_constraint
90+
- alter_default
91+
92+
# V2 compatible migrations
93+
v2_compatible_migrations:
94+
breaking:
95+
- drop_table
96+
- drop_column
97+
- delete_data
98+
- truncate_table
99+
- rename_table
100+
- rename_column
101+
- set_not_null
102+
- add_column_not_null_no_default
103+
warnings:
104+
- alter_column_type
105+
- drop_index
106+
- drop_constraint
107+
- alter_default
108+

.github/workflows/api-migrations-compatibility.yml

Lines changed: 113 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ concurrency:
2727

2828
jobs:
2929
api-compatibility-check:
30-
name: API & Migration Compatibility Check
30+
name: API Compatibility Check
3131
runs-on: ubuntu-latest
3232

3333
steps:
@@ -65,7 +65,7 @@ jobs:
6565
6666
if oasdiff breaking \
6767
--fail-on ERR \
68-
--warn-ignore .github/oasdiff/.oasdiff-warn-ignore.yaml \
68+
--warn-ignore .github/api-migration-compatibility/.oasdiff-warn-ignore.yaml \
6969
base-v1-schema.json pr-v1-schema.json > v1-breaking-report.txt 2>&1; then
7070
echo "V1 API is backward compatible" > v1-breaking-status.txt
7171
else
@@ -76,7 +76,7 @@ jobs:
7676
7777
if oasdiff breaking \
7878
--fail-on ERR \
79-
--warn-ignore .github/oasdiff/.oasdiff-warn-ignore.yaml \
79+
--warn-ignore .github/api-migration-compatibility/.oasdiff-warn-ignore.yaml \
8080
base-v2-schema.json pr-v2-schema.json > v2-breaking-report.txt 2>&1; then
8181
echo "V2 API is backward compatible" > v2-breaking-status.txt
8282
else
@@ -139,39 +139,65 @@ jobs:
139139
id: migration_check
140140
run: |
141141
BASE_REF="origin/${{ github.event.pull_request.base.ref }}"
142-
NEW_MIGRATIONS=$(git diff --name-only --diff-filter=AM "$BASE_REF"...HEAD -- "migrations/**/*.sql" 2>/dev/null || echo "")
142+
NEW_MIGRATIONS=$(git diff --name-only --diff-filter=AM "$BASE_REF"...HEAD -- "migrations/**/up.sql" 2>/dev/null || echo "")
143143
144144
if [[ -z "$NEW_MIGRATIONS" ]]; then
145145
echo "breaking_changes=0" >> $GITHUB_OUTPUT
146146
echo "warnings=0" >> $GITHUB_OUTPUT
147147
exit 0
148148
fi
149149
150+
# Load migration rules using yq
151+
RULES_FILE=".github/api-migration-compatibility/migration-rules.yaml"
152+
153+
# Extract breaking rule patterns for migrations folder
154+
BREAKING_PATTERNS=$(
155+
for rule in $(yq '.folder_rules.migrations.breaking[]' "$RULES_FILE"); do
156+
yq ".rule_patterns.$rule.pattern" "$RULES_FILE"
157+
done | paste -sd'|' -
158+
)
159+
160+
# Extract warning rule patterns for migrations folder
161+
WARNING_PATTERNS=$(
162+
for rule in $(yq '.folder_rules.migrations.warnings[]' "$RULES_FILE"); do
163+
yq ".rule_patterns.$rule.pattern" "$RULES_FILE"
164+
done | paste -sd'|' -
165+
)
166+
150167
BREAKING=0
151168
WARNINGS=0
152169
153170
while IFS= read -r file; do
154171
[[ -z "$file" ]] && continue
155-
CONTENT=$(cat "$file")
156172
157-
if echo "$CONTENT" | grep -iE "(DROP TABLE|DROP COLUMN|DELETE FROM|TRUNCATE)" >/dev/null; then
173+
# Check for breaking changes
174+
if grep -iEq "$BREAKING_PATTERNS" "$file"; then
158175
echo "BREAKING: $file"
159-
echo "$CONTENT" | grep -inE "(DROP TABLE|DROP COLUMN|DELETE FROM|TRUNCATE)" | sed 's/^/ /'
160-
BREAKING=$((BREAKING + 1))
176+
MATCH_COUNT=$(grep -icE "$BREAKING_PATTERNS" "$file")
177+
grep -inE "$BREAKING_PATTERNS" "$file" | sed 's/^/ /' || true
178+
BREAKING=$((BREAKING + MATCH_COUNT))
161179
fi
162180
163-
if echo "$CONTENT" | grep -iE "(ALTER COLUMN .* TYPE|ALTER COLUMN .* SET NOT NULL|DROP INDEX|DROP CONSTRAINT)" >/dev/null; then
181+
# Check for warnings
182+
if grep -iEq "$WARNING_PATTERNS" "$file"; then
164183
echo "WARNING: $file"
165-
echo "$CONTENT" | grep -inE "(ALTER COLUMN .* TYPE|ALTER COLUMN .* SET NOT NULL|DROP INDEX|DROP CONSTRAINT)" | sed 's/^/ /'
166-
WARNINGS=$((WARNINGS + 1))
184+
MATCH_COUNT=$(grep -icE "$WARNING_PATTERNS" "$file")
185+
grep -inE "$WARNING_PATTERNS" "$file" | sed 's/^/ /' || true
186+
WARNINGS=$((WARNINGS + MATCH_COUNT))
167187
fi
168188
done <<< "$NEW_MIGRATIONS"
169189
170190
echo "breaking_changes=$BREAKING" >> $GITHUB_OUTPUT
171191
echo "warnings=$WARNINGS" >> $GITHUB_OUTPUT
172-
continue-on-error: true
192+
193+
if [[ $BREAKING -gt 0 ]]; then
194+
echo "::error::Breaking changes detected in V1 database migrations."
195+
echo "::error::Found $BREAKING breaking change(s)."
196+
exit 1
197+
fi
173198
174199
- name: Validate V2 migrations
200+
if: success() || failure()
175201
id: v2_migration_check
176202
run: |
177203
if [[ ! -d "v2_migrations" ]]; then
@@ -181,38 +207,65 @@ jobs:
181207
fi
182208
183209
BASE_REF="origin/${{ github.event.pull_request.base.ref }}"
184-
NEW_MIGRATIONS=$(git diff --name-only --diff-filter=AM "$BASE_REF"...HEAD -- "v2_migrations/**/*.sql" 2>/dev/null || echo "")
210+
NEW_MIGRATIONS=$(git diff --name-only --diff-filter=AM "$BASE_REF"...HEAD -- "v2_migrations/**/up.sql" 2>/dev/null || echo "")
185211
186212
if [[ -z "$NEW_MIGRATIONS" ]]; then
187213
echo "breaking_changes=0" >> $GITHUB_OUTPUT
188214
echo "warnings=0" >> $GITHUB_OUTPUT
189215
exit 0
190216
fi
191217
218+
# Load migration rules using yq
219+
RULES_FILE=".github/api-migration-compatibility/migration-rules.yaml"
220+
221+
# Extract breaking rule patterns for v2_migrations folder
222+
BREAKING_PATTERNS=$(
223+
for rule in $(yq '.folder_rules.v2_migrations.breaking[]' "$RULES_FILE"); do
224+
yq ".rule_patterns.$rule.pattern" "$RULES_FILE"
225+
done | paste -sd'|' -
226+
)
227+
228+
# Extract warning rule patterns for v2_migrations folder
229+
WARNING_PATTERNS=$(
230+
for rule in $(yq '.folder_rules.v2_migrations.warnings[]' "$RULES_FILE"); do
231+
yq ".rule_patterns.$rule.pattern" "$RULES_FILE"
232+
done | paste -sd'|' -
233+
)
234+
192235
BREAKING=0
193236
WARNINGS=0
194237
195238
while IFS= read -r file; do
196239
[[ -z "$file" ]] && continue
197-
CONTENT=$(cat "$file")
198240
199-
if echo "$CONTENT" | grep -iE "(DROP TABLE|DROP COLUMN|DELETE FROM|TRUNCATE)" >/dev/null; then
241+
# Check for breaking changes
242+
if grep -iEq "$BREAKING_PATTERNS" "$file"; then
200243
echo "BREAKING: $file"
201-
echo "$CONTENT" | grep -inE "(DROP TABLE|DROP COLUMN|DELETE FROM|TRUNCATE)" | sed 's/^/ /'
202-
BREAKING=$((BREAKING + 1))
244+
MATCH_COUNT=$(grep -icE "$BREAKING_PATTERNS" "$file")
245+
grep -inE "$BREAKING_PATTERNS" "$file" | sed 's/^/ /' || true
246+
BREAKING=$((BREAKING + MATCH_COUNT))
203247
fi
204-
if echo "$CONTENT" | grep -iE "(ALTER COLUMN .* TYPE|ALTER COLUMN .* SET NOT NULL|DROP INDEX|DROP CONSTRAINT)" >/dev/null; then
248+
249+
# Check for warnings
250+
if grep -iEq "$WARNING_PATTERNS" "$file"; then
205251
echo "WARNING: $file"
206-
echo "$CONTENT" | grep -inE "(ALTER COLUMN .* TYPE|ALTER COLUMN .* SET NOT NULL|DROP INDEX|DROP CONSTRAINT)" | sed 's/^/ /'
207-
WARNINGS=$((WARNINGS + 1))
252+
MATCH_COUNT=$(grep -icE "$WARNING_PATTERNS" "$file")
253+
grep -inE "$WARNING_PATTERNS" "$file" | sed 's/^/ /' || true
254+
WARNINGS=$((WARNINGS + MATCH_COUNT))
208255
fi
209256
done <<< "$NEW_MIGRATIONS"
210257
211258
echo "breaking_changes=$BREAKING" >> $GITHUB_OUTPUT
212259
echo "warnings=$WARNINGS" >> $GITHUB_OUTPUT
213-
continue-on-error: true
260+
261+
if [[ $BREAKING -gt 0 ]]; then
262+
echo "::error::Breaking changes detected in V2 database migrations."
263+
echo "::error::Found $BREAKING breaking change(s)."
264+
exit 1
265+
fi
214266
215267
- name: Validate V2 compatible migrations
268+
if: success() || failure()
216269
id: v2_compat_migration_check
217270
run: |
218271
if [[ ! -d "v2_compatible_migrations" ]]; then
@@ -222,39 +275,65 @@ jobs:
222275
fi
223276
224277
BASE_REF="origin/${{ github.event.pull_request.base.ref }}"
225-
NEW_MIGRATIONS=$(git diff --name-only --diff-filter=AM "$BASE_REF"...HEAD -- "v2_compatible_migrations/**/*.sql" 2>/dev/null || echo "")
278+
NEW_MIGRATIONS=$(git diff --name-only --diff-filter=AM "$BASE_REF"...HEAD -- "v2_compatible_migrations/**/up.sql" 2>/dev/null || echo "")
226279
227280
if [[ -z "$NEW_MIGRATIONS" ]]; then
228281
echo "breaking_changes=0" >> $GITHUB_OUTPUT
229282
echo "warnings=0" >> $GITHUB_OUTPUT
230283
exit 0
231284
fi
232285
286+
# Load migration rules using yq
287+
RULES_FILE=".github/api-migration-compatibility/migration-rules.yaml"
288+
289+
# Extract breaking rule patterns for v2_compatible_migrations folder
290+
BREAKING_PATTERNS=$(
291+
for rule in $(yq '.folder_rules.v2_compatible_migrations.breaking[]' "$RULES_FILE"); do
292+
yq ".rule_patterns.$rule.pattern" "$RULES_FILE"
293+
done | paste -sd'|' -
294+
)
295+
296+
# Extract warning rule patterns for v2_compatible_migrations folder
297+
WARNING_PATTERNS=$(
298+
for rule in $(yq '.folder_rules.v2_compatible_migrations.warnings[]' "$RULES_FILE"); do
299+
yq ".rule_patterns.$rule.pattern" "$RULES_FILE"
300+
done | paste -sd'|' -
301+
)
302+
233303
BREAKING=0
234304
WARNINGS=0
235305
236306
while IFS= read -r file; do
237307
[[ -z "$file" ]] && continue
238-
CONTENT=$(cat "$file")
239308
240-
if echo "$CONTENT" | grep -iE "(DROP TABLE|DROP COLUMN|DELETE FROM|TRUNCATE)" >/dev/null; then
309+
# Check for breaking changes
310+
if grep -iEq "$BREAKING_PATTERNS" "$file"; then
241311
echo "BREAKING: $file"
242-
echo "$CONTENT" | grep -inE "(DROP TABLE|DROP COLUMN|DELETE FROM|TRUNCATE)" | sed 's/^/ /'
243-
BREAKING=$((BREAKING + 1))
312+
MATCH_COUNT=$(grep -icE "$BREAKING_PATTERNS" "$file")
313+
grep -inE "$BREAKING_PATTERNS" "$file" | sed 's/^/ /' || true
314+
BREAKING=$((BREAKING + MATCH_COUNT))
244315
fi
245316
246-
if echo "$CONTENT" | grep -iE "(ALTER COLUMN .* TYPE|ALTER COLUMN .* SET NOT NULL|DROP INDEX|DROP CONSTRAINT)" >/dev/null; then
317+
# Check for warnings
318+
if grep -iEq "$WARNING_PATTERNS" "$file"; then
247319
echo "WARNING: $file"
248-
echo "$CONTENT" | grep -inE "(ALTER COLUMN .* TYPE|ALTER COLUMN .* SET NOT NULL|DROP INDEX|DROP CONSTRAINT)" | sed 's/^/ /'
249-
WARNINGS=$((WARNINGS + 1))
320+
MATCH_COUNT=$(grep -icE "$WARNING_PATTERNS" "$file")
321+
grep -inE "$WARNING_PATTERNS" "$file" | sed 's/^/ /' || true
322+
WARNINGS=$((WARNINGS + MATCH_COUNT))
250323
fi
251324
done <<< "$NEW_MIGRATIONS"
252325
253326
echo "breaking_changes=$BREAKING" >> $GITHUB_OUTPUT
254327
echo "warnings=$WARNINGS" >> $GITHUB_OUTPUT
255-
continue-on-error: true
328+
329+
if [[ $BREAKING -gt 0 ]]; then
330+
echo "::error::Breaking changes detected in V2 compatible database migrations."
331+
echo "::error::Found $BREAKING breaking change(s)."
332+
exit 1
333+
fi
256334
257335
- name: Calculate migration totals
336+
if: always()
258337
id: migration_report
259338
env:
260339
MIGRATION_BREAKING: ${{ steps.migration_check.outputs.breaking_changes }}
@@ -301,9 +380,7 @@ jobs:
301380
echo "Status: ${{ steps.migration_report.outputs.validation_status }}"
302381
echo "=========================================="
303382
304-
- name: Fail if migration breaking changes detected
305-
if: steps.migration_report.outputs.total_breaking != '0' && steps.migration_report.outputs.total_breaking != ''
306-
run: |
307-
echo "::error::Breaking changes detected in database migrations."
308-
echo "::error::Found ${{ steps.migration_report.outputs.total_breaking }} breaking change(s)."
309-
exit 1
383+
if [[ "${{ steps.migration_report.outputs.total_breaking }}" != "0" && "${{ steps.migration_report.outputs.total_breaking }}" != "" ]]; then
384+
echo "::error::Migration validation failed with breaking changes."
385+
exit 1
386+
fi

0 commit comments

Comments
 (0)