Skip to content

Commit 2eafb6c

Browse files
fix(system-update): sql setup regression (#15320)
Co-authored-by: David Leifker <david.leifker@acryl.io>
1 parent cdb403e commit 2eafb6c

File tree

10 files changed

+68
-43
lines changed

10 files changed

+68
-43
lines changed

.github/workflows/docker-unified-nightly.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -323,6 +323,7 @@ jobs:
323323
TEST_STRATEGY: ${{ matrix.test_strategy }}
324324
BATCH_COUNT: "1" # since this workflow runs only on schedule trigger, batching isn't really needed.
325325
BATCH_NUMBER: "0"
326+
PROFILE_NAME: ${{ matrix.profile }}
326327
run: |
327328
echo "$DATAHUB_VERSION"
328329
./gradlew --stop

datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/sqlsetup/CreateCdcUserStep.java

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -80,10 +80,12 @@ SqlSetupResult createCdcUser(SqlSetupArgs args) throws SQLException {
8080
stmt.executeUpdate();
8181
}
8282

83-
String grantCdcPrivilegesSql =
83+
java.util.List<String> grantCdcPrivilegesStmts =
8484
dbOps.grantCdcPrivilegesSql(args.getCdcUser(), args.getDatabaseName());
85-
try (PreparedStatement grantStmt = connection.prepareStatement(grantCdcPrivilegesSql)) {
86-
grantStmt.executeUpdate();
85+
for (String grantSql : grantCdcPrivilegesStmts) {
86+
try (PreparedStatement grantStmt = connection.prepareStatement(grantSql)) {
87+
grantStmt.executeUpdate();
88+
}
8789
}
8890

8991
result.setCdcUserCreated(true);

datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/sqlsetup/DatabaseOperations.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -88,13 +88,14 @@ static DatabaseOperations create(DatabaseType dbType) {
8888
String createCdcUserSql(String cdcUser, String cdcPassword);
8989

9090
/**
91-
* Generate SQL for granting CDC privileges to a user.
91+
* Generate SQL statements for granting CDC privileges to a user. Returns a list of individual SQL
92+
* statements that can be executed separately.
9293
*
9394
* @param cdcUser the CDC username
9495
* @param databaseName the database name
95-
* @return the SQL statement for granting CDC privileges
96+
* @return list of SQL statements for granting CDC privileges
9697
*/
97-
String grantCdcPrivilegesSql(String cdcUser, String databaseName);
98+
java.util.List<String> grantCdcPrivilegesSql(String cdcUser, String databaseName);
9899

99100
/**
100101
* Generate SQL statements for creating the metadata_aspect_v2 table and its indexes. Returns a

datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/sqlsetup/MySqlDatabaseOperations.java

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -78,17 +78,18 @@ public String createCdcUserSql(String cdcUser, String cdcPassword) {
7878
}
7979

8080
@Override
81-
public String grantCdcPrivilegesSql(String cdcUser, String databaseName) {
81+
public java.util.List<String> grantCdcPrivilegesSql(String cdcUser, String databaseName) {
8282
// MySQL - comprehensive CDC privileges (matching original init-cdc.sql)
83-
return String.format(
84-
"""
85-
GRANT SELECT ON `%s`.* TO '%s'@'%%';
86-
GRANT RELOAD ON *.* TO '%s'@'%%';
87-
GRANT REPLICATION CLIENT ON *.* TO '%s'@'%%';
88-
GRANT REPLICATION SLAVE ON *.* TO '%s'@'%%';
89-
FLUSH PRIVILEGES;
90-
""",
91-
databaseName, cdcUser, cdcUser, cdcUser, cdcUser);
83+
// Return as separate statements since JDBC doesn't support multiple statements in one execution
84+
String escapedUser = escapeMysqlStringLiteral(cdcUser);
85+
String escapedDatabase = escapeMysqlIdentifier(databaseName);
86+
87+
return java.util.Arrays.asList(
88+
String.format("GRANT SELECT ON %s.* TO %s@'%%'", escapedDatabase, escapedUser),
89+
String.format("GRANT RELOAD ON *.* TO %s@'%%'", escapedUser),
90+
String.format("GRANT REPLICATION CLIENT ON *.* TO %s@'%%'", escapedUser),
91+
String.format("GRANT REPLICATION SLAVE ON *.* TO %s@'%%'", escapedUser),
92+
"FLUSH PRIVILEGES");
9293
}
9394

9495
@Override

datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/sqlsetup/PostgresDatabaseOperations.java

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -104,21 +104,23 @@ IF NOT EXISTS (SELECT FROM pg_catalog.pg_roles WHERE rolname = %s) THEN
104104
}
105105

106106
@Override
107-
public String grantCdcPrivilegesSql(String cdcUser, String databaseName) {
107+
public java.util.List<String> grantCdcPrivilegesSql(String cdcUser, String databaseName) {
108108
// PostgreSQL comprehensive CDC privileges (matching original init-cdc.sql)
109-
return String.format(
110-
"""
111-
GRANT CONNECT ON DATABASE "%s" TO "%s";
112-
GRANT USAGE ON SCHEMA public TO "%s";
113-
GRANT CREATE ON DATABASE "%s" TO "%s";
114-
GRANT SELECT ON ALL TABLES IN SCHEMA public TO "%s";
115-
ALTER DEFAULT PRIVILEGES IN SCHEMA public GRANT SELECT ON TABLES TO "%s";
116-
ALTER USER "%s" WITH SUPERUSER;
117-
ALTER TABLE public.metadata_aspect_v2 OWNER TO "%s";
118-
ALTER TABLE public.metadata_aspect_v2 REPLICA IDENTITY FULL;
119-
CREATE PUBLICATION dbz_publication FOR TABLE public.metadata_aspect_v2;
120-
""",
121-
databaseName, cdcUser, cdcUser, databaseName, cdcUser, cdcUser, cdcUser, cdcUser, cdcUser);
109+
// Return as separate statements since JDBC doesn't support multiple statements in one execution
110+
String escapedUser = escapePostgresIdentifier(cdcUser);
111+
String escapedDatabase = escapePostgresIdentifier(databaseName);
112+
113+
return java.util.Arrays.asList(
114+
String.format("GRANT CONNECT ON DATABASE %s TO %s", escapedDatabase, escapedUser),
115+
String.format("GRANT USAGE ON SCHEMA public TO %s", escapedUser),
116+
String.format("GRANT CREATE ON DATABASE %s TO %s", escapedDatabase, escapedUser),
117+
String.format("GRANT SELECT ON ALL TABLES IN SCHEMA public TO %s", escapedUser),
118+
String.format(
119+
"ALTER DEFAULT PRIVILEGES IN SCHEMA public GRANT SELECT ON TABLES TO %s", escapedUser),
120+
String.format("ALTER USER %s WITH SUPERUSER", escapedUser),
121+
String.format("ALTER TABLE public.metadata_aspect_v2 OWNER TO %s", escapedUser),
122+
"ALTER TABLE public.metadata_aspect_v2 REPLICA IDENTITY FULL",
123+
"CREATE PUBLICATION dbz_publication FOR TABLE public.metadata_aspect_v2");
122124
}
123125

124126
@Override

datahub-upgrade/src/test/java/com/linkedin/datahub/upgrade/sqlsetup/CreateCdcUserStepTest.java

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -478,9 +478,10 @@ public void testCreateCdcUserSuccess() throws SQLException {
478478
assertEquals(result.isCdcUserCreated(), true);
479479
assertTrue(result.getExecutionTimeMs() >= 0);
480480
// Verify PreparedStatement approach - should call dataSource() and prepareStatement()
481+
// MySQL: 1 createCdcUser + 5 grantCdcPrivileges statements = 6 total
481482
verify(mockDatabase).dataSource();
482-
verify(mockConnection, times(2)).prepareStatement(anyString());
483-
verify(mockPreparedStatement, times(2)).executeUpdate();
483+
verify(mockConnection, times(6)).prepareStatement(anyString());
484+
verify(mockPreparedStatement, times(6)).executeUpdate();
484485
}
485486

486487
@Test
@@ -533,9 +534,10 @@ public void testCreateCdcUserWithPostgresComplexSql() throws SQLException {
533534
assertEquals(result.isCdcUserCreated(), true);
534535
assertTrue(result.getExecutionTimeMs() >= 0);
535536
// Verify PreparedStatement approach - should call dataSource() and prepareStatement()
537+
// MySQL: 1 createCdcUser + 5 grantCdcPrivileges statements = 6 total
536538
verify(mockDatabase).dataSource();
537-
verify(mockConnection, times(2)).prepareStatement(anyString());
538-
verify(mockPreparedStatement, times(2)).executeUpdate();
539+
verify(mockConnection, times(6)).prepareStatement(anyString());
540+
verify(mockPreparedStatement, times(6)).executeUpdate();
539541
}
540542

541543
@Test
@@ -562,8 +564,9 @@ public void testCreateCdcUserWithMysqlSimpleSql() throws SQLException {
562564
assertEquals(result.isCdcUserCreated(), true);
563565
assertTrue(result.getExecutionTimeMs() >= 0);
564566
// Verify PreparedStatement approach - should call dataSource() and prepareStatement()
567+
// MySQL: 1 createCdcUser + 5 grantCdcPrivileges statements = 6 total
565568
verify(mockDatabase).dataSource();
566-
verify(mockConnection, times(2)).prepareStatement(anyString());
567-
verify(mockPreparedStatement, times(2)).executeUpdate();
569+
verify(mockConnection, times(6)).prepareStatement(anyString());
570+
verify(mockPreparedStatement, times(6)).executeUpdate();
568571
}
569572
}

datahub-upgrade/src/test/java/com/linkedin/datahub/upgrade/sqlsetup/DatabaseOperationsTest.java

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -113,16 +113,22 @@ public void testMysqlCreateCdcUserSql() {
113113

114114
@Test
115115
public void testPostgresGrantCdcPrivilegesSql() {
116-
String result = postgresOps.grantCdcPrivilegesSql("cdcuser", "testdb");
117-
assertTrue(result.contains("GRANT CONNECT ON DATABASE \"testdb\" TO \"cdcuser\""));
118-
assertTrue(result.contains("CREATE PUBLICATION dbz_publication"));
116+
java.util.List<String> statements = postgresOps.grantCdcPrivilegesSql("cdcuser", "testdb");
117+
assertNotNull(statements);
118+
assertTrue(statements.size() > 0);
119+
String allStatements = String.join(" ", statements);
120+
assertTrue(allStatements.contains("GRANT CONNECT ON DATABASE \"testdb\" TO \"cdcuser\""));
121+
assertTrue(allStatements.contains("CREATE PUBLICATION dbz_publication"));
119122
}
120123

121124
@Test
122125
public void testMysqlGrantCdcPrivilegesSql() {
123-
String result = mysqlOps.grantCdcPrivilegesSql("cdcuser", "testdb");
124-
assertTrue(result.contains("GRANT SELECT ON `testdb`.* TO 'cdcuser'@'%'"));
125-
assertTrue(result.contains("GRANT REPLICATION CLIENT ON *.* TO 'cdcuser'@'%'"));
126+
java.util.List<String> statements = mysqlOps.grantCdcPrivilegesSql("cdcuser", "testdb");
127+
assertNotNull(statements);
128+
assertTrue(statements.size() > 0);
129+
String allStatements = String.join(" ", statements);
130+
assertTrue(allStatements.contains("GRANT SELECT ON `testdb`.* TO 'cdcuser'@'%'"));
131+
assertTrue(allStatements.contains("GRANT REPLICATION CLIENT ON *.* TO 'cdcuser'@'%'"));
126132
}
127133

128134
@Test

docker/profiles/docker-compose.gms.yml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,11 @@ x-datahub-system-update-service: &datahub-system-update-service
7676
- ${DATAHUB_LOCAL_SYS_UPDATE_ENV:-empty2.env}
7777
environment: &datahub-system-update-env
7878
<<: [*primary-datastore-mysql-env, *graph-datastore-search-env, *search-datastore-env, *kafka-env, *datahub-common-env]
79+
EBEAN_DATASOURCE_USERNAME: ${DATAHUB_EBEAN_DATASOURCE_ROOT_USERNAME:-root}
80+
EBEAN_DATASOURCE_PASSWORD: ${DATAHUB_EBEAN_DATASOURCE_ROOT_PASSWORD:-datahub}
81+
CREATE_USER: ${DATAHUB_CREATE_USER:-false}
82+
CREATE_USER_USERNAME: ${DATAHUB_CREATE_USER_USERNAME:-datahub}
83+
CREATE_USER_PASSWORD: ${DATAHUB_CREATE_USER_PASSWORD:-datahub}
7984
SCHEMA_REGISTRY_SYSTEM_UPDATE: ${SCHEMA_REGISTRY_SYSTEM_UPDATE:-true}
8085
SPRING_KAFKA_PROPERTIES_AUTO_REGISTER_SCHEMAS: ${SPRING_KAFKA_PROPERTIES_AUTO_REGISTER_SCHEMAS:-true}
8186
SPRING_KAFKA_PROPERTIES_USE_LATEST_VERSION: ${SPRING_KAFKA_PROPERTIES_USE_LATEST_VERSION:-true}

docker/quickstart/docker-compose.quickstart-profile.yml

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -406,6 +406,9 @@ services:
406406
required: true
407407
environment:
408408
BACKFILL_BROWSE_PATHS_V2: 'true'
409+
CREATE_USER: 'false'
410+
CREATE_USER_PASSWORD: datahub
411+
CREATE_USER_USERNAME: datahub
409412
DATAHUB_BASE_PATH: /
410413
DATAHUB_GMS_BASE_PATH: /
411414
DATAHUB_GMS_HOST: datahub-gms
@@ -416,7 +419,7 @@ services:
416419
EBEAN_DATASOURCE_HOST: mysql:3306
417420
EBEAN_DATASOURCE_PASSWORD: datahub
418421
EBEAN_DATASOURCE_URL: jdbc:mysql://mysql:3306/datahub?verifyServerCertificate=false&useSSL=true&useUnicode=yes&characterEncoding=UTF-8&enabledTLSProtocols=TLSv1.2
419-
EBEAN_DATASOURCE_USERNAME: datahub
422+
EBEAN_DATASOURCE_USERNAME: root
420423
ELASTICSEARCH_BUILD_INDICES_CLONE_INDICES: 'false'
421424
ELASTICSEARCH_HOST: search
422425
ELASTICSEARCH_IMPLEMENTATION: opensearch

docs/how/updating-datahub.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
99
- #13726: Default search results per page (new: 5000, old: 10000) can be configured with environment variable `ELASTICSEARCH_LIMIT_RESULTS_API_DEFAULT`
1010
- #13726: Maximum lineage visualization hops (new: 20, old: 1000) can be configured with environment variable `ELASTICSEARCH_SEARCH_GRAPH_LINEAGE_MAX_HOPS`
11+
- #15044: If using the new system-update sqlSetup, the system-update job must be granted privileges to create users, databases, and tables.
1112
1213
### Known Issues
1314

0 commit comments

Comments
 (0)