Skip to content

Commit 2c2f53b

Browse files
FedeDPFedeDP
committed
chore(cmd/driver,pkg/driver,internal/config): avoid string concatenation for hostroot.
Moreover, hostRoot will now default to `/`, and it will be stored as package local variable in driverdistro package, to forwarding it where needed. Signed-off-by: Federico Di Pierro <[email protected]>
1 parent d762a9c commit 2c2f53b

File tree

17 files changed

+125
-76
lines changed

17 files changed

+125
-76
lines changed

‎cmd/driver/config/config.go

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,9 @@ func (o *driverConfigOptions) RunDriverConfig(ctx context.Context, cmd *cobra.Co
125125
})
126126
}
127127
if f := cmd.Flags().Lookup("host-root"); f != nil && f.Changed {
128+
if !filepath.IsAbs(o.HostRoot) {
129+
return fmt.Errorf("host-root must be an absolute path: %s", o.HostRoot)
130+
}
128131
driverCfg.HostRoot = o.HostRoot
129132
loggerArgs = append(loggerArgs, pterm.LoggerArgument{
130133
Key: "driver host root",
@@ -153,7 +156,7 @@ func (o *driverConfigOptions) RunDriverConfig(ctx context.Context, cmd *cobra.Co
153156
"kernel release", info.String(),
154157
"kernel version", info.KernelVersion))
155158

156-
d, err := driverdistro.DiscoverDistro(info, driverCfg.HostRoot)
159+
d, err := driverdistro.Discover(info, driverCfg.HostRoot)
157160
if err != nil {
158161
return err
159162
}
@@ -170,7 +173,7 @@ func (o *driverConfigOptions) RunDriverConfig(ctx context.Context, cmd *cobra.Co
170173
o.Printer.Logger.Info("Running falcoctl driver config", loggerArgs)
171174

172175
if o.Update {
173-
err = o.commit(ctx, dType, driverCfg.HostRoot)
176+
err = o.commit(ctx, dType)
174177
if err != nil {
175178
return err
176179
}
@@ -189,8 +192,8 @@ func checkFalcoRunsWithDrivers(engineKind string) error {
189192
return nil
190193
}
191194

192-
func (o *driverConfigOptions) replaceDriverTypeInFalcoConfig(hostRoot string, driverType drivertype.DriverType) error {
193-
falcoCfgFile := filepath.Join(hostRoot, "etc", "falco", "falco.yaml")
195+
func (o *driverConfigOptions) replaceDriverTypeInFalcoConfig(driverType drivertype.DriverType) error {
196+
falcoCfgFile := filepath.Clean(filepath.Join(string(os.PathSeparator), "etc", "falco", "falco.yaml"))
194197
type engineCfg struct {
195198
Kind string `yaml:"kind"`
196199
}
@@ -275,10 +278,10 @@ func (o *driverConfigOptions) replaceDriverTypeInK8SConfigMap(ctx context.Contex
275278

276279
// commit saves the updated driver type to Falco config,
277280
// either to the local falco.yaml or updating the deployment configmap.
278-
func (o *driverConfigOptions) commit(ctx context.Context, driverType drivertype.DriverType, hostroot string) error {
281+
func (o *driverConfigOptions) commit(ctx context.Context, driverType drivertype.DriverType) error {
279282
if o.Namespace != "" {
280283
// Ok we are on k8s
281284
return o.replaceDriverTypeInK8SConfigMap(ctx, driverType)
282285
}
283-
return o.replaceDriverTypeInFalcoConfig(hostroot, driverType)
286+
return o.replaceDriverTypeInFalcoConfig(driverType)
284287
}

‎cmd/driver/install/install.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ func (o *driverInstallOptions) RunDriverInstall(ctx context.Context, driver *con
159159
return "", nil
160160
}
161161

162-
d, err := driverdistro.DiscoverDistro(kr, driver.HostRoot)
162+
d, err := driverdistro.Discover(kr, driver.HostRoot)
163163
if err != nil {
164164
if errors.Is(err, driverdistro.ErrUnsupported) && o.Compile {
165165
o.Download = false
@@ -190,7 +190,7 @@ func (o *driverInstallOptions) RunDriverInstall(ctx context.Context, driver *con
190190
}
191191

192192
if o.Compile {
193-
dest, err = driverdistro.Build(ctx, d, o.Printer, kr, driver.Name, driver.Type, driver.Version, driver.HostRoot)
193+
dest, err = driverdistro.Build(ctx, d, o.Printer, kr, driver.Name, driver.Type, driver.Version)
194194
if err == nil {
195195
return dest, nil
196196
}

‎cmd/driver/printenv/printenv.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ func (o *driverPrintenvOptions) RunDriverPrintenv(_ context.Context) error {
6565
return err
6666
}
6767

68-
d, err := driverdistro.DiscoverDistro(kr, driver.HostRoot)
68+
d, err := driverdistro.Discover(kr, driver.HostRoot)
6969
if err != nil {
7070
return err
7171
}

‎internal/config/config.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,7 @@ func init() {
208208
Name: "falco",
209209
Repos: []string{"https://download.falco.org/driver"},
210210
Version: "",
211-
HostRoot: "",
211+
HostRoot: string(os.PathSeparator),
212212
}
213213
}
214214

‎pkg/driver/distro/centos.go

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,11 @@
1515

1616
package driverdistro
1717

18-
import "github.com/falcosecurity/falcoctl/internal/utils"
18+
import (
19+
"path/filepath"
20+
21+
"github.com/falcosecurity/falcoctl/internal/utils"
22+
)
1923

2024
func init() {
2125
distros["centos"] = &centos{generic: &generic{}}
@@ -25,7 +29,11 @@ type centos struct {
2529
*generic
2630
}
2731

28-
func (c *centos) check(hostRoot string) bool {
29-
exist, _ := utils.FileExists(hostRoot + "/etc/centos-release")
32+
func (c *centos) check() bool {
33+
exist, _ := utils.FileExists(c.releaseFile())
3034
return exist
3135
}
36+
37+
func (c *centos) releaseFile() string {
38+
return filepath.Clean(filepath.Join(hostRoot, "etc", "centos-release"))
39+
}

‎pkg/driver/distro/centos_test.go

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,12 @@ func TestDistroCentosCheck(t *testing.T) {
3030
postFn func()
3131
retExpected bool
3232
}
33+
34+
const (
35+
etcPath = "/etc/"
36+
releaseFilePath = etcPath + "centos-release"
37+
)
38+
3339
testCases := []testCase{
3440
{
3541
// No centos-release file
@@ -44,39 +50,40 @@ func TestDistroCentosCheck(t *testing.T) {
4450
// centos-release file present under hostroot
4551
hostRoot: ".",
4652
preFn: func() error {
47-
if err := os.MkdirAll("./etc/", 0o755); err != nil {
53+
if err := os.MkdirAll(hostRoot+etcPath, 0o755); err != nil {
4854
return err
4955
}
50-
_, err := os.Create("./etc/centos-release")
56+
_, err := os.Create(hostRoot + releaseFilePath)
5157
return err
5258
},
5359
postFn: func() {
54-
_ = os.RemoveAll("./etc/centos-release")
60+
_ = os.RemoveAll(hostRoot + releaseFilePath)
5561
},
5662
retExpected: true,
5763
},
5864
{
5965
// centos-release file present but not under hostroot
6066
hostRoot: "/foo",
6167
preFn: func() error {
62-
if err := os.MkdirAll("./etc/", 0o755); err != nil {
68+
if err := os.MkdirAll("."+etcPath, 0o755); err != nil {
6369
return err
6470
}
65-
_, err := os.Create("./etc/centos-release")
71+
_, err := os.Create("." + releaseFilePath)
6672
return err
6773
},
6874
postFn: func() {
69-
_ = os.RemoveAll("./etc/centos-release")
75+
_ = os.RemoveAll("." + releaseFilePath)
7076
},
7177
retExpected: false,
7278
},
7379
}
7480

7581
for _, tCase := range testCases {
82+
hostRoot = tCase.hostRoot
7683
c := &centos{generic: &generic{}}
7784
err := tCase.preFn()
7885
require.NoError(t, err)
79-
assert.Equal(t, tCase.retExpected, c.check(tCase.hostRoot))
86+
assert.Equal(t, tCase.retExpected, c.check())
8087
tCase.postFn()
8188
}
8289
}

‎pkg/driver/distro/cos.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,6 @@ func (c *cos) customizeBuild(ctx context.Context,
5858
printer *output.Printer,
5959
driverType drivertype.DriverType,
6060
kr kernelrelease.KernelRelease,
61-
hostRoot string,
6261
) (map[string]string, error) {
6362
switch driverType.String() {
6463
case drivertype.TypeBpf:
@@ -70,7 +69,7 @@ func (c *cos) customizeBuild(ctx context.Context,
7069
printer.Logger.Info("COS detected, using COS kernel headers.", printer.Logger.Args("build ID", c.buildID))
7170
bpfKernelSrcURL := fmt.Sprintf("https://storage.googleapis.com/cos-tools/%s/kernel-headers.tgz", c.buildID)
7271
kr.Extraversion = "+"
73-
env, err := downloadKernelSrc(ctx, printer, &kr, bpfKernelSrcURL, hostRoot, 0)
72+
env, err := downloadKernelSrc(ctx, printer, &kr, bpfKernelSrcURL, 0)
7473
if err != nil {
7574
return nil, err
7675
}

‎pkg/driver/distro/debian.go

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ package driverdistro
1717

1818
import (
1919
"fmt"
20+
"path/filepath"
2021
"regexp"
2122

2223
"github.com/falcosecurity/driverkit/pkg/kernelrelease"
@@ -36,11 +37,15 @@ type debian struct {
3637
var debianKernelReleaseRegex = regexp.MustCompile(`-?(rt-|cloud-|)(amd64|arm64)`)
3738
var debianKernelVersionRegex = regexp.MustCompile(`\d+\.\d+\.\d+-\d+`)
3839

39-
func (d *debian) check(hostRoot string) bool {
40-
exist, _ := utils.FileExists(hostRoot + "/etc/debian_version")
40+
func (d *debian) check() bool {
41+
exist, _ := utils.FileExists(d.releaseFile())
4142
return exist
4243
}
4344

45+
func (d *debian) releaseFile() string {
46+
return filepath.Clean(filepath.Join(hostRoot, "etc", "debian_version"))
47+
}
48+
4449
//nolint:gocritic // the method shall not be able to modify kr
4550
func (d *debian) FixupKernel(kr kernelrelease.KernelRelease) kernelrelease.KernelRelease {
4651
// Workaround: debian kernelreleases might not be actual kernel running;

‎pkg/driver/distro/debian_test.go

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,10 @@ import (
2525
)
2626

2727
func TestDistroDebianCheck(t *testing.T) {
28+
const (
29+
etcPath = "/etc/"
30+
releaseFilePath = etcPath + "debian_version"
31+
)
2832
type testCase struct {
2933
hostRoot string
3034
preFn func() error
@@ -43,41 +47,42 @@ func TestDistroDebianCheck(t *testing.T) {
4347
},
4448
{
4549
// debian_version file present under hostroot
46-
hostRoot: ".",
50+
hostRoot: os.TempDir(),
4751
preFn: func() error {
48-
if err := os.MkdirAll("./etc/", 0o755); err != nil {
52+
if err := os.MkdirAll(hostRoot+etcPath, 0o755); err != nil {
4953
return err
5054
}
51-
_, err := os.Create("./etc/debian_version")
55+
_, err := os.Create(hostRoot + releaseFilePath)
5256
return err
5357
},
5458
postFn: func() {
55-
_ = os.RemoveAll("./etc/debian_version")
59+
_ = os.RemoveAll(hostRoot + releaseFilePath)
5660
},
5761
retExpected: true,
5862
},
5963
{
6064
// debian_version file present but not under hostroot
6165
hostRoot: "/foo",
6266
preFn: func() error {
63-
if err := os.MkdirAll("./etc/", 0o755); err != nil {
67+
if err := os.MkdirAll("."+etcPath, 0o755); err != nil {
6468
return err
6569
}
66-
_, err := os.Create("./etc/debian_version")
70+
_, err := os.Create("." + releaseFilePath)
6771
return err
6872
},
6973
postFn: func() {
70-
_ = os.RemoveAll("./etc/debian_version")
74+
_ = os.RemoveAll("." + releaseFilePath)
7175
},
7276
retExpected: false,
7377
},
7478
}
7579

7680
for _, tCase := range testCases {
81+
hostRoot = tCase.hostRoot
7782
deb := &debian{generic: &generic{}}
7883
err := tCase.preFn()
7984
require.NoError(t, err)
80-
assert.Equal(t, tCase.retExpected, deb.check(tCase.hostRoot))
85+
assert.Equal(t, tCase.retExpected, deb.check())
8186
tCase.postFn()
8287
}
8388
}

‎pkg/driver/distro/distro.go

Lines changed: 22 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -43,40 +43,46 @@ const (
4343
kernelSrcDownloadFolder = "kernel-sources"
4444
)
4545

46-
var distros = map[string]Distro{}
47-
48-
// ErrUnsupported is the error returned when the target distro is not supported.
49-
var ErrUnsupported = errors.New("failed to determine distro")
46+
var (
47+
distros = map[string]Distro{}
48+
hostRoot = string(os.PathSeparator)
49+
// ErrUnsupported is the error returned when the target distro is not supported.
50+
ErrUnsupported = errors.New("failed to determine distro")
51+
)
5052

5153
// Distro is the common interface used by distro-specific implementations.
5254
// Most of the distro-specific only partially override the default `generic` implementation.
5355
type Distro interface {
5456
init(kr kernelrelease.KernelRelease, id string, cfg *ini.File) error // private
5557
FixupKernel(kr kernelrelease.KernelRelease) kernelrelease.KernelRelease // private
5658
customizeBuild(ctx context.Context, printer *output.Printer, driverType drivertype.DriverType,
57-
kr kernelrelease.KernelRelease, hostRoot string) (map[string]string, error)
59+
kr kernelrelease.KernelRelease) (map[string]string, error)
5860
PreferredDriver(kr kernelrelease.KernelRelease) drivertype.DriverType
5961
fmt.Stringer
6062
}
6163

6264
type checker interface {
63-
check(hostRoot string) bool // private
65+
check() bool // private
6466
}
6567

66-
// DiscoverDistro tries to fetch the correct Distro by looking at /etc/os-release or
68+
// Discover tries to fetch the correct Distro by looking at /etc/os-release or
6769
// by cycling on all supported distros and checking them one by one.
6870
//
6971
//nolint:gocritic // the method shall not be able to modify kr
70-
func DiscoverDistro(kr kernelrelease.KernelRelease, hostRoot string) (Distro, error) {
71-
distro, err := getOSReleaseDistro(&kr, hostRoot)
72+
func Discover(kr kernelrelease.KernelRelease, hostroot string) (Distro, error) {
73+
// Implicitly store hostroot to a package local variable
74+
// to avoid passing it in other APIs
75+
hostRoot = hostroot
76+
77+
distro, err := getOSReleaseDistro(&kr)
7278
if err == nil {
7379
return distro, nil
7480
}
7581

7682
// Fallback to check any distro checker
7783
for id, d := range distros {
7884
dd, ok := d.(checker)
79-
if ok && dd.check(hostRoot) {
85+
if ok && dd.check() {
8086
err = d.init(kr, id, nil)
8187
return d, err
8288
}
@@ -90,7 +96,7 @@ func DiscoverDistro(kr kernelrelease.KernelRelease, hostRoot string) (Distro, er
9096
return distro, ErrUnsupported
9197
}
9298

93-
func getOSReleaseDistro(kr *kernelrelease.KernelRelease, hostRoot string) (Distro, error) {
99+
func getOSReleaseDistro(kr *kernelrelease.KernelRelease) (Distro, error) {
94100
cfg, err := ini.Load(hostRoot + "/etc/os-release")
95101
if err != nil {
96102
return nil, err
@@ -104,7 +110,7 @@ func getOSReleaseDistro(kr *kernelrelease.KernelRelease, hostRoot string) (Distr
104110
// Overwrite the OS_ID if /etc/VERSION file is present.
105111
// Not sure if there is a better way to detect minikube.
106112
dd := distros["minikube"].(checker)
107-
if dd.check(hostRoot) {
113+
if dd.check() {
108114
id = "minikube"
109115
}
110116

@@ -155,9 +161,8 @@ func Build(ctx context.Context,
155161
driverName string,
156162
driverType drivertype.DriverType,
157163
driverVer string,
158-
hostRoot string,
159164
) (string, error) {
160-
env, err := d.customizeBuild(ctx, printer, driverType, kr, hostRoot)
165+
env, err := d.customizeBuild(ctx, printer, driverType, kr)
161166
if err != nil {
162167
return "", err
163168
}
@@ -237,7 +242,7 @@ func customizeDownloadKernelSrcBuild(printer *output.Printer, kr *kernelrelease.
237242
return err
238243
}
239244

240-
func getKernelConfig(printer *output.Printer, kr *kernelrelease.KernelRelease, hostRoot string) (string, error) {
245+
func getKernelConfig(printer *output.Printer, kr *kernelrelease.KernelRelease) (string, error) {
241246
bootConfig := fmt.Sprintf("/boot/config-%s", kr.String())
242247
hrBootConfig := fmt.Sprintf("%s%s", hostRoot, bootConfig)
243248
ostreeConfig := fmt.Sprintf("/usr/lib/ostree-boot/config-%s", kr.String())
@@ -265,7 +270,7 @@ func getKernelConfig(printer *output.Printer, kr *kernelrelease.KernelRelease, h
265270
func downloadKernelSrc(ctx context.Context,
266271
printer *output.Printer,
267272
kr *kernelrelease.KernelRelease,
268-
url string, hostRoot string,
273+
url string,
269274
stripComponents int,
270275
) (map[string]string, error) {
271276
env := make(map[string]string)
@@ -309,7 +314,7 @@ func downloadKernelSrc(ctx context.Context,
309314
return env, err
310315
}
311316

312-
kernelConfigPath, err := getKernelConfig(printer, kr, hostRoot)
317+
kernelConfigPath, err := getKernelConfig(printer, kr)
313318
if err != nil {
314319
return nil, err
315320
}

0 commit comments

Comments
 (0)