Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions SPEC.md
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ The keywords "must", "must not", "required", "shall", "shall not", "should", "sh
// * r - allows container to read from the specified device.
// * w - allows container to write to the specified device.
// * m - allows container to create device files that do not yet exist.
// Omitted or empty permissions default to 'rwm'. 'none' requests empty permissions.
"permissions": "<permissions>" (optional),
"uid": <int> (optional),
"gid": <int> (optional)
Expand Down
20 changes: 14 additions & 6 deletions pkg/cdi/container-edits.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ const (
PoststartHook = "poststart"
// PoststopHook is the name of the OCI "poststop" hook.
PoststopHook = "poststop"

// NoPermissions requests empty cgroup permissions for a device.
NoPermissions = "none"
)

var (
Expand Down Expand Up @@ -106,8 +109,11 @@ func (e *ContainerEdits) Apply(spec *oci.Spec) error {

if dev.Type == "b" || dev.Type == "c" {
access := d.Permissions
if access == "" {
switch access {
case "":
access = "rwm"
case NoPermissions:
access = ""
}
specgen.AddLinuxResourcesDevice(true, dev.Type, &dev.Major, &dev.Minor, access)
}
Expand Down Expand Up @@ -361,12 +367,14 @@ func (d *DeviceNode) Validate() error {
if _, ok := validTypes[d.Type]; !ok {
return fmt.Errorf("device %q: invalid type %q", d.Path, d.Type)
}
for _, bit := range d.Permissions {
if bit != 'r' && bit != 'w' && bit != 'm' {
return fmt.Errorf("device %q: invalid permissions %q",
d.Path, d.Permissions)
}
switch {
case d.Permissions == "":
case d.Permissions == NoPermissions:
case strings.Trim(d.Permissions, "rwm") != "":
return fmt.Errorf("device %q: invalid permissions %q",
d.Path, d.Permissions)
Comment on lines +373 to +375
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor nit: If we invert the logic here, we can have the switch cover all the valid cases and then return an error in the default case. Not a blocker though. Merging for now and we can always revisit.

}

return nil
}

Expand Down
88 changes: 88 additions & 0 deletions pkg/cdi/container-edits_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,18 @@ func TestValidateContainerEdits(t *testing.T) {
},
invalid: true,
},
{
name: "valid device, with NoPermissions",
edits: &cdi.ContainerEdits{
DeviceNodes: []*cdi.DeviceNode{
{
Path: "/dev/vendorctl",
Type: "b",
Permissions: NoPermissions,
},
},
},
},
{
name: "valid mount",
edits: &cdi.ContainerEdits{
Expand Down Expand Up @@ -420,6 +432,82 @@ func TestApplyContainerEdits(t *testing.T) {
},
},
},
{
name: "empty spec, device with explicitly empty permissions",
spec: &oci.Spec{},
edits: &cdi.ContainerEdits{
DeviceNodes: []*cdi.DeviceNode{
{
Path: "/dev/nil",
Type: "c",
Major: 1,
Minor: 3,
Permissions: NoPermissions,
},
},
},
result: &oci.Spec{
Linux: &oci.Linux{
Devices: []oci.LinuxDevice{
{
Path: "/dev/nil",
Type: "c",
Major: nullDeviceMajor,
Minor: nullDeviceMinor,
},
},
Resources: &oci.LinuxResources{
Devices: []oci.LinuxDeviceCgroup{
{
Allow: true,
Type: "c",
Major: &nullDeviceMajor,
Minor: &nullDeviceMinor,
Access: "",
},
},
},
},
},
},
{
name: "empty spec, device with omitted permissions",
spec: &oci.Spec{},
edits: &cdi.ContainerEdits{
DeviceNodes: []*cdi.DeviceNode{
{
Path: "/dev/nil",
Type: "c",
Major: 1,
Minor: 3,
Permissions: "",
},
},
},
result: &oci.Spec{
Linux: &oci.Linux{
Devices: []oci.LinuxDevice{
{
Path: "/dev/nil",
Type: "c",
Major: nullDeviceMajor,
Minor: nullDeviceMinor,
},
},
Resources: &oci.LinuxResources{
Devices: []oci.LinuxDeviceCgroup{
{
Allow: true,
Type: "c",
Major: &nullDeviceMajor,
Minor: &nullDeviceMinor,
Access: "rwm",
},
},
},
},
},
},
{
name: "empty spec, device, env var",
spec: &oci.Spec{},
Expand Down