Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move options from format to the import subcommand #1354

Merged
merged 9 commits into from Feb 9, 2021

Conversation

tobim
Copy link
Member

@tobim tobim commented Feb 7, 2021

馃摂 Description

The options listen, read, schema, schema-file, type, and uds can from now on be supplied to the import command directly. They can still be used after the format subcommand, but that usage is deprecated.

馃摑 Checklist

  • All user-facing changes have changelog entries.
  • The changes are reflected on docs.tenzir.com/vast, if necessary.
  • The PR description contains instructions for the reviewer, if necessary.

馃幆 Review Instructions

By commit.

@tobim tobim added the maintenance Tasks for keeping up the infrastructure label Feb 7, 2021
@tobim tobim requested a review from a team February 7, 2021 23:17
@tobim tobim changed the title Story/ch22397/streamline options Move options from format to the import subcommand Feb 7, 2021
@tobim
Copy link
Member Author

tobim commented Feb 7, 2021

I guess it would be a good idea to do the same for the sink options write and uds.

Copy link
Member

@dominiklohmann dominiklohmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quoting myself from the story:

I have another thought on this: Consider this file:

vast:
  import:
    pcap:
      interface: en0
    read: path/to/file.pcap

What does vast import pcap do in this case?

Similarly, consider this currently valid file:

vast:
  import:
    suricata:
      read: path/to/eve.json
    zeek
      read: path/to/conn.log

How would you express this with your proposed changes?

I did not look at the code yet, but I would like to see these questions answered before we merge a change like this.

@tobim
Copy link
Member Author

tobim commented Feb 8, 2021

Quoting myself from the story:

I have another thought on this: Consider this file:

vast:
  import:
    pcap:
      interface: en0
    read: path/to/file.pcap

This is not different from

vast:
  import:
    pcap:
      interface: en0
      read: path/to/file.pcap

The pcap implementation prefers the interface key in both cases.

Similarly, consider this currently valid file:

vast:
  import:
    suricata:
      read: path/to/eve.json
    zeek
      read: path/to/conn.log

How would you express this with your proposed changes?

While valid, the utility of such a config is extremely limited. If we really want to allow users to specify their imports declaratively they should either have a separate config file per import process or this has to become a list in a unified config file.

@dominiklohmann
Copy link
Member

Reasoning checks out. I think it'd be nice if this same PR can also make the changes on the export side so it stays symmetrical.

@tobim tobim force-pushed the story/ch22397/streamline-options branch from 52a2b99 to 34efc36 Compare February 8, 2021 14:32
@tobim
Copy link
Member Author

tobim commented Feb 8, 2021

Reasoning checks out. I think it'd be nice if this same PR can also make the changes on the export side so it stays symmetrical.

Will do after the rebase on top of #1356.

@tobim tobim force-pushed the story/ch22397/streamline-options branch 2 times, most recently from c7b0df0 to 87013c3 Compare February 8, 2021 20:20
The options `listen`, `read`, `schema`, `schema-file`, `type`, and
`uds` can from now on be supplied to the `import` command directly.
They can still be used after the format subcommand, but that usage
is deprecated.
@tobim tobim force-pushed the story/ch22397/streamline-options branch from 87013c3 to d686396 Compare February 8, 2021 20:20
libvast/src/command.cpp Outdated Show resolved Hide resolved
libvast/src/command.cpp Outdated Show resolved Hide resolved
libvast/src/command.cpp Show resolved Hide resolved
tobim and others added 2 commits February 9, 2021 08:57
Co-authored-by: Dominik Lohmann <mail@dominiklohmann.de>
@tobim tobim merged commit 0e4bbe8 into master Feb 9, 2021
@tobim tobim deleted the story/ch22397/streamline-options branch February 9, 2021 09:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Tasks for keeping up the infrastructure
Projects
None yet
2 participants