-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
RANGER-4314: Delete nonexistent user return error status code #1
base: master
Are you sure you want to change the base?
Conversation
Change-Id: I5921e1720d97ccb0d0fe7a12d88e32ab24a28938 Signed-off-by: mehul <[email protected]>
…l view Signed-off-by: Madhan Neethiraj <[email protected]>
…he datashare Signed-off-by: Madhan Neethiraj <[email protected]>
Signed-off-by: Madhan Neethiraj <[email protected]>
Signed-off-by: Madhan Neethiraj <[email protected]>
…ogin fails Signed-off-by: Kishor Gollapalliwar <[email protected]>
Signed-off-by: Dineshkumar Yadav <[email protected]>
…ject Signed-off-by: Madhan Neethiraj <[email protected]>
…ogin fails Signed-off-by: Kishor Gollapalliwar <[email protected]>
Signed-off-by: Dineshkumar Yadav <[email protected]>
…ject Signed-off-by: Madhan Neethiraj <[email protected]>
Signed-off-by: Madhan Neethiraj <[email protected]>
…ant / Revoke request
Signed-off-by: Madhan Neethiraj <[email protected]>
Signed-off-by: Madhan Neethiraj <[email protected]>
…re improper Signed-off-by: Dhaval.Rajpara <[email protected]>
…o, snakeyaml; excluded io.netty Signed-off-by: Madhan Neethiraj <[email protected]>
…nAction in calls to UserGroupInformation.doAs()
… from Ranger KMS assembly Signed-off-by: Madhan Neethiraj <[email protected]>
Change-Id: I8208d5b9c6d71ae69249b89640f4a8828b750531 Signed-off-by: Pradeep Agrawal <[email protected]>
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 <[email protected]>
…ld & space optimizations (apache#288) Co-authored-by: abhishek-kumar <[email protected]>
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 inRangerServiceResourceMatcher
The 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.RangerResourceDef
toRangerResourceDef
(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
isAncestorOf
method now acceptsRangerResourceDef
instead ofRangerServiceDef.RangerResourceDef
(line 71). This change should be verified across all usages to ensure compatibility.New Method Addition:
- The
isLeaf
method (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
RangerGdsInfoRetriever
class 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
RangerPrincipal
class 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
Serializable
with a static version UID is correctly done, ensuring that changes to the class do not inadvertently affect serialization compatibility.The
equals
andhashCode
methods are properly overridden to ensure thatRangerPrincipal
objects can be used effectively in collections likeHashSet
andHashMap
.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
RangerServiceHeaderInfo
class 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
, andisGdsService
are 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
@JsonAutoDetect
and@JsonInclude
annotations 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
AuditFilter
class 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@JsonInclude
to 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 forid
is correctly implemented.This method is simple and follows Java standards for getter methods.
41-43
: Setter method forid
is correctly implemented.This method is simple and follows Java standards for setter methods.
45-47
: Getter method forfilePath
is correctly implemented.This method is simple and follows Java standards for getter methods.
49-51
: Setter method forfilePath
is correctly implemented.This method is simple and follows Java standards for setter methods.
53-55
: Getter method forlinePosition
is correctly implemented.This method is simple and follows Java standards for getter methods.
57-59
: Setter method forlinePosition
is correctly implemented.This method is simple and follows Java standards for setter methods.
61-63
: Getter method forstatus
is correctly implemented.This method is simple and follows Java standards for getter methods.
65-67
: Setter method forstatus
is correctly implemented.This method is simple and follows Java standards for setter methods.
69-71
: Getter method forfileCreateTime
is correctly implemented.This method is simple and follows Java standards for getter methods.
73-75
: Setter method forfileCreateTime
is correctly implemented.This method is simple and follows Java standards for setter methods.
77-79
: Getter method forwriteCompleteTime
is correctly implemented.This method is simple and follows Java standards for getter methods.
81-83
: Setter method forwriteCompleteTime
is correctly implemented.This method is simple and follows Java standards for setter methods.
85-87
: Getter method fordoneCompleteTime
is correctly implemented.This method is simple and follows Java standards for getter methods.
89-91
: Setter method fordoneCompleteTime
is correctly implemented.This method is simple and follows Java standards for setter methods.
93-95
: Getter method forlastSuccessTime
is correctly implemented.This method is simple and follows Java standards for getter methods.
97-99
: Setter method forlastSuccessTime
is correctly implemented.This method is simple and follows Java standards for setter methods.
101-103
: Getter method forlastFailedTime
is correctly implemented.This method is simple and follows Java standards for getter methods.
105-107
: Setter method forlastFailedTime
is correctly implemented.This method is simple and follows Java standards for setter methods.
109-111
: Getter method forfailedAttemptCount
is correctly implemented.This method is simple and follows Java standards for getter methods.
113-115
: Setter method forfailedAttemptCount
is correctly implemented.This method is simple and follows Java standards for setter methods.
117-119
: Getter method forlastAttempt
is correctly implemented.This method is simple and follows Java standards for getter methods.
121-123
: Setter method forlastAttempt
is correctly implemented.This method is simple and follows Java standards for setter methods.
125-136
:toString
method is well-implemented.The
toString
method 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@XmlAccessorType
annotations 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
RangerServerHealth
uses the Builder pattern effectively to maintain immutability. The use of final fields and a private constructor ensures that instances ofRangerServerHealth
are 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
, andtoString
are 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
RangerRequestScriptEvaluator
constructor to includescriptEngine
andenableJsonCtx
as 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
evaluateConditionScript
method call by removing previously passed parameters and only requiring thescript
parameter 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
RangerRequestScriptEvaluator
is 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.java
andRangerScriptConditionEvaluator.java
demonstrate proper usage.- Test cases in
RangerRequestScriptEvaluatorTest.java
verify 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
auditFileType
to 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
auditWriter
is 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-agent
module 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
auditProviderName
parameter in theinit
method at line 57 allows for more contextual initialization and enhances the functionality of the method. The method call within theinit
method 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
stop
method at line 121 fromsynchronized public
topublic synchronized
is 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
getORCFileWrite
method at line 139 fromsynchronized protected
toprotected synchronized
is 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
init
method at line 177, including additional property settings and the initialization of theORCFileUtil
instance, 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 ofhashCode
andequals
methods.The implementation of
hashCode
andequals
methods is correctly done, using fields such asname
,description
,options
,users
,groups
, androles
. This ensures thatRangerRole
instances 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
RoleMember
class correctly applies JSON annotations (@JsonInclude
and@JsonIgnoreProperties
), ensuring consistent serialization behavior with the outerRangerRole
class.
135-135
: Approved: Addition ofhashCode
andequals
methods in nested class.The
RoleMember
class includes well-implementedhashCode
andequals
methods, 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
, andisInitial
properties 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
readFromFile
method 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
tagFilesCount
is correctly implemented in thereadFromFile
method. 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
readFromFile
method 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_V2
and related methods.The introduction of
NULL_SAFE_SUPPLIER_V2
and 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
, andnullSafeMap
are well-implemented, utilizing theNullSafeSupplier
to 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
, andgetUpdatableMap
are 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:NullSafeSupplier
classes for optimized collection handling.The
NullSafeSupplierV1
andNullSafeSupplierV2
classes are well-implemented, providing configurable behavior for handling null or empty collections.NullSafeSupplierV1
creates new instances, which is suitable for scenarios where modifications are expected, whileNullSafeSupplierV2
optimizes 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-compress
andorg.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.slf4j
testing 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.jackson
libraries 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
getBearerToken
method 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
getBearerToken
method:Verification successful
Integration of
getBearerToken
verified.The
getBearerToken
method is used internally within theGetFromURL
class 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
verifyToken
method 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
verifyToken
method:
81-81
: Approved: Use ofJsonUtils.jsonToObject
ingetFromURL
.The change to use
JsonUtils.jsonToObject
for JSON parsing in thegetFromURL
method 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
getFromURL
method: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
logJSON
method 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
logJSON
method:Verification successful
Verified: The
logJSON
method changes are effectively covered by tests.The updates to the
logJSON
method, 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 inpreCleanup
method.The
preCleanup
method 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 ofenrich
method.The
enrich
method 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
RangerGdsInfoRefresher
inner 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
PrivilegedAction
toPrivilegedExceptionAction
allows 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
PrivilegedExceptionAction
in therun
method enhances readability and allows for exception handling within the privileged context.
247-251
: Verify streamlined logic and updated logging inopenFile
.The changes in the
openFile
method 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
openFile
are correct and consistent.The
openFile
method inLocalFileLogBuffer.java
has 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
JsonUtils
andJsonUtilsV2
centralizes 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.jsonToObject
for 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.objectToWriter
for serialization aligns with the new JSON handling strategy, replacing Gson with a more centralized approach.
381-391
: Approved: Use ofPrivilegedExceptionAction
ingetUserStoreIfUpdated
.Replacing
PrivilegedAction
withPrivilegedExceptionAction
allows 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.readResponse
for 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_STATUS
enum to a separate package and updating the import statement inAuditFileSpool.java
centralizes 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-localObjectMapper
instance. 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
stringify
method has been refactored to use Jackson'sObjectMapper
for 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
fromJson
method has been updated to use Jackson'sreadValue
method 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.
RangerGdsBaseModelObject
is well-implemented with appropriate getters, setters, and a customtoString
method. 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.
RangerDataset
is correctly implemented as an extension ofRangerGdsBaseModelObject
. It manages additional dataset-specific properties with appropriate getters and setters. The overriddentoString
method provides a comprehensive representation of the dataset's state, which is useful for debugging and logging.
131-175
: Consistent implementation for RangerProject.
RangerProject
mirrors 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.
RangerDataShare
is 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.
RangerSharedResource
effectively 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.
RangerDataShareInDataset
provides 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
MiscUtil
for JSON parsing inAuditFileCacheProviderSpool
aligns 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_STATUS
andAuditIndexRecord
are necessary due to the refactoring and enhanced encapsulation in the codebase.
251-261
: Refactoring and error handling ininit
method approved.The use of getter methods for
AuditIndexRecord
properties 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
loadIndexFile
method enhances the robustness and maintainability of the code by providing clearer diagnostics in case of parsing failures.
654-657
: Detailed logging inappendToDoneFile
method approved.The enhanced logging provides valuable information for debugging and auditing, which improves the traceability of file processing activities.
627-628
: Enhanced logging inremoveIndexRecord
method 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 ofsaveIndexFile
method approved.The method effectively serializes
AuditIndexRecord
objects to JSON and writes them to the index file, ensuring the persistence of index state.
654-657
: Lifecycle management inappendToDoneFile
method 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 inremoveOldFiles
method 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 inlogError
method 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 inrun
method 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 inlogEvent
method 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
Predicate
is necessary for the newSelfOrAncestorPredicate
class which implements this interface.
41-41
: Addition of RangerPluginContext import.The import of
RangerPluginContext
is necessary due to the new parameter in thecreateRangerServiceResourceMatcher
method which now requires aRangerPluginContext
object.
98-99
: Initialization of tagDownloadTimer.The initialization of
tagDownloadTimer
within the class constructor ensures that the timer is ready for use immediately after an object ofRangerTagEnricher
is 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
RangerReadWriteLock
and the instantiation ofCachedResourceEvaluators
are 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
init
method 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
isTagsDeduped
flag 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
isTagsDeduped
flag in theapplyDelta
method 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
clearCache
after 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
createRangerServiceResourceMatcher
to includeRangerPluginContext
as 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
RangerResourceTrie
objects 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
removeOldServiceResource
has 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
createRangerServiceResourceMatcher
to includeRangerPluginContext
and the subsequent handling of this context within theRangerDefaultPolicyResourceMatcher
setup 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
CachedResourceEvaluators
class 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
getEvaluators
method, 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
excludeDescendantMatches
method 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
SelfOrAncestorPredicate
class implements thePredicate
interface 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
JsonUtils
class 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 toJsonUtils
inRangerTagEnricher.java
is 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@JsonInclude
for better JSON serialization control.The change from
@JsonSerialize
to@JsonInclude
is 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_POLICIES
as 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
markerAccessTypes
tonull
in the constructor and its handling in theupdateFrom
method 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
markerAccessTypes
follows 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
AccessTypeCategory
enum 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
: EnhancedRangerAccessTypeDef
with 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
markerAccessTypes
as a new property in theRangerServiceDef
class 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
@JsonSerialize
with@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 tochangeTypeNames
array.The introduction of
CHANGE_TYPE_GDS_UPDATE
as a new constant and its inclusion in thechangeTypeNames
array 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_PROP
andRANGER_USE_LEGACY_SUBACCESS_AUTHORIZATION_DEFAULT
allows 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
@JsonSerialize
with@JsonInclude
and the removal of JAXB annotations streamline and modernize the JSON serialization process of theRangerTagForEval
class. 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
: EnhancedtoSolrDoc
method to include new fields.The addition of "datasets" and "projects" fields to the
toSolrDoc
method 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 inreadProperties
MethodThe refactoring of the
readProperties
method 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
IOException
instead ofMalformedURLException
is 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
getDatasets
andgetProjects
methods is a valuable enhancement to theRangerDefaultAuditHandler
. These methods efficiently extract datasets and projects from theGdsAccessResult
object, 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 ingetAuthzEvents
MethodThe updates to the
getAuthzEvents
method, which now includes calls togetDatasets
andgetProjects
, 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 inRangerPolicyResourceSignature
The addition of new constructors and the static method
from
significantly enhance the flexibility of theRangerPolicyResourceSignature
class. 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 intoSignatureString
MethodThe
toSignatureString
method forRangerSharedResource
is a valuable addition to theRangerPolicyResourceSignature
class. 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
, andenableImplicitGdsInfoEnricher
is 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
RangerPluginConfig
class.
149-153
: Approval for field copying in copy constructorThe direct copying of boolean fields from another
RangerPluginConfig
instance 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-compress
andzookeeper
.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.0
can 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-core
enhances 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.orc
andio.airlift
dependencies.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.orc
andio.airlift
dependencies.The dependencies
org.apache.orc
andio.airlift
are 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.orc
andio.airlift
:
security-admin/src/test/java/org/apache/ranger/audit/TestAuditQueue.java
ranger-presto-plugin-shim/src/main/java/org/apache/ranger/authorization/presto/authorizer/RangerConfig.java
agents-audit/src/main/java/org/apache/ranger/audit/utils/RangerORCAuditWriter.java
agents-audit/src/main/java/org/apache/ranger/audit/utils/ORCFileUtil.java
XML files including these dependencies:
ranger-presto-plugin-shim/pom.xml
plugin-trino/pom.xml
- Various assembly XML files in the
distro/src/main/assembly
directoryScripts 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
isMatched
method 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
isMatched
method 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
isMatched
method 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
@SerializedName
to@JsonProperty
is 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
: EnsuretoString
method includes new fields.The
toString
method has been updated to include the new fieldsdatasets
andprojects
. 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
client
variable with anAtomicReference
is 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 intoDoc
method.The
toDoc
method has been updated to include the new fieldsdatasets
andprojects
. 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
JsonProperty
replacesSerializedName
, 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
castLongObject
method 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
Krb5HttpClientBuilder
ensures 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 totoSolrDoc
method enhance data logging capabilities.The addition of
datasets
andprojects
fields to theSolrInputDocument
allows 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.jackson
annotations 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 ofgetGdsInfoIfUpdated
method 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
nullSafeList
andnullSafeMap
utility 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 utilizegetUpdatableList
to prevent potentialNullPointerExceptions
and 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
RangerPolicyResource
andRangerPolicyItem
to usenullSafeList
andgetUpdatableList
for 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 = ", ";
}
@@ -64,4 +65,5 @@ public interface RangerAdminClient { | |||
|
|||
RangerUserStore getUserStoreIfUpdated(long lastKnownUserStoreVersion, long lastActivationTimeInMillis) throws Exception; | |||
|
|||
ServiceGdsInfo getGdsInfoIfUpdated(long lastKnownVersion, long lastActivationTimeInMillis) throws Exception; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review of RangerAdminGdsInfoRetriever
class
The class is well-structured for its intended functionality. Here are some observations and suggestions:
-
Initialization and Error Handling:
- The
init
method (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
init
method 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
retrieveGdsInfo
method (lines 67-88) handles potential interruptions and other exceptions well. The use of specific exception types likeClosedByInterruptException
is 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
retrieveGdsInfo
could benefit from a brief comment explaining the conditions under whichdedupStrings
is 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
, andJsonIgnoreProperties
annotations (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
XmlRootElement
andXmlAccessorType
(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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
AuditIndexRecord
class for managing audit records.AuthzAuditEvent
class to includedatasets
andprojects
fields.Enhancements
ElasticSearchAuditDestination
andSolrAuditDestination
.Bug Fixes
HDFSAuditDestination
.Chores