From 2a5de1c5c525754b8d27fb42c2c5c9054265b63a Mon Sep 17 00:00:00 2001
From: Karol Sobczak <napewnotrafi@gmail.com>
Date: Thu, 6 Aug 2020 13:34:22 +0200
Subject: [PATCH] Move impersonation check to DefaultRubixHdfsInitializer

---
 .../hive/authentication/HiveAuthenticationModule.java     | 6 ------
 .../java/io/prestosql/plugin/hive/rubix/RubixModule.java  | 8 ++++++++
 .../io/prestosql/plugin/hive/rubix/TestRubixCaching.java  | 5 +++--
 3 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/presto-hive/src/main/java/io/prestosql/plugin/hive/authentication/HiveAuthenticationModule.java b/presto-hive/src/main/java/io/prestosql/plugin/hive/authentication/HiveAuthenticationModule.java
index 7ac43b540adf..9954cb12f782 100644
--- a/presto-hive/src/main/java/io/prestosql/plugin/hive/authentication/HiveAuthenticationModule.java
+++ b/presto-hive/src/main/java/io/prestosql/plugin/hive/authentication/HiveAuthenticationModule.java
@@ -17,11 +17,9 @@
 import com.google.inject.Module;
 import io.airlift.configuration.AbstractConfigurationAwareModule;
 import io.prestosql.plugin.hive.authentication.HiveAuthenticationConfig.HdfsAuthenticationType;
-import io.prestosql.plugin.hive.rubix.RubixEnabledConfig;
 
 import java.util.function.Predicate;
 
-import static com.google.common.base.Preconditions.checkArgument;
 import static io.airlift.configuration.ConditionalModule.installModuleIf;
 import static io.prestosql.plugin.hive.authentication.AuthenticationModules.kerberosHdfsAuthenticationModule;
 import static io.prestosql.plugin.hive.authentication.AuthenticationModules.kerberosImpersonatingHdfsAuthenticationModule;
@@ -49,10 +47,6 @@ protected void setup(Binder binder)
         bindAuthenticationModule(
                 config -> kerberosHdfsAuth(config) && config.isHdfsImpersonationEnabled(),
                 kerberosImpersonatingHdfsAuthenticationModule());
-
-        checkArgument(
-                !buildConfigObject(HiveAuthenticationConfig.class).isHdfsImpersonationEnabled() || !buildConfigObject(RubixEnabledConfig.class).isCacheEnabled(),
-                "Hdfs impersonation is not compatible with Hive caching");
     }
 
     private void bindAuthenticationModule(Predicate<HiveAuthenticationConfig> predicate, Module module)
diff --git a/presto-hive/src/main/java/io/prestosql/plugin/hive/rubix/RubixModule.java b/presto-hive/src/main/java/io/prestosql/plugin/hive/rubix/RubixModule.java
index 2f5cbecbcb51..9ff414924d86 100644
--- a/presto-hive/src/main/java/io/prestosql/plugin/hive/rubix/RubixModule.java
+++ b/presto-hive/src/main/java/io/prestosql/plugin/hive/rubix/RubixModule.java
@@ -20,6 +20,7 @@
 import com.google.inject.Scopes;
 import com.qubole.rubix.prestosql.CachingPrestoDistributedFileSystem;
 import io.prestosql.plugin.hive.DynamicConfigurationProvider;
+import io.prestosql.plugin.hive.authentication.HiveAuthenticationConfig;
 import org.apache.hadoop.conf.Configuration;
 
 import java.util.Set;
@@ -38,6 +39,7 @@ public class RubixModule
     public void configure(Binder binder)
     {
         configBinder(binder).bindConfig(RubixConfig.class);
+        configBinder(binder).bindConfig(HiveAuthenticationConfig.class);
         binder.bind(RubixConfigurationInitializer.class).in(Scopes.SINGLETON);
         binder.bind(RubixInitializer.class).in(Scopes.SINGLETON);
         // Make initialization of Rubix happen just once.
@@ -65,6 +67,12 @@ private RubixStarter(RubixInitializer rubixInitializer, Set<DynamicConfiguration
     static class DefaultRubixHdfsInitializer
             implements RubixHdfsInitializer
     {
+        @Inject
+        public DefaultRubixHdfsInitializer(HiveAuthenticationConfig authenticationConfig)
+        {
+            checkArgument(!authenticationConfig.isHdfsImpersonationEnabled(), "Hdfs impersonation is not compatible with Hive caching");
+        }
+
         @Override
         public void initializeConfiguration(Configuration config)
         {
diff --git a/presto-hive/src/test/java/io/prestosql/plugin/hive/rubix/TestRubixCaching.java b/presto-hive/src/test/java/io/prestosql/plugin/hive/rubix/TestRubixCaching.java
index 20525cbd6703..dfd32eaf943c 100644
--- a/presto-hive/src/test/java/io/prestosql/plugin/hive/rubix/TestRubixCaching.java
+++ b/presto-hive/src/test/java/io/prestosql/plugin/hive/rubix/TestRubixCaching.java
@@ -35,6 +35,7 @@
 import io.prestosql.plugin.hive.HdfsEnvironment.HdfsContext;
 import io.prestosql.plugin.hive.HiveConfig;
 import io.prestosql.plugin.hive.HiveHdfsConfiguration;
+import io.prestosql.plugin.hive.authentication.HiveAuthenticationConfig;
 import io.prestosql.plugin.hive.authentication.NoHdfsAuthentication;
 import io.prestosql.plugin.hive.orc.OrcReaderConfig;
 import io.prestosql.plugin.hive.rubix.RubixConfig.ReadMode;
@@ -187,7 +188,7 @@ private void initializeRubix(RubixConfig rubixConfig, List<Node> nodes)
                 nodeManager,
                 new CatalogName("catalog"),
                 configurationInitializer,
-                new DefaultRubixHdfsInitializer());
+                new DefaultRubixHdfsInitializer(new HiveAuthenticationConfig()));
         rubixConfigInitializer = new RubixConfigurationInitializer(rubixInitializer);
         rubixInitializer.initializeRubix();
         retry().run("wait for rubix to startup", () -> {
@@ -303,7 +304,7 @@ public void testCoordinatorNotJoining()
                 new TestingNodeManager(ImmutableList.of(workerNode)),
                 new CatalogName("catalog"),
                 configurationInitializer,
-                new DefaultRubixHdfsInitializer());
+                new DefaultRubixHdfsInitializer(new HiveAuthenticationConfig()));
         assertThatThrownBy(rubixInitializer::initializeRubix)
                 .hasMessage("No coordinator node available");
     }