+
Skip to content

test: Provide test cases for datasources ENV vars handling #40760

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 1, 2025
Merged

test: Provide test cases for datasources ENV vars handling #40760

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
48 changes: 48 additions & 0 deletions ...rc/test/java/org/keycloak/quarkus/runtime/configuration/DatasourcesConfigurationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -387,4 +387,52 @@ public void poolSizeInherit() {
"quarkus.datasource.\"users\".jdbc.max-size", "115"
));
}

@Test
public void envVarsHandling() {
putEnvVars(Map.of(
"KC_DB_KIND_USER_STORE", "postgres",
"KC_DB_URL_FULL_USER_STORE", "jdbc:postgresql://localhost/KEYCLOAK",
"KC_DB_USERNAME_USER_STORE", "my-username",
"KC_DB_KIND_MY_STORE", "mariadb"
));
initConfig();

assertConfig(Map.of(
"db-kind-user-store", "postgres",
"db-url-full-user-store", "jdbc:postgresql://localhost/KEYCLOAK",
"db-username-user-store", "my-username",
"db-kind-my-store", "mariadb",
"db-kind-my.store", "mariadb"
));

assertExternalConfig(Map.of(
"quarkus.datasource.\"user-store\".db-kind", "postgresql",
"quarkus.datasource.\"user-store\".jdbc.url", "jdbc:postgresql://localhost/KEYCLOAK",
"quarkus.datasource.\"user-store\".username", "my-username",
"quarkus.datasource.\"my-store\".db-kind", "mariadb",
"quarkus.datasource.\"my.store\".db-kind", "mariadb"
Comment on lines +413 to +414
Copy link
Contributor Author

@mabartos mabartos Jun 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Even though we're able to obtain the configuration for both variants (my-store and my.store), the named datasource in Quarkus is my.store. See https://github.com/keycloak/keycloak/pull/40760/files#diff-161196ef0d1fee87940b57c35aa7d52ccc6da78c93eed92064e51483e047e101R36

It would be probably better to have the - (minus) char the default for datasources, but probably not a big deal when we document it.

cc: @shawkins

Copy link
Contributor

Choose a reason for hiding this comment

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

Even though we're able to obtain the configuration for both variants (my-store and my.store)

That is a side effect of the quarkus env config source. Anything that could map to the env form can be used. Only the my.store form is the canonical name that is advertised to quarkus (my!store). It would be fine to remove the assertions looking for the my-store form.

It would be probably better to have the - (minus) char the default for datasources

Yes, it would be best to do that for everything that is not logging. It doesn't make sense to default to the quarkus convention when we use '-'. I was going to add this for the spi work, but that didn't move forward, so it can be done here if you want.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it would be best to do that for everything that is not logging. It doesn't make sense to default to the quarkus convention when we use '-'. I was going to add this for the spi work, but that didn't move forward, so it can be done here if you want.

@shawkins Agree. So, could you please provide such changes?

Copy link
Contributor

Choose a reason for hiding this comment

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

it would be best to do that for everything that is not logging. It doesn't make sense to default to the quarkus convention when we use '-'

+1
Though, it would be considered a breaking change. We're already releasing the initial additional datasources support in 26.3

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Though, it would be considered a breaking change. We're already releasing the initial additional datasources support in 26.3

@vmuzikar Depends on how you look at it. Understand that we already have these options for it, but we don't state anywhere that it's supported, and we don't even have the documentation for it yet. So maybe it should not be a big deal.

Copy link
Contributor

Choose a reason for hiding this comment

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

@mabartos Good point that there's no docs. Though it's included in --help and NOT marked as a preview or something. We should've probably done that, I didn't realize it. :/

));
}

@Test
public void envVarsSpecialChars() {
putEnvVars(Map.of(
"KC_USER_STORE_DB_KIND", "mariadb",
"KCKEY_USER_STORE_DB_KIND", "db-kind-user_store$something",
"KC_CLIENT_STORE_PW", "password",
"KCKEY_CLIENT_STORE_PW", "db-password-client.store_123"
));
initConfig();

assertConfig(Map.of(
"db-kind-user_store$something", "mariadb",
"db-password-client.store_123", "password"
));

assertExternalConfig(Map.of(
"quarkus.datasource.\"user_store$something\".db-kind", "mariadb",
"quarkus.datasource.\"client.store_123\".password", "password"
));
}
}
10 changes: 10 additions & 0 deletions .../integration/src/test/java/org/keycloak/it/storage/database/dist/DatasourcesDistTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import org.keycloak.it.junit5.extension.CLIResult;
import org.keycloak.it.junit5.extension.DistributionTest;
import org.keycloak.it.junit5.extension.RawDistOnly;
import org.keycloak.it.junit5.extension.WithEnvVars;
import picocli.CommandLine;

@DistributionTest
Expand All @@ -27,4 +28,13 @@ public void multipleDatasourcesPrint(CLIResult result) {
result.assertMessage("Multiple datasources are specified: clients, <default>, users");
result.assertBuild();
}

@Test
@WithEnvVars({"KC_DB_KIND_USERS", "postgres", "KC_DB_KIND_MY_AWESOME_CLIENTS", "mariadb"})
@Launch({"build"})
public void specifiedViaEnvVars(CLIResult result) {
result.assertMessage("Multiple datasources are specified: <default>, my.awesome.clients, users");
result.assertBuild();
}

}
Loading
点击 这是indexloc提供的php浏览器服务，不要输入任何密码和下载