falco-driver-loader - break apart logic and improvements

[leogr]

What type of PR is this?

Uncomment one (or more) /kind <> lines:

/kind bug

/kind cleanup

/kind design

/kind documentation

/kind failing-test

/kind feature

If contributing rules or changes to rules, please make sure to also uncomment one of the following line:

/kind rule-update

/kind rule-create

Any specific area of the project related to this PR?

Uncomment one (or more) /area <> lines:

/area build

/area engine

/area examples

/area rules

/area integrations

/area tests

/area proposals

What this PR does / why we need it:
See #1193

Which issue(s) this PR fixes:

Fixes #1193
Fixes #1125 (again)

Special notes for your reviewer:

Usage:
  falco-driver-loader [driver] [options]

Available drivers:
  module        kernel module (default)
  bpf           eBPF probe

Options:
  --help        show brief help
  --compile     try to compile the driver locally
  --download    try to download a prebuilt driver

With no options and arguments, it preserves the previous behavior (we should test it deeply).

Notable improvements in messages:

• All info messages start with * Message...
• Success messages with * Success: message...
• Messages without * represent the output of another program or an error
• Verbosity increased

/cc @kris-nova
/cc @leodido
/cc @fntlnz

falco-driver-loader (pre refactoring behavior)

Preflight check:

• if not root, then exit 1
• if not curl, then exit 1

module

Preflight check:

• if not lsmod, then exit 1
• if not modprobe, then exit 1
• if not rmmod, then exit 1

1. Unload module, if present
2. If module still loaded, then exit 0
3. If UEK host, then
   • do nothing
     else
   • if dkms install and insmod then exit 0
4. if modprobe module, then exit 0
5. if unsupported OS, then exit 1 (see get_target_id)
6. if ~/.falco/falco_<target_id>_<kernel_release>_<kernel_version>.ko, then:
   • if insmod then exit 0, else exit 1
7. if curl module, then
   • if insmod then exit 0, else exit 1
     else
   • exit 1
8. exit 0

bpf

Preflight check:

• Mount debugfs (continue on error)
• if kernel config not found, exit 1 (see get_kernel_config)
• if unsupported OS, then exit 1 (see get_target_id)

1. if NOT ~/.falco/falco_<target_id>_<kernel_release>_<kernel_version>.o, then go to step 6.
   • if COS, MINKUBE, or BPF_USE_LOCAL_KERNEL_SOURCES then download kernel source
     • if download failed, then exit 1
     • customize_kernel_build
   • make bpf probe to ~/.falco/falco_<target_id>_<kernel_release>_<kernel_version>.o
2. if NOT ~/.falco/falco_<target_id>_<kernel_release>_<kernel_version>.o,
   • curl bpf probe (continue on error)
3. if ~/.falco/falco_<target_id>_<kernel_release>_<kernel_version>.o, then
   • if symlink to ~/.falco/falco-bpf.o then exit 0, else exit 1
     else
   • exit 1
4. exit 0

N.B.: bpf exists with 0 even though curl failed (see step 2.)

Does this PR introduce a user-facing change?:

update(scripts): improve `falco-driver-loader` output messages
new(scripts): options and command-line usage for `falco-driver-loader`

[leodido]

SGTM but need time to verify it a bit more.

Left one comment, thanks! 🎉

[leodido]

As said this looks mostly good.

The only thing I'm not really sure about is the usage of check_skip_load to circuit-break only the load phase of the drivers (not their installation).

So, implemented in this way, what's the use case of --skip-load? Maybe I'm missing something..

[leogr]

The only thing I'm not really sure about is the usage of check_skip_load to circuit-break only the load phase of the drivers (not their installation).

So, implemented in this way, what's the use case of --skip-load? Maybe I'm missing something..

My thought when I saw the proposal in the issue was... we might need this for testing purposes only. Ofc, Falco will not work if --skip-load is used.

Should we remove it?

[fntlnz]

It's very hard for me to review this one.
Commit 70b0d0b contains many things that spans from formatting to adjusting old logic to new logic.

Even if looking at the PR it looks good to me it's very hard to me to spot a possible error here.

This script is fragile for many reasons:

• Does not have any tests
• Contains lots of OS specific logic
• It's used virtually by every user using Falco in a container

[leogr]

Hey @fntlnz
I'm working on tests, so we can add them.

That being said, really important design question: after talking with @leodido I believe we should remove the --skip-load option. WDYT?
If you agree on to remove it too, I will remove it ASAP.

[leodido]

I think --skip-load flag should be removed: it adds more complexity than it removes.

The --download and --compile flags are pretty explanatory and they cover the use cases

My 2 cents

[leogr]

Update:

• --skip-load has been removed
• do not continue when epbf download fails

[leogr]

IMPORTANT

I have noticed that:

	echo "* Trying to load a system ${DRIVER_NAME} driver, if present"
	if modprobe "${DRIVER_NAME}" > /dev/null 2>&1; then
		echo "* Success: ${DRIVER_NAME} module found and loaded with modprobe"
		exit 0
	fi

might load a kernel module that does not fit the installed Falco version. Currently, it happens before trying a local prebuilt and the download strategy, but that should be the last resort.

[leogr]

Rebased on top of master to include the fix introduced by #1212

I removed WIP, because it's ok for me, though it's not possible to test every single case.

@fntlnz @leodido So, question: do we want this PR to be merged for the 0.23.0?

[leogr]

Rebased on top of master to include lastest fixes.

[leogr]

/milestone 0.23.0

[poiana]

LGTM label has been added.

Git tree hash: 947bf6a4f4a5ca071a6c0ec2bb56c12d1edb4971

[poiana]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fntlnz, leodido

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:

• OWNERS [fntlnz,leodido]

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