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

falco-driver-loader improvements proposal #1125

Closed
1 of 5 tasks
leogr opened this issue Mar 30, 2020 · 2 comments · Fixed by #1158, #1160 or #1200
Closed
1 of 5 tasks

falco-driver-loader improvements proposal #1125

leogr opened this issue Mar 30, 2020 · 2 comments · Fixed by #1158, #1160 or #1200

Comments

@leogr
Copy link
Member

leogr commented Mar 30, 2020

Motivation

As part of the ongoing discussions about the scope of Falco artifacts we need an officially supported tool for building, downloading, and installing various drivers. Currently, the falco-driver-loader takes care of that, and it is already included in the new stable binary distribution.

Furthermore, the source of falco-driver-loader needs to stay into the Falco repo, so it can easily be kept in sync with Falco and the right parameters can be substituted into the script during the building process. Also, having it in the same place helps with maintenance.

That being said, there are other tools that need to perform similar actions: for example falcoctl installs the kernel module by replicating some of the logic already included in falco-driver-loader. Moreover, the installation logic can change over time and we will need a way to version this process too.

For these reasons, I would like to propose several improvements in order to make it usable by other tools and avoid duplicating the same logic in other places.

Feature

  • Make explicit that falco-driver-loader is versioned alongside Falco (mostly a documentation issue)
  • Do not unload the driver if is already present and the version matches (needed to avoid problems with manually loaded drivers)
  • Distribute another artifact that includes just falco-driver-loader so it can be quickly downloaded and used by both users and other tools (ie. falcoctl), the artifact will be versioned as same as Falco 👉 falco-driver-loader also as separate artifact #1159
  • Add a simple interface that allows executing single actions (ie. only download, only install, or only build) so it can be easily used by other tools (ie. falcoctl)
  • Update falco-driver-loader in order to download artifact from https://bintray.com/falcosecurity

Alternatives

Additional context

@leodido
Copy link
Member

leodido commented Mar 30, 2020

Thanks @leogr for reporting here the discussion we had this morning.

Let me add some considerations, on top of what you reported here :)

  1. Since falco-driver-loader is part of the Falco output artifact it seems obvious to me that it has the same version. I'd not advertise it as a separate thing, honestly. And documenting its version explicitly would go in that direction. But I'm open to change my mind. :)

  2. With PR Falco version and driver version are not coupled anymore #1111 we completed the process of uncoupling the Falco version from the driver(s) version.
    This means that the drivers will be located into usr/src/falco-<driver_version> from now on. The <driver_version> is the value here.
    Furthermore, the falco-driver-loader contains DRIVER_VERSION=<driver_version> variable, and we will update it to unload the driver IFF the driver loaded (kernel module or eBPF probe) have a different <driver_version> string.

  3. Do not really agree on this, like I said this morning.
    Assuming you downloaded the binary distribution, you can directly obtain the script (artifact) from it as follows:

    tar -zxvf falco-0.21.0-20+6834649-x86_64.tar.gz falco-0.21.0-20+6834649-x86_64/usr/bin/falco-probe-loader

    Feel free to add another step to the release system to push it separately in another (generic) bintray repo anyway. I just don't think the effort is worth since we already have such artifact pushed :)

  4. Totally agree, this is the first and most important upgrade we have to do to this falco-driver-loader script

I'd add:

  1. Point (4) can be unblocked as soon we have the drivers build grid in place (main PR)

Anyway, I'm happy to know that we agree on the fact that Falco cannot depend on other tools for this core feature (on the installation side). Other tools (eg., falcoctl) can rather wrap this feature and extend it.

An example.

falcoctl can wrap the falco-driver-loader mechanism and when it fails it can use driverkit kubernetes ..., or better its libraries, to try to build the drivers.

Notice that there would be no need to have a local processor in driverkit since if falco-driver-loader fails to build the drivers locally there is no reason driverkit will be able to do it (given the same environment and context).

@leogr
Copy link
Member Author

leogr commented Mar 31, 2020

Thanks @leodido, btw I've realized right now I forgot to cc you and @fntlnz in the issue, sorry!

Some further considerations:

  1. Since falco-driver-loader is part of the Falco output artifact it seems obvious to me that it has the same version. I'd not advertise it as a separate thing, honestly. And documenting its version explicitly would go in that direction. But I'm open to change my mind. :)

Although that's obvious - I agree on that -, it may be not so clear for users approaching Falco for the first time. For example, one could even try to use the script source from the repo, that's not the intended usage method.
Actually, we should at least well document where the script can be found and how to use it. If we also agree on distributing it as a separate thing (point 3. below), we will also have to document the versioning IMHO.

  1. With PR Falco version and driver version are not coupled anymore #1111 we completed the process of uncoupling the Falco version from the driver(s) version.
    This means that the drivers will be located into usr/src/falco-<driver_version> from now on. The <driver_version> is the value here.
    Furthermore, the falco-driver-loader contains DRIVER_VERSION=<driver_version> variable, and we will update it to unload the driver IFF the driver loaded (kernel module or eBPF probe) have a different <driver_version> string.

Great!

  1. Do not really agree on this, like I said this morning.
    Assuming you downloaded the binary distribution, you can directly obtain the script (artifact) from it as follows:
    tar -zxvf falco-0.21.0-20+6834649-x86_64.tar.gz falco-0.21.0-20+6834649-x86_64/usr/bin/falco-probe-loader

For some use cases, it can happen that tools or users just need the script (few bytes, or just <1MB if drivers sources were included), not the full package (~40MB). For example, the user might:

  • use falcoctl install module on the host machine (the script is needed here)
  • then run Falco in a container (the container image will be downloaded)

In this case, the script is needed, but the binary package is not.
WDYT?

Feel free to add another step to the release system to push it separately in another (generic) bintray repo anyway. I just don't think the effort is worth since we already have such artifact pushed :)

Ok, I will do, unless we really agree that's not useful :)

  1. Totally agree, this is the first and most important upgrade we have to do to this falco-driver-loader script

I'd add:

  1. Point (4) can be unblocked as soon we have the drivers build grid in place (main PR)

Great, again!

Anyway, I'm happy to know that we agree on the fact that Falco cannot depend on other tools for this core feature (on the installation side). Other tools (eg., falcoctl) can rather wrap this feature and extend it.

An example.

falcoctl can wrap the falco-driver-loader mechanism and when it fails it can use driverkit kubernetes ..., or better its libraries, to try to build the drivers.

Notice that there would be no need to have a local processor in driverkit since if falco-driver-loader fails to build the drivers locally there is no reason driverkit will be able to do it (given the same environment and context).

Totally agree, thanks for clarifying that. Once we have made a decision about point 3. above, I will update the issue including your clarifications.

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