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)) + }) +})