Commit graph

3362 commits

Author SHA1 Message Date
Sebastiaan van Stijn
8a8202fcdc
pkg/chrootarchive: TestChrootTarUntar fix copy/paste mistake
Introduced in 3ac6394b80, which makes no mention
of a reason for extracting to the same directory as we created the archive from,
so I assume this was a copy/paste mistake and the path was meant to be "dest",
not "src".

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-10-16 13:15:23 +02:00
Sebastiaan van Stijn
0955c88c2e
pkg/chrootarchive: use t.TempDir()
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-10-16 13:02:45 +02:00
Sebastiaan van Stijn
d006242d73
Merge pull request #44251 from thaJeztah/pkg_dir_cleanup
pkg/directory: remove unused MoveToSubdir() utility, and some refactoring
2022-10-15 22:48:19 +02:00
Sebastiaan van Stijn
69f72417f4
pkg/idtools: remove CanAccess(), and move to daemon
The implementation of CanAccess() is very rudimentary, and should
not be used for anything other than a basic check (and maybe not
even for that). It's only used in a single location in the daemon,
so move it there, and un-export it to not encourage others to use
it out of context.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-10-15 22:42:39 +02:00
Sebastiaan van Stijn
ee34a8ac29
pkg/idtools: setPermissions() accept Identity as argument
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-10-15 22:42:39 +02:00
Sebastiaan van Stijn
bca90530fa
pkg/idtools: simplify if-statement
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-10-15 22:42:38 +02:00
Sebastiaan van Stijn
d68fa0382d
pkg/idtools: don't use system.Stat() on unix
Looks like we don't need the abstraction, so we can reduce the
dependency on pkg/system.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-10-15 22:42:35 +02:00
Sebastiaan van Stijn
1311687d0d
Merge pull request #44254 from thaJeztah/idtools_cleanup2
pkg/idtools: various cleanups
2022-10-15 22:42:09 +02:00
Sebastiaan van Stijn
081c00c7df
Merge pull request #44265 from thaJeztah/pkg_system_move_init_step1
pkg/system: cleanup, test-fixes and improvements and minor fixes
2022-10-15 21:28:13 +02:00
Sebastiaan van Stijn
43d6eb7173
pkg/pidfile: remove PIDFile type, rename New() to Write()
This type felt really redundant; `pidfile.New()` takes the path of the file to
create as an argument, so this is already known. The only thing the PIDFile
type provided was a `Remove()` method, which was just calling `os.Remove()` on
the path of the file.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-10-15 16:40:16 +02:00
Sebastiaan van Stijn
dd8983f96c
pkg/pidfile: reduce cyclomatic complexity, and small optimisation
Use bytes.TrimSpace instead of using the strings package, which is
more performant, and allows us to skip the intermediate variable.

Also combined some "if" statements to reduce cyclomatic complexity.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-10-15 15:11:40 +02:00
Sebastiaan van Stijn
4917bcc039
pkg/pidfile: don't ignore all errors when reading file
It's ok to ignore if the file doesn't exist, or if the file doesn't
have a PID in it, but we should produce an error if the file exists,
but we're unable to read it for other reasons.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-10-15 15:11:40 +02:00
Sebastiaan van Stijn
3ce2a7d026
pkg/pidfile: pkg/pidfile: use strconv instead of fmt.Sprintf(), and unconvert
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-10-15 15:11:40 +02:00
Sebastiaan van Stijn
f058afc861
pkg/system: synchronize mkdirall() with latest os.MkDirAll()
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-10-15 13:24:43 +02:00
Sebastiaan van Stijn
2e66c0b6f0
pkg/system: create SecurityAttribute only once (Windows)
The same attribute was generated for each path that was created, but always
the same, so instead of generating it in each iteration, generate it once,
and pass it to our mkdirall() implementation.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-10-15 13:24:43 +02:00
Sebastiaan van Stijn
55ceb5047c
pkg/system: update volumePath regex to allow returning earlier
The regex only matched volume paths without a trailing path-separator. In cases
where a path would be passed with a trailing path-separator, it would depend on
further code in mkdirall to strip the trailing slash, then to perform the regex
again in the next iteration.

While regexes aren't ideal, we're already executing this one, so we may as well
use it to match those situations as well (instead of executing it twice), to
allow us to return early.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-10-15 13:24:43 +02:00
Sebastiaan van Stijn
cfef1b11e5
pkg/system: compile volume-path regex once, and update GoDoc
Ideally, we would construct this lazily, but adding a function and a
sync.Once felt like a bit "too much".

Also updated the GoDoc for some functions to better describe what they do.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-10-15 13:24:40 +02:00
Sebastiaan van Stijn
ec3c08d618
Merge pull request #44261 from thaJeztah/remove_deprecated_pkgs
remove aliases for deprecated pkg/urlutil, pkg/fsutils, pkg/pubsub
2022-10-14 01:35:01 +02:00
Brian Goff
058010187b
Merge pull request #44271 from thaJeztah/pidfile_windows_cleanup
pkg/pidfile: windows: replace magic consts for golang.org/x/sys consts
2022-10-10 09:30:06 -07:00
Samuel Karp
c9d2b7df77
Merge pull request #44222 from thaJeztah/godoc_instead_of_readme 2022-10-10 00:06:17 -07:00
Sebastiaan van Stijn
ec000ce555
pkg/archive: use filepath.WalkDir instead of filepath.Walk
WalkDir is more performant as it doesn't perform an os.Lstat on every visited
file or directory.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-10-09 17:12:04 +02:00
Sebastiaan van Stijn
d33428f0bf
pkg/system: remove solaris left-over
It was removed everywhere else, so we may as well remove it here.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-10-09 14:59:19 +02:00
Sebastiaan van Stijn
a35bcd01c5
pkg/pidfile: replace uses of windows.Close() with windows.CloseHandle()
Since https://golang.org/cl/4600042, Close is a straight wrapper of CloseHandle.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-10-09 13:27:58 +02:00
Sebastiaan van Stijn
89de943401
pkg/pidfile: windows: replace magic consts for golang.org/x/sys consts
These consts were defined locally, but are now defined in golang.org/x/sys, so
we can use those.

Also added some documentation about how this function works, taking the description
from the GetExitCodeProcess function (processthreadsapi.h) API reference:
https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-getexitcodeprocess

> The GetExitCodeProcess function returns a valid error code defined by the
> application only after the thread terminates. Therefore, an application should
> not use `STILL_ACTIVE` (259) as an error code (`STILL_ACTIVE` is a macro for
> `STATUS_PENDING` (minwinbase.h)). If a thread returns `STILL_ACTIVE` (259) as
> an error code, then applications that test for that value could interpret it
> to mean that the thread is still running, and continue to test for the
> completion of the thread after the thread has terminated, which could put
> the application into an infinite loop.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-10-09 12:48:35 +02:00
Sebastiaan van Stijn
11cceea58a
pkg/idtools: cleanup errors
Most of the package was using stdlib's errors package, so replacing two calls
to pkg/errors with stdlib. Also fixing capitalization of error strings.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-10-08 21:56:28 +02:00
Sebastiaan van Stijn
8d5b17e939
pkg/idtools: don't use system.MkdirAll() where not needed
On unix, it's an alias for os.MkdirAll, so remove its use to be
more transparent what's being used.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-10-08 21:55:18 +02:00
Sebastiaan van Stijn
2e74e307d0
pkg/idtools: format code with gofumpt
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-10-08 21:55:04 +02:00
Sebastiaan van Stijn
0fc13104e7
pkg/idtools: CanAccess(): reorder checks to allow early return
Merge the accessible() function into CanAccess, and check world-
readable permissions first, before checking owner and group.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-10-08 21:54:23 +02:00
Sebastiaan van Stijn
c7c02eea81
pkg/loopback: use ioctl helpers from x/sys/unix
Use the IoctlRetInt, IoctlSetInt and IoctlLoopSetStatus64 helper
functions defined in the golang.org/x/sys/unix package instead of
manually wrapping these using a locally defined function.

Inspired by 3cc3d8a560

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-10-08 21:20:29 +02:00
Sebastiaan van Stijn
1fccb39316
pkg/idtools: remove unused CanAccess() stub for Windows
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-10-08 18:46:05 +02:00
Sebastiaan van Stijn
3b9b5842b3
pkg/idtools: mkdirAs(): move var and comment to where it's used
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-10-08 18:45:33 +02:00
Sebastiaan van Stijn
27aea4956c
pkg/idtools: mkdirAs() be more explicit about ignored args on Windows
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-10-08 17:11:05 +02:00
Sebastiaan van Stijn
a19ee75bd1
pkg/system: fix missing assertions and use sub-tests for ChTimes
These tests were effectively doing "subtests", using comments to describe each,
however;

- due to the use of `t.Fatal()` would terminate before completing all "subtests"
- The error returned by the function being tested (`Chtimes`), was not checked,
  and the test used "indirect" checks to verify if it worked correctly. Adding
  assertions to check if the function didn't produce an error.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-10-07 22:57:01 +02:00
Sebastiaan van Stijn
a9c5a40087
pkg/system: rename some tests to be more descriptive
Removing the "Linux" suffix from one test, which should probably be
rewritten to be run on "unix", to provide test-coverage for those
implementations.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-10-07 18:36:25 +02:00
Sebastiaan van Stijn
ab7bc6b7d2
pkg/system: use t.TempDir(), remove some test-utils
With t.TempDir(), some of the test-utilities became so small that
it was more transparent to inline them. This also helps separating
concenrs, as we're in the process of thinning out and decoupling
some packages.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-10-07 18:36:13 +02:00
Sebastiaan van Stijn
7bd051eeec
pkg/system: windows: setCTime(): remove redundant conversion
It looks like this function was converting the time (`windows.NsecToTimespec()`),
only to convert it back (`windows.TimespecToNsec()`). This became clear when
moving the lines together:

```go
ctimespec := windows.NsecToTimespec(ctime.UnixNano())
c := windows.NsecToFiletime(windows.TimespecToNsec(ctimespec))
```

And looking at the Golang code, it looks like they're indeed the exact reverse:

```go
func TimespecToNsec(ts Timespec) int64 { return int64(ts.Sec)*1e9 + int64(ts.Nsec) }

func NsecToTimespec(nsec int64) (ts Timespec) {
    ts.Sec = nsec / 1e9
    ts.Nsec = nsec % 1e9
    return
}
```

While modifying this code, also renaming the `e` variable to a more common `err`.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-10-07 18:34:44 +02:00
Sebastiaan van Stijn
0b8444aa0c
pkg/system: rename maxTime and re-use, define unixEpochTime, update GoDoc
This more closely matches to how it's used everywhere. Also move the comment
describing "what" ChTimes() does inside its GoDoc.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-10-07 18:34:44 +02:00
Sebastiaan van Stijn
2c9684e35c
pkg/system: add note about maxTime
This code caused me some head-scratches, and initially I wondered
if this was a bug, but it looks to be intentional to set nsec, not
sec, as time.Unix() internally divides nsec, and sets sec accordingly;
https://github.com/golang/go/blob/go1.19.2/src/time/time.go#L1364-L1380

    // Unix returns the local Time corresponding to the given Unix time,
    // sec seconds and nsec nanoseconds since January 1, 1970 UTC.
    // It is valid to pass nsec outside the range [0, 999999999].
    // Not all sec values have a corresponding time value. One such
    // value is 1<<63-1 (the largest int64 value).
    func Unix(sec int64, nsec int64) Time {
        if nsec < 0 || nsec >= 1e9 {
            n := nsec / 1e9
            sec += n
            nsec -= n * 1e9
            if nsec < 0 {
                nsec += 1e9
                sec--
            }
        }
        return unixTime(sec, int32(nsec))
    }

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-10-07 18:34:34 +02:00
Sebastiaan van Stijn
247f90c82e
pkg/system: move maxTime init() back to Chtimes code
This code was moved to a separate file in fe5b34ba88,
but it's unclear why it was moved (as this file is not excluded on Windows).

Moving the code back into the chtimes file, to move it closer to where it's used.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-10-07 18:13:05 +02:00
Sebastiaan van Stijn
c218211012
Merge pull request #44255 from thaJeztah/pkg_system_remove_GetExitCode
pkg/system: move GetExitCode() to pkg/idtools, and un-export
2022-10-07 18:10:03 +02:00
Sebastiaan van Stijn
4347080b46
pkg/system: remove Umask() utility
It was only used in a couple of places, and in most places shouldn't be used
as those locations were in unix/linux-only files, so didn't need the wrapper.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-10-06 22:22:29 +02:00
Sebastiaan van Stijn
59c77c8f5b
Merge pull request #44250 from thaJeztah/fix_pkg_dir
pkg/directory: Size(): add back type-casts to account for platform differences
2022-10-06 21:44:46 +02:00
Sebastiaan van Stijn
1515e02c8a
Merge pull request #44215 from corhere/fix-unlockosthread-pdeathsig
Stop subprocesses from getting unexpectedly killed
2022-10-06 20:08:53 +02:00
Sebastiaan van Stijn
76ce3fd9c9
remove aliases for deprecated pkg/pubsub
The aliases are included in the 22 release branch, so we can remove them
from master.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-10-06 16:41:00 +02:00
Sebastiaan van Stijn
c1729f876c
remove aliases for deprecated pkg/fsutils
The aliases are included in the 22 release branch, so we can remove them
from master.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-10-06 02:51:24 +02:00
Sebastiaan van Stijn
ce2abb82c0
remove aliases for deprecated pkg/urlutil
The aliases are included in the 22 release branch, so we can remove them
from master.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-10-06 02:48:42 +02:00
Cory Snider
1f22b15030 Lock OS threads when exec'ing with Pdeathsig
On Linux, when (os/exec.Cmd).SysProcAttr.Pdeathsig is set, the signal
will be sent to the process when the OS thread on which cmd.Start() was
executed dies. The runtime terminates an OS thread when a goroutine
exits after being wired to the thread with runtime.LockOSThread(). If
other goroutines are allowed to be scheduled onto a thread which called
cmd.Start(), an unrelated goroutine could cause the thread to be
terminated and prematurely signal the command. See
https://github.com/golang/go/issues/27505 for more information.

Prevent started subprocesses with Pdeathsig from getting signaled
prematurely by wiring the starting goroutine to the OS thread until the
subprocess has exited. No other goroutines can be scheduled onto a
locked thread so it will remain alive until unlocked or the daemon
process exits.

Signed-off-by: Cory Snider <csnider@mirantis.com>
2022-10-05 12:18:03 -04:00
Sebastiaan van Stijn
ab677c41ea
pkg/system: unconvert
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-10-05 16:21:04 +02:00
Sebastiaan van Stijn
07b1aa822c
pkg/system: move GetExitCode() to pkg/idtools, and un-export
This utility was only used in a single place, and had no external consumers.
Move it to where it's used.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-10-05 16:20:33 +02:00
Sebastiaan van Stijn
5b6b42162b
pkg/fsutils: deprecate in favor of containerd/continuity/fs
The pkg/fsutils package was forked in containerd, and later moved to
containerd/continuity/fs. As we're moving more bits to containerd, let's also
use the same implementation to reduce code-duplication and to prevent them from
diverging.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-10-05 11:36:04 +02:00