You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "ss77892 (via GitHub)" <gi...@apache.org> on 2025/11/14 19:20:26 UTC
[PR] HDDS-13855. Move ACL check in Volume, Bucket and Keys requests to preexecute. [ozone]
ss77892 opened a new pull request, #9297:
URL: https://github.com/apache/ozone/pull/9297
## What changes were proposed in this pull request?
HDDS-13855. Move ACL check in Volume, Bucket and Keys requests to preexecute.
Please describe your PR in detail:
We are moving ACL checks from the validate cache method to the pre-execute stage to avoid multiple executions on all Ratis nodes.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-1385
## How was this patch tested?
Manual tests, UT
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13855. Move ACL check to preExecute in Volume, Bucket and Keys requests [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #9297:
URL: https://github.com/apache/ozone/pull/9297#discussion_r2596621896
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOMHALeaderSpecificACLEnforcement.java:
##########
@@ -373,6 +375,287 @@ public void testKeySetTimesAclEnforcementAfterLeadershipChange() throws Exceptio
}
}
+ /**
+ * Tests that setQuota ACL check is enforced in preExecute and is leader-specific.
+ */
+ @Test
+ public void testVolumeSetQuotaAclEnforcementAfterLeadershipChange() throws Exception {
+ ObjectStore adminObjectStore = client.getObjectStore();
+ String testVolume = "quotavol-" +
+ RandomStringUtils.secure().nextAlphabetic(5).toLowerCase(Locale.ROOT);
+
+ // Create volume as admin
+ VolumeArgs volumeArgs = VolumeArgs.newBuilder()
+ .setOwner(adminUserUgi.getShortUserName())
+ .build();
+ adminObjectStore.createVolume(testVolume, volumeArgs);
+
+ // Add test user as admin on current leader
+ OzoneManager currentLeader = cluster.getOMLeader();
+ addAdminToSpecificOM(currentLeader, TEST_USER);
+ assertTrue(currentLeader.getOmAdminUsernames().contains(TEST_USER));
Review Comment:
nit: please use `assertThat(...).contains(...)` for better assertion failure message. Similarly, `doesNotContain(...)` instead of `assertFalse(...)`.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeDeleteRequest.java:
##########
@@ -57,6 +60,36 @@ public OMVolumeDeleteRequest(OMRequest omRequest) {
super(omRequest);
}
+ @Override
+ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
+ super.preExecute(ozoneManager);
+ DeleteVolumeRequest deleteVolumeRequest =
+ getOmRequest().getDeleteVolumeRequest();
+ Preconditions.checkNotNull(deleteVolumeRequest);
Review Comment:
nit: prefer JDK's `Objects.requireNonNull`
```suggestion
Objects.requireNonNull(deleteVolumeRequest);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13855. Move ACL check to preExecute in Volume, Bucket and Keys requests [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on PR #9297:
URL: https://github.com/apache/ozone/pull/9297#issuecomment-3543120951
@yandrey321 Can you take a look at this change?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13855. Move ACL check in Volume, Bucket and Keys requests to preexecute. [ozone]
Posted by "sarvekshayr (via GitHub)" <gi...@apache.org>.
sarvekshayr commented on code in PR #9297:
URL: https://github.com/apache/ozone/pull/9297#discussion_r2533653341
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java:
##########
@@ -77,6 +77,59 @@ public OMKeysRenameRequest(OMRequest omRequest, BucketLayout bucketLayout) {
super(omRequest, bucketLayout);
}
+ @Override
+ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
+ super.preExecute(ozoneManager);
+
+ RenameKeysRequest renameKeysRequest = getOmRequest().getRenameKeysRequest();
+ RenameKeysArgs renameKeysArgs = renameKeysRequest.getRenameKeysArgs();
+
+ String volumeName = renameKeysArgs.getVolumeName();
+ String bucketName = renameKeysArgs.getBucketName();
+
+ // Resolve bucket link
+ ResolvedBucket resolvedBucketObj = ozoneManager.resolveBucketLink(
+ Pair.of(volumeName, bucketName));
+ String resolvedVolume = resolvedBucketObj.realVolume();
+ String resolvedBucket = resolvedBucketObj.realBucket();
+
+ // ACL check during preExecute - check all key pairs
+ if (ozoneManager.getAclsEnabled()) {
+ for (RenameKeysMap renameKey : renameKeysArgs.getRenameKeysMapList()) {
+ String fromKeyName = renameKey.getFromKeyName();
+ String toKeyName = renameKey.getToKeyName();
+
+ // Skip empty key names - they will be handled in validateAndUpdateCache
+ if (fromKeyName.isEmpty() || toKeyName.isEmpty()) {
+ continue;
+ }
+
+ try {
+ // Check ACLs: DELETE permission on source key, CREATE permission on destination key
+ checkKeyAcls(ozoneManager, resolvedVolume, resolvedBucket, fromKeyName,
+ IAccessAuthorizer.ACLType.DELETE, OzoneObj.ResourceType.KEY);
+ checkKeyAcls(ozoneManager, resolvedVolume, resolvedBucket, toKeyName,
+ IAccessAuthorizer.ACLType.CREATE, OzoneObj.ResourceType.KEY);
+ } catch (IOException ex) {
+ // Ensure audit log captures preExecute failures
+ Map<String, String> auditMap = new LinkedHashMap<>();
+ auditMap.put("volume", resolvedVolume);
+ auditMap.put("bucket", resolvedBucket);
+ auditMap.put("fromKey", fromKeyName);
+ auditMap.put("toKey", toKeyName);
Review Comment:
To keep the audit map keys consistent across the codebase, can we avoid hardcoding here?
We should either use the existing constants or add new ones if needed for consistency.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13855. Move ACL check to preExecute in Volume, Bucket and Keys requests [ozone]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9297:
URL: https://github.com/apache/ozone/pull/9297#issuecomment-3708555772
Thank you for your contribution. This PR is being closed due to inactivity. If needed, feel free to reopen it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13855. Move ACL check to preExecute in Volume, Bucket and Keys requests [ozone]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9297:
URL: https://github.com/apache/ozone/pull/9297#issuecomment-3695158214
This PR has been marked as stale due to 21 days of inactivity. Please comment or remove the stale label to keep it open. Otherwise, it will be automatically closed in 7 days.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13855. Move ACL check to preExecute in Volume, Bucket and Keys requests [ozone]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #9297: HDDS-13855. Move ACL check to preExecute in Volume, Bucket and Keys requests
URL: https://github.com/apache/ozone/pull/9297
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org