Skip to content

Commit

Permalink
[agroal#403][agroal#410] Improve error messages for pgagroal-cli
Browse files Browse the repository at this point in the history 
This commit improves the error messages for pgagroal-cli and pgagroal-admin commands by modifying the parse_command function. The parsing step now involves two tables (command_table) defined in each cli.c and admin.c files, which guide the parsing of commands.

Now adding a command with the structure "<command> [subcommand] [arg] [arg] ..." requires the addition of the command enum and its struct in the command_table.
  • Loading branch information
@decarv
decarv committed Mar 9, 2024
1 parent d9f9253 commit 63d4687
Show file tree
Hide file tree
Showing 4 changed files with 828 additions and 435 deletions.
241 changes: 169 additions & 72 deletions src/admin.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,121 @@ static int remove_user(char* users_path, char* username);
static int list_users(char* users_path);
static char* generate_password(int pwd_length);

enum pgagroal_admin_command_code
{
/* Command codes: add commands to the end of this list */
COMMAND_MASTER_KEY = 0,
COMMAND_USER_ADD,
COMMAND_USER_EDIT,
COMMAND_USER_DEL,
COMMAND_USER_LS,

/* Deprecated command codes */
DEPRECATED_COMMAND_ADD_USER,
DEPRECATED_COMMAND_UPDATE_USER,
DEPRECATED_COMMAND_REMOVE_USER,
DEPRECATED_COMMAND_LIST_USERS,

COMMAND_COUNT // ALWAYS LAST, DO NOT REMOVE
};

const struct pgagroal_command command_table[] =
{
[COMMAND_MASTER_KEY] =
{
.command = "master-key",
.subcommand = "",
.accepted_argument_count = {0},
.deprecated = false,
.action = ACTION_MASTER_KEY,
.log_trace = "<master-key>",
},
[COMMAND_USER_ADD] =
{
.command = "user",
.subcommand = "add",
.accepted_argument_count = {0},
.deprecated = false,
.action = ACTION_ADD_USER,
.log_trace = "<user add> [%s]",
},
[COMMAND_USER_EDIT] =
{
.command = "user",
.subcommand = "edit",
.accepted_argument_count = {0},
.deprecated = false,
.action = ACTION_UPDATE_USER,
.log_trace = "<user edit> [%s]",
},
[COMMAND_USER_DEL] =
{
.command = "user",
.subcommand = "del",
.accepted_argument_count = {0},
.deprecated = false,
.action = ACTION_REMOVE_USER,
.log_trace = "<user del> [%s]",
},
[COMMAND_USER_LS] =
{
.command = "user",
.subcommand = "ls",
.accepted_argument_count = {0},
.deprecated = false,
.action = ACTION_LIST_USERS,
.log_trace = "<user ls>",
},
[DEPRECATED_COMMAND_ADD_USER] =
{
.command = "add-user",
.subcommand = "",
.accepted_argument_count = {0},
.deprecated = true,
.action = ACTION_ADD_USER,
.log_trace = "<deprecated: use 'user add'> [%s]",
.deprecated_since_major = 1,
.deprecated_since_minor = 6,
.deprecated_by = "user add",
},
[DEPRECATED_COMMAND_UPDATE_USER] =
{
.command = "update-user",
.subcommand = "",
.accepted_argument_count = {0},
.deprecated = true,
.action = ACTION_UPDATE_USER,
.log_trace = "<deprecated: use 'user edit'> [%s]",
.deprecated_since_major = 1,
.deprecated_since_minor = 6,
.deprecated_by = "user edit",
},
[DEPRECATED_COMMAND_REMOVE_USER] =
{
.command = "remove-user",
.subcommand = "",
.accepted_argument_count = {0},
.deprecated = true,
.action = ACTION_REMOVE_USER,
.log_trace = "<deprecated: use 'user del'>",
.deprecated_since_major = 1,
.deprecated_since_minor = 6,
.deprecated_by = "user del",
},
[DEPRECATED_COMMAND_LIST_USERS] =
{
.command = "list-users",
.subcommand = "",
.accepted_argument_count = {0},
.deprecated = true,
.action = ACTION_LIST_USERS,
.log_trace = "<deprecated: use 'user ls'>",
.deprecated_since_major = 1,
.deprecated_since_minor = 6,
.deprecated_by = "user ls",
},
};

static void
version(void)
{
Expand Down Expand Up @@ -117,7 +232,14 @@ main(int argc, char** argv)
bool generate_pwd = false;
int pwd_length = DEFAULT_PASSWORD_LENGTH;
int option_index = 0;
int32_t action = ACTION_UNKNOWN;
struct pgagroal_parsed_command cmd =
{
.args = {0},
.command_code = COMMAND_UNKNOWN,
.arg_count = -1,
.action = ACTION_UNKNOWN,
.error_message = {0},
};

while (1)
{
Expand Down Expand Up @@ -174,96 +296,71 @@ main(int argc, char** argv)
errx(1, "Using the root account is not allowed");
}

if (argc > 0)
if (!parse_command(argc, argv, optind, &cmd, command_table, COMMAND_COUNT))
{
if (!strcmp("master-key", argv[argc - 1]))
{
action = ACTION_MASTER_KEY;
}
else if (parse_command_simple(argc, argv, optind, "user", "add")
|| parse_deprecated_command(argc, argv, optind, "add-user", NULL, "user add", 1, 6))
{
action = ACTION_ADD_USER;
}
else if (parse_command_simple(argc, argv, optind, "user", "edit")
|| parse_deprecated_command(argc, argv, optind, "update-user", NULL, "user edit", 1, 6))
{
action = ACTION_UPDATE_USER;
}
else if (parse_command_simple(argc, argv, optind, "user", "del")
|| parse_deprecated_command(argc, argv, optind, "remove-user", NULL, "user del", 1, 6))
{
action = ACTION_REMOVE_USER;
}
else if (parse_command_simple(argc, argv, optind, "user", "ls")
|| parse_deprecated_command(argc, argv, optind, "list-users", NULL, "user ls", 1, 6))
{
action = ACTION_LIST_USERS;
}
goto error;
}

// exit immediatly if the action is not understood!
if (action == ACTION_UNKNOWN)
{
warnx("unknown command or subcommand <%s>", argv[optind]);
usage();
goto error;
}
// if here, the action is understood, but we need
// the file to oeprate onto!
// Therefore, if the user did not specify any config file
// the default one is used. Note that in the case of ACTION_MASTER_KEY
// there is no need for the file_path to be set, so setting to a default
// value does nothing.
// Setting the file also means we don't have to check against the file_path value.
if (file_path == NULL)
{
file_path = PGAGROAL_DEFAULT_USERS_FILE;
}

// if here, the action is understood, but we need
// the file to oeprate onto!
// Therefore, if the user did not specify any config file
// the default one is used. Note that in the case of ACTION_MASTER_KEY
// there is no need for the file_path to be set, so setting to a default
// value does nothing.
// Setting the file also means we don't have to check against the file_path value.
if (file_path == NULL)
if (cmd.action == ACTION_MASTER_KEY)
{
if (master_key(password, generate_pwd, pwd_length))
{
file_path = PGAGROAL_DEFAULT_USERS_FILE;
errx(1, "Error for master key");
}

if (action == ACTION_MASTER_KEY)
}
else if (cmd.action == ACTION_ADD_USER)
{
if (add_user(file_path, username, password, generate_pwd, pwd_length))
{
if (master_key(password, generate_pwd, pwd_length))
{
errx(1, "Error for master key");
}
errx(1, "Error for <user add>");
}
else if (action == ACTION_ADD_USER)
}
else if (cmd.action == ACTION_UPDATE_USER)
{
if (update_user(file_path, username, password, generate_pwd, pwd_length))
{
if (add_user(file_path, username, password, generate_pwd, pwd_length))
{
errx(1, "Error for <user add>");
}
errx(1, "Error for <user edit>");
}
else if (action == ACTION_UPDATE_USER)
}
else if (cmd.action == ACTION_REMOVE_USER)
{

if (remove_user(file_path, username))
{
if (update_user(file_path, username, password, generate_pwd, pwd_length))
{
errx(1, "Error for <user edit>");
}
errx(1, "Error for <user del>");
}
else if (action == ACTION_REMOVE_USER)
{
}
else if (cmd.action == ACTION_LIST_USERS)
{

if (remove_user(file_path, username))
{
errx(1, "Error for <user del>");
}
}
else if (action == ACTION_LIST_USERS)
if (list_users(file_path))
{

if (list_users(file_path))
{
errx(1, "Error for <user ls>");
}

errx(1, "Error for <user ls>");
}

}

exit(0);

error:
if (cmd.error_message[0] != 0)
{
printf("%s", cmd.error_message);
usage();
}

exit(1);
}

Expand Down
Loading

0 comments on commit 63d4687

Please sign in to comment.