-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
configure dedicated db user for database migrations: DML-only user fo… #29649
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
package io.quarkus.flyway.test; | ||
|
||
import static org.junit.jupiter.api.Assertions.assertEquals; | ||
|
||
import java.sql.Connection; | ||
import java.sql.PreparedStatement; | ||
import java.sql.ResultSet; | ||
import java.sql.SQLException; | ||
|
||
import javax.inject.Inject; | ||
import javax.sql.DataSource; | ||
|
||
import org.junit.jupiter.api.DisplayName; | ||
import org.junit.jupiter.api.Test; | ||
import org.junit.jupiter.api.extension.RegisterExtension; | ||
|
||
import io.quarkus.test.QuarkusUnitTest; | ||
|
||
public class FlywayExtensionDifferentUsernameTest { | ||
|
||
@Inject | ||
DataSource datasource; | ||
|
||
@RegisterExtension | ||
static final QuarkusUnitTest config = new QuarkusUnitTest() | ||
.withApplicationRoot((jar) -> jar | ||
|
||
.addAsResource("config-for-different-username.properties", "application.properties")); | ||
|
||
@Test | ||
@DisplayName("Check if connected with new username") | ||
public void testFlywayInitSql() throws SQLException { | ||
|
||
int var = 0; | ||
try (Connection con = datasource.getConnection(); | ||
PreparedStatement ps = con.prepareStatement("SELECT 100"); | ||
ResultSet rs = ps.executeQuery()) { | ||
if (rs.next()) { | ||
var = rs.getInt(1); | ||
} | ||
} | ||
assertEquals(100, var, "Different username and password was not executed"); | ||
|
||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
# Flyway config properties | ||
quarkus.flyway.username=sa | ||
quarkus.flyway.password=sa | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,72 @@ | ||
package io.quarkus.flyway.runtime; | ||
|
||
import java.io.PrintWriter; | ||
import java.sql.Connection; | ||
import java.sql.SQLException; | ||
import java.sql.SQLFeatureNotSupportedException; | ||
import java.util.Optional; | ||
import java.util.logging.Logger; | ||
|
||
import javax.sql.DataSource; | ||
|
||
public class DataSourceDecorator implements DataSource { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The name doesn't really explain what this class does, but I am not very good at naming, so I'll let others decide about it |
||
|
||
private DataSource dataSource; | ||
private Optional<String> username; | ||
private Optional<String> password; | ||
|
||
public DataSourceDecorator(DataSource datasource, Optional<String> username, Optional<String> password) { | ||
|
||
this.dataSource = datasource; | ||
this.username = username; | ||
this.password = password; | ||
|
||
} | ||
|
||
public PrintWriter getLogWriter() throws SQLException { | ||
// TODO Auto-generated method stub | ||
return dataSource.getLogWriter(); | ||
} | ||
|
||
public int getLoginTimeout() throws SQLException { | ||
// TODO Auto-generated method stub | ||
return dataSource.getLoginTimeout(); | ||
} | ||
|
||
public Logger getParentLogger() throws SQLFeatureNotSupportedException { | ||
// TODO Auto-generated method stub | ||
return dataSource.getParentLogger(); | ||
} | ||
|
||
public void setLogWriter(PrintWriter arg0) throws SQLException { | ||
// TODO Auto-generated method stub | ||
dataSource.setLogWriter(arg0); | ||
} | ||
|
||
public void setLoginTimeout(int arg0) throws SQLException { | ||
// TODO Auto-generated method stub | ||
dataSource.setLoginTimeout(arg0); | ||
} | ||
|
||
public boolean isWrapperFor(Class<?> arg0) throws SQLException { | ||
// TODO Auto-generated method stub | ||
return dataSource.isWrapperFor(arg0); | ||
} | ||
|
||
public <T> T unwrap(Class<T> arg0) throws SQLException { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Replace |
||
// TODO Auto-generated method stub | ||
return dataSource.unwrap(arg0); | ||
} | ||
|
||
public Connection getConnection() throws SQLException { | ||
// TODO Auto-generated method stub | ||
return dataSource.getConnection(username.get(), password.get()); | ||
} | ||
|
||
public Connection getConnection(String username, String password) | ||
throws SQLException { | ||
// TODO Auto-generated method stub | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please remove these comments |
||
return dataSource.getConnection(username, password); | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -33,7 +33,13 @@ public FlywayCreator withCallbacks(Collection<Callback> callbacks) { | |
|
||
public Flyway createFlyway(DataSource dataSource) { | ||
FluentConfiguration configure = Flyway.configure(); | ||
configure.dataSource(dataSource); | ||
|
||
if (flywayRuntimeConfig.username.isPresent() && flywayRuntimeConfig.password.isPresent()) { | ||
configure.dataSource( | ||
new DataSourceDecorator(dataSource, flywayRuntimeConfig.username, flywayRuntimeConfig.password)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can invoke There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't undertstand the usage of this pattern. I gave you good hints on how you can deal with this in your previous PR with some pseudo code that should allow you to do what you want and some advice on how to deal with the JDBC URL. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @gsmet Before I read your comments on the previous PR, I suggested @mfpc to use this pattern. The idea was to reuse the DataSource configuration and call But now I see that your suggestion seems nicer, so 👍🏻 to that |
||
} else { | ||
configure.dataSource(dataSource); | ||
} | ||
if (flywayRuntimeConfig.initSql.isPresent()) { | ||
configure.initSql(flywayRuntimeConfig.initSql.get()); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -43,6 +43,26 @@ public static FlywayDataSourceRuntimeConfig defaultConfig() { | |
@ConfigItem | ||
public Optional<String> defaultSchema = Optional.empty(); | ||
|
||
/** | ||
* This is a safety measurement against theoretical situations where the application goes haywire for any reason. | ||
* This includes e.g. accidentally setting quarkus.hibernate-orm.database.generation to drop-and-create. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This description looks misleading and confusing |
||
* If the application runs with a restricted database user, damage in such situations can be mitigated. | ||
* # Name of a custom database user for flyway to use. Must have DDL rights | ||
* quarkus.flyway.username=my_schema_owner | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This can be removed |
||
* | ||
*/ | ||
|
||
@ConfigItem | ||
public Optional<String> username = Optional.empty(); | ||
|
||
/** | ||
* Set the password for connection; | ||
* quarkus.flyway.password=my_secret_password | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This line can be removed |
||
*/ | ||
|
||
@ConfigItem | ||
public Optional<String> password = Optional.empty(); | ||
|
||
/** | ||
* Comma-separated case-sensitive list of schemas managed by Flyway. | ||
* The first schema in the list will be automatically set as the default one during the migration. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test doesn't really verify the feature you provided
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test if the connection is working with new configs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will change the message for "Database connected with new config", its ok?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you should rather get the datasource and check the user there. In your previous PR, I outlined how you could get the datasource URL. I'm pretty sure you can get the user from there too.