Skip to content

Commit cc06887

Browse files
yan-3005yan-3005
authored
feat(migration): add webhook secretKey to authType migration in v1130 (#27438)
* feat(migration): add webhook secretKey to authType migration in v1130 * test(migration): add idempotency test for already-migrated webhook rows in v1130
1 parent 077982c commit cc06887

4 files changed

Lines changed: 326 additions & 0 deletions

File tree

‎openmetadata-service/src/main/java/org/openmetadata/service/migration/mysql/v1130/Migration.java‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
11
package org.openmetadata.service.migration.mysql.v1130;
22

33
import lombok.SneakyThrows;
4+
import lombok.extern.slf4j.Slf4j;
45
import org.openmetadata.service.migration.api.MigrationProcessImpl;
56
import org.openmetadata.service.migration.utils.MigrationFile;
67
import org.openmetadata.service.migration.utils.v1130.MigrationUtil;
78

9+
@Slf4j
810
public class Migration extends MigrationProcessImpl {
911

1012
public Migration(MigrationFile migrationFile) {
@@ -15,5 +17,13 @@ public Migration(MigrationFile migrationFile) {
1517
@SneakyThrows
1618
public void runDataMigration() {
1719
MigrationUtil.updateOwnerChartFormulas();
20+
try {
21+
MigrationUtil.migrateWebhookSecretKeyToAuthType(handle);
22+
} catch (Exception e) {
23+
LOG.error(
24+
"Failed to migrate webhook secretKey to authType in v1130 migration. "
25+
+ "Webhook authentication may not work correctly until re-saved.",
26+
e);
27+
}
1828
}
1929
}

‎openmetadata-service/src/main/java/org/openmetadata/service/migration/postgres/v1130/Migration.java‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
11
package org.openmetadata.service.migration.postgres.v1130;
22

33
import lombok.SneakyThrows;
4+
import lombok.extern.slf4j.Slf4j;
45
import org.openmetadata.service.migration.api.MigrationProcessImpl;
56
import org.openmetadata.service.migration.utils.MigrationFile;
67
import org.openmetadata.service.migration.utils.v1130.MigrationUtil;
78

9+
@Slf4j
810
public class Migration extends MigrationProcessImpl {
911

1012
public Migration(MigrationFile migrationFile) {
@@ -15,5 +17,13 @@ public Migration(MigrationFile migrationFile) {
1517
@SneakyThrows
1618
public void runDataMigration() {
1719
MigrationUtil.updateOwnerChartFormulas();
20+
try {
21+
MigrationUtil.migrateWebhookSecretKeyToAuthType(handle);
22+
} catch (Exception e) {
23+
LOG.error(
24+
"Failed to migrate webhook secretKey to authType in v1130 migration. "
25+
+ "Webhook authentication may not work correctly until re-saved.",
26+
e);
27+
}
1828
}
1929
}

‎openmetadata-service/src/main/java/org/openmetadata/service/migration/utils/v1130/MigrationUtil.java‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,26 @@
11
package org.openmetadata.service.migration.utils.v1130;
22

3+
import com.fasterxml.jackson.databind.JsonNode;
4+
import com.fasterxml.jackson.databind.node.ObjectNode;
5+
import java.util.List;
6+
import java.util.Map;
37
import lombok.extern.slf4j.Slf4j;
8+
import org.jdbi.v3.core.Handle;
49
import org.openmetadata.schema.dataInsight.custom.DataInsightCustomChart;
10+
import org.openmetadata.schema.entity.events.SubscriptionDestination;
11+
import org.openmetadata.schema.utils.JsonUtils;
512
import org.openmetadata.service.jdbi3.DataInsightSystemChartRepository;
13+
import org.openmetadata.service.resources.databases.DatasourceConfig;
614
import org.openmetadata.service.util.EntityUtil;
715

816
@Slf4j
917
public class MigrationUtil {
1018
private MigrationUtil() {}
1119

20+
private static final String UPDATE_MYSQL =
21+
"UPDATE event_subscription_entity SET json = :json WHERE id = :id";
22+
private static final String UPDATE_POSTGRES =
23+
"UPDATE event_subscription_entity SET json = :json::jsonb WHERE id = :id";
1224
private static final String OLD_FIELD = "owners.name.keyword";
1325
private static final String NEW_FIELD = "ownerName";
1426

@@ -48,4 +60,73 @@ public static void updateOwnerChartFormulas() {
4860
}
4961
}
5062
}
63+
64+
public static void migrateWebhookSecretKeyToAuthType(Handle handle) {
65+
LOG.info("Starting migration of webhook secretKey to authType");
66+
List<Map<String, Object>> rows =
67+
handle.createQuery("SELECT id, json FROM event_subscription_entity").mapToMap().list();
68+
int migratedCount = 0;
69+
70+
for (Map<String, Object> row : rows) {
71+
String id = row.get("id").toString();
72+
String jsonStr = row.get("json").toString();
73+
74+
try {
75+
ObjectNode root = (ObjectNode) JsonUtils.readTree(jsonStr);
76+
JsonNode destinations = root.get("destinations");
77+
if (destinations == null || !destinations.isArray()) {
78+
continue;
79+
}
80+
81+
boolean modified = false;
82+
for (JsonNode destination : destinations) {
83+
String type =
84+
destination.get("type") != null
85+
? destination.get("type").asText().toLowerCase()
86+
: null;
87+
if (!SubscriptionDestination.SubscriptionType.WEBHOOK
88+
.value()
89+
.toLowerCase()
90+
.equals(type)) {
91+
continue;
92+
}
93+
94+
JsonNode config = destination.get("config");
95+
if (config == null || !config.isObject()) {
96+
continue;
97+
}
98+
99+
JsonNode secretKeyNode = config.get("secretKey");
100+
if (secretKeyNode == null
101+
|| secretKeyNode.isNull()
102+
|| secretKeyNode.asText().trim().isEmpty()) {
103+
continue;
104+
}
105+
106+
ObjectNode configObj = (ObjectNode) config;
107+
ObjectNode bearerAuth =
108+
JsonUtils.getObjectMapper()
109+
.createObjectNode()
110+
.put("type", "bearer")
111+
.put("secretKey", secretKeyNode.asText());
112+
configObj.set("authType", bearerAuth);
113+
configObj.remove("secretKey");
114+
modified = true;
115+
}
116+
117+
if (modified) {
118+
String updateSql =
119+
Boolean.TRUE.equals(DatasourceConfig.getInstance().isMySQL())
120+
? UPDATE_MYSQL
121+
: UPDATE_POSTGRES;
122+
handle.createUpdate(updateSql).bind("json", root.toString()).bind("id", id).execute();
123+
migratedCount++;
124+
}
125+
} catch (Exception e) {
126+
LOG.warn("Error migrating event subscription {}", id, e);
127+
}
128+
}
129+
130+
LOG.info("Migrated {} event subscriptions with secretKey to authType", migratedCount);
131+
}
51132
}

‎openmetadata-service/src/test/java/org/openmetadata/service/migration/utils/v1130/MigrationUtilTest.java‎

Lines changed: 225 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,225 @@
1+
package org.openmetadata.service.migration.utils.v1130;
2+
3+
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
4+
import static org.junit.jupiter.api.Assertions.assertEquals;
5+
import static org.junit.jupiter.api.Assertions.assertNotNull;
6+
import static org.junit.jupiter.api.Assertions.assertNull;
7+
import static org.mockito.Answers.RETURNS_DEEP_STUBS;
8+
import static org.mockito.ArgumentMatchers.any;
9+
import static org.mockito.ArgumentMatchers.eq;
10+
import static org.mockito.Mockito.mock;
11+
import static org.mockito.Mockito.mockStatic;
12+
import static org.mockito.Mockito.never;
13+
import static org.mockito.Mockito.verify;
14+
import static org.mockito.Mockito.when;
15+
16+
import com.fasterxml.jackson.databind.JsonNode;
17+
import com.fasterxml.jackson.databind.ObjectMapper;
18+
import java.util.List;
19+
import java.util.Map;
20+
import java.util.UUID;
21+
import org.jdbi.v3.core.Handle;
22+
import org.jdbi.v3.core.statement.Update;
23+
import org.junit.jupiter.api.Test;
24+
import org.mockito.ArgumentCaptor;
25+
import org.mockito.MockedStatic;
26+
import org.openmetadata.service.resources.databases.DatasourceConfig;
27+
28+
class MigrationUtilTest {
29+
30+
private static final ObjectMapper MAPPER = new ObjectMapper();
31+
32+
private static final String WEBHOOK_WITH_SECRET_KEY =
33+
"""
34+
{
35+
"destinations": [
36+
{
37+
"type": "Webhook",
38+
"config": {
39+
"endpoint": "https://example.com/hook",
40+
"secretKey": "mysecret"
41+
}
42+
}
43+
]
44+
}
45+
""";
46+
47+
private static final String WEBHOOK_WITHOUT_SECRET_KEY =
48+
"""
49+
{
50+
"destinations": [
51+
{
52+
"type": "Webhook",
53+
"config": {
54+
"endpoint": "https://example.com/hook"
55+
}
56+
}
57+
]
58+
}
59+
""";
60+
61+
private static final String WEBHOOK_WITH_EMPTY_SECRET_KEY =
62+
"""
63+
{
64+
"destinations": [
65+
{
66+
"type": "Webhook",
67+
"config": {
68+
"endpoint": "https://example.com/hook",
69+
"secretKey": ""
70+
}
71+
}
72+
]
73+
}
74+
""";
75+
76+
private static final String WEBHOOK_ALREADY_MIGRATED =
77+
"""
78+
{
79+
"destinations": [
80+
{
81+
"type": "Webhook",
82+
"config": {
83+
"endpoint": "https://example.com/hook",
84+
"authType": { "type": "bearer", "secretKey": "mysecret" }
85+
}
86+
}
87+
]
88+
}
89+
""";
90+
91+
private static final String NON_WEBHOOK_DESTINATION =
92+
"""
93+
{
94+
"destinations": [
95+
{
96+
"type": "Slack",
97+
"config": {
98+
"secretKey": "mysecret"
99+
}
100+
}
101+
]
102+
}
103+
""";
104+
105+
private Map<String, Object> row(String json) {
106+
return Map.of("id", UUID.randomUUID().toString(), "json", json);
107+
}
108+
109+
private Handle handleReturningRows(List<Map<String, Object>> rows) {
110+
Handle handle = mock(Handle.class, RETURNS_DEEP_STUBS);
111+
when(handle.createQuery(any(String.class)).mapToMap().list()).thenReturn(rows);
112+
return handle;
113+
}
114+
115+
private Handle handleWithUpdateCapture(List<Map<String, Object>> rows, Update mockUpdate) {
116+
Handle handle = mock(Handle.class, RETURNS_DEEP_STUBS);
117+
when(handle.createQuery(any(String.class)).mapToMap().list()).thenReturn(rows);
118+
when(handle.createUpdate(any(String.class))).thenReturn(mockUpdate);
119+
return handle;
120+
}
121+
122+
@Test
123+
void migrateWebhookSecretKeyToAuthTypeIsNoOpWhenNoRows() {
124+
Handle handle = handleReturningRows(List.of());
125+
126+
assertDoesNotThrow(() -> MigrationUtil.migrateWebhookSecretKeyToAuthType(handle));
127+
128+
verify(handle, never()).createUpdate(any());
129+
}
130+
131+
@Test
132+
void migrateWebhookSecretKeyToAuthTypeIsNoOpWhenNoSecretKey() {
133+
Handle handle = handleReturningRows(List.of(row(WEBHOOK_WITHOUT_SECRET_KEY)));
134+
135+
assertDoesNotThrow(() -> MigrationUtil.migrateWebhookSecretKeyToAuthType(handle));
136+
137+
verify(handle, never()).createUpdate(any());
138+
}
139+
140+
@Test
141+
void migrateWebhookSecretKeyToAuthTypeIsNoOpWhenEmptySecretKey() {
142+
Handle handle = handleReturningRows(List.of(row(WEBHOOK_WITH_EMPTY_SECRET_KEY)));
143+
144+
assertDoesNotThrow(() -> MigrationUtil.migrateWebhookSecretKeyToAuthType(handle));
145+
146+
verify(handle, never()).createUpdate(any());
147+
}
148+
149+
@Test
150+
void migrateWebhookSecretKeyToAuthTypeIsNoOpWhenAlreadyMigrated() {
151+
Handle handle = handleReturningRows(List.of(row(WEBHOOK_ALREADY_MIGRATED)));
152+
153+
assertDoesNotThrow(() -> MigrationUtil.migrateWebhookSecretKeyToAuthType(handle));
154+
155+
verify(handle, never()).createUpdate(any());
156+
}
157+
158+
@Test
159+
void migrateWebhookSecretKeyToAuthTypeIsNoOpForNonWebhookDestinations() {
160+
Handle handle = handleReturningRows(List.of(row(NON_WEBHOOK_DESTINATION)));
161+
162+
assertDoesNotThrow(() -> MigrationUtil.migrateWebhookSecretKeyToAuthType(handle));
163+
164+
verify(handle, never()).createUpdate(any());
165+
}
166+
167+
@Test
168+
void migrateWebhookSecretKeyToAuthTypeMigratesMysql() throws Exception {
169+
Update mockUpdate = mock(Update.class, RETURNS_DEEP_STUBS);
170+
Handle handle = handleWithUpdateCapture(List.of(row(WEBHOOK_WITH_SECRET_KEY)), mockUpdate);
171+
172+
try (MockedStatic<DatasourceConfig> ds = mockStatic(DatasourceConfig.class)) {
173+
DatasourceConfig mockConfig = mock(DatasourceConfig.class);
174+
ds.when(DatasourceConfig::getInstance).thenReturn(mockConfig);
175+
when(mockConfig.isMySQL()).thenReturn(true);
176+
177+
assertDoesNotThrow(() -> MigrationUtil.migrateWebhookSecretKeyToAuthType(handle));
178+
179+
ArgumentCaptor<String> sqlCaptor = ArgumentCaptor.forClass(String.class);
180+
verify(handle).createUpdate(sqlCaptor.capture());
181+
assertEquals(
182+
"UPDATE event_subscription_entity SET json = :json WHERE id = :id", sqlCaptor.getValue());
183+
184+
ArgumentCaptor<String> jsonCaptor = ArgumentCaptor.forClass(String.class);
185+
verify(mockUpdate).bind(eq("json"), jsonCaptor.capture());
186+
187+
JsonNode config =
188+
MAPPER.readTree(jsonCaptor.getValue()).get("destinations").get(0).get("config");
189+
assertNull(config.get("secretKey"));
190+
assertNotNull(config.get("authType"));
191+
assertEquals("bearer", config.get("authType").get("type").asText());
192+
assertEquals("mysecret", config.get("authType").get("secretKey").asText());
193+
}
194+
}
195+
196+
@Test
197+
void migrateWebhookSecretKeyToAuthTypeMigratesPostgres() throws Exception {
198+
Update mockUpdate = mock(Update.class, RETURNS_DEEP_STUBS);
199+
Handle handle = handleWithUpdateCapture(List.of(row(WEBHOOK_WITH_SECRET_KEY)), mockUpdate);
200+
201+
try (MockedStatic<DatasourceConfig> ds = mockStatic(DatasourceConfig.class)) {
202+
DatasourceConfig mockConfig = mock(DatasourceConfig.class);
203+
ds.when(DatasourceConfig::getInstance).thenReturn(mockConfig);
204+
when(mockConfig.isMySQL()).thenReturn(false);
205+
206+
assertDoesNotThrow(() -> MigrationUtil.migrateWebhookSecretKeyToAuthType(handle));
207+
208+
ArgumentCaptor<String> sqlCaptor = ArgumentCaptor.forClass(String.class);
209+
verify(handle).createUpdate(sqlCaptor.capture());
210+
assertEquals(
211+
"UPDATE event_subscription_entity SET json = :json::jsonb WHERE id = :id",
212+
sqlCaptor.getValue());
213+
214+
ArgumentCaptor<String> jsonCaptor = ArgumentCaptor.forClass(String.class);
215+
verify(mockUpdate).bind(eq("json"), jsonCaptor.capture());
216+
217+
JsonNode config =
218+
MAPPER.readTree(jsonCaptor.getValue()).get("destinations").get(0).get("config");
219+
assertNull(config.get("secretKey"));
220+
assertNotNull(config.get("authType"));
221+
assertEquals("bearer", config.get("authType").get("type").asText());
222+
assertEquals("mysecret", config.get("authType").get("secretKey").asText());
223+
}
224+
}
225+
}

0 commit comments

Comments
 (0)