From 769457c165488fb8059ecb843a1c200f69dce95d Mon Sep 17 00:00:00 2001 From: David Härdeman Date: Tue, 30 Jun 2020 08:54:52 +0200 Subject: Centralize scfg validation and use in minecctl to implement a lint command --- minecctl/minecctl-commands.h | 5 ++ minecctl/minecctl.c | 12 +++- minecctl/misc-commands.c | 24 ++++++++ minecctl/misc-commands.h | 2 + minecproxy/server.c | 141 +++--------------------------------------- shared/config-parser.c | 142 +++++++++++++++++++++++++++++++++++++++++++ shared/config-parser.h | 2 + 7 files changed, 193 insertions(+), 135 deletions(-) diff --git a/minecctl/minecctl-commands.h b/minecctl/minecctl-commands.h index e8bb132..b1729f0 100644 --- a/minecctl/minecctl-commands.h +++ b/minecctl/minecctl-commands.h @@ -4,6 +4,7 @@ enum commands { CMD_INVALID = 0, CMD_LIST, + CMD_LINT, CMD_STATUS, CMD_PING, CMD_STOP, @@ -21,6 +22,10 @@ static struct command_list { .name = "list", .cmd = CMD_LIST, }, + { + .name = "lint", + .cmd = CMD_LINT, + }, { .name = "status", .cmd = CMD_STATUS, diff --git a/minecctl/minecctl.c b/minecctl/minecctl.c index 231b150..69286fc 100644 --- a/minecctl/minecctl.c +++ b/minecctl/minecctl.c @@ -69,7 +69,7 @@ _noreturn_ static void usage(bool no_error) " -m, --mc-address=ADDR connect to Minecraft server at ADDR\n" " (only relevant for some commands, can also\n" " use environment variable MC_ADDRESS)\n" - " -c, --cfgdir=DIR look for server configurations in DIR\n" + " -c, --cfgdir=DIR look for server configuration files in DIR\n" " (default: %s)\n" " -f, --force stop server even if it has players\n" " -v, --verbose enable extra logging\n" @@ -77,6 +77,7 @@ _noreturn_ static void usage(bool no_error) "\n" "Valid commands:\n" " list list known servers\n" + " lint check validity of server configuration files\n" " status [SERVER] show status of SERVER (or all known servers)\n" " ping [SERVER] check if SERVER is running\n" " stop [SERVER] stop SERVER\n" @@ -225,6 +226,13 @@ static void parse_command(struct cfg *cfg, char *const *argv) } cfg->cmd = do_list; break; + case CMD_LINT: + if (*argv) { + error("Too many arguments"); + usage(false); + } + cfg->cmd = do_lint; + break; case CMD_STOPALL: if (*argv) { error("Too many arguments"); @@ -300,7 +308,7 @@ static void parse_cmdline(struct cfg *cfg, int argc, char *const *argv) cfg->cfgdir = DEFAULT_CFG_DIR; - /* FIXME: add lint and debug options */ + /* FIXME: add debug option */ while (true) { int option_index = 0; diff --git a/minecctl/misc-commands.c b/minecctl/misc-commands.c index f20cac5..1268138 100644 --- a/minecctl/misc-commands.c +++ b/minecctl/misc-commands.c @@ -17,6 +17,30 @@ bool do_list(struct cfg *cfg) return true; } +bool do_lint(struct cfg *cfg) +{ + struct server *server; + bool rv = true; + + /* server->scfg.filename check excludes servers created from cmdline */ + list_for_each_entry(server, &cfg->servers, list) { + if (!server->scfg.filename) + continue; + + info("%s", server->name); + + /* FIXME: should return bool */ + server_read_config(cfg, server); + + if (!scfg_validate(&server->scfg)) { + error("%s: invalid", server->name); + rv = false; + } + } + + return rv; +} + bool do_pcount(struct cfg *cfg) { unsigned x, y; diff --git a/minecctl/misc-commands.h b/minecctl/misc-commands.h index 1a4363f..eb03fd9 100644 --- a/minecctl/misc-commands.h +++ b/minecctl/misc-commands.h @@ -3,6 +3,8 @@ bool do_list(struct cfg *cfg); +bool do_lint(struct cfg *cfg); + bool do_pcount(struct cfg *cfg); #endif diff --git a/minecproxy/server.c b/minecproxy/server.c index 81d148b..a2f8332 100644 --- a/minecproxy/server.c +++ b/minecproxy/server.c @@ -17,6 +17,7 @@ #include "server-config.h" #include "server-proxy.h" #include "server-rcon.h" +#include "config-parser.h" #include "idle.h" #include "systemd.h" @@ -422,7 +423,6 @@ bool server_announce(struct server *server, int fd) bool server_commit(struct server *server) { struct saddr *saddr, *tmp; - uint16_t port; int r; assert_return(server && server->name, false); @@ -439,143 +439,19 @@ bool server_commit(struct server *server) } if (!list_empty(&server->scfg.dnslookups)) { - debug(DBG_SRV, "called with pending DNS requests"); - return true; - } - - if (server->scfg.stop_method == SERVER_STOP_METHOD_RCON && - list_empty(&server->scfg.rcons)) { - error("%s: rcon stop method missing rcon address", + debug(DBG_SRV, "%s: called with pending DNS requests", server->name); - return false; - } - - if (server->scfg.stop_method == SERVER_STOP_METHOD_RCON && - !server->scfg.rcon_password) { - error("%s: rcon stop method missing rcon password", - server->name); - return false; - } - - if ((server->scfg.start_method == SERVER_START_METHOD_SYSTEMD || - server->scfg.stop_method == SERVER_STOP_METHOD_SYSTEMD) && - !server->scfg.systemd_service) { - error("%s: systemd start/stop method missing systemd service", - server->name); - return false; - } - - if ((server->scfg.systemd_service && !server->scfg.systemd_obj) || - (!server->scfg.systemd_service && server->scfg.systemd_obj)) { - error("%s: systemd service but no object path or vice versa", - server->name); - return false; - } - - if (server->scfg.idle_timeout > 0 && - server->scfg.stop_method == SERVER_STOP_METHOD_UNDEFINED) { - error("%s: idle_timeout set but missing stop method", - server->name); - return false; + return true; } - switch (server->scfg.type) { - case SERVER_TYPE_ANNOUNCE: - if (server->scfg.announce_port == 0) { - error("%s: missing announce port", server->name); - return false; - } - - if (server->scfg.start_method != SERVER_START_METHOD_UNDEFINED) { - error("%s: can't set start_method for announce server", - server->name); - return false; - } - - if (!list_empty(&server->scfg.locals)) { - error("%s: can't set local addresses for announce server", - server->name); - return false; - } - - if (!list_empty(&server->scfg.remotes)) { - error("%s: can't set remote addresses for announce server", - server->name); - return false; - } - - break; - - case SERVER_TYPE_PROXY: - if (server->scfg.announce_port == 0) { - error("%s: can't set announce port for proxy server", - server->name); - return false; - } - - if (list_empty(&server->scfg.locals)) { - error("%s: missing local addresses for proxy server", - server->name); - return false; - } - - if (list_empty(&server->scfg.remotes)) { - error("%s: missing remote addresses for proxy server", - server->name); - return false; - } - - list_for_each_entry(saddr, &server->scfg.locals, list) { - port = saddr_port(saddr); - - if (port == 0) { - error("%s: invalid local port", server->name); - return false; - } - - if (server->scfg.announce_port == 0) - server->scfg.announce_port = port; - - if (server->scfg.announce_port != port) { - error("%s: multiple local ports", server->name); - return false; - } - } - - if (server->scfg.announce_port == 0) { - error("%s: can't determine which port to announce", - server->name); - return false; - } - - break; - - default: - error("%s: can't determine server type", server->name); + if (!scfg_validate(&server->scfg)) { + debug(DBG_SRV, "%s: config file not valid", server->name); return false; } - if (!server->scfg.pretty_name) { - char *suffix; - - suffix = strrchr(server->name, '.'); - if (!suffix || suffix == server->name) { - error("invalid server name: %s", server->name); - return false; - } - - server->scfg.pretty_name = - xstrndup(server->name, suffix - server->name); - if (!server->scfg.pretty_name) { - error("failed to create display name: %s", - server->name); - return false; - } - } - r = snprintf(server->ann_buf.buf, sizeof(server->ann_buf.buf), "[MOTD]%s[/MOTD][AD]%" PRIu16 "[/AD]", - server->scfg.pretty_name, + server->scfg.pretty_name ? server->scfg.pretty_name : server->name, server->scfg.announce_port); if (r < 1 || r >= sizeof(server->ann_buf.buf)) { error("%s: unable to create announce msg: %i\n", server->name, @@ -584,8 +460,7 @@ bool server_commit(struct server *server) } server->ann_buf.len = r; - /* FIXME: config, dont reread config if server running, make sure fd is - * available before this is called */ + /* FIXME: check interactions with inotify */ server_dump(server); list_for_each_entry_safe(saddr, tmp, &server->scfg.locals, list) { @@ -602,7 +477,7 @@ bool server_commit(struct server *server) server_check_running(server); - debug(DBG_SRV, "success"); + debug(DBG_SRV, "%s: success", server->name); return true; } diff --git a/shared/config-parser.c b/shared/config-parser.c index ee30156..6c2d33d 100644 --- a/shared/config-parser.c +++ b/shared/config-parser.c @@ -221,6 +221,148 @@ static char *systemd_object_path(const char *service) return r; } +/* FIXME: Add a strict argument */ +bool scfg_validate(struct server_config *scfg) +{ + struct saddr *saddr; + uint16_t port; + + if (!scfg) { + error("invalid arguments"); + return false; + } + + if (!scfg->filename) { + error("filename not set"); + return false; + } + + if (!list_empty(&scfg->dnslookups)) { + error("%s: pending DNS requests", + scfg->filename); + return false; + } + + if (scfg->stop_method == SERVER_STOP_METHOD_RCON && + list_empty(&scfg->rcons)) { + error("%s: rcon stop method missing rcon address", + scfg->filename); + return false; + } + + if (scfg->stop_method == SERVER_STOP_METHOD_RCON && + !scfg->rcon_password) { + error("%s: rcon stop method missing rcon password", + scfg->filename); + return false; + } + + if ((scfg->start_method == SERVER_START_METHOD_SYSTEMD || + scfg->stop_method == SERVER_STOP_METHOD_SYSTEMD)) { + if (!scfg->systemd_service || !scfg->systemd_obj) { + error("%s: systemd start/stop method missing " + "systemd service", scfg->filename); + return false; + } else { + if (scfg->systemd_service || scfg->systemd_obj) { + error("%s: systemd service set but not used", + scfg->filename); + return false; + } + } + } + + if (scfg->idle_timeout > 0 && + scfg->stop_method == SERVER_STOP_METHOD_UNDEFINED) { + error("%s: idle_timeout set but missing stop method", + scfg->filename); + return false; + } + + switch (scfg->type) { + case SERVER_TYPE_ANNOUNCE: + if (scfg->announce_port == 0) { + error("%s: missing announce port", scfg->filename); + return false; + } + + if (scfg->start_method != SERVER_START_METHOD_UNDEFINED) { + error("%s: can't set start_method for announce server", + scfg->filename); + return false; + } + + if (scfg->start_exec) { + error("%s: can't set start_exec for announce server", + scfg->filename); + return false; + } + + if (!list_empty(&scfg->locals)) { + error("%s: can't set local addresses for announce server", + scfg->filename); + return false; + } + + if (!list_empty(&scfg->remotes)) { + error("%s: can't set remote addresses for announce server", + scfg->filename); + return false; + } + + break; + + case SERVER_TYPE_PROXY: + if (scfg->announce_port == 0) { + error("%s: can't set announce port for proxy server", + scfg->filename); + return false; + } + + if (list_empty(&scfg->locals)) { + error("%s: missing local addresses for proxy server", + scfg->filename); + return false; + } + + if (list_empty(&scfg->remotes)) { + error("%s: missing remote addresses for proxy server", + scfg->filename); + return false; + } + + list_for_each_entry(saddr, &scfg->locals, list) { + port = saddr_port(saddr); + + if (port == 0) { + error("%s: invalid local port", scfg->filename); + return false; + } + + if (scfg->announce_port == 0) + scfg->announce_port = port; + else if (scfg->announce_port != port) { + error("%s: multiple local ports", scfg->filename); + return false; + } + } + + if (scfg->announce_port == 0) { + error("%s: can't determine which port to announce", + scfg->filename); + return false; + } + + break; + + default: + error("%s: can't determine server type", scfg->filename); + return false; + } + + return true; +} + #define INVALID(fmt, ...) \ do { \ verbose("%s: " fmt, scfg->filename __VA_OPT__(,) __VA_ARGS__); \ diff --git a/shared/config-parser.h b/shared/config-parser.h index 2220221..2990349 100644 --- a/shared/config-parser.h +++ b/shared/config-parser.h @@ -85,6 +85,8 @@ struct cfg_value { }; }; +bool scfg_validate(struct server_config *scfg); + bool scfg_parse(struct server_config *scfg, char *buf, notification_cb_t notification_cb); -- cgit v1.2.3