-
Notifications
You must be signed in to change notification settings - Fork 590
HDDS-7716. Add ACL errors to OM audit logs for read requests #4136
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
Conversation
adoroszlai
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @myskov for the patch. LGTM overall.
| if (isAclEnabled) { | ||
| captureLatencyNs(perfMetrics.getGetKeyInfoAclCheckLatencyNs(), () -> | ||
| checkAcls(ResourceType.KEY, StoreType.OZONE, ACLType.READ, | ||
| bucket.realVolume(), bucket.realBucket(), args.getKeyName()) | ||
| ); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a similar call in lookupKey which is not moved into the try-catch block.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice catch
| /** | ||
| * Utility class to read audit logs. | ||
| */ | ||
| public final class AuditLogUtils { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this is only for tests, I think AuditLogTestUtils would be better. Also, moving it to hdds-common (e.g. in hadoop-hdds/common/src/test/java/org/apache/hadoop/ozone/audit) would allow more reuse.
For that s3gateway dependencies would need a new entry like:
ozone/hadoop-ozone/ozone-manager/pom.xml
Lines 106 to 111 in b352ad0
| <dependency> | |
| <groupId>org.apache.ozone</groupId> | |
| <artifactId>hdds-common</artifactId> | |
| <type>test-jar</type> | |
| <scope>test</scope> | |
| </dependency> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will update the PR
* master: (176 commits) HDDS-7726. EC: Enhance datanode reconstruction log message (apache#4155) HDDS-7739. EC: Increase the information in the RM sending command log message (apache#4153) HDDS-7652. Volume Quota not enforced during write when bucket quota is not set (apache#4124) HDDS-7628. Intermittent failure in TestOzoneContainerWithTLS (apache#4142) HDDS-7695. EC metrics related to replication commands don't add up (apache#4152) HDDS-7729. EC: ECContainerReplicaCount should handle pending delete of unhealthy replicas (apache#4146) HDDS-7738. SCM terminates when adding container to a closed pipeline (apache#4154) HDDS-7243. Remove RequestFeatureValidator from echoRPC method which supports only ValidationCondition.OLDER_CLIENT_REQUESTS (apache#4051) HDDS-7708. No check for certificate duration config scenarios. (apache#4149) HDDS-7727. EC: SCM unregistered event handler for DatanodeCommandCountUpdated (apache#4147) HDDS-7606. Add SCM HA support in intellij run (apache#4058) HDDS-7666. EC: Unrecoverable EC containers with some remaining replicas may block decommissioning (apache#4118) HDDS-7339. Implement Certificate renewal task for services (apache#3982) HDDS-7696. MisReplicationHandler does not consider QUASI_CLOSED replicas as sources (apache#4144) HDDS-7714. Docker cluster ozone-om-ha fails during docker-compose up (apache#4137) HDDS-7716. Log read requests rejected with permission denied in OM audit (apache#4136) HDDS-7588. Intermittent failure in TestObjectStoreWithLegacyFS#testFlatKeyStructureWithOBS (apache#4040) HDDS-7633. Compile error with Java 11: package com.sun.jmx.mbeanserver is not visible (apache#4077) HDDS-7648. Add a servername tag in UGI metrics. (apache#4094) HDDS-7564. Update Ozone version after 1.3.0 release (apache#4115) ...
What changes were proposed in this pull request?
adding log entries to OM audit log in case a request is prohibited by ACL policies. Also, a way of testing audit logs is added along with integration tests (see AuditLogUtils.class).
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-7716
How was this patch tested?