This is an automated email from the ASF dual-hosted git repository.
apucher pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git
The following commit(s) were added to refs/heads/master by this push:
new bf66eeeef4 add-table refactor of auth (#9228)
bf66eeeef4 is described below
commit bf66eeeef42a4e1890b9a57cd38153b51f2f38a4
Author: Alexander Pucher <[email protected]>
AuthorDate: Thu Aug 18 10:12:53 2022 -0700
add-table refactor of auth (#9228)
---
.../controller/api/access/AccessControlUtils.java | 85 ++++++++++------------
.../api/access/AuthenticationFilter.java | 45 ++++++------
.../PinotAccessControlUserRestletResource.java | 17 ++---
.../api/resources/PinotSchemaRestletResource.java | 5 +-
.../api/resources/PinotTableRestletResource.java | 5 +-
.../api/resources/TableConfigsRestletResource.java | 7 +-
.../api/access/AuthenticationFilterTest.java | 23 ++----
.../pinot/tools/admin/command/AddTableCommand.java | 56 +++++++-------
8 files changed, 109 insertions(+), 134 deletions(-)
diff --git
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/access/AccessControlUtils.java
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/access/AccessControlUtils.java
index c12ba307bd..a19cad5054 100644
---
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/access/AccessControlUtils.java
+++
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/access/AccessControlUtils.java
@@ -19,9 +19,10 @@
package org.apache.pinot.controller.api.access;
-import java.util.Optional;
+import javax.annotation.Nullable;
import javax.ws.rs.core.HttpHeaders;
import javax.ws.rs.core.Response;
+import org.apache.commons.lang3.StringUtils;
import
org.apache.pinot.controller.api.exception.ControllerApplicationException;
import org.apache.pinot.spi.utils.builder.TableNameBuilder;
import org.slf4j.Logger;
@@ -31,7 +32,11 @@ import org.slf4j.LoggerFactory;
/**
* Utility class to simplify access control validation. This class is simple
wrapper around AccessControl class.
*/
-public class AccessControlUtils {
+public final class AccessControlUtils {
+ private AccessControlUtils() {
+ // left blank
+ }
+
private static final Logger LOGGER =
LoggerFactory.getLogger(AccessControlUtils.class);
/**
@@ -43,9 +48,28 @@ public class AccessControlUtils {
* @param endpointUrl the request url for which this access control is called
* @param accessControl AccessControl object which does the actual validation
*/
- public void validatePermission(String tableName, AccessType accessType,
HttpHeaders httpHeaders, String endpointUrl,
- AccessControl accessControl) {
- validatePermission(Optional.of(tableName), accessType, httpHeaders,
endpointUrl, accessControl);
+ public static void validatePermission(@Nullable String tableName, AccessType
accessType,
+ @Nullable HttpHeaders httpHeaders, @Nullable String endpointUrl,
AccessControl accessControl) {
+ String message = null;
+ try {
+ if (StringUtils.isBlank(tableName)) {
+ message = String.format("%s '%s'", accessType, endpointUrl);
+ if (!accessControl.hasAccess(accessType, httpHeaders, endpointUrl)) {
+ accessDenied(message);
+ }
+ } else {
+ message = String.format("%s '%s' for table '%s'", accessType,
endpointUrl, tableName);
+ String rawTableName = TableNameBuilder.extractRawTableName(tableName);
+ if (!accessControl.hasAccess(rawTableName, accessType, httpHeaders,
endpointUrl)) {
+ accessDenied(message);
+ }
+ }
+ } catch (ControllerApplicationException e) {
+ throw e;
+ } catch (Exception e) {
+ throw new ControllerApplicationException(LOGGER, "Caught exception while
validating permission for " + message,
+ Response.Status.INTERNAL_SERVER_ERROR, e);
+ }
}
/**
@@ -56,55 +80,26 @@ public class AccessControlUtils {
* @param endpointUrl the request url for which this access control is called
* @param accessControl AccessControl object which does the actual validation
*/
- public void validatePermission(AccessType accessType, HttpHeaders
httpHeaders, String endpointUrl,
- AccessControl accessControl) {
- validatePermission(Optional.empty(), accessType, httpHeaders, endpointUrl,
accessControl);
- }
-
- /**
- * Validate permission for the given access type against the given table
- *
- * @param tableNameOpt name of the table to be accessed; if `none`, it's a
non-table level endpoint.
- * @param accessType type of the access
- * @param httpHeaders HTTP headers containing requester identity required by
access control object
- * @param endpointUrl the request url for which this access control is called
- * @param accessControl AccessControl object which does the actual validation
- */
- public void validatePermission(Optional<String> tableNameOpt, AccessType
accessType, HttpHeaders httpHeaders,
- String endpointUrl, AccessControl accessControl) {
- boolean hasPermission;
- String accessTypeToEndpointMsg =
- String.format("access type '%s' to the endpoint '%s'", accessType,
endpointUrl) + tableNameOpt
- .map(name -> String.format(" for table '%s'", name)).orElse("");
- try {
- if (tableNameOpt.isPresent()) {
- String rawTableName =
TableNameBuilder.extractRawTableName(tableNameOpt.get());
- hasPermission = accessControl.hasAccess(rawTableName, accessType,
httpHeaders, endpointUrl);
- } else {
- hasPermission = accessControl.hasAccess(accessType, httpHeaders,
endpointUrl);
- }
- } catch (Exception e) {
- throw new ControllerApplicationException(LOGGER,
- "Caught exception while validating permission for " +
accessTypeToEndpointMsg,
- Response.Status.INTERNAL_SERVER_ERROR, e);
- }
- if (!hasPermission) {
- throw new ControllerApplicationException(LOGGER, "Permission is denied
for " + accessTypeToEndpointMsg,
- Response.Status.FORBIDDEN);
- }
+ public static void validatePermission(AccessType accessType, @Nullable
HttpHeaders httpHeaders,
+ @Nullable String endpointUrl, AccessControl accessControl) {
+ validatePermission(null, accessType, httpHeaders, endpointUrl,
accessControl);
}
/**
- * Validate permission for the given access type against the given table
+ * Validate permission for the given access type and endpointUrl
*
* @param httpHeaders HTTP headers containing requester identity required by
access control object
* @param endpointUrl the request url for which this access control is called
*/
- public void validatePermission(HttpHeaders httpHeaders, String endpointUrl,
+ public static void validatePermission(@Nullable HttpHeaders httpHeaders,
@Nullable String endpointUrl,
AccessControl accessControl) {
if (!accessControl.hasAccess(httpHeaders)) {
- throw new ControllerApplicationException(LOGGER, "Permission is denied
for " + endpointUrl,
- Response.Status.FORBIDDEN);
+ accessDenied(endpointUrl);
}
}
+
+ private static void accessDenied(String resource) {
+ throw new ControllerApplicationException(LOGGER, "Permission is denied for
" + resource,
+ Response.Status.FORBIDDEN);
+ }
}
diff --git
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/access/AuthenticationFilter.java
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/access/AuthenticationFilter.java
index b25dbdcc9b..8ebd1a2883 100644
---
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/access/AuthenticationFilter.java
+++
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/access/AuthenticationFilter.java
@@ -24,7 +24,6 @@ import java.io.IOException;
import java.lang.reflect.Method;
import java.util.Arrays;
import java.util.HashSet;
-import java.util.Optional;
import java.util.Set;
import javax.inject.Inject;
import javax.inject.Provider;
@@ -49,6 +48,9 @@ import org.glassfish.grizzly.http.server.Request;
public class AuthenticationFilter implements ContainerRequestFilter {
private static final Set<String> UNPROTECTED_PATHS =
new HashSet<>(Arrays.asList("", "help", "auth/info", "auth/verify",
"health"));
+ private static final String KEY_TABLE_NAME = "tableName";
+ private static final String KEY_TABLE_NAME_WITH_TYPE = "tableNameWithType";
+ private static final String KEY_SCHEMA_NAME = "schemaName";
@Inject
Provider<Request> _requestProvider;
@@ -86,49 +88,50 @@ public class AuthenticationFilter implements
ContainerRequestFilter {
// - "tableNameWithType", or
// - "schemaName"
// If table name is not available, it means the endpoint is not a
table-level endpoint.
- Optional<String> tableName = extractTableName(uriInfo.getPathParameters(),
uriInfo.getQueryParameters());
+ String tableName = extractTableName(uriInfo.getPathParameters(),
uriInfo.getQueryParameters());
AccessType accessType = extractAccessType(endpointMethod);
- new AccessControlUtils().validatePermission(tableName, accessType,
_httpHeaders, endpointUrl, accessControl);
+ AccessControlUtils.validatePermission(tableName, accessType, _httpHeaders,
endpointUrl, accessControl);
}
@VisibleForTesting
AccessType extractAccessType(Method endpointMethod) {
- // default access type
- AccessType accessType = AccessType.READ;
if (endpointMethod.isAnnotationPresent(Authenticate.class)) {
- accessType = endpointMethod.getAnnotation(Authenticate.class).value();
+ return endpointMethod.getAnnotation(Authenticate.class).value();
} else {
// heuristically infer access type via javax.ws.rs annotations
if (endpointMethod.getAnnotation(POST.class) != null) {
- accessType = AccessType.CREATE;
+ return AccessType.CREATE;
} else if (endpointMethod.getAnnotation(PUT.class) != null) {
- accessType = AccessType.UPDATE;
+ return AccessType.UPDATE;
} else if (endpointMethod.getAnnotation(DELETE.class) != null) {
- accessType = AccessType.DELETE;
+ return AccessType.DELETE;
}
}
- return accessType;
+
+ return AccessType.READ;
}
@VisibleForTesting
- Optional<String> extractTableName(MultivaluedMap<String, String>
pathParameters,
+ static String extractTableName(MultivaluedMap<String, String> pathParameters,
MultivaluedMap<String, String> queryParameters) {
- Optional<String> tableName = extractTableName(pathParameters);
- if (tableName.isPresent()) {
+ String tableName = extractTableName(pathParameters);
+ if (tableName != null) {
return tableName;
}
return extractTableName(queryParameters);
}
- private Optional<String> extractTableName(MultivaluedMap<String, String>
mmap) {
- String tableName = mmap.getFirst("tableName");
- if (tableName == null) {
- tableName = mmap.getFirst("tableNameWithType");
- if (tableName == null) {
- tableName = mmap.getFirst("schemaName");
- }
+ private static String extractTableName(MultivaluedMap<String, String> mmap) {
+ if (mmap.containsKey(KEY_TABLE_NAME)) {
+ return mmap.getFirst(KEY_TABLE_NAME);
+ }
+ if (mmap.containsKey(KEY_TABLE_NAME_WITH_TYPE)) {
+ return mmap.getFirst(KEY_TABLE_NAME_WITH_TYPE);
+ }
+ if (mmap.containsKey(KEY_SCHEMA_NAME)) {
+ return mmap.getFirst(KEY_SCHEMA_NAME);
}
- return Optional.ofNullable(tableName);
+ return null;
}
private static boolean isBaseFile(String path) {
diff --git
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotAccessControlUserRestletResource.java
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotAccessControlUserRestletResource.java
index c5e5c3a75b..b1d3b9a437 100644
---
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotAccessControlUserRestletResource.java
+++
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotAccessControlUserRestletResource.java
@@ -93,11 +93,8 @@ public class PinotAccessControlUserRestletResource {
@Inject
PinotHelixResourceManager _pinotHelixResourceManager;
-
@Inject
AccessControlFactory _accessControlFactory;
- AccessControlUtils _accessControlUtils = new AccessControlUtils();
-
@GET
@Produces(MediaType.APPLICATION_JSON)
@@ -106,8 +103,7 @@ public class PinotAccessControlUserRestletResource {
public String listUers(@Context HttpHeaders httpHeaders, @Context Request
request) {
try {
String endpointUrl = request.getRequestURL().toString();
- _accessControlUtils
- .validatePermission(httpHeaders, endpointUrl,
_accessControlFactory.create());
+ AccessControlUtils.validatePermission(httpHeaders, endpointUrl,
_accessControlFactory.create());
ZkHelixPropertyStore<ZNRecord> propertyStore =
_pinotHelixResourceManager.getPropertyStore();
Map<String, UserConfig> allUserInfo =
ZKMetadataProvider.getAllUserInfo(propertyStore);
return JsonUtils.newObjectNode().set("users",
JsonUtils.objectToJsonNode(allUserInfo)).toString();
@@ -124,8 +120,7 @@ public class PinotAccessControlUserRestletResource {
@Context HttpHeaders httpHeaders, @Context Request request) {
try {
String endpointUrl = request.getRequestURL().toString();
- _accessControlUtils
- .validatePermission(httpHeaders, endpointUrl,
_accessControlFactory.create());
+ AccessControlUtils.validatePermission(httpHeaders, endpointUrl,
_accessControlFactory.create());
ZkHelixPropertyStore<ZNRecord> propertyStore =
_pinotHelixResourceManager.getPropertyStore();
ComponentType componentType =
Constants.validateComponentType(componentTypeStr);
String usernameWithType = username + "_" + componentType.name();
@@ -150,8 +145,7 @@ public class PinotAccessControlUserRestletResource {
userConfig = JsonUtils.stringToObject(userConfigStr,
UserConfig.class);
username = userConfig.getUserName();
String endpointUrl = request.getRequestURL().toString();
- _accessControlUtils
- .validatePermission(httpHeaders, endpointUrl,
_accessControlFactory.create());
+ AccessControlUtils.validatePermission(httpHeaders, endpointUrl,
_accessControlFactory.create());
if (username.contains(".") || username.contains(" ")) {
throw new IllegalStateException("Username: " + username + "
containing '.' or space is not allowed");
}
@@ -189,7 +183,7 @@ public class PinotAccessControlUserRestletResource {
userExist = _pinotHelixResourceManager.hasUser(username,
componentTypeStr);
String endpointUrl = request.getRequestURL().toString();
- _accessControlUtils.validatePermission(httpHeaders, endpointUrl,
_accessControlFactory.create());
+ AccessControlUtils.validatePermission(httpHeaders, endpointUrl,
_accessControlFactory.create());
_pinotHelixResourceManager.deleteUser(usernameWithComponentType);
if (userExist) {
@@ -224,8 +218,7 @@ public class PinotAccessControlUserRestletResource {
String usernameWithComponentType = username + "_" + componentTypeStr;
try {
String endpointUrl = request.getRequestURL().toString();
- _accessControlUtils
- .validatePermission(httpHeaders, endpointUrl,
_accessControlFactory.create());
+ AccessControlUtils.validatePermission(httpHeaders, endpointUrl,
_accessControlFactory.create());
userConfig = JsonUtils.stringToObject(userConfigString,
UserConfig.class);
if (passwordChanged) {
diff --git
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSchemaRestletResource.java
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSchemaRestletResource.java
index bee1161465..2bb05d0553 100644
---
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSchemaRestletResource.java
+++
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSchemaRestletResource.java
@@ -96,7 +96,6 @@ public class PinotSchemaRestletResource {
@Inject
AccessControlFactory _accessControlFactory;
- AccessControlUtils _accessControlUtils = new AccessControlUtils();
@GET
@Produces(MediaType.APPLICATION_JSON)
@@ -219,7 +218,7 @@ public class PinotSchemaRestletResource {
Schema schema = schemaAndUnrecognizedProps.getLeft();
String endpointUrl = request.getRequestURL().toString();
validateSchemaName(schema.getSchemaName());
- _accessControlUtils.validatePermission(schema.getSchemaName(),
AccessType.CREATE, httpHeaders, endpointUrl,
+ AccessControlUtils.validatePermission(schema.getSchemaName(),
AccessType.CREATE, httpHeaders, endpointUrl,
_accessControlFactory.create());
SuccessResponse successResponse = addSchema(schema, override);
return new ConfigSuccessResponse(successResponse.getStatus(),
schemaAndUnrecognizedProps.getRight());
@@ -251,7 +250,7 @@ public class PinotSchemaRestletResource {
Schema schema = schemaAndUnrecognizedProperties.getLeft();
String endpointUrl = request.getRequestURL().toString();
validateSchemaName(schema.getSchemaName());
- _accessControlUtils.validatePermission(schema.getSchemaName(),
AccessType.CREATE, httpHeaders, endpointUrl,
+ AccessControlUtils.validatePermission(schema.getSchemaName(),
AccessType.CREATE, httpHeaders, endpointUrl,
_accessControlFactory.create());
SuccessResponse successResponse = addSchema(schema, override);
return new ConfigSuccessResponse(successResponse.getStatus(),
schemaAndUnrecognizedProperties.getRight());
diff --git
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java
index 3daa463b54..5d5030a4e6 100644
---
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java
+++
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java
@@ -152,7 +152,6 @@ public class PinotTableRestletResource {
@Inject
AccessControlFactory _accessControlFactory;
- AccessControlUtils _accessControlUtils = new AccessControlUtils();
@Inject
Executor _executor;
@@ -186,7 +185,7 @@ public class PinotTableRestletResource {
// validate permission
tableName = tableConfig.getTableName();
String endpointUrl = request.getRequestURL().toString();
- _accessControlUtils.validatePermission(tableName, AccessType.CREATE,
httpHeaders, endpointUrl,
+ AccessControlUtils.validatePermission(tableName, AccessType.CREATE,
httpHeaders, endpointUrl,
_accessControlFactory.create());
Schema schema =
_pinotHelixResourceManager.getSchemaForTableConfig(tableConfig);
@@ -372,7 +371,7 @@ public class PinotTableRestletResource {
// validate if user has permission to change the table state
String endpointUrl = request.getRequestURL().toString();
- _accessControlUtils
+ AccessControlUtils
.validatePermission(tableName, AccessType.UPDATE, httpHeaders,
endpointUrl, _accessControlFactory.create());
ArrayNode ret = JsonUtils.newArrayNode();
diff --git
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/TableConfigsRestletResource.java
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/TableConfigsRestletResource.java
index 75c5e930c7..d238756783 100644
---
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/TableConfigsRestletResource.java
+++
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/TableConfigsRestletResource.java
@@ -99,7 +99,6 @@ public class TableConfigsRestletResource {
@Inject
AccessControlFactory _accessControlFactory;
- AccessControlUtils _accessControlUtils = new AccessControlUtils();
/**
* List all {@link TableConfigs}, where each is a group of the offline table
config, realtime table config and
@@ -193,18 +192,18 @@ public class TableConfigsRestletResource {
try {
String endpointUrl = request.getRequestURL().toString();
AccessControl accessControl = _accessControlFactory.create();
- _accessControlUtils
+ AccessControlUtils
.validatePermission(schema.getSchemaName(), AccessType.CREATE,
httpHeaders, endpointUrl, accessControl);
if (offlineTableConfig != null) {
tuneConfig(offlineTableConfig, schema);
- _accessControlUtils
+ AccessControlUtils
.validatePermission(offlineTableConfig.getTableName(),
AccessType.CREATE, httpHeaders, endpointUrl,
accessControl);
}
if (realtimeTableConfig != null) {
tuneConfig(realtimeTableConfig, schema);
- _accessControlUtils
+ AccessControlUtils
.validatePermission(realtimeTableConfig.getTableName(),
AccessType.CREATE, httpHeaders, endpointUrl,
accessControl);
}
diff --git
a/pinot-controller/src/test/java/org/apache/pinot/controller/api/access/AuthenticationFilterTest.java
b/pinot-controller/src/test/java/org/apache/pinot/controller/api/access/AuthenticationFilterTest.java
index 45c8f8daa2..36ba895b79 100644
---
a/pinot-controller/src/test/java/org/apache/pinot/controller/api/access/AuthenticationFilterTest.java
+++
b/pinot-controller/src/test/java/org/apache/pinot/controller/api/access/AuthenticationFilterTest.java
@@ -20,7 +20,6 @@
package org.apache.pinot.controller.api.access;
import java.lang.reflect.Method;
-import java.util.Optional;
import javax.ws.rs.DELETE;
import javax.ws.rs.GET;
import javax.ws.rs.POST;
@@ -30,6 +29,7 @@ import javax.ws.rs.core.MultivaluedMap;
import org.testng.annotations.Test;
import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertNull;
public class AuthenticationFilterTest {
@@ -45,8 +45,7 @@ public class AuthenticationFilterTest {
queryParams.putSingle("tableName", "D");
queryParams.putSingle("tableNameWithType", "E");
queryParams.putSingle("schemaName", "F");
- Optional<String> actual = _authFilter.extractTableName(pathParams,
queryParams);
- assertEquals(actual, Optional.of("A"));
+ assertEquals(AuthenticationFilter.extractTableName(pathParams,
queryParams), "A");
}
@Test
@@ -58,8 +57,7 @@ public class AuthenticationFilterTest {
queryParams.putSingle("tableName", "D");
queryParams.putSingle("tableNameWithType", "E");
queryParams.putSingle("schemaName", "F");
- Optional<String> actual = _authFilter.extractTableName(pathParams,
queryParams);
- assertEquals(actual, Optional.of("B"));
+ assertEquals(AuthenticationFilter.extractTableName(pathParams,
queryParams), "B");
}
@Test
@@ -70,8 +68,7 @@ public class AuthenticationFilterTest {
queryParams.putSingle("tableName", "D");
queryParams.putSingle("tableNameWithType", "E");
queryParams.putSingle("schemaName", "F");
- Optional<String> actual = _authFilter.extractTableName(pathParams,
queryParams);
- assertEquals(actual, Optional.of("C"));
+ assertEquals(AuthenticationFilter.extractTableName(pathParams,
queryParams), "C");
}
@Test
@@ -81,8 +78,7 @@ public class AuthenticationFilterTest {
queryParams.putSingle("tableName", "D");
queryParams.putSingle("tableNameWithType", "E");
queryParams.putSingle("schemaName", "F");
- Optional<String> actual = _authFilter.extractTableName(pathParams,
queryParams);
- assertEquals(actual, Optional.of("D"));
+ assertEquals(AuthenticationFilter.extractTableName(pathParams,
queryParams), "D");
}
@Test
@@ -91,8 +87,7 @@ public class AuthenticationFilterTest {
MultivaluedMap<String, String> queryParams = new MultivaluedHashMap<>();
queryParams.putSingle("tableNameWithType", "E");
queryParams.putSingle("schemaName", "F");
- Optional<String> actual = _authFilter.extractTableName(pathParams,
queryParams);
- assertEquals(actual, Optional.of("E"));
+ assertEquals(AuthenticationFilter.extractTableName(pathParams,
queryParams), "E");
}
@Test
@@ -100,16 +95,14 @@ public class AuthenticationFilterTest {
MultivaluedMap<String, String> pathParams = new MultivaluedHashMap<>();
MultivaluedMap<String, String> queryParams = new MultivaluedHashMap<>();
queryParams.putSingle("schemaName", "F");
- Optional<String> actual = _authFilter.extractTableName(pathParams,
queryParams);
- assertEquals(actual, Optional.of("F"));
+ assertEquals(AuthenticationFilter.extractTableName(pathParams,
queryParams), "F");
}
@Test
public void testExtractTableNameWithEmptyParams() {
MultivaluedMap<String, String> pathParams = new MultivaluedHashMap<>();
MultivaluedMap<String, String> queryParams = new MultivaluedHashMap<>();
- Optional<String> actual = _authFilter.extractTableName(pathParams,
queryParams);
- assertEquals(actual, Optional.empty());
+ assertNull(AuthenticationFilter.extractTableName(pathParams, queryParams));
}
@Test
diff --git
a/pinot-tools/src/main/java/org/apache/pinot/tools/admin/command/AddTableCommand.java
b/pinot-tools/src/main/java/org/apache/pinot/tools/admin/command/AddTableCommand.java
index 8792f960f4..8eed222630 100644
---
a/pinot-tools/src/main/java/org/apache/pinot/tools/admin/command/AddTableCommand.java
+++
b/pinot-tools/src/main/java/org/apache/pinot/tools/admin/command/AddTableCommand.java
@@ -21,14 +21,16 @@ package org.apache.pinot.tools.admin.command;
import com.fasterxml.jackson.databind.JsonNode;
import java.io.File;
import java.io.IOException;
-import java.util.Collections;
-import org.apache.pinot.common.utils.FileUploadDownloadClient;
+import java.util.concurrent.Callable;
import org.apache.pinot.spi.auth.AuthProvider;
+import org.apache.pinot.spi.config.TableConfigs;
+import org.apache.pinot.spi.config.table.TableConfig;
import org.apache.pinot.spi.data.Schema;
import org.apache.pinot.spi.utils.CommonConstants;
import org.apache.pinot.spi.utils.JsonUtils;
import org.apache.pinot.spi.utils.NetUtils;
import org.apache.pinot.spi.utils.builder.ControllerRequestURLBuilder;
+import org.apache.pinot.spi.utils.builder.TableNameBuilder;
import org.apache.pinot.tools.Command;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -157,33 +159,11 @@ public class AddTableCommand extends
AbstractBaseAdminCommand implements Command
return this;
}
- public void uploadSchema()
- throws Exception {
- File schemaFile;
- Schema schema;
- try {
- schemaFile = new File(_schemaFile);
- schema = Schema.fromFile(schemaFile);
- } catch (Exception e) {
- LOGGER.error("Got exception while reading Pinot schema from file: [" +
_schemaFile + "]");
- throw e;
- }
- try (FileUploadDownloadClient fileUploadDownloadClient = new
FileUploadDownloadClient()) {
- fileUploadDownloadClient.addSchema(FileUploadDownloadClient
- .getUploadSchemaURI(_controllerProtocol, _controllerHost,
Integer.parseInt(_controllerPort)),
- schema.getSchemaName(), schemaFile,
makeAuthHeaders(makeAuthProvider(_authProvider, _authTokenUrl, _authToken,
- _user, _password)), Collections.emptyList());
- } catch (Exception e) {
- LOGGER.error("Got Exception to upload Pinot Schema: " +
schema.getSchemaName(), e);
- throw e;
- }
- }
-
public boolean sendTableCreationRequest(JsonNode node)
throws IOException {
- String res = AbstractBaseAdminCommand
- .sendRequest("POST",
ControllerRequestURLBuilder.baseUrl(_controllerAddress).forTableCreate(),
node.toString(),
- makeAuthHeaders(makeAuthProvider(_authProvider, _authTokenUrl,
_authToken, _user, _password)));
+ String res = AbstractBaseAdminCommand.sendRequest("POST",
+
ControllerRequestURLBuilder.baseUrl(_controllerAddress).forTableConfigsCreate(),
node.toString(),
+ makeAuthHeaders(makeAuthProvider(_authProvider, _authTokenUrl,
_authToken, _user, _password)));
LOGGER.info(res);
return res.contains("successfully added");
}
@@ -204,10 +184,24 @@ public class AddTableCommand extends
AbstractBaseAdminCommand implements Command
LOGGER.info("Executing command: " + toString());
- // Backward compatible
- if (_schemaFile != null) {
- uploadSchema();
+ TableConfig tableConfig = attempt(() -> JsonUtils.fileToObject(new
File(_tableConfigFile), TableConfig.class),
+ "Failed reading table config " + _tableConfigFile);
+
+ Schema schema = attempt(() -> JsonUtils.fileToObject(new
File(_schemaFile), Schema.class),
+ "Failed reading schema " + _schemaFile);
+
+ String tableName =
TableNameBuilder.extractRawTableName(tableConfig.getTableName());
+ TableConfigs tableConfigs = new TableConfigs(tableName, schema,
tableConfig, null);
+
+ return sendTableCreationRequest(JsonUtils.objectToJsonNode(tableConfigs));
+ }
+
+ private static <T> T attempt(Callable<T> callable, String errorMessage) {
+ try {
+ return callable.call();
+ } catch (Throwable t) {
+ LOGGER.error(errorMessage, t);
+ throw new IllegalStateException(t);
}
- return sendTableCreationRequest(JsonUtils.fileToJsonNode(new
File(_tableConfigFile)));
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
