This is an automated email from the ASF dual-hosted git repository. jmclean pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push: new 5bc8fefe3 [#5569] improvement(CLI): Add ability to grant or revoke multiple roles or groups at once in the Gravitino CLI (#5811) 5bc8fefe3 is described below commit 5bc8fefe381934aba25fb2a7f27370bbd747e516 Author: Lord of Abyss <103809695+abyss-l...@users.noreply.github.com> AuthorDate: Wed Dec 11 05:33:33 2024 +0800 [#5569] improvement(CLI): Add ability to grant or revoke multiple roles or groups at once in the Gravitino CLI (#5811) ### What changes were proposed in this pull request? Add ability to grant or revoke multiple roles or groups at once in the Gravitino CLI, ### Why are the changes needed? Fix:[#5569](https://github.com/apache/gravitino/issues/5569) ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? #### I. bash test **Grant\revoke role to user** 1. Grant single role to a user  2. Grant multiple role to a user  3. Revoke multiple role to from user  **Grant\revoke role to group** 1. Grant single role to a group  2. Grant multiple role to a group  3. Revoke multiple role to from group  #### II. unit test Please check the [commit](https://github.com/apache/gravitino/commit/788396aab379638707ca5733e89069fab0e0002f) --- .../apache/gravitino/cli/GravitinoCommandLine.java | 23 +++++--- .../org/apache/gravitino/cli/GravitinoOptions.java | 4 +- .../apache/gravitino/cli/TestGroupCommands.java | 66 ++++++++++++++++++++- .../org/apache/gravitino/cli/TestUserCommands.java | 67 ++++++++++++++++++++++ 4 files changed, 148 insertions(+), 12 deletions(-) diff --git a/clients/cli/src/main/java/org/apache/gravitino/cli/GravitinoCommandLine.java b/clients/cli/src/main/java/org/apache/gravitino/cli/GravitinoCommandLine.java index 58255849a..843593487 100644 --- a/clients/cli/src/main/java/org/apache/gravitino/cli/GravitinoCommandLine.java +++ b/clients/cli/src/main/java/org/apache/gravitino/cli/GravitinoCommandLine.java @@ -19,6 +19,7 @@ package org.apache.gravitino.cli; +import com.google.common.base.Joiner; import com.google.common.base.Preconditions; import java.io.BufferedReader; import java.io.IOException; @@ -49,6 +50,8 @@ public class GravitinoCommandLine extends TestableCommandLine { public static final String CMD = "gcli"; // recommended name public static final String DEFAULT_URL = "http://localhost:8090"; + // This joiner is used to join multiple outputs to be displayed, e.g. roles or groups + private static final Joiner COMMA_JOINER = Joiner.on(", ").skipNulls(); /** * Gravitino Command line. @@ -382,15 +385,17 @@ public class GravitinoCommandLine extends TestableCommandLine { boolean force = line.hasOption(GravitinoOptions.FORCE); newDeleteUser(url, ignore, force, metalake, user).handle(); } else if (CommandActions.REVOKE.equals(command)) { - String role = line.getOptionValue(GravitinoOptions.ROLE); - if (role != null) { + String[] roles = line.getOptionValues(GravitinoOptions.ROLE); + for (String role : roles) { newRemoveRoleFromUser(url, ignore, metalake, user, role).handle(); } + System.out.printf("Remove roles %s from user %s%n", COMMA_JOINER.join(roles), user); } else if (CommandActions.GRANT.equals(command)) { - String role = line.getOptionValue(GravitinoOptions.ROLE); - if (role != null) { + String[] roles = line.getOptionValues(GravitinoOptions.ROLE); + for (String role : roles) { newAddRoleToUser(url, ignore, metalake, user, role).handle(); } + System.out.printf("Grant roles %s to user %s%n", String.join(", ", roles), user); } else { System.err.println(ErrorMessages.UNSUPPORTED_ACTION); } @@ -417,15 +422,17 @@ public class GravitinoCommandLine extends TestableCommandLine { boolean force = line.hasOption(GravitinoOptions.FORCE); newDeleteGroup(url, ignore, force, metalake, group).handle(); } else if (CommandActions.REVOKE.equals(command)) { - String role = line.getOptionValue(GravitinoOptions.ROLE); - if (role != null) { + String[] roles = line.getOptionValues(GravitinoOptions.ROLE); + for (String role : roles) { newRemoveRoleFromGroup(url, ignore, metalake, group, role).handle(); } + System.out.printf("Remove roles %s from group %s%n", COMMA_JOINER.join(roles), group); } else if (CommandActions.GRANT.equals(command)) { - String role = line.getOptionValue(GravitinoOptions.ROLE); - if (role != null) { + String[] roles = line.getOptionValues(GravitinoOptions.ROLE); + for (String role : roles) { newAddRoleToGroup(url, ignore, metalake, group, role).handle(); } + System.out.printf("Grant roles %s to group %s%n", COMMA_JOINER.join(roles), group); } else { System.err.println(ErrorMessages.UNSUPPORTED_ACTION); } diff --git a/clients/cli/src/main/java/org/apache/gravitino/cli/GravitinoOptions.java b/clients/cli/src/main/java/org/apache/gravitino/cli/GravitinoOptions.java index 7b7728a0b..91993226b 100644 --- a/clients/cli/src/main/java/org/apache/gravitino/cli/GravitinoOptions.java +++ b/clients/cli/src/main/java/org/apache/gravitino/cli/GravitinoOptions.java @@ -100,12 +100,12 @@ public class GravitinoOptions { options.addOption(createArgOption(AUTO, "column value auto-increments (true/false)")); options.addOption(createArgOption(DEFAULT, "default column value")); options.addOption(createSimpleOption("o", OWNER, "display entity owner")); - options.addOption(createArgOption("r", ROLE, "role name")); options.addOption(createArgOption(COLUMNFILE, "CSV file describing columns")); - // Properties and tags can have multiple values + // Options that support multiple values options.addOption(createArgsOption("p", PROPERTIES, "property name/value pairs")); options.addOption(createArgsOption("t", TAG, "tag name")); + options.addOption(createArgsOption("r", ROLE, "role name")); // Force delete entities and rename metalake operations options.addOption(createSimpleOption("f", FORCE, "force operation")); diff --git a/clients/cli/src/test/java/org/apache/gravitino/cli/TestGroupCommands.java b/clients/cli/src/test/java/org/apache/gravitino/cli/TestGroupCommands.java index 59af42803..780f33844 100644 --- a/clients/cli/src/test/java/org/apache/gravitino/cli/TestGroupCommands.java +++ b/clients/cli/src/test/java/org/apache/gravitino/cli/TestGroupCommands.java @@ -135,7 +135,6 @@ class TestGroupCommands { verify(mockDelete).handle(); } - @Test void testRemoveRoleFromGroupCommand() { RemoveRoleFromGroup mockRemove = mock(RemoveRoleFromGroup.class); when(mockCommandLine.hasOption(GravitinoOptions.METALAKE)).thenReturn(true); @@ -156,7 +155,6 @@ class TestGroupCommands { verify(mockRemove).handle(); } - @Test void testAddRoleToGroupCommand() { AddRoleToGroup mockAdd = mock(AddRoleToGroup.class); when(mockCommandLine.hasOption(GravitinoOptions.METALAKE)).thenReturn(true); @@ -176,4 +174,68 @@ class TestGroupCommands { commandLine.handleCommandLine(); verify(mockAdd).handle(); } + + @Test + void testRemoveRolesFromGroupCommand() { + RemoveRoleFromGroup mockRemoveFirstRole = mock(RemoveRoleFromGroup.class); + RemoveRoleFromGroup mockRemoveSecondRole = mock(RemoveRoleFromGroup.class); + when(mockCommandLine.hasOption(GravitinoOptions.METALAKE)).thenReturn(true); + when(mockCommandLine.getOptionValue(GravitinoOptions.METALAKE)).thenReturn("metalake_demo"); + when(mockCommandLine.hasOption(GravitinoOptions.GROUP)).thenReturn(true); + when(mockCommandLine.getOptionValue(GravitinoOptions.GROUP)).thenReturn("groupA"); + when(mockCommandLine.hasOption(GravitinoOptions.ROLE)).thenReturn(true); + when(mockCommandLine.getOptionValues(GravitinoOptions.ROLE)) + .thenReturn(new String[] {"admin", "role1"}); + GravitinoCommandLine commandLine = + spy( + new GravitinoCommandLine( + mockCommandLine, mockOptions, CommandEntities.GROUP, CommandActions.REVOKE)); + // Verify first role + doReturn(mockRemoveFirstRole) + .when(commandLine) + .newRemoveRoleFromGroup( + GravitinoCommandLine.DEFAULT_URL, false, "metalake_demo", "groupA", "admin"); + commandLine.handleCommandLine(); + verify(mockRemoveFirstRole).handle(); + + // Verify second role + doReturn(mockRemoveSecondRole) + .when(commandLine) + .newRemoveRoleFromGroup( + GravitinoCommandLine.DEFAULT_URL, false, "metalake_demo", "groupA", "role1"); + commandLine.handleCommandLine(); + verify(mockRemoveSecondRole).handle(); + } + + @Test + void testAddRolesToGroupCommand() { + AddRoleToGroup mockAddFirstRole = mock(AddRoleToGroup.class); + AddRoleToGroup mockAddSecondRole = mock(AddRoleToGroup.class); + when(mockCommandLine.hasOption(GravitinoOptions.METALAKE)).thenReturn(true); + when(mockCommandLine.getOptionValue(GravitinoOptions.METALAKE)).thenReturn("metalake_demo"); + when(mockCommandLine.hasOption(GravitinoOptions.GROUP)).thenReturn(true); + when(mockCommandLine.getOptionValue(GravitinoOptions.GROUP)).thenReturn("groupA"); + when(mockCommandLine.hasOption(GravitinoOptions.ROLE)).thenReturn(true); + when(mockCommandLine.getOptionValues(GravitinoOptions.ROLE)) + .thenReturn(new String[] {"admin", "role1"}); + GravitinoCommandLine commandLine = + spy( + new GravitinoCommandLine( + mockCommandLine, mockOptions, CommandEntities.GROUP, CommandActions.GRANT)); + // Verify first role + doReturn(mockAddFirstRole) + .when(commandLine) + .newAddRoleToGroup( + GravitinoCommandLine.DEFAULT_URL, false, "metalake_demo", "groupA", "admin"); + commandLine.handleCommandLine(); + verify(mockAddFirstRole).handle(); + + // Verify second role + doReturn(mockAddSecondRole) + .when(commandLine) + .newAddRoleToGroup( + GravitinoCommandLine.DEFAULT_URL, false, "metalake_demo", "groupA", "role1"); + commandLine.handleCommandLine(); + verify(mockAddSecondRole).handle(); + } } diff --git a/clients/cli/src/test/java/org/apache/gravitino/cli/TestUserCommands.java b/clients/cli/src/test/java/org/apache/gravitino/cli/TestUserCommands.java index ab6ff3a7c..0628d1f63 100644 --- a/clients/cli/src/test/java/org/apache/gravitino/cli/TestUserCommands.java +++ b/clients/cli/src/test/java/org/apache/gravitino/cli/TestUserCommands.java @@ -27,6 +27,7 @@ import static org.mockito.Mockito.when; import org.apache.commons.cli.CommandLine; import org.apache.commons.cli.Options; +import org.apache.gravitino.cli.commands.AddRoleToUser; import org.apache.gravitino.cli.commands.CreateUser; import org.apache.gravitino.cli.commands.DeleteUser; import org.apache.gravitino.cli.commands.ListUsers; @@ -173,4 +174,70 @@ class TestUserCommands { commandLine.handleCommandLine(); verify(mockAdd).handle(); } + + @Test + void testRemoveRolesFromUserCommand() { + RemoveRoleFromUser mockRemoveFirstRole = mock(RemoveRoleFromUser.class); + RemoveRoleFromUser mockRemoveSecondRole = mock(RemoveRoleFromUser.class); + + when(mockCommandLine.hasOption(GravitinoOptions.METALAKE)).thenReturn(true); + when(mockCommandLine.getOptionValue(GravitinoOptions.METALAKE)).thenReturn("metalake_demo"); + when(mockCommandLine.hasOption(GravitinoOptions.USER)).thenReturn(true); + when(mockCommandLine.getOptionValue(GravitinoOptions.USER)).thenReturn("user"); + when(mockCommandLine.hasOption(GravitinoOptions.ROLE)).thenReturn(true); + when(mockCommandLine.getOptionValues(GravitinoOptions.ROLE)) + .thenReturn(new String[] {"admin", "role1"}); + GravitinoCommandLine commandLine = + spy( + new GravitinoCommandLine( + mockCommandLine, mockOptions, CommandEntities.USER, CommandActions.REVOKE)); + // Verify first role + doReturn(mockRemoveFirstRole) + .when(commandLine) + .newRemoveRoleFromUser( + GravitinoCommandLine.DEFAULT_URL, false, "metalake_demo", "user", "admin"); + commandLine.handleCommandLine(); + verify(mockRemoveFirstRole).handle(); + + // Verify second role + doReturn(mockRemoveSecondRole) + .when(commandLine) + .newRemoveRoleFromUser( + GravitinoCommandLine.DEFAULT_URL, false, "metalake_demo", "user", "role1"); + commandLine.handleCommandLine(); + verify(mockRemoveSecondRole).handle(); + } + + @Test + void testAddRolesToUserCommand() { + AddRoleToUser mockAddFirstRole = mock(AddRoleToUser.class); + AddRoleToUser mockAddSecondRole = mock(AddRoleToUser.class); + + when(mockCommandLine.hasOption(GravitinoOptions.METALAKE)).thenReturn(true); + when(mockCommandLine.getOptionValue(GravitinoOptions.METALAKE)).thenReturn("metalake_demo"); + when(mockCommandLine.hasOption(GravitinoOptions.USER)).thenReturn(true); + when(mockCommandLine.getOptionValue(GravitinoOptions.USER)).thenReturn("user"); + when(mockCommandLine.hasOption(GravitinoOptions.ROLE)).thenReturn(true); + when(mockCommandLine.getOptionValues(GravitinoOptions.ROLE)) + .thenReturn(new String[] {"admin", "role1"}); + GravitinoCommandLine commandLine = + spy( + new GravitinoCommandLine( + mockCommandLine, mockOptions, CommandEntities.USER, CommandActions.GRANT)); + // Verify first role + doReturn(mockAddFirstRole) + .when(commandLine) + .newAddRoleToUser( + GravitinoCommandLine.DEFAULT_URL, false, "metalake_demo", "user", "admin"); + commandLine.handleCommandLine(); + verify(mockAddFirstRole).handle(); + + // Verify second role + doReturn(mockAddSecondRole) + .when(commandLine) + .newAddRoleToUser( + GravitinoCommandLine.DEFAULT_URL, false, "metalake_demo", "user", "role1"); + commandLine.handleCommandLine(); + verify(mockAddSecondRole).handle(); + } }