Skip to content

Commit 0a57b43

Browse files
echonesisechonesis
authored
HDDS-14360. Move BucketEndpoint#getAcl to BucketAclHandler (#9607)
1 parent 60f84b5 commit 0a57b43

File tree

5 files changed

+171
-69
lines changed

5 files changed

+171
-69
lines changed

‎hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketAclHandler.java‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,11 @@
2727
import java.io.InputStream;
2828
import java.util.ArrayList;
2929
import java.util.EnumSet;
30+
import java.util.HashSet;
3031
import java.util.List;
32+
import java.util.Set;
3133
import javax.annotation.PostConstruct;
34+
import javax.ws.rs.core.MediaType;
3235
import javax.ws.rs.core.Response;
3336
import org.apache.commons.lang3.StringUtils;
3437
import org.apache.hadoop.ozone.OzoneAcl;
@@ -38,6 +41,7 @@
3841
import org.apache.hadoop.ozone.om.exceptions.OMException;
3942
import org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes;
4043
import org.apache.hadoop.ozone.om.helpers.OzoneAclUtil;
44+
import org.apache.hadoop.ozone.s3.endpoint.S3BucketAcl.Grant;
4145
import org.apache.hadoop.ozone.s3.exception.OS3Exception;
4246
import org.apache.hadoop.ozone.s3.exception.S3ErrorTable;
4347
import org.apache.hadoop.ozone.s3.util.S3Consts.QueryParams;
@@ -66,6 +70,64 @@ private boolean shouldHandle() {
6670
return queryParams().get(QueryParams.ACL) != null;
6771
}
6872

73+
/**
74+
* Implement acl get.
75+
* <p>
76+
* see: https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetBucketAcl.html
77+
*/
78+
@Override
79+
public Response handleGetRequest(String bucketName)
80+
throws IOException, OS3Exception {
81+
82+
if (!shouldHandle()) {
83+
return null; // Not responsible for this request
84+
}
85+
86+
long startNanos = Time.monotonicNowNanos();
87+
S3GAction s3GAction = S3GAction.GET_ACL;
88+
89+
try {
90+
OzoneBucket bucket = getBucket(bucketName);
91+
S3Owner.verifyBucketOwnerCondition(getHeaders(), bucketName, bucket.getOwner());
92+
S3Owner owner = S3Owner.of(bucket.getOwner());
93+
94+
S3BucketAcl result = new S3BucketAcl();
95+
result.setOwner(owner);
96+
97+
// TODO: remove this duplication avoid logic when ACCESS and DEFAULT scope
98+
// TODO: are merged.
99+
// Use set to remove ACLs with different scopes(ACCESS and DEFAULT)
100+
Set<Grant> grantSet = new HashSet<>();
101+
// Return ACL list
102+
for (OzoneAcl acl : bucket.getAcls()) {
103+
List<Grant> grants = S3Acl.ozoneNativeAclToS3Acl(acl);
104+
grantSet.addAll(grants);
105+
}
106+
ArrayList<Grant> grantList = new ArrayList<>();
107+
grantList.addAll(grantSet);
108+
result.setAclList(
109+
new S3BucketAcl.AccessControlList(grantList));
110+
111+
getMetrics().updateGetAclSuccessStats(startNanos);
112+
auditReadSuccess(s3GAction);
113+
return Response.ok(result, MediaType.APPLICATION_XML_TYPE).build();
114+
} catch (OMException ex) {
115+
getMetrics().updateGetAclFailureStats(startNanos);
116+
auditReadFailure(s3GAction, ex);
117+
if (ex.getResult() == ResultCodes.BUCKET_NOT_FOUND) {
118+
throw newError(S3ErrorTable.NO_SUCH_BUCKET, bucketName, ex);
119+
} else if (isAccessDenied(ex)) {
120+
throw newError(S3ErrorTable.ACCESS_DENIED, bucketName, ex);
121+
} else {
122+
throw newError(S3ErrorTable.INTERNAL_ERROR, bucketName, ex);
123+
}
124+
} catch (OS3Exception ex) {
125+
getMetrics().updateGetAclFailureStats(startNanos);
126+
auditReadFailure(s3GAction, ex);
127+
throw ex;
128+
}
129+
}
130+
69131
/**
70132
* Implement acl put.
71133
* <p>

‎hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java‎

Lines changed: 8 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -31,12 +31,10 @@
3131
import java.io.IOException;
3232
import java.io.InputStream;
3333
import java.util.ArrayList;
34-
import java.util.HashSet;
3534
import java.util.Iterator;
3635
import java.util.List;
3736
import java.util.Map;
3837
import java.util.Objects;
39-
import java.util.Set;
4038
import javax.annotation.PostConstruct;
4139
import javax.ws.rs.DELETE;
4240
import javax.ws.rs.GET;
@@ -50,7 +48,6 @@
5048
import javax.ws.rs.core.MediaType;
5149
import javax.ws.rs.core.Response;
5250
import org.apache.commons.lang3.StringUtils;
53-
import org.apache.hadoop.ozone.OzoneAcl;
5451
import org.apache.hadoop.ozone.audit.AuditEventStatus;
5552
import org.apache.hadoop.ozone.audit.AuditMessage;
5653
import org.apache.hadoop.ozone.audit.S3GAction;
@@ -65,7 +62,6 @@
6562
import org.apache.hadoop.ozone.s3.endpoint.MultiDeleteRequest.DeleteObject;
6663
import org.apache.hadoop.ozone.s3.endpoint.MultiDeleteResponse.DeletedObject;
6764
import org.apache.hadoop.ozone.s3.endpoint.MultiDeleteResponse.Error;
68-
import org.apache.hadoop.ozone.s3.endpoint.S3BucketAcl.Grant;
6965
import org.apache.hadoop.ozone.s3.exception.OS3Exception;
7066
import org.apache.hadoop.ozone.s3.exception.S3ErrorTable;
7167
import org.apache.hadoop.ozone.s3.util.ContinueToken;
@@ -107,6 +103,14 @@ public Response get(
107103
S3GAction s3GAction = S3GAction.GET_BUCKET;
108104
PerformanceStringBuilder perf = new PerformanceStringBuilder();
109105

106+
// Chain of responsibility: let each handler try to handle the request
107+
for (BucketOperationHandler handler : handlers) {
108+
Response response = handler.handleGetRequest(bucketName);
109+
if (response != null) {
110+
return response; // Handler handled the request
111+
}
112+
}
113+
110114
final String continueToken = queryParams().get(QueryParams.CONTINUATION_TOKEN);
111115
final String delimiter = queryParams().get(QueryParams.DELIMITER);
112116
final String encodingType = queryParams().get(QueryParams.ENCODING_TYPE);
@@ -122,15 +126,6 @@ public Response get(
122126
OzoneBucket bucket = null;
123127

124128
try {
125-
final String aclMarker = queryParams().get(QueryParams.ACL);
126-
if (aclMarker != null) {
127-
s3GAction = S3GAction.GET_ACL;
128-
S3BucketAcl result = getAcl(bucketName);
129-
getMetrics().updateGetAclSuccessStats(startNanos);
130-
auditReadSuccess(s3GAction);
131-
return Response.ok(result, MediaType.APPLICATION_XML_TYPE).build();
132-
}
133-
134129
final String uploads = queryParams().get(QueryParams.UPLOADS);
135130
if (uploads != null) {
136131
s3GAction = S3GAction.LIST_MULTIPART_UPLOAD;
@@ -534,51 +529,6 @@ public MultiDeleteResponse multiDelete(
534529
return result;
535530
}
536531

537-
/**
538-
* Implement acl get.
539-
* <p>
540-
* see: https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetBucketAcl.html
541-
*/
542-
public S3BucketAcl getAcl(String bucketName)
543-
throws OS3Exception, IOException {
544-
long startNanos = Time.monotonicNowNanos();
545-
S3BucketAcl result = new S3BucketAcl();
546-
try {
547-
OzoneBucket bucket = getBucket(bucketName);
548-
S3Owner.verifyBucketOwnerCondition(getHeaders(), bucketName, bucket.getOwner());
549-
S3Owner owner = S3Owner.of(bucket.getOwner());
550-
result.setOwner(owner);
551-
552-
// TODO: remove this duplication avoid logic when ACCESS and DEFAULT scope
553-
// TODO: are merged.
554-
// Use set to remove ACLs with different scopes(ACCESS and DEFAULT)
555-
Set<Grant> grantSet = new HashSet<>();
556-
// Return ACL list
557-
for (OzoneAcl acl : bucket.getAcls()) {
558-
List<Grant> grants = S3Acl.ozoneNativeAclToS3Acl(acl);
559-
grantSet.addAll(grants);
560-
}
561-
ArrayList<Grant> grantList = new ArrayList<>();
562-
grantList.addAll(grantSet);
563-
result.setAclList(
564-
new S3BucketAcl.AccessControlList(grantList));
565-
return result;
566-
} catch (OMException ex) {
567-
getMetrics().updateGetAclFailureStats(startNanos);
568-
auditReadFailure(S3GAction.GET_ACL, ex);
569-
if (ex.getResult() == ResultCodes.BUCKET_NOT_FOUND) {
570-
throw newError(S3ErrorTable.NO_SUCH_BUCKET, bucketName, ex);
571-
} else if (isAccessDenied(ex)) {
572-
throw newError(S3ErrorTable.ACCESS_DENIED, bucketName, ex);
573-
} else {
574-
throw newError(S3ErrorTable.INTERNAL_ERROR, bucketName, ex);
575-
}
576-
} catch (OS3Exception ex) {
577-
getMetrics().updateGetAclFailureStats(startNanos);
578-
throw ex;
579-
}
580-
}
581-
582532
private void addKey(ListObjectResponse response, OzoneKey next) {
583533
KeyMetadata keyMetadata = new KeyMetadata();
584534
keyMetadata.setKey(EncodingTypeObject.createNullable(next.getName(),

‎hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketOperationHandler.java‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,4 +47,19 @@ default Response handlePutRequest(String bucketName, InputStream body)
4747
throws IOException, OS3Exception {
4848
return null;
4949
}
50+
51+
/**
52+
* Handle the bucket GET operation if this handler is responsible for it.
53+
* The handler inspects the request (query parameters, headers, etc.) to determine
54+
* if it should handle the request.
55+
*
56+
* @param bucketName the name of the bucket
57+
* @return Response if this handler handles the request, null otherwise
58+
* @throws IOException if an I/O error occurs
59+
* @throws OS3Exception if an S3-specific error occurs
60+
*/
61+
default Response handleGetRequest(String bucketName)
62+
throws IOException, OS3Exception {
63+
return null;
64+
}
5065
}

‎hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestBucketAcl.java‎

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import static java.net.HttpURLConnection.HTTP_NOT_IMPLEMENTED;
2222
import static java.net.HttpURLConnection.HTTP_OK;
2323
import static org.junit.jupiter.api.Assertions.assertEquals;
24+
import static org.junit.jupiter.api.Assertions.assertInstanceOf;
2425
import static org.junit.jupiter.api.Assertions.assertThrows;
2526
import static org.mockito.Mockito.mock;
2627
import static org.mockito.Mockito.when;
@@ -79,6 +80,15 @@ public void clean() throws IOException {
7980
}
8081
}
8182

83+
/**
84+
* Helper method to get ACL from bucket and validate response.
85+
*/
86+
private S3BucketAcl getBucketAcl(String bucketName) throws Exception {
87+
Response response = bucketEndpoint.get(bucketName);
88+
assertEquals(HTTP_OK, response.getStatus());
89+
return assertInstanceOf(S3BucketAcl.class, response.getEntity());
90+
}
91+
8292
@Test
8393
public void testGetAcl() throws Exception {
8494
when(parameterMap.containsKey(ACL_MARKER)).thenReturn(true);
@@ -105,7 +115,7 @@ public void testRead() throws Exception {
105115
Response response =
106116
bucketEndpoint.put(BUCKET_NAME, null);
107117
assertEquals(HTTP_OK, response.getStatus());
108-
S3BucketAcl getResponse = bucketEndpoint.getAcl(BUCKET_NAME);
118+
S3BucketAcl getResponse = getBucketAcl(BUCKET_NAME);
109119
assertEquals(1, getResponse.getAclList().getGrantList().size());
110120
assertEquals(S3Acl.ACLType.READ.getValue(),
111121
getResponse.getAclList().getGrantList().get(0).getPermission());
@@ -119,7 +129,7 @@ public void testWrite() throws Exception {
119129
Response response =
120130
bucketEndpoint.put(BUCKET_NAME, null);
121131
assertEquals(HTTP_OK, response.getStatus());
122-
S3BucketAcl getResponse = bucketEndpoint.getAcl(BUCKET_NAME);
132+
S3BucketAcl getResponse = getBucketAcl(BUCKET_NAME);
123133
assertEquals(1, getResponse.getAclList().getGrantList().size());
124134
assertEquals(S3Acl.ACLType.WRITE.getValue(),
125135
getResponse.getAclList().getGrantList().get(0).getPermission());
@@ -133,8 +143,7 @@ public void testReadACP() throws Exception {
133143
Response response =
134144
bucketEndpoint.put(BUCKET_NAME, null);
135145
assertEquals(HTTP_OK, response.getStatus());
136-
S3BucketAcl getResponse =
137-
bucketEndpoint.getAcl(BUCKET_NAME);
146+
S3BucketAcl getResponse = getBucketAcl(BUCKET_NAME);
138147
assertEquals(1, getResponse.getAclList().getGrantList().size());
139148
assertEquals(S3Acl.ACLType.READ_ACP.getValue(),
140149
getResponse.getAclList().getGrantList().get(0).getPermission());
@@ -148,7 +157,7 @@ public void testWriteACP() throws Exception {
148157
Response response =
149158
bucketEndpoint.put(BUCKET_NAME, null);
150159
assertEquals(HTTP_OK, response.getStatus());
151-
S3BucketAcl getResponse = bucketEndpoint.getAcl(BUCKET_NAME);
160+
S3BucketAcl getResponse = getBucketAcl(BUCKET_NAME);
152161
assertEquals(1, getResponse.getAclList().getGrantList().size());
153162
assertEquals(S3Acl.ACLType.WRITE_ACP.getValue(),
154163
getResponse.getAclList().getGrantList().get(0).getPermission());
@@ -162,7 +171,7 @@ public void testFullControl() throws Exception {
162171
Response response =
163172
bucketEndpoint.put(BUCKET_NAME, null);
164173
assertEquals(HTTP_OK, response.getStatus());
165-
S3BucketAcl getResponse = bucketEndpoint.getAcl(BUCKET_NAME);
174+
S3BucketAcl getResponse = getBucketAcl(BUCKET_NAME);
166175
assertEquals(1, getResponse.getAclList().getGrantList().size());
167176
assertEquals(S3Acl.ACLType.FULL_CONTROL.getValue(),
168177
getResponse.getAclList().getGrantList().get(0).getPermission());
@@ -184,7 +193,7 @@ public void testCombination() throws Exception {
184193
Response response =
185194
bucketEndpoint.put(BUCKET_NAME, null);
186195
assertEquals(HTTP_OK, response.getStatus());
187-
S3BucketAcl getResponse = bucketEndpoint.getAcl(BUCKET_NAME);
196+
S3BucketAcl getResponse = getBucketAcl(BUCKET_NAME);
188197
assertEquals(5, getResponse.getAclList().getGrantList().size());
189198
}
190199

@@ -197,7 +206,7 @@ public void testPutClearOldAcls() throws Exception {
197206
Response response =
198207
bucketEndpoint.put(BUCKET_NAME, null);
199208
assertEquals(HTTP_OK, response.getStatus());
200-
S3BucketAcl getResponse = bucketEndpoint.getAcl(BUCKET_NAME);
209+
S3BucketAcl getResponse = getBucketAcl(BUCKET_NAME);
201210
assertEquals(1, getResponse.getAclList().getGrantList().size());
202211
assertEquals(S3Acl.ACLType.READ.getValue(),
203212
getResponse.getAclList().getGrantList().get(0).getPermission());
@@ -214,7 +223,7 @@ public void testPutClearOldAcls() throws Exception {
214223
response =
215224
bucketEndpoint.put(BUCKET_NAME, null);
216225
assertEquals(HTTP_OK, response.getStatus());
217-
getResponse = bucketEndpoint.getAcl(BUCKET_NAME);
226+
getResponse = getBucketAcl(BUCKET_NAME);
218227
assertEquals(1, getResponse.getAclList().getGrantList().size());
219228
assertEquals(S3Acl.ACLType.WRITE.getValue(),
220229
getResponse.getAclList().getGrantList().get(0).getPermission());
@@ -241,7 +250,7 @@ public void testAclInBody() throws Exception {
241250
Response response =
242251
bucketEndpoint.put(BUCKET_NAME, inputBody);
243252
assertEquals(HTTP_OK, response.getStatus());
244-
S3BucketAcl getResponse = bucketEndpoint.getAcl(BUCKET_NAME);
253+
S3BucketAcl getResponse = getBucketAcl(BUCKET_NAME);
245254
assertEquals(2, getResponse.getAclList().getGrantList().size());
246255
}
247256

@@ -251,7 +260,7 @@ public void testBucketNotExist() throws Exception {
251260
when(headers.getHeaderString(S3Acl.GRANT_READ))
252261
.thenReturn(S3Acl.ACLIdentityType.USER.getHeaderType() + "=root");
253262
OS3Exception e = assertThrows(OS3Exception.class, () ->
254-
bucketEndpoint.getAcl("bucket-not-exist"));
263+
bucketEndpoint.get("bucket-not-exist"));
255264
assertEquals(e.getHttpCode(), HTTP_NOT_FOUND);
256265
}
257266
}

0 commit comments

Comments
 (0)