chore: api config validation tweak (#4600)

move the validation of the client auth config into the config class.
confluentinc · Feb 27, 2020 · e0cd731 · e0cd731
1 parent 849255a
commit e0cd731
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 24 deletions.
diff --git a/ksql-rest-app/src/main/java/io/confluent/ksql/api/server/ApiServerConfig.java b/ksql-rest-app/src/main/java/io/confluent/ksql/api/server/ApiServerConfig.java
@@ -18,7 +18,9 @@
 import static io.confluent.ksql.configdef.ConfigValidators.oneOrMore;
 import static io.confluent.ksql.configdef.ConfigValidators.zeroOrPositive;
 
+import io.confluent.ksql.configdef.ConfigValidators;
 import io.confluent.ksql.util.KsqlConfig;
+import io.vertx.core.http.ClientAuth;
 import java.util.Map;
 import org.apache.kafka.common.config.AbstractConfig;
 import org.apache.kafka.common.config.ConfigDef;
@@ -144,6 +146,7 @@ private static String propertyName(final String name) {
           TLS_CLIENT_AUTH_REQUIRED,
           Type.STRING,
           DEFAULT_TLS_CLIENT_AUTH_REQUIRED,
+          ConfigValidators.enumValues(ClientAuth.class),
           Importance.MEDIUM,
           TLS_CLIENT_AUTH_REQUIRED_DOC)
       .define(
@@ -165,5 +168,7 @@ public ApiServerConfig(final Map<?, ?> map) {
     super(CONFIG_DEF, map);
   }
 
-
+  public ClientAuth getClientAuth() {
+    return ClientAuth.valueOf(getString(TLS_CLIENT_AUTH_REQUIRED).toUpperCase());
+  }
 }
diff --git a/ksql-rest-app/src/main/java/io/confluent/ksql/api/server/Server.java b/ksql-rest-app/src/main/java/io/confluent/ksql/api/server/Server.java
@@ -21,7 +21,6 @@
 import io.vertx.core.DeploymentOptions;
 import io.vertx.core.Vertx;
 import io.vertx.core.WorkerExecutor;
-import io.vertx.core.http.ClientAuth;
 import io.vertx.core.http.HttpConnection;
 import io.vertx.core.http.HttpServerOptions;
 import io.vertx.core.impl.ConcurrentHashSet;
@@ -33,7 +32,6 @@
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.atomic.AtomicInteger;
-import org.apache.kafka.common.config.ConfigException;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -55,7 +53,7 @@ public class Server {
   private final int maxPushQueryCount;
   private String deploymentID;
   private WorkerExecutor workerExecutor;
-  private AtomicInteger actualPort = new AtomicInteger(-1);
+  private final AtomicInteger actualPort = new AtomicInteger(-1);
 
   public Server(final Vertx vertx, final ApiServerConfig config, final Endpoints endpoints) {
     this.vertx = Objects.requireNonNull(vertx);
@@ -148,7 +146,7 @@ public int getActualPort() {
     return actualPort.get();
   }
 
-  private HttpServerOptions createHttpServerOptions(final ApiServerConfig apiServerConfig) {
+  private static HttpServerOptions createHttpServerOptions(final ApiServerConfig apiServerConfig) {
 
     final HttpServerOptions options = new HttpServerOptions()
         .setHost(apiServerConfig.getString(ApiServerConfig.LISTEN_HOST))
@@ -168,27 +166,9 @@ private HttpServerOptions createHttpServerOptions(final ApiServerConfig apiServe
                   .setPath(apiServerConfig.getString(ApiServerConfig.TLS_TRUST_STORE_PATH))
                   .setPassword(
                       apiServerConfig.getString(ApiServerConfig.TLS_TRUST_STORE_PASSWORD)))
-          .setClientAuth(
-              toClientAuth(apiServerConfig.getString(ApiServerConfig.TLS_CLIENT_AUTH_REQUIRED)));
+          .setClientAuth(apiServerConfig.getClientAuth());
     }
 
     return options;
   }
-
-  private static ClientAuth toClientAuth(final String val) {
-    switch (val) {
-      case "none":
-        return ClientAuth.NONE;
-      case "request":
-        return ClientAuth.REQUEST;
-      case "required":
-        return ClientAuth.REQUIRED;
-      default:
-        throw new ConfigException(
-            "Invalid value for %s (%s) should be one of 'none', 'request' or 'required'",
-            ApiServerConfig.TLS_CLIENT_AUTH_REQUIRED, val);
-    }
-  }
-
-
 }
diff --git a/ksql-rest-app/src/test/java/io/confluent/ksql/api/ApiServerConfigTest.java b/ksql-rest-app/src/test/java/io/confluent/ksql/api/ApiServerConfigTest.java
@@ -18,9 +18,12 @@
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.is;
 
+import com.google.common.collect.ImmutableMap;
 import io.confluent.ksql.api.server.ApiServerConfig;
+import io.vertx.core.http.ClientAuth;
 import java.util.HashMap;
 import java.util.Map;
+import org.apache.kafka.common.config.ConfigException;
 import org.junit.Test;
 
 public class ApiServerConfigTest {
@@ -76,4 +79,22 @@ public void shouldUseDefaults() {
     assertThat(config.getString(ApiServerConfig.TLS_TRUST_STORE_PASSWORD), is(""));
     assertThat(config.getString(ApiServerConfig.TLS_CLIENT_AUTH_REQUIRED), is("none"));
   }
+
+  @Test(expected = ConfigException.class)
+  public void shouldThrowOnUnknownClientAuth() {
+    new ApiServerConfig(ImmutableMap.of(
+        ApiServerConfig.TLS_CLIENT_AUTH_REQUIRED, "blah"
+    ));
+  }
+
+  @Test
+  public void shouldGetValidClientAuth() {
+    // Given:
+    final ApiServerConfig config = new ApiServerConfig(ImmutableMap.of(
+        ApiServerConfig.TLS_CLIENT_AUTH_REQUIRED, "ReQuIrEd"
+    ));
+
+    // Then:
+    assertThat(config.getClientAuth(), is(ClientAuth.REQUIRED));
+  }
 }