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 all 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
7 changes: 6 additions & 1 deletion hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/OzoneAcl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import java.util.EnumSet;
import java.util.List;
import java.util.Objects;
import java.util.Set;
import java.util.function.Function;
import java.util.function.IntFunction;
import java.util.function.Supplier;
Expand Down Expand Up @@ -81,7 +82,7 @@ public static OzoneAcl of(ACLIdentityType type, String name, AclScope scope, ACL
return new OzoneAcl(type, name, scope, toInt(acls));
}

public static OzoneAcl of(ACLIdentityType type, String name, AclScope scope, EnumSet<ACLType> acls) {
public static OzoneAcl of(ACLIdentityType type, String name, AclScope scope, Set<ACLType> acls) {
return new OzoneAcl(type, name, scope, toInt(acls));
}

Expand Down Expand Up @@ -321,6 +322,10 @@ public List<ACLType> getAclList() {
return getAclList(aclBits, Function.identity());
}

public Set<ACLType> getAclSet() {
return Collections.unmodifiableSet(EnumSet.copyOf(getAclList()));
}

private static <T> List<T> getAclList(int aclBits, Function<ACLType, T> converter) {
if (aclBits == 0) {
return Collections.emptyList();
Expand Down
58 changes: 58 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,16 @@

import com.google.common.base.Preconditions;
import java.time.Duration;
import java.util.Collections;
import java.util.EnumSet;
import java.util.Set;
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;

/**
* Ozone Manager configuration.
Expand Down Expand Up @@ -81,6 +85,27 @@ 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 Set<ACLType> userDefaultRightSet;

@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 Set<ACLType> groupDefaultRightSet;

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

public Set<ACLType> getUserDefaultRights() {
if (userDefaultRightSet == null) {
userDefaultRightSet = getUserDefaultRightSet();
}
return userDefaultRightSet;
}

private Set<ACLType> getUserDefaultRightSet() {
return userDefaultRights == null
? Collections.singleton(ACLType.ALL)
: ACLType.parseList(userDefaultRights);
}

public Set<ACLType> getGroupDefaultRights() {
if (groupDefaultRightSet == null) {
groupDefaultRightSet = getGroupDefaultRightSet();
}
return groupDefaultRightSet;
}

private Set<ACLType> getGroupDefaultRightSet() {
return groupDefaultRights == null
? Collections.unmodifiableSet(EnumSet.of(ACLType.READ, ACLType.LIST))
: ACLType.parseList(groupDefaultRights);
}

@PostConstruct
public void validate() {
if (maxListSize <= 0) {
Expand All @@ -119,6 +170,9 @@ public void validate() {

Preconditions.checkArgument(this.maxUserVolumeCount > 0,
Keys.USER_MAX_VOLUME + " value should be greater than zero");

userDefaultRightSet = getUserDefaultRightSet();
groupDefaultRightSet = getGroupDefaultRightSet();
}

public OmConfig copy() {
Expand All @@ -131,6 +185,10 @@ public void setFrom(OmConfig other) {
fileSystemPathEnabled = other.fileSystemPathEnabled;
maxListSize = other.maxListSize;
maxUserVolumeCount = other.maxUserVolumeCount;
userDefaultRights = other.userDefaultRights;
groupDefaultRights = other.groupDefaultRights;

validate();
}

/**
Expand Down
17 changes: 4 additions & 13 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;
Copy link
Contributor

@ChenSammi ChenSammi May 12, 2025

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.

Copy link
Contributor Author

@adoroszlai adoroszlai May 12, 2025

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,19 +55,14 @@ 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();
}
List<OzoneAcl> listOfAcls = new ArrayList<>();
// User ACL.
listOfAcls.add(OzoneAcl.of(USER, ugi.getShortUserName(), ACCESS, userRights));
listOfAcls.add(OzoneAcl.of(USER, ugi.getShortUserName(), ACCESS, conf.getUserDefaultRights()));
try {
String groupName = ugi.getPrimaryGroupName();
listOfAcls.add(OzoneAcl.of(GROUP, groupName, ACCESS, groupRights));
listOfAcls.add(OzoneAcl.of(GROUP, groupName, ACCESS, conf.getGroupDefaultRights()));
} catch (IOException e) {
// do nothing, since user has the permission, user can add ACL for selected groups later.
LOG.warn("Failed to get primary group from user {}", ugi);
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 java.util.Arrays;
import java.util.BitSet;
import java.util.Collections;
import java.util.EnumSet;
import java.util.Objects;
import java.util.Set;
import java.util.stream.Collectors;
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 Set<ACLType> parseList(String conf) {
String[] array = Objects.requireNonNull(conf, "conf == null")
.trim()
.split(",");
return Collections.unmodifiableSet(Arrays.stream(array)
.map(each -> ACLType.valueOf(each.trim()))
.collect(Collectors.toCollection(() -> EnumSet.noneOf(ACLType.class))));
}
}

/**
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.

11 changes: 4 additions & 7 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,8 @@
import java.util.Arrays;
import java.util.List;
import org.apache.hadoop.ozone.OzoneAcl;
import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
import org.apache.hadoop.ozone.om.OmConfig;
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,16 +180,14 @@ 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);

OzoneAclUtil.addAcl(ozoneAcls, OzoneAcl.of(USER,
ugi.getUserName(), ACCESS, userRights));
ugi.getUserName(), ACCESS, omConfig.getUserDefaultRights()));
//Group ACLs of the User
List<String> userGroups = Arrays.asList(ugi.getGroupNames());
userGroups.stream().forEach((group) -> OzoneAclUtil.addAcl(ozoneAcls,
OzoneAcl.of(GROUP, group, ACCESS, groupRights)));
OzoneAcl.of(GROUP, group, ACCESS, omConfig.getGroupDefaultRights())));
return ozoneAcls;
}

Expand Down
19 changes: 8 additions & 11 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,7 @@ 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();
// Construct ACL for world access
// ACL admin owner, world read+write
EnumSet<ACLType> aclRights = EnumSet.of(READ, WRITE);
Expand All @@ -1202,7 +1201,7 @@ void testSharedTmpDir() throws IOException {
.setAdmin("admin")
.setOwner("admin")
.addAcl(OzoneAcl.of(ACLIdentityType.WORLD, "", ACCESS, aclRights))
.addAcl(OzoneAcl.of(ACLIdentityType.USER, "admin", ACCESS, userRights))
.addAcl(OzoneAcl.of(ACLIdentityType.USER, "admin", ACCESS, omConfig.getUserDefaultRights()))
.setQuotaInNamespace(1000)
.setQuotaInBytes(Long.MAX_VALUE).build();
// Sanity check
Expand Down Expand Up @@ -1237,7 +1236,7 @@ void testSharedTmpDir() throws IOException {
BucketArgs bucketArgs = new BucketArgs.Builder()
.setOwner("admin")
.addAcl(OzoneAcl.of(ACLIdentityType.WORLD, "", ACCESS, READ, WRITE, LIST))
.addAcl(OzoneAcl.of(ACLIdentityType.USER, "admin", ACCESS, userRights))
.addAcl(OzoneAcl.of(ACLIdentityType.USER, "admin", ACCESS, omConfig.getUserDefaultRights()))
.setQuotaInNamespace(1000)
.setQuotaInBytes(Long.MAX_VALUE).build();

Expand Down Expand Up @@ -1293,11 +1292,10 @@ 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();
// Construct ACL for world access
OzoneAcl aclWorldAccess = OzoneAcl.of(ACLIdentityType.WORLD, "",
ACCESS, userRights);
ACCESS, omConfig.getUserDefaultRights());
// Construct VolumeArgs
VolumeArgs volumeArgs = VolumeArgs.newBuilder()
.addAcl(aclWorldAccess)
Expand Down Expand Up @@ -2273,11 +2271,10 @@ 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();
// Construct ACL for world access
OzoneAcl aclWorldAccess = OzoneAcl.of(ACLIdentityType.WORLD, "",
ACCESS, userRights);
ACCESS, omConfig.getUserDefaultRights());
// Construct VolumeArgs, set ACL to world access
VolumeArgs volumeArgs = VolumeArgs.newBuilder()
.addAcl(aclWorldAccess)
Expand Down
Loading