Skip to content

Conversation

@komamitsu
Copy link
Contributor

@komamitsu komamitsu commented Oct 24, 2025

Description

This PR adds APIs for the roll based access control feature.

Related issues and/or PRs

https://github.com/scalar-labs/scalardb-cluster/pull/1342

Changes made

  • Ported the new RBAC APIs of TentativeAuthAdmin and TentativeDecoratedDistributedAdmin from scalardb-cluster project to AuthAdmin and DecoratedDistributedAdmin, respectively

Checklist

The following is a best-effort checklist. If any items in this checklist are not applicable to this PR or are dependent on other, unmerged PRs, please still mark the checkboxes after you have read and understood each item.

  • I have commented my code, particularly in hard-to-understand areas.
  • I have updated the documentation to reflect the changes.
  • I have considered whether similar issues could occur in other products, components, or modules if this PR is for bug fixes.
  • Any remaining open issues linked to this PR are documented and up-to-date (Jira, GitHub, etc.).
  • Tests (unit, integration, etc.) have been added for the changes.
  • My changes generate no new warnings.
  • Any dependent changes in other PRs have been merged and published.

Additional notes (optional)

None

Release notes

N/A

Copilot AI review requested due to automatic review settings October 24, 2025 06:04
@komamitsu komamitsu self-assigned this Oct 24, 2025
@komamitsu komamitsu added the enhancement New feature or request label Oct 24, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR ports RBAC (Role-Based Access Control) APIs from tentative implementations to the main AuthAdmin and DecoratedDistributedTransactionAdmin classes.

Key changes:

  • Adds role management APIs (create, drop, get roles)
  • Adds role-to-user and role-to-role assignment operations with admin options
  • Adds privilege management APIs for roles at namespace and table levels

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
core/src/main/java/com/scalar/db/api/AuthAdmin.java Defines new RBAC API methods with comprehensive documentation and introduces supporting interfaces (Role, RoleDetail, UserRole, RoleHierarchy)
core/src/main/java/com/scalar/db/common/DecoratedDistributedTransactionAdmin.java Implements delegation of all new RBAC methods to the underlying distributedTransactionAdmin instance

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @komamitsu, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request introduces a comprehensive set of APIs for Role-Based Access Control (RBAC). This enhancement allows for the creation and management of roles, assigning roles to users, defining role hierarchies, and granting or revoking privileges based on roles at both namespace and table levels, providing a more granular and flexible access control mechanism.

Highlights

  • Introduction of RBAC APIs: New APIs for Role-Based Access Control (RBAC) have been added to AuthAdmin to manage roles, user-role assignments, role hierarchies, and role-based privileges.
  • API Implementation: The new RBAC APIs, previously in TentativeAuthAdmin and TentativeDecoratedDistributedAdmin, have been ported and implemented in AuthAdmin and DecoratedDistributedTransactionAdmin respectively.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@komamitsu
Copy link
Contributor Author

/gemini review

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a set of new APIs for Role-Based Access Control (RBAC) by extending the AuthAdmin interface with new default methods and interfaces. It also adds the corresponding decorator implementations in DecoratedDistributedTransactionAdmin. The changes are well-structured. My review includes a few suggestions to improve the clarity and documentation of the new APIs, such as renaming a parameter for better consistency, clarifying the purpose of a parameter in a method's Javadoc, and adding a missing Javadoc for a new interface. These changes will help make the new RBAC APIs more intuitive and easier to use.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This PR introduces new RBAC APIs to AuthAdmin and DecoratedDistributedAdmin. The changes involve adding default methods to the AuthAdmin interface for role management and privilege granting/revoking, and implementing these methods in DecoratedDistributedAdmin by delegating to the underlying distributedTransactionAdmin. The review focuses on the correctness of the new APIs and their implementation.

komamitsu and others added 6 commits October 24, 2025 15:24
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
and rename `memberRole` to `memberRoleName`
and return it from getRolesForUser()
…labs/scalardb into add-auth-role-to-admin-interface
@komamitsu
Copy link
Contributor Author

This PR will be ready once https://github.com/scalar-labs/scalardb-cluster/pull/1379 is merged

Copy link
Collaborator

@brfrn169 brfrn169 left a comment

Choose a reason for hiding this comment

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

Left a comment. Other than that, LGTM! Thank you!

Copy link
Contributor

@Torch3333 Torch3333 left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

Copy link
Contributor

@feeblefakie feeblefakie left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you!

@komamitsu komamitsu merged commit 69a70fa into master Nov 6, 2025
125 of 128 checks passed
@komamitsu komamitsu deleted the add-auth-role-to-admin-interface branch November 6, 2025 06:34
feeblefakie added a commit that referenced this pull request Nov 6, 2025
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: Toshihiro Suzuki <[email protected]>
Co-authored-by: Hiroyuki Yamada <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants