Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#6536] improvement(authz): Create Ranger service if service is absent #6575

Merged
merged 17 commits into from
Mar 7, 2025
Merged

[#6536] improvement(authz): Create Ranger service if service is absent #6575

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
3cdb335
[#6536] improvement: Create Ranger service if service is absent
jerqi Feb 28, 2025
5b018ce
fix ut
jerqi Mar 3, 2025
52444df
address comment
jerqi Mar 3, 2025
9f0743f
Address comment
jerqi Mar 4, 2025
74b88fd
fix ut
jerqi Mar 4, 2025
d60cc87
add comment and revert change
jerqi Mar 4, 2025
69e34b2
remove the logic of deleting service
jerqi Mar 4, 2025
69e261f
Add a new assert
jerqi Mar 5, 2025
384e6e4
Merge branch 'main' into ISSUE-6536
jerqi Mar 5, 2025
531ec52
rename method
jerqi Mar 5, 2025
2fb0d96
optimize the usage
jerqi Mar 5, 2025
83a5f42
add ut
jerqi Mar 6, 2025
2fdc026
fix ut
jerqi Mar 6, 2025
bef8a8f
fix ut
jerqi Mar 6, 2025
2793f78
Address comment
jerqi Mar 6, 2025
2b07cef
address comments
jerqi Mar 6, 2025
bdbca22
address comment
jerqi Mar 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions ...in/src/main/java/org/apache/gravitino/authorization/chain/ChainedAuthorizationPlugin.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,16 @@ private void initPlugins(String catalogProvider, Map<String, String> properties)
Map<String, String> pluginConfig =
chainedAuthzProperties.fetchAuthPluginProperties(pluginName);

for (Map.Entry<String, String> entry : properties.entrySet()) {
if (!entry
.getKey()
.startsWith(ChainedAuthorizationProperties.CHAIN_PLUGINS_PROPERTIES_KEY)) {
pluginConfig.put(entry.getKey(), entry.getValue());
}
}
pluginConfig.put(
BaseAuthorization.UUID, pluginConfig.get(BaseAuthorization.UUID) + pluginName);

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you went pass hadoop.security.authentication properties to the plugin, you can use authorization.chain.hdfs-xxx.hadoop.security.authentication.
Another thing is, I found you added some new properties key in the ranger-hive-plugin and ranger-hdfs-plugin, please update relation doc. thanks.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

ArrayList<String> libAndResourcesPaths = Lists.newArrayList();
BaseAuthorization.buildAuthorizationPkgPath(
ImmutableMap.of(Catalog.AUTHORIZATION_PROVIDER, authzProvider))
Expand Down
17 changes: 11 additions & 6 deletions ...rc/main/java/org/apache/gravitino/authorization/common/RangerAuthorizationProperties.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ public class RangerAuthorizationProperties extends AuthorizationProperties {
*/
public static final String RANGER_PASSWORD = "authorization.ranger.password";

public static final String RANGER_SERVICE_CREATE_IF_ABSENT =
"authorization.ranger.service.create-if-absent";

public RangerAuthorizationProperties(Map<String, String> properties) {
super(properties);
}
Expand All @@ -63,9 +66,6 @@ public void validate() {
Preconditions.checkArgument(
properties.containsKey(RANGER_SERVICE_TYPE),
String.format("%s is required", RANGER_SERVICE_TYPE));
Preconditions.checkArgument(
properties.containsKey(RANGER_SERVICE_NAME),
String.format("%s is required", RANGER_SERVICE_NAME));
Preconditions.checkArgument(
properties.containsKey(RANGER_AUTH_TYPE),
String.format("%s is required", RANGER_AUTH_TYPE));
Expand All @@ -76,15 +76,20 @@ public void validate() {
Preconditions.checkArgument(
properties.get(RANGER_ADMIN_URL) != null,
String.format("%s is required", RANGER_ADMIN_URL));
Preconditions.checkArgument(
properties.get(RANGER_SERVICE_NAME) != null,
String.format("%s is required", RANGER_SERVICE_NAME));
Preconditions.checkArgument(
properties.get(RANGER_AUTH_TYPE) != null,
String.format("%s is required", RANGER_AUTH_TYPE));
Preconditions.checkArgument(
properties.get(RANGER_USERNAME) != null, String.format("%s is required", RANGER_USERNAME));
Preconditions.checkArgument(
properties.get(RANGER_PASSWORD) != null, String.format("%s is required", RANGER_PASSWORD));

if (!Boolean.parseBoolean(properties.get(RANGER_SERVICE_CREATE_IF_ABSENT))) {
Preconditions.checkArgument(
properties.get(RANGER_SERVICE_NAME) != null,
String.format(
"%s is required unless %s is specified",
RANGER_SERVICE_NAME, RANGER_SERVICE_CREATE_IF_ABSENT));
}
}
}
35 changes: 35 additions & 0 deletions ...rc/main/java/org/apache/gravitino/authorization/ranger/RangerAuthorizationHDFSPlugin.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,17 @@
public class RangerAuthorizationHDFSPlugin extends RangerAuthorizationPlugin {
private static final Logger LOG = LoggerFactory.getLogger(RangerAuthorizationHDFSPlugin.class);
private static final Pattern HDFS_PATTERN = Pattern.compile("^hdfs://[^/]*");
public static final String USERNAME = "username";
public static final String DEFAULT_USERNAME = "admin";
public static final String PASSWORD = "password";
public static final String DEFAULT_PASSWORD = "admin";
public static final String HADOOP_SECURITY_AUTHENTICATION = "hadoop.security.authentication";
public static final String DEFAULT_HADOOP_SECURITY_AUTHENTICATION = "simple";
public static final String HADOOP_RPC_PROTECTION = "hadoop.rpc.protection";
public static final String DEFAULT_HADOOP_RPC_PROTECTION = "authentication";
public static final String HADOOP_SECURITY_AUTHORIZATION = "hadoop.security.authorization";
public static final String FS_DEFAULT_NAME = "fs.default.name";
public static final String FS_DEFAULT_VALUE = "hdfs://127.0.0.1:8090";

public RangerAuthorizationHDFSPlugin(String metalake, Map<String, String> config) {
super(metalake, config);
Expand Down Expand Up @@ -679,4 +690,28 @@ public Boolean onMetadataUpdated(MetadataObjectChange... changes) throws Runtime
}
return Boolean.TRUE;
}

@Override
protected String getServiceType() {
return HDFS_SERVICE_TYPE;
}

@Override
protected Map<String, String> getServiceConfigs(Map<String, String> config) {
return ImmutableMap.<String, String>builder()
.put(USERNAME, getConfValue(config, USERNAME, DEFAULT_USERNAME))
.put(PASSWORD, getConfValue(config, PASSWORD, DEFAULT_PASSWORD))
.put(
HADOOP_SECURITY_AUTHENTICATION,
getConfValue(
config, HADOOP_SECURITY_AUTHENTICATION, DEFAULT_HADOOP_SECURITY_AUTHENTICATION))
.put(
HADOOP_RPC_PROTECTION,
getConfValue(config, HADOOP_RPC_PROTECTION, DEFAULT_HADOOP_RPC_PROTECTION))
.put(
HADOOP_SECURITY_AUTHORIZATION,
getConfValue(config, HADOOP_SECURITY_AUTHORIZATION, "false"))
.put(FS_DEFAULT_NAME, getConfValue(config, FS_DEFAULT_NAME, FS_DEFAULT_VALUE))
.build();
}
}
25 changes: 25 additions & 0 deletions ...in/java/org/apache/gravitino/authorization/ranger/RangerAuthorizationHadoopSQLPlugin.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,16 @@
import org.slf4j.LoggerFactory;

public class RangerAuthorizationHadoopSQLPlugin extends RangerAuthorizationPlugin {
public static final String JDBC_URL = "jdbc.url";
private static final Logger LOG =
LoggerFactory.getLogger(RangerAuthorizationHadoopSQLPlugin.class);
public static final String USERNAME = "username";
public static final String DEFAULT_USERNAME = "admin";
public static final String PASSWORD = "password";
public static final String DEFAULT_PASSWORD = "admin";
public static final String JDBC_DRIVER_CLASS_NAME = "jdbc.driverClassName";
public static final String DEFAULT_JDBC_DRIVER_CLASS_NAME = "org.apache.hive.jdbc.HiveDriver";
public static final String DEFAULT_JDBC_URL = "jdbc:hive2://127.0.0.1:8081";

public RangerAuthorizationHadoopSQLPlugin(String metalake, Map<String, String> config) {
super(metalake, config);
Expand Down Expand Up @@ -800,4 +808,21 @@ public Boolean onMetadataUpdated(MetadataObjectChange... changes) throws Runtime
}
return Boolean.TRUE;
}

@Override
protected String getServiceType() {
return HADOOP_SQL_SERVICE_TYPE;
}

@Override
protected Map<String, String> getServiceConfigs(Map<String, String> config) {
return ImmutableMap.<String, String>builder()
.put(USERNAME, getConfValue(config, USERNAME, DEFAULT_USERNAME))
.put(PASSWORD, getConfValue(config, PASSWORD, DEFAULT_PASSWORD))
.put(
JDBC_DRIVER_CLASS_NAME,
getConfValue(config, JDBC_DRIVER_CLASS_NAME, DEFAULT_JDBC_DRIVER_CLASS_NAME))
.put(JDBC_URL, DEFAULT_JDBC_URL)
.build();
}
}
179 changes: 123 additions & 56 deletions ...er/src/main/java/org/apache/gravitino/authorization/ranger/RangerAuthorizationPlugin.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableMap;
import com.sun.jersey.api.client.ClientResponse;
import java.io.IOException;
import java.time.Instant;
import java.util.Arrays;
Expand Down Expand Up @@ -48,14 +49,17 @@
import org.apache.gravitino.authorization.ranger.reference.VXGroupList;
import org.apache.gravitino.authorization.ranger.reference.VXUser;
import org.apache.gravitino.authorization.ranger.reference.VXUserList;
import org.apache.gravitino.connector.BaseCatalog;
import org.apache.gravitino.connector.authorization.AuthorizationPlugin;
import org.apache.gravitino.connector.authorization.BaseAuthorization;
import org.apache.gravitino.exceptions.AuthorizationPluginException;
import org.apache.gravitino.meta.AuditInfo;
import org.apache.gravitino.meta.GroupEntity;
import org.apache.gravitino.meta.UserEntity;
import org.apache.gravitino.utils.PrincipalUtils;
import org.apache.ranger.RangerServiceException;
import org.apache.ranger.plugin.model.RangerPolicy;
import org.apache.ranger.plugin.model.RangerService;
import org.apache.ranger.plugin.util.GrantRevokeRoleRequest;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand All @@ -74,12 +78,15 @@
public abstract class RangerAuthorizationPlugin
implements AuthorizationPlugin, AuthorizationPrivilegesMappingProvider {
private static final Logger LOG = LoggerFactory.getLogger(RangerAuthorizationPlugin.class);
protected static final String HDFS_SERVICE_TYPE = "hdfs";
protected static final String HADOOP_SQL_SERVICE_TYPE = "hive";

protected String metalake;
protected final String rangerServiceName;
protected final RangerClientExtension rangerClient;
protected final RangerHelper rangerHelper;
@VisibleForTesting public final String rangerAdminName;
private boolean isCreatedByPlugin = false;

protected RangerAuthorizationPlugin(String metalake, Map<String, String> config) {
this.metalake = metalake;
Expand All @@ -91,9 +98,20 @@ protected RangerAuthorizationPlugin(String metalake, Map<String, String> config)
rangerAdminName = config.get(RangerAuthorizationProperties.RANGER_USERNAME);
// Apache Ranger Password should be minimum 8 characters with min one alphabet and one numeric.
String password = config.get(RangerAuthorizationProperties.RANGER_PASSWORD);
rangerServiceName = config.get(RangerAuthorizationProperties.RANGER_SERVICE_NAME);

String serviceName = config.get(RangerAuthorizationProperties.RANGER_SERVICE_NAME);
rangerClient = new RangerClientExtension(rangerUrl, authType, rangerAdminName, password);

if (Boolean.parseBoolean(
config.get(RangerAuthorizationProperties.RANGER_SERVICE_CREATE_IF_ABSENT))) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How can we be so sure that this property is there?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Boolean.parseBoolean can handle null value. Null value equals false.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please make sure there won't be uncaught exceptions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can see the code

    public static boolean parseBoolean(String s) {
        return ((s != null) && s.equalsIgnoreCase("true"));
    }

if (serviceName == null) {
serviceName = config.get(BaseAuthorization.UUID);
}

createRangerServiceIfNecessary(config, serviceName);
}

rangerServiceName = serviceName;
rangerHelper =
new RangerHelper(
rangerClient,
Expand Down Expand Up @@ -745,6 +763,110 @@ public Boolean onGroupAcquired(Group group) {
return Boolean.TRUE;
}

@Override
public void close() throws IOException {
if (!isCreatedByPlugin) {
return;
}

try {
rangerClient.deleteService(rangerServiceName);
} catch (RangerServiceException rse) {
throw new AuthorizationPluginException(
"Fail to delete Ranger service %s, exception: %s", rangerServiceName, rse.getMessage());
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need the automated delete Reanger service?
Is the RangerAuthorizationProperties.RANGER_SERVICE_CREATE_IF_ABSENT only used by ITs?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. It's not only used by UT.
If we have a Ranger server and Ranger HDFS or Hive services are not pre-created , we need the to create the by our Gravitino.
If these Ranger Hive services or HDFS services are created by Gravitino, Gravitino should delete them, too.

}

/** Generate authorization securable object */
public abstract AuthorizationSecurableObject generateAuthorizationSecurableObject(
List<String> names,
String path,
AuthorizationMetadataObject.Type type,
Set<AuthorizationPrivilege> privileges);

public boolean validAuthorizationOperation(List<SecurableObject> securableObjects) {
return securableObjects.stream()
.allMatch(
securableObject -> {
AtomicBoolean match = new AtomicBoolean(true);
securableObject.privileges().stream()
.forEach(
privilege -> {
if (!privilege.canBindTo(securableObject.type())) {
LOG.error(
"The privilege({}) is not supported for the metadata object({})!",
privilege.name(),
securableObject.fullName());
match.set(false);
}
});
return match.get();
});
}

/**
* IF rename the SCHEMA, Need to rename these the relevant policies, `{schema}`, `{schema}.*`,
* `{schema}.*.*` <br>
* IF rename the TABLE, Need to rename these the relevant policies, `{schema}.*`, `{schema}.*.*`
* <br>
* IF rename the COLUMN, Only need to rename `{schema}.*.*` <br>
*/
protected abstract void renameMetadataObject(
AuthorizationMetadataObject authzMetadataObject,
AuthorizationMetadataObject newAuthzMetadataObject);

protected abstract void removeMetadataObject(AuthorizationMetadataObject authzMetadataObject);

/**
* Remove the policy by the metadata object names. <br>
*
* @param authzMetadataObject The authorization metadata object.
*/
protected void removePolicyByMetadataObject(AuthorizationMetadataObject authzMetadataObject) {
RangerPolicy policy = findManagedPolicy(authzMetadataObject);
if (policy != null) {
rangerHelper.removeAllGravitinoManagedPolicyItem(policy);
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here it is because of the changes brought about by moving the code, which makes it more difficult to review, and it is better to be able to fall back.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. I just put public methods together, protected methods together, private methods together. I can revert this change.


protected String getConfValue(Map<String, String> conf, String key, String defaultValue) {
if (conf.containsKey(BaseCatalog.CATALOG_BYPASS_PREFIX + key)) {
return conf.get(BaseCatalog.CATALOG_BYPASS_PREFIX + key);
}
return defaultValue;
}

protected abstract String getServiceType();

protected abstract Map<String, String> getServiceConfigs(Map<String, String> config);

private void createRangerServiceIfNecessary(Map<String, String> config, String serviceName) {
try {
rangerClient.getService(serviceName);
} catch (RangerServiceException rse) {
if (rse.getStatus().equals(ClientResponse.Status.NOT_FOUND)) {
try {
RangerService rangerService = new RangerService();
rangerService.setType(getServiceType());
rangerService.setName(serviceName);
rangerService.setConfigs(getServiceConfigs(config));
rangerClient.createService(rangerService);
List<RangerPolicy> policies = rangerClient.getPoliciesInService(serviceName);
for (RangerPolicy policy : policies) {
rangerClient.deletePolicy(policy.getId());
}
Comment on lines +798 to +801
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a description of why the policy should be deleted

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. I can add the comment.

isCreatedByPlugin = true;
} catch (RangerServiceException crse) {
throw new AuthorizationPluginException(
"Fail to create ranger service %s, exception: %s", serviceName, crse.getMessage());
}
} else {
throw new AuthorizationPluginException(
"Fail to get ranger service name %s, exception: %s", serviceName, rse.getMessage());
}
}
}

/**
* Add the securable object's privilege to the Ranger policy. <br>
* 1. Find the policy base the metadata object. <br>
Expand Down Expand Up @@ -909,59 +1031,4 @@ private void removePolicyItemIfEqualRoleName(
policyItem.getRoles().removeIf(roleName::equals);
}
}

/**
* IF rename the SCHEMA, Need to rename these the relevant policies, `{schema}`, `{schema}.*`,
* `{schema}.*.*` <br>
* IF rename the TABLE, Need to rename these the relevant policies, `{schema}.*`, `{schema}.*.*`
* <br>
* IF rename the COLUMN, Only need to rename `{schema}.*.*` <br>
*/
protected abstract void renameMetadataObject(
AuthorizationMetadataObject authzMetadataObject,
AuthorizationMetadataObject newAuthzMetadataObject);

protected abstract void removeMetadataObject(AuthorizationMetadataObject authzMetadataObject);

/**
* Remove the policy by the metadata object names. <br>
*
* @param authzMetadataObject The authorization metadata object.
*/
protected void removePolicyByMetadataObject(AuthorizationMetadataObject authzMetadataObject) {
RangerPolicy policy = findManagedPolicy(authzMetadataObject);
if (policy != null) {
rangerHelper.removeAllGravitinoManagedPolicyItem(policy);
}
}

@Override
public void close() throws IOException {}

/** Generate authorization securable object */
public abstract AuthorizationSecurableObject generateAuthorizationSecurableObject(
List<String> names,
String path,
AuthorizationMetadataObject.Type type,
Set<AuthorizationPrivilege> privileges);

public boolean validAuthorizationOperation(List<SecurableObject> securableObjects) {
return securableObjects.stream()
.allMatch(
securableObject -> {
AtomicBoolean match = new AtomicBoolean(true);
securableObject.privileges().stream()
.forEach(
privilege -> {
if (!privilege.canBindTo(securableObject.type())) {
LOG.error(
"The privilege({}) is not supported for the metadata object({})!",
privilege.name(),
securableObject.fullName());
match.set(false);
}
});
return match.get();
});
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here it is because of the changes brought about by moving the code, which makes it more difficult to review, and it is better to be able to fall back.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can revert this change.

}
Loading