From 99061948eb7b56885fe67f0135d317d1779a394f Mon Sep 17 00:00:00 2001 From: Russole <54737788+Russole@users.noreply.github.com> Date: Sat, 13 Dec 2025 22:43:09 +0800 Subject: [PATCH] HDDS-14104. Refactor RequestContext creation (#9493) (cherry picked from commit 59ad0a1d7fb6fd5543cc9c9cf11d8b9473325478) Conflicts: hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/security/acl/RequestContext.java --- .../ozone/security/acl/RequestContext.java | 87 +++------------ .../acl/TestOzoneNativeAuthorizer.java | 2 +- .../ozone/security/acl/TestParentAcl.java | 5 +- .../security/acl/TestRequestContext.java | 102 ++---------------- .../ozone/security/acl/TestVolumeOwner.java | 9 +- 5 files changed, 38 insertions(+), 167 deletions(-) diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/security/acl/RequestContext.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/security/acl/RequestContext.java index f2d25c2ad231..44e0f9284abf 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/security/acl/RequestContext.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/security/acl/RequestContext.java @@ -18,7 +18,6 @@ package org.apache.hadoop.ozone.security.acl; import java.net.InetAddress; -import org.apache.hadoop.ipc.ProtobufRpcEngine; import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLIdentityType; import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLType; import org.apache.hadoop.security.UserGroupInformation; @@ -26,7 +25,7 @@ /** * This class encapsulates information required for Ozone ACLs. * */ -public class RequestContext { +public final class RequestContext { private final String host; private final InetAddress ip; private final UserGroupInformation clientUgi; @@ -51,43 +50,22 @@ public class RequestContext { */ private final String sessionPolicy; - @SuppressWarnings("parameternumber") - public RequestContext(String host, InetAddress ip, - UserGroupInformation clientUgi, String serviceId, - ACLIdentityType aclType, ACLType aclRights, - String ownerName) { - this(host, ip, clientUgi, serviceId, aclType, aclRights, ownerName, - false, null); - } - - @SuppressWarnings("parameternumber") - public RequestContext(String host, InetAddress ip, - UserGroupInformation clientUgi, String serviceId, - ACLIdentityType aclType, ACLType aclRights, - String ownerName, boolean recursiveAccessCheck) { - this(host, ip, clientUgi, serviceId, aclType, aclRights, ownerName, - recursiveAccessCheck, null); - } - - @SuppressWarnings("parameternumber") - public RequestContext(String host, InetAddress ip, UserGroupInformation clientUgi, String serviceId, - ACLIdentityType aclType, ACLType aclRights, String ownerName, boolean recursiveAccessCheck, - String sessionPolicy) { - this.host = host; - this.ip = ip; - this.clientUgi = clientUgi; - this.serviceId = serviceId; - this.aclType = aclType; - this.aclRights = aclRights; - this.ownerName = ownerName; - this.recursiveAccessCheck = recursiveAccessCheck; - this.sessionPolicy = sessionPolicy; + private RequestContext(Builder builder) { + this.host = builder.host; + this.ip = builder.ip; + this.clientUgi = builder.clientUgi; + this.serviceId = builder.serviceId; + this.aclType = builder.aclType; + this.aclRights = builder.aclRights; + this.ownerName = builder.ownerName; + this.recursiveAccessCheck = builder.recursiveAccessCheck; + this.sessionPolicy = builder.sessionPolicy; } /** * Builder class for @{@link RequestContext}. */ - public static class Builder { + public static final class Builder { private String host; private InetAddress ip; private UserGroupInformation clientUgi; @@ -104,6 +82,10 @@ public static class Builder { private boolean recursiveAccessCheck; private String sessionPolicy; + private Builder() { + + } + public Builder setHost(String bHost) { this.host = bHost; return this; @@ -154,8 +136,7 @@ public Builder setSessionPolicy(String sessionPolicy) { } public RequestContext build() { - return new RequestContext(host, ip, clientUgi, serviceId, aclType, - aclRights, ownerName, recursiveAccessCheck, sessionPolicy); + return new RequestContext(this); } } @@ -163,40 +144,6 @@ public static Builder newBuilder() { return new Builder(); } - public static RequestContext.Builder getBuilder( - UserGroupInformation ugi, InetAddress remoteAddress, String hostName, - ACLType aclType, String ownerName) { - return getBuilder(ugi, remoteAddress, hostName, aclType, ownerName, - false); - } - - public static RequestContext.Builder getBuilder( - UserGroupInformation ugi, InetAddress remoteAddress, String hostName, - ACLType aclType, String ownerName, boolean recursiveAccessCheck) { - return getBuilder(ugi, remoteAddress, hostName, aclType, ownerName, recursiveAccessCheck, null); - } - - public static RequestContext.Builder getBuilder(UserGroupInformation ugi, InetAddress remoteAddress, String hostName, - ACLType aclType, String ownerName, boolean recursiveAccessCheck, String sessionPolicy) { - return RequestContext.newBuilder() - .setClientUgi(ugi) - .setIp(remoteAddress) - .setHost(hostName) - .setAclType(ACLIdentityType.USER) - .setAclRights(aclType) - .setOwnerName(ownerName) - .setRecursiveAccessCheck(recursiveAccessCheck) - .setSessionPolicy(sessionPolicy); - } - - public static RequestContext.Builder getBuilder(UserGroupInformation ugi, - ACLType aclType, String ownerName) { - return getBuilder(ugi, - ProtobufRpcEngine.Server.getRemoteIp(), - ProtobufRpcEngine.Server.getRemoteIp().getHostName(), - aclType, ownerName); - } - public String getHost() { return host; } diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/acl/TestOzoneNativeAuthorizer.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/acl/TestOzoneNativeAuthorizer.java index fbee5cdad08c..a7b2fa42a8dc 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/acl/TestOzoneNativeAuthorizer.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/acl/TestOzoneNativeAuthorizer.java @@ -335,7 +335,7 @@ private void resetAclsAndValidateAccess( String group = (!testUgi.getGroups().isEmpty()) ? testUgi.getGroups().get(0) : ""; - RequestContext.Builder builder = new RequestContext.Builder() + RequestContext.Builder builder = RequestContext.newBuilder() .setClientUgi(testUgi) .setAclType(accessType); diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/acl/TestParentAcl.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/acl/TestParentAcl.java index ed47b9578263..8b0556de4c62 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/acl/TestParentAcl.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/acl/TestParentAcl.java @@ -210,10 +210,11 @@ private void resetAcl(String vol, List volAcls, private void testParentChild(OzoneObj child, ACLType parentAclType, ACLType childAclType) throws IOException { - RequestContext requestContext = new RequestContext.Builder() + RequestContext requestContext = RequestContext.newBuilder() .setClientUgi(testUgi1) .setAclType(USER) - .setAclRights(childAclType).build(); + .setAclRights(childAclType) + .build(); OzoneAcl childAcl = OzoneAcl.of(USER, testUgi1.getUserName(), ACCESS, childAclType); diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/acl/TestRequestContext.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/acl/TestRequestContext.java index cb05c2ef6260..a0b9bfbc7f81 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/acl/TestRequestContext.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/acl/TestRequestContext.java @@ -22,8 +22,6 @@ import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; -import java.io.IOException; -import org.apache.hadoop.security.UserGroupInformation; import org.junit.jupiter.api.Test; /** @@ -32,103 +30,25 @@ public class TestRequestContext { @Test - public void testRecursiveAccessFlag() throws IOException { - RequestContext context = getUserRequestContext("om", - IAccessAuthorizer.ACLType.CREATE, false, "volume1", - true); - assertTrue(context.isRecursiveAccessCheck(), - "Wrongly sets recursiveAccessCheck flag value"); + void testRecursiveAccessFlag() { + RequestContext.Builder builder = RequestContext.newBuilder(); - context = getUserRequestContext("om", - IAccessAuthorizer.ACLType.CREATE, false, "volume1", - false); - assertFalse(context.isRecursiveAccessCheck(), - "Wrongly sets recursiveAccessCheck flag value"); - - context = getUserRequestContext( - "user1", IAccessAuthorizer.ACLType.CREATE, - true, "volume1"); - assertFalse(context.isRecursiveAccessCheck(), - "Wrongly sets recursiveAccessCheck flag value"); - - RequestContext.Builder builder = new RequestContext.Builder(); - - assertFalse(builder.build().isRecursiveAccessCheck(), - "Wrongly sets recursive flag value"); + assertFalse(builder.build().isRecursiveAccessCheck(), "default value"); builder.setRecursiveAccessCheck(true); - assertTrue(builder.build().isRecursiveAccessCheck(), - "Wrongly sets recursive flag value"); - - context = new RequestContext("host", null, - null, "serviceId", - IAccessAuthorizer.ACLIdentityType.GROUP, - IAccessAuthorizer.ACLType.CREATE, "owner"); - assertFalse(context.isRecursiveAccessCheck(), - "Wrongly sets recursive flag value"); + assertTrue(builder.build().isRecursiveAccessCheck()); - context = new RequestContext("host", null, - null, "serviceId", - IAccessAuthorizer.ACLIdentityType.GROUP, - IAccessAuthorizer.ACLType.CREATE, "owner", false); - assertFalse(context.isRecursiveAccessCheck(), - "Wrongly sets recursive flag value"); - - context = new RequestContext("host", null, - null, "serviceId", - IAccessAuthorizer.ACLIdentityType.GROUP, - IAccessAuthorizer.ACLType.CREATE, "owner", true); - assertTrue(context.isRecursiveAccessCheck(), - "Wrongly sets recursive flag value"); + builder.setRecursiveAccessCheck(false); + assertFalse(builder.build().isRecursiveAccessCheck()); } @Test - public void testSessionPolicy() { - final RequestContext.Builder builder = new RequestContext.Builder(); - RequestContext context = builder.build(); - assertNull(context.getSessionPolicy(), "sessionPolicy should default to null"); + void testSessionPolicy() { + RequestContext.Builder builder = RequestContext.newBuilder(); + assertNull(builder.build().getSessionPolicy(), "default value"); final String policy = "{\"Statement\":[]}"; - context = new RequestContext.Builder() - .setSessionPolicy(policy) - .build(); - assertEquals(policy, context.getSessionPolicy(), "sessionPolicy should be set via builder"); - - context = new RequestContext( - "host", null, null, "serviceId", IAccessAuthorizer.ACLIdentityType.GROUP, - IAccessAuthorizer.ACLType.CREATE, "owner", true, policy); - assertTrue(context.isRecursiveAccessCheck(), "recursiveAccessCheck should be true"); - assertEquals(policy, context.getSessionPolicy(), "sessionPolicy should be set via constructor"); - - context = RequestContext.getBuilder( - UserGroupInformation.createRemoteUser("user1"), null, null, - IAccessAuthorizer.ACLType.CREATE, "volume1", true) - .setSessionPolicy(policy) - .build(); - assertEquals(policy, context.getSessionPolicy(), "sessionPolicy should be set via getBuilder + builder"); - - context = RequestContext.getBuilder( - UserGroupInformation.createRemoteUser("user1"), null, null, - IAccessAuthorizer.ACLType.CREATE, "volume1", true, policy) - .build(); - assertEquals( - policy, context.getSessionPolicy(), - "sessionPolicy should be set via getBuilder (all params) + builder"); - } - - private RequestContext getUserRequestContext(String username, - IAccessAuthorizer.ACLType type, boolean isOwner, String ownerName, - boolean recursiveAccessCheck) throws IOException { - - return RequestContext.getBuilder( - UserGroupInformation.createRemoteUser(username), null, null, - type, ownerName, recursiveAccessCheck).build(); - } - - private RequestContext getUserRequestContext(String username, - IAccessAuthorizer.ACLType type, boolean isOwner, String ownerName) { - return RequestContext.getBuilder( - UserGroupInformation.createRemoteUser(username), null, null, - type, ownerName).build(); + builder.setSessionPolicy(policy); + assertEquals(policy, builder.build().getSessionPolicy()); } } diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/acl/TestVolumeOwner.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/acl/TestVolumeOwner.java index 43c0d0a39ac4..1fad71c058e7 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/acl/TestVolumeOwner.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/acl/TestVolumeOwner.java @@ -245,9 +245,12 @@ public void testKeyOps() throws Exception { private RequestContext getUserRequestContext(String username, IAccessAuthorizer.ACLType type, boolean isOwner, String ownerName) { - return RequestContext.getBuilder( - UserGroupInformation.createRemoteUser(username), null, null, - type, ownerName).build(); + return RequestContext.newBuilder() + .setClientUgi(UserGroupInformation.createRemoteUser(username)) + .setAclType(IAccessAuthorizer.ACLIdentityType.USER) + .setAclRights(type) + .setOwnerName(ownerName) + .build(); } private static String getTestVolumeName(int index) {