Add support for cgroup-only device nodes

This change allows the following device node CDI spec to be considered
valid:

path: "*"
type: "c"
major: 195
minor: -1
permissions: "rwm"

When this is included in the spec, no device nodes are added to the OCI
runtime spec, but the linux resources (cgroups) are updated to allow
access to all devices in the same way as the Docker `--device-cgroup-rule`
flag.

Note that a major or minor number of -1 is equivalent to a * cgroup rule.

This is useful when starting a container that needs to be able to create
device nodes that do not exist when it is started.

Signed-off-by: Evan Lezar <elezar@nvidia.com>

Author: elezar

SPEC.md

@@ -34,6 +34,7 @@ Released versions of the spec are available as Git tags.
 | v0.8.0 |   | Remove .ToOCI() functions from specs-go package. |
 | v1.0.0 |   | Move minimum version logic to specs-go package. |
 | v1.1.0 |   | Add `NetDevices` to `ContainerEdits`, `Schemata` and `EnableMonitoring` to `IntelRdt`. Dropped `EnableCMT` and `EnableMBM` fields from `IntelRdt`. |
+| v1.2.0 |   | Allow wildcard device nodes to be specified. These have a `Path` of `*` and allow for device `Major` and `Minor` values of `-1` which is mapped to the `*` device in the OCI runtime spec. |
 
 *Note*: spec loading fails on unknown fields and when the minimum required version is higher than the version specified in the spec. The minimum required version is determined based on the usage of fields mentioned in the table above. For example the minimum required version is v0.6.0 if the `Annotations` field is used in the spec, but `IntelRdt` is not.
 `MinimumRequiredVersion` API can be used to get the minimum required version.
@@ -229,11 +230,11 @@ The `containerEdits` field is referenced in two places in the specification:
 The `containerEdits` field has the following definition:
   * `env` (array of strings in the format of "VARNAME=VARVALUE", OPTIONAL) describes the environment variables that should be set. These values are appended to the container environment array.
   * `deviceNodes` (array of objects, OPTIONAL) describes the device nodes that should be mounted:
-    * `path` (string, REQUIRED) path of the device within the container.
+    * `path` (string, REQUIRED) path of the device within the container. As of v1.2.0, if the path is `*` the device node is used to only update the cgroups for the container.
     * `hostPath` (string, OPTIONAL) path of the device node on the host. If not specified the value for `path` is used. Added in v0.5.0.
     * `type` (string, OPTIONAL) Device type: block, char, etc.
-    * `major` (int64, OPTIONAL) Device major number.
-    * `minor` (int64, OPTIONAL) Device minor number.
+    * `major` (int64, OPTIONAL) Device major number. As of v1.2.0, if the `path` of the device node is `*` the value of `-1` is allowed and represents a group rule matching all device major numbers.
+    * `minor` (int64, OPTIONAL) Device minor number. As of v1.2.0, if the `path` of the device node is `*` the value of `-1` is allowed and represents a group rule matching all device minor numbers.
     * `fileMode` (int64, OPTIONAL) file mode for the device.
     * `permissions` (string, OPTIONAL) Cgroups permissions of the device, candidates are one or more of:
       * r - allows container to read from the specified device.

pkg/cdi/container-edits.go

@@ -328,19 +328,11 @@ type DeviceNode struct {
 
 // Validate a CDI Spec DeviceNode.
 func (d *DeviceNode) Validate() error {
-	validTypes := map[string]struct{}{
-		"":  {},
-		"b": {},
-		"c": {},
-		"u": {},
-		"p": {},
-	}
-
 	if d.Path == "" {
 		return errors.New("invalid (empty) device path")
 	}
-	if _, ok := validTypes[d.Type]; !ok {
-		return fmt.Errorf("device %q: invalid type %q", d.Path, d.Type)
+	if strings.Contains(d.Path, "*") && d.Path != "*" {
+		return errors.New("invalid device device path; wildcard device path")
 	}
 	switch {
 	case d.Permissions == "":
@@ -350,10 +342,41 @@ func (d *DeviceNode) Validate() error {
 			d.Path, d.Permissions)
 	}
 
+	switch {
+	case d.isWildcardDevice():
+		validTypes := map[string]struct{}{"b": {}, "c": {}}
+		if _, ok := validTypes[d.Type]; !ok {
+			return fmt.Errorf("device %q: invalid type %q", d.Path, d.Type)
+		}
+	default:
+		validTypes := map[string]struct{}{
+			"":  {},
+			"b": {},
+			"c": {},
+			"u": {},
+			"p": {},
+		}
+		if _, ok := validTypes[d.Type]; !ok {
+			return fmt.Errorf("device %q: invalid type %q", d.Path, d.Type)
+		}
+		// For a regular device, we also check that we have valid major and
+		// minor numbers.
+		if d.Major == -1 {
+			return fmt.Errorf("wildcard device major requires a wildcard device")
+		}
+		if d.Minor == -1 {
+			return fmt.Errorf("wildcard device minor requires a wildcard device")
+		}
+	}
+
 	return nil
 }
 
 func (d *DeviceNode) addToGenerator(specgen *ocigen.Generator, spec *oci.Spec) error {
+	if d.isWildcardDevice() {
+		return d.addAsWildcardDevice(specgen)
+	}
+
 	err := d.fillMissingInfo()
 	if err != nil {
 		return err
@@ -389,8 +412,33 @@ func (d *DeviceNode) getAccessString() string {
 		return d.Permissions
 	}
 }
+
+// isWildcardDevice returns whether the device node represents a "wildcard" device.
+// Such devices do not cause device nodes to be created in the container, but
+// do update the cgroups to allow device access. A wildcard device always has
+// the path specified as "*" and setting major or minor numbers to -1 will add
+// a cgroup rule that matches all major (or minor) numbers.
+func (d *DeviceNode) isWildcardDevice() bool {
+	return d.Path == "*"
+}
+
+func (d *DeviceNode) addAsWildcardDevice(specgen *ocigen.Generator) error {
+	if d.Type != "b" && d.Type != "c" {
+		return fmt.Errorf("wildcard device node not supported for device type %v", d.Type)
+	}
+	var major *int64
+	if d.Major != -1 {
+		major = &d.Major
+	}
+	var minor *int64
+	if d.Minor != -1 {
+		minor = &d.Minor
+	}
+	specgen.AddLinuxResourcesDevice(true, d.Type, major, minor, d.getAccessString())
+
 	return nil
 }
+
 // Hook is a CDI Spec Hook wrapper, used for validating hooks.
 type Hook struct {
 	*cdi.Hook

pkg/cdi/container-edits_test.go

@@ -97,6 +97,31 @@ func TestValidateContainerEdits(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "valid wildcard device nodes",
+			edits: &cdi.ContainerEdits{
+				DeviceNodes: []*cdi.DeviceNode{
+					{
+						Path: "*", Type: "c",
+					},
+					{
+						Path: "*", Type: "b",
+					},
+					{
+						Path: "*", Type: "c", Major: -1,
+					},
+					{
+						Path: "*", Type: "b", Minor: -1,
+					},
+					{
+						Path: "*", Type: "c", Permissions: NoPermissions,
+					},
+					{
+						Path: "*", Type: "c", Permissions: "rwm",
+					},
+				},
+			},
+		},
 		{
 			name: "invalid device, empty path",
 			edits: &cdi.ContainerEdits{
@@ -133,6 +158,54 @@ func TestValidateContainerEdits(t *testing.T) {
 			},
 			invalid: true,
 		},
+		{
+			name: "invalid wildcard device, wildcard in path",
+			edits: &cdi.ContainerEdits{
+				DeviceNodes: []*cdi.DeviceNode{
+					{
+						Path: "/dev/vendor*",
+						Type: "b",
+					},
+				},
+			},
+			invalid: true,
+		},
+		{
+			name: "invalid wildcard device, non-wildcard with major",
+			edits: &cdi.ContainerEdits{
+				DeviceNodes: []*cdi.DeviceNode{
+					{
+						Path:  "/dev/vendorctl",
+						Major: -1,
+					},
+				},
+			},
+			invalid: true,
+		},
+		{
+			name: "invalid wildcard device, non-wildcard with minor",
+			edits: &cdi.ContainerEdits{
+				DeviceNodes: []*cdi.DeviceNode{
+					{
+						Path:  "/dev/vendorctl",
+						Minor: -1,
+					},
+				},
+			},
+			invalid: true,
+		},
+		{
+			name: "invalid wildcard device, with incorrect type",
+			edits: &cdi.ContainerEdits{
+				DeviceNodes: []*cdi.DeviceNode{
+					{
+						Path: "*",
+						Type: "p",
+					},
+				},
+			},
+			invalid: true,
+		},
 		{
 			name: "valid device, with NoPermissions",
 			edits: &cdi.ContainerEdits{

specs-go/version.go

@@ -41,6 +41,7 @@ const (
 	v080 version = "v0.8.0"
 	v100 version = "v1.0.0"
 	v110 version = "v1.1.0"
+	v120 version = "v1.2.0"
 
 	// vEarliest is the earliest supported version of the CDI specification
 	vEarliest version = v030
@@ -60,6 +61,7 @@ var validSpecVersions = requiredVersionMap{
 	v080: requiresV080,
 	v100: requiresV100,
 	v110: requiresV110,
+	v120: requiresV120,
 }
 
 // ValidateVersion checks whether the specified spec version is valid.
@@ -142,6 +144,39 @@ func (r requiredVersionMap) requiredVersion(spec *Spec) version {
 	return minVersion
 }
 
+// requiresV120 returns true if the spec uses v1.2.0 features.
+func requiresV120(spec *Spec) bool {
+	if spec.ContainerEdits.requiresV120() {
+		return true
+	}
+
+	for _, d := range spec.Devices {
+		if d.ContainerEdits.requiresV120() {
+			return true
+		}
+	}
+
+	return false
+}
+
+// requiresV120 returns true if the specified containers uses v1.2.0 features.
+// These are:
+//   - Wildcard devices that can be used to only inject cgroup rules for a
+//     container.
+func (e ContainerEdits) requiresV120() bool {
+	for _, dn := range e.DeviceNodes {
+		switch {
+		case dn.Path == "*":
+			return true
+		case dn.Major == -1:
+			return true
+		case dn.Minor == -1:
+			return true
+		}
+	}
+	return false
+}
+
 // requiresV110 returns true if the spec uses v1.1.0 features.
 func requiresV110(spec *Spec) bool {
 	if i := spec.ContainerEdits.IntelRdt; i != nil {