From 0824ca5f27e57b2f79be96ad9bd5c90c4886deb8 Mon Sep 17 00:00:00 2001 From: Sai Dixith Date: Thu, 23 Jul 2026 13:30:08 +0530 Subject: [PATCH 1/4] feat: pluggable CredentialWriter for principal credentials Replace hardcoded plaintext credential logging in create-omnipotent-principal and sync-polaris with a pluggable CredentialWriter interface (mirrors the existing ETagManager pattern), supporting console (default, behavior- preserving), JSON Lines file output with owner-only permissions, and a CUSTOM classname escape hatch for future backends without CLI changes. --- polaris-synchronizer/README.md | 43 +++++- .../sync/polaris/PolarisSynchronizer.java | 15 +- .../sync/polaris/access/CredentialWriter.java | 43 ++++++ polaris-synchronizer/cli/build.gradle.kts | 1 + .../sync/polaris/ConsoleCredentialWriter.java | 51 +++++++ .../CreateOmnipotentPrincipalCommand.java | 35 +++-- .../sync/polaris/CredentialWriterFactory.java | 84 ++++++++++ .../polaris/JsonFileCredentialWriter.java | 122 +++++++++++++++ .../sync/polaris/SyncPolarisCommand.java | 29 +++- .../polaris/CredentialWriterFactoryTest.java | 71 +++++++++ .../polaris/JsonFileCredentialWriterTest.java | 143 ++++++++++++++++++ 11 files changed, 614 insertions(+), 23 deletions(-) create mode 100644 polaris-synchronizer/api/src/main/java/org/apache/polaris/tools/sync/polaris/access/CredentialWriter.java create mode 100644 polaris-synchronizer/cli/src/main/java/org/apache/polaris/tools/sync/polaris/ConsoleCredentialWriter.java create mode 100644 polaris-synchronizer/cli/src/main/java/org/apache/polaris/tools/sync/polaris/CredentialWriterFactory.java create mode 100644 polaris-synchronizer/cli/src/main/java/org/apache/polaris/tools/sync/polaris/JsonFileCredentialWriter.java create mode 100644 polaris-synchronizer/cli/src/test/java/org/apache/polaris/tools/sync/polaris/CredentialWriterFactoryTest.java create mode 100644 polaris-synchronizer/cli/src/test/java/org/apache/polaris/tools/sync/polaris/JsonFileCredentialWriterTest.java diff --git a/polaris-synchronizer/README.md b/polaris-synchronizer/README.md index c7e754a3..4041c2b3 100644 --- a/polaris-synchronizer/README.md +++ b/polaris-synchronizer/README.md @@ -89,16 +89,22 @@ java -jar cli/build/libs/polaris-synchronizer-cli.jar create-omnipotent-principa Upon finishing execution, the tool will output the principal name and client credentials for this principal. **Make sure to note these down as they will be necessary for the migration step.** +By default, credentials are logged to the console: + **Example Output:** ``` ====================================================== -Omnipotent Principal Credentials: +Principal Credentials: name = omnipotent-principal-XXXXX clientId = ff7s8f9asbX10 clientSecret = ====================================================== ``` +> :warning: Credential output is pluggable via the `--credential-output-type` and `--credential-output-properties` +> options, see [Configuring Credential Output](#configuring-credential-output) below. Console output should be +> securely managed; client credentials should only ever be stored in a secure vault. + Additionally, at the end of execution the command will output a list of catalogs for which catalog setup failed. **These catalogs may experience failure during migration**. @@ -136,7 +142,7 @@ for subsequent steps. **Example Output:** ``` ====================================================== -Omnipotent Principal Credentials: +Principal Credentials: name = omnipotent-principal-YYYYY clientId = 0af20a3a0037a40d clientSecret = @@ -150,6 +156,36 @@ clientSecret = > grants for the catalog, and assign it to the principal used to run this tool (presumably, a principal with the `servic_admin` > principal role). Then, re-running `create-omnipotent-principal` should be able to create the relevant entities for that catalog. +### Configuring Credential Output + +Both `create-omnipotent-principal` and `sync-polaris` (when run with `--sync-principals`) generate new principal +credentials. Where those credentials are output is controlled by two options, available on both commands: + +* `--credential-output-type`: One of `CONSOLE` (default), `FILE`, or `CUSTOM`. + * `CONSOLE`: Logs credentials to the console, matching the output shown above. This is the default, so existing + invocations are unaffected unless you opt in to a different type. + * `FILE`: Writes credentials as [JSON Lines](https://jsonlines.org/) (one JSON object per principal, per line) to + a file. The file is created with owner-only read/write permissions, since it contains plaintext secrets. + * `CUSTOM`: Loads a user-supplied class implementing `CredentialWriter`, allowing you to plug in your own storage + backend (e.g. a secrets manager) without modifying this tool. +* `--credential-output-properties`: Properties to configure the selected type. + * For `FILE`: + * `json-file`: (required) path to the file to write credentials to. + * `append`: (default: `false`) if `true`, appends to an existing file instead of truncating it on each run. + * For `CUSTOM`: + * `custom-impl`: the fully-qualified classname of your `CredentialWriter` implementation. + +**Example:** Write credentials generated by `create-omnipotent-principal` to a JSON Lines file instead of the console: +``` +java -jar cli/build/libs/polaris-synchronizer-cli.jar create-omnipotent-principal \ +--polaris-api-connection-properties base-url=http://localhost:8181 \ +--polaris-api-connection-properties oauth2-server-uri=http://localhost:8181/api/catalog/v1/oauth/tokens \ +--polaris-api-connection-properties credential=: \ +--polaris-api-connection-properties scope=PRINCIPAL_ROLE:ALL \ +--credential-output-type FILE \ +--credential-output-properties json-file=./omnipotent-principal-credentials.jsonl +``` + ### Step 3: Running the Migration/Synchronization Running the synchronization requires minimal reconfiguration, can be run idempotently, and will attempt to only copy over the @@ -157,7 +193,8 @@ diff between the source and target Polaris instances. This can be achieved using > :warning: If you want to migrate principals and their assignments to principal-roles as well, run the tool with the > `--sync-principals` flag. Please note that this will reset the client credentials for that principal on the target -> Polaris instance. The new credentials will be logged to stdout, ONLY for each newly created or overwritten principal. +> Polaris instance. The new credentials will be output via the configured `--credential-output-type`, by default this +> means they are logged to the console, ONLY for each newly created or overwritten principal. > Please note that this output should be securely managed, client credentials should only ever be stored in a secure vault. **Example** Running the synchronization between source Polaris instance using a bearer token, and a target Polaris instance diff --git a/polaris-synchronizer/api/src/main/java/org/apache/polaris/tools/sync/polaris/PolarisSynchronizer.java b/polaris-synchronizer/api/src/main/java/org/apache/polaris/tools/sync/polaris/PolarisSynchronizer.java index dad781f8..18fed264 100644 --- a/polaris-synchronizer/api/src/main/java/org/apache/polaris/tools/sync/polaris/PolarisSynchronizer.java +++ b/polaris-synchronizer/api/src/main/java/org/apache/polaris/tools/sync/polaris/PolarisSynchronizer.java @@ -32,6 +32,7 @@ import org.apache.polaris.core.admin.model.Principal; import org.apache.polaris.core.admin.model.PrincipalRole; import org.apache.polaris.core.admin.model.PrincipalWithCredentials; +import org.apache.polaris.tools.sync.polaris.access.CredentialWriter; import org.apache.polaris.tools.sync.polaris.catalog.BaseTableWithETag; import org.apache.polaris.tools.sync.polaris.catalog.ETagManager; import org.apache.polaris.tools.sync.polaris.catalog.MetadataNotModifiedException; @@ -59,6 +60,8 @@ public class PolarisSynchronizer { private final ETagManager etagManager; + private final CredentialWriter credentialWriter; + private final boolean haltOnFailure; private final boolean diffOnly; @@ -70,6 +73,7 @@ public PolarisSynchronizer( PolarisService source, PolarisService target, ETagManager etagManager, + CredentialWriter credentialWriter, boolean diffOnly) { this.clientLogger = clientLogger == null ? LoggerFactory.getLogger(PolarisSynchronizer.class) : clientLogger; @@ -78,6 +82,7 @@ public PolarisSynchronizer( this.source = source; this.target = target; this.etagManager = etagManager; + this.credentialWriter = credentialWriter; this.diffOnly = diffOnly; } @@ -140,10 +145,9 @@ public void syncPrincipals() { for (Principal principal : principalSyncPlan.entitiesToCreate()) { try { PrincipalWithCredentials createdPrincipal = target.createPrincipal(principal); - clientLogger.info("Created principal {} on target. Target credentials: {}:{} - {}/{}", + credentialWriter.writeCredentials(createdPrincipal); + clientLogger.info("Created principal {} on target. - {}/{}", principal.getName(), - createdPrincipal.getCredentials().getClientId(), - createdPrincipal.getCredentials().getClientSecret(), ++syncsCompleted, totalSyncsToComplete ); @@ -158,10 +162,9 @@ public void syncPrincipals() { try { target.dropPrincipal(principal.getName()); PrincipalWithCredentials overwrittenPrincipal = target.createPrincipal(principal); - clientLogger.info("Overwrote principal {} on target. Target credentials: {}:{} - {}/{}", + credentialWriter.writeCredentials(overwrittenPrincipal); + clientLogger.info("Overwrote principal {} on target. - {}/{}", principal.getName(), - overwrittenPrincipal.getCredentials().getClientId(), - overwrittenPrincipal.getCredentials().getClientSecret(), ++syncsCompleted, totalSyncsToComplete ); diff --git a/polaris-synchronizer/api/src/main/java/org/apache/polaris/tools/sync/polaris/access/CredentialWriter.java b/polaris-synchronizer/api/src/main/java/org/apache/polaris/tools/sync/polaris/access/CredentialWriter.java new file mode 100644 index 00000000..06e75887 --- /dev/null +++ b/polaris-synchronizer/api/src/main/java/org/apache/polaris/tools/sync/polaris/access/CredentialWriter.java @@ -0,0 +1,43 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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.apache.polaris.tools.sync.polaris.access; + +import java.util.Map; +import org.apache.polaris.core.admin.model.PrincipalWithCredentials; + +/** + * Generic interface to output newly generated/rotated principal credentials. This allows the + * destination of the credentials to be completely independent from the tool. + */ +public interface CredentialWriter extends AutoCloseable { + + /** + * Used to initialize the instance for use. Should be called prior to calling any methods. + * + * @param properties properties to configure instance with + */ + void initialize(Map properties); + + /** + * Outputs the given principal's credentials. + * + * @param principalWithCredentials the principal and its associated credentials + */ + void writeCredentials(PrincipalWithCredentials principalWithCredentials); +} diff --git a/polaris-synchronizer/cli/build.gradle.kts b/polaris-synchronizer/cli/build.gradle.kts index 230d27dc..c1b8fe74 100644 --- a/polaris-synchronizer/cli/build.gradle.kts +++ b/polaris-synchronizer/cli/build.gradle.kts @@ -37,6 +37,7 @@ dependencies { implementation("org.slf4j:log4j-over-slf4j:2.0.17") implementation("org.apache.iceberg:iceberg-spark-runtime-3.3_2.12:1.7.1") implementation("org.apache.commons:commons-csv:1.13.0") + implementation("com.fasterxml.jackson.core:jackson-databind:2.18.3") runtimeOnly("ch.qos.logback:logback-classic:1.5.17") testImplementation("org.junit.jupiter:junit-jupiter-params:5.10.0") diff --git a/polaris-synchronizer/cli/src/main/java/org/apache/polaris/tools/sync/polaris/ConsoleCredentialWriter.java b/polaris-synchronizer/cli/src/main/java/org/apache/polaris/tools/sync/polaris/ConsoleCredentialWriter.java new file mode 100644 index 00000000..b6e99a60 --- /dev/null +++ b/polaris-synchronizer/cli/src/main/java/org/apache/polaris/tools/sync/polaris/ConsoleCredentialWriter.java @@ -0,0 +1,51 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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.apache.polaris.tools.sync.polaris; + +import java.util.Map; +import org.apache.polaris.core.admin.model.PrincipalWithCredentials; +import org.apache.polaris.tools.sync.polaris.access.CredentialWriter; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** Implementation that logs principal credentials to the console. */ +public class ConsoleCredentialWriter implements CredentialWriter { + + private final Logger consoleLog = LoggerFactory.getLogger("console-log"); + + @Override + public void initialize(Map properties) {} + + @Override + public void writeCredentials(PrincipalWithCredentials principalWithCredentials) { + consoleLog.info( + "\n======================================================\n" + + "Principal Credentials:\n" + + "\tname = {}\n" + + "\tclientId = {}\n" + + "\tclientSecret = {}\n" + + "======================================================", + principalWithCredentials.getPrincipal().getName(), + principalWithCredentials.getCredentials().getClientId(), + principalWithCredentials.getCredentials().getClientSecret()); + } + + @Override + public void close() {} +} diff --git a/polaris-synchronizer/cli/src/main/java/org/apache/polaris/tools/sync/polaris/CreateOmnipotentPrincipalCommand.java b/polaris-synchronizer/cli/src/main/java/org/apache/polaris/tools/sync/polaris/CreateOmnipotentPrincipalCommand.java index 39394c71..172dce80 100644 --- a/polaris-synchronizer/cli/src/main/java/org/apache/polaris/tools/sync/polaris/CreateOmnipotentPrincipalCommand.java +++ b/polaris-synchronizer/cli/src/main/java/org/apache/polaris/tools/sync/polaris/CreateOmnipotentPrincipalCommand.java @@ -32,6 +32,7 @@ import org.apache.polaris.core.admin.model.PrincipalRole; import org.apache.polaris.core.admin.model.PrincipalWithCredentials; import org.apache.polaris.tools.sync.polaris.access.AccessControlService; +import org.apache.polaris.tools.sync.polaris.access.CredentialWriter; import org.apache.polaris.tools.sync.polaris.service.PolarisService; import org.apache.polaris.tools.sync.polaris.service.impl.PolarisApiService; import org.slf4j.Logger; @@ -83,13 +84,34 @@ public class CreateOmnipotentPrincipalCommand implements Callable { }) private int concurrency; + @CommandLine.Option( + names = {"--credential-output-type"}, + defaultValue = "CONSOLE", + description = "One of { CONSOLE, FILE, CUSTOM }. Default: CONSOLE. Controls how the newly generated " + + "principal credentials are output." + ) + private CredentialWriterFactory.Type credentialWriterType; + + @CommandLine.Option( + names = {"--credential-output-properties"}, + description = "Properties to initialize credential output." + + "\nFor type FILE:" + + "\n\t- " + JsonFileCredentialWriter.JSON_FILE_PROPERTY + ": The JSON Lines file to write principal credentials to." + + "\n\t- " + JsonFileCredentialWriter.APPEND_PROPERTY + ": (default: false) Whether to append to an existing file instead of overwriting it." + + "\nFor type CUSTOM:" + + "\n\t- " + CredentialWriterFactory.CUSTOM_CLASS_NAME_PROPERTY + ": The classname for the custom CredentialWriter implementation." + ) + private Map credentialWriterProperties; + @Override public Integer call() throws Exception { polarisApiConnectionProperties.putIfAbsent(PolarisApiService.ICEBERG_WRITE_ACCESS_PROPERTY, String.valueOf(withWriteAccess)); try (PolarisService polaris = PolarisServiceFactory.createPolarisService( - PolarisServiceFactory.ServiceType.API, polarisApiConnectionProperties)) { + PolarisServiceFactory.ServiceType.API, polarisApiConnectionProperties); + CredentialWriter credentialWriter = + CredentialWriterFactory.createCredentialWriter(credentialWriterType, credentialWriterProperties)) { AccessControlService accessControlService = new AccessControlService((PolarisApiService) polaris); @@ -171,16 +193,7 @@ public Integer call() throws Exception { "Encountered issues creating catalog roles for the following catalogs: {}", failedCatalogs.stream().map(Catalog::getName).toList()); - consoleLog.info( - "\n======================================================\n" - + "Omnipotent Principal Credentials:\n" - + "\tname = {}\n" - + "\tclientId = {}\n" - + "\tclientSecret = {}\n" - + "======================================================", - principalWithCredentials.getPrincipal().getName(), - principalWithCredentials.getCredentials().getClientId(), - principalWithCredentials.getCredentials().getClientSecret()); + credentialWriter.writeCredentials(principalWithCredentials); } diff --git a/polaris-synchronizer/cli/src/main/java/org/apache/polaris/tools/sync/polaris/CredentialWriterFactory.java b/polaris-synchronizer/cli/src/main/java/org/apache/polaris/tools/sync/polaris/CredentialWriterFactory.java new file mode 100644 index 00000000..d81d92a3 --- /dev/null +++ b/polaris-synchronizer/cli/src/main/java/org/apache/polaris/tools/sync/polaris/CredentialWriterFactory.java @@ -0,0 +1,84 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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.apache.polaris.tools.sync.polaris; + +import org.apache.polaris.tools.sync.polaris.access.CredentialWriter; + +import java.util.HashMap; +import java.util.Map; + +/** + * Factory class to construct configurable {@link CredentialWriter} implementations. + */ +public class CredentialWriterFactory { + + /** + * Property that will hold class name for custom {@link CredentialWriter} implementation. + */ + public static final String CUSTOM_CLASS_NAME_PROPERTY = "custom-impl"; + + private CredentialWriterFactory() {} + + /** + * Recognized types of {@link CredentialWriter} implementations + */ + public enum Type { + CONSOLE, + FILE, + CUSTOM + } + + /** + * Construct a new {@link CredentialWriter} instance. + * @param type the recognized type of the {@link CredentialWriter} to construct + * @param properties properties to use when initializing the {@link CredentialWriter} + * @return the constructed and initialized {@link CredentialWriter} + */ + public static CredentialWriter createCredentialWriter(Type type, Map properties) { + try { + properties = properties == null ? new HashMap<>() : properties; + + CredentialWriter writer = switch (type) { + case CONSOLE -> new ConsoleCredentialWriter(); + case FILE -> new JsonFileCredentialWriter(); + case CUSTOM -> { + String customWriterClassname = properties.get(CUSTOM_CLASS_NAME_PROPERTY); + + if (customWriterClassname == null) { + throw new IllegalArgumentException("Missing required property " + CUSTOM_CLASS_NAME_PROPERTY); + } + + Object custom = Class.forName(customWriterClassname).getDeclaredConstructor().newInstance(); + + if (custom instanceof CredentialWriter customWriter) { + yield customWriter; + } + + throw new InstantiationException("Custom CredentialWriter '" + customWriterClassname + "' does not implement CredentialWriter"); + } + }; + + writer.initialize(properties); + return writer; + } catch (Exception e) { + throw new RuntimeException("Failed to construct CredentialWriter", e); + } + } + +} diff --git a/polaris-synchronizer/cli/src/main/java/org/apache/polaris/tools/sync/polaris/JsonFileCredentialWriter.java b/polaris-synchronizer/cli/src/main/java/org/apache/polaris/tools/sync/polaris/JsonFileCredentialWriter.java new file mode 100644 index 00000000..ab859cac --- /dev/null +++ b/polaris-synchronizer/cli/src/main/java/org/apache/polaris/tools/sync/polaris/JsonFileCredentialWriter.java @@ -0,0 +1,122 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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.apache.polaris.tools.sync.polaris; + +import static java.nio.charset.StandardCharsets.UTF_8; + +import com.fasterxml.jackson.databind.ObjectMapper; +import java.io.BufferedWriter; +import java.io.Closeable; +import java.io.File; +import java.io.IOException; +import java.nio.file.FileSystems; +import java.nio.file.Files; +import java.nio.file.StandardOpenOption; +import java.nio.file.attribute.PosixFilePermissions; +import java.util.Map; +import org.apache.polaris.core.admin.model.PrincipalWithCredentials; +import org.apache.polaris.tools.sync.polaris.access.CredentialWriter; + +/** + * Implementation that writes principal credentials as JSON Lines (one JSON object per line) to a + * file. The file is created with owner-only read/write permissions where the filesystem supports + * it, since it contains plaintext secrets. + */ +public class JsonFileCredentialWriter implements CredentialWriter, Closeable { + + public static final String JSON_FILE_PROPERTY = "json-file"; + + public static final String APPEND_PROPERTY = "append"; + + private final ObjectMapper objectMapper = new ObjectMapper(); + + private BufferedWriter writer; + + @Override + public void initialize(Map properties) { + if (!properties.containsKey(JSON_FILE_PROPERTY)) { + throw new IllegalArgumentException("Missing required property " + JSON_FILE_PROPERTY); + } + + boolean append = Boolean.parseBoolean(properties.getOrDefault(APPEND_PROPERTY, "false")); + + File file = new File(properties.get(JSON_FILE_PROPERTY)); + + try { + if (file.getParentFile() != null) { + Files.createDirectories(file.getParentFile().toPath()); + } + + if (!file.exists()) { + createRestrictedFile(file); + } else { + restrictExistingFilePermissions(file); + } + + this.writer = + Files.newBufferedWriter( + file.toPath(), + UTF_8, + StandardOpenOption.CREATE, + StandardOpenOption.WRITE, + append ? StandardOpenOption.APPEND : StandardOpenOption.TRUNCATE_EXISTING); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + private void createRestrictedFile(File file) throws IOException { + if (FileSystems.getDefault().supportedFileAttributeViews().contains("posix")) { + Files.createFile( + file.toPath(), PosixFilePermissions.asFileAttribute(PosixFilePermissions.fromString("rw-------"))); + } else { + Files.createFile(file.toPath()); + restrictExistingFilePermissions(file); + } + } + + private void restrictExistingFilePermissions(File file) throws IOException { + if (FileSystems.getDefault().supportedFileAttributeViews().contains("posix")) { + Files.setPosixFilePermissions(file.toPath(), PosixFilePermissions.fromString("rw-------")); + } else { + file.setReadable(false, false); + file.setReadable(true, true); + file.setWritable(false, false); + file.setWritable(true, true); + } + } + + @Override + public void writeCredentials(PrincipalWithCredentials principalWithCredentials) { + try { + writer.write(objectMapper.writeValueAsString(principalWithCredentials)); + writer.newLine(); + writer.flush(); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + @Override + public void close() throws IOException { + if (writer != null) { + writer.close(); + } + } +} diff --git a/polaris-synchronizer/cli/src/main/java/org/apache/polaris/tools/sync/polaris/SyncPolarisCommand.java b/polaris-synchronizer/cli/src/main/java/org/apache/polaris/tools/sync/polaris/SyncPolarisCommand.java index 851d66f4..d66fd0c3 100644 --- a/polaris-synchronizer/cli/src/main/java/org/apache/polaris/tools/sync/polaris/SyncPolarisCommand.java +++ b/polaris-synchronizer/cli/src/main/java/org/apache/polaris/tools/sync/polaris/SyncPolarisCommand.java @@ -20,6 +20,7 @@ import java.util.Map; import java.util.concurrent.Callable; +import org.apache.polaris.tools.sync.polaris.access.CredentialWriter; import org.apache.polaris.tools.sync.polaris.catalog.ETagManager; import org.apache.polaris.tools.sync.polaris.planning.AccessControlAwarePlanner; import org.apache.polaris.tools.sync.polaris.planning.CatalogNameFilterPlanner; @@ -80,11 +81,30 @@ public class SyncPolarisCommand implements Callable { names = {"--sync-principals"}, description = "Enable synchronization of principals across the source and target, and assign them to " + "the appropriate principal roles. WARNING: Principal client-id and client-secret will be reset on " + - "the target Polaris instance, and the new credentials for the principals created on the target will " + - "be logged to stdout." + "the target Polaris instance. The new credentials for principals created on the target will be " + + "output via the configured --credential-output-type (default: logged to stdout)." ) private boolean shouldSyncPrincipals; + @CommandLine.Option( + names = {"--credential-output-type"}, + defaultValue = "CONSOLE", + description = "One of { CONSOLE, FILE, CUSTOM }. Default: CONSOLE. Controls how newly generated/rotated " + + "principal credentials on the target are output." + ) + private CredentialWriterFactory.Type credentialWriterType; + + @CommandLine.Option( + names = {"--credential-output-properties"}, + description = "Properties to initialize credential output." + + "\nFor type FILE:" + + "\n\t- " + JsonFileCredentialWriter.JSON_FILE_PROPERTY + ": The JSON Lines file to write principal credentials to." + + "\n\t- " + JsonFileCredentialWriter.APPEND_PROPERTY + ": (default: false) Whether to append to an existing file instead of overwriting it." + + "\nFor type CUSTOM:" + + "\n\t- " + CredentialWriterFactory.CUSTOM_CLASS_NAME_PROPERTY + ": The classname for the custom CredentialWriter implementation." + ) + private Map credentialWriterProperties; + @CommandLine.Option( names = {"--halt-on-failure"}, description = "Hard fail and stop the synchronization when an error occurs." @@ -135,7 +155,9 @@ public Integer call() throws Exception { PolarisServiceFactory.ServiceType.API, sourceProperties); PolarisService target = PolarisServiceFactory.createPolarisService( PolarisServiceFactory.ServiceType.API, targetProperties); - ETagManager etagManager = ETagManagerFactory.createETagManager(etagManagerType, etagManagerProperties) + ETagManager etagManager = ETagManagerFactory.createETagManager(etagManagerType, etagManagerProperties); + CredentialWriter credentialWriter = + CredentialWriterFactory.createCredentialWriter(credentialWriterType, credentialWriterProperties) ) { PolarisSynchronizer synchronizer = new PolarisSynchronizer( @@ -145,6 +167,7 @@ public Integer call() throws Exception { source, target, etagManager, + credentialWriter, diffOnly); synchronizer.syncPrincipalRoles(); if (shouldSyncPrincipals) { diff --git a/polaris-synchronizer/cli/src/test/java/org/apache/polaris/tools/sync/polaris/CredentialWriterFactoryTest.java b/polaris-synchronizer/cli/src/test/java/org/apache/polaris/tools/sync/polaris/CredentialWriterFactoryTest.java new file mode 100644 index 00000000..9262707c --- /dev/null +++ b/polaris-synchronizer/cli/src/test/java/org/apache/polaris/tools/sync/polaris/CredentialWriterFactoryTest.java @@ -0,0 +1,71 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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.apache.polaris.tools.sync.polaris; + +import java.nio.file.Path; +import java.util.Map; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +public class CredentialWriterFactoryTest { + + @TempDir + Path tempDir; + + @Test + public void constructConsoleWriterSuccessfully() throws Exception { + try (var writer = CredentialWriterFactory.createCredentialWriter( + CredentialWriterFactory.Type.CONSOLE, Map.of())) { + Assertions.assertNotNull(writer); + } + } + + @Test + public void constructFileWriterSuccessfully() throws Exception { + String path = tempDir.resolve("creds.jsonl").toString(); + try (var writer = CredentialWriterFactory.createCredentialWriter( + CredentialWriterFactory.Type.FILE, + Map.of(JsonFileCredentialWriter.JSON_FILE_PROPERTY, path))) { + Assertions.assertNotNull(writer); + } + } + + @Test + public void failToConstructFileWriterMissingProperty() { + Assertions.assertThrows(Exception.class, () -> + CredentialWriterFactory.createCredentialWriter(CredentialWriterFactory.Type.FILE, Map.of())); + } + + @Test + public void constructCustomCredentialWriterSuccessfully() throws Exception { + try (var writer = CredentialWriterFactory.createCredentialWriter( + CredentialWriterFactory.Type.CUSTOM, + Map.of(CredentialWriterFactory.CUSTOM_CLASS_NAME_PROPERTY, ConsoleCredentialWriter.class.getName()))) { + Assertions.assertNotNull(writer); + } + } + + @Test + public void failToConstructCustomCredentialWriter() { + Assertions.assertThrows(Exception.class, () -> + CredentialWriterFactory.createCredentialWriter(CredentialWriterFactory.Type.CUSTOM, Map.of())); + } + +} diff --git a/polaris-synchronizer/cli/src/test/java/org/apache/polaris/tools/sync/polaris/JsonFileCredentialWriterTest.java b/polaris-synchronizer/cli/src/test/java/org/apache/polaris/tools/sync/polaris/JsonFileCredentialWriterTest.java new file mode 100644 index 00000000..fb0c4c70 --- /dev/null +++ b/polaris-synchronizer/cli/src/test/java/org/apache/polaris/tools/sync/polaris/JsonFileCredentialWriterTest.java @@ -0,0 +1,143 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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.apache.polaris.tools.sync.polaris; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import java.nio.file.FileSystems; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.attribute.PosixFilePermission; +import java.util.List; +import java.util.Map; +import java.util.Set; +import org.apache.polaris.core.admin.model.Principal; +import org.apache.polaris.core.admin.model.PrincipalWithCredentials; +import org.apache.polaris.core.admin.model.PrincipalWithCredentialsCredentials; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.DisabledOnOs; +import org.junit.jupiter.api.condition.OS; +import org.junit.jupiter.api.io.TempDir; + +public class JsonFileCredentialWriterTest { + + @TempDir + Path tempDir; + + private final ObjectMapper objectMapper = new ObjectMapper(); + + private PrincipalWithCredentials buildPrincipal(String name, String clientId, String clientSecret) { + return new PrincipalWithCredentials() + .principal(new Principal().name(name)) + .credentials(new PrincipalWithCredentialsCredentials() + .clientId(clientId) + .clientSecret(clientSecret)); + } + + @Test + public void writesSingleLineOfValidJson() throws Exception { + Path path = tempDir.resolve("single.jsonl"); + + try (JsonFileCredentialWriter writer = new JsonFileCredentialWriter()) { + writer.initialize(Map.of(JsonFileCredentialWriter.JSON_FILE_PROPERTY, path.toString())); + writer.writeCredentials(buildPrincipal("test-principal", "client-id", "client-secret")); + } + + List lines = Files.readAllLines(path); + Assertions.assertEquals(1, lines.size()); + + JsonNode node = objectMapper.readTree(lines.get(0)); + Assertions.assertEquals("test-principal", node.get("principal").get("name").asText()); + Assertions.assertEquals("client-id", node.get("credentials").get("clientId").asText()); + Assertions.assertEquals("client-secret", node.get("credentials").get("clientSecret").asText()); + } + + @Test + public void appendsMultipleEntriesAsJsonLines() throws Exception { + Path path = tempDir.resolve("multi.jsonl"); + + try (JsonFileCredentialWriter writer = new JsonFileCredentialWriter()) { + writer.initialize(Map.of(JsonFileCredentialWriter.JSON_FILE_PROPERTY, path.toString())); + writer.writeCredentials(buildPrincipal("principal-1", "id-1", "secret-1")); + writer.writeCredentials(buildPrincipal("principal-2", "id-2", "secret-2")); + writer.writeCredentials(buildPrincipal("principal-3", "id-3", "secret-3")); + } + + List lines = Files.readAllLines(path); + Assertions.assertEquals(3, lines.size()); + + for (String line : lines) { + Assertions.assertDoesNotThrow(() -> objectMapper.readTree(line)); + } + } + + @Test + public void defaultOverwritesExistingFile() throws Exception { + Path path = tempDir.resolve("overwrite.jsonl"); + Files.writeString(path, "{\"stale\":\"data\"}\n"); + + try (JsonFileCredentialWriter writer = new JsonFileCredentialWriter()) { + writer.initialize(Map.of(JsonFileCredentialWriter.JSON_FILE_PROPERTY, path.toString())); + writer.writeCredentials(buildPrincipal("fresh-principal", "id", "secret")); + } + + List lines = Files.readAllLines(path); + Assertions.assertEquals(1, lines.size()); + Assertions.assertTrue(lines.get(0).contains("fresh-principal")); + } + + @Test + public void appendPropertyPreservesExistingContent() throws Exception { + Path path = tempDir.resolve("append.jsonl"); + Files.writeString(path, "{\"principal\":{\"name\":\"existing\"}}\n"); + + try (JsonFileCredentialWriter writer = new JsonFileCredentialWriter()) { + writer.initialize(Map.of( + JsonFileCredentialWriter.JSON_FILE_PROPERTY, path.toString(), + JsonFileCredentialWriter.APPEND_PROPERTY, "true")); + writer.writeCredentials(buildPrincipal("new-principal", "id", "secret")); + } + + List lines = Files.readAllLines(path); + Assertions.assertEquals(2, lines.size()); + Assertions.assertTrue(lines.get(0).contains("existing")); + Assertions.assertTrue(lines.get(1).contains("new-principal")); + } + + @Test + @DisabledOnOs(OS.WINDOWS) + public void filePermissionsRestrictedToOwner() throws Exception { + Path path = tempDir.resolve("secure.jsonl"); + + org.junit.jupiter.api.Assumptions.assumeTrue( + FileSystems.getDefault().supportedFileAttributeViews().contains("posix")); + + try (JsonFileCredentialWriter writer = new JsonFileCredentialWriter()) { + writer.initialize(Map.of(JsonFileCredentialWriter.JSON_FILE_PROPERTY, path.toString())); + writer.writeCredentials(buildPrincipal("principal", "id", "secret")); + } + + Set permissions = Files.getPosixFilePermissions(path); + Assertions.assertEquals( + Set.of(PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_WRITE), + permissions); + } + +} From d27762a2d7531526e2de6c2abeaf0f0b7d513703 Mon Sep 17 00:00:00 2001 From: Sai Dixith Date: Fri, 24 Jul 2026 11:54:00 +0530 Subject: [PATCH 2/4] fix: address review feedback on pluggable CredentialWriter Remove initialize() from the CredentialWriter interface so the factory returns fully pre-configured instances instead of requiring callers to configure them after construction. Replace the reflective Class.forName instantiation for the CUSTOM type with java.util.ServiceLoader-based discovery, matching the pattern used elsewhere in the main Polaris codebase. --- polaris-synchronizer/README.md | 9 ++- .../sync/polaris/access/CredentialWriter.java | 12 ++-- .../sync/polaris/ConsoleCredentialWriter.java | 4 -- .../CreateOmnipotentPrincipalCommand.java | 2 +- .../sync/polaris/CredentialWriterFactory.java | 60 +++++++++++-------- .../polaris/JsonFileCredentialWriter.java | 5 +- .../sync/polaris/SyncPolarisCommand.java | 2 +- .../polaris/CredentialWriterFactoryTest.java | 13 +++- .../polaris/JsonFileCredentialWriterTest.java | 24 ++++---- .../sync/polaris/TestCredentialWriter.java | 37 ++++++++++++ ...tools.sync.polaris.access.CredentialWriter | 1 + 11 files changed, 110 insertions(+), 59 deletions(-) create mode 100644 polaris-synchronizer/cli/src/test/java/org/apache/polaris/tools/sync/polaris/TestCredentialWriter.java create mode 100644 polaris-synchronizer/cli/src/test/resources/META-INF/services/org.apache.polaris.tools.sync.polaris.access.CredentialWriter diff --git a/polaris-synchronizer/README.md b/polaris-synchronizer/README.md index 4041c2b3..040b4496 100644 --- a/polaris-synchronizer/README.md +++ b/polaris-synchronizer/README.md @@ -166,14 +166,17 @@ credentials. Where those credentials are output is controlled by two options, av invocations are unaffected unless you opt in to a different type. * `FILE`: Writes credentials as [JSON Lines](https://jsonlines.org/) (one JSON object per principal, per line) to a file. The file is created with owner-only read/write permissions, since it contains plaintext secrets. - * `CUSTOM`: Loads a user-supplied class implementing `CredentialWriter`, allowing you to plug in your own storage - backend (e.g. a secrets manager) without modifying this tool. + * `CUSTOM`: Discovers a user-supplied class implementing `CredentialWriter` via `java.util.ServiceLoader`, allowing + you to plug in your own storage backend (e.g. a secrets manager) without modifying this tool. Your implementation + must be registered as a service provider (declared in a + `META-INF/services/org.apache.polaris.tools.sync.polaris.access.CredentialWriter` file on the classpath) and + placed on the classpath when running the CLI. * `--credential-output-properties`: Properties to configure the selected type. * For `FILE`: * `json-file`: (required) path to the file to write credentials to. * `append`: (default: `false`) if `true`, appends to an existing file instead of truncating it on each run. * For `CUSTOM`: - * `custom-impl`: the fully-qualified classname of your `CredentialWriter` implementation. + * `custom-impl`: the fully-qualified classname of your registered `CredentialWriter` service provider. **Example:** Write credentials generated by `create-omnipotent-principal` to a JSON Lines file instead of the console: ``` diff --git a/polaris-synchronizer/api/src/main/java/org/apache/polaris/tools/sync/polaris/access/CredentialWriter.java b/polaris-synchronizer/api/src/main/java/org/apache/polaris/tools/sync/polaris/access/CredentialWriter.java index 06e75887..437eeefd 100644 --- a/polaris-synchronizer/api/src/main/java/org/apache/polaris/tools/sync/polaris/access/CredentialWriter.java +++ b/polaris-synchronizer/api/src/main/java/org/apache/polaris/tools/sync/polaris/access/CredentialWriter.java @@ -18,22 +18,18 @@ */ package org.apache.polaris.tools.sync.polaris.access; -import java.util.Map; import org.apache.polaris.core.admin.model.PrincipalWithCredentials; /** * Generic interface to output newly generated/rotated principal credentials. This allows the * destination of the credentials to be completely independent from the tool. + * + *

Implementations should be fully configured and ready to use once constructed; obtaining and + * applying any configuration is the responsibility of {@code CredentialWriterFactory}, not of + * callers of this interface. */ public interface CredentialWriter extends AutoCloseable { - /** - * Used to initialize the instance for use. Should be called prior to calling any methods. - * - * @param properties properties to configure instance with - */ - void initialize(Map properties); - /** * Outputs the given principal's credentials. * diff --git a/polaris-synchronizer/cli/src/main/java/org/apache/polaris/tools/sync/polaris/ConsoleCredentialWriter.java b/polaris-synchronizer/cli/src/main/java/org/apache/polaris/tools/sync/polaris/ConsoleCredentialWriter.java index b6e99a60..2c4fa815 100644 --- a/polaris-synchronizer/cli/src/main/java/org/apache/polaris/tools/sync/polaris/ConsoleCredentialWriter.java +++ b/polaris-synchronizer/cli/src/main/java/org/apache/polaris/tools/sync/polaris/ConsoleCredentialWriter.java @@ -18,7 +18,6 @@ */ package org.apache.polaris.tools.sync.polaris; -import java.util.Map; import org.apache.polaris.core.admin.model.PrincipalWithCredentials; import org.apache.polaris.tools.sync.polaris.access.CredentialWriter; import org.slf4j.Logger; @@ -29,9 +28,6 @@ public class ConsoleCredentialWriter implements CredentialWriter { private final Logger consoleLog = LoggerFactory.getLogger("console-log"); - @Override - public void initialize(Map properties) {} - @Override public void writeCredentials(PrincipalWithCredentials principalWithCredentials) { consoleLog.info( diff --git a/polaris-synchronizer/cli/src/main/java/org/apache/polaris/tools/sync/polaris/CreateOmnipotentPrincipalCommand.java b/polaris-synchronizer/cli/src/main/java/org/apache/polaris/tools/sync/polaris/CreateOmnipotentPrincipalCommand.java index 172dce80..c04f19a3 100644 --- a/polaris-synchronizer/cli/src/main/java/org/apache/polaris/tools/sync/polaris/CreateOmnipotentPrincipalCommand.java +++ b/polaris-synchronizer/cli/src/main/java/org/apache/polaris/tools/sync/polaris/CreateOmnipotentPrincipalCommand.java @@ -99,7 +99,7 @@ public class CreateOmnipotentPrincipalCommand implements Callable { "\n\t- " + JsonFileCredentialWriter.JSON_FILE_PROPERTY + ": The JSON Lines file to write principal credentials to." + "\n\t- " + JsonFileCredentialWriter.APPEND_PROPERTY + ": (default: false) Whether to append to an existing file instead of overwriting it." + "\nFor type CUSTOM:" + - "\n\t- " + CredentialWriterFactory.CUSTOM_CLASS_NAME_PROPERTY + ": The classname for the custom CredentialWriter implementation." + "\n\t- " + CredentialWriterFactory.CUSTOM_CLASS_NAME_PROPERTY + ": The classname of the CredentialWriter service provider to load via ServiceLoader." ) private Map credentialWriterProperties; diff --git a/polaris-synchronizer/cli/src/main/java/org/apache/polaris/tools/sync/polaris/CredentialWriterFactory.java b/polaris-synchronizer/cli/src/main/java/org/apache/polaris/tools/sync/polaris/CredentialWriterFactory.java index d81d92a3..20676f0c 100644 --- a/polaris-synchronizer/cli/src/main/java/org/apache/polaris/tools/sync/polaris/CredentialWriterFactory.java +++ b/polaris-synchronizer/cli/src/main/java/org/apache/polaris/tools/sync/polaris/CredentialWriterFactory.java @@ -22,14 +22,17 @@ import java.util.HashMap; import java.util.Map; +import java.util.ServiceLoader; /** - * Factory class to construct configurable {@link CredentialWriter} implementations. + * Factory class to construct configurable, pre-configured {@link CredentialWriter} instances. */ public class CredentialWriterFactory { /** - * Property that will hold class name for custom {@link CredentialWriter} implementation. + * Property that identifies which {@link ServiceLoader}-discovered {@link CredentialWriter} + * implementation to use for the {@link Type#CUSTOM} type, matched against + * {@link CredentialWriter#getClass()}'s fully-qualified classname. */ public static final String CUSTOM_CLASS_NAME_PROPERTY = "custom-impl"; @@ -45,40 +48,47 @@ public enum Type { } /** - * Construct a new {@link CredentialWriter} instance. + * Construct a new, pre-configured {@link CredentialWriter} instance. * @param type the recognized type of the {@link CredentialWriter} to construct - * @param properties properties to use when initializing the {@link CredentialWriter} - * @return the constructed and initialized {@link CredentialWriter} + * @param properties properties to use when constructing the {@link CredentialWriter} + * @return the constructed and ready-to-use {@link CredentialWriter} */ public static CredentialWriter createCredentialWriter(Type type, Map properties) { try { properties = properties == null ? new HashMap<>() : properties; - CredentialWriter writer = switch (type) { + return switch (type) { case CONSOLE -> new ConsoleCredentialWriter(); - case FILE -> new JsonFileCredentialWriter(); - case CUSTOM -> { - String customWriterClassname = properties.get(CUSTOM_CLASS_NAME_PROPERTY); - - if (customWriterClassname == null) { - throw new IllegalArgumentException("Missing required property " + CUSTOM_CLASS_NAME_PROPERTY); - } - - Object custom = Class.forName(customWriterClassname).getDeclaredConstructor().newInstance(); - - if (custom instanceof CredentialWriter customWriter) { - yield customWriter; - } - - throw new InstantiationException("Custom CredentialWriter '" + customWriterClassname + "' does not implement CredentialWriter"); - } + case FILE -> new JsonFileCredentialWriter(properties); + case CUSTOM -> loadCustomCredentialWriter(properties); }; - - writer.initialize(properties); - return writer; } catch (Exception e) { throw new RuntimeException("Failed to construct CredentialWriter", e); } } + /** + * Discovers a {@link CredentialWriter} implementation on the classpath via {@link ServiceLoader}, + * matching the classname supplied via {@link #CUSTOM_CLASS_NAME_PROPERTY}. Custom implementations + * must be registered as a service provider (i.e. declared in a + * {@code META-INF/services/org.apache.polaris.tools.sync.polaris.access.CredentialWriter} file) + * for {@link ServiceLoader} to discover them. + */ + private static CredentialWriter loadCustomCredentialWriter(Map properties) { + String customWriterClassname = properties.get(CUSTOM_CLASS_NAME_PROPERTY); + + if (customWriterClassname == null) { + throw new IllegalArgumentException("Missing required property " + CUSTOM_CLASS_NAME_PROPERTY); + } + + return ServiceLoader.load(CredentialWriter.class).stream() + .filter(provider -> provider.type().getName().equals(customWriterClassname)) + .findFirst() + .map(ServiceLoader.Provider::get) + .orElseThrow(() -> new IllegalArgumentException( + "No CredentialWriter service provider found for classname '" + customWriterClassname + + "'. Ensure it is registered as a service provider under META-INF/services/" + + CredentialWriter.class.getName())); + } + } diff --git a/polaris-synchronizer/cli/src/main/java/org/apache/polaris/tools/sync/polaris/JsonFileCredentialWriter.java b/polaris-synchronizer/cli/src/main/java/org/apache/polaris/tools/sync/polaris/JsonFileCredentialWriter.java index ab859cac..248eddaa 100644 --- a/polaris-synchronizer/cli/src/main/java/org/apache/polaris/tools/sync/polaris/JsonFileCredentialWriter.java +++ b/polaris-synchronizer/cli/src/main/java/org/apache/polaris/tools/sync/polaris/JsonFileCredentialWriter.java @@ -46,10 +46,9 @@ public class JsonFileCredentialWriter implements CredentialWriter, Closeable { private final ObjectMapper objectMapper = new ObjectMapper(); - private BufferedWriter writer; + private final BufferedWriter writer; - @Override - public void initialize(Map properties) { + public JsonFileCredentialWriter(Map properties) { if (!properties.containsKey(JSON_FILE_PROPERTY)) { throw new IllegalArgumentException("Missing required property " + JSON_FILE_PROPERTY); } diff --git a/polaris-synchronizer/cli/src/main/java/org/apache/polaris/tools/sync/polaris/SyncPolarisCommand.java b/polaris-synchronizer/cli/src/main/java/org/apache/polaris/tools/sync/polaris/SyncPolarisCommand.java index d66fd0c3..1848751c 100644 --- a/polaris-synchronizer/cli/src/main/java/org/apache/polaris/tools/sync/polaris/SyncPolarisCommand.java +++ b/polaris-synchronizer/cli/src/main/java/org/apache/polaris/tools/sync/polaris/SyncPolarisCommand.java @@ -101,7 +101,7 @@ public class SyncPolarisCommand implements Callable { "\n\t- " + JsonFileCredentialWriter.JSON_FILE_PROPERTY + ": The JSON Lines file to write principal credentials to." + "\n\t- " + JsonFileCredentialWriter.APPEND_PROPERTY + ": (default: false) Whether to append to an existing file instead of overwriting it." + "\nFor type CUSTOM:" + - "\n\t- " + CredentialWriterFactory.CUSTOM_CLASS_NAME_PROPERTY + ": The classname for the custom CredentialWriter implementation." + "\n\t- " + CredentialWriterFactory.CUSTOM_CLASS_NAME_PROPERTY + ": The classname of the CredentialWriter service provider to load via ServiceLoader." ) private Map credentialWriterProperties; diff --git a/polaris-synchronizer/cli/src/test/java/org/apache/polaris/tools/sync/polaris/CredentialWriterFactoryTest.java b/polaris-synchronizer/cli/src/test/java/org/apache/polaris/tools/sync/polaris/CredentialWriterFactoryTest.java index 9262707c..8a6038cd 100644 --- a/polaris-synchronizer/cli/src/test/java/org/apache/polaris/tools/sync/polaris/CredentialWriterFactoryTest.java +++ b/polaris-synchronizer/cli/src/test/java/org/apache/polaris/tools/sync/polaris/CredentialWriterFactoryTest.java @@ -57,15 +57,24 @@ public void failToConstructFileWriterMissingProperty() { public void constructCustomCredentialWriterSuccessfully() throws Exception { try (var writer = CredentialWriterFactory.createCredentialWriter( CredentialWriterFactory.Type.CUSTOM, - Map.of(CredentialWriterFactory.CUSTOM_CLASS_NAME_PROPERTY, ConsoleCredentialWriter.class.getName()))) { + Map.of(CredentialWriterFactory.CUSTOM_CLASS_NAME_PROPERTY, TestCredentialWriter.class.getName()))) { Assertions.assertNotNull(writer); + Assertions.assertInstanceOf(TestCredentialWriter.class, writer); } } @Test - public void failToConstructCustomCredentialWriter() { + public void failToConstructCustomCredentialWriterMissingProperty() { Assertions.assertThrows(Exception.class, () -> CredentialWriterFactory.createCredentialWriter(CredentialWriterFactory.Type.CUSTOM, Map.of())); } + @Test + public void failToConstructCustomCredentialWriterUnregisteredClass() { + Assertions.assertThrows(Exception.class, () -> + CredentialWriterFactory.createCredentialWriter( + CredentialWriterFactory.Type.CUSTOM, + Map.of(CredentialWriterFactory.CUSTOM_CLASS_NAME_PROPERTY, ConsoleCredentialWriter.class.getName()))); + } + } diff --git a/polaris-synchronizer/cli/src/test/java/org/apache/polaris/tools/sync/polaris/JsonFileCredentialWriterTest.java b/polaris-synchronizer/cli/src/test/java/org/apache/polaris/tools/sync/polaris/JsonFileCredentialWriterTest.java index fb0c4c70..1a4c1ec2 100644 --- a/polaris-synchronizer/cli/src/test/java/org/apache/polaris/tools/sync/polaris/JsonFileCredentialWriterTest.java +++ b/polaris-synchronizer/cli/src/test/java/org/apache/polaris/tools/sync/polaris/JsonFileCredentialWriterTest.java @@ -55,8 +55,8 @@ private PrincipalWithCredentials buildPrincipal(String name, String clientId, St public void writesSingleLineOfValidJson() throws Exception { Path path = tempDir.resolve("single.jsonl"); - try (JsonFileCredentialWriter writer = new JsonFileCredentialWriter()) { - writer.initialize(Map.of(JsonFileCredentialWriter.JSON_FILE_PROPERTY, path.toString())); + try (JsonFileCredentialWriter writer = + new JsonFileCredentialWriter(Map.of(JsonFileCredentialWriter.JSON_FILE_PROPERTY, path.toString()))) { writer.writeCredentials(buildPrincipal("test-principal", "client-id", "client-secret")); } @@ -73,8 +73,8 @@ public void writesSingleLineOfValidJson() throws Exception { public void appendsMultipleEntriesAsJsonLines() throws Exception { Path path = tempDir.resolve("multi.jsonl"); - try (JsonFileCredentialWriter writer = new JsonFileCredentialWriter()) { - writer.initialize(Map.of(JsonFileCredentialWriter.JSON_FILE_PROPERTY, path.toString())); + try (JsonFileCredentialWriter writer = + new JsonFileCredentialWriter(Map.of(JsonFileCredentialWriter.JSON_FILE_PROPERTY, path.toString()))) { writer.writeCredentials(buildPrincipal("principal-1", "id-1", "secret-1")); writer.writeCredentials(buildPrincipal("principal-2", "id-2", "secret-2")); writer.writeCredentials(buildPrincipal("principal-3", "id-3", "secret-3")); @@ -93,8 +93,8 @@ public void defaultOverwritesExistingFile() throws Exception { Path path = tempDir.resolve("overwrite.jsonl"); Files.writeString(path, "{\"stale\":\"data\"}\n"); - try (JsonFileCredentialWriter writer = new JsonFileCredentialWriter()) { - writer.initialize(Map.of(JsonFileCredentialWriter.JSON_FILE_PROPERTY, path.toString())); + try (JsonFileCredentialWriter writer = + new JsonFileCredentialWriter(Map.of(JsonFileCredentialWriter.JSON_FILE_PROPERTY, path.toString()))) { writer.writeCredentials(buildPrincipal("fresh-principal", "id", "secret")); } @@ -108,10 +108,10 @@ public void appendPropertyPreservesExistingContent() throws Exception { Path path = tempDir.resolve("append.jsonl"); Files.writeString(path, "{\"principal\":{\"name\":\"existing\"}}\n"); - try (JsonFileCredentialWriter writer = new JsonFileCredentialWriter()) { - writer.initialize(Map.of( - JsonFileCredentialWriter.JSON_FILE_PROPERTY, path.toString(), - JsonFileCredentialWriter.APPEND_PROPERTY, "true")); + try (JsonFileCredentialWriter writer = + new JsonFileCredentialWriter(Map.of( + JsonFileCredentialWriter.JSON_FILE_PROPERTY, path.toString(), + JsonFileCredentialWriter.APPEND_PROPERTY, "true"))) { writer.writeCredentials(buildPrincipal("new-principal", "id", "secret")); } @@ -129,8 +129,8 @@ public void filePermissionsRestrictedToOwner() throws Exception { org.junit.jupiter.api.Assumptions.assumeTrue( FileSystems.getDefault().supportedFileAttributeViews().contains("posix")); - try (JsonFileCredentialWriter writer = new JsonFileCredentialWriter()) { - writer.initialize(Map.of(JsonFileCredentialWriter.JSON_FILE_PROPERTY, path.toString())); + try (JsonFileCredentialWriter writer = + new JsonFileCredentialWriter(Map.of(JsonFileCredentialWriter.JSON_FILE_PROPERTY, path.toString()))) { writer.writeCredentials(buildPrincipal("principal", "id", "secret")); } diff --git a/polaris-synchronizer/cli/src/test/java/org/apache/polaris/tools/sync/polaris/TestCredentialWriter.java b/polaris-synchronizer/cli/src/test/java/org/apache/polaris/tools/sync/polaris/TestCredentialWriter.java new file mode 100644 index 00000000..5efe74e0 --- /dev/null +++ b/polaris-synchronizer/cli/src/test/java/org/apache/polaris/tools/sync/polaris/TestCredentialWriter.java @@ -0,0 +1,37 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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.apache.polaris.tools.sync.polaris; + +import org.apache.polaris.core.admin.model.PrincipalWithCredentials; +import org.apache.polaris.tools.sync.polaris.access.CredentialWriter; + +/** + * No-op {@link CredentialWriter} registered as a service provider (see {@code + * META-INF/services/org.apache.polaris.tools.sync.polaris.access.CredentialWriter}) so that {@link + * CredentialWriterFactory}'s {@code CUSTOM} type can be exercised in tests via {@link + * java.util.ServiceLoader}. + */ +public class TestCredentialWriter implements CredentialWriter { + + @Override + public void writeCredentials(PrincipalWithCredentials principalWithCredentials) {} + + @Override + public void close() {} +} diff --git a/polaris-synchronizer/cli/src/test/resources/META-INF/services/org.apache.polaris.tools.sync.polaris.access.CredentialWriter b/polaris-synchronizer/cli/src/test/resources/META-INF/services/org.apache.polaris.tools.sync.polaris.access.CredentialWriter new file mode 100644 index 00000000..d0dc1553 --- /dev/null +++ b/polaris-synchronizer/cli/src/test/resources/META-INF/services/org.apache.polaris.tools.sync.polaris.access.CredentialWriter @@ -0,0 +1 @@ +org.apache.polaris.tools.sync.polaris.TestCredentialWriter From d9b3ed3cf6aab9f7cbe68b28e0c291044bdf9a56 Mon Sep 17 00:00:00 2001 From: Sai Dixith Date: Mon, 27 Jul 2026 11:27:30 +0530 Subject: [PATCH 3/4] fix: resolve merge conflict in PolarisSynchronizer constructor Merging main (which added skipIcebergContent via #256) into this branch duplicated the constructor parameter and call-site lists instead of combining them with the credentialWriter parameter already on this branch. Merge them into a single correct signature, update all call sites, and pass a no-op credentialWriter in the skip-iceberg-content tests since they only exercise syncCatalogs(). --- .../apache/polaris/tools/sync/polaris/PolarisSynchronizer.java | 1 - .../polaris/PolarisSynchronizerSkipIcebergContentTest.java | 3 +++ .../apache/polaris/tools/sync/polaris/SyncPolarisCommand.java | 1 - 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/polaris-synchronizer/api/src/main/java/org/apache/polaris/tools/sync/polaris/PolarisSynchronizer.java b/polaris-synchronizer/api/src/main/java/org/apache/polaris/tools/sync/polaris/PolarisSynchronizer.java index ada77493..44f4e206 100644 --- a/polaris-synchronizer/api/src/main/java/org/apache/polaris/tools/sync/polaris/PolarisSynchronizer.java +++ b/polaris-synchronizer/api/src/main/java/org/apache/polaris/tools/sync/polaris/PolarisSynchronizer.java @@ -76,7 +76,6 @@ public PolarisSynchronizer( PolarisService target, ETagManager etagManager, CredentialWriter credentialWriter, - boolean diffOnly) { boolean diffOnly, boolean skipIcebergContent) { this.clientLogger = diff --git a/polaris-synchronizer/api/src/test/java/org/apache/polaris/tools/sync/polaris/PolarisSynchronizerSkipIcebergContentTest.java b/polaris-synchronizer/api/src/test/java/org/apache/polaris/tools/sync/polaris/PolarisSynchronizerSkipIcebergContentTest.java index e9e0a2c3..c23084ad 100644 --- a/polaris-synchronizer/api/src/test/java/org/apache/polaris/tools/sync/polaris/PolarisSynchronizerSkipIcebergContentTest.java +++ b/polaris-synchronizer/api/src/test/java/org/apache/polaris/tools/sync/polaris/PolarisSynchronizerSkipIcebergContentTest.java @@ -298,6 +298,7 @@ public void testSkipIcebergContentSkipsIcebergSyncButStillSyncsCatalogRoles() { source, target, new NoOpETagManager(), + null, false, true); @@ -322,6 +323,7 @@ public void testIcebergContentSyncedWhenNotSkipped() { source, target, new NoOpETagManager(), + null, false, false); @@ -353,6 +355,7 @@ public void testTableScopedGrantsStillAttemptedWhenIcebergContentSkipped() { source, target, new NoOpETagManager(), + null, false, true); diff --git a/polaris-synchronizer/cli/src/main/java/org/apache/polaris/tools/sync/polaris/SyncPolarisCommand.java b/polaris-synchronizer/cli/src/main/java/org/apache/polaris/tools/sync/polaris/SyncPolarisCommand.java index 376738c1..2b9c8b4b 100644 --- a/polaris-synchronizer/cli/src/main/java/org/apache/polaris/tools/sync/polaris/SyncPolarisCommand.java +++ b/polaris-synchronizer/cli/src/main/java/org/apache/polaris/tools/sync/polaris/SyncPolarisCommand.java @@ -175,7 +175,6 @@ public Integer call() throws Exception { target, etagManager, credentialWriter, - diffOnly); diffOnly, skipIcebergContent); synchronizer.syncPrincipalRoles(); From 5cf138e4a78969cb8d7a9a440e5dd88c04885575 Mon Sep 17 00:00:00 2001 From: Sai Dixith Date: Tue, 28 Jul 2026 09:33:41 +0530 Subject: [PATCH 4/4] fix: exclude META-INF/services files from Apache Rat license check ServiceLoader provider-config files are plain classnames with no comment syntax to hold a license header, so Rat was flagging the new CredentialWriter test service file as an unapproved license, failing the :rat task in CI. --- polaris-synchronizer/build.gradle.kts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/polaris-synchronizer/build.gradle.kts b/polaris-synchronizer/build.gradle.kts index b727a0e4..4590c8e3 100644 --- a/polaris-synchronizer/build.gradle.kts +++ b/polaris-synchronizer/build.gradle.kts @@ -78,6 +78,9 @@ tasks.named("rat").configure { // Rat can't scan binary images excludes.add("**/*.png") + + // ServiceLoader provider-config files are plain classnames with no comment syntax for a header + excludes.add("**/META-INF/services/**") } tasks.named("wrapper") {