Merge pull request #1354

Move options from format to the import subcommand

CHANGELOG.md

@@ -13,8 +13,6 @@ This changelog documents all notable user-facing changes of VAST.
 
 ## Unreleased
 
-### ⚡️ Breaking Changes
-
 - ⚠️ The `infer` command has an improved heuristic for the number types `int`,
   `count`, and `real`. [#1343](https://github.com/tenzir/vast/pull/1343)
   [#1356](https://github.com/tenzir/vast/pull/1356)
@@ -27,6 +25,12 @@ This changelog documents all notable user-facing changes of VAST.
   [#1356](https://github.com/tenzir/vast/pull/1356)
   [@ngrodzitski](https://github.com/ngrodzitski)
 
+- ⚠️ The options `listen`, `read`, `schema`, `schema-file`, `type`, and `uds` can
+  from now on be supplied to the `import` command directly. Similarly, the
+  options `write` and `uds` can be supplied to the `export` command. All options
+  can still be used after the format subcommand, but that usage is deprecated.
+  [#1354](https://github.com/tenzir/vast/pull/1354)
+
 - ⚡️ VAST now requires [{fmt} >= 5.2.1](https://fmt.dev) to be installed.
   [#1330](https://github.com/tenzir/vast/pull/1330)
 

doc/cli/vast-import-csv.md

@@ -13,5 +13,5 @@ E.g., to import Threat Intelligence data into VAST, the known type
 `intel.indicator` can be used:
 
 ```bash
-vast import csv --type=intel.indicator --read=path/to/indicators.csv
+vast import --type=intel.indicator --read=path/to/indicators.csv csv
 ```

libvast/src/command.cpp

@@ -357,6 +357,35 @@ caf::error parse_impl(invocation& result, const command& cmd,
   return parse_impl(result, **i, position + 1, last, target);
 }
 
+void fixup_options(invocation& inv) {
+  using namespace std::string_literals;
+  auto move_option_arg = [&](const char* cmd, const char* format,
+                             const char* opt) {
+    auto path = "vast."s + cmd + '.' + format + '.' + opt;
+    if (auto x = caf::get_if<std::string>(&inv.options, path)) {
+      // The logger isn't initialized yet.
+#if VAST_LOG_LEVEL >= VAST_LOG_LEVEL_WARNING
+      fmt::print(stderr,
+                 "The option '{}' is deprecated for the '{}' subcommand and "
+                 "should be specified at the {} subcommand instead\n",
+                 opt, format, cmd);
+#endif
+      put(inv.options, "vast."s + cmd + '.' + opt, *x);
+    }
+  };
+  for (const auto& cmd : {"import", "spawn.source"})
+    for (const auto& format :
+         {"csv", "json", "suricata", "syslog", "test", "zeek", "zeek-json"})
+      for (const auto& opt :
+           {"listen", "read", "schema", "schema-file", "type", "uds"})
+        move_option_arg(cmd, format, opt);
+  for (const auto& cmd : {"export", "spawn.sink"})
+    for (const auto& format :
+         {"arrow", "ascii", "csv", "json", "null", "pcap", "zeek"})
+      for (const auto& opt : {"write", "uds"})
+        move_option_arg(cmd, format, opt);
+}
+
 caf::expected<invocation>
 parse(const command& root, command::argument_iterator first,
       command::argument_iterator last) {
@@ -380,6 +409,7 @@ parse(const command& root, command::argument_iterator first,
     manfooter(std::cout);
     return caf::no_error;
   }
+  fixup_options(result);
   return result;
 }
 

libvast/src/defaults.cpp

@@ -23,8 +23,7 @@
 namespace vast::defaults::import {
 
 size_t test::seed(const caf::settings& options) {
-  std::string cat = category;
-  if (auto val = caf::get_if<size_t>(&options, cat + ".seed"))
+  if (auto val = caf::get_if<size_t>(&options, "vast.import.test.seed"))
     return *val;
   std::random_device rd;
   return rd();

libvast/src/format/pcap.cpp

@@ -71,10 +71,10 @@ reader::reader(const caf::settings& options, std::unique_ptr<std::istream>)
   : super(options) {
   using defaults_t = vast::defaults::import::pcap;
   using caf::get_if;
-  std::string category = defaults_t::category;
+  std::string category = "vast.import.pcap";
   if (auto interface = get_if<std::string>(&options, category + ".interface"))
     interface_ = *interface;
-  input_ = get_or(options, category + ".read", defaults_t::read);
+  input_ = get_or(options, "vast.import.read", vast::defaults::import::read);
   cutoff_ = get_or(options, category + ".cutoff", defaults_t::cutoff);
   max_flows_ = get_or(options, category + ".max-flows", defaults_t::max_flows);
   max_age_
@@ -482,7 +482,7 @@ void reader::shrink_to_max_size() {
 writer::writer(const caf::settings& options) {
   flush_interval_ = get_or(options, "vast.export.pcap.flush-interval",
                            defaults::flush_interval);
-  trace_ = get_or(options, "vast.export.pcap.write", defaults::write);
+  trace_ = get_or(options, "vast.export.write", vast::defaults::export_::write);
 }
 
 writer::~writer() {

libvast/src/format/writer_factory.cpp

@@ -36,10 +36,9 @@ template <class Writer>
 caf::expected<std::unique_ptr<format::writer>>
 make_writer(const caf::settings& options) {
   using namespace std::string_literals;
-  using defaults = typename Writer::defaults;
   using ostream_ptr = std::unique_ptr<std::ostream>;
   if constexpr (std::is_constructible_v<Writer, ostream_ptr, caf::settings>) {
-    auto out = detail::make_output_stream<defaults>(options);
+    auto out = detail::make_output_stream(options);
     if (!out)
       return out.error();
     return std::make_unique<Writer>(std::move(*out), options);

libvast/src/format/zeek.cpp

@@ -515,7 +515,8 @@ caf::error reader::parse_header() {
 }
 
 writer::writer(const caf::settings& options) {
-  auto output = get_or(options, "vast.export.zeek.write", defaults::write);
+  auto output
+    = get_or(options, "vast.export.write", vast::defaults::export_::write);
   if (output != "-")
     dir_ = std::move(output);
   show_timestamp_tags_

libvast/src/schema.cpp

@@ -196,14 +196,13 @@ bool convert(const schema& s, data& d) {
   return true;
 }
 
-caf::expected<schema>
-get_schema(const caf::settings& options, const std::string& category) {
+caf::expected<schema> get_schema(const caf::settings& options) {
   // Get the default schema from the registry.
   auto schema_reg_ptr = event_types::get();
   auto schema = schema_reg_ptr ? *schema_reg_ptr : vast::schema{};
   // Update with an alternate schema, if requested.
-  auto sc = caf::get_if<std::string>(&options, category + ".schema");
-  auto sf = caf::get_if<std::string>(&options, category + ".schema-file");
+  auto sc = caf::get_if<std::string>(&options, "vast.import.schema");
+  auto sf = caf::get_if<std::string>(&options, "vast.import.schema-file");
   if (sc && sf)
     caf::make_error(ec::invalid_configuration,
                     "had both schema and schema-file "

libvast/src/system/application.cpp

@@ -125,7 +125,11 @@ auto make_export_command() {
       .add<bool>("unified,u", "marks a query as unified")
       .add<bool>("disable-taxonomies", "don't substitute taxonomy identifiers")
       .add<size_t>("max-events,n", "maximum number of results")
-      .add<std::string>("read,r", "path for reading the query"));
+      .add<std::string>("read,r", "path for reading the query")
+      .add<std::string>("write,w", "path to write events to")
+      .add<bool>("uds,d", "treat -w as UNIX domain socket to connect to"));
+  // NOTICE: The `sink_opts` are relocated in `command.cpp:fixup_options()`,
+  // That function must be kept in sync with the commands added here.
   export_->add_subcommand("zeek", "exports query results in Zeek format",
                           documentation::vast_export_zeek,
                           sink_opts("?vast.export.zeek"));
@@ -184,9 +188,18 @@ auto make_import_command() {
       .add<size_t>("batch-size", "upper bound for the size of a table slice")
       .add<std::string>("batch-timeout", "timeout after which batched "
                                          "table slices are forwarded")
-      .add<std::string>("read-timeout", "timeout for waiting for incoming data")
       .add<bool>("blocking,b", "block until the IMPORTER forwarded all data")
-      .add<size_t>("max-events,n", "the maximum number of events to import"));
+      .add<std::string>("listen,l", "the endpoint to listen on "
+                                    "([host]:port/type)")
+      .add<size_t>("max-events,n", "the maximum number of events to import")
+      .add<std::string>("read,r", "path to input where to read events from")
+      .add<std::string>("read-timeout", "timeout for waiting for incoming data")
+      .add<std::string>("schema,S", "alternate schema as string")
+      .add<std::string>("schema-file,s", "path to alternate schema")
+      .add<std::string>("type,t", "filter event type based on prefix matching")
+      .add<bool>("uds,d", "treat -r as listening UNIX domain socket"));
+  // NOTICE: The `source_opts` are relocated in `command.cpp:fixup_options()`,
+  // That function must be kept in sync with the commands added here.
   import_->add_subcommand("zeek", "imports Zeek TSV logs from STDIN or file",
                           documentation::vast_import_zeek,
                           source_opts("?vast.import.zeek"));
@@ -268,8 +281,15 @@ auto make_spawn_source_command() {
       .add<size_t>("batch-size", "upper bound for the size of a table slice")
       .add<std::string>("batch-timeout", "timeout after which batched "
                                          "table slices are forwarded")
+      .add<std::string>("listen,l", "the endpoint to listen on "
+                                    "([host]:port/type)")
+      .add<size_t>("max-events,n", "the maximum number of events to import")
+      .add<std::string>("read,r", "path to input where to read events from")
       .add<std::string>("read-timeout", "timeout for waiting for incoming data")
-      .add<size_t>("max-events,n", "the maximum number of events to import"));
+      .add<std::string>("schema,S", "alternate schema as string")
+      .add<std::string>("schema-file,s", "path to alternate schema")
+      .add<std::string>("type,t", "filter event type based on prefix matching")
+      .add<bool>("uds,d", "treat -r as listening UNIX domain socket"));
   spawn_source->add_subcommand("csv",
                                "creates a new CSV source inside the node",
                                documentation::vast_spawn_source_csv,
@@ -424,26 +444,19 @@ auto make_command_factory() {
     {"export zeek", make_writer_command("zeek")},
     {"get", get_command},
     {"infer", infer_command},
-    {"import csv", import_command<format::csv::reader, defaults::import::csv>},
+    {"import csv", import_command<format::csv::reader>},
     {"import json", import_command<
-      format::json::reader<format::json::default_selector>,
-      defaults::import::json>},
+      format::json::reader<format::json::default_selector>>},
 #if VAST_ENABLE_PCAP
-    {"import pcap", import_command<format::pcap::reader,
-      defaults::import::pcap>},
+    {"import pcap", import_command<format::pcap::reader>},
 #endif
     {"import suricata", import_command<
-      format::json::reader<format::json::suricata_selector>,
-      defaults::import::suricata>},
-    {"import syslog", import_command<format::syslog::reader,
-      defaults::import::syslog>},
-    {"import test", import_command<format::test::reader,
-      defaults::import::test>},
-    {"import zeek", import_command<format::zeek::reader,
-      defaults::import::zeek>},
+      format::json::reader<format::json::suricata_selector>>},
+    {"import syslog", import_command<format::syslog::reader>},
+    {"import test", import_command<format::test::reader>},
+    {"import zeek", import_command<format::zeek::reader>},
     {"import zeek-json", import_command<
-      format::json::reader<format::json::zeek_selector>,
-      defaults::import::zeek_json>},
+      format::json::reader<format::json::zeek_selector>>},
     {"kill", remote_command},
     {"peer", remote_command},
     {"pivot", pivot_command},
@@ -475,7 +488,7 @@ auto make_command_factory() {
     {"version", version_command},
   };
   // clang-format on
-}
+} // namespace
 
 auto make_root_command(std::string_view path) {
   // We're only interested in the application name, not in its path. For
@@ -589,20 +602,31 @@ void render_error(const command& root, const caf::error& err,
 }
 
 command::opts_builder source_opts(std::string_view category) {
+  // The deprecated options are still accepted but get copied over to their
+  // new location in `command.cpp:fixup_options()`.
   return command::opts(category)
-    .add<std::string>("listen,l", "the endpoint to listen on "
-                                  "([host]:port/type)")
-    .add<std::string>("read,r", "path to input where to read events from")
-    .add<std::string>("schema-file,s", "path to alternate schema")
-    .add<std::string>("schema,S", "alternate schema as string")
-    .add<std::string>("type,t", "filter event type based on prefix matching")
-    .add<bool>("uds,d", "treat -r as listening UNIX domain socket");
+    .add<std::string>("listen,l", "deprecated - this option now applies to the "
+                                  "import command")
+    .add<std::string>("read,r", "deprecated - this option now applies to the "
+                                "import command")
+    .add<std::string>("schema-file,s", "deprecated - this option now applies "
+                                       "to the import command")
+    .add<std::string>("schema,S", "deprecated - this option now applies to the "
+                                  "import command")
+    .add<std::string>("type,t", "deprecated - this option now applies to the "
+                                "import command")
+    .add<bool>("uds,d", "deprecated - this option now applies to the import "
+                        "command");
 }
 
 command::opts_builder sink_opts(std::string_view category) {
+  // The deprecated options are still accepted but get copied over to their
+  // new location in `command.cpp:fixup_options()`.
   return command::opts(category)
-    .add<std::string>("write,w", "path to write events to")
-    .add<bool>("uds,d", "treat -w as UNIX domain socket to connect to");
+    .add<std::string>("write,w", "deprecated - this option now applies to the "
+                                 "import command")
+    .add<bool>("uds,d", "deprecated - this option now applies to the import "
+                        "command");
 }
 
 command::opts_builder opts(std::string_view category) {

libvast/src/system/infer_command.cpp

@@ -138,7 +138,7 @@ caf::message
 infer_command(const invocation& inv, [[maybe_unused]] caf::actor_system& sys) {
   VAST_TRACE_SCOPE("{}", inv);
   const auto& options = inv.options;
-  auto input = detail::make_input_stream<defaults::infer>(options);
+  auto input = detail::make_input_stream(options);
   if (!input)
     return make_message(input.error());
   // Setup buffer for input data.

libvast/src/system/node.cpp

@@ -358,34 +358,26 @@ auto make_component_factory() {
       {"spawn index", lift_component_factory<spawn_index>()},
       {"spawn pivoter", lift_component_factory<spawn_pivoter>()},
       {"spawn source csv",
-       lift_component_factory<
-         spawn_source<format::csv::reader, defaults::import::csv>>()},
+       lift_component_factory<spawn_source<format::csv::reader>>()},
       {"spawn source json",
        lift_component_factory<
-         spawn_source<format::json::reader<format::json::default_selector>,
-                      defaults::import::json>>()},
+         spawn_source<format::json::reader<format::json::default_selector>>>()},
 #if VAST_ENABLE_PCAP
       {"spawn source pcap",
-       lift_component_factory<
-         spawn_source<format::pcap::reader, defaults::import::pcap>>()},
+       lift_component_factory<spawn_source<format::pcap::reader>>()},
 #endif
       {"spawn source suricata",
        lift_component_factory<
-         spawn_source<format::json::reader<format::json::suricata_selector>,
-                      defaults::import::suricata>>()},
+         spawn_source<format::json::reader<format::json::suricata_selector>>>()},
       {"spawn source syslog",
-       lift_component_factory<
-         spawn_source<format::syslog::reader, defaults::import::syslog>>()},
+       lift_component_factory<spawn_source<format::syslog::reader>>()},
       {"spawn source test",
-       lift_component_factory<
-         spawn_source<format::test::reader, defaults::import::test>>()},
+       lift_component_factory<spawn_source<format::test::reader>>()},
       {"spawn source zeek",
-       lift_component_factory<
-         spawn_source<format::zeek::reader, defaults::import::zeek>>()},
+       lift_component_factory<spawn_source<format::zeek::reader>>()},
       {"spawn source zeek-json",
        lift_component_factory<
-         spawn_source<format::json::reader<format::json::zeek_selector>,
-                      defaults::import::zeek>>()},
+         spawn_source<format::json::reader<format::json::zeek_selector>>>()},
       {"spawn sink pcap", lift_component_factory<spawn_pcap_sink>()},
       {"spawn sink zeek", lift_component_factory<spawn_zeek_sink>()},
       {"spawn sink csv", lift_component_factory<spawn_csv_sink>()},

libvast/src/system/spawn_sink.cpp

@@ -14,9 +14,8 @@
 #include "vast/system/spawn_sink.hpp"
 
 #include "vast/config.hpp"
-#include "vast/defaults.hpp"
+#include "vast/error.hpp"
 #include "vast/format/writer.hpp"
-#include "vast/format/zeek.hpp"
 #include "vast/system/sink.hpp"
 #include "vast/system/spawn_arguments.hpp"
 
@@ -57,8 +56,6 @@ spawn_generic_sink(caf::local_actor* self, spawn_arguments& args,
 caf::expected<caf::actor>
 spawn_pcap_sink([[maybe_unused]] caf::local_actor* self,
                 [[maybe_unused]] spawn_arguments& args) {
-  using defaults_t = defaults::export_::pcap;
-  std::string category = defaults_t::category;
 #if !VAST_ENABLE_PCAP
   return caf::make_error(ec::unspecified, "not compiled with pcap support");
 #else // VAST_ENABLE_PCAP

libvast/test/format/pcap.cpp

@@ -73,7 +73,7 @@ TEST(PCAP read/write 1) {
   // Initialize a PCAP source with no cutoff (-1), and at most 5 flow table
   // entries.
   caf::settings settings;
-  caf::put(settings, "vast.import.pcap.read", artifacts::traces::nmap_vsn);
+  caf::put(settings, "vast.import.read", artifacts::traces::nmap_vsn);
   caf::put(settings, "vast.import.pcap.cutoff", static_cast<uint64_t>(-1));
   caf::put(settings, "vast.import.pcap.max-flows", static_cast<size_t>(5));
   // A non-positive value disables the timeout. We need to do this because the
@@ -104,7 +104,7 @@ TEST(PCAP read/write 1) {
     CHECK_VARIANT_EQUAL((*community_id_column)[row], community_ids[row]);
   MESSAGE("write out read packets");
   auto file = "vast-unit-test-nmap-vsn.pcap";
-  caf::put(settings, "vast.export.pcap.write", file);
+  caf::put(settings, "vast.export.write", file);
   format::pcap::writer writer{settings};
   auto deleter = caf::detail::make_scope_guard([&] { rm(file); });
   REQUIRE_EQUAL(writer.write(slice), caf::none);
@@ -114,7 +114,7 @@ TEST(PCAP read/write 2) {
   // Spawn a PCAP source with a 64-byte cutoff, at most 100 flow table entries,
   // with flows inactive for more than 5 seconds to be evicted every 2 seconds.
   caf::settings settings;
-  caf::put(settings, "vast.import.pcap.read", artifacts::traces::nmap_vsn);
+  caf::put(settings, "vast.import.read", artifacts::traces::nmap_vsn);
   caf::put(settings, "vast.import.pcap.cutoff", static_cast<uint64_t>(64));
   caf::put(settings, "vast.import.pcap.max-flows", static_cast<size_t>(100));
   caf::put(settings, "vast.import.pcap.max-flow-age", static_cast<size_t>(5));
@@ -139,7 +139,7 @@ TEST(PCAP read/write 2) {
   CHECK_EQUAL(layout.name(), "pcap.packet");
   MESSAGE("write out read packets");
   auto file = "vast-unit-test-workshop-2011-browse.pcap";
-  caf::put(settings, "vast.export.pcap.write", file);
+  caf::put(settings, "vast.export.write", file);
   format::pcap::writer writer{settings};
   auto deleter = caf::detail::make_scope_guard([&] { rm(file); });
   REQUIRE_EQUAL(writer.write(slice), caf::none);

libvast/test/format/zeek.cpp

@@ -396,7 +396,7 @@ FIXTURE_SCOPE(zeek_writer_tests, writer_fixture)
 TEST(zeek writer) {
   // Perform the writing.
   caf::settings options;
-  caf::put(options, "vast.export.zeek.write", directory.str());
+  caf::put(options, "vast.export.write", directory.str());
   format::zeek::writer writer{options};
   for (auto& slice : zeek_conn_log)
     if (auto err = writer.write(slice))