Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RANGER-4314: Delete nonexistent user return error status code #1

Open
wants to merge 524 commits into
base: master
Choose a base branch
from

Conversation

coldestlin
Copy link
Owner

@coldestlin coldestlin commented Jul 13, 2023

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

    • Introduced multi-JDK support in the CI/CD pipeline with builds for JDK 8 and JDK 11.
    • Added a new Docker build job to enhance deployment processes.
    • New AuditIndexRecord class for managing audit records.
    • Expanded AuthzAuditEvent class to include datasets and projects fields.
  • Enhancements

    • Improved logging mechanisms across various audit destination classes for better performance and readability.
    • Enhanced thread safety and resource management in ElasticSearchAuditDestination and SolrAuditDestination.
  • Bug Fixes

    • Resolved issues with logging and privileged action handling in HDFSAuditDestination.
  • Chores

    • Updated dependency management in the project to optimize performance and testing capabilities.

mneethiraj and others added 29 commits December 10, 2023 23:46
Change-Id: I5921e1720d97ccb0d0fe7a12d88e32ab24a28938
Signed-off-by: mehul <[email protected]>
fimugdha and others added 27 commits July 30, 2024 07:42
…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
Copy link

coderabbitai bot commented Sep 9, 2024

Walkthrough

The 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 pom.xml for improved dependency management and testing capabilities. Several Java classes are refactored for better logging practices, thread safety, and data handling, including the introduction of new fields in audit-related classes.

Changes

File Path Change Summary
.github/workflows/maven.yml Renamed workflow, added jobs for JDK 8 and JDK 11, and introduced a Docker build job.
agents-audit/pom.xml Updated dependencies, added exclusions, and included new testing dependencies.
agents-audit/src/main/java/.../AmazonCloudWatchAuditDestination.java Modified logger visibility and access modifier for the log method.
agents-audit/src/main/java/.../ElasticSearchAuditDestination.java Replaced client variable with AtomicReference, improved logging, and expanded toDoc method.
agents-audit/src/main/java/.../FileAuditDestination.java Enhanced logging to use parameterized messages.
agents-audit/src/main/java/.../HDFSAuditDestination.java Updated logging and privileged action handling, improved flush method.
agents-audit/src/main/java/.../SolrAuditDestination.java Refactored connection logic and updated toSolrDoc method to include new fields.
agents-audit/src/main/java/.../AuditIndexRecord.java Introduced new class for managing audit records with various attributes and methods.
agents-audit/src/main/java/.../AuthzAuditEvent.java Replaced Gson annotations with Jackson annotations, added new fields for datasets and projects.

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
Loading

🐰 "In the meadow, changes bloom,
New builds and Docker, making room.
With Java versions, side by side,
Artifacts and images, a joyful ride!
Hops of progress, swift and bright,
In every line, we take delight!" 🌼

Tip

New features

Walkthrough comment now includes:

  • Possibly related PRs: A list of potentially related PRs to help you recall past context.
  • Suggested labels: CodeRabbit can now suggest labels by learning from your past PRs. You can also provide custom labeling instructions in the UI or configuration file.

Notes:

  • Please share any feedback in the discussion post on our Discord.
  • Possibly related PRs, automatic label suggestions based on past PRs, learnings, and possibly related issues require data opt-in (enabled by default).

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?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a 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 in RangerServiceResourceMatcher

The modifications and additions in this class are significant and impact how resources are matched and evaluated. Here are some observations and suggestions:

  1. Variable and Type Changes:

    • The change from RangerServiceDef.RangerResourceDef to RangerResourceDef (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.
  2. 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.
  3. Method Signature Update:

    • The isAncestorOf method now accepts RangerResourceDef instead of RangerServiceDef.RangerResourceDef (line 71). This change should be verified across all usages to ensure compatibility.
  4. 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 new logStatus() 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

Commits

Files that changed from the base of the PR and between 593d15a and dd78538.

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 and hashCode methods are properly overridden to ensure that RangerPrincipal objects can be used effectively in collections like HashSet and HashMap.

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, and isGdsService 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 for id is correctly implemented.

This method is simple and follows Java standards for getter methods.


41-43: Setter method for id is correctly implemented.

This method is simple and follows Java standards for setter methods.


45-47: Getter method for filePath is correctly implemented.

This method is simple and follows Java standards for getter methods.


49-51: Setter method for filePath is correctly implemented.

This method is simple and follows Java standards for setter methods.


53-55: Getter method for linePosition is correctly implemented.

This method is simple and follows Java standards for getter methods.


57-59: Setter method for linePosition is correctly implemented.

This method is simple and follows Java standards for setter methods.


61-63: Getter method for status is correctly implemented.

This method is simple and follows Java standards for getter methods.


65-67: Setter method for status is correctly implemented.

This method is simple and follows Java standards for setter methods.


69-71: Getter method for fileCreateTime is correctly implemented.

This method is simple and follows Java standards for getter methods.


73-75: Setter method for fileCreateTime is correctly implemented.

This method is simple and follows Java standards for setter methods.


77-79: Getter method for writeCompleteTime is correctly implemented.

This method is simple and follows Java standards for getter methods.


81-83: Setter method for writeCompleteTime is correctly implemented.

This method is simple and follows Java standards for setter methods.


85-87: Getter method for doneCompleteTime is correctly implemented.

This method is simple and follows Java standards for getter methods.


89-91: Setter method for doneCompleteTime is correctly implemented.

This method is simple and follows Java standards for setter methods.


93-95: Getter method for lastSuccessTime is correctly implemented.

This method is simple and follows Java standards for getter methods.


97-99: Setter method for lastSuccessTime is correctly implemented.

This method is simple and follows Java standards for setter methods.


101-103: Getter method for lastFailedTime is correctly implemented.

This method is simple and follows Java standards for getter methods.


105-107: Setter method for lastFailedTime is correctly implemented.

This method is simple and follows Java standards for setter methods.


109-111: Getter method for failedAttemptCount is correctly implemented.

This method is simple and follows Java standards for getter methods.


113-115: Setter method for failedAttemptCount is correctly implemented.

This method is simple and follows Java standards for setter methods.


117-119: Getter method for lastAttempt is correctly implemented.

This method is simple and follows Java standards for getter methods.


121-123: Setter method for lastAttempt 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 of RangerServerHealth 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, and toString 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 include scriptEngine and enableJsonCtx 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 the script 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 and RangerScriptConditionEvaluator.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 the size() 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 for init.

The addition of the auditProviderName parameter in the init method at line 57 allows for more contextual initialization and enhances the functionality of the method. The method call within the init method correctly passes the new parameter to the superclass method.


121-121: Approve the reordered access modifier for stop.

The reordering of the access modifier for the stop method at line 121 from synchronized public to public 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 for getORCFileWrite.

The reordering of the access modifier for the getORCFileWrite method at line 139 from synchronized protected to protected 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 of init.

The updates to the init method at line 177, including additional property settings and the initialization of the ORCFileUtil 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 of hashCode and equals methods.

The implementation of hashCode and equals methods is correctly done, using fields such as name, description, options, users, groups, and roles. This ensures that RangerRole 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 outer RangerRole class.


135-135: Approved: Addition of hashCode and equals methods in nested class.

The RoleMember class includes well-implemented hashCode and equals 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, and isInitial properties is well thought out, enhancing the flexibility and control over the tag file management process in RangerFileBasedTagRetriever.


157-220: Approved: Consolidation of tag file reading logic in readFromFile.

The readFromFile method is effectively implemented, consolidating the tag file reading logic and handling initial reads as well as subsequent reads based on tagFilesCount. The method includes comprehensive error handling and logging, which enhances maintainability and debuggability.


190-213: Approved: Handling of multiple tag files in readFromFile.

The logic to handle multiple tag files by cycling through them based on tagFilesCount is correctly implemented in the readFromFile 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 in readFromFile.

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 of NULL_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, and nullSafeMap are well-implemented, utilizing the NullSafeSupplier to ensure that operations on collections are safe and do not risk null pointer exceptions. This enhancement significantly improves the robustness of collection handling in RangerBaseModelObject.


178-212: Approved: Methods for getting updatable collections.

The methods getUpdatableList, getUpdatableSet, and getUpdatableMap 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 and NullSafeSupplierV2 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, while NullSafeSupplierV2 optimizes memory usage by returning empty collections, suitable for read-only scenarios.

agents-common/pom.xml (4)

50-55: Approved: Exclusion of javax.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 of org.apache.commons:commons-compress and org.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, and org.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 in getBearerToken.

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 the GetFromURL 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 in verifyToken.

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 of JsonUtils.jsonToObject in getFromURL.

The change to use JsonUtils.jsonToObject for JSON parsing in the getFromURL 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 in logJSON.

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 in RangerJSONAuditWriterTest.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 in preCleanup 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 of enrich 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 in RangerGdsInfoRefresher.

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 to PrivilegedExceptionAction allows for handling exceptions within privileged code blocks, which is a good practice for robustness.


385-385: Approved: Improved thread synchronization with notifyAll().

Changing from notify() to notifyAll() 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 for PrivilegedExceptionAction.

The update to use a lambda expression with PrivilegedExceptionAction in the run method enhances readability and allows for exception handling within the privileged context.


247-251: Verify streamlined logic and updated logging in openFile.

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 in LocalFileLogBuffer.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 and JsonUtilsV2 centralizes JSON operations, enhancing maintainability and potentially improving error handling.

Also applies to: 44-44


262-262: Approved: Updated JSON deserialization in loadFromCache.

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 in saveToCache.

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 of PrivilegedExceptionAction in getUserStoreIfUpdated.

Replacing PrivilegedAction with PrivilegedExceptionAction allows for better exception management by enabling the method to throw checked exceptions, enhancing error handling capabilities.


414-414: Approved: Updated JSON parsing in getUserStoreIfUpdated.

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 file RangerSecurityZoneV2.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 for SPOOL_FILE_STATUS.

Moving the SPOOL_FILE_STATUS enum to a separate package and updating the import statement in AuditFileSpool.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 in AuditFileSpool.

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 of ObjectMapper, 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-local ObjectMapper instance. This method ensures that the JSON handling is thread-safe across different parts of the application that use MiscUtil.


323-329: Refactor JSON serialization to use Jackson.

The stringify method has been refactored to use Jackson's ObjectMapper 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 default toString() 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's readValue 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 custom toString 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 of RangerGdsBaseModelObject. It manages additional dataset-specific properties with appropriate getters and setters. The overridden toString 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 of RangerDataset, 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 in AuditFileCacheProviderSpool 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 and AuditIndexRecord are necessary due to the refactoring and enhanced encapsulation in the codebase.


251-261: Refactoring and error handling in init 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 in appendToDoneFile method approved.

The enhanced logging provides valuable information for debugging and auditing, which improves the traceability of file processing activities.


627-628: Enhanced logging in removeIndexRecord 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 of saveIndexFile 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 in appendToDoneFile 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 in removeOldFiles 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 in logError 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 in run 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 in logEvent 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 new SelfOrAncestorPredicate class which implements this interface.


41-41: Addition of RangerPluginContext import.

The import of RangerPluginContext is necessary due to the new parameter in the createRangerServiceResourceMatcher method which now requires a RangerPluginContext 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 of RangerTagEnricher 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 of CachedResourceEvaluators 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 the applyDelta 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 include RangerPluginContext 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 include RangerPluginContext and the subsequent handling of this context within the RangerDefaultPolicyResourceMatcher 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 the Predicate 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 to JsonUtils in RangerTagEnricher.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.java

Length 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.java

Length 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.java

Length 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 to null in the constructor and its handling in the updateFrom 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: Enhanced RangerAccessTypeDef 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 the RangerServiceDef 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 to changeTypeNames array.

The introduction of CHANGE_TYPE_GDS_UPDATE as a new constant and its inclusion in the changeTypeNames 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 in RangerServiceResource.

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 and RANGER_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 the RangerTagForEval 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: Enhanced toSolrDoc 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 in readProperties Method

The 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 of MalformedURLException 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 Projects

The addition of getDatasets and getProjects methods is a valuable enhancement to the RangerDefaultAuditHandler. These methods efficiently extract datasets and projects from the GdsAccessResult 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 in getAuthzEvents Method

The updates to the getAuthzEvents method, which now includes calls to getDatasets and getProjects, 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 in RangerPolicyResourceSignature

The addition of new constructors and the static method from significantly enhance the flexibility of the RangerPolicyResourceSignature 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 in toSignatureString Method

The toSignatureString method for RangerSharedResource is a valuable addition to the RangerPolicyResourceSignature 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 fields

The addition of the new boolean fields useRangerGroups, useOnlyRangerGroups, convertEmailToUsername, enableImplicitUserStoreEnricher, and enableImplicitGdsInfoEnricher 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 logic

The 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 constructor

The 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 methods

The 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 for commons-compress and zookeeper.

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 for lucene-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, and mockito-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 of org.apache.orc and io.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 and io.airlift dependencies.

The dependencies org.apache.orc and io.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 and io.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 directory
Scripts 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: Ensure toString method includes new fields.

The toString method has been updated to include the new fields datasets and projects. 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 with AtomicReference.

The replacement of the volatile client variable with an AtomicReference 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 in toDoc method.

The toDoc method has been updated to include the new fields datasets and projects. 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 replaces SerializedName, 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 in castLongObject.

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 to toSolrDoc method enhance data logging capabilities.

The addition of datasets and projects fields to the SolrInputDocument 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 of getGdsInfoIfUpdated 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 Enhancements

The 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 Handling

The use of nullSafeList and nullSafeMap 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 Methods

The addition of methods like addPolicyLabel, addPolicyItem, addDenyPolicyItem, etc., which ensure collections are mutable before adding items, is a robust enhancement. These methods utilize getUpdatableList to prevent potential NullPointerExceptions 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 Classes

The changes in nested classes like RangerPolicyResource and RangerPolicyItem to use nullSafeList and getUpdatableList 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 Classes

The 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

Comment on lines +50 to +55
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;
}
Copy link

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("} ");

Comment on lines +44 to +66
@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;
Copy link

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;
Copy link

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.

Comment on lines +33 to +89
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;
}
}
Copy link

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:

  1. 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.
  2. GDS Information Retrieval:

    • The retrieveGdsInfo method (lines 67-88) handles potential interruptions and other exceptions well. The use of specific exception types like ClosedByInterruptException is a good practice.
    • Consider adding more specific logging or metrics if the retrieval operations are critical or expected to fail under known conditions.
  3. 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 which dedupStrings is called (line 83).

Overall, the class is implemented with good practices in mind, but consider enhancing error messages and adding comments for complex logic to improve maintainability.

Comment on lines +31 to +98
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;
}
}
Copy link

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:

  1. Serialization and Annotations:

    • The use of JsonAutoDetect, JsonInclude, and JsonIgnoreProperties 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 and XmlAccessorType (lines 35-36) are correctly applied to the nested class, ensuring it can be serialized as part of XML-based configurations or APIs.
  2. 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.
  3. 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.

Comment on lines +124 to +127
@Override
public ServiceGdsInfo getGdsInfoIfUpdated(long lastKnownVersion, long lastActivationTimeInMillis) throws Exception {
return null;
}
Copy link

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.

Comment on lines +64 to +74
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;
}
}
Copy link

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.

Comment on lines +65 to +75
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;
}
}
Copy link

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.

Comment on lines +67 to +81
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;
}
Copy link

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.

Comment on lines +116 to +121
@JsonProperty("datasets")
protected Set<String> datasets = null;

@JsonProperty("projects")
protected Set<String> projects = null;

Copy link

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.

Suggested change
@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<>();

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

Successfully merging this pull request may close these issues.