Merge pull request #690 from elezar/ignore-ldconfig-option
Some checks failed
CodeQL / Analyze Go code with CodeQL (push) Has been cancelled
Golang / check (push) Has been cancelled
Golang / Unit test (push) Has been cancelled
Golang / Build (push) Has been cancelled
image / packages (${{github.event_name == 'pull_request'}}, centos7-aarch64) (push) Has been cancelled
image / packages (${{github.event_name == 'pull_request'}}, centos7-x86_64) (push) Has been cancelled
image / packages (${{github.event_name == 'pull_request'}}, centos8-ppc64le) (push) Has been cancelled
image / packages (${{github.event_name == 'pull_request'}}, ubuntu18.04-amd64) (push) Has been cancelled
image / packages (${{github.event_name == 'pull_request'}}, ubuntu18.04-arm64) (push) Has been cancelled
image / packages (${{github.event_name == 'pull_request'}}, ubuntu18.04-ppc64le) (push) Has been cancelled
image / image (packaging, ${{github.event_name == 'pull_request'}}) (push) Has been cancelled
image / image (ubi8, ${{github.event_name == 'pull_request'}}) (push) Has been cancelled
image / image (ubuntu20.04, ${{github.event_name == 'pull_request'}}) (push) Has been cancelled

Only allow host-relative LDConfig paths
This commit is contained in:
Evan Lezar 2024-11-26 13:35:33 +01:00 committed by GitHub
commit dcd65a6410
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
15 changed files with 320 additions and 136 deletions

View File

@ -157,7 +157,7 @@ func getDevicesFromEnvvar(containerImage image.CUDA, swarmResourceEnvvars []stri
return containerImage.VisibleDevicesFromEnvVar() return containerImage.VisibleDevicesFromEnvVar()
} }
func getDevices(hookConfig *HookConfig, image image.CUDA, privileged bool) []string { func (hookConfig *hookConfig) getDevices(image image.CUDA, privileged bool) []string {
// If enabled, try and get the device list from volume mounts first // If enabled, try and get the device list from volume mounts first
if hookConfig.AcceptDeviceListAsVolumeMounts { if hookConfig.AcceptDeviceListAsVolumeMounts {
devices := image.VisibleDevicesFromMounts() devices := image.VisibleDevicesFromMounts()
@ -197,7 +197,7 @@ func getMigDevices(image image.CUDA, envvar string) *string {
return &devices return &devices
} }
func getImexChannels(hookConfig *HookConfig, image image.CUDA, privileged bool) []string { func (hookConfig *hookConfig) getImexChannels(image image.CUDA, privileged bool) []string {
// If enabled, try and get the device list from volume mounts first // If enabled, try and get the device list from volume mounts first
if hookConfig.AcceptDeviceListAsVolumeMounts { if hookConfig.AcceptDeviceListAsVolumeMounts {
devices := image.ImexChannelsFromMounts() devices := image.ImexChannelsFromMounts()
@ -217,10 +217,10 @@ func getImexChannels(hookConfig *HookConfig, image image.CUDA, privileged bool)
return nil return nil
} }
func (c *HookConfig) getDriverCapabilities(cudaImage image.CUDA, legacyImage bool) image.DriverCapabilities { func (hookConfig *hookConfig) getDriverCapabilities(cudaImage image.CUDA, legacyImage bool) image.DriverCapabilities {
// We use the default driver capabilities by default. This is filtered to only include the // We use the default driver capabilities by default. This is filtered to only include the
// supported capabilities // supported capabilities
supportedDriverCapabilities := image.NewDriverCapabilities(c.SupportedDriverCapabilities) supportedDriverCapabilities := image.NewDriverCapabilities(hookConfig.SupportedDriverCapabilities)
capabilities := supportedDriverCapabilities.Intersection(image.DefaultDriverCapabilities) capabilities := supportedDriverCapabilities.Intersection(image.DefaultDriverCapabilities)
@ -244,10 +244,10 @@ func (c *HookConfig) getDriverCapabilities(cudaImage image.CUDA, legacyImage boo
return capabilities return capabilities
} }
func getNvidiaConfig(hookConfig *HookConfig, image image.CUDA, privileged bool) *nvidiaConfig { func (hookConfig *hookConfig) getNvidiaConfig(image image.CUDA, privileged bool) *nvidiaConfig {
legacyImage := image.IsLegacy() legacyImage := image.IsLegacy()
devices := getDevices(hookConfig, image, privileged) devices := hookConfig.getDevices(image, privileged)
if len(devices) == 0 { if len(devices) == 0 {
// empty devices means this is not a GPU container. // empty devices means this is not a GPU container.
return nil return nil
@ -269,7 +269,7 @@ func getNvidiaConfig(hookConfig *HookConfig, image image.CUDA, privileged bool)
log.Panicln("cannot set MIG_MONITOR_DEVICES in non privileged container") log.Panicln("cannot set MIG_MONITOR_DEVICES in non privileged container")
} }
imexChannels := getImexChannels(hookConfig, image, privileged) imexChannels := hookConfig.getImexChannels(image, privileged)
driverCapabilities := hookConfig.getDriverCapabilities(image, legacyImage).String() driverCapabilities := hookConfig.getDriverCapabilities(image, legacyImage).String()
@ -288,7 +288,7 @@ func getNvidiaConfig(hookConfig *HookConfig, image image.CUDA, privileged bool)
} }
} }
func getContainerConfig(hook HookConfig) (config containerConfig) { func (hookConfig *hookConfig) getContainerConfig() (config containerConfig) {
var h HookState var h HookState
d := json.NewDecoder(os.Stdin) d := json.NewDecoder(os.Stdin)
if err := d.Decode(&h); err != nil { if err := d.Decode(&h); err != nil {
@ -305,7 +305,7 @@ func getContainerConfig(hook HookConfig) (config containerConfig) {
image, err := image.New( image, err := image.New(
image.WithEnv(s.Process.Env), image.WithEnv(s.Process.Env),
image.WithMounts(s.Mounts), image.WithMounts(s.Mounts),
image.WithDisableRequire(hook.DisableRequire), image.WithDisableRequire(hookConfig.DisableRequire),
) )
if err != nil { if err != nil {
log.Panicln(err) log.Panicln(err)
@ -316,6 +316,6 @@ func getContainerConfig(hook HookConfig) (config containerConfig) {
Pid: h.Pid, Pid: h.Pid,
Rootfs: s.Root.Path, Rootfs: s.Root.Path,
Image: image, Image: image,
Nvidia: getNvidiaConfig(&hook, image, privileged), Nvidia: hookConfig.getNvidiaConfig(image, privileged),
} }
} }

View File

@ -7,6 +7,7 @@ import (
"github.com/opencontainers/runtime-spec/specs-go" "github.com/opencontainers/runtime-spec/specs-go"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
"github.com/NVIDIA/nvidia-container-toolkit/internal/config"
"github.com/NVIDIA/nvidia-container-toolkit/internal/config/image" "github.com/NVIDIA/nvidia-container-toolkit/internal/config/image"
) )
@ -15,7 +16,7 @@ func TestGetNvidiaConfig(t *testing.T) {
description string description string
env map[string]string env map[string]string
privileged bool privileged bool
hookConfig *HookConfig hookConfig *hookConfig
expectedConfig *nvidiaConfig expectedConfig *nvidiaConfig
expectedPanic bool expectedPanic bool
}{ }{
@ -394,8 +395,10 @@ func TestGetNvidiaConfig(t *testing.T) {
image.EnvVarNvidiaDriverCapabilities: "all", image.EnvVarNvidiaDriverCapabilities: "all",
}, },
privileged: true, privileged: true,
hookConfig: &HookConfig{ hookConfig: &hookConfig{
SupportedDriverCapabilities: "video,display", Config: &config.Config{
SupportedDriverCapabilities: "video,display",
},
}, },
expectedConfig: &nvidiaConfig{ expectedConfig: &nvidiaConfig{
Devices: []string{"all"}, Devices: []string{"all"},
@ -409,8 +412,10 @@ func TestGetNvidiaConfig(t *testing.T) {
image.EnvVarNvidiaDriverCapabilities: "video,display", image.EnvVarNvidiaDriverCapabilities: "video,display",
}, },
privileged: true, privileged: true,
hookConfig: &HookConfig{ hookConfig: &hookConfig{
SupportedDriverCapabilities: "video,display,compute,utility", Config: &config.Config{
SupportedDriverCapabilities: "video,display,compute,utility",
},
}, },
expectedConfig: &nvidiaConfig{ expectedConfig: &nvidiaConfig{
Devices: []string{"all"}, Devices: []string{"all"},
@ -423,8 +428,10 @@ func TestGetNvidiaConfig(t *testing.T) {
image.EnvVarNvidiaVisibleDevices: "all", image.EnvVarNvidiaVisibleDevices: "all",
}, },
privileged: true, privileged: true,
hookConfig: &HookConfig{ hookConfig: &hookConfig{
SupportedDriverCapabilities: "video,display,utility,compute", Config: &config.Config{
SupportedDriverCapabilities: "video,display,utility,compute",
},
}, },
expectedConfig: &nvidiaConfig{ expectedConfig: &nvidiaConfig{
Devices: []string{"all"}, Devices: []string{"all"},
@ -438,9 +445,11 @@ func TestGetNvidiaConfig(t *testing.T) {
"DOCKER_SWARM_RESOURCE": "GPU1,GPU2", "DOCKER_SWARM_RESOURCE": "GPU1,GPU2",
}, },
privileged: true, privileged: true,
hookConfig: &HookConfig{ hookConfig: &hookConfig{
SwarmResource: "DOCKER_SWARM_RESOURCE", Config: &config.Config{
SupportedDriverCapabilities: "video,display,utility,compute", SwarmResource: "DOCKER_SWARM_RESOURCE",
SupportedDriverCapabilities: "video,display,utility,compute",
},
}, },
expectedConfig: &nvidiaConfig{ expectedConfig: &nvidiaConfig{
Devices: []string{"GPU1", "GPU2"}, Devices: []string{"GPU1", "GPU2"},
@ -454,9 +463,11 @@ func TestGetNvidiaConfig(t *testing.T) {
"DOCKER_SWARM_RESOURCE": "GPU1,GPU2", "DOCKER_SWARM_RESOURCE": "GPU1,GPU2",
}, },
privileged: true, privileged: true,
hookConfig: &HookConfig{ hookConfig: &hookConfig{
SwarmResource: "NOT_DOCKER_SWARM_RESOURCE,DOCKER_SWARM_RESOURCE", Config: &config.Config{
SupportedDriverCapabilities: "video,display,utility,compute", SwarmResource: "NOT_DOCKER_SWARM_RESOURCE,DOCKER_SWARM_RESOURCE",
SupportedDriverCapabilities: "video,display,utility,compute",
},
}, },
expectedConfig: &nvidiaConfig{ expectedConfig: &nvidiaConfig{
Devices: []string{"GPU1", "GPU2"}, Devices: []string{"GPU1", "GPU2"},
@ -470,14 +481,14 @@ func TestGetNvidiaConfig(t *testing.T) {
image.WithEnvMap(tc.env), image.WithEnvMap(tc.env),
) )
// Wrap the call to getNvidiaConfig() in a closure. // Wrap the call to getNvidiaConfig() in a closure.
var config *nvidiaConfig var cfg *nvidiaConfig
getConfig := func() { getConfig := func() {
hookConfig := tc.hookConfig hookCfg := tc.hookConfig
if hookConfig == nil { if hookCfg == nil {
defaultConfig, _ := getDefaultHookConfig() defaultConfig, _ := config.GetDefault()
hookConfig = &defaultConfig hookCfg = &hookConfig{defaultConfig}
} }
config = getNvidiaConfig(hookConfig, image, tc.privileged) cfg = hookCfg.getNvidiaConfig(image, tc.privileged)
} }
// For any tests that are expected to panic, make sure they do. // For any tests that are expected to panic, make sure they do.
@ -491,18 +502,18 @@ func TestGetNvidiaConfig(t *testing.T) {
// And start comparing the test results to the expected results. // And start comparing the test results to the expected results.
if tc.expectedConfig == nil { if tc.expectedConfig == nil {
require.Nil(t, config, tc.description) require.Nil(t, cfg, tc.description)
return return
} }
require.NotNil(t, config, tc.description) require.NotNil(t, cfg, tc.description)
require.Equal(t, tc.expectedConfig.Devices, config.Devices) require.Equal(t, tc.expectedConfig.Devices, cfg.Devices)
require.Equal(t, tc.expectedConfig.MigConfigDevices, config.MigConfigDevices) require.Equal(t, tc.expectedConfig.MigConfigDevices, cfg.MigConfigDevices)
require.Equal(t, tc.expectedConfig.MigMonitorDevices, config.MigMonitorDevices) require.Equal(t, tc.expectedConfig.MigMonitorDevices, cfg.MigMonitorDevices)
require.Equal(t, tc.expectedConfig.DriverCapabilities, config.DriverCapabilities) require.Equal(t, tc.expectedConfig.DriverCapabilities, cfg.DriverCapabilities)
require.ElementsMatch(t, tc.expectedConfig.Requirements, config.Requirements) require.ElementsMatch(t, tc.expectedConfig.Requirements, cfg.Requirements)
}) })
} }
} }
@ -612,10 +623,11 @@ func TestDeviceListSourcePriority(t *testing.T) {
), ),
image.WithMounts(tc.mountDevices), image.WithMounts(tc.mountDevices),
) )
hookConfig, _ := getDefaultHookConfig() defaultConfig, _ := config.GetDefault()
hookConfig.AcceptEnvvarUnprivileged = tc.acceptUnprivileged cfg := &hookConfig{defaultConfig}
hookConfig.AcceptDeviceListAsVolumeMounts = tc.acceptMounts cfg.AcceptEnvvarUnprivileged = tc.acceptUnprivileged
devices = getDevices(&hookConfig, image, tc.privileged) cfg.AcceptDeviceListAsVolumeMounts = tc.acceptMounts
devices = cfg.getDevices(image, tc.privileged)
} }
// For all other tests, just grab the devices and check the results // For all other tests, just grab the devices and check the results
@ -940,8 +952,10 @@ func TestGetDriverCapabilities(t *testing.T) {
t.Run(tc.description, func(t *testing.T) { t.Run(tc.description, func(t *testing.T) {
var capabilities string var capabilities string
c := HookConfig{ c := hookConfig{
SupportedDriverCapabilities: tc.supportedCapabilities, Config: &config.Config{
SupportedDriverCapabilities: tc.supportedCapabilities,
},
} }
image, _ := image.New( image, _ := image.New(

View File

@ -17,16 +17,10 @@ const (
driverPath = "/run/nvidia/driver" driverPath = "/run/nvidia/driver"
) )
// HookConfig : options for the nvidia-container-runtime-hook. // hookConfig wraps the toolkit config.
type HookConfig config.Config // This allows for functions to be defined on the local type.
type hookConfig struct {
func getDefaultHookConfig() (HookConfig, error) { *config.Config
defaultCfg, err := config.GetDefault()
if err != nil {
return HookConfig{}, err
}
return *(*HookConfig)(defaultCfg), nil
} }
// loadConfig loads the required paths for the hook config. // loadConfig loads the required paths for the hook config.
@ -56,12 +50,12 @@ func loadConfig() (*config.Config, error) {
return config.GetDefault() return config.GetDefault()
} }
func getHookConfig() (*HookConfig, error) { func getHookConfig() (*hookConfig, error) {
cfg, err := loadConfig() cfg, err := loadConfig()
if err != nil { if err != nil {
return nil, fmt.Errorf("failed to load config: %v", err) return nil, fmt.Errorf("failed to load config: %v", err)
} }
config := (*HookConfig)(cfg) config := &hookConfig{cfg}
allSupportedDriverCapabilities := image.SupportedDriverCapabilities allSupportedDriverCapabilities := image.SupportedDriverCapabilities
if config.SupportedDriverCapabilities == "all" { if config.SupportedDriverCapabilities == "all" {
@ -79,7 +73,7 @@ func getHookConfig() (*HookConfig, error) {
// getConfigOption returns the toml config option associated with the // getConfigOption returns the toml config option associated with the
// specified struct field. // specified struct field.
func (c HookConfig) getConfigOption(fieldName string) string { func (c hookConfig) getConfigOption(fieldName string) string {
t := reflect.TypeOf(c) t := reflect.TypeOf(c)
f, ok := t.FieldByName(fieldName) f, ok := t.FieldByName(fieldName)
if !ok { if !ok {
@ -93,7 +87,7 @@ func (c HookConfig) getConfigOption(fieldName string) string {
} }
// getSwarmResourceEnvvars returns the swarm resource envvars for the config. // getSwarmResourceEnvvars returns the swarm resource envvars for the config.
func (c *HookConfig) getSwarmResourceEnvvars() []string { func (c *hookConfig) getSwarmResourceEnvvars() []string {
if c.SwarmResource == "" { if c.SwarmResource == "" {
return nil return nil
} }

View File

@ -23,6 +23,7 @@ import (
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
"github.com/NVIDIA/nvidia-container-toolkit/internal/config"
"github.com/NVIDIA/nvidia-container-toolkit/internal/config/image" "github.com/NVIDIA/nvidia-container-toolkit/internal/config/image"
) )
@ -89,10 +90,10 @@ func TestGetHookConfig(t *testing.T) {
} }
} }
var config HookConfig var cfg hookConfig
getHookConfig := func() { getHookConfig := func() {
c, _ := getHookConfig() c, _ := getHookConfig()
config = *c cfg = *c
} }
if tc.expectedPanic { if tc.expectedPanic {
@ -102,7 +103,7 @@ func TestGetHookConfig(t *testing.T) {
getHookConfig() getHookConfig()
require.EqualValues(t, tc.expectedDriverCapabilities, config.SupportedDriverCapabilities) require.EqualValues(t, tc.expectedDriverCapabilities, cfg.SupportedDriverCapabilities)
}) })
} }
} }
@ -144,8 +145,10 @@ func TestGetSwarmResourceEnvvars(t *testing.T) {
for i, tc := range testCases { for i, tc := range testCases {
t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { t.Run(fmt.Sprintf("%d", i), func(t *testing.T) {
c := &HookConfig{ c := &hookConfig{
SwarmResource: tc.value, Config: &config.Config{
SwarmResource: tc.value,
},
} }
envvars := c.getSwarmResourceEnvvars() envvars := c.getSwarmResourceEnvvars()

View File

@ -75,7 +75,7 @@ func doPrestart() {
} }
cli := hook.NVIDIAContainerCLIConfig cli := hook.NVIDIAContainerCLIConfig
container := getContainerConfig(*hook) container := hook.getContainerConfig()
nvidia := container.Nvidia nvidia := container.Nvidia
if nvidia == nil { if nvidia == nil {
// Not a GPU container, nothing to do. // Not a GPU container, nothing to do.

View File

@ -17,6 +17,7 @@
package config package config
import ( import (
"fmt"
"os" "os"
"strings" "strings"
) )
@ -34,29 +35,62 @@ type ContainerCLIConfig struct {
NoPivot bool `toml:"no-pivot,omitempty"` NoPivot bool `toml:"no-pivot,omitempty"`
NoCgroups bool `toml:"no-cgroups"` NoCgroups bool `toml:"no-cgroups"`
User string `toml:"user"` User string `toml:"user"`
Ldconfig string `toml:"ldconfig"` // Ldconfig represents the path to the ldconfig binary to be used to update
// the ldcache in a container as it is being created.
// If this path starts with a '@' the path is relative to the host and if
// not it is treated as a container path.
//
// Note that the use of container paths are disabled by default and if this
// is required, the features.allow-ldconfig-from-container feature gate must
// be enabled explicitly.
Ldconfig ldconfigPath `toml:"ldconfig"`
} }
// NormalizeLDConfigPath returns the resolved path of the configured LDConfig binary. // NormalizeLDConfigPath returns the resolved path of the configured LDConfig binary.
// This is only done for host LDConfigs and is required to handle systems where // This is only done for host LDConfigs and is required to handle systems where
// /sbin/ldconfig is a wrapper around /sbin/ldconfig.real. // /sbin/ldconfig is a wrapper around /sbin/ldconfig.real.
func (c *ContainerCLIConfig) NormalizeLDConfigPath() string { func (c *ContainerCLIConfig) NormalizeLDConfigPath() string {
return NormalizeLDConfigPath(c.Ldconfig) return string(c.Ldconfig.normalize())
}
// An ldconfigPath is used to represent the path to ldconfig.
type ldconfigPath string
func (p ldconfigPath) assertValid(allowContainerRelativePath bool) error {
if p.isHostRelative() {
return nil
}
if allowContainerRelativePath {
return nil
}
return fmt.Errorf("nvidia-container-cli.ldconfig value %q is not host-relative (does not start with a '@')", p)
}
func (p ldconfigPath) isHostRelative() bool {
return strings.HasPrefix(string(p), "@")
}
// normalize returns the resolved path of the configured LDConfig binary.
// This is only done for host LDConfigs and is required to handle systems where
// /sbin/ldconfig is a wrapper around /sbin/ldconfig.real.
func (p ldconfigPath) normalize() ldconfigPath {
if !p.isHostRelative() {
return p
}
path := string(p)
trimmedPath := strings.TrimSuffix(strings.TrimPrefix(path, "@"), ".real")
// If the .real path exists, we return that.
if _, err := os.Stat(trimmedPath + ".real"); err == nil {
return ldconfigPath("@" + trimmedPath + ".real")
}
// If the .real path does not exists (or cannot be read) we return the non-.real path.
return ldconfigPath("@" + trimmedPath)
} }
// NormalizeLDConfigPath returns the resolved path of the configured LDConfig binary. // NormalizeLDConfigPath returns the resolved path of the configured LDConfig binary.
// This is only done for host LDConfigs and is required to handle systems where // This is only done for host LDConfigs and is required to handle systems where
// /sbin/ldconfig is a wrapper around /sbin/ldconfig.real. // /sbin/ldconfig is a wrapper around /sbin/ldconfig.real.
func NormalizeLDConfigPath(path string) string { func NormalizeLDConfigPath(path string) string {
if !strings.HasPrefix(path, "@") { return string(ldconfigPath(path).normalize())
return path
}
trimmedPath := strings.TrimSuffix(strings.TrimPrefix(path, "@"), ".real")
// If the .real path exists, we return that.
if _, err := os.Stat(trimmedPath + ".real"); err == nil {
return "@" + trimmedPath + ".real"
}
// If the .real path does not exists (or cannot be read) we return the non-.real path.
return "@" + trimmedPath
} }

View File

@ -33,7 +33,7 @@ func TestNormalizeLDConfigPath(t *testing.T) {
testCases := []struct { testCases := []struct {
description string description string
ldconfig string ldconfig ldconfigPath
expected string expected string
}{ }{
{ {
@ -51,12 +51,12 @@ func TestNormalizeLDConfigPath(t *testing.T) {
}, },
{ {
description: "host .real file exists is returned", description: "host .real file exists is returned",
ldconfig: "@" + filepath.Join(testDir, "exists.real"), ldconfig: ldconfigPath("@" + filepath.Join(testDir, "exists.real")),
expected: "@" + filepath.Join(testDir, "exists.real"), expected: "@" + filepath.Join(testDir, "exists.real"),
}, },
{ {
description: "host resolves .real file", description: "host resolves .real file",
ldconfig: "@" + filepath.Join(testDir, "exists"), ldconfig: ldconfigPath("@" + filepath.Join(testDir, "exists")),
expected: "@" + filepath.Join(testDir, "exists.real"), expected: "@" + filepath.Join(testDir, "exists.real"),
}, },
{ {

View File

@ -18,6 +18,7 @@ package config
import ( import (
"bufio" "bufio"
"errors"
"os" "os"
"path/filepath" "path/filepath"
"strings" "strings"
@ -51,6 +52,8 @@ var (
NVIDIAContainerToolkitExecutable = "nvidia-container-toolkit" NVIDIAContainerToolkitExecutable = "nvidia-container-toolkit"
) )
var errInvalidConfig = errors.New("invalid config value")
// Config represents the contents of the config.toml file for the NVIDIA Container Toolkit // Config represents the contents of the config.toml file for the NVIDIA Container Toolkit
// Note: This is currently duplicated by the HookConfig in cmd/nvidia-container-toolkit/hook_config.go // Note: This is currently duplicated by the HookConfig in cmd/nvidia-container-toolkit/hook_config.go
type Config struct { type Config struct {
@ -127,8 +130,20 @@ func GetDefault() (*Config, error) {
return &d, nil return &d, nil
} }
func getLdConfigPath() string { // assertValid checks for a valid config.
return NormalizeLDConfigPath("@/sbin/ldconfig") func (c *Config) assertValid() error {
err := c.NVIDIAContainerCLIConfig.Ldconfig.assertValid(c.Features.AllowLDConfigFromContainer.IsEnabled())
if err != nil {
return errors.Join(err, errInvalidConfig)
}
return nil
}
// getLdConfigPath allows us to override this function for testing.
var getLdConfigPath = getLdConfigPathStub
func getLdConfigPathStub() ldconfigPath {
return ldconfigPath("@/sbin/ldconfig").normalize()
} }
func getUserGroup() string { func getUserGroup() string {

View File

@ -44,23 +44,21 @@ func TestGetConfigWithCustomConfig(t *testing.T) {
func TestGetConfig(t *testing.T) { func TestGetConfig(t *testing.T) {
testCases := []struct { testCases := []struct {
description string description string
contents []string contents []string
expectedError error expectedError error
inspectLdconfig bool distIdsLike []string
distIdsLike []string expectedConfig *Config
expectedConfig *Config
}{ }{
{ {
description: "empty config is default", description: "empty config is default",
inspectLdconfig: true,
expectedConfig: &Config{ expectedConfig: &Config{
AcceptEnvvarUnprivileged: true, AcceptEnvvarUnprivileged: true,
SupportedDriverCapabilities: "compat32,compute,display,graphics,ngx,utility,video", SupportedDriverCapabilities: "compat32,compute,display,graphics,ngx,utility,video",
NVIDIAContainerCLIConfig: ContainerCLIConfig{ NVIDIAContainerCLIConfig: ContainerCLIConfig{
Root: "", Root: "",
LoadKmods: true, LoadKmods: true,
Ldconfig: "WAS_CHECKED", Ldconfig: "@/test/ld/config/path",
}, },
NVIDIAContainerRuntimeConfig: RuntimeConfig{ NVIDIAContainerRuntimeConfig: RuntimeConfig{
DebugFilePath: "/dev/null", DebugFilePath: "/dev/null",
@ -93,7 +91,7 @@ func TestGetConfig(t *testing.T) {
"supported-driver-capabilities = \"compute,utility\"", "supported-driver-capabilities = \"compute,utility\"",
"nvidia-container-cli.root = \"/bar/baz\"", "nvidia-container-cli.root = \"/bar/baz\"",
"nvidia-container-cli.load-kmods = false", "nvidia-container-cli.load-kmods = false",
"nvidia-container-cli.ldconfig = \"/foo/bar/ldconfig\"", "nvidia-container-cli.ldconfig = \"@/foo/bar/ldconfig\"",
"nvidia-container-cli.user = \"foo:bar\"", "nvidia-container-cli.user = \"foo:bar\"",
"nvidia-container-runtime.debug = \"/foo/bar\"", "nvidia-container-runtime.debug = \"/foo/bar\"",
"nvidia-container-runtime.discover-mode = \"not-legacy\"", "nvidia-container-runtime.discover-mode = \"not-legacy\"",
@ -113,7 +111,7 @@ func TestGetConfig(t *testing.T) {
NVIDIAContainerCLIConfig: ContainerCLIConfig{ NVIDIAContainerCLIConfig: ContainerCLIConfig{
Root: "/bar/baz", Root: "/bar/baz",
LoadKmods: false, LoadKmods: false,
Ldconfig: "/foo/bar/ldconfig", Ldconfig: "@/foo/bar/ldconfig",
User: "foo:bar", User: "foo:bar",
}, },
NVIDIAContainerRuntimeConfig: RuntimeConfig{ NVIDIAContainerRuntimeConfig: RuntimeConfig{
@ -146,6 +144,53 @@ func TestGetConfig(t *testing.T) {
}, },
}, },
}, },
{
description: "feature allows ldconfig to be overridden",
contents: []string{
"[nvidia-container-cli]",
"ldconfig = \"/foo/bar/ldconfig\"",
"[features]",
"allow-ldconfig-from-container = true",
},
expectedConfig: &Config{
AcceptEnvvarUnprivileged: true,
SupportedDriverCapabilities: "compat32,compute,display,graphics,ngx,utility,video",
NVIDIAContainerCLIConfig: ContainerCLIConfig{
Ldconfig: "/foo/bar/ldconfig",
LoadKmods: true,
},
NVIDIAContainerRuntimeConfig: RuntimeConfig{
DebugFilePath: "/dev/null",
LogLevel: "info",
Runtimes: []string{"docker-runc", "runc", "crun"},
Mode: "auto",
Modes: modesConfig{
CSV: csvModeConfig{
MountSpecPath: "/etc/nvidia-container-runtime/host-files-for-container.d",
},
CDI: cdiModeConfig{
DefaultKind: "nvidia.com/gpu",
AnnotationPrefixes: []string{
"cdi.k8s.io/",
},
SpecDirs: []string{
"/etc/cdi",
"/var/run/cdi",
},
},
},
},
NVIDIAContainerRuntimeHookConfig: RuntimeHookConfig{
Path: "nvidia-container-runtime-hook",
},
NVIDIACTKConfig: CTKConfig{
Path: "nvidia-ctk",
},
Features: features{
AllowLDConfigFromContainer: ptr(feature(true)),
},
},
},
{ {
description: "config options set in section", description: "config options set in section",
contents: []string{ contents: []string{
@ -154,7 +199,7 @@ func TestGetConfig(t *testing.T) {
"[nvidia-container-cli]", "[nvidia-container-cli]",
"root = \"/bar/baz\"", "root = \"/bar/baz\"",
"load-kmods = false", "load-kmods = false",
"ldconfig = \"/foo/bar/ldconfig\"", "ldconfig = \"@/foo/bar/ldconfig\"",
"user = \"foo:bar\"", "user = \"foo:bar\"",
"[nvidia-container-runtime]", "[nvidia-container-runtime]",
"debug = \"/foo/bar\"", "debug = \"/foo/bar\"",
@ -179,7 +224,7 @@ func TestGetConfig(t *testing.T) {
NVIDIAContainerCLIConfig: ContainerCLIConfig{ NVIDIAContainerCLIConfig: ContainerCLIConfig{
Root: "/bar/baz", Root: "/bar/baz",
LoadKmods: false, LoadKmods: false,
Ldconfig: "/foo/bar/ldconfig", Ldconfig: "@/foo/bar/ldconfig",
User: "foo:bar", User: "foo:bar",
}, },
NVIDIAContainerRuntimeConfig: RuntimeConfig{ NVIDIAContainerRuntimeConfig: RuntimeConfig{
@ -213,16 +258,15 @@ func TestGetConfig(t *testing.T) {
}, },
}, },
{ {
description: "suse config", description: "suse config",
distIdsLike: []string{"suse", "opensuse"}, distIdsLike: []string{"suse", "opensuse"},
inspectLdconfig: true,
expectedConfig: &Config{ expectedConfig: &Config{
AcceptEnvvarUnprivileged: true, AcceptEnvvarUnprivileged: true,
SupportedDriverCapabilities: "compat32,compute,display,graphics,ngx,utility,video", SupportedDriverCapabilities: "compat32,compute,display,graphics,ngx,utility,video",
NVIDIAContainerCLIConfig: ContainerCLIConfig{ NVIDIAContainerCLIConfig: ContainerCLIConfig{
Root: "", Root: "",
LoadKmods: true, LoadKmods: true,
Ldconfig: "WAS_CHECKED", Ldconfig: "@/test/ld/config/path",
User: "root:video", User: "root:video",
}, },
NVIDIAContainerRuntimeConfig: RuntimeConfig{ NVIDIAContainerRuntimeConfig: RuntimeConfig{
@ -250,9 +294,8 @@ func TestGetConfig(t *testing.T) {
}, },
}, },
{ {
description: "suse config overrides user", description: "suse config overrides user",
distIdsLike: []string{"suse", "opensuse"}, distIdsLike: []string{"suse", "opensuse"},
inspectLdconfig: true,
contents: []string{ contents: []string{
"nvidia-container-cli.user = \"foo:bar\"", "nvidia-container-cli.user = \"foo:bar\"",
}, },
@ -262,7 +305,7 @@ func TestGetConfig(t *testing.T) {
NVIDIAContainerCLIConfig: ContainerCLIConfig{ NVIDIAContainerCLIConfig: ContainerCLIConfig{
Root: "", Root: "",
LoadKmods: true, LoadKmods: true,
Ldconfig: "WAS_CHECKED", Ldconfig: "@/test/ld/config/path",
User: "foo:bar", User: "foo:bar",
}, },
NVIDIAContainerRuntimeConfig: RuntimeConfig{ NVIDIAContainerRuntimeConfig: RuntimeConfig{
@ -293,6 +336,7 @@ func TestGetConfig(t *testing.T) {
for _, tc := range testCases { for _, tc := range testCases {
t.Run(tc.description, func(t *testing.T) { t.Run(tc.description, func(t *testing.T) {
defer setGetLdConfigPathForTest()()
defer setGetDistIDLikeForTest(tc.distIdsLike)() defer setGetDistIDLikeForTest(tc.distIdsLike)()
reader := strings.NewReader(strings.Join(tc.contents, "\n")) reader := strings.NewReader(strings.Join(tc.contents, "\n"))
@ -305,21 +349,63 @@ func TestGetConfig(t *testing.T) {
cfg, err := tomlCfg.Config() cfg, err := tomlCfg.Config()
require.NoError(t, err) require.NoError(t, err)
// We first handle the ldconfig path since this is currently system-dependent.
if tc.inspectLdconfig {
ldconfig := cfg.NVIDIAContainerCLIConfig.Ldconfig
require.True(t, strings.HasPrefix(ldconfig, "@/sbin/ldconfig"))
remaining := strings.TrimPrefix(ldconfig, "@/sbin/ldconfig")
require.True(t, remaining == ".real" || remaining == "")
cfg.NVIDIAContainerCLIConfig.Ldconfig = "WAS_CHECKED"
}
require.EqualValues(t, tc.expectedConfig, cfg) require.EqualValues(t, tc.expectedConfig, cfg)
}) })
} }
} }
func TestAssertValid(t *testing.T) {
defer setGetLdConfigPathForTest()()
testCases := []struct {
description string
config *Config
expectedError error
}{
{
description: "default is valid",
config: func() *Config {
config, _ := GetDefault()
return config
}(),
},
{
description: "alternative host ldconfig path is valid",
config: &Config{
NVIDIAContainerCLIConfig: ContainerCLIConfig{
Ldconfig: "@/some/host/path",
},
},
},
{
description: "non-host path is invalid",
config: &Config{
NVIDIAContainerCLIConfig: ContainerCLIConfig{
Ldconfig: "/non/host/path",
},
},
expectedError: errInvalidConfig,
},
{
description: "feature flag allows non-host path",
config: &Config{
NVIDIAContainerCLIConfig: ContainerCLIConfig{
Ldconfig: "/non/host/path",
},
Features: features{
AllowLDConfigFromContainer: ptr(feature(true)),
},
},
},
}
for _, tc := range testCases {
t.Run(tc.description, func(t *testing.T) {
require.ErrorIs(t, tc.config.assertValid(), tc.expectedError)
})
}
}
// setGetDistIDsLikeForTest overrides the distribution IDs that would normally be read from the /etc/os-release file. // setGetDistIDsLikeForTest overrides the distribution IDs that would normally be read from the /etc/os-release file.
func setGetDistIDLikeForTest(ids []string) func() { func setGetDistIDLikeForTest(ids []string) func() {
if ids == nil { if ids == nil {
@ -335,3 +421,18 @@ func setGetDistIDLikeForTest(ids []string) func() {
getDistIDLike = original getDistIDLike = original
} }
} }
// prt returns a reference to whatever type is passed into it
func ptr[T any](x T) *T {
return &x
}
func setGetLdConfigPathForTest() func() {
previous := getLdConfigPath
getLdConfigPath = func() ldconfigPath {
return "@/test/ld/config/path"
}
return func() {
getLdConfigPath = previous
}
}

View File

@ -21,14 +21,15 @@ type features struct {
// DisableImexChannelCreation ensures that the implicit creation of // DisableImexChannelCreation ensures that the implicit creation of
// requested IMEX channels is skipped when invoking the nvidia-container-cli. // requested IMEX channels is skipped when invoking the nvidia-container-cli.
DisableImexChannelCreation *feature `toml:"disable-imex-channel-creation,omitempty"` DisableImexChannelCreation *feature `toml:"disable-imex-channel-creation,omitempty"`
// AllowLDConfigFromContainer allows non-host ldconfig paths to be used.
// If this feature flag is not set to 'true' only host-rooted config paths
// (i.e. paths starting with an '@' are considered valid)
AllowLDConfigFromContainer *feature `toml:"allow-ldconfig-from-container,omitempty"`
} }
//nolint:unused
type feature bool type feature bool
// IsEnabled checks whether a feature is explicitly enabled. // IsEnabled checks whether a feature is explicitly enabled.
//
//nolint:unused
func (f *feature) IsEnabled() bool { func (f *feature) IsEnabled() bool {
if f != nil { if f != nil {
return bool(*f) return bool(*f)

View File

@ -24,13 +24,3 @@ type RuntimeHookConfig struct {
// SkipModeDetection disables the mode check for the runtime hook. // SkipModeDetection disables the mode check for the runtime hook.
SkipModeDetection bool `toml:"skip-mode-detection"` SkipModeDetection bool `toml:"skip-mode-detection"`
} }
// GetDefaultRuntimeHookConfig defines the default values for the config
func GetDefaultRuntimeHookConfig() (*RuntimeHookConfig, error) {
cfg, err := GetDefault()
if err != nil {
return nil, err
}
return &cfg.NVIDIAContainerRuntimeHookConfig, nil
}

View File

@ -45,13 +45,3 @@ type cdiModeConfig struct {
type csvModeConfig struct { type csvModeConfig struct {
MountSpecPath string `toml:"mount-spec-path"` MountSpecPath string `toml:"mount-spec-path"`
} }
// GetDefaultRuntimeConfig defines the default values for the config
func GetDefaultRuntimeConfig() (*RuntimeConfig, error) {
cfg, err := GetDefault()
if err != nil {
return nil, err
}
return &cfg.NVIDIAContainerRuntimeConfig, nil
}

View File

@ -108,6 +108,19 @@ func loadConfigTomlFrom(reader io.Reader) (*Toml, error) {
// Config returns the typed config associated with the toml tree. // Config returns the typed config associated with the toml tree.
func (t *Toml) Config() (*Config, error) { func (t *Toml) Config() (*Config, error) {
cfg, err := t.configNoOverrides()
if err != nil {
return nil, err
}
if err := cfg.assertValid(); err != nil {
return nil, err
}
return cfg, nil
}
// configNoOverrides returns the typed config associated with the toml tree.
// This config does not include feature-specific overrides.
func (t *Toml) configNoOverrides() (*Config, error) {
cfg, err := GetDefault() cfg, err := GetDefault()
if err != nil { if err != nil {
return nil, err return nil, err

View File

@ -198,9 +198,12 @@ func TestTomlContents(t *testing.T) {
} }
func TestConfigFromToml(t *testing.T) { func TestConfigFromToml(t *testing.T) {
defer setGetLdConfigPathForTest()()
testCases := []struct { testCases := []struct {
description string description string
contents map[string]interface{} contents map[string]interface{}
expectedError error
expectedConfig *Config expectedConfig *Config
}{ }{
{ {
@ -226,13 +229,39 @@ func TestConfigFromToml(t *testing.T) {
return c return c
}(), }(),
}, },
{
description: "invalid ldconfig value raises error",
contents: map[string]interface{}{
"nvidia-container-cli": map[string]interface{}{
"ldconfig": "/some/ldconfig/path",
},
},
expectedError: errInvalidConfig,
},
{
description: "feature allows ldconfig override",
contents: map[string]interface{}{
"nvidia-container-cli": map[string]interface{}{
"ldconfig": "/some/ldconfig/path",
},
"features": map[string]interface{}{
"allow-ldconfig-from-container": true,
},
},
expectedConfig: func() *Config {
c, _ := GetDefault()
c.NVIDIAContainerCLIConfig.Ldconfig = "/some/ldconfig/path"
c.Features.AllowLDConfigFromContainer = ptr(feature(true))
return c
}(),
},
} }
for _, tc := range testCases { for _, tc := range testCases {
t.Run(tc.description, func(t *testing.T) { t.Run(tc.description, func(t *testing.T) {
tomlCfg := fromMap(tc.contents) tomlCfg := fromMap(tc.contents)
config, err := tomlCfg.Config() config, err := tomlCfg.Config()
require.NoError(t, err) require.ErrorIs(t, err, tc.expectedError)
require.EqualValues(t, tc.expectedConfig, config) require.EqualValues(t, tc.expectedConfig, config)
}) })
} }

View File

@ -161,7 +161,7 @@ kind: example.com/class
// Ensure that the config file has the required contents. // Ensure that the config file has the required contents.
// TODO: Add checks for additional config options. // TODO: Add checks for additional config options.
require.Equal(t, "/host/driver/root", cfg.NVIDIAContainerCLIConfig.Root) require.Equal(t, "/host/driver/root", cfg.NVIDIAContainerCLIConfig.Root)
require.Equal(t, "@/host/driver/root/sbin/ldconfig", cfg.NVIDIAContainerCLIConfig.Ldconfig) require.Equal(t, "@/host/driver/root/sbin/ldconfig", string(cfg.NVIDIAContainerCLIConfig.Ldconfig))
require.EqualValues(t, filepath.Join(toolkitRoot, "nvidia-container-cli"), cfg.NVIDIAContainerCLIConfig.Path) require.EqualValues(t, filepath.Join(toolkitRoot, "nvidia-container-cli"), cfg.NVIDIAContainerCLIConfig.Path)
require.EqualValues(t, filepath.Join(toolkitRoot, "nvidia-ctk"), cfg.NVIDIACTKConfig.Path) require.EqualValues(t, filepath.Join(toolkitRoot, "nvidia-ctk"), cfg.NVIDIACTKConfig.Path)