Make the review-policy workflow reusable (via workflow_call), so other
opencontainers repositories with the same merge rules (such as
opencontainers/cgroups, see opencontainers/cgroups#91) can call it
instead of carrying a copy.
The calling repository gets the same logic, which is fine, since:
- CHANGELOG.md and vendor/ exceptions are no-ops where there's no
such file/directory;
- the merge_group trigger is the caller's, and it's harmless when unused.
The only difference was CODEOWNERS, which cgroups treats as a governance
file. Add it here, too, as it does define who reviews what.
Document how to use it from another repository.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The libseccomp and libpathrs libraries built by Dockerfile are installed
into $DYLIB_DIR/lib, which is only made known to the dynamic linker via
LD_LIBRARY_PATH. Yet runc init is executed with a clean environment, so
it can not find libpathrs.so.0 and dies, resulting in
unable to start container process: can't copy bootstrap data
to pipe: write init-p: broken pipe
As a result, "make integration" and some of "make unittest" fail.
Use ldconfig instead of LD_LIBRARY_PATH.
This went unnoticed because CI only builds the image, and never runs
tests in it. Add a quick smoke test (run.bats) to the release job, which
is the only one that builds the image.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Since bats does not set pipefail, the exit code of jq in update_config
is ignored, and a jq error results in an empty config.json (as awk
happily writes an empty input), so the test fails later, for an
unrelated reason (or, worse, passes, as it happened with the
"timens offsets with no timens" test).
Save jq output to a variable, and only write it to config.json if jq
succeeded.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The test uses ".linux.namespace" (rather than ".linux.namespaces") in
its jq expression, which results in a jq error. As update_config does
not check jq exit code, config.json ends up being empty, and so the
subsequent runc run fails because of that, not because of what the test
is supposed to check.
Fix the jq expression, so that runc now fails with:
time namespace offsets specified, but time namespace isn't enabled in the config
Fixes: aa5f4c11 ("tests: add several timens tests")
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The "runc run (hugetlb limits)" test uses "pagesize" key in
linux.resources.hugepageLimits entries in config.json.
The OCI runtime spec uses "pageSize" key. The test only works because
Go's encoding/json matches keys case-insensitively, which is not the
case for other OCI runtimes, such as crun, when running runc's
integration tests against those.
Use the proper key.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The "update cpuset parameters via resources.CPU" test uses "CPU",
"Cpus", and "Mems" keys in linux.resources in config.json, and in the
input for runc update -r.
The OCI runtime spec uses "cpu", "cpus", and "mems" keys. The test only
works because Go's encoding/json matches keys case-insensitively, which
is not the case for other OCI runtimes, such as crun, when running
runc's integration tests against those.
Use the proper keys. Since the CPU resources are already set by setup(),
merge the new values into them (which is what effectively happened with
the old keys, too).
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The log file is opened with O_SYNC since commit 8f206929 ("Ensure logs
are flushed", PR #637). The intent was to make sure log entries are not
lingering in a buffer, but *os.File is not buffered: logrus does a
single Write per entry, which is a single write(2), and the data is
visible to any reader (e.g. containerd reading the log after runc
exits) as soon as it returns.
What O_SYNC actually does is wait for the data to hit the disk on every
write, which only helps to preserve the log in case of a host crash or
power loss. This is of little value for runc logs, while the cost on a
slow disk (e.g. a network-attached cloud volume) is substantial,
especially with --debug.
Remove O_SYNC.
Fixes: #5540
Suggested-by: Benjamin Peterson <benjamin@engflow.com>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
With the merge queue enabled, all required checks must also pass for the
merge group commit. As review-policy only runs on pull_request_review, it
is never reported there, and the queue times out.
Add the merge_group trigger, and skip the actual check for it: a pull
request can only be added to the queue after review-policy has passed.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Using host's / as the container rootfs does not work: with a private
mount namespace, pivot_root (or MS_MOVE in case of no-pivot) fails,
and without it, container mounts (such as proc, dev etc.) are mounted
on top of the host ones, effectively breaking the host.
Disallow it.
Add an integration test.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
When the container does not have its own mount namespace, using
--no-pivot results in msMoveRoot being run in the host mount namespace.
It unmounts host's procfs and sysfs mounts, and then moves the container
rootfs on top of the host's /, effectively breaking the host.
Similarly, rootfs propagation is applied to / and to the parent mount of
the container rootfs. Without a private mount namespace, this changes
the propagation of host mounts.
Disallow both configurations.
Add integration tests (the --no-pivot one is marked as unsafe, since
without the fix it breaks the host).
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Split security into mountns and selinuxCheck, as these are unrelated.
In mountns, return early if there is a private mount namespace.
No functional change.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The order of checks is not important (none of them modify the config),
so sort the list alphabetically, making it easier to add new entries.
The only visible effect is which error is reported first in case
the configuration has multiple issues.
Suggested-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The runtime spec says that if umask is unspecified, it should not be
changed from the calling process' umask. runc, though, sets the umask
of the container init process to 0022 in this case (as it did before
umask support was added, and as crun does, too). For runc exec, the
umask is not changed.
Document this deviation.
Fixes#3533.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The runtime spec says ociVersion is REQUIRED and MUST be in SemVer
format, and a runtime must generate an error when encountering an
invalid value. runc does not validate ociVersion at all, accepting
a missing, empty, or invalid value, as well as a version with a
different major.
Document this deviation.
Fixes#1846.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The test_mount_order test used host rootfs paths as mount destinations,
relying on runc stripping the rootfs prefix, which is a legacy runc
behavior not described by the runtime spec.
Use paths inside the container instead, as the spec says, so the test
can also be used with other runtimes (e.g. crun). Bind mount sources
inside the container rootfs are still used, as this is what the test
is about.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Add a check enforcing the pull request merge rules from
MAINTAINERS_GUIDE.md, to be used as a required status check (in
addition to the GitHub's required number of approvals, which can not
depend on who the author is, and also counts approvals from anyone
having write access, not just from current maintainers).
The check is a pull_request_review workflow with a read-only token,
which evaluates the current state of the pull request using the GitHub
API. No other triggers are used, since branch protection requires the
latest run for each event type to pass, so a failed run triggered by,
say, a push would keep blocking the merge after a review fixes it.
As a result, the check needs a review (a comment-only one is fine) to
be run after a push, and it needs to be re-run manually once a review
period is over.
Note that since the workflow runs from the pull request, a pull request
can change it. This is the price for not using privileged triggers
(pull_request_target or workflow_run) and write permissions. To
mitigate, the workflow is a governance file (so changing it requires
two LGTMs), and GitHub's own required approval stays as a backstop.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Many pull requests stay open for weeks waiting for a second review,
which slows down development considerably.
Spell out the merge rules, and relax them for maintainers: a pull
request authored by a maintainer can be merged with a single LGTM
from another maintainer, provided it has been open for review for at
least 7 days (14 days for large changes, i.e. more than 100 lines not
counting vendor/), so that other maintainers had a chance to object.
Also, allow routine pull requests (opened by Dependabot, or only
changing documentation and/or CI configuration) to be merged with a
single LGTM.
Neither of the above applies to governance changes, i.e. changes to
the project rules (such as this document) or their enforcement, which
always require two LGTMs.
While at it, clarify that:
- an LGTM is a pull request approval;
- the author's own LGTM does not count;
- a maintainer requesting changes blocks the merge.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This, together with godbus/dbus bump to v5.2.3 and commit f61eed56,
removes runc binary dependency on DNS resolver from net package,
making the binary slightly smaller.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Install bats to /usr rather than /usr/local, so we no longer need to add
/usr/local/bin to PATH and sudo's secure_path in script/setup_host.sh.
For Dockerfile, it makes no difference (because Debian comes with
/usr/local preconfigured for both PATH and sudo), but do the same change
for uniformity.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
When installing bats from source (in Dockerfile and setup_host.sh),
do a shallow clone of the needed tag, rather than cloning the whole
repo and then checking out the tag.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
