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.

Sign up for GitHub

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

[#5985] improvement(CLI): Fix role command that supports handling multiple values #5988

Merged
merged 4 commits into from
Dec 28, 2024
Merged

[#5985] improvement(CLI): Fix role command that supports handling multiple values #5988

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3948e02
[#5985] improvement(CLI): Fix role command that supports handling mul…
Abyss-lord Dec 25, 2024
b96f648
[#5985] improvement(CLI): Fix role command that supports handling mul…
Abyss-lord Dec 25, 2024
22d34a1
[#5985] improvement(CLI): Fix role command that supports handling mul…
Abyss-lord Dec 26, 2024
3610008
Merge branch 'main' into fix-role
Abyss-lord Dec 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions clients/cli/src/main/java/org/apache/gravitino/cli/ErrorMessages.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 METALAKE_EXISTS = "Metalake already exists.";
public static final String CATALOG_EXISTS = "Catalog already exists.";
public static final String SCHEMA_EXISTS = "Schema already exists.";
Expand All @@ -42,6 +43,8 @@ public class ErrorMessages {
public static final String UNKNOWN_TAG = "Unknown tag.";
public static final String MULTIPLE_TAG_COMMAND_ERROR =
"Error: The current command only supports one --tag option.";
public static final String MULTIPLE_ROLE_COMMAND_ERROR =
"Error: The current command only supports one --role option.";
public static final String TAG_EXISTS = "Tag already exists.";
public static final String UNKNOWN_COLUMN = "Unknown column.";
public static final String COLUMN_EXISTS = "Column already exists.";
Expand Down
30 changes: 23 additions & 7 deletions clients/cli/src/main/java/org/apache/gravitino/cli/GravitinoCommandLine.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -733,17 +733,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);
}
Abyss-lord marked this conversation as resolved.
Show resolved Hide resolved

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)).handle();
} else {
newRoleDetails(url, ignore, metalake, role).handle();
newRoleDetails(url, ignore, metalake, getOneRole(roles)).handle();
}
break;

Expand All @@ -752,20 +761,22 @@ 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), name, privileges)
.handle();
break;

case CommandActions.REVOKE:
newRevokePrivilegesFromRole(url, ignore, metalake, role, name, privileges).handle();
newRevokePrivilegesFromRole(url, ignore, metalake, getOneRole(roles), name, privileges)
.handle();
break;

default:
Expand All @@ -775,6 +786,11 @@ protected void handleRoleCommand() {
}
}

private String getOneRole(String[] roles) {
Preconditions.checkArgument(roles.length <= 1, ErrorMessages.MULTIPLE_ROLE_COMMAND_ERROR);
Abyss-lord marked this conversation as resolved.
Show resolved Hide resolved
return roles[0];
}

/**
* Handles the command execution for Columns based on command type and the command line options.
*/
Expand Down
8 changes: 4 additions & 4 deletions clients/cli/src/main/java/org/apache/gravitino/cli/TestableCommandLine.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
15 changes: 9 additions & 6 deletions clients/cli/src/main/java/org/apache/gravitino/cli/commands/CreateRole.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -27,28 +28,30 @@

public class CreateRole extends Command {
protected String metalake;
protected String role;
protected String[] roles;

/**
* Create a new role.
*
* @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. */
@Override
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) {
Expand All @@ -57,6 +60,6 @@ public void handle() {
exitWithError(exp.getMessage());
}

System.out.println(role + " created");
System.out.println(Joiner.on(", ").join(roles) + " created");
}
}
19 changes: 11 additions & 8 deletions clients/cli/src/main/java/org/apache/gravitino/cli/commands/DeleteRole.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

package org.apache.gravitino.cli.commands;

import com.google.common.base.Joiner;
import org.apache.gravitino.cli.AreYouSure;
import org.apache.gravitino.cli.ErrorMessages;
import org.apache.gravitino.client.GravitinoClient;
Expand All @@ -28,7 +29,7 @@
public class DeleteRole extends Command {

protected String metalake;
protected String role;
protected String[] roles;
protected boolean force;

/**
Expand All @@ -38,28 +39,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;
boolean deleted = true;
Abyss-lord marked this conversation as resolved.
Show resolved Hide resolved

if (!AreYouSure.really(force)) {
return;
}

try {
GravitinoClient client = buildClient(metalake);
deleted = client.deleteRole(role);
for (String role : roles) {
deleted &= client.deleteRole(role);
}
} catch (NoSuchMetalakeException err) {
exitWithError(ErrorMessages.UNKNOWN_METALAKE);
} catch (NoSuchRoleException err) {
Expand All @@ -69,9 +72,9 @@ public void handle() {
}

if (deleted) {
System.out.println(role + " deleted.");
System.out.println(Joiner.on(", ").join(roles) + " deleted.");
} else {
System.out.println(role + " not deleted.");
System.out.println(Joiner.on(", ").join(roles) + " not deleted.");
}
}
}
Loading
Loading