Merge pull request #3698 from cloudfoundry/flyway-upgrade

duanemay · web-flow · commit 7e3740d294ff · 2026-01-07T17:13:24.000-05:00
Flyway upgrade
diff --git a/dependencies.gradle b/dependencies.gradle
@@ -15,9 +15,6 @@ versions.braveVersion = "6.3.0"
 versions.opensaml = "4.0.1"
 
 // Versions we're overriding from the Spring Boot Bom (Dependabot does not issue PRs to bump these versions, so we need to manually bump them)
-ext["mariadb.version"] = "2.7.12" // Bumping to v3 breaks some pipeline jobs (and compatibility with Amazon Aurora MySQL), so pinning to v2 for now. v2 (current version) is stable and will be supported until about September 2025 (https://mariadb.com/kb/en/about-mariadb-connector-j/).
-ext["flyway.version"] = "7.15.0" // the next major (v8)'s community edition drops support with MySQL 5.7, which UAA still needs to support. Can bump to v8 once we solve this issue.
-ext["hsqldb.version"] = "2.7.4" // HSQL-DB used for tests but not supported for productive usage
 ext["selenium.version"] = "${versions.seleniumVersion}" // Selenium for integration tests only
 
 // Dependencies (some rely on shared versions, some are shared between projects)
@@ -37,6 +34,9 @@ libraries.commonsIo = "commons-io:commons-io:2.20.0"
 libraries.dumbster = "dumbster:dumbster:1.6"
 libraries.eclipseJgit = "org.eclipse.jgit:org.eclipse.jgit:7.3.0.202506031305-r"
 libraries.flywayCore = "org.flywaydb:flyway-core"
+libraries.flywayHsqlDb = "org.flywaydb:flyway-database-hsqldb"
+libraries.flywayMySql = "org.flywaydb:flyway-mysql"
+libraries.flywayPostgresql = "org.flywaydb:flyway-database-postgresql"
 libraries.greenmail = "com.icegreen:greenmail:2.1.5"
 libraries.guava = "com.google.guava:guava:${versions.guavaVersion}"
 libraries.guavaTestLib = "com.google.guava:guava-testlib:${versions.guavaVersion}"
diff --git a/server/build.gradle b/server/build.gradle
@@ -60,8 +60,9 @@ dependencies {
     }
 
     implementation(libraries.hibernateValidator)
-    implementation(libraries.flywayCore)
-    implementation(libraries.mariaJdbcDriver)
+    implementation(libraries.flywayHsqlDb)
+    implementation(libraries.flywayMySql)
+    implementation(libraries.flywayPostgresql)
     implementation(libraries.hsqldb)
 
     implementation(libraries.snakeyaml)
@@ -94,6 +95,7 @@ dependencies {
     testImplementation(libraries.mockitoJunit5)
 
     testImplementation(libraries.postgresql)
+    testImplementation(libraries.mariaJdbcDriver)
 
     testImplementation(libraries.tomcatElApi)
     testImplementation(libraries.tomcatJasperEl)
diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/db/DatabaseInformation1_5_3.java b/server/src/main/java/org/cloudfoundry/identity/uaa/db/DatabaseInformation1_5_3.java
@@ -1,6 +1,6 @@
 /*
  * *****************************************************************************
- *     Cloud Foundry 
+ *     Cloud Foundry
  *     Copyright (c) [2009-2016] Pivotal Software, Inc. All Rights Reserved.
  *
  *     This product is licensed to you under the Apache License, Version 2.0 (the "License").
@@ -13,20 +13,18 @@
  *******************************************************************************/
 package org.cloudfoundry.identity.uaa.db;
 
+import org.springframework.jdbc.core.RowMapper;
+
 import java.sql.ResultSet;
 import java.sql.SQLException;
-import java.util.Arrays;
-import java.util.Collections;
 import java.util.List;
 
-import org.springframework.jdbc.core.RowMapper;
-
 /**
  * Created by fhanik on 3/5/14.
  */
 public class DatabaseInformation1_5_3 {
 
-    public static List<String> tableNames = Collections.unmodifiableList(Arrays.asList(
+    public static final List<String> tableNames = List.of(
             "users",
             "sec_audit",
             "oauth_client_details",
@@ -35,8 +33,7 @@ public class DatabaseInformation1_5_3 {
             "oauth_code",
             "authz_approvals",
             "external_group_mapping"
-
-    ));
+    );
 
     public static boolean processColumn(ColumnInfo column) {
         return (!column.columnName.equals(column.columnName.toLowerCase())) &&
diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/db/beans/DatabaseConfiguration.java b/server/src/main/java/org/cloudfoundry/identity/uaa/db/beans/DatabaseConfiguration.java
@@ -5,6 +5,7 @@
 import org.cloudfoundry.identity.uaa.resources.jdbc.LimitSqlAdapter;
 import org.cloudfoundry.identity.uaa.resources.jdbc.MySqlLimitSqlAdapter;
 import org.cloudfoundry.identity.uaa.resources.jdbc.PostgresLimitSqlAdapter;
+import org.jspecify.annotations.NonNull;
 import org.springframework.boot.autoconfigure.jdbc.DataSourceTransactionManagerAutoConfiguration;
 import org.springframework.boot.autoconfigure.jdbc.JdbcTemplateAutoConfiguration;
 import org.springframework.boot.autoconfigure.transaction.TransactionAutoConfiguration;
@@ -34,7 +35,7 @@
  * can be overridden by users, or in {@link DatabasePlatform} when they are static.
  * <p>
  * Note that we reference property sources directly here, without relying on Boot auto-discovery. We do this so
- * that all configuration is visible from a single place.
+ * that all configurations are visible from a single place.
  * <p>
  * The following beans and configurations are wired by Spring Boot auto-configuration.
  * <p>
@@ -93,11 +94,40 @@ JdbcUrlCustomizer jdbcUrlAddTimeoutCustomizer(DatabaseProperties databasePropert
         return url -> {
             DatabasePlatform databasePlatform = databaseProperties.getDatabasePlatform();
             var timeout = Duration.ofSeconds(databaseProperties.getConnecttimeout());
-            String connectorCharacter = url.contains("?") ? "&" : "?";
-            return url + connectorCharacter + "connectTimeout=" + databasePlatform.getJdbcUrlTimeoutValue(timeout);
+            return url + getConnectorCharacter(url) + "connectTimeout=" + databasePlatform.getJdbcUrlTimeoutValue(timeout);
         };
     }
 
+    /**
+     * Use lower case table names with the mariadb driver
+     * and allow connection to mysql scheme urls
+     */
+    @Bean
+    @Profile("mysql")
+    JdbcUrlCustomizer jdbcUrlMariaDbSchemeCustomizer(DatabaseProperties databaseProperties) {
+        return url -> {
+            if (!url.startsWith("jdbc:mysql")) {
+                return url;
+            }
+            if (!databaseProperties.getDriverClassName().contains("mariadb")) {
+                return url;
+            }
+
+            // this is a mysql scheme url with the mariadb driver
+            if (!url.contains("permitMysqlScheme=")) {
+                url += getConnectorCharacter(url) + "permitMysqlScheme=true";
+            }
+            if (!url.contains("lower_case_table_names")) {
+                url += getConnectorCharacter(url) + "lower_case_table_names=1";
+            }
+            return url;
+        };
+    }
+
+    private static @NonNull String getConnectorCharacter(String url) {
+        return url.contains("?") ? "&" : "?";
+    }
+
     @Bean
     public MBeanExporter dataSourceMBeanExporter(DataSource dataSource) {
         MBeanExporter exporter = new MBeanExporter();
@@ -129,7 +159,7 @@ LimitSqlAdapter limitSqlAdapter() {
 
     @Configuration
     @Profile("postgresql")
-    // The property source location is already inferred by the profile but we make it explicit
+    // The property source location is already inferred by the profile, but we make it explicit
     @PropertySource("classpath:application-postgresql.properties")
     public static class PostgresConfiguration {
 
@@ -142,7 +172,7 @@ LimitSqlAdapter limitSqlAdapter() {
 
     @Configuration
     @Profile("mysql")
-    // The property source location is already inferred by the profile but we make it explicit
+    // The property source location is already inferred by the profile, but we make it explicit
     @PropertySource("classpath:application-mysql.properties")
     public static class MysqlConfiguration {
 
diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/db/beans/FlywayConfiguration.java b/server/src/main/java/org/cloudfoundry/identity/uaa/db/beans/FlywayConfiguration.java
@@ -2,6 +2,7 @@
 
 import org.cloudfoundry.identity.uaa.db.FixFailedBackportMigrations_4_0_4;
 import org.flywaydb.core.Flyway;
+import org.springframework.context.ApplicationContext;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Condition;
 import org.springframework.context.annotation.ConditionContext;
@@ -22,16 +23,20 @@ public class FlywayConfiguration {
      * We need to maintain backwards compatibility due to {@link FixFailedBackportMigrations_4_0_4}
      */
     static final String VERSION_TABLE = "schema_version";
+    static final String FLYWAY_CLEAN_DISABLED = "spring.flyway.clean-disabled";
 
     @Bean
-    public Flyway baseFlyway(DataSource dataSource, DatabaseProperties databaseProperties) {
+    public Flyway baseFlyway(ApplicationContext context, DataSource dataSource, DatabaseProperties databaseProperties) {
+        boolean cleanDisabled = context.getEnvironment().getProperty(FLYWAY_CLEAN_DISABLED, "true").equalsIgnoreCase("true");
+
         return Flyway.configure()
                 .baselineOnMigrate(true)
                 .dataSource(dataSource)
                 .locations("classpath:org/cloudfoundry/identity/uaa/db/" + databaseProperties.getDatabasePlatform().type + "/")
                 .baselineVersion("1.5.2")
                 .validateOnMigrate(false)
                 .table(VERSION_TABLE)
+                .cleanDisabled(cleanDisabled)
                 .load();
     }
 
diff --git a/server/src/main/resources/org/cloudfoundry/identity/uaa/db/postgresql/V4_101_1631562784__Add_LowerIndex_To_Users.sql b/server/src/main/resources/org/cloudfoundry/identity/uaa/db/postgresql/V4_101_1631562784__Add_LowerIndex_To_Users.sql
@@ -1 +1,4 @@
-CREATE UNIQUE INDEX concurrently IF NOT EXISTS users_unique_key_lower ON users (LOWER(origin),LOWER(username),LOWER(identity_zone_id));
+-- Create index without CONCURRENTLY to allow execution within a transaction
+-- CONCURRENTLY cannot be used inside a transaction, which causes hangs with newer Flyway versions
+-- The IF NOT EXISTS clause prevents errors if the index already exists
+CREATE UNIQUE INDEX IF NOT EXISTS users_unique_key_lower ON users (LOWER(origin),LOWER(username),LOWER(identity_zone_id));
diff --git a/server/src/main/resources/org/cloudfoundry/identity/uaa/db/postgresql/V4_101_1639764160__Add_LowerIndex_To_Users_Wo_Origin.sql b/server/src/main/resources/org/cloudfoundry/identity/uaa/db/postgresql/V4_101_1639764160__Add_LowerIndex_To_Users_Wo_Origin.sql
@@ -1 +1,4 @@
-CREATE INDEX CONCURRENTLY IF NOT EXISTS users_key_lower_wo_origin ON users (LOWER(username),LOWER(identity_zone_id));
+-- Create index without CONCURRENTLY to allow execution within a transaction
+-- CONCURRENTLY cannot be used inside a transaction, which causes hangs with newer Flyway versions
+-- The IF NOT EXISTS clause prevents errors if the index already exists
+CREATE INDEX IF NOT EXISTS users_key_lower_wo_origin ON users (LOWER(username),LOWER(identity_zone_id));
diff --git a/server/src/main/resources/org/cloudfoundry/identity/uaa/db/postgresql/V4_109__IdP_AliasZid_IdzId_Index.sql b/server/src/main/resources/org/cloudfoundry/identity/uaa/db/postgresql/V4_109__IdP_AliasZid_IdzId_Index.sql
@@ -1 +1,4 @@
-CREATE INDEX CONCURRENTLY IF NOT EXISTS alias_in_zone on identity_provider (identity_zone_id, alias_zid) WHERE alias_zid IS NOT NULL;
+-- Create index without CONCURRENTLY to allow execution within a transaction
+-- CONCURRENTLY cannot be used inside a transaction, which causes hangs with newer Flyway versions
+-- The IF NOT EXISTS clause prevents errors if the index already exists
+CREATE INDEX IF NOT EXISTS alias_in_zone on identity_provider (identity_zone_id, alias_zid) WHERE alias_zid IS NOT NULL;
diff --git a/server/src/main/resources/org/cloudfoundry/identity/uaa/db/postgresql/V4_111__IdP_active_and_type_in_zone_Index.sql b/server/src/main/resources/org/cloudfoundry/identity/uaa/db/postgresql/V4_111__IdP_active_and_type_in_zone_Index.sql
@@ -1 +1,4 @@
-CREATE INDEX CONCURRENTLY IF NOT EXISTS active_and_type_in_zone on identity_provider (identity_zone_id, active, type);
+-- Create index without CONCURRENTLY to allow execution within a transaction
+-- CONCURRENTLY cannot be used inside a transaction, which causes hangs with newer Flyway versions
+-- The IF NOT EXISTS clause prevents errors if the index already exists
+CREATE INDEX IF NOT EXISTS active_and_type_in_zone on identity_provider (identity_zone_id, active, type);
diff --git a/server/src/main/resources/org/cloudfoundry/identity/uaa/db/postgresql/V4_113__Add_group_membership_idz_origin_idx.sql b/server/src/main/resources/org/cloudfoundry/identity/uaa/db/postgresql/V4_113__Add_group_membership_idz_origin_idx.sql
@@ -1 +1,4 @@
-CREATE INDEX CONCURRENTLY IF NOT EXISTS group_membership_idz_origin_idx ON group_membership(identity_zone_id, origin);
+-- Create index without CONCURRENTLY to allow execution within a transaction
+-- CONCURRENTLY cannot be used inside a transaction, which causes hangs with newer Flyway versions
+-- The IF NOT EXISTS clause prevents errors if the index already exists
+CREATE INDEX IF NOT EXISTS group_membership_idz_origin_idx ON group_membership(identity_zone_id, origin);
diff --git a/server/src/main/resources/org/cloudfoundry/identity/uaa/db/postgresql/V4_99_1575367461__revocable_token_index.sql b/server/src/main/resources/org/cloudfoundry/identity/uaa/db/postgresql/V4_99_1575367461__revocable_token_index.sql
@@ -1 +1,5 @@
-CREATE INDEX CONCURRENTLY IF NOT EXISTS revocable_tokens_user_id_client_id_response_type_identity__idx on revocable_tokens(user_id, client_id, response_type, identity_zone_id);
+-- Create index without CONCURRENTLY to allow execution within a transaction
+-- CONCURRENTLY cannot be used inside a transaction, which causes hangs with newer Flyway versions
+-- The IF NOT EXISTS clause prevents errors if the index already exists
+CREATE INDEX IF NOT EXISTS revocable_tokens_user_id_client_id_response_type_identity__idx
+    ON revocable_tokens(user_id, client_id, response_type, identity_zone_id);
diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/db/DbMigrationIntegrationTestParent.java b/server/src/test/java/org/cloudfoundry/identity/uaa/db/DbMigrationIntegrationTestParent.java
@@ -7,10 +7,12 @@
 import org.junit.jupiter.api.BeforeEach;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.jdbc.core.JdbcTemplate;
+import org.springframework.test.context.TestPropertySource;
 
 import java.sql.SQLException;
 
 @WithDatabaseContext
+@TestPropertySource(properties = {"spring.flyway.clean-disabled=false"})
 public abstract class DbMigrationIntegrationTestParent {
 
     @Autowired
diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/db/TableAndColumnNormalizationTest.java b/server/src/test/java/org/cloudfoundry/identity/uaa/db/TableAndColumnNormalizationTest.java
@@ -24,10 +24,12 @@
 import java.sql.Connection;
 import java.sql.DatabaseMetaData;
 import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.util.ArrayList;
+import java.util.List;
 
 import static org.assertj.core.api.Assertions.assertThat;
 
-
 /**
  * For MySQL, the database name is hardcoded in the {@link V1_5_4__NormalizeTableAndColumnNames} migration as
  * {@code uaa}. But the {@link TestDatabaseNameCustomizer} class dynamically allocates a DB name based on the
@@ -43,7 +45,7 @@ class MySQLConfiguration {
     @Order(TestDatabaseNameCustomizer.ORDER + 1)
     @Profile("mysql")
     JdbcUrlCustomizer mysqlHardcodedJdbcUrlCustomizer() {
-        return url -> "jdbc:mysql://127.0.0.1:3306/uaa?useSSL=true&trustServerCertificate=true";
+        return url -> "jdbc:mysql://127.0.0.1:3306/uaa?useSSL=true&trustServerCertificate=true&permitMysqlScheme=true";
     }
 }
 
@@ -56,7 +58,6 @@ JdbcUrlCustomizer mysqlHardcodedJdbcUrlCustomizer() {
         TestDatabaseNameCustomizer.class,
         MySQLConfiguration.class
 })
-@EnabledIfProfile({"postgresql", "mysql"})
 class TableAndColumnNormalizationTest {
 
     private final Logger logger = LoggerFactory.getLogger(getClass());
@@ -65,41 +66,71 @@ class TableAndColumnNormalizationTest {
     private DataSource dataSource;
 
     @Test
-    void checkTables() throws Exception {
+    @EnabledIfProfile({"postgresql", "mysql"})
+    void tableNamesAreLowercase() throws SQLException {
         try (Connection connection = dataSource.getConnection()) {
             DatabaseMetaData metaData = connection.getMetaData();
             ResultSet rs = metaData.getTables(null, null, null, new String[]{"TABLE"});
-            int count = 0;
+            List<String> validatedTables = new ArrayList<>();
+            List<String> failures = new ArrayList<>();
+
             while (rs.next()) {
-                String name = rs.getString("TABLE_NAME");
-                logger.info("Checking table [{}]", name);
-                if (name != null && DatabaseInformation1_5_3.tableNames.contains(name.toLowerCase())) {
-                    count++;
-                    logger.info("Validating table [{}]", name);
-                    assertThat(name).as("Table[%s] is not lower case.".formatted(name)).isEqualTo(name.toLowerCase());
+                String catalog = rs.getString("TABLE_CAT");
+                String table = rs.getString("TABLE_NAME");
+
+                logger.info("Checking table [{}.{}]", catalog, table);
+                if (isTableInUaaCatalog(catalog, table)) {
+                    logger.info("Validating table [{}.{}]", catalog, table);
+                    if (table.equals(table.toLowerCase())) {
+                        validatedTables.add(table);
+                    } else {
+                        failures.add("Table[%s.%s] is not lower case.".formatted(catalog, table));
+                    }
                 }
             }
-            assertThat(count).as("Table count:").isEqualTo(DatabaseInformation1_5_3.tableNames.size());
+            assertThat(validatedTables).hasSameElementsAs(DatabaseInformation1_5_3.tableNames);
+            assertThat(failures).isEmpty();
         }
     }
 
+    /**
+     * Only on postgresql are column names case-sensitive.
+     * Column names are not case-sensitive in MySQL on any platform.
+     *
+     * @throws SQLException
+     */
     @Test
-    void checkColumns() throws Exception {
+    @EnabledIfProfile({"postgresql"})
+    void columnNamesAreLowercase() throws SQLException {
         try (Connection connection = dataSource.getConnection()) {
             DatabaseMetaData metaData = connection.getMetaData();
             ResultSet rs = metaData.getColumns(null, null, null, null);
             boolean hadSomeResults = false;
+            List<String> failures = new ArrayList<>();
+
             while (rs.next()) {
                 hadSomeResults = true;
-                String name = rs.getString("TABLE_NAME");
+                String catalog = rs.getString("TABLE_CAT");
+                String table = rs.getString("TABLE_NAME");
                 String col = rs.getString("COLUMN_NAME");
-                logger.info("Checking column [{}.{}]", name, col);
-                if (name != null && DatabaseInformation1_5_3.tableNames.contains(name.toLowerCase())) {
-                    logger.info("Validating column [{}.{}]", name, col);
-                    assertThat(col.toLowerCase()).as("Column[%s.%s] is not lower case.".formatted(name, col)).isEqualTo(col);
+                logger.info("Checking column [{}.{}.{}]", catalog, table, col);
+
+                if (isTableInUaaCatalog(catalog, table)) {
+                    logger.info("Validating column [{}.{}]", table, col);
+                    if (!col.equals(col.toLowerCase())) {
+                        failures.add("Column[%s.%s.%s] is not lower case.".formatted(catalog, table, col));
+                    }
                 }
             }
             assertThat(hadSomeResults).as("Getting columns from db metadata should have returned some results").isTrue();
+            assertThat(failures).isEmpty();
         }
     }
+
+    private static boolean isTableInUaaCatalog(String catalog, String table) {
+        return catalog != null
+                && catalog.startsWith("uaa")
+                && table != null
+                && DatabaseInformation1_5_3.tableNames.contains(table.toLowerCase());
+    }
 }
diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/db/beans/DatabasePropertiesTest.java b/server/src/test/java/org/cloudfoundry/identity/uaa/db/beans/DatabasePropertiesTest.java
diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/resources/jdbc/JdbcPagingListTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/resources/jdbc/JdbcPagingListTests.java
diff --git a/uaa/build.gradle b/uaa/build.gradle
