diff --git a/.golangci.yml b/.golangci.yml index 684f98960ef..c5ed654c715 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -37,6 +37,12 @@ linters: # going to be tricked into overwriting host files. - pattern: ^os\.Create$ pkg: ^os$ + # os.Is* error checking functions predate errors.Is. Therefore, they + # only support errors returned by the os package and subtly fail + # to deal with other wrapped error types. + # New code should use errors.Is(err, error-type) instead. + - pattern: ^os\.Is(Exist|NotExist|Permission|Timeout)$ + pkg: ^os$ analyze-types: true exclusions: rules: diff --git a/libcontainer/configs/validate/validator.go b/libcontainer/configs/validate/validator.go index 9851e4057d3..d70a211ceea 100644 --- a/libcontainer/configs/validate/validator.go +++ b/libcontainer/configs/validate/validator.go @@ -142,7 +142,7 @@ func security(config *configs.Config) error { func namespaces(config *configs.Config) error { if config.Namespaces.Contains(configs.NEWUSER) { - if _, err := os.Stat("/proc/self/ns/user"); os.IsNotExist(err) { + if _, err := os.Stat("/proc/self/ns/user"); errors.Is(err, os.ErrNotExist) { return errors.New("user namespaces aren't enabled in the kernel") } hasPath := config.Namespaces.PathOf(configs.NEWUSER) != "" @@ -160,13 +160,13 @@ func namespaces(config *configs.Config) error { } if config.Namespaces.Contains(configs.NEWCGROUP) { - if _, err := os.Stat("/proc/self/ns/cgroup"); os.IsNotExist(err) { + if _, err := os.Stat("/proc/self/ns/cgroup"); errors.Is(err, os.ErrNotExist) { return errors.New("cgroup namespaces aren't enabled in the kernel") } } if config.Namespaces.Contains(configs.NEWTIME) { - if _, err := os.Stat("/proc/self/timens_offsets"); os.IsNotExist(err) { + if _, err := os.Stat("/proc/self/timens_offsets"); errors.Is(err, os.ErrNotExist) { return errors.New("time namespaces aren't enabled in the kernel") } hasPath := config.Namespaces.PathOf(configs.NEWTIME) != "" diff --git a/libcontainer/configs/validate/validator_test.go b/libcontainer/configs/validate/validator_test.go index 160679efef1..dee9e5e0614 100644 --- a/libcontainer/configs/validate/validator_test.go +++ b/libcontainer/configs/validate/validator_test.go @@ -1,6 +1,7 @@ package validate import ( + "errors" "os" "path/filepath" "strings" @@ -172,7 +173,7 @@ func TestValidateSecurityWithoutNEWNS(t *testing.T) { } func TestValidateUserNamespace(t *testing.T) { - if _, err := os.Stat("/proc/self/ns/user"); os.IsNotExist(err) { + if _, err := os.Stat("/proc/self/ns/user"); errors.Is(err, os.ErrNotExist) { t.Skip("Test requires userns.") } config := &configs.Config{ @@ -206,7 +207,7 @@ func TestValidateUsernsMappingWithoutNamespace(t *testing.T) { } func TestValidateTimeNamespace(t *testing.T) { - if _, err := os.Stat("/proc/self/ns/time"); os.IsNotExist(err) { + if _, err := os.Stat("/proc/self/ns/time"); errors.Is(err, os.ErrNotExist) { t.Skip("Test requires timens.") } config := &configs.Config{ @@ -225,7 +226,7 @@ func TestValidateTimeNamespace(t *testing.T) { } func TestValidateTimeNamespaceWithBothPathAndTimeOffset(t *testing.T) { - if _, err := os.Stat("/proc/self/ns/time"); os.IsNotExist(err) { + if _, err := os.Stat("/proc/self/ns/time"); errors.Is(err, os.ErrNotExist) { t.Skip("Test requires timens.") } config := &configs.Config{ @@ -1020,7 +1021,7 @@ func TestValidateNetDevices(t *testing.T) { } func TestValidateUserSysctlWithUserNamespace(t *testing.T) { - if _, err := os.Stat("/proc/self/ns/user"); os.IsNotExist(err) { + if _, err := os.Stat("/proc/self/ns/user"); errors.Is(err, os.ErrNotExist) { t.Skip("Test requires userns.") } config := &configs.Config{ diff --git a/libcontainer/container_linux.go b/libcontainer/container_linux.go index ad904641723..af9172a89a4 100644 --- a/libcontainer/container_linux.go +++ b/libcontainer/container_linux.go @@ -295,7 +295,7 @@ func handleFifoResult(result openResult) error { return err } err := os.Remove(f.Name()) - if err == nil || os.IsNotExist(err) { + if err == nil || errors.Is(err, os.ErrNotExist) { return nil } return err diff --git a/libcontainer/criu_linux.go b/libcontainer/criu_linux.go index 8aa7a1f2754..488095cdeea 100644 --- a/libcontainer/criu_linux.go +++ b/libcontainer/criu_linux.go @@ -125,7 +125,7 @@ func (c *Container) addMaskPaths(req *criurpc.CriuReq) error { for _, path := range c.config.MaskPaths { fi, err := os.Stat(fmt.Sprintf("/proc/%d/root/%s", c.initProcess.pid(), path)) if err != nil { - if os.IsNotExist(err) { + if errors.Is(err, os.ErrNotExist) { continue } return err @@ -318,7 +318,7 @@ func (c *Container) Checkpoint(criuOpts *CriuOpts) error { // Since a container can be C/R'ed multiple times, // the checkpoint directory may already exist. - if err := os.Mkdir(criuOpts.ImagesDirectory, 0o700); err != nil && !os.IsExist(err) { + if err := os.Mkdir(criuOpts.ImagesDirectory, 0o700); err != nil && !errors.Is(err, os.ErrExist) { return err } @@ -353,7 +353,7 @@ func (c *Container) Checkpoint(criuOpts *CriuOpts) error { // if criuOpts.WorkDirectory is not set, criu default is used. if criuOpts.WorkDirectory != "" { - if err := os.Mkdir(criuOpts.WorkDirectory, 0o700); err != nil && !os.IsExist(err) { + if err := os.Mkdir(criuOpts.WorkDirectory, 0o700); err != nil && !errors.Is(err, os.ErrExist) { return err } workDir, err := os.Open(criuOpts.WorkDirectory) @@ -718,7 +718,7 @@ func (c *Container) Restore(process *Process, criuOpts *CriuOpts) error { if criuOpts.WorkDirectory != "" { // Since a container can be C/R'ed multiple times, // the work directory may already exist. - if err := os.Mkdir(criuOpts.WorkDirectory, 0o700); err != nil && !os.IsExist(err) { + if err := os.Mkdir(criuOpts.WorkDirectory, 0o700); err != nil && !errors.Is(err, os.ErrExist) { return err } workDir, err := os.Open(criuOpts.WorkDirectory) @@ -1169,7 +1169,7 @@ func (c *Container) criuNotifications(resp *criurpc.CriuResp, process *Process, return err } if err := os.Remove(filepath.Join(c.stateDir, "checkpoint")); err != nil { - if !os.IsNotExist(err) { + if !errors.Is(err, os.ErrNotExist) { logrus.Error(err) } } diff --git a/libcontainer/devices/device_unix.go b/libcontainer/devices/device_unix.go index c533eb1c67a..409e58e963d 100644 --- a/libcontainer/devices/device_unix.go +++ b/libcontainer/devices/device_unix.go @@ -98,7 +98,7 @@ func GetDevices(path string) ([]*Device, error) { if errors.Is(err, ErrNotADevice) { continue } - if os.IsNotExist(err) { + if errors.Is(err, os.ErrNotExist) { continue } return nil, err diff --git a/libcontainer/factory_linux.go b/libcontainer/factory_linux.go index 70d935c0d38..f1c223e9816 100644 --- a/libcontainer/factory_linux.go +++ b/libcontainer/factory_linux.go @@ -51,7 +51,7 @@ func Create(root, id string, config *configs.Config) (*Container, error) { } if _, err := os.Stat(stateDir); err == nil { return nil, ErrExist - } else if !os.IsNotExist(err) { + } else if !errors.Is(err, os.ErrNotExist) { return nil, err } @@ -154,7 +154,7 @@ func loadState(root string) (*State, error) { } f, err := os.Open(stateFilePath) if err != nil { - if os.IsNotExist(err) { + if errors.Is(err, os.ErrNotExist) { return nil, ErrNotExist } return nil, err diff --git a/libcontainer/init_linux.go b/libcontainer/init_linux.go index 99b99debae6..632d7c3491b 100644 --- a/libcontainer/init_linux.go +++ b/libcontainer/init_linux.go @@ -314,7 +314,7 @@ func finalizeNamespace(config *initConfig) error { switch { case err == nil: doChdir = false - case os.IsPermission(err): + case errors.Is(err, os.ErrPermission): // If we hit an EPERM, we should attempt again after setting up user. // This will allow us to successfully chdir if the container user has access // to the directory, but the user running runc does not. @@ -480,7 +480,7 @@ func setupUser(config *initConfig) error { setgroups, err = io.ReadAll(setgroupsFile) _ = setgroupsFile.Close() } - if err != nil && !os.IsNotExist(err) { + if err != nil && !errors.Is(err, os.ErrNotExist) { return err } diff --git a/libcontainer/integration/exec_test.go b/libcontainer/integration/exec_test.go index 2333dc5470b..5cbcd2da41b 100644 --- a/libcontainer/integration/exec_test.go +++ b/libcontainer/integration/exec_test.go @@ -3,6 +3,7 @@ package integration import ( "bytes" "encoding/json" + "errors" "fmt" "os" "os/exec" @@ -1093,7 +1094,7 @@ func TestHook(t *testing.T) { for _, hook := range []string{"prestart", "createRuntime", "poststart"} { fi, err := os.Stat(filepath.Join(config.Rootfs, hook)) - if err == nil || !os.IsNotExist(err) { + if err == nil || !errors.Is(err, os.ErrNotExist) { t.Fatalf("expected file '%s to not exists, but it does", fi.Name()) } } @@ -1637,7 +1638,7 @@ func TestTmpfsCopyUp(t *testing.T) { } func TestCGROUPPrivate(t *testing.T) { - if _, err := os.Stat("/proc/self/ns/cgroup"); os.IsNotExist(err) { + if _, err := os.Stat("/proc/self/ns/cgroup"); errors.Is(err, os.ErrNotExist) { t.Skip("Test requires cgroupns.") } if testing.Short() { @@ -1657,7 +1658,7 @@ func TestCGROUPPrivate(t *testing.T) { } func TestCGROUPHost(t *testing.T) { - if _, err := os.Stat("/proc/self/ns/cgroup"); os.IsNotExist(err) { + if _, err := os.Stat("/proc/self/ns/cgroup"); errors.Is(err, os.ErrNotExist) { t.Skip("Test requires cgroupns.") } if testing.Short() { diff --git a/libcontainer/intelrdt/intelrdt.go b/libcontainer/intelrdt/intelrdt.go index d8f98e1c24d..3dcd71b7e07 100644 --- a/libcontainer/intelrdt/intelrdt.go +++ b/libcontainer/intelrdt/intelrdt.go @@ -460,7 +460,7 @@ func (m *Manager) Apply(pid int) (err error) { } } - if err := os.Mkdir(path, 0o755); err != nil && !os.IsExist(err) { + if err := os.Mkdir(path, 0o755); err != nil && !errors.Is(err, os.ErrExist) { return newLastCmdError(err) } @@ -470,7 +470,7 @@ func (m *Manager) Apply(pid int) (err error) { // Create MON group if monPath := m.GetMonPath(); monPath != "" { - if err := os.Mkdir(monPath, 0o755); err != nil && !os.IsExist(err) { + if err := os.Mkdir(monPath, 0o755); err != nil && !errors.Is(err, os.ErrExist) { return newLastCmdError(err) } if err := WriteIntelRdtTasks(monPath, pid); err != nil { @@ -493,14 +493,14 @@ func (m *Manager) Destroy() error { if m.config.IntelRdt.ClosID == "" { m.mu.Lock() defer m.mu.Unlock() - if err := os.Remove(m.GetPath()); err != nil && !os.IsNotExist(err) { + if err := os.Remove(m.GetPath()); err != nil && !errors.Is(err, os.ErrNotExist) { return err } m.path = "" } else if monPath := m.GetMonPath(); monPath != "" { // If ClosID is not specified the possible monintoring group was // removed with the CLOS above. - if err := os.Remove(monPath); err != nil && !os.IsNotExist(err) { + if err := os.Remove(monPath); err != nil && !errors.Is(err, os.ErrNotExist) { return err } } diff --git a/libcontainer/notify_linux.go b/libcontainer/notify_linux.go index f118be1cf6d..af2f43ebab0 100644 --- a/libcontainer/notify_linux.go +++ b/libcontainer/notify_linux.go @@ -51,7 +51,7 @@ func registerMemoryEvent(cgDir, evName, arg string) (<-chan struct{}, error) { } // When a cgroup is destroyed, an event is sent to eventfd. // So if the control path is gone, return instead of notifying. - if _, err := os.Lstat(eventControlPath); os.IsNotExist(err) { + if _, err := os.Lstat(eventControlPath); errors.Is(err, os.ErrNotExist) { return } ch <- struct{}{} diff --git a/libcontainer/process_linux.go b/libcontainer/process_linux.go index 51c7b0d877d..b3487fd0b1b 100644 --- a/libcontainer/process_linux.go +++ b/libcontainer/process_linux.go @@ -1103,7 +1103,7 @@ func getPipeFds(pid int) ([]string, error) { // Ignore permission errors, for rootless containers and other // non-dumpable processes. if we can't get the fd for a particular // file, there's not much we can do. - if os.IsPermission(err) { + if errors.Is(err, os.ErrPermission) { continue } return fds, err diff --git a/libcontainer/rootfs_linux.go b/libcontainer/rootfs_linux.go index 7f4fcde89b9..1ba386ed015 100644 --- a/libcontainer/rootfs_linux.go +++ b/libcontainer/rootfs_linux.go @@ -364,7 +364,7 @@ func mountCgroupV1(m mountEntry, c *mountConfig) error { // symlink(2) is very dumb, it will just shove the path into // the link and doesn't do any checks or relative path // conversion. Also, don't error out if the cgroup already exists. - if err := os.Symlink(mc, filepath.Join(c.root, m.Destination, ss)); err != nil && !os.IsExist(err) { + if err := os.Symlink(mc, filepath.Join(c.root, m.Destination, ss)); err != nil && !errors.Is(err, os.ErrExist) { return err } } @@ -602,7 +602,7 @@ func mountToRootfs(c *mountConfig, m mountEntry) error { return err } if fi, err := os.Lstat(dest); err != nil { - if !os.IsNotExist(err) { + if !errors.Is(err, os.ErrNotExist) { return err } } else if !fi.IsDir() { @@ -899,7 +899,7 @@ func setupDevSymlinks(rootfs string) error { src = link[0] dst = filepath.Join(rootfs, link[1]) ) - if err := os.Symlink(src, dst); err != nil && !os.IsExist(err) { + if err := os.Symlink(src, dst); err != nil && !errors.Is(err, os.ErrExist) { return err } } @@ -1109,7 +1109,7 @@ func setReadonly() error { func setupPtmx(config *configs.Config) error { ptmx := filepath.Join(config.Rootfs, "dev/ptmx") - if err := os.Remove(ptmx); err != nil && !os.IsNotExist(err) { + if err := os.Remove(ptmx); err != nil && !errors.Is(err, os.ErrNotExist) { return err } if err := os.Symlink("pts/ptmx", ptmx); err != nil { diff --git a/libcontainer/specconv/spec_linux_test.go b/libcontainer/specconv/spec_linux_test.go index 4c3c4769348..b96893c7f75 100644 --- a/libcontainer/specconv/spec_linux_test.go +++ b/libcontainer/specconv/spec_linux_test.go @@ -1,6 +1,7 @@ package specconv import ( + "errors" "os" "strings" "testing" @@ -609,7 +610,7 @@ func TestDupNamespaces(t *testing.T) { } func TestUserNamespaceMappingAndPath(t *testing.T) { - if _, err := os.Stat("/proc/self/ns/user"); os.IsNotExist(err) { + if _, err := os.Stat("/proc/self/ns/user"); errors.Is(err, os.ErrNotExist) { t.Skip("Test requires userns.") } @@ -643,7 +644,7 @@ func TestUserNamespaceMappingAndPath(t *testing.T) { } func TestNonZeroEUIDCompatibleSpecconvValidate(t *testing.T) { - if _, err := os.Stat("/proc/self/ns/user"); os.IsNotExist(err) { + if _, err := os.Stat("/proc/self/ns/user"); errors.Is(err, os.ErrNotExist) { t.Skip("Test requires userns.") } diff --git a/libcontainer/state_linux.go b/libcontainer/state_linux.go index 2b7b8b5bc36..1f758e2a301 100644 --- a/libcontainer/state_linux.go +++ b/libcontainer/state_linux.go @@ -1,6 +1,7 @@ package libcontainer import ( + "errors" "fmt" "os" "path/filepath" @@ -213,7 +214,7 @@ func (r *restoredState) transition(s containerState) error { func (r *restoredState) destroy() error { if _, err := os.Stat(filepath.Join(r.c.stateDir, "checkpoint")); err != nil { - if !os.IsNotExist(err) { + if !errors.Is(err, os.ErrNotExist) { return err } } diff --git a/spec.go b/spec.go index d03d644e28e..15e933eb0b9 100644 --- a/spec.go +++ b/spec.go @@ -91,7 +91,7 @@ created by an unprivileged user. if err == nil { return fmt.Errorf("File %s exists. Remove it first", name) } - if !os.IsNotExist(err) { + if !errors.Is(err, os.ErrNotExist) { return err } return nil @@ -117,7 +117,7 @@ created by an unprivileged user. func loadSpec(cPath string) (spec *specs.Spec, err error) { cf, err := os.Open(cPath) if err != nil { - if os.IsNotExist(err) { + if errors.Is(err, os.ErrNotExist) { return nil, fmt.Errorf("JSON specification file %s not found", cPath) } return nil, err