search

falcosecurity / libs

libsinsp, libscap, the kernel module driver, and the eBPF driver sources
https://falcosecurity.github.io/libs/
Apache License 2.0
222 stars 162 forks source link

Write automated tests based on `sinsp-example` #271

Closed Molter73 closed 1 year ago

Molter73 commented 2 years ago

Motivation

In an effort to improve the reliability of the repository, I think it would be a good idea if we wrote automated tests that could (hopefully one day) run as part of the PR checks.

Feature

There are many variations of tests we could create, but particularly I've been playing around with the idea of using a containerized version of sinsp-example to capture events triggered from scripts and validating the logs generated, you can check out my PoC here. The TL;DR of it is:

This approach, even if rough at this point, combined with the ability to spawn additional containers from the test itself, should allow us to implement any number of tests we want to test different functionalities of the libraries as a whole, some examples I can think of:

Alternatives

It's been suggested to use gtest or catch2 as the driver for the tests themselves, my PoC simply used pytest because of my familiarity with it and Python just seemed the easiest way to achieve my goal. I think it would be worth it to compare the amount of boilerplate/extra code needed, as well as the level of effort needed to implement a similar concept with either of those frameworks.

Additionally, in order to make this testing approach a little easier, I think implementing a flag in sinsp-example for it to output JSON would come a long way into making parsing its logs a lot easier. Furthermore, we could implement a JSON schema for those events and validate all events generated against it, (including the events that don't match the ones we are looking for in the test), which would help us ensure all fields have a valid format and are not somehow corrupt.

Additional context

Because right now the only check needed to merge a change is for the libs to compile, it's relatively easy for bugs to slip past into master. Adding any sort of testing would go a long way into preventing some of them and this is the broadest, yet simple approach I could come up with for testing multiple components at once.

poiana commented 2 years ago

Issues go stale after 90d of inactivity.

Mark the issue as fresh with /remove-lifecycle stale.

Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Provide feedback via https://github.com/falcosecurity/community.

/lifecycle stale

Molter73 commented 2 years ago

This issue is about to go rotten, though I would still like to see automated testing implemented in this repo.

I've put some more effort in my PoC repo, adding tests based on the event-generator, running all tests with both the kernel module and the eBPF probe and having sinsp-example compiled with AddressSanitizer for additional memory checks. It is still at a point where we could rewrite it using a language that Falco maintainers are more familiar with like Go or C++, though Python is making it look pretty good to me.

FedeDP commented 2 years ago

/remove-lifecycle stale

FedeDP commented 2 years ago

Moreover, the PoC repo indeed actually helped us spot at least a couple of issues. I :100: agree on having something similar (if not exactly that repo) running in our CI. Once #453 is merged and we have gh actions, perhaps you can try to port your CI here?

Molter73 commented 2 years ago

I've been putting a bit of thought on what it would take to merge the existing tests from my PoC into the libs repo, this is what I think would work best, at least as a first approach that can be refined and tuned afterwards.

First thing would be to move all required new files into the repo (tests, dockerfiles, makefiles), those could live in one of two places:

Because the current CI already builds sinsp-example and drivers for the system it runs on, we could simply add steps to the current workflow to build all the required container images and run the tests. We could also look into splitting the workflow so that the binaries are reused in a separate job, but that can be an improvement after we get a minimal version of this running. It's also worth noting that building sinsp-example using address sanitizer could provide added value, since it would catch wrong memory uses for us, but for a first attempt, we can skip this for now.

The PoC right now uses raw makefiles for building and running, we can probably change it to use CMake instead with custom commands, it might be overkill but it would also mean we can use the same build system with these tests as well.

One thing I would like to get feedback on is how does people feel about adding a bunch of Python into the repository, I could rewrite the tests to use a different language but I find Python is great for quickly prototyping and debugging these type of tests.

As a final note, the PoC only runs tests for Linux on x86 systems. I think getting those merged would be a good first step, we can expand from that to add tests on other platforms and operating systems.

Unless there is any strong push back against this, I'll try to get a draft PR for this as soon as I get a chance, since I feel that might inspire people to be more vocal on this subject 🙂

Andreagit97 commented 2 years ago

I've been putting a bit of thought on what it would take to merge the existing tests from my PoC into the libs repo, this is what I think would work best, at least as a first approach that can be refined and tuned afterwards.

First thing would be to move all required new files into the repo (tests, dockerfiles, makefiles), those could live in one of two places:

  • Because sinsp-example is the main binary under test, we could place all files under userspace/libsinsp/tests/e2e/

  • We could add an e2e-tests (or simply tests) directory in the root of the repository, this might be a little cleaner and could make it easier to add tests based on the files found there for other example binaries like scap-open.

Personally, I prefer this second option and the test folder should be already there if the https://github.com/falcosecurity/libs/pull/484 is merged. I find this approach cleaner when more than one component is involved in the testing phase.

Because the current CI already builds sinsp-example and drivers for the system it runs on, we could simply add steps to the current workflow to build all the required container images and run the tests. We could also look into splitting the workflow so that the binaries are reused in a separate job, but that can be an improvement after we get a minimal version of this running. It's also worth noting that building sinsp-example using address sanitizer could provide added value, since it would catch wrong memory uses for us, but for a first attempt, we can skip this for now.

Having a minimal version already brings a huge value IMHO. big +1 :tada:

The PoC right now uses raw makefiles for building and running, we can probably change it to use CMake instead with custom commands, it might be overkill but it would also mean we can use the same build system with these tests as well.

It would be great to have just a unique build system, but it depends on the effort! If it becomes too complicated I don't see big issues in using plain makefiles for building these tests.

One thing I would like to get feedback on is how does people feel about adding a bunch of Python into the repository, I could rewrite the tests to use a different language but I find Python is great for quickly prototyping and debugging these type of tests.

To be honest, I am not so familiar with python, so I cannot say if it is the best language for doing this, but again I don't see a big issue in using it, if tests are clear and well documented there shouldn't be problems :) @FedeDP @leogr

As a final note, the PoC only runs tests for Linux on x86 systems. I think getting those merged would be a good first step, we can expand from that to add tests on other platforms and operating systems.

Oh yes, it would be great to have something at least for ARM64, but I agree with you we can add it in the following steps.

Unless there is any strong push back against this, I'll try to get a draft PR for this as soon as I get a chance, since I feel that might inspire people to be more vocal on this subject slightly_smiling_face

It would be amazing, we really need tests in the repo and mainly in libsinsp! Thank you for all your effort @Molter73! :clap: :tada:

FedeDP commented 2 years ago

Personally, I prefer this second option and the test folder should be already there if the https://github.com/falcosecurity/libs/pull/484 is merged. I find this approach cleaner when more than one component is involved in the testing phase.

Agree with Andrea!

Having a minimal version already brings a huge value IMHO. big +1 tada

Big :+1: :laughing:

The PoC right now uses raw makefiles for building and running, we can probably change it to use CMake instead with custom commands, it might be overkill but it would also mean we can use the same build system with these tests as well.

Yeah, i'd go with cmake if we can!

One thing I would like to get feedback on is how does people feel about adding a bunch of Python into the repository, I could rewrite the tests to use a different language but I find Python is great for quickly prototyping and debugging these type of tests.

IMHO Falco tests are already written in python; i see no issues in adding some python code in the repo :)

As a final note, the PoC only runs tests for Linux on x86 systems. I think getting those merged would be a good first step, we can expand from that to add tests on other platforms and operating systems.

Arm64 can be the following logical step, should be quite simple to add it

Thank you Mauro!