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 2020/09/04 19:59:32 UTC
[GitHub] [hadoop-ozone] xiaoyuyao opened a new pull request #1395: HDDS-4088. Adding Owner info for Authorizer plugin to honor owner acc…
xiaoyuyao opened a new pull request #1395:
URL: https://github.com/apache/hadoop-ozone/pull/1395
…ess rights.
## What changes were proposed in this pull request?
adding isowner flag and volume owner info to acl check request context so that Ranger and 3rd party plugin can allow volume owner has full control over the volume
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-4088
## How was this patch tested?
UT added.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] smengcl commented on a change in pull request #1395: HDDS-4088. Adding Owner info for Authorizer plugin to honor owner access rights
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #1395:
URL: https://github.com/apache/hadoop-ozone/pull/1395#discussion_r510108891
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
##########
@@ -1642,25 +1644,42 @@ private boolean hasAcls(String userName, ResourceType resType,
UserGroupInformation.createRemoteUser(userName),
ProtobufRpcEngine.Server.getRemoteIp(),
ProtobufRpcEngine.Server.getRemoteIp().getHostName(),
- false);
+ false, getVolumeOwner(vol, acl));
} catch (OMException ex) {
// Should not trigger exception here at all
return false;
}
}
- /**
- * CheckAcls for the ozone object.
- *
- * @throws OMException ResultCodes.PERMISSION_DENIED if permission denied.
- */
- @SuppressWarnings("parameternumber")
- public void checkAcls(ResourceType resType, StoreType storeType,
- ACLType aclType, String vol, String bucket, String key,
- UserGroupInformation ugi, InetAddress remoteAddress, String hostName)
- throws OMException {
- checkAcls(resType, storeType, aclType, vol, bucket, key,
- ugi, remoteAddress, hostName, true);
+ public String getVolumeOwner(String vol, ACLType type) throws OMException {
+ String volOwnerName = null;
+ if (!vol.equals(OzoneConsts.OZONE_ROOT) && (type != ACLType.CREATE)) {
+ volOwnerName = getVolumeOwner(vol);
+ }
+ return volOwnerName;
+ }
+
+ private String getVolumeOwner(String volume) throws OMException {
+ Boolean lockAcquired = metadataManager.getLock().acquireReadLock(
+ VOLUME_LOCK, volume);
+ String dbVolumeKey = metadataManager.getVolumeKey(volume);
+ OmVolumeArgs volumeArgs = null;
+ try {
+ volumeArgs = metadataManager.getVolumeTable().get(dbVolumeKey);
Review comment:
`getVolumeTable().get(vol)` seems to return `null` when volume not found rather than throwing.
the exception message in the following catch block needs to be changed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao commented on pull request #1395: HDDS-4088. Adding Owner info for Authorizer plugin to honor owner access rights
Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on pull request #1395:
URL: https://github.com/apache/hadoop-ozone/pull/1395#issuecomment-716722366
Thanks @smengcl for the review.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao merged pull request #1395: HDDS-4088. Adding Owner info for Authorizer plugin to honor owner access rights
Posted by GitBox <gi...@apache.org>.
xiaoyuyao merged pull request #1395:
URL: https://github.com/apache/hadoop-ozone/pull/1395
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao commented on a change in pull request #1395: HDDS-4088. Adding Owner info for Authorizer plugin to honor owner access rights
Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #1395:
URL: https://github.com/apache/hadoop-ozone/pull/1395#discussion_r510410249
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
##########
@@ -1642,25 +1644,42 @@ private boolean hasAcls(String userName, ResourceType resType,
UserGroupInformation.createRemoteUser(userName),
ProtobufRpcEngine.Server.getRemoteIp(),
ProtobufRpcEngine.Server.getRemoteIp().getHostName(),
- false);
+ false, getVolumeOwner(vol, acl));
} catch (OMException ex) {
// Should not trigger exception here at all
return false;
}
}
- /**
- * CheckAcls for the ozone object.
- *
- * @throws OMException ResultCodes.PERMISSION_DENIED if permission denied.
- */
- @SuppressWarnings("parameternumber")
- public void checkAcls(ResourceType resType, StoreType storeType,
- ACLType aclType, String vol, String bucket, String key,
- UserGroupInformation ugi, InetAddress remoteAddress, String hostName)
- throws OMException {
- checkAcls(resType, storeType, aclType, vol, bucket, key,
- ugi, remoteAddress, hostName, true);
+ public String getVolumeOwner(String vol, ACLType type) throws OMException {
+ String volOwnerName = null;
+ if (!vol.equals(OzoneConsts.OZONE_ROOT) && (type != ACLType.CREATE)) {
+ volOwnerName = getVolumeOwner(vol);
+ }
+ return volOwnerName;
+ }
+
+ private String getVolumeOwner(String volume) throws OMException {
+ Boolean lockAcquired = metadataManager.getLock().acquireReadLock(
+ VOLUME_LOCK, volume);
+ String dbVolumeKey = metadataManager.getVolumeKey(volume);
+ OmVolumeArgs volumeArgs = null;
+ try {
+ volumeArgs = metadataManager.getVolumeTable().get(dbVolumeKey);
Review comment:
Thanks for the clarification. I just pushed a change to refine the exception for this case as suggested.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] smengcl commented on a change in pull request #1395: HDDS-4088. Adding Owner info for Authorizer plugin to honor owner access rights
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #1395:
URL: https://github.com/apache/hadoop-ozone/pull/1395#discussion_r510375521
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
##########
@@ -1642,25 +1644,42 @@ private boolean hasAcls(String userName, ResourceType resType,
UserGroupInformation.createRemoteUser(userName),
ProtobufRpcEngine.Server.getRemoteIp(),
ProtobufRpcEngine.Server.getRemoteIp().getHostName(),
- false);
+ false, getVolumeOwner(vol, acl));
} catch (OMException ex) {
// Should not trigger exception here at all
return false;
}
}
- /**
- * CheckAcls for the ozone object.
- *
- * @throws OMException ResultCodes.PERMISSION_DENIED if permission denied.
- */
- @SuppressWarnings("parameternumber")
- public void checkAcls(ResourceType resType, StoreType storeType,
- ACLType aclType, String vol, String bucket, String key,
- UserGroupInformation ugi, InetAddress remoteAddress, String hostName)
- throws OMException {
- checkAcls(resType, storeType, aclType, vol, bucket, key,
- ugi, remoteAddress, hostName, true);
+ public String getVolumeOwner(String vol, ACLType type) throws OMException {
+ String volOwnerName = null;
+ if (!vol.equals(OzoneConsts.OZONE_ROOT) && (type != ACLType.CREATE)) {
+ volOwnerName = getVolumeOwner(vol);
+ }
+ return volOwnerName;
+ }
+
+ private String getVolumeOwner(String volume) throws OMException {
+ Boolean lockAcquired = metadataManager.getLock().acquireReadLock(
+ VOLUME_LOCK, volume);
+ String dbVolumeKey = metadataManager.getVolumeKey(volume);
+ OmVolumeArgs volumeArgs = null;
+ try {
+ volumeArgs = metadataManager.getVolumeTable().get(dbVolumeKey);
+ } catch (IOException ioe) {
+ throw new OMException("Volume " + volume + " is not found",
+ OMException.ResultCodes.VOLUME_NOT_FOUND);
Review comment:
I should tag here :) #tag
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] smengcl commented on a change in pull request #1395: HDDS-4088. Adding Owner info for Authorizer plugin to honor owner access rights
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #1395:
URL: https://github.com/apache/hadoop-ozone/pull/1395#discussion_r510376263
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
##########
@@ -1642,25 +1644,42 @@ private boolean hasAcls(String userName, ResourceType resType,
UserGroupInformation.createRemoteUser(userName),
ProtobufRpcEngine.Server.getRemoteIp(),
ProtobufRpcEngine.Server.getRemoteIp().getHostName(),
- false);
+ false, getVolumeOwner(vol, acl));
} catch (OMException ex) {
// Should not trigger exception here at all
return false;
}
}
- /**
- * CheckAcls for the ozone object.
- *
- * @throws OMException ResultCodes.PERMISSION_DENIED if permission denied.
- */
- @SuppressWarnings("parameternumber")
- public void checkAcls(ResourceType resType, StoreType storeType,
- ACLType aclType, String vol, String bucket, String key,
- UserGroupInformation ugi, InetAddress remoteAddress, String hostName)
- throws OMException {
- checkAcls(resType, storeType, aclType, vol, bucket, key,
- ugi, remoteAddress, hostName, true);
+ public String getVolumeOwner(String vol, ACLType type) throws OMException {
+ String volOwnerName = null;
+ if (!vol.equals(OzoneConsts.OZONE_ROOT) && (type != ACLType.CREATE)) {
+ volOwnerName = getVolumeOwner(vol);
+ }
+ return volOwnerName;
+ }
+
+ private String getVolumeOwner(String volume) throws OMException {
+ Boolean lockAcquired = metadataManager.getLock().acquireReadLock(
+ VOLUME_LOCK, volume);
+ String dbVolumeKey = metadataManager.getVolumeKey(volume);
+ OmVolumeArgs volumeArgs = null;
+ try {
+ volumeArgs = metadataManager.getVolumeTable().get(dbVolumeKey);
Review comment:
Yes @xiaoyuyao . I do see the Line 1677-1682 check and message.
I meant Line 1670-1671 OMException, since `volumeArgs` won't throw if volume is not found, we should change that message. Or just wrap IOException inside OMException?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao commented on a change in pull request #1395: HDDS-4088. Adding Owner info for Authorizer plugin to honor owner access rights
Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #1395:
URL: https://github.com/apache/hadoop-ozone/pull/1395#discussion_r510293188
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
##########
@@ -1642,25 +1644,42 @@ private boolean hasAcls(String userName, ResourceType resType,
UserGroupInformation.createRemoteUser(userName),
ProtobufRpcEngine.Server.getRemoteIp(),
ProtobufRpcEngine.Server.getRemoteIp().getHostName(),
- false);
+ false, getVolumeOwner(vol, acl));
} catch (OMException ex) {
// Should not trigger exception here at all
return false;
}
}
- /**
- * CheckAcls for the ozone object.
- *
- * @throws OMException ResultCodes.PERMISSION_DENIED if permission denied.
- */
- @SuppressWarnings("parameternumber")
- public void checkAcls(ResourceType resType, StoreType storeType,
- ACLType aclType, String vol, String bucket, String key,
- UserGroupInformation ugi, InetAddress remoteAddress, String hostName)
- throws OMException {
- checkAcls(resType, storeType, aclType, vol, bucket, key,
- ugi, remoteAddress, hostName, true);
+ public String getVolumeOwner(String vol, ACLType type) throws OMException {
+ String volOwnerName = null;
+ if (!vol.equals(OzoneConsts.OZONE_ROOT) && (type != ACLType.CREATE)) {
+ volOwnerName = getVolumeOwner(vol);
+ }
+ return volOwnerName;
+ }
+
+ private String getVolumeOwner(String volume) throws OMException {
+ Boolean lockAcquired = metadataManager.getLock().acquireReadLock(
+ VOLUME_LOCK, volume);
+ String dbVolumeKey = metadataManager.getVolumeKey(volume);
+ OmVolumeArgs volumeArgs = null;
+ try {
+ volumeArgs = metadataManager.getVolumeTable().get(dbVolumeKey);
Review comment:
Thanks @smengcl for the review. The null check of volumeArgs for volume not found is done outside the lock between Line 1677-1682.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org