diff --git a/clients/cli/src/main/java/org/apache/gravitino/cli/ErrorMessages.java b/clients/cli/src/main/java/org/apache/gravitino/cli/ErrorMessages.java index 1d6db1a5acd..a5253664501 100644 --- a/clients/cli/src/main/java/org/apache/gravitino/cli/ErrorMessages.java +++ b/clients/cli/src/main/java/org/apache/gravitino/cli/ErrorMessages.java @@ -32,6 +32,7 @@ public class ErrorMessages { public static final String MISSING_NAME = "Missing --name option."; public static final String MISSING_GROUP = "Missing --group option."; public static final String MISSING_USER = "Missing --user option."; + public static final String MISSING_ROLE = "Missing --role option."; public static final String MISSING_TAG = "Missing --tag option."; public static final String METALAKE_EXISTS = "Metalake already exists."; public static final String CATALOG_EXISTS = "Catalog already exists."; 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 48d97294350..06ac09d673b 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 @@ -720,17 +720,26 @@ protected void handleRoleCommand() { String userName = line.getOptionValue(GravitinoOptions.LOGIN); FullName name = new FullName(line); String metalake = name.getMetalakeName(); - String role = line.getOptionValue(GravitinoOptions.ROLE); String[] privileges = line.getOptionValues(GravitinoOptions.PRIVILEGE); Command.setAuthenticationMode(auth, userName); + String[] roles = line.getOptionValues(GravitinoOptions.ROLE); + if (roles == null && !CommandActions.LIST.equals(command)) { + System.err.println(ErrorMessages.MISSING_ROLE); + Main.exit(-1); + } + + if (roles != null) { + roles = Arrays.stream(roles).distinct().toArray(String[]::new); + } + switch (command) { case CommandActions.DETAILS: if (line.hasOption(GravitinoOptions.AUDIT)) { - newRoleAudit(url, ignore, metalake, role).handle(); + newRoleAudit(url, ignore, metalake, getOneRole(roles, CommandActions.DETAILS)).handle(); } else { - newRoleDetails(url, ignore, metalake, role).handle(); + newRoleDetails(url, ignore, metalake, getOneRole(roles, CommandActions.DETAILS)).handle(); } break; @@ -739,20 +748,24 @@ protected void handleRoleCommand() { break; case CommandActions.CREATE: - newCreateRole(url, ignore, metalake, role).handle(); + newCreateRole(url, ignore, metalake, roles).handle(); break; case CommandActions.DELETE: boolean forceDelete = line.hasOption(GravitinoOptions.FORCE); - newDeleteRole(url, ignore, forceDelete, metalake, role).handle(); + newDeleteRole(url, ignore, forceDelete, metalake, roles).handle(); break; case CommandActions.GRANT: - newGrantPrivilegesToRole(url, ignore, metalake, role, name, privileges).handle(); + newGrantPrivilegesToRole( + url, ignore, metalake, getOneRole(roles, CommandActions.GRANT), name, privileges) + .handle(); break; case CommandActions.REVOKE: - newRevokePrivilegesFromRole(url, ignore, metalake, role, name, privileges).handle(); + newRevokePrivilegesFromRole( + url, ignore, metalake, getOneRole(roles, CommandActions.REMOVE), name, privileges) + .handle(); break; default: @@ -762,6 +775,12 @@ protected void handleRoleCommand() { } } + private String getOneRole(String[] roles, String command) { + Preconditions.checkArgument( + roles.length == 1, command + " requires only one role, but multiple are currently passed."); + return roles[0]; + } + /** * Handles the command execution for Columns based on command type and the command line options. */ diff --git a/clients/cli/src/main/java/org/apache/gravitino/cli/TestableCommandLine.java b/clients/cli/src/main/java/org/apache/gravitino/cli/TestableCommandLine.java index effe0da1f10..f07244c0053 100644 --- a/clients/cli/src/main/java/org/apache/gravitino/cli/TestableCommandLine.java +++ b/clients/cli/src/main/java/org/apache/gravitino/cli/TestableCommandLine.java @@ -468,13 +468,13 @@ protected RoleAudit newRoleAudit(String url, boolean ignore, String metalake, St return new RoleAudit(url, ignore, metalake, role); } - protected CreateRole newCreateRole(String url, boolean ignore, String metalake, String role) { - return new CreateRole(url, ignore, metalake, role); + protected CreateRole newCreateRole(String url, boolean ignore, String metalake, String[] roles) { + return new CreateRole(url, ignore, metalake, roles); } protected DeleteRole newDeleteRole( - String url, boolean ignore, boolean force, String metalake, String role) { - return new DeleteRole(url, ignore, force, metalake, role); + String url, boolean ignore, boolean force, String metalake, String[] roles) { + return new DeleteRole(url, ignore, force, metalake, roles); } protected TagDetails newTagDetails(String url, boolean ignore, String metalake, String tag) { diff --git a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/CreateRole.java b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/CreateRole.java index fea6fe4a720..e821013471f 100644 --- a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/CreateRole.java +++ b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/CreateRole.java @@ -19,6 +19,7 @@ package org.apache.gravitino.cli.commands; +import com.google.common.base.Joiner; import java.util.Collections; import org.apache.gravitino.cli.ErrorMessages; import org.apache.gravitino.client.GravitinoClient; @@ -27,7 +28,7 @@ public class CreateRole extends Command { protected String metalake; - protected String role; + protected String[] roles; /** * Create a new role. @@ -35,12 +36,12 @@ public class CreateRole extends Command { * @param url The URL of the Gravitino server. * @param ignoreVersions If true don't check the client/server versions match. * @param metalake The name of the metalake. - * @param role The name of the role. + * @param roles The array of roles. */ - public CreateRole(String url, boolean ignoreVersions, String metalake, String role) { + public CreateRole(String url, boolean ignoreVersions, String metalake, String[] roles) { super(url, ignoreVersions); this.metalake = metalake; - this.role = role; + this.roles = roles; } /** Create a new role. */ @@ -48,7 +49,9 @@ public CreateRole(String url, boolean ignoreVersions, String metalake, String ro public void handle() { try { GravitinoClient client = buildClient(metalake); - client.createRole(role, null, Collections.EMPTY_LIST); + for (String role : roles) { + client.createRole(role, null, Collections.EMPTY_LIST); + } } catch (NoSuchMetalakeException err) { exitWithError(ErrorMessages.UNKNOWN_METALAKE); } catch (RoleAlreadyExistsException err) { @@ -57,6 +60,6 @@ public void handle() { exitWithError(exp.getMessage()); } - System.out.println(role + " created"); + System.out.println(Joiner.on(", ").join(roles) + " created"); } } diff --git a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/DeleteRole.java b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/DeleteRole.java index f175d95043f..fa7c8cacc2c 100644 --- a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/DeleteRole.java +++ b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/DeleteRole.java @@ -19,6 +19,9 @@ package org.apache.gravitino.cli.commands; +import com.google.common.base.Joiner; +import com.google.common.collect.Lists; +import java.util.List; import org.apache.gravitino.cli.AreYouSure; import org.apache.gravitino.cli.ErrorMessages; import org.apache.gravitino.client.GravitinoClient; @@ -26,9 +29,9 @@ import org.apache.gravitino.exceptions.NoSuchRoleException; public class DeleteRole extends Command { - + public static final Joiner COMMA_JOINER = Joiner.on(", ").skipNulls(); protected String metalake; - protected String role; + protected String[] roles; protected boolean force; /** @@ -38,28 +41,30 @@ public class DeleteRole extends Command { * @param ignoreVersions If true don't check the client/server versions match. * @param force Force operation. * @param metalake The name of the metalake. - * @param role The name of the role. + * @param roles The name of the role. */ public DeleteRole( - String url, boolean ignoreVersions, boolean force, String metalake, String role) { + String url, boolean ignoreVersions, boolean force, String metalake, String[] roles) { super(url, ignoreVersions); this.metalake = metalake; this.force = force; - this.role = role; + this.roles = roles; } /** Delete a role. */ @Override public void handle() { - boolean deleted = false; - if (!AreYouSure.really(force)) { return; } + List failedRoles = Lists.newArrayList(); + List successRoles = Lists.newArrayList(); try { GravitinoClient client = buildClient(metalake); - deleted = client.deleteRole(role); + for (String role : roles) { + (client.deleteRole(role) ? successRoles : failedRoles).add(role); + } } catch (NoSuchMetalakeException err) { exitWithError(ErrorMessages.UNKNOWN_METALAKE); } catch (NoSuchRoleException err) { @@ -68,10 +73,15 @@ public void handle() { exitWithError(exp.getMessage()); } - if (deleted) { - System.out.println(role + " deleted."); + if (failedRoles.isEmpty()) { + System.out.println(COMMA_JOINER.join(successRoles) + " deleted."); } else { - System.out.println(role + " not deleted."); + System.err.println( + COMMA_JOINER.join(successRoles) + + " deleted, " + + "but " + + COMMA_JOINER.join(failedRoles) + + " is not deleted."); } } } diff --git a/clients/cli/src/test/java/org/apache/gravitino/cli/TestRoleCommands.java b/clients/cli/src/test/java/org/apache/gravitino/cli/TestRoleCommands.java index 88b380d63ee..0e671067e3f 100644 --- a/clients/cli/src/test/java/org/apache/gravitino/cli/TestRoleCommands.java +++ b/clients/cli/src/test/java/org/apache/gravitino/cli/TestRoleCommands.java @@ -19,14 +19,20 @@ package org.apache.gravitino.cli; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThrows; import static org.mockito.Mockito.any; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.eq; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import java.io.ByteArrayOutputStream; +import java.io.PrintStream; +import java.nio.charset.StandardCharsets; import org.apache.commons.cli.CommandLine; import org.apache.commons.cli.Options; import org.apache.gravitino.cli.commands.CreateRole; @@ -36,17 +42,30 @@ import org.apache.gravitino.cli.commands.RevokePrivilegesFromRole; import org.apache.gravitino.cli.commands.RoleAudit; import org.apache.gravitino.cli.commands.RoleDetails; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; class TestRoleCommands { private CommandLine mockCommandLine; private Options mockOptions; + private final ByteArrayOutputStream outContent = new ByteArrayOutputStream(); + private final ByteArrayOutputStream errContent = new ByteArrayOutputStream(); + private final PrintStream originalOut = System.out; + private final PrintStream originalErr = System.err; @BeforeEach void setUp() { mockCommandLine = mock(CommandLine.class); mockOptions = mock(Options.class); + System.setOut(new PrintStream(outContent)); + System.setErr(new PrintStream(errContent)); + } + + @AfterEach + public void restoreStreams() { + System.setOut(originalOut); + System.setErr(originalErr); } @Test @@ -71,7 +90,7 @@ void testRoleDetailsCommand() { when(mockCommandLine.hasOption(GravitinoOptions.METALAKE)).thenReturn(true); when(mockCommandLine.getOptionValue(GravitinoOptions.METALAKE)).thenReturn("metalake_demo"); when(mockCommandLine.hasOption(GravitinoOptions.ROLE)).thenReturn(true); - when(mockCommandLine.getOptionValue(GravitinoOptions.ROLE)).thenReturn("admin"); + when(mockCommandLine.getOptionValues(GravitinoOptions.ROLE)).thenReturn(new String[] {"admin"}); GravitinoCommandLine commandLine = spy( new GravitinoCommandLine( @@ -83,13 +102,31 @@ void testRoleDetailsCommand() { verify(mockDetails).handle(); } + @Test + void testRoleDetailsCommandWithMultipleRoles() { + when(mockCommandLine.hasOption(GravitinoOptions.METALAKE)).thenReturn(true); + when(mockCommandLine.getOptionValue(GravitinoOptions.METALAKE)).thenReturn("metalake_demo"); + when(mockCommandLine.hasOption(GravitinoOptions.ROLE)).thenReturn(true); + when(mockCommandLine.getOptionValues(GravitinoOptions.ROLE)) + .thenReturn(new String[] {"admin", "roleA", "roleB"}); + GravitinoCommandLine commandLine = + spy( + new GravitinoCommandLine( + mockCommandLine, mockOptions, CommandEntities.ROLE, CommandActions.DETAILS)); + + assertThrows(IllegalArgumentException.class, commandLine::handleCommandLine); + verify(commandLine, never()) + .newRoleDetails( + eq(GravitinoCommandLine.DEFAULT_URL), eq(false), eq("metalake_demo"), any()); + } + @Test void testRoleAuditCommand() { RoleAudit mockAudit = mock(RoleAudit.class); when(mockCommandLine.hasOption(GravitinoOptions.METALAKE)).thenReturn(true); when(mockCommandLine.getOptionValue(GravitinoOptions.METALAKE)).thenReturn("metalake_demo"); when(mockCommandLine.hasOption(GravitinoOptions.ROLE)).thenReturn(true); - when(mockCommandLine.getOptionValue(GravitinoOptions.ROLE)).thenReturn("group"); + when(mockCommandLine.getOptionValues(GravitinoOptions.ROLE)).thenReturn(new String[] {"group"}); when(mockCommandLine.hasOption(GravitinoOptions.AUDIT)).thenReturn(true); GravitinoCommandLine commandLine = spy( @@ -108,14 +145,39 @@ void testCreateRoleCommand() { when(mockCommandLine.hasOption(GravitinoOptions.METALAKE)).thenReturn(true); when(mockCommandLine.getOptionValue(GravitinoOptions.METALAKE)).thenReturn("metalake_demo"); when(mockCommandLine.hasOption(GravitinoOptions.ROLE)).thenReturn(true); - when(mockCommandLine.getOptionValue(GravitinoOptions.ROLE)).thenReturn("admin"); + when(mockCommandLine.getOptionValues(GravitinoOptions.ROLE)).thenReturn(new String[] {"admin"}); GravitinoCommandLine commandLine = spy( new GravitinoCommandLine( mockCommandLine, mockOptions, CommandEntities.ROLE, CommandActions.CREATE)); doReturn(mockCreate) .when(commandLine) - .newCreateRole(GravitinoCommandLine.DEFAULT_URL, false, "metalake_demo", "admin"); + .newCreateRole( + GravitinoCommandLine.DEFAULT_URL, false, "metalake_demo", new String[] {"admin"}); + commandLine.handleCommandLine(); + verify(mockCreate).handle(); + } + + @Test + void testCreateRolesCommand() { + CreateRole mockCreate = mock(CreateRole.class); + when(mockCommandLine.hasOption(GravitinoOptions.METALAKE)).thenReturn(true); + when(mockCommandLine.getOptionValue(GravitinoOptions.METALAKE)).thenReturn("metalake_demo"); + when(mockCommandLine.hasOption(GravitinoOptions.ROLE)).thenReturn(true); + when(mockCommandLine.getOptionValues(GravitinoOptions.ROLE)) + .thenReturn(new String[] {"admin", "engineer", "scientist"}); + GravitinoCommandLine commandLine = + spy( + new GravitinoCommandLine( + mockCommandLine, mockOptions, CommandEntities.ROLE, CommandActions.CREATE)); + + doReturn(mockCreate) + .when(commandLine) + .newCreateRole( + eq(GravitinoCommandLine.DEFAULT_URL), + eq(false), + eq("metalake_demo"), + eq(new String[] {"admin", "engineer", "scientist"})); commandLine.handleCommandLine(); verify(mockCreate).handle(); } @@ -126,14 +188,45 @@ void testDeleteRoleCommand() { when(mockCommandLine.hasOption(GravitinoOptions.METALAKE)).thenReturn(true); when(mockCommandLine.getOptionValue(GravitinoOptions.METALAKE)).thenReturn("metalake_demo"); when(mockCommandLine.hasOption(GravitinoOptions.ROLE)).thenReturn(true); - when(mockCommandLine.getOptionValue(GravitinoOptions.ROLE)).thenReturn("admin"); + when(mockCommandLine.getOptionValues(GravitinoOptions.ROLE)).thenReturn(new String[] {"admin"}); GravitinoCommandLine commandLine = spy( new GravitinoCommandLine( mockCommandLine, mockOptions, CommandEntities.ROLE, CommandActions.DELETE)); doReturn(mockDelete) .when(commandLine) - .newDeleteRole(GravitinoCommandLine.DEFAULT_URL, false, false, "metalake_demo", "admin"); + .newDeleteRole( + GravitinoCommandLine.DEFAULT_URL, + false, + false, + "metalake_demo", + new String[] {"admin"}); + commandLine.handleCommandLine(); + verify(mockDelete).handle(); + } + + @Test + void testDeleteRolesCommand() { + DeleteRole mockDelete = mock(DeleteRole.class); + when(mockCommandLine.hasOption(GravitinoOptions.METALAKE)).thenReturn(true); + when(mockCommandLine.getOptionValue(GravitinoOptions.METALAKE)).thenReturn("metalake_demo"); + when(mockCommandLine.hasOption(GravitinoOptions.ROLE)).thenReturn(true); + when(mockCommandLine.getOptionValues(GravitinoOptions.ROLE)) + .thenReturn(new String[] {"admin", "engineer", "scientist"}); + when(mockCommandLine.hasOption(GravitinoOptions.FORCE)).thenReturn(false); + GravitinoCommandLine commandLine = + spy( + new GravitinoCommandLine( + mockCommandLine, mockOptions, CommandEntities.ROLE, CommandActions.DELETE)); + + doReturn(mockDelete) + .when(commandLine) + .newDeleteRole( + eq(GravitinoCommandLine.DEFAULT_URL), + eq(false), + eq(false), + eq("metalake_demo"), + eq(new String[] {"admin", "engineer", "scientist"})); commandLine.handleCommandLine(); verify(mockDelete).handle(); } @@ -144,7 +237,7 @@ void testDeleteRoleForceCommand() { when(mockCommandLine.hasOption(GravitinoOptions.METALAKE)).thenReturn(true); when(mockCommandLine.getOptionValue(GravitinoOptions.METALAKE)).thenReturn("metalake_demo"); when(mockCommandLine.hasOption(GravitinoOptions.ROLE)).thenReturn(true); - when(mockCommandLine.getOptionValue(GravitinoOptions.ROLE)).thenReturn("admin"); + when(mockCommandLine.getOptionValues(GravitinoOptions.ROLE)).thenReturn(new String[] {"admin"}); when(mockCommandLine.hasOption(GravitinoOptions.FORCE)).thenReturn(true); GravitinoCommandLine commandLine = spy( @@ -152,7 +245,8 @@ void testDeleteRoleForceCommand() { mockCommandLine, mockOptions, CommandEntities.ROLE, CommandActions.DELETE)); doReturn(mockDelete) .when(commandLine) - .newDeleteRole(GravitinoCommandLine.DEFAULT_URL, false, true, "metalake_demo", "admin"); + .newDeleteRole( + GravitinoCommandLine.DEFAULT_URL, false, true, "metalake_demo", new String[] {"admin"}); commandLine.handleCommandLine(); verify(mockDelete).handle(); } @@ -166,7 +260,7 @@ void testGrantPrivilegesToRole() { when(mockCommandLine.hasOption(GravitinoOptions.NAME)).thenReturn(true); when(mockCommandLine.getOptionValue(GravitinoOptions.NAME)).thenReturn("catalog"); when(mockCommandLine.hasOption(GravitinoOptions.ROLE)).thenReturn(true); - when(mockCommandLine.getOptionValue(GravitinoOptions.ROLE)).thenReturn("admin"); + when(mockCommandLine.getOptionValues(GravitinoOptions.ROLE)).thenReturn(new String[] {"admin"}); when(mockCommandLine.hasOption(GravitinoOptions.PRIVILEGE)).thenReturn(true); when(mockCommandLine.getOptionValues(GravitinoOptions.PRIVILEGE)).thenReturn(privileges); GravitinoCommandLine commandLine = @@ -195,7 +289,7 @@ void testRevokePrivilegesFromRole() { when(mockCommandLine.hasOption(GravitinoOptions.NAME)).thenReturn(true); when(mockCommandLine.getOptionValue(GravitinoOptions.NAME)).thenReturn("catalog"); when(mockCommandLine.hasOption(GravitinoOptions.ROLE)).thenReturn(true); - when(mockCommandLine.getOptionValue(GravitinoOptions.ROLE)).thenReturn("admin"); + when(mockCommandLine.getOptionValues(GravitinoOptions.ROLE)).thenReturn(new String[] {"admin"}); when(mockCommandLine.hasOption(GravitinoOptions.PRIVILEGE)).thenReturn(true); when(mockCommandLine.getOptionValues(GravitinoOptions.PRIVILEGE)).thenReturn(privileges); GravitinoCommandLine commandLine = @@ -214,4 +308,23 @@ void testRevokePrivilegesFromRole() { commandLine.handleCommandLine(); verify(mockRevoke).handle(); } + + @Test + void testDeleteRoleCommandWithoutRole() { + Main.useExit = false; + when(mockCommandLine.hasOption(GravitinoOptions.METALAKE)).thenReturn(true); + when(mockCommandLine.getOptionValue(GravitinoOptions.METALAKE)).thenReturn("metalake_demo"); + when(mockCommandLine.hasOption(GravitinoOptions.ROLE)).thenReturn(false); + GravitinoCommandLine commandLine = + spy( + new GravitinoCommandLine( + mockCommandLine, mockOptions, CommandEntities.ROLE, CommandActions.REVOKE)); + + assertThrows(RuntimeException.class, commandLine::handleCommandLine); + verify(commandLine, never()) + .newDeleteRole( + eq(GravitinoCommandLine.DEFAULT_URL), eq(false), eq(false), eq("metalake_demo"), any()); + String output = new String(errContent.toByteArray(), StandardCharsets.UTF_8).trim(); + assertEquals(output, ErrorMessages.MISSING_ROLE); + } }