Skip to content

HDDS-12299. Merge OzoneAclConfig into OmConfig #8383

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ChenSammi merged 12 commits into from
May 14, 2025
Merged

HDDS-12299. Merge OzoneAclConfig into OmConfig #8383

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
b0733ae
HDDS-12299. Merge OzoneAclConfig into OmConfig
adoroszlai May 3, 2025
767a237
use OmConfig from OM in integration test
adoroszlai May 3, 2025
eb31b12
update rights in OmConfig.setFrom
adoroszlai May 3, 2025
9e73ec9
store parsed list
adoroszlai May 3, 2025
2a947b2
pass OMs OmConfig to OzoneAclUtil
adoroszlai May 3, 2025
6800abd
remove static state
adoroszlai May 3, 2025
f384524
rename aclConfig variables to omConfig
adoroszlai May 3, 2025
91817e1
fix unit tests
adoroszlai May 3, 2025
233d3f3
Merge remote-tracking branch 'origin/master' into HDDS-12299
adoroszlai May 7, 2025
cbec288
Merge remote-tracking branch 'origin/master' into HDDS-12299
adoroszlai May 12, 2025
9c1efa5
avoid short-lived array
adoroszlai May 12, 2025
44d2b6a
Merge remote-tracking branch 'origin/master' into HDDS-12299
adoroszlai May 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 51 additions & 0 deletions hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OmConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,18 @@

import com.google.common.base.Preconditions;
import java.time.Duration;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.function.Supplier;
import org.apache.hadoop.hdds.conf.Config;
import org.apache.hadoop.hdds.conf.ConfigGroup;
import org.apache.hadoop.hdds.conf.ConfigTag;
import org.apache.hadoop.hdds.conf.ConfigType;
import org.apache.hadoop.hdds.conf.PostConstruct;
import org.apache.hadoop.hdds.conf.ReconfigurableConfig;
import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLType;
import org.apache.ratis.util.MemoizedSupplier;

/**
* Ozone Manager configuration.
Expand Down Expand Up @@ -81,6 +87,29 @@ public class OmConfig extends ReconfigurableConfig {
)
private long ratisBasedFinalizationTimeout = Duration.ofSeconds(30).getSeconds();

// OM Default user/group permissions
@Config(key = "user.rights",
defaultValue = "ALL",
type = ConfigType.STRING,
tags = {ConfigTag.OM, ConfigTag.SECURITY},
description = "Default user permissions set for an object in " +
"OzoneManager."
)
private String userDefaultRights;
private final Supplier<List<ACLType>> userDefaultRightList =
MemoizedSupplier.valueOf(this::getUserDefaultRightList);

@Config(key = "group.rights",
defaultValue = "READ, LIST",
type = ConfigType.STRING,
tags = {ConfigTag.OM, ConfigTag.SECURITY},
description = "Default group permissions set for an object in " +
"OzoneManager."
)
private String groupDefaultRights;
private final Supplier<List<ACLType>> groupDefaultRightList =
MemoizedSupplier.valueOf(this::getGroupDefaultRightList);

public long getRatisBasedFinalizationTimeout() {
return ratisBasedFinalizationTimeout;
}
Expand Down Expand Up @@ -111,6 +140,26 @@ public void setMaxUserVolumeCount(int newValue) {
validate();
}

public ACLType[] getUserDefaultRights() {
return userDefaultRightList.get().toArray(new ACLType[0]);
}

private List<ACLType> getUserDefaultRightList() {
return userDefaultRights == null
? Collections.singletonList(ACLType.ALL)
: ACLType.parseList(userDefaultRights);
}

public ACLType[] getGroupDefaultRights() {
return groupDefaultRightList.get().toArray(new ACLType[0]);
}

private List<ACLType> getGroupDefaultRightList() {
return groupDefaultRights == null
? Collections.unmodifiableList(Arrays.asList(ACLType.READ, ACLType.LIST))
: ACLType.parseList(groupDefaultRights);
}

@PostConstruct
public void validate() {
if (maxListSize <= 0) {
Expand All @@ -131,6 +180,8 @@ public void setFrom(OmConfig other) {
fileSystemPathEnabled = other.fileSystemPathEnabled;
maxListSize = other.maxListSize;
maxUserVolumeCount = other.maxUserVolumeCount;
userDefaultRights = other.userDefaultRights;
groupDefaultRights = other.groupDefaultRights;
}

/**
Expand Down
15 changes: 4 additions & 11 deletions hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OzoneAclUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,12 @@
import java.util.Objects;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import org.apache.hadoop.hdds.conf.OzoneConfiguration;
import org.apache.hadoop.ozone.OzoneAcl;
import org.apache.hadoop.ozone.om.OmConfig;
import org.apache.hadoop.ozone.om.exceptions.OMException;
import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OzoneAclInfo;
import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLType;
import org.apache.hadoop.ozone.security.acl.OzoneAclConfig;
import org.apache.hadoop.ozone.security.acl.RequestContext;
import org.apache.hadoop.security.UserGroupInformation;
import org.slf4j.Logger;
Expand All @@ -46,9 +45,6 @@
public final class OzoneAclUtil {
static final Logger LOG = LoggerFactory.getLogger(OzoneAclUtil.class);

private static ACLType[] userRights;

@ChenSammi ChenSammi May 12, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer keep these two global variables, to avoid creating the short lived collections object and array during each create key request. Others look good to me.

@adoroszlai adoroszlai May 12, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @ChenSammi for the review. Collections object is not short-lived, because OzoneManager keeps a single instance of OmConfig, which uses the MemoizedSupplier to remember the List<ACLType collection. Only the result of toArray() is short-lived. I'll update the PR to avoid that.

private static ACLType[] groupRights;

private OzoneAclUtil() {
}

Expand All @@ -59,13 +55,10 @@ private OzoneAclUtil() {
* @param conf current configuration
* @return list of OzoneAcls
* */
public static List<OzoneAcl> getDefaultAclList(UserGroupInformation ugi, OzoneConfiguration conf) {
public static List<OzoneAcl> getDefaultAclList(UserGroupInformation ugi, OmConfig conf) {
// Get default acl rights for user and group.
if (userRights == null || groupRights == null) {
OzoneAclConfig aclConfig = conf.getObject(OzoneAclConfig.class);
userRights = aclConfig.getUserDefaultRights();
groupRights = aclConfig.getGroupDefaultRights();
}
ACLType[] userRights = conf.getUserDefaultRights();
ACLType[] groupRights = conf.getGroupDefaultRights();
List<OzoneAcl> listOfAcls = new ArrayList<>();
// User ACL.
listOfAcls.add(OzoneAcl.of(USER, ugi.getShortUserName(), ACCESS, userRights));
Expand Down
16 changes: 15 additions & 1 deletion ...op-ozone/common/src/main/java/org/apache/hadoop/ozone/security/acl/IAccessAuthorizer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,13 @@

package org.apache.hadoop.ozone.security.acl;

import static java.util.stream.Collectors.toList;

import java.util.Arrays;
import java.util.BitSet;
import java.util.Collections;
import java.util.List;
import java.util.Objects;
import org.apache.hadoop.hdds.annotation.InterfaceAudience;
import org.apache.hadoop.hdds.annotation.InterfaceStability;
import org.apache.hadoop.ozone.OzoneConsts;
Expand Down Expand Up @@ -119,7 +125,6 @@ public static ACLType getACLRight(String type) {
throw new IllegalArgumentException("[" + type + "] ACL right is not " +
"recognized");
}

}

/**
Expand Down Expand Up @@ -160,6 +165,15 @@ public static String getAclString(ACLType acl) {
throw new IllegalArgumentException("ACL right is not recognized");
}
}

public static List<ACLType> parseList(String conf) {
String[] array = Objects.requireNonNull(conf, "conf == null")
.trim()
.split(",");
return Collections.unmodifiableList(Arrays.stream(array)
.map(each -> ACLType.valueOf(each.trim()))
.collect(toList()));
}
}

/**
Expand Down
76 changes: 0 additions & 76 deletions hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/security/acl/OzoneAclConfig.java
View file
Open in desktop

This file was deleted.

8 changes: 4 additions & 4 deletions hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOzoneAclUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,9 @@
import java.util.Arrays;
import java.util.List;
import org.apache.hadoop.ozone.OzoneAcl;
import org.apache.hadoop.ozone.om.OmConfig;
import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLType;
import org.apache.hadoop.ozone.security.acl.OzoneAclConfig;
import org.apache.hadoop.security.UserGroupInformation;
import org.junit.jupiter.api.Test;

Expand Down Expand Up @@ -181,9 +181,9 @@ private static List<OzoneAcl> getDefaultAcls() {
ugi = UserGroupInformation.createRemoteUser("user0");
}

OzoneAclConfig aclConfig = newInstanceOf(OzoneAclConfig.class);
IAccessAuthorizer.ACLType[] userRights = aclConfig.getUserDefaultRights();
IAccessAuthorizer.ACLType[] groupRights = aclConfig.getGroupDefaultRights();
OmConfig omConfig = newInstanceOf(OmConfig.class);
IAccessAuthorizer.ACLType[] userRights = omConfig.getUserDefaultRights();
IAccessAuthorizer.ACLType[] groupRights = omConfig.getGroupDefaultRights();

OzoneAclUtil.addAcl(ozoneAcls, OzoneAcl.of(USER,
ugi.getUserName(), ACCESS, userRights));
Expand Down
14 changes: 7 additions & 7 deletions ...tion-test/src/test/java/org/apache/hadoop/fs/ozone/AbstractRootedOzoneFileSystemTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,13 +116,13 @@
import org.apache.hadoop.ozone.client.protocol.ClientProtocol;
import org.apache.hadoop.ozone.om.OMConfigKeys;
import org.apache.hadoop.ozone.om.OMMetrics;
import org.apache.hadoop.ozone.om.OmConfig;
import org.apache.hadoop.ozone.om.TrashPolicyOzone;
import org.apache.hadoop.ozone.om.exceptions.OMException;
import org.apache.hadoop.ozone.om.helpers.BucketLayout;
import org.apache.hadoop.ozone.om.helpers.QuotaUtil;
import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLIdentityType;
import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLType;
import org.apache.hadoop.ozone.security.acl.OzoneAclConfig;
import org.apache.hadoop.security.UserGroupInformation;
import org.apache.hadoop.tools.DistCp;
import org.apache.hadoop.tools.DistCpOptions;
Expand Down Expand Up @@ -1190,8 +1190,8 @@ void testSharedTmpDir() throws IOException {
// Use ClientProtocol to pass in volume ACL, ObjectStore won't do it
ClientProtocol proxy = objectStore.getClientProxy();
// Get default acl rights for user
OzoneAclConfig aclConfig = conf.getObject(OzoneAclConfig.class);
ACLType[] userRights = aclConfig.getUserDefaultRights();
OmConfig omConfig = cluster.getOzoneManager().getConfig();
ACLType[] userRights = omConfig.getUserDefaultRights();
// Construct ACL for world access
// ACL admin owner, world read+write
EnumSet<ACLType> aclRights = EnumSet.of(READ, WRITE);
Expand Down Expand Up @@ -1293,8 +1293,8 @@ void testTempMount() throws IOException {
// Use ClientProtocol to pass in volume ACL, ObjectStore won't do it
ClientProtocol proxy = objectStore.getClientProxy();
// Get default acl rights for user
OzoneAclConfig aclConfig = conf.getObject(OzoneAclConfig.class);
ACLType[] userRights = aclConfig.getUserDefaultRights();
OmConfig omConfig = cluster.getOzoneManager().getConfig();
ACLType[] userRights = omConfig.getUserDefaultRights();
// Construct ACL for world access
OzoneAcl aclWorldAccess = OzoneAcl.of(ACLIdentityType.WORLD, "",
ACCESS, userRights);
Expand Down Expand Up @@ -2273,8 +2273,8 @@ void testNonPrivilegedUserMkdirCreateBucket() throws IOException {
ClientProtocol proxy = objectStore.getClientProxy();

// Get default acl rights for user
OzoneAclConfig aclConfig = conf.getObject(OzoneAclConfig.class);
ACLType[] userRights = aclConfig.getUserDefaultRights();
OmConfig omConfig = cluster.getOzoneManager().getConfig();
ACLType[] userRights = omConfig.getUserDefaultRights();
// Construct ACL for world access
OzoneAcl aclWorldAccess = OzoneAcl.of(ACLIdentityType.WORLD, "",
ACCESS, userRights);
Expand Down
8 changes: 4 additions & 4 deletions ...ntegration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/OzoneRpcClientTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@
import org.apache.hadoop.ozone.container.keyvalue.KeyValueContainerData;
import org.apache.hadoop.ozone.container.keyvalue.helpers.BlockUtils;
import org.apache.hadoop.ozone.om.OMMetadataManager;
import org.apache.hadoop.ozone.om.OmConfig;
import org.apache.hadoop.ozone.om.OmFailoverProxyUtil;
import org.apache.hadoop.ozone.om.OzoneManager;
import org.apache.hadoop.ozone.om.ResolvedBucket;
Expand All @@ -187,7 +188,6 @@
import org.apache.hadoop.ozone.om.ratis.OzoneManagerStateMachine;
import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos;
import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLType;
import org.apache.hadoop.ozone.security.acl.OzoneAclConfig;
import org.apache.hadoop.ozone.security.acl.OzoneObj;
import org.apache.hadoop.ozone.security.acl.OzoneObjInfo;
import org.apache.hadoop.security.UserGroupInformation;
Expand Down Expand Up @@ -4108,9 +4108,9 @@ private List<OzoneAcl> getAclList(OzoneConfiguration conf)
List<OzoneAcl> listOfAcls = new ArrayList<>();
//User ACL
UserGroupInformation ugi = UserGroupInformation.getCurrentUser();
OzoneAclConfig aclConfig = conf.getObject(OzoneAclConfig.class);
ACLType[] userRights = aclConfig.getUserDefaultRights();
ACLType[] groupRights = aclConfig.getGroupDefaultRights();
OmConfig omConfig = conf.getObject(OmConfig.class);
ACLType[] userRights = omConfig.getUserDefaultRights();
ACLType[] groupRights = omConfig.getGroupDefaultRights();

listOfAcls.add(OzoneAcl.of(USER, ugi.getShortUserName(), ACCESS, userRights));
//Group ACL of the User
Expand Down
19 changes: 9 additions & 10 deletions ...ne/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestRecursiveAclWithFSO.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@
import org.apache.hadoop.ozone.client.OzoneVolume;
import org.apache.hadoop.ozone.client.protocol.ClientProtocol;
import org.apache.hadoop.ozone.om.exceptions.OMException;
import org.apache.hadoop.ozone.security.acl.OzoneAclConfig;
import org.apache.hadoop.ozone.security.acl.OzoneObj;
import org.apache.hadoop.ozone.security.acl.OzoneObjInfo;
import org.apache.hadoop.security.UserGroupInformation;
Expand Down Expand Up @@ -213,12 +212,12 @@ public void testKeyDefaultACL() throws Exception {
List<OzoneAcl> acls = objectStore.getAcl(obj);
assertEquals(3, acls.size());
assertEquals(AclTests.ADMIN_UGI.getShortUserName(), acls.get(0).getName());
OzoneAclConfig aclConfig = cluster().getConf().getObject(OzoneAclConfig.class);
assertArrayEquals(aclConfig.getUserDefaultRights(), acls.get(0).getAclList().toArray());
OmConfig omConfig = cluster().getOzoneManager().getConfig();
assertArrayEquals(omConfig.getUserDefaultRights(), acls.get(0).getAclList().toArray());
assertEquals(AclTests.ADMIN_UGI.getPrimaryGroupName(), acls.get(1).getName());
assertArrayEquals(aclConfig.getGroupDefaultRights(), acls.get(1).getAclList().toArray());
assertArrayEquals(omConfig.getGroupDefaultRights(), acls.get(1).getAclList().toArray());
assertEquals("WORLD", acls.get(2).getName());
assertArrayEquals(aclConfig.getUserDefaultRights(), acls.get(2).getAclList().toArray());
assertArrayEquals(omConfig.getUserDefaultRights(), acls.get(2).getAclList().toArray());
}

// set LoginUser as user3
Expand All @@ -239,10 +238,10 @@ public void testKeyDefaultACL() throws Exception {
List<OzoneAcl> acls = objectStore.getAcl(obj);
assertEquals(2, acls.size());
assertEquals(user3.getShortUserName(), acls.get(0).getName());
OzoneAclConfig aclConfig = cluster().getConf().getObject(OzoneAclConfig.class);
assertArrayEquals(aclConfig.getUserDefaultRights(), acls.get(0).getAclList().toArray());
OmConfig omConfig = cluster().getOzoneManager().getConfig();
assertArrayEquals(omConfig.getUserDefaultRights(), acls.get(0).getAclList().toArray());
assertEquals(user3.getPrimaryGroupName(), acls.get(1).getName());
assertArrayEquals(aclConfig.getGroupDefaultRights(), acls.get(1).getAclList().toArray());
assertArrayEquals(omConfig.getGroupDefaultRights(), acls.get(1).getAclList().toArray());

// verify key default ACLs
int length = 10;
Expand All @@ -256,9 +255,9 @@ public void testKeyDefaultACL() throws Exception {
acls = objectStore.getAcl(obj);
assertEquals(2, acls.size());
assertEquals(user3.getShortUserName(), acls.get(0).getName());
assertArrayEquals(aclConfig.getUserDefaultRights(), acls.get(0).getAclList().toArray());
assertArrayEquals(omConfig.getUserDefaultRights(), acls.get(0).getAclList().toArray());
assertEquals(user3.getPrimaryGroupName(), acls.get(1).getName());
assertArrayEquals(aclConfig.getGroupDefaultRights(), acls.get(1).getAclList().toArray());
assertArrayEquals(omConfig.getGroupDefaultRights(), acls.get(1).getAclList().toArray());
}
}

Expand Down
Loading