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 d4570c50a80 HDDS-14111. Make OmBucketInfo ACL list immutable (#9466)
d4570c50a80 is described below
commit d4570c50a807a25a07c2762473bd05db93c5cb90
Author: Doroszlai, Attila <[email protected]>
AuthorDate: Wed Dec 10 15:26:38 2025 +0100
HDDS-14111. Make OmBucketInfo ACL list immutable (#9466)
---
.../apache/hadoop/ozone/client/rpc/RpcClient.java | 2 +-
.../hadoop/ozone/om/helpers/AclListBuilder.java | 31 +++++---
.../hadoop/ozone/om/helpers/OmBucketInfo.java | 48 +++----------
.../hadoop/ozone/om/helpers/OzoneAclUtil.java | 18 ++++-
.../ozone/om/helpers/TestAclListBuilder.java | 84 +++++++++++-----------
.../hadoop/ozone/om/helpers/TestOmBucketInfo.java | 21 ------
.../ozone/om/TestOzoneManagerHAWithAllRunning.java | 4 +-
.../om/request/bucket/OMBucketCreateRequest.java | 32 ++++-----
.../om/request/bucket/acl/OMBucketAclRequest.java | 13 ++--
.../request/bucket/acl/OMBucketAddAclRequest.java | 8 +--
.../bucket/acl/OMBucketRemoveAclRequest.java | 9 ++-
.../request/bucket/acl/OMBucketSetAclRequest.java | 10 +--
.../apache/hadoop/ozone/om/request/util/AclOp.java | 30 ++++++++
.../om/request/volume/acl/OMVolumeAclRequest.java | 13 +---
.../request/volume/acl/OMVolumeAddAclRequest.java | 1 +
.../volume/acl/OMVolumeRemoveAclRequest.java | 1 +
.../request/volume/acl/OMVolumeSetAclRequest.java | 1 +
.../apache/hadoop/ozone/om/TestKeyManagerUnit.java | 1 -
.../ozone/om/response/TestCleanupTableInfo.java | 2 -
.../ozone/security/acl/OzoneNativeAclTestUtil.java | 14 ++--
20 files changed, 165 insertions(+), 178 deletions(-)
diff --git
a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
index 254f686eb24..5fce2b83fd5 100644
---
a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
+++
b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
@@ -658,7 +658,7 @@ public void createBucket(
.setOwner(owner);
if (bucketArgs.getAcls() != null) {
- builder.setAcls(bucketArgs.getAcls());
+ builder.acls().addAll(bucketArgs.getAcls());
}
// Link bucket default acl
diff --git
a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/AclListBuilder.java
b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/AclListBuilder.java
index 436885a83a7..ae2ff885762 100644
---
a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/AclListBuilder.java
+++
b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/AclListBuilder.java
@@ -59,31 +59,39 @@ public ImmutableList<OzoneAcl> build() {
return changed ? ImmutableList.copyOf(updatedList) : originalList;
}
- public void add(@Nonnull OzoneAcl acl) {
+ public boolean add(@Nonnull OzoneAcl acl) {
Objects.requireNonNull(acl, "acl == null");
ensureInitialized();
- changed |= OzoneAclUtil.addAcl(updatedList, acl);
+ boolean added = OzoneAclUtil.addAcl(updatedList, acl);
+ changed |= added;
+ return added;
}
- public void addAll(@Nullable List<OzoneAcl> newAcls) {
+ public boolean addAll(@Nullable List<OzoneAcl> newAcls) {
if (newAcls == null || newAcls.isEmpty()) {
- return;
+ return false;
}
ensureInitialized();
- changed |= OzoneAclUtil.addAllAcl(updatedList, newAcls);
+ boolean added = OzoneAclUtil.addAllAcl(updatedList, newAcls);
+ changed |= added;
+ return added;
}
/** Set the list being built to {@code acls}. For further mutations to
work, it must be modifiable. */
- public void set(@Nonnull List<OzoneAcl> acls) {
+ public boolean set(@Nonnull List<OzoneAcl> acls) {
Objects.requireNonNull(acls, "acls == null");
- changed |= !acls.equals(updatedList != null ? updatedList : originalList);
+ boolean set = !acls.equals(updatedList != null ? updatedList :
originalList);
+ changed |= set;
updatedList = acls;
+ return set;
}
- public void remove(@Nonnull OzoneAcl acl) {
+ public boolean remove(@Nonnull OzoneAcl acl) {
Objects.requireNonNull(acl, "acl == null");
ensureInitialized();
- changed |= OzoneAclUtil.removeAcl(updatedList, acl);
+ boolean removed = OzoneAclUtil.removeAcl(updatedList, acl);
+ changed |= removed;
+ return removed;
}
private void ensureInitialized() {
@@ -91,4 +99,9 @@ private void ensureInitialized() {
updatedList = new ArrayList<>(originalList);
}
}
+
+ @Override
+ public String toString() {
+ return build().toString();
+ }
}
diff --git
a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketInfo.java
b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketInfo.java
index 8b91b55859d..b8d8f409894 100644
---
a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketInfo.java
+++
b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketInfo.java
@@ -18,12 +18,10 @@
package org.apache.hadoop.ozone.om.helpers;
import com.google.common.collect.ImmutableList;
-import java.util.ArrayList;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
-import java.util.concurrent.CopyOnWriteArrayList;
import java.util.stream.Collectors;
import org.apache.hadoop.hdds.client.DefaultReplicationConfig;
import org.apache.hadoop.hdds.protocol.StorageType;
@@ -56,9 +54,9 @@ public final class OmBucketInfo extends WithObjectID
implements Auditable, CopyO
*/
private final String bucketName;
/**
- * ACL Information (mutable).
+ * ACL Information.
*/
- private final CopyOnWriteArrayList<OzoneAcl> acls;
+ private final ImmutableList<OzoneAcl> acls;
/**
* Bucket Version flag.
*/
@@ -113,7 +111,7 @@ private OmBucketInfo(Builder b) {
super(b);
this.volumeName = b.volumeName;
this.bucketName = b.bucketName;
- this.acls = new CopyOnWriteArrayList<>(b.acls);
+ this.acls = b.acls.build();
this.isVersionEnabled = b.isVersionEnabled;
this.storageType = b.storageType;
this.creationTime = b.creationTime;
@@ -157,36 +155,7 @@ public String getBucketName() {
* @return {@literal List<OzoneAcl>}
*/
public List<OzoneAcl> getAcls() {
- return ImmutableList.copyOf(acls);
- }
-
- /**
- * Add an ozoneAcl to list of existing Acl set.
- * @param ozoneAcl
- * @return true - if successfully added, false if not added or acl is
- * already existing in the acl list.
- */
- public boolean addAcl(OzoneAcl ozoneAcl) {
- return OzoneAclUtil.addAcl(acls, ozoneAcl);
- }
-
- /**
- * Remove acl from existing acl list.
- * @param ozoneAcl
- * @return true - if successfully removed, false if not able to remove due
- * to that acl is not in the existing acl list.
- */
- public boolean removeAcl(OzoneAcl ozoneAcl) {
- return OzoneAclUtil.removeAcl(acls, ozoneAcl);
- }
-
- /**
- * Reset the existing acl list.
- * @param ozoneAcls
- * @return true - if successfully able to reset.
- */
- public boolean setAcls(List<OzoneAcl> ozoneAcls) {
- return OzoneAclUtil.setAcl(acls, ozoneAcls);
+ return acls;
}
/**
@@ -411,7 +380,6 @@ public Builder toBuilder() {
.setBucketEncryptionKey(bekInfo)
.setSourceVolume(sourceVolume)
.setSourceBucket(sourceBucket)
- .setAcls(acls)
.setUsedBytes(usedBytes)
.setUsedNamespace(usedNamespace)
.setQuotaInBytes(quotaInBytes)
@@ -429,7 +397,7 @@ public Builder toBuilder() {
public static class Builder extends WithObjectID.Builder<OmBucketInfo> {
private String volumeName;
private String bucketName;
- private final List<OzoneAcl> acls = new ArrayList<>();
+ private final AclListBuilder acls;
private boolean isVersionEnabled;
private StorageType storageType = StorageType.DISK;
private long creationTime;
@@ -448,10 +416,12 @@ public static class Builder extends
WithObjectID.Builder<OmBucketInfo> {
private long snapshotUsedNamespace;
public Builder() {
+ acls = AclListBuilder.empty();
}
private Builder(OmBucketInfo obj) {
super(obj);
+ acls = AclListBuilder.of(obj.acls);
}
public Builder setVolumeName(String volume) {
@@ -466,12 +436,12 @@ public Builder setBucketName(String bucket) {
public Builder setAcls(List<OzoneAcl> listOfAcls) {
if (listOfAcls != null) {
- this.acls.addAll(listOfAcls);
+ this.acls.set(listOfAcls);
}
return this;
}
- public List<OzoneAcl> getAcls() {
+ public AclListBuilder acls() {
return acls;
}
diff --git
a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OzoneAclUtil.java
b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OzoneAclUtil.java
index e2b3cd04db2..3dae69f7110 100644
---
a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OzoneAclUtil.java
+++
b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OzoneAclUtil.java
@@ -26,6 +26,7 @@
import java.util.ArrayList;
import java.util.List;
import java.util.Objects;
+import java.util.function.Predicate;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import org.apache.hadoop.ozone.OzoneAcl;
@@ -150,6 +151,11 @@ public static boolean checkAclRights(List<OzoneAcl> acls,
return false;
}
+ public static boolean inheritDefaultAcls(AclListBuilder acls,
+ List<OzoneAcl> parentAcls, OzoneAcl.AclScope scope) {
+ return inheritDefaultAcls(acls::add, parentAcls, scope);
+ }
+
/**
* Helper function to inherit default ACL with given {@code scope} for child
object.
* @param acls child object ACL list
@@ -159,6 +165,11 @@ public static boolean checkAclRights(List<OzoneAcl> acls,
*/
public static boolean inheritDefaultAcls(List<OzoneAcl> acls,
List<OzoneAcl> parentAcls, OzoneAcl.AclScope scope) {
+ return inheritDefaultAcls(acl -> addAcl(acls, acl), parentAcls, scope);
+ }
+
+ private static boolean inheritDefaultAcls(Predicate<OzoneAcl> op,
+ List<OzoneAcl> parentAcls, OzoneAcl.AclScope scope) {
if (parentAcls != null && !parentAcls.isEmpty()) {
Stream<OzoneAcl> aclStream = parentAcls.stream()
.filter(a -> a.getAclScope() == DEFAULT);
@@ -168,10 +179,11 @@ public static boolean inheritDefaultAcls(List<OzoneAcl>
acls,
}
List<OzoneAcl> inheritedAcls = aclStream.collect(Collectors.toList());
- if (!inheritedAcls.isEmpty()) {
- inheritedAcls.forEach(acl -> addAcl(acls, acl));
- return true;
+ boolean changed = false;
+ for (OzoneAcl acl : inheritedAcls) {
+ changed |= op.test(acl);
}
+ return changed;
}
return false;
diff --git
a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestAclListBuilder.java
b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestAclListBuilder.java
index d8f6cf41345..5e8ceff057b 100644
---
a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestAclListBuilder.java
+++
b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestAclListBuilder.java
@@ -18,7 +18,6 @@
package org.apache.hadoop.ozone.om.helpers;
import static java.util.Arrays.asList;
-import static java.util.Collections.emptyList;
import static java.util.Collections.singletonList;
import static org.apache.hadoop.ozone.OzoneAcl.AclScope.ACCESS;
import static
org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLIdentityType.USER;
@@ -29,7 +28,7 @@
import com.google.common.collect.ImmutableList;
import java.util.ArrayList;
import java.util.List;
-import java.util.function.Consumer;
+import java.util.function.Predicate;
import java.util.stream.Stream;
import org.apache.hadoop.ozone.OzoneAcl;
import org.junit.jupiter.api.Test;
@@ -43,28 +42,21 @@ class TestAclListBuilder {
private static final OzoneAcl ALICE_READ_WRITE = ALICE_READ.add(ALICE_WRITE);
private static final OzoneAcl BOB_READ = OzoneAcl.of(USER, "bob", ACCESS,
READ);
- public static Stream<List<OzoneAcl>> initialLists() {
+ public static Stream<ImmutableList<OzoneAcl>> initialLists() {
return Stream.of(
- emptyList(),
- singletonList(ALICE_READ),
- asList(ALICE_READ, ALICE_WRITE)
+ ImmutableList.of(),
+ ImmutableList.of(ALICE_READ),
+ ImmutableList.of(ALICE_READ, ALICE_WRITE)
);
}
- @Test
- void returnsInitialListIfUnchanged() {
- ImmutableList<OzoneAcl> list = ImmutableList.copyOf(asList(ALICE_READ,
ALICE_WRITE));
- AclListBuilder subject = AclListBuilder.of(list);
-
- assertThat(subject.build())
- .isSameAs(list);
- }
-
@Test
void testAdd() {
testAdd(subject -> {
- subject.add(ALICE_WRITE);
- subject.add(BOB_READ);
+ boolean added = false;
+ added |= subject.add(ALICE_WRITE);
+ added |= subject.add(BOB_READ);
+ return added;
});
}
@@ -74,13 +66,10 @@ void testAddAll() {
}
// op should be adding ALICE_WRITE and BOB_READ
- private void testAdd(Consumer<AclListBuilder> op) {
+ private void testAdd(Predicate<AclListBuilder> op) {
AclListBuilder subject = AclListBuilder.copyOf(singletonList(ALICE_READ));
- op.accept(subject);
-
- assertThat(subject.isChanged())
- .isTrue();
+ assertChangedBy(subject, op);
assertThat(subject.build())
.contains(ALICE_READ_WRITE)
.contains(BOB_READ)
@@ -89,28 +78,22 @@ private void testAdd(Consumer<AclListBuilder> op) {
@ParameterizedTest
@MethodSource("initialLists")
- void testSetSame(List<OzoneAcl> initialList) {
- AclListBuilder subject = AclListBuilder.copyOf(initialList);
-
- subject.set(initialList);
+ void testSetSame(ImmutableList<OzoneAcl> initialList) {
+ AclListBuilder subject = AclListBuilder.of(initialList);
- assertThat(subject.isChanged())
- .isFalse();
+ assertUnchangedBy(subject, b -> b.set(initialList));
assertThat(subject.build())
- .isEqualTo(initialList);
+ .isSameAs(initialList);
}
@ParameterizedTest
@MethodSource("initialLists")
- void testSetEqual(List<OzoneAcl> initialList) {
- AclListBuilder subject = AclListBuilder.copyOf(initialList);
-
- subject.set(new ArrayList<>(initialList));
+ void testSetEqual(ImmutableList<OzoneAcl> initialList) {
+ AclListBuilder subject = AclListBuilder.of(initialList);
- assertThat(subject.isChanged())
- .isFalse();
+ assertUnchangedBy(subject, b -> b.set(new ArrayList<>(initialList)));
assertThat(subject.build())
- .isEqualTo(initialList);
+ .isSameAs(initialList);
}
@ParameterizedTest
@@ -120,10 +103,7 @@ void testSetDifferent(List<OzoneAcl> initialList) {
List<OzoneAcl> differentList = new ArrayList<>(initialList);
differentList.add(BOB_READ);
- subject.set(differentList);
-
- assertThat(subject.isChanged())
- .isTrue();
+ assertChangedBy(subject, b -> b.set(differentList));
assertThat(subject.build())
.isEqualTo(differentList);
}
@@ -132,12 +112,28 @@ void testSetDifferent(List<OzoneAcl> initialList) {
void testRemove() {
AclListBuilder subject = AclListBuilder.copyOf(asList(ALICE_READ_WRITE,
BOB_READ));
- subject.remove(ALICE_WRITE);
- subject.remove(BOB_READ);
+ assertChangedBy(subject, b -> b.remove(ALICE_WRITE));
+ assertChangedBy(subject, b -> b.remove(BOB_READ));
+ assertThat(subject.build())
+ .isEqualTo(singletonList(ALICE_READ));
+ }
+
+ private static void assertUnchangedBy(AclListBuilder subject,
Predicate<AclListBuilder> op) {
+ final boolean wasChanged = subject.isChanged();
+ assertThat(op.test(subject))
+ .isFalse();
+ assertThat(subject.isChanged())
+ .isEqualTo(wasChanged);
+ }
+ private static void assertChangedBy(AclListBuilder subject,
Predicate<AclListBuilder> op) {
+ assertThat(op.test(subject)).isTrue();
+ assertThat(subject.isChanged()).isTrue();
+
+ // already made the same change
+ assertThat(op.test(subject)).isFalse();
assertThat(subject.isChanged())
+ .describedAs("isChanged() should not be reset by no-op change")
.isTrue();
- assertThat(subject.build())
- .isEqualTo(singletonList(ALICE_READ));
}
}
diff --git
a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOmBucketInfo.java
b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOmBucketInfo.java
index 709c6fa17c4..857103a20c0 100644
---
a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOmBucketInfo.java
+++
b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOmBucketInfo.java
@@ -19,7 +19,6 @@
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
-import static org.junit.jupiter.api.Assertions.assertNotEquals;
import static org.junit.jupiter.api.Assertions.assertNotSame;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
@@ -90,32 +89,12 @@ public void testClone() {
"Expected " + omBucketInfo + " and " + cloneBucketInfo
+ " to be equal");
- /* Reset acl & check not equal. */
- omBucketInfo.setAcls(Collections.singletonList(OzoneAcl.of(
- IAccessAuthorizer.ACLIdentityType.USER,
- "newUser",
- OzoneAcl.AclScope.ACCESS, IAccessAuthorizer.ACLType.WRITE_ACL
- )));
- assertNotEquals(
- omBucketInfo.getAcls().get(0),
- cloneBucketInfo.getAcls().get(0));
-
/* Clone acl & check equal. */
cloneBucketInfo = omBucketInfo.copyObject();
assertEquals(omBucketInfo, cloneBucketInfo);
assertEquals(
omBucketInfo.getAcls().get(0),
cloneBucketInfo.getAcls().get(0));
-
- /* Remove acl & check. */
- omBucketInfo.removeAcl(OzoneAcl.of(
- IAccessAuthorizer.ACLIdentityType.USER,
- "newUser",
- OzoneAcl.AclScope.ACCESS, IAccessAuthorizer.ACLType.WRITE_ACL
- ));
- assertEquals(0, omBucketInfo.getAcls().size());
- assertEquals(1, cloneBucketInfo.getAcls().size());
-
}
@Test
diff --git
a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerHAWithAllRunning.java
b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerHAWithAllRunning.java
index ce4c778ddb6..5ba971d1967 100644
---
a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerHAWithAllRunning.java
+++
b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerHAWithAllRunning.java
@@ -743,8 +743,8 @@ void testLinkBucketRemoveBucketAcl() throws Exception {
OzoneBucket linkedBucket = linkBucket(srcBucket);
OzoneObj linkObj = buildBucketObj(linkedBucket);
OzoneObj srcObj = buildBucketObj(srcBucket);
- // As by default create will add some default acls in RpcClient.
- List<OzoneAcl> acls = getObjectStore().getAcl(linkObj);
+ // choose from src's ACLs to avoid trying to remove
OzoneAcl#LINK_BUCKET_DEFAULT_ACL
+ List<OzoneAcl> acls = getObjectStore().getAcl(srcObj);
assertFalse(acls.isEmpty());
// Remove an existing acl.
boolean removeAcl = getObjectStore().removeAcl(linkObj, acls.get(0));
diff --git
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java
index 23cbac4704a..d66b5d37081 100644
---
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java
+++
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java
@@ -26,9 +26,7 @@
import java.io.IOException;
import java.nio.file.InvalidPathException;
-import java.util.ArrayList;
import java.util.List;
-import java.util.stream.Collectors;
import org.apache.hadoop.crypto.key.KeyProviderCryptoExtension;
import org.apache.hadoop.hdds.client.DefaultReplicationConfig;
import org.apache.hadoop.hdds.utils.db.cache.CacheKey;
@@ -47,6 +45,7 @@
import org.apache.hadoop.ozone.om.exceptions.OMException;
import org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes;
import org.apache.hadoop.ozone.om.execution.flowcontrol.ExecutionContext;
+import org.apache.hadoop.ozone.om.helpers.AclListBuilder;
import org.apache.hadoop.ozone.om.helpers.BucketLayout;
import org.apache.hadoop.ozone.om.helpers.OmBucketInfo;
import org.apache.hadoop.ozone.om.helpers.OmVolumeArgs;
@@ -253,12 +252,13 @@ public OMClientResponse
validateAndUpdateCache(OzoneManager ozoneManager, Execut
}
// Add objectID and updateID
- omBucketInfo = omBucketInfo.toBuilder()
+ OmBucketInfo.Builder builder = omBucketInfo.toBuilder()
.setObjectID(ozoneManager.getObjectIdFromTxId(transactionLogIndex))
- .setUpdateID(transactionLogIndex)
- .build();
+ .setUpdateID(transactionLogIndex);
+
+ addDefaultAcls(builder.acls(), omVolumeArgs, ozoneManager);
- addDefaultAcls(omBucketInfo, omVolumeArgs, ozoneManager);
+ omBucketInfo = builder.build();
// check namespace quota
checkQuotaInNamespace(omVolumeArgs, 1L);
@@ -329,26 +329,20 @@ private boolean isECBucket(BucketInfo bucketInfo) {
/**
* Add default acls for bucket. These acls are inherited from volume
* default acl list.
- *
- * @param omBucketInfo
- * @param omVolumeArgs
*/
- private void addDefaultAcls(OmBucketInfo omBucketInfo,
+ private void addDefaultAcls(AclListBuilder bucketAcls,
OmVolumeArgs omVolumeArgs, OzoneManager ozoneManager) throws OMException
{
- List<OzoneAcl> acls = new ArrayList<>();
// Add default acls
- acls.addAll(getDefaultAclList(createUGIForApi(),
ozoneManager.getConfig()));
- if (omBucketInfo.getAcls() != null &&
!ozoneManager.getConfig().ignoreClientACLs()) {
- // Add acls for bucket creator.
- acls.addAll(omBucketInfo.getAcls());
+ List<OzoneAcl> defaultAclList = getDefaultAclList(createUGIForApi(),
ozoneManager.getConfig());
+ if (ozoneManager.getConfig().ignoreClientACLs()) {
+ bucketAcls.set(defaultAclList);
+ } else {
+ bucketAcls.addAll(defaultAclList);
}
// Add default acls from volume.
List<OzoneAcl> defaultVolumeAcls = omVolumeArgs.getDefaultAcls();
- OzoneAclUtil.inheritDefaultAcls(acls, defaultVolumeAcls, ACCESS);
- // Remove the duplicates
- acls = acls.stream().distinct().collect(Collectors.toList());
- omBucketInfo.setAcls(acls);
+ OzoneAclUtil.inheritDefaultAcls(bucketAcls, defaultVolumeAcls, ACCESS);
}
/**
diff --git
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/acl/OMBucketAclRequest.java
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/acl/OMBucketAclRequest.java
index 8f6323aa86b..6faac1a24dc 100644
---
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/acl/OMBucketAclRequest.java
+++
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/acl/OMBucketAclRequest.java
@@ -23,7 +23,6 @@
import java.nio.file.InvalidPathException;
import java.util.List;
import java.util.Map;
-import java.util.function.BiPredicate;
import org.apache.commons.lang3.tuple.Pair;
import org.apache.hadoop.hdds.utils.db.cache.CacheKey;
import org.apache.hadoop.hdds.utils.db.cache.CacheValue;
@@ -38,6 +37,7 @@
import org.apache.hadoop.ozone.om.execution.flowcontrol.ExecutionContext;
import org.apache.hadoop.ozone.om.helpers.OmBucketInfo;
import org.apache.hadoop.ozone.om.request.OMClientRequest;
+import org.apache.hadoop.ozone.om.request.util.AclOp;
import org.apache.hadoop.ozone.om.request.util.ObjectParser;
import org.apache.hadoop.ozone.om.response.OMClientResponse;
import org.apache.hadoop.ozone.om.response.bucket.acl.OMBucketAclResponse;
@@ -52,10 +52,9 @@
*/
public abstract class OMBucketAclRequest extends OMClientRequest {
- private final BiPredicate<List<OzoneAcl>, OmBucketInfo> omBucketAclOp;
+ private final AclOp omBucketAclOp;
- public OMBucketAclRequest(OMRequest omRequest,
- BiPredicate<List<OzoneAcl>, OmBucketInfo> aclOp) {
+ public OMBucketAclRequest(OMRequest omRequest, AclOp aclOp) {
super(omRequest);
omBucketAclOp = aclOp;
}
@@ -105,7 +104,9 @@ public OMClientResponse validateAndUpdateCache(OzoneManager
ozoneManager, Execut
throw new OMException(OMException.ResultCodes.BUCKET_NOT_FOUND);
}
- operationResult = omBucketAclOp.test(ozoneAcls, omBucketInfo);
+ OmBucketInfo.Builder builder = omBucketInfo.toBuilder();
+
+ operationResult = omBucketAclOp.test(ozoneAcls, builder.acls());
if (operationResult) {
// Update the modification time when updating ACLs of Bucket.
@@ -120,7 +121,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager
ozoneManager, Execut
modificationTime = getOmRequest().getRemoveAclRequest()
.getModificationTime();
}
- omBucketInfo = omBucketInfo.toBuilder()
+ omBucketInfo = builder
.setUpdateID(transactionLogIndex)
.setModificationTime(modificationTime)
.build();
diff --git
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/acl/OMBucketAddAclRequest.java
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/acl/OMBucketAddAclRequest.java
index dad5ef6efd3..1b3ce4e3f84 100644
---
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/acl/OMBucketAddAclRequest.java
+++
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/acl/OMBucketAddAclRequest.java
@@ -17,7 +17,8 @@
package org.apache.hadoop.ozone.om.request.bucket.acl;
-import com.google.common.collect.Lists;
+import static java.util.Collections.singletonList;
+
import java.io.IOException;
import java.util.List;
import java.util.Map;
@@ -67,13 +68,12 @@ public OMRequest preExecute(OzoneManager ozoneManager)
throws IOException {
}
public OMBucketAddAclRequest(OMRequest omRequest) {
- super(omRequest, (acls, omBucketInfo) -> omBucketInfo.addAcl(acls.get(0)));
+ super(omRequest, (acls, builder) -> builder.add(acls.get(0)));
OzoneManagerProtocolProtos.AddAclRequest addAclRequest =
getOmRequest().getAddAclRequest();
obj = OzoneObjInfo.fromProtobuf(addAclRequest.getObj());
path = obj.getPath();
- ozoneAcls = Lists.newArrayList(
- OzoneAcl.fromProtobuf(addAclRequest.getAcl()));
+ ozoneAcls = singletonList(OzoneAcl.fromProtobuf(addAclRequest.getAcl()));
}
@Override
diff --git
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/acl/OMBucketRemoveAclRequest.java
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/acl/OMBucketRemoveAclRequest.java
index 6dd12babe24..13839ddb58a 100644
---
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/acl/OMBucketRemoveAclRequest.java
+++
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/acl/OMBucketRemoveAclRequest.java
@@ -17,7 +17,8 @@
package org.apache.hadoop.ozone.om.request.bucket.acl;
-import com.google.common.collect.Lists;
+import static java.util.Collections.singletonList;
+
import java.io.IOException;
import java.util.List;
import java.util.Map;
@@ -66,14 +67,12 @@ public OMRequest preExecute(OzoneManager ozoneManager)
throws IOException {
}
public OMBucketRemoveAclRequest(OMRequest omRequest) {
- super(omRequest,
- (acls, omBucketInfo) -> omBucketInfo.removeAcl(acls.get(0)));
+ super(omRequest, (acls, builder) -> builder.remove(acls.get(0)));
OzoneManagerProtocolProtos.RemoveAclRequest removeAclRequest =
getOmRequest().getRemoveAclRequest();
obj = OzoneObjInfo.fromProtobuf(removeAclRequest.getObj());
path = obj.getPath();
- ozoneAcls = Lists.newArrayList(
- OzoneAcl.fromProtobuf(removeAclRequest.getAcl()));
+ ozoneAcls =
singletonList(OzoneAcl.fromProtobuf(removeAclRequest.getAcl()));
}
@Override
diff --git
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/acl/OMBucketSetAclRequest.java
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/acl/OMBucketSetAclRequest.java
index 0b2b9102f4c..97dca83c197 100644
---
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/acl/OMBucketSetAclRequest.java
+++
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/acl/OMBucketSetAclRequest.java
@@ -18,9 +18,9 @@
package org.apache.hadoop.ozone.om.request.bucket.acl;
import java.io.IOException;
-import java.util.ArrayList;
import java.util.List;
import java.util.Map;
+import java.util.stream.Collectors;
import org.apache.hadoop.ozone.OzoneAcl;
import org.apache.hadoop.ozone.audit.AuditLogger;
import org.apache.hadoop.ozone.audit.OMAction;
@@ -66,14 +66,14 @@ public OMRequest preExecute(OzoneManager ozoneManager)
throws IOException {
}
public OMBucketSetAclRequest(OMRequest omRequest) {
- super(omRequest, (acls, omBucketInfo) -> omBucketInfo.setAcls(acls));
+ super(omRequest, (acls, builder) -> builder.set(acls));
OzoneManagerProtocolProtos.SetAclRequest setAclRequest =
getOmRequest().getSetAclRequest();
obj = OzoneObjInfo.fromProtobuf(setAclRequest.getObj());
path = obj.getPath();
- ozoneAcls = new ArrayList<>();
- setAclRequest.getAclList().forEach(aclInfo ->
- ozoneAcls.add(OzoneAcl.fromProtobuf(aclInfo)));
+ ozoneAcls = setAclRequest.getAclList().stream()
+ .map(OzoneAcl::fromProtobuf)
+ .collect(Collectors.toList());
}
@Override
diff --git
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/util/AclOp.java
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/util/AclOp.java
new file mode 100644
index 00000000000..034c38fbc77
--- /dev/null
+++
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/util/AclOp.java
@@ -0,0 +1,30 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.om.request.util;
+
+import java.util.List;
+import java.util.function.BiPredicate;
+import org.apache.hadoop.ozone.OzoneAcl;
+import org.apache.hadoop.ozone.om.helpers.AclListBuilder;
+
+/**
+ * ACL operation.
+ */
+public interface AclOp extends BiPredicate<List<OzoneAcl>, AclListBuilder> {
+ // just a shortcut to avoid having to repeat long list of generic parameters
+}
diff --git
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/acl/OMVolumeAclRequest.java
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/acl/OMVolumeAclRequest.java
index 347fbefacd9..23b522ad77a 100644
---
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/acl/OMVolumeAclRequest.java
+++
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/acl/OMVolumeAclRequest.java
@@ -23,7 +23,6 @@
import java.nio.file.InvalidPathException;
import java.util.List;
import java.util.Map;
-import java.util.function.BiConsumer;
import org.apache.hadoop.hdds.utils.db.cache.CacheKey;
import org.apache.hadoop.hdds.utils.db.cache.CacheValue;
import org.apache.hadoop.ozone.OzoneAcl;
@@ -33,8 +32,8 @@
import org.apache.hadoop.ozone.om.OMMetrics;
import org.apache.hadoop.ozone.om.OzoneManager;
import org.apache.hadoop.ozone.om.execution.flowcontrol.ExecutionContext;
-import org.apache.hadoop.ozone.om.helpers.AclListBuilder;
import org.apache.hadoop.ozone.om.helpers.OmVolumeArgs;
+import org.apache.hadoop.ozone.om.request.util.AclOp;
import org.apache.hadoop.ozone.om.request.volume.OMVolumeRequest;
import org.apache.hadoop.ozone.om.response.OMClientResponse;
import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos;
@@ -85,8 +84,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager
ozoneManager, Execut
OmVolumeArgs.Builder builder = omVolumeArgs.toBuilder();
// result is false upon add existing acl or remove non-existing acl
- omVolumeAclOp.accept(ozoneAcls, builder.acls());
- boolean applyAcl = builder.acls().isChanged();
+ boolean applyAcl = omVolumeAclOp.test(ozoneAcls, builder.acls());
// Update only when
if (applyAcl) {
@@ -186,11 +184,4 @@ abstract OMClientResponse onFailure(OMResponse.Builder
omResponse,
abstract void onComplete(Result result, Exception ex, long trxnLogIndex,
AuditLogger auditLogger, Map<String, String> auditMap);
- /**
- * Volume ACL operation.
- */
- public interface AclOp extends
- BiConsumer<List<OzoneAcl>, AclListBuilder> {
- // just a shortcut to avoid having to repeat long list of generic
parameters
- }
}
diff --git
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/acl/OMVolumeAddAclRequest.java
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/acl/OMVolumeAddAclRequest.java
index 95f42d8ae39..c0e87043ea3 100644
---
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/acl/OMVolumeAddAclRequest.java
+++
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/acl/OMVolumeAddAclRequest.java
@@ -29,6 +29,7 @@
import org.apache.hadoop.ozone.om.OzoneManager;
import org.apache.hadoop.ozone.om.execution.flowcontrol.ExecutionContext;
import org.apache.hadoop.ozone.om.helpers.OmVolumeArgs;
+import org.apache.hadoop.ozone.om.request.util.AclOp;
import org.apache.hadoop.ozone.om.request.util.OmResponseUtil;
import org.apache.hadoop.ozone.om.response.OMClientResponse;
import org.apache.hadoop.ozone.om.response.volume.OMVolumeAclOpResponse;
diff --git
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/acl/OMVolumeRemoveAclRequest.java
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/acl/OMVolumeRemoveAclRequest.java
index 92e5a529b9b..05f338957ee 100644
---
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/acl/OMVolumeRemoveAclRequest.java
+++
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/acl/OMVolumeRemoveAclRequest.java
@@ -29,6 +29,7 @@
import org.apache.hadoop.ozone.om.OzoneManager;
import org.apache.hadoop.ozone.om.execution.flowcontrol.ExecutionContext;
import org.apache.hadoop.ozone.om.helpers.OmVolumeArgs;
+import org.apache.hadoop.ozone.om.request.util.AclOp;
import org.apache.hadoop.ozone.om.request.util.OmResponseUtil;
import org.apache.hadoop.ozone.om.response.OMClientResponse;
import org.apache.hadoop.ozone.om.response.volume.OMVolumeAclOpResponse;
diff --git
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/acl/OMVolumeSetAclRequest.java
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/acl/OMVolumeSetAclRequest.java
index ac4fcfe393d..6abffc2197f 100644
---
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/acl/OMVolumeSetAclRequest.java
+++
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/acl/OMVolumeSetAclRequest.java
@@ -28,6 +28,7 @@
import org.apache.hadoop.ozone.om.OzoneManager;
import org.apache.hadoop.ozone.om.execution.flowcontrol.ExecutionContext;
import org.apache.hadoop.ozone.om.helpers.OmVolumeArgs;
+import org.apache.hadoop.ozone.om.request.util.AclOp;
import org.apache.hadoop.ozone.om.request.util.OmResponseUtil;
import org.apache.hadoop.ozone.om.response.OMClientResponse;
import org.apache.hadoop.ozone.om.response.volume.OMVolumeAclOpResponse;
diff --git
a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerUnit.java
b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerUnit.java
index 8c0c936985f..9b184421207 100644
---
a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerUnit.java
+++
b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerUnit.java
@@ -437,7 +437,6 @@ private void createBucket(OMMetadataManager
omMetadataManager,
.setBucketName(bucket)
.setStorageType(StorageType.DISK)
.setIsVersionEnabled(false)
- .setAcls(new ArrayList<>())
.build();
OMRequestTestUtils.addBucketToOM(omMetadataManager, omBucketInfo);
}
diff --git
a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/TestCleanupTableInfo.java
b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/TestCleanupTableInfo.java
index 3683110af1e..fd6e4206e0b 100644
---
a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/TestCleanupTableInfo.java
+++
b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/TestCleanupTableInfo.java
@@ -34,7 +34,6 @@
import java.lang.reflect.Modifier;
import java.nio.file.Path;
import java.util.Arrays;
-import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
@@ -303,7 +302,6 @@ private OmBucketInfo aBucketInfo() {
return OmBucketInfo.newBuilder()
.setVolumeName(TEST_VOLUME_NAME)
.setBucketName(TEST_BUCKET_NAME)
- .setAcls(Collections.emptyList())
.setIsVersionEnabled(false)
.setStorageType(StorageType.DEFAULT)
.build();
diff --git
a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/acl/OzoneNativeAclTestUtil.java
b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/acl/OzoneNativeAclTestUtil.java
index cf6ea4450c6..040871f58a8 100644
---
a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/acl/OzoneNativeAclTestUtil.java
+++
b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/acl/OzoneNativeAclTestUtil.java
@@ -56,9 +56,10 @@ public static void addBucketAcl(
OzoneAcl ozoneAcl) throws IOException {
final String bucketKey = metadataManager.getBucketKey(volume, bucket);
final Table<String, OmBucketInfo> bucketTable =
metadataManager.getBucketTable();
- final OmBucketInfo omBucketInfo = bucketTable.get(bucketKey);
-
- omBucketInfo.addAcl(ozoneAcl);
+ final OmBucketInfo omBucketInfo = bucketTable.get(bucketKey)
+ .toBuilder()
+ .addAcl(ozoneAcl)
+ .build();
bucketTable.addCacheEntry(
new CacheKey<>(bucketKey),
@@ -107,9 +108,10 @@ public static void setBucketAcl(
List<OzoneAcl> ozoneAcls) throws IOException {
final String bucketKey = metadataManager.getBucketKey(volume, bucket);
final Table<String, OmBucketInfo> bucketTable =
metadataManager.getBucketTable();
- final OmBucketInfo omBucketInfo = bucketTable.get(bucketKey);
-
- omBucketInfo.setAcls(ozoneAcls);
+ final OmBucketInfo omBucketInfo = bucketTable.get(bucketKey)
+ .toBuilder()
+ .setAcls(ozoneAcls)
+ .build();
bucketTable.addCacheEntry(
new CacheKey<>(bucketKey),
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
