Require setting DB kind for additional datasources #41087
Conversation
0de3c1a to
f51691a
Compare
...src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/WildcardPropertyMapper.java
Outdated
Show resolved
Hide resolved
quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/KeycloakRecorder.java
Outdated
Show resolved
Hide resolved
Unreported flaky test detectedIf the flaky tests below are affected by the changes, please review and update the changes accordingly. Otherwise, a maintainer should report the flaky tests prior to merging the PR. org.keycloak.testsuite.account.AccountRestServiceTest#listApplicationsWithoutPermissionKeycloak CI - Java Distribution IT (windows-latest - temurin - 17) org.keycloak.testsuite.account.AccountRestServiceTest#testUpdateProfileWithRegistrationEmailAsUsernameKeycloak CI - Java Distribution IT (windows-latest - temurin - 17) |
f51691a to
94d9548
Compare
94d9548 to
7db64cc
Compare
Unreported flaky test detectedIf the flaky tests below are affected by the changes, please review and update the changes accordingly. Otherwise, a maintainer should report the flaky tests prior to merging the PR. org.keycloak.testsuite.cluster.RoleInvalidationClusterTest#crudWithFailover |
Closes keycloak#41161 Signed-off-by: Martin Bartoš <mabartos@redhat.com>
7db64cc to
88268a1
Compare
| .filter(dbKind -> Configuration.getKcConfigValue(dbKind).getValue() == null) // not provided | ||
| .toList(); | ||
|
|
||
| if (!notSetPersistenceUnitsDBKinds.isEmpty()) { |
There was a problem hiding this comment.
Just to make sure, we're okay with this level of a breaking change? We talked previously about how persistence.xml is supported, but the use of quarkus properties still isn't, so this is grey area.
cc @vmuzikar
There was a problem hiding this comment.
@shawkins What about to ask for the quarkus.datasource."<datasource>".db-kind instead of kc.db-kind-<datasource> as we could fetch values from the quarkus.properties? In that case, we could still support both variants.
But in that case, we would not know anything about the wildcard name and we would not be able to advertise more properties such as JDBC URL to Quarkus used here: #41026
I'll think about it if we have some better alternatives.
There was a problem hiding this comment.
But in that case, we would not know anything about the wildcard name
Our current logic presumes quoting and will associate the corresponding quoted quarkus property with the keycloak property. But I believe the smallrye handling does not do quoting normalizations, so if they leave the name of the datasource unquoted we won't find it.
|
I'd like to take a look at this early this week. |
Closes #41161
@shawkins Could you please check it? Thanks!