cri/server: use containerd/oci instead of libcontainer/devices

Looks like we had our own copy of the "getDevices" code already, so use
that code (which also matches the code that's used to _generate_ the spec,
so a better match).

Moving the code to a separate file, I also noticed that the _unix and _linux
code was _exactly_ the same (baring some `//nolint:` comments), so also
removing the duplicated code.

With this patch applied, we removed the dependency on the libcontainer/devices
package (leaving only libcontainer/user).

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

oci/spec_opts_linux.go

@@ -20,22 +20,17 @@ package oci
 
 import (
 	"context"
-	"io/ioutil"
-	"os"
-	"path/filepath"
 
 	"github.com/containerd/containerd/containers"
 	"github.com/containerd/containerd/pkg/cap"
 	specs "github.com/opencontainers/runtime-spec/specs-go"
-	"github.com/pkg/errors"
-	"golang.org/x/sys/unix"
 )
 
 // WithHostDevices adds all the hosts device nodes to the container's spec
 func WithHostDevices(_ context.Context, _ Client, _ *containers.Container, s *Spec) error {
 	setLinux(s)
 
-	devs, err := getDevices("/dev", "")
+	devs, err := HostDevices()
 	if err != nil {
 		return err
 	}
@@ -43,8 +38,6 @@ func WithHostDevices(_ context.Context, _ Client, _ *containers.Container, s *Sp
 	return nil
 }
 
-var errNotADevice = errors.New("not a device node")
-
 // WithDevices recursively adds devices from the passed in path and associated cgroup rules for that device.
 // If devicePath is a dir it traverses the dir to add all devices in that dir.
 // If devicePath is not a dir, it attempts to add the single device.
@@ -69,108 +62,6 @@ func WithDevices(devicePath, containerPath, permissions string) SpecOpts {
 	}
 }
 
-func getDevices(path, containerPath string) ([]specs.LinuxDevice, error) {
-	stat, err := os.Stat(path)
-	if err != nil {
-		return nil, errors.Wrap(err, "error stating device path")
-	}
-
-	if !stat.IsDir() {
-		dev, err := deviceFromPath(path)
-		if err != nil {
-			return nil, err
-		}
-		if containerPath != "" {
-			dev.Path = containerPath
-		}
-		return []specs.LinuxDevice{*dev}, nil
-	}
-
-	files, err := ioutil.ReadDir(path)
-	if err != nil {
-		return nil, err
-	}
-	var out []specs.LinuxDevice
-	for _, f := range files {
-		switch {
-		case f.IsDir():
-			switch f.Name() {
-			// ".lxc" & ".lxd-mounts" added to address https://github.com/lxc/lxd/issues/2825
-			// ".udev" added to address https://github.com/opencontainers/runc/issues/2093
-			case "pts", "shm", "fd", "mqueue", ".lxc", ".lxd-mounts", ".udev":
-				continue
-			default:
-				var cp string
-				if containerPath != "" {
-					cp = filepath.Join(containerPath, filepath.Base(f.Name()))
-				}
-				sub, err := getDevices(filepath.Join(path, f.Name()), cp)
-				if err != nil {
-					return nil, err
-				}
-
-				out = append(out, sub...)
-				continue
-			}
-		case f.Name() == "console":
-			continue
-		}
-		device, err := deviceFromPath(filepath.Join(path, f.Name()))
-		if err != nil {
-			if err == errNotADevice {
-				continue
-			}
-			if os.IsNotExist(err) {
-				continue
-			}
-			return nil, err
-		}
-		if containerPath != "" {
-			device.Path = filepath.Join(containerPath, filepath.Base(f.Name()))
-		}
-		out = append(out, *device)
-	}
-	return out, nil
-}
-
-func deviceFromPath(path string) (*specs.LinuxDevice, error) {
-	var stat unix.Stat_t
-	if err := unix.Lstat(path, &stat); err != nil {
-		return nil, err
-	}
-
-	var (
-		// The type is 32bit on mips.
-		devNumber = uint64(stat.Rdev) // nolint: unconvert
-		major     = unix.Major(devNumber)
-		minor     = unix.Minor(devNumber)
-	)
-	if major == 0 {
-		return nil, errNotADevice
-	}
-
-	var (
-		devType string
-		mode    = stat.Mode
-	)
-	switch {
-	case mode&unix.S_IFBLK == unix.S_IFBLK:
-		devType = "b"
-	case mode&unix.S_IFCHR == unix.S_IFCHR:
-		devType = "c"
-	}
-	fm := os.FileMode(mode &^ unix.S_IFMT)
-	return &specs.LinuxDevice{
-		Type:     devType,
-		Path:     path,
-		Major:    int64(major),
-		Minor:    int64(minor),
-		FileMode: &fm,
-		UID:      &stat.Uid,
-		GID:      &stat.Gid,
-	}, nil
-}
-
 // WithMemorySwap sets the container's swap in bytes
 func WithMemorySwap(swap int64) SpecOpts {
 	return func(ctx context.Context, _ Client, c *containers.Container, s *Spec) error {

oci/spec_opts_unix.go

@@ -20,21 +20,15 @@ package oci
 
 import (
 	"context"
-	"io/ioutil"
-	"os"
-	"path/filepath"
 
 	"github.com/containerd/containerd/containers"
-	specs "github.com/opencontainers/runtime-spec/specs-go"
-	"github.com/pkg/errors"
-	"golang.org/x/sys/unix"
 )
 
 // WithHostDevices adds all the hosts device nodes to the container's spec
 func WithHostDevices(_ context.Context, _ Client, _ *containers.Container, s *Spec) error {
 	setLinux(s)
 
-	devs, err := getDevices("/dev", "")
+	devs, err := HostDevices()
 	if err != nil {
 		return err
 	}
@@ -42,11 +36,9 @@ func WithHostDevices(_ context.Context, _ Client, _ *containers.Container, s *Sp
 	return nil
 }
 
-var errNotADevice = errors.New("not a device node")
-
 // WithDevices recursively adds devices from the passed in path and associated cgroup rules for that device.
 // If devicePath is a dir it traverses the dir to add all devices in that dir.
-// If devicePath is not a dir, it attempts to add the signle device.
+// If devicePath is not a dir, it attempts to add the single device.
 func WithDevices(devicePath, containerPath, permissions string) SpecOpts {
 	return func(_ context.Context, _ Client, _ *containers.Container, s *Spec) error {
 		devs, err := getDevices(devicePath, containerPath)
@@ -58,107 +50,6 @@ func WithDevices(devicePath, containerPath, permissions string) SpecOpts {
 	}
 }
 
-func getDevices(path, containerPath string) ([]specs.LinuxDevice, error) {
-	stat, err := os.Stat(path)
-	if err != nil {
-		return nil, errors.Wrap(err, "error stating device path")
-	}
-
-	if !stat.IsDir() {
-		dev, err := deviceFromPath(path)
-		if err != nil {
-			return nil, err
-		}
-		if containerPath != "" {
-			dev.Path = containerPath
-		}
-		return []specs.LinuxDevice{*dev}, nil
-	}
-
-	files, err := ioutil.ReadDir(path)
-	if err != nil {
-		return nil, err
-	}
-	var out []specs.LinuxDevice
-	for _, f := range files {
-		switch {
-		case f.IsDir():
-			switch f.Name() {
-			// ".lxc" & ".lxd-mounts" added to address https://github.com/lxc/lxd/issues/2825
-			// ".udev" added to address https://github.com/opencontainers/runc/issues/2093
-			case "pts", "shm", "fd", "mqueue", ".lxc", ".lxd-mounts", ".udev":
-				continue
-			default:
-				var cp string
-				if containerPath != "" {
-					cp = filepath.Join(containerPath, filepath.Base(f.Name()))
-				}
-				sub, err := getDevices(filepath.Join(path, f.Name()), cp)
-				if err != nil {
-					return nil, err
-				}
-
-				out = append(out, sub...)
-				continue
-			}
-		case f.Name() == "console":
-			continue
-		}
-		device, err := deviceFromPath(filepath.Join(path, f.Name()))
-		if err != nil {
-			if err == errNotADevice {
-				continue
-			}
-			if os.IsNotExist(err) {
-				continue
-			}
-			return nil, err
-		}
-		if containerPath != "" {
-			device.Path = filepath.Join(containerPath, filepath.Base(f.Name()))
-		}
-		out = append(out, *device)
-	}
-	return out, nil
-}
-
-func deviceFromPath(path string) (*specs.LinuxDevice, error) {
-	var stat unix.Stat_t
-	if err := unix.Lstat(path, &stat); err != nil {
-		return nil, err
-	}
-
-	var (
-		devNumber = uint64(stat.Rdev)
-		major     = unix.Major(devNumber)
-		minor     = unix.Minor(devNumber)
-	)
-	if major == 0 {
-		return nil, errNotADevice
-	}
-
-	var (
-		devType string
-		mode    = stat.Mode
-	)
-	switch {
-	case mode&unix.S_IFBLK == unix.S_IFBLK:
-		devType = "b"
-	case mode&unix.S_IFCHR == unix.S_IFCHR:
-		devType = "c"
-	}
-	fm := os.FileMode(mode &^ unix.S_IFMT)
-	return &specs.LinuxDevice{
-		Type:     devType,
-		Path:     path,
-		Major:    int64(major),
-		Minor:    int64(minor),
-		FileMode: &fm,
-		UID:      &stat.Uid,
-		GID:      &stat.Gid,
-	}, nil
-}
-
 // WithCPUCFS sets the container's Completely fair scheduling (CFS) quota and period
 func WithCPUCFS(quota int64, period uint64) SpecOpts {
 	return func(ctx context.Context, _ Client, c *containers.Container, s *Spec) error {

oci/utils_unix.go

@@ -0,0 +1,137 @@
+// +build !windows
+
+/*
+   Copyright The containerd Authors.
+
+   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 oci
+
+import (
+	"io/ioutil"
+	"os"
+	"path/filepath"
+
+	specs "github.com/opencontainers/runtime-spec/specs-go"
+	"github.com/pkg/errors"
+	"golang.org/x/sys/unix"
+)
+
+var errNotADevice = errors.New("not a device node")
+
+// HostDevices returns all devices that can be found under /dev directory.
+func HostDevices() ([]specs.LinuxDevice, error) {
+	return getDevices("/dev", "")
+}
+
+func getDevices(path, containerPath string) ([]specs.LinuxDevice, error) {
+	stat, err := os.Stat(path)
+	if err != nil {
+		return nil, errors.Wrap(err, "error stating device path")
+	}
+
+	if !stat.IsDir() {
+		dev, err := deviceFromPath(path)
+		if err != nil {
+			return nil, err
+		}
+		if containerPath != "" {
+			dev.Path = containerPath
+		}
+		return []specs.LinuxDevice{*dev}, nil
+	}
+
+	files, err := ioutil.ReadDir(path)
+	if err != nil {
+		return nil, err
+	}
+	var out []specs.LinuxDevice
+	for _, f := range files {
+		switch {
+		case f.IsDir():
+			switch f.Name() {
+			// ".lxc" & ".lxd-mounts" added to address https://github.com/lxc/lxd/issues/2825
+			// ".udev" added to address https://github.com/opencontainers/runc/issues/2093
+			case "pts", "shm", "fd", "mqueue", ".lxc", ".lxd-mounts", ".udev":
+				continue
+			default:
+				var cp string
+				if containerPath != "" {
+					cp = filepath.Join(containerPath, filepath.Base(f.Name()))
+				}
+				sub, err := getDevices(filepath.Join(path, f.Name()), cp)
+				if err != nil {
+					return nil, err
+				}
+
+				out = append(out, sub...)
+				continue
+			}
+		case f.Name() == "console":
+			continue
+		}
+		device, err := deviceFromPath(filepath.Join(path, f.Name()))
+		if err != nil {
+			if err == errNotADevice {
+				continue
+			}
+			if os.IsNotExist(err) {
+				continue
+			}
+			return nil, err
+		}
+		if containerPath != "" {
+			device.Path = filepath.Join(containerPath, filepath.Base(f.Name()))
+		}
+		out = append(out, *device)
+	}
+	return out, nil
+}
+
+func deviceFromPath(path string) (*specs.LinuxDevice, error) {
+	var stat unix.Stat_t
+	if err := unix.Lstat(path, &stat); err != nil {
+		return nil, err
+	}
+
+	var (
+		devNumber = uint64(stat.Rdev) //nolint: unconvert // the type is 32bit on mips.
+		major     = unix.Major(devNumber)
+		minor     = unix.Minor(devNumber)
+	)
+	if major == 0 {
+		return nil, errNotADevice
+	}
+
+	var (
+		devType string
+		mode    = stat.Mode
+	)
+	switch {
+	case mode&unix.S_IFBLK == unix.S_IFBLK:
+		devType = "b"
+	case mode&unix.S_IFCHR == unix.S_IFCHR:
+		devType = "c"
+	}
+	fm := os.FileMode(mode &^ unix.S_IFMT)
+	return &specs.LinuxDevice{
+		Type:     devType,
+		Path:     path,
+		Major:    int64(major),
+		Minor:    int64(minor),
+		FileMode: &fm,
+		UID:      &stat.Uid,
+		GID:      &stat.Gid,
+	}, nil
+}

pkg/cri/server/container_create_linux_test.go

@@ -31,7 +31,6 @@ import (
 	"github.com/containerd/containerd/mount"
 	"github.com/containerd/containerd/oci"
 	imagespec "github.com/opencontainers/image-spec/specs-go/v1"
-	"github.com/opencontainers/runc/libcontainer/devices"
 	runtimespec "github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/opencontainers/selinux/go-selinux"
 	"github.com/pkg/errors"
@@ -1349,12 +1348,12 @@ func TestPrivilegedDevices(t *testing.T) {
 		spec, err := c.containerSpec(t.Name(), testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, ociRuntime)
 		assert.NoError(t, err)
 
-		hostDevicesRaw, err := devices.HostDevices()
+		hostDevicesRaw, err := oci.HostDevices()
 		assert.NoError(t, err)
 		var hostDevices = make([]string, 0)
 		for _, dev := range hostDevicesRaw {
 			// https://github.com/containerd/cri/pull/1521#issuecomment-652807951
-			if dev.Rule.Major != 0 {
+			if dev.Major != 0 {
 				hostDevices = append(hostDevices, dev.Path)
 			}
 		}

vendor/github.com/opencontainers/runc/libcontainer/devices/device.go

@@ -1,170 +0,0 @@
-package devices
-
-import (
-	"fmt"
-	"os"
-	"strconv"
-)
-
-const (
-	Wildcard = -1
-)
-
-type Device struct {
-	Rule
-
-	// Path to the device.
-	Path string `json:"path"`
-
-	// FileMode permission bits for the device.
-	FileMode os.FileMode `json:"file_mode"`
-
-	// Uid of the device.
-	Uid uint32 `json:"uid"`
-
-	// Gid of the device.
-	Gid uint32 `json:"gid"`
-}
-
-// Permissions is a cgroupv1-style string to represent device access. It
-// has to be a string for backward compatibility reasons, hence why it has
-// methods to do set operations.
-type Permissions string
-
-const (
-	deviceRead uint = (1 << iota)
-	deviceWrite
-	deviceMknod
-)
-
-func (p Permissions) toSet() uint {
-	var set uint
-	for _, perm := range p {
-		switch perm {
-		case 'r':
-			set |= deviceRead
-		case 'w':
-			set |= deviceWrite
-		case 'm':
-			set |= deviceMknod
-		}
-	}
-	return set
-}
-
-func fromSet(set uint) Permissions {
-	var perm string
-	if set&deviceRead == deviceRead {
-		perm += "r"
-	}
-	if set&deviceWrite == deviceWrite {
-		perm += "w"
-	}
-	if set&deviceMknod == deviceMknod {
-		perm += "m"
-	}
-	return Permissions(perm)
-}
-
-// Union returns the union of the two sets of Permissions.
-func (p Permissions) Union(o Permissions) Permissions {
-	lhs := p.toSet()
-	rhs := o.toSet()
-	return fromSet(lhs | rhs)
-}
-
-// Difference returns the set difference of the two sets of Permissions.
-// In set notation, A.Difference(B) gives you A\B.
-func (p Permissions) Difference(o Permissions) Permissions {
-	lhs := p.toSet()
-	rhs := o.toSet()
-	return fromSet(lhs &^ rhs)
-}
-
-// Intersection computes the intersection of the two sets of Permissions.
-func (p Permissions) Intersection(o Permissions) Permissions {
-	lhs := p.toSet()
-	rhs := o.toSet()
-	return fromSet(lhs & rhs)
-}
-
-// IsEmpty returns whether the set of permissions in a Permissions is
-// empty.
-func (p Permissions) IsEmpty() bool {
-	return p == Permissions("")
-}
-
-// IsValid returns whether the set of permissions is a subset of valid
-// permissions (namely, {r,w,m}).
-func (p Permissions) IsValid() bool {
-	return p == fromSet(p.toSet())
-}
-
-type Type rune
-
-const (
-	WildcardDevice Type = 'a'
-	BlockDevice    Type = 'b'
-	CharDevice     Type = 'c' // or 'u'
-	FifoDevice     Type = 'p'
-)
-
-func (t Type) IsValid() bool {
-	switch t {
-	case WildcardDevice, BlockDevice, CharDevice, FifoDevice:
-		return true
-	default:
-		return false
-	}
-}
-
-func (t Type) CanMknod() bool {
-	switch t {
-	case BlockDevice, CharDevice, FifoDevice:
-		return true
-	default:
-		return false
-	}
-}
-
-func (t Type) CanCgroup() bool {
-	switch t {
-	case WildcardDevice, BlockDevice, CharDevice:
-		return true
-	default:
-		return false
-	}
-}
-
-type Rule struct {
-	// Type of device ('c' for char, 'b' for block). If set to 'a', this rule
-	// acts as a wildcard and all fields other than Allow are ignored.
-	Type Type `json:"type"`
-
-	// Major is the device's major number.
-	Major int64 `json:"major"`
-
-	// Minor is the device's minor number.
-	Minor int64 `json:"minor"`
-
-	// Permissions is the set of permissions that this rule applies to (in the
-	// cgroupv1 format -- any combination of "rwm").
-	Permissions Permissions `json:"permissions"`
-
-	// Allow specifies whether this rule is allowed.
-	Allow bool `json:"allow"`
-}
-
-func (d *Rule) CgroupString() string {
-	var (
-		major = strconv.FormatInt(d.Major, 10)
-		minor = strconv.FormatInt(d.Minor, 10)
-	)
-	if d.Major == Wildcard {
-		major = "*"
-	}
-	if d.Minor == Wildcard {
-		minor = "*"
-	}
-	return fmt.Sprintf("%c %s:%s %s", d.Type, major, minor, d.Permissions)
-}

vendor/github.com/opencontainers/runc/libcontainer/devices/device_unix.go

@@ -1,16 +0,0 @@
-// +build !windows
-
-package devices
-
-import (
-	"errors"
-
-	"golang.org/x/sys/unix"
-)
-
-func (d *Rule) Mkdev() (uint64, error) {
-	if d.Major == Wildcard || d.Minor == Wildcard {
-		return 0, errors.New("cannot mkdev() device with wildcards")
-	}
-	return unix.Mkdev(uint32(d.Major), uint32(d.Minor)), nil
-}

vendor/github.com/opencontainers/runc/libcontainer/devices/device_windows.go

@@ -1,5 +0,0 @@
-package devices
-
-func (d *Rule) Mkdev() (uint64, error) {
-	return 0, nil
-}

vendor/github.com/opencontainers/runc/libcontainer/devices/devices.go

@@ -1,112 +0,0 @@
-package devices
-
-import (
-	"errors"
-	"io/ioutil"
-	"os"
-	"path/filepath"
-
-	"golang.org/x/sys/unix"
-)
-
-var (
-	// ErrNotADevice denotes that a file is not a valid linux device.
-	ErrNotADevice = errors.New("not a device node")
-)
-
-// Testing dependencies
-var (
-	unixLstat     = unix.Lstat
-	ioutilReadDir = ioutil.ReadDir
-)
-
-// Given the path to a device and its cgroup_permissions(which cannot be easily queried) look up the
-// information about a linux device and return that information as a Device struct.
-func DeviceFromPath(path, permissions string) (*Device, error) {
-	var stat unix.Stat_t
-	err := unixLstat(path, &stat)
-	if err != nil {
-		return nil, err
-	}
-
-	var (
-		devType   Type
-		mode      = stat.Mode
-		devNumber = uint64(stat.Rdev)
-		major     = unix.Major(devNumber)
-		minor     = unix.Minor(devNumber)
-	)
-	switch mode & unix.S_IFMT {
-	case unix.S_IFBLK:
-		devType = BlockDevice
-	case unix.S_IFCHR:
-		devType = CharDevice
-	case unix.S_IFIFO:
-		devType = FifoDevice
-	default:
-		return nil, ErrNotADevice
-	}
-	return &Device{
-		Rule: Rule{
-			Type:        devType,
-			Major:       int64(major),
-			Minor:       int64(minor),
-			Permissions: Permissions(permissions),
-		},
-		Path:     path,
-		FileMode: os.FileMode(mode),
-		Uid:      stat.Uid,
-		Gid:      stat.Gid,
-	}, nil
-}
-
-// HostDevices returns all devices that can be found under /dev directory.
-func HostDevices() ([]*Device, error) {
-	return GetDevices("/dev")
-}
-
-// GetDevices recursively traverses a directory specified by path
-// and returns all devices found there.
-func GetDevices(path string) ([]*Device, error) {
-	files, err := ioutilReadDir(path)
-	if err != nil {
-		return nil, err
-	}
-	var out []*Device
-	for _, f := range files {
-		switch {
-		case f.IsDir():
-			switch f.Name() {
-			// ".lxc" & ".lxd-mounts" added to address https://github.com/lxc/lxd/issues/2825
-			// ".udev" added to address https://github.com/opencontainers/runc/issues/2093
-			case "pts", "shm", "fd", "mqueue", ".lxc", ".lxd-mounts", ".udev":
-				continue
-			default:
-				sub, err := GetDevices(filepath.Join(path, f.Name()))
-				if err != nil {
-					return nil, err
-				}
-
-				out = append(out, sub...)
-				continue
-			}
-		case f.Name() == "console":
-			continue
-		}
-		device, err := DeviceFromPath(filepath.Join(path, f.Name()), "rwm")
-		if err != nil {
-			if err == ErrNotADevice {
-				continue
-			}
-			if os.IsNotExist(err) {
-				continue
-			}
-			return nil, err
-		}
-		if device.Type == FifoDevice {
-			continue
-		}
-		out = append(out, device)
-	}
-	return out, nil
-}

vendor/modules.txt

@@ -265,7 +265,6 @@ github.com/opencontainers/image-spec/specs-go
 github.com/opencontainers/image-spec/specs-go/v1
 # github.com/opencontainers/runc v1.0.0-rc93
 ## explicit
-github.com/opencontainers/runc/libcontainer/devices
 github.com/opencontainers/runc/libcontainer/user
 # github.com/opencontainers/runtime-spec v1.0.3-0.20200929063507-e6143ca7d51d
 ## explicit