Set extended attributes on files _after_ setting their ownership, so
that security-sensitive attributes ("security.capability" among them)
won't get quietly cleared from under us if we set them before calling
chown().
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
During a copy operation which descends through a directory tree,
It's possible for a referenced file to become inaccessible (by unlink
or permission change or whatever). During the walk of paths to copy,
an `Lstat()` is run on each item, and any error passed into the handler
function to deal with. Subsequently, if there is no error, the file
is examined for inclusion/exclusion by the handler.
Unfortunately, this introduces a TOCTOU race condition for files which
become inaccessible even if they would otherwise be excluded. For
example a file or directory under /proc or /sys (which frequently and
unpredictably change). This was the original cause encountered during
podman integration testing.
It's impractical to actually fix this race at the file-level, without
introducing negative effects to any source-container operations. It's
also questionably useful to offer a command-line option to offload the
choice to the user. Instead, follow the behavior of the `tar` command
for this situation: Issue a warning to the user, and ignore the
problematic item (don't copy it).
Also add a test resembling the podman test which originally caught this
race. While not reliable, it does introduce a non-zero chance of
hitting the race condition - and handling the new warning properly.
Signed-off-by: Chris Evich <cevich@redhat.com>
The ignore-socket test in copy.bats has to create a UNIX socket
file. As it turns out, mknod won't do that: there are only
horrible kludgy options. The least bad of those is to run 'nc'
and control-C it; we do this hands-off via background job.
Until now, the test then infinite-looped a 'test -e socketfile'.
This failed catastrophically in gating tests, because 'nc' was
not installed (it was not a requirement of the RPM) (it is now).
To make things worse, it took months to track this down because
the gating-test logs aren't flushed. But that's another story.
Here we add a fixed number of retries to the 'test -e' loop,
and abort with a timeout error if the socket isn't created.
This may save someone on a distro where nc hasn't yet been
added as a dependency.
Signed-off-by: Ed Santiago <santiago@redhat.com>
Add a --from option to `buildah add` and `buildah copy`, mirroring the
option for the Dockerfile instruction.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Allow users to override location of the .dockerignore file.
If user specified an --ignorefile flag, buildah will read the
file and pass in the exclude lines based on the .dockerignore
contents.
Also add a --contextdir flag to buildah copy and buildah add to
specify where the context directory is located. For these two
commands the --ignorefile flag requires the --contextdir flag.
When the --ignorefile flag is passed in, the .dockerignore files
in the context directory will be ignored.
Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
When extracting archives that are added using ADD, don't override
permissions and ownership information. We regressed on this when we
switched to using the copier package to handle them.
Add a conformance test to prevent regressions on this.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Use the copier package to rework how we handle ADD and COPY.
When evaluating cache for content that's being copied/added in, switch
from (digest the data, check for a cache entry, then maybe copy the data
and create the new layer) to (copy the data and create the new layer,
digesting as we go, check for a cache entry, either commit or discard
the new layer).
Use the copier package for ADD, COPY, and for ensuring that a specified
directory exists in the working container's rootfs.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
When ADDing an archive file, the destination file-name is blanked out to ensure
that archive contents are extracted in-place. However, when COPYing an
archive-file, we don’t want to blank out the destination filename.
Fixes: #2549
Signed-off-by: Sebastian Reuße <seb@wirrsal.net>
Buildah run was exiting with the correct exit code, when a container
failed. Buildah bud was not, so this should fix this.
Also switched to the proper exit codes when containers fail. When
Buildah fails to execute it will exit with a 125 exit code like
Podman does. If a command fails to execute inside of a container
we will exit with a 126. Currently we do not support the 127 for
exiting when the command does not exist.
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>
Make sure we fail a build when the source for a COPY --from instruction
can't be found.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Closes: #2111
Approved by: rhatdan
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
(Replaces #1873 as it had lint issues that were timing out tests that I couldn't
track down easily)
Prior to this fix, if someone did `buildah bud --pull=false .` and the image in
the Containerfile's FROM statement was not local, the build would fail. The same
build on Docker will succeed. In Docker, when `--pull` is set to false, it only
pulls the image from the registry if there was not one locally. Buildah would never
pull the image and if the image was not locally available, it would throw an error.
In certain Kubernetes environments, this was especially troublesome.
To retain the old `--pull=false` functionality, I've created a new `--pull-never`
option that fails if an image is not locally available just like the old
`--pull=false` option used to do.
In addition, if there was a newer version of the image on the repository than
the one locally, the `--pull=true` option would not pull the image as it should
have, this corrects that.
Changes both the from and bud commands.
Addresses: #1675
Signed-off-by: TomSweeneyRedHat <tsweeney@redhat.com>
Closes: #1959
Approved by: rhatdan
The newer-than and older-than test was failing (desired result)
but it's because the command had a syntax error (not desired)!
Rewrite that section a little, making it more robust (actually
confirming that mtime is preserved, versus simple mtime match
because of time of execution) and also with more readable
diagnostic in case of error.
Signed-off-by: Ed Santiago <santiago@redhat.com>
Closes: #1935
Approved by: giuseppe
First, and possibly most controversial, remove --debug flag.
Output on test failure is painful to read. It is unlikely
(but not inconceivable) that --debug will be of any use.
Second, fix a failure message so it properly uses %s instead
of string interpolation. (If the test name itself includes
a percent sign, we get undefined behavior on output).
Fix a few instances of 'buildah' to be 'run_buildah'.
Remove some unnecessary 'run/echo/check-status' sequences,
sticking with the much cleaner 'test'. When BATS shows
an error of the form 'this failed: [ $status -eq 0 ]'
I weep in despair.
Signed-off-by: Ed Santiago <santiago@redhat.com>
Closes: #1935
Approved by: giuseppe
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
This is the third part of adding flags/args order verficiation.
In this part I am adding it to command lines that take no
arguments or more than 1 arguments such as buildah-images.
Added tests too to verify it works correctly.
Signed-off-by: Boaz Shuster <boaz.shuster.github@gmail.com>
Closes: #1002
Approved by: rhatdan
Users expect to be able to use Symbolic links in the
ADD and COPY commands in Dockerfiles. We need to evaluate
these and pull in the correct content.
Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
Closes: #1007
Approved by: nalind
When testing the --chown flag, expect ownership of content that we copy
in while specifying an owner to match the IDs of that owner as the
container knows them.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Closes: #700
Approved by: rhatdan
When saving the contents of a URL to a local file, attempt to set mtime
based on the response's Last-Modified header, if there is one.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Closes: #58
Approved by: nalind
We can use "scratch" now as a source for building images, so speed
things up a bit by using it instead of pulling alpine in tests where it
isn't expected to affect the test result.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Closes: #55
Approved by: rhatdan
When the destination for an ADD or COPY operation ends with a path
separator, take that as an indicator that the destination should be a
directory, that we should create it if it doesn't already exist, and
that any files we're copying should be placed in the directory.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Closes: #56
Approved by: rhatdan
When copying or adding a source directory, copy the directory's contents
to the destination directory, to better match Dockerfile COPY behavior.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Closes: #50
Approved by: rhatdan
Rename the "list" and "delete" commands to "containers" and "rm",
respectively, and add "images" and "rmi" counterparts for them.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Closes: #32
Approved by: rhatdan
Add options to Run() for passing in additional environment variables,
overriding the default command, user, and working directory, and a flag
for controlling whether or not we attach to the host's network.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Closes: #46
Approved by: rhatdan
Closes: #39
Approved by: nalind
We need to match the syntax of Dockerfile, three forms
buildah copy $CID SOURCE
buildah copy $CID SOURCE DESTINATION
buildah copy $CID SOURCE SOURCE SOURCE DESTINATION
Closes: #40
Approved by: nalind
When flags that we'd require be specified aren't, if there are command
line arguments, use their values as flag values.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Clean up the tests so that data files that we generate don't get dropped
in the root directory of the host, or in a part of ${TMPDIR} that we
won't clean up. Add tests to exercise "add"'s extracting logic.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Simplify our implementation of copy/add to always assume that the
destination location will be a directory. Trying to be cleverer, like
cp is, would just be confusing.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Nalin Dahyabhai
Daniel J Walsh
Chris Evich
Ed Santiago
Giuseppe Scrivano
Hironori Shiina
Jakub Guzik
Sebastian Reuße
TomSweeneyRedHat
Boaz Shuster
Fabio Bertinatto