Skip to content

Commit e4bc4fe

Browse files
adoroszlaiechonesis
adoroszlai
authored and
echonesis
committed
HDDS-14204. Reduce duplication in auditing S3G requests (apache#9524)
1 parent 937c157 commit e4bc4fe

6 files changed

Lines changed: 77 additions & 121 deletions

File tree

‎hadoop-hdds/framework/src/main/java/org/apache/hadoop/ozone/audit/AuditMessage.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,10 @@ public Builder withParams(Map<String, String> args) {
101101
return this;
102102
}
103103

104+
public Map<String, String> getParams() {
105+
return params;
106+
}
107+
104108
public Builder withResult(AuditEventStatus result) {
105109
this.ret = result.getStatus();
106110
return this;

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

Lines changed: 27 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,8 @@
5353
import javax.ws.rs.core.Response;
5454
import org.apache.commons.lang3.StringUtils;
5555
import org.apache.hadoop.ozone.OzoneAcl;
56+
import org.apache.hadoop.ozone.audit.AuditEventStatus;
57+
import org.apache.hadoop.ozone.audit.AuditMessage;
5658
import org.apache.hadoop.ozone.audit.S3GAction;
5759
import org.apache.hadoop.ozone.client.OzoneBucket;
5860
import org.apache.hadoop.ozone.client.OzoneKey;
@@ -138,8 +140,7 @@ public Response get(
138140
s3GAction = S3GAction.GET_ACL;
139141
S3BucketAcl result = getAcl(bucketName);
140142
getMetrics().updateGetAclSuccessStats(startNanos);
141-
AUDIT.logReadSuccess(
142-
buildAuditMessageForSuccess(s3GAction, getAuditParameters()));
143+
auditReadSuccess(s3GAction);
143144
return Response.ok(result, MediaType.APPLICATION_XML_TYPE).build();
144145
}
145146

@@ -175,8 +176,7 @@ public Response get(
175176
ozoneKeyIterator = bucket.listKeys(prefix, prevKey, shallow);
176177

177178
} catch (OMException ex) {
178-
AUDIT.logReadFailure(
179-
buildAuditMessageForFailure(s3GAction, getAuditParameters(), ex));
179+
auditReadFailure(s3GAction, ex);
180180
getMetrics().updateGetBucketFailureStats(startNanos);
181181
if (isAccessDenied(ex)) {
182182
throw newError(S3ErrorTable.ACCESS_DENIED, bucketName, ex);
@@ -188,8 +188,7 @@ public Response get(
188188
}
189189
} catch (Exception ex) {
190190
getMetrics().updateGetBucketFailureStats(startNanos);
191-
AUDIT.logReadFailure(
192-
buildAuditMessageForFailure(s3GAction, getAuditParameters(), ex));
191+
auditReadFailure(s3GAction, ex);
193192
throw ex;
194193
}
195194

@@ -298,8 +297,7 @@ public Response get(
298297
getMetrics().incListKeyCount(keyCount);
299298
perf.appendCount(keyCount);
300299
perf.appendOpLatencyNanos(opLatencyNs);
301-
AUDIT.logReadSuccess(buildAuditMessageForSuccess(s3GAction,
302-
getAuditParameters(), perf));
300+
auditReadSuccess(s3GAction, perf);
303301
response.setKeyCount(keyCount);
304302
return Response.ok(response).build();
305303
}
@@ -320,28 +318,15 @@ public Response put(@PathParam("bucket") String bucketName,
320318
S3GAction s3GAction = S3GAction.CREATE_BUCKET;
321319

322320
try {
323-
// Build map of query parameters
324-
Map<String, String> queryParams = new HashMap<>();
325-
queryParams.put("acl", aclMarker);
326-
// Future handlers: queryParams.put("lifecycle", lifecycleMarker);
327-
328-
// Check for subresource operations using handlers
329-
String queryParam = HANDLER_FACTORY.findFirstSupportedQueryParam(queryParams);
330-
331-
if (queryParam != null) {
332-
BucketOperationHandler handler = HANDLER_FACTORY.getHandler(queryParam);
333-
// Delegate to specific handler
334-
s3GAction = getActionForQueryParam(queryParam);
335-
Response response = handler.handlePutRequest(
336-
bucketName, body, getHeaders(), getBucketContext(), startNanos);
337-
AUDIT.logWriteSuccess(
338-
buildAuditMessageForSuccess(s3GAction, getAuditParameters()));
321+
if (aclMarker != null) {
322+
s3GAction = S3GAction.PUT_ACL;
323+
Response response = putAcl(bucketName, body);
324+
auditWriteSuccess(s3GAction);
339325
return response;
340326
}
341327

342328
String location = createS3Bucket(bucketName);
343-
AUDIT.logWriteSuccess(
344-
buildAuditMessageForSuccess(s3GAction, getAuditParameters()));
329+
auditWriteSuccess(s3GAction);
345330
getMetrics().updateCreateBucketSuccessStats(startNanos);
346331
return Response.status(HttpStatus.SC_OK).header("Location", location)
347332
.build();
@@ -353,8 +338,7 @@ public Response put(@PathParam("bucket") String bucketName,
353338
}
354339
throw exception;
355340
} catch (Exception ex) {
356-
AUDIT.logWriteFailure(
357-
buildAuditMessageForFailure(s3GAction, getAuditParameters(), ex));
341+
auditWriteFailure(s3GAction, ex);
358342
throw ex;
359343
}
360344
}
@@ -413,22 +397,18 @@ public Response listMultipartUploads(
413397
upload.getCreationTime(),
414398
S3StorageType.fromReplicationConfig(upload.getReplicationConfig())
415399
)));
416-
AUDIT.logReadSuccess(buildAuditMessageForSuccess(s3GAction,
417-
getAuditParameters()));
400+
auditReadSuccess(s3GAction);
418401
getMetrics().updateListMultipartUploadsSuccessStats(startNanos);
419402
return Response.ok(result).build();
420403
} catch (OMException exception) {
421-
AUDIT.logReadFailure(
422-
buildAuditMessageForFailure(s3GAction, getAuditParameters(),
423-
exception));
404+
auditReadFailure(s3GAction, exception);
424405
getMetrics().updateListMultipartUploadsFailureStats(startNanos);
425406
if (isAccessDenied(exception)) {
426407
throw newError(S3ErrorTable.ACCESS_DENIED, prefix, exception);
427408
}
428409
throw exception;
429410
} catch (Exception ex) {
430-
AUDIT.logReadFailure(
431-
buildAuditMessageForFailure(s3GAction, getAuditParameters(), ex));
411+
auditReadFailure(s3GAction, ex);
432412
throw ex;
433413
}
434414
}
@@ -447,13 +427,11 @@ public Response head(@PathParam("bucket") String bucketName)
447427
try {
448428
OzoneBucket bucket = getBucket(bucketName);
449429
S3Owner.verifyBucketOwnerCondition(getHeaders(), bucketName, bucket.getOwner());
450-
AUDIT.logReadSuccess(
451-
buildAuditMessageForSuccess(s3GAction, getAuditParameters()));
430+
auditReadSuccess(s3GAction);
452431
getMetrics().updateHeadBucketSuccessStats(startNanos);
453432
return Response.ok().build();
454433
} catch (Exception e) {
455-
AUDIT.logReadFailure(
456-
buildAuditMessageForFailure(s3GAction, getAuditParameters(), e));
434+
auditReadFailure(s3GAction, e);
457435
throw e;
458436
}
459437
}
@@ -477,8 +455,7 @@ public Response delete(@PathParam("bucket") String bucketName)
477455
}
478456
deleteS3Bucket(bucketName);
479457
} catch (OMException ex) {
480-
AUDIT.logWriteFailure(
481-
buildAuditMessageForFailure(s3GAction, getAuditParameters(), ex));
458+
auditWriteFailure(s3GAction, ex);
482459
getMetrics().updateDeleteBucketFailureStats(startNanos);
483460
if (ex.getResult() == ResultCodes.BUCKET_NOT_EMPTY) {
484461
throw newError(S3ErrorTable.BUCKET_NOT_EMPTY, bucketName, ex);
@@ -490,13 +467,11 @@ public Response delete(@PathParam("bucket") String bucketName)
490467
throw ex;
491468
}
492469
} catch (Exception ex) {
493-
AUDIT.logWriteFailure(
494-
buildAuditMessageForFailure(s3GAction, getAuditParameters(), ex));
470+
auditWriteFailure(s3GAction, ex);
495471
throw ex;
496472
}
497473

498-
AUDIT.logWriteSuccess(buildAuditMessageForSuccess(s3GAction,
499-
getAuditParameters()));
474+
auditWriteSuccess(s3GAction);
500475
getMetrics().updateDeleteBucketSuccessStats(startNanos);
501476
return Response
502477
.status(HttpStatus.SC_NO_CONTENT)
@@ -555,15 +530,16 @@ public MultiDeleteResponse multiDelete(@PathParam("bucket") String bucketName,
555530
}
556531
}
557532

558-
Map<String, String> auditMap = getAuditParameters();
559-
auditMap.put("failedDeletes", deleteKeys.toString());
533+
AuditMessage.Builder message = auditMessageFor(s3GAction);
534+
message.getParams().put("failedDeletes", deleteKeys.toString());
535+
560536
if (!result.getErrors().isEmpty()) {
561-
AUDIT.logWriteFailure(buildAuditMessageForFailure(s3GAction,
562-
auditMap, new Exception("MultiDelete Exception")));
537+
AUDIT.logWriteFailure(message.withResult(AuditEventStatus.FAILURE)
538+
.withException(new Exception("MultiDelete Exception")).build());
563539
} else {
564-
AUDIT.logWriteSuccess(
565-
buildAuditMessageForSuccess(s3GAction, auditMap));
540+
AUDIT.logWriteSuccess(message.withResult(AuditEventStatus.SUCCESS).build());
566541
}
542+
567543
return result;
568544
}
569545

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

Lines changed: 26 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,6 @@
6060
import org.apache.hadoop.ozone.audit.AuditLogger.PerformanceStringBuilder;
6161
import org.apache.hadoop.ozone.audit.AuditLoggerType;
6262
import org.apache.hadoop.ozone.audit.AuditMessage;
63-
import org.apache.hadoop.ozone.audit.Auditor;
6463
import org.apache.hadoop.ozone.client.OzoneBucket;
6564
import org.apache.hadoop.ozone.client.OzoneClient;
6665
import org.apache.hadoop.ozone.client.OzoneKey;
@@ -84,7 +83,7 @@
8483
/**
8584
* Basic helpers for all the REST endpoints.
8685
*/
87-
public abstract class EndpointBase implements Auditor {
86+
public abstract class EndpointBase {
8887

8988
protected static final String ETAG_CUSTOM = "etag-custom";
9089

@@ -107,7 +106,7 @@ public abstract class EndpointBase implements Auditor {
107106
@Context
108107
private HttpHeaders headers;
109108

110-
private Set<String> excludeMetadataFields =
109+
private final Set<String> excludeMetadataFields =
111110
new HashSet<>(Arrays.asList(OzoneConsts.GDPR_FLAG, STORAGE_CONFIG_HEADER));
112111
private static final Logger LOG =
113112
LoggerFactory.getLogger(EndpointBase.class);
@@ -456,8 +455,8 @@ protected static <KV> Map<String, String> validateAndGetTagging(
456455
return Collections.unmodifiableMap(tags);
457456
}
458457

459-
private AuditMessage.Builder auditMessageBaseBuilder(AuditAction op,
460-
Map<String, String> auditMap) {
458+
protected AuditMessage.Builder auditMessageFor(AuditAction op) {
459+
Map<String, String> auditMap = getAuditParameters();
461460
auditMap.put("x-amz-request-id", requestIdentifier.getRequestId());
462461
auditMap.put("x-amz-id-2", requestIdentifier.getAmzId());
463462

@@ -475,29 +474,15 @@ private AuditMessage.Builder auditMessageBaseBuilder(AuditAction op,
475474
return builder;
476475
}
477476

478-
@Override
479-
public AuditMessage buildAuditMessageForSuccess(AuditAction op,
480-
Map<String, String> auditMap) {
481-
AuditMessage.Builder builder = auditMessageBaseBuilder(op, auditMap)
477+
protected AuditMessage.Builder auditMessageForSuccess(AuditAction op) {
478+
return auditMessageFor(op)
482479
.withResult(AuditEventStatus.SUCCESS);
483-
return builder.build();
484480
}
485481

486-
public AuditMessage buildAuditMessageForSuccess(AuditAction op,
487-
Map<String, String> auditMap, PerformanceStringBuilder performance) {
488-
AuditMessage.Builder builder = auditMessageBaseBuilder(op, auditMap)
489-
.withResult(AuditEventStatus.SUCCESS);
490-
builder.setPerformance(performance);
491-
return builder.build();
492-
}
493-
494-
@Override
495-
public AuditMessage buildAuditMessageForFailure(AuditAction op,
496-
Map<String, String> auditMap, Throwable throwable) {
497-
AuditMessage.Builder builder = auditMessageBaseBuilder(op, auditMap)
482+
protected AuditMessage.Builder auditMessageForFailure(AuditAction op, Throwable throwable) {
483+
return auditMessageFor(op)
498484
.withResult(AuditEventStatus.FAILURE)
499485
.withException(throwable);
500-
return builder.build();
501486
}
502487

503488
@VisibleForTesting
@@ -556,14 +541,28 @@ protected Map<String, String> getAuditParameters() {
556541
return AuditUtils.getAuditParameters(context);
557542
}
558543

544+
protected void auditWriteSuccess(AuditAction action, PerformanceStringBuilder perf) {
545+
AUDIT.logWriteSuccess(auditMessageForSuccess(action).setPerformance(perf).build());
546+
}
547+
548+
protected void auditWriteSuccess(AuditAction action) {
549+
AUDIT.logWriteSuccess(auditMessageForSuccess(action).build());
550+
}
551+
552+
protected void auditReadSuccess(AuditAction action, PerformanceStringBuilder perf) {
553+
AUDIT.logReadSuccess(auditMessageForSuccess(action).setPerformance(perf).build());
554+
}
555+
556+
protected void auditReadSuccess(AuditAction action) {
557+
AUDIT.logReadSuccess(auditMessageForSuccess(action).build());
558+
}
559+
559560
protected void auditWriteFailure(AuditAction action, Throwable ex) {
560-
AUDIT.logWriteFailure(
561-
buildAuditMessageForFailure(action, getAuditParameters(), ex));
561+
AUDIT.logWriteFailure(auditMessageForFailure(action, ex).build());
562562
}
563563

564564
protected void auditReadFailure(AuditAction action, Exception ex) {
565-
AUDIT.logReadFailure(
566-
buildAuditMessageForFailure(action, getAuditParameters(), ex));
565+
AUDIT.logReadFailure(auditMessageForFailure(action, ex).build());
567566
}
568567

569568
protected boolean isAccessDenied(OMException ex) {

0 commit comments

Comments
 (0)