From e0cd731d43649af980ab3aa573a49442b6434d2f Mon Sep 17 00:00:00 2001 From: Andy Coates <8012398+big-andy-coates@users.noreply.github.com> Date: Thu, 27 Feb 2020 14:49:47 +0000 Subject: [PATCH] chore: api config validation tweak (#4600) move the validation of the client auth config into the config class. --- .../ksql/api/server/ApiServerConfig.java | 7 ++++- .../io/confluent/ksql/api/server/Server.java | 26 +++---------------- .../ksql/api/ApiServerConfigTest.java | 21 +++++++++++++++ 3 files changed, 30 insertions(+), 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 index fa0885d6a0f1..b1cfb759c61b 100644 --- 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 index 1ba02d103b47..c119e6dc3850 100644 --- 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 index 584b7bb5581f..50cfce6bedc4 100644 --- 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)); + } }