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 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:

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 gRPC server and client documentation 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 gRPC outputs: fanout queuing mechanism #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 September 2, 2019 18:38
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
Copy link
Member

mfdii commented Sep 23, 2019

/lgtm

fntlnz and others added 2 commits September 24, 2019 15:33
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 September 24, 2019 15:41
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 mstemm left a comment

Choose a reason for hiding this comment

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

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 September 25, 2019 09:19
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
Copy link
Contributor

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
@poiana
Copy link
Contributor

poiana commented Sep 25, 2019

LGTM label has been added.

Git tree hash: ab296d4bb2d66cff5c0bc7f4eb4680e06b600c46

@poiana poiana added the lgtm label Sep 25, 2019
@poiana
Copy link
Contributor

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

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

Successfully merging this pull request may close these issues.

Review the Falco signal handler ROADMAP: Improved Falco Outputs
7 participants