cxorm commented on issue #94: HDDS-2255. Improve Acl Handler Messages
URL: https://github.com/apache/hadoop-ozone/pull/94#issuecomment-548683050
 
 
   > > Thanks @bharatviswa504 for the comment.
   > > `RemoveAclHandler` updated.
   > > And I found that
   > > `ozone sh vol setacl 
--acls=user:newUser:wr,user:newUser:w,user:newUser:r /vol` will return `false` 
for the already existed acl.
   > > `ozone sh bucket setacl 
--acls=user:newUser:wr,user:newUser:w,user:newUser:r /vol/buc` will return 
`true`
   > > `ozone sh key setacl 
--acls=user:newUser:wr,user:newUser:w,user:newUser:r /vol/buc/ikey` will return 
`true`
   > > So we update the `SetAclVolumeHandler`.
   > > Could we fix the `SetAclBucketHandler#setAcl()` & 
`SetAclKeyHandler#setAcl()` to show the right message for the existed acl ?
   > 
   > OzoneAclUtil.java
   > 
   > ```
   >  public static boolean setAcl(List<OzoneAcl> existingAcls,
   >       List<OzoneAcl> newAcls) {
   >     if (existingAcls == null) {
   >       return false;
   >     } else {
   >       existingAcls.clear();
   >       if (newAcls != null) {
   >         existingAcls.addAll(newAcls);
   >       }
   >     }
   >     return true;
   >   }
   > ```
   > 
   > setAcl logic used by Bucket/Volume/Key SetAcl. if new Acls are being set 
we just override and return true. and top existingAcls == null will mostly not 
happen, as we set in builder like below.
   > 
   > OmBucketInfo.java L256:
   > 
   > ```
   >     public Builder() {
   >       //Default values
   >       this.acls = new ArrayList<>();
   >       this.isVersionEnabled = false;
   >       this.storageType = StorageType.DISK;
   >       this.metadata = new HashMap<>();
   >     }
   > ```
   > 
   > So, I have not got how we return false for already existing acl.
   
   Thanks @bharatviswa504 for the review.
   
   Yes, the Bucket/Key ```SetAcl``` use the ```OzoneAclUtil#setAcl```, and it 
will return ```ture``` mostly.
   
   But the Volume ```Setacl``` use the ```OmOzoneAclMap#setAcl``` that call the 
```add``` which causes ```OMException``` when adding the existing acl, and the 
```OMException``` would result in ```false```.
   
   A command would return ```false``` may like: 
   ```ozone sh vol setacl --acls=user:newUser:wr,user:newUser:w,user:newUser:r 
/vol```

----------------------------------------------------------------
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:
us...@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: hdfs-dev-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-dev-h...@hadoop.apache.org

Reply via email to