RANGER-4314: Delete nonexistent user return error status code - #1
RANGER-4314: Delete nonexistent user return error status code#1coldestlin wants to merge 524 commits into
Conversation
Change-Id: I5921e1720d97ccb0d0fe7a12d88e32ab24a28938 Signed-off-by: mehul <mehul@gadgetzones-MacBook-Pro.local>
…l view Signed-off-by: Madhan Neethiraj <madhan@apache.org>
…he datashare Signed-off-by: Madhan Neethiraj <madhan@apache.org>
Signed-off-by: Madhan Neethiraj <madhan@apache.org>
Signed-off-by: Madhan Neethiraj <madhan@apache.org>
…ogin fails Signed-off-by: Kishor Gollapalliwar <kishor.gollapalliwar@gmail.com>
Signed-off-by: Dineshkumar Yadav <dineshkumar.yadav@outlook.com>
…ject Signed-off-by: Madhan Neethiraj <madhan@apache.org>
…ogin fails Signed-off-by: Kishor Gollapalliwar <kishor.gollapalliwar@gmail.com>
Signed-off-by: Dineshkumar Yadav <dineshkumar.yadav@outlook.com>
…ject Signed-off-by: Madhan Neethiraj <madhan@apache.org>
Signed-off-by: Madhan Neethiraj <madhan@apache.org>
…ant / Revoke request
Signed-off-by: Madhan Neethiraj <madhan@apache.org>
Signed-off-by: Madhan Neethiraj <madhan@apache.org>
…re improper Signed-off-by: Dhaval.Rajpara <dhavalrajpara1304@gmail.com>
…o, snakeyaml; excluded io.netty Signed-off-by: Madhan Neethiraj <madhan@apache.org>
…nAction in calls to UserGroupInformation.doAs()
… from Ranger KMS assembly Signed-off-by: Madhan Neethiraj <madhan@apache.org>
Change-Id: I8208d5b9c6d71ae69249b89640f4a8828b750531 Signed-off-by: Pradeep Agrawal <pradeep@apache.org>
Change-Id: Id6fd44454ba4e2b2b482dde97e092d749f9f7bc4
…nger Admin React JS
… automatic expiry
…n Ranger Admin React JS
… list of resource keys - apache#2
…on_rotate (apache#294) Co-authored-by: zhaoshuaihua <zhaoshuaihua@kingsoft.com>
…ld & space optimizations (apache#288) Co-authored-by: abhishek-kumar <abhishek.kumar@cloudera.com>
WalkthroughThe pull request introduces significant updates to the project’s CI/CD pipeline by enhancing the GitHub Actions workflow to support builds for multiple Java versions and integrating Docker image creation. Additionally, it modifies the Maven Changes
Sequence Diagram(s)sequenceDiagram
participant CI as CI Pipeline
participant JDK8 as JDK 8 Build Job
participant JDK11 as JDK 11 Build Job
participant Docker as Docker Build Job
CI->>JDK8: Start Build
JDK8->>CI: Upload Artifacts
CI->>JDK11: Start Build
JDK11->>CI: Upload Artifacts
CI->>Docker: Start Docker Build
Docker->>Docker: Prepare Docker Images
Docker->>CI: Complete Docker Build
Tip New featuresWalkthrough comment now includes:
Notes:
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 17
Outside diff range, codebase verification and nitpick comments (2)
agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerServiceResourceMatcher.java (1)
Line range hint
22-92: Review of changes inRangerServiceResourceMatcherThe modifications and additions in this class are significant and impact how resources are matched and evaluated. Here are some observations and suggestions:
Variable and Type Changes:
- The change from
RangerServiceDef.RangerResourceDeftoRangerResourceDef(line 42) simplifies the type usage and potentially aligns better with other parts of the application. Ensure that this change is reflected everywhere this type is used.Constructor Modification:
- The constructor now includes a boolean parameter in the call to
ServiceDefUtil.getLeafResourceDef(line 47). This change should be documented to explain the significance of the boolean parameter and its impact on resource definition retrieval.Method Signature Update:
- The
isAncestorOfmethod now acceptsRangerResourceDefinstead ofRangerServiceDef.RangerResourceDef(line 71). This change should be verified across all usages to ensure compatibility.New Method Addition:
- The
isLeafmethod (lines 91-92) is a useful addition for quickly determining if a resource is a leaf node. This method should be tested thoroughly to ensure it correctly identifies leaf nodes based on the resource name.Overall, these changes appear to be well-thought-out and aimed at improving the flexibility and maintainability of resource matching in the system. Ensure thorough testing and documentation are in place to support these changes.
agents-audit/src/main/java/org/apache/ranger/audit/queue/AuditAsyncQueue.java (1)
142-148: Approve the newlogStatus()method with a suggestion for enhancement.The implementation of
logStatus()is robust, providing detailed conditional logging based on the queue's status. Consider adding a comment explaining the conditions under which logging occurs for future maintainability.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (82)
- .github/workflows/maven.yml (2 hunks)
- agents-audit/pom.xml (4 hunks)
- agents-audit/src/main/java/org/apache/ranger/audit/destination/AmazonCloudWatchAuditDestination.java (2 hunks)
- agents-audit/src/main/java/org/apache/ranger/audit/destination/ElasticSearchAuditDestination.java (8 hunks)
- agents-audit/src/main/java/org/apache/ranger/audit/destination/FileAuditDestination.java (7 hunks)
- agents-audit/src/main/java/org/apache/ranger/audit/destination/HDFSAuditDestination.java (7 hunks)
- agents-audit/src/main/java/org/apache/ranger/audit/destination/SolrAuditDestination.java (2 hunks)
- agents-audit/src/main/java/org/apache/ranger/audit/model/AuditIndexRecord.java (1 hunks)
- agents-audit/src/main/java/org/apache/ranger/audit/model/AuthzAuditEvent.java (4 hunks)
- agents-audit/src/main/java/org/apache/ranger/audit/model/SPOOL_FILE_STATUS.java (1 hunks)
- agents-audit/src/main/java/org/apache/ranger/audit/provider/AsyncAuditProvider.java (2 hunks)
- agents-audit/src/main/java/org/apache/ranger/audit/provider/AuditProviderFactory.java (4 hunks)
- agents-audit/src/main/java/org/apache/ranger/audit/provider/AuditWriterFactory.java (1 hunks)
- agents-audit/src/main/java/org/apache/ranger/audit/provider/BaseAuditHandler.java (11 hunks)
- agents-audit/src/main/java/org/apache/ranger/audit/provider/LocalFileLogBuffer.java (4 hunks)
- agents-audit/src/main/java/org/apache/ranger/audit/provider/MiscUtil.java (6 hunks)
- agents-audit/src/main/java/org/apache/ranger/audit/provider/kafka/KafkaAuditProvider.java (4 hunks)
- agents-audit/src/main/java/org/apache/ranger/audit/provider/solr/SolrAuditProvider.java (1 hunks)
- agents-audit/src/main/java/org/apache/ranger/audit/queue/AuditAsyncQueue.java (3 hunks)
- agents-audit/src/main/java/org/apache/ranger/audit/queue/AuditBatchQueue.java (7 hunks)
- agents-audit/src/main/java/org/apache/ranger/audit/queue/AuditFileCacheProviderSpool.java (9 hunks)
- agents-audit/src/main/java/org/apache/ranger/audit/queue/AuditFileQueueSpool.java (24 hunks)
- agents-audit/src/main/java/org/apache/ranger/audit/queue/AuditFileSpool.java (32 hunks)
- agents-audit/src/main/java/org/apache/ranger/audit/test/TestEvents.java (1 hunks)
- agents-audit/src/main/java/org/apache/ranger/audit/utils/AbstractRangerAuditWriter.java (5 hunks)
- agents-audit/src/main/java/org/apache/ranger/audit/utils/ORCFileUtil.java (5 hunks)
- agents-audit/src/main/java/org/apache/ranger/audit/utils/RangerJSONAuditWriter.java (3 hunks)
- agents-audit/src/main/java/org/apache/ranger/audit/utils/RangerORCAuditWriter.java (6 hunks)
- agents-audit/src/test/java/org/apache/ranger/audit/utils/RangerJSONAuditWriterTest.java (1 hunks)
- agents-common/dev-support/spotbugsIncludeFile.xml (1 hunks)
- agents-common/pom.xml (5 hunks)
- agents-common/scripts/enable-agent.sh (2 hunks)
- agents-common/src/main/java/org/apache/ranger/admin/client/AbstractRangerAdminClient.java (1 hunks)
- agents-common/src/main/java/org/apache/ranger/admin/client/RangerAdminClient.java (2 hunks)
- agents-common/src/main/java/org/apache/ranger/admin/client/RangerAdminRESTClient.java (25 hunks)
- agents-common/src/main/java/org/apache/ranger/admin/client/datatype/GrantRevokeData.java (2 hunks)
- agents-common/src/main/java/org/apache/ranger/admin/client/datatype/RESTResponse.java (2 hunks)
- agents-common/src/main/java/org/apache/ranger/authorization/hadoop/config/RangerPluginConfig.java (4 hunks)
- agents-common/src/main/java/org/apache/ranger/authorization/hadoop/constants/RangerHadoopConstants.java (1 hunks)
- agents-common/src/main/java/org/apache/ranger/authorization/utils/JsonUtils.java (6 hunks)
- agents-common/src/main/java/org/apache/ranger/authorization/utils/StringUtil.java (3 hunks)
- agents-common/src/main/java/org/apache/ranger/plugin/audit/RangerDefaultAuditHandler.java (3 hunks)
- agents-common/src/main/java/org/apache/ranger/plugin/conditionevaluator/RangerAnyOfExpectedTagsPresentConditionEvaluator.java (2 hunks)
- agents-common/src/main/java/org/apache/ranger/plugin/conditionevaluator/RangerNoneOfExpectedTagsPresentConditionEvaluator.java (2 hunks)
- agents-common/src/main/java/org/apache/ranger/plugin/conditionevaluator/RangerScriptConditionEvaluator.java (2 hunks)
- agents-common/src/main/java/org/apache/ranger/plugin/conditionevaluator/RangerTagsAllPresentConditionEvaluator.java (2 hunks)
- agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerAbstractContextEnricher.java (4 hunks)
- agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerAdminGdsInfoRetriever.java (1 hunks)
- agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerAdminTagRetriever.java (3 hunks)
- agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerFileBasedTagRetriever.java (3 hunks)
- agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerGdsEnricher.java (1 hunks)
- agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerGdsInfoRetriever.java (1 hunks)
- agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerServiceResourceMatcher.java (4 hunks)
- agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerTagEnricher.java (22 hunks)
- agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerTagForEval.java (2 hunks)
- agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerTagRetriever.java (1 hunks)
- agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerUserStoreEnricher.java (1 hunks)
- agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerUserStoreRefresher.java (8 hunks)
- agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/externalretrievers/GetFromURL.java (8 hunks)
- agents-common/src/main/java/org/apache/ranger/plugin/errors/ValidationErrorCode.java (4 hunks)
- agents-common/src/main/java/org/apache/ranger/plugin/model/AuditFilter.java (1 hunks)
- agents-common/src/main/java/org/apache/ranger/plugin/model/GroupInfo.java (1 hunks)
- agents-common/src/main/java/org/apache/ranger/plugin/model/RangerBaseModelObject.java (3 hunks)
- agents-common/src/main/java/org/apache/ranger/plugin/model/RangerDatasetHeader.java (1 hunks)
- agents-common/src/main/java/org/apache/ranger/plugin/model/RangerGds.java (1 hunks)
- agents-common/src/main/java/org/apache/ranger/plugin/model/RangerMetrics.java (1 hunks)
- agents-common/src/main/java/org/apache/ranger/plugin/model/RangerPluginInfo.java (4 hunks)
- agents-common/src/main/java/org/apache/ranger/plugin/model/RangerPolicy.java (26 hunks)
- agents-common/src/main/java/org/apache/ranger/plugin/model/RangerPolicyDelta.java (2 hunks)
- agents-common/src/main/java/org/apache/ranger/plugin/model/RangerPolicyResourceSignature.java (4 hunks)
- agents-common/src/main/java/org/apache/ranger/plugin/model/RangerPrincipal.java (1 hunks)
- agents-common/src/main/java/org/apache/ranger/plugin/model/RangerRole.java (3 hunks)
- agents-common/src/main/java/org/apache/ranger/plugin/model/RangerSecurityZone.java (3 hunks)
- agents-common/src/main/java/org/apache/ranger/plugin/model/RangerSecurityZoneHeaderInfo.java (2 hunks)
- agents-common/src/main/java/org/apache/ranger/plugin/model/RangerSecurityZoneV2.java (1 hunks)
- agents-common/src/main/java/org/apache/ranger/plugin/model/RangerServerHealth.java (1 hunks)
- agents-common/src/main/java/org/apache/ranger/plugin/model/RangerService.java (1 hunks)
- agents-common/src/main/java/org/apache/ranger/plugin/model/RangerServiceDef.java (19 hunks)
- agents-common/src/main/java/org/apache/ranger/plugin/model/RangerServiceHeaderInfo.java (2 hunks)
- agents-common/src/main/java/org/apache/ranger/plugin/model/RangerServiceResource.java (1 hunks)
- agents-common/src/main/java/org/apache/ranger/plugin/model/RangerServiceResourceWithTags.java (1 hunks)
- agents-common/src/main/java/org/apache/ranger/plugin/model/RangerServiceTags.java (2 hunks)
Files not processed due to max files limit (54)
- agents-common/src/main/java/org/apache/ranger/plugin/model/RangerTag.java
- agents-common/src/main/java/org/apache/ranger/plugin/model/RangerTagDef.java
- agents-common/src/main/java/org/apache/ranger/plugin/model/RangerTagResourceMap.java
- agents-common/src/main/java/org/apache/ranger/plugin/model/RangerValidityRecurrence.java
- agents-common/src/main/java/org/apache/ranger/plugin/model/RangerValiditySchedule.java
- agents-common/src/main/java/org/apache/ranger/plugin/model/ServiceDeleteResponse.java
- agents-common/src/main/java/org/apache/ranger/plugin/model/UserInfo.java
- agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerPolicyValidator.java
- agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerSecurityZoneValidator.java
- agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerServiceDefHelper.java
- agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerServiceDefValidator.java
- agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerServiceValidator.java
- agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerValidator.java
- agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerZoneResourceMatcher.java
- agents-common/src/main/java/org/apache/ranger/plugin/model/validation/ValidationFailureDetails.java
- agents-common/src/main/java/org/apache/ranger/plugin/policyengine/PolicyEngine.java
- agents-common/src/main/java/org/apache/ranger/plugin/policyengine/PolicyEvaluatorForTag.java
- agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerAccessResult.java
- agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerPluginContext.java
- agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerPolicyEngineImpl.java
- agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerPolicyEngineOptions.java
- agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerPolicyRepository.java
- agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerRequestScriptEvaluator.java
- agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerResourceACLs.java
- agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerResourceTrie.java
- agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerSecurityZoneMatcher.java
- agents-common/src/main/java/org/apache/ranger/plugin/policyengine/gds/GdsAccessResult.java
- agents-common/src/main/java/org/apache/ranger/plugin/policyengine/gds/GdsDataShareEvaluator.java
- agents-common/src/main/java/org/apache/ranger/plugin/policyengine/gds/GdsDatasetEvaluator.java
- agents-common/src/main/java/org/apache/ranger/plugin/policyengine/gds/GdsDipEvaluator.java
- agents-common/src/main/java/org/apache/ranger/plugin/policyengine/gds/GdsDshidEvaluator.java
- agents-common/src/main/java/org/apache/ranger/plugin/policyengine/gds/GdsPolicyEngine.java
- agents-common/src/main/java/org/apache/ranger/plugin/policyengine/gds/GdsProjectEvaluator.java
- agents-common/src/main/java/org/apache/ranger/plugin/policyengine/gds/GdsSharedResourceEvaluator.java
- agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerAbstractPolicyEvaluator.java
- agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerAbstractPolicyItemEvaluator.java
- agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerAuditPolicyEvaluator.java
- agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerCustomConditionEvaluator.java
- agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerDefaultPolicyEvaluator.java
- agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerDefaultPolicyItemEvaluator.java
- agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerDefaultRowFilterPolicyItemEvaluator.java
- agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerOptimizedPolicyEvaluator.java
- agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerPolicyEvaluator.java
- agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerPolicyItemEvaluator.java
- agents-common/src/main/java/org/apache/ranger/plugin/policyresourcematcher/RangerDefaultPolicyResourceMatcher.java
- agents-common/src/main/java/org/apache/ranger/plugin/policyresourcematcher/RangerPolicyResourceMatcher.java
- agents-common/src/main/java/org/apache/ranger/plugin/policyresourcematcher/RangerResourceEvaluator.java
- agents-common/src/main/java/org/apache/ranger/plugin/resourcematcher/RangerAbstractResourceMatcher.java
- agents-common/src/main/java/org/apache/ranger/plugin/resourcematcher/RangerDefaultResourceMatcher.java
- agents-common/src/main/java/org/apache/ranger/plugin/resourcematcher/RangerPathResourceMatcher.java
- agents-common/src/main/java/org/apache/ranger/plugin/resourcematcher/RangerResourceMatcher.java
- agents-common/src/main/java/org/apache/ranger/plugin/resourcematcher/RangerURLResourceMatcher.java
- agents-common/src/main/java/org/apache/ranger/plugin/resourcematcher/ResourceMatcher.java
- agents-common/src/main/java/org/apache/ranger/plugin/service/RangerAuthContext.java
Files not reviewed due to server errors (9)
- agents-audit/src/main/java/org/apache/ranger/audit/provider/kafka/KafkaAuditProvider.java
- agents-audit/src/main/java/org/apache/ranger/audit/destination/HDFSAuditDestination.java
- agents-audit/src/test/java/org/apache/ranger/audit/utils/RangerJSONAuditWriterTest.java
- agents-common/src/main/java/org/apache/ranger/authorization/utils/JsonUtils.java
- agents-common/src/main/java/org/apache/ranger/plugin/model/RangerSecurityZone.java
- agents-common/src/main/java/org/apache/ranger/authorization/utils/StringUtil.java
- agents-audit/src/main/java/org/apache/ranger/audit/utils/AbstractRangerAuditWriter.java
- agents-audit/src/main/java/org/apache/ranger/audit/provider/BaseAuditHandler.java
- agents-common/src/main/java/org/apache/ranger/plugin/errors/ValidationErrorCode.java
Files skipped from review due to trivial changes (11)
- agents-audit/src/main/java/org/apache/ranger/audit/destination/AmazonCloudWatchAuditDestination.java
- agents-audit/src/main/java/org/apache/ranger/audit/model/SPOOL_FILE_STATUS.java
- agents-audit/src/main/java/org/apache/ranger/audit/provider/AsyncAuditProvider.java
- agents-audit/src/main/java/org/apache/ranger/audit/provider/AuditProviderFactory.java
- agents-common/dev-support/spotbugsIncludeFile.xml
- agents-common/scripts/enable-agent.sh
- agents-common/src/main/java/org/apache/ranger/admin/client/datatype/GrantRevokeData.java
- agents-common/src/main/java/org/apache/ranger/admin/client/datatype/RESTResponse.java
- agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerAdminTagRetriever.java
- agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerTagRetriever.java
- agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerUserStoreEnricher.java
Additional context used
actionlint
.github/workflows/maven.yml
134-134: shellcheck reported issue in this script: SC2086:info:5:52: Double quote to prevent globbing and word splitting
(shellcheck)
134-134: shellcheck reported issue in this script: SC2046:warning:15:17: Quote this to prevent word splitting
(shellcheck)
134-134: shellcheck reported issue in this script: SC2046:warning:15:46: Quote this to prevent word splitting
(shellcheck)
134-134: shellcheck reported issue in this script: SC2046:warning:17:17: Quote this to prevent word splitting
(shellcheck)
134-134: shellcheck reported issue in this script: SC2046:warning:17:46: Quote this to prevent word splitting
(shellcheck)
Additional comments not posted (174)
agents-common/src/main/java/org/apache/ranger/plugin/model/RangerMetrics.java (1)
24-30: Approve JSON serialization enhancements.The changes to the JSON serialization annotations are appropriate and align with best practices for JSON handling in Java. The removal of XML annotations simplifies the class and focuses it on JSON usage.
Ensure that these changes do not affect other parts of the system that might expect XML outputs. This can be verified by checking the system's integration points or by running end-to-end tests.
agents-common/src/main/java/org/apache/ranger/plugin/model/RangerSecurityZoneHeaderInfo.java (1)
20-25: Approve JSON serialization updates.The updates to JSON serialization annotations are well-executed, enhancing the control over serialized output. The removal of XML annotations is consistent with the shift towards JSON serialization.
agents-common/src/main/java/org/apache/ranger/plugin/model/RangerServiceResourceWithTags.java (1)
24-30: Approve JSON serialization setup for the new class.The JSON serialization annotations are correctly applied, ensuring that only non-empty fields are included and unknown properties are ignored. This setup is consistent with the application's standards.
agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerGdsInfoRetriever.java (1)
29-57: Well-structured abstract class design.The
RangerGdsInfoRetrieverclass is well-designed with clear abstract methods that ensure subclasses will properly implement necessary functionality. The use of protected properties is appropriate for inheritance. This class structure adheres to Java best practices for abstract classes, promoting good software design principles.agents-common/src/main/java/org/apache/ranger/plugin/model/GroupInfo.java (1)
23-31: Updated serialization annotations enhance compatibility and performance.The transition from older Jackson annotations to
@JsonInclude(JsonInclude.Include.NON_EMPTY)and the removal of XML serialization annotations are positive changes. These updates not only reduce the serialization payload but also align with modern best practices for JSON processing. The shift towards JSON-only serialization suggests a strategic architectural decision that could simplify data handling and improve performance across the application.agents-common/src/main/java/org/apache/ranger/plugin/model/RangerPrincipal.java (1)
36-97: Well-structured class for user roles management.The
RangerPrincipalclass is well-structured and includes all necessary components for effective serialization and management of principal entities (users, groups, roles). The use of JSON and XML annotations ensures compatibility with different serialization frameworks, which is crucial for a distributed system like Apache Ranger.The implementation of
Serializablewith a static version UID is correctly done, ensuring that changes to the class do not inadvertently affect serialization compatibility.The
equalsandhashCodemethods are properly overridden to ensure thatRangerPrincipalobjects can be used effectively in collections likeHashSetandHashMap.Overall, the class is well-implemented and adheres to Java best practices.
agents-common/src/main/java/org/apache/ranger/plugin/model/RangerServiceHeaderInfo.java (1)
Line range hint
32-100: Enhanced class for service header information with improved serialization.The
RangerServiceHeaderInfoclass has been significantly enhanced with new fields and constructors to support additional service characteristics. The transition from JAXB to Jackson annotations is well-executed, ensuring better integration with JSON-based systems.The new fields
displayName,type, andisGdsServiceare accompanied by appropriate getter and setter methods, which are crucial for encapsulation and data manipulation. The constructors are well-designed to initialize these fields correctly.The use of
@JsonAutoDetectand@JsonIncludeannotations ensures that only relevant fields are included in the serialized output, which is important for performance and bandwidth considerations in distributed systems.Overall, the modifications to the class are thoughtful and align well with the objectives of enhancing functionality and adaptability.
agents-common/src/main/java/org/apache/ranger/plugin/model/AuditFilter.java (1)
27-33: Streamlined JSON serialization for audit filtering.The
AuditFilterclass has been updated to use Jackson annotations exclusively, reflecting a clear shift towards JSON-based serialization. The removal of JAXB annotations and the adoption of@JsonIncludeto ensure only non-empty fields are serialized are appropriate changes that align with modern JSON processing practices.This update simplifies the class's integration with JSON-based logging and monitoring systems, which is crucial for effective audit management in Apache Ranger.
The class structure allows for comprehensive filtering based on various criteria, which is essential for detailed audit analysis and compliance reporting.
agents-audit/src/main/java/org/apache/ranger/audit/model/AuditIndexRecord.java (23)
37-39: Getter method foridis correctly implemented.This method is simple and follows Java standards for getter methods.
41-43: Setter method foridis correctly implemented.This method is simple and follows Java standards for setter methods.
45-47: Getter method forfilePathis correctly implemented.This method is simple and follows Java standards for getter methods.
49-51: Setter method forfilePathis correctly implemented.This method is simple and follows Java standards for setter methods.
53-55: Getter method forlinePositionis correctly implemented.This method is simple and follows Java standards for getter methods.
57-59: Setter method forlinePositionis correctly implemented.This method is simple and follows Java standards for setter methods.
61-63: Getter method forstatusis correctly implemented.This method is simple and follows Java standards for getter methods.
65-67: Setter method forstatusis correctly implemented.This method is simple and follows Java standards for setter methods.
69-71: Getter method forfileCreateTimeis correctly implemented.This method is simple and follows Java standards for getter methods.
73-75: Setter method forfileCreateTimeis correctly implemented.This method is simple and follows Java standards for setter methods.
77-79: Getter method forwriteCompleteTimeis correctly implemented.This method is simple and follows Java standards for getter methods.
81-83: Setter method forwriteCompleteTimeis correctly implemented.This method is simple and follows Java standards for setter methods.
85-87: Getter method fordoneCompleteTimeis correctly implemented.This method is simple and follows Java standards for getter methods.
89-91: Setter method fordoneCompleteTimeis correctly implemented.This method is simple and follows Java standards for setter methods.
93-95: Getter method forlastSuccessTimeis correctly implemented.This method is simple and follows Java standards for getter methods.
97-99: Setter method forlastSuccessTimeis correctly implemented.This method is simple and follows Java standards for setter methods.
101-103: Getter method forlastFailedTimeis correctly implemented.This method is simple and follows Java standards for getter methods.
105-107: Setter method forlastFailedTimeis correctly implemented.This method is simple and follows Java standards for setter methods.
109-111: Getter method forfailedAttemptCountis correctly implemented.This method is simple and follows Java standards for getter methods.
113-115: Setter method forfailedAttemptCountis correctly implemented.This method is simple and follows Java standards for setter methods.
117-119: Getter method forlastAttemptis correctly implemented.This method is simple and follows Java standards for getter methods.
121-123: Setter method forlastAttemptis correctly implemented.This method is simple and follows Java standards for setter methods.
125-136:toStringmethod is well-implemented.The
toStringmethod provides a comprehensive string representation of the object, which is useful for debugging and logging.agents-common/src/main/java/org/apache/ranger/plugin/model/RangerServerHealth.java (2)
33-37: Well-structured annotations for serialization.The use of
@JsonAutoDetect,@JsonIgnoreProperties,@XmlRootElement, and@XmlAccessorTypeannotations are appropriate for ensuring that the class is correctly serialized and deserialized across different mediums (JSON, XML). This setup helps in maintaining the visibility and ignoring unknown properties which is crucial for API version compatibility.
38-126: Proper use of the Builder pattern and immutability.The class
RangerServerHealthuses the Builder pattern effectively to maintain immutability. The use of final fields and a private constructor ensures that instances ofRangerServerHealthare immutable once constructed. This design is beneficial for thread-safety and consistency, especially in a multi-threaded environment like server health monitoring.Additionally, the methods
equals,hashCode, andtoStringare properly overridden to ensure correct behavior in collections and debugging scenarios.agents-common/src/main/java/org/apache/ranger/plugin/conditionevaluator/RangerScriptConditionEvaluator.java (1)
108-110: Refined instantiation and method call simplification.The changes to the
RangerRequestScriptEvaluatorconstructor to includescriptEngineandenableJsonCtxas parameters are a positive improvement. This encapsulation allows the evaluator to be more aware of its context, potentially improving the performance and reliability of script evaluations.The simplification of the
evaluateConditionScriptmethod call by removing previously passed parameters and only requiring thescriptparameter reduces complexity and improves readability.Run the following script to verify the integration with the rest of the system:
Verification successful
Verification Successful: Integration of New Constructor Parameters
The integration of the new constructor parameters for
RangerRequestScriptEvaluatoris handled correctly in the codebase. The constructor is used with the new parameters in both the main and test code, ensuring proper functionality and testing.
- Instances found in
RangerRequestExprResolver.javaandRangerScriptConditionEvaluator.javademonstrate proper usage.- Test cases in
RangerRequestScriptEvaluatorTest.javaverify the behavior.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the integration of the new constructor parameters with the rest of the system. # Test: Search for the usage of `RangerRequestScriptEvaluator` in the codebase. Expect: Proper handling of the new parameters. rg --type java -A 5 $'new RangerRequestScriptEvaluator'Length of output: 9831
agents-audit/src/main/java/org/apache/ranger/audit/provider/AuditWriterFactory.java (1)
64-73: Updated property key and improved logging condition.The update to the property key for retrieving the
auditFileTypeto include a more specific path (propPrefix + ".batch.filequeue.filetype") is a thoughtful change that likely aligns with a new configuration schema. This should help in better organizing the properties and avoiding conflicts.Additionally, moving the debug logging statement inside the conditional check for
auditWriteris a good practice. It ensures that the logging only occurs when necessary, which can improve performance by avoiding unnecessary string concatenations when debug logging is turned off.Run the following script to verify the usage of the new property key across the system:
agents-audit/src/main/java/org/apache/ranger/audit/queue/AuditAsyncQueue.java (2)
59-65: Verify thread safety and approve monitoring enhancements.The modifications in the
log()method enhance monitoring by tracking total and failed log attempts. However, ensure that the operations related to checking the queue size and adding to the queue are thread-safe, given the concurrent nature of the application.
151-151: Approve the addition of thesize()method.The new
size()method provides a simple and effective way to retrieve the current size of the queue, enhancing the class's interface..github/workflows/maven.yml (2)
34-50: Well-configured JDK 8 build job.The job setup for JDK 8 is well-configured with appropriate steps for checking out the code, setting up Java, and building with Maven. The use of parallel threads is a good practice to speed up the build process.
52-68: Well-configured JDK 11 build job.The job setup for JDK 11 is consistent with JDK 8, with the added step of excluding the
knox-agentmodule during the Maven build. Ensure that the exclusion is well-documented and justified in the project documentation or build scripts.agents-common/src/main/java/org/apache/ranger/plugin/model/RangerService.java (1)
27-34: Improved JSON serialization annotations.The updates to the JSON serialization annotations are well-implemented, focusing on efficiency and reducing overhead. Ensure that these changes do not negatively impact existing clients that consume the serialized data.
agents-common/src/main/java/org/apache/ranger/plugin/model/RangerServiceTags.java (1)
Line range hint
24-173: Updated JSON serialization annotations and method signature.The updates to the JSON serialization annotations and the addition of a boolean parameter in the method signature are well-implemented. Ensure that the changes are well-documented and that the impact on existing method calls is verified.
agents-audit/src/main/java/org/apache/ranger/audit/test/TestEvents.java (1)
52-52: Approve the property settings in the else block.The properties set in the else block are appropriate for the audit functionality and are correctly implemented.
agents-audit/src/main/java/org/apache/ranger/audit/destination/FileAuditDestination.java (5)
78-78: Approve the enhanced logging statement for directory configuration.The use of parameterized messages in the logging statement at line 78 improves performance and readability, and provides clear information about the missing configuration.
85-85: Approve the enhanced logging statement for directory creation issues.The use of parameterized messages in the logging statement at line 85 is appropriate and provides specific information about the directory that could not be created.
89-89: Approve the enhanced informational logging for directory and file name format.The use of parameterized messages in the logging statements at lines 89 and 95 is appropriate and provides specific information about the log folder and file name format.
Also applies to: 95-95
174-174: Approve the enhanced error logging for stopping the logging process.The use of parameterized messages in the logging statement at line 174 is appropriate and provides specific information about the exception and the context in which it occurred.
206-206: Approve the enhanced error logging for file renaming issues.The use of parameterized messages in the logging statement at line 206 is appropriate and provides specific information about the files involved in the renaming process.
agents-audit/src/main/java/org/apache/ranger/audit/utils/RangerORCAuditWriter.java (4)
57-57: Approve the updated method signature forinit.The addition of the
auditProviderNameparameter in theinitmethod at line 57 allows for more contextual initialization and enhances the functionality of the method. The method call within theinitmethod correctly passes the new parameter to the superclass method.
121-121: Approve the reordered access modifier forstop.The reordering of the access modifier for the
stopmethod at line 121 fromsynchronized publictopublic synchronizedis stylistic and does not impact the functionality of the method. This change improves readability and consistency in method declaration order.
139-139: Approve the reordered access modifier forgetORCFileWrite.The reordering of the access modifier for the
getORCFileWritemethod at line 139 fromsynchronized protectedtoprotected synchronizedis stylistic and does not impact the functionality of the method. This change improves readability and consistency in method declaration order.
177-177: Approve the enhanced implementation ofinit.The updates to the
initmethod at line 177, including additional property settings and the initialization of theORCFileUtilinstance, enhance the functionality of the method by allowing for more detailed configuration. The method correctly handles exceptions and logs errors, which is appropriate for error handling.agents-common/src/main/java/org/apache/ranger/plugin/model/RangerRole.java (4)
23-25: Approved: Updated JSON serialization annotations.The changes to the JSON serialization annotations are correctly implemented, reflecting a shift to more modern and maintained libraries. This should enhance compatibility and maintainability of the JSON processing in
RangerRole.Also applies to: 34-34
191-246: Approved: Addition ofhashCodeandequalsmethods.The implementation of
hashCodeandequalsmethods is correctly done, using fields such asname,description,options,users,groups, androles. This ensures thatRangerRoleinstances are compared based on their content rather than reference identity, which is crucial for their correct behavior in collections.
135-135: Approved: Consistent JSON annotations in nested class.The
RoleMemberclass correctly applies JSON annotations (@JsonIncludeand@JsonIgnoreProperties), ensuring consistent serialization behavior with the outerRangerRoleclass.
135-135: Approved: Addition ofhashCodeandequalsmethods in nested class.The
RoleMemberclass includes well-implementedhashCodeandequalsmethods, ensuring objects are compared based on their content. This is essential for collections that might contain these objects.agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerFileBasedTagRetriever.java (4)
40-42: Approved: Addition of properties for managing multiple tag files.The introduction of
tagFilesCount,currentTagFileIndex, andisInitialproperties is well thought out, enhancing the flexibility and control over the tag file management process inRangerFileBasedTagRetriever.
157-220: Approved: Consolidation of tag file reading logic inreadFromFile.The
readFromFilemethod is effectively implemented, consolidating the tag file reading logic and handling initial reads as well as subsequent reads based ontagFilesCount. The method includes comprehensive error handling and logging, which enhances maintainability and debuggability.
190-213: Approved: Handling of multiple tag files inreadFromFile.The logic to handle multiple tag files by cycling through them based on
tagFilesCountis correctly implemented in thereadFromFilemethod. This ensures that all configured tag files are used effectively, enhancing the flexibility of the tag retrieval process.
183-213: Approved: Comprehensive error handling and logging inreadFromFile.The error handling and logging within the
readFromFilemethod are well-implemented, ensuring that any issues encountered during the file reading process are appropriately logged. This is crucial for troubleshooting and maintaining the reliability of the tag retrieval process.agents-common/src/main/java/org/apache/ranger/plugin/model/RangerBaseModelObject.java (5)
22-38: Approved: Updated imports and JSON serialization annotations.The changes to the imports and JSON serialization annotations are correctly implemented, reflecting the addition of new utility methods for collections and a shift to more modern JSON processing practices. This should enhance compatibility and maintainability of the JSON processing in
RangerBaseModelObject.
Line range hint
43-248: Approved: Addition ofNULL_SAFE_SUPPLIER_V2and related methods.The introduction of
NULL_SAFE_SUPPLIER_V2and methods to set and get the null-safe supplier (setNullSafeSupplier) provides significant flexibility and configurability in handling collections safely. This is a robust enhancement that allows for optimized memory usage and safer operations with collections.
166-212: Approved: Utility methods for handling collections safely.The utility methods
nullSafeList,nullSafeSet, andnullSafeMapare well-implemented, utilizing theNullSafeSupplierto ensure that operations on collections are safe and do not risk null pointer exceptions. This enhancement significantly improves the robustness of collection handling inRangerBaseModelObject.
178-212: Approved: Methods for getting updatable collections.The methods
getUpdatableList,getUpdatableSet, andgetUpdatableMapare correctly implemented to ensure that modifications to collections are isolated and do not affect the original collections. This is crucial for maintaining data integrity when collections are modified.
250-299: Approved:NullSafeSupplierclasses for optimized collection handling.The
NullSafeSupplierV1andNullSafeSupplierV2classes are well-implemented, providing configurable behavior for handling null or empty collections.NullSafeSupplierV1creates new instances, which is suitable for scenarios where modifications are expected, whileNullSafeSupplierV2optimizes memory usage by returning empty collections, suitable for read-only scenarios.agents-common/pom.xml (4)
50-55: Approved: Exclusion ofjavax.ws.rs:jsr311-api.This exclusion helps maintain a clean classpath and avoid potential conflicts from multiple versions of the same dependency.
105-112: Approved: Exclusion oforg.apache.commons:commons-compressandorg.apache.zookeeper:zookeeper.These exclusions are crucial for avoiding version conflicts and reducing the artifact size by preventing unnecessary transitive dependencies.
200-222: Approved: Addition of testing dependencies.The addition of
org.junit.jupiter,org.junit.vintage,org.mockito, andorg.slf4jtesting dependencies is crucial for enhancing the project's testing framework. Ensure comprehensive tests are implemented to leverage these tools effectively.
126-144: Approved: Addition of Jackson libraries.The addition of
com.fasterxml.jacksonlibraries modernizes the JSON processing capabilities of the project. Ensure that these libraries are integrated properly across the project.Run the following script to verify the integration of Jackson libraries:
Also applies to: 184-187
Verification successful
Verified: Jackson libraries are well-integrated across the project.
The Jackson libraries are extensively used throughout the codebase, indicating proper integration for JSON processing and RESTful services. The imports and annotations related to Jackson are present in multiple files across different modules, confirming their effective utilization.
- Usage of Jackson libraries is confirmed in various Java files, including
RangerUgSyncRESTClient.java,VXAccessAudit.java, and many others.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the integration of Jackson libraries across the project. # Test: Search for usage of Jackson libraries. Expect: Proper usage and configuration. rg --type java -A 5 $'com.fasterxml.jackson'Length of output: 174235
agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/externalretrievers/GetFromURL.java (3)
98-102: Approved: Jackson usage ingetBearerToken.The update to use Jackson's JSON handling methods in the
getBearerTokenmethod enhances the robustness and efficiency of the application's authentication mechanisms. Verify that this change integrates well with the rest of the authentication flow.Run the following script to verify the integration of the
getBearerTokenmethod:Verification successful
Integration of
getBearerTokenverified.The
getBearerTokenmethod is used internally within theGetFromURLclass to set up HTTP requests with authorization headers. Its integration is limited to this class, and there are no issues with its usage. No further action is needed.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the integration of the `getBearerToken` method. # Test: Search for usage of `getBearerToken`. Expect: Proper integration and usage. rg --type java -A 5 $'getBearerToken'Length of output: 4182
Line range hint
195-208: Approved: Enhanced token verification inverifyToken.The update to use Jackson in the
verifyTokenmethod improves the security and robustness of token verification. Ensure that this method is thoroughly tested, especially for its security implications.Run the following script to verify the security implications of the
verifyTokenmethod:
81-81: Approved: Use ofJsonUtils.jsonToObjectingetFromURL.The change to use
JsonUtils.jsonToObjectfor JSON parsing in thegetFromURLmethod is a positive step towards modernizing the application's JSON handling capabilities. Ensure that this change is thoroughly tested.Run the following script to verify the functionality of the
getFromURLmethod:agents-audit/src/main/java/org/apache/ranger/audit/utils/RangerJSONAuditWriter.java (1)
Line range hint
94-132: Approved: Enhanced logging and error handling inlogJSON.The updates to use parameterized logging and improve error handling in the
logJSONmethod are significant improvements. These changes should enhance both the performance and reliability of logging. Verify that these changes are effective and do not introduce any new issues.Run the following script to verify the effectiveness of the changes in the
logJSONmethod:Verification successful
Verified: The
logJSONmethod changes are effectively covered by tests.The updates to the
logJSONmethod, including enhanced logging and error handling, are well-covered by tests inRangerJSONAuditWriterTest.java. These tests ensure that the changes are effective and do not introduce new issues.
- The method is tested for various scenarios, including error handling and logging behavior.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the effectiveness of the changes in the `logJSON` method. # Test: Search for logging tests covering the `logJSON` method. Expect: Comprehensive test coverage. rg --type java -A 5 $'logJSON'Length of output: 26773
agents-audit/src/main/java/org/apache/ranger/audit/queue/AuditBatchQueue.java (1)
Line range hint
240-342: Approve logging format changes; verify new queue handling logic.The changes to use parameterized logging messages are approved as they enhance performance and security. However, the new logic for handling queue states and spooling conditions appears complex and could benefit from additional inline documentation or simplification to improve maintainability.
Please ensure that the new queue handling logic is thoroughly tested, especially under scenarios where the queue is full or nearly full, to verify that it behaves as expected without causing performance bottlenecks or data loss.
agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerGdsEnricher.java (3)
111-130: Confirm resource cleanup inpreCleanupmethod.The
preCleanupmethod attempts to clean up the GDS info refresher thread. Ensure that all resources, such as timers and queues, are released properly to avoid memory leaks. Consider adding more robust checks or logs to confirm that all resources are fully cleaned up.
132-154: Approve the implementation ofenrichmethod.The
enrichmethod effectively handles the enrichment of Ranger access requests with GDS information. The use of delegation and logging within the method is appropriate and aids in maintainability and traceability.
180-352: Improve error handling and resource management inRangerGdsInfoRefresher.The
RangerGdsInfoRefresherinner class uses threading and timers to refresh GDS information periodically. Consider enhancing error handling to cover more failure scenarios and ensure that all resources, such as timers and threads, are managed properly to avoid leaks. Additionally, verify that the thread safety of shared resources is maintained throughout the operation.agents-audit/src/main/java/org/apache/ranger/audit/provider/LocalFileLogBuffer.java (4)
35-35: Approved: Updated import for better exception handling.The change from
PrivilegedActiontoPrivilegedExceptionActionallows for handling exceptions within privileged code blocks, which is a good practice for robustness.
385-385: Approved: Improved thread synchronization withnotifyAll().Changing from
notify()tonotifyAll()ensures that all waiting threads are notified, which can prevent potential deadlocks or missed signals in multi-threaded environments.
418-426: Approved: Use of lambda expression forPrivilegedExceptionAction.The update to use a lambda expression with
PrivilegedExceptionActionin therunmethod enhances readability and allows for exception handling within the privileged context.
247-251: Verify streamlined logic and updated logging inopenFile.The changes in the
openFilemethod are intended to enhance readability and maintain functionality. Please ensure that the streamlined logic and updated logging are correct and consistent with the application's requirements.Verification successful
Streamlined logic and updated logging in
openFileare correct and consistent.The
openFilemethod inLocalFileLogBuffer.javahas been reviewed, and the streamlined logic and updated logging are consistent with the application's requirements. The method's usage across the codebase supports its role in file handling operations, and the logging provides clear and informative messages. No issues were found.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of the streamlined logic and updated logging in the `openFile` method. # Test: Search for the `openFile` method usage. Expect: Consistent usage and correct implementation. rg --type java -A 5 $'openFile'Length of output: 13807
agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerUserStoreRefresher.java (5)
26-29: Approved: Transition from Gson to custom JSON utilities.The replacement of Gson with
JsonUtilsandJsonUtilsV2centralizes JSON operations, enhancing maintainability and potentially improving error handling.Also applies to: 44-44
262-262: Approved: Updated JSON deserialization inloadFromCache.The use of
JsonUtils.jsonToObjectfor deserialization aligns with the new JSON handling strategy, replacing Gson with a more centralized approach.
300-300: Approved: Updated JSON serialization insaveToCache.The use of
JsonUtils.objectToWriterfor serialization aligns with the new JSON handling strategy, replacing Gson with a more centralized approach.
381-391: Approved: Use ofPrivilegedExceptionActioningetUserStoreIfUpdated.Replacing
PrivilegedActionwithPrivilegedExceptionActionallows for better exception management by enabling the method to throw checked exceptions, enhancing error handling capabilities.
414-414: Approved: Updated JSON parsing ingetUserStoreIfUpdated.The use of
JsonUtilsV2.readResponsefor parsing the response aligns with the new JSON handling strategy, replacing Gson with a more centralized approach.agents-common/src/main/java/org/apache/ranger/plugin/model/RangerSecurityZoneV2.java (1)
1-506: Approved: New fileRangerSecurityZoneV2.java.The new file introduces
RangerSecurityZoneV2, which includes enhancements and additional features over the previous version. The class is well-structured and follows Java best practices with proper use of annotations and encapsulation of fields.agents-audit/src/main/java/org/apache/ranger/audit/queue/AuditFileSpool.java (3)
41-42: Approved: Updated imports forSPOOL_FILE_STATUS.Moving the
SPOOL_FILE_STATUSenum to a separate package and updating the import statement inAuditFileSpool.javacentralizes the status management and allows for better reuse across different classes.
70-70: Approved: Use of diamond operator in collections.The use of the diamond operator (
<>) simplifies the code by removing explicit type arguments in generic class instantiation, which enhances readability and maintainability.Also applies to: 88-88
118-118: Approved: Refactoring and enhancements inAuditFileSpool.The refactoring includes the use of getter and setter methods for
AuditIndexRecord, parameterized logging, and more specific exception handling. These changes improve code clarity, maintainability, and performance.Also applies to: 145-145, 150-150, 157-157, 161-161, 167-167, 177-177, 181-181, 198-198, 202-202, 214-214, 218-218, 223-223, 226-226, 229-229, 231-231, 238-238, 239-239, 241-241, 262-262, 266-266, 277-277, 280-280, 299-299, 314-314, 321-321, 338-338, 357-357, 360-360, 371-371, 378-378, 385-385, 388-388, 396-396, 403-403, 417-417, 425-425, 451-451, 458-458, 459-459, 460-460, 461-461, 462-462, 471-471, 473-473, 479-479, 489-489, 491-491, 493-493, 502-502, 503-503, 505-505, 518-518, 519-519, 520-520, 521-521, 522-522, 523-523, 524-524, 525-525, 526-526, 536-536, 546-546, 547-547, 556-556, 561-561, 562-562, 563-563, 564-564, 565-565, 570-570, 571-571, 572-572, 583-583, 586-586, 587-587, 588-588, 591-591, 598-598, 602-602, 606-606, 607-607, 608-608, 609-609, 613-613, 616-616, 620-620, 624-624, 625-625, 626-626, 631-631, 638-638, 641-641, 662-662, 672-672, 673-673, 696-696, 698-698, 703-703, 704-704, 707-707, 723-723, 731-731, 733-733, 734-734, 735-735, 740-740, 743-743, 744-744, 745-745, 758-758, 762-762, 772-772, 775-775, 776-776, 777-777, 778-778, 783-783, 786-786, 787-787
agents-audit/src/main/java/org/apache/ranger/audit/provider/MiscUtil.java (4)
84-94: Thread-safe ObjectMapper initialization.The implementation of
ThreadLocal<ObjectMapper>ensures that each thread has its own instance ofObjectMapper, which is configured with specific settings such as date format and handling of unknown properties. This approach is thread-safe and avoids potential issues with shared mutable state in multi-threaded environments.
96-98: Proper retrieval of ObjectMapper instance.The
getMapper()method correctly retrieves the thread-localObjectMapperinstance. This method ensures that the JSON handling is thread-safe across different parts of the application that useMiscUtil.
323-329: Refactor JSON serialization to use Jackson.The
stringifymethod has been refactored to use Jackson'sObjectMapperfor JSON serialization. This change is part of the transition from Gson to Jackson, which can offer better performance and memory management. The error handling within the method logs the exception and falls back to the defaulttoString()method, which is a good practice to ensure that the application can gracefully handle serialization errors.
339-343: Refactor JSON deserialization to use Jackson.The
fromJsonmethod has been updated to use Jackson'sreadValuemethod for JSON deserialization. This change aligns with the overall transition to Jackson in the application. The method includes error handling that logs issues during the deserialization process, which enhances the robustness of the method.agents-common/src/main/java/org/apache/ranger/plugin/model/RangerGds.java (6)
44-80: Well-implemented base model object.
RangerGdsBaseModelObjectis well-implemented with appropriate getters, setters, and a customtoStringmethod. The use of JSON and XML annotations ensures that the object can be easily serialized and deserialized in different contexts within the application.
84-128: Detailed implementation of RangerDataset.
RangerDatasetis correctly implemented as an extension ofRangerGdsBaseModelObject. It manages additional dataset-specific properties with appropriate getters and setters. The overriddentoStringmethod provides a comprehensive representation of the dataset's state, which is useful for debugging and logging.
131-175: Consistent implementation for RangerProject.
RangerProjectmirrors the structure and functionality ofRangerDataset, adapted for project-specific data. This consistency in design ensures that similar types of objects in the application are handled in a uniform way, which is good for maintainability and scalability.
178-254: Comprehensive implementation of RangerDataShare.
RangerDataShareis implemented to handle complex data sharing settings within the Ranger application. It includes detailed management of access types and tag masks, which are essential for controlling access to shared data. The class structure is robust, ensuring that data sharing settings are handled securely and efficiently.
257-335: Effective management of shared resources in RangerSharedResource.
RangerSharedResourceeffectively manages shared resources, including sub-resources and associated policies. This functionality is crucial for the Ranger application's data governance capabilities, ensuring that resources are shared according to specified policies and conditions.
338-396: Well-defined linkage between data shares and datasets in RangerDataShareInDataset.
RangerDataShareInDatasetprovides a well-defined structure for linking data shares with datasets, including handling the status and validity of these shares. This functionality is essential for the effective management of data shares within the Ranger application, ensuring that data sharing agreements are adhered to and properly tracked.agents-audit/src/main/java/org/apache/ranger/audit/queue/AuditFileCacheProviderSpool.java (1)
594-600: Improved JSON handling and error management.The transition from Gson to
MiscUtilfor JSON parsing inAuditFileCacheProviderSpoolaligns with the broader application changes. The addition of error handling around JSON parsing operations enhances the robustness of the code by ensuring that parsing errors are logged and do not disrupt the overall functionality.agents-audit/src/main/java/org/apache/ranger/audit/queue/AuditFileQueueSpool.java (11)
23-25: Approved import changes.The new imports for
SPOOL_FILE_STATUSandAuditIndexRecordare necessary due to the refactoring and enhanced encapsulation in the codebase.
251-261: Refactoring and error handling ininitmethod approved.The use of getter methods for
AuditIndexRecordproperties enhances encapsulation and maintainability. The added error handling for JSON parsing improves the robustness of the method.Also applies to: 269-270
595-601: Improved error handling in JSON parsing.The addition of detailed error logging in the
loadIndexFilemethod enhances the robustness and maintainability of the code by providing clearer diagnostics in case of parsing failures.
654-657: Detailed logging inappendToDoneFilemethod approved.The enhanced logging provides valuable information for debugging and auditing, which improves the traceability of file processing activities.
627-628: Enhanced logging inremoveIndexRecordmethod approved.The detailed logging when removing a file from the index enhances the transparency and auditability of the index management process.
645-648: Correct implementation ofsaveIndexFilemethod approved.The method effectively serializes
AuditIndexRecordobjects to JSON and writes them to the index file, ensuring the persistence of index state.
654-657: Lifecycle management inappendToDoneFilemethod approved.The method effectively manages the lifecycle of processed files by moving them to the archive directory. The detailed logging enhances traceability and auditability.
Also applies to: 671-671
Line range hint
705-728: Disk space management inremoveOldFilesmethod approved.The method effectively manages disk space by removing old files from the archive directory based on a maximum file count, which prevents indefinite growth of the archive directory.
Line range hint
1086-1086: Throttled error logging inlogErrormethod approved.The method's implementation of throttled logging helps manage the volume of log data and prevents performance issues, which is crucial for maintaining system stability.
798-798: Main processing loop inrunmethod approved.The method correctly implements the main processing loop, handling various operational states such as destination down, draining, and normal operations. The structured handling of these states ensures robust and reliable file processing.
Also applies to: 807-807, 812-812, 820-820, 824-824, 836-838
863-863: Event logging inlogEventmethod approved.The method effectively handles the logging of audit events, ensuring that events are processed and logged correctly. The implementation includes appropriate error handling, which enhances the reliability of the audit logging system.
Also applies to: 904-907
agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerTagEnricher.java (18)
24-24: Addition of Predicate import is appropriate.The import of
Predicateis necessary for the newSelfOrAncestorPredicateclass which implements this interface.
41-41: Addition of RangerPluginContext import.The import of
RangerPluginContextis necessary due to the new parameter in thecreateRangerServiceResourceMatchermethod which now requires aRangerPluginContextobject.
98-99: Initialization of tagDownloadTimer.The initialization of
tagDownloadTimerwithin the class constructor ensures that the timer is ready for use immediately after an object ofRangerTagEnricheris created. This is a good practice as it encapsulates the setup logic within the class itself.
102-103: Proper use of synchronization primitives.The use of
RangerReadWriteLockand the instantiation ofCachedResourceEvaluatorsare appropriate here. These changes support the enhancements in caching and thread-safety as described in the summary.
116-116: Configuration handling in init method.The handling of configuration options such as polling intervals and string deduplication settings in the
initmethod is correctly implemented. It's important to ensure that these settings are loaded and applied correctly at initialization.
321-325: Handling of tag deduplication in setServiceTags.The logic to handle tag deduplication based on the
isTagsDedupedflag is a significant improvement. It helps in maintaining the integrity and uniqueness of tags, which is crucial for the correct functioning of the tag management system.
332-332: Refactored method call to include deduplication flag.The modification to include the
isTagsDedupedflag in theapplyDeltamethod call is crucial for ensuring that tag deduplication logic is considered during the delta application process. This change aligns with the enhancements mentioned in the summary.
363-364: Clearing cache post tag setting.The invocation of
clearCacheafter setting new service tags ensures that stale data is not used, which is crucial for the accuracy and performance of the tag enrichment process.
451-451: Dynamic matcher creation with context.The update to
createRangerServiceResourceMatcherto includeRangerPluginContextas a parameter allows for more contextual information during matcher creation, enhancing the flexibility and capability of resource matching.
469-469: Resource trie initialization based on service definitions.The initialization of
RangerResourceTrieobjects within a loop that iterates over service definitions ensures that each resource definition has a corresponding trie for efficient tag lookup. This is a good practice for optimizing resource matching performance.
498-502: Enhanced resource matcher creation logic.The changes to the resource matcher creation process, including the addition of context and handling of different resource definitions, enhance the system's ability to accurately match resources based on more complex criteria.
Line range hint
561-601: Improved handling of old service resource removal.The method
removeOldServiceResourcehas been enhanced to handle the removal of old service resources more effectively by checking match types and updating the trie structure accordingly. This is crucial for maintaining the accuracy of resource matching.
627-627: Updated method signature and context handling in matcher.The update to the method signature of
createRangerServiceResourceMatcherto includeRangerPluginContextand the subsequent handling of this context within theRangerDefaultPolicyResourceMatchersetup are critical for enabling more detailed and context-aware resource matching.Also applies to: 658-658
770-797: Implementation of caching mechanism for resource evaluators.The introduction of the
CachedResourceEvaluatorsclass and its methods for managing a cache of resource evaluators is a significant enhancement. This caching mechanism will improve performance by reducing redundant computations and ensuring quick access to previously computed data.
Line range hint
804-837: Refined evaluator retrieval logic with caching.The enhancements to the
getEvaluatorsmethod, including the use of caching and the introduction of a predicate to filter results, significantly improve the efficiency and accuracy of evaluator retrieval. This is a crucial improvement for systems with a large number of resources.
852-883: Improved descendant match exclusion logic.The
excludeDescendantMatchesmethod has been refined to better handle the exclusion of descendant matches based on the resource's depth in the hierarchy. This improvement is important for ensuring that resource matches are accurate and relevant to the specific access request.
1241-1258: Addition of SelfOrAncestorPredicate class.The new
SelfOrAncestorPredicateclass implements thePredicateinterface to determine if a resource evaluator is a leaf or an ancestor of a specified resource definition. This addition enhances the flexibility and accuracy of resource matching.
27-27: Switch to JsonUtils for JSON operations.The change from using Gson to JsonUtils for JSON operations is noted. This should be verified to ensure that it does not introduce any regressions or compatibility issues with existing JSON data handling.
Verification successful
Verification Successful: JsonUtils is a Suitable Replacement for Gson
The
JsonUtilsclass provides comprehensive support for JSON operations using Jackson, covering typical serialization and deserialization needs. Its extensive use across the project further supports its reliability. The switch from Gson toJsonUtilsinRangerTagEnricher.javais unlikely to introduce regressions or compatibility issues.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that JsonUtils handles JSON operations correctly in all usages within the project. # Test: Search for JsonUtils usage and ensure it handles all data correctly. rg --type java "JsonUtils"Length of output: 55569
Script:
#!/bin/bash # Extract methods from the JsonUtils class to understand its capabilities. ast-grep --lang java --pattern $'class JsonUtils { $$$ }' agents-common/src/main/java/org/apache/ranger/authorization/utils/JsonUtils.javaLength of output: 141
Script:
#!/bin/bash # Search for method definitions within the JsonUtils class to understand its capabilities. rg --type java "public .*JsonUtils" -A 10 agents-common/src/main/java/org/apache/ranger/authorization/utils/JsonUtils.javaLength of output: 1227
Script:
#!/bin/bash # Search for method definitions within the JsonUtils class to understand its capabilities. rg --type java "public .*\\(" -A 5 agents-common/src/main/java/org/apache/ranger/authorization/utils/JsonUtils.javaLength of output: 5840
agents-common/src/main/java/org/apache/ranger/plugin/model/RangerServiceDef.java (7)
34-34: Refactor: Use@JsonIncludefor better JSON serialization control.The change from
@JsonSerializeto@JsonIncludeis a good practice as it provides more control over how null or empty values are handled during serialization, aligning with modern JSON handling practices.
45-45: Addition of new constant for feature toggle.Adding
OPTION_ENABLE_TAG_BASED_POLICIESas a constant is a clean way to manage feature toggles. This allows for easier modifications and readability in the code where this feature's enablement is checked.
103-103: Ensure proper initialization and copying of new property.The initialization of
markerAccessTypestonullin the constructor and its handling in theupdateFrommethod are correctly implemented. This ensures that the property is correctly set up during object creation and updated from another instance.Also applies to: 137-137
422-440: Proper handling of getter and setter for the new list property.The implementation of getter and setter methods for
markerAccessTypesfollows best practices for managing list properties in Java, including null checks and immutability handling. This is crucial for avoiding common pitfalls like external modifications to mutable objects.
1952-1952: Addition of new enum for categorizing access types.The introduction of the
AccessTypeCategoryenum is a significant enhancement for the clarity and maintainability of access type definitions. This categorization aids in better understanding and managing the access control logic.
1959-1959: EnhancedRangerAccessTypeDefwith category support.Adding a category to the access type definitions allows for more nuanced control and easier management of access permissions. The implementation of the category property with appropriate getter and setter methods is well-done.
Also applies to: 2073-2080
63-63: Introduction of new property for access type definitions.The addition of
markerAccessTypesas a new property in theRangerServiceDefclass allows for extended functionality in managing access control. Ensure that this property is properly integrated and used where necessary.agents-common/src/main/java/org/apache/ranger/plugin/model/RangerPolicyDelta.java (2)
31-31: Modernization of JSON serialization annotations.The replacement of
@JsonSerializewith@JsonInclude(JsonInclude.Include.NON_EMPTY)is a positive change, aligning with modern JSON handling practices by excluding null and empty fields from serialization. This change helps in reducing the payload size and improving the clarity of the serialized data.
44-46: Addition of new constant and update tochangeTypeNamesarray.The introduction of
CHANGE_TYPE_GDS_UPDATEas a new constant and its inclusion in thechangeTypeNamesarray is a good practice for maintaining consistency and clarity in the representation of change types. This addition ensures that the new change type is properly represented and can be utilized effectively in the system.agents-common/src/main/java/org/apache/ranger/plugin/model/RangerServiceResource.java (1)
30-30: Streamlining of serialization behavior inRangerServiceResource.The adoption of
@JsonInclude(JsonInclude.Include.NON_EMPTY)and the removal of XML-related annotations reflect a strategic shift towards using JSON as the sole serialization format. This change simplifies the serialization process and aligns with modern best practices, enhancing the maintainability and performance of the serialization code.agents-common/src/main/java/org/apache/ranger/authorization/hadoop/constants/RangerHadoopConstants.java (1)
28-29: New constants added for legacy subaccess authorization.The addition of
RANGER_USE_LEGACY_SUBACCESS_AUTHORIZATION_PROPandRANGER_USE_LEGACY_SUBACCESS_AUTHORIZATION_DEFAULTallows for configurable legacy subaccess authorization. This is a straightforward and beneficial enhancement to the configuration options.agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerTagForEval.java (1)
Line range hint
29-44: Updated imports and annotations for modern JSON handling.The replacement of
@JsonSerializewith@JsonIncludeand the removal of JAXB annotations streamline and modernize the JSON serialization process of theRangerTagForEvalclass. These changes enhance compatibility and performance with newer JSON processing libraries.agents-audit/src/main/java/org/apache/ranger/audit/provider/solr/SolrAuditProvider.java (1)
293-294: EnhancedtoSolrDocmethod to include new fields.The addition of "datasets" and "projects" fields to the
toSolrDocmethod enriches the audit information captured in Solr documents, providing more detailed context for audit events. This is a valuable enhancement for auditing purposes.agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerAbstractContextEnricher.java (1)
299-303: Refactoring and Improved Error Handling inreadPropertiesMethodThe refactoring of the
readPropertiesmethod to use try-with-resources is correctly implemented, ensuring that resources are properly managed and automatically closed, which helps prevent resource leaks.Additionally, updating the exception handling to catch
IOExceptioninstead ofMalformedURLExceptionis a positive change, as it covers a broader spectrum of I/O errors, enhancing the robustness of the method.Also applies to: 323-323
agents-common/src/main/java/org/apache/ranger/plugin/audit/RangerDefaultAuditHandler.java (2)
271-275: New Methods for Retrieving Datasets and ProjectsThe addition of
getDatasetsandgetProjectsmethods is a valuable enhancement to theRangerDefaultAuditHandler. These methods efficiently extract datasets and projects from theGdsAccessResultobject, which is part of the access request context, thereby enriching the audit data with more detailed information about the access request.Also applies to: 277-281
136-143: Enhanced Audit Event Data ingetAuthzEventsMethodThe updates to the
getAuthzEventsmethod, which now includes calls togetDatasetsandgetProjects, effectively enrich the audit event data by adding datasets and projects associated with the access request. This enhancement improves the granularity and usefulness of the audit logs.agents-common/src/main/java/org/apache/ranger/plugin/model/RangerPolicyResourceSignature.java (2)
62-74: New Constructors and Static Method inRangerPolicyResourceSignatureThe addition of new constructors and the static method
fromsignificantly enhance the flexibility of theRangerPolicyResourceSignatureclass. These constructors allow the class to be instantiated with different types of resources, and the static method provides an alternative way to create instances from a map of resources. This flexibility is beneficial in various contexts where different resource representations are needed.
223-241: Enhanced Resource Representation intoSignatureStringMethodThe
toSignatureStringmethod forRangerSharedResourceis a valuable addition to theRangerPolicyResourceSignatureclass. It generates a comprehensive signature string that includes sub-resource details and condition expressions, providing a detailed representation of resources. This functionality is particularly useful for logging, debugging, and other purposes where a detailed string representation of resources is beneficial.agents-common/src/main/java/org/apache/ranger/authorization/hadoop/config/RangerPluginConfig.java (4)
51-55: Approval for new configuration fieldsThe addition of the new boolean fields
useRangerGroups,useOnlyRangerGroups,convertEmailToUsername,enableImplicitUserStoreEnricher, andenableImplicitGdsInfoEnricheris well-implemented. Making these fields private and final is a good practice for ensuring immutability and thread safety.Also applies to: 149-153
124-128: Approval for field initialization logicThe initialization of the new boolean fields in the primary constructor is correctly implemented. The use of configuration properties to set these fields allows for dynamic configuration based on external settings, which enhances the flexibility of the
RangerPluginConfigclass.
149-153: Approval for field copying in copy constructorThe direct copying of boolean fields from another
RangerPluginConfiginstance in the copy constructor is correctly implemented. This ensures that the new instance maintains the same configuration state as the original, which is crucial for the consistency of the application's behavior.
188-206: Approval for new getter methodsThe addition of public getter methods for the new boolean fields (
isUseRangerGroups,isUseOnlyRangerGroups,isConvertEmailToUsername,isEnableImplicitUserStoreEnricher,isEnableImplicitGdsInfoEnricher) is correctly implemented. These methods provide necessary external access to the configuration settings while maintaining encapsulation.agents-audit/pom.xml (4)
82-89: Approved: Exclusions forcommons-compressandzookeeper.The exclusions are added to avoid potential conflicts or unnecessary inclusions, which is a good practice to maintain a clean classpath. Ensure that the removal of these dependencies does not affect other parts of the project that might rely on them.
318-318: Approved: Version set forlucene-spatial.Setting the version to
8.4.0can help ensure compatibility or introduce new features. Verify that this version is compatible with other components of the project.
395-417: Approved: Addition of testing dependencies.The addition of
junit-jupiter-api,junit-vintage-engine,testng, andmockito-coreenhances the testing framework, which is crucial for maintaining high-quality code. Verify the integration of these testing frameworks with existing tests to ensure they are properly utilized.
367-388: Approved: Addition oforg.apache.orcandio.airliftdependencies.The addition of these dependencies with exclusions to prevent all transitive dependencies from being included is a good practice to minimize potential conflicts and maintain a clean classpath. Verify the necessity and impact of these dependencies on the project.
Verification successful
Verified: The necessity and impact of
org.apache.orcandio.airliftdependencies.The dependencies
org.apache.orcandio.airliftare actively used in the codebase, as evidenced by their presence in Java files for audit writing, configuration, and system access control. They are also included in XML files for assembly and plugin configurations, indicating their integral role in the project's functionality. No issues or negative impacts were found related to these dependencies.
Java files using
org.apache.orcandio.airlift:
security-admin/src/test/java/org/apache/ranger/audit/TestAuditQueue.javaranger-presto-plugin-shim/src/main/java/org/apache/ranger/authorization/presto/authorizer/RangerConfig.javaagents-audit/src/main/java/org/apache/ranger/audit/utils/RangerORCAuditWriter.javaagents-audit/src/main/java/org/apache/ranger/audit/utils/ORCFileUtil.javaXML files including these dependencies:
ranger-presto-plugin-shim/pom.xmlplugin-trino/pom.xml- Various assembly XML files in the
distro/src/main/assemblydirectoryScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the necessity and impact of `org.apache.orc` and `io.airlift` on the project. # Test: Search for any issues or discussions related to these dependencies in project documentation or issues. Expect: No negative impacts reported. rg --type md -A 5 $'org.apache.orc|io.airlift'Length of output: 47
Script:
#!/bin/bash # Description: Broaden the search for mentions of `org.apache.orc` and `io.airlift` across various file types. # Search for mentions in Java files rg --type java -A 5 'org.apache.orc|io.airlift' # Search for mentions in XML files rg --type xml -A 5 'org.apache.orc|io.airlift' # Search for mentions in other documentation formats (e.g., txt, adoc) rg --type txt -A 5 'org.apache.orc|io.airlift' rg --type adoc -A 5 'org.apache.orc|io.airlift'Length of output: 23359
agents-common/src/main/java/org/apache/ranger/plugin/conditionevaluator/RangerAnyOfExpectedTagsPresentConditionEvaluator.java (1)
22-24: Approved import changes.The new imports are necessary for the updated logic in the
isMatchedmethod and are correctly placed.agents-common/src/main/java/org/apache/ranger/plugin/conditionevaluator/RangerNoneOfExpectedTagsPresentConditionEvaluator.java (1)
22-24: Approved import changes.The new imports are necessary for the updated logic in the
isMatchedmethod and are correctly placed.agents-common/src/main/java/org/apache/ranger/plugin/conditionevaluator/RangerTagsAllPresentConditionEvaluator.java (1)
23-25: Approved import changes.The new imports are necessary for the updated logic in the
isMatchedmethod and are correctly placed.agents-audit/src/main/java/org/apache/ranger/audit/model/AuthzAuditEvent.java (2)
26-30: Update to Jackson annotations is appropriate.The switch from
@SerializedNameto@JsonPropertyis correctly implemented across all fields. This change aligns with the shift to using Jackson for JSON serialization, which is generally more performant and feature-rich compared to Gson.
590-592: EnsuretoStringmethod includes new fields.The
toStringmethod has been updated to include the new fieldsdatasetsandprojects. This ensures that the string representation of the object is comprehensive and up-to-date.agents-audit/src/main/java/org/apache/ranger/audit/destination/ElasticSearchAuditDestination.java (2)
79-79: Improved thread safety withAtomicReference.The replacement of the volatile
clientvariable with anAtomicReferenceis a significant improvement. This change enhances thread safety by ensuring that updates to the client instance are atomic and visible across threads.
343-344: Correct inclusion of new fields intoDocmethod.The
toDocmethod has been updated to include the new fieldsdatasetsandprojects. This ensures that these fields are correctly serialized into the document sent to Elasticsearch, enhancing the detail and usefulness of the audit logs.agents-audit/src/main/java/org/apache/ranger/audit/utils/ORCFileUtil.java (2)
22-22: Correct update to Jackson annotations.The import of
JsonPropertyreplacesSerializedName, aligning with the shift to using Jackson for JSON serialization. This change is correctly implemented and should ensure that fields are appropriately annotated for serialization.
278-278: Simplified casting incastLongObject.The simplification of the casting logic in the
castLongObjectmethod enhances readability and maintainability without altering the underlying functionality. This change is a positive improvement.agents-audit/src/main/java/org/apache/ranger/audit/destination/SolrAuditDestination.java (2)
166-182: Refactoring of Solr connection logic is well-implemented.The use of try-with-resources for managing
Krb5HttpClientBuilderensures that resources are properly closed, preventing potential memory leaks. This change enhances the robustness and maintainability of the connection logic.Also applies to: 188-209
298-299: Enhancements totoSolrDocmethod enhance data logging capabilities.The addition of
datasetsandprojectsfields to theSolrInputDocumentallows for more detailed audit logging and analysis. This change effectively enriches the data model used in auditing events.agents-common/src/main/java/org/apache/ranger/plugin/model/RangerPluginInfo.java (2)
24-28: Transition to newer Jackson library enhances JSON processing capabilities.The use of
com.fasterxml.jacksonannotations ensures better compatibility and performance with modern JSON processing frameworks. This update is well-implemented and improves the maintainability of the code.
67-70: Addition of GDS-related constants and methods enhances class functionality.The new constants and getter/setter methods for GDS-related data allow for effective tracking and management of this data type. The implementation is consistent with existing patterns in the class, enhancing its functionality while maintaining code consistency.
Also applies to: 402-444
agents-common/src/main/java/org/apache/ranger/admin/client/RangerAdminRESTClient.java (1)
1007-1078: Introduction ofgetGdsInfoIfUpdatedmethod and improvements in session cookie handling are well-implemented.The new method for retrieving GDS information is consistent with existing patterns, enhancing the functionality of the REST client. The streamlined session cookie handling improves code clarity and maintainability.
agents-common/src/main/java/org/apache/ranger/plugin/model/RangerPolicy.java (5)
23-24: Refactor: Import and Annotation EnhancementsThe addition of imports for
Collection,MapUtils, and updates to JSON annotations (JsonInclude,JsonIgnoreProperties) are appropriate and align with modern Java practices. These changes support the refactoring and enhancements described in the summary, particularly in handling collections more efficiently and ensuring JSON serialization only includes non-empty values.Also applies to: 30-30, 32-35, 39-39
282-283: Refactor: Simplified Collection HandlingThe use of
nullSafeListandnullSafeMaputility methods across various setters (setPolicyLabels,setResources,setPolicyItems, etc.) is a significant improvement. It reduces the need for explicit null checks and ensures that the collections are always properly initialized, which enhances the robustness and maintainability of the code.Also applies to: 302-303, 342-343, 361-362, 381-382, 401-402, 415-416, 429-430, 441-441, 447-447
285-288: Enhancement: Addition of Collection Management MethodsThe addition of methods like
addPolicyLabel,addPolicyItem,addDenyPolicyItem, etc., which ensure collections are mutable before adding items, is a robust enhancement. These methods utilizegetUpdatableListto prevent potentialNullPointerExceptionsand make the class more flexible and easier to use programmatically.Also applies to: 345-348, 364-367, 384-387, 404-407, 418-421, 432-435, 450-453, 473-476
757-758: Refactor: Enhanced Collection Manipulation in Nested ClassesThe changes in nested classes like
RangerPolicyResourceandRangerPolicyItemto usenullSafeListandgetUpdatableListfor managing collections of values, users, groups, roles, and conditions are well-implemented. These changes follow the same pattern of reducing boilerplate and ensuring safety in collection operations, which is consistent with the changes in the outer class.Also applies to: 760-775, 782-786, 931-940, 953-966, 982-994, 1011-1024, 1040-1052
715-715: Refactor: JSON Annotation Updates in Nested ClassesThe update of JSON serialization annotations (
JsonInclude,JsonIgnoreProperties) in all nested classes to ensure consistency and modern practices across the entire file is a good practice. This ensures that serialization behavior is uniform and only non-empty values are included, which can be crucial for APIs and data interchange where these models are used.Also applies to: 889-889, 1218-1218, 1303-1303, 1388-1388, 1494-1494, 1611-1611, 1718-1718
| public StringBuilder toString(StringBuilder sb) { | ||
| sb.append("id={").append(getId()).append("} "); | ||
| sb.append("name={").append(name).append("} "); | ||
| sb.append("isEnabled={").append(getIsEnabled()).append("} "); | ||
| return sb; | ||
| } |
There was a problem hiding this comment.
Enhance the new toString method.
The addition of the toString(StringBuilder sb) method is a good enhancement. However, consider adding null checks for name and other properties to avoid potential NullPointerExceptions.
Consider adding null checks:
sb.append("name={").append(name != null ? name : "null").append("} ");| @Override | ||
| public StringBuilder toString(StringBuilder sb) { | ||
| sb.append("RangerServiceResourceWithTags={ "); | ||
|
|
||
| super.toString(sb); | ||
|
|
||
| sb.append("associatedTags=["); | ||
| if (associatedTags != null) { | ||
| String prefix = ""; | ||
|
|
||
| for (RangerTag associatedTag : associatedTags) { | ||
| sb.append(prefix); | ||
|
|
||
| associatedTag.toString(sb); | ||
|
|
||
| prefix = ", "; | ||
| } | ||
| } | ||
| sb.append("] "); | ||
|
|
||
| sb.append(" }"); | ||
|
|
||
| return sb; |
There was a problem hiding this comment.
Refine the toString method in the new class.
The toString method is well-implemented, providing a detailed representation of the object. However, consider enhancing the readability and efficiency of the loop handling associated tags.
Refactor the loop to improve readability and efficiency:
for (RangerTag associatedTag : associatedTags) {
sb.append(prefix).append(associatedTag.toString(sb));
prefix = ", ";
}|
|
||
| RangerUserStore getUserStoreIfUpdated(long lastKnownUserStoreVersion, long lastActivationTimeInMillis) throws Exception; | ||
|
|
||
| ServiceGdsInfo getGdsInfoIfUpdated(long lastKnownVersion, long lastActivationTimeInMillis) throws Exception; |
There was a problem hiding this comment.
New method enhances interface functionality but consider refining exception handling.
The addition of getGdsInfoIfUpdated to the RangerAdminClient interface is a valuable update that supports more dynamic data management. However, it would be beneficial to specify more granular exceptions instead of a generic Exception. This change would aid implementers in handling potential errors more effectively.
| public class RangerAdminGdsInfoRetriever extends RangerGdsInfoRetriever { | ||
| private static final Logger LOG = LoggerFactory.getLogger(RangerAdminGdsInfoRetriever.class); | ||
|
|
||
| private static final String OPTION_DEDUP_TAGS = "deDupTags"; | ||
| private static final Boolean OPTION_DEDUP_TAGS_DEFAULT = true; | ||
|
|
||
|
|
||
| private RangerAdminClient adminClient; | ||
| private boolean deDupTags; | ||
|
|
||
| @Override | ||
| public void init(Map<String, String> options) { | ||
| try { | ||
| if (StringUtils.isNotBlank(serviceName) && serviceDef != null && StringUtils.isNotBlank(appId)) { | ||
| RangerPluginConfig pluginConfig = super.pluginConfig; | ||
|
|
||
| if (pluginConfig == null) { | ||
| pluginConfig = new RangerPluginConfig(serviceDef.getName(), serviceName, appId, null, null, null); | ||
| } | ||
|
|
||
| String deDupTagsVal = options != null ? options.get(OPTION_DEDUP_TAGS) : null; | ||
| RangerPluginContext pluginContext = getPluginContext(); | ||
| RangerAdminClient rangerAdmin = pluginContext.getAdminClient(); | ||
|
|
||
| this.deDupTags = StringUtils.isNotBlank(deDupTagsVal) ? Boolean.parseBoolean(deDupTagsVal) : OPTION_DEDUP_TAGS_DEFAULT; | ||
| this.adminClient = (rangerAdmin != null) ? rangerAdmin : pluginContext.createAdminClient(pluginConfig); | ||
| } else { | ||
| LOG.error("FATAL: Cannot find service/serviceDef to use for retrieving tags. Will NOT be able to retrieve GdsInfo."); | ||
| } | ||
| } catch (Exception excp) { | ||
| LOG.error("FATAL: Failed to initialize GDS retriever. Will not be able to enforce GDS policies", excp); | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| public ServiceGdsInfo retrieveGdsInfo(long lastKnownVersion, long lastActivationTimeInMillis) throws InterruptedException { | ||
| ServiceGdsInfo ret = null; | ||
|
|
||
| if (adminClient != null) { | ||
| try { | ||
| ret = adminClient.getGdsInfoIfUpdated(lastKnownVersion, lastActivationTimeInMillis); | ||
| } catch (ClosedByInterruptException excp) { | ||
| LOG.error("gdsInfo retriever thread was interrupted while blocked on I/O", excp); | ||
|
|
||
| throw new InterruptedException(); | ||
| } catch (Exception e) { | ||
| LOG.error("gdsInfo retriever encountered exception. Returning null gdsInfo", e); | ||
| } | ||
| } | ||
|
|
||
| if (ret != null && deDupTags) { | ||
| ret.dedupStrings(); | ||
| } | ||
|
|
||
| return ret; | ||
| } | ||
| } |
There was a problem hiding this comment.
Review of RangerAdminGdsInfoRetriever class
The class is well-structured for its intended functionality. Here are some observations and suggestions:
-
Initialization and Error Handling:
- The
initmethod (lines 43-65) properly checks for necessary conditions before proceeding with initialization. However, the error message in line 60 could be more descriptive by including the missing values. - Exception handling in the
initmethod logs the error but does not rethrow it, which might be appropriate depending on the caller's expectation to handle initialization failures.
- The
-
GDS Information Retrieval:
- The
retrieveGdsInfomethod (lines 67-88) handles potential interruptions and other exceptions well. The use of specific exception types likeClosedByInterruptExceptionis a good practice. - Consider adding more specific logging or metrics if the retrieval operations are critical or expected to fail under known conditions.
- The
-
Code Quality and Maintainability:
- The use of constants for options like
OPTION_DEDUP_TAGS(line 36) enhances readability and maintainability. - The method
retrieveGdsInfocould benefit from a brief comment explaining the conditions under whichdedupStringsis called (line 83).
- The use of constants for options like
Overall, the class is implemented with good practices in mind, but consider enhancing error messages and adding comments for complex logic to improve maintainability.
| public class RangerDatasetHeader { | ||
| @JsonAutoDetect(fieldVisibility = JsonAutoDetect.Visibility.ANY) | ||
| @JsonInclude(JsonInclude.Include.NON_EMPTY) | ||
| @JsonIgnoreProperties(ignoreUnknown = true) | ||
| @XmlRootElement | ||
| @XmlAccessorType(XmlAccessType.FIELD) | ||
| public static class RangerDatasetHeaderInfo extends RangerBaseModelObject implements java.io.Serializable { | ||
| private static final long serialVersionUID = 1L; | ||
|
|
||
| private String name; | ||
| private Map<RangerGds.GdsShareStatus, Long> dataSharesCountByStatus; | ||
| private Map<RangerPrincipal.PrincipalType, Long> principalsCountByType; | ||
| private Long projectsCount; | ||
| private String permissionForCaller; | ||
| private Long resourceCount; | ||
|
|
||
| public RangerDatasetHeaderInfo() { | ||
| super(); | ||
| } | ||
|
|
||
| public String getName() { | ||
| return name; | ||
| } | ||
|
|
||
| public void setName(String name) { | ||
| this.name = name; | ||
| } | ||
|
|
||
| public Map<RangerGds.GdsShareStatus, Long> getDataSharesCountByStatus() { | ||
| return dataSharesCountByStatus; | ||
| } | ||
|
|
||
| public void setDataSharesCountByStatus(Map<RangerGds.GdsShareStatus, Long> dataSharesCountByStatus) { | ||
| this.dataSharesCountByStatus = dataSharesCountByStatus; | ||
| } | ||
|
|
||
| public Map<RangerPrincipal.PrincipalType, Long> getPrincipalsCountByType() { | ||
| return principalsCountByType; | ||
| } | ||
|
|
||
| public void setPrincipalsCountByType(Map<RangerPrincipal.PrincipalType, Long> principalsCountByType) { | ||
| this.principalsCountByType = principalsCountByType; | ||
| } | ||
|
|
||
| public Long getProjectsCount() { | ||
| return projectsCount; | ||
| } | ||
|
|
||
| public void setProjectsCount(Long projectsCount) { | ||
| this.projectsCount = projectsCount; | ||
| } | ||
|
|
||
| public String getPermissionForCaller() { | ||
| return permissionForCaller; | ||
| } | ||
|
|
||
| public void setPermissionForCaller(String permissionForCaller) { | ||
| this.permissionForCaller = permissionForCaller; | ||
| } | ||
|
|
||
| public Long getResourceCount() { | ||
| return resourceCount; | ||
| } | ||
|
|
||
| public void setResourceCount(Long resourceCount) { | ||
| this.resourceCount = resourceCount; | ||
| } | ||
| } |
There was a problem hiding this comment.
Review of RangerDatasetHeader and its nested class
The class structure is appropriate for a data model in a Java-based application. Here are some observations and suggestions:
-
Serialization and Annotations:
- The use of
JsonAutoDetect,JsonInclude, andJsonIgnorePropertiesannotations (lines 32-34) is appropriate for JSON serialization. This ensures that only relevant fields are included and unknown properties do not cause issues. - XML annotations like
XmlRootElementandXmlAccessorType(lines 35-36) are correctly applied to the nested class, ensuring it can be serialized as part of XML-based configurations or APIs.
- The use of
-
Data Encapsulation and Access Methods:
- The properties are well encapsulated with private visibility and public getters and setters (lines 51-97). This follows Java bean conventions and allows for controlled access to the properties.
- Consider adding null checks or validations in the setters to prevent invalid data assignments, especially for collections and strings.
-
Maintainability and Readability:
- The class is straightforward and easy to understand, which is good for maintainability.
- Adding a few comments describing the purpose of each property, especially less obvious ones like
permissionForCaller(line 84), could enhance readability and maintainability.
Overall, the class is well-implemented with attention to serialization needs and Java standards. Minor enhancements could be made to improve data validation and documentation.
| @Override | ||
| public ServiceGdsInfo getGdsInfoIfUpdated(long lastKnownVersion, long lastActivationTimeInMillis) throws Exception { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
Placeholder implementation and exception handling in getGdsInfoIfUpdated.
The method getGdsInfoIfUpdated currently returns null and throws a generic Exception. Consider providing a more specific exception type that better describes the potential errors that might occur during the execution of this method. Additionally, if this method is intended to be overridden, it would be beneficial to include a comment or documentation indicating that subclasses are expected to provide a concrete implementation.
| boolean matched = false; | ||
| Set<RangerTagForEval> resourceTags = RangerAccessRequestUtil.getRequestTagsFromContext(request.getContext()); | ||
|
|
||
| if (resourceTags != null) { | ||
| // check if resource Tags does contain any of the policy Condition Tags | ||
| matched = (!Collections.disjoint(resourceTags, policyConditionTags)); | ||
| for (RangerTagForEval tag : resourceTags) { | ||
| if (policyConditionTags.contains(tag.getType())) { | ||
| matched = true; | ||
| break; | ||
| } | ||
| } |
There was a problem hiding this comment.
Approved method changes with a suggestion for further clarity.
The changes in the isMatched method improve clarity and control over the tag evaluation process. Using RangerAccessRequestUtil.getRequestTagsFromContext simplifies the retrieval of tags, and the explicit loop allows for an early exit, optimizing performance.
Consider adding a comment before the loop to explain why the loop exits early upon finding a match, as this could enhance readability for future maintainers.
| boolean matched = true; | ||
| Set<RangerTagForEval> resourceTags = RangerAccessRequestUtil.getRequestTagsFromContext(request.getContext()); | ||
|
|
||
| if (resourceTags != null) { | ||
| // check if resource Tags does not contain any tags in the policy condition | ||
| matched = (Collections.disjoint(resourceTags, policyConditionTags)); | ||
| for (RangerTagForEval tag : resourceTags) { | ||
| if (policyConditionTags.contains(tag.getType())) { | ||
| matched = false; | ||
| break; | ||
| } | ||
| } |
There was a problem hiding this comment.
Approved method changes with a suggestion for further clarity.
The changes in the isMatched method improve clarity and control over the tag evaluation process. Using RangerAccessRequestUtil.getRequestTagsFromContext simplifies the retrieval of tags, and the explicit loop allows for an early exit, optimizing performance.
Consider adding a comment before the loop to explain why the loop exits early upon finding a match, as this could enhance readability for future maintainers.
| if (CollectionUtils.isNotEmpty(policyConditionTags)) { | ||
| Set<RangerTagForEval> resourceTags = RangerAccessRequestUtil.getRequestTagsFromContext(request.getContext()); | ||
|
|
||
| // check if resource Tags atleast have to have all the tags in policy Condition | ||
| matched = resourceTags != null && resourceTags.containsAll(policyConditionTags); | ||
| if (CollectionUtils.isNotEmpty(resourceTags)) { | ||
| Set<String> tags = new HashSet<>(resourceTags.size()); | ||
|
|
||
| for (RangerTagForEval tag : resourceTags) { | ||
| tags.add(tag.getType()); | ||
| } | ||
|
|
||
| matched = tags.containsAll(policyConditionTags); | ||
| } else { | ||
| matched = false; | ||
| } |
There was a problem hiding this comment.
Approved method changes with a suggestion for further clarity.
The changes in the isMatched method improve clarity and robustness. Using RangerAccessRequestUtil.getRequestTagsFromContext simplifies the retrieval of tags, and the explicit handling of empty resourceTags by setting matched to false is a thoughtful approach.
Consider adding a comment before the check for resourceTags to explain why matched is set to false when no tags are present, as this could enhance readability for future maintainers.
| @JsonProperty("datasets") | ||
| protected Set<String> datasets = null; | ||
|
|
||
| @JsonProperty("projects") | ||
| protected Set<String> projects = null; | ||
|
|
There was a problem hiding this comment.
Proper handling of new fields datasets and projects.
The new fields datasets and projects are properly declared and initialized as null. However, it would be safer to initialize these as empty sets to avoid potential NullPointerExceptions when they are accessed before being explicitly set.
Consider initializing these fields with empty sets:
-protected Set<String> datasets = null;
-protected Set<String> projects = null;
+protected Set<String> datasets = new HashSet<>();
+protected Set<String> projects = new HashSet<>();Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @JsonProperty("datasets") | |
| protected Set<String> datasets = null; | |
| @JsonProperty("projects") | |
| protected Set<String> projects = null; | |
| @JsonProperty("datasets") | |
| protected Set<String> datasets = new HashSet<>(); | |
| @JsonProperty("projects") | |
| protected Set<String> projects = new HashSet<>(); | |
What changes were proposed in this pull request?
(Please fill in changes proposed in this fix. Create an issue in ASF JIRA before opening a pull request and
set the title of the pull request which starts with
the corresponding JIRA issue number. (e.g. RANGER-XXXX: Fix a typo in YYY))
How was this patch tested?
(Please explain how this patch was tested. Ex: unit tests, manual tests)
(If this patch involves UI changes, please attach a screen-shot; otherwise, remove this)
Summary by CodeRabbit
New Features
AuditIndexRecordclass for managing audit records.AuthzAuditEventclass to includedatasetsandprojectsfields.Enhancements
ElasticSearchAuditDestinationandSolrAuditDestination.Bug Fixes
HDFSAuditDestination.Chores