Merge ' tools/utils: tool_app_template: handle the case of no args ' …

…from Botond Dénes

Currently, `tool_app_template::run_async()` crashes when invoked with empty argv (with just `argv[0]` populated). This can happen if the tool app is invoked without any further args, e.g. just invoking `scylla nodetool`. The crash happens because unconditional dereferencing of `argv[1]` to get the current operation.

To fix, add an early-exit for this case, just printing a usage message and exiting with exit code 2.

Fixes: #16451

Closes #16456

* github.com:scylladb/scylladb:
  test: add regression tests for invoking tools with no args
  tools/utils: tool_app_template: handle the case of no args
  tools/utils: tool_app_template: remove "scylla-" prefix from app name

(cherry picked from commit 5866d26)

test/cql-pytest/test_tools.py

@@ -843,3 +843,13 @@ def test_scrub_validate_mode(scylla_path, scrub_workdir, scrub_schema_file, scru
 
         # Check that validate did not move the bad sstable into qurantine
         assert os.path.exists(scrub_bad_sstable)
+
+
+def test_scylla_sstable_no_args(scylla_path):
+    res = subprocess.run([scylla_path, "sstable"], capture_output=True, text=True)
+
+    assert res.stdout == ""
+    assert res.stderr == """\
+Usage: scylla sstable OPERATION [OPTIONS] ...
+Try `scylla sstable --help` for more information.
+"""

test/nodetool/test_nodetool.py

@@ -5,6 +5,7 @@
 #
 
 from rest_api_mock import expected_request
+import subprocess
 
 
 def test_jmx_compatibility_args(nodetool, scylla_only):
@@ -29,3 +30,13 @@ def test_jmx_compatibility_args(nodetool, scylla_only):
              expected_requests=dummy_request)
     nodetool("compact", "system_schema", "--print-port",
              expected_requests=dummy_request)
+
+
+def test_nodetool_no_args(nodetool_path, scylla_only):
+    res = subprocess.run([nodetool_path, "nodetool"], capture_output=True, text=True)
+
+    assert res.stdout == ""
+    assert res.stderr == """\
+Usage: scylla nodetool OPERATION [OPTIONS] ...
+Try `scylla nodetool --help` for more information.
+"""

tools/scylla-nodetool.cc

@@ -25,9 +25,9 @@ using namespace tools::utils;
 
 namespace {
 
-const auto app_name = "scylla-nodetool";
+const auto app_name = "nodetool";
 
-logging::logger nlog(app_name);
+logging::logger nlog(format("scylla-{}", app_name));
 
 class scylla_rest_client {
     sstring _host;
@@ -170,7 +170,7 @@ void help_operation(const tool_app_template::config& cfg, const bpo::variables_m
         // This will be addressed once https://github.com/scylladb/seastar/pull/1762
         // goes in.
 
-        bpo::options_description opts_desc(fmt::format("{} options", app_name));
+        bpo::options_description opts_desc(fmt::format("scylla-{} options", app_name));
         opts_desc.add_options()
                 ("help,h", "show help message")
                 ;
@@ -519,7 +519,7 @@ int scylla_nodetool_main(int argc, char** argv) {
     nlog.debug("replacement argv: {}", replacement_argv);
 
     constexpr auto description_template =
-R"(scylla-nodetool - a command-line tool to administer local or remote ScyllaDB nodes
+R"(scylla-{} - a command-line tool to administer local or remote ScyllaDB nodes
 
 # Operations
 
@@ -536,10 +536,10 @@ For more information, see: https://opensource.docs.scylladb.com/stable/operating
     const auto operations = boost::copy_range<std::vector<operation>>(get_operations_with_func() | boost::adaptors::map_keys);
     tool_app_template::config app_cfg{
             .name = app_name,
-            .description = format(description_template, app_name, boost::algorithm::join(operations | boost::adaptors::transformed([] (const auto& op) {
+            .description = format(description_template, app_name, nlog.name(), boost::algorithm::join(operations | boost::adaptors::transformed([] (const auto& op) {
                 return format("* {}: {}", op.name(), op.summary());
             }), "\n")),
-            .logger_name = app_name,
+            .logger_name = nlog.name(),
             .lsa_segment_pool_backend_size_mb = 1,
             .operations = std::move(operations),
             .global_options = &global_options};

tools/scylla-sstable.cc

@@ -50,9 +50,9 @@ using operation_func = void(*)(schema_ptr, reader_permit, const std::vector<ssta
 
 namespace {
 
-const auto app_name = "scylla-sstable";
+const auto app_name = "sstable";
 
-logging::logger sst_log(app_name);
+logging::logger sst_log(format("scylla-{}", app_name));
 
 db::nop_large_data_handler large_data_handler;
 
@@ -2797,7 +2797,7 @@ namespace tools {
 
 int scylla_sstable_main(int argc, char** argv) {
     constexpr auto description_template =
-R"(scylla-sstable - a multifunctional command-line tool to examine the content of sstables.
+R"(scylla-{} - a multifunctional command-line tool to examine the content of sstables.
 
 Usage: scylla sstable {{operation}} [--option1] [--option2] ... [{{sstable_path1}}] [{{sstable_path2}}] ...
 
@@ -2887,10 +2887,10 @@ Validate the specified sstables:
     const auto operations = boost::copy_range<std::vector<operation>>(operations_with_func | boost::adaptors::map_keys);
     tool_app_template::config app_cfg{
             .name = app_name,
-            .description = format(description_template, app_name, boost::algorithm::join(operations | boost::adaptors::transformed([] (const auto& op) {
+            .description = format(description_template, app_name, sst_log.name(), boost::algorithm::join(operations | boost::adaptors::transformed([] (const auto& op) {
                 return format("* {}: {}", op.name(), op.summary());
             }), "\n")),
-            .logger_name = app_name,
+            .logger_name = sst_log.name(),
             .lsa_segment_pool_backend_size_mb = 100,
             .operations = std::move(operations),
             .global_options = &global_options,

tools/scylla-types.cc

@@ -23,6 +23,8 @@ namespace bpo = boost::program_options;
 
 namespace {
 
+const auto app_name = "types";
+
 using type_variant = std::variant<
         data_type,
         compound_type<allow_prefixes::yes>,
@@ -347,9 +349,9 @@ namespace tools {
 
 int scylla_types_main(int argc, char** argv) {
     auto description_template =
-R"(scylla-types - a command-line tool to examine values belonging to scylla types.
+R"(scylla-{} - a command-line tool to examine values belonging to scylla types.
 
-Usage: scylla types {{action}} [--option1] [--option2] ... {{hex_value1}} [{{hex_value2}}] ...
+Usage: scylla {} {{action}} [--option1] [--option2] ... {{hex_value1}} [{{hex_value2}}] ...
 
 Allows examining raw values obtained from e.g. sstables, logs or coredumps and
 executing various actions on them. Values should be provided in hex form,
@@ -372,8 +374,8 @@ For more information about individual actions, see their specific help:
 
     const auto operations = boost::copy_range<std::vector<operation>>(operations_with_func | boost::adaptors::map_keys);
     tool_app_template::config app_cfg{
-        .name = "scylla-types",
-        .description = format(description_template, boost::algorithm::join(operations | boost::adaptors::transformed(
+        .name = app_name,
+        .description = format(description_template, app_name, app_name, boost::algorithm::join(operations | boost::adaptors::transformed(
                 [] (const operation& op) { return format("* {} - {}", op.name(), op.summary()); } ), "\n")),
         .operations = std::move(operations),
         .global_options = &global_options,

tools/utils.cc

@@ -81,13 +81,20 @@ void configure_tool_mode(app_template::seastar_options& opts, const sstring& log
 } // anonymous namespace
 
 int tool_app_template::run_async(int argc, char** argv, noncopyable_function<int(const operation&, const boost::program_options::variables_map&)> main_func) {
+    if (argc <= 1) {
+        auto program_name = argc ? std::filesystem::path(argv[0]).filename().native() : "scylla";
+        fmt::print(std::cerr, "Usage: {} {} OPERATION [OPTIONS] ...\nTry `{} {} --help` for more information.\n",
+                program_name, _cfg.name, program_name, _cfg.name);
+        return 2;
+    }
+
     const operation* found_op = nullptr;
     if (std::strncmp(argv[1], "--help", 6) != 0 && std::strcmp(argv[1], "-h") != 0) {
         found_op = &tools::utils::get_selected_operation(argc, argv, _cfg.operations, "operation");
     }
 
     app_template::seastar_options app_cfg;
-    app_cfg.name = _cfg.name;
+    app_cfg.name = format("scylla-{}", _cfg.name);
 
     if (found_op) {
         app_cfg.description = format("{}\n\n{}\n", found_op->summary(), found_op->description());