Expose security-sensitive properties for Hudi connector

Author: piotrrzysko

Diff for: plugin/trino-hudi/src/main/java/io/trino/plugin/hudi/HudiConnectorFactory.java

@@ -13,19 +13,22 @@
  */
 package io.trino.plugin.hudi;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.ImmutableSet;
 import com.google.inject.Injector;
 import com.google.inject.Key;
 import com.google.inject.Module;
 import io.airlift.bootstrap.Bootstrap;
 import io.airlift.bootstrap.LifeCycleManager;
+import io.airlift.configuration.ConfigPropertyMetadata;
 import io.airlift.json.JsonModule;
 import io.opentelemetry.api.OpenTelemetry;
 import io.opentelemetry.api.trace.Tracer;
 import io.trino.filesystem.manager.FileSystemModule;
 import io.trino.plugin.base.classloader.ClassLoaderSafeConnectorPageSourceProvider;
 import io.trino.plugin.base.classloader.ClassLoaderSafeConnectorSplitManager;
 import io.trino.plugin.base.classloader.ClassLoaderSafeNodePartitioningProvider;
+import io.trino.plugin.base.config.ConfigUtils;
 import io.trino.plugin.base.jmx.MBeanServerModule;
 import io.trino.plugin.base.session.SessionPropertiesProvider;
 import io.trino.plugin.hive.NodeVersion;
@@ -52,6 +55,8 @@
 public class HudiConnectorFactory
         implements ConnectorFactory
 {
+    private static final Module DEFAULT_ADDITIONAL_MODULE = EMPTY_MODULE;
+
     @Override
     public String getName()
     {
@@ -62,38 +67,34 @@ public String getName()
     public Connector create(String catalogName, Map<String, String> config, ConnectorContext context)
     {
         checkStrictSpiVersionMatch(context, this);
-        return createConnector(catalogName, config, context, Optional.empty());
+        return createConnector(catalogName, config, context, DEFAULT_ADDITIONAL_MODULE);
+    }
+
+    @Override
+    public Set<String> getSecuritySensitivePropertyNames(String catalogName, Map<String, String> config, ConnectorContext context)
+    {
+        ClassLoader classLoader = HudiConnectorFactory.class.getClassLoader();
+        try (ThreadContextClassLoader _ = new ThreadContextClassLoader(classLoader)) {
+            Bootstrap app = createBootstrap(catalogName, config, context, DEFAULT_ADDITIONAL_MODULE, true);
+
+            Set<ConfigPropertyMetadata> usedProperties = app.configureAndGetUsedProperties();
+
+            return ConfigUtils.getSecuritySensitivePropertyNames(config, usedProperties);
+        }
     }
 
+    @VisibleForTesting
     public static Connector createConnector(
             String catalogName,
             Map<String, String> config,
             ConnectorContext context,
-            Optional<Module> module)
+            Module module)
     {
         ClassLoader classLoader = HudiConnectorFactory.class.getClassLoader();
         try (ThreadContextClassLoader _ = new ThreadContextClassLoader(classLoader)) {
-            Bootstrap app = new Bootstrap(
-                    new MBeanModule(),
-                    new JsonModule(),
-                    new HudiModule(),
-                    new HiveMetastoreModule(Optional.empty()),
-                    new FileSystemModule(catalogName, context.getNodeManager(), context.getOpenTelemetry(), false, false),
-                    new MBeanServerModule(),
-                    module.orElse(EMPTY_MODULE),
-                    binder -> {
-                        binder.bind(OpenTelemetry.class).toInstance(context.getOpenTelemetry());
-                        binder.bind(Tracer.class).toInstance(context.getTracer());
-                        binder.bind(NodeVersion.class).toInstance(new NodeVersion(context.getNodeManager().getCurrentNode().getVersion()));
-                        binder.bind(NodeManager.class).toInstance(context.getNodeManager());
-                        binder.bind(TypeManager.class).toInstance(context.getTypeManager());
-                        binder.bind(CatalogName.class).toInstance(new CatalogName(catalogName));
-                    });
-
-            Injector injector = app
-                    .doNotInitializeLogging()
-                    .setRequiredConfigurationProperties(config)
-                    .initialize();
+            Bootstrap app = createBootstrap(catalogName, config, context, module, false);
+
+            Injector injector = app.initialize();
 
             LifeCycleManager lifeCycleManager = injector.getInstance(LifeCycleManager.class);
             HudiTransactionManager transactionManager = injector.getInstance(HudiTransactionManager.class);
@@ -115,4 +116,38 @@ public static Connector createConnector(
                     hudiTableProperties.getTableProperties());
         }
     }
+
+    private static Bootstrap createBootstrap(
+            String catalogName,
+            Map<String, String> config,
+            ConnectorContext context,
+            Module module,
+            boolean quietBootstrap)
+    {
+        Bootstrap app = new Bootstrap(
+                new MBeanModule(),
+                new JsonModule(),
+                new HudiModule(),
+                new HiveMetastoreModule(Optional.empty()),
+                new FileSystemModule(catalogName, context.getNodeManager(), context.getOpenTelemetry(), false, quietBootstrap),
+                new MBeanServerModule(),
+                module,
+                binder -> {
+                    binder.bind(OpenTelemetry.class).toInstance(context.getOpenTelemetry());
+                    binder.bind(Tracer.class).toInstance(context.getTracer());
+                    binder.bind(NodeVersion.class).toInstance(new NodeVersion(context.getNodeManager().getCurrentNode().getVersion()));
+                    binder.bind(NodeManager.class).toInstance(context.getNodeManager());
+                    binder.bind(TypeManager.class).toInstance(context.getTypeManager());
+                    binder.bind(CatalogName.class).toInstance(new CatalogName(catalogName));
+                });
+
+        if (quietBootstrap) {
+            app.quiet()
+                    .suppressErrorsAndWarnings();
+        }
+
+        return app
+                .doNotInitializeLogging()
+                .setRequiredConfigurationProperties(config);
+    }
 }

Diff for: plugin/trino-hudi/src/test/java/io/trino/plugin/hudi/TestHudiPlugin.java

@@ -18,7 +18,11 @@
 import io.trino.testing.TestingConnectorContext;
 import org.junit.jupiter.api.Test;
 
+import java.util.Map;
+import java.util.Set;
+
 import static com.google.common.collect.Iterables.getOnlyElement;
+import static org.assertj.core.api.Assertions.assertThat;
 
 final class TestHudiPlugin
 {
@@ -35,4 +39,24 @@ void testCreateConnector()
                         new TestingConnectorContext())
                 .shutdown();
     }
+
+    @Test
+    void testGetSecuritySensitivePropertyNames()
+    {
+        ConnectorFactory factory = getOnlyElement(new HudiPlugin().getConnectorFactories());
+        Map<String, String> config = ImmutableMap.of(
+                "non-existent-property", "value",
+                "fs.hadoop.enabled", "true",
+                "hive.azure.abfs.oauth.client-id", "test-client-id", // security-sensitive property from trino-hdfs
+                "hive.azure.adl-proxy-host", "proxy-host:9800", // non-sensitive property from trino-hdfs
+                "hive.dfs-timeout", "invalidValue", // property from trino-hdfs with invalid value
+                "hive.metastore.uri", "thrift://foo:1234",
+                "hive.metastore.thrift.client.ssl.key-password", "password",
+                "hudi.size-based-split-weights-enabled", "shouldBeBoolean");
+
+        Set<String> sensitiveProperties = factory.getSecuritySensitivePropertyNames("catalog", config, new TestingConnectorContext());
+
+        assertThat(sensitiveProperties)
+                .containsExactlyInAnyOrder("non-existent-property", "hive.azure.abfs.oauth.client-id", "hive.metastore.thrift.client.ssl.key-password");
+    }
 }

Diff for: plugin/trino-hudi/src/test/java/io/trino/plugin/hudi/TestingHudiConnectorFactory.java

@@ -23,7 +23,6 @@
 
 import java.nio.file.Path;
 import java.util.Map;
-import java.util.Optional;
 
 import static com.google.inject.multibindings.MapBinder.newMapBinder;
 import static io.airlift.configuration.ConfigBinder.configBinder;
@@ -55,10 +54,10 @@ public Connector create(String catalogName, Map<String, String> config, Connecto
         if (!config.containsKey("hive.metastore")) {
             configBuilder.put("hive.metastore", "file");
         }
-        return createConnector(catalogName, configBuilder.buildOrThrow(), context, Optional.of(binder -> {
+        return createConnector(catalogName, configBuilder.buildOrThrow(), context, binder -> {
             newMapBinder(binder, String.class, TrinoFileSystemFactory.class)
                     .addBinding("local").toInstance(new LocalFileSystemFactory(localFileSystemRootPath));
             configBinder(binder).bindConfigDefaults(FileHiveMetastoreConfig.class, metastoreConfig -> metastoreConfig.setCatalogDirectory("local:///managed/"));
-        }));
+        });
     }
 }