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

AG-237: Ability to change DataSource settings after starting #102

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

liubagha
Copy link

@liubagha liubagha commented Apr 9, 2024

Hi, Agroal Community!

Quartus supports database management via Agroal Connection Pool (https://quarkus.io/guides/datasource).

When initializing an application, Quarkus allows you to create unremovable beans, for example DataSource. https://quarkus.io/guides/cdi-reference#remove_unused_beans

The existing library functionality does not allow you to restart the connection with new credentials in an existing bean. This functionality would be useful when switching to a backup database in case of an accident.

Can we add this functionality to the new version?

Example:

      Properties properties = new Properties();
        properties.setProperty(ReloadDataSourceUtil.AGROAL_JDBC_URL, Configuration.getConfig().getConfigValue("kc.db-url").getValue());
        properties.setProperty(ReloadDataSourceUtil.AGROAL_USERNAME, Configuration.getConfig().getConfigValue("kc.db-username").getValue());
        properties.setProperty(ReloadDataSourceUtil.AGROAL_PASSWORD, Configuration.getConfig().getConfigValue("kc.db-password").getValue());

        io.agroal.pool.DataSource dataSource = (io.agroal.pool.DataSource) Arc.container().instance(AgroalDataSource.class).get();
        dataSource.reloadDataSourceWithNewCredentials(properties, AgroalDataSource.FlushMode.ALL);

@barreiro
Copy link
Contributor

hi @liubagha.

if I understand correctly, you want the pool be recreated with a new URL that uses the backup instance. I'm not sure if it's the best way to handle fail-over, from an architectural standpoint.

I personally feel a bit uncomfortable with changing the URL of a running pool, and I believe the fail-over should be handled externally.

@jesperpedersen I would also like to hear your opinion on this.

@liubagha
Copy link
Author

liubagha commented Apr 12, 2024

@barreiro, I would like to highlight the additional use case.

There are additional circumstances beyond the backup and recovery best practice. Maybe it will be important.
If database credentials have been compromised, or following password policies, we must change password(s) every N days. Considering the features of the Quarkus platform, we don't have an alternative to reloading the application. This may have an impact on how the system will be maintained in the prom.

@yrodiere
Copy link

yrodiere commented Apr 24, 2024

IMO if you need to change DB credentials without downtime, you should have a load-balancer (or similar indirection) in front of your app, allowing you to simply spawn a new instance of the application (with updated settings), and to redirect the load-balancer to that new instance once it's up.

Renewing the connection pool is something I could be convinced makes sense in some edge cases where restarting the app is not an option, but I'd be surprised if this wasn't already possible.

What I completely disagree with, on the other hand, is an approach allowing anyone to programmatically pass secrets like you're suggesting in this PR.
The right way to do this IMO would be through credential providers. In your app configuration, you wouldn't specify credentials directly, but through a credentials provider (e.g. using Vault). Then when you need an update:

  1. Update the DB to allow new credentials.
  2. Update the credentials in the source of information used by your credential provider (e.g. Vault).
  3. Somehow get the application to renew its connection pool, thereby retrieving the new credentials and opening new connections using these.
  4. Update the DB to disallow the old credentials.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants