Skip to content

Commit

Permalink
Dev services check property value existence with expression expansion
Browse files Browse the repository at this point in the history
Removed self-referencing property expansion in oidc integration test

Fixes #41128
  • Loading branch information
ozangunalp committed Jun 27, 2024
1 parent ed77ee2 commit 6ff262e
Show file tree
Hide file tree
Showing 17 changed files with 59 additions and 28 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

import org.eclipse.microprofile.config.Config;
import org.eclipse.microprofile.config.ConfigProvider;
import org.eclipse.microprofile.config.ConfigValue;
import org.eclipse.microprofile.config.spi.ConfigSource;

import io.quarkus.runtime.LaunchMode;
Expand Down Expand Up @@ -111,6 +112,19 @@ public static boolean isPropertyPresent(String propertyName) {
return ConfigProvider.getConfig().unwrap(SmallRyeConfig.class).isPropertyPresent(propertyName);
}

/**
* Checks if a property has non-empty value in the current Configuration.
* <p>
* This method is similar to {@link #isPropertyPresent(String)}, but does not ignore expression expansion.
*
* @param propertyName the property name.
* @return true if the property is present or false otherwise.
*/
public static boolean isPropertyNonEmpty(String propertyName) {
ConfigValue configValue = ConfigProvider.getConfig().getConfigValue(propertyName);
return configValue.getValue() != null && !configValue.getValue().isEmpty();
}

/**
* Checks if any of the given properties is present in the current Configuration.
* <p>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ private DevServicesResultBuildItem.RunningDevService startElasticsearch(

for (String hostsConfigProperty : buildItemConfig.hostsConfigProperties) {
// Check if elasticsearch hosts property is set
if (ConfigUtils.isPropertyPresent(hostsConfigProperty)) {
if (ConfigUtils.isPropertyNonEmpty(hostsConfigProperty)) {
log.debugf("Not starting Dev Services for Elasticsearch, the %s property is configured.", hostsConfigProperty);
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ void devServicesAutoGenerateByDefault(List<JdbcDataSourceSchemaReadyBuildItem> s
String databaseGenerationPropertyKey = HibernateOrmRuntimeConfig.puPropertyKey(entry.getKey(),
"database.generation");
if (!ConfigUtils.isAnyPropertyPresent(propertyKeysIndicatingDataSourceConfigured)
&& !ConfigUtils.isPropertyPresent(databaseGenerationPropertyKey)) {
&& !ConfigUtils.isPropertyNonEmpty(databaseGenerationPropertyKey)) {
devServicesAdditionalConfigProducer
.produce(new DevServicesAdditionalConfigBuildItem(devServicesConfig -> {
// Only force DB generation if the datasource is configured through dev services
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -190,8 +190,8 @@ private RunningDevService startContainer(String clientName, DockerStatusBuildIte

String configPrefix = getConfigPrefix(clientName);

boolean needToStart = !ConfigUtils.isPropertyPresent(configPrefix + "hosts")
&& !ConfigUtils.isPropertyPresent(configPrefix + "server-list");
boolean needToStart = !ConfigUtils.isPropertyNonEmpty(configPrefix + "hosts")
&& !ConfigUtils.isPropertyNonEmpty(configPrefix + "server-list");

if (!needToStart) {
log.debug("Not starting Dev Services for Infinispan as 'hosts', 'uri' or 'server-list' have been provided");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ private RunningDevService startKafka(DockerStatusBuildItem dockerStatusBuildItem
}

// Check if kafka.bootstrap.servers is set
if (ConfigUtils.isPropertyPresent(KAFKA_BOOTSTRAP_SERVERS)) {
if (ConfigUtils.isPropertyNonEmpty(KAFKA_BOOTSTRAP_SERVERS)) {
log.debug("Not starting dev services for Kafka, the kafka.bootstrap.servers is configured.");
return null;
}
Expand Down Expand Up @@ -295,7 +295,7 @@ private boolean hasKafkaChannelWithoutBootstrapServers() {
&& "smallrye-kafka".equals(config.getOptionalValue(name, String.class).orElse("ignored"));
boolean isConfigured = false;
if ((isIncoming || isOutgoing) && isKafka) {
isConfigured = ConfigUtils.isPropertyPresent(name.replace(".connector", ".bootstrap.servers"));
isConfigured = ConfigUtils.isPropertyNonEmpty(name.replace(".connector", ".bootstrap.servers"));
}
if (!isConfigured) {
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ private RunningDevService startKubernetes(DockerStatusBuildItem dockerStatusBuil
}

// Check if kubernetes-client.api-server-url is set
if (ConfigUtils.isPropertyPresent(KUBERNETES_CLIENT_MASTER_URL)) {
if (ConfigUtils.isPropertyNonEmpty(KUBERNETES_CLIENT_MASTER_URL)) {
log.debug("Not starting Dev Services for Kubernetes, the " + KUBERNETES_CLIENT_MASTER_URL + " is configured.");
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ private RunningDevService startMongo(DockerStatusBuildItem dockerStatusBuildItem
String configPrefix = getConfigPrefix(connectionName);

// TODO: do we need to check the hosts as well?
boolean needToStart = !ConfigUtils.isPropertyPresent(configPrefix + "connection-string");
boolean needToStart = !ConfigUtils.isPropertyNonEmpty(configPrefix + "connection-string");
if (!needToStart) {
// a connection string has been provided
log.debug("Not starting devservices for " + (isDefault(connectionName) ? "default datasource" : connectionName)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,15 @@

import static org.hamcrest.Matchers.equalTo;

import jakarta.annotation.PostConstruct;
import jakarta.annotation.security.RolesAllowed;
import jakarta.inject.Inject;
import jakarta.inject.Singleton;
import jakarta.ws.rs.GET;
import jakarta.ws.rs.Path;
import jakarta.ws.rs.QueryParam;

import org.eclipse.microprofile.config.inject.ConfigProperty;
import org.eclipse.microprofile.jwt.JsonWebToken;
import org.eclipse.microprofile.rest.client.inject.RegisterRestClient;
import org.eclipse.microprofile.rest.client.inject.RestClient;
Expand Down Expand Up @@ -39,7 +41,7 @@ public class AccessTokenAnnotationTest {
.addAsResource(
new StringAsset(
"""
quarkus.oidc.auth-server-url=${keycloak.url}/realms/quarkus
quarkus.oidc.auth-server-url=${keycloak.url:replaced-by-test}/realms/quarkus
quarkus.oidc.client-id=quarkus-app
quarkus.oidc.credentials.secret=secret
Expand Down Expand Up @@ -144,6 +146,22 @@ public static class MultiProviderFrontendResource {
@Inject
JsonWebToken jwt;

@ConfigProperty(name = "quarkus.oidc.auth-server-url")
String authServerUrl;

@ConfigProperty(name = "quarkus.oidc-client.auth-server-url")
String clientAuthServerUrl;

@ConfigProperty(name = "keycloak.url")
String keycloakUrl;

@PostConstruct
void init() {
System.out.println("keycloakUrl: " + keycloakUrl);
System.out.println("authServerUrl: " + authServerUrl);
System.out.println("clientAuthServerUrl: " + clientAuthServerUrl);
}

@GET
@Path("token-propagation")
@RolesAllowed("admin")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public class AccessTokenAnnotationTest {
.addAsResource(
new StringAsset(
"""
quarkus.oidc.auth-server-url=${keycloak.url}/realms/quarkus
quarkus.oidc.auth-server-url=${keycloak.url:replaced-by-test}/realms/quarkus
quarkus.oidc.client-id=quarkus-app
quarkus.oidc.credentials.secret=secret
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -336,11 +336,11 @@ private RunningDevService startContainer(DockerStatusBuildItem dockerStatusBuild
LOG.debug("Not starting Dev Services for Keycloak as 'quarkus.oidc.tenant.enabled' is false");
return null;
}
if (ConfigUtils.isPropertyPresent(AUTH_SERVER_URL_CONFIG_KEY)) {
if (ConfigUtils.isPropertyNonEmpty(AUTH_SERVER_URL_CONFIG_KEY)) {
LOG.debug("Not starting Dev Services for Keycloak as 'quarkus.oidc.auth-server-url' has been provided");
return null;
}
if (ConfigUtils.isPropertyPresent(PROVIDER_CONFIG_KEY)) {
if (ConfigUtils.isPropertyNonEmpty(PROVIDER_CONFIG_KEY)) {
LOG.debug("Not starting Dev Services for Keycloak as 'quarkus.oidc.provider' has been provided");
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ private RunningDevService startContainer(DockerStatusBuildItem dockerStatusBuild

String configPrefix = getConfigPrefix(name);

boolean needToStart = !ConfigUtils.isPropertyPresent(configPrefix + RedisConfig.HOSTS_CONFIG_NAME);
boolean needToStart = !ConfigUtils.isPropertyNonEmpty(configPrefix + RedisConfig.HOSTS_CONFIG_NAME);
if (!needToStart) {
log.debug("Not starting devservices for " + (RedisConfig.isDefaultClient(name) ? "default redis client" : name)
+ " as hosts have been provided");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,12 +146,12 @@ private RunningDevService startApicurioRegistry(DockerStatusBuildItem dockerStat
return null;
}

if (ConfigUtils.isPropertyPresent(APICURIO_REGISTRY_URL_CONFIG)) {
if (ConfigUtils.isPropertyNonEmpty(APICURIO_REGISTRY_URL_CONFIG)) {
log.debug("Not starting dev services for Apicurio Registry, " + APICURIO_REGISTRY_URL_CONFIG + " is configured.");
return null;
}

if (ConfigUtils.isPropertyPresent(CONFLUENT_SCHEMA_REGISTRY_URL_CONFIG)) {
if (ConfigUtils.isPropertyNonEmpty(CONFLUENT_SCHEMA_REGISTRY_URL_CONFIG)) {
log.debug("Not starting dev services for Apicurio Registry, " + CONFLUENT_SCHEMA_REGISTRY_URL_CONFIG
+ " is configured.");
return null;
Expand Down Expand Up @@ -198,8 +198,8 @@ private boolean hasKafkaChannelWithoutRegistry() {
&& "smallrye-kafka".equals(config.getOptionalValue(name, String.class).orElse("ignored"));
boolean isConfigured = false;
if ((isIncoming || isOutgoing) && isKafka) {
isConfigured = ConfigUtils.isPropertyPresent(name.replace(".connector", ".apicurio.registry.url"))
|| ConfigUtils.isPropertyPresent(name.replace(".connector", ".schema.registry.url"));
isConfigured = ConfigUtils.isPropertyNonEmpty(name.replace(".connector", ".apicurio.registry.url"))
|| ConfigUtils.isPropertyNonEmpty(name.replace(".connector", ".schema.registry.url"));
}
if (!isConfigured) {
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ private RunningDevService startAmqpBroker(DockerStatusBuildItem dockerStatusBuil
}

// Check if amqp.port or amqp.host are set
if (ConfigUtils.isPropertyPresent(AMQP_HOST_PROP) || ConfigUtils.isPropertyPresent(AMQP_PORT_PROP)) {
if (ConfigUtils.isPropertyNonEmpty(AMQP_HOST_PROP) || ConfigUtils.isPropertyNonEmpty(AMQP_PORT_PROP)) {
log.debug("Not starting Dev Services for AMQP, the amqp.host and/or amqp.port are configured.");
return null;
}
Expand Down Expand Up @@ -223,8 +223,8 @@ private boolean hasAmqpChannelWithoutHostAndPort() {
if ((isIncoming || isOutgoing) && isConnector) {
String connectorValue = config.getValue(name, String.class);
boolean isAmqp = connectorValue.equalsIgnoreCase("smallrye-amqp");
boolean hasHost = ConfigUtils.isPropertyPresent(name.replace(".connector", ".host"));
boolean hasPort = ConfigUtils.isPropertyPresent(name.replace(".connector", ".port"));
boolean hasHost = ConfigUtils.isPropertyNonEmpty(name.replace(".connector", ".host"));
boolean hasPort = ConfigUtils.isPropertyNonEmpty(name.replace(".connector", ".port"));
isConfigured = isAmqp && (hasHost || hasPort);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -207,8 +207,8 @@ private boolean hasMqttChannelWithoutHostAndPort() {
if ((isIncoming || isOutgoing) && isConnector) {
String connectorValue = config.getValue(name, String.class);
boolean isMqtt = connectorValue.equalsIgnoreCase("smallrye-mqtt");
boolean hasHost = ConfigUtils.isPropertyPresent(name.replace(".connector", ".host"));
boolean hasPort = ConfigUtils.isPropertyPresent(name.replace(".connector", ".port"));
boolean hasHost = ConfigUtils.isPropertyNonEmpty(name.replace(".connector", ".host"));
boolean hasPort = ConfigUtils.isPropertyNonEmpty(name.replace(".connector", ".port"));
isConfigured = isMqtt && (hasHost || hasPort);
if (!isConfigured) {
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ private RunningDevService startPulsarContainer(DockerStatusBuildItem dockerStatu
}

// Check if pulsar.serviceUrl is set
if (ConfigUtils.isPropertyPresent(PULSAR_CLIENT_SERVICE_URL)) {
if (ConfigUtils.isPropertyNonEmpty(PULSAR_CLIENT_SERVICE_URL)) {
log.debug("Not starting Dev Services for Pulsar, the pulsar.serviceUrl is configured.");
return null;
}
Expand Down Expand Up @@ -216,7 +216,7 @@ private boolean hasPulsarChannelWithoutHostAndPort() {
if ((isIncoming || isOutgoing) && isConnector) {
String connectorValue = config.getValue(name, String.class);
boolean isPulsar = connectorValue.equalsIgnoreCase("smallrye-pulsar");
boolean hasServiceUrl = ConfigUtils.isPropertyPresent(name.replace(".connector", ".serviceUrl"));
boolean hasServiceUrl = ConfigUtils.isPropertyNonEmpty(name.replace(".connector", ".serviceUrl"));
isConfigured = isPulsar && hasServiceUrl;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ private RunningDevService startRabbitMQBroker(DockerStatusBuildItem dockerStatus
}

// Check if rabbitmq-port or rabbitmq-host are set
if (ConfigUtils.isPropertyPresent(RABBITMQ_HOST_PROP) || ConfigUtils.isPropertyPresent(RABBITMQ_PORT_PROP)) {
if (ConfigUtils.isPropertyNonEmpty(RABBITMQ_HOST_PROP) || ConfigUtils.isPropertyNonEmpty(RABBITMQ_PORT_PROP)) {
log.debug("Not starting Dev Services for RabbitMQ, the rabbitmq-host and/or rabbitmq-port are configured.");
return null;
}
Expand Down Expand Up @@ -229,8 +229,8 @@ private boolean hasRabbitMQChannelWithoutHostAndPort() {
if ((isIncoming || isOutgoing) && isConnector) {
String connectorValue = config.getValue(name, String.class);
boolean isRabbitMQ = connectorValue.equalsIgnoreCase("smallrye-rabbitmq");
boolean hasHost = ConfigUtils.isPropertyPresent(name.replace(".connector", ".host"));
boolean hasPort = ConfigUtils.isPropertyPresent(name.replace(".connector", ".port"));
boolean hasHost = ConfigUtils.isPropertyNonEmpty(name.replace(".connector", ".host"));
boolean hasPort = ConfigUtils.isPropertyNonEmpty(name.replace(".connector", ".port"));
isConfigured = isRabbitMQ && (hasHost || hasPort);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
# Configuration file
quarkus.oidc.auth-server-url=${quarkus.oidc.auth-server-url}
quarkus.oidc.client-id=quarkus-service-app
quarkus.oidc.credentials.secret=secret
quarkus.oidc.token.principal-claim=email
Expand Down

0 comments on commit 6ff262e

Please sign in to comment.