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/02/03 04:16:49 UTC
[GitHub] [ozone] sadanand48 opened a new pull request #1888: HDDS-4787.Trash emptier fails to create checkpoints in a secure setup
sadanand48 opened a new pull request #1888:
URL: https://github.com/apache/ozone/pull/1888
## What changes were proposed in this pull request?
Since in `TrashOzoneFilesystem` the calls are internal and not through RPC, NPE is generated during `checkacls`(), Since `Server.getRemoteIp` and other such parameters would be null, the security authorizers too generate an NPE. The fix here is if `Server.getRemoteIp()` is null replace it with ip of the node that starts OM
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-4787#
## How was this patch tested?
Existing unit tests. also tried on cluster
----------------------------------------------------------------
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: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] mukul1987 merged pull request #1888: HDDS-4787. Trash emptier fails to create checkpoints in a secure setup
Posted by GitBox <gi...@apache.org>.
mukul1987 merged pull request #1888:
URL: https://github.com/apache/ozone/pull/1888
----------------------------------------------------------------
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: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] sadanand48 commented on a change in pull request #1888: HDDS-4787.Trash emptier fails to create checkpoints in a secure setup
Posted by GitBox <gi...@apache.org>.
sadanand48 commented on a change in pull request #1888:
URL: https://github.com/apache/ozone/pull/1888#discussion_r569969159
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
##########
@@ -3605,14 +3620,20 @@ public ResolvedBucket resolveBucketLink(Pair<String, String> requested)
throws IOException {
Pair<String, String> resolved;
- if (isAclEnabled) {
- resolved = resolveBucketLink(requested, new HashSet<>(),
- Server.getRemoteUser(),
- Server.getRemoteIp(),
- Server.getRemoteIp().getHostName());
- } else {
- resolved = resolveBucketLink(requested, new HashSet<>(),
- null, null, null);
+ try {
+ if (isAclEnabled) {
Review comment:
done
----------------------------------------------------------------
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: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] mukul1987 merged pull request #1888: HDDS-4787. Trash emptier fails to create checkpoints in a secure setup
Posted by GitBox <gi...@apache.org>.
mukul1987 merged pull request #1888:
URL: https://github.com/apache/ozone/pull/1888
----------------------------------------------------------------
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: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] sadanand48 commented on pull request #1888: HDDS-4787.Trash emptier fails to create checkpoints in a secure setup
Posted by GitBox <gi...@apache.org>.
sadanand48 commented on pull request #1888:
URL: https://github.com/apache/ozone/pull/1888#issuecomment-773113007
Thanks @xiaoyuyao for reviewing this .` TestOzoneFilesystem testTrash()` is the unit test for this and this issue would arise when `OZONE_ACL_ENABLED `is set to true. I have made that change. There is also a robot test case for trash deletion inside `ozone-fs.robot` . The secure acceptance CI check will test that in a secure environment with native acls enabled.
----------------------------------------------------------------
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: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai commented on pull request #1888: HDDS-4787.Trash emptier fails to create checkpoints in a secure setup
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on pull request #1888:
URL: https://github.com/apache/ozone/pull/1888#issuecomment-773118665
Thanks @sadanand48 for updating the patch.
----------------------------------------------------------------
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: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai commented on a change in pull request #1888: HDDS-4787.Trash emptier fails to create checkpoints in a secure setup
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on a change in pull request #1888:
URL: https://github.com/apache/ozone/pull/1888#discussion_r569189075
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
##########
@@ -1688,11 +1697,17 @@ public void createVolume(OmVolumeArgs args) throws IOException {
*/
private void checkAcls(ResourceType resType, StoreType store,
ACLType acl, String vol, String bucket, String key)
- throws OMException {
+ throws IOException {
checkAcls(resType, store, acl, vol, bucket, key,
- ProtobufRpcEngine.Server.getRemoteUser(),
- ProtobufRpcEngine.Server.getRemoteIp(),
- ProtobufRpcEngine.Server.getRemoteIp().getHostName(),
+ ProtobufRpcEngine.Server.getRemoteUser() != null ?
+ ProtobufRpcEngine.Server.getRemoteUser() :
Review comment:
Nit: can you please store these in local variables instead of multiple calls?
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/TrashOzoneFileSystem.java
##########
@@ -490,15 +497,38 @@ boolean processKeyPath(List<String> keyPathList) {
.setDeleteKeys(deleteKeyArgs)
.build();
OzoneManagerProtocolProtos.OMRequest omRequest =
- OzoneManagerProtocolProtos.OMRequest.newBuilder()
- .setClientId(CLIENT_ID.toString())
- .setDeleteKeysRequest(deleteKeysRequest)
- .setCmdType(OzoneManagerProtocolProtos.Type.DeleteKeys)
- .build();
+ null;
+ try {
+ omRequest = OzoneManagerProtocolProtos.OMRequest.newBuilder()
+ .setClientId(CLIENT_ID.toString())
+ .setUserInfo(getUserInfo())
+ .setDeleteKeysRequest(deleteKeysRequest)
+ .setCmdType(OzoneManagerProtocolProtos.Type.DeleteKeys)
+ .build();
+ } catch (IOException e) {
+ LOG.error("Couldn't get userinfo", e);
+ }
return omRequest;
}
}
+ OzoneManagerProtocolProtos.UserInfo getUserInfo() throws IOException {
+ UserGroupInformation user = UserGroupInformation.getCurrentUser();
+ InetAddress remoteAddress = ozoneManager.getOmRpcServerAddr().getAddress();
+ OzoneManagerProtocolProtos.UserInfo.Builder userInfo =
+ OzoneManagerProtocolProtos.UserInfo.newBuilder();
+ if (user != null) {
+ userInfo.setUserName(user.getUserName());
+ }
+
+ if (remoteAddress != null) {
+ userInfo.setHostName(remoteAddress.getHostName());
+ userInfo.setRemoteAddress(remoteAddress.getHostAddress()).build();
Review comment:
```suggestion
userInfo.setRemoteAddress(remoteAddress.getHostAddress());
```
----------------------------------------------------------------
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: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] sadanand48 commented on pull request #1888: HDDS-4787.Trash emptier fails to create checkpoints in a secure setup
Posted by GitBox <gi...@apache.org>.
sadanand48 commented on pull request #1888:
URL: https://github.com/apache/ozone/pull/1888#issuecomment-773113007
----------------------------------------------------------------
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: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai commented on pull request #1888: HDDS-4787.Trash emptier fails to create checkpoints in a secure setup
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on pull request #1888:
URL: https://github.com/apache/ozone/pull/1888#issuecomment-773118665
Thanks @sadanand48 for updating the patch.
----------------------------------------------------------------
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: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] sadanand48 commented on pull request #1888: HDDS-4787.Trash emptier fails to create checkpoints in a secure setup
Posted by GitBox <gi...@apache.org>.
sadanand48 commented on pull request #1888:
URL: https://github.com/apache/ozone/pull/1888#issuecomment-773117019
Thanks @adoroszlai for the review and pointing out the error in the secure acceptance check. It's passing 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.
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] xiaoyuyao commented on a change in pull request #1888: HDDS-4787.Trash emptier fails to create checkpoints in a secure setup
Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #1888:
URL: https://github.com/apache/ozone/pull/1888#discussion_r569787990
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
##########
@@ -3605,14 +3620,20 @@ public ResolvedBucket resolveBucketLink(Pair<String, String> requested)
throws IOException {
Pair<String, String> resolved;
- if (isAclEnabled) {
- resolved = resolveBucketLink(requested, new HashSet<>(),
- Server.getRemoteUser(),
- Server.getRemoteIp(),
- Server.getRemoteIp().getHostName());
- } else {
- resolved = resolveBucketLink(requested, new HashSet<>(),
- null, null, null);
+ try {
+ if (isAclEnabled) {
Review comment:
agree with @adoroszlai can we use a local variable to hold the result of Server.getRemoteIp().
----------------------------------------------------------------
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: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] sadanand48 commented on a change in pull request #1888: HDDS-4787.Trash emptier fails to create checkpoints in a secure setup
Posted by GitBox <gi...@apache.org>.
sadanand48 commented on a change in pull request #1888:
URL: https://github.com/apache/ozone/pull/1888#discussion_r569968400
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
##########
@@ -1688,11 +1697,17 @@ public void createVolume(OmVolumeArgs args) throws IOException {
*/
private void checkAcls(ResourceType resType, StoreType store,
ACLType acl, String vol, String bucket, String key)
- throws OMException {
+ throws IOException {
checkAcls(resType, store, acl, vol, bucket, key,
- ProtobufRpcEngine.Server.getRemoteUser(),
- ProtobufRpcEngine.Server.getRemoteIp(),
- ProtobufRpcEngine.Server.getRemoteIp().getHostName(),
+ ProtobufRpcEngine.Server.getRemoteUser() != null ?
+ ProtobufRpcEngine.Server.getRemoteUser() :
Review comment:
Done.
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/TrashOzoneFileSystem.java
##########
@@ -490,15 +497,38 @@ boolean processKeyPath(List<String> keyPathList) {
.setDeleteKeys(deleteKeyArgs)
.build();
OzoneManagerProtocolProtos.OMRequest omRequest =
- OzoneManagerProtocolProtos.OMRequest.newBuilder()
- .setClientId(CLIENT_ID.toString())
- .setDeleteKeysRequest(deleteKeysRequest)
- .setCmdType(OzoneManagerProtocolProtos.Type.DeleteKeys)
- .build();
+ null;
+ try {
+ omRequest = OzoneManagerProtocolProtos.OMRequest.newBuilder()
+ .setClientId(CLIENT_ID.toString())
+ .setUserInfo(getUserInfo())
+ .setDeleteKeysRequest(deleteKeysRequest)
+ .setCmdType(OzoneManagerProtocolProtos.Type.DeleteKeys)
+ .build();
+ } catch (IOException e) {
+ LOG.error("Couldn't get userinfo", e);
+ }
return omRequest;
}
}
+ OzoneManagerProtocolProtos.UserInfo getUserInfo() throws IOException {
+ UserGroupInformation user = UserGroupInformation.getCurrentUser();
+ InetAddress remoteAddress = ozoneManager.getOmRpcServerAddr().getAddress();
+ OzoneManagerProtocolProtos.UserInfo.Builder userInfo =
+ OzoneManagerProtocolProtos.UserInfo.newBuilder();
+ if (user != null) {
+ userInfo.setUserName(user.getUserName());
+ }
+
+ if (remoteAddress != null) {
+ userInfo.setHostName(remoteAddress.getHostName());
+ userInfo.setRemoteAddress(remoteAddress.getHostAddress()).build();
Review comment:
Done.
----------------------------------------------------------------
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: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org