[GitHub] [ozone] neils-dev commented on a diff in pull request #3553: HDDS-6942. Ozone Vols/Buckets/Objects created via S3 should not allow group access

[GitBox <gi...@apache.org>]

neils-dev commented on code in PR #3553:
URL: https://github.com/apache/ozone/pull/3553#discussion_r907937325


##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/EndpointBase.java:
##########
@@ -63,6 +64,8 @@ public abstract class EndpointBase implements Auditor {
   private S3Auth s3Auth;
   @Context
   private ContainerRequestContext context;
+  @Inject
+  private OzoneConfiguration ozoneConfiguration;

Review Comment:
   Instead of injecting the `ozoneConfiguration` here at the` EndpointBase` and redefining the default value for the _ozone.om.group.rights_, we can set the default value when we define the configuration at the _CDI_ producer either `Gateway.java` or `OzoneConfigurationHolder`.
   https://github.com/apache/ozone/blob/a8808d1c3781627c40e0ed25d0bb4ec1e74e3de2/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/Gateway.java#L70
   or
   https://github.com/apache/ozone/blob/a8808d1c3781627c40e0ed25d0bb4ec1e74e3de2/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/OzoneConfigurationHolder.java#L41
   
   Having the configuration default for group access set here, defines it for all injected `OzoneConfiguration` instances (endpoints, client producer - all s3 consumers).  Also, this is S3 specific, and does _not_ affect any other `RPCClient`. 



##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/EndpointBase.java:
##########
@@ -98,6 +101,7 @@ protected OzoneBucket getBucket(OzoneVolume volume, String bucketName)
   @PostConstruct
   public void initialization() {
     LOG.debug("S3 access id: {}", s3Auth.getAccessID());
+    ozoneConfiguration.setIfUnset("ozone.om.group.rights", "NONE");

Review Comment:
   OzoneAclConfig defines the default value as ALL for group access,
   ```
     @Config(key = "group.rights",
         defaultValue = "ALL",
   ```
   
   > I don't think this setting is specific to S3.
   
   The default setting is not exclusively for S3 but for all `RpcClient`s as is currently implemented in the `RpcClient`.  For an S3 specific default group privilege, we may need to set it in the the s3gateway (see previous comment to place in the` gateway CDI producer`).   



-- 
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: issues-unsubscribe@ozone.apache.org

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