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/09/13 17:23:02 UTC
[GitHub] [ozone] avijayanhwx opened a new pull request #2635: HDDS-5476. Support Ozone s3 authentication with arbitrary accessId that is not same as the kerberos ID
avijayanhwx opened a new pull request #2635:
URL: https://github.com/apache/ozone/pull/2635
## What changes were proposed in this pull request?
As part of the feature to support multi tenancy, we allow a tenant admin to assign a user to a tenant with an optional flag to specify a different accessId than the kerberos user name. To support this, changes are needed Token based authentication between S3 and Ozone Manager.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-5476
## How was this patch tested?
Manually tested on top of #2564.
--
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] avijayanhwx commented on a change in pull request #2635: HDDS-5476. [Multi-Tenant] Support Ozone s3 authentication with arbitrary accessId that is not same as the kerberos ID
Posted by GitBox <gi...@apache.org>.
avijayanhwx commented on a change in pull request #2635:
URL: https://github.com/apache/ozone/pull/2635#discussion_r712320690
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OMMultiTenantManagerImpl.java
##########
@@ -332,7 +335,7 @@ public String assignUserToTenant(String tenantName, String userName) {
inMemoryUserNameToListOfGroupsMap.put(
userPrincipal.getFullMultiTenantPrincipalID(), userGroupIDs);
- return userID;
+ return null;
Review comment:
DEBUG change, removed.
--
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 #2635: HDDS-5476. [Multi-Tenant] Support Ozone s3 authentication with arbitrary accessId that is not same as the kerberos ID
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2635:
URL: https://github.com/apache/ozone/pull/2635#discussion_r710473479
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OMMultiTenantManagerImpl.java
##########
@@ -332,7 +335,7 @@ public String assignUserToTenant(String tenantName, String userName) {
inMemoryUserNameToListOfGroupsMap.put(
userPrincipal.getFullMultiTenantPrincipalID(), userGroupIDs);
- return userID;
+ return null;
Review comment:
Is this intentional, or for testing? Maybe put a TODO here?
--
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] avijayanhwx merged pull request #2635: HDDS-5476. [Multi-Tenant] Support Ozone s3 authentication with arbitrary accessId that is not same as the kerberos ID
Posted by GitBox <gi...@apache.org>.
avijayanhwx merged pull request #2635:
URL: https://github.com/apache/ozone/pull/2635
--
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 #2635: HDDS-5476. [Multi-Tenant] Support Ozone s3 authentication with arbitrary accessId that is not same as the kerberos ID
Posted by GitBox <gi...@apache.org>.
prashantpogde commented on a change in pull request #2635:
URL: https://github.com/apache/ozone/pull/2635#discussion_r709441873
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OMMultiTenantManagerImpl.java
##########
@@ -325,14 +328,14 @@ public String createUser(String tenantName, String userName) {
List<String> userGroupIDs = new ArrayList<>();
userGroupIDs.add(idGroupTenantAllUsers);
- String userID = authorizer.createUser(userPrincipal, userGroupIDs);
+ //String userID = authorizer.createUser(userPrincipal, userGroupIDs);
Review comment:
why do we need this change ?
--
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] errose28 commented on a change in pull request #2635: HDDS-5476. [Multi-Tenant] Support Ozone s3 authentication with arbitrary accessId that is not same as the kerberos ID
Posted by GitBox <gi...@apache.org>.
errose28 commented on a change in pull request #2635:
URL: https://github.com/apache/ozone/pull/2635#discussion_r709639875
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/security/OzoneDelegationTokenSecretManager.java
##########
@@ -157,11 +161,23 @@ public Builder setOmServiceId(String serviceId) {
this.omServiceId = serviceId;
return this;
}
+
+ public Builder setOMMultiTenantManager(OMMultiTenantManager
+ multiTenantManager) {
+ this.omMultiTenantManager = multiTenantManager;
+ return this;
+ }
+
}
@Override
public OzoneTokenIdentifier createIdentifier() {
- return OzoneTokenIdentifier.newInstance();
+ OzoneTokenIdentifier tokenId = OzoneTokenIdentifier.newInstance();
+ if (multiTenantManager != null) {
+ tokenId.setGetUserForAccessId(
+ multiTenantManager::getUserNameGivenAccessId);
Review comment:
It looks like the `OzoneTokenIdentifier` takes an operation returning the user name so it does not need to depend on the entire `OMMultiTenantManager` for this small part. Should we do the same thing for the `OzoneDelegationTokenSecretManager`, where it only depends on the operator and not the whole `OMMultiTenantManager` class, since the field's only use seems to be for retrieving this method? Or will there be more uses of `OMMultiTenantManager` here such that it's better to contain the entire class?
--
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] avijayanhwx commented on a change in pull request #2635: HDDS-5476. [Multi-Tenant] Support Ozone s3 authentication with arbitrary accessId that is not same as the kerberos ID
Posted by GitBox <gi...@apache.org>.
avijayanhwx commented on a change in pull request #2635:
URL: https://github.com/apache/ozone/pull/2635#discussion_r710304168
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OMMultiTenantManagerImpl.java
##########
@@ -325,14 +328,14 @@ public String createUser(String tenantName, String userName) {
List<String> userGroupIDs = new ArrayList<>();
userGroupIDs.add(idGroupTenantAllUsers);
- String userID = authorizer.createUser(userPrincipal, userGroupIDs);
+ //String userID = authorizer.createUser(userPrincipal, userGroupIDs);
Review comment:
Debug change left in, will remove.
--
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 #2635: HDDS-5476. [Multi-Tenant] Support Ozone s3 authentication with arbitrary accessId that is not same as the kerberos ID
Posted by GitBox <gi...@apache.org>.
smengcl commented on pull request #2635:
URL: https://github.com/apache/ozone/pull/2635#issuecomment-920270310
Thanks for the patch @avijayanhwx . I have just merged the latest multi-tenant branch changes into your PR, which should include #2564 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] avijayanhwx commented on a change in pull request #2635: HDDS-5476. [Multi-Tenant] Support Ozone s3 authentication with arbitrary accessId that is not same as the kerberos ID
Posted by GitBox <gi...@apache.org>.
avijayanhwx commented on a change in pull request #2635:
URL: https://github.com/apache/ozone/pull/2635#discussion_r710303802
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/security/OzoneDelegationTokenSecretManager.java
##########
@@ -157,11 +161,23 @@ public Builder setOmServiceId(String serviceId) {
this.omServiceId = serviceId;
return this;
}
+
+ public Builder setOMMultiTenantManager(OMMultiTenantManager
+ multiTenantManager) {
+ this.omMultiTenantManager = multiTenantManager;
+ return this;
+ }
+
}
@Override
public OzoneTokenIdentifier createIdentifier() {
- return OzoneTokenIdentifier.newInstance();
+ OzoneTokenIdentifier tokenId = OzoneTokenIdentifier.newInstance();
+ if (multiTenantManager != null) {
+ tokenId.setGetUserForAccessId(
+ multiTenantManager::getUserNameGivenAccessId);
Review comment:
I don't mind moving the functional reference one step higher. The reason I kept a reference of the MultiTenantManager instance inside the ODTSM is because this class is a "management" class and is long lived. As opposed to the OzoneTokenIdentifier class that is a short lived entity. I do not see any more uses of the OMTM in the ODTSM as of now though.
--
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] avijayanhwx commented on pull request #2635: HDDS-5476. [Multi-Tenant] Support Ozone s3 authentication with arbitrary accessId that is not same as the kerberos ID
Posted by GitBox <gi...@apache.org>.
avijayanhwx commented on pull request #2635:
URL: https://github.com/apache/ozone/pull/2635#issuecomment-925324968
Thanks for the review @errose28, @smengcl & @prashantpogde.
--
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