sumitagrawl commented on code in PR #4738:
URL: https://github.com/apache/ozone/pull/4738#discussion_r1219496825
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileRequest.java:
##########
@@ -226,7 +228,7 @@ public static OMPathInfoWithFSO verifyDirectoryKeysInPath(
if (elements.hasNext()) {
Review Comment:
filterDefaultScope is not done for omBucketInfo.getAcls() present at line no
197, so if no parent dir, only bucket, then will take all acls from bucket
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java:
##########
@@ -307,6 +316,23 @@ protected List< OzoneAcl > getAclsForKey(KeyArgs keyArgs,
return acls;
}
+ /**
+ * Inherit parent's default acls and generate its own acls.
+ * @param keyArgs
+ * @param parentAcls
+ * @return Acls of inherited and its own
+ */
+ protected static List<OzoneAcl> buildAcls(KeyArgs keyArgs,
+ List<OzoneAcl> parentAcls) {
+ // inherit parent acls and convert to DEFAULT scope
+ List<OzoneAcl> acls = new ArrayList<>();
+ OzoneAclUtil.inheritDefaultAcls(acls, parentAcls);
+ OzoneAclUtil.toDefaultScope(acls);
Review Comment:
method itself will inherit only default acl, it seems no need set again
toDefaultScope() and this method may not be useful
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OzoneAclUtil.java:
##########
@@ -180,6 +180,25 @@ public static boolean inheritDefaultAcls(List<OzoneAcl>
acls,
return false;
}
+ /**
+ * Helper function to convert the scope of ACLs to DEFAULT.
+ * This method is called in ACL inheritance scenarios.
+ * @param acls
+ */
+ public static void toDefaultScope(List<OzoneAcl> acls) {
+ acls.forEach(a -> a.setAclScope(DEFAULT));
+ }
+
+ /**
+ * Helper function to filter the DEFAULT scope ACLs.
+ * This method is called in ACL inheritance scenarios.
+ * @param acls
+ */
+ public static List<OzoneAcl> filterDefaultScope(List<OzoneAcl> acls) {
Review Comment:
This is not needed as can use inheritDefaultAcl can be used.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/multipart/S3InitiateMultipartUploadRequest.java:
##########
@@ -202,7 +202,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager
ozoneManager,
.setReplicationConfig(replicationConfig)
.setOmKeyLocationInfos(Collections.singletonList(
new OmKeyLocationInfoGroup(0, new ArrayList<>())))
- .setAcls(getAclsForKey(keyArgs, bucketInfo,
+ .setAcls(getAclsForKey(keyArgs, bucketInfo, null,
ozoneManager.getPrefixManager()))
Review Comment:
missing similar handling in S3InitiateMultipartUploadFSO
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileRequest.java:
##########
@@ -226,7 +228,7 @@ public static OMPathInfoWithFSO verifyDirectoryKeysInPath(
if (elements.hasNext()) {
Review Comment:
IMO, instead of changing PathInfo, it can filter Default at time of
Directory / file creation. FileInfo creation, its handled. But directory
creation for all flow, this needs handle.
--
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]
