You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2021/10/22 18:05:27 UTC
[GitHub] [ozone] prashantpogde commented on a change in pull request #2734: HDDS-5776. [Multi-Tenant] Implement AssignTenantAdmin, RevokeTenantAdmin, ListTenant, RevokeAccessID
prashantpogde commented on a change in pull request #2734:
URL: https://github.com/apache/ozone/pull/2734#discussion_r734011436
##########
File path: hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmDBAccessIdInfo.java
##########
@@ -33,39 +33,60 @@
*/
private final String kerberosPrincipal;
/**
- * Shared secret of the accessId. TODO: Encryption?
+ * Shared secret of the accessId.
*/
private final String sharedSecret;
+ /**
+ * Whether this accessId is an administrator of the tenant.
+ */
+ private final boolean isAdmin;
+ /**
+ * Whether this accessId is a delegated admin of the tenant.
+ * Only effective if isAdmin is true.
+ */
+ private final boolean isDelegatedAdmin;
// This implies above String fields should NOT contain the split key.
public static final String SERIALIZATION_SPLIT_KEY = ";";
public OmDBAccessIdInfo(String tenantId,
- String kerberosPrincipal, String sharedSecret) {
+ String kerberosPrincipal, String sharedSecret,
+ boolean isAdmin, boolean isDelegatedAdmin) {
this.tenantId = tenantId;
this.kerberosPrincipal = kerberosPrincipal;
this.sharedSecret = sharedSecret;
+ this.isAdmin = isAdmin;
+ this.isDelegatedAdmin = isDelegatedAdmin;
}
private OmDBAccessIdInfo(String accessIdInfoString) {
String[] tInfo = accessIdInfoString.split(SERIALIZATION_SPLIT_KEY);
- Preconditions.checkState(tInfo.length == 3,
+ Preconditions.checkState(tInfo.length == 3 || tInfo.length == 5,
Review comment:
hard coded ?
##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/ObjectStore.java
##########
@@ -196,18 +197,49 @@ public void createTenant(String tenantName) throws IOException {
// proxy.createTenant(tenantName, tenantArgs);
// }
- // TODO: modify, delete
-
/**
- * Assign user to tenant.
+ * Assign user accessId to tenant.
* @param username user name to be assigned.
* @param tenantName tenant name.
- * @param accessId access ID.
+ * @param accessId Specified accessId.
* @throws IOException
*/
- public S3SecretValue assignUserToTenant(
+ // TODO: Rename this to tenantAssignUserAccessId ?
+ public S3SecretValue tenantAssignUserAccessId(
String username, String tenantName, String accessId) throws IOException {
- return proxy.assignUserToTenant(username, tenantName, accessId);
+ return proxy.tenantAssignUserAccessId(username, tenantName, accessId);
+ }
+
+ /**
+ * Revoke user accessId to tenant.
+ * @param accessId accessId to be revoked.
+ * @throws IOException
+ */
+ public void tenantRevokeUserAccessId(String accessId) throws IOException {
+ proxy.tenantRevokeUserAccessId(accessId);
+ }
+
+ /**
+ * Assign admin role to an accessId in a tenant.
+ * @param accessId access ID.
+ * @param tenantName tenant name.
+ * @param delegated true if making delegated admin.
Review comment:
Why do we need this parameter "delegate"?
##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/protocol/ClientProtocol.java
##########
@@ -588,9 +589,34 @@ void cancelDelegationToken(Token<OzoneTokenIdentifier> token)
* @param accessId access ID.
* @throws IOException
*/
- S3SecretValue assignUserToTenant(String username, String tenantName,
+ S3SecretValue tenantAssignUserAccessId(String username, String tenantName,
String accessId) throws IOException;
+ /**
+ * Revoke user accessId to tenant.
+ * @param accessId accessId to be revoked.
+ * @throws IOException
+ */
+ void tenantRevokeUserAccessId(String accessId) throws IOException;
+
+ /**
+ * Assign admin role to an accessId in a tenant.
+ * @param accessId access ID.
+ * @param tenantName tenant name.
+ * @param delegated true if making delegated admin.
Review comment:
same comment as above, the API already conveys that this accessId shud be made tenant-admin.
##########
File path: hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/multitenant/MultiTenantAccessAuthorizer.java
##########
@@ -53,30 +53,32 @@
void shutdown() throws Exception;
/**
- * Create User Principal entity for MultiTenantGatekeeper plugin.
- * @param principal
- * @param groupIDs : groupIDs that this user will belong to
+ * Assign user to an existing role in the Authorizer.
+ * @param principal User principal
+ * @param existingRole A JSON String representation of the existing role
+ * returned from the Authorizer (Ranger).
+ * @param isAdmin
* @return unique and opaque userID that can be used to refer to the user in
* MultiTenantGateKeeperplugin Implementation. E.g. a Ranger
* based Implementation can return some ID thats relevant for it.
*/
- String createUser(BasicUserPrincipal principal,
- List<String> groupIDs) throws Exception;
+ String assignUser(BasicUserPrincipal principal, String existingRole,
Review comment:
nit : class OzoneTenantRolePrincipal{} still uses "groupName" string instead of "roleName".
##########
File path: hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/multitenant/CephCompatibleTenantImpl.java
##########
@@ -83,7 +83,7 @@ public void removeTenantAccessPolicy(AccessPolicy policy) {
}
@Override
- public void addTenantAccessGroup(String groupID) {
+ public void addTenantAccessRole(String groupID) {
Review comment:
:s/groupId/roleId/g
##########
File path: hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/lock/OzoneManagerLock.java
##########
@@ -390,6 +390,7 @@ private void unlock(Resource resource, String resourceName,
S3_SECRET_LOCK((byte) 4, "S3_SECRET_LOCK"), // 31
PREFIX_LOCK((byte) 5, "PREFIX_LOCK"); //63
+// TENANT_LOCK((byte) 6, "TENANT_LOCK"); // 127
Review comment:
are we planning to use this ?
##########
File path: hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmDBAccessIdInfo.java
##########
@@ -33,39 +33,60 @@
*/
private final String kerberosPrincipal;
/**
- * Shared secret of the accessId. TODO: Encryption?
+ * Shared secret of the accessId.
*/
private final String sharedSecret;
+ /**
+ * Whether this accessId is an administrator of the tenant.
+ */
+ private final boolean isAdmin;
+ /**
+ * Whether this accessId is a delegated admin of the tenant.
+ * Only effective if isAdmin is true.
+ */
+ private final boolean isDelegatedAdmin;
// This implies above String fields should NOT contain the split key.
public static final String SERIALIZATION_SPLIT_KEY = ";";
public OmDBAccessIdInfo(String tenantId,
- String kerberosPrincipal, String sharedSecret) {
+ String kerberosPrincipal, String sharedSecret,
+ boolean isAdmin, boolean isDelegatedAdmin) {
this.tenantId = tenantId;
this.kerberosPrincipal = kerberosPrincipal;
this.sharedSecret = sharedSecret;
+ this.isAdmin = isAdmin;
+ this.isDelegatedAdmin = isDelegatedAdmin;
}
private OmDBAccessIdInfo(String accessIdInfoString) {
String[] tInfo = accessIdInfoString.split(SERIALIZATION_SPLIT_KEY);
- Preconditions.checkState(tInfo.length == 3,
+ Preconditions.checkState(tInfo.length == 3 || tInfo.length == 5,
"Incorrect accessIdInfoString");
tenantId = tInfo[0];
kerberosPrincipal = tInfo[1];
sharedSecret = tInfo[2];
+ if (tInfo.length == 5) {
+ isAdmin = Boolean.parseBoolean(tInfo[3]);
+ isDelegatedAdmin = Boolean.parseBoolean(tInfo[4]);
+ } else {
+ isAdmin = false;
+ isDelegatedAdmin = false;
+ }
}
public String getTenantId() {
return tenantId;
}
private String serialize() {
- StringBuilder sb = new StringBuilder();
- sb.append(tenantId).append(SERIALIZATION_SPLIT_KEY);
- sb.append(kerberosPrincipal).append(SERIALIZATION_SPLIT_KEY);
- sb.append(sharedSecret);
+ final StringBuilder sb = new StringBuilder();
+ sb.append(tenantId);
+ sb.append(SERIALIZATION_SPLIT_KEY).append(kerberosPrincipal);
+ sb.append(SERIALIZATION_SPLIT_KEY).append(sharedSecret);
+ sb.append(SERIALIZATION_SPLIT_KEY).append(isAdmin);
+ sb.append(SERIALIZATION_SPLIT_KEY).append(isDelegatedAdmin);
Review comment:
do we need this ? Same comment for all other similar places.
##########
File path: hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmDBAccessIdInfo.java
##########
@@ -33,39 +33,60 @@
*/
private final String kerberosPrincipal;
/**
- * Shared secret of the accessId. TODO: Encryption?
+ * Shared secret of the accessId.
*/
private final String sharedSecret;
+ /**
+ * Whether this accessId is an administrator of the tenant.
+ */
+ private final boolean isAdmin;
+ /**
+ * Whether this accessId is a delegated admin of the tenant.
+ * Only effective if isAdmin is true.
+ */
+ private final boolean isDelegatedAdmin;
// This implies above String fields should NOT contain the split key.
public static final String SERIALIZATION_SPLIT_KEY = ";";
public OmDBAccessIdInfo(String tenantId,
- String kerberosPrincipal, String sharedSecret) {
+ String kerberosPrincipal, String sharedSecret,
+ boolean isAdmin, boolean isDelegatedAdmin) {
this.tenantId = tenantId;
this.kerberosPrincipal = kerberosPrincipal;
this.sharedSecret = sharedSecret;
+ this.isAdmin = isAdmin;
+ this.isDelegatedAdmin = isDelegatedAdmin;
Review comment:
How is delegatedAdmin different from isAdmin ?
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org