From d60cdf61312d2d9a7ceed6e2b7eb2f0da8f449ea Mon Sep 17 00:00:00 2001 From: Rajat Chopra Date: Tue, 14 Jul 2026 08:49:18 -0700 Subject: [PATCH] fix: Disable chmod hook. Existing CDI specs invoking chmod now use the unsupported-hook handler: they emit a warning, perform no chmod, and do not prevent container startup. Signed-off-by: Rajat Chopra --- cmd/nvidia-cdi-hook/README.md | 1 - cmd/nvidia-cdi-hook/commands/commands.go | 2 - cmd/nvidia-cdi-hook/commands/commands_test.go | 33 +++++++++++++++++ cmd/nvidia-ctk/cdi/generate/generate_test.go | 14 +------ internal/discover/hooks.go | 13 +++++-- internal/discover/hooks_test.go | 37 ++++++++----------- pkg/nvcdi/options.go | 1 + 7 files changed, 61 insertions(+), 40 deletions(-) create mode 100644 cmd/nvidia-cdi-hook/commands/commands_test.go diff --git a/cmd/nvidia-cdi-hook/README.md b/cmd/nvidia-cdi-hook/README.md index 807cf6c75..8ea22a326 100644 --- a/cmd/nvidia-cdi-hook/README.md +++ b/cmd/nvidia-cdi-hook/README.md @@ -26,6 +26,5 @@ on generating a CDI file. The `nvidia-cdi-hook` CLI provides the following functionality: -* `chmod` - Change the permissions of a file or directory inside the directory path to be mounted into a container. * `create-symlinks` - Create symlinks inside the directory path to be mounted into a container. * `update-ldcache` - Update the dynamic linker cache inside the directory path to be mounted into a container. diff --git a/cmd/nvidia-cdi-hook/commands/commands.go b/cmd/nvidia-cdi-hook/commands/commands.go index a9f6c05ce..13ca4edfa 100644 --- a/cmd/nvidia-cdi-hook/commands/commands.go +++ b/cmd/nvidia-cdi-hook/commands/commands.go @@ -22,7 +22,6 @@ import ( "github.com/urfave/cli/v3" - "github.com/NVIDIA/nvidia-container-toolkit/cmd/nvidia-cdi-hook/chmod" symlinks "github.com/NVIDIA/nvidia-container-toolkit/cmd/nvidia-cdi-hook/create-symlinks" "github.com/NVIDIA/nvidia-container-toolkit/cmd/nvidia-cdi-hook/cudacompat" disabledevicenodemodification "github.com/NVIDIA/nvidia-container-toolkit/cmd/nvidia-cdi-hook/disable-device-node-modification" @@ -87,7 +86,6 @@ func ConfigureCDIHookCommand(logger logger.Interface, base *cli.Command) *cli.Co base.Commands = []*cli.Command{ ldcache.NewCommand(logger), symlinks.NewCommand(logger), - chmod.NewCommand(logger), cudacompat.NewCommand(logger), disabledevicenodemodification.NewCommand(logger), { diff --git a/cmd/nvidia-cdi-hook/commands/commands_test.go b/cmd/nvidia-cdi-hook/commands/commands_test.go new file mode 100644 index 000000000..ec71fe9ec --- /dev/null +++ b/cmd/nvidia-cdi-hook/commands/commands_test.go @@ -0,0 +1,33 @@ +/** +# Copyright (c) NVIDIA CORPORATION. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +**/ + +package commands + +import ( + "testing" + + "github.com/stretchr/testify/require" + "github.com/urfave/cli/v3" + + "github.com/NVIDIA/nvidia-container-toolkit/internal/logger" +) + +func TestConfigureCDIHookCommandDisablesChmod(t *testing.T) { + command := ConfigureCDIHookCommand(&logger.NullLogger{}, &cli.Command{}) + + require.Nil(t, command.Command("chmod")) + require.NotNil(t, command.Command("create-symlinks")) +} diff --git a/cmd/nvidia-ctk/cdi/generate/generate_test.go b/cmd/nvidia-ctk/cdi/generate/generate_test.go index 998d8bd0e..05bb84ef5 100644 --- a/cmd/nvidia-ctk/cdi/generate/generate_test.go +++ b/cmd/nvidia-ctk/cdi/generate/generate_test.go @@ -388,7 +388,7 @@ containerEdits: `, }, { - description: "enableChmodHook", + description: "cannotEnableChmodHook", options: options{ format: "yaml", mode: "management", @@ -427,18 +427,6 @@ devices: hostPath: {{ .driverRoot }}/dev/nvidia-caps-imex-channels/channel2047 - path: /dev/nvidia-caps/nvidia-cap1 hostPath: {{ .driverRoot }}/dev/nvidia-caps/nvidia-cap1 - hooks: - - hookName: createContainer - path: /usr/bin/nvidia-cdi-hook - args: - - nvidia-cdi-hook - - chmod - - --mode - - "755" - - --path - - /dev/nvidia-caps - env: - - NVIDIA_CTK_DEBUG=false containerEdits: env: - NVIDIA_CTK_LIBCUDA_DIR=/lib/x86_64-linux-gnu diff --git a/internal/discover/hooks.go b/internal/discover/hooks.go index a31e097fc..8f66b5d5c 100644 --- a/internal/discover/hooks.go +++ b/internal/discover/hooks.go @@ -63,10 +63,9 @@ const ( defaultNvidiaCDIHookPath = "/usr/bin/nvidia-cdi-hook" ) -// defaultDisabledHooks defines hooks that are disabled by default. -// These hooks can be explicitly enabled using the WithEnabledHooks option. +// defaultDisabledHooks defines hooks that cannot be enabled. var defaultDisabledHooks = []HookName{ - // ChmodHook is disabled by default as it was a workaround for older + // ChmodHook is disabled as it was a workaround for older // versions of crun that has since been fixed. ChmodHook, } @@ -146,6 +145,7 @@ func WithDisabledHooks(hooks ...HookName) Option { // WithEnabledHooks explicitly enables the specified hooks. // This is useful for enabling hooks that are disabled by default. +// Permanently disabled hooks cannot be enabled with this option. func WithEnabledHooks(hooks ...HookName) Option { return func(c *hookCreatorOptions) { c.enabledHooks = append(c.enabledHooks, hooks...) @@ -185,6 +185,9 @@ func NewHookCreator(opts ...Option) HookCreator { } for _, h := range o.enabledHooks { + if h == ChmodHook { + continue + } disabledHooks[h] = false } @@ -224,6 +227,10 @@ func (c cdiHookCreator) getOCIHookType(name HookName) OCIHookType { } func (c cdiHookCreator) isDisabled(name HookName, args ...string) bool { + if name == ChmodHook { + return true + } + disabled, ok := c.disabledHooks[name] if ok { return disabled diff --git a/internal/discover/hooks_test.go b/internal/discover/hooks_test.go index 666a3f933..03991ca97 100644 --- a/internal/discover/hooks_test.go +++ b/internal/discover/hooks_test.go @@ -76,7 +76,7 @@ func TestNewHookCreator(t *testing.T) { }, }, { - name: "multiple hooks disabled and enabled", + name: "permanently disabled hooks cannot be enabled", opts: []Option{ WithDisabledHooks(UpdateLDCacheHook, CreateSymlinksHook, EnableCudaCompatHook, DisableDeviceNodeModificationHook), WithEnabledHooks(ChmodHook, UpdateLDCacheHook), @@ -88,7 +88,7 @@ func TestNewHookCreator(t *testing.T) { UpdateLDCacheHook: false, CreateSymlinksHook: true, EnableCudaCompatHook: true, - ChmodHook: false, + ChmodHook: true, DisableDeviceNodeModificationHook: true, }, }, @@ -112,7 +112,7 @@ func TestNewHookCreator(t *testing.T) { }, }, { - name: "WithEnabledHooks overrides defaultDisabledHooks", + name: "WithEnabledHooks does not override permanently disabled hooks", opts: []Option{ WithEnabledHooks(ChmodHook), }, @@ -120,7 +120,7 @@ func TestNewHookCreator(t *testing.T) { nvidiaCDIHookPath: defaultNvidiaCDIHookPath, fixedArgs: []string{"nvidia-cdi-hook"}, disabledHooks: map[HookName]bool{ - ChmodHook: false, // ChmodHook is enabled + ChmodHook: true, }, }, }, @@ -188,19 +188,14 @@ func TestCDIHookCreator_Create(t *testing.T) { expectedHook: nil, }, { - name: "ChmodHook with args (when enabled)", + name: "ChmodHook with args cannot be enabled", hookCreator: NewHookCreator( WithNVIDIACDIHookPath(defaultNvidiaCDIHookPath), WithEnabledHooks(ChmodHook), ), - hookName: ChmodHook, - args: []string{"/path/to/file1", "/path/to/file2"}, - expectedHook: &Hook{ - Lifecycle: "createContainer", - Path: defaultNvidiaCDIHookPath, - Args: []string{"nvidia-cdi-hook", "chmod", "--mode", "755", "--path", "/path/to/file1", "--path", "/path/to/file2"}, - Env: []string{"NVIDIA_CTK_DEBUG=false"}, - }, + hookName: ChmodHook, + args: []string{"/path/to/file1", "/path/to/file2"}, + expectedHook: nil, }, { name: "ChmodHook disabled by default returns nil", @@ -345,12 +340,12 @@ func TestCDIHookCreator_isDisabled(t *testing.T) { expectedResult: true, // ChmodHook is disabled by default and explicitly disabled }, { - name: "hook explicitly enabled overrides disabled", + name: "permanently disabled hook cannot be explicitly enabled", disabledHooks: []HookName{ChmodHook}, enabledHooks: []HookName{ChmodHook}, hookName: ChmodHook, args: []string{"/path/to/file"}, - expectedResult: false, + expectedResult: true, }, { name: "hook not in disabled map and not AllHooks disabled", @@ -381,12 +376,12 @@ func TestCDIHookCreator_isDisabled(t *testing.T) { expectedResult: true, }, { - name: "ChmodHook requires args - args provided", + name: "ChmodHook remains disabled with args", disabledHooks: []HookName{}, - enabledHooks: []HookName{ChmodHook}, // Enable ChmodHook since it's disabled by default + enabledHooks: []HookName{ChmodHook}, hookName: ChmodHook, args: []string{"/path/to/file"}, - expectedResult: false, + expectedResult: true, }, { name: "UpdateLDCacheHook doesn't require args - no args provided", @@ -440,12 +435,12 @@ func TestCDIHookCreator_isDisabled(t *testing.T) { expectedResult: false, }, { - name: "ChmodHook with multiple args", + name: "ChmodHook with multiple args remains disabled", disabledHooks: []HookName{}, - enabledHooks: []HookName{ChmodHook}, // Enable ChmodHook since it's disabled by default + enabledHooks: []HookName{ChmodHook}, hookName: ChmodHook, args: []string{"/path1", "/path2"}, - expectedResult: false, + expectedResult: true, }, { name: "UpdateLDCacheHook with multiple args", diff --git a/pkg/nvcdi/options.go b/pkg/nvcdi/options.go index 111ae6981..575451903 100644 --- a/pkg/nvcdi/options.go +++ b/pkg/nvcdi/options.go @@ -317,6 +317,7 @@ func WithDisabledHooks[T string | HookName](hooks ...T) Option { } // WithEnabledHooks explicitly enables a specific set of hooks. +// Permanently disabled hooks cannot be enabled with this option. // If a hook is explicitly enabled, this takes precedence over it being disabled. func WithEnabledHooks[T string | HookName](hooks ...T) Option { return func(o *options) {