From 2efee0f2ad61505add8a2ec37776fc0738a5bec2 Mon Sep 17 00:00:00 2001 From: Nikhil R Deshpande Date: Thu, 10 Sep 2026 17:48:40 -0700 Subject: [PATCH] Add repoConfig.nodeLocalPaths for node-local package repositories Resolves #2884 A package repository on the node's own disk cannot be used today: a "file:///opt/local-packages" URI supplied via driver.repoConfig does not resolve inside the driver container, and there is no supported way to expose the host directory to it. Add an optional nodeLocalPaths list to the existing repoConfig API on both ClusterPolicy and NVIDIADriver. Each entry is bind-mounted read-only into the driver container at the same path, so file:// URIs resolve identically on the host and in the container. Paths are validated as clean, absolute and non-root, then deduplicated and sorted so volume naming is deterministic. Defaults to empty, is ignored for precompiled drivers to match configMapName, and is rejected when set without a repo ConfigMap. Signed-off-by: Nikhil R Deshpande --- api/nvidia/v1/clusterpolicy_types.go | 24 ++ api/nvidia/v1/zz_generated.deepcopy.go | 7 +- api/nvidia/v1alpha1/nvidiadriver_types.go | 33 +++ api/nvidia/v1alpha1/zz_generated.deepcopy.go | 7 +- .../manifests/nvidia.com_clusterpolicies.yaml | 24 ++ .../manifests/nvidia.com_nvidiadrivers.yaml | 24 ++ .../crd/bases/nvidia.com_clusterpolicies.yaml | 24 ++ .../crd/bases/nvidia.com_nvidiadrivers.yaml | 24 ++ .../samples/nvidia_v1alpha1_nvidiadriver.yaml | 4 + config/samples/v1_clusterpolicy.yaml | 4 + controllers/object_controls.go | 16 ++ controllers/transforms_test.go | 216 +++++++++++++++ .../crds/nvidia.com_clusterpolicies.yaml | 24 ++ .../crds/nvidia.com_nvidiadrivers.yaml | 24 ++ .../gpu-operator/templates/nvidiadriver.yaml | 3 + deployments/gpu-operator/values.yaml | 11 + internal/config/driver_config_digest_test.go | 38 +++ internal/state/driver_nodelocalrepo_test.go | 258 ++++++++++++++++++ internal/state/driver_volumes.go | 16 ++ internal/utils/nodelocalrepo.go | 97 +++++++ internal/utils/nodelocalrepo_test.go | 149 ++++++++++ tests/e2e/helpers/clusterpolicy.go | 18 ++ tests/e2e/suites/driver_repoconfig_test.go | 235 ++++++++++++++++ 23 files changed, 1278 insertions(+), 2 deletions(-) create mode 100644 internal/state/driver_nodelocalrepo_test.go create mode 100644 internal/utils/nodelocalrepo.go create mode 100644 internal/utils/nodelocalrepo_test.go create mode 100644 tests/e2e/suites/driver_repoconfig_test.go diff --git a/api/nvidia/v1/clusterpolicy_types.go b/api/nvidia/v1/clusterpolicy_types.go index 16d95220c5..a77996b170 100644 --- a/api/nvidia/v1/clusterpolicy_types.go +++ b/api/nvidia/v1/clusterpolicy_types.go @@ -1280,6 +1280,30 @@ type DriverRepoConfigSpec struct { // +operator-sdk:gen-csv:customresourcedefinitions.specDescriptors.displayName="ConfigMap Name" // +operator-sdk:gen-csv:customresourcedefinitions.specDescriptors.x-descriptors="urn:alm:descriptor:com.tectonic.ui:text" ConfigMapName string `json:"configMapName,omitempty"` + + // NodeLocalPaths is an optional list of absolute directory paths on the host which hold a + // node-local package repository referenced by the repository configuration in + // ConfigMapName (for example "URIs: file:///opt/local-packages"). Each path is + // bind-mounted read-only into the NVIDIA driver container at the same path, so that the + // package manager running inside the container can resolve file:// repository URIs. + // + // Each directory must exist on every node targeted by the driver DaemonSet; the driver + // pod will not start on a node where it is missing. Ignored when precompiled drivers are + // used, and rejected when ConfigMapName is empty. + // + // Packages installed from a node-local repository are compiled into a kernel module which + // is loaded into the host kernel, so the repository is part of the node's trusted + // computing base. The directory and all of its parents should be owned by root and not be + // group or world writable, and the repository should be GPG signed where possible. + // +kubebuilder:validation:Optional + // +kubebuilder:validation:MaxItems=8 + // +kubebuilder:validation:items:MinLength=2 + // +kubebuilder:validation:items:MaxLength=4096 + // +kubebuilder:validation:items:Pattern="^/" + // +listType=set + // +operator-sdk:gen-csv:customresourcedefinitions.specDescriptors=true + // +operator-sdk:gen-csv:customresourcedefinitions.specDescriptors.displayName="Node-local package repository paths" + NodeLocalPaths []string `json:"nodeLocalPaths,omitempty"` } // DriverCertConfigSpec defines custom certificates configuration for NVIDIA Driver container diff --git a/api/nvidia/v1/zz_generated.deepcopy.go b/api/nvidia/v1/zz_generated.deepcopy.go index 9e936de60d..69ee87bad8 100644 --- a/api/nvidia/v1/zz_generated.deepcopy.go +++ b/api/nvidia/v1/zz_generated.deepcopy.go @@ -680,6 +680,11 @@ func (in *DriverManagerSpec) DeepCopy() *DriverManagerSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *DriverRepoConfigSpec) DeepCopyInto(out *DriverRepoConfigSpec) { *out = *in + if in.NodeLocalPaths != nil { + in, out := &in.NodeLocalPaths, &out.NodeLocalPaths + *out = make([]string, len(*in)) + copy(*out, *in) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new DriverRepoConfigSpec. @@ -764,7 +769,7 @@ func (in *DriverSpec) DeepCopyInto(out *DriverSpec) { if in.RepoConfig != nil { in, out := &in.RepoConfig, &out.RepoConfig *out = new(DriverRepoConfigSpec) - **out = **in + (*in).DeepCopyInto(*out) } if in.CertConfig != nil { in, out := &in.CertConfig, &out.CertConfig diff --git a/api/nvidia/v1alpha1/nvidiadriver_types.go b/api/nvidia/v1alpha1/nvidiadriver_types.go index 70aa59b0ff..a706bde06c 100644 --- a/api/nvidia/v1alpha1/nvidiadriver_types.go +++ b/api/nvidia/v1alpha1/nvidiadriver_types.go @@ -453,6 +453,30 @@ type DriverRepoConfigSpec struct { // +operator-sdk:gen-csv:customresourcedefinitions.specDescriptors.displayName="ConfigMap Name" // +operator-sdk:gen-csv:customresourcedefinitions.specDescriptors.x-descriptors="urn:alm:descriptor:com.tectonic.ui:text" Name string `json:"name,omitempty"` + + // NodeLocalPaths is an optional list of absolute directory paths on the host which hold a + // node-local package repository referenced by the repository configuration in Name (for + // example "URIs: file:///opt/local-packages"). Each path is bind-mounted read-only into + // the NVIDIA driver container at the same path, so that the package manager running + // inside the container can resolve file:// repository URIs. + // + // Each directory must exist on every node targeted by the driver DaemonSet; the driver + // pod will not start on a node where it is missing. Ignored when precompiled drivers are + // used, and rejected when Name is empty. + // + // Packages installed from a node-local repository are compiled into a kernel module which + // is loaded into the host kernel, so the repository is part of the node's trusted + // computing base. The directory and all of its parents should be owned by root and not be + // group or world writable, and the repository should be GPG signed where possible. + // +kubebuilder:validation:Optional + // +kubebuilder:validation:MaxItems=8 + // +kubebuilder:validation:items:MinLength=2 + // +kubebuilder:validation:items:MaxLength=4096 + // +kubebuilder:validation:items:Pattern="^/" + // +listType=set + // +operator-sdk:gen-csv:customresourcedefinitions.specDescriptors=true + // +operator-sdk:gen-csv:customresourcedefinitions.specDescriptors.displayName="Node-local package repository paths" + NodeLocalPaths []string `json:"nodeLocalPaths,omitempty"` } // DriverLicensingConfigSpec defines licensing server configuration for NVIDIA Driver container @@ -772,6 +796,15 @@ func (d *NVIDIADriverSpec) IsRepoConfigEnabled() bool { return d.RepoConfig.Name != "" } +// RepoConfigNodeLocalPaths returns the node-local package repository host paths configured +// for the driver, or nil if none are set. +func (d *NVIDIADriverSpec) RepoConfigNodeLocalPaths() []string { + if d.RepoConfig == nil { + return nil + } + return d.RepoConfig.NodeLocalPaths +} + // IsCertConfigEnabled returns true if additional certificate config is provided func (d *NVIDIADriverSpec) IsCertConfigEnabled() bool { if d.CertConfig == nil { diff --git a/api/nvidia/v1alpha1/zz_generated.deepcopy.go b/api/nvidia/v1alpha1/zz_generated.deepcopy.go index c75265398d..ed4c05b266 100644 --- a/api/nvidia/v1alpha1/zz_generated.deepcopy.go +++ b/api/nvidia/v1alpha1/zz_generated.deepcopy.go @@ -253,6 +253,11 @@ func (in *DriverManagerSpec) DeepCopy() *DriverManagerSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *DriverRepoConfigSpec) DeepCopyInto(out *DriverRepoConfigSpec) { *out = *in + if in.NodeLocalPaths != nil { + in, out := &in.NodeLocalPaths, &out.NodeLocalPaths + *out = make([]string, len(*in)) + copy(*out, *in) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new DriverRepoConfigSpec. @@ -675,7 +680,7 @@ func (in *NVIDIADriverSpec) DeepCopyInto(out *NVIDIADriverSpec) { if in.RepoConfig != nil { in, out := &in.RepoConfig, &out.RepoConfig *out = new(DriverRepoConfigSpec) - **out = **in + (*in).DeepCopyInto(*out) } if in.CertConfig != nil { in, out := &in.CertConfig, &out.CertConfig diff --git a/bundle/manifests/nvidia.com_clusterpolicies.yaml b/bundle/manifests/nvidia.com_clusterpolicies.yaml index e8d0be746c..908f6ba41c 100644 --- a/bundle/manifests/nvidia.com_clusterpolicies.yaml +++ b/bundle/manifests/nvidia.com_clusterpolicies.yaml @@ -1137,6 +1137,30 @@ spec: properties: configMapName: type: string + nodeLocalPaths: + description: |- + NodeLocalPaths is an optional list of absolute directory paths on the host which hold a + node-local package repository referenced by the repository configuration in + ConfigMapName (for example "URIs: file:///opt/local-packages"). Each path is + bind-mounted read-only into the NVIDIA driver container at the same path, so that the + package manager running inside the container can resolve file:// repository URIs. + + Each directory must exist on every node targeted by the driver DaemonSet; the driver + pod will not start on a node where it is missing. Ignored when precompiled drivers are + used, and rejected when ConfigMapName is empty. + + Packages installed from a node-local repository are compiled into a kernel module which + is loaded into the host kernel, so the repository is part of the node's trusted + computing base. The directory and all of its parents should be owned by root and not be + group or world writable, and the repository should be GPG signed where possible. + items: + maxLength: 4096 + minLength: 2 + pattern: ^/ + type: string + maxItems: 8 + type: array + x-kubernetes-list-type: set type: object repository: description: NVIDIA Driver image repository diff --git a/bundle/manifests/nvidia.com_nvidiadrivers.yaml b/bundle/manifests/nvidia.com_nvidiadrivers.yaml index 7ad08f203c..3f3fa77f1c 100644 --- a/bundle/manifests/nvidia.com_nvidiadrivers.yaml +++ b/bundle/manifests/nvidia.com_nvidiadrivers.yaml @@ -834,6 +834,30 @@ spec: properties: name: type: string + nodeLocalPaths: + description: |- + NodeLocalPaths is an optional list of absolute directory paths on the host which hold a + node-local package repository referenced by the repository configuration in Name (for + example "URIs: file:///opt/local-packages"). Each path is bind-mounted read-only into + the NVIDIA driver container at the same path, so that the package manager running + inside the container can resolve file:// repository URIs. + + Each directory must exist on every node targeted by the driver DaemonSet; the driver + pod will not start on a node where it is missing. Ignored when precompiled drivers are + used, and rejected when Name is empty. + + Packages installed from a node-local repository are compiled into a kernel module which + is loaded into the host kernel, so the repository is part of the node's trusted + computing base. The directory and all of its parents should be owned by root and not be + group or world writable, and the repository should be GPG signed where possible. + items: + maxLength: 4096 + minLength: 2 + pattern: ^/ + type: string + maxItems: 8 + type: array + x-kubernetes-list-type: set type: object repository: description: NVIDIA Driver repository diff --git a/config/crd/bases/nvidia.com_clusterpolicies.yaml b/config/crd/bases/nvidia.com_clusterpolicies.yaml index e8d0be746c..908f6ba41c 100644 --- a/config/crd/bases/nvidia.com_clusterpolicies.yaml +++ b/config/crd/bases/nvidia.com_clusterpolicies.yaml @@ -1137,6 +1137,30 @@ spec: properties: configMapName: type: string + nodeLocalPaths: + description: |- + NodeLocalPaths is an optional list of absolute directory paths on the host which hold a + node-local package repository referenced by the repository configuration in + ConfigMapName (for example "URIs: file:///opt/local-packages"). Each path is + bind-mounted read-only into the NVIDIA driver container at the same path, so that the + package manager running inside the container can resolve file:// repository URIs. + + Each directory must exist on every node targeted by the driver DaemonSet; the driver + pod will not start on a node where it is missing. Ignored when precompiled drivers are + used, and rejected when ConfigMapName is empty. + + Packages installed from a node-local repository are compiled into a kernel module which + is loaded into the host kernel, so the repository is part of the node's trusted + computing base. The directory and all of its parents should be owned by root and not be + group or world writable, and the repository should be GPG signed where possible. + items: + maxLength: 4096 + minLength: 2 + pattern: ^/ + type: string + maxItems: 8 + type: array + x-kubernetes-list-type: set type: object repository: description: NVIDIA Driver image repository diff --git a/config/crd/bases/nvidia.com_nvidiadrivers.yaml b/config/crd/bases/nvidia.com_nvidiadrivers.yaml index 7ad08f203c..3f3fa77f1c 100644 --- a/config/crd/bases/nvidia.com_nvidiadrivers.yaml +++ b/config/crd/bases/nvidia.com_nvidiadrivers.yaml @@ -834,6 +834,30 @@ spec: properties: name: type: string + nodeLocalPaths: + description: |- + NodeLocalPaths is an optional list of absolute directory paths on the host which hold a + node-local package repository referenced by the repository configuration in Name (for + example "URIs: file:///opt/local-packages"). Each path is bind-mounted read-only into + the NVIDIA driver container at the same path, so that the package manager running + inside the container can resolve file:// repository URIs. + + Each directory must exist on every node targeted by the driver DaemonSet; the driver + pod will not start on a node where it is missing. Ignored when precompiled drivers are + used, and rejected when Name is empty. + + Packages installed from a node-local repository are compiled into a kernel module which + is loaded into the host kernel, so the repository is part of the node's trusted + computing base. The directory and all of its parents should be owned by root and not be + group or world writable, and the repository should be GPG signed where possible. + items: + maxLength: 4096 + minLength: 2 + pattern: ^/ + type: string + maxItems: 8 + type: array + x-kubernetes-list-type: set type: object repository: description: NVIDIA Driver repository diff --git a/config/samples/nvidia_v1alpha1_nvidiadriver.yaml b/config/samples/nvidia_v1alpha1_nvidiadriver.yaml index c65243ca30..9c3167b489 100644 --- a/config/samples/nvidia_v1alpha1_nvidiadriver.yaml +++ b/config/samples/nvidia_v1alpha1_nvidiadriver.yaml @@ -43,6 +43,10 @@ spec: # Private mirror repository configuration repoConfig: name: "" + # absolute host directories holding a node-local package repository referenced + # by the repo config above, bind-mounted read-only at the same path + # nodeLocalPaths: + # - /opt/local-packages # custom ssl key/certificate configuration certConfig: name: "" diff --git a/config/samples/v1_clusterpolicy.yaml b/config/samples/v1_clusterpolicy.yaml index c7cd88567d..aafcbcc040 100644 --- a/config/samples/v1_clusterpolicy.yaml +++ b/config/samples/v1_clusterpolicy.yaml @@ -85,6 +85,10 @@ spec: # private mirror repository configuration repoConfig: configMapName: "" + # absolute host directories holding a node-local package repository referenced + # by the repo config above, bind-mounted read-only at the same path + # nodeLocalPaths: + # - /opt/local-packages # custom ssl key/certificate configuration certConfig: name: "" diff --git a/controllers/object_controls.go b/controllers/object_controls.go index c1bf59e4cf..e677beab79 100644 --- a/controllers/object_controls.go +++ b/controllers/object_controls.go @@ -3722,6 +3722,22 @@ func transformDriverContainer(obj *appsv1.DaemonSet, config *gpuv1.ClusterPolicy } driverContainer.VolumeMounts = append(driverContainer.VolumeMounts, volumeMounts...) podSpec.Volumes = append(podSpec.Volumes, createConfigMapVolume(config.Driver.RepoConfig.ConfigMapName, itemsToInclude)) + + // expose any node-local package repositories referenced by the repo configuration + // (e.g. "URIs: file:///opt/local-packages") to the driver container + if len(config.Driver.RepoConfig.NodeLocalPaths) > 0 { + repoVolumes, repoVolumeMounts, err := utils.NodeLocalRepoVolumes(config.Driver.RepoConfig.NodeLocalPaths) + if err != nil { + return fmt.Errorf("ERROR: invalid repoConfig.nodeLocalPaths: %w", err) + } + n.logger.Info("Mounting node-local package repositories into the driver container", + "nodeLocalPaths", config.Driver.RepoConfig.NodeLocalPaths) + driverContainer.VolumeMounts = append(driverContainer.VolumeMounts, repoVolumeMounts...) + podSpec.Volumes = append(podSpec.Volumes, repoVolumes...) + } + } else if config.Driver.RepoConfig != nil && len(config.Driver.RepoConfig.NodeLocalPaths) > 0 { + return fmt.Errorf("ERROR: driver.repoConfig.nodeLocalPaths is set but driver.repoConfig.configMapName is empty; " + + "nodeLocalPaths only applies when a custom repository configuration is provided") } // set any custom ssl key/certificate configuration provided diff --git a/controllers/transforms_test.go b/controllers/transforms_test.go index 8931298a48..f4fd705753 100644 --- a/controllers/transforms_test.go +++ b/controllers/transforms_test.go @@ -5078,3 +5078,219 @@ func TestHashDriverInstallConfigZeroFieldInvariant(t *testing.T) { assert.NotEqual(t, originalDigest, changedDigest, "a non-zero new field should change the digest") } + +// nodeLocalRepoVolumes returns the node-local package repository hostPath volumes +// present in the given pod spec, keyed by host path. +func nodeLocalRepoVolumes(t *testing.T, volumes []corev1.Volume) map[string]corev1.Volume { + t.Helper() + + actual := map[string]corev1.Volume{} + for _, volume := range volumes { + if !strings.HasPrefix(volume.Name, "node-local-repo-") { + continue + } + require.NotNil(t, volume.HostPath, "volume %s must be a hostPath volume", volume.Name) + actual[volume.HostPath.Path] = volume + } + return actual +} + +// nodeLocalRepoVolumeMounts returns the node-local package repository volume mounts +// present on a container, keyed by mount path. +func nodeLocalRepoVolumeMounts(volumeMounts []corev1.VolumeMount) map[string]corev1.VolumeMount { + actual := map[string]corev1.VolumeMount{} + for _, volumeMount := range volumeMounts { + if !strings.HasPrefix(volumeMount.Name, "node-local-repo-") { + continue + } + actual[volumeMount.MountPath] = volumeMount + } + return actual +} + +// transformDriverForNodeLocalPaths runs TransformDriver against a minimal ClusterPolicy +// configured with the given repo config settings, and returns the resulting DaemonSet. +func transformDriverForNodeLocalPaths(t *testing.T, c client.Client, configMapName string, + nodeLocalPaths []string, usePrecompiled bool) (Daemonset, error) { + t.Helper() + + ds := NewDaemonset().WithContainer(corev1.Container{Name: "nvidia-driver-ctr"}). + WithInitContainer(corev1.Container{Name: "k8s-driver-manager"}) + cpSpec := &gpuv1.ClusterPolicySpec{ + Driver: gpuv1.DriverSpec{ + Repository: "nvcr.io/nvidia", + Image: "driver", + ImagePullPolicy: "IfNotPresent", + Version: "580.126.16", + Manager: gpuv1.DriverManagerSpec{ + Repository: "nvcr.io/nvidia/cloud-native", + Image: "k8s-driver-manager", + ImagePullPolicy: "IfNotPresent", + Version: "v0.8.0", + }, + }, + } + if usePrecompiled { + cpSpec.Driver.UsePrecompiled = new(true) + // the precompiled path makes the DaemonSet kernel-version specific by writing + // into these maps, which the minimal fixture above leaves nil + ds.Labels = map[string]string{} + ds.Spec.Template.Labels = map[string]string{} + ds.Spec.Template.Spec.NodeSelector = map[string]string{} + } + if configMapName != "" || len(nodeLocalPaths) > 0 { + cpSpec.Driver.RepoConfig = &gpuv1.DriverRepoConfigSpec{ + ConfigMapName: configMapName, + NodeLocalPaths: nodeLocalPaths, + } + } + + err := TransformDriver(ds.DaemonSet, cpSpec, ClusterPolicyController{ + client: c, + runtime: gpuv1.Containerd, + operatorNamespace: "test-ns", + logger: ctrl.Log.WithName("test"), + gpuNodeOSRelease: "ubuntu", + gpuNodeOSTag: "ubuntu24.04", + }) + return ds, err +} + +func newNodeLocalRepoTestClient() client.Client { + return fake.NewFakeClient(&corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-repo-config", + Namespace: "test-ns", + }, + Data: map[string]string{ + "local-packages.sources": "Types: deb", + }, + }) +} + +func TestTransformDriverRepoConfigNodeLocalPaths(t *testing.T) { + mockClient := newNodeLocalRepoTestClient() + + ds, err := transformDriverForNodeLocalPaths(t, mockClient, "test-repo-config", + []string{"/opt/local-packages"}, false) + require.NoError(t, err) + + volumes := nodeLocalRepoVolumes(t, ds.Spec.Template.Spec.Volumes) + require.Len(t, volumes, 1) + volume := volumes["/opt/local-packages"] + require.Equal(t, "node-local-repo-0", volume.Name) + require.NotNil(t, volume.HostPath.Type) + require.Equal(t, corev1.HostPathDirectory, *volume.HostPath.Type) + + driverContainer := findContainerByName(ds.Spec.Template.Spec.Containers, "nvidia-driver-ctr") + require.NotNil(t, driverContainer) + mounts := nodeLocalRepoVolumeMounts(driverContainer.VolumeMounts) + require.Len(t, mounts, 1) + mount := mounts["/opt/local-packages"] + require.Equal(t, "node-local-repo-0", mount.Name) + require.True(t, mount.ReadOnly) + + // the repo ConfigMap mount must still be applied alongside the host path + require.Contains(t, nodeLocalRepoTestConfigMapVolumeNames(ds.Spec.Template.Spec.Volumes), "test-repo-config") +} + +func nodeLocalRepoTestConfigMapVolumeNames(volumes []corev1.Volume) []string { + var names []string + for _, volume := range volumes { + if volume.ConfigMap != nil { + names = append(names, volume.Name) + } + } + return names +} + +func TestTransformDriverRepoConfigMultipleNodeLocalPathsSorted(t *testing.T) { + mockClient := newNodeLocalRepoTestClient() + + ds, err := transformDriverForNodeLocalPaths(t, mockClient, "test-repo-config", + []string{"/srv/pkgs", "/opt/local-packages"}, false) + require.NoError(t, err) + + volumes := nodeLocalRepoVolumes(t, ds.Spec.Template.Spec.Volumes) + require.Len(t, volumes, 2) + require.Equal(t, "node-local-repo-0", volumes["/opt/local-packages"].Name) + require.Equal(t, "node-local-repo-1", volumes["/srv/pkgs"].Name) +} + +func TestTransformDriverRepoConfigNodeLocalPathsWithoutConfigMapName(t *testing.T) { + mockClient := newNodeLocalRepoTestClient() + + _, err := transformDriverForNodeLocalPaths(t, mockClient, "", []string{"/opt/local-packages"}, false) + require.Error(t, err) + require.Contains(t, err.Error(), "configMapName is empty") +} + +func TestTransformDriverRepoConfigNodeLocalPathsInvalidPath(t *testing.T) { + mockClient := newNodeLocalRepoTestClient() + + _, err := transformDriverForNodeLocalPaths(t, mockClient, "test-repo-config", + []string{"relative/path"}, false) + require.Error(t, err) + require.Contains(t, err.Error(), "not an absolute path") +} + +func TestTransformDriverRepoConfigNodeLocalPathsIgnoredForPrecompiled(t *testing.T) { + mockClient := newNodeLocalRepoTestClient() + + ds, err := transformDriverForNodeLocalPaths(t, mockClient, "test-repo-config", + []string{"/opt/local-packages"}, true) + require.NoError(t, err) + require.Empty(t, nodeLocalRepoVolumes(t, ds.Spec.Template.Spec.Volumes)) +} + +// TestTransformDriverNoNodeLocalPaths is the regression guard for the feature being +// opt-in: with the field unset, nothing named node-local-repo-* may appear. +func TestTransformDriverNoNodeLocalPaths(t *testing.T) { + mockClient := newNodeLocalRepoTestClient() + + ds, err := transformDriverForNodeLocalPaths(t, mockClient, "test-repo-config", nil, false) + require.NoError(t, err) + require.Empty(t, nodeLocalRepoVolumes(t, ds.Spec.Template.Spec.Volumes)) + + driverContainer := findContainerByName(ds.Spec.Template.Spec.Containers, "nvidia-driver-ctr") + require.NotNil(t, driverContainer) + require.Empty(t, nodeLocalRepoVolumeMounts(driverContainer.VolumeMounts)) +} + +// TestTransformDriverNodeLocalPathsChangeDigest pins the behaviour that changing the +// node-local repository paths rolls the driver pods. +func TestTransformDriverNodeLocalPathsChangeDigest(t *testing.T) { + mockClient := newNodeLocalRepoTestClient() + + digestFor := func(nodeLocalPaths []string) string { + ds, err := transformDriverForNodeLocalPaths(t, mockClient, "test-repo-config", nodeLocalPaths, false) + require.NoError(t, err) + driverContainer := findContainerByName(ds.Spec.Template.Spec.Containers, "nvidia-driver-ctr") + require.NotNil(t, driverContainer) + for _, env := range driverContainer.Env { + if env.Name == driverconfig.DriverConfigDigestEnvName { + return env.Value + } + } + t.Fatalf("%s env var not set on the driver container", driverconfig.DriverConfigDigestEnvName) + return "" + } + + withoutPaths := digestFor(nil) + withPath := digestFor([]string{"/opt/local-packages"}) + withOtherPath := digestFor([]string{"/srv/pkgs"}) + + require.NotEqual(t, withoutPaths, withPath, "adding a node-local path must change the driver config digest") + require.NotEqual(t, withPath, withOtherPath, "changing a node-local path must change the driver config digest") +} + +// TestTransformDriverEmptyNodeLocalPathsWithEmptyConfigMapName covers what a default helm +// install now produces: repoConfig present with an empty configMapName and an empty (but +// non-nil) nodeLocalPaths list. This must be a no-op, not an error. +func TestTransformDriverEmptyNodeLocalPathsWithEmptyConfigMapName(t *testing.T) { + mockClient := newNodeLocalRepoTestClient() + + ds, err := transformDriverForNodeLocalPaths(t, mockClient, "", []string{}, false) + require.NoError(t, err) + require.Empty(t, nodeLocalRepoVolumes(t, ds.Spec.Template.Spec.Volumes)) +} diff --git a/deployments/gpu-operator/crds/nvidia.com_clusterpolicies.yaml b/deployments/gpu-operator/crds/nvidia.com_clusterpolicies.yaml index e8d0be746c..908f6ba41c 100644 --- a/deployments/gpu-operator/crds/nvidia.com_clusterpolicies.yaml +++ b/deployments/gpu-operator/crds/nvidia.com_clusterpolicies.yaml @@ -1137,6 +1137,30 @@ spec: properties: configMapName: type: string + nodeLocalPaths: + description: |- + NodeLocalPaths is an optional list of absolute directory paths on the host which hold a + node-local package repository referenced by the repository configuration in + ConfigMapName (for example "URIs: file:///opt/local-packages"). Each path is + bind-mounted read-only into the NVIDIA driver container at the same path, so that the + package manager running inside the container can resolve file:// repository URIs. + + Each directory must exist on every node targeted by the driver DaemonSet; the driver + pod will not start on a node where it is missing. Ignored when precompiled drivers are + used, and rejected when ConfigMapName is empty. + + Packages installed from a node-local repository are compiled into a kernel module which + is loaded into the host kernel, so the repository is part of the node's trusted + computing base. The directory and all of its parents should be owned by root and not be + group or world writable, and the repository should be GPG signed where possible. + items: + maxLength: 4096 + minLength: 2 + pattern: ^/ + type: string + maxItems: 8 + type: array + x-kubernetes-list-type: set type: object repository: description: NVIDIA Driver image repository diff --git a/deployments/gpu-operator/crds/nvidia.com_nvidiadrivers.yaml b/deployments/gpu-operator/crds/nvidia.com_nvidiadrivers.yaml index 7ad08f203c..3f3fa77f1c 100644 --- a/deployments/gpu-operator/crds/nvidia.com_nvidiadrivers.yaml +++ b/deployments/gpu-operator/crds/nvidia.com_nvidiadrivers.yaml @@ -834,6 +834,30 @@ spec: properties: name: type: string + nodeLocalPaths: + description: |- + NodeLocalPaths is an optional list of absolute directory paths on the host which hold a + node-local package repository referenced by the repository configuration in Name (for + example "URIs: file:///opt/local-packages"). Each path is bind-mounted read-only into + the NVIDIA driver container at the same path, so that the package manager running + inside the container can resolve file:// repository URIs. + + Each directory must exist on every node targeted by the driver DaemonSet; the driver + pod will not start on a node where it is missing. Ignored when precompiled drivers are + used, and rejected when Name is empty. + + Packages installed from a node-local repository are compiled into a kernel module which + is loaded into the host kernel, so the repository is part of the node's trusted + computing base. The directory and all of its parents should be owned by root and not be + group or world writable, and the repository should be GPG signed where possible. + items: + maxLength: 4096 + minLength: 2 + pattern: ^/ + type: string + maxItems: 8 + type: array + x-kubernetes-list-type: set type: object repository: description: NVIDIA Driver repository diff --git a/deployments/gpu-operator/templates/nvidiadriver.yaml b/deployments/gpu-operator/templates/nvidiadriver.yaml index 262d551dbe..aba739e0d2 100644 --- a/deployments/gpu-operator/templates/nvidiadriver.yaml +++ b/deployments/gpu-operator/templates/nvidiadriver.yaml @@ -65,6 +65,9 @@ spec: {{- if .Values.driver.repoConfig.configMapName }} repoConfig: name: {{ .Values.driver.repoConfig.configMapName }} + {{- with .Values.driver.repoConfig.nodeLocalPaths }} + nodeLocalPaths: {{ toYaml . | nindent 6 }} + {{- end }} {{- end }} {{- if .Values.driver.certConfig.name }} certConfig: diff --git a/deployments/gpu-operator/values.yaml b/deployments/gpu-operator/values.yaml index f6222b0d5c..e30bf5d4b9 100644 --- a/deployments/gpu-operator/values.yaml +++ b/deployments/gpu-operator/values.yaml @@ -212,6 +212,17 @@ driver: # Private mirror repository configuration repoConfig: configMapName: "" + # Optional. Absolute directory paths on the host that hold a node-local package + # repository referenced by the repository configuration above, e.g. a deb822 source + # with "URIs: file:///opt/local-packages". Each path is bind-mounted read-only into + # the driver container at the same path. The directory must exist on every GPU node. + # Only used when configMapName is set and precompiled drivers are not in use. + # + # Packages installed from a node-local repository end up in a kernel module loaded + # into the host kernel, so the repository is part of the node's trusted computing + # base: it should be GPG signed, and the directory and all of its parents should be + # owned by root and not group or world writable. + nodeLocalPaths: [] # custom ssl key/certificate configuration certConfig: name: "" diff --git a/internal/config/driver_config_digest_test.go b/internal/config/driver_config_digest_test.go index 10da9cd0a4..04df1fa9bf 100644 --- a/internal/config/driver_config_digest_test.go +++ b/internal/config/driver_config_digest_test.go @@ -20,7 +20,10 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" + + "github.com/NVIDIA/gpu-operator/internal/utils" ) func TestExtractEnvVars(t *testing.T) { @@ -387,3 +390,38 @@ func TestDriverConfigDigestFromPodSpec(t *testing.T) { }) } } + +// TestDriverInstallStateDigestChangesWithHostPath pins the behaviour that a change to a +// host path volume — such as a node-local package repository path — produces a different +// driver config digest, and therefore rolls the driver pods. See VolumeConfig.HostPath. +func TestDriverInstallStateDigestChangesWithHostPath(t *testing.T) { + base := &DriverInstallState{ + DriverImage: "nvcr.io/nvidia/driver:580.126.16", + AdditionalVolumes: []VolumeConfig{ + {Name: "node-local-repo-0", HostPath: "/opt/local-packages"}, + }, + AdditionalVolumeMounts: []VolumeMountConfig{ + {Name: "node-local-repo-0", MountPath: "/opt/local-packages", ReadOnly: true}, + }, + } + + changedHostPath := &DriverInstallState{ + DriverImage: base.DriverImage, + AdditionalVolumes: []VolumeConfig{ + {Name: "node-local-repo-0", HostPath: "/srv/pkgs"}, + }, + AdditionalVolumeMounts: base.AdditionalVolumeMounts, + } + + noHostPath := &DriverInstallState{ + DriverImage: base.DriverImage, + } + + baseDigest := utils.GetObjectHashIgnoreEmptyKeys(base) + require.NotEqual(t, baseDigest, utils.GetObjectHashIgnoreEmptyKeys(changedHostPath), + "changing a host path must change the driver config digest") + require.NotEqual(t, baseDigest, utils.GetObjectHashIgnoreEmptyKeys(noHostPath), + "adding a host path volume must change the driver config digest") + require.Equal(t, baseDigest, utils.GetObjectHashIgnoreEmptyKeys(base), + "the driver config digest must be stable for identical configuration") +} diff --git a/internal/state/driver_nodelocalrepo_test.go b/internal/state/driver_nodelocalrepo_test.go new file mode 100644 index 0000000000..cfdf603d2b --- /dev/null +++ b/internal/state/driver_nodelocalrepo_test.go @@ -0,0 +1,258 @@ +/** +# 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 state + +import ( + "context" + "strings" + "testing" + + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + nvidiav1alpha1 "github.com/NVIDIA/gpu-operator/api/nvidia/v1alpha1" + "github.com/NVIDIA/gpu-operator/internal/consts" + "github.com/NVIDIA/gpu-operator/internal/render" + "github.com/NVIDIA/gpu-operator/internal/utils" +) + +// newNodeLocalRepoDriverState returns a stateDriver with a repo ConfigMap already present. +func newNodeLocalRepoDriverState(t *testing.T) *stateDriver { + t.Helper() + + repoConfigMap := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-repo-config", + Namespace: "test-ns", + }, + Data: map[string]string{ + "local-packages.sources": "Types: deb", + }, + } + fakeClient := fake.NewClientBuilder().WithScheme(driverTestScheme(t)).WithObjects(repoConfigMap).Build() + return &stateDriver{stateSkel: stateSkel{client: fakeClient, namespace: "test-ns"}} +} + +func nodeLocalRepoDriverCR(configMapName string, nodeLocalPaths []string, usePrecompiled bool) *nvidiav1alpha1.NVIDIADriver { + cr := &nvidiav1alpha1.NVIDIADriver{ + Spec: nvidiav1alpha1.NVIDIADriverSpec{ + RepoConfig: &nvidiav1alpha1.DriverRepoConfigSpec{ + Name: configMapName, + NodeLocalPaths: nodeLocalPaths, + }, + }, + } + if usePrecompiled { + cr.Spec.UsePrecompiled = new(true) + } + return cr +} + +func nodeLocalRepoConfigs(t *testing.T, cfgs *additionalConfigs) (map[string]corev1.Volume, map[string]corev1.VolumeMount) { + t.Helper() + + volumes := map[string]corev1.Volume{} + for _, volume := range cfgs.Volumes { + if !strings.HasPrefix(volume.Name, "node-local-repo-") { + continue + } + require.NotNil(t, volume.HostPath, "volume %s must be a hostPath volume", volume.Name) + volumes[volume.HostPath.Path] = volume + } + + mounts := map[string]corev1.VolumeMount{} + for _, mount := range cfgs.VolumeMounts { + if !strings.HasPrefix(mount.Name, "node-local-repo-") { + continue + } + mounts[mount.MountPath] = mount + } + return volumes, mounts +} + +func TestGetDriverAdditionalConfigsRepoConfigNodeLocalPaths(t *testing.T) { + driverState := newNodeLocalRepoDriverState(t) + + cfgs, err := driverState.getDriverAdditionalConfigs(context.Background(), + nodeLocalRepoDriverCR("test-repo-config", []string{"/opt/local-packages"}, false), + fakeClusterInfo{containerRuntime: consts.Containerd}, + nodePool{osRelease: "ubuntu", osVersion: "24.04"}) + require.NoError(t, err) + + volumes, mounts := nodeLocalRepoConfigs(t, cfgs) + require.Len(t, volumes, 1) + volume := volumes["/opt/local-packages"] + require.Equal(t, "node-local-repo-0", volume.Name) + require.NotNil(t, volume.HostPath.Type) + require.Equal(t, corev1.HostPathDirectory, *volume.HostPath.Type) + + require.Len(t, mounts, 1) + mount := mounts["/opt/local-packages"] + require.Equal(t, "node-local-repo-0", mount.Name) + require.True(t, mount.ReadOnly) + + // the repo ConfigMap volume must still be present alongside the host path + var sawConfigMapVolume bool + for _, volume := range cfgs.Volumes { + if volume.ConfigMap != nil && volume.ConfigMap.Name == "test-repo-config" { + sawConfigMapVolume = true + } + } + require.True(t, sawConfigMapVolume, "repo ConfigMap volume must still be applied") +} + +func TestGetDriverAdditionalConfigsRepoConfigMultipleNodeLocalPathsSorted(t *testing.T) { + driverState := newNodeLocalRepoDriverState(t) + + cfgs, err := driverState.getDriverAdditionalConfigs(context.Background(), + nodeLocalRepoDriverCR("test-repo-config", []string{"/srv/pkgs", "/opt/local-packages"}, false), + fakeClusterInfo{containerRuntime: consts.Containerd}, + nodePool{osRelease: "ubuntu", osVersion: "24.04"}) + require.NoError(t, err) + + volumes, _ := nodeLocalRepoConfigs(t, cfgs) + require.Len(t, volumes, 2) + require.Equal(t, "node-local-repo-0", volumes["/opt/local-packages"].Name) + require.Equal(t, "node-local-repo-1", volumes["/srv/pkgs"].Name) +} + +func TestGetDriverAdditionalConfigsRepoConfigNodeLocalPathsWithoutConfigMap(t *testing.T) { + driverState := newNodeLocalRepoDriverState(t) + + _, err := driverState.getDriverAdditionalConfigs(context.Background(), + nodeLocalRepoDriverCR("", []string{"/opt/local-packages"}, false), + fakeClusterInfo{containerRuntime: consts.Containerd}, + nodePool{osRelease: "ubuntu", osVersion: "24.04"}) + require.ErrorContains(t, err, "repoConfig.name is empty") +} + +func TestGetDriverAdditionalConfigsRepoConfigNodeLocalPathsInvalid(t *testing.T) { + driverState := newNodeLocalRepoDriverState(t) + + _, err := driverState.getDriverAdditionalConfigs(context.Background(), + nodeLocalRepoDriverCR("test-repo-config", []string{"/opt/local-packages/"}, false), + fakeClusterInfo{containerRuntime: consts.Containerd}, + nodePool{osRelease: "ubuntu", osVersion: "24.04"}) + require.ErrorContains(t, err, "not a clean path") +} + +func TestGetDriverAdditionalConfigsRepoConfigNodeLocalPathsPrecompiled(t *testing.T) { + driverState := newNodeLocalRepoDriverState(t) + + cfgs, err := driverState.getDriverAdditionalConfigs(context.Background(), + nodeLocalRepoDriverCR("test-repo-config", []string{"/opt/local-packages"}, true), + fakeClusterInfo{containerRuntime: consts.Containerd}, + nodePool{osRelease: "ubuntu", osVersion: "24.04"}) + require.NoError(t, err) + + volumes, mounts := nodeLocalRepoConfigs(t, cfgs) + require.Empty(t, volumes) + require.Empty(t, mounts) +} + +// TestGetDriverAdditionalConfigsNoNodeLocalPaths is the regression guard for the feature +// being opt-in. +func TestGetDriverAdditionalConfigsNoNodeLocalPaths(t *testing.T) { + driverState := newNodeLocalRepoDriverState(t) + + cfgs, err := driverState.getDriverAdditionalConfigs(context.Background(), + nodeLocalRepoDriverCR("test-repo-config", nil, false), + fakeClusterInfo{containerRuntime: consts.Containerd}, + nodePool{osRelease: "ubuntu", osVersion: "24.04"}) + require.NoError(t, err) + + volumes, mounts := nodeLocalRepoConfigs(t, cfgs) + require.Empty(t, volumes) + require.Empty(t, mounts) +} + +// TestDriverNodeLocalRepoVolumesRender renders the driver DaemonSet with the volumes and +// mounts produced by the real helper, proving the manifest template emits them correctly +// without requiring a golden-file update. +func TestDriverNodeLocalRepoVolumesRender(t *testing.T) { + volumes, mounts, err := utils.NodeLocalRepoVolumes([]string{"/opt/local-packages"}) + require.NoError(t, err) + + state, err := NewStateDriver(nil, "", nil, manifestDir) + require.NoError(t, err) + stateDriver, ok := state.(*stateDriver) + require.True(t, ok) + + renderData := getMinimalDriverRenderData() + renderData.AdditionalConfigs = &additionalConfigs{Volumes: volumes, VolumeMounts: mounts} + + objs, err := stateDriver.renderer.RenderObjects(&render.TemplatingData{Data: renderData}) + require.NoError(t, err) + + actual, err := getYAMLString(objs) + require.NoError(t, err) + + require.Contains(t, actual, "- mountPath: /opt/local-packages\n name: node-local-repo-0\n readOnly: true", + "driver container must mount the node-local repository read-only") + require.Contains(t, actual, "- hostPath:\n path: /opt/local-packages\n type: Directory\n name: node-local-repo-0", + "pod spec must declare the node-local repository as a Directory hostPath volume") +} + +// TestDriverRepoConfigDeb822SourcesOverride pins the documented air-gap recipe: a repo +// ConfigMap key named after the base image's own default deb822 source file is mounted +// with a subPath, and therefore overrides the defaults rather than adding to them. +func TestDriverRepoConfigDeb822SourcesOverride(t *testing.T) { + repoConfigMap := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "local-package-repo", + Namespace: "test-ns", + }, + Data: map[string]string{ + "ubuntu.sources": "Types: deb\nURIs: file:///opt/local-packages\nSuites: ./\n", + }, + } + fakeClient := fake.NewClientBuilder().WithScheme(driverTestScheme(t)).WithObjects(repoConfigMap).Build() + driverState := &stateDriver{stateSkel: stateSkel{client: fakeClient, namespace: "test-ns"}} + + mounts, items, err := driverState.createConfigMapVolumeMounts(context.Background(), "test-ns", + "local-package-repo", "/etc/apt/sources.list.d") + require.NoError(t, err) + + require.Len(t, mounts, 1) + require.Equal(t, "/etc/apt/sources.list.d/ubuntu.sources", mounts[0].MountPath) + require.Equal(t, "ubuntu.sources", mounts[0].SubPath, + "the key must be mounted with a subPath so it replaces the base image's default sources") + require.True(t, mounts[0].ReadOnly) + + require.Len(t, items, 1) + require.Equal(t, "ubuntu.sources", items[0].Key) + require.Equal(t, "ubuntu.sources", items[0].Path) +} + +// TestGetDriverAdditionalConfigsEmptyNodeLocalPathsWithoutConfigMap covers what a default +// helm install now produces: an empty but non-nil nodeLocalPaths list with no repo +// ConfigMap. This must be a no-op, not an error. +func TestGetDriverAdditionalConfigsEmptyNodeLocalPathsWithoutConfigMap(t *testing.T) { + driverState := newNodeLocalRepoDriverState(t) + + cfgs, err := driverState.getDriverAdditionalConfigs(context.Background(), + nodeLocalRepoDriverCR("", []string{}, false), + fakeClusterInfo{containerRuntime: consts.Containerd}, + nodePool{osRelease: "ubuntu", osVersion: "24.04"}) + require.NoError(t, err) + + volumes, mounts := nodeLocalRepoConfigs(t, cfgs) + require.Empty(t, volumes) + require.Empty(t, mounts) +} diff --git a/internal/state/driver_volumes.go b/internal/state/driver_volumes.go index 34f60ea609..7d132bfe8f 100644 --- a/internal/state/driver_volumes.go +++ b/internal/state/driver_volumes.go @@ -28,6 +28,7 @@ import ( "github.com/NVIDIA/gpu-operator/api/nvidia/v1alpha1" "github.com/NVIDIA/gpu-operator/controllers/clusterinfo" "github.com/NVIDIA/gpu-operator/internal/consts" + "github.com/NVIDIA/gpu-operator/internal/utils" ) // RepoConfigPathMap indicates standard OS specific paths for repository configuration files @@ -155,6 +156,21 @@ func (s *stateDriver) getDriverAdditionalConfigs(ctx context.Context, cr *v1alph } additionalCfgs.VolumeMounts = append(additionalCfgs.VolumeMounts, volumeMounts...) additionalCfgs.Volumes = append(additionalCfgs.Volumes, createConfigMapVolume(cr.Spec.RepoConfig.Name, itemsToInclude)) + + // expose any node-local package repositories referenced by the repo configuration + // (e.g. "URIs: file:///opt/local-packages") to the driver container + if nodeLocalPaths := cr.Spec.RepoConfigNodeLocalPaths(); len(nodeLocalPaths) > 0 { + repoVolumes, repoVolumeMounts, err := utils.NodeLocalRepoVolumes(nodeLocalPaths) + if err != nil { + return nil, fmt.Errorf("ERROR: invalid repoConfig.nodeLocalPaths: %w", err) + } + logger.Info("Mounting node-local package repositories into the driver container", "nodeLocalPaths", nodeLocalPaths) + additionalCfgs.VolumeMounts = append(additionalCfgs.VolumeMounts, repoVolumeMounts...) + additionalCfgs.Volumes = append(additionalCfgs.Volumes, repoVolumes...) + } + } else if len(cr.Spec.RepoConfigNodeLocalPaths()) > 0 { + return nil, fmt.Errorf("ERROR: repoConfig.nodeLocalPaths is set but repoConfig.name is empty; " + + "nodeLocalPaths only applies when a custom repository configuration is provided") } // set any custom ssl key/certificate configuration provided diff --git a/internal/utils/nodelocalrepo.go b/internal/utils/nodelocalrepo.go new file mode 100644 index 0000000000..8b576c5989 --- /dev/null +++ b/internal/utils/nodelocalrepo.go @@ -0,0 +1,97 @@ +/** +# 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 utils + +import ( + "fmt" + "path/filepath" + "sort" + + corev1 "k8s.io/api/core/v1" + "k8s.io/utils/ptr" +) + +// NodeLocalRepoVolumePrefix is the volume-name prefix used for node-local package +// repository volumes mounted into the NVIDIA driver container. +const NodeLocalRepoVolumePrefix = "node-local-repo" + +// ValidateNodeLocalRepoPaths validates and normalizes a list of node-local package repository +// host paths. It returns the paths deduplicated and sorted, so that callers construct volumes +// deterministically. An error is returned if any path is not a clean, absolute, non-root +// directory path. +// +// Paths are never rewritten on the user's behalf: a silently normalized path would no longer +// match the file:// URI in the repository configuration, which is the exact failure this +// feature exists to remove. +func ValidateNodeLocalRepoPaths(paths []string) ([]string, error) { + seen := make(map[string]struct{}, len(paths)) + out := make([]string, 0, len(paths)) + for _, p := range paths { + if !filepath.IsAbs(p) { + return nil, fmt.Errorf("node-local repository path %q is not an absolute path", p) + } + if filepath.Clean(p) != p { + return nil, fmt.Errorf("node-local repository path %q is not a clean path (did you mean %q?)", p, filepath.Clean(p)) + } + if p == "/" { + return nil, fmt.Errorf("node-local repository path %q is not allowed: the host root filesystem cannot be used as a package repository path", p) + } + if _, ok := seen[p]; ok { + continue + } + seen[p] = struct{}{} + out = append(out, p) + } + sort.Strings(out) + return out, nil +} + +// NodeLocalRepoVolumes builds the read-only host path volumes and corresponding volume mounts +// which expose node-local package repositories to the NVIDIA driver container. Each host path +// is mounted at the same path inside the container so that file:// URIs in the repository +// configuration resolve identically on the host and in the container. +// +// The mounts are read-only to prevent the driver install scripts from mutating the node's +// package repository. This is a guardrail and not a security boundary: the driver container is +// privileged and can remount it writable. +func NodeLocalRepoVolumes(paths []string) ([]corev1.Volume, []corev1.VolumeMount, error) { + validated, err := ValidateNodeLocalRepoPaths(paths) + if err != nil { + return nil, nil, err + } + + volumes := make([]corev1.Volume, 0, len(validated)) + mounts := make([]corev1.VolumeMount, 0, len(validated)) + for i, p := range validated { + name := fmt.Sprintf("%s-%d", NodeLocalRepoVolumePrefix, i) + volumes = append(volumes, corev1.Volume{ + Name: name, + VolumeSource: corev1.VolumeSource{ + HostPath: &corev1.HostPathVolumeSource{ + Path: p, + Type: ptr.To(corev1.HostPathDirectory), + }, + }, + }) + mounts = append(mounts, corev1.VolumeMount{ + Name: name, + MountPath: p, + ReadOnly: true, + }) + } + return volumes, mounts, nil +} diff --git a/internal/utils/nodelocalrepo_test.go b/internal/utils/nodelocalrepo_test.go new file mode 100644 index 0000000000..511a309ac5 --- /dev/null +++ b/internal/utils/nodelocalrepo_test.go @@ -0,0 +1,149 @@ +/** +# 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 utils + +import ( + "testing" + + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" +) + +func TestValidateNodeLocalRepoPaths(t *testing.T) { + testCases := []struct { + description string + input []string + expected []string + errContains string + }{ + { + description: "nil input", + input: nil, + expected: []string{}, + }, + { + description: "empty input", + input: []string{}, + expected: []string{}, + }, + { + description: "single valid path", + input: []string{"/opt/local-packages"}, + expected: []string{"/opt/local-packages"}, + }, + { + description: "duplicates removed and result sorted", + input: []string{"/b", "/a", "/b"}, + expected: []string{"/a", "/b"}, + }, + { + description: "relative path rejected", + input: []string{"opt/local-packages"}, + errContains: "not an absolute path", + }, + { + description: "empty string rejected", + input: []string{""}, + errContains: "not an absolute path", + }, + { + description: "trailing slash rejected with suggestion", + input: []string{"/opt/local-packages/"}, + errContains: `not a clean path (did you mean "/opt/local-packages"?)`, + }, + { + description: "double slash rejected", + input: []string{"//opt/pkgs"}, + errContains: "not a clean path", + }, + { + description: "parent traversal rejected", + input: []string{"/opt/../etc"}, + errContains: "not a clean path", + }, + { + description: "host root rejected", + input: []string{"/"}, + errContains: "host root filesystem", + }, + } + + for _, tc := range testCases { + t.Run(tc.description, func(t *testing.T) { + actual, err := ValidateNodeLocalRepoPaths(tc.input) + if tc.errContains != "" { + require.Error(t, err) + require.Contains(t, err.Error(), tc.errContains) + require.Nil(t, actual) + return + } + require.NoError(t, err) + require.Equal(t, tc.expected, actual) + }) + } +} + +func TestNodeLocalRepoVolumes(t *testing.T) { + volumes, mounts, err := NodeLocalRepoVolumes([]string{"/srv/pkgs", "/opt/local-packages"}) + require.NoError(t, err) + require.Len(t, volumes, 2) + require.Len(t, mounts, 2) + + // names are assigned over the sorted list + require.Equal(t, "node-local-repo-0", volumes[0].Name) + require.Equal(t, "/opt/local-packages", volumes[0].HostPath.Path) + require.Equal(t, "node-local-repo-1", volumes[1].Name) + require.Equal(t, "/srv/pkgs", volumes[1].HostPath.Path) + + for i, v := range volumes { + require.NotNil(t, v.HostPath, "volume %d must be a hostPath volume", i) + require.NotNil(t, v.HostPath.Type) + require.Equal(t, corev1.HostPathDirectory, *v.HostPath.Type) + } + + for i, m := range mounts { + require.Equal(t, volumes[i].Name, m.Name) + require.True(t, m.ReadOnly, "mount %d must be read-only", i) + // the container path must equal the host path so that file:// URIs resolve identically + require.Equal(t, volumes[i].HostPath.Path, m.MountPath) + } +} + +func TestNodeLocalRepoVolumesIsDeterministic(t *testing.T) { + volumesA, mountsA, err := NodeLocalRepoVolumes([]string{"/b", "/a"}) + require.NoError(t, err) + volumesB, mountsB, err := NodeLocalRepoVolumes([]string{"/a", "/b"}) + require.NoError(t, err) + + require.Equal(t, volumesA, volumesB) + require.Equal(t, mountsA, mountsB) +} + +func TestNodeLocalRepoVolumesEmpty(t *testing.T) { + volumes, mounts, err := NodeLocalRepoVolumes(nil) + require.NoError(t, err) + require.Empty(t, volumes) + require.Empty(t, mounts) +} + +func TestNodeLocalRepoVolumesPropagatesValidationError(t *testing.T) { + volumes, mounts, err := NodeLocalRepoVolumes([]string{"relative/path"}) + require.Error(t, err) + require.Contains(t, err.Error(), "not an absolute path") + require.Nil(t, volumes) + require.Nil(t, mounts) +} diff --git a/tests/e2e/helpers/clusterpolicy.go b/tests/e2e/helpers/clusterpolicy.go index 6db333ea92..18d142d825 100644 --- a/tests/e2e/helpers/clusterpolicy.go +++ b/tests/e2e/helpers/clusterpolicy.go @@ -123,6 +123,24 @@ func (h *ClusterPolicyClient) SetMIGStrategy(ctx context.Context, name, strategy }) } +// SetDriverRepoConfig sets the driver repository configuration. An empty configMapName +// with non-empty nodeLocalPaths is a deliberate misconfiguration used by the negative test. +func (h *ClusterPolicyClient) SetDriverRepoConfig(ctx context.Context, name, configMapName string, nodeLocalPaths []string) error { + return h.modify(ctx, name, func(clusterPolicy *nvidiav1.ClusterPolicy) { + clusterPolicy.Spec.Driver.RepoConfig = &nvidiav1.DriverRepoConfigSpec{ + ConfigMapName: configMapName, + NodeLocalPaths: nodeLocalPaths, + } + }) +} + +// ClearDriverRepoConfig removes any driver repository configuration. +func (h *ClusterPolicyClient) ClearDriverRepoConfig(ctx context.Context, name string) error { + return h.modify(ctx, name, func(clusterPolicy *nvidiav1.ClusterPolicy) { + clusterPolicy.Spec.Driver.RepoConfig = nil + }) +} + func (h *ClusterPolicyClient) WaitForReady(ctx context.Context, name string, timeout time.Duration) error { return wait.PollUntilContextTimeout(ctx, defaultPollingInterval, timeout, true, func(ctx context.Context) (bool, error) { clusterPolicy, err := h.Get(ctx, name) diff --git a/tests/e2e/suites/driver_repoconfig_test.go b/tests/e2e/suites/driver_repoconfig_test.go new file mode 100644 index 0000000000..4d1d5923d7 --- /dev/null +++ b/tests/e2e/suites/driver_repoconfig_test.go @@ -0,0 +1,235 @@ +/** +# 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 suites + +import ( + "context" + "strings" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes" + + gpuclientset "github.com/NVIDIA/gpu-operator/api/versioned" + "github.com/NVIDIA/gpu-operator/tests/e2e/framework" + e2elog "github.com/NVIDIA/gpu-operator/tests/e2e/framework/logs" + "github.com/NVIDIA/gpu-operator/tests/e2e/helpers" +) + +const ( + nodeLocalRepoConfigMapName = "e2e-local-repo" + nodeLocalRepoPath = "/opt/local-packages" + nodeLocalRepoVolumePrefix = "node-local-repo-" + driverDaemonSetName = "nvidia-driver-daemonset" + driverContainerName = "nvidia-driver-ctr" + driverConfigDigestEnvName = "DRIVER_CONFIG_DIGEST" +) + +// nodeLocalRepoVolumesFromDaemonSet returns the node-local repository hostPath volumes on +// the DaemonSet pod template, keyed by host path. +func nodeLocalRepoVolumesFromDaemonSet(ds *appsv1.DaemonSet) map[string]corev1.Volume { + volumes := map[string]corev1.Volume{} + for _, volume := range ds.Spec.Template.Spec.Volumes { + if strings.HasPrefix(volume.Name, nodeLocalRepoVolumePrefix) && volume.HostPath != nil { + volumes[volume.HostPath.Path] = volume + } + } + return volumes +} + +// nodeLocalRepoMountsFromDaemonSet returns the node-local repository volume mounts on the +// driver container, keyed by mount path. +func nodeLocalRepoMountsFromDaemonSet(ds *appsv1.DaemonSet) map[string]corev1.VolumeMount { + mounts := map[string]corev1.VolumeMount{} + for _, container := range ds.Spec.Template.Spec.Containers { + if container.Name != driverContainerName { + continue + } + for _, mount := range container.VolumeMounts { + if strings.HasPrefix(mount.Name, nodeLocalRepoVolumePrefix) { + mounts[mount.MountPath] = mount + } + } + } + return mounts +} + +// driverConfigDigestFromDaemonSet returns the driver config digest env value on the driver +// container, which changes whenever the driver install configuration changes. +func driverConfigDigestFromDaemonSet(ds *appsv1.DaemonSet) string { + for _, container := range ds.Spec.Template.Spec.Containers { + if container.Name != driverContainerName { + continue + } + for _, env := range container.Env { + if env.Name == driverConfigDigestEnvName { + return env.Value + } + } + } + return "" +} + +// hasRepoConfigMapVolume reports whether the repo ConfigMap is still mounted alongside the +// node-local repository host paths. +func hasRepoConfigMapVolume(ds *appsv1.DaemonSet, configMapName string) bool { + for _, volume := range ds.Spec.Template.Spec.Volumes { + if volume.ConfigMap != nil && volume.ConfigMap.Name == configMapName { + return true + } + } + return false +} + +var _ = Describe("Driver node-local package repository", Label("clusterPolicy"), func() { + f := framework.NewFramework("gpu-operator") + f.SkipNamespaceCreation = true + + var ( + clusterPolicyClient *helpers.ClusterPolicyClient + daemonSetClient *helpers.DaemonSetClient + clientSet kubernetes.Interface + initialDigest string + ) + + BeforeEach(func(ctx context.Context) { + clientSet = f.ClientSet + gpuClient, err := gpuclientset.NewForConfig(f.ClientConfig()) + Expect(err).NotTo(HaveOccurred()) + clusterPolicyClient = helpers.NewClusterPolicyClient(gpuClient) + daemonSetClient = helpers.NewDaemonSetClient(clientSet) + + getClusterPolicyOrSkip(ctx, clusterPolicyClient, defaultPolicyName) + + // the repository configuration is required; nodeLocalPaths is only honoured with it + _, err = clientSet.CoreV1().ConfigMaps(defaultNamespace).Create(ctx, &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: nodeLocalRepoConfigMapName, + Namespace: defaultNamespace, + }, + Data: map[string]string{ + "local-packages.sources": "Types: deb\nURIs: file://" + nodeLocalRepoPath + "\nSuites: ./\nTrusted: yes\n", + }, + }, metav1.CreateOptions{}) + if err != nil && !strings.Contains(err.Error(), "already exists") { + Expect(err).NotTo(HaveOccurred()) + } + + ds, err := daemonSetClient.Get(ctx, defaultNamespace, driverDaemonSetName) + Expect(err).NotTo(HaveOccurred()) + initialDigest = driverConfigDigestFromDaemonSet(ds) + }) + + AfterEach(func(ctx context.Context) { + Expect(clusterPolicyClient.ClearDriverRepoConfig(ctx, defaultPolicyName)).To(Succeed()) + _ = clientSet.CoreV1().ConfigMaps(defaultNamespace).Delete(ctx, + nodeLocalRepoConfigMapName, metav1.DeleteOptions{}) + }) + + // Assertions are made on the DaemonSet object rather than pod readiness, so the test is + // meaningful on a cluster where the driver cannot actually come up. + It("should mount the node-local repository into the driver container", func(ctx context.Context) { + Expect(clusterPolicyClient.SetDriverRepoConfig(ctx, defaultPolicyName, + nodeLocalRepoConfigMapName, []string{nodeLocalRepoPath})).To(Succeed()) + + Eventually(func(ctx context.Context) bool { + ds, err := daemonSetClient.Get(ctx, defaultNamespace, driverDaemonSetName) + if err != nil { + e2elog.Logf("WARN: error getting driver daemonset: %v", err) + return false + } + _, ok := nodeLocalRepoVolumesFromDaemonSet(ds)[nodeLocalRepoPath] + return ok + }).WithPolling(2 * time.Second).Within(specUpdateTimeout).WithContext(ctx).Should(BeTrue()) + + ds, err := daemonSetClient.Get(ctx, defaultNamespace, driverDaemonSetName) + Expect(err).NotTo(HaveOccurred()) + + volume := nodeLocalRepoVolumesFromDaemonSet(ds)[nodeLocalRepoPath] + Expect(volume.HostPath).NotTo(BeNil()) + Expect(volume.HostPath.Type).NotTo(BeNil()) + Expect(*volume.HostPath.Type).To(Equal(corev1.HostPathDirectory)) + + mount, ok := nodeLocalRepoMountsFromDaemonSet(ds)[nodeLocalRepoPath] + Expect(ok).To(BeTrue(), "driver container must mount the node-local repository") + Expect(mount.ReadOnly).To(BeTrue()) + Expect(mount.Name).To(Equal(volume.Name)) + + // the repo ConfigMap must still be mounted alongside the host path + Expect(hasRepoConfigMapVolume(ds, nodeLocalRepoConfigMapName)).To(BeTrue()) + + // changing the driver install configuration must roll the driver pods + Expect(driverConfigDigestFromDaemonSet(ds)).NotTo(Equal(initialDigest)) + }) + + It("should remove the mount when the node-local paths are cleared", func(ctx context.Context) { + Expect(clusterPolicyClient.SetDriverRepoConfig(ctx, defaultPolicyName, + nodeLocalRepoConfigMapName, []string{nodeLocalRepoPath})).To(Succeed()) + + Eventually(func(ctx context.Context) bool { + ds, err := daemonSetClient.Get(ctx, defaultNamespace, driverDaemonSetName) + if err != nil { + return false + } + return len(nodeLocalRepoVolumesFromDaemonSet(ds)) == 1 + }).WithPolling(2 * time.Second).Within(specUpdateTimeout).WithContext(ctx).Should(BeTrue()) + + Expect(clusterPolicyClient.SetDriverRepoConfig(ctx, defaultPolicyName, + nodeLocalRepoConfigMapName, nil)).To(Succeed()) + + Eventually(func(ctx context.Context) bool { + ds, err := daemonSetClient.Get(ctx, defaultNamespace, driverDaemonSetName) + if err != nil { + return false + } + return len(nodeLocalRepoVolumesFromDaemonSet(ds)) == 0 && + len(nodeLocalRepoMountsFromDaemonSet(ds)) == 0 + }).WithPolling(2 * time.Second).Within(specUpdateTimeout).WithContext(ctx).Should(BeTrue()) + }) + + It("should not mount anything when node-local paths are unset", func(ctx context.Context) { + Expect(clusterPolicyClient.SetDriverRepoConfig(ctx, defaultPolicyName, + nodeLocalRepoConfigMapName, nil)).To(Succeed()) + + Consistently(func(ctx context.Context) int { + ds, err := daemonSetClient.Get(ctx, defaultNamespace, driverDaemonSetName) + if err != nil { + return -1 + } + return len(nodeLocalRepoVolumesFromDaemonSet(ds)) + }).WithPolling(2 * time.Second).Within(15 * time.Second).WithContext(ctx).Should(Equal(0)) + }) + + It("should reject node-local paths without a repository ConfigMap", func(ctx context.Context) { + Expect(clusterPolicyClient.SetDriverRepoConfig(ctx, defaultPolicyName, + "", []string{nodeLocalRepoPath})).To(Succeed()) + + // the operator must surface the misconfiguration rather than mounting anything + Consistently(func(ctx context.Context) int { + ds, err := daemonSetClient.Get(ctx, defaultNamespace, driverDaemonSetName) + if err != nil { + return -1 + } + return len(nodeLocalRepoVolumesFromDaemonSet(ds)) + }).WithPolling(2 * time.Second).Within(15 * time.Second).WithContext(ctx).Should(Equal(0)) + }) +})