From 9bc8d63c9fe788dc4dbd7fce61844df865b9c3d3 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 8 Apr 2021 22:51:13 +0200 Subject: [PATCH] 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 --- oci/spec_opts_linux.go | 111 +----------- oci/spec_opts_unix.go | 113 +----------- oci/utils_unix.go | 137 ++++++++++++++ pkg/cri/server/container_create_linux_test.go | 5 +- .../runc/libcontainer/devices/device.go | 170 ------------------ .../runc/libcontainer/devices/device_unix.go | 16 -- .../libcontainer/devices/device_windows.go | 5 - .../runc/libcontainer/devices/devices.go | 112 ------------ vendor/modules.txt | 1 - 9 files changed, 142 insertions(+), 528 deletions(-) create mode 100644 oci/utils_unix.go delete mode 100644 vendor/github.com/opencontainers/runc/libcontainer/devices/device.go delete mode 100644 vendor/github.com/opencontainers/runc/libcontainer/devices/device_unix.go delete mode 100644 vendor/github.com/opencontainers/runc/libcontainer/devices/device_windows.go delete mode 100644 vendor/github.com/opencontainers/runc/libcontainer/devices/devices.go diff --git a/oci/spec_opts_linux.go b/oci/spec_opts_linux.go index b20930d0e..9c5fd6c84 100644 --- a/oci/spec_opts_linux.go +++ b/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 { diff --git a/oci/spec_opts_unix.go b/oci/spec_opts_unix.go index acf13e1f7..80a522356 100644 --- a/oci/spec_opts_unix.go +++ b/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 { diff --git a/oci/utils_unix.go b/oci/utils_unix.go new file mode 100644 index 000000000..108cacf5b --- /dev/null +++ b/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 +} diff --git a/pkg/cri/server/container_create_linux_test.go b/pkg/cri/server/container_create_linux_test.go index 58d5c8d64..18d368344 100644 --- a/pkg/cri/server/container_create_linux_test.go +++ b/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) } } diff --git a/vendor/github.com/opencontainers/runc/libcontainer/devices/device.go b/vendor/github.com/opencontainers/runc/libcontainer/devices/device.go deleted file mode 100644 index 3eb73cc7c..000000000 --- a/vendor/github.com/opencontainers/runc/libcontainer/devices/device.go +++ /dev/null @@ -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) -} diff --git a/vendor/github.com/opencontainers/runc/libcontainer/devices/device_unix.go b/vendor/github.com/opencontainers/runc/libcontainer/devices/device_unix.go deleted file mode 100644 index a400341e4..000000000 --- a/vendor/github.com/opencontainers/runc/libcontainer/devices/device_unix.go +++ /dev/null @@ -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 -} diff --git a/vendor/github.com/opencontainers/runc/libcontainer/devices/device_windows.go b/vendor/github.com/opencontainers/runc/libcontainer/devices/device_windows.go deleted file mode 100644 index 8511bf00e..000000000 --- a/vendor/github.com/opencontainers/runc/libcontainer/devices/device_windows.go +++ /dev/null @@ -1,5 +0,0 @@ -package devices - -func (d *Rule) Mkdev() (uint64, error) { - return 0, nil -} diff --git a/vendor/github.com/opencontainers/runc/libcontainer/devices/devices.go b/vendor/github.com/opencontainers/runc/libcontainer/devices/devices.go deleted file mode 100644 index 5011f373d..000000000 --- a/vendor/github.com/opencontainers/runc/libcontainer/devices/devices.go +++ /dev/null @@ -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 -} diff --git a/vendor/modules.txt b/vendor/modules.txt index bd01f33c8..b15bd535b 100644 --- a/vendor/modules.txt +++ b/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