From ac01a90003f11e1737810268ae08cb9073d496d2 Mon Sep 17 00:00:00 2001 From: Tariq Ibrahim Date: Thu, 8 Aug 2024 15:40:00 -0700 Subject: [PATCH] fetch current container runtime config through the command line Signed-off-by: Tariq Ibrahim --- .golangci.yml | 4 ++ pkg/config/engine/containerd/option.go | 41 ++++++++++++++------- pkg/config/engine/crio/option.go | 47 ++++++++++++++---------- tools/container/containerd/containerd.go | 1 + tools/container/crio/crio.go | 1 + 5 files changed, 61 insertions(+), 33 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index b4c2f1c2..f6bb86b1 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -18,6 +18,10 @@ linters: linters-settings: goimports: local-prefixes: github.com/NVIDIA/nvidia-container-toolkit + gosec: + excludes: + # TODO: Consider hardening security of command line invocations + - G204 issues: exclude: diff --git a/pkg/config/engine/containerd/option.go b/pkg/config/engine/containerd/option.go index ae1d9944..af824b08 100644 --- a/pkg/config/engine/containerd/option.go +++ b/pkg/config/engine/containerd/option.go @@ -18,7 +18,7 @@ package containerd import ( "fmt" - "os" + "os/exec" "github.com/pelletier/go-toml" @@ -36,6 +36,7 @@ type builder struct { runtimeType string useLegacyConfig bool containerAnnotations []string + hostRootMount string } // Option defines a function that can be used to configure the config builder @@ -76,6 +77,13 @@ func WithContainerAnnotations(containerAnnotations ...string) Option { } } +// WithHostRootMount sets the container annotations for the config builder +func WithHostRootMount(hostRootMount string) Option { + return func(b *builder) { + b.hostRootMount = hostRootMount + } +} + func (b *builder) build() (engine.Interface, error) { if b.path == "" { return nil, fmt.Errorf("config path is empty") @@ -85,7 +93,7 @@ func (b *builder) build() (engine.Interface, error) { b.runtimeType = defaultRuntimeType } - config, err := b.loadConfig(b.path) + config, err := b.loadConfig() if err != nil { return nil, fmt.Errorf("failed to load config: %v", err) } @@ -109,23 +117,28 @@ func (b *builder) build() (engine.Interface, error) { } // loadConfig loads the containerd config from disk -func (b *builder) loadConfig(config string) (*Config, error) { - info, err := os.Stat(config) - if os.IsExist(err) && info.IsDir() { - return nil, fmt.Errorf("config file is a directory") - } +func (b *builder) loadConfig() (*Config, error) { + var args []string - if os.IsNotExist(err) { - b.logger.Infof("Config file does not exist; using empty config") - config = "/dev/null" - } else { - b.logger.Infof("Loading config from %v", config) + if b.hostRootMount != "" { + args = append(args, "chroot", b.hostRootMount) } + args = append(args, "containerd", "config", "dump") - tomlConfig, err := toml.LoadFile(config) + b.logger.Infof("Getting current containerd config") + + // TODO: Can we harden this so that there is less risk of command injection + cmd := exec.Command(args[0], args[1:]...) + output, err := cmd.Output() if err != nil { - return nil, err + return nil, fmt.Errorf("error executing containerd confg command: %w", err) } + tomlConfig, err := toml.LoadBytes(output) + if err != nil { + return nil, fmt.Errorf("error unmarshaling containerd config toml: %w", err) + } + + b.logger.Infof("Successfully loaded containerd config") cfg := Config{ Tree: tomlConfig, diff --git a/pkg/config/engine/crio/option.go b/pkg/config/engine/crio/option.go index 9f4d10fc..7476a088 100644 --- a/pkg/config/engine/crio/option.go +++ b/pkg/config/engine/crio/option.go @@ -18,7 +18,7 @@ package crio import ( "fmt" - "os" + "os/exec" "github.com/pelletier/go-toml" @@ -26,8 +26,9 @@ import ( ) type builder struct { - logger logger.Interface - path string + logger logger.Interface + path string + hostRootMount string } // Option defines a function that can be used to configure the config builder @@ -47,6 +48,13 @@ func WithPath(path string) Option { } } +// WithHostRootMount sets the host root mount for the config builder +func WithHostRootMount(hostRootMount string) Option { + return func(b *builder) { + b.hostRootMount = hostRootMount + } +} + func (b *builder) build() (*Config, error) { if b.logger == nil { b.logger = logger.New() @@ -60,34 +68,35 @@ func (b *builder) build() (*Config, error) { return &c, nil } - return b.loadConfig(b.path) + return b.loadConfig() } // loadConfig loads the cri-o config from disk -func (b *builder) loadConfig(config string) (*Config, error) { - b.logger.Infof("Loading config: %v", config) +func (b *builder) loadConfig() (*Config, error) { - info, err := os.Stat(config) - if os.IsExist(err) && info.IsDir() { - return nil, fmt.Errorf("config file is a directory") + var args []string + if b.hostRootMount != "" { + args = append(args, "chroot", b.hostRootMount) } + args = append(args, "crio", "status", "config") - if os.IsNotExist(err) { - b.logger.Infof("Config file does not exist; using empty config") - config = "/dev/null" - } else { - b.logger.Infof("Loading config from %v", config) - } + b.logger.Infof("Getting current crio config") - cfg, err := toml.LoadFile(config) + // TODO: Can we harden this so that there is less risk of command injection + cmd := exec.Command(args[0], args[1:]...) + output, err := cmd.Output() if err != nil { - return nil, err + return nil, fmt.Errorf("error executing crio status command: %w", err) + } + tomlConfig, err := toml.LoadBytes(output) + if err != nil { + return nil, fmt.Errorf("error unmarshaling crio config toml: %w", err) } - b.logger.Infof("Successfully loaded config") + b.logger.Infof("Successfully loaded crio config") c := Config{ - Tree: cfg, + Tree: tomlConfig, Logger: b.logger, } return &c, nil diff --git a/tools/container/containerd/containerd.go b/tools/container/containerd/containerd.go index b5dff882..cd67a0f0 100644 --- a/tools/container/containerd/containerd.go +++ b/tools/container/containerd/containerd.go @@ -193,6 +193,7 @@ func Setup(c *cli.Context, o *options) error { containerd.WithRuntimeType(o.runtimeType), containerd.WithUseLegacyConfig(o.useLegacyConfig), containerd.WithContainerAnnotations(o.containerAnnotationsFromCDIPrefixes()...), + containerd.WithHostRootMount(o.HostRootMount), ) if err != nil { return fmt.Errorf("unable to load config: %v", err) diff --git a/tools/container/crio/crio.go b/tools/container/crio/crio.go index 653998c7..29c42725 100644 --- a/tools/container/crio/crio.go +++ b/tools/container/crio/crio.go @@ -222,6 +222,7 @@ func setupConfig(o *options) error { cfg, err := crio.New( crio.WithPath(o.Config), + crio.WithHostRootMount(o.HostRootMount), ) if err != nil { return fmt.Errorf("unable to load config: %v", err)