[smengcl]

Generated-by: Claude Code (Opus 5.5)

What changes were proposed in this pull request?

OM converts the ACLs sent with key, file, directory, multipart upload, bucket and volume create requests (KeyArgs.acls, BucketInfo.acls, VolumeInfo.volumeAcls) with OzoneAcl.fromProtobuf only in validateAndUpdateCache. An ACL with an empty user or group name, a WORLD or ANONYMOUS ACL with another name, or rights longer than 4 bytes fails there with an IllegalArgumentException or AssertionError instead of an OMException with a result code.

Rights bits beyond the known ACL types are accepted by all requests that carry ACLs, including AddAcl, SetAcl and RemoveAcl. IAccessAuthorizer.ACLType.getACLString (used by OzoneAcl.toString, for example in audit logs) then fails with an ArrayIndexOutOfBoundsException, and so does OzoneAclStorage.fromProtobuf when reading such a prefix ACL from the prefix table. The latter also fails for a prefix ACL without any rights.

This PR:

• adds OMClientRequest.toOzoneAcls, which converts client ACLs and throws OMException with INVALID_REQUEST for a malformed one, and OMClientRequest.checkAclRights, which does the same for rights beyond the known ACL types;
• calls toOzoneAcls in preExecute of the create requests (OMKeyRequest.resolveBucketAndCheckKeyAcls, shared by key, file, directory and multipart upload requests, plus OMBucketCreateRequest and OMVolumeCreateRequest), and checkAclRights in preExecute of the volume, bucket, key and prefix AddAcl, SetAcl and RemoveAcl requests, so such ACLs are rejected before the request is replicated;
• uses toOzoneAcls in validateAndUpdateCache of the create requests (OMKeyRequest.getAclsForKey and getAclsForDir, OMBucketCreateRequest, OMVolumeCreateRequest), so such a request also gets an error response there. OMBucketCreateRequest and OMVolumeCreateRequest now set the client ACLs inside the try block instead of in OmBucketInfo.getFromProtobuf and OmVolumeArgs.builderFromProtobuf;
• skips both of the above when ozone.om.object.creation.ignore.client.acls is true, so the create requests ignore the client ACLs without validating them;
• makes ACLType.getACLString and OzoneAclStorage.fromProtobuf skip unknown rights bits, and the latter accept no rights, so existing ACLs can still be printed and read;
• throws IllegalArgumentException instead of AssertionError in OzoneAcl.fromProtobuf for rights longer than 4 bytes.

The Ozone client and the S3 Gateway build ACLs that already pass this validation, so they see no difference. No protocol, version or layout change.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-17298

How was this patch tested?

• New testInvalidAcl in TestOMKeyCreateRequest (and so TestOMKeyCreateRequestWithFSO), TestOMBucketCreateRequest and TestOMVolumeCreateRequest, and testRejectInvalidAcl in TestOMDirectoryCreateRequest, for the malformed ACLs in OMRequestTestUtils.invalidAcls(). By default, each checks that preExecute fails with INVALID_REQUEST, and that validateAndUpdateCache returns INVALID_REQUEST for an already pre-executed request with such an ACL. testInvalidAcl also checks that the request succeeds when client ACLs are ignored.
• New testRejectUnknownAclRights in TestOMKeyAclRequest (and so TestOMKeyAclRequestWithFSO), TestOMPrefixAclRequest, TestOMBucketAddAclRequest and TestOMVolumeAddAclRequest: preExecute fails with INVALID_REQUEST, and validateAndUpdateCache completes with its usual result.
• New TestOzoneAcls#testToStringIgnoresUnknownRights and TestOmPrefixInfo#testGetFromProtobufWithUnknownOrNoRights.
• Each new test fails without the change. The touched test classes in ozone-common, ozone-interface-storage and ozone-manager pass locally.

🤖 Generated with Claude Code

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[jojochuang]

Looks good.

• Both legacy and new, split-part-table approach are supported.
• backward compatible
• looks like read/write throughput shouldn't be impacted
• a real upgrade compatibility test would be nice. There's one upgrade test but client and server still run the new implementation. Can defer to a future task.

[Copilot]

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

2 open issues

• When wrapping IllegalArgumentException into OMException, the original exception is discarded,… · New
• toOzoneAcls(keyArgs.getAclsList()) is invoked unconditionally in preExecute-related flow. If… · New

_{Review effort: Lite}