fix(build): USE_BUNDLED_DRIVER=OFF allows userspace to be built correctly

[Molter73]

trafficstars

What type of PR is this?

/kind bug

Any specific area of the project related to this PR?

/area build

What this PR does / why we need it: Configuring the project with -DUSE_BUNDLED_DRIVER=OFF fails to compile with the following message:

/libs/userspace/libscap/scap_engine_util.c:25:10: fatal error: driver_config.h: No such file or directory
   25 | #include "driver_config.h"
      |          ^~~~~~~~~~~~~~~~~
compilation terminated.
make[2]: *** [libscap/CMakeFiles/scap_engine_util.dir/build.make:76: libscap/CMakeFiles/scap_engine_util.dir/scap_engine_util.c.o] Error 1
make[2]: *** Waiting for unfinished jobs....

This PR moves the USE_BUNDLED_DRIVER into the driver configuration so that needed files can be configured and copied to the build directories, but stops before the driver targets are defined.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE

[poiana]

Hi @Molter73. Thanks for your PR.

I'm waiting for a falcosecurity member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[poiana]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Molter73 To complete the pull request process, please assign ldegio after the PR has been reviewed. You can assign the PR to them by writing /assign @ldegio in a comment when ready.

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

Needs approval from an approver in each of these files:

• OWNERS

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

[FedeDP]

/cc @leogr

[Molter73]

@leogr Gotcha! Sorry I was a little hasty with the fix, let me know if I can help getting this sorted out or tested more thoroughly!

[poiana]

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]

/remove-lifecycle stale

I do think that being able to build the userspace components on their own is a valuable thing.

[FedeDP]

I think this fix makes sense, actually. We need to be able to build userspace without build drivers; i really like the patch because it is small and simple. @leogr can you take another look? :)

[FedeDP]

/milestone 0.11.0

[poiana]

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]

/remove-lifecycle stale

I'd still like something like this to be added, there's value in being able to build the userspace components without the driver IMO.

[hhoffstaette]

I'd still like something like this to be added, there's value in being able to build the userspace components withour the driver IMO.

:+1: It's not just "value" - this problem has been preventing me from updating the Gentoo build to anything >0.29.3 where we build the driver separately with our kmod utilities.

[leogr]

/ok-to-test

[FedeDP]

@leogr i agree with you! We already have a BUILD_DRIVER option though: https://github.com/falcosecurity/libs/blob/master/driver/CMakeLists.txt#L31

[Molter73]

If it's a naming problem, I'll circle back around to this ASAP and find a better name, maybe SKIP_DRIVER_BUILD or something of the sort.

Unfortunately, BUILD_DRIVER seems to actually mean BUILD_KERNEL_MODULE and renaming it is probably more painful than anything else.

[Molter73]

I also need to rebase the PR because it's really old and it's missing a bunch of required CI steps.

[Molter73]

After putting some more thought into this, building the userspace alone can be achieved by letting USE_BUNDLED_DRIVER=ON and setting BUILD_DRIVER=OFF and BUILD_BPF=OFF.

I still find it weird that setting USE_BUNDLED_DRIVER=OFF makes compilation fail because driver_config.h is not generated and I don't get why it works when compiling falco as a whole.

[FedeDP]

/milestone 0.12.0

[leogr]

Hey @Molter73

I apologize this is still blocked after so long time. Sorry :pray:

I still find it weird that setting USE_BUNDLED_DRIVER=OFF makes compilation fail because driver_config.h is not generated and I don't get why it works when compiling falco as a whole.

Although I agree it's weird, I still find it consistent. Let me explain:

USE_BUNDLED_DRIVER=OFF do not use the bundled driver, however, the user still has to provide one. So it will fail if no driver code is provided. Likewise, USE_BUNDLED_DEPS=OFF will fail if the required deps are not found in the system.

In Falco, it works because Falco cmake pulls the driver source code separately :point_down: https://github.com/falcosecurity/falco/blob/master/cmake/modules/driver.cmake

That's the original purpose of having USE_BUNDLED_DRIVER=OFF. Indeed, "USE_BUNDLED_DRIVER=OFF allows userspace to be built correctly" is already true if we add "only if the driver code is provided by other means"

In the end, the real point is that USE_BUNDLED_DRIVER=OFF is NOT meant for disabling the driver compilation.

Thus, I think :point_down: is consistent with the current design.

BUILD_DRIVER=OFF and BUILD_BPF=OFF.

Does it make sense? Or am I missing something? :thinking:

Now, I still see a UX issue that we should address. Thinking out loud, the options would be:

• document that better?
• rename cmake options if they are confusing?
• introduce a shortcut to set both BUILD_DRIVER=OFF and BUILD_BPF=OFF?

[Molter73]

Hi @leogr,

I agree the wording is a bit weird and we need to do something to address it, but it is probably better documenting our options. I'm closing this PR and promise to revisit documentation/a shortcut for skipping both drivers sometime in the near future.