errose28 commented on code in PR #3397:
URL: https://github.com/apache/ozone/pull/3397#discussion_r870736198
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java:
##########
@@ -653,8 +707,12 @@ private void logVersionMismatch(OzoneConfiguration conf,
ScmInfo scmInfo) {
private void instantiateServices(boolean withNewSnapshot) throws IOException
{
metadataManager = new OmMetadataManagerImpl(configuration);
- multiTenantManager = new OMMultiTenantManagerImpl(this, configuration);
- OzoneAclUtils.setOMMultiTenantManager(multiTenantManager);
+ LOG.info("S3 Multi-Tenancy is {}",
+ isS3MultiTenancyEnabled ? "enabled" : "disabled");
Review Comment:
Left over debug statement? We should have covered necessary logging for
enabling/disabling in the OM constructor.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java:
##########
@@ -534,6 +542,52 @@ private OzoneManager(OzoneConfiguration conf,
StartupOption startupOption)
blockTokenMgr = createBlockTokenSecretManager(configuration);
}
+ // Get S3 multi-tenancy enabled flag
+ this.isS3MultiTenancyEnabled = conf.getBoolean(
+ OZONE_OM_MULTITENANCY_ENABLED, OZONE_OM_MULTITENANCY_ENABLED_DEFAULT);
+
+ boolean devSkipMTCheck = conf.getBoolean(OZONE_OM_TENANT_DEV_SKIP_RANGER,
+ false);
+
+ if (isS3MultiTenancyEnabled && !devSkipMTCheck) {
+ // Validate required configs to enable S3 multi-tenancy
Review Comment:
I think making these fatal would be better user experience. If a user has
enabled multi-tenancy, they are not expecting the OM start with it disabled,
and may miss a log message.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java:
##########
@@ -534,6 +542,52 @@ private OzoneManager(OzoneConfiguration conf,
StartupOption startupOption)
blockTokenMgr = createBlockTokenSecretManager(configuration);
}
+ // Get S3 multi-tenancy enabled flag
Review Comment:
Can we do this in a helper method? OM constructor is already quite large.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java:
##########
@@ -3125,6 +3203,8 @@ public S3VolumeContext getS3VolumeContext() throws
IOException {
multiTenantManager.getTenantForAccessID(accessId);
if (optionalTenantId.isPresent()) {
+ // To block all S3 multi-tenancy requests if disabled
Review Comment:
The offline answer to this question was that access to multi-tenant data
through non-S3 methods would still work as long as Ranger was still configured
with acls to allow access.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java:
##########
@@ -756,6 +814,26 @@ public boolean isGrpcBlockTokenEnabled() {
return grpcBlockTokenEnabled;
}
+ /**
+ * Return config value of {@link OMConfigKeys#OZONE_OM_MULTITENANCY_ENABLED}.
+ */
+ public boolean isS3MultiTenancyEnabled() {
Review Comment:
I don't see any callers of this method. If we make config errors fatal it
would be the same as checking the value of the config key.
--
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]
