This is an automated email from the ASF dual-hosted git repository. apucher pushed a commit to branch add-table-auth-cleanup in repository https://gitbox.apache.org/repos/asf/pinot.git
commit 1bf9931a534913b6a137040d032a4e034335ce50 Author: Alexander Pucher <apuc...@apache.org> AuthorDate: Tue Aug 16 21:31:53 2022 -0700 add-table refactor for clean auth --- .../controller/api/access/AccessControlUtils.java | 79 ++++++++++------------ .../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 | 52 +++++++------- 8 files changed, 103 insertions(+), 130 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..88d71736ba 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,9 @@ package org.apache.pinot.controller.api.access; -import java.util.Optional; 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 +31,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 +47,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(String tableName, AccessType accessType, HttpHeaders httpHeaders, + String endpointUrl, AccessControl accessControl) { + String accessTypeToEndpointMsg = + String.format("access type '%s' to the endpoint '%s' for table '%s'", accessType, endpointUrl, tableName); + try { + if (StringUtils.isBlank(tableName)) { + if (!accessControl.hasAccess(accessType, httpHeaders, endpointUrl)) { + accessDenied(accessTypeToEndpointMsg); + } + } else { + String rawTableName = TableNameBuilder.extractRawTableName(tableName); + if (!accessControl.hasAccess(rawTableName, accessType, httpHeaders, endpointUrl)) { + accessDenied(accessTypeToEndpointMsg); + } + } + } catch (Exception e) { + if (!(e instanceof ControllerApplicationException)) { + throw new ControllerApplicationException(LOGGER, + "Caught exception while validating permission for " + accessTypeToEndpointMsg, + Response.Status.INTERNAL_SERVER_ERROR, e); + } + } } /** @@ -56,55 +79,25 @@ 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, + public static void validatePermission(AccessType accessType, HttpHeaders httpHeaders, String endpointUrl, AccessControl accessControl) { - validatePermission(Optional.empty(), accessType, httpHeaders, endpointUrl, accessControl); + validatePermission(null, 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(HttpHeaders httpHeaders, String endpointUrl, AccessControl accessControl) { + if (!accessControl.hasAccess(httpHeaders)) { + accessDenied(endpointUrl); } } - /** - * Validate permission for the given access type against the given table - * - * @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, - AccessControl accessControl) { - if (!accessControl.hasAccess(httpHeaders)) { - throw new ControllerApplicationException(LOGGER, "Permission is denied for " + endpointUrl, - Response.Status.FORBIDDEN); - } + 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..dba0a87180 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,32 +159,10 @@ 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(), + .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: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org