- remove gotest.tools dependency as it was only used in one test,
and only for a trivial check
- use t.TempDir()
- rename vars that collided with package types
- don't use un-keyed structs
- explicitly ignore some errors to please linters
- use iotest.ErrReader
- TestReadCloserWrapperClose: verify reading works before closing :)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
For local communications (npipe://, unix://), the hostname is not used,
but we need valid and meaningful hostname.
The current code used the socket path as hostname, which gets rejected by
go1.20.6 and go1.19.11 because of a security fix for [CVE-2023-29406 ][1],
which was implemented in https://go.dev/issue/60374.
Prior versions go Go would clean the host header, and strip slashes in the
process, but go1.20.6 and go1.19.11 no longer do, and reject the host
header.
Before this patch, tests would fail on go1.20.6:
=== FAIL: pkg/authorization TestAuthZRequestPlugin (15.01s)
time="2023-07-12T12:53:45Z" level=warning msg="Unable to connect to plugin: //tmp/authz2422457390/authz-test-plugin.sock/AuthZPlugin.AuthZReq: Post \"http://%2F%2Ftmp%2Fauthz2422457390%2Fauthz-test-plugin.sock/AuthZPlugin.AuthZReq\": http: invalid Host header, retrying in 1s"
time="2023-07-12T12:53:46Z" level=warning msg="Unable to connect to plugin: //tmp/authz2422457390/authz-test-plugin.sock/AuthZPlugin.AuthZReq: Post \"http://%2F%2Ftmp%2Fauthz2422457390%2Fauthz-test-plugin.sock/AuthZPlugin.AuthZReq\": http: invalid Host header, retrying in 2s"
time="2023-07-12T12:53:48Z" level=warning msg="Unable to connect to plugin: //tmp/authz2422457390/authz-test-plugin.sock/AuthZPlugin.AuthZReq: Post \"http://%2F%2Ftmp%2Fauthz2422457390%2Fauthz-test-plugin.sock/AuthZPlugin.AuthZReq\": http: invalid Host header, retrying in 4s"
time="2023-07-12T12:53:52Z" level=warning msg="Unable to connect to plugin: //tmp/authz2422457390/authz-test-plugin.sock/AuthZPlugin.AuthZReq: Post \"http://%2F%2Ftmp%2Fauthz2422457390%2Fauthz-test-plugin.sock/AuthZPlugin.AuthZReq\": http: invalid Host header, retrying in 8s"
authz_unix_test.go:82: Failed to authorize request Post "http://%2F%2Ftmp%2Fauthz2422457390%2Fauthz-test-plugin.sock/AuthZPlugin.AuthZReq": http: invalid Host header
[1]: https://github.com/advisories/GHSA-f8f7-69v5-w4vx
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Some tests are testing timeouts and take a long time to run. Run the tests
in parallel, so that the test-suite takes shorter to run.
Before:
ok github.com/docker/docker/pkg/plugins 34.013s
After:
ok github.com/docker/docker/pkg/plugins 17.945s
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Refactor setupRemotePluginServer() to be a helper, and to spin up a test-
server for each test instead of sharing the same instance between tests.
This allows the tests to be run in parallel without stepping on each-other's
toes (tearing down the server).
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This patch contains some optimizations I still had stashed when working
on eaa9494b71.
- Use the bytes package for handling the output of "lsof", instead of
converting to a string.
- Count the number of newlines in the output, instead of splitting the
output into a slice of strings. We're only interested in the number
of lines in the output.
- Use lsof's -F option to only print the file-descriptor for each line,
as we don't need other information.
- Use the -l, -n, and -P options to omit converting usernames, host names,
and port numbers.
From the [LSOF(8)][1] man-page:
-l This option inhibits the conversion of user ID numbers to
login names. It is also useful when login name lookup is
working improperly or slowly.
-n This option inhibits the conversion of network numbers to host
names for network files. Inhibiting conversion can make lsof run faster.
It is also useful when host name lookup is not working properly.
-P This option inhibits the conversion of port numbers to port names for network files.
Inhibiting the conversion can make lsof run a little faster.
It is also useful when host name lookup is not working properly.
Output looks something like;
lsof -lnP -Ff -p 39849
p39849
fcwd
ftxt
ftxt
f0
f1
f2
f3
f4
f5
f6
f7
f8
f9
f10
f11
Before/After:
BenchmarkGetTotalUsedFds-10 122 9479384 ns/op 10816 B/op 63 allocs/op
BenchmarkGetTotalUsedFds-10 154 7814697 ns/op 7257 B/op 60 allocs/op
[1]: https://opensource.apple.com/source/lsof/lsof-49/lsof/lsof.man.auto.html
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
TestClientWithRequestTimeout has been observed to flake in CI. The
timing in the test is quite tight, only giving the client a 10ms window
to time out, which could potentially be missed if the host is under
load and the goroutine scheduling is unlucky. Give the client a full
five seconds of grace to time out before failing the test.
Signed-off-by: Cory Snider <csnider@mirantis.com>
They're not used anywhere, so let's remove them; better to have
a compile error than a panic at runtime.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Linux 6.2 and up (commit [f1f1f2569901ec5b9d425f2e91c09a0e320768f3][1])
provides a fast path for the number of open files for the process.
From the [Linux docs][2]:
> The number of open files for the process is stored in 'size' member of
> `stat()` output for /proc/<pid>/fd for fast access.
[1]: f1f1f25699
[2]: https://docs.kernel.org/filesystems/proc.html#proc-pid-fd-list-of-symlinks-to-open-files
This patch adds a fast-path for Kernels that support this, and falls back
to the slow path if the Size fields is zero.
Comparing on a Fedora 38 (kernel 6.2.9-300.fc38.x86_64):
Before/After:
go test -bench ^BenchmarkGetTotalUsedFds$ -run ^$ ./pkg/fileutils/
BenchmarkGetTotalUsedFds 57264 18595 ns/op 408 B/op 10 allocs/op
BenchmarkGetTotalUsedFds 370392 3271 ns/op 40 B/op 3 allocs/op
Note that the slow path has 1 more file-descriptor, due to the open
file-handle for /proc/<pid>/fd during the calculation.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Use File.Readdirnames instead of os.ReadDir, as we're only interested in
the number of files, and results don't have to be sorted.
Before:
BenchmarkGetTotalUsedFds-5 149272 7896 ns/op 945 B/op 20 allocs/op
After:
BenchmarkGetTotalUsedFds-5 153517 7644 ns/op 408 B/op 10 allocs/op
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Commit 8d56108ffb moved this function from
the generic (no build-tags) fileutils.go to a unix file, adding "freebsd"
to the build-tags.
This likely was a wrong assumption (as other files had freebsd build-tags).
FreeBSD's procfs does not mention `/proc/<pid>/fd` in the manpage, and
we don't test FreeBSD in CI, so let's drop it, and make this a Linux-only
file.
While updating also dropping the import-tag, as we're planning to move
this file internal to the daemon.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This utility was only used in tests, and internally, and no longer
used since we switch to using os.UserHomeDir() from stdlib.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This function was last used in the pkg/mflag package, which was removed
in 14712f9ff0, and is no longer used in
libnetwork code since e6de8aec2f
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This utility was only used in a single location (as part of `docker info`),
but the `pkg/rootless` package is imported in various locations, causing
rootlesskit to be a dependency for consumers of that package.
Move GetRootlessKitClient to the daemon code, which is the only location
it was used.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Audit the OCI spec options used for Linux containers to ensure they are
less order-dependent. Ensure they don't assume that any pointer fields
are non-nil and that they don't unintentionally clobber mutations to the
spec applied by other options.
Signed-off-by: Cory Snider <csnider@mirantis.com>
Extended attributes are set on files in container images for a reason.
Fail to unpack if extended attributes are present in a layer and setting
the attributes on the unpacked files fails for any reason.
Add an option to the vfs graph driver to opt into the old behaviour
where ENOTSUPP and EPERM errors encountered when setting extended
attributes are ignored. Make it abundantly clear to users and anyone
triaging their bug reports that they are shooting themselves in the
foot by enabling this option.
Signed-off-by: Cory Snider <csnider@mirantis.com>
Now that most uses of reexec have been replaced with non-reexec
solutions, most of the reexec.Init() calls peppered throughout the test
suites are unnecessary. Furthermore, most of the reexec.Init() calls in
test code neglects to check the return value to determine whether to
exit, which would result in the reexec'ed subprocesses proceeding to run
the tests, which would reexec another subprocess which would proceed to
run the tests, recursively. (That would explain why every reexec
callback used to unconditionally call os.Exit() instead of returning...)
Remove unneeded reexec.Init() calls from test and example code which no
longer needs it, and fix the reexec.Init() calls which are not inert to
exit after a reexec callback is invoked.
Signed-off-by: Cory Snider <csnider@mirantis.com>
Our templates no longer contain version-specific rules, so this function
is no longer used. This patch deprecates it.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
These were deprecated in 9d5e754caa, which
is part of the v24.0.0 release, so we can remove it from master.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This was deprecated in 9f3e5eead5, which
is part of the v24.0.0 release, so we can remove it from master.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
These were deprecated in 2d49080056, which
is part of the v24.0.0 release, so we can remove it from master.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This function was deprecated in c63ea32a17, which
is part of the v24.0.0 release, so we can remove it from master.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This const was deprecated in 5c78cbd3be, which
is part of the v24.0.0 release, so we can remove it from master.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
