You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2022/04/20 16:30:59 UTC

[GitHub] [ozone] symious opened a new pull request, #3325: HDDS-6620. Gatter AuditLog params for S3g

symious opened a new pull request, #3325:
URL: https://github.com/apache/ozone/pull/3325

   ## What changes were proposed in this pull request?
   
   In [HDDS-6525](https://issues.apache.org/jira/browse/HDDS-6525), we added auditLog for S3g. But the auditLog param is collected in each request.
   
   As adviced by [Ritesh H Shukla](https://issues.apache.org/jira/secure/ViewProfile.jspa?name=kerneltime) , we can generate this parameter in one place.
   
   This ticket is to gather the generation for this parameter.
   
   ## What is the link to the Apache JIRA
   
   https://issues.apache.org/jira/browse/HDDS-6620
   
   ## How was this patch tested?
   
   manual test. Audit logs works fine.
   ```
   2022-04-20 16:12:16,073 | INFO  | S3GAudit | user=yiyang.zhou | ip=172.21.0.1 | op=LIST_S3_BUCKETS {} | ret=SUCCESS |
   2022-04-20 16:12:25,812 | INFO  | S3GAudit | user=yiyang.zhou | ip=172.21.0.1 | op=CREATE_BUCKET {bucket=[test-bucket]} | ret=SUCCESS |
   2022-04-20 16:12:32,391 | INFO  | S3GAudit | user=yiyang.zhou | ip=172.21.0.1 | op=INIT_MULTIPART_UPLOAD {bucket=[test-bucket], path=[yiyang-mutest], uploads=[]} | ret=SUCCESS |
   2022-04-20 16:19:07,834 | INFO  | S3GAudit | user=yiyang.zhou | ip=172.21.0.1 | op=CREATE_MULTIPART_KEY {bucket=[test-bucket], path=[yiyang-mutest], uploadId=[589d46ef-c8d0-453d-b813-48750c72aa5a-108165277442375680], partNumber=[1]} | ret=SUCCESS |
   2022-04-20 16:20:31,382 | INFO  | S3GAudit | user=yiyang.zhou | ip=172.21.0.1 | op=CREATE_MULTIPART_KEY {bucket=[test-bucket], path=[yiyang-mutest], uploadId=[589d46ef-c8d0-453d-b813-48750c72aa5a-108165277442375680], partNumber=[2]} | ret=SUCCESS |
   2022-04-20 16:24:13,763 | INFO  | S3GAudit | user=yiyang.zhou | ip=172.21.0.1 | op=COMPLETE_MULTIPART_UPLOAD {bucket=[test-bucket], path=[yiyang-mutest], uploadId=[589d46ef-c8d0-453d-b813-48750c72aa5a-108165277442375680]} | ret=SUCCESS |
   ```
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] adoroszlai commented on pull request #3325: HDDS-6620. Gatter AuditLog params for S3g

Posted by GitBox <gi...@apache.org>.
adoroszlai commented on PR #3325:
URL: https://github.com/apache/ozone/pull/3325#issuecomment-1108391475

   Thanks @symious for the patch, @kerneltime for the review.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] kerneltime commented on pull request #3325: HDDS-6620. Gatter AuditLog params for S3g

Posted by GitBox <gi...@apache.org>.
kerneltime commented on PR #3325:
URL: https://github.com/apache/ozone/pull/3325#issuecomment-1104393938

   LGTM barring comment from @adoroszlai.
   Thank you @symious for the quick turn around.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] adoroszlai commented on a diff in pull request #3325: HDDS-6620. Gatter AuditLog params for S3g

Posted by GitBox <gi...@apache.org>.
adoroszlai commented on code in PR #3325:
URL: https://github.com/apache/ozone/pull/3325#discussion_r854339720


##########
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/TestS3GatewayAuditLog.java:
##########
@@ -101,7 +101,7 @@ public static void tearDown() {
   public void testHeadBucket() throws Exception {
     bucketEndpoint.head(bucketName);
     String expected = "INFO  | S3GAudit | ? | user=null | ip=null | " +
-        "op=HEAD_BUCKET {bucket=bucket} | ret=SUCCESS";
+        "op=HEAD_BUCKET {} | ret=SUCCESS";

Review Comment:
   This seems to be loss of information (name of bucket (also key name for the `HEAD_KEY`)).



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] adoroszlai merged pull request #3325: HDDS-6620. Gatter AuditLog params for S3g

Posted by GitBox <gi...@apache.org>.
adoroszlai merged PR #3325:
URL: https://github.com/apache/ozone/pull/3325


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] adoroszlai commented on a diff in pull request #3325: HDDS-6620. Gatter AuditLog params for S3g

Posted by GitBox <gi...@apache.org>.
adoroszlai commented on code in PR #3325:
URL: https://github.com/apache/ozone/pull/3325#discussion_r854353784


##########
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/TestS3GatewayAuditLog.java:
##########
@@ -101,7 +101,7 @@ public static void tearDown() {
   public void testHeadBucket() throws Exception {
     bucketEndpoint.head(bucketName);
     String expected = "INFO  | S3GAudit | ? | user=null | ip=null | " +
-        "op=HEAD_BUCKET {bucket=bucket} | ret=SUCCESS";
+        "op=HEAD_BUCKET {} | ret=SUCCESS";

Review Comment:
   Thanks for the info.  I wonder if it's possible to set the context for unit test.  If the unit test has too weak assertions, and an important part of functionality needs to be verified manually, I think we have a problem.  We should prevent changes that might accidentally break it.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] symious commented on a diff in pull request #3325: HDDS-6620. Gatter AuditLog params for S3g

Posted by GitBox <gi...@apache.org>.
symious commented on code in PR #3325:
URL: https://github.com/apache/ozone/pull/3325#discussion_r854340970


##########
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/TestS3GatewayAuditLog.java:
##########
@@ -101,7 +101,7 @@ public static void tearDown() {
   public void testHeadBucket() throws Exception {
     bucketEndpoint.head(bucketName);
     String expected = "INFO  | S3GAudit | ? | user=null | ip=null | " +
-        "op=HEAD_BUCKET {bucket=bucket} | ret=SUCCESS";
+        "op=HEAD_BUCKET {} | ret=SUCCESS";

Review Comment:
   This is because the context is null in unit test. So the details can not be shown here.
   I added the manual test result in the description, the details are normal.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] kerneltime commented on a diff in pull request #3325: HDDS-6620. Gatter AuditLog params for S3g

Posted by GitBox <gi...@apache.org>.
kerneltime commented on code in PR #3325:
URL: https://github.com/apache/ozone/pull/3325#discussion_r854497029


##########
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/TestS3GatewayAuditLog.java:
##########
@@ -101,7 +101,7 @@ public static void tearDown() {
   public void testHeadBucket() throws Exception {
     bucketEndpoint.head(bucketName);
     String expected = "INFO  | S3GAudit | ? | user=null | ip=null | " +
-        "op=HEAD_BUCKET {bucket=bucket} | ret=SUCCESS";
+        "op=HEAD_BUCKET {} | ret=SUCCESS";

Review Comment:
   +1



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] symious commented on a diff in pull request #3325: HDDS-6620. Gatter AuditLog params for S3g

Posted by GitBox <gi...@apache.org>.
symious commented on code in PR #3325:
URL: https://github.com/apache/ozone/pull/3325#discussion_r854704552


##########
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/TestS3GatewayAuditLog.java:
##########
@@ -101,7 +101,7 @@ public static void tearDown() {
   public void testHeadBucket() throws Exception {
     bucketEndpoint.head(bucketName);
     String expected = "INFO  | S3GAudit | ? | user=null | ip=null | " +
-        "op=HEAD_BUCKET {bucket=bucket} | ret=SUCCESS";
+        "op=HEAD_BUCKET {} | ret=SUCCESS";

Review Comment:
   @adoroszlai @kerneltime Thanks for the review.
   Have brought back the detail message in audit log. Since set the context would need to add new dependency to use MultiValueMapImpl, so I chose to use override to include the detail params.
   Please have a check.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org