Skip to content

Core: Add Java doc for the class PolarisPrivilege#3757

Open
flyrain wants to merge 1 commit intoapache:mainfrom
flyrain:addJavadoc
Open

Core: Add Java doc for the class PolarisPrivilege#3757
flyrain wants to merge 1 commit intoapache:mainfrom
flyrain:addJavadoc

Conversation

@flyrain
Copy link
Contributor

@flyrain flyrain commented Feb 13, 2026

To clarify PolarisPrivilege may only needed by the built-in RBAC

Checklist

  • 🛡️ Don't disclose security issues! (contact security@apache.org)
  • 🔗 Clearly explained why the changes are needed, or linked related issues: Fixes #
  • 🧪 Added/updated tests with good coverage, or manually tested (and explained how)
  • 💡 Added comments for complex logic
  • 🧾 Updated CHANGELOG.md (if needed)
  • 📚 Updated documentation in site/content/in-dev/unreleased (if needed)

*
* <p>Alternative authorizer implementations such as the OPA-based authorizer may not use these
* privileges. They operate at the {@link org.apache.polaris.core.auth.PolarisAuthorizableOperation}
* level and delegates all privilege/permission logic to external PDPs.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* level and delegates all privilege/permission logic to external PDPs.
* level and delegate all privilege/permission logic to external PDPs.

Copy link
Contributor

@dimas-b dimas-b left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the javadoc effort, @flyrain ! Please consider my comments optional and only an attempt to reach greater clarity.

/**
* Enumerates the privileges used by the built-in RBAC authorizer ({@link
* org.apache.polaris.core.auth.PolarisAuthorizerImpl}). These privileges are granted to roles and
* checked against securables during authorization.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: "securable" is not a common term (not in my experience)... It might be good to define it or link to where it is defined, or rephrase using more common terms.

* org.apache.polaris.core.auth.PolarisAuthorizerImpl}). These privileges are granted to roles and
* checked against securables during authorization.
*
* <p>Alternative authorizer implementations such as the OPA-based authorizer may not use these
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: While this statement is valid, is it relevant to this class? Why should the reader be concerned with OPA in the context of PolarisPrivilege? The paragraph above already scopes it down to the Internal Authorizer 🤔

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.

3 participants