8000 Additional datasources now require XA by vmuzikar · Pull Request #32403 · keycloak/keycloak · GitHub
[go: up one dir, main page]

Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,15 @@ Additionally, the following resources have been removed from the `common` theme:

If you previously used any of the removed resources in your theme, make sure to add them to your own theme resources instead.

= Additional datasources now require using XA

{project_name} by default does not use XA datasources. However, this is considered unsafe if more than one datasource is used.
Starting with this release, you need to use XA datasources if you are adding additional datasources to {project_name}.
If the default datasource supports XA, you can do this by setting the `--transaction-xa-enabled=true` option. For additional datasources, you need to use
the `quarkus.datasource.<your-datasource-name>.jdbc.transactions=xa` option in your `quarkus.properties` file.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should also mention that you'll get a warning about recovery not being enabled, and likely mention that there isn't out-of-the box support via the operator.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think I noticed any warning.

Operator supports the .spec.transaction. xaEnabled field.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I had forgotten that we set quarkus.transaction-manager.enable-recovery=true along with the other changes to disable XA by default - so there will be no warning. In that case we just need to document that recovery isn't supported when you don't have persistent storage for the transaction store which is especially true when using the operator.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a note on the recovery.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a note on the recovery.

We should expand on that with a WARNING and call out containerized / operator usage specifically. If it's not clear when the full xa features are not available, then this solution is not much better than setting the workaround flag.

cc @yrodiere

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shawkins I wonder if we could do that as a follow-up. This change here is to just document the breaking change we did that XA is now required. The upgrading guide is IMHO not supposed to supplement a proper 8000 docs for additional datasources which will be added as part of #29116. On a side note, AFAIK, Quarkus itself doesn't provide such warning in their docs.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we could do that as a follow-up.

Sounds good.

On a side note, AFAIK, Quarkus itself doesn't provide such warning in their docs.

I agree https://quarkus.io/guides/transaction does not spell out the containerized scenario, but it seems like it could be less obvious or assumed to be turn-key when utilized via Keycloak so we should be clear about expectations.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a comment in #29116 to remind us later.

At most one datasource can be non-XA.
Recovery isn't supported when you don't have persistent storage for the transaction store.

= Hostname v1 feature removed

The deprecated hostname v1 feature was removed. This feature was deprecated in {project_name} 25 and replaced by hostname v2. If you are still using this feature, you must migrate to hostname v2. For more details, see the https://www.keycloak.org/server/hostname[Configuring the hostname (v2)] and https://www.keycloak.org/docs/latest/upgrading/#new-hostname-options[the initial migration guide].
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/*
* Copyright 2024 Red Hat, Inc. and/or its affiliates
* and other contributors as indicated by the @author tags.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.keycloak.quarkus.deployment;

import io.quarkus.builder.item.EmptyBuildItem;

/**
* @author Vaclav Muzikar <vmuzikar@redhat.com>
*/
public class CheckMultipleDatasourcesBuildStep extends EmptyBuildItem {}
8BD0
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,17 @@

package org.keycloak.quarkus.deployment;

import io.quarkus.agroal.runtime.DataSourcesJdbcBuildTimeConfig;
import io.quarkus.agroal.runtime.TransactionIntegration;
import io.quarkus.agroal.runtime.health.DataSourceHealthCheck;
import io.quarkus.agroal.spi.JdbcDataSourceBuildItem;
import io.quarkus.agroal.spi.JdbcDriverBuildItem;
import io.quarkus.arc.deployment.AnnotationsTransformerBuildItem;
import io.quarkus.arc.deployment.BuildTimeConditionBuildItem;
import io.quarkus.bootstrap.logging.InitialConfigurator;
import io.quarkus.builder.item.EmptyBuildItem;
import io.quarkus.datasource.deployment.spi.DevServicesDatasourceResultBuildItem;
import io.quarkus.datasource.runtime.DataSourcesBuildTimeConfig;
import io.quarkus.deployment.IsDevelopment;
import io.quarkus.deployment.annotations.BuildProducer;
import io.quarkus.deployment.annotations.BuildStep;
Expand Down Expand Up @@ -267,13 +271,33 @@ void checkJdbcDriver(BuildProducer<JdbcDriverBuildItem> ignore) {
// We do not want to initialize the JDBC driver class
Class.forName(dbDriver.get(), false, Thread.currentThread().getContextClassLoader());
} catch (ClassNotFoundException e) {
// Ignore queued TRACE and DEBUG messages for not initialized log handlers
InitialConfigurator.DELAYED_HANDLER.setBuildTimeHandlers(new Handler[]{});
throw new ConfigurationException(String.format("Unable to find the JDBC driver (%s). You need to install it.", dbDriver.get()));
throwConfigError(String.format("Unable to find the JDBC driver (%s). You need to install it.", dbDriver.get()));
}
}
}

// Inspired by AgroalProcessor
@BuildStep
@Produce(CheckMultipleDatasourcesBuildStep.class)
void checkMultipleDatasourcesUseXA(DataSourcesBuildTimeConfig dataSourcesConfig, DataSourcesJdbcBuildTimeConfig jdbcConfig) {
long nonXADatasourcesCount = dataSourcesConfig.dataSources().keySet().stream()
.map(ds -> jdbcConfig.dataSources().get(ds).jdbc())
.filter(jdbc -> jdbc.enabled() && jdbc.transactions() != TransactionIntegration.XA)
.count();
if (nonXADatasourcesCount > 1) {
throwConfigError("Multiple datasources are configured but more than 1 is using non-XA transactions. " +
"All the datasources except one must must be XA to be able to use Last Resource Commit Optimization (LRCO). " +
Comment thread
vmuzikar marked this conversation as resolved.
Resolved
"Please update your configuration by setting --transaction-xa-enabled=true " +
"and/or quarkus.datasource.<your-datasource-name>.jdbc.transactions=xa.");
}
}

private void throwConfigError(String msg) {
// Ignore queued TRACE and DEBUG messages for not initialized log handlers
InitialConfigurator.DELAYED_HANDLER.setBuildTimeHandlers(new Handler[]{});
throw new ConfigurationException(msg);
}

/**
* Parse the default configuration for the User Profile provider
*/
Expand Down Expand Up @@ -344,6 +368,7 @@ void configurePersistenceUnits(HibernateOrmConfig config,

@BuildStep
@Consume(CheckJdbcBuildStep.class)
@Consume(CheckMultipleDatasourcesBuildStep.class)
void produceDefaultPersistenceUnit(BuildProducer<PersistenceXmlDescriptorBuildItem> producer) {
ParsedPersistenceXmlDescriptor descriptor = PersistenceXmlParser.locateIndividualPersistenceUnit(
Thread.currentThread().getContextClassLoader().getResource("default-persistence.xml"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,5 @@

quarkus.datasource.user-store.db-kind=h2
quarkus.datasource.user-store.username=sa
quarkus.datasource.user-store.jdbc.url=jdbc:h2:mem:user-store;DB_CLOSE_DELAY=-1
quarkus.datasource.user-store.jdbc.url=jdbc:h2:mem:user-store;DB_CLOSE_DELAY=-1
quarkus.datasource.user-store.jdbc.transactions=xa
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@

package org.keycloak.it.cli.dist;

import static io.restassured.RestAssured.when;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;

Expand Down Expand Up @@ -114,6 +113,15 @@ void testWrappedBuildPropertyTriggersBuildButGetsIgnoredWhenSetByQuarkus(LaunchR
cliResult.assertStarted();
}

@Test
@BeforeStartDistribution(AddNonXADatasource.class)
@Launch({ "start" })
@Order(9)
void nonXADatasourceFailsToStart(LaunchResult result) {
CLIResult cliResult = (CLIResult) result;
cliResult.assertError("Multiple datasources are configured but more than 1 is using non-XA transactions.");
}

public static class UpdateConsoleLogLevelToWarn implements Consumer<KeycloakDistribution> {
@Override
public void accept(KeycloakDistribution distribution) {
Expand All @@ -135,6 +143,7 @@ public void accept(KeycloakDistribution distribution) {
distribution.setQuarkusProperty("quarkus.datasource.user-store.db-kind", "h2");
distribution.setQuarkusProperty("quarkus.datasource.user-store.username","sa");
distribution.setQuarkusProperty("quarkus.datasource.user-store.jdbc.url","jdbc:h2:mem:user-store;DB_CLOSE_DELAY=-1");
distribution.setQuarkusProperty("quarkus.datasource.user-store.jdbc.transactions", "xa");
}
}

Expand All @@ -145,6 +154,17 @@ public void accept(KeycloakDistribution distribution) {
distribution.setQuarkusProperty("quarkus.datasource.user-store2.db-transactions", "enabled");
distribution.setQuarkusProperty("quarkus.datasource.user-store2.username","sa");
distribution.setQuarkusProperty("quarkus.datasource.user-store2.jdbc.url","jdbc:h2:mem:user-store2;DB_CLOSE_DELAY=-1");
distribution.setQuarkusProperty("quarkus.datasource.user-store2.jdbc.transactions", "xa");
}
}

public static class AddNonXADatasource implements Consumer<KeycloakDistribution> {
@Override
public void accept(KeycloakDistribution distribution) {
distribution.setQuarkusProperty("quarkus.datasource.user-store3.db-kind", "h2");
distribution.setQuarkusProperty("quarkus.datasource.user-store3.db-transactions", "enabled");
distribution.setQuarkusProperty("quarkus.datasource.user-store3.username","sa");
distribution.setQuarkusProperty("quarkus.datasource.user-store3.jdbc.url","jdbc:h2:mem:user-store2;DB_CLOSE_DELAY=-1");
}
}

Expand All @@ -169,4 +189,4 @@ public void accept(KeycloakDistribution distribution) {
distribution.setQuarkusProperty("quarkus.datasource.db-kind", "postgres");
}
}
}
}
0