Skip to content

HDDS-13855. Move ACL check in Volume, Bucket and Keys requests to preExecute #9653

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

Open
ss77892 wants to merge 2 commits into
base: master
Choose a base branch
from
Open

HDDS-13855. Move ACL check in Volume, Bucket and Keys requests to preExecute #9653

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9cb3372
HDDS-13855. Move ACL check in Volume, Bucket and Keys requests to pre…
ss77892 Oct 28, 2025
6e75f53
ACL requests with FSO refactored
ss77892 Jan 27, 2026
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
733 changes: 673 additions & 60 deletions ...n-test/src/test/java/org/apache/hadoop/ozone/om/TestOMHALeaderSpecificACLEnforcement.java
View file
Open in desktop

Large diffs are not rendered by default.

43 changes: 34 additions & 9 deletions ...anager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketDeleteRequest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import java.io.IOException;
import java.nio.file.InvalidPathException;
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.Map;
import org.apache.hadoop.hdds.utils.db.Table;
import org.apache.hadoop.hdds.utils.db.TableIterator;
Expand Down Expand Up @@ -73,6 +74,37 @@ public OMBucketDeleteRequest(OMRequest omRequest) {
super(omRequest);
}

@Override
public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
super.preExecute(ozoneManager);
DeleteBucketRequest deleteBucketRequest =
getOmRequest().getDeleteBucketRequest();
String volumeName = deleteBucketRequest.getVolumeName();
String bucketName = deleteBucketRequest.getBucketName();

// ACL check during preExecute
if (ozoneManager.getAclsEnabled()) {
try {
checkAcls(ozoneManager, OzoneObj.ResourceType.BUCKET,
OzoneObj.StoreType.OZONE, IAccessAuthorizer.ACLType.DELETE,
volumeName, bucketName, null);
} catch (IOException ex) {
// Ensure audit log captures preExecute failures
Map<String, String> auditMap = new LinkedHashMap<>();
auditMap.put(OzoneConsts.VOLUME, volumeName);
auditMap.put(OzoneConsts.BUCKET, bucketName);
markForAudit(ozoneManager.getAuditLogger(),
buildAuditMessage(OMAction.DELETE_BUCKET, auditMap, ex,
getOmRequest().getUserInfo()));
throw ex;
}
}

return getOmRequest().toBuilder()
.setUserInfo(getUserIfNotExists(ozoneManager))
.build();
}

@Override
public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, ExecutionContext context) {
final long transactionLogIndex = context.getIndex();
Expand Down Expand Up @@ -101,13 +133,6 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut
boolean success = true;
OMClientResponse omClientResponse = null;
try {
// check Acl
if (ozoneManager.getAclsEnabled()) {
checkAcls(ozoneManager, OzoneObj.ResourceType.BUCKET,
OzoneObj.StoreType.OZONE, IAccessAuthorizer.ACLType.DELETE,
volumeName, bucketName, null);
}

// acquire lock
mergeOmLockDetails(
omMetadataManager.getLock().acquireReadLock(VOLUME_LOCK, volumeName));
Expand All @@ -118,7 +143,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut
acquiredBucketLock = getOmLockDetails().isLockAcquired();

// No need to check volume exists here, as bucket cannot be created
// with out volume creation. Check if bucket exists
// without volume creation. Check if bucket exists
String bucketKey = omMetadataManager.getBucketKey(volumeName, bucketName);

OmBucketInfo omBucketInfo =
Expand Down Expand Up @@ -265,7 +290,7 @@ private boolean bucketContainsSnapshotInCache(

/**
* Validates bucket delete requests.
* Handles the cases where an older client attempts to delete a bucket
* Handles the cases where an older client attempts to delete a bucket with
* a new bucket layout.
* We do not want to allow this to happen, since this would cause the client
* to be able to delete buckets it cannot understand.
Expand Down
35 changes: 27 additions & 8 deletions ...ager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketSetOwnerRequest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

import java.io.IOException;
import java.nio.file.InvalidPathException;
import java.util.Map;
import java.util.Objects;
import org.apache.hadoop.hdds.utils.db.cache.CacheKey;
import org.apache.hadoop.hdds.utils.db.cache.CacheValue;
Expand Down Expand Up @@ -63,15 +64,40 @@ public OMBucketSetOwnerRequest(OMRequest omRequest) {
@Override
public OMRequest preExecute(OzoneManager ozoneManager)
throws IOException {
super.preExecute(ozoneManager);

long modificationTime = Time.now();
OzoneManagerProtocolProtos.SetBucketPropertyRequest.Builder
setBucketPropertyRequestBuilder = getOmRequest()
.getSetBucketPropertyRequest().toBuilder()
.setModificationTime(modificationTime);

SetBucketPropertyRequest setBucketPropertyRequest =
getOmRequest().getSetBucketPropertyRequest();
BucketArgs bucketArgs = setBucketPropertyRequest.getBucketArgs();
String volumeName = bucketArgs.getVolumeName();
String bucketName = bucketArgs.getBucketName();

// ACL check during preExecute
if (ozoneManager.getAclsEnabled()) {
try {
checkAcls(ozoneManager, OzoneObj.ResourceType.BUCKET,
OzoneObj.StoreType.OZONE, IAccessAuthorizer.ACLType.WRITE_ACL,
volumeName, bucketName, null);
} catch (IOException ex) {
// Ensure audit log captures preExecute failures
OmBucketArgs omBucketArgs = OmBucketArgs.getFromProtobuf(bucketArgs);
Map<String, String> auditMap = omBucketArgs.toAuditMap();
markForAudit(ozoneManager.getAuditLogger(),
buildAuditMessage(OMAction.SET_OWNER, auditMap, ex,
getOmRequest().getUserInfo()));
throw ex;
}
}

return getOmRequest().toBuilder()
.setSetBucketPropertyRequest(setBucketPropertyRequestBuilder)
.setUserInfo(getUserInfo())
.setUserInfo(getUserIfNotExists(ozoneManager))
.build();
}

Expand Down Expand Up @@ -109,13 +135,6 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut
boolean acquiredBucketLock = false, success = true;
OMClientResponse omClientResponse = null;
try {
// check Acl
if (ozoneManager.getAclsEnabled()) {
checkAcls(ozoneManager, OzoneObj.ResourceType.BUCKET,
OzoneObj.StoreType.OZONE, IAccessAuthorizer.ACLType.WRITE_ACL,
volumeName, bucketName, null);
}

// acquire lock.
mergeOmLockDetails(omMetadataManager.getLock().acquireWriteLock(
BUCKET_LOCK, volumeName, bucketName));
Expand Down
33 changes: 24 additions & 9 deletions ...r/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketSetPropertyRequest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import java.io.IOException;
import java.nio.file.InvalidPathException;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import org.apache.hadoop.crypto.key.KeyProviderCryptoExtension;
import org.apache.hadoop.hdds.client.DefaultReplicationConfig;
Expand Down Expand Up @@ -79,6 +80,8 @@ public OMBucketSetPropertyRequest(OMRequest omRequest) {
@Override
public OMRequest preExecute(OzoneManager ozoneManager)
throws IOException {
super.preExecute(ozoneManager);

long modificationTime = Time.now();
OzoneManagerProtocolProtos.SetBucketPropertyRequest.Builder
setBucketPropertyRequestBuilder = getOmRequest()
Expand All @@ -97,9 +100,26 @@ public OMRequest preExecute(OzoneManager ozoneManager)
setBucketPropertyRequestBuilder.setBucketArgs(bucketArgsBuilder.build());
}

// ACL check during preExecute
if (ozoneManager.getAclsEnabled()) {
String volumeName = bucketArgs.getVolumeName();
String bucketName = bucketArgs.getBucketName();
try {
checkAclPermission(ozoneManager, volumeName, bucketName);
} catch (IOException ex) {
// Ensure audit log captures preExecute failures
OmBucketArgs omBucketArgs = OmBucketArgs.getFromProtobuf(bucketArgs);
Map<String, String> auditMap = omBucketArgs.toAuditMap();
markForAudit(ozoneManager.getAuditLogger(),
buildAuditMessage(OMAction.UPDATE_BUCKET, auditMap, ex,
getOmRequest().getUserInfo()));
throw ex;
}
}

return getOmRequest().toBuilder()
.setSetBucketPropertyRequest(setBucketPropertyRequestBuilder)
.setUserInfo(getUserInfo())
.setUserInfo(getUserIfNotExists(ozoneManager))
.build();
}

Expand Down Expand Up @@ -132,11 +152,6 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut
boolean acquiredBucketLock = false, success = true;
OMClientResponse omClientResponse = null;
try {
// check Acl
if (ozoneManager.getAclsEnabled()) {
checkAclPermission(ozoneManager, volumeName, bucketName);
}

// acquire lock.
mergeOmLockDetails(omMetadataManager.getLock().acquireWriteLock(
BUCKET_LOCK, volumeName, bucketName));
Expand Down Expand Up @@ -301,12 +316,12 @@ public boolean checkQuotaBytesValid(OMMetadataManager metadataManager,
OMException.ResultCodes.QUOTA_ERROR);
}
}

// avoid iteration of other bucket if quota set is less than previous set
if (quotaInBytes < dbBucketInfo.getQuotaInBytes()) {
return true;
}

List<OmBucketInfo> bucketList = metadataManager.listBuckets(
omVolumeArgs.getVolume(), null, null, Integer.MAX_VALUE, false);
for (OmBucketInfo bucketInfo : bucketList) {
Expand Down Expand Up @@ -342,7 +357,7 @@ public boolean checkQuotaNamespaceValid(OmVolumeArgs omVolumeArgs,
if (quotaInNamespace < OzoneConsts.QUOTA_RESET || quotaInNamespace == 0) {
return false;
}

if (quotaInNamespace != OzoneConsts.QUOTA_RESET
&& quotaInNamespace < dbBucketInfo.getTotalBucketNamespace()) {
throw new OMException("Cannot update bucket quota. NamespaceQuota " +
Expand Down
51 changes: 45 additions & 6 deletions ...nager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/acl/OMBucketAclRequest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

import java.io.IOException;
import java.nio.file.InvalidPathException;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import org.apache.commons.lang3.tuple.Pair;
Expand All @@ -29,6 +30,7 @@
import org.apache.hadoop.ozone.OzoneAcl;
import org.apache.hadoop.ozone.OzoneConsts;
import org.apache.hadoop.ozone.audit.AuditLogger;
import org.apache.hadoop.ozone.audit.OMAction;
import org.apache.hadoop.ozone.om.OMMetadataManager;
import org.apache.hadoop.ozone.om.OMMetrics;
import org.apache.hadoop.ozone.om.OzoneManager;
Expand Down Expand Up @@ -59,6 +61,49 @@ public OMBucketAclRequest(OMRequest omRequest, AclOp aclOp) {
omBucketAclOp = aclOp;
}

@Override
public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
OMRequest omRequest = super.preExecute(ozoneManager);

// ACL check during preExecute
if (ozoneManager.getAclsEnabled()) {
try {
ObjectParser objectParser = new ObjectParser(getPath(),
ObjectType.BUCKET);
ResolvedBucket resolvedBucket = ozoneManager.resolveBucketLink(
Pair.of(objectParser.getVolume(), objectParser.getBucket()));
String volume = resolvedBucket.realVolume();
String bucket = resolvedBucket.realBucket();

checkAcls(ozoneManager, OzoneObj.ResourceType.BUCKET,
OzoneObj.StoreType.OZONE, IAccessAuthorizer.ACLType.WRITE_ACL,
volume, bucket, null);
} catch (IOException ex) {
// Ensure audit log captures preExecute failures
Map<String, String> auditMap = new LinkedHashMap<>();
OzoneObj obj = getObject();
auditMap.putAll(obj.toAuditMap());
List<OzoneAcl> acls = getAcls();
if (acls != null) {
auditMap.put(OzoneConsts.ACL, acls.toString());
}
// Determine which action based on request type
OMAction action = OMAction.SET_ACL;
if (omRequest.hasAddAclRequest()) {
action = OMAction.ADD_ACL;
} else if (omRequest.hasRemoveAclRequest()) {
action = OMAction.REMOVE_ACL;
}
markForAudit(ozoneManager.getAuditLogger(),
buildAuditMessage(action, auditMap, ex,
omRequest.getUserInfo()));
throw ex;
}
}

return omRequest;
}

@Override
public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, ExecutionContext context) {
final long transactionLogIndex = context.getIndex();
Expand Down Expand Up @@ -87,12 +132,6 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut
volume = resolvedBucket.realVolume();
bucket = resolvedBucket.realBucket();

// check Acl
if (ozoneManager.getAclsEnabled()) {
checkAcls(ozoneManager, OzoneObj.ResourceType.BUCKET,
OzoneObj.StoreType.OZONE, IAccessAuthorizer.ACLType.WRITE_ACL,
volume, bucket, null);
}
mergeOmLockDetails(
omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK, volume,
bucket));
Expand Down
1 change: 1 addition & 0 deletions ...er/src/main/java/org/apache/hadoop/ozone/om/request/bucket/acl/OMBucketSetAclRequest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ public class OMBucketSetAclRequest extends OMBucketAclRequest {

@Override
public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
super.preExecute(ozoneManager);
long modificationTime = Time.now();
OzoneManagerProtocolProtos.SetAclRequest.Builder setAclRequestBuilder =
getOmRequest().getSetAclRequest().toBuilder()
Expand Down
Loading