Skip to content

feat(migration): add webhook secretKey to authType migration in v1130#27438

Merged
yan-3005 merged 2 commits intomainfrom
ram/webhook-migrations
Apr 16, 2026
Merged

feat(migration): add webhook secretKey to authType migration in v1130#27438
yan-3005 merged 2 commits intomainfrom
ram/webhook-migrations

Conversation

@yan-3005
Copy link
Copy Markdown
Contributor

@yan-3005 yan-3005 commented Apr 16, 2026

Summary

On the 1.12.5 branch, migrateWebhookSecretKeyToAuthType was incorrectly cherry-picked from the certification refactor, causing Jackson deserialization failures since webhook.json on 1.12.x had no authType field. That migration was removed from v1125 and a reverse migration was added in v1126 to fix users who had already upgraded.

This PR ensures users who were on 1.12.4 or older and upgrade directly to 1.13 (skipping 1.12.5 entirely) still get the webhook migration. It adds migrateWebhookSecretKeyToAuthType to the v1130 migration, which converts:

"config": { "secretKey": "value" }

"config": { "authType": { "type": "bearer", "secretKey": "value" } }

This is the correct canonical format on main/1.13 — SubscriptionUtil, WebhookRecipient, and Fernet all read authType from the webhook config.

The migration is idempotent: rows that already have authType (no secretKey) are skipped.

Changes

  • v1130/MigrationUtil.java — added migrateWebhookSecretKeyToAuthType(Handle handle)
  • mysql/v1130/Migration.java + postgres/v1130/Migration.java — call the new method in runDataMigration()
  • v1130/MigrationUtilTest.java — 6 unit tests covering: no rows, no secretKey, empty secretKey, non-webhook destination, MySQL path (JSON + SQL verified), Postgres path (JSON + SQL verified)

Type of change

  • Bug fix

Checklist

  • I have read the CONTRIBUTING document.
  • I have added a test that covers the exact scenario we are fixing.
  • For JSON Schema changes: I updated the migration scripts or explained why it is not needed.

@yan-3005 yan-3005 self-assigned this Apr 16, 2026
@yan-3005 yan-3005 added the safe to test Add this label to run secure Github workflows on PRs label Apr 16, 2026
Copilot AI review requested due to automatic review settings April 16, 2026 14:32
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a v1130 data migration to move legacy webhook secretKey configuration into the newer authType structure for event subscription webhook destinations, and introduces unit tests to validate the migration behavior across MySQL/Postgres.

Changes:

  • Add migrateWebhookSecretKeyToAuthType(Handle) to v1130 MigrationUtil to rewrite webhook destination config from secretKeyauthType: { type: bearer, secretKey }.
  • Invoke the new migration from both MySQL and Postgres v1130 migration runners with error logging.
  • Add a v1130 migration unit test suite covering no-op and migration paths for MySQL/Postgres SQL updates.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
openmetadata-service/src/main/java/org/openmetadata/service/migration/utils/v1130/MigrationUtil.java Implements the webhook secretKeyauthType JSON rewrite + DB update logic.
openmetadata-service/src/main/java/org/openmetadata/service/migration/postgres/v1130/Migration.java Wires the new migration step into the Postgres v1130 data migration flow.
openmetadata-service/src/main/java/org/openmetadata/service/migration/mysql/v1130/Migration.java Wires the new migration step into the MySQL v1130 data migration flow.
openmetadata-service/src/test/java/org/openmetadata/service/migration/utils/v1130/MigrationUtilTest.java Adds tests validating the migration behavior and SQL selection for MySQL vs Postgres.

@gitar-bot
Copy link
Copy Markdown

gitar-bot bot commented Apr 16, 2026

Code Review ✅ Approved 1 resolved / 1 findings

Adds webhook secretKey support to the v1130 authType migration and includes a new idempotency test, addressing the previously missing coverage for already-migrated rows.

✅ 1 resolved
Edge Case: Missing idempotency test for already-migrated rows

📄 openmetadata-service/src/test/java/org/openmetadata/service/migration/utils/v1130/MigrationUtilTest.java:28-42
The PR summary states the migration is idempotent (rows with authType already set are skipped), but there's no test covering a row that already has the authType structure and no secretKey. Adding such a test would guard against regressions to the idempotency guarantee, which is the key safety property for users upgrading through different version paths (e.g., 1.12.5 → 1.12.6 → 1.13).

Options

Display: compact → Showing less information.

Comment with these commands to change:

Compact
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

@sonarqubecloud
Copy link
Copy Markdown

@yan-3005 yan-3005 enabled auto-merge (squash) April 16, 2026 15:56
@github-actions
Copy link
Copy Markdown
Contributor

🟡 Playwright Results — all passed (21 flaky)

✅ 3638 passed · ❌ 0 failed · 🟡 21 flaky · ⏭️ 89 skipped

Shard Passed Failed Flaky Skipped
🟡 Shard 1 477 0 3 4
🟡 Shard 2 645 0 2 7
🟡 Shard 3 646 0 6 1
🟡 Shard 4 621 0 5 27
✅ Shard 5 614 0 0 42
🟡 Shard 6 635 0 5 8
🟡 21 flaky test(s) (passed on retry)
  • Features/DataAssetRulesDisabled.spec.ts › Verify the ApiEndpoint entity item action after rules disabled (shard 1, 1 retry)
  • Features/DataAssetRulesDisabled.spec.ts › Verify the Database entity item action after rules disabled (shard 1, 1 retry)
  • Pages/UserCreationWithPersona.spec.ts › Create user with persona and verify on profile (shard 1, 1 retry)
  • Features/BulkEditEntity.spec.ts › Glossary (shard 2, 1 retry)
  • Features/ChangeSummaryBadge.spec.ts › Automated badge should appear on entity description with Automated source (shard 2, 1 retry)
  • Features/RestoreEntityInheritedFields.spec.ts › Validate restore with Inherited domain and data products assigned (shard 3, 2 retries)
  • Features/RestoreEntityInheritedFields.spec.ts › Validate restore with Inherited domain and data products assigned (shard 3, 1 retry)
  • Features/RestoreEntityInheritedFields.spec.ts › Validate restore with Inherited domain and data products assigned (shard 3, 1 retry)
  • Features/RestoreEntityInheritedFields.spec.ts › Validate restore with Inherited domain and data products assigned (shard 3, 1 retry)
  • Features/RestoreEntityInheritedFields.spec.ts › Validate restore with Inherited domain and data products assigned (shard 3, 1 retry)
  • Features/RTL.spec.ts › Verify Following widget functionality (shard 3, 1 retry)
  • Pages/Customproperties-part2.spec.ts › entityReferenceList shows item count, scrollable list, no expand toggle (shard 4, 1 retry)
  • Pages/DataContracts.spec.ts › Create Data Contract and validate for Table (shard 4, 2 retries)
  • Pages/DataContracts.spec.ts › Create Data Contract and validate for ApiEndpoint (shard 4, 1 retry)
  • Pages/Domains.spec.ts › Rename domain with assets (tables, topics, dashboards) preserves associations (shard 4, 1 retry)
  • Pages/Domains.spec.ts › Subdomain rename does not affect parent domain and updates nested children (shard 4, 1 retry)
  • Pages/HyperlinkCustomProperty.spec.ts › should accept valid http and https URLs (shard 6, 1 retry)
  • Pages/Lineage/DataAssetLineage.spec.ts › verify create lineage for entity - Table (shard 6, 1 retry)
  • Pages/Lineage/LineageFilters.spec.ts › Verify lineage schema filter selection (shard 6, 1 retry)
  • Pages/Lineage/LineageRightPanel.spec.ts › Verify custom properties tab IS visible for supported type: searchIndex (shard 6, 1 retry)
  • Pages/Users.spec.ts › Permissions for table details page for Data Consumer (shard 6, 1 retry)

📦 Download artifacts

How to debug locally
# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip    # view trace

@yan-3005 yan-3005 merged commit cc06887 into main Apr 16, 2026
68 of 71 checks passed
@yan-3005 yan-3005 deleted the ram/webhook-migrations branch April 16, 2026 17:02
yan-3005 added a commit that referenced this pull request Apr 16, 2026
…#27438)

* feat(migration): add webhook secretKey to authType migration in v1130

* test(migration): add idempotency test for already-migrated webhook rows in v1130

(cherry picked from commit cc06887)
siddhant1 pushed a commit that referenced this pull request Apr 17, 2026
…#27438)

* feat(migration): add webhook secretKey to authType migration in v1130

* test(migration): add idempotency test for already-migrated webhook rows in v1130
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend safe to test Add this label to run secure Github workflows on PRs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants