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

add support to get and set db credentials in an atomic operation #2189

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
49 changes: 37 additions & 12 deletions src/main/java/com/zaxxer/hikari/HikariConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import com.codahale.metrics.health.HealthCheckRegistry;
import com.zaxxer.hikari.metrics.MetricsTrackerFactory;
import com.zaxxer.hikari.util.Credentials;
import com.zaxxer.hikari.util.PropertyElf;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand All @@ -36,6 +37,7 @@
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.ThreadFactory;
import java.util.concurrent.ThreadLocalRandom;
import java.util.concurrent.atomic.AtomicReference;

import static com.zaxxer.hikari.util.UtilityElf.getNullIfEmpty;
import static com.zaxxer.hikari.util.UtilityElf.safeIsAssignableFrom;
Expand Down Expand Up @@ -68,8 +70,7 @@ public class HikariConfig implements HikariConfigMXBean
private volatile long maxLifetime;
private volatile int maxPoolSize;
private volatile int minIdle;
private volatile String username;
private volatile String password;
private final AtomicReference<Credentials> credentials = new AtomicReference<>(Credentials.of(null, null));

// Properties NOT changeable at runtime
//
Expand Down Expand Up @@ -283,7 +284,7 @@ public void setMinimumIdle(int minIdle)
*/
public String getPassword()
{
return password;
return credentials.get().getPassword();
}

/**
Expand All @@ -293,7 +294,7 @@ public String getPassword()
@Override
public void setPassword(String password)
{
this.password = password;
credentials.updateAndGet(current -> Credentials.of(current.getUsername(), password));
}

/**
Expand All @@ -303,7 +304,7 @@ public void setPassword(String password)
*/
public String getUsername()
{
return username;
return credentials.get().getUsername();
}

/**
Expand All @@ -314,7 +315,28 @@ public String getUsername()
@Override
public void setUsername(String username)
{
this.username = username;
credentials.updateAndGet(current -> Credentials.of(username, current.getPassword()));
}

/**
* Atomically set the default username and password to use for DataSource.getConnection(username, password) calls.
*
* @param credentials the username and password pair
*/
@Override
public void setCredentials(final Credentials credentials)
{
this.credentials.set(credentials);
}

/**
* Atomically get the default username and password to use for DataSource.getConnection(username, password) calls.
*
* @return the username and password pair
*/
public Credentials getCredentials()
{
return credentials.get();
}

/** {@inheritDoc} */
Expand Down Expand Up @@ -945,17 +967,20 @@ void seal()
*
* @param other Other {@link HikariConfig} to copy the state to.
*/
@SuppressWarnings({"rawtypes", "unchecked"})
public void copyStateTo(HikariConfig other)
{
for (var field : HikariConfig.class.getDeclaredFields()) {
if (!Modifier.isFinal(field.getModifiers())) {
field.setAccessible(true);
try {
try {
if (!Modifier.isFinal(field.getModifiers())) {
field.setAccessible(true);
field.set(other, field.get(this));
} else if (field.getType().isAssignableFrom(AtomicReference.class)) {
((AtomicReference) field.get(other)).set(((AtomicReference) field.get(this)).get());
}
catch (Exception e) {
throw new RuntimeException("Failed to copy HikariConfig state: " + e.getMessage(), e);
}
}
catch (Exception e) {
throw new RuntimeException("Failed to copy HikariConfig state: " + e.getMessage(), e);
}
}

Expand Down
10 changes: 10 additions & 0 deletions src/main/java/com/zaxxer/hikari/HikariConfigMXBean.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@

package com.zaxxer.hikari;

import com.zaxxer.hikari.util.Credentials;

/**
* The javax.management MBean for a Hikari pool configuration.
*
Expand Down Expand Up @@ -167,6 +169,14 @@ public interface HikariConfigMXBean
*/
void setUsername(String username);

/**
* Set the username and password used for authentication. Changing this at runtime will apply to new
* connections only. Altering this at runtime only works for DataSource-based connections, not Driver-class
* or JDBC URL-based connections.
*
* @param credentials the database username and password pair
*/
void setCredentials(Credentials credentials);

/**
* The name of the connection pool.
Expand Down
10 changes: 5 additions & 5 deletions src/main/java/com/zaxxer/hikari/pool/PoolBase.java
Original file line number Diff line number Diff line change
Expand Up @@ -311,8 +311,7 @@ else if (mBeanServer.isRegistered(beanConfigName)) {
private void initializeDataSource()
{
final var jdbcUrl = config.getJdbcUrl();
final var username = config.getUsername();
final var password = config.getPassword();
final var credentials = config.getCredentials();
final var dsClassName = config.getDataSourceClassName();
final var driverClassName = config.getDriverClassName();
final var dataSourceJNDI = config.getDataSourceJNDI();
Expand All @@ -324,7 +323,7 @@ private void initializeDataSource()
PropertyElf.setTargetFromProperties(ds, dataSourceProperties);
}
else if (jdbcUrl != null && ds == null) {
ds = new DriverDataSource(jdbcUrl, driverClassName, dataSourceProperties, username, password);
ds = new DriverDataSource(jdbcUrl, driverClassName, dataSourceProperties, credentials.getUsername(), credentials.getPassword());
}
else if (dataSourceJNDI != null && ds == null) {
try {
Expand Down Expand Up @@ -354,8 +353,9 @@ private Connection newConnection() throws Exception

Connection connection = null;
try {
var username = config.getUsername();
var password = config.getPassword();
final var credentials = config.getCredentials();
final var username = credentials.getUsername();
final var password = credentials.getPassword();

connection = (username == null) ? dataSource.getConnection() : dataSource.getConnection(username, password);
if (connection == null) {
Expand Down
57 changes: 57 additions & 0 deletions src/main/java/com/zaxxer/hikari/util/Credentials.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
package com.zaxxer.hikari.util;

import javax.management.ConstructorParameters;

/**
* A simple class to hold connection credentials and is designed to be immutable.
*/
public final class Credentials
{

private final String username;
private final String password;

/**
* Construct an immutable Credentials object with the supplied username and password.
*
* @param username the username
* @param password the password
* @return a new Credentials object
*/
public static Credentials of(final String username, final String password) {
return new Credentials(username, password);
}

/**
* Construct an immutable Credentials object with the supplied username and password.
*
* @param username the username
* @param password the password
*/
@ConstructorParameters({ "username", "password" })
public Credentials(final String username, final String password)
{
this.username = username;
this.password = password;
}

/**
* Get the username.
*
* @return the username
*/
public String getUsername()
{
return username;
}

/**
* Get the password.
*
* @return the password
*/
public String getPassword()
{
return password;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import com.zaxxer.hikari.HikariConfig;
import com.zaxxer.hikari.HikariDataSource;
import com.zaxxer.hikari.util.Credentials;
import org.junit.Test;

import java.sql.Connection;
Expand Down Expand Up @@ -68,6 +69,7 @@ public void testSealedAccessibleMethods() throws SQLException
ds.setMaximumPoolSize(8);
ds.setPassword("password");
ds.setUsername("username");
ds.setCredentials(Credentials.of("anothername", "anotherpassword"));
}
}
}
27 changes: 27 additions & 0 deletions src/test/java/com/zaxxer/hikari/pool/PostgresTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicInteger;

import com.zaxxer.hikari.util.Credentials;
import org.junit.After;
import org.junit.Before;

Expand Down Expand Up @@ -105,6 +106,24 @@ public void testCase4() throws Exception
exerciseConfig(config, 3);
}

@Test
public void testCredentialRotation()
{
HikariConfig config = createConfig(postgres);
config.setMinimumIdle(3);
config.setMaximumPoolSize(10);
config.setConnectionTimeout(1000);
config.setIdleTimeout(SECONDS.toMillis(20));

exerciseConfig(config, 3);

updatePostgresCredentials("newuser", "newpassword");
config.setJdbcUrl(postgres.getJdbcUrl());
config.setCredentials(Credentials.of("newuser", "newpassword"));

exerciseConfig(config, 3);
}

static private void exerciseConfig(HikariConfig config, int numThreads) {
try (final HikariDataSource ds = new HikariDataSource(config)) {
assertTrue(ds.isRunning());
Expand Down Expand Up @@ -193,4 +212,12 @@ static private HikariConfig createConfig(PostgreSQLContainer<?> postgres) {
config.setDriverClassName(postgres.getDriverClassName());
return config;
}

private void updatePostgresCredentials(String username, String password) {
postgres.stop();
postgres = new PostgreSQLContainer<>(IMAGE_NAME)
.withUsername(username)
.withPassword(password);
postgres.start();
}
}
21 changes: 21 additions & 0 deletions src/test/java/com/zaxxer/hikari/pool/TestMBean.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import com.zaxxer.hikari.HikariDataSource;
import com.zaxxer.hikari.HikariPoolMXBean;
import com.zaxxer.hikari.mocks.StubDataSource;
import com.zaxxer.hikari.util.Credentials;
import org.junit.Test;

import javax.management.JMX;
Expand Down Expand Up @@ -170,4 +171,24 @@ public void testMBeanConnectionTimeoutChange() throws SQLException {
System.clearProperty("com.zaxxer.hikari.housekeeping.periodMs");
}
}

@Test
public void testMBeanCredentialRotation() {
HikariConfig config = newHikariConfig();
config.setMinimumIdle(3);
config.setMaximumPoolSize(5);
config.setRegisterMbeans(true);
config.setConnectionTimeout(2800);
config.setConnectionTestQuery("VALUES 1");
config.setDataSourceClassName("com.zaxxer.hikari.mocks.StubDataSource");
config.setCredentials(Credentials.of("foo", "bar"));

try (HikariDataSource ds = new HikariDataSource(config)) {
HikariConfigMXBean hikariConfigMXBean = ds.getHikariConfigMXBean();
hikariConfigMXBean.setCredentials(Credentials.of("newFoo", "newBar"));

assertEquals("newFoo", ds.getUsername());
assertEquals("newBar", ds.getPassword());
}
}
}