Skip to content

HDDS-14204. Reduce duplication in auditing S3G requests #9524

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
peterxcli merged 1 commit into from
Dec 18, 2025
Merged

HDDS-14204. Reduce duplication in auditing S3G requests #9524

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions hadoop-hdds/framework/src/main/java/org/apache/hadoop/ozone/audit/AuditMessage.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,10 @@ public Builder withParams(Map<String, String> args) {
return this;
}

public Map<String, String> getParams() {
return params;
}

public Builder withResult(AuditEventStatus result) {
this.ret = result.getStatus();
return this;
Expand Down
61 changes: 24 additions & 37 deletions hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@
import javax.ws.rs.core.Response;
import org.apache.commons.lang3.StringUtils;
import org.apache.hadoop.ozone.OzoneAcl;
import org.apache.hadoop.ozone.audit.AuditEventStatus;
import org.apache.hadoop.ozone.audit.AuditMessage;
import org.apache.hadoop.ozone.audit.S3GAction;
import org.apache.hadoop.ozone.client.OzoneBucket;
import org.apache.hadoop.ozone.client.OzoneKey;
Expand Down Expand Up @@ -128,8 +130,7 @@ public Response get(
s3GAction = S3GAction.GET_ACL;
S3BucketAcl result = getAcl(bucketName);
getMetrics().updateGetAclSuccessStats(startNanos);
AUDIT.logReadSuccess(
buildAuditMessageForSuccess(s3GAction, getAuditParameters()));
auditReadSuccess(s3GAction);
return Response.ok(result, MediaType.APPLICATION_XML_TYPE).build();
}

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

} catch (OMException ex) {
AUDIT.logReadFailure(
buildAuditMessageForFailure(s3GAction, getAuditParameters(), ex));
auditReadFailure(s3GAction, ex);
getMetrics().updateGetBucketFailureStats(startNanos);
if (isAccessDenied(ex)) {
throw newError(S3ErrorTable.ACCESS_DENIED, bucketName, ex);
Expand All @@ -178,8 +178,7 @@ public Response get(
}
} catch (Exception ex) {
getMetrics().updateGetBucketFailureStats(startNanos);
AUDIT.logReadFailure(
buildAuditMessageForFailure(s3GAction, getAuditParameters(), ex));
auditReadFailure(s3GAction, ex);
throw ex;
}

Expand Down Expand Up @@ -288,8 +287,7 @@ public Response get(
getMetrics().incListKeyCount(keyCount);
perf.appendCount(keyCount);
perf.appendOpLatencyNanos(opLatencyNs);
AUDIT.logReadSuccess(buildAuditMessageForSuccess(s3GAction,
getAuditParameters(), perf));
auditReadSuccess(s3GAction, perf);
response.setKeyCount(keyCount);
return Response.ok(response).build();
}
Expand All @@ -313,13 +311,11 @@ public Response put(@PathParam("bucket") String bucketName,
if (aclMarker != null) {
s3GAction = S3GAction.PUT_ACL;
Response response = putAcl(bucketName, body);
AUDIT.logWriteSuccess(
buildAuditMessageForSuccess(s3GAction, getAuditParameters()));
auditWriteSuccess(s3GAction);
return response;
}
String location = createS3Bucket(bucketName);
AUDIT.logWriteSuccess(
buildAuditMessageForSuccess(s3GAction, getAuditParameters()));
auditWriteSuccess(s3GAction);
getMetrics().updateCreateBucketSuccessStats(startNanos);
return Response.status(HttpStatus.SC_OK).header("Location", location)
.build();
Expand All @@ -331,8 +327,7 @@ public Response put(@PathParam("bucket") String bucketName,
}
throw exception;
} catch (Exception ex) {
AUDIT.logWriteFailure(
buildAuditMessageForFailure(s3GAction, getAuditParameters(), ex));
auditWriteFailure(s3GAction, ex);
throw ex;
}
}
Expand Down Expand Up @@ -379,22 +374,18 @@ public Response listMultipartUploads(
upload.getCreationTime(),
S3StorageType.fromReplicationConfig(upload.getReplicationConfig())
)));
AUDIT.logReadSuccess(buildAuditMessageForSuccess(s3GAction,
getAuditParameters()));
auditReadSuccess(s3GAction);
getMetrics().updateListMultipartUploadsSuccessStats(startNanos);
return Response.ok(result).build();
} catch (OMException exception) {
AUDIT.logReadFailure(
buildAuditMessageForFailure(s3GAction, getAuditParameters(),
exception));
auditReadFailure(s3GAction, exception);
getMetrics().updateListMultipartUploadsFailureStats(startNanos);
if (isAccessDenied(exception)) {
throw newError(S3ErrorTable.ACCESS_DENIED, prefix, exception);
}
throw exception;
} catch (Exception ex) {
AUDIT.logReadFailure(
buildAuditMessageForFailure(s3GAction, getAuditParameters(), ex));
auditReadFailure(s3GAction, ex);
throw ex;
}
}
Expand All @@ -413,13 +404,11 @@ public Response head(@PathParam("bucket") String bucketName)
try {
OzoneBucket bucket = getBucket(bucketName);
S3Owner.verifyBucketOwnerCondition(getHeaders(), bucketName, bucket.getOwner());
AUDIT.logReadSuccess(
buildAuditMessageForSuccess(s3GAction, getAuditParameters()));
auditReadSuccess(s3GAction);
getMetrics().updateHeadBucketSuccessStats(startNanos);
return Response.ok().build();
} catch (Exception e) {
AUDIT.logReadFailure(
buildAuditMessageForFailure(s3GAction, getAuditParameters(), e));
auditReadFailure(s3GAction, e);
throw e;
}
}
Expand All @@ -443,8 +432,7 @@ public Response delete(@PathParam("bucket") String bucketName)
}
deleteS3Bucket(bucketName);
} catch (OMException ex) {
AUDIT.logWriteFailure(
buildAuditMessageForFailure(s3GAction, getAuditParameters(), ex));
auditWriteFailure(s3GAction, ex);
getMetrics().updateDeleteBucketFailureStats(startNanos);
if (ex.getResult() == ResultCodes.BUCKET_NOT_EMPTY) {
throw newError(S3ErrorTable.BUCKET_NOT_EMPTY, bucketName, ex);
Expand All @@ -456,13 +444,11 @@ public Response delete(@PathParam("bucket") String bucketName)
throw ex;
}
} catch (Exception ex) {
AUDIT.logWriteFailure(
buildAuditMessageForFailure(s3GAction, getAuditParameters(), ex));
auditWriteFailure(s3GAction, ex);
throw ex;
}

AUDIT.logWriteSuccess(buildAuditMessageForSuccess(s3GAction,
getAuditParameters()));
auditWriteSuccess(s3GAction);
getMetrics().updateDeleteBucketSuccessStats(startNanos);
return Response
.status(HttpStatus.SC_NO_CONTENT)
Expand Down Expand Up @@ -521,15 +507,16 @@ public MultiDeleteResponse multiDelete(@PathParam("bucket") String bucketName,
}
}

Map<String, String> auditMap = getAuditParameters();
auditMap.put("failedDeletes", deleteKeys.toString());
AuditMessage.Builder message = auditMessageFor(s3GAction);
message.getParams().put("failedDeletes", deleteKeys.toString());

if (!result.getErrors().isEmpty()) {
AUDIT.logWriteFailure(buildAuditMessageForFailure(s3GAction,
auditMap, new Exception("MultiDelete Exception")));
AUDIT.logWriteFailure(message.withResult(AuditEventStatus.FAILURE)
.withException(new Exception("MultiDelete Exception")).build());
} else {
AUDIT.logWriteSuccess(
buildAuditMessageForSuccess(s3GAction, auditMap));
AUDIT.logWriteSuccess(message.withResult(AuditEventStatus.SUCCESS).build());
}

return result;
}

Expand Down
53 changes: 26 additions & 27 deletions hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/EndpointBase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@
import org.apache.hadoop.ozone.audit.AuditLogger.PerformanceStringBuilder;
import org.apache.hadoop.ozone.audit.AuditLoggerType;
import org.apache.hadoop.ozone.audit.AuditMessage;
import org.apache.hadoop.ozone.audit.Auditor;
import org.apache.hadoop.ozone.client.OzoneBucket;
import org.apache.hadoop.ozone.client.OzoneClient;
import org.apache.hadoop.ozone.client.OzoneKey;
Expand All @@ -84,7 +83,7 @@
/**
* Basic helpers for all the REST endpoints.
*/
public abstract class EndpointBase implements Auditor {
public abstract class EndpointBase {

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

Expand All @@ -107,7 +106,7 @@ public abstract class EndpointBase implements Auditor {
@Context
private HttpHeaders headers;

private Set<String> excludeMetadataFields =
private final Set<String> excludeMetadataFields =
new HashSet<>(Arrays.asList(OzoneConsts.GDPR_FLAG, STORAGE_CONFIG_HEADER));
private static final Logger LOG =
LoggerFactory.getLogger(EndpointBase.class);
Expand Down Expand Up @@ -456,8 +455,8 @@ protected static <KV> Map<String, String> validateAndGetTagging(
return Collections.unmodifiableMap(tags);
}

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

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

@Override
public AuditMessage buildAuditMessageForSuccess(AuditAction op,
Map<String, String> auditMap) {
AuditMessage.Builder builder = auditMessageBaseBuilder(op, auditMap)
protected AuditMessage.Builder auditMessageForSuccess(AuditAction op) {
return auditMessageFor(op)
.withResult(AuditEventStatus.SUCCESS);
return builder.build();
}

public AuditMessage buildAuditMessageForSuccess(AuditAction op,
Map<String, String> auditMap, PerformanceStringBuilder performance) {
AuditMessage.Builder builder = auditMessageBaseBuilder(op, auditMap)
.withResult(AuditEventStatus.SUCCESS);
builder.setPerformance(performance);
return builder.build();
}

@Override
public AuditMessage buildAuditMessageForFailure(AuditAction op,
Map<String, String> auditMap, Throwable throwable) {
AuditMessage.Builder builder = auditMessageBaseBuilder(op, auditMap)
protected AuditMessage.Builder auditMessageForFailure(AuditAction op, Throwable throwable) {
return auditMessageFor(op)
.withResult(AuditEventStatus.FAILURE)
.withException(throwable);
return builder.build();
}

@VisibleForTesting
Expand Down Expand Up @@ -556,14 +541,28 @@ protected Map<String, String> getAuditParameters() {
return AuditUtils.getAuditParameters(context);
}

protected void auditWriteSuccess(AuditAction action, PerformanceStringBuilder perf) {
AUDIT.logWriteSuccess(auditMessageForSuccess(action).setPerformance(perf).build());
}

protected void auditWriteSuccess(AuditAction action) {
AUDIT.logWriteSuccess(auditMessageForSuccess(action).build());
}

protected void auditReadSuccess(AuditAction action, PerformanceStringBuilder perf) {
AUDIT.logReadSuccess(auditMessageForSuccess(action).setPerformance(perf).build());
}

protected void auditReadSuccess(AuditAction action) {
AUDIT.logReadSuccess(auditMessageForSuccess(action).build());
}

protected void auditWriteFailure(AuditAction action, Throwable ex) {
AUDIT.logWriteFailure(
buildAuditMessageForFailure(action, getAuditParameters(), ex));
AUDIT.logWriteFailure(auditMessageForFailure(action, ex).build());
}

protected void auditReadFailure(AuditAction action, Exception ex) {
AUDIT.logReadFailure(
buildAuditMessageForFailure(action, getAuditParameters(), ex));
AUDIT.logReadFailure(auditMessageForFailure(action, ex).build());
}

protected boolean isAccessDenied(OMException ex) {
Expand Down
Loading
Loading