This is an automated email from the ASF dual-hosted git repository. emaynard pushed a commit to branch revert-806-bootstrap-only-root in repository https://gitbox.apache.org/repos/asf/polaris.git
commit 3ea8db436ad08ab5053ad44c3d0127dccd1cae0d Author: Eric Maynard <[email protected]> AuthorDate: Wed Jan 22 10:00:23 2025 -0800 Revert "Simplify credentials format when bootstrapping (#806)" This reverts commit 4eee4fe00eb834655f24c57d697764984f3f4731. --- .../persistence/PolarisCredentialsBootstrap.java | 61 +++++++++-------- .../PolarisCredentialsBootstrapTest.java | 79 +++++++++++++--------- .../persistence/PrincipalSecretsGeneratorTest.java | 5 +- .../apache/polaris/admintool/BootstrapCommand.java | 2 +- .../polaris/admintool/BootstrapCommandTest.java | 4 +- quarkus/service/build.gradle.kts | 5 +- .../configuring-polaris-for-production.md | 10 +-- 7 files changed, 92 insertions(+), 74 deletions(-) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisCredentialsBootstrap.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisCredentialsBootstrap.java index f12f58db..dc4f5b87 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisCredentialsBootstrap.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisCredentialsBootstrap.java @@ -25,8 +25,8 @@ import java.util.AbstractMap.SimpleEntry; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Map.Entry; import java.util.Optional; -import org.apache.polaris.core.entity.PolarisEntityConstants; import org.apache.polaris.core.entity.PolarisPrincipalSecrets; /** @@ -54,7 +54,7 @@ public class PolarisCredentialsBootstrap { * Parse a string of credentials in the format: * * <pre> - * realm1,client1,secret1;realm2,client2,secret2;... + * realm1,user1a,client1a,secret1a;realm1,user1b,client1b,secret1b;realm2,user2a,client2a,secret2a;... * </pre> */ public static PolarisCredentialsBootstrap fromString(@Nullable String credentialsString) { @@ -65,51 +65,52 @@ public class PolarisCredentialsBootstrap { /** * Parse a list of credentials; each element should be in the format: {@code - * realm,clientId,clientSecret}. + * realm,principal,clientId,clientSecret}. */ public static PolarisCredentialsBootstrap fromList(List<String> credentialsList) { - Map<String, Map.Entry<String, String>> credentials = new HashMap<>(); - for (String triplet : credentialsList) { - if (!triplet.isBlank()) { - List<String> parts = Splitter.on(',').trimResults().splitToList(triplet); - if (parts.size() != 3) { - throw new IllegalArgumentException("Invalid credentials format: " + triplet); + Map<String, Map<String, Map.Entry<String, String>>> credentials = new HashMap<>(); + for (String quadruple : credentialsList) { + if (!quadruple.isBlank()) { + List<String> parts = Splitter.on(',').trimResults().splitToList(quadruple); + if (parts.size() != 4) { + throw new IllegalArgumentException("Invalid credentials format: " + quadruple); } String realmName = parts.get(0); - String clientId = parts.get(1); - String clientSecret = parts.get(2); - - if (credentials.containsKey(realmName)) { - throw new IllegalArgumentException("Duplicate realm: " + realmName); - } - credentials.put(realmName, new SimpleEntry<>(clientId, clientSecret)); + String principalName = parts.get(1); + String clientId = parts.get(2); + String clientSecret = parts.get(3); + credentials + .computeIfAbsent(realmName, k -> new HashMap<>()) + .merge( + principalName, + new SimpleEntry<>(clientId, clientSecret), + (a, b) -> { + throw new IllegalArgumentException("Duplicate principal: " + principalName); + }); } } return credentials.isEmpty() ? EMPTY : new PolarisCredentialsBootstrap(credentials); } - @VisibleForTesting final Map<String, Map.Entry<String, String>> credentials; + @VisibleForTesting final Map<String, Map<String, Map.Entry<String, String>>> credentials; - private PolarisCredentialsBootstrap(Map<String, Map.Entry<String, String>> credentials) { + private PolarisCredentialsBootstrap(Map<String, Map<String, Entry<String, String>>> credentials) { this.credentials = credentials; } /** * Get the secrets for the specified principal in the specified realm, if available among the - * credentials that were supplied for bootstrap. Only credentials for the root principal are - * supported. + * credentials that were supplied for bootstrap. */ public Optional<PolarisPrincipalSecrets> getSecrets( String realmName, long principalId, String principalName) { - if (principalName.equals(PolarisEntityConstants.getRootPrincipalName())) { - return Optional.ofNullable(credentials.get(realmName)) - .map( - credentials -> { - String clientId = credentials.getKey(); - String secret = credentials.getValue(); - return new PolarisPrincipalSecrets(principalId, clientId, secret, secret); - }); - } - return Optional.empty(); + return Optional.ofNullable(credentials.get(realmName)) + .flatMap(principals -> Optional.ofNullable(principals.get(principalName))) + .map( + credentials -> { + String clientId = credentials.getKey(); + String secret = credentials.getValue(); + return new PolarisPrincipalSecrets(principalId, clientId, secret, secret); + }); } } diff --git a/polaris-core/src/test/java/org/apache/polaris/core/persistence/PolarisCredentialsBootstrapTest.java b/polaris-core/src/test/java/org/apache/polaris/core/persistence/PolarisCredentialsBootstrapTest.java index bd4ecec6..4cba20a7 100644 --- a/polaris-core/src/test/java/org/apache/polaris/core/persistence/PolarisCredentialsBootstrapTest.java +++ b/polaris-core/src/test/java/org/apache/polaris/core/persistence/PolarisCredentialsBootstrapTest.java @@ -21,11 +21,22 @@ package org.apache.polaris.core.persistence; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import java.util.Comparator; import java.util.List; +import org.apache.polaris.core.entity.PolarisPrincipalSecrets; import org.junit.jupiter.api.Test; class PolarisCredentialsBootstrapTest { + private final Comparator<PolarisPrincipalSecrets> comparator = + (a, b) -> + a.getPrincipalId() == b.getPrincipalId() + && a.getPrincipalClientId().equals(b.getPrincipalClientId()) + && a.getMainSecret().equals(b.getMainSecret()) + && a.getSecondarySecret().equals(b.getSecondarySecret()) + ? 0 + : 1; + @Test void nullString() { PolarisCredentialsBootstrap credentials = PolarisCredentialsBootstrap.fromString(null); @@ -51,28 +62,51 @@ class PolarisCredentialsBootstrapTest { } @Test - void duplicateRealm() { + void duplicatePrincipal() { assertThatThrownBy( () -> PolarisCredentialsBootstrap.fromString( - "realm1,client1a,secret1a;realm1,client1b,secret1b")) - .hasMessage("Duplicate realm: realm1"); + "realm1,user1a,client1a,secret1a;realm1,user1a,client1b,secret1b")) + .hasMessage("Duplicate principal: user1a"); } @Test void getSecretsValidString() { PolarisCredentialsBootstrap credentials = PolarisCredentialsBootstrap.fromString( - " ; realm1 , client1 , secret1 ; realm2 , client2 , secret2 ; "); - assertCredentials(credentials); + " ; realm1 , user1a , client1a , secret1a ; realm1 , user1b , client1b , secret1b ; realm2 , user2a , client2a , secret2a ; "); + assertThat(credentials.getSecrets("realm1", 123, "nonexistent")).isEmpty(); + assertThat(credentials.getSecrets("nonexistent", 123, "user1a")).isEmpty(); + assertThat(credentials.getSecrets("realm1", 123, "user1a")) + .usingValueComparator(comparator) + .contains(new PolarisPrincipalSecrets(123, "client1a", "secret1a", "secret1a")); + assertThat(credentials.getSecrets("realm1", 123, "user1b")) + .usingValueComparator(comparator) + .contains(new PolarisPrincipalSecrets(123, "client1b", "secret1b", "secret1b")); + assertThat(credentials.getSecrets("realm2", 123, "user2a")) + .usingValueComparator(comparator) + .contains(new PolarisPrincipalSecrets(123, "client2a", "secret2a", "secret2a")); } @Test void getSecretsValidList() { PolarisCredentialsBootstrap credentials = PolarisCredentialsBootstrap.fromList( - List.of("realm1,client1,secret1", "realm2,client2,secret2")); - assertCredentials(credentials); + List.of( + "realm1,user1a,client1a,secret1a", + "realm1,user1b,client1b,secret1b", + "realm2,user2a,client2a,secret2a")); + assertThat(credentials.getSecrets("realm1", 123, "nonexistent")).isEmpty(); + assertThat(credentials.getSecrets("nonexistent", 123, "user1a")).isEmpty(); + assertThat(credentials.getSecrets("realm1", 123, "user1a")) + .usingValueComparator(comparator) + .contains(new PolarisPrincipalSecrets(123, "client1a", "secret1a", "secret1a")); + assertThat(credentials.getSecrets("realm1", 123, "user1b")) + .usingValueComparator(comparator) + .contains(new PolarisPrincipalSecrets(123, "client1b", "secret1b", "secret1b")); + assertThat(credentials.getSecrets("realm2", 123, "user2a")) + .usingValueComparator(comparator) + .contains(new PolarisPrincipalSecrets(123, "client2a", "secret2a", "secret2a")); } @Test @@ -80,34 +114,15 @@ class PolarisCredentialsBootstrapTest { PolarisCredentialsBootstrap credentials = PolarisCredentialsBootstrap.fromEnvironment(); assertThat(credentials.credentials).isEmpty(); try { - System.setProperty( - "polaris.bootstrap.credentials", "realm1,client1,secret1;realm2,client2,secret2"); + System.setProperty("polaris.bootstrap.credentials", "realm1,user1a,client1a,secret1a"); credentials = PolarisCredentialsBootstrap.fromEnvironment(); - assertCredentials(credentials); + assertThat(credentials.getSecrets("realm1", 123, "nonexistent")).isEmpty(); + assertThat(credentials.getSecrets("nonexistent", 123, "user1a")).isEmpty(); + assertThat(credentials.getSecrets("realm1", 123, "user1a")) + .usingValueComparator(comparator) + .contains(new PolarisPrincipalSecrets(123, "client1a", "secret1a", "secret1a")); } finally { System.clearProperty("polaris.bootstrap.credentials"); } } - - private void assertCredentials(PolarisCredentialsBootstrap credentials) { - assertThat(credentials.getSecrets("realm3", 123, "root")).isEmpty(); - assertThat(credentials.getSecrets("nonexistent", 123, "root")).isEmpty(); - assertThat(credentials.getSecrets("realm1", 123, "non-root")).isEmpty(); - assertThat(credentials.getSecrets("realm1", 123, "root")) - .hasValueSatisfying( - secrets -> { - assertThat(secrets.getPrincipalId()).isEqualTo(123); - assertThat(secrets.getPrincipalClientId()).isEqualTo("client1"); - assertThat(secrets.getMainSecret()).isEqualTo("secret1"); - assertThat(secrets.getSecondarySecret()).isEqualTo("secret1"); - }); - assertThat(credentials.getSecrets("realm2", 123, "root")) - .hasValueSatisfying( - secrets -> { - assertThat(secrets.getPrincipalId()).isEqualTo(123); - assertThat(secrets.getPrincipalClientId()).isEqualTo("client2"); - assertThat(secrets.getMainSecret()).isEqualTo("secret2"); - assertThat(secrets.getSecondarySecret()).isEqualTo("secret2"); - }); - } } diff --git a/polaris-core/src/test/java/org/apache/polaris/core/persistence/PrincipalSecretsGeneratorTest.java b/polaris-core/src/test/java/org/apache/polaris/core/persistence/PrincipalSecretsGeneratorTest.java index d220d7ef..b03b368b 100644 --- a/polaris-core/src/test/java/org/apache/polaris/core/persistence/PrincipalSecretsGeneratorTest.java +++ b/polaris-core/src/test/java/org/apache/polaris/core/persistence/PrincipalSecretsGeneratorTest.java @@ -39,8 +39,9 @@ class PrincipalSecretsGeneratorTest { @Test void testSecretOverride() { PrincipalSecretsGenerator gen = - bootstrap("test-Realm", PolarisCredentialsBootstrap.fromString("test-Realm,client1,sec2")); - PolarisPrincipalSecrets s = gen.produceSecrets("root", 123); + bootstrap( + "test-Realm", PolarisCredentialsBootstrap.fromString("test-Realm,user1,client1,sec2")); + PolarisPrincipalSecrets s = gen.produceSecrets("user1", 123); assertThat(s).isNotNull(); assertThat(s.getPrincipalId()).isEqualTo(123); assertThat(s.getPrincipalClientId()).isEqualTo("client1"); diff --git a/quarkus/admin/src/main/java/org/apache/polaris/admintool/BootstrapCommand.java b/quarkus/admin/src/main/java/org/apache/polaris/admintool/BootstrapCommand.java index c9765420..22a2e8ac 100644 --- a/quarkus/admin/src/main/java/org/apache/polaris/admintool/BootstrapCommand.java +++ b/quarkus/admin/src/main/java/org/apache/polaris/admintool/BootstrapCommand.java @@ -39,7 +39,7 @@ public class BootstrapCommand extends BaseCommand { @CommandLine.Option( names = {"-c", "--credential"}, description = - "Root principal credentials to bootstrap. Must be of the form 'realm,clientId,clientSecret'.") + "Principal credentials to bootstrap. Must be of the form 'realm,userName,clientId,clientSecret'.") List<String> credentials; @Override diff --git a/quarkus/admin/src/test/java/org/apache/polaris/admintool/BootstrapCommandTest.java b/quarkus/admin/src/test/java/org/apache/polaris/admintool/BootstrapCommandTest.java index fd503cb9..cf290823 100644 --- a/quarkus/admin/src/test/java/org/apache/polaris/admintool/BootstrapCommandTest.java +++ b/quarkus/admin/src/test/java/org/apache/polaris/admintool/BootstrapCommandTest.java @@ -37,9 +37,9 @@ class BootstrapCommandTest { "-r", "realm2", "-c", - "realm1,root,s3cr3t", + "realm1,root,root,s3cr3t", "-c", - "realm2,root,s3cr3t" + "realm2,root,root,s3cr3t" }) public void testBootstrap(LaunchResult result) { assertThat(result.getOutput()).contains("Bootstrap completed successfully."); diff --git a/quarkus/service/build.gradle.kts b/quarkus/service/build.gradle.kts index 99d72e8c..669e54c5 100644 --- a/quarkus/service/build.gradle.kts +++ b/quarkus/service/build.gradle.kts @@ -150,9 +150,8 @@ tasks.withType(Test::class.java).configureEach { if (System.getenv("AWS_REGION") == null) { environment("AWS_REGION", "us-west-2") } - // Note: the test secrets are referenced in - // org.apache.polaris.service.quarkus.it.QuarkusServerManager - environment("POLARIS_BOOTSTRAP_CREDENTIALS", "POLARIS,test-admin,test-secret") + // Note: the test secrets are referenced in DropwizardServerManager + environment("POLARIS_BOOTSTRAP_CREDENTIALS", "POLARIS,root,test-admin,test-secret") jvmArgs("--add-exports", "java.base/sun.nio.ch=ALL-UNNAMED") // Need to allow a java security manager after Java 21, for Subject.getSubject to work // "getSubject is supported only if a security manager is allowed". diff --git a/site/content/in-dev/unreleased/configuring-polaris-for-production.md b/site/content/in-dev/unreleased/configuring-polaris-for-production.md index ed43052d..ebf4c9d4 100644 --- a/site/content/in-dev/unreleased/configuring-polaris-for-production.md +++ b/site/content/in-dev/unreleased/configuring-polaris-for-production.md @@ -75,19 +75,21 @@ Before using Polaris when using a metastore manager other than `in-memory`, you By default, Polaris will create randomised `CLIENT_ID` and `CLIENT_SECRET` for the `root` principal and store their hashes in the metastore backend. In order to provide your own credentials for `root` principal (so you can request tokens via `api/catalog/v1/oauth/tokens`), set the `POLARIS_BOOTSTRAP_CREDENTIALS` environment variable as follows: ``` -export POLARIS_BOOTSTRAP_CREDENTIALS=my_realm,my-client-id,my-client-secret +export POLARIS_BOOTSTRAP_CREDENTIALS=my_realm,root,my-client-id,my-client-secret ``` -The format of the environment variable is `realm,client_id,client_secret`. You can provide multiple credentials separated by `;`. For example, to provide credentials for two realms `my_realm` and `my_realm2`: +The format of the environment variable is `realm,principal,client_id,client_secret`. You can provide multiple credentials separated by `;`. For example, to provide credentials for two realms `my_realm` and `my_realm2`: ``` -export POLARIS_BOOTSTRAP_CREDENTIALS=my_realm,my-client-id,my-client-secret;my_realm2,my-client-id2,my-client-secret2 +export POLARIS_BOOTSTRAP_CREDENTIALS=my_realm,root,my-client-id,my-client-secret;my_realm2,root,my-client-id2,my-client-secret2 ``` +You can also provide credentials for other users too. + It is also possible to use system properties to provide the credentials: ``` -java -Dpolaris.bootstrap.credentials=my_realm,my-client-id,my-client-secret -jar /path/to/jar/polaris-service-all.jar bootstrap polaris-server.yml +java -Dpolaris.bootstrap.credentials=my_realm,root,my-client-id,my-client-secret -jar /path/to/jar/polaris-service-all.jar bootstrap polaris-server.yml ``` Now, to bootstrap Polaris, run:
