chore(cmd/driver,pkg/driver,internal/config): avoid string concatenat…

…ion 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]>

cmd/driver/config/config.go

@@ -125,6 +125,9 @@ func (o *driverConfigOptions) RunDriverConfig(ctx context.Context, cmd *cobra.Co
 		})
 	}
 	if f := cmd.Flags().Lookup("host-root"); f != nil && f.Changed {
+		if !filepath.IsAbs(o.HostRoot) {
+			return fmt.Errorf("host-root must be an absolute path: %s", o.HostRoot)
+		}
 		driverCfg.HostRoot = o.HostRoot
 		loggerArgs = append(loggerArgs, pterm.LoggerArgument{
 			Key:   "driver host root",
@@ -153,7 +156,7 @@ func (o *driverConfigOptions) RunDriverConfig(ctx context.Context, cmd *cobra.Co
 				"kernel release", info.String(),
 				"kernel version", info.KernelVersion))
 
-			d, err := driverdistro.DiscoverDistro(info, driverCfg.HostRoot)
+			d, err := driverdistro.Discover(info, driverCfg.HostRoot)
 			if err != nil {
 				return err
 			}
@@ -170,7 +173,7 @@ func (o *driverConfigOptions) RunDriverConfig(ctx context.Context, cmd *cobra.Co
 	o.Printer.Logger.Info("Running falcoctl driver config", loggerArgs)
 
 	if o.Update {
-		err = o.commit(ctx, dType, driverCfg.HostRoot)
+		err = o.commit(ctx, dType)
 		if err != nil {
 			return err
 		}
@@ -189,8 +192,8 @@ func checkFalcoRunsWithDrivers(engineKind string) error {
 	return nil
 }
 
-func (o *driverConfigOptions) replaceDriverTypeInFalcoConfig(hostRoot string, driverType drivertype.DriverType) error {
-	falcoCfgFile := hostRoot + "/etc/falco/falco.yaml"
+func (o *driverConfigOptions) replaceDriverTypeInFalcoConfig(driverType drivertype.DriverType) error {
+	falcoCfgFile := filepath.Clean(filepath.Join(string(os.PathSeparator), "etc", "falco", "falco.yaml"))
 	type engineCfg struct {
 		Kind string `yaml:"kind"`
 	}
@@ -275,10 +278,10 @@ func (o *driverConfigOptions) replaceDriverTypeInK8SConfigMap(ctx context.Contex
 
 // commit saves the updated driver type to Falco config,
 // either to the local falco.yaml or updating the deployment configmap.
-func (o *driverConfigOptions) commit(ctx context.Context, driverType drivertype.DriverType, hostroot string) error {
+func (o *driverConfigOptions) commit(ctx context.Context, driverType drivertype.DriverType) error {
 	if o.Namespace != "" {
 		// Ok we are on k8s
 		return o.replaceDriverTypeInK8SConfigMap(ctx, driverType)
 	}
-	return o.replaceDriverTypeInFalcoConfig(hostroot, driverType)
+	return o.replaceDriverTypeInFalcoConfig(driverType)
 }

cmd/driver/install/install.go

@@ -159,7 +159,7 @@ func (o *driverInstallOptions) RunDriverInstall(ctx context.Context, driver *con
 		return "", nil
 	}
 
-	d, err := driverdistro.DiscoverDistro(kr, driver.HostRoot)
+	d, err := driverdistro.Discover(kr, driver.HostRoot)
 	if err != nil {
 		if errors.Is(err, driverdistro.ErrUnsupported) && o.Compile {
 			o.Download = false
@@ -190,7 +190,7 @@ func (o *driverInstallOptions) RunDriverInstall(ctx context.Context, driver *con
 	}
 
 	if o.Compile {
-		dest, err = driverdistro.Build(ctx, d, o.Printer, kr, driver.Name, driver.Type, driver.Version, driver.HostRoot)
+		dest, err = driverdistro.Build(ctx, d, o.Printer, kr, driver.Name, driver.Type, driver.Version)
 		if err == nil {
 			return dest, nil
 		}

cmd/driver/printenv/printenv.go

@@ -65,7 +65,7 @@ func (o *driverPrintenvOptions) RunDriverPrintenv(_ context.Context) error {
 		return err
 	}
 
-	d, err := driverdistro.DiscoverDistro(kr, driver.HostRoot)
+	d, err := driverdistro.Discover(kr, driver.HostRoot)
 	if err != nil {
 		return err
 	}

internal/config/config.go

@@ -208,7 +208,7 @@ func init() {
 		Name:     "falco",
 		Repos:    []string{"https://download.falco.org/driver"},
 		Version:  "",
-		HostRoot: "",
+		HostRoot: string(os.PathSeparator),
 	}
 }
 

pkg/driver/distro/centos.go

@@ -15,7 +15,11 @@
 
 package driverdistro
 
-import "github.com/falcosecurity/falcoctl/internal/utils"
+import (
+	"path/filepath"
+
+	"github.com/falcosecurity/falcoctl/internal/utils"
+)
 
 func init() {
 	distros["centos"] = &centos{generic: &generic{}}
@@ -25,7 +29,11 @@ type centos struct {
 	*generic
 }
 
-func (c *centos) check(hostRoot string) bool {
-	exist, _ := utils.FileExists(hostRoot + "/etc/centos-release")
+func (c *centos) check() bool {
+	exist, _ := utils.FileExists(c.releaseFile())
 	return exist
 }
+
+func (c *centos) releaseFile() string {
+	return filepath.Clean(filepath.Join(hostRoot, "etc", "centos-release"))
+}

pkg/driver/distro/centos_test.go

@@ -30,6 +30,12 @@ func TestDistroCentosCheck(t *testing.T) {
 		postFn      func()
 		retExpected bool
 	}
+
+	const (
+		etcPath         = "/etc/"
+		releaseFilePath = etcPath + "centos-release"
+	)
+
 	testCases := []testCase{
 		{
 			// No centos-release file
@@ -44,39 +50,40 @@ func TestDistroCentosCheck(t *testing.T) {
 			// centos-release file present under hostroot
 			hostRoot: ".",
 			preFn: func() error {
-				if err := os.MkdirAll("./etc/", 0o755); err != nil {
+				if err := os.MkdirAll(hostRoot+etcPath, 0o755); err != nil {
 					return err
 				}
-				_, err := os.Create("./etc/centos-release")
+				_, err := os.Create(hostRoot + releaseFilePath)
 				return err
 			},
 			postFn: func() {
-				_ = os.RemoveAll("./etc/centos-release")
+				_ = os.RemoveAll(hostRoot + releaseFilePath)
 			},
 			retExpected: true,
 		},
 		{
 			// centos-release file present but not under hostroot
 			hostRoot: "/foo",
 			preFn: func() error {
-				if err := os.MkdirAll("./etc/", 0o755); err != nil {
+				if err := os.MkdirAll("."+etcPath, 0o755); err != nil {
 					return err
 				}
-				_, err := os.Create("./etc/centos-release")
+				_, err := os.Create("." + releaseFilePath)
 				return err
 			},
 			postFn: func() {
-				_ = os.RemoveAll("./etc/centos-release")
+				_ = os.RemoveAll("." + releaseFilePath)
 			},
 			retExpected: false,
 		},
 	}
 
 	for _, tCase := range testCases {
+		hostRoot = tCase.hostRoot
 		c := &centos{generic: &generic{}}
 		err := tCase.preFn()
 		require.NoError(t, err)
-		assert.Equal(t, tCase.retExpected, c.check(tCase.hostRoot))
+		assert.Equal(t, tCase.retExpected, c.check())
 		tCase.postFn()
 	}
 }

pkg/driver/distro/cos.go

@@ -58,7 +58,6 @@ func (c *cos) customizeBuild(ctx context.Context,
 	printer *output.Printer,
 	driverType drivertype.DriverType,
 	kr kernelrelease.KernelRelease,
-	hostRoot string,
 ) (map[string]string, error) {
 	switch driverType.String() {
 	case drivertype.TypeBpf:
@@ -70,7 +69,7 @@ func (c *cos) customizeBuild(ctx context.Context,
 	printer.Logger.Info("COS detected, using COS kernel headers.", printer.Logger.Args("build ID", c.buildID))
 	bpfKernelSrcURL := fmt.Sprintf("https://storage.googleapis.com/cos-tools/%s/kernel-headers.tgz", c.buildID)
 	kr.Extraversion = "+"
-	env, err := downloadKernelSrc(ctx, printer, &kr, bpfKernelSrcURL, hostRoot, 0)
+	env, err := downloadKernelSrc(ctx, printer, &kr, bpfKernelSrcURL, 0)
 	if err != nil {
 		return nil, err
 	}

pkg/driver/distro/debian.go

@@ -17,6 +17,7 @@ package driverdistro
 
 import (
 	"fmt"
+	"path/filepath"
 	"regexp"
 
 	"github.com/falcosecurity/driverkit/pkg/kernelrelease"
@@ -36,11 +37,15 @@ type debian struct {
 var debianKernelReleaseRegex = regexp.MustCompile(`-?(rt-|cloud-|)(amd64|arm64)`)
 var debianKernelVersionRegex = regexp.MustCompile(`\d+\.\d+\.\d+-\d+`)
 
-func (d *debian) check(hostRoot string) bool {
-	exist, _ := utils.FileExists(hostRoot + "/etc/debian_version")
+func (d *debian) check() bool {
+	exist, _ := utils.FileExists(d.releaseFile())
 	return exist
 }
 
+func (d *debian) releaseFile() string {
+	return filepath.Clean(filepath.Join(hostRoot, "etc", "debian_version"))
+}
+
 //nolint:gocritic // the method shall not be able to modify kr
 func (d *debian) FixupKernel(kr kernelrelease.KernelRelease) kernelrelease.KernelRelease {
 	// Workaround: debian kernelreleases might not be actual kernel running;

pkg/driver/distro/debian_test.go

@@ -25,6 +25,10 @@ import (
 )
 
 func TestDistroDebianCheck(t *testing.T) {
+	const (
+		etcPath         = "/etc/"
+		releaseFilePath = etcPath + "debian_version"
+	)
 	type testCase struct {
 		hostRoot    string
 		preFn       func() error
@@ -43,41 +47,42 @@ func TestDistroDebianCheck(t *testing.T) {
 		},
 		{
 			// debian_version file present under hostroot
-			hostRoot: ".",
+			hostRoot: os.TempDir(),
 			preFn: func() error {
-				if err := os.MkdirAll("./etc/", 0o755); err != nil {
+				if err := os.MkdirAll(hostRoot+etcPath, 0o755); err != nil {
 					return err
 				}
-				_, err := os.Create("./etc/debian_version")
+				_, err := os.Create(hostRoot + releaseFilePath)
 				return err
 			},
 			postFn: func() {
-				_ = os.RemoveAll("./etc/debian_version")
+				_ = os.RemoveAll(hostRoot + releaseFilePath)
 			},
 			retExpected: true,
 		},
 		{
 			// debian_version file present but not under hostroot
 			hostRoot: "/foo",
 			preFn: func() error {
-				if err := os.MkdirAll("./etc/", 0o755); err != nil {
+				if err := os.MkdirAll("."+etcPath, 0o755); err != nil {
 					return err
 				}
-				_, err := os.Create("./etc/debian_version")
+				_, err := os.Create("." + releaseFilePath)
 				return err
 			},
 			postFn: func() {
-				_ = os.RemoveAll("./etc/debian_version")
+				_ = os.RemoveAll("." + releaseFilePath)
 			},
 			retExpected: false,
 		},
 	}
 
 	for _, tCase := range testCases {
+		hostRoot = tCase.hostRoot
 		deb := &debian{generic: &generic{}}
 		err := tCase.preFn()
 		require.NoError(t, err)
-		assert.Equal(t, tCase.retExpected, deb.check(tCase.hostRoot))
+		assert.Equal(t, tCase.retExpected, deb.check())
 		tCase.postFn()
 	}
 }

pkg/driver/distro/distro.go

@@ -43,40 +43,46 @@ const (
 	kernelSrcDownloadFolder = "kernel-sources"
 )
 
-var distros = map[string]Distro{}
-
-// ErrUnsupported is the error returned when the target distro is not supported.
-var ErrUnsupported = errors.New("failed to determine distro")
+var (
+	distros  = map[string]Distro{}
+	hostRoot = string(os.PathSeparator)
+	// ErrUnsupported is the error returned when the target distro is not supported.
+	ErrUnsupported = errors.New("failed to determine distro")
+)
 
 // Distro is the common interface used by distro-specific implementations.
 // Most of the distro-specific only partially override the default `generic` implementation.
 type Distro interface {
 	init(kr kernelrelease.KernelRelease, id string, cfg *ini.File) error    // private
 	FixupKernel(kr kernelrelease.KernelRelease) kernelrelease.KernelRelease // private
 	customizeBuild(ctx context.Context, printer *output.Printer, driverType drivertype.DriverType,
-		kr kernelrelease.KernelRelease, hostRoot string) (map[string]string, error)
+		kr kernelrelease.KernelRelease) (map[string]string, error)
 	PreferredDriver(kr kernelrelease.KernelRelease) drivertype.DriverType
 	fmt.Stringer
 }
 
 type checker interface {
-	check(hostRoot string) bool // private
+	check() bool // private
 }
 
-// DiscoverDistro tries to fetch the correct Distro by looking at /etc/os-release or
+// Discover tries to fetch the correct Distro by looking at /etc/os-release or
 // by cycling on all supported distros and checking them one by one.
 //
 //nolint:gocritic // the method shall not be able to modify kr
-func DiscoverDistro(kr kernelrelease.KernelRelease, hostRoot string) (Distro, error) {
-	distro, err := getOSReleaseDistro(&kr, hostRoot)
+func Discover(kr kernelrelease.KernelRelease, hostroot string) (Distro, error) {
+	// Implicitly store hostroot to a package local variable
+	// to avoid passing it in other APIs
+	hostRoot = hostroot
+
+	distro, err := getOSReleaseDistro(&kr)
 	if err == nil {
 		return distro, nil
 	}
 
 	// Fallback to check any distro checker
 	for id, d := range distros {
 		dd, ok := d.(checker)
-		if ok && dd.check(hostRoot) {
+		if ok && dd.check() {
 			err = d.init(kr, id, nil)
 			return d, err
 		}
@@ -90,7 +96,7 @@ func DiscoverDistro(kr kernelrelease.KernelRelease, hostRoot string) (Distro, er
 	return distro, ErrUnsupported
 }
 
-func getOSReleaseDistro(kr *kernelrelease.KernelRelease, hostRoot string) (Distro, error) {
+func getOSReleaseDistro(kr *kernelrelease.KernelRelease) (Distro, error) {
 	cfg, err := ini.Load(hostRoot + "/etc/os-release")
 	if err != nil {
 		return nil, err
@@ -104,7 +110,7 @@ func getOSReleaseDistro(kr *kernelrelease.KernelRelease, hostRoot string) (Distr
 	// Overwrite the OS_ID if /etc/VERSION file is present.
 	// Not sure if there is a better way to detect minikube.
 	dd := distros["minikube"].(checker)
-	if dd.check(hostRoot) {
+	if dd.check() {
 		id = "minikube"
 	}
 
@@ -155,9 +161,8 @@ func Build(ctx context.Context,
 	driverName string,
 	driverType drivertype.DriverType,
 	driverVer string,
-	hostRoot string,
 ) (string, error) {
-	env, err := d.customizeBuild(ctx, printer, driverType, kr, hostRoot)
+	env, err := d.customizeBuild(ctx, printer, driverType, kr)
 	if err != nil {
 		return "", err
 	}
@@ -237,7 +242,7 @@ func customizeDownloadKernelSrcBuild(printer *output.Printer, kr *kernelrelease.
 	return err
 }
 
-func getKernelConfig(printer *output.Printer, kr *kernelrelease.KernelRelease, hostRoot string) (string, error) {
+func getKernelConfig(printer *output.Printer, kr *kernelrelease.KernelRelease) (string, error) {
 	bootConfig := fmt.Sprintf("/boot/config-%s", kr.String())
 	hrBootConfig := fmt.Sprintf("%s%s", hostRoot, bootConfig)
 	ostreeConfig := fmt.Sprintf("/usr/lib/ostree-boot/config-%s", kr.String())
@@ -265,7 +270,7 @@ func getKernelConfig(printer *output.Printer, kr *kernelrelease.KernelRelease, h
 func downloadKernelSrc(ctx context.Context,
 	printer *output.Printer,
 	kr *kernelrelease.KernelRelease,
-	url, hostRoot string,
+	url string,
 	stripComponents int,
 ) (map[string]string, error) {
 	env := make(map[string]string)
@@ -309,7 +314,7 @@ func downloadKernelSrc(ctx context.Context,
 		return env, err
 	}
 
-	kernelConfigPath, err := getKernelConfig(printer, kr, hostRoot)
+	kernelConfigPath, err := getKernelConfig(printer, kr)
 	if err != nil {
 		return nil, err
 	}