xunliu commented on code in PR #3946: URL: https://github.com/apache/gravitino/pull/3946#discussion_r1671612858
########## core/src/main/java/com/datastrato/gravitino/connector/BaseCatalog.java: ########## @@ -57,6 +65,13 @@ public abstract class BaseCatalog<T extends BaseCatalog> // The object you used is not stable, don't use it unless you know what you are doing. @VisibleForTesting public static final String CATALOG_OPERATION_IMPL = "ops-impl"; + // Different catalogs may have different authorization provider, this variable is used as a + // key in properties of catalogs to inject custom authorization to Gravitino. + public static final String AUTHORIZATION_PROVIDER = "authorization-provider"; Review Comment: OK, I move `AUTHORIZATION_PROVIDER` into `Catalog.java`. ########## api/src/main/java/com/datastrato/gravitino/authorization/RoleChange.java: ########## @@ -0,0 +1,155 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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.datastrato.gravitino.authorization; + +import com.datastrato.gravitino.annotation.Evolving; + +/** The RoleChange interface defines the public API for managing roles in an authorization. */ +@Evolving +public interface RoleChange { + /** + * Create a RoleChange to Add a securable object into a role. Review Comment: Fixed. ########## core/src/main/java/com/datastrato/gravitino/connector/BaseCatalog.java: ########## @@ -169,12 +184,57 @@ public CatalogOperations ops() { return ops; } + public AuthorizationPlugin getAuthorizationPlugin() { + if (authorization == null) { + synchronized (this) { + if (authorization == null) { + authorization = createAuthorizationPluginInstance(); + } + } + } + return authorization.plugin(); + } + + private BaseAuthorization<?> createAuthorizationPluginInstance() { + String provider = entity.getProperties().get(AUTHORIZATION_PROVIDER); Review Comment: DONE. ########## api/src/main/java/com/datastrato/gravitino/Catalog.java: ########## @@ -88,6 +89,34 @@ enum CloudName { */ String CLOUD_REGION_CODE = "cloud.region-code"; + /** + * This variable is used as a key in properties of catalogs to use authorization plugin provider + * in Gravitino. <br> + * You can use your own authorized provider with this configuration The object you use may be + * unstable, so don't use it unless you know what you're doing. + */ + String AUTHORIZATION_PLUGIN = "authorization-plugin"; Review Comment: Improvement of these code comments. ########## core/src/main/java/com/datastrato/gravitino/connector/authorization/UserGroupAuthorizationPlugin.java: ########## @@ -0,0 +1,133 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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.datastrato.gravitino.connector.authorization; + +import com.datastrato.gravitino.authorization.Group; +import com.datastrato.gravitino.authorization.Role; +import com.datastrato.gravitino.authorization.User; +import java.util.List; + +/** + * Interface for authorization User and Group plugin operation of the underlying access control + * system. + */ +interface UserGroupAuthorizationPlugin { Review Comment: Sorry, I forgot the open access permission for the design document, Now anyone can access it. First, All permission systems have `User` and `Group` concept, So Gravitino need provider `User` and `Group` interface API to set it in the underlying ACL systems. ########## core/src/main/java/com/datastrato/gravitino/connector/authorization/UserGroupAuthorizationPlugin.java: ########## @@ -0,0 +1,133 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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.datastrato.gravitino.connector.authorization; + +import com.datastrato.gravitino.authorization.Group; +import com.datastrato.gravitino.authorization.Role; +import com.datastrato.gravitino.authorization.User; +import java.util.List; + +/** + * Interface for authorization User and Group plugin operation of the underlying access control + * system. + */ +interface UserGroupAuthorizationPlugin { + /** + * Add a new User to the underlying access control system. + * + * @param user The user entity. + * @return True if the add User was successfully added, false if the add User failed. + * @throws RuntimeException If adding the User encounters storage issues. + */ + Boolean onAddUser(User user) throws RuntimeException; + + /** + * Removes a User from the underlying access control system. + * + * @param user The name of the User. + * @return True if the User was successfully removed, false if the remove User failed. + * @throws RuntimeException If removing the User encounters storage issues. + */ + Boolean onRemoveUser(String user) throws RuntimeException; + + /** + * Check if a user exists from the underlying access control system. <br> + * Because User information is already stored in the Gravition, so we don't need to get the User Review Comment: hi @lmccay Sorry, I don't find Typo `Garvitino` in this PR. ########## core/src/main/java/com/datastrato/gravitino/connector/BaseCatalogPropertiesMetadata.java: ########## @@ -48,6 +48,13 @@ public abstract class BaseCatalogPropertiesMetadata extends BasePropertiesMetada null, false, false), + PropertyEntry.stringImmutablePropertyEntry( + BaseCatalog.AUTHORIZATION_PROVIDER, + "The classname of custom authorization provider", Review Comment: Fixed. ########## api/src/main/java/com/datastrato/gravitino/Catalog.java: ########## @@ -88,6 +89,34 @@ enum CloudName { */ String CLOUD_REGION_CODE = "cloud.region-code"; + /** + * This variable is used as a key in properties of catalogs to use authorization plugin provider + * in Gravitino. <br> + * You can use your own authorized provider with this configuration The object you use may be + * unstable, so don't use it unless you know what you're doing. + */ + String AUTHORIZATION_PLUGIN = "authorization-plugin"; + + /** Authorization plugin types */ + enum AuthorizationPluginType { + /** Ranger authorization plugin */ + RANGER("ranger"), + /** MySQL authorization plugin */ + MYSQL("mysql"); + /** Authorization plugin name */ + private final String pluginName; + + AuthorizationPluginType(String pluginName) { + Preconditions.checkArgument(pluginName != null, "plugin must not be null"); + this.pluginName = pluginName; + } + + /** @return The plugin name. */ + public String getPluginName() { + return pluginName; + } + } Review Comment: Fixed. ########## core/src/main/java/com/datastrato/gravitino/connector/BaseCatalog.java: ########## @@ -169,12 +180,60 @@ public CatalogOperations ops() { return ops; } + public AuthorizationPlugin getAuthorizationPlugin() { + if (authorization == null) { + synchronized (this) { + if (authorization == null) { + authorization = createAuthorizationPluginInstance(); + } + } + } + return authorization.plugin(); + } + + private BaseAuthorization<?> createAuthorizationPluginInstance() { + AuthorizationPluginType pluginType = + (AuthorizationPluginType) + catalogPropertiesMetadata().getOrDefault(conf, AUTHORIZATION_PLUGIN); + if (pluginType == null) { Review Comment: Fixed. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
