You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "art9440 (via GitHub)" <gi...@apache.org> on 2026/02/17 06:32:12 UTC
[PR] HDDS-6931. Support for serialized pre-process validators [ozone]
art9440 opened a new pull request, #9774:
URL: https://github.com/apache/ozone/pull/9774
## What changes were proposed in this pull request?
This pull request refactors the request processing logic in Ozone Manager by moving selected ACL checks from validateAndUpdateCache to preExecute and introducing unified early validation for object names.
@duongkame in the [HDDS-6931](https://issues.apache.org/jira/browse/HDDS-6931) mentioned:
"I think pre-process validation should only perform logic on data which is not supposed to change between pre-process and actual execution in validateAndUpdateCache. Otherwise, that validation should be moved to {{validateAndUpdateCache, }}and is done together with the real data change in a proper lock scope to ensure consistency."
This change improves the architectural consistency of request handling in Ozone Manager by ensuring that access control checks and basic input validation are performed at the earliest possible stage of request processing.
Currently, ACL checks are implemented inconsistently across different OM request types. In some requests (e.g., volume and bucket creation), ACL validation is already performed in preExecute, while in others it is deferred to validateAndUpdateCache. This inconsistency leads to:
- Unnecessary lock acquisition for requests that will eventually fail due to insufficient permissions
- Increased lock holding time under high load
- Reduced predictability of request lifecycle
This patch moves ACL checks for selected operations (volume, bucket, key and related management requests) to the preExecute stage, where it is safe to do so — i.e., only when the validation logic depends on data that does not change between preExecute and validateAndUpdateCache.
The approach follows the principle discussed in HDDS-6931: pre-processing validation should only operate on immutable or request-local data. All validations that depend on mutable metadata state remain in validateAndUpdateCache under proper lock scope.
Additionally, this PR introduces unified early validation of volume, bucket and key names. Previously, name validation was either duplicated or missing in some request classes. Validation logic is now centralized and executed during preExecute, ensuring:
- Early rejection of malformed requests
- Reduced propagation of invalid inputs into metadata handling
- Improved maintainability and consistency across request implementations
No functional changes are introduced in terms of authorization semantics; the patch strictly changes when the validation occurs, not how it is evaluated.
This refactoring improves scalability (by reducing unnecessary lock usage), strengthens architectural consistency, and simplifies future maintenance of OM request logic.
## What is the link to the Apache JIRA
[HDDS-6931](https://issues.apache.org/jira/browse/HDDS-6931)
## How was this patch tested?
For all changes in preExecute with ACL checks were written Unit tests.
--
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-6931. Support for serialized pre-process validators [ozone]
Posted by "art9440 (via GitHub)" <gi...@apache.org>.
art9440 commented on code in PR #9774:
URL: https://github.com/apache/ozone/pull/9774#discussion_r2815819587
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequestUtils.java:
##########
@@ -111,4 +112,27 @@ public static boolean shouldLogClientRequestFailure(IOException exception) {
return true;
}
}
+
+ public static void validateVolumeName(String volumeName) throws IOException {
+ if (StringUtils.isBlank(volumeName)) {
+ if (StringUtils.isBlank(volumeName)) {
+ throw new OMException("Invalid, volume name is empty",
+ OMException.ResultCodes.INVALID_VOLUME_NAME);
+ }
Review Comment:
Fixed
--
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-6931. Support for serialized pre-process validators [ozone]
Posted by "Gargi-jais11 (via GitHub)" <gi...@apache.org>.
Gargi-jais11 commented on code in PR #9774:
URL: https://github.com/apache/ozone/pull/9774#discussion_r2816037209
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMSetTimesRequest.java:
##########
@@ -61,6 +67,33 @@ public void testKeySetTimesRequest() throws Exception {
assertEquals(mtime, keyMtime);
}
+ @Test
+ public void preExecutePermissionDeniedWhenAclEnabled() throws Exception {
+ OMRequestTestUtils.addVolumeAndBucketToDB(volumeName, bucketName,
+ omMetadataManager, getBucketLayout());
+ addKeyToTable();
+
+ when(ozoneManager.getAclsEnabled()).thenReturn(true);
+
+ OMRequest req = createSetTimesKeyRequest(2000, 1000);
+
+ OMKeySetTimesRequest setTimes = new OMKeySetTimesRequest(req, getBucketLayout()) {
+ @Override
+ public void checkAcls(OzoneManager ozoneManager,
+ OzoneObj.ResourceType resType,
+ OzoneObj.StoreType storeType,
+ IAccessAuthorizer.ACLType aclType,
+ String vol, String bucket, String key) throws IOException
+ {
+ throw new OMException("denied", OMException.ResultCodes.PERMISSION_DENIED);
+ }
Review Comment:
I think its better to use Mockito spy instead of override for ACL Permission-Denied Tests. One of the reason is that if checkAcls signature changes, only the stubbing call needs updating.
```
OMKeySetTimesRequest setTimes = spy(new OMKeySetTimesRequest(req, getBucketLayout()));
doThrow(new OMException("denied", OMException.ResultCodes.PERMISSION_DENIED))
.when(setTimes).checkAcls(any(), any(), any(), any(), any(), any(), any());
```
Files needed to change: TestOMSetTimesRequest, TestOMVolumeDeleteRequest, TestOMBucketSetPropertyRequest, TestOMBucketDeleteRequest.
--
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-6931. Support for serialized pre-process validators [ozone]
Posted by "art9440 (via GitHub)" <gi...@apache.org>.
art9440 commented on code in PR #9774:
URL: https://github.com/apache/ozone/pull/9774#discussion_r2815828650
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequestUtils.java:
##########
@@ -111,4 +112,27 @@ public static boolean shouldLogClientRequestFailure(IOException exception) {
return true;
}
}
+
+ public static void validateVolumeName(String volumeName) throws IOException {
+ if (StringUtils.isBlank(volumeName)) {
+ if (StringUtils.isBlank(volumeName)) {
+ throw new OMException("Invalid, volume name is empty",
+ OMException.ResultCodes.INVALID_VOLUME_NAME);
+ }
Review Comment:
Fixed
--
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-6931. Support for serialized pre-process validators [ozone]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9774:
URL: https://github.com/apache/ozone/pull/9774#issuecomment-4233051187
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-6931. Support for serialized pre-process validators [ozone]
Posted by "art9440 (via GitHub)" <gi...@apache.org>.
art9440 commented on code in PR #9774:
URL: https://github.com/apache/ozone/pull/9774#discussion_r2820643634
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMSetTimesRequest.java:
##########
@@ -61,6 +67,33 @@ public void testKeySetTimesRequest() throws Exception {
assertEquals(mtime, keyMtime);
}
+ @Test
+ public void preExecutePermissionDeniedWhenAclEnabled() throws Exception {
+ OMRequestTestUtils.addVolumeAndBucketToDB(volumeName, bucketName,
+ omMetadataManager, getBucketLayout());
+ addKeyToTable();
+
+ when(ozoneManager.getAclsEnabled()).thenReturn(true);
+
+ OMRequest req = createSetTimesKeyRequest(2000, 1000);
+
+ OMKeySetTimesRequest setTimes = new OMKeySetTimesRequest(req, getBucketLayout()) {
+ @Override
+ public void checkAcls(OzoneManager ozoneManager,
+ OzoneObj.ResourceType resType,
+ OzoneObj.StoreType storeType,
+ IAccessAuthorizer.ACLType aclType,
+ String vol, String bucket, String key) throws IOException
+ {
+ throw new OMException("denied", OMException.ResultCodes.PERMISSION_DENIED);
+ }
Review Comment:
Fixed
--
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-6931. Support for serialized pre-process validators [ozone]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9774:
URL: https://github.com/apache/ozone/pull/9774#issuecomment-4277147929
Thank you for your contribution. This PR is being closed due to inactivity. Please contact a maintainer if you would like 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-6931. Support for serialized pre-process validators [ozone]
Posted by "Gargi-jais11 (via GitHub)" <gi...@apache.org>.
Gargi-jais11 commented on code in PR #9774:
URL: https://github.com/apache/ozone/pull/9774#discussion_r2815352119
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequestUtils.java:
##########
@@ -111,4 +112,27 @@ public static boolean shouldLogClientRequestFailure(IOException exception) {
return true;
}
}
+
+ public static void validateVolumeName(String volumeName) throws IOException {
+ if (StringUtils.isBlank(volumeName)) {
+ if (StringUtils.isBlank(volumeName)) {
+ throw new OMException("Invalid, volume name is empty",
+ OMException.ResultCodes.INVALID_VOLUME_NAME);
+ }
Review Comment:
Nit: Remove this redundant check twice.
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMSetTimesRequest.java:
##########
@@ -61,6 +67,32 @@ public void testKeySetTimesRequest() throws Exception {
assertEquals(mtime, keyMtime);
}
+ @Test
+ public void preExecutePermissionDeniedWhenAclEnabled() throws Exception {
+ OMRequestTestUtils.addVolumeAndBucketToDB(volumeName, bucketName,
+ omMetadataManager, getBucketLayout());
+ addKeyToTable();
+
+ when(ozoneManager.getAclsEnabled()).thenReturn(true);
+
+ OMRequest req = createSetTimesKeyRequest(2000, 1000);
+
+ OMKeySetTimesRequest setTimes = new OMKeySetTimesRequest(req, getBucketLayout()) {
+ @Override
+ public void checkAcls(OzoneManager ozoneManager,
+ OzoneObj.ResourceType resType,
+ OzoneObj.StoreType storeType,
+ IAccessAuthorizer.ACLType aclType,
+ String vol, String bucket, String key) throws IOException {
Review Comment:
Please fix the indent of this function statement. It is preferred to have 4 back space indent.
```suggestion
public void checkAcls(OzoneManager ozoneManager,
OzoneObj.ResourceType resType,
OzoneObj.StoreType storeType,
IAccessAuthorizer.ACLType aclType,
String vol, String bucket, String key) throws IOException
```
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/volume/TestOMVolumeDeleteRequest.java:
##########
@@ -48,6 +55,33 @@ public void testPreExecute() throws Exception {
assertNotEquals(originalRequest, modifiedRequest);
}
+ @Test
+ public void preExecutePermissionDeniedWhenAclEnabledForDeleteVolume()
+ throws Exception {
+
+ String volumeName = UUID.randomUUID().toString();
+ when(ozoneManager.getAclsEnabled()).thenReturn(true);
+
+ OMRequest originalRequest = createDeleteVolumeRequest(volumeName);
+
+ OMVolumeDeleteRequest req = new OMVolumeDeleteRequest(originalRequest) {
+ @Override
+ public void checkAcls(OzoneManager ozoneManager,
+ OzoneObj.ResourceType resType,
+ OzoneObj.StoreType storeType,
+ IAccessAuthorizer.ACLType aclType,
+ String vol, String bucket, String key) throws IOException {
Review Comment:
Please fix the indent of this function statement. It is preferred to have 4 back space indent.
```suggestion
public void checkAcls(OzoneManager ozoneManager,
OzoneObj.ResourceType resType,
OzoneObj.StoreType storeType,
IAccessAuthorizer.ACLType aclType,
String vol, String bucket, String key) throws IOException
```
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/bucket/TestOMBucketSetPropertyRequest.java:
##########
@@ -77,6 +84,35 @@ public void testPreExecute() throws Exception {
assertNotEquals(omRequest, omBucketSetPropertyRequest.preExecute(ozoneManager));
}
+ @Test
+ public void preExecutePermissionDeniedWhenAclEnabled() throws Exception {
+ String volumeName = UUID.randomUUID().toString();
+ String bucketName = UUID.randomUUID().toString();
+
+ when(ozoneManager.getAclsEnabled()).thenReturn(true);
+
+ IAccessAuthorizer authorizer = mock(IAccessAuthorizer.class);
+ when(authorizer.isNative()).thenReturn(false);
+ when(ozoneManager.getAccessAuthorizer()).thenReturn(authorizer);
+
+ OMRequest originalRequest = createSetBucketPropertyRequest(
+ volumeName, bucketName, true, Long.MAX_VALUE);
+
+ OMBucketSetPropertyRequest req = new OMBucketSetPropertyRequest(originalRequest) {
+ @Override
+ public void checkAcls(OzoneManager ozoneManager,
+ OzoneObj.ResourceType resType,
+ OzoneObj.StoreType storeType,
+ IAccessAuthorizer.ACLType aclType,
Review Comment:
Please fix the indent of this function statement. It is preferred to have 4 back space indent.
```suggestion
public void checkAcls(OzoneManager ozoneManager,
OzoneObj.ResourceType resType,
OzoneObj.StoreType storeType,
IAccessAuthorizer.ACLType aclType,
String vol, String bucket, String key) throws IOException
```
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/bucket/TestOMBucketDeleteRequest.java:
##########
@@ -59,6 +67,32 @@ public void testPreExecute() throws Exception {
assertNotEquals(omRequest, omBucketDeleteRequest.preExecute(ozoneManager));
}
+ @Test
+ public void preExecutePermissionDeniedWhenAclEnabled() {
+ String volumeName = UUID.randomUUID().toString();
+ String bucketName = UUID.randomUUID().toString();
+
+ when(ozoneManager.getAclsEnabled()).thenReturn(true);
+
+ OMRequest originalRequest = newDeleteBucketRequest(volumeName, bucketName);
+
+ OMBucketDeleteRequest req = new OMBucketDeleteRequest(originalRequest) {
+ @Override
+ public void checkAcls(OzoneManager ozoneManager,
+ OzoneObj.ResourceType resType,
+ OzoneObj.StoreType storeType,
+ IAccessAuthorizer.ACLType aclType,
+ String vol, String bucket, String key) throws IOException {
Review Comment:
Please fix the indent of this function statement. It is preferred to have 4 back space indent.
```suggestion
public void checkAcls(OzoneManager ozoneManager,
OzoneObj.ResourceType resType,
OzoneObj.StoreType storeType,
IAccessAuthorizer.ACLType aclType,
String vol, String bucket, String key) throws IOException
```
--
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-6931. Support for serialized pre-process validators [ozone]
Posted by "Gargi-jais11 (via GitHub)" <gi...@apache.org>.
Gargi-jais11 commented on code in PR #9774:
URL: https://github.com/apache/ozone/pull/9774#discussion_r2816025955
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMSetTimesRequest.java:
##########
@@ -61,6 +67,32 @@ public void testKeySetTimesRequest() throws Exception {
assertEquals(mtime, keyMtime);
}
+ @Test
+ public void preExecutePermissionDeniedWhenAclEnabled() throws Exception {
+ OMRequestTestUtils.addVolumeAndBucketToDB(volumeName, bucketName,
+ omMetadataManager, getBucketLayout());
+ addKeyToTable();
+
+ when(ozoneManager.getAclsEnabled()).thenReturn(true);
+
+ OMRequest req = createSetTimesKeyRequest(2000, 1000);
+
+ OMKeySetTimesRequest setTimes = new OMKeySetTimesRequest(req, getBucketLayout()) {
+ @Override
+ public void checkAcls(OzoneManager ozoneManager,
+ OzoneObj.ResourceType resType,
+ OzoneObj.StoreType storeType,
+ IAccessAuthorizer.ACLType aclType,
+ String vol, String bucket, String key) throws IOException {
Review Comment:
I think its better to use **Mockito spy** instead of override for ACL Permission-Denied Tests. One of the reason is that if `checkAcls` signature changes, only the stubbing call needs updating.
```
OMKeySetTimesRequest setTimes = spy(new OMKeySetTimesRequest(req, getBucketLayout()));
doThrow(new OMException("denied", OMException.ResultCodes.PERMISSION_DENIED))
.when(setTimes).checkAcls(any(), any(), any(), any(), any(), any(), any());
```
Files needed to change: `TestOMSetTimesRequest`, `TestOMVolumeDeleteRequest`, `TestOMBucketSetPropertyRequest`, `TestOMBucketDeleteRequest`.
--
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-6931. Support for serialized pre-process validators [ozone]
Posted by "Gargi-jais11 (via GitHub)" <gi...@apache.org>.
Gargi-jais11 commented on code in PR #9774:
URL: https://github.com/apache/ozone/pull/9774#discussion_r2816198910
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeySetTimesRequest.java:
##########
Review Comment:
@art9440 This is not related to your changes, found this will understanding your changes.
As per my understanding `SetTimes` updates the key’s modification time (which is metadata update) only. It does not modify ACLs, so don't you think the ACL check should use **WRITE**, not **WRITE_ACL**?
--
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-6931. Support for serialized pre-process validators [ozone]
Posted by "art9440 (via GitHub)" <gi...@apache.org>.
art9440 commented on code in PR #9774:
URL: https://github.com/apache/ozone/pull/9774#discussion_r2821614856
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeySetTimesRequest.java:
##########
Review Comment:
@Gargi-jais11 I think your remark is correct, so this `checkAcls` should use **WRITE**.
--
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-6931. Support for serialized pre-process validators [ozone]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #9774: HDDS-6931. Support for serialized pre-process validators
URL: https://github.com/apache/ozone/pull/9774
--
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-6931. Support for serialized pre-process validators [ozone]
Posted by "Gargi-jais11 (via GitHub)" <gi...@apache.org>.
Gargi-jais11 commented on code in PR #9774:
URL: https://github.com/apache/ozone/pull/9774#discussion_r2816025955
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMSetTimesRequest.java:
##########
@@ -61,6 +67,32 @@ public void testKeySetTimesRequest() throws Exception {
assertEquals(mtime, keyMtime);
}
+ @Test
+ public void preExecutePermissionDeniedWhenAclEnabled() throws Exception {
+ OMRequestTestUtils.addVolumeAndBucketToDB(volumeName, bucketName,
+ omMetadataManager, getBucketLayout());
+ addKeyToTable();
+
+ when(ozoneManager.getAclsEnabled()).thenReturn(true);
+
+ OMRequest req = createSetTimesKeyRequest(2000, 1000);
+
+ OMKeySetTimesRequest setTimes = new OMKeySetTimesRequest(req, getBucketLayout()) {
+ @Override
+ public void checkAcls(OzoneManager ozoneManager,
+ OzoneObj.ResourceType resType,
+ OzoneObj.StoreType storeType,
+ IAccessAuthorizer.ACLType aclType,
+ String vol, String bucket, String key) throws IOException {
Review Comment:
I think its better to use **Mockito spy** instead of override for ACL Permission-Denied Tests. One of the reason is that if `checkAcls` signature changes, only the stubbing call needs updating.
```
OMKeySetTimesRequest setTimes = spy(new OMKeySetTimesRequest(req, getBucketLayout()));
doThrow(new OMException("denied", OMException.ResultCodes.PERMISSION_DENIED))
.when(setTimes).checkAcls(any(), any(), any(), any(), any(), any(), any());
```
Files needed to change: `TestOMSetTimesRequest`, `TestOMVolumeDeleteRequest`, `TestOMBucketSetPropertyRequest`, `TestOMBucketDeleteRequest`.
--
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-6931. Support for serialized pre-process validators [ozone]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9774:
URL: https://github.com/apache/ozone/pull/9774#issuecomment-4071435670
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-6931. Support for serialized pre-process validators [ozone]
Posted by "art9440 (via GitHub)" <gi...@apache.org>.
art9440 commented on code in PR #9774:
URL: https://github.com/apache/ozone/pull/9774#discussion_r2815828650
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequestUtils.java:
##########
@@ -111,4 +112,27 @@ public static boolean shouldLogClientRequestFailure(IOException exception) {
return true;
}
}
+
+ public static void validateVolumeName(String volumeName) throws IOException {
+ if (StringUtils.isBlank(volumeName)) {
+ if (StringUtils.isBlank(volumeName)) {
+ throw new OMException("Invalid, volume name is empty",
+ OMException.ResultCodes.INVALID_VOLUME_NAME);
+ }
Review Comment:
Fixed
--
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