From 4476b2e0f1be1f23fc8538f59f54cd6bc7df0fe5 Mon Sep 17 00:00:00 2001 From: "Doroszlai, Attila" Date: Fri, 7 Feb 2025 16:09:43 +0100 Subject: [PATCH 1/2] HDDS-12248. Make allowListAllVolumes reconfigurable in OM --- .../ozone/reconfig/TestOmReconfiguration.java | 12 ++++++++++++ .../org/apache/hadoop/ozone/om/OzoneManager.java | 13 ++++++++++++- .../ozone/security/acl/OzoneAuthorizerFactory.java | 2 +- .../ozone/security/acl/OzoneNativeAuthorizer.java | 9 +++++---- 4 files changed, 30 insertions(+), 6 deletions(-) diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/reconfig/TestOmReconfiguration.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/reconfig/TestOmReconfiguration.java index a7c471c81b19..8646bd4ebbc0 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/reconfig/TestOmReconfiguration.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/reconfig/TestOmReconfiguration.java @@ -27,6 +27,7 @@ import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_ADMINISTRATORS; import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_READONLY_ADMINISTRATORS; import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_KEY_DELETING_LIMIT_PER_TASK; +import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_VOLUME_LISTALL_ALLOWED; import static org.junit.jupiter.api.Assertions.assertEquals; /** @@ -43,6 +44,7 @@ ReconfigurationHandler getSubject() { void reconfigurableProperties() { assertProperties(getSubject(), ImmutableSet.of(OZONE_ADMINISTRATORS, OZONE_READONLY_ADMINISTRATORS, + OZONE_OM_VOLUME_LISTALL_ALLOWED, OZONE_KEY_DELETING_LIMIT_PER_TASK)); } @@ -81,4 +83,14 @@ public void keyDeletingLimitPerTask() throws ReconfigurationException { .getKeyManager().getDeletingService().getKeyLimitPerTask()); } + @Test + void allowListAllVolumes() throws ReconfigurationException { + final boolean newValue = !getCluster().getOzoneManager().getAllowListAllVolumes(); + + getSubject().reconfigurePropertyImpl(OZONE_OM_VOLUME_LISTALL_ALLOWED, + String.valueOf(newValue)); + + assertEquals(newValue, getCluster().getOzoneManager().getAllowListAllVolumes()); + } + } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java index 4a54f2f0f252..6f4fe89f9f91 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java @@ -527,6 +527,7 @@ private OzoneManager(OzoneConfiguration conf, StartupOption startupOption) .register(OZONE_ADMINISTRATORS, this::reconfOzoneAdmins) .register(OZONE_READONLY_ADMINISTRATORS, this::reconfOzoneReadOnlyAdmins) + .register(OZONE_OM_VOLUME_LISTALL_ALLOWED, this::reconfigureAllowListAllVolumes) .register(OZONE_KEY_DELETING_LIMIT_PER_TASK, this::reconfOzoneKeyDeletingLimitPerTask); @@ -759,7 +760,11 @@ public String getThreadNamePrefix() { private void setInstanceVariablesFromConf() { this.isAclEnabled = configuration.getBoolean(OZONE_ACL_ENABLED, OZONE_ACL_ENABLED_DEFAULT); - this.allowListAllVolumes = configuration.getBoolean( + setAllowListAllVolumesFromConfig(); + } + + public void setAllowListAllVolumesFromConfig() { + allowListAllVolumes = configuration.getBoolean( OZONE_OM_VOLUME_LISTALL_ALLOWED, OZONE_OM_VOLUME_LISTALL_ALLOWED_DEFAULT); } @@ -4978,6 +4983,12 @@ private String reconfOzoneKeyDeletingLimitPerTask(String newVal) { return newVal; } + private String reconfigureAllowListAllVolumes(String newVal) { + getConfiguration().setBoolean(OZONE_OM_VOLUME_LISTALL_ALLOWED, Boolean.parseBoolean(newVal)); + setAllowListAllVolumesFromConfig(); + return newVal; + } + public void validateReplicationConfig(ReplicationConfig replicationConfig) throws OMException { try { diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/security/acl/OzoneAuthorizerFactory.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/security/acl/OzoneAuthorizerFactory.java index ea2f2a1da310..b814cb6b03b4 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/security/acl/OzoneAuthorizerFactory.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/security/acl/OzoneAuthorizerFactory.java @@ -112,7 +112,7 @@ private static OzoneNativeAuthorizer configure( authorizer.setPrefixManager(pm); authorizer.setAdminCheck(om::isAdmin); authorizer.setReadOnlyAdminCheck(om::isReadOnlyAdmin); - authorizer.setAllowListAllVolumes(om.getAllowListAllVolumes()); + authorizer.setAllowListAllVolumes(om::getAllowListAllVolumes); return authorizer; } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/security/acl/OzoneNativeAuthorizer.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/security/acl/OzoneNativeAuthorizer.java index 28194115e48c..2a3d6b1dc800 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/security/acl/OzoneNativeAuthorizer.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/security/acl/OzoneNativeAuthorizer.java @@ -32,6 +32,7 @@ import org.slf4j.LoggerFactory; import java.util.Objects; +import java.util.function.BooleanSupplier; import java.util.function.Predicate; import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.INVALID_REQUEST; @@ -54,7 +55,7 @@ public class OzoneNativeAuthorizer implements IAccessAuthorizer { private PrefixManager prefixManager; private Predicate adminCheck = NO_ADMIN; private Predicate readOnlyAdminCheck = NO_ADMIN; - private boolean allowListAllVolumes; + private BooleanSupplier allowListAllVolumes = () -> false; public OzoneNativeAuthorizer() { // required for instantiation in OmMetadataReader#getACLAuthorizerInstance @@ -222,12 +223,12 @@ public void setReadOnlyAdminCheck(Predicate check) { readOnlyAdminCheck = Objects.requireNonNull(check, "read-only admin check"); } - public void setAllowListAllVolumes(boolean allowListAllVolumes) { - this.allowListAllVolumes = allowListAllVolumes; + public void setAllowListAllVolumes(BooleanSupplier allowListAllVolumes) { + this.allowListAllVolumes = Objects.requireNonNull(allowListAllVolumes, "allowListAllVolumes"); } public boolean getAllowListAllVolumes() { - return allowListAllVolumes; + return allowListAllVolumes.getAsBoolean(); } private static boolean isOwner(UserGroupInformation ugi, String ownerName) { From 40bdaef09db230cc3f977a8c1cc94a873d55dc5c Mon Sep 17 00:00:00 2001 From: "Doroszlai, Attila" Date: Sat, 8 Feb 2025 14:11:11 +0100 Subject: [PATCH 2/2] fallback to default setting for empty or invalid value --- .../hadoop/ozone/reconfig/TestOmReconfiguration.java | 11 +++++++++++ .../java/org/apache/hadoop/ozone/om/OzoneManager.java | 4 ++-- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/reconfig/TestOmReconfiguration.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/reconfig/TestOmReconfiguration.java index 8646bd4ebbc0..6fa8d3e931ce 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/reconfig/TestOmReconfiguration.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/reconfig/TestOmReconfiguration.java @@ -22,12 +22,15 @@ import org.apache.hadoop.conf.ReconfigurationException; import org.apache.hadoop.hdds.conf.ReconfigurationHandler; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import static org.apache.commons.lang3.RandomStringUtils.randomAlphabetic; import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_ADMINISTRATORS; import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_READONLY_ADMINISTRATORS; import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_KEY_DELETING_LIMIT_PER_TASK; import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_VOLUME_LISTALL_ALLOWED; +import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_VOLUME_LISTALL_ALLOWED_DEFAULT; import static org.junit.jupiter.api.Assertions.assertEquals; /** @@ -93,4 +96,12 @@ void allowListAllVolumes() throws ReconfigurationException { assertEquals(newValue, getCluster().getOzoneManager().getAllowListAllVolumes()); } + @ParameterizedTest + @ValueSource(strings = {"", "invalid"}) + void unsetAllowListAllVolumes(String newValue) throws ReconfigurationException { + getSubject().reconfigurePropertyImpl(OZONE_OM_VOLUME_LISTALL_ALLOWED, newValue); + + assertEquals(OZONE_OM_VOLUME_LISTALL_ALLOWED_DEFAULT, getCluster().getOzoneManager().getAllowListAllVolumes()); + } + } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java index 6f4fe89f9f91..25684cc4527d 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java @@ -4984,9 +4984,9 @@ private String reconfOzoneKeyDeletingLimitPerTask(String newVal) { } private String reconfigureAllowListAllVolumes(String newVal) { - getConfiguration().setBoolean(OZONE_OM_VOLUME_LISTALL_ALLOWED, Boolean.parseBoolean(newVal)); + getConfiguration().set(OZONE_OM_VOLUME_LISTALL_ALLOWED, newVal); setAllowListAllVolumesFromConfig(); - return newVal; + return String.valueOf(allowListAllVolumes); } public void validateReplicationConfig(ReplicationConfig replicationConfig)