xichen01 commented on code in PR #7455:
URL: https://github.com/apache/ozone/pull/7455#discussion_r1862388277
##########
hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java:
##########
@@ -688,10 +652,13 @@ public void createBucket(
.setSourceBucket(bucketArgs.getSourceBucket())
.setQuotaInBytes(bucketArgs.getQuotaInBytes())
.setQuotaInNamespace(bucketArgs.getQuotaInNamespace())
- .setAcls(listOfAcls.stream().distinct().collect(Collectors.toList()))
.setBucketLayout(bucketLayout)
.setOwner(owner);
+ if (bucketArgs.getAcls() != null) {
+ builder.setAcls(bucketArgs.getAcls());
Review Comment:
Should we remove the duplicates for bucket too?
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OzoneAclUtil.java:
##########
@@ -47,24 +51,54 @@ public final class OzoneAclUtil {
private OzoneAclUtil() {
}
+ private static ACLType[] userRights;
+ private static ACLType[] groupRights;
+
/**
- * Helper function to get access acl list for current user.
+ * Helper function to get default access acl list for current user.
*
- * @param userName
- * @param userGroups
+ * @param ugi current login user
+ * @param conf current configuration
* @return list of OzoneAcls
* */
- public static List<OzoneAcl> getAclList(String userName,
- String[] userGroups, ACLType userRights, ACLType groupRights) {
-
+ public static List<OzoneAcl> getDefaultAclList(UserGroupInformation ugi,
OzoneConfiguration conf) {
+ // Get default acl rights for user and group.
+ if (userRights == null || groupRights == null) {
+ OzoneAclConfig aclConfig = conf.getObject(OzoneAclConfig.class);
Review Comment:
The `conf.getObject` is an expensive operation, we better do not call it
frequently on critical IO paths.
FYI:
seeļ¼ `getObject -> injectConfiguration -> injectConfigurationToObject`,
injectConfigurationToObject have a loop wgich may need to be looped many times.
a related PR: https://github.com/apache/ozone/pull/5789
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java:
##########
@@ -452,12 +469,15 @@ protected List< OzoneAcl > getAclsForKey(KeyArgs keyArgs,
* @param keyArgs
* @param bucketInfo
* @param omPathInfo
+ * @param config
* @return Acls which inherited parent DEFAULT and keyArgs ACCESS acls.
*/
- protected static List<OzoneAcl> getAclsForDir(KeyArgs keyArgs,
- OmBucketInfo bucketInfo, OMFileRequest.OMPathInfo omPathInfo) {
+ protected List<OzoneAcl> getAclsForDir(KeyArgs keyArgs, OmBucketInfo
bucketInfo,
+ OMFileRequest.OMPathInfo omPathInfo, OzoneConfiguration config) throws
OMException {
// Acls inherited from parent or bucket will convert to DEFAULT scope
List<OzoneAcl> acls = new ArrayList<>();
+ // add default ACLs
+ acls.addAll(getDefaultAclList(createUGIForApi(), config));
Review Comment:
Should we try to remove the duplicate ACL? this be done by client before.
##########
hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java:
##########
@@ -667,17 +642,6 @@ public void createBucket(
.setKeyName(bucketArgs.getEncryptionKey()).build();
}
- List<OzoneAcl> listOfAcls = getAclList();
- //ACLs from BucketArgs
- if (bucketArgs.getAcls() != null) {
- listOfAcls.addAll(bucketArgs.getAcls());
- }
- // Link bucket default acl
- if (bucketArgs.getSourceVolume() != null
- && bucketArgs.getSourceBucket() != null) {
- listOfAcls.add(linkBucketDefaultAcl());
Review Comment:
If the user use this client send `createBucket` to the old version OM, the
created linked bucket will no any ACL permissions. Is this cause some
compatibility issues?
--
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]
