Extract KerberosConfig from SecurityConfig

Introduced a new property called "http-server.authentication.type"
which can have values NONE, KERBEROS (and LDAP- separate commit).

presto-main/src/main/java/com/facebook/presto/server/security/KerberosConfig.java

@@ -0,0 +1,61 @@
+/*
+ * Licensed 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 com.facebook.presto.server.security;
+
+import io.airlift.configuration.Config;
+
+import java.io.File;
+
+public class KerberosConfig
+{
+    private File kerberosConfig;
+    private String serviceName;
+    private File keytab;
+
+    public File getKerberosConfig()
+    {
+        return kerberosConfig;
+    }
+
+    @Config("http.authentication.krb5.config")
+    public KerberosConfig setKerberosConfig(File kerberosConfig)
+    {
+        this.kerberosConfig = kerberosConfig;
+        return this;
+    }
+
+    public String getServiceName()
+    {
+        return serviceName;
+    }
+
+    @Config("http.server.authentication.krb5.service-name")
+    public KerberosConfig setServiceName(String serviceName)
+    {
+        this.serviceName = serviceName;
+        return this;
+    }
+
+    public File getKeytab()
+    {
+        return keytab;
+    }
+
+    @Config("http.server.authentication.krb5.keytab")
+    public KerberosConfig setKeytab(File keytab)
+    {
+        this.keytab = keytab;
+        return this;
+    }
+}

presto-main/src/main/java/com/facebook/presto/server/security/SecurityConfig.java

@@ -14,61 +14,33 @@
 package com.facebook.presto.server.security;
 
 import io.airlift.configuration.Config;
+import io.airlift.configuration.ConfigDescription;
+import io.airlift.configuration.DefunctConfig;
 
-import java.io.File;
+import javax.validation.constraints.NotNull;
 
+@DefunctConfig("http.server.authentication.enabled")
 public class SecurityConfig
 {
-    private boolean authenticationEnabled;
-    private File kerberosConfig;
-    private String serviceName;
-    private File keytab;
+    private AuthenticationType authenticationType = AuthenticationType.NONE;
 
-    public File getKerberosConfig()
+    public enum AuthenticationType
     {
-        return kerberosConfig;
+        NONE,
+        KERBEROS
     }
 
-    @Config("http.authentication.krb5.config")
-    public SecurityConfig setKerberosConfig(File kerberosConfig)
+    @NotNull
+    public AuthenticationType getAuthenticationType()
     {
-        this.kerberosConfig = kerberosConfig;
-        return this;
-    }
-
-    public boolean getAuthenticationEnabled()
-    {
-        return authenticationEnabled;
-    }
-
-    @Config("http.server.authentication.enabled")
-    public SecurityConfig setAuthenticationEnabled(boolean enabled)
-    {
-        this.authenticationEnabled = enabled;
-        return this;
-    }
-
-    public String getServiceName()
-    {
-        return serviceName;
-    }
-
-    @Config("http.server.authentication.krb5.service-name")
-    public SecurityConfig setServiceName(String serviceName)
-    {
-        this.serviceName = serviceName;
-        return this;
-    }
-
-    public File getKeytab()
-    {
-        return keytab;
+        return authenticationType;
     }
 
-    @Config("http.server.authentication.krb5.keytab")
-    public SecurityConfig setKeytab(File keytab)
+    @Config("http-server.authentication.type")
+    @ConfigDescription("Authentication type (supported types: NONE, KERBEROS)")
+    public SecurityConfig setAuthenticationType(AuthenticationType authenticationType)
     {
-        this.keytab = keytab;
+        this.authenticationType = authenticationType;
         return this;
     }
 }

presto-main/src/main/java/com/facebook/presto/server/security/ServerSecurityModule.java

@@ -14,29 +14,39 @@
 package com.facebook.presto.server.security;
 
 import com.google.inject.Binder;
+import com.google.inject.Module;
 import com.google.inject.Scopes;
 import com.google.inject.multibindings.Multibinder;
 import io.airlift.configuration.AbstractConfigurationAwareModule;
 import io.airlift.http.server.TheServlet;
 
 import javax.servlet.Filter;
 
+import java.util.function.Predicate;
+
+import static com.facebook.presto.server.security.SecurityConfig.AuthenticationType.KERBEROS;
+import static io.airlift.configuration.ConditionalModule.installModuleIf;
 import static io.airlift.configuration.ConfigBinder.configBinder;
 
 public class ServerSecurityModule
         extends AbstractConfigurationAwareModule
 {
     @Override
-    protected void setup(Binder binder)
+    protected void setup(Binder securityBinder)
     {
-        configBinder(binder).bindConfig(SecurityConfig.class);
+        bindSecurityConfig(
+                securityConfig -> securityConfig.getAuthenticationType() == KERBEROS,
+                binder -> {
+                    configBinder(binder).bindConfig(KerberosConfig.class);
+                    Multibinder.newSetBinder(binder, Filter.class, TheServlet.class)
+                            .addBinding()
+                            .to(SpnegoFilter.class)
+                            .in(Scopes.SINGLETON);
+                });
+    }
 
-        SecurityConfig config = buildConfigObject(SecurityConfig.class);
-        if (config.getAuthenticationEnabled()) {
-            Multibinder.newSetBinder(binder, Filter.class, TheServlet.class)
-                    .addBinding()
-                    .to(SpnegoFilter.class)
-                    .in(Scopes.SINGLETON);
-        }
+    private void bindSecurityConfig(Predicate<SecurityConfig> predicate, Module module)
+    {
+        install(installModuleIf(SecurityConfig.class, predicate, module));
     }
 }

presto-main/src/main/java/com/facebook/presto/server/security/SpnegoFilter.java

@@ -75,7 +75,7 @@ public class SpnegoFilter
     private final GSSCredential serverCredential;
 
     @Inject
-    public SpnegoFilter(SecurityConfig config)
+    public SpnegoFilter(KerberosConfig config)
     {
         System.setProperty("java.security.krb5.conf", config.getKerberosConfig().getAbsolutePath());
 

presto-main/src/test/java/com/facebook/presto/server/security/TestKerberosConfig.java

@@ -0,0 +1,50 @@
+/*
+ * Licensed 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 com.facebook.presto.server.security;
+
+import com.google.common.collect.ImmutableMap;
+import io.airlift.configuration.testing.ConfigAssertions;
+import org.testng.annotations.Test;
+
+import java.io.File;
+import java.util.Map;
+
+public class TestKerberosConfig
+{
+    @Test
+    public void testDefaults()
+    {
+        ConfigAssertions.assertRecordedDefaults(ConfigAssertions.recordDefaults(KerberosConfig.class)
+                .setKerberosConfig(null)
+                .setServiceName(null)
+                .setKeytab(null));
+    }
+
+    @Test
+    public void testExplicitPropertyMappings()
+    {
+        Map<String, String> properties = new ImmutableMap.Builder<String, String>()
+                .put("http.authentication.krb5.config", "/etc/krb5.conf")
+                .put("http.server.authentication.krb5.service-name", "airlift")
+                .put("http.server.authentication.krb5.keytab", "/tmp/presto.keytab")
+                .build();
+
+        KerberosConfig expected = new KerberosConfig()
+                .setKerberosConfig(new File("/etc/krb5.conf"))
+                .setServiceName("airlift")
+                .setKeytab(new File("/tmp/presto.keytab"));
+
+        ConfigAssertions.assertFullMapping(properties, expected);
+    }
+}

presto-main/src/test/java/com/facebook/presto/server/security/TestSecurityConfig.java

@@ -17,36 +17,29 @@
 import io.airlift.configuration.testing.ConfigAssertions;
 import org.testng.annotations.Test;
 
-import java.io.File;
 import java.util.Map;
 
+import static com.facebook.presto.server.security.SecurityConfig.AuthenticationType.KERBEROS;
+import static com.facebook.presto.server.security.SecurityConfig.AuthenticationType.NONE;
+
 public class TestSecurityConfig
 {
     @Test
     public void testDefaults()
     {
         ConfigAssertions.assertRecordedDefaults(ConfigAssertions.recordDefaults(SecurityConfig.class)
-                .setKerberosConfig(null)
-                .setAuthenticationEnabled(false)
-                .setServiceName(null)
-                .setKeytab(null));
+                .setAuthenticationType(NONE));
     }
 
     @Test
     public void testExplicitPropertyMappings()
     {
         Map<String, String> properties = new ImmutableMap.Builder<String, String>()
-                .put("http.authentication.krb5.config", "/etc/krb5.conf")
-                .put("http.server.authentication.enabled", "true")
-                .put("http.server.authentication.krb5.service-name", "airlift")
-                .put("http.server.authentication.krb5.keytab", "/tmp/presto.keytab")
+                .put("http-server.authentication.type", "KERBEROS")
                 .build();
 
         SecurityConfig expected = new SecurityConfig()
-                .setKerberosConfig(new File("/etc/krb5.conf"))
-                .setAuthenticationEnabled(true)
-                .setServiceName("airlift")
-                .setKeytab(new File("/tmp/presto.keytab"));
+                .setAuthenticationType(KERBEROS);
 
         ConfigAssertions.assertFullMapping(properties, expected);
     }

presto-product-tests/conf/presto/etc/singlenode-kerberized.properties

@@ -17,7 +17,7 @@ discovery-server.enabled=true
 discovery.uri=http://presto-master.docker.cluster:8080
 
 http.authentication.krb5.config=/etc/krb5.conf
-http.server.authentication.enabled=true
+http-server.authentication.type=KERBEROS
 http.server.authentication.krb5.service-name=presto-server
 http-server.https.enabled=true
 http-server.https.port=7778