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/12 07:13:22 UTC
[GitHub] [ozone] smengcl opened a new pull request #2734: [WIP] HDDS-5776. [Multi-Tenant]
smengcl opened a new pull request #2734:
URL: https://github.com/apache/ozone/pull/2734
https://issues.apache.org/jira/browse/HDDS-5776
- [x] Implement `ListTenant`.
- [x] Implement `AssignTenantAdmin` and `RevokeTenantAdmin`. Ratis/OM DB side fully working. Authorizer(Ranger) part pending.
- [x] Add Ozone admin / tenant admin check in `AssignUser`.
- [x] Minor bug fixes (TBD)
- [ ] Implement `RevokeAccessID`.
--
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
[GitHub] [ozone] smengcl merged pull request #2734: HDDS-5776. [Multi-Tenant] Implement AssignTenantAdmin, RevokeTenantAdmin, ListTenant, RevokeAccessID
Posted by GitBox <gi...@apache.org>.
smengcl merged pull request #2734:
URL: https://github.com/apache/ozone/pull/2734
--
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
[GitHub] [ozone] smengcl commented on pull request #2734: HDDS-5776. [Multi-Tenant] Implement AssignTenantAdmin, RevokeTenantAdmin, ListTenant, RevokeAccessID
Posted by GitBox <gi...@apache.org>.
smengcl commented on pull request #2734:
URL: https://github.com/apache/ozone/pull/2734#issuecomment-954205975
The only failing test in the latest CI run is a known flaky test `TestSCMInstallSnapshotWithHA.testInstallOldCheckpointFailure `, as listed in HDDS-5626.
--
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
[GitHub] [ozone] prashantpogde commented on pull request #2734: HDDS-5776. [Multi-Tenant] Implement AssignTenantAdmin, RevokeTenantAdmin, ListTenant, RevokeAccessID
Posted by GitBox <gi...@apache.org>.
prashantpogde commented on pull request #2734:
URL: https://github.com/apache/ozone/pull/2734#issuecomment-951181250
Are the CI failures related ?
--
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
[GitHub] [ozone] smengcl commented on a change in pull request #2734: HDDS-5776. [Multi-Tenant] Implement AssignTenantAdmin, RevokeTenantAdmin, ListTenant, RevokeAccessID
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2734:
URL: https://github.com/apache/ozone/pull/2734#discussion_r734750387
##########
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:
Yep. Because the number of fields we are serializing in `OmDBAccessIdInfo` is fixed.
Or we could use Java reflection tricks. But this can have some performance impact in production because it is dynamically inspecting the object.
--
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
[GitHub] [ozone] smengcl commented on a change in pull request #2734: HDDS-5776. [Multi-Tenant] Implement AssignTenantAdmin, RevokeTenantAdmin, ListTenant, RevokeAccessID
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2734:
URL: https://github.com/apache/ozone/pull/2734#discussion_r734751100
##########
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:
This is not implemented now. Pending discussion. I feel like the current tenant logic depends on `VOLUME_LOCK` where it isn't necessary, for example in AssignUser Ratis request. If we have `TENANT_LOCK` we can have finer granularity lock than what we use currently.
--
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
[GitHub] [ozone] prashantpogde commented on a change in pull request #2734: HDDS-5776. [Multi-Tenant] Implement AssignTenantAdmin, RevokeTenantAdmin, ListTenant, RevokeAccessID
Posted by GitBox <gi...@apache.org>.
prashantpogde commented on a change in pull request #2734:
URL: https://github.com/apache/ozone/pull/2734#discussion_r735847635
##########
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:
we perhaps do not need to differentiate them as two different kind of admins. We can just one one type of tenant-admin who that have complete admin rights on the tenancy.
--
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
[GitHub] [ozone] prashantpogde commented on a change in pull request #2734: HDDS-5776. [Multi-Tenant] Implement AssignTenantAdmin, RevokeTenantAdmin, ListTenant, RevokeAccessID
Posted by GitBox <gi...@apache.org>.
prashantpogde commented on a change in pull request #2734:
URL: https://github.com/apache/ozone/pull/2734#discussion_r735846887
##########
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:
I guess its ok for now.
--
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
[GitHub] [ozone] smengcl commented on a change in pull request #2734: HDDS-5776. [Multi-Tenant] Implement AssignTenantAdmin, RevokeTenantAdmin, ListTenant, RevokeAccessID
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2734:
URL: https://github.com/apache/ozone/pull/2734#discussion_r734751100
##########
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:
This is not implemented now. Pending discussion. I feel like the current tenant logic depends on `VOLUME_LOCK` too much.
--
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
[GitHub] [ozone] smengcl edited a comment on pull request #2734: HDDS-5776. [Multi-Tenant] Implement AssignTenantAdmin, RevokeTenantAdmin, ListTenant, RevokeAccessID
Posted by GitBox <gi...@apache.org>.
smengcl edited a comment on pull request #2734:
URL: https://github.com/apache/ozone/pull/2734#issuecomment-954205975
The only failing test in the latest CI run is a known flaky test `TestSCMInstallSnapshotWithHA.testInstallOldCheckpointFailure`, as listed in HDDS-5626.
--
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
[GitHub] [ozone] smengcl commented on a change in pull request #2734: HDDS-5776. [Multi-Tenant] Implement AssignTenantAdmin, RevokeTenantAdmin, ListTenant, RevokeAccessID
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2734:
URL: https://github.com/apache/ozone/pull/2734#discussion_r734748741
##########
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:
Delegated tenant admins can make new admins in his tenant. While non-delegated admin can't.
This is the only difference right now.
--
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
[GitHub] [ozone] smengcl commented on a change in pull request #2734: HDDS-5776. [Multi-Tenant] Implement AssignTenantAdmin, RevokeTenantAdmin, ListTenant, RevokeAccessID
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2734:
URL: https://github.com/apache/ozone/pull/2734#discussion_r734755543
##########
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:
The delegated admin flag is added just in case one wants an admin that can't make new admins.
It's easy enough to implement here. But more difficult to add this later if we had not implemented it (because of the new field, logic change and so on).
--
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
[GitHub] [ozone] smengcl commented on a change in pull request #2734: HDDS-5776. [Multi-Tenant] Implement AssignTenantAdmin, RevokeTenantAdmin, ListTenant, RevokeAccessID
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2734:
URL: https://github.com/apache/ozone/pull/2734#discussion_r734751100
##########
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:
This is not implemented now. Pending discussion. I feel like the current tenant logic depends on `VOLUME_LOCK` where it isn't necessary, for example in AssignUser Ratis request.
--
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
[GitHub] [ozone] smengcl edited a comment on pull request #2734: HDDS-5776. [Multi-Tenant] Implement AssignTenantAdmin, RevokeTenantAdmin, ListTenant, RevokeAccessID
Posted by GitBox <gi...@apache.org>.
smengcl edited a comment on pull request #2734:
URL: https://github.com/apache/ozone/pull/2734#issuecomment-954205975
The only failing test in the latest CI run is a known flaky test `TestSCMInstallSnapshotWithHA.testInstallOldCheckpointFailure`, as listed in HDDS-5626.
All other checks passed.
--
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
[GitHub] [ozone] smengcl commented on pull request #2734: HDDS-5776. [Multi-Tenant] Implement AssignTenantAdmin, RevokeTenantAdmin, ListTenant, RevokeAccessID
Posted by GitBox <gi...@apache.org>.
smengcl commented on pull request #2734:
URL: https://github.com/apache/ozone/pull/2734#issuecomment-954265173
Thanks @prashantpogde . I will merge this shortly.
--
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
[GitHub] [ozone] smengcl commented on a change in pull request #2734: HDDS-5776. [Multi-Tenant] Implement AssignTenantAdmin, RevokeTenantAdmin, ListTenant, RevokeAccessID
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2734:
URL: https://github.com/apache/ozone/pull/2734#discussion_r734748080
##########
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:
Because I have implemented the delagated admin flag for tenant admins. By default new tenant admins are all delegated. But in some cases admin may want to assign non-delegated admins.
--
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
[GitHub] [ozone] prashantpogde commented on a change in pull request #2734: HDDS-5776. [Multi-Tenant] Implement AssignTenantAdmin, RevokeTenantAdmin, ListTenant, RevokeAccessID
Posted by GitBox <gi...@apache.org>.
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