This is an automated email from the ASF dual-hosted git repository.
adoroszlai pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ozone.git
The following commit(s) were added to refs/heads/master by this push:
new d0e15b171a HDDS-8710. Expose read-only interface of OzoneAdmins to
OzoneNativeAuthorizer (#4784)
d0e15b171a is described below
commit d0e15b171a6ce21f0984de0a6c3ff13c71d45b19
Author: Doroszlai, Attila <[email protected]>
AuthorDate: Mon Jun 5 08:11:03 2023 +0200
HDDS-8710. Expose read-only interface of OzoneAdmins to
OzoneNativeAuthorizer (#4784)
---
.../apache/hadoop/ozone/om/OmMetadataReader.java | 4 +-
.../org/apache/hadoop/ozone/om/OzoneManager.java | 11 ++-
.../ozone/security/acl/OzoneNativeAuthorizer.java | 65 +++++++----------
.../security/acl/TestOzoneNativeAuthorizer.java | 83 ++++++++++------------
4 files changed, 69 insertions(+), 94 deletions(-)
diff --git
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataReader.java
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataReader.java
index e429b82d35..d8c0ac0e7d 100644
---
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataReader.java
+++
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataReader.java
@@ -113,8 +113,8 @@ public class OmMetadataReader implements IOmMetadataReader,
Auditor {
authorizer.setBucketManager(bucketManager);
authorizer.setKeyManager(keyManager);
authorizer.setPrefixManager(prefixManager);
- authorizer.setOzoneAdmins(ozoneManager.getOmAdmins());
- authorizer.setOzoneReadOnlyAdmins(ozoneManager.getReadOnlyAdmins());
+ authorizer.setAdminCheck(ozoneManager::isAdmin);
+ authorizer.setReadOnlyAdminCheck(ozoneManager::isReadOnlyAdmin);
authorizer.setAllowListAllVolumes(allowListAllVolumes);
} else {
isNativeAuthorizerEnabled = false;
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 aa31bacad6..3555658201 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
@@ -4023,14 +4023,11 @@ public final class OzoneManager extends
ServiceRuntimeInfoImpl
}
/**
- * Return OzoneAdmins.
+ * @param callerUgi Caller UserGroupInformation
+ * @return return true if the {@code ugi} is a read-only OM admin
*/
- public OzoneAdmins getOmAdmins() {
- return omAdmins;
- }
-
- public OzoneAdmins getReadOnlyAdmins() {
- return readOnlyAdmins;
+ public boolean isReadOnlyAdmin(UserGroupInformation callerUgi) {
+ return callerUgi != null && readOnlyAdmins.isAdmin(callerUgi);
}
/**
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 d7079fa952..7d75a3a7a2 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
@@ -16,7 +16,7 @@
*/
package org.apache.hadoop.ozone.security.acl;
-import com.google.common.base.Preconditions;
+import com.google.common.annotations.VisibleForTesting;
import org.apache.hadoop.hdds.annotation.InterfaceAudience;
import org.apache.hadoop.hdds.annotation.InterfaceStability;
import org.apache.hadoop.hdds.server.OzoneAdmins;
@@ -32,6 +32,7 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.util.Objects;
+import java.util.function.Predicate;
import static
org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.INVALID_REQUEST;
@@ -45,25 +46,29 @@ public class OzoneNativeAuthorizer implements
IAccessAuthorizer {
private static final Logger LOG =
LoggerFactory.getLogger(OzoneNativeAuthorizer.class);
+
+ private static final Predicate<UserGroupInformation> NO_ADMIN = any -> false;
+
private VolumeManager volumeManager;
private BucketManager bucketManager;
private KeyManager keyManager;
private PrefixManager prefixManager;
- private OzoneAdmins ozAdmins;
- private OzoneAdmins ozReadOnlyAdmins;
+ private Predicate<UserGroupInformation> adminCheck = NO_ADMIN;
+ private Predicate<UserGroupInformation> readOnlyAdminCheck = NO_ADMIN;
private boolean allowListAllVolumes;
public OzoneNativeAuthorizer() {
+ // required for instantiation in OmMetadataReader#getACLAuthorizerInstance
}
- public OzoneNativeAuthorizer(VolumeManager volumeManager,
+ OzoneNativeAuthorizer(VolumeManager volumeManager,
BucketManager bucketManager, KeyManager keyManager,
PrefixManager prefixManager, OzoneAdmins ozoneAdmins) {
this.volumeManager = volumeManager;
this.bucketManager = bucketManager;
this.keyManager = keyManager;
this.prefixManager = prefixManager;
- this.ozAdmins = ozoneAdmins;
+ this.adminCheck = ozoneAdmins::isAdmin;
}
/**
@@ -91,15 +96,12 @@ public class OzoneNativeAuthorizer implements
IAccessAuthorizer {
}
// bypass all checks for admin
- boolean isAdmin = isAdmin(ozAdmins, context.getClientUgi());
- if (isAdmin) {
+ if (adminCheck.test(context.getClientUgi())) {
return true;
}
// bypass read checks for read only admin users
- boolean isReadOnlyAdmin = isAdmin(ozReadOnlyAdmins,
- context.getClientUgi());
- if (isReadOnlyAdmin
+ if (readOnlyAdminCheck.test(context.getClientUgi())
&& (context.getAclRights() == ACLType.READ
|| context.getAclRights() == ACLType.READ_ACL
|| context.getAclRights() == ACLType.LIST)) {
@@ -136,9 +138,7 @@ public class OzoneNativeAuthorizer implements
IAccessAuthorizer {
// only admin is allowed to create volume and list all volumes
return false;
}
- boolean volumeAccess = isOwner ||
- volumeManager.checkAccess(objInfo, context);
- return volumeAccess;
+ return isOwner || volumeManager.checkAccess(objInfo, context);
case BUCKET:
LOG.trace("Checking access for bucket: {}", objInfo);
// Skip check for volume owner
@@ -200,16 +200,22 @@ public class OzoneNativeAuthorizer implements
IAccessAuthorizer {
this.prefixManager = prefixManager;
}
- public void setOzoneAdmins(OzoneAdmins ozoneAdmins) {
- this.ozAdmins = ozoneAdmins;
+ @VisibleForTesting
+ void setOzoneAdmins(OzoneAdmins admins) {
+ setAdminCheck(admins::isAdmin);
+ }
+
+ @VisibleForTesting
+ void setOzoneReadOnlyAdmins(OzoneAdmins readOnlyAdmins) {
+ setReadOnlyAdminCheck(readOnlyAdmins::isAdmin);
}
- public void setOzoneReadOnlyAdmins(OzoneAdmins ozoneReadOnlyAdmins) {
- this.ozReadOnlyAdmins = ozoneReadOnlyAdmins;
+ public void setAdminCheck(Predicate<UserGroupInformation> check) {
+ adminCheck = Objects.requireNonNull(check, "admin check");
}
- public OzoneAdmins getOzoneAdmins() {
- return ozAdmins;
+ public void setReadOnlyAdminCheck(Predicate<UserGroupInformation> check) {
+ readOnlyAdminCheck = Objects.requireNonNull(check, "read-only admin
check");
}
public void setAllowListAllVolumes(boolean allowListAllVolumes) {
@@ -220,24 +226,7 @@ public class OzoneNativeAuthorizer implements
IAccessAuthorizer {
return allowListAllVolumes;
}
- private boolean isOwner(UserGroupInformation callerUgi, String ownerName) {
- if (ownerName == null) {
- return false;
- }
- if (callerUgi.getShortUserName().equals(ownerName)) {
- return true;
- }
- return false;
- }
-
- private boolean isAdmin(OzoneAdmins pOzAdmins,
- UserGroupInformation callerUgi) {
- Preconditions.checkNotNull(callerUgi, "callerUgi should not be null!");
-
- if (pOzAdmins == null) {
- return false;
- }
-
- return pOzAdmins.isAdmin(callerUgi);
+ private static boolean isOwner(UserGroupInformation ugi, String ownerName) {
+ return ownerName != null && ownerName.equals(ugi.getShortUserName());
}
}
diff --git
a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/acl/TestOzoneNativeAuthorizer.java
b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/acl/TestOzoneNativeAuthorizer.java
index 86b0229439..31a68e6e1a 100644
---
a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/acl/TestOzoneNativeAuthorizer.java
+++
b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/acl/TestOzoneNativeAuthorizer.java
@@ -52,10 +52,10 @@ import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
-import java.util.Collections;
import java.util.List;
import java.util.stream.Collectors;
+import static java.util.Collections.singletonList;
import static org.apache.hadoop.hdds.HddsConfigKeys.OZONE_METADATA_DIRS;
import static org.apache.hadoop.ozone.OzoneAcl.AclScope.ACCESS;
import static
org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_ACL_AUTHORIZER_CLASS;
@@ -85,20 +85,16 @@ import static org.junit.Assert.assertTrue;
@RunWith(Parameterized.class)
public class TestOzoneNativeAuthorizer {
- private static OzoneConfiguration ozConfig;
- private String vol;
- private String buck;
+ private static final List<String> ADMIN_USERNAMES = singletonList("om");
+ private final String vol;
+ private final String buck;
private String key;
- private String prefix;
- private ACLType parentDirUserAcl;
- private ACLType parentDirGroupAcl;
+ private final String prefix;
+ private final ACLType parentDirUserAcl;
+ private final ACLType parentDirGroupAcl;
private boolean expectedAclResult;
private static OzoneManagerProtocol writeClient;
- private static KeyManager keyManager;
- private static VolumeManager volumeManager;
- private static BucketManager bucketManager;
- private static PrefixManager prefixManager;
private static OMMetadataManager metadataManager;
private static OzoneNativeAuthorizer nativeAuthorizer;
private static UserGroupInformation adminUgi;
@@ -107,7 +103,6 @@ public class TestOzoneNativeAuthorizer {
private OzoneObj volObj;
private OzoneObj buckObj;
private OzoneObj keyObj;
- private OzoneObj prefixObj;
@Parameterized.Parameters
public static Collection<Object[]> data() {
@@ -142,7 +137,7 @@ public class TestOzoneNativeAuthorizer {
@BeforeClass
public static void setup() throws Exception {
- ozConfig = new OzoneConfiguration();
+ OzoneConfiguration ozConfig = new OzoneConfiguration();
ozConfig.set(OZONE_ACL_AUTHORIZER_CLASS,
OZONE_ACL_AUTHORIZER_CLASS_NATIVE);
File dir = GenericTestUtils.getRandomizedTestDir();
@@ -152,14 +147,13 @@ public class TestOzoneNativeAuthorizer {
OmTestManagers omTestManagers =
new OmTestManagers(ozConfig);
metadataManager = omTestManagers.getMetadataManager();
- volumeManager = omTestManagers.getVolumeManager();
- bucketManager = omTestManagers.getBucketManager();
- prefixManager = omTestManagers.getPrefixManager();
- keyManager = omTestManagers.getKeyManager();
+ VolumeManager volumeManager = omTestManagers.getVolumeManager();
+ BucketManager bucketManager = omTestManagers.getBucketManager();
+ PrefixManager prefixManager = omTestManagers.getPrefixManager();
+ KeyManager keyManager = omTestManagers.getKeyManager();
writeClient = omTestManagers.getWriteClient();
nativeAuthorizer = new OzoneNativeAuthorizer(volumeManager, bucketManager,
- keyManager, prefixManager,
- new OzoneAdmins(Collections.singletonList("om")));
+ keyManager, prefixManager, new OzoneAdmins(ADMIN_USERNAMES));
adminUgi = UserGroupInformation.createUserForTesting("om",
new String[]{"ozone"});
testUgi = UserGroupInformation.createUserForTesting("testuser",
@@ -276,7 +270,7 @@ public class TestOzoneNativeAuthorizer {
@Test
public void testCheckAccessForPrefix() throws Exception {
- prefixObj = new OzoneObjInfo.Builder()
+ OzoneObj prefixObj = new OzoneObjInfo.Builder()
.setVolumeName(vol)
.setBucketName(buck)
.setPrefixName(prefix)
@@ -361,29 +355,29 @@ public class TestOzoneNativeAuthorizer {
List<ACLType> allAcls = Arrays.stream(ACLType.values()).
collect(Collectors.toList());
- /**
+ /*
* 1. Reset default acls to an acl.
* 2. Test if user/group has access only to it.
* 3. Add remaining acls one by one and then test
* if user/group has access to them.
- * */
+ */
for (ACLType a1 : allAcls) {
OzoneAcl newAcl = new OzoneAcl(accessType, getAclName(accessType), a1,
ACCESS);
// Reset acls to only one right.
if (obj.getResourceType() == VOLUME) {
- setVolumeAcl(Collections.singletonList(newAcl));
+ setVolumeAcl(singletonList(newAcl));
} else if (obj.getResourceType() == BUCKET) {
- setBucketAcl(Collections.singletonList(newAcl));
+ setBucketAcl(singletonList(newAcl));
} else {
- aclImplementor.setAcl(obj, Collections.singletonList(newAcl));
+ aclImplementor.setAcl(obj, singletonList(newAcl));
}
// Fetch current acls and validate.
acls = aclImplementor.getAcl(obj);
- assertTrue(acls.size() == 1);
+ assertEquals(1, acls.size());
assertTrue(acls.contains(newAcl));
// Special handling for ALL.
@@ -397,17 +391,17 @@ public class TestOzoneNativeAuthorizer {
validateNone(obj, builder);
continue;
}
+
String msg = "Acl to check:" + a1 + " accessType:" +
accessType + " path:" + obj.getPath();
- if (a1.equals(CREATE) && obj.getResourceType().equals(VOLUME)) {
- assertEquals(msg, nativeAuthorizer.getOzoneAdmins()
- .getAdminUsernames().contains(user),
- nativeAuthorizer.checkAccess(obj,
- builder.setAclRights(a1).build()));
- } else {
- assertEquals(msg, expectedAclResult, nativeAuthorizer.checkAccess(obj,
- builder.setAclRights(a1).build()));
- }
+ RequestContext context = builder.setAclRights(a1).build();
+ boolean expectedResult =
+ a1.equals(CREATE) && obj.getResourceType().equals(VOLUME)
+ ? ADMIN_USERNAMES.contains(user)
+ : expectedAclResult;
+ assertEquals(msg, expectedResult,
+ nativeAuthorizer.checkAccess(obj, context));
+
List<ACLType> aclsToBeValidated =
Arrays.stream(ACLType.values()).collect(Collectors.toList());
List<ACLType> aclsToBeAdded =
@@ -434,8 +428,9 @@ public class TestOzoneNativeAuthorizer {
if (!a2.equals(a1)) {
acls = aclImplementor.getAcl(obj);
- List right = acls.stream().map(a -> a.getAclList()).collect(
- Collectors.toList());
+ List<List<ACLType>> right = acls.stream()
+ .map(OzoneAcl::getAclList)
+ .collect(Collectors.toList());
assertFalse("Did not expect client to have " + a2 + " acl. " +
"Current acls found:" + right + ". Type:" + accessType + ","
+ " name:" + (accessType == USER ? user : group),
@@ -512,8 +507,6 @@ public class TestOzoneNativeAuthorizer {
/**
* Helper function to test acl rights with user/group had ALL acl bit set.
- * @param obj
- * @param builder
*/
private void validateAll(OzoneObj obj, RequestContext.Builder
builder) throws OMException {
@@ -521,21 +514,17 @@ public class TestOzoneNativeAuthorizer {
allAcls.remove(ALL);
allAcls.remove(NONE);
RequestContext ctx = builder.build();
- boolean expectedResult = expectedAclResult;
- if (nativeAuthorizer.getOzoneAdmins().getAdminUsernames().contains(
- ctx.getClientUgi().getUserName())) {
- expectedResult = true;
- }
+ String userName = ctx.getClientUgi().getUserName();
+ boolean expectedResult = expectedAclResult
+ || ADMIN_USERNAMES.contains(userName);
for (ACLType a : allAcls) {
- assertEquals("User should have right " + a + ".",
+ assertEquals("User " + userName + " should have right " + a + ".",
expectedResult, nativeAuthorizer.checkAccess(obj, ctx));
}
}
/**
* Helper function to test acl rights with user/group had NONE acl bit set.
- * @param obj
- * @param builder
*/
private void validateNone(OzoneObj obj, RequestContext.Builder
builder) throws OMException {
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
