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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gRPC server and gRPC outputs service #822

Merged
merged 76 commits into from Sep 25, 2019
Merged

gRPC server and gRPC outputs service #822

merged 76 commits into from Sep 25, 2019

Conversation

@leodido
Copy link
Member

leodido commented Sep 6, 2019

What type of PR is this?

/kind feature

Any specific area of the project related to this PR?

NONE

What this PR does / why we need it:

This PR adds a gRPC server to falco in its own thread.
Implementors can use it to create and extend the falco API.
The only implemented call by this PR is the subscribe call that can be used to subscribe to falco output events directly.

The code this PR contains refers to the initial proposal but implements a streaming server rather than a bidirectional client/server paradigm.

Which issue(s) this PR fixes:

  • Original proposal: #789
  • Fixes #528
  • Fixes #791
  • Refs to #818 (umbrella issue for various related topics)

Special notes for your reviewer:

This PR is still in WIP. Me and @fntlnz are still working on it.

/assign @leodido
/assign @fntlnz

Work we still need to do:

  • Implement abstract structure to handle various kind of gRPC services
  • Implement streaming service - ie., subscribe
  • Get the outputs through handle_event in falco_outputs.cpp
  • Separate the sending of events from the sending of messages in Lua too
  • Proto definitions
  • Mutual TLS authentication
  • Make it work for k8s_audit
  • Source is not showed in the outputs, investigate why
  • Make sure all the threads have their cleaning logic (signal handlers etc..)
  • Make all the parameters configurable
    • certificates (private key, root cert etc.)
    • the grpc server
      • enable
      • threadiness
      • bind address + bind port
    • the grpc output itself
  • Do some name and file refactoring, right now everything is in grpc_server.cpp and grpc_server.h, we can do better
  • Go SDK
    • Write the Go library
    • Usage examples

Next items for which we need to open issues:

  • Unit tests
  • Integration tests (the same way are done for other features?)
  • Add tags in the response struct for falco_outputs.proto (this also requires a client change)
  • Catch gRPC status and logs (?) (for example when input certificates are wrong it segfaults, handle it)
  • Documentation (issue falcosecurity/falco-website#80)
    • Falco gRPC server configuration and usage
    • Falco gRPC outputs configuration and usage
    • gRPC API documentation
    • How to generate the certificates for mutual TLS
    • SDK examples
  • Implement other output queuing mechanism, right now we only support a shared queue in a round robin fashion (issue opened #857)

Some notes about this implementation:

  • We feel like it is not PoC quality even if it's a PoC, needs some refactoring but overall we think the design is solid enough. It's even thread safe thanks to TBB; Lots of things happened since this.
  • When multiple clients Subscribe to the outputs, messages are dispatched in a Round Robin fashion. Single queue, multiple consumers - We can have other strategies here, like a queue for every consumer (fanout) or even more sophisticated things. This first approach seems good enough now, all future approaches will be "configurable" so, we are open to different strategies but this is the default one for now

Does this PR introduce a user-facing change?:

Falco gRPC api server implementation, contains a subscribe method to subscribe to outputs from any gRPC capable language
fntlnz and others added 30 commits Sep 2, 2019
Signed-off-by: Lorenzo Fontana <lo@linux.com>
Co-Authored-by: Leonardo Di Donato <leodidonato@gmail.com>
Co-Authored-By: Leonardo Di Donato <leodidonato@gmail.com>
Signed-off-by: Lorenzo Fontana <lo@linux.com>
Co-Authored-By: Leonardo Di Donato <leodidonato@gmail.com>
Signed-off-by: Lorenzo Fontana <lo@linux.com>
Co-authored-by: Lorenzo Fontana <lo@linux.com>
Signed-off-by: Leonardo Di Donato <leodidonato@gmail.com>
Co-authored-by: Lorenzo Fontana <lo@linux.com>
Signed-off-by: Leonardo Di Donato <leodidonato@gmail.com>
Co-authored-by: Lorenzo Fontana <lo@linux.com>
Signed-off-by: Leonardo Di Donato <leodidonato@gmail.com>
Co-authored-by: Lorenzo Fontana <lo@linux.com>
Signed-off-by: Leonardo Di Donato <leodidonato@gmail.com>
Co-authored-by: Lorenzo Fontana <lo@linux.com>
Signed-off-by: Leonardo Di Donato <leodidonato@gmail.com>
Signed-off-by: Leonardo Di Donato <leodidonato@gmail.com>
Co-authored-by: Lorenzo Fontana <lo@linux.com>
Signed-off-by: Leonardo Di Donato <leodidonato@gmail.com>
Co-authored-by: Lorenzo Fontana <lo@linux.com>
Signed-off-by: Leonardo Di Donato <leodidonato@gmail.com>
…am context classes

Co-authored-by: Lorenzo Fontana <lo@linux.com>
Signed-off-by: Leonardo Di Donato <leodidonato@gmail.com>
…ess stream macro

Co-authored-by: Lorenzo Fontana <lo@linux.com>
Signed-off-by: Leonardo Di Donato <leodidonato@gmail.com>
Co-authored-by: Lorenzo Fontana <lo@linux.com>
Signed-off-by: Leonardo Di Donato <leodidonato@gmail.com>
Signed-off-by: Leonardo Di Donato <leodidonato@gmail.com>
…andled for threads

Signed-off-by: Leonardo Di Donato <leodidonato@gmail.com>
Co-authored-by: Lorenzo Fontana <lo@linux.com>
Signed-off-by: Leonardo Di Donato <leodidonato@gmail.com>
Co-Authored-By: Leonardo Di Donato <leodidonato@gmail.com>
Signed-off-by: Lorenzo Fontana <lo@linux.com>
Co-Authored-By: Leonardo Di Donato <leodidonato@gmail.com>
Signed-off-by: Lorenzo Fontana <lo@linux.com>
Co-Authored-By: Leonardo Di Donato <leodidonato@gmail.com>
Signed-off-by: Lorenzo Fontana <lo@linux.com>
Co-Authored-By: Leonardo Di Donato <leodidonato@gmail.com>
Signed-off-by: Lorenzo Fontana <lo@linux.com>
…palive

Co-Authored-By: Leonardo Di Donato <leodidonato@gmail.com>
Signed-off-by: Lorenzo Fontana <lo@linux.com>
…ing mechanisms, only round robin fashion is implemented now

Co-Authored-By: Leonardo Di Donato <leodidonato@gmail.com>
Signed-off-by: Lorenzo Fontana <lo@linux.com>
Co-authored-by: Lorenzo Fontana <lo@linux.com>
Signed-off-by: Leonardo Di Donato <leodidonato@gmail.com>
Co-authored-by: Lorenzo Fontana <lo@linux.com>
Signed-off-by: Leonardo Di Donato <leodidonato@gmail.com>
Co-Authored-by: Lorenzo Fontana <lo@linux.com>
Signed-off-by: Leonardo Di Donato <leodidonato@gmail.com>
Co-authored-by: Lorenzo Fontana <lo@linux.com>
Signed-off-by: Leonardo Di Donato <leodidonato@gmail.com>
Co-authored-by: Lorenzo Fontana <lo@linux.com>
Signed-off-by: Leonardo Di Donato <leodidonato@gmail.com>
Co-authored-by: Lorenzo Fontana <lo@linux.com>
Signed-off-by: Leonardo Di Donato <leodidonato@gmail.com>
…puts using different lua functions

Co-authored-by: Lorenzo Fontana <lo@linux.com>
Signed-off-by: Leonardo Di Donato <leodidonato@gmail.com>
@mfdii

This comment has been minimized.

Copy link
Member

mfdii commented Sep 23, 2019

/lgtm

fntlnz and others added 2 commits Sep 24, 2019
Co-Authored-By: Leonardo Di Donato <leodidonato@gmail.com>
Signed-off-by: Lorenzo Fontana <lo@linux.com>
Co-Authored-By: Leonardo Di Donato <leodidonato@gmail.com>
Signed-off-by: Lorenzo Fontana <lo@linux.com>
@poiana poiana removed the lgtm label Sep 24, 2019
fntlnz and others added 4 commits Sep 24, 2019
Co-Authored-By: Leonardo Di Donato <leodidonato@gmail.com>
Signed-off-by: Lorenzo Fontana <lo@linux.com>
Co-Authored-By: Leonardo Di Donato <leodidonato@gmail.com>
Signed-off-by: Lorenzo Fontana <lo@linux.com>
Co-Authored-By: Leonardo Di Donato <leodidonato@gmail.com>
Signed-off-by: Lorenzo Fontana <lo@linux.com>
Co-authored-by: Lorenzo Fontana <lo@linux.com>

Signed-off-by: Leonardo Di Donato <leodidonato@gmail.com>
Copy link
Contributor

mstemm left a comment

To me I think the biggest suggestion would be to more clearly separate the notion of a grpc server within falco and the grpc service that passes along outputs. I'd like to build on this to add conversion from k8s psps to falco rules (see #826). Currently it's done at the command line but I could imagine it would be useful to do this via grpc as well.

Also, where are the tests? :)

userspace/falco/output.proto Show resolved Hide resolved
userspace/falco/lua/output.lua Show resolved Hide resolved
userspace/falco/grpc_server.cpp Outdated Show resolved Hide resolved
leodido and others added 3 commits Sep 25, 2019
Co-authored-by: Lorenzo Fontana <lo@linux.com>
Signed-off-by: Leonardo Di Donato <leodidonato@gmail.com>
Co-authored-by: Lorenzo Fontana <lo@linux.com>
Signed-off-by: Leonardo Di Donato <leodidonato@gmail.com>
Co-authored-by: Lorenzo Fontana <lo@linux.com>
Signed-off-by: Leonardo Di Donato <leodidonato@gmail.com>
@fntlnz

This comment has been minimized.

Copy link
Member

fntlnz commented Sep 25, 2019

@mstemm I agree that this is lacking tests, at the planning meetings we agreed to do them in a subsequent iteration over this code to let everyone start integrating with this as soon as possible while we work. It's in the "Next action items" section in the PR description

@leodido leodido changed the title wip: gRPC server and gRPC outputs service gRPC server and gRPC outputs service Sep 25, 2019
@fntlnz
fntlnz approved these changes Sep 25, 2019
@poiana

This comment has been minimized.

Copy link

poiana commented Sep 25, 2019

LGTM label has been added.

Git tree hash: ab296d4bb2d66cff5c0bc7f4eb4680e06b600c46

@poiana poiana added the lgtm label Sep 25, 2019
@poiana

This comment has been minimized.

Copy link

poiana commented Sep 25, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fntlnz, kris-nova, mfdii

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@leodido leodido merged commit c0721b3 into dev Sep 25, 2019
3 of 4 checks passed
3 of 4 checks passed
tide Not mergeable. Needs lgtm label.
Travis CI - Branch Build Passed
Details
Travis CI - Pull Request Build Passed
Details
dco All commits have Signed-off-by
Details
@poiana poiana deleted the feat/grpc-server-poc branch Sep 25, 2019
@kris-nova kris-nova mentioned this pull request Sep 25, 2019
0 of 2 tasks complete
@fntlnz fntlnz added this to the 0.18.0 milestone Oct 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.