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

new: driver command #343

Merged
merged 1 commit into from Nov 21, 2023
Merged

new: driver command #343

merged 1 commit into from Nov 21, 2023

Conversation

FedeDP
Copy link
Contributor

@FedeDP FedeDP commented Nov 3, 2023

What type of PR is this?

/kind feature

Any specific area of the project related to this PR?

/area library
/area cli

What this PR does / why we need it:

First draft at implementing falcoctl driver command, see #327

Which issue(s) this PR fixes:

Eventually #327 will get fixed, but i don't think a single PR will make it :)

Fixes #

Special notes for your reviewer:

What we need:

  • select subcmd with possibility for an "autoselection" smart logic in place; this is not required by Falco but might be useful for other consumers of the falcoctl libraries and for us in the future
  • select stores the currently selected driver in the falcoctl config
  • select must be able to communicate to Falco the driver it must use (we need a patch to allow Falco config to specify a driver to be used) -> new: driver selection in falco.yaml falco#2413 -> this is implemented in falcoctl, even if will need some more fixes
  • select tests
  • prepare skeleton (this will be the step where we either download or build the drivers)
  • autoload of target driverversion from running Falco process, if any Retired with new: driver command #343 (comment)
  • prepare download
  • prepare build
  • specific distros support (similar to the get_target_id falco-driver-loader function)
  • prepare tests
  • Support options that were supported by falco-driver-loader:
    • echo " DRIVER_INSECURE_DOWNLOAD whether you want to allow insecure downloads or not"
    • echo " DRIVER_CURL_OPTIONS specify additional options to be passed to curl command used to download Falco drivers" -> DEPRECATED
    • echo " DRIVER_KERNEL_RELEASE specify the kernel release for which to download/build the driver in the same format used by 'uname -r' (e.g. '6.1.0-10-cloud-amd64')"
    • echo " DRIVER_KERNEL_VERSION specify the kernel version for which to download/build the driver in the same format used by 'uname -v' (e.g. '#1 SMP PREEMPT_DYNAMIC Debian 6.1.38-2 (2023-07-27)')"

@FedeDP
Copy link
Contributor Author

FedeDP commented Nov 3, 2023

About select smart autologic; this is as simple as adding new "distro" overriding the PreferredDriver interface method, like this:

func init() {
	distros["arch"] = &arch{generic: &generic{}}
}

type arch struct {
	*generic
}

func (c *arch) PreferredDriver(info kernel.Info) _type.DriverType {
        if info.Architecture == "aarch64" {
            dtype, _ := _type.Parse("bpf")
            return dtype
        }
        dtype, _ := _type.Parse("modern-bpf")
        return dtype
}

(ok this is a super stupid example btw!)

@FedeDP
Copy link
Contributor Author

FedeDP commented Nov 3, 2023

Example output:

sudo ./falcoctl driver prepare --driver-version 6.0.1+driver
[sudo] password di federico:
2023-11-03 15:31:36 INFO  Running falcoctl driver prepare
                      ├ driver version: 6.0.1+driver
                      ├ driver type: kmod
                      ├ arch: x86_64
                      ├ kernel release: 6.5.7-arch1-1
                      └ kernel version: 1
2023-11-03 15:31:36 INFO  found distro target: arch
2023-11-03 15:31:36 INFO  Trying to download a driver url: https://download.falco.org/driver/6.0.1+driver/x86_64/falco_arch_6.5.7-arch1-1_1.ko
2023-11-03 15:31:36 ERROR unable to find a prebuilt falco driver
2023-11-03 15:31:36 ERROR build unimplemented

@FedeDP FedeDP force-pushed the new/driver_prepare branch 3 times, most recently from 8b8730b to fceb81a Compare November 6, 2023 08:41
Makefile Outdated
@@ -56,7 +56,7 @@ fmt: gci addlicense
go mod tidy
go fmt ./...
find . -type f -name '*.go' -a -exec $(GCI) write -s standard -s default -s "prefix(github.com/falcosecurity/falcoctl)" {} \;
find . -type f -name '*.go' -exec $(ADDLICENSE) -l apache -c "The Falco Authors" -y "$(shell date +%Y)" {} \;
find . -type f -name '*.go' -exec $(ADDLICENSE) -l apache -s -c "The Falco Authors" -y "$(shell date +%Y)" {} \;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

  -s    Include SPDX identifier in license header. Set -s=only to only include SPDX identifier.

)

// NewDriverCmd returns the driver command.
func NewDriverCmd(ctx context.Context, opt *commonoptions.Common) *cobra.Command {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

driver subcmd is only available on linux systems.


// NewDriverCmd returns an empty driver command since it is not supported on non linuxes
func NewDriverCmd(ctx context.Context, opt *commonoptions.Common) *cobra.Command {
return &cobra.Command{}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

By returning an empty command, cobra will skip it.


d, err := driverdistro.DiscoverDistro(o.Printer, o.HostRoot)
if err != nil {
if errors.Is(err, driverdistro.ErrUnsupported) && o.Build {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

errUnsupported returns a generic distro, to attempt a build.

@FedeDP
Copy link
Contributor Author

FedeDP commented Nov 6, 2023

What is implemented so far:

What we miss:

  • build kmod
  • build bpf
  • cos_version_greater
  • get_kernel_config
  • flatcar_relocate_tools
  • insmod/modprobe of kmod once built/downloaded
  • DRIVER_INSECURE_DOWNLOAD, DRIVER_CURL_OPTIONS, DRIVER_KERNEL_RELEASE, DRIVER_KERNEL_VERSION support

Open points:

  • who will pass --driver-version parameter? Falco knows it, but falcoctl does not (unless we let it call the Falco endpoint to gather the info, but since falcoctl driver needs to run before Falco...)
  • how to support source_only and print_env falco-driver-loader options?

@FedeDP FedeDP added this to the v0.7.0 milestone Nov 6, 2023
@FedeDP FedeDP self-assigned this Nov 6, 2023
@FedeDP
Copy link
Contributor Author

FedeDP commented Nov 6, 2023

I think this is already big enough as a starting point.
/cc @alacuku @leogr

@poiana poiana requested a review from leogr November 6, 2023 15:42
@FedeDP FedeDP changed the title wip: new: driver command new: driver command Nov 6, 2023
@FedeDP
Copy link
Contributor Author

FedeDP commented Nov 8, 2023

/hold

@FedeDP FedeDP changed the title new: driver command wip: new: driver command Nov 8, 2023
Copy link
Member

@alacuku alacuku left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, left some comments.

pkg/driver/type/type.go Outdated Show resolved Hide resolved
cmd/driver/config/config.go Outdated Show resolved Hide resolved
cmd/driver/config/config.go Outdated Show resolved Hide resolved
cmd/driver/config/config.go Outdated Show resolved Hide resolved
cmd/driver/config/config.go Outdated Show resolved Hide resolved
cmd/driver/config/config.go Outdated Show resolved Hide resolved
cmd/driver/config/config.go Outdated Show resolved Hide resolved
cmd/driver/install/install.go Outdated Show resolved Hide resolved
cmd/driver/install/install.go Outdated Show resolved Hide resolved
cmd/driver/printenv/printenv.go Outdated Show resolved Hide resolved
@FedeDP
Copy link
Contributor Author

FedeDP commented Nov 21, 2023

I consider this PR ready!
I will add tests and eventually improve things (together with @alacuku perhaps!) in next PRs (yes there will surely be other PRs) 😄

@FedeDP
Copy link
Contributor Author

FedeDP commented Nov 21, 2023

Squashed to a single commit :) @alacuku is now happy 😃

Copy link
Member

@alacuku alacuku left a comment

Choose a reason for hiding this comment

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

/LGTM

It exposes 4 subcmds:
* `install` to install (ie: either download or build) kmod or eBPF probe
* `cleanup` to cleanup a driver
* `printenv` to print environment variables about driver-loader
* `config` to configure the driver-loader feature

Signed-off-by: Federico Di Pierro <nierro92@gmail.com>
@FedeDP
Copy link
Contributor Author

FedeDP commented Nov 21, 2023

/unhold

Copy link
Member

@maxgio92 maxgio92 left a comment

Choose a reason for hiding this comment

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

/approve

@poiana
Copy link
Contributor

poiana commented Nov 21, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alacuku, FedeDP, maxgio92

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

@poiana poiana merged commit 4228e71 into main Nov 21, 2023
14 checks passed
@poiana poiana deleted the new/driver_prepare branch November 21, 2023 16:34
@maxgio92
Copy link
Member

Great job @FedeDP! 👏🏻 ❤️

FedeDP added a commit to falcosecurity/falco that referenced this pull request Nov 27, 2023
Signed-off-by: Federico Di Pierro <nierro92@gmail.com>
FedeDP added a commit to falcosecurity/falco that referenced this pull request Dec 1, 2023
Signed-off-by: Federico Di Pierro <nierro92@gmail.com>
FedeDP added a commit to falcosecurity/falco that referenced this pull request Dec 1, 2023
Signed-off-by: Federico Di Pierro <nierro92@gmail.com>
FedeDP added a commit to falcosecurity/falco that referenced this pull request Dec 6, 2023
Signed-off-by: Federico Di Pierro <nierro92@gmail.com>
poiana pushed a commit to falcosecurity/falco that referenced this pull request Dec 11, 2023
Signed-off-by: Federico Di Pierro <nierro92@gmail.com>
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.

None yet

5 participants