This is a rework of Qi Wang's patches.
Import package pkg/config from containers/common to read containers.conf
This patch allows users to specify default values stored in containers.conf
that will modify the behaviour of buildah tool.
Signed-off-by: Qi Wang <qiwan@redhat.com>
Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
Closes: #2011
Approved by: TomSweeneyRedHat
(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
When the base image or an image that we're inspecting is a reference to
a manifest list, resolve it to a runnable image instance, then try to
read the configuration blob from the runnable image.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Closes: #1960
Approved by: TomSweeneyRedHat
Some Dockerfiles (fuse-overlay) require additional devices to be in the
build environment.
This patch allows the user to specify additional devices.
Also I noticed that CapAdd and CapDrop was not working in buildah bud situations,
so this patch also fixes this.
Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
Closes: #1820
Approved by: @TomSweeneyRedHat
Record the digest of the base image's manifest, if there is a base
image.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Closes: #1724
Approved by: rhatdan
Overlay mounts allow buildah bud and buildah from to
specify a directory on the disk that will be mounted
as an overlay into the container, where the overlay can be written to
but when the RUN or buildah run exits, the modified files will dissapear.
The basic idea is to be able to mount cache from the disk for things like yum/dnf/apt
to be able to be used and modified in the contianer on a run command, but to be
kept fresh for each RUN.
Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
Closes: #1560
Approved by: giuseppe
Miloslav had some good comments on a previous commit.
https://github.com/containers/buildah/pull/1411
These changes address his issues by removing them.
Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
Closes: #1412
Approved by: mtrmac
Currently rootless podman attempts to write to /var/lib/containers/cache
and fails. This causes us to repeatedly push images that have already been
pushed. This cache directory should be relative to the location of containers/storage
and not always stored in the same directory.
Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
Closes: #1411
Approved by: TomSweeneyRedHat
Use a typed value, to hopefully decrease further temptation to process strings
manually, and to avoid the unnecessary alltransports.ParseImageName which
resolveImage has already called.
This may change the strings used in some error/debug messages, which
now use transports.ImageName instead of the original input; the strings
should by definition have the same semantics.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Closes: #1361
Approved by: rhatdan
This only moves the code, does not modify it at all; a separate
commit to make review easier.
pullImage eventually computes the same value anyway, so this
should not change behavior. We will soon remove the redundant
value in pullImage.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Closes: #1361
Approved by: rhatdan
It should always be redundant with the reference itself; so,
use srcRef.StringWithinTransport() in the cases where we do
need to understand and hard-code the string syntax, after all.
Also improve the oci: format parsing a bit, to be robust
against including an image name.
NOTE: This might change the semantics a bit because StringWithinTransport
does not guarantee preserving the original string (e.g. paths
tend to be normalized not to contain symlinks). Using local paths
as docker/distribution image names is conceptually so problematic
that this seems worth the code cleanup - but I might be wrong.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Closes: #1361
Approved by: rhatdan
ResolveName now guarantees that the transport, if it exists,
is not a part of the image name; the semantics is no longer
ambiguous, so use the value only as expected.
This could possibly fix incorrect handling of some strings
(pull docker://dir:localpath), and the debug log will no longer
contain "error parsing image name %q as given, trying with transport" for every
name parsing attempt.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Closes: #1361
Approved by: rhatdan
Use the right variable to make sure transport and image are
colon-separated in error reports.
Changes user-visible strings.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Closes: #1361
Approved by: rhatdan
This case was originally here for options.Transport, which
no longer exists; and the previous commit has made it impossible
for transport == "" to reach this code path.
Should not change behavior.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Closes: #1361
Approved by: rhatdan
When ResolveName has already determined that the value is an
ID (prefix), and returned the full ID, rely on that knowledge
and don't try at all to pull the image from a 'remote transport ""';
also, don't try to match strings that are already known not to be
ID prefixes, or that are known to use a different transport, against
local storage.
Should not change behavior, except possibly in theoretical
inconsistency cases when store.Image(knownImageID) fails; the code
now does not report other unrelated errors on the transport == ""
path below.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Closes: #1361
Approved by: rhatdan
For some reason, the CI does not report any of these; on macOS
I see many more reports (including complaints about the standard
library), this only cleans up the trivial cases.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Closes: #1365
Approved by: rhatdan
Change references to Transfer to transfer to make it internal only.
It should be determined from the image specification and only determined
in one place.
Make buildah.Pull use registries.conf
Currently buildah pull does not resolve images based on registries.conf
This does not match the behaviour of buildah from or buildah bud
Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
Closes: #1319
Approved by: rhatdan
Add API hooks for designating locations to be used as blob caches when
pulling and pushing images. When we commit read-only copies of
container layers for use in images, if we're using blob caching, store a
copy of the layer in the blob cache directory so that it can be found.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Closes: #1149
Approved by: rhatdan
Also speed up container name selection by making sure the container name
is not chosen before trying it out.
Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
ReserveSELinuxLabels() checks if an error returned by OpenBuilder() is a
does-not-exist error, but OpenBuilder() returns wrapped errors now, and
it wasn't checking the root cause error.
When newBuilder() fails, check the right error value when deciding
whether or not deleting the partially-constructed container failed.
OpenBuildersByPath() shouldn't choke on non-buildah containers, so have
it handle does-not-exist errors the same way OpenAllBuilders() does.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Closes: #1109
Approved by: rhatdan
Instead of just using multierror to collect the error strings (and hope that they
contain enough context about the attempted image names), explicitly collect pairs of
(attempted image name, error).
Then, report an appropriate error text depending on the failures and environment.
Notably, if there was only one attempt (e.g. a fully-qualified name), just report
the error with minimal context, instead of adding extra "1 error occurred:\n\n".
If search registries were used and empty, note that in the error message (now
for real).
Also, make sure we return _some_ error if util.ResolveName ever returned
an empty list for osome reason.
It seems fairly attractive now to split resolveImage into the outer loop
and a separate function for making one attempt, to consolidate the
repetitive failures = append(...) code. OTOH that would force us to add
another return value as a "fatal" indication. Maybe later.
NOTABLY CHANGES BEHAVIOR:
- Changes the error format.
- Restores more detailed error reporting if we have no search registries,
but had multiple candidates anyway, which was given up in the previous commit.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Closes: #909
Approved by: rhatdan
Use the value now returned by util.ResolveImage instead of trying to
recompute it.
Then drop the no longer used getRegistries. (It might be reasonable
to split that part of util.ResolveImage to make it shorter; but it should
not ideally have any independent second-guessing callers. So, just
keep the inlined one instead; that way we certainly don't break it.)
Also drop the no longer used hasRegistry.
CHANGES BEHAVIOR:
- Most notably, the "short name but no search registries" code
has been broken for some time; pullImage was called with
localhost/$shortname, which was a qualified name, so the
specialized error handling was never attempted.
- Temporarily, the error handling in the "short name but no
search registries" code trigers even if there were actually
valid values to try (in practice there is always localhost/$shortname,
and possibly also options.Registry/$shortname). The next commit
will improve it again.
- We now have more legitimate access to the original short name,
so include it in the error message (it was technically available
before, but using it was awkward).
NOTE: registriesConfPath is computed using the sysregistries
package, but actual access happens using the sysregistriesv2 package.
That should be cleaned up eventually.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Closes: #909
Approved by: rhatdan
pullImage now tries (incorrectly) to redundantly compute the same
value as part of error handling. So, return the actually used
data in util.ResolveName.
The computed value is not used yet, so should not change behavior.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Closes: #909
Approved by: rhatdan
resolveImage already either returns an image, or an error; never (nil, nil, nil),
so this is dead code; drop it.
Should not change behavior.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Closes: #909
Approved by: rhatdan
The loop will not run because util.ResolveName("", ...) returns nil;
so, don't even invoke it.
That way, if we don't get an image from resolveImage, we know
we should have and it is an error.
Should not change behavior.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Closes: #909
Approved by: rhatdan
Inspecting the code flow, img seems to end up nil nil when we exhaust
all options and exit the loop, so img and ref don't need to be long-lived.
Should not change behavior, except possibly for a theoretical
case where store.Image(image) succeeds but !strings.HasPrefix(img.ID, image);
OTOH in that case, if it could happen, we would end up with a nil ref, so
that was very likely never intended in the first place.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Closes: #909
Approved by: rhatdan
... instead of breaking out of the loop and returning.
This will eventually allow us to differentiate between finding
an image and exhausting all options.
Should not change behavior.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Closes: #909
Approved by: rhatdan
Make sure that when attempting to diagnose an error, if we encounter an
error during the diagnostic attempt, we return the original error rather
than the error encountered in trying to diagnose it. Log that one.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Closes: #1072
Approved by: rhatdan
We want to make sure that buildah and podman don't use the
same SELinux MCS Labels. So we need to export this function
so libpod can use it.
Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
Closes: #1041
Approved by: rhatdan
Instead of throwing out the upstream error message, save it and return
it if we can't come up with a more-specific suggestion. This makes
debugging easier [1].
Using multierror gives us both pretty formatting (when we print this
for the user) and programmatic access (for any callers that need to
inspect the constituent errors). This is our first direct dependency
on multierror, but we've been vendoring it for a while now because
opencontainers/runtime-tools uses it for config validation.
The error message for the Bats pull is now:
error creating build container: 1 error occurred:
* Error determining manifest MIME type for docker://non-existent-registry.com/alpine:latest: pinging docker registry returned: Get https://non-existent-registry.com/v2/: dial tcp: lookup non-existent-registry.com on 172.16.69.5:53: no such host
[1]: https://github.com/projectatomic/buildah/issues/849
Signed-off-by: W. Trevor King <wking@tremily.us>
Closes: #1014
Approved by: rhatdan
newBuilder(): When store.CreateContainer() returns a duplicate-name
error for a name that we just made up, try again with a different
made-up name.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Closes: #1009
Approved by: rhatdan
Allow util.ResolveName() to return errors from libraries that it uses.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Closes: #948
Approved by: rhatdan
Provide a Pull() function that can be called directly, to go with the
Push() function.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Closes: #918
Approved by: rhatdan
Add the basics of handling the "--isolation" option, though at the
moment, the only recognized option is "oci", which is our default.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Closes: #822
Approved by: rhatdan
The latest runtime-tools is aware of other OS's than Linux. Libpod needs the newer
version to compile on darwin. Unfortunately, the API for generator.New() changed
and requires a string representation of the OS; furthermore, it also returns a
a generator and an error so code had to be adjusted for this too.
Signed-off-by: baude <bbaude@redhat.com>
Currently we are not adding the ARGS passed in via Dockerfile
or --build-args into the running container as environment variables.
Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
Closes: #816
Approved by: umohnani8
Add RunOptions and BuildOptions flags for modifying the list of granted
capabilities from the default.
Default to granting the current (as of this writing) defaults from
runtime-tools, with CAP_NET_RAW removed:
* CAP_AUDIT_WRITE
* CAP_CHOWN
* CAP_DAC_OVERRIDE
* CAP_FOWNER
* CAP_FSETID
* CAP_KILL
* CAP_MKNOD
* CAP_NET_BIND_SERVICE
* CAP_SETFCAP
* CAP_SETGID
* CAP_SETPCAP
* CAP_SETUID
* CAP_SYS_CHROOT
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Closes: #799
Approved by: rhatdan
Both callers of initConfig contain exactly the same code
to load the manifest and config; move it inside initConfig
now that initConfig has the necessary types.Image available.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Closes: #776
Approved by: rhatdan
Daniel J Walsh
Boaz Shuster
TomSweeneyRedHat
Nalin Dahyabhai
Miloslav Trmač
Valentin Rothberg
W. Trevor King
baude