Skip to content

Commit

Permalink
Deactivate datasources when their URL is unset at runtime
Browse files Browse the repository at this point in the history
Consequently:

* Skip the health check for datasources without a URL
* Fail on startup if datasources without URL are (statically)
  injected into user bean
* Fail on first access if datasources without URL are retrieved
  dynamically through CDI
  • Loading branch information
yrodiere committed Sep 30, 2024
1 parent a0b2ee1 commit ee4f40a
Show file tree
Hide file tree
Showing 64 changed files with 579 additions and 343 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@
import org.junit.jupiter.api.extension.RegisterExtension;

import io.agroal.api.AgroalDataSource;
import io.quarkus.arc.InactiveBeanException;
import io.quarkus.arc.InjectableInstance;
import io.quarkus.runtime.configuration.ConfigurationException;
import io.quarkus.test.QuarkusUnitTest;

public class ConfigUrlMissingDefaultDatasourceDynamicInjectionTest {
Expand Down Expand Up @@ -46,7 +46,10 @@ private void doTest(InjectableInstance<? extends DataSource> instance) {
assertThat(ds).isNotNull();
// However, any attempt to use it at runtime will fail.
assertThatThrownBy(() -> ds.getConnection())
.isInstanceOf(ConfigurationException.class)
.hasMessageContainingAll("quarkus.datasource.jdbc.url has not been defined");
.isInstanceOf(InactiveBeanException.class)
.hasMessageContainingAll("Datasource '<default>' was deactivated automatically because its URL is not set.",
"To avoid this exception while keeping the bean inactive", // Message from Arc with generic hints
"To activate the datasource, set configuration property 'quarkus.datasource.jdbc.url'",
"Refer to https://quarkus.io/guides/datasource for guidance.");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@ public class ConfigUrlMissingDefaultDatasourceHealthCheckTest {
public void testDataSourceHealthCheckExclusion() {
RestAssured.when().get("/q/health/ready")
.then()
// UnconfiguredDataSource always reports as healthy...
// https://github.com/quarkusio/quarkus/issues/36666
// A datasource without a URL is inactive, and thus not checked for health.
// Note however we have checks in place to fail on startup if such a datasource is injected statically.
.body("status", CoreMatchers.equalTo("UP"))
.body("checks[0].data.\"<default>\"", CoreMatchers.equalTo("UP"));
.body("checks[0].data.\"<default>\"", CoreMatchers.nullValue());
}

}
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package io.quarkus.agroal.test;

import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.assertj.core.api.Assertions.assertThat;

import java.sql.SQLException;

Expand All @@ -9,27 +9,35 @@
import jakarta.enterprise.context.ApplicationScoped;
import jakarta.inject.Inject;

import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import io.quarkus.runtime.configuration.ConfigurationException;
import io.quarkus.arc.InactiveBeanException;
import io.quarkus.test.QuarkusUnitTest;

public class ConfigUrlMissingDefaultDatasourceStaticInjectionTest {

@RegisterExtension
static final QuarkusUnitTest config = new QuarkusUnitTest()
// The URL won't be missing if dev services are enabled
.overrideConfigKey("quarkus.devservices.enabled", "false");
.overrideConfigKey("quarkus.devservices.enabled", "false")
.assertException(e -> assertThat(e)
// Can't use isInstanceOf due to weird classloading in tests
.satisfies(t -> assertThat(t.getClass().getName()).isEqualTo(InactiveBeanException.class.getName()))
.hasMessageContainingAll("Datasource '<default>' was deactivated automatically because its URL is not set.",
"To avoid this exception while keeping the bean inactive", // Message from Arc with generic hints
"To activate the datasource, set configuration property 'quarkus.datasource.jdbc.url'",
"Refer to https://quarkus.io/guides/datasource for guidance.",
"This bean is injected into",
MyBean.class.getName() + "#ds"));

@Inject
MyBean myBean;

@Test
public void test() {
assertThatThrownBy(() -> myBean.useDatasource())
.isInstanceOf(ConfigurationException.class)
.hasMessageContainingAll("quarkus.datasource.jdbc.url has not been defined");
Assertions.fail("Startup should have failed");
}

@ApplicationScoped
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@
import org.junit.jupiter.api.extension.RegisterExtension;

import io.agroal.api.AgroalDataSource;
import io.quarkus.arc.InactiveBeanException;
import io.quarkus.arc.InjectableInstance;
import io.quarkus.runtime.configuration.ConfigurationException;
import io.quarkus.test.QuarkusUnitTest;

public class ConfigUrlMissingNamedDatasourceDynamicInjectionTest {
Expand Down Expand Up @@ -51,7 +51,10 @@ private void doTest(InjectableInstance<? extends DataSource> instance) {
assertThat(ds).isNotNull();
// However, any attempt to use it at runtime will fail.
assertThatThrownBy(() -> ds.getConnection())
.isInstanceOf(ConfigurationException.class)
.hasMessageContainingAll("quarkus.datasource.\"users\".jdbc.url has not been defined");
.isInstanceOf(InactiveBeanException.class)
.hasMessageContainingAll("Datasource 'users' was deactivated automatically because its URL is not set.",
"To avoid this exception while keeping the bean inactive", // Message from Arc with generic hints
"To activate the datasource, set configuration property 'quarkus.datasource.\"users\".jdbc.url'",
"Refer to https://quarkus.io/guides/datasource for guidance.");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@ public class ConfigUrlMissingNamedDatasourceHealthCheckTest {
public void testDataSourceHealthCheckExclusion() {
RestAssured.when().get("/q/health/ready")
.then()
// UnconfiguredDataSource always reports as healthy...
// https://github.com/quarkusio/quarkus/issues/36666
// A datasource without a JDBC URL is inactive, and thus not checked for health.
// Note however we have checks in place to fail on startup if such a datasource is injected statically.
.body("status", CoreMatchers.equalTo("UP"))
.body("checks[0].data.\"users\"", CoreMatchers.equalTo("UP"));
.body("checks[0].data.\"users\"", CoreMatchers.nullValue());
}

}
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package io.quarkus.agroal.test;

import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.assertj.core.api.Assertions.assertThat;

import java.sql.SQLException;

Expand All @@ -9,10 +9,11 @@
import jakarta.enterprise.context.ApplicationScoped;
import jakarta.inject.Inject;

import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import io.quarkus.runtime.configuration.ConfigurationException;
import io.quarkus.arc.InactiveBeanException;
import io.quarkus.test.QuarkusUnitTest;

public class ConfigUrlMissingNamedDatasourceStaticInjectionTest {
Expand All @@ -23,16 +24,23 @@ public class ConfigUrlMissingNamedDatasourceStaticInjectionTest {
.overrideConfigKey("quarkus.devservices.enabled", "false")
// We need at least one build-time property for the datasource,
// otherwise it's considered unconfigured at build time...
.overrideConfigKey("quarkus.datasource.users.db-kind", "h2");
.overrideConfigKey("quarkus.datasource.users.db-kind", "h2")
.assertException(e -> assertThat(e)
// Can't use isInstanceOf due to weird classloading in tests
.satisfies(t -> assertThat(t.getClass().getName()).isEqualTo(InactiveBeanException.class.getName()))
.hasMessageContainingAll("Datasource 'users' was deactivated automatically because its URL is not set.",
"To avoid this exception while keeping the bean inactive", // Message from Arc with generic hints
"To activate the datasource, set configuration property 'quarkus.datasource.\"users\".jdbc.url'",
"Refer to https://quarkus.io/guides/datasource for guidance.",
"This bean is injected into",
MyBean.class.getName() + "#ds"));

@Inject
MyBean myBean;

@Test
public void test() {
assertThatThrownBy(() -> myBean.useDatasource())
.isInstanceOf(ConfigurationException.class)
.hasMessageContainingAll("quarkus.datasource.\"users\".jdbc.url has not been defined");
Assertions.fail("Startup should have failed");
}

@ApplicationScoped
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
import io.agroal.api.AgroalDataSource;
import io.quarkus.agroal.DataSource;
import io.quarkus.arc.Arc;
import io.quarkus.arc.ClientProxy;
import io.quarkus.arc.InjectableInstance;
import io.quarkus.datasource.common.runtime.DataSourceUtil;

Expand All @@ -27,8 +26,7 @@ public static Optional<AgroalDataSource> dataSourceIfActive(String dataSourceNam
var instance = dataSourceInstance(dataSourceName);
// We want to call get() and throw an exception if the name points to an undefined datasource.
if (!instance.isResolvable() || instance.getHandle().getBean().isActive().result()) {
// TODO remove ClientProxy.unwrap() once we get rid of UnconfiguredDatasource
return Optional.ofNullable(ClientProxy.unwrap(instance.get()));
return Optional.ofNullable(instance.get());
} else {
return Optional.empty();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,13 @@
public class AgroalRecorder {

private final RuntimeValue<DataSourcesRuntimeConfig> runtimeConfig;
private final RuntimeValue<DataSourcesJdbcRuntimeConfig> jdbcRuntimeConfig;

@Inject
public AgroalRecorder(RuntimeValue<DataSourcesRuntimeConfig> runtimeConfig) {
public AgroalRecorder(RuntimeValue<DataSourcesRuntimeConfig> runtimeConfig,
RuntimeValue<DataSourcesJdbcRuntimeConfig> jdbcRuntimeConfig) {
this.runtimeConfig = runtimeConfig;
this.jdbcRuntimeConfig = jdbcRuntimeConfig;
}

public Supplier<AgroalDataSourceSupport> dataSourceSupportSupplier(AgroalDataSourceSupport agroalDataSourceSupport) {
Expand All @@ -39,7 +42,10 @@ public ActiveResult get() {
if (!runtimeConfig.getValue().dataSources().get(dataSourceName).active()) {
return ActiveResult.inactive(DataSourceUtil.dataSourceInactiveReasonDeactivated(dataSourceName));
}

if (jdbcRuntimeConfig.getValue().dataSources().get(dataSourceName).jdbc().url().isEmpty()) {
return ActiveResult.inactive(DataSourceUtil.dataSourceInactiveReasonUrlMissing(dataSourceName,
"jdbc.url"));
}
return ActiveResult.active();
}
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
import io.quarkus.arc.ClientProxy;
import io.quarkus.credentials.CredentialsProvider;
import io.quarkus.credentials.runtime.CredentialsProviderFinder;
import io.quarkus.datasource.common.runtime.DataSourceUtil;
import io.quarkus.datasource.runtime.DataSourceRuntimeConfig;
import io.quarkus.datasource.runtime.DataSourcesBuildTimeConfig;
import io.quarkus.datasource.runtime.DataSourcesRuntimeConfig;
Expand Down Expand Up @@ -159,10 +158,8 @@ public AgroalDataSource createDataSource(String dataSourceName) {
DataSourceJdbcRuntimeConfig dataSourceJdbcRuntimeConfig = dataSourcesJdbcRuntimeConfig
.dataSources().get(dataSourceName).jdbc();
if (!dataSourceJdbcRuntimeConfig.url().isPresent()) {
//this is not an error situation, because we want to allow the situation where a JDBC extension
//is installed but has not been configured
return new UnconfiguredDataSource(
DataSourceUtil.dataSourcePropertyKey(dataSourceName, "jdbc.url") + " has not been defined");
throw new IllegalArgumentException(
"Datasource " + dataSourceName + " does not have a JDBC URL and should not be created");
}

// we first make sure that all available JDBC drivers are loaded in the current TCCL
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@
import io.agroal.api.configuration.AgroalDataSourceConfiguration;
import io.quarkus.runtime.configuration.ConfigurationException;

/**
* @deprecated This is never instantiated by Quarkus. Do not use.
*/
@Deprecated(forRemoval = true)
public class UnconfiguredDataSource implements AgroalDataSource {

private final String errorMessage;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,14 @@ public static String dataSourceInactiveReasonDeactivated(String dataSourceName)
dataSourceName, dataSourcePropertyKey(dataSourceName, "active"), dataSourceName);
}

public static String dataSourceInactiveReasonUrlMissing(String dataSourceName, String urlPropertyRadical) {
return String.format(Locale.ROOT,
"Datasource '%s' was deactivated automatically because its URL is not set."
+ " To activate the datasource, set configuration property '%s'."
+ " Refer to https://quarkus.io/guides/datasource for guidance.",
dataSourceName, dataSourcePropertyKey(dataSourceName, urlPropertyRadical));
}

private DataSourceUtil() {
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,9 @@ public void testBootSucceedsButFlywayDeactivated() {
.isInstanceOf(CreationException.class)
.cause()
.hasMessageContainingAll("Unable to find datasource '<default>' for Flyway",
"Datasource '<default>' is not configured.",
"To solve this, configure datasource '<default>'.",
"Datasource '<default>' was deactivated automatically because its URL is not set.",
"To avoid this exception while keeping the bean inactive", // Message from Arc with generic hints
"To activate the datasource, set configuration property 'quarkus.datasource.jdbc.url'.",
"Refer to https://quarkus.io/guides/datasource for guidance.");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,9 @@ public void testBootSucceedsButFlywayDeactivated() {
assertThatThrownBy(() -> myBean.useFlyway())
.cause()
.hasMessageContainingAll("Unable to find datasource '<default>' for Flyway",
"Datasource '<default>' is not configured.",
"To solve this, configure datasource '<default>'.",
"Datasource '<default>' was deactivated automatically because its URL is not set.",
"To avoid this exception while keeping the bean inactive", // Message from Arc with generic hints
"To activate the datasource, set configuration property 'quarkus.datasource.jdbc.url'.",
"Refer to https://quarkus.io/guides/datasource for guidance.");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,9 @@ public void testBootSucceedsButFlywayDeactivated() {
.isInstanceOf(CreationException.class)
.cause()
.hasMessageContainingAll("Unable to find datasource 'users' for Flyway",
"Datasource 'users' is not configured.",
"To solve this, configure datasource 'users'.",
"Datasource 'users' was deactivated automatically because its URL is not set.",
"To avoid this exception while keeping the bean inactive", // Message from Arc with generic hints
"To activate the datasource, set configuration property 'quarkus.datasource.\"users\".jdbc.url'.",
"Refer to https://quarkus.io/guides/datasource for guidance.");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,9 @@ public void testBootSucceedsButFlywayDeactivated() {
assertThatThrownBy(() -> myBean.useFlyway())
.cause()
.hasMessageContainingAll("Unable to find datasource 'users' for Flyway",
"Datasource 'users' is not configured.",
"To solve this, configure datasource 'users'.",
"Datasource 'users' was deactivated automatically because its URL is not set.",
"To avoid this exception while keeping the bean inactive", // Message from Arc with generic hints
"To activate the datasource, set configuration property 'quarkus.datasource.\"users\".jdbc.url'.",
"Refer to https://quarkus.io/guides/datasource for guidance.");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,9 @@ public void testBootSucceedsButFlywayDeactivated() {
.isInstanceOf(CreationException.class)
.cause()
.hasMessageContainingAll("Unable to find datasource '<default>' for Flyway",
"Datasource '<default>' is not configured.",
"To solve this, configure datasource '<default>'.",
"Datasource '<default>' was deactivated automatically because its URL is not set.",
"To avoid this exception while keeping the bean inactive", // Message from Arc with generic hints
"To activate the datasource, set configuration property 'quarkus.datasource.jdbc.url'.",
"Refer to https://quarkus.io/guides/datasource for guidance.");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,9 @@ public void testBootSucceedsButFlywayDeactivated() {
.isInstanceOf(CreationException.class)
.cause()
.hasMessageContainingAll("Unable to find datasource 'users' for Flyway",
"Datasource 'users' is not configured.",
"To solve this, configure datasource 'users'.",
"Datasource 'users' was deactivated automatically because its URL is not set.",
"To avoid this exception while keeping the bean inactive", // Message from Arc with generic hints
"To activate the datasource, set configuration property 'quarkus.datasource.\"users\".jdbc.url'.",
"Refer to https://quarkus.io/guides/datasource for guidance.");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,11 @@
import org.jboss.logging.Logger;

import io.quarkus.agroal.runtime.AgroalDataSourceUtil;
import io.quarkus.agroal.runtime.UnconfiguredDataSource;
import io.quarkus.arc.Arc;
import io.quarkus.arc.ClientProxy;
import io.quarkus.arc.InactiveBeanException;
import io.quarkus.arc.InstanceHandle;
import io.quarkus.arc.SyntheticCreationalContext;
import io.quarkus.datasource.common.runtime.DataSourceUtil;
import io.quarkus.runtime.RuntimeValue;
import io.quarkus.runtime.annotations.Recorder;
import io.quarkus.runtime.configuration.ConfigurationException;
Expand Down Expand Up @@ -69,9 +67,6 @@ public FlywayContainer apply(SyntheticCreationalContext<FlywayContainer> context
// ClientProxy.unwrap is necessary to trigger exceptions on inactive datasources
dataSource = ClientProxy.unwrap(context.getInjectedReference(
DataSource.class, AgroalDataSourceUtil.qualifier(dataSourceName)));
if (dataSource instanceof UnconfiguredDataSource) {
throw DataSourceUtil.dataSourceNotConfigured(dataSourceName);
}
} catch (ConfigurationException | InactiveBeanException e) {
// TODO do we really want to enable retrieval of a FlywayContainer for an unconfigured/inactive datasource?
// Marking the FlywayContainer bean as inactive when the datasource is inactive/unconfigured
Expand Down
Loading

0 comments on commit ee4f40a

Please sign in to comment.