Skip to content

Commit 6e75f53

Browse files
ss77892ss77892
committed
ACL requests with FSO refactored
1 parent 9cb3372 commit 6e75f53

File tree

5 files changed

+193
-15
lines changed

5 files changed

+193
-15
lines changed

‎hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOMHALeaderSpecificACLEnforcement.java‎

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
import org.apache.hadoop.hdds.utils.IOUtils;
4444
import org.apache.hadoop.ozone.MiniOzoneCluster;
4545
import org.apache.hadoop.ozone.MiniOzoneHAClusterImpl;
46+
import org.apache.hadoop.ozone.OzoneAcl;
4647
import org.apache.hadoop.ozone.client.BucketArgs;
4748
import org.apache.hadoop.ozone.client.ObjectStore;
4849
import org.apache.hadoop.ozone.client.OzoneBucket;
@@ -56,6 +57,8 @@
5657
import org.apache.hadoop.ozone.om.helpers.BucketLayout;
5758
import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
5859
import org.apache.hadoop.ozone.security.acl.OzoneNativeAuthorizer;
60+
import org.apache.hadoop.ozone.security.acl.OzoneObj;
61+
import org.apache.hadoop.ozone.security.acl.OzoneObjInfo;
5962
import org.apache.hadoop.security.UserGroupInformation;
6063
import org.apache.ozone.test.GenericTestUtils;
6164
import org.junit.jupiter.api.AfterAll;
@@ -857,6 +860,132 @@ public void testKeysRenameAclEnforcementAfterLeadershipChange() throws Exception
857860
assertEquals(OMException.ResultCodes.KEY_NOT_FOUND, ex2.getResult());
858861
}
859862

863+
/**
864+
* Tests that key ACL operations (addAcl, removeAcl, setAcl) are enforced in preExecute
865+
* and are leader-specific. Uses FILE_SYSTEM_OPTIMIZED bucket layout.
866+
*
867+
* <p>This test verifies that ACL operations on keys work correctly across leadership changes:
868+
* <ul>
869+
* <li>Admin user can modify ACLs on any key when they're admin on the current leader</li>
870+
* <li>Admin user cannot modify ACLs after leadership transfer to a node where they're not admin</li>
871+
* <li>ACL checks happen in preExecute phase before any transaction execution</li>
872+
* </ul>
873+
*/
874+
@Test
875+
public void testKeyAclOperationsEnforcementAfterLeadershipChangeWithFSO() throws Exception {
876+
ObjectStore adminObjectStore = client.getObjectStore();
877+
String testVolume = "keyaclvol-" +
878+
RandomStringUtils.secure().nextAlphabetic(5).toLowerCase(Locale.ROOT);
879+
String testBucket = "keyaclbucket-" +
880+
RandomStringUtils.secure().nextAlphabetic(5).toLowerCase(Locale.ROOT);
881+
String keyName = "keyacl-" +
882+
RandomStringUtils.secure().nextAlphabetic(5).toLowerCase(Locale.ROOT);
883+
884+
// Step 1: Create volume, bucket with FSO layout, and key as admin
885+
VolumeArgs volumeArgs = VolumeArgs.newBuilder()
886+
.setOwner(adminUserUgi.getShortUserName())
887+
.build();
888+
adminObjectStore.createVolume(testVolume, volumeArgs);
889+
OzoneVolume adminVolume = adminObjectStore.getVolume(testVolume);
890+
891+
// Use FILE_SYSTEM_OPTIMIZED bucket layout
892+
BucketArgs bucketArgs = BucketArgs.newBuilder()
893+
.setBucketLayout(BucketLayout.FILE_SYSTEM_OPTIMIZED)
894+
.build();
895+
adminVolume.createBucket(testBucket, bucketArgs);
896+
OzoneBucket adminBucket = adminVolume.getBucket(testBucket);
897+
898+
// Create a key as admin (so test user is NOT the owner)
899+
try (OzoneOutputStream out = adminBucket.createKey(keyName, 0)) {
900+
out.write("test data for ACL operations".getBytes(UTF_8));
901+
}
902+
903+
OzoneKey key = adminBucket.getKey(keyName);
904+
assertNotNull(key, "Key should be created successfully");
905+
906+
// Create OzoneObj for ACL operations
907+
OzoneObj keyObj = OzoneObjInfo.Builder.newBuilder()
908+
.setVolumeName(testVolume)
909+
.setBucketName(testBucket)
910+
.setKeyName(keyName)
911+
.setResType(OzoneObj.ResourceType.KEY)
912+
.setStoreType(OzoneObj.StoreType.OZONE)
913+
.build();
914+
915+
int originalAclCount = adminObjectStore.getAcl(keyObj).size();
916+
917+
// Step 2: Add test user as admin on current leader
918+
OzoneManager currentLeader = cluster.getOMLeader();
919+
String leaderNodeId = currentLeader.getOMNodeId();
920+
addAdminToSpecificOM(currentLeader, TEST_USER);
921+
assertThat(currentLeader.getOmAdminUsernames()).contains(TEST_USER);
922+
923+
// Step 3: Test user performs ACL operations as admin (should succeed)
924+
UserGroupInformation.setLoginUser(testUserUgi);
925+
try (OzoneClient userClient = OzoneClientFactory.getRpcClient(OM_SERVICE_ID, cluster.getConf())) {
926+
ObjectStore userObjectStore = userClient.getObjectStore();
927+
928+
// Add ACL - should succeed
929+
OzoneAcl addAcl = OzoneAcl.parseAcl("user:anotheruser:rw[ACCESS]");
930+
boolean addResult = userObjectStore.addAcl(keyObj, addAcl);
931+
assertThat(addResult).isTrue();
932+
933+
// Verify ACL was added
934+
List<OzoneAcl> acls = userObjectStore.getAcl(keyObj);
935+
assertThat(acls).hasSize(originalAclCount + 1);
936+
assertThat(acls).contains(addAcl);
937+
938+
// Set ACL - should succeed
939+
OzoneAcl setAcl = OzoneAcl.parseAcl("user:setuser:rwx[ACCESS]");
940+
boolean setResult = userObjectStore.setAcl(keyObj, Collections.singletonList(setAcl));
941+
assertThat(setResult).isTrue();
942+
943+
// Verify ACL was set (replaced all previous ACLs)
944+
acls = userObjectStore.getAcl(keyObj);
945+
assertThat(acls).hasSize(1);
946+
assertThat(acls).contains(setAcl);
947+
948+
// Step 4: Transfer leadership to another node where test user is NOT admin
949+
OzoneManager newLeader = transferLeadershipToAnotherNode(currentLeader);
950+
assertNotEquals(leaderNodeId, newLeader.getOMNodeId(),
951+
"Leadership should have transferred to a different node");
952+
assertThat(newLeader.getOmAdminUsernames()).doesNotContain(TEST_USER);
953+
954+
// Step 5: Try ACL operations on new leader - should fail with PERMISSION_DENIED
955+
OzoneAcl anotherAcl = OzoneAcl.parseAcl("user:yetanotheruser:r[ACCESS]");
956+
957+
// Add ACL should fail
958+
OMException addException = assertThrows(OMException.class, () -> {
959+
userObjectStore.addAcl(keyObj, anotherAcl);
960+
}, "addAcl should fail for non-admin user on new leader");
961+
assertEquals(PERMISSION_DENIED, addException.getResult(),
962+
"Should get PERMISSION_DENIED when ACL check fails in preExecute");
963+
964+
// Remove ACL should fail
965+
OMException removeException = assertThrows(OMException.class, () -> {
966+
userObjectStore.removeAcl(keyObj, setAcl);
967+
}, "removeAcl should fail for non-admin user on new leader");
968+
assertEquals(PERMISSION_DENIED, removeException.getResult(),
969+
"Should get PERMISSION_DENIED when ACL check fails in preExecute");
970+
971+
// Set ACL should fail
972+
OzoneAcl newSetAcl = OzoneAcl.parseAcl("user:failuser:w[ACCESS]");
973+
OMException setException = assertThrows(OMException.class, () -> {
974+
userObjectStore.setAcl(keyObj, Collections.singletonList(newSetAcl));
975+
}, "setAcl should fail for non-admin user on new leader");
976+
assertEquals(PERMISSION_DENIED, setException.getResult(),
977+
"Should get PERMISSION_DENIED when ACL check fails in preExecute");
978+
979+
} finally {
980+
UserGroupInformation.setLoginUser(adminUserUgi);
981+
}
982+
983+
// Step 6: Verify the ACLs remain unchanged (operations were rejected in preExecute)
984+
List<OzoneAcl> finalAcls = adminObjectStore.getAcl(keyObj);
985+
assertThat(finalAcls).hasSize(1);
986+
assertThat(finalAcls).contains(OzoneAcl.parseAcl("user:setuser:rwx[ACCESS]"));
987+
}
988+
860989
/**
861990
* Helper method to check if volume exists.
862991
*/

‎hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/acl/OMKeyAclRequestWithFSO.java‎

Lines changed: 43 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,13 @@
2222

2323
import java.io.IOException;
2424
import java.nio.file.InvalidPathException;
25+
import java.util.LinkedHashMap;
2526
import java.util.Map;
2627
import org.apache.commons.lang3.tuple.Pair;
2728
import org.apache.hadoop.hdds.utils.db.Table;
2829
import org.apache.hadoop.hdds.utils.db.cache.CacheKey;
2930
import org.apache.hadoop.hdds.utils.db.cache.CacheValue;
31+
import org.apache.hadoop.ozone.audit.OMAction;
3032
import org.apache.hadoop.ozone.om.OMMetadataManager;
3133
import org.apache.hadoop.ozone.om.OzoneManager;
3234
import org.apache.hadoop.ozone.om.ResolvedBucket;
@@ -42,6 +44,7 @@
4244
import org.apache.hadoop.ozone.om.response.OMClientResponse;
4345
import org.apache.hadoop.ozone.om.response.key.acl.OMKeyAclResponseWithFSO;
4446
import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos;
47+
import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest;
4548
import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
4649
import org.apache.hadoop.ozone.security.acl.OzoneObj;
4750

@@ -56,6 +59,46 @@ public OMKeyAclRequestWithFSO(OzoneManagerProtocolProtos.OMRequest omReq,
5659
setBucketLayout(bucketLayout);
5760
}
5861

62+
@Override
63+
public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
64+
OMRequest omRequest = super.preExecute(ozoneManager);
65+
66+
// ACL check during preExecute
67+
if (ozoneManager.getAclsEnabled()) {
68+
try {
69+
ObjectParser objectParser = new ObjectParser(getPath(),
70+
OzoneManagerProtocolProtos.OzoneObj.ObjectType.KEY);
71+
ResolvedBucket resolvedBucket = ozoneManager.resolveBucketLink(
72+
Pair.of(objectParser.getVolume(), objectParser.getBucket()));
73+
String volume = resolvedBucket.realVolume();
74+
String bucket = resolvedBucket.realBucket();
75+
String key = objectParser.getKey();
76+
77+
checkAcls(ozoneManager, OzoneObj.ResourceType.KEY,
78+
OzoneObj.StoreType.OZONE, IAccessAuthorizer.ACLType.WRITE_ACL,
79+
volume, bucket, key);
80+
} catch (IOException ex) {
81+
// Ensure audit log captures preExecute failures
82+
Map<String, String> auditMap = new LinkedHashMap<>();
83+
OzoneObj obj = getObject();
84+
auditMap.putAll(obj.toAuditMap());
85+
// Determine which action based on request type
86+
OMAction action = OMAction.SET_ACL;
87+
if (omRequest.hasAddAclRequest()) {
88+
action = OMAction.ADD_ACL;
89+
} else if (omRequest.hasRemoveAclRequest()) {
90+
action = OMAction.REMOVE_ACL;
91+
}
92+
markForAudit(ozoneManager.getAuditLogger(),
93+
buildAuditMessage(action, auditMap, ex,
94+
omRequest.getUserInfo()));
95+
throw ex;
96+
}
97+
}
98+
99+
return omRequest;
100+
}
101+
59102
@Override
60103
public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, ExecutionContext context) {
61104
final long trxnLogIndex = context.getIndex();
@@ -81,12 +124,6 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut
81124
bucket = resolvedBucket.realBucket();
82125
key = objectParser.getKey();
83126

84-
// check Acl
85-
if (ozoneManager.getAclsEnabled()) {
86-
checkAcls(ozoneManager, OzoneObj.ResourceType.KEY,
87-
OzoneObj.StoreType.OZONE, IAccessAuthorizer.ACLType.WRITE_ACL,
88-
volume, bucket, key);
89-
}
90127
mergeOmLockDetails(omMetadataManager.getLock()
91128
.acquireWriteLock(BUCKET_LOCK, volume, bucket));
92129
lockAcquired = getOmLockDetails().isLockAcquired();

‎hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/acl/OMKeyAddAclRequestWithFSO.java‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,13 +55,17 @@ public class OMKeyAddAclRequestWithFSO extends OMKeyAclRequestWithFSO {
5555
@Override
5656
public OzoneManagerProtocolProtos.OMRequest preExecute(
5757
OzoneManager ozoneManager) throws IOException {
58+
// Call parent's preExecute to perform ACL checks
59+
OzoneManagerProtocolProtos.OMRequest omRequest =
60+
super.preExecute(ozoneManager);
61+
5862
long modificationTime = Time.now();
5963
OzoneManagerProtocolProtos.AddAclRequest.Builder addAclRequestBuilder =
60-
getOmRequest().getAddAclRequest().toBuilder()
64+
omRequest.getAddAclRequest().toBuilder()
6165
.setModificationTime(modificationTime);
6266

63-
return getOmRequest().toBuilder().setAddAclRequest(addAclRequestBuilder)
64-
.setUserInfo(getUserInfo()).build();
67+
return omRequest.toBuilder().setAddAclRequest(addAclRequestBuilder)
68+
.build();
6569
}
6670

6771
public OMKeyAddAclRequestWithFSO(

‎hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/acl/OMKeyRemoveAclRequestWithFSO.java‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,14 +55,18 @@ public class OMKeyRemoveAclRequestWithFSO extends OMKeyAclRequestWithFSO {
5555
@Override
5656
public OzoneManagerProtocolProtos.OMRequest preExecute(
5757
OzoneManager ozoneManager) throws IOException {
58+
// Call parent's preExecute to perform ACL checks
59+
OzoneManagerProtocolProtos.OMRequest omRequest =
60+
super.preExecute(ozoneManager);
61+
5862
long modificationTime = Time.now();
5963
OzoneManagerProtocolProtos.RemoveAclRequest.Builder
6064
removeAclRequestBuilder =
61-
getOmRequest().getRemoveAclRequest().toBuilder()
65+
omRequest.getRemoveAclRequest().toBuilder()
6266
.setModificationTime(modificationTime);
6367

64-
return getOmRequest().toBuilder()
65-
.setRemoveAclRequest(removeAclRequestBuilder).setUserInfo(getUserInfo())
68+
return omRequest.toBuilder()
69+
.setRemoveAclRequest(removeAclRequestBuilder)
6670
.build();
6771
}
6872

‎hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/acl/OMKeySetAclRequestWithFSO.java‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -56,13 +56,17 @@ public class OMKeySetAclRequestWithFSO extends OMKeyAclRequestWithFSO {
5656
@Override
5757
public OzoneManagerProtocolProtos.OMRequest preExecute(
5858
OzoneManager ozoneManager) throws IOException {
59+
// Call parent's preExecute to perform ACL checks
60+
OzoneManagerProtocolProtos.OMRequest omRequest =
61+
super.preExecute(ozoneManager);
62+
5963
long modificationTime = Time.now();
6064
OzoneManagerProtocolProtos.SetAclRequest.Builder setAclRequestBuilder =
61-
getOmRequest().getSetAclRequest().toBuilder()
65+
omRequest.getSetAclRequest().toBuilder()
6266
.setModificationTime(modificationTime);
6367

64-
return getOmRequest().toBuilder().setSetAclRequest(setAclRequestBuilder)
65-
.setUserInfo(getUserInfo()).build();
68+
return omRequest.toBuilder().setSetAclRequest(setAclRequestBuilder)
69+
.build();
6670
}
6771

6872
public OMKeySetAclRequestWithFSO(

0 commit comments

Comments
 (0)