Add AdditionalGIDs field to ContainerEdits

This change adds an AdditionalGIDs field to the ContainerEdits structure.
This allows CDI spec producers to associate a group ID with the container's
user to allow access to, for example, device nodes with specific permissions.

Signed-off-by: Evan Lezar <[email protected]>

Author: elezar

SPEC.md

@@ -27,7 +27,8 @@ Released versions of the spec are available as Git tags.
 | v0.4.0 |   | Added `type` field to Mount specification |
 | v0.5.0 |   | Add `HostPath` to `DeviceNodes` |
 | v0.6.0 |   | Add `Annotations` field to `Spec` and `Device` specifications |
-|            |    | Allow dots (`.`)  in name segment of `Kind` field |
+|        |   | Allow dots (`.`)  in name segment of `Kind` field |
+| v0.7.0 |   | Add `AdditionalGIDs` to `ContainerEdits` |
 
 *Note*: The initial release of a **spec** with version `v0.x.0` will be tagged as
 `v0.x.0` with subsequent changes to the API applicable to this version tagged as `v0.x.y`.
@@ -82,7 +83,7 @@ The keywords "must", "must not", "required", "shall", "shall not", "should", "sh
 
 ```
 {
-    "cdiVersion": "0.6.0",
+    "cdiVersion": "0.7.0",
     "kind": "<name>",
 
     // This field contains a set of key-value pairs that may be used to provide
@@ -149,6 +150,11 @@ The keywords "must", "must not", "required", "shall", "shall not", "should", "sh
                     "env":  [ "<envName>=<envValue>"], (optional)
                     "timeout": <int> (optional)
                 }
+            ],
+            // Additional GIDs to add to the container process.
+            // Note that a value of 0 is ignored.
+            additionalGIDs: [ (optional)
+              <uint32>
             ]
         }
     ]
@@ -220,6 +226,7 @@ The `containerEdits` field has the following definition:
     * `args` (array of strings, OPTIONAL) with the same semantics as IEEE Std 1003.1-2008 execv's argv.
     * `env` (array of strings, OPTIONAL) with the same semantics as IEEE Std 1003.1-2008's environ.
     * `timeout` (int, OPTIONAL) is the number of seconds before aborting the hook. If set, timeout MUST be greater than zero. If not set container runtime will wait for the hook to return.
+  * `additionalGids` (array of uint32s, OPTIONAL) A list of additional group IDs to add with the container process. These values are added to the `user.additionalGids` field in the OCI runtime specification. Values of 0 are ignored.
 
 ## Error Handling
   * Kind requested is not present in any CDI file.

pkg/cdi/container-edits.go

@@ -144,6 +144,13 @@ func (e *ContainerEdits) Apply(spec *oci.Spec) error {
 		}
 	}
 
+	for _, additionalGID := range e.AdditionalGIDs {
+		if additionalGID == 0 {
+			continue
+		}
+		specgen.AddProcessAdditionalGid(additionalGID)
+	}
+
 	return nil
 }
 
@@ -192,6 +199,7 @@ func (e *ContainerEdits) Append(o *ContainerEdits) *ContainerEdits {
 	e.DeviceNodes = append(e.DeviceNodes, o.DeviceNodes...)
 	e.Hooks = append(e.Hooks, o.Hooks...)
 	e.Mounts = append(e.Mounts, o.Mounts...)
+	e.AdditionalGIDs = append(e.AdditionalGIDs, o.AdditionalGIDs...)
 
 	return e
 }
@@ -202,7 +210,7 @@ func (e *ContainerEdits) isEmpty() bool {
 	if e == nil {
 		return false
 	}
-	return len(e.Env)+len(e.DeviceNodes)+len(e.Hooks)+len(e.Mounts) == 0
+	return len(e.Env)+len(e.DeviceNodes)+len(e.Hooks)+len(e.Mounts)+len(e.AdditionalGIDs) == 0
 }
 
 // ValidateEnv validates the given environment variables.

pkg/cdi/container-edits_test.go

@@ -467,6 +467,42 @@ func TestApplyContainerEdits(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "additional GIDs are applied",
+			spec: &oci.Spec{},
+			edits: &cdi.ContainerEdits{
+				AdditionalGIDs: []uint32{4, 5, 6},
+			},
+			result: &oci.Spec{
+				Process: &oci.Process{
+					User: oci.User{
+						AdditionalGids: []uint32{4, 5, 6},
+					},
+				},
+			},
+		},
+		{
+			name: "duplicate GIDs are ignored",
+			spec: &oci.Spec{},
+			edits: &cdi.ContainerEdits{
+				AdditionalGIDs: []uint32{4, 5, 6, 5, 6, 4},
+			},
+			result: &oci.Spec{
+				Process: &oci.Process{
+					User: oci.User{
+						AdditionalGids: []uint32{4, 5, 6},
+					},
+				},
+			},
+		},
+		{
+			name: "additional GID 0 is skipped",
+			spec: &oci.Spec{},
+			edits: &cdi.ContainerEdits{
+				AdditionalGIDs: []uint32{0},
+			},
+			result: &oci.Spec{},
+		},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
 			edits := ContainerEdits{tc.edits}
@@ -612,6 +648,36 @@ func TestAppend(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "merge additional GIDs does not deduplicate",
+			dst: &ContainerEdits{
+				ContainerEdits: &cdi.ContainerEdits{
+					AdditionalGIDs: []uint32{5},
+				},
+			},
+			src: []*ContainerEdits{
+				{
+					ContainerEdits: &cdi.ContainerEdits{
+						AdditionalGIDs: []uint32{0},
+					},
+				},
+				{
+					ContainerEdits: &cdi.ContainerEdits{
+						AdditionalGIDs: []uint32{5},
+					},
+				},
+				{
+					ContainerEdits: &cdi.ContainerEdits{
+						AdditionalGIDs: []uint32{6, 7, 6},
+					},
+				},
+			},
+			result: &ContainerEdits{
+				ContainerEdits: &cdi.ContainerEdits{
+					AdditionalGIDs: []uint32{5, 0, 5, 6, 7, 6},
+				},
+			},
+		},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
 			dst := tc.dst

pkg/cdi/version.go

@@ -39,6 +39,7 @@ const (
 	v040 version = "v0.4.0"
 	v050 version = "v0.5.0"
 	v060 version = "v0.6.0"
+	v070 version = "v0.7.0"
 
 	// vEarliest is the earliest supported version of the CDI specification
 	vEarliest version = v030
@@ -54,6 +55,7 @@ var validSpecVersions = requiredVersionMap{
 	v040: requiresV040,
 	v050: requiresV050,
 	v060: requiresV060,
+	v070: requiresV070,
 }
 
 // MinimumRequiredVersion determines the minimum spec version for the input spec.
@@ -118,6 +120,23 @@ func (r requiredVersionMap) requiredVersion(spec *cdi.Spec) version {
 	return minVersion
 }
 
+// requiresV070 returns true if the spec uses v0.7.0 features
+func requiresV070(spec *cdi.Spec) bool {
+	// The v0.7.0 spec allows additional GIDs to be specified at a spec level.
+	if len(spec.ContainerEdits.AdditionalGIDs) > 0 {
+		return true
+	}
+
+	for _, d := range spec.Devices {
+		// The v0.7.0 spec allows additional GIDs to be specified at a device level.
+		if len(d.ContainerEdits.AdditionalGIDs) > 0 {
+			return true
+		}
+	}
+
+	return false
+}
+
 // requiresV060 returns true if the spec uses v0.6.0 features
 func requiresV060(spec *cdi.Spec) bool {
 	// The v0.6.0 spec allows annotations to be specified at a spec level

schema/defs.json

@@ -134,6 +134,12 @@
                     "items": {
                         "$ref": "#/definitions/Hook"
                     }
+                },
+                "additionalGids": {
+                    "type": "array",
+                    "items": {
+                        "$ref": "#/definitions/uint32"
+                    }
                 }
             }
         },

specs-go/config.go

@@ -3,7 +3,7 @@ package specs
 import "os"
 
 // CurrentVersion is the current version of the Spec.
-const CurrentVersion = "0.6.0"
+const CurrentVersion = "0.7.0"
 
 // Spec is the base configuration for CDI
 type Spec struct {
@@ -25,10 +25,11 @@ type Device struct {
 
 // ContainerEdits are edits a container runtime must make to the OCI spec to expose the device.
 type ContainerEdits struct {
-	Env         []string      `json:"env,omitempty"`
-	DeviceNodes []*DeviceNode `json:"deviceNodes,omitempty"`
-	Hooks       []*Hook       `json:"hooks,omitempty"`
-	Mounts      []*Mount      `json:"mounts,omitempty"`
+	Env            []string      `json:"env,omitempty"`
+	DeviceNodes    []*DeviceNode `json:"deviceNodes,omitempty"`
+	Hooks          []*Hook       `json:"hooks,omitempty"`
+	Mounts         []*Mount      `json:"mounts,omitempty"`
+	AdditionalGIDs []uint32      `json:"additionalGids,omitempty"`
 }
 
 // DeviceNode represents a device node that needs to be added to the OCI spec.