quarkusio / quarkus

Quarkus: Supersonic Subatomic Java.
https://quarkus.io
Apache License 2.0
13.73k stars 2.67k forks source link

Quarkus datasource critical errors for missing configurations #18797

Open lgtti opened 3 years ago

lgtti commented 3 years ago

Describe the bug

If you don't set the datasource configurations username and passwords, the agroal extension prints: 08:47:26 WARN [io.ag.pool]] (agroal-11) Datasource '': The server requested password-based authentication, but no password was provided. 08:47:26 WARN [or.hi.en.jd.en.in.JdbcEnvironmentInitiator]] (main) HHH000342: Could not obtain connection to query metadata: org.postgresql.util.PSQLException: The server requested password-based authentication, but no password was provided. -/ // / // / |/ , / ,< / // /\ \ --___// |//|//||____// 09:16:59 INFO [it.in.ra.co.RaneStarter]] (main) starting rane 09:16:59 INFO [it.in.ra.co.RaneStarter]] (main) rane started 09:17:00 WARN [io.ag.pool]] (agroal-11) Datasource '': The server requested password-based authentication, but no password was provided. 09:17:00 WARN [or.hi.en.jd.en.in.JdbcEnvironmentInitiator]] (main) HHH000342: Could not obtain connection to query metadata: org.postgresql.util.PSQLException: The server requested password-based authentication, but no password was provided. at org.postgresql.core.v3.ConnectionFactoryImpl.doAuthentication(ConnectionFactoryImpl.java:633) at org.postgresql.core.v3.ConnectionFactoryImpl.tryConnect(ConnectionFactoryImpl.java:161) at org.postgresql.core.v3.ConnectionFactoryImpl.openConnectionImpl(ConnectionFactoryImpl.java:213) at org.postgresql.core.ConnectionFactory.openConnection(ConnectionFactory.java:51) at org.postgresql.jdbc.PgConnection.(PgConnection.java:223) at org.postgresql.Driver.makeConnection(Driver.java:465) at org.postgresql.Driver.connect(Driver.java:264) at io.agroal.pool.ConnectionFactory.createConnection(ConnectionFactory.java:204) at io.agroal.pool.ConnectionPool$CreateConnectionTask.call(ConnectionPool.java:490) at io.agroal.pool.ConnectionPool$CreateConnectionTask.call(ConnectionPool.java:472) at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264) at io.agroal.pool.util.PriorityScheduledExecutor.beforeExecute(PriorityScheduledExecutor.java:68) at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1126) at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628) at java.base/java.lang.Thread.run(Thread.java:829)

These are only warning logs but the database connection is not started.

Expected behavior

The execution must be stopped because this is a critical and not a recoverable error. Basically mandatory configurations are missing.

Actual behavior

The log prints a simple log warning and the execution still continue.

How to Reproduce?

Don't configure any username or password configuration for the datasource.

Output of uname -a or ver

5.4.72-microsoft-standard-WSL2

Output of java -version

OpenJDK 64-Bit Server VM (build 11.0.10+9-Ubuntu-0ubuntu1.20.04, mixed mode)

GraalVM version (if different from Java)

No response

Quarkus version or git rev

1.13.2.Final

Build tool (ie. output of mvnw --version or gradlew --version)

Apache Maven 3.6.3

Additional information

No response

gsmet commented 3 years ago

/cc @barreiro @Sanne

quarkus-bot[bot] commented 3 years ago

/cc @Sanne, @barreiro, @yrodiere

Sanne commented 3 years ago

@lgtti thanks for the report.

Did you set any other configuration properties? I'm particularly interested in those for the datasource, as some option exist intentionally to start the pool lazily - which is a requirement for some users.

yrodiere commented 3 years ago

@Sanne From what I can see the error happens in JdbcEnvironmentInitiator, probably when auto-detecting some options by opening a connection. IIRC, we ignore errors when we do this, because the database might be down.

Once again (and I hate to admit it), it seems hardcoding the value of these options might be a good idea. If we had such hardcoded values available for the most common setups, then in the rare cases where we don't know which values to use (unknown DB, unknown JDBC driver, ...), we could open a connection to use this "auto-detection", and fail if we cannot open a connection. Which would solve this ticket.

Of course, all this hardcoding has to be done first, and I'm not even sure that's possible. We'd have to have a closer look.

Sanne commented 3 years ago

Sure, but don't forget also that Agroal is supposed by default to start eagerly, precisely to validate connections but also to pre-fill the pool. Hibernate ORM is just a client and at this point the bootstrap should already have failed.

But yes we might also want to make ORM more picky, since as I mentioned it's possible to configure Agroal to be lazy: so I guess we have a gap in validation when these situations are combined.

lgtti commented 3 years ago

@lgtti thanks for the report.

Did you set any other configuration properties? I'm particularly interested in those for the datasource, as some option exist intentionally to start the pool lazily - which is a requirement for some users.

My config/application.properties contains very few datasource configurations:

quarkus.datasource.jdbc.url=jdbc:postgresql://database:5432/postgres
quarkus.datasource.username=postgres
quarkus.datasource.password=postgres
quarkus.datasource.health.enabled=true

I have noticed the problem when i have deployed the file with kubernetes and i have written quarkus,datasource.password (note the ',' after quarkus word). I'm very careful during an update and in the log i have read the Warning but without this check my pods were all ok and green. This is the real problem.

barreiro commented 3 years ago

Is debatable that this a non recoverable error. I certainly get your arguments, but in Agroal the security is pluggable and every time a new connection is requested the security layer is queried for principal and credentials. There are scenarios where these change over time and Agroal is designed to deal with it.

The health check should not succeed though, as it won't be able to get a connection in the first place (let along a valid one).

lgtti commented 3 years ago

Is debatable that this a non recoverable error. I certainly get your arguments, but in Agroal the security is pluggable and every time a new connection is requested the security layer is queried for principal and credentials. There are scenarios where these change over time and Agroal is designed to deal with it.

The health check should not succeed though, as it won't be able to get a connection in the first place (let along a valid one).

I can agree with your assertion, but i think you cannot assume that every configuration needs a lazy check. It is correct to support deployments with lazy connection startup or dynamic configurations, but it is also correct to support static ones.

Adding a new configuration option, quarkus.datasource.lazy (true=fail on connection open at runtime, false=check the configuration at startup (make a simple query to test connection)), could be a solution?

Sanne commented 3 years ago

I agree, some applications will need lazy checking, others require an immediate check.

I'm pretty sure this was working in Quarkus 1.x by controlling quarkus.datasource.jdbc.initial-size: if I set this to any N>0 I would expect a fatal error immediately on boot, if the N connections can't be established. (But we're only getting a WARN logged today, and we should fix that)

barreiro commented 3 years ago

Are you sure !? ... It would imply delaying the boot for the time it takes to establish all the connections. It may take seconds, witch I believe is unacceptable.

This should be handled by health check, that's what that feature is for!

Sanne commented 3 years ago

if initial-size is being set, it should be honoured no? Why would that be unacceptable?

Do you have a better suggestion? For example if Agroal is not configured to use a pluggable password source, it would be reasonable to fail fast when the password is missing.

jcunliffe1 commented 2 years ago

It's fairly confusing to get a custom quarkus.datasource.devservices.image-name working, without knowing how to then provide/set the password on this custom image.