Skip to content

Commit da68b9e

Browse files
adoroszlaiadoroszlai
authored
HDDS-14204. Reduce duplication in auditing S3G requests (apache#9524)
1 parent 1c817b0 commit da68b9e

File tree

6 files changed

+74
-107
lines changed

6 files changed

+74
-107
lines changed

‎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: 24 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,8 @@
5555
import javax.ws.rs.core.Response;
5656
import org.apache.commons.lang3.StringUtils;
5757
import org.apache.hadoop.ozone.OzoneAcl;
58+
import org.apache.hadoop.ozone.audit.AuditEventStatus;
59+
import org.apache.hadoop.ozone.audit.AuditMessage;
5860
import org.apache.hadoop.ozone.audit.S3GAction;
5961
import org.apache.hadoop.ozone.client.OzoneBucket;
6062
import org.apache.hadoop.ozone.client.OzoneKey;
@@ -128,8 +130,7 @@ public Response get(
128130
s3GAction = S3GAction.GET_ACL;
129131
S3BucketAcl result = getAcl(bucketName);
130132
getMetrics().updateGetAclSuccessStats(startNanos);
131-
AUDIT.logReadSuccess(
132-
buildAuditMessageForSuccess(s3GAction, getAuditParameters()));
133+
auditReadSuccess(s3GAction);
133134
return Response.ok(result, MediaType.APPLICATION_XML_TYPE).build();
134135
}
135136

@@ -165,8 +166,7 @@ public Response get(
165166
ozoneKeyIterator = bucket.listKeys(prefix, prevKey, shallow);
166167

167168
} catch (OMException ex) {
168-
AUDIT.logReadFailure(
169-
buildAuditMessageForFailure(s3GAction, getAuditParameters(), ex));
169+
auditReadFailure(s3GAction, ex);
170170
getMetrics().updateGetBucketFailureStats(startNanos);
171171
if (isAccessDenied(ex)) {
172172
throw newError(S3ErrorTable.ACCESS_DENIED, bucketName, ex);
@@ -178,8 +178,7 @@ public Response get(
178178
}
179179
} catch (Exception ex) {
180180
getMetrics().updateGetBucketFailureStats(startNanos);
181-
AUDIT.logReadFailure(
182-
buildAuditMessageForFailure(s3GAction, getAuditParameters(), ex));
181+
auditReadFailure(s3GAction, ex);
183182
throw ex;
184183
}
185184

@@ -288,8 +287,7 @@ public Response get(
288287
getMetrics().incListKeyCount(keyCount);
289288
perf.appendCount(keyCount);
290289
perf.appendOpLatencyNanos(opLatencyNs);
291-
AUDIT.logReadSuccess(buildAuditMessageForSuccess(s3GAction,
292-
getAuditParameters(), perf));
290+
auditReadSuccess(s3GAction, perf);
293291
response.setKeyCount(keyCount);
294292
return Response.ok(response).build();
295293
}
@@ -313,13 +311,11 @@ public Response put(@PathParam("bucket") String bucketName,
313311
if (aclMarker != null) {
314312
s3GAction = S3GAction.PUT_ACL;
315313
Response response = putAcl(bucketName, body);
316-
AUDIT.logWriteSuccess(
317-
buildAuditMessageForSuccess(s3GAction, getAuditParameters()));
314+
auditWriteSuccess(s3GAction);
318315
return response;
319316
}
320317
String location = createS3Bucket(bucketName);
321-
AUDIT.logWriteSuccess(
322-
buildAuditMessageForSuccess(s3GAction, getAuditParameters()));
318+
auditWriteSuccess(s3GAction);
323319
getMetrics().updateCreateBucketSuccessStats(startNanos);
324320
return Response.status(HttpStatus.SC_OK).header("Location", location)
325321
.build();
@@ -331,8 +327,7 @@ public Response put(@PathParam("bucket") String bucketName,
331327
}
332328
throw exception;
333329
} catch (Exception ex) {
334-
AUDIT.logWriteFailure(
335-
buildAuditMessageForFailure(s3GAction, getAuditParameters(), ex));
330+
auditWriteFailure(s3GAction, ex);
336331
throw ex;
337332
}
338333
}
@@ -379,22 +374,18 @@ public Response listMultipartUploads(
379374
upload.getCreationTime(),
380375
S3StorageType.fromReplicationConfig(upload.getReplicationConfig())
381376
)));
382-
AUDIT.logReadSuccess(buildAuditMessageForSuccess(s3GAction,
383-
getAuditParameters()));
377+
auditReadSuccess(s3GAction);
384378
getMetrics().updateListMultipartUploadsSuccessStats(startNanos);
385379
return Response.ok(result).build();
386380
} catch (OMException exception) {
387-
AUDIT.logReadFailure(
388-
buildAuditMessageForFailure(s3GAction, getAuditParameters(),
389-
exception));
381+
auditReadFailure(s3GAction, exception);
390382
getMetrics().updateListMultipartUploadsFailureStats(startNanos);
391383
if (isAccessDenied(exception)) {
392384
throw newError(S3ErrorTable.ACCESS_DENIED, prefix, exception);
393385
}
394386
throw exception;
395387
} catch (Exception ex) {
396-
AUDIT.logReadFailure(
397-
buildAuditMessageForFailure(s3GAction, getAuditParameters(), ex));
388+
auditReadFailure(s3GAction, ex);
398389
throw ex;
399390
}
400391
}
@@ -413,13 +404,11 @@ public Response head(@PathParam("bucket") String bucketName)
413404
try {
414405
OzoneBucket bucket = getBucket(bucketName);
415406
S3Owner.verifyBucketOwnerCondition(getHeaders(), bucketName, bucket.getOwner());
416-
AUDIT.logReadSuccess(
417-
buildAuditMessageForSuccess(s3GAction, getAuditParameters()));
407+
auditReadSuccess(s3GAction);
418408
getMetrics().updateHeadBucketSuccessStats(startNanos);
419409
return Response.ok().build();
420410
} catch (Exception e) {
421-
AUDIT.logReadFailure(
422-
buildAuditMessageForFailure(s3GAction, getAuditParameters(), e));
411+
auditReadFailure(s3GAction, e);
423412
throw e;
424413
}
425414
}
@@ -443,8 +432,7 @@ public Response delete(@PathParam("bucket") String bucketName)
443432
}
444433
deleteS3Bucket(bucketName);
445434
} catch (OMException ex) {
446-
AUDIT.logWriteFailure(
447-
buildAuditMessageForFailure(s3GAction, getAuditParameters(), ex));
435+
auditWriteFailure(s3GAction, ex);
448436
getMetrics().updateDeleteBucketFailureStats(startNanos);
449437
if (ex.getResult() == ResultCodes.BUCKET_NOT_EMPTY) {
450438
throw newError(S3ErrorTable.BUCKET_NOT_EMPTY, bucketName, ex);
@@ -456,13 +444,11 @@ public Response delete(@PathParam("bucket") String bucketName)
456444
throw ex;
457445
}
458446
} catch (Exception ex) {
459-
AUDIT.logWriteFailure(
460-
buildAuditMessageForFailure(s3GAction, getAuditParameters(), ex));
447+
auditWriteFailure(s3GAction, ex);
461448
throw ex;
462449
}
463450

464-
AUDIT.logWriteSuccess(buildAuditMessageForSuccess(s3GAction,
465-
getAuditParameters()));
451+
auditWriteSuccess(s3GAction);
466452
getMetrics().updateDeleteBucketSuccessStats(startNanos);
467453
return Response
468454
.status(HttpStatus.SC_NO_CONTENT)
@@ -521,15 +507,16 @@ public MultiDeleteResponse multiDelete(@PathParam("bucket") String bucketName,
521507
}
522508
}
523509

524-
Map<String, String> auditMap = getAuditParameters();
525-
auditMap.put("failedDeletes", deleteKeys.toString());
510+
AuditMessage.Builder message = auditMessageFor(s3GAction);
511+
message.getParams().put("failedDeletes", deleteKeys.toString());
512+
526513
if (!result.getErrors().isEmpty()) {
527-
AUDIT.logWriteFailure(buildAuditMessageForFailure(s3GAction,
528-
auditMap, new Exception("MultiDelete Exception")));
514+
AUDIT.logWriteFailure(message.withResult(AuditEventStatus.FAILURE)
515+
.withException(new Exception("MultiDelete Exception")).build());
529516
} else {
530-
AUDIT.logWriteSuccess(
531-
buildAuditMessageForSuccess(s3GAction, auditMap));
517+
AUDIT.logWriteSuccess(message.withResult(AuditEventStatus.SUCCESS).build());
532518
}
519+
533520
return result;
534521
}
535522

‎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)