adoroszlai commented on code in PR #4738:
URL: https://github.com/apache/ozone/pull/4738#discussion_r1199654424
##########
hadoop-ozone/dist/src/main/smoketest/ozonefs/ozonefs.robot:
##########
@@ -163,6 +163,18 @@ Get file
Execute ozone fs -get
${BASE_URL}${SCHEME}.txt /tmp/GET.txt
File Should Exist /tmp/GET.txt
+Inherit Acls
+ Execute ozone sh bucket addacl
-a=user:user1:rlw[DEFAULT] ${VOLUME}/${BUCKET}
+ Execute ozone fs -mkdir -p
${BASE_URL}dir1/dir2
+ Verify ACL key ${VOLUME}/${BUCKET}/dir1/
USER user1 READ WRITE LIST
Review Comment:
`Verify ACL` requires importing `shell.robot`:
https://github.com/apache/ozone/blob/8847b2c60bcdc831a7d515c414cd9c28bbf2bbf4/hadoop-ozone/dist/src/main/smoketest/basic/links.robot#L20
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMDirectoryCreateRequestWithFSO.java:
##########
@@ -172,7 +171,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager
ozoneManager,
omPathInfo.getLeafNodeName(),
keyArgs, omPathInfo.getLeafNodeObjectId(),
omPathInfo.getLastKnownParentId(), trxnLogIndex,
- OzoneAclUtil.fromProtobuf(keyArgs.getAclsList()));
+ inheritDefaultAcls(keyArgs, omBucketInfo.getAcls()));
Review Comment:
Per the [design
doc](https://issues.apache.org/jira/secure/attachment/12997587/Design%20Doc-%20Native%20ACL%20support%20for%20Ozone.pdf)
for ACLs in Ozone, inheritance is only supported on default ACLs. I don't
know if actual implementation follows that.
##########
hadoop-ozone/dist/src/main/smoketest/ozonefs/ozonefs.robot:
##########
@@ -163,6 +163,18 @@ Get file
Execute ozone fs -get
${BASE_URL}${SCHEME}.txt /tmp/GET.txt
File Should Exist /tmp/GET.txt
+Inherit Acls
+ Execute ozone sh bucket addacl
-a=user:user1:rlw[DEFAULT] ${VOLUME}/${BUCKET}
+ Execute ozone fs -mkdir -p
${BASE_URL}dir1/dir2
+ Verify ACL key ${VOLUME}/${BUCKET}/dir1/
USER user1 READ WRITE LIST
+ Verify ACL key ${VOLUME}/${BUCKET}/dir1/dir2/
USER user1 READ WRITE LIST
+
+ Execute ozone sh key addacl
-a=user:user2:rw[DEFAULT] ${VOLUME}/${BUCKET}/dir1/
+ Execute ozone fs -mkdir ${BASE_URL}dir1/dir3
+ Verify ACL key ${VOLUME}/${BUCKET}/dir1/dir3/
USER user1 ${EMPTY}
Review Comment:
This fails with:
```
!= READ WRITE LIST
```
(expected: empty, actual: `READ WRITE LIST`)
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestRecursiveAclWithFSO.java:
##########
@@ -199,8 +200,14 @@ public void testKeyDeleteAndRenameWithoutPermission()
throws Exception {
// Remove acl from directory c2, delete/rename a/b1 should throw
// permission denied since c2 is a subdirectory
- UserGroupInformation.setLoginUser(user1);
- removeAclsFromKey(objectStore, ozoneBucket, "a/b1/c2");
+ user1.doAs((PrivilegedExceptionAction<Void>) () -> {
+ try (OzoneClient c = cluster.newClient()) {
+ ObjectStore o = c.getObjectStore();
+ OzoneBucket b = o.getVolume("volume1").getBucket("bucket1");
+ removeAclsFromKey(o, b, "a/b1/c2");
+ }
+ return null;
+ });
Review Comment:
Please try to extract parts of `testKeyDeleteAndRenameWithoutPermission()`,
checkstyle says it's too long:
```
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestRecursiveAclWithFSO.java
84: Method testKeyDeleteAndRenameWithoutPermission length is 159 lines (max
allowed is 150).
```
https://github.com/whbing/ozone/actions/runs/5032997814/jobs/9026948835#step:6:520
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
