-
Notifications
You must be signed in to change notification settings - Fork 52
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(cmd,pkg): added new local
build processor to build drivers using local kernel sources/gcc/clang
#306
Conversation
local
build processor to build local kernel using local kernel sources/gcc/clanglocal
build processor to build drivers using local kernel sources/gcc/clang
}, | ||
} | ||
// Add root flags, but not the ones unneeded | ||
unusedFlagsSet := map[string]struct{}{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These flags aren't needed for this command, therefore they are removed.
I think we might want to do the same eg: for images
cmd.
} | ||
|
||
// Partially overrides rootCmd.persistentPreRunFunc setting some defaults before config init/validation stage. | ||
func persistentPreRunFunc(rootCommand *RootCmd, rootOpts *RootOptions) func(c *cobra.Command, args []string) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This gets called in place of rootCmd.PersistentPreRunE
since we shadowed it.
It does just set a bunch of default values (that are not used anyway by the local
build processor, but are logged) and calls the rootCmd.PersistentPreRunE
(ie: persistentValidateFunc)
// nor registered in byTarget, just use "arch". | ||
// NOTE: it won't be used anyway, as driverbuilder/local.go | ||
// manually creates a "local" builder. | ||
if target.String() == "local" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a workaround to let local
cmd set local
target and passing the root options validation step.
Note: local
builder is not mapped inside the byTarget
map because i did not want it to be visible by the user when running eg: docker
builder processor, since it makes no sense.
So, the new local
builder is hidden and manually instantiated by local
builder processor.
var localTemplate string | ||
|
||
type LocalBuilder struct { | ||
GccPath string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GccPath
is the full path to a discovered Gcc, and will be set by local
build processor.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since there might be multiple gcc
present, we tried all of them until we can build the module.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no kernel headers download step in the local
script.
pkg/driverbuilder/local.go
Outdated
kr := b.KernelReleaseFromBuildConfig() | ||
|
||
// create a builder based on the choosen build type | ||
v := &builder.LocalBuilder{} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Manually instantiated; see https://github.com/falcosecurity/driverkit/pull/306/files#r1400657216
gccs = []string{"gcc"} | ||
} | ||
|
||
for _, gcc := range gccs { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For each discovered gcc, try to build module and probe.
pkg/driverbuilder/local.go
Outdated
} | ||
// If we received an error, perhaps we must just rebuilt the kmod. | ||
// Check if we were able to build anything. | ||
if _, err = os.Stat(builder.ModuleFullPath); !os.IsNotExist(err) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If any of module or probe could be built, disable its build for the subsequent step.
Ie: if we have multiple GCCs available, and the first one is not able to build the kmod, but the system clang was able to build the probe, disable its build.
Example output:
|
/hold for discussion, eventually :) |
…ocal kernel sources / gccs / clang. Signed-off-by: Federico Di Pierro <nierro92@gmail.com>
…options in local builder. Signed-off-by: Federico Di Pierro <nierro92@gmail.com>
LGTM! Incorporating Is |
PersistentPreRunE: persistentPreRunFunc(rootCommand, rootOpts), | ||
Run: func(c *cobra.Command, args []string) { | ||
slog.With("processor", c.Name()).Info("driver building, it will take a few seconds") | ||
if !configOptions.DryRun { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a stylistic nit: you could remove a level of indentation in this closure with
if configOptions.DryRun {
return
}
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dwindsor, FedeDP 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 |
Well, since eg: Arch builder is hit by #303, i was able to build the drivers on my machine using the new local build processor :D |
/unhold |
What type of PR is this?
/kind feature
Any specific area of the project related to this PR?
/area cmd
/area pkg
What this PR does / why we need it:
This PR introduces a new
local
build processor that will just build chosen driver version against local kernel sources using local gcc/clang.Therefore, to work, it needs kernel-headers installed (eg:
/lib/modules/6.6.1-arch1-1/build
) and clang/gcc on the machine.It does not use docker, of course.
Moreover, it supports building kmod through
dkms
, and supports directly using a provided preconfiguredsrc-dir
.The bigger aim is to completely implement driver building logic that is today part of
falco-driver-loader script
; this would allow us to properly usedriverkit
insidefalcoctl
to implement thedrivertype.Build
method (eg: https://github.com/falcosecurity/falcoctl/blob/main/pkg/driver/type/kmod.go#L183).IMHO driverkit should be the only project responsible of building drivers.
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?: