Skip to content

Commit

Permalink
feat: read database config from vault (#4129)
Browse files Browse the repository at this point in the history 
* feat: read database config from vault

* DEPENDENCIES

* DEPENDENCIES

* improve log message
  • Loading branch information
@paullatzelsperger
paullatzelsperger authored Apr 22, 2024
1 parent 4fd16b8 commit 41a22c1
Show file tree
Hide file tree
Showing 4 changed files with 106 additions and 66 deletions.
6 changes: 3 additions & 3 deletions DEPENDENCIES
View file
Original file line number Diff line number Diff line change
Expand Up @@ -226,11 +226,11 @@ maven/mavencentral/org.apiguardian/apiguardian-api/1.1.2, Apache-2.0, approved,
maven/mavencentral/org.assertj/assertj-core/3.25.3, Apache-2.0, approved, #12585
maven/mavencentral/org.awaitility/awaitility/4.2.1, Apache-2.0, approved, #14178
maven/mavencentral/org.bouncycastle/bcpkix-jdk18on/1.72, MIT, approved, #3789
maven/mavencentral/org.bouncycastle/bcpkix-jdk18on/1.78, MIT, approved, #14235
maven/mavencentral/org.bouncycastle/bcpkix-jdk18on/1.78, MIT, approved, #14434
maven/mavencentral/org.bouncycastle/bcprov-jdk18on/1.72, MIT AND CC0-1.0, approved, #3538
maven/mavencentral/org.bouncycastle/bcprov-jdk18on/1.78, MIT AND CC0-1.0, approved, #14237
maven/mavencentral/org.bouncycastle/bcprov-jdk18on/1.78, MIT AND CC0-1.0, approved, #14433
maven/mavencentral/org.bouncycastle/bcutil-jdk18on/1.72, MIT, approved, #3790
maven/mavencentral/org.bouncycastle/bcutil-jdk18on/1.78, MIT, approved, #14238
maven/mavencentral/org.bouncycastle/bcutil-jdk18on/1.78, MIT, approved, #14435
maven/mavencentral/org.ccil.cowan.tagsoup/tagsoup/1.2.1, Apache-2.0, approved, clearlydefined
maven/mavencentral/org.checkerframework/checker-qual/3.12.0, MIT, approved, clearlydefined
maven/mavencentral/org.checkerframework/checker-qual/3.42.0, MIT, approved, clearlydefined
Expand Down
1 change: 1 addition & 0 deletions extensions/common/sql/sql-pool/sql-pool-apache-commons/build.gradle.kts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ dependencies {

// required for statically mocking the JDBC DriverManager
testImplementation(libs.mockito.inline)
testImplementation(project(":core:common:lib:boot-lib")) //in-mem vault
}


84 changes: 42 additions & 42 deletions ...src/main/java/org/eclipse/edc/sql/pool/commons/CommonsConnectionPoolServiceExtension.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,77 +17,54 @@
import org.eclipse.edc.runtime.metamodel.annotation.Extension;
import org.eclipse.edc.runtime.metamodel.annotation.Inject;
import org.eclipse.edc.runtime.metamodel.annotation.Setting;
import org.eclipse.edc.spi.EdcException;
import org.eclipse.edc.spi.monitor.Monitor;
import org.eclipse.edc.spi.security.Vault;
import org.eclipse.edc.spi.system.ServiceExtension;
import org.eclipse.edc.spi.system.ServiceExtensionContext;
import org.eclipse.edc.spi.system.configuration.Config;
import org.eclipse.edc.sql.ConnectionFactory;
import org.eclipse.edc.sql.datasource.ConnectionFactoryDataSource;
import org.eclipse.edc.sql.datasource.ConnectionPoolDataSource;
import org.eclipse.edc.transaction.datasource.spi.DataSourceRegistry;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

import java.util.HashMap;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Properties;
import java.util.function.BiFunction;
import java.util.function.Consumer;
import java.util.function.Supplier;
import javax.sql.DataSource;

import static java.lang.String.format;
import static java.util.Optional.ofNullable;

@Extension(value = CommonsConnectionPoolServiceExtension.NAME)
public class CommonsConnectionPoolServiceExtension implements ServiceExtension {

public static final String NAME = "Commons Connection Pool";

public static final String EDC_DATASOURCE_PREFIX = "edc.datasource";
@Deprecated(since = "0.3.1")
@Setting(required = false)
public static final String DEPRACATED_POOL_MAX_IDLE_CONNECTIONS = "pool.maxIdleConnections";
public static final String POOL_CONNECTIONS_MAX_IDLE = "pool.connections.max-idle";
@Deprecated(since = "0.3.1")
@Setting(required = false)
public static final String DEPRACATED_POOL_MAX_TOTAL_CONNECTIONS = "pool.maxTotalConnections";
public static final String POOL_CONNECTIONS_MAX_TOTAL = "pool.connections.max-total";
@Deprecated(since = "0.3.1")
@Setting(required = false)
public static final String DEPRACATED_POOL_MIN_IDLE_CONNECTIONS = "pool.minIdleConnections";
public static final String POOL_CONNECTIONS_MIN_IDLE = "pool.connections.min-idle";
@Deprecated(since = "0.3.1")
@Setting(required = false)
public static final String DEPRACATED_POOL_TEST_CONNECTION_ON_BORROW = "pool.testConnectionOnBorrow";
public static final String POOL_CONNECTION_TEST_ON_BORROW = "pool.connection.test.on-borrow";
@Deprecated(since = "0.3.1")
@Setting(required = false)
public static final String DEPRACATED_POOL_TEST_CONNECTION_ON_CREATE = "pool.testConnectionOnCreate";
public static final String POOL_CONNECTION_TEST_ON_CREATE = "pool.connection.test.on-create";
@Deprecated(since = "0.3.1")
@Setting(required = false)
public static final String DEPRACATED_POOL_TEST_CONNECTION_ON_RETURN = "pool.testConnectionOnReturn";
public static final String POOL_CONNECTION_TEST_ON_RETURN = "pool.connection.test.on-return";
@Deprecated(since = "0.3.1")
@Setting(required = false)
public static final String DEPRACATED_POOL_TEST_CONNECTION_WHILE_IDLE = "pool.testConnectionWhileIdle";
public static final String POOL_CONNECTION_TEST_WHILE_IDLE = "pool.connection.test.while-idle";
@Deprecated(since = "0.3.1")
@Setting(required = false)
public static final String DEPRACATED_POOL_TEST_QUERY = "pool.testQuery";
public static final String POOL_CONNECTION_TEST_QUERY = "pool.connection.test.query";
public static final Map<String, String> CONFIGURATION_MAPPING = Map.of(
POOL_CONNECTIONS_MAX_IDLE, DEPRACATED_POOL_MAX_IDLE_CONNECTIONS,
POOL_CONNECTIONS_MIN_IDLE, DEPRACATED_POOL_MIN_IDLE_CONNECTIONS,
POOL_CONNECTIONS_MAX_TOTAL, DEPRACATED_POOL_MAX_TOTAL_CONNECTIONS,
POOL_CONNECTION_TEST_ON_BORROW, DEPRACATED_POOL_TEST_CONNECTION_ON_BORROW,
POOL_CONNECTION_TEST_ON_CREATE, DEPRACATED_POOL_TEST_CONNECTION_ON_CREATE,
POOL_CONNECTION_TEST_ON_RETURN, DEPRACATED_POOL_TEST_CONNECTION_ON_RETURN,
POOL_CONNECTION_TEST_WHILE_IDLE, DEPRACATED_POOL_TEST_CONNECTION_WHILE_IDLE,
POOL_CONNECTION_TEST_QUERY, DEPRACATED_POOL_TEST_QUERY);

@Setting(required = true)
public static final String URL = "url";

@Setting(value = "Username to be used for the JDBC connection. Can be omitted if not required, or if the user is encoded in the JDBC url.")
public static final String USER = "user";
@Setting(value = "Username to be used for the JDBC connection. Can be omitted if not required, or if the password is encoded in the JDBC url.")
public static final String PASSWORD = "password";

private final List<CommonsConnectionPool> commonsConnectionPools = new LinkedList<>();
@Inject
private DataSourceRegistry dataSourceRegistry;
Expand All @@ -98,6 +75,9 @@ public class CommonsConnectionPoolServiceExtension implements ServiceExtension {
@Inject
private ConnectionFactory connectionFactory;

@Inject
private Vault vault;

@Override
public String name() {
return NAME;
Expand Down Expand Up @@ -127,6 +107,14 @@ public List<CommonsConnectionPool> getCommonsConnectionPools() {
return commonsConnectionPools;
}

private @NotNull Supplier<@Nullable String> readFromConfig(Config config, String value) {
return () -> {
var entry = EDC_DATASOURCE_PREFIX + "." + config.currentNode() + "." + value;
monitor.warning("Database configuration value '%s' not found in vault, will fall back to Config. Please consider putting database configuration into the vault.".formatted(entry));
return config.getString(value, null);
};
}

private void setIfProvidedString(String key, Consumer<String> setter, Config config) {
setIfProvided(key, setter, config::getString);
}
Expand All @@ -140,12 +128,7 @@ private void setIfProvidedInt(String key, Consumer<Integer> setter, Config confi
}

private <T> void setIfProvided(String key, Consumer<T> setter, BiFunction<String, T, T> getter) {
var oldKey = CONFIGURATION_MAPPING.get(key);
var oldValue = getter.apply(oldKey, null);
if (oldValue != null) {
monitor.warning(format("Configuration setting %s has been deprecated, please use %s instead", oldKey, key));
}
var value = getter.apply(key, oldValue);
var value = getter.apply(key, null);
if (value != null) {
setter.accept(value);
}
Expand All @@ -165,11 +148,28 @@ private Map<String, CommonsConnectionPool> createConnectionPools(Config parent)
}

private DataSource createDataSource(Config config) {
var jdbcUrl = Objects.requireNonNull(config.getString(URL));
var rootPath = EDC_DATASOURCE_PREFIX + "." + config.currentNode();

// read values from the vault first, fall back to config
var urlProperty = rootPath + "." + URL;
var jdbcUrl = ofNullable(vault.resolveSecret(urlProperty)).orElseGet(readFromConfig(config, URL));

if (jdbcUrl == null) {
throw new EdcException("Mandatory config '%s' not found. Please provide a value for the '%s' property, either as a secret in the vault or an application property.".formatted(urlProperty, urlProperty));
}

var jdbcUser = ofNullable(vault.resolveSecret(rootPath + "." + USER))
.orElseGet(readFromConfig(config, USER));
var jdbcPassword = ofNullable(vault.resolveSecret(rootPath + "." + PASSWORD))
.orElseGet(readFromConfig(config, PASSWORD));

var properties = new Properties();
properties.putAll(config.getRelativeEntries());

// only set if not-null, otherwise Properties#add throws a NPE
ofNullable(jdbcUser).ifPresent(u -> properties.put(USER, u));
ofNullable(jdbcPassword).ifPresent(p -> properties.put(PASSWORD, p));

return new ConnectionFactoryDataSource(connectionFactory, jdbcUrl, properties);
}

Expand Down
81 changes: 60 additions & 21 deletions ...test/java/org/eclipse/edc/sql/pool/commons/CommonsConnectionPoolServiceExtensionTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,17 @@
package org.eclipse.edc.sql.pool.commons;

import org.assertj.core.api.ThrowingConsumer;
import org.eclipse.edc.boot.vault.InMemoryVault;
import org.eclipse.edc.junit.extensions.DependencyInjectionExtension;
import org.eclipse.edc.spi.EdcException;
import org.eclipse.edc.spi.security.Vault;
import org.eclipse.edc.spi.system.ServiceExtensionContext;
import org.eclipse.edc.spi.system.configuration.Config;
import org.eclipse.edc.spi.system.configuration.ConfigFactory;
import org.eclipse.edc.sql.ConnectionFactory;
import org.eclipse.edc.transaction.datasource.spi.DataSourceRegistry;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.junit.jupiter.api.extension.ExtensionContext;
import org.junit.jupiter.params.ParameterizedTest;
Expand All @@ -33,14 +38,7 @@
import java.util.stream.Stream;

import static org.assertj.core.api.Assertions.assertThat;
import static org.eclipse.edc.sql.pool.commons.CommonsConnectionPoolServiceExtension.DEPRACATED_POOL_MAX_IDLE_CONNECTIONS;
import static org.eclipse.edc.sql.pool.commons.CommonsConnectionPoolServiceExtension.DEPRACATED_POOL_MAX_TOTAL_CONNECTIONS;
import static org.eclipse.edc.sql.pool.commons.CommonsConnectionPoolServiceExtension.DEPRACATED_POOL_MIN_IDLE_CONNECTIONS;
import static org.eclipse.edc.sql.pool.commons.CommonsConnectionPoolServiceExtension.DEPRACATED_POOL_TEST_CONNECTION_ON_BORROW;
import static org.eclipse.edc.sql.pool.commons.CommonsConnectionPoolServiceExtension.DEPRACATED_POOL_TEST_CONNECTION_ON_CREATE;
import static org.eclipse.edc.sql.pool.commons.CommonsConnectionPoolServiceExtension.DEPRACATED_POOL_TEST_CONNECTION_ON_RETURN;
import static org.eclipse.edc.sql.pool.commons.CommonsConnectionPoolServiceExtension.DEPRACATED_POOL_TEST_CONNECTION_WHILE_IDLE;
import static org.eclipse.edc.sql.pool.commons.CommonsConnectionPoolServiceExtension.DEPRACATED_POOL_TEST_QUERY;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.eclipse.edc.sql.pool.commons.CommonsConnectionPoolServiceExtension.EDC_DATASOURCE_PREFIX;
import static org.eclipse.edc.sql.pool.commons.CommonsConnectionPoolServiceExtension.POOL_CONNECTIONS_MAX_IDLE;
import static org.eclipse.edc.sql.pool.commons.CommonsConnectionPoolServiceExtension.POOL_CONNECTIONS_MAX_TOTAL;
Expand All @@ -51,6 +49,7 @@
import static org.eclipse.edc.sql.pool.commons.CommonsConnectionPoolServiceExtension.POOL_CONNECTION_TEST_QUERY;
import static org.eclipse.edc.sql.pool.commons.CommonsConnectionPoolServiceExtension.POOL_CONNECTION_TEST_WHILE_IDLE;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.argThat;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
Expand All @@ -61,10 +60,13 @@
class CommonsConnectionPoolServiceExtensionTest {
private static final String DS_1_NAME = "ds1";
private final DataSourceRegistry dataSourceRegistry = mock();
private final ConnectionFactory connectionFactory = mock();

@BeforeEach
void setUp(ServiceExtensionContext context) {
context.registerService(DataSourceRegistry.class, dataSourceRegistry);
context.registerService(Vault.class, new InMemoryVault(mock()));
context.registerService(ConnectionFactory.class, connectionFactory);
}

@ParameterizedTest
Expand All @@ -89,6 +91,55 @@ void initialize_withConfig(Map<String, String> configuration, ThrowingConsumer<C
.satisfies(checker);
}

@Test
void initialize_fromVault(CommonsConnectionPoolServiceExtension extension, ServiceExtensionContext context) {
when(context.getConfig(EDC_DATASOURCE_PREFIX))
.thenReturn(ConfigFactory.fromMap(Map.of("ds1.name", "ds1")));
var vault = context.getService(Vault.class);

vault.storeSecret("edc.datasource." + DS_1_NAME + ".user", "test-user");
vault.storeSecret("edc.datasource." + DS_1_NAME + ".password", "test-pwd");
vault.storeSecret("edc.datasource." + DS_1_NAME + ".url", "jdbc://whatever");

extension.initialize(context);

verify(dataSourceRegistry).register(eq(DS_1_NAME), any());
assertThat(extension.getCommonsConnectionPools()).hasSize(1).first()
.satisfies(pool -> {
assertThatThrownBy(pool::getConnection).isInstanceOf(EdcException.class); //we need this only to invoke the connection factory
verify(connectionFactory).create(eq("jdbc://whatever"), argThat(p ->
p.size() == 3 &&
p.containsValue("test-pwd") &&
p.containsValue("test-user")));
});
}

@Test
void initialize_fromVault_shouldOverrideConfig(CommonsConnectionPoolServiceExtension extension, ServiceExtensionContext context) {
when(context.getConfig(EDC_DATASOURCE_PREFIX))
.thenReturn(ConfigFactory.fromMap(
Map.of("ds1.name", "ds1",
"ds1.user", "this-should-be-ignored",
"ds1.password", "this-as-well")));
var vault = context.getService(Vault.class);

vault.storeSecret("edc.datasource." + DS_1_NAME + ".user", "test-user");
vault.storeSecret("edc.datasource." + DS_1_NAME + ".password", "test-pwd");
vault.storeSecret("edc.datasource." + DS_1_NAME + ".url", "jdbc://whatever");

extension.initialize(context);

verify(dataSourceRegistry).register(eq(DS_1_NAME), any());
assertThat(extension.getCommonsConnectionPools()).hasSize(1).first()
.satisfies(pool -> {
assertThatThrownBy(pool::getConnection).isInstanceOf(EdcException.class); //we need this only to invoke the connection factory
verify(connectionFactory).create(eq("jdbc://whatever"), argThat(p ->
p.size() == 3 &&
p.containsValue("test-pwd") &&
p.containsValue("test-user")));
});
}


static class ConfigProvider implements ArgumentsProvider {

Expand All @@ -106,17 +157,6 @@ static class ConfigProvider implements ArgumentsProvider {
DS_1_NAME + "." + POOL_CONNECTIONS_MAX_TOTAL, "10");


private final Map<String, String> deprecatedConfig = Map.of(
DS_1_NAME + ".url", DS_1_NAME,
DS_1_NAME + "." + DEPRACATED_POOL_TEST_CONNECTION_ON_CREATE, "false",
DS_1_NAME + "." + DEPRACATED_POOL_TEST_CONNECTION_ON_BORROW, "false",
DS_1_NAME + "." + DEPRACATED_POOL_TEST_CONNECTION_ON_RETURN, "true",
DS_1_NAME + "." + DEPRACATED_POOL_TEST_CONNECTION_WHILE_IDLE, "true",
DS_1_NAME + "." + DEPRACATED_POOL_TEST_QUERY, "SELECT foo FROM bar;",
DS_1_NAME + "." + DEPRACATED_POOL_MIN_IDLE_CONNECTIONS, "10",
DS_1_NAME + "." + DEPRACATED_POOL_MAX_IDLE_CONNECTIONS, "10",
DS_1_NAME + "." + DEPRACATED_POOL_MAX_TOTAL_CONNECTIONS, "10");

@Override
public Stream<? extends Arguments> provideArguments(ExtensionContext context) {
ThrowingConsumer<CommonsConnectionPoolConfig> checkDefault = this::checkDefault;
Expand All @@ -128,8 +168,7 @@ public Stream<? extends Arguments> provideArguments(ExtensionContext context) {
return Stream.of(
Arguments.of(defaultConfig, checkDefault, false),
Arguments.of(configuration, checkWithConfig, false),
Arguments.of(envConfiguration, checkWithConfig, true),
Arguments.of(deprecatedConfig, checkWithConfig, false)
Arguments.of(envConfiguration, checkWithConfig, true)
);
}

Expand Down

0 comments on commit 41a22c1

Please sign in to comment.