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(cmd,internal/utils,pkg/driver): use correct engine.kind config key #357

Merged
merged 3 commits into from
Nov 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 60 additions & 22 deletions cmd/driver/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,16 @@ package driverconfig

import (
"encoding/json"
"errors"
"fmt"
"os"
"path/filepath"
"strings"

"github.com/pterm/pterm"
"github.com/spf13/cobra"
"golang.org/x/net/context"
"gopkg.in/yaml.v3"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/kubernetes"
Expand All @@ -38,7 +42,12 @@ import (
)

const (
configMapDriverTypeKey = "driver_mode"
configMapEngineKindKey = "engine.kind"
longConfig = `Configure a driver for future usages with other driver subcommands.
It will also update local Falco configuration or k8s configmap depending on the environment where it is running, to let Falco use chosen driver.
Only supports deployments of Falco that use a driver engine, ie: one between kmod, ebpf and modern-ebpf.
If engine.kind key is set to a non-driver driven engine, Falco configuration won't be touched.
`
)

type driverConfigOptions struct {
Expand All @@ -64,7 +73,7 @@ func NewDriverConfigCmd(ctx context.Context, opt *options.Common) *cobra.Command
Use: "config [flags]",
DisableFlagsInUseLine: true,
Short: "Configure a driver",
Long: "Configure a driver for future usages with other driver subcommands",
Long: longConfig,
RunE: func(cmd *cobra.Command, args []string) error {
return o.RunDriverConfig(ctx, cmd)
},
Expand Down Expand Up @@ -161,26 +170,58 @@ func (o *driverConfigOptions) RunDriverConfig(ctx context.Context, cmd *cobra.Co
o.Printer.Logger.Info("Running falcoctl driver config", loggerArgs)

if o.Update {
err = commit(ctx, dType, driverCfg.HostRoot, o.Namespace, o.KubeConfig)
err = o.commit(ctx, dType, driverCfg.HostRoot)
if err != nil {
return err
}
}
return config.StoreDriver(&driverCfg, o.ConfigFile)
}

func replaceDriverTypeInFalcoConfig(hostRoot string, driverType drivertype.DriverType) error {
return utils.ReplaceLineInFile(hostRoot+"/etc/falco/falco.yaml", "driver_mode:", "driver_mode: "+driverType.String(), 1)
func checkFalcoRunsWithDrivers(engineKind string) error {
// Modify the data in the ConfigMap/Falco config file ONLY if engine.kind is set to a known driver type.
// This ensures that we modify the config only for Falcos running with drivers, and not plugins/gvisor.
// Scenario: user has multiple Falco pods deployed in its cluster, one running with driver,
// other running with plugins. We must only touch the one running with driver.
if _, err := drivertype.Parse(engineKind); err != nil {
return fmt.Errorf("engine.kind is not driver driven: %s", engineKind)
}
return nil
}

func replaceDriverTypeInK8SConfigMap(ctx context.Context, namespace, kubeconfig string, driverType drivertype.DriverType) error {
func (o *driverConfigOptions) replaceDriverTypeInFalcoConfig(hostRoot string, driverType drivertype.DriverType) error {
falcoCfgFile := filepath.Join(hostRoot, "etc", "falco", "falco.yaml")
type engineCfg struct {
Kind string `yaml:"kind"`
}
type falcoCfg struct {
Engine engineCfg `yaml:"engine"`
}
Comment on lines +194 to +199
Copy link
Member

Choose a reason for hiding this comment

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

Can we please declare these new types outside the function?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think they are good there since they are only needed to unmarshal the Falco engine.kind configuration key; no need to expose them elsewhere in the package.

yamlFile, err := os.ReadFile(filepath.Clean(falcoCfgFile))
if err != nil {
return err
}
cfg := falcoCfg{}
if err = yaml.Unmarshal(yamlFile, &cfg); err != nil {
return err
}
Comment on lines +200 to +207
Copy link
Member

Choose a reason for hiding this comment

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

Wondering if there is a better way to do this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could just manually parse the file line by line, but i think this is the best way to achieve it actually.

if err = checkFalcoRunsWithDrivers(cfg.Engine.Kind); err != nil {
o.Printer.Logger.Warn("Avoid updating Falco configuration",
o.Printer.Logger.Args("config", falcoCfgFile, "reason", err))
return nil
}
const configKindKey = "kind: "
return utils.ReplaceTextInFile(falcoCfgFile, configKindKey+cfg.Engine.Kind, configKindKey+driverType.String(), 1)
}

func (o *driverConfigOptions) replaceDriverTypeInK8SConfigMap(ctx context.Context, driverType drivertype.DriverType) error {
var (
err error
cfg *rest.Config
)

if kubeconfig != "" {
cfg, err = clientcmd.BuildConfigFromFlags("", kubeconfig)
if o.KubeConfig != "" {
cfg, err = clientcmd.BuildConfigFromFlags("", o.KubeConfig)
} else {
cfg, err = rest.InClusterConfig()
}
Expand All @@ -193,14 +234,14 @@ func replaceDriverTypeInK8SConfigMap(ctx context.Context, namespace, kubeconfig
return err
}

configMapList, err := cl.CoreV1().ConfigMaps(namespace).List(ctx, metav1.ListOptions{
configMapList, err := cl.CoreV1().ConfigMaps(o.Namespace).List(ctx, metav1.ListOptions{
LabelSelector: "app.kubernetes.io/instance: falco",
})
if err != nil {
return err
}
if configMapList.Size() == 0 {
return fmt.Errorf(`no configmaps matching "app.kubernetes.io/instance: falco" label were found`)
return errors.New(`no configmaps matching "app.kubernetes.io/instance: falco" label were found`)
}

type patchDriverTypeValue struct {
Expand All @@ -210,22 +251,19 @@ func replaceDriverTypeInK8SConfigMap(ctx context.Context, namespace, kubeconfig
}
payload := []patchDriverTypeValue{{
Op: "replace",
Path: "/data/" + configMapDriverTypeKey,
Path: "/data/" + configMapEngineKindKey,
Value: driverType.String(),
}}
plBytes, _ := json.Marshal(payload)

for i := 0; i < configMapList.Size(); i++ {
configMap := configMapList.Items[i]
// Modify the data in the ConfigMap ONLY if driver_mode is NOT set to plugin
// TODO: we must be sure that we are modifying the configmap for a Falco
// that is running with drivers, and not plugins.
// Scenario: user has multiple Falco pods deployed in its cluster, one running with driver,
// other running with plugins. We must only touch the one running with driver.
if val, ok := configMap.Data[configMapDriverTypeKey]; !ok || val == "none" {
currEngineKind := configMap.Data[configMapEngineKindKey]
if err = checkFalcoRunsWithDrivers(currEngineKind); err != nil {
o.Printer.Logger.Warn("Avoid updating Falco configMap",
o.Printer.Logger.Args("configMap", configMap.Name, "reason", err))
continue
}

// Patch the configMap
if _, err = cl.CoreV1().ConfigMaps(configMap.Namespace).Patch(
ctx, configMap.Name, types.JSONPatchType, plBytes, metav1.PatchOptions{}); err != nil {
Expand All @@ -237,10 +275,10 @@ func replaceDriverTypeInK8SConfigMap(ctx context.Context, namespace, kubeconfig

// commit saves the updated driver type to Falco config,
// either to the local falco.yaml or updating the deployment configmap.
func commit(ctx context.Context, driverType drivertype.DriverType, hostroot, namespace, kubeconfig string) error {
if namespace != "" {
func (o *driverConfigOptions) commit(ctx context.Context, driverType drivertype.DriverType, hostroot string) error {
if o.Namespace != "" {
// Ok we are on k8s
return replaceDriverTypeInK8SConfigMap(ctx, namespace, kubeconfig, driverType)
return o.replaceDriverTypeInK8SConfigMap(ctx, driverType)
}
return replaceDriverTypeInFalcoConfig(hostroot, driverType)
return o.replaceDriverTypeInFalcoConfig(hostroot, driverType)
}
24 changes: 22 additions & 2 deletions internal/utils/file.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,31 @@ import (
"strings"
)

// ReplaceTextInFile searches for occurrences of searchFor in the file pointed by filePath,
// and substitutes the matching string with the provided one.
// At most n substitutions are made.
// If n < 0, there is no limit on the number of replacements.
func ReplaceTextInFile(filePath, searchFor, newText string, n int) error {
return replaceInFile(filePath, searchFor, n, func(line string) string {
return strings.Replace(line, searchFor, newText, 1)
})
}

// ReplaceLineInFile searches for occurrences of searchFor in the file pointed by filePath,
// and substitutes the matching line with the provided one.
// and substitutes the whole matching line with the provided one.
// At most n substitutions are made.
// If n < 0, there is no limit on the number of replacements.
func ReplaceLineInFile(filePath, searchFor, newLine string, n int) error {
return replaceInFile(filePath, searchFor, n, func(_ string) string {
return newLine
})
}

func replaceInFile(filePath, searchFor string, n int, replacementCB func(string) string) error {
if n == 0 {
return nil
}

stat, err := os.Stat(filePath)
if err != nil {
return err
Expand All @@ -42,7 +62,7 @@ func ReplaceLineInFile(filePath, searchFor, newLine string, n int) error {
replaced := 0
for i, line := range lines {
if strings.Contains(line, searchFor) {
lines[i] = newLine
lines[i] = replacementCB(line)
replaced++
if replaced == n {
break
Expand Down
Loading