diff --git a/doc/appendices/command-line/traffic_ctl.en.rst b/doc/appendices/command-line/traffic_ctl.en.rst index 67ba5801349..d1f108cb066 100644 --- a/doc/appendices/command-line/traffic_ctl.en.rst +++ b/doc/appendices/command-line/traffic_ctl.en.rst @@ -429,7 +429,8 @@ Display the current value of a configuration record. - ``directive_key`` — the directive name understood by that handler - ``value`` — the directive value (always passed as a string on the wire) - Multiple directives are passed as space-separated values after a single ``-D``: + Multiple directives are passed as space-separated values after a single ``-D``, or by + repeating the option. Both spellings accumulate, and they may be mixed: .. code-block:: bash @@ -442,6 +443,12 @@ Display the current value of a configuration record. # Directives for different handlers in the same reload $ traffic_ctl config reload -D myconfig.id=foo sni.fqdn=example.com + # The same, written as a repeated option + $ traffic_ctl config reload -D myconfig.id=foo -D sni.fqdn=example.com + + # Repeating it is how a directive is written after another option + $ traffic_ctl config reload -D myconfig.id=foo --monitor -D sni.fqdn=example.com + On the wire, ``-D myconfig.id=foo`` translates to: .. code-block:: json @@ -456,11 +463,23 @@ Display the current value of a configuration record. .. note:: - ``-D`` uses variable-argument parsing and must appear as the **last option** - on the command line. Any flags placed after ``-D`` will be consumed as directive - values. ``-D`` and ``-d`` cannot be combined in the same invocation due to this - same constraint. Use ``-d`` with full YAML when you need both directives and - inline content in a single reload request. + ``-D`` accepts values until the next option or the end of the command line, so it + may appear anywhere among the options and can be combined with ``-d`` — directives + and inline content merge under the same config key: + + .. code-block:: bash + + $ traffic_ctl config reload -D myconfig.id=foo --monitor + $ traffic_ctl config reload -D myconfig.id=foo -d 'myconfig: {rules: [a]}' + + To pass a directive value that begins with ``-``, place ``--`` before it. Option + recognition then stays off for the rest of the line, so every remaining token becomes + a directive value and any option written afterwards is swallowed. Use + ``--directive=-value`` instead when options still have to follow: + + .. code-block:: bash + + $ traffic_ctl config reload --directive=-weird.id=foo --monitor .. note:: @@ -563,6 +582,42 @@ Display the current value of a configuration record. Specifying the file name is not needed as `traffic_ctl` will try to use the build(or the runroot if used) information to figure out the path to the `records.yaml`. + ``-c`` accepts at most one file name, so it may be written before or after the record + names: + + .. code-block:: bash + + $ traffic_ctl config get -c records.yaml proxy.config.diags.debug.enabled + $ traffic_ctl config get proxy.config.diags.debug.enabled -c records.yaml + $ traffic_ctl config get --cold=records.yaml proxy.config.diags.debug.enabled + + When no file name is given, write ``-c`` last, or use the ``--cold=`` form for the + explicit file. A bare ``-c`` followed by a record name takes the record as the file name, + which leaves the command short of its own arguments. Each command reports this in terms of + what it was left without: + + .. code-block:: bash + + $ traffic_ctl config get proxy.config.diags.debug.enabled -c # default records.yaml + $ traffic_ctl config get -c proxy.config.diags.debug.enabled + Error: at least one argument expected by get + + $ traffic_ctl config set proxy.config.diags.debug.enabled 1 -c # default records.yaml + $ traffic_ctl config set -c proxy.config.diags.debug.enabled 1 + Error: 2 argument(s) expected by set + + An empty file name is not a file name, so it is reported rather than taken as a request for + the default file. This matters when the name comes from a variable that is unset, where + reading or writing the live :file:`records.yaml` is unlikely to be what was meant: + + .. code-block:: bash + + $ traffic_ctl config set -c "" proxy.config.diags.debug.enabled 1 + Error: missing argument for '-c' + + ``-c`` is also given at most once, so repeating it is a usage error rather than the last + file name silently winning. + If the file exists and is empty a new document will be created. If a file does not exist, an attempt to create a new file will be done. This option(only for the config file changes) lets you use the prefix `proxy.config.` or `ts.` for variable names, either would work. diff --git a/doc/developer-guide/internal-libraries/ArgParser.en.rst b/doc/developer-guide/internal-libraries/ArgParser.en.rst index c15a552ff5a..cdaf5cbe4f9 100644 --- a/doc/developer-guide/internal-libraries/ArgParser.en.rst +++ b/doc/developer-guide/internal-libraries/ArgParser.en.rst @@ -104,6 +104,48 @@ To add options to the parser or current command: This function call returns the new :class:`Option` instance. (0 is also number of arguments expected) +.. Note:: + + For options, the number of arguments may also be one of the following, which mirror the + ``nargs`` values of Python's ``argparse``: + + ================================ ======================================================= + Value Meaning + ================================ ======================================================= + ``AT_MOST_ONE_ARG_N`` Zero or one value (``argparse`` ``nargs='?'``) + ``MORE_THAN_ZERO_ARG_N`` Zero or more values (``argparse`` ``nargs='*'``) + ``MORE_THAN_ONE_ARG_N`` One or more values (``argparse`` ``nargs='+'``) + ================================ ======================================================= + + An option taking a variable number of values stops collecting when it reaches a token + naming another option of the same command, so options written afterwards keep their own + arguments. Use ``AT_MOST_ONE_ARG_N`` rather than ``MORE_THAN_ZERO_ARG_N`` for an option + whose value is optional, otherwise it also consumes the positional arguments of its + command. + + A token naming another option is not a value for a fixed number of arguments either. An + option written where a value is expected leaves the value missing, which is reported as a + usage error rather than the option being consumed and applied as the value. + + Because collection stops at the following option, an option taking an unbounded number of + values may be written more than once, and the occurrences accumulate. This matches the + ``--option=value`` form, which has always appended. An option taking a fixed number of + values keeps its last-one-wins behaviour instead, and ``AT_MOST_ONE_ARG_N`` reports a + repetition as a usage error since it permits only one value in total. + + An empty token is not a value for ``AT_MOST_ONE_ARG_N``. It is reported as a missing + argument rather than read as the option having been given without one, so a value taken + from an unset variable cannot silently select the declared default. + + A ``--`` token stops option recognition for the values being collected, which is how a + value beginning with ``-`` is passed. Note this differs from the POSIX ``--``: it does + not end the value list nor force the remainder to be positional arguments. + + Option recognition stays off for the rest of that collection, so for a variable number of + values every remaining token becomes a value and no later option is recognized. Use the + ``--option=value`` form instead when options still have to follow a value that begins with + ``-``. + We can also use the following chained way to add subcommand or option: .. code-block:: cpp diff --git a/include/tscore/ArgParser.h b/include/tscore/ArgParser.h index fdb6086eba0..1b29504a83f 100644 --- a/include/tscore/ArgParser.h +++ b/include/tscore/ArgParser.h @@ -34,10 +34,23 @@ constexpr unsigned MORE_THAN_ZERO_ARG_N = ~0; // more than one arguments constexpr unsigned MORE_THAN_ONE_ARG_N = ~0 - 1; +// zero or one argument +constexpr unsigned AT_MOST_ONE_ARG_N = ~0 - 2; // customizable indent for help message constexpr int INDENT_ONE = 32; constexpr int INDENT_TWO = 46; +/** Whether @a arg_num asks for a variable rather than a fixed number of values. + + Use this in preference to comparing against the sentinels, so that adding another + variable arity does not silently leave a sentinel being treated as a literal count. + */ +constexpr bool +is_variable_arg_num(unsigned arg_num) +{ + return arg_num == MORE_THAN_ZERO_ARG_N || arg_num == MORE_THAN_ONE_ARG_N || arg_num == AT_MOST_ONE_ARG_N; +} + namespace ts { using AP_StrVec = std::vector; @@ -89,6 +102,12 @@ class Arguments ~Arguments(); ArgumentData get(std::string const &name); + /** Whether @a name has an entry. + + @return @c true when the command or option has been parsed. Unlike get(), the called + flag is left alone, so this can be asked while parsing. + */ + bool has(std::string const &name) const noexcept; void append(std::string const &key, ArgumentData const &value); // Append value to the arg to the map of key @@ -222,6 +241,10 @@ class ArgParser void version_message() const; // Helper method for parse() void append_option_data(Arguments &ret, AP_StrVec &args, int index); + // Helper method to collect the values of an option or command into @a ret + std::string handle_args(Arguments &ret, AP_StrVec &args, std::string const &name, unsigned arg_num, unsigned &index) const; + // Whether @a token names an option registered on this command + bool is_registered_option(std::string const &token) const; // Helper method to validate mutually exclusive groups void validate_mutex_groups(Arguments &ret) const; // Helper method to validate option dependencies diff --git a/src/traffic_ctl/CtrlCommands.cc b/src/traffic_ctl/CtrlCommands.cc index 88d29637ece..c4d539251bc 100644 --- a/src/traffic_ctl/CtrlCommands.cc +++ b/src/traffic_ctl/CtrlCommands.cc @@ -555,6 +555,14 @@ ConfigCommand::config_reload() _printer->write_output(""); } + // Without content the request would silently degrade to a full reload of every handler, + // which is the opposite of the scoped reload the operator asked for. + if (data_args && data_args.size() == 0) { + _printer->write_output("Error: --data (-d) requires content: @file, @- or a YAML string"); + App_Exit_Status_Code = CTRL_EX_ERROR; + return; + } + // Parse inline config data if provided (supports multiple -d arguments) YAML::Node configs; for (auto const &data_arg : data_args) { @@ -587,17 +595,15 @@ ConfigCommand::config_reload() // Parse --directive (-D) arguments into configs[key]["_reload"][directive] = value auto dir_args = get_parsed_arguments()->get("directive"); + if (dir_args && dir_args.size() == 0) { + _printer->write_output("Error: --directive (-D) requires at least one config_key.directive_key=value"); + App_Exit_Status_Code = CTRL_EX_ERROR; + return; + } for (auto const &dir : dir_args) { if (dir.empty()) { continue; } - if (dir[0] == '-') { - _printer->write_output("Error: '" + dir + - "' looks like a flag, not a directive. " - "Place -D as the last option on the command line."); - App_Exit_Status_Code = CTRL_EX_ERROR; - return; - } std::string err; if (!parse_directive(dir, configs, err)) { _printer->write_output("Error: " + err); diff --git a/src/traffic_ctl/traffic_ctl.cc b/src/traffic_ctl/traffic_ctl.cc index 1bda6e8521d..c077b130934 100644 --- a/src/traffic_ctl/traffic_ctl.cc +++ b/src/traffic_ctl/traffic_ctl.cc @@ -118,7 +118,7 @@ main([[maybe_unused]] int argc, const char **argv) .add_example_usage("traffic_ctl config get [OPTIONS] RECORD [RECORD ...]") .add_option("--cold", "-c", "Save the value in a configuration file. This does not save the value in TS. Local file change only", - "TS_RECORD_YAML", MORE_THAN_ZERO_ARG_N) + "TS_RECORD_YAML", AT_MOST_ONE_ARG_N) .add_option("--records", "", "Emit output in YAML format") .add_option("--default", "", "Include default value"); config_command.add_command("match", "Get configuration matching a regular expression", "", MORE_THAN_ONE_ARG_N, Command_Execute) @@ -186,7 +186,7 @@ main([[maybe_unused]] int argc, const char **argv) config_command.add_command("set", "Set a configuration value", "", 2, Command_Execute) .add_option("--cold", "-c", "Save the value in a configuration file. This does not save the value in TS. Local file change only", - "TS_RECORD_YAML", MORE_THAN_ZERO_ARG_N) + "TS_RECORD_YAML", AT_MOST_ONE_ARG_N) .add_option("--update", "-u", "Update a configuration value. [only relevant if --cold set]") .add_option( "--type", "-t", diff --git a/src/tscore/ArgParser.cc b/src/tscore/ArgParser.cc index 90c3c2f5e43..eff0c42e9ec 100644 --- a/src/tscore/ArgParser.cc +++ b/src/tscore/ArgParser.cc @@ -195,6 +195,9 @@ ArgParser::parse(const char **argv) if (!default_command.empty()) { args = _argv; args.insert(args.begin() + 1, default_command); + // The pass that failed may have collected options before it gave up. Those values would + // now accumulate on top of the ones the retry collects rather than be replaced. + ret = Arguments{}; _top_level_command.parse(ret, args); } }; @@ -418,6 +421,8 @@ ArgParser::Command::output_option() const return {" [ ...]"}; } else if (num == MORE_THAN_ONE_ARG_N) { return {" ..."}; + } else if (num == AT_MOST_ONE_ARG_N) { + return {" []"}; } else { return " ... "; } @@ -518,34 +523,113 @@ ArgParser::Command::output_option() const } } +bool +ArgParser::Command::is_registered_option(std::string const &token) const +{ + if (_option_list.find(token) != _option_list.end() || _option_map.find(token) != _option_map.end()) { + return true; + } + // The --option=value form. + if (token.size() > 2 && token[0] == '-' && token[1] == '-') { + if (auto const pos = token.find_first_of('='); pos != std::string::npos) { + return _option_list.find(token.substr(0, pos)) != _option_list.end(); + } + } + return false; +} + // helper method to handle the arguments and put them nicely in arguments // can be switched to ts::errata -static std::string -handle_args(Arguments &ret, AP_StrVec &args, std::string const &name, unsigned arg_num, unsigned &index) +std::string +ArgParser::Command::handle_args(Arguments &ret, AP_StrVec &args, std::string const &name, unsigned arg_num, unsigned &index) const { - ArgumentData data; - ret.append(name, data); + // A repeated option taking an unbounded number of values accumulates, as the --option=value + // form always has, so an entry already written by this pass keeps the values it collected. A + // fixed arity option keeps its last-one-wins behaviour, which is a separate concern. + bool const accumulates = MORE_THAN_ZERO_ARG_N == arg_num || MORE_THAN_ONE_ARG_N == arg_num; + + if (!accumulates || !ret.has(name)) { + ArgumentData data; + ret.append(name, data); + } // handle the args - if (arg_num == MORE_THAN_ZERO_ARG_N || arg_num == MORE_THAN_ONE_ARG_N) { - // infinite arguments - if (arg_num == MORE_THAN_ONE_ARG_N && args.size() <= index + 1) { - return "at least one argument expected by " + name; + if (arg_num == AT_MOST_ONE_ARG_N) { + // Zero or one value. A value is taken only when the following token does not name + // another option of this command, which leaves this command's positional arguments + // in place. A "--" token makes whatever follows it a value rather than an option. + unsigned j{index + 1}; + bool takes_value{false}; + + if (j < args.size()) { + if (args[j] == "--") { + ++j; + takes_value = j < args.size(); + } else { + takes_value = !is_registered_option(args[j]); + } } - for (unsigned j = index + 1; j < args.size(); j++) { + if (takes_value) { + ret.append_arg(name, args[j]); + ++j; + } + args.erase(args.begin() + index, args.begin() + j); + index -= 1; + return ""; + } + if (arg_num == MORE_THAN_ZERO_ARG_N || arg_num == MORE_THAN_ONE_ARG_N) { + // Variable number of arguments. Stop collecting at a token that names another option of this + // command, so options written afterwards keep their own values. Every other token is taken as + // a value, including a positional argument of the command, which is why an option whose value + // is optional wants AT_MOST_ONE_ARG_N rather than MORE_THAN_ZERO_ARG_N. A "--" token ends + // option recognition, which is how a value that starts with '-' can be passed. + unsigned j{index + 1}; + unsigned collected{0}; + bool recognize_options{true}; + + for (; j < args.size(); j++) { + if (recognize_options) { + if (args[j] == "--") { + recognize_options = false; + continue; + } + if (is_registered_option(args[j])) { + break; + } + } ret.append_arg(name, args[j]); + ++collected; } - args.erase(args.begin() + index, args.end()); + if (arg_num == MORE_THAN_ONE_ARG_N && collected == 0) { + return "at least one argument expected by " + name; + } + args.erase(args.begin() + index, args.begin() + j); + index -= 1; return ""; } - // finite number of argument handling - for (unsigned j = 0; j < arg_num; j++) { - if (args.size() < index + j + 2 || args[index + j + 1].empty()) { + // Fixed number of arguments. A token naming another option of this command is not a value, so + // the missing value is reported rather than the following option being consumed as one. A "--" + // token ends option recognition, which is how a value that starts with '-' is passed. + unsigned j{index + 1}; + bool recognize_options{true}; + + for (unsigned collected{0}; collected < arg_num; ++j) { + if (j >= args.size() || args[j].empty()) { return std::to_string(arg_num) + " argument(s) expected by " + name; } - ret.append_arg(name, args[index + j + 1]); + if (recognize_options) { + if (args[j] == "--") { + recognize_options = false; + continue; + } + if (is_registered_option(args[j])) { + return std::to_string(arg_num) + " argument(s) expected by " + name; + } + } + ret.append_arg(name, args[j]); + ++collected; } // erase the used arguments and append the data to the return structure - args.erase(args.begin() + index, args.begin() + index + arg_num + 1); + args.erase(args.begin() + index, args.begin() + j); index -= 1; return ""; } @@ -658,7 +742,7 @@ ArgParser::Command::append_option_data(Arguments &ret, AP_StrVec &args, int inde if (args[i][0] == '-' && args[i][1] == '-' && args[i].find('=') != std::string::npos) { // deal with --args= std::string option_name = args[i].substr(0, args[i].find_first_of('=')); - std::string value = args[i].substr(args[i].find_last_of('=') + 1); + std::string value = args[i].substr(args[i].find_first_of('=') + 1); if (value.empty()) { help_message("missing argument for '" + option_name + "'"); } @@ -705,6 +789,16 @@ ArgParser::Command::append_option_data(Arguments &ret, AP_StrVec &args, int inde } else { cur_option = _option_list.at(short_it->second); } + // Counted for the same repetition check as the --option=value form, so that an option + // taking at most one value cannot be given more by mixing the two spellings. + if (cur_option.arg_num == AT_MOST_ONE_ARG_N) { + check_map[cur_option.long_option] += 1; + // An empty token is not a value, and the --option=value spelling already refuses one, + // so refuse it here rather than silently falling back to the declared default. + if (i + 1 < args.size() && args[i + 1].empty()) { + help_message("missing argument for '" + args[i] + "'"); + } + } // handle the arguments std::string err = handle_args(ret, args, cur_option.key, cur_option.arg_num, i); if (!err.empty()) { @@ -720,9 +814,15 @@ ArgParser::Command::append_option_data(Arguments &ret, AP_StrVec &args, int inde } // check for wrong number of arguments for --arg=... for (const auto &it : check_map) { - unsigned num = _option_list.at(it.first).arg_num; - if (num != it.second && num < MORE_THAN_ONE_ARG_N) { - help_message(std::to_string(_option_list.at(it.first).arg_num) + " arguments expected by " + it.first); + unsigned const num = _option_list.at(it.first).arg_num; + if (num == AT_MOST_ONE_ARG_N) { + // At most one, so a repeated option is as wrong as a repeated fixed arity one, which + // is_variable_arg_num() would otherwise wave through. + if (it.second > 1) { + help_message("at most one argument expected by " + it.first); + } + } else if (num != it.second && !is_variable_arg_num(num)) { + help_message(std::to_string(num) + " arguments expected by " + it.first); } } } @@ -841,6 +941,12 @@ Arguments::get(std::string const &name) return ArgumentData(); } +bool +Arguments::has(std::string const &name) const noexcept +{ + return _data_map.find(name) != _data_map.end(); +} + void Arguments::append(std::string const &key, ArgumentData const &value) { diff --git a/src/tscore/unit_tests/test_ArgParser.cc b/src/tscore/unit_tests/test_ArgParser.cc index 0a32502e964..8803fa2eff1 100644 --- a/src/tscore/unit_tests/test_ArgParser.cc +++ b/src/tscore/unit_tests/test_ArgParser.cc @@ -217,3 +217,271 @@ TEST_CASE("with_required does not trigger on default values", "[parse]") REQUIRE(parsed.get("threshold").value() == "300"); REQUIRE(parsed.get("verbose") == true); } + +TEST_CASE("Variable argument option stops at a following option", "[parse]") +{ + ts::ArgParser parser; + parser.add_global_usage("test_prog [OPTIONS]"); + + ts::ArgParser::Command &cmd = parser.add_command("reload", "reload configs"); + cmd.add_option("--directive", "-D", "reload directives", "", MORE_THAN_ZERO_ARG_N, ""); + cmd.add_option("--token", "-t", "a token", "", 1, ""); + cmd.add_option("--monitor", "-m", "monitor progress"); + + // A flag after a variable argument option is not swallowed as a value. + const char *argv1[] = {"test_prog", "reload", "-D", "a.id=1", "b.id=2", "-m", nullptr}; + ts::Arguments parsed = parser.parse(argv1); + REQUIRE(parsed.get("directive").size() == 2); + REQUIRE(parsed.get("directive")[0] == "a.id=1"); + REQUIRE(parsed.get("directive")[1] == "b.id=2"); + REQUIRE(parsed.get("monitor") == true); + + // A following option keeps its own argument. + const char *argv2[] = {"test_prog", "reload", "-D", "a.id=1", "-t", "my_token", nullptr}; + parsed = parser.parse(argv2); + REQUIRE(parsed.get("directive").size() == 1); + REQUIRE(parsed.get("directive")[0] == "a.id=1"); + REQUIRE(parsed.get("token").value() == "my_token"); + + // The long form of the following option is recognized too. + const char *argv3[] = {"test_prog", "reload", "-D", "a.id=1", "--monitor", nullptr}; + parsed = parser.parse(argv3); + REQUIRE(parsed.get("directive").size() == 1); + REQUIRE(parsed.get("monitor") == true); + + // So is its --option=value form. + const char *argv4[] = {"test_prog", "reload", "-D", "a.id=1", "--token=my_token", nullptr}; + parsed = parser.parse(argv4); + REQUIRE(parsed.get("directive").size() == 1); + REQUIRE(parsed.get("token").value() == "my_token"); +} + +TEST_CASE("Double dash ends option recognition for variable argument options", "[parse]") +{ + ts::ArgParser parser; + parser.add_global_usage("test_prog [OPTIONS]"); + + ts::ArgParser::Command &cmd = parser.add_command("reload", "reload configs"); + cmd.add_option("--directive", "-D", "reload directives", "", MORE_THAN_ZERO_ARG_N, ""); + cmd.add_option("--monitor", "-m", "monitor progress"); + + // After "--" a token that looks like an option is taken as a value instead. + const char *argv[] = {"test_prog", "reload", "-D", "--", "-m", "a.id=1", nullptr}; + ts::Arguments parsed = parser.parse(argv); + REQUIRE(parsed.get("directive").size() == 2); + REQUIRE(parsed.get("directive")[0] == "-m"); + REQUIRE(parsed.get("directive")[1] == "a.id=1"); + REQUIRE(parsed.get("monitor") == false); +} + +TEST_CASE("Option value keeps embedded equal signs", "[parse]") +{ + ts::ArgParser parser; + parser.add_global_usage("test_prog [OPTIONS]"); + + ts::ArgParser::Command &cmd = parser.add_command("reload", "reload configs"); + cmd.add_option("--directive", "-D", "reload directives", "", MORE_THAN_ZERO_ARG_N, ""); + + // Only the first '=' separates the option from its value. + const char *argv[] = {"test_prog", "reload", "--directive=ip_allow.id=foo", nullptr}; + ts::Arguments parsed = parser.parse(argv); + REQUIRE(parsed.get("directive").size() == 1); + REQUIRE(parsed.get("directive")[0] == "ip_allow.id=foo"); +} + +TEST_CASE("An option taking at most one argument leaves the positional arguments alone", "[parse]") +{ + ts::ArgParser parser; + parser.add_global_usage("test_prog [OPTIONS]"); + + // Mirrors "traffic_ctl config get [--cold [FILE]] RECORD [RECORD ...]". + ts::ArgParser::Command &cmd = parser.add_command("get", "get values", "", MORE_THAN_ONE_ARG_N, nullptr); + cmd.add_option("--cold", "-c", "read from a file", "", AT_MOST_ONE_ARG_N); + cmd.add_option("--records", "", "yaml output"); + + // The option takes its single value and stops, so the command keeps its own arguments. + const char *argv1[] = {"test_prog", "get", "-c", "records.yaml", "proxy.config.x", nullptr}; + ts::Arguments parsed = parser.parse(argv1); + REQUIRE(parsed.get("cold").value() == "records.yaml"); + REQUIRE(parsed.get("get").size() == 1); + REQUIRE(parsed.get("get")[0] == "proxy.config.x"); + + // Several positional arguments are unaffected. + const char *argv2[] = {"test_prog", "get", "-c", "records.yaml", "proxy.config.x", "proxy.config.y", nullptr}; + parsed = parser.parse(argv2); + REQUIRE(parsed.get("cold").value() == "records.yaml"); + REQUIRE(parsed.get("get").size() == 2); + REQUIRE(parsed.get("get")[1] == "proxy.config.y"); + + // Trailing placement keeps working. + const char *argv3[] = {"test_prog", "get", "proxy.config.x", "-c", "records.yaml", nullptr}; + parsed = parser.parse(argv3); + REQUIRE(parsed.get("cold").value() == "records.yaml"); + REQUIRE(parsed.get("get").size() == 1); + + // The --option=value form is not mistaken for a fixed arity mismatch. + const char *argv4[] = {"test_prog", "get", "--cold=records.yaml", "proxy.config.x", nullptr}; + parsed = parser.parse(argv4); + REQUIRE(parsed.get("cold").value() == "records.yaml"); + REQUIRE(parsed.get("get").size() == 1); +} + +TEST_CASE("An option taking at most one argument accepts no value at all", "[parse]") +{ + ts::ArgParser parser; + parser.add_global_usage("test_prog [OPTIONS]"); + + ts::ArgParser::Command &cmd = parser.add_command("get", "get values", "", MORE_THAN_ONE_ARG_N, nullptr); + cmd.add_option("--cold", "-c", "read from a file", "", AT_MOST_ONE_ARG_N); + cmd.add_option("--records", "", "yaml output"); + + // Called with no value, so the caller falls back to its own default. + const char *argv1[] = {"test_prog", "get", "proxy.config.x", "-c", nullptr}; + ts::Arguments parsed = parser.parse(argv1); + REQUIRE(parsed.get("cold") == true); + REQUIRE(parsed.get("cold").size() == 0); + REQUIRE(parsed.get("cold").value().empty()); + REQUIRE(parsed.get("get").size() == 1); + + // A following option is never taken as the value. + const char *argv2[] = {"test_prog", "get", "-c", "--records", "proxy.config.x", nullptr}; + parsed = parser.parse(argv2); + REQUIRE(parsed.get("cold").size() == 0); + REQUIRE(parsed.get("records") == true); + REQUIRE(parsed.get("get").size() == 1); + REQUIRE(parsed.get("get")[0] == "proxy.config.x"); + + // After "--" even a token shaped like an option becomes the value. + const char *argv3[] = {"test_prog", "get", "-c", "--", "-weird-name.yaml", "proxy.config.x", nullptr}; + parsed = parser.parse(argv3); + REQUIRE(parsed.get("cold").value() == "-weird-name.yaml"); + REQUIRE(parsed.get("get").size() == 1); + REQUIRE(parsed.get("get")[0] == "proxy.config.x"); +} + +TEST_CASE("An option taking a fixed number of arguments can be given a value shaped like an option", "[parse]") +{ + ts::ArgParser parser; + parser.add_global_usage("test_prog [OPTIONS]"); + + // Mirrors "traffic_ctl server debug enable [--tags TAGS] [--append]". + ts::ArgParser::Command &cmd = parser.add_command("enable", "enable debug"); + cmd.add_option("--tags", "-t", "debug tags", "", 1); + cmd.add_option("--append", "-a", "append to the existing tags"); + + // A value that starts with '-' is passed after "--", which is otherwise taken as naming an + // option and reported as a missing value. + const char *argv1[] = {"test_prog", "enable", "-t", "--", "-a", nullptr}; + ts::Arguments parsed = parser.parse(argv1); + REQUIRE(parsed.get("tags").value() == "-a"); + REQUIRE(parsed.get("append") == false); + + // The --option=value form needs no escape. + const char *argv2[] = {"test_prog", "enable", "--tags=-a", nullptr}; + parsed = parser.parse(argv2); + REQUIRE(parsed.get("tags").value() == "-a"); + REQUIRE(parsed.get("append") == false); + + // An option written after the value keeps its own meaning. + const char *argv3[] = {"test_prog", "enable", "-t", "http", "-a", nullptr}; + parsed = parser.parse(argv3); + REQUIRE(parsed.get("tags").value() == "http"); + REQUIRE(parsed.get("append") == true); +} + +TEST_CASE("A repeated variable argument option accumulates its values", "[parse]") +{ + ts::ArgParser parser; + parser.add_global_usage("test_prog [OPTIONS]"); + + // Mirrors "traffic_ctl config reload [-D DIRECTIVE...] [-d SOURCE...] [-m]". + ts::ArgParser::Command &cmd = parser.add_command("reload", "reload configs"); + cmd.add_option("--directive", "-D", "reload directives", "", MORE_THAN_ZERO_ARG_N, ""); + cmd.add_option("--data", "-d", "inline config data", "", MORE_THAN_ZERO_ARG_N, ""); + cmd.add_option("--monitor", "-m", "monitor progress"); + + // Collection stops at the second -D, so the values of the first must survive it. + const char *argv1[] = {"test_prog", "reload", "-D", "a.id=1", "-D", "b.id=2", nullptr}; + ts::Arguments parsed = parser.parse(argv1); + REQUIRE(parsed.get("directive").size() == 2); + REQUIRE(parsed.get("directive")[0] == "a.id=1"); + REQUIRE(parsed.get("directive")[1] == "b.id=2"); + + // Each occurrence keeps every value it collected, in the order written. + const char *argv2[] = {"test_prog", "reload", "-D", "a.id=1", "b.id=2", "-D", "c.id=3", "d.id=4", nullptr}; + parsed = parser.parse(argv2); + REQUIRE(parsed.get("directive").size() == 4); + REQUIRE(parsed.get("directive")[0] == "a.id=1"); + REQUIRE(parsed.get("directive")[3] == "d.id=4"); + + // An unrelated option written between the two occurrences keeps its own meaning. + const char *argv3[] = {"test_prog", "reload", "-D", "a.id=1", "-m", "-D", "b.id=2", nullptr}; + parsed = parser.parse(argv3); + REQUIRE(parsed.get("directive").size() == 2); + REQUIRE(parsed.get("directive")[1] == "b.id=2"); + REQUIRE(parsed.get("monitor") == true); + + // The two spellings count against the same option, in either order. + const char *argv4[] = {"test_prog", "reload", "-D", "a.id=1", "--directive=b.id=2", nullptr}; + parsed = parser.parse(argv4); + REQUIRE(parsed.get("directive").size() == 2); + REQUIRE(parsed.get("directive")[0] == "a.id=1"); + REQUIRE(parsed.get("directive")[1] == "b.id=2"); + + const char *argv5[] = {"test_prog", "reload", "--directive=a.id=1", "--directive=b.id=2", nullptr}; + parsed = parser.parse(argv5); + REQUIRE(parsed.get("directive").size() == 2); + + // Repeated -d merges the same way, which is what the documented multi-source reload needs. + const char *argv6[] = {"test_prog", "reload", "-d", "@ip_allow.yaml", "-d", "@sni.yaml", nullptr}; + parsed = parser.parse(argv6); + REQUIRE(parsed.get("data").size() == 2); + REQUIRE(parsed.get("data")[0] == "@ip_allow.yaml"); + REQUIRE(parsed.get("data")[1] == "@sni.yaml"); + + // Two different options each keep their own values. + const char *argv7[] = {"test_prog", "reload", "-D", "a.id=1", "-d", "@f.yaml", "-D", "b.id=2", nullptr}; + parsed = parser.parse(argv7); + REQUIRE(parsed.get("directive").size() == 2); + REQUIRE(parsed.get("data").size() == 1); + REQUIRE(parsed.get("data")[0] == "@f.yaml"); +} + +TEST_CASE("A repeated option requiring at least one argument accumulates too", "[parse]") +{ + ts::ArgParser parser; + parser.add_global_usage("test_prog [OPTIONS]"); + + // Mirrors "traffic_ctl rpc invoke [--params PARAM...]". + ts::ArgParser::Command &cmd = parser.add_command("invoke", "invoke a method"); + cmd.add_option("--params", "-p", "request parameters", "", MORE_THAN_ONE_ARG_N, ""); + cmd.add_option("--format", "-f", "output format", "", 1, ""); + + const char *argv1[] = {"test_prog", "invoke", "-p", "one", "-p", "two", nullptr}; + ts::Arguments parsed = parser.parse(argv1); + REQUIRE(parsed.get("params").size() == 2); + REQUIRE(parsed.get("params")[0] == "one"); + REQUIRE(parsed.get("params")[1] == "two"); + + // The arity is satisfied by the first occurrence, so a later one is not left short. + const char *argv2[] = {"test_prog", "invoke", "-p", "one", "-f", "json", "-p", "two", nullptr}; + parsed = parser.parse(argv2); + REQUIRE(parsed.get("params").size() == 2); + REQUIRE(parsed.get("format").value() == "json"); +} + +TEST_CASE("A repeated option taking a fixed number of arguments keeps the last value", "[parse]") +{ + ts::ArgParser parser; + parser.add_global_usage("test_prog [OPTIONS]"); + + // Mirrors "traffic_ctl server debug enable [--tags TAGS]". Only an unbounded arity + // accumulates; a fixed one keeps the behaviour it has always had. + ts::ArgParser::Command &cmd = parser.add_command("enable", "enable debug"); + cmd.add_option("--tags", "-t", "debug tags", "", 1); + + const char *argv1[] = {"test_prog", "enable", "-t", "http", "-t", "cache", nullptr}; + ts::Arguments parsed = parser.parse(argv1); + REQUIRE(parsed.get("tags").size() == 1); + REQUIRE(parsed.get("tags").value() == "cache"); +} diff --git a/tests/gold_tests/jsonrpc/config_reload_directive_cli.test.py b/tests/gold_tests/jsonrpc/config_reload_directive_cli.test.py new file mode 100644 index 00000000000..be9eceac777 --- /dev/null +++ b/tests/gold_tests/jsonrpc/config_reload_directive_cli.test.py @@ -0,0 +1,155 @@ +''' +Verify traffic_ctl command line parsing for the reload options that take a +variable number of values, --directive (-D) and --data (-d). + +Options declared with MORE_THAN_ZERO_ARG_N used to consume every remaining +token, so any option written after -D was silently swallowed as a directive +value and never parsed. -D therefore had to be the last option, and -D could +not be combined with -d. Once collection stops at the following option, the +option can be written more than once, and each occurrence has to keep the +values it collected rather than replace the ones before it. These runs assert +on the JSONRPC request that traffic_ctl builds (printed by -f rpc), because the +subject under test is the command line parsing rather than the server side +handling of the reload. +''' +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +Test.Summary = 'Verify traffic_ctl -D/-d argument parsing for config reload' +Test.ContinueOnFail = True + +ts = Test.MakeATSProcess("ts") +ts.StartupTimeout = 30 + +ts.Disk.records_config.update({ + 'proxy.config.diags.debug.enabled': 1, + 'proxy.config.diags.debug.tags': 'rpc|config.reload', +}) + +ts.Disk.ip_allow_yaml.AddLines([ + 'ip_allow:', + '- apply: in', + ' ip_addrs: 0/0', + ' action: allow', + ' methods: ALL', +]) + +# ============================================================================ +# Test 1: an option written after -D keeps its own argument +# ============================================================================ +tr = Test.AddTestRun("Option after -D is not consumed as a directive value") +tr.Processes.Default.StartBefore(ts) +tr.Processes.Default.Command = "traffic_ctl config reload -D ip_allow.id=foo -t cli_token_1 -f rpc" +tr.Processes.Default.Env = ts.Env +tr.Processes.Default.Streams.stdout += Testers.ContainsExpression('"token": "cli_token_1"', "-t must survive after -D") +tr.Processes.Default.Streams.stdout += Testers.ContainsExpression('"id": "foo"', "the directive must still be parsed") +tr.StillRunningAfter = ts + +# ============================================================================ +# Test 2: several directives, then an option +# ============================================================================ +tr = Test.AddTestRun("Multiple directives followed by an option") +tr.Processes.Default.Command = "traffic_ctl config reload -D ip_allow.id=1 sni.id=2 -t cli_token_2 -f rpc" +tr.Processes.Default.Env = ts.Env +tr.Processes.Default.Streams.stdout += Testers.ContainsExpression('"token": "cli_token_2"', "-t must survive after -D") +tr.Processes.Default.Streams.stdout += Testers.ContainsExpression('"ip_allow"', "first directive key must be present") +tr.Processes.Default.Streams.stdout += Testers.ContainsExpression('"sni"', "second directive key must be present") +tr.StillRunningAfter = ts + +# ============================================================================ +# Test 3: -D combined with -d, which the parser previously made impossible +# ============================================================================ +tr = Test.AddTestRun("-D can be combined with -d") +tr.Processes.Default.Command = "traffic_ctl config reload -D ip_allow.id=foo -d 'ip_allow: {rules: [x]}' -f rpc" +tr.Processes.Default.Env = ts.Env +tr.Processes.Default.Streams.stdout += Testers.ContainsExpression('"rules"', "inline content from -d must be present") +tr.Processes.Default.Streams.stdout += Testers.ContainsExpression('"_reload"', "directives from -D must be present") +tr.StillRunningAfter = ts + +# ============================================================================ +# Test 4: --directive=value keeps a value that itself contains '=' +# ============================================================================ +tr = Test.AddTestRun("--directive=value preserves embedded equal signs") +tr.Processes.Default.Command = "traffic_ctl config reload --directive=ip_allow.id=foo -f rpc" +tr.Processes.Default.Env = ts.Env +tr.Processes.Default.Streams.stdout += Testers.ContainsExpression('"id": "foo"', "the whole value must reach the request") +tr.Processes.Default.Streams.stdout += Testers.ExcludesExpression("Invalid directive format", "the value must parse cleanly") +tr.StillRunningAfter = ts + +# ============================================================================ +# Test 5: "--" ends option recognition, so the value is taken literally and +# then rejected by the directive format check +# ============================================================================ +tr = Test.AddTestRun("A value after -- is taken literally") +tr.Processes.Default.Command = "traffic_ctl config reload -D -- -m" +tr.Processes.Default.Env = ts.Env +tr.Processes.Default.ReturnCode = 2 +tr.Processes.Default.Streams.stdout += Testers.ContainsExpression( + "Invalid directive format '-m'", "-m must be treated as a directive value, not as --monitor") +tr.StillRunningAfter = ts + +# ============================================================================ +# Test 6: -D without any directive would silently reload every handler +# ============================================================================ +tr = Test.AddTestRun("-D requires at least one directive") +tr.Processes.Default.Command = "traffic_ctl config reload -D" +tr.Processes.Default.Env = ts.Env +tr.Processes.Default.ReturnCode = 2 +tr.Processes.Default.Streams.stdout += Testers.ContainsExpression("requires at least one", "-D must not be a silent no-op") +tr.StillRunningAfter = ts + +# ============================================================================ +# Test 7: same for -d, where a silent full reload is especially misleading +# ============================================================================ +tr = Test.AddTestRun("-d requires content") +tr.Processes.Default.Command = "traffic_ctl config reload -d" +tr.Processes.Default.Env = ts.Env +tr.Processes.Default.ReturnCode = 2 +tr.Processes.Default.Streams.stdout += Testers.ContainsExpression("requires content", "-d must not be a silent no-op") +tr.StillRunningAfter = ts + +# ============================================================================ +# Test 8: a repeated -D keeps the directives of every occurrence +# ============================================================================ +tr = Test.AddTestRun("A repeated -D accumulates its directives") +tr.Processes.Default.Command = "traffic_ctl config reload -D ip_allow.id=1 -D sni.id=2 -f rpc" +tr.Processes.Default.Env = ts.Env +tr.Processes.Default.Streams.stdout += Testers.ContainsExpression('"ip_allow"', "the first occurrence must survive the second") +tr.Processes.Default.Streams.stdout += Testers.ContainsExpression('"sni"', "the second occurrence must be present") +tr.StillRunningAfter = ts + +# ============================================================================ +# Test 9: repeating the option is how a directive is written after another +# option, since collection stops at the option rather than at the value +# ============================================================================ +tr = Test.AddTestRun("A repeated -D survives an option written between the two") +tr.Processes.Default.Command = "traffic_ctl config reload -D ip_allow.id=1 -t cli_token_8 -D sni.id=2 -f rpc" +tr.Processes.Default.Env = ts.Env +tr.Processes.Default.Streams.stdout += Testers.ContainsExpression('"token": "cli_token_8"', "-t must keep its own value") +tr.Processes.Default.Streams.stdout += Testers.ContainsExpression('"ip_allow"', "the directive before -t must survive") +tr.Processes.Default.Streams.stdout += Testers.ContainsExpression('"sni"', "the directive after -t must be parsed") +tr.StillRunningAfter = ts + +# ============================================================================ +# Test 10: the documented multi source reload, where dropping one -d would +# leave its handler out of the reload without reporting anything +# ============================================================================ +tr = Test.AddTestRun("A repeated -d merges every source") +tr.Processes.Default.Command = ("traffic_ctl config reload -d 'ip_allow: {rules: [x]}' -d 'sni: {rules: [y]}' -f rpc") +tr.Processes.Default.Env = ts.Env +tr.Processes.Default.Streams.stdout += Testers.ContainsExpression('"ip_allow"', "content from the first -d must be present") +tr.Processes.Default.Streams.stdout += Testers.ContainsExpression('"sni"', "content from the second -d must be present") +tr.StillRunningAfter = ts diff --git a/tests/gold_tests/records/traffic_ctl_cold_config.test.py b/tests/gold_tests/records/traffic_ctl_cold_config.test.py index c27925baf2b..f605a4deac0 100644 --- a/tests/gold_tests/records/traffic_ctl_cold_config.test.py +++ b/tests/gold_tests/records/traffic_ctl_cold_config.test.py @@ -92,3 +92,106 @@ tr.Processes.Default.ReturnCode = 0 tr.Processes.Default.Env = ts.Env tr.Disk.File(file).Content = 'gold/records.yaml.cold_test5.gold' + +# --cold takes at most one file name, so it does not consume the record names that follow +# it. Before that was the case it had to be written after them, which the runs above do. +records_file = os.path.join(ts.Variables.CONFIGDIR, "records.yaml") + +# 6 +tr = Test.AddTestRun("Get a value with the file name given before the record") +tr.Processes.Default.Command = f'traffic_ctl config get -c {records_file} proxy.config.diags.debug.tags' +tr.Processes.Default.ReturnCode = 0 +tr.Processes.Default.Env = ts.Env +tr.Processes.Default.Streams.stdout += Testers.ContainsExpression( + 'proxy.config.diags.debug.tags: http', 'The record must still be parsed as a record') + +# 7 +tr = Test.AddTestRun("Get several values with the file name given before them") +tr.Processes.Default.Command = ( + f'traffic_ctl config get -c {records_file} ' + 'proxy.config.diags.debug.tags proxy.config.cache.limits.http.max_alts') +tr.Processes.Default.ReturnCode = 0 +tr.Processes.Default.Env = ts.Env +tr.Processes.Default.Streams.stdout += Testers.ContainsExpression( + 'proxy.config.diags.debug.tags: http', 'The first record must be parsed as a record') +tr.Processes.Default.Streams.stdout += Testers.ContainsExpression( + 'proxy.config.cache.limits.http.max_alts: 1', 'The second record must be parsed as a record') + +# 8 +tr = Test.AddTestRun("Get a value using the --cold=FILE form") +tr.Processes.Default.Command = f'traffic_ctl config get --cold={records_file} proxy.config.diags.debug.tags' +tr.Processes.Default.ReturnCode = 0 +tr.Processes.Default.Env = ts.Env +tr.Processes.Default.Streams.stdout += Testers.ContainsExpression( + 'proxy.config.diags.debug.tags: http', 'The record must still be parsed as a record') + +# 9 +file = os.path.join(ts.Variables.CONFIGDIR, "new_records3.yaml") +tr = Test.AddTestRun("Set a value with the file name given before the record and the value") +tr.Processes.Default.Command = f'traffic_ctl config set -c {file} proxy.config.cache.limits.http.max_alts 3' +tr.Processes.Default.ReturnCode = 0 +tr.Processes.Default.Env = ts.Env +tr.Disk.File(file).Content = 'gold/records.yaml.cold_test5.gold' + +# 10 +tr = Test.AddTestRun("--cold takes at most one file name, so repeating it is an error") +tr.Processes.Default.Command = f'traffic_ctl config get --cold={records_file} --cold={records_file} proxy.config.diags.debug.tags' +tr.Processes.Default.ReturnCode = 64 # EX_USAGE - command line usage error +tr.Processes.Default.Env = ts.Env +tr.Processes.Default.Streams.All = Testers.ContainsExpression( + 'at most one argument expected by --cold', 'A repeated --cold must be reported rather than the last one winning') + +# 11 +tr = Test.AddTestRun("A repeated --cold is an error in the space-separated form too") +tr.Processes.Default.Command = f'traffic_ctl config get -c {records_file} -c {records_file} proxy.config.diags.debug.tags' +tr.Processes.Default.ReturnCode = 64 # EX_USAGE - command line usage error +tr.Processes.Default.Env = ts.Env +tr.Processes.Default.Streams.All = Testers.ContainsExpression( + 'at most one argument expected by --cold', 'A repeated -c must be reported rather than the last one winning') + +# 12 +tr = Test.AddTestRun("Mixing the two --cold spellings cannot smuggle in a second file name") +tr.Processes.Default.Command = f'traffic_ctl config get -c {records_file} --cold={records_file} proxy.config.diags.debug.tags' +tr.Processes.Default.ReturnCode = 64 # EX_USAGE - command line usage error +tr.Processes.Default.Env = ts.Env +tr.Processes.Default.Streams.All = Testers.ContainsExpression( + 'at most one argument expected by --cold', 'The two forms must be counted together') + +# An empty file name reaches traffic_ctl when it is taken from a variable that is unset. The +# --cold=FILE form has always rejected it; the space-separated form used to fall through to the +# default records.yaml instead, so a run meant for another file read or wrote the live one. +# These runs go through "sh -c" because autest indexes the first character of every argument +# it splits, so an empty argument written straight into Command raises IndexError before the +# process starts. + +# 13 +tr = Test.AddTestRun("An empty file name is reported rather than taken as the default file") +tr.Processes.Default.Command = """sh -c 'traffic_ctl config get -c "" proxy.config.diags.debug.tags'""" +tr.Processes.Default.ReturnCode = 64 # EX_USAGE - command line usage error +tr.Processes.Default.Env = ts.Env +tr.Processes.Default.Streams.All = Testers.ContainsExpression( + "missing argument for '-c'", 'An empty -c value must be reported, naming the option as written') + +# 14 +tr = Test.AddTestRun("An empty file name is reported before anything is written") +tr.Processes.Default.Command = """sh -c 'traffic_ctl config set -c "" proxy.config.cache.limits.http.max_alts 9'""" +tr.Processes.Default.ReturnCode = 64 # EX_USAGE - command line usage error +tr.Processes.Default.Env = ts.Env +tr.Processes.Default.Streams.All = Testers.ContainsExpression( + "missing argument for '-c'", 'A set with an empty -c value must not fall back to the live records.yaml') + +# 15 +tr = Test.AddTestRun("The long spelling of an empty file name is reported as written") +tr.Processes.Default.Command = """sh -c 'traffic_ctl config get --cold "" proxy.config.diags.debug.tags'""" +tr.Processes.Default.ReturnCode = 64 # EX_USAGE - command line usage error +tr.Processes.Default.Env = ts.Env +tr.Processes.Default.Streams.All = Testers.ContainsExpression( + "missing argument for '--cold'", 'The error must name the spelling the caller used') + +# 16 +tr = Test.AddTestRun("A bare -c before the record leaves set short of its own arguments") +tr.Processes.Default.Command = 'traffic_ctl config set -c proxy.config.cache.limits.http.max_alts 9' +tr.Processes.Default.ReturnCode = 64 # EX_USAGE - command line usage error +tr.Processes.Default.Env = ts.Env +tr.Processes.Default.Streams.All = Testers.ContainsExpression( + r'2 argument\(s\) expected by set', 'set must report what it was left without, as the docs show') diff --git a/tests/gold_tests/traffic_ctl/traffic_ctl_server_debug.test.py b/tests/gold_tests/traffic_ctl/traffic_ctl_server_debug.test.py index f899f980708..30b781732b2 100644 --- a/tests/gold_tests/traffic_ctl/traffic_ctl_server_debug.test.py +++ b/tests/gold_tests/traffic_ctl/traffic_ctl_server_debug.test.py @@ -78,3 +78,21 @@ tr.Processes.Default.Streams.All = Testers.ContainsExpression( "Option \'--append\' requires \'--tags\' to be specified", "Should show error that --append requires --tags") tr.StillRunningAfter = traffic_ctl._ts + +# Test 15: An option written where the tags are expected leaves them missing, rather than being +# applied as the tags themselves. +tr = Test.AddTestRun("test --tags followed by another option") +tr.Processes.Default.Env = traffic_ctl._ts.Env +tr.Processes.Default.Command = "traffic_ctl server debug enable --tags --append" +tr.Processes.Default.ReturnCode = 64 # EX_USAGE - command line usage error +tr.Processes.Default.Streams.All = Testers.ContainsExpression( + "1 argument\\(s\\) expected by tags", "Should report the tags as missing") +tr.StillRunningAfter = traffic_ctl._ts + +# Test 16: Tags that are shaped like an option are passed after "--". +tr = Test.AddTestRun("test tags shaped like an option") +tr.Processes.Default.Env = traffic_ctl._ts.Env +tr.Processes.Default.Command = "traffic_ctl server debug enable --tags -- -a" +tr.Processes.Default.ReturnCode = 0 +tr.Processes.Default.Streams.stdout = Testers.ContainsExpression('tags »"-a"«', "The value after -- must be taken as the tags") +tr.StillRunningAfter = traffic_ctl._ts