Skip to content
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.

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

HDDS-12095. Include AWS request ID in S3G audit logs #7725

Merged

Conversation

peterxcli
Copy link
Contributor

@peterxcli peterxcli commented Jan 20, 2025

What changes were proposed in this pull request?

We have a RequestIdentifier object that is used to generate the request ID and amz ID to the S3 response.
It will be useful to include this S3G audit so that it's easier to for the administrators to locate problematic request from the audit logs when only request ID is provided.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-12095

How was this patch tested?

unit tests

@tanvipenumudy tanvipenumudy self-requested a review January 20, 2025 11:36
@adoroszlai
Copy link
Contributor

Thanks @peterxcli for the patch.

S3 acceptance tests are failing due to:

UnsupportedOperationException
	at java.base/java.util.AbstractMap.put(AbstractMap.java:213)
	at org.apache.hadoop.ozone.s3.endpoint.EndpointBase.auditMessageBaseBuilder(EndpointBase.java:451)
	at org.apache.hadoop.ozone.s3.endpoint.EndpointBase.buildAuditMessageForSuccess(EndpointBase.java:471)
	at org.apache.hadoop.ozone.s3.endpoint.RootEndpoint.get(RootEndpoint.java:87)

https://github.com/peterxcli/ozone/actions/runs/12866814923

Also, integration tests are failing with:

Error:  org.apache.hadoop.ozone.s3.endpoint.TestObjectTaggingPut.testPutInvalidObjectTagging  Time elapsed: 1.847 s  <<< ERROR!
java.lang.NullPointerException: Cannot invoke "org.apache.hadoop.ozone.s3.RequestIdentifier.getRequestId()" because "this.requestIdentifier" is null
	at org.apache.hadoop.ozone.s3.endpoint.EndpointBase.auditMessageBaseBuilder(EndpointBase.java:451)
	at org.apache.hadoop.ozone.s3.endpoint.EndpointBase.buildAuditMessageForSuccess(EndpointBase.java:478)
	at org.apache.hadoop.ozone.s3.endpoint.ObjectEndpoint.put(ObjectEndpoint.java:397)
	at org.apache.hadoop.ozone.s3.endpoint.TestObjectTaggingPut.setup(TestObjectTaggingPut.java:87)

https://github.com/peterxcli/ozone/actions/runs/12866814923/job/35870718714#step:6:2861

@peterxcli
Copy link
Contributor Author

@adoroszlai Thanks for reminding the error tests, just fixed them, PTAL. Thanks!

@adoroszlai
Copy link
Contributor

Thanks @peterxcli for fixing the tests. There is one more: TestMultipartObjectGet.

@peterxcli peterxcli force-pushed the h12095-add-aws-ids-in-s3g-audit-log branch from f64d1f7 to 33e3032 Compare January 21, 2025 08:54
@adoroszlai adoroszlai requested a review from ivandika3 January 21, 2025 10:40
@peterxcli
Copy link
Contributor Author

@peterxcli
Copy link
Contributor Author

Hi @adoroszlai, I’ve just started contributing to this project, and I had a quick question. Do I need to attach the CI run link to each PR, or is there a more convenient way to locate the CI runs without navigating to my fork and finding the action through the latest commit name?

Copy link
Contributor

@ivandika3 ivandika3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@peterxcli Thanks for picking this up. LGTM +1.

@adoroszlai
Copy link
Contributor

I’ve just started contributing to this project

Thank you, and welcome to Ozone!

Do I need to attach the CI run link to each PR,

When opening new PR:

  • Please wait for clean CI run in fork before opening PR. This lets you tweak the change until it passes tests.
  • Please provide link in the PR description to the successful CI run.
  • Once PR is opened like that, if new commit is pushed e.g. for review, there is no need to provide further links to runs from fork.

or is there a more convenient way to locate the CI runs without navigating to my fork and finding the action through the latest commit name?

This is a direct link to CI runs in your fork:
https://github.com/peterxcli/ozone/actions/workflows/post-commit.yml

@adoroszlai adoroszlai changed the title HDDS-12095: Include AWS request ID to S3G audit logs HDDS-12095. Include AWS request ID in S3G audit logs Jan 21, 2025
@adoroszlai adoroszlai merged commit a0be99a into apache:master Jan 21, 2025
42 of 43 checks passed
@adoroszlai
Copy link
Contributor

Thanks @peterxcli for the patch, @ivandika3 for the review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants