Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow the use of specific user for DML updates in Flyway #29704

Closed
wants to merge 1 commit into from
Closed

Allow the use of specific user for DML updates in Flyway #29704

wants to merge 1 commit into from

Conversation

mfpc
Copy link
Contributor

@mfpc mfpc commented Dec 6, 2022

Closes #24639

@quarkus-bot

This comment was marked as resolved.

@geoand geoand changed the title configure dedicated db user for database migrations: DML-only user for datasource, but DDL user for migration #24639 Allow the use of specific user for DML Dec 7, 2022
@geoand geoand changed the title Allow the use of specific user for DML Allow the use of specific user for DML updates in Flyway Dec 7, 2022
@gastaldi gastaldi requested a review from gsmet December 7, 2022 12:35
Comment on lines +47 to +64
void testDifferentUserConfig() throws SQLException {
FlywayDataSourceRuntimeConfig runtimeConfig = FlywayDataSourceRuntimeConfig.defaultConfig();
FlywayDataSourceBuildTimeConfig buildConfig = FlywayDataSourceBuildTimeConfig.defaultConfig();
runtimeConfig.username = runtimeConfig.username.of("sa");
runtimeConfig.password = runtimeConfig.password.of("sa");
runtimeConfig.jdbcUrl = runtimeConfig.jdbcUrl.of("jdbc:h2:~/test");
creator = new FlywayCreator(runtimeConfig, buildConfig);
Flyway flyway = creator.createFlyway(dataSource);
boolean error = false;
try {
flyway.validate();
} catch (Exception ex) {
ex.printStackTrace();
error = true;
}

assertEquals(false, error, "Database connection with different user failed! ");
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I still don't think this test is right.
Ideally the test(s) should check if the Flyway configuration is using the expected JDBC URL, user and password when provided


import javax.sql.DataSource;

public class DataSourceDecorator implements DataSource {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
public class DataSourceDecorator implements DataSource {
/**
* Calls getConnection(user,password) for a given when getConnection() is called
*/
class CredentialsDataSource implements DataSource {

Comment on lines +13 to +15
private DataSource dataSource;
private String username;
private String password;
Copy link
Contributor

Choose a reason for hiding this comment

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

Make them final

Comment on lines +41 to +45
<dependency>
<groupId>io.quarkus</groupId>
<artifactId>quarkus-test-h2</artifactId>
<scope>test</scope>
</dependency>
Copy link
Contributor

Choose a reason for hiding this comment

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

This needs to be moved to the <!-- test dependencies --> section below

@mfpc mfpc closed this by deleting the head repository Dec 7, 2022
@quarkus-bot quarkus-bot bot added the triage/invalid This doesn't seem right label Dec 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/flyway triage/invalid This doesn't seem right
Projects
None yet
Development

Successfully merging this pull request may close these issues.

configure dedicated db user for database migrations: DML-only user for datasource, but DDL user for migration
2 participants