Add initial REST API server for VAST #2567

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.

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

Merged

lava merged 29 commits into master from story/sc-36675/rest-endpoint

Sep 20, 2022

Member

lava commented Sep 7, 2022

📝 Reviewer Checklist

Review this pull request by ensuring the following items:

All user-facing changes have changelog entries
User-facing changes are reflected on vast.io

lava force-pushed the story/sc-36675/rest-endpoint branch from 314ea93 to aa2815e Compare

September 7, 2022 13:58

lava commented

View reviewed changes

plugins/rest/src/plugin.cpp Outdated Show resolved Hide resolved

lava commented

View reviewed changes

plugins/rest/CMakeLists.txt Outdated Show resolved Hide resolved

lava commented

View reviewed changes

plugins/rest/www/status.html Outdated Show resolved Hide resolved

lava commented

View reviewed changes

libvast/native-plugins/rest_api/status.cpp Outdated Show resolved Hide resolved

lava commented

View reviewed changes

plugins/rest/src/server_command.cpp Outdated Show resolved Hide resolved

mavam reviewed

View reviewed changes

libvast/native-plugins/rest_api/export.cpp Outdated Show resolved Hide resolved

lava force-pushed the story/sc-36675/rest-endpoint branch 3 times, most recently from 5994a54 to 118c144 Compare

September 12, 2022 14:43

lava added the enhancement label

lava force-pushed the story/sc-36675/rest-endpoint branch from 118c144 to e42a965 Compare

September 12, 2022 14:50

lava marked this pull request as ready for review

September 12, 2022 14:58

dominiklohmann reviewed

View reviewed changes

Member

dominiklohmann left a comment

I looked at the code more in depth and left a bunch of comments on things that I think should be looked at.

There are no major issues with the code. It'd be nice if the REST endpoints didn't have to implement a whole actor interface, but I don't know how feasible that actually is.

The biggest remaining piece of work is probably documentation, both for the usage of the REST plugin and the plugin API of the REST endpoints.

I also gave this a quick spin locally, and it looks like it's working fine. Once we addressed the remaining comments I think we can quickly approve and merge this. I'm also really interesting in seeing how this integrates with #2574.

Dockerfile Show resolved Hide resolved

libvast/include/vast/fwd.hpp Outdated Show resolved Hide resolved

plugins/rest/src/server_command.cpp Outdated Show resolved Hide resolved

plugins/rest/src/server_command.cpp Outdated Show resolved Hide resolved

plugins/rest/src/generate_token_command.cpp Outdated Show resolved Hide resolved

plugins/rest/src/plugin.cpp Outdated Show resolved Hide resolved

plugins/rest/CMakeLists.txt Outdated Show resolved Hide resolved

plugins/rest/CMakeLists.txt Outdated Show resolved Hide resolved

plugins/rest/include/rest/configuration.hpp Outdated Show resolved Hide resolved

libvast/native-plugins/rest_api/export.cpp Outdated Show resolved Hide resolved

dominiklohmann reviewed

View reviewed changes

plugins/rest/CMakeLists.txt Outdated Show resolved Hide resolved

lava force-pushed the story/sc-36675/rest-endpoint branch 2 times, most recently from 8ef092e to 724306d Compare

September 15, 2022 12:49

lava changed the title ~~Add initial REST API for VAST~~ Add initial REST API server for VAST

lava force-pushed the story/sc-36675/rest-endpoint branch from 274ee4f to bb3448c Compare

September 15, 2022 16:01

netantho reviewed

View reviewed changes

plugins/web/CHANGELOG.md Outdated Show resolved Hide resolved

lava force-pushed the story/sc-36675/rest-endpoint branch 2 times, most recently from d6d8796 to da490e5 Compare

September 17, 2022 00:29

dominiklohmann approved these changes

View reviewed changes

Member

dominiklohmann left a comment

We peer-reviewed this. Approving this as-is modulo the remaining comments. We have a few things left to do in follow-ups.

.github/workflows/vast.yaml Show resolved Hide resolved

.github/workflows/vast.yaml Outdated Show resolved Hide resolved

.github/workflows/vast.yaml Outdated Show resolved Hide resolved

libvast/include/vast/type.hpp Outdated Show resolved Hide resolved

libvast/include/vast/http_api.hpp Outdated Show resolved Hide resolved

libvast/native-plugins/rest_endpoints/export.cpp Outdated Show resolved Hide resolved

libvast/native-plugins/rest_endpoints/export.cpp Outdated Show resolved Hide resolved

libvast_test/src/node.cpp Outdated Show resolved Hide resolved

libvast/src/system/spawn_catalog.cpp Outdated Show resolved Hide resolved

lava added 7 commits

September 19, 2022 19:21


          Add function to get all plugins of a given type

ae5623a


          Add an 'inspect()' overload for record_type

0b874d2


          Add integration test step keyword to run commands without prepending …

b126715

…'vast'


          Add a REST API plugin

c606022

This polishes the results of a recent hackathon by pulling some
high-level support for request handling into libvast, enabling
closed-source plugins to also provide routes.


          Update REST API with example endpoints

783f014

Large parts of this commit were done while pairing
during the latest Tenzir Hackathon.

Co-Authored-By: @patszt
Co-Authored-By: @mavam


          Clean up REST code and fix open issues

47af329


          Add localhost TLS certificate for testing

a55cad1

Generated using `python3 -m trustme`.

lava added 20 commits

September 19, 2022 19:21


          Add unit and integration tests

0e859cf


          Address review feedback

dc84719


          Take out unfinished features

02427a7


          Add a new parameter to status endpoint

7309cd3


          Update node fixture

af28eaf


          Rename plugin from rest/ to web/

1df05f9


          Use older simdjson API

f11f4f4


          Fix warnings in fwd header

474c5fa


          Adjust workflow definitions

5b0eca0


          Further fixes for -Wpedantic

4d8b243


          Depend on openssl explicitly

952845c


          Update API spec

f5e12d3


          Add an option to run the catalog non-detached

bb81e4a

Running the catalog detached hides it from the `test_coordinator`,
which introduces race conditions in all tests that need to wait
for a catalog result.

Additionally, introduce a missing error handler in the index
that would lead to silent self-destruction of the index.


          Add missing error handler

Add a missing error handler in the index that would lead to
silent self-destruction if the catalog returns an error or
wasn't spawned.


          Add additional error messages

c03e711

Without these, failure to start the server will
be silen when run as a 'start command'.


          Fix parameter access again

f5324d3

This used to be updated in an earlier commit, but was
accidentally reverted.


          Update integration tests to account for changes

59a77c1


          Fix unit test parameters

a2856b9


          Fix typo in changelog

0044aa9


          Adjust cmake formatting

31d9517

lava force-pushed the story/sc-36675/rest-endpoint branch from 2abbb3e to 6d2af8e Compare

September 19, 2022 17:22


          Address review feedback

2e25017

lava force-pushed the story/sc-36675/rest-endpoint branch from 6d2af8e to 2e25017 Compare

September 19, 2022 19:07


          Move web plugin build into separate job

1b04ff5

lava merged commit 68a3ab1 into master

lava deleted the story/sc-36675/rest-endpoint branch

September 20, 2022 14:13

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet