Regular primitive bats uses assertions like '[ $foo = something ]'.
These are worthless for debugging: when they fail, all you know
is that foo is not "something" but you don't know what foo _is_.
Find and replace those assertions with 'assert', which is
more informative. Instances found via:
$ ack '^ *\[' tests/*.bats
There are many matches for 'test' (instead of '[') but those
mostly look like file-existence ones, which are less evil
than string-check tests. I'm leaving those be for now.
Signed-off-by: Ed Santiago <santiago@redhat.com>
Podman adds an Error: to every error message. So starting an error
message with "error" ends up being reported to the user as
Error: error ...
This patch removes the stutter.
Also ioutil.ReadFile errors report the Path, so wrapping the err message
with the path causes a stutter.
Signed-off-by: Daniel J Walsh dwalsh@redhat.com
[NO NEW TESTS NEEDED]
Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
The current logic in setupRootlessSpecChanges has quite a few problems.
First of this logic is a noop for rootful and when we have a netns.
However pid and ipc could still need special mounts.
In Podman we have already working code for this so this reuses the same
logic and adapts it to buildah.
4ba71f955a/pkg/specgen/generate/oci.go (L178)
Since there is already logic to handle these mounts in buildah we will
add it there and also handle the special rootless case there.
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Buildah ignores the container.conf settings. Commit 05d5d22dc6 tried
to fix this but I cannot see how this would have worked, there were also
no test added related to containers.conf.
The code below were we read the default just overwrites everything we
already set so the config settings are ignored.
Also there are many weird places were settings are just overwritten for
no particular reason. The namespaces code path looks like mess to me.
Namespaces are added/changed/removed in many different places and there
is no explanation why. Maybe I am just not familar enough with this code
base but I cannot really understand this. Hopefully a maintainer can
take a closer look to see if my changes are indeed correct.
Fixescontainers/podman#13294
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Instrument test so that they can be executed in rootless environment.
Certain tests perform mount directly and they need a seperate
usernamespace, these tests are marked to be skipped for `rootless
environment`
Signed-off-by: Aditya R <arajan@redhat.com>
Commit d05957a6f6 added the private value for namespace flags and kept
container as alias for backwards compat. Commit b480ce832f added the
cgroup falg so the test has 3^6 combinations to test. This takes way to
long. Since container is the same as private we only test private. The
container value is already covered in the other namespace tests, see
general_namespace() function.
With this commit the test time went down from 23 min to 3 min on my
laptop.
Fixes#3768
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Buildah is mainly building with Host Namespaces, this changes most
namespaces to be private matching Podman and using containers.conf
Fixes: https://github.com/containers/buildah/issues/3634
Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
This is much scarier than I had intended. The intention is to
start migrating from 'expect_output' and to use 'assert'
instead; the reason is that 'assert' allows much more
flexibility, particularly negative tests.
We've long wanted something like "assert that output does
not contain 'foo'". I've been too lazy to implement it,
but last week I noticed code in bud.bats that does:
! expect_output "sdfsdfsdf"
This is a really super bad idea: although it works fine when
things are good and tests pass, it's a disaster when tests
fail because the poor person debugging test output now sees
a test failure log, and starts tracking it down, and then
much later realizes that it was an intentional failure but
the '!' was negating it, and all that track-down work was
wasted.
To keep this PR reasonable, I'm keeping all positive uses
of expect_output untouched. We can migrate those (if desired)
over time. All negative uses are now assertions.
Signed-off-by: Ed Santiago <santiago@redhat.com>
Followup to #3173 - just a little further cleanup of idmapping
test. Plus, oops, fix a longstanding error-failure bug
Signed-off-by: Ed Santiago <santiago@redhat.com>
Add 'bud' subcommand to the 'namespaces' system test, which already
verifies namespace flags with 'from' and 'run' subcommands.
Signed-off-by: Hironori Shiina <shiina.hironori@jp.fujitsu.com>
Tests for the following options are added:
- buildah run --runtime --runtime-flag
- buildah run --terminal
- buildah run with namespace flags (Fix a bug in the existing test)
Signed-off-by: Hironori Shiina <shiina.hironori@jp.fujitsu.com>
If UID and GID mappings are specified, the container has a
mapped-layer, whose diffID is not computed when created.
Committing the image fails due to lack of diffID. This fix
computes diffID at creating an image source if a layer
doesn't have a diffID (UncompressedDigest).
This fix also tests if a container with UID and GID mappings
can be committed.
Signed-off-by: Hironori Shiina <shiina.hironori@fujitsu.com>
The format of the userns-uid-map and userns-gid-map fields must be
the form of a triple `ctrUid:hostUid:length` but we were not validating
that, doing a lot of useless processing, and then not even failing.
Addresses: #2676
Signed-off-by: TomSweeneyRedHat <tsweeney@redhat.com>
Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
Podman namespaces flags handle the options slightly differently.
Specifically namespace flag `container` is very different.
Replace the `container` option with `private` to better match podman
but continue to support `container`
Similarly Podman expects --network ns:/PATH to allow the indication of a
path to a precreated network namespace. Buildah just supported
--network /PATH, buildah will now support both formats, but document
ns:/path.
Finally use Aliases for fix some duplicated code in buildah.
Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
Show of hands: who here loves submitting a PR, then coming back
hours later to find one job failed, then spending time poring
over logs and finding a network error? Anyone? Anyone?
This is a lame attempt to minimize such flakes by caching
commonly-used images and restoring them on demand. We
introduce a new helper, _prefetch(), which podman-pulls
an image the first time, podman-saves it, then on
subsequent calls (for the same image) podman-loads it:
@test foo {
_prefetch alpine busybox
...tests that run buildah-from either
}
This is an imperfect solution: it is incomplete and will
grow more so over time as new tests are added. It is
difficult to verify its coverage. I'm really unhappy
with it but if it works, the Total Sum Of Unhappiness
might decrease overall thanks to fewer flakes. If it
doesn't work, it's trivial to remove _prefetch calls
using a sed script. Shall we give it a chance?
Signed-off-by: Ed Santiago <santiago@redhat.com>
This is the manual part of the cleanup.
- remove most 'buildah rm's from cleanup steps
- remove unused variables
- use expect_output when possible, not 'test [ x = y ]'
- includes fixing some not-what-you-think-it-is uses
of run_buildah | grep
- be careful not to leave droppings in current directory
Special attention to:
- authenticate.bats: create Dockerfile in temp dir, not pwd!
- push.bats: remove broken output check
- registries.bats: refactor, remove duplicate tests & code
- rmi.bats: better checking of messages
- secrets.bats: use TESTDIR (tmp), not TESTSDIR (/usr/share) !
We don't want to leave droppings behind if we fail; this has
bitten us before. And, since there's only one test, there's
no need to override setup() / teardown(). Do setup inline,
and take advantage of existing defaults.
Signed-off-by: Ed Santiago <santiago@redhat.com>
Closes: #2029
Approved by: rhatdan
Replace plain 'buildah' invocations with run_buildah
For ease of review, this commit was entirely machine-generated via:
$ perl -pi -e 's/^(\s+)buildah(\s)/${1}run_buildah${2}/' *.bats
Signed-off-by: Ed Santiago <santiago@redhat.com>
Closes: #2029
Approved by: rhatdan
Replace this pattern:
cid=$(buildah ...)
with:
run_buildah ...
cid=$output
As a special case, if the buildah command is 'from' and the
string 'scratch' does not appear in the arguments, add '--quiet'.
Otherwise we'll get Pulling messages as part of $output. This
is an overkill solution.
For ease of review, this commit was entirely machine-generated via:
$ perl -pi -e 's{^(\s+)(\S+)=\$\(buildah (.*)\)$}{$indent=$1;$var=$2;$cmd=$3; $cmd =~ s/(\bfrom)\s(?!.*scratch)/$1 --quiet $2/; "${indent}run_buildah $cmd\n$indent$var=\$output"}e' *.bats
Signed-off-by: Ed Santiago <santiago@redhat.com>
Closes: #2029
Approved by: rhatdan
PR #1935 removed the default --debug logging; so all run_buildah
calls now use the default (error). It is safe to remove unnecessary
instances of --log-level=error .
For ease of review, this commit was entirely machine-generated via:
$ perl -pi -e 's/ --log-level=error / /' *.bats
Signed-off-by: Ed Santiago <santiago@redhat.com>
Closes: #2029
Approved by: rhatdan
Refactor many cumbersome BUILDAH_ISOLATION and 'which runc'
checks into common 'skip_if_{ chroot, rootless, no_runtime}'
calls.
This is in preparation for the next PR, which introduces
additional cgroups-v2 skips.
Signed-off-by: Ed Santiago <santiago@redhat.com>
Closes: #1965
Approved by: rhatdan
The previous log-level implementation does not seem to work, which is
now fixed and aligns to other projects like podman and CRI-O. Therefore
the `--debug` flag has been deprecated and is now hidden.
Added documentation as well as integration tests.
Signed-off-by: Sascha Grunert <sgrunert@suse.com>
Closes: #1811
Approved by: rhatdan
fix the detection code for running in a user namespace. When buildah
is running in rootless mode, a user namespace is automatically created
even if there are no mappings configured.
Closes: https://github.com/containers/libpod/issues/2972
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Closes: #1775
Approved by: rhatdan
Fix handling of ID mapping for COPY: when copying from other containers,
use their mappings, and when copying from the host, use host mappings.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Closes: #1630
Approved by: TomSweeneyRedHat
...and make the "$output" argument implicit, as well as the
description text. This greatly simplifies its invocation.
Also: make it test for exact matches unless --substring
option is passed; this eases my mind about a few potentially
ambiguous situations such as the one in #1464.
Also: add comments to two truth-table check functions
Also: reorder some config checks alphabetically, for ease
of reading.
Thanks to Tom Sweeney for the suggestions and encouragement.
Signed-off-by: Ed Santiago <santiago@redhat.com>
Closes: #1492
Approved by: rhatdan
This started off as bug fixes necessary to get BATS tests
actually working on RHEL8 (they weren't). It grew. I will
defend my actions in the first comment post.
Primary change: import some helpers from podman BATS tests,
most importantly 'run_buildah' and 'is'. The vast majority
of the changes you'll see here are of the form:
- run buildah ...
- [ $status = 0 ]
- [ check $output ]
+ run_buildah ... ! automatically checks status
+ is "$output" "..."
Also: chmod'ed some files -x. Necessary because rpmbuild tries
to be oh-so-clever about requirements, and when it sees an
executable file with a shebang line like '#!env bats' it
helpfully adds 'Requires: /usr/bin/bats' to the rpm, which
then fails to install because RHEL8 does not have bats.
Also: refactored duplicate code in a few places, by writing
and invoking module-specific helper functions.
Also: changed a handful of 'buildah's to run_buildah, in
order to get error checking and debug logging.
Also: added descriptive reasons to many "skip"s.
Also: selinux test: some tweakery to make it run on production
system (context is different if /usr/bin/buildah is chcon'ed
appropriately). I can't get this test to pass on Fedora from
a build dir, and I'm actually not convinced that this test
has ever passed, but let's see what CI shows.
Also: selinux test: skip broken test (#1465).
Also: version test: skip parts of it if running w/o sources.
Tests are now passing as root on RHEL8; rootless has numerous
failures which I don't believe are related to this PR.
Signed-off-by: Ed Santiago <santiago@redhat.com>
Closes: #1472
Approved by: TomSweeneyRedHat
We were building the --userns-uid-map and --userns-gid-map CLI flags
into a single flag/value argument.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Closes: #1069
Approved by: rhatdan
When we're testing with rootless isolation, skip tests that check that
we were able to influence namespace and control group settings, since we
hardwire namespace settings and don't try to configure control groups.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Closes: #873
Approved by: rhatdan
Add an IsolationChroot that trades flexibility and isolation for being
able to do what it does in a host environment that's already isolated to
the point where we're not allowed to set up some of that isolation,
producing a result that leans more toward chroot(1) than runc(1) does.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Closes: #836
Approved by: rhatdan
Test that the --ipc, --net/--network, --pid, --userns, --uts options can
be used to control whether or not we create new namespaces of those
types in "buildah run".
Test that when ID mappings are not specified, we default to not using a
new user namespace, but when ID mappings are specified, we create a new
user namespace and ID mappings are set correctly in the running
container.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Closes: #700
Approved by: rhatdan
Ed Santiago
Daniel J Walsh
Aditya R
Paul Holzinger
Giuseppe Scrivano
Hironori Shiina
OpenShift Merge Robot
Hironori Shiina
TomSweeneyRedHat
Sascha Grunert
Nalin Dahyabhai