1
0
mirror of https://github.com/containers/buildah.git synced 2025-07-31 15:24:26 +03:00

Stop excessive wrapping

Golang built in functions like os.Create and others print the name of
the file system object when they fail.  Wrapping them a second time
with the file system object, makes the error message look like crap
when reported to the user.

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
This commit is contained in:
Daniel J Walsh
2020-10-15 05:16:50 -04:00
parent 7699b6e68c
commit 39f4cfb79d
11 changed files with 74 additions and 69 deletions

6
add.go
View File

@ -74,11 +74,11 @@ func sourceIsRemote(source string) bool {
func getURL(src, mountpoint, renameTarget string, writer io.Writer) error {
url, err := url.Parse(src)
if err != nil {
return errors.Wrapf(err, "error parsing URL %q", url)
return err
}
response, err := http.Get(src)
if err != nil {
return errors.Wrapf(err, "error parsing URL %q", url)
return err
}
defer response.Body.Close()
// Figure out what to name the new content.
@ -93,7 +93,7 @@ func getURL(src, mountpoint, renameTarget string, writer io.Writer) error {
if lastModified != "" {
d, err := time.Parse(time.RFC1123, lastModified)
if err != nil {
return errors.Wrapf(err, "error parsing last-modified time %q", lastModified)
return errors.Wrapf(err, "error parsing last-modified time")
}
date = d
}

View File

@ -453,7 +453,7 @@ func OpenBuilder(store storage.Store, container string) (*Builder, error) {
}
buildstate, err := ioutil.ReadFile(filepath.Join(cdir, stateFile))
if err != nil {
return nil, errors.Wrapf(err, "error reading %q", filepath.Join(cdir, stateFile))
return nil, err
}
b := &Builder{}
if err = json.Unmarshal(buildstate, &b); err != nil {
@ -476,7 +476,7 @@ func OpenBuilderByPath(store storage.Store, path string) (*Builder, error) {
}
abs, err := filepath.Abs(path)
if err != nil {
return nil, errors.Wrapf(err, "error turning %q into an absolute path", path)
return nil, err
}
builderMatchesPath := func(b *Builder, path string) bool {
return (b.MountPoint == path)
@ -492,7 +492,7 @@ func OpenBuilderByPath(store storage.Store, path string) (*Builder, error) {
logrus.Debugf("error reading %q: %v, ignoring container %q", filepath.Join(cdir, stateFile), err, container.ID)
continue
}
return nil, errors.Wrapf(err, "error reading %q", filepath.Join(cdir, stateFile))
return nil, err
}
b := &Builder{}
err = json.Unmarshal(buildstate, &b)
@ -528,7 +528,7 @@ func OpenAllBuilders(store storage.Store) (builders []*Builder, err error) {
logrus.Debugf("error reading %q: %v, ignoring container %q", filepath.Join(cdir, stateFile), err, container.ID)
continue
}
return nil, errors.Wrapf(err, "error reading %q", filepath.Join(cdir, stateFile))
return nil, err
}
b := &Builder{}
err = json.Unmarshal(buildstate, &b)

View File

@ -419,7 +419,7 @@ func (b *Builder) Commit(ctx context.Context, dest types.ImageReference, options
}
if options.IIDFile != "" {
if err = ioutil.WriteFile(options.IIDFile, []byte(img.ID), 0644); err != nil {
return imgID, nil, "", errors.Wrapf(err, "failed to write image ID to file %q", options.IIDFile)
return imgID, nil, "", err
}
}
}

View File

@ -620,7 +620,7 @@ func (i *containerImageRef) Transport() types.ImageTransport {
func (i *containerImageSource) Close() error {
err := os.RemoveAll(i.path)
if err != nil {
return errors.Wrapf(err, "error removing layer blob directory %q", i.path)
return errors.Wrapf(err, "error removing layer blob directory")
}
return nil
}

View File

@ -208,7 +208,7 @@ func BuildDockerfiles(ctx context.Context, store storage.Store, options BuildOpt
logrus.Debugf("reading remote Dockerfile %q", dfile)
resp, err := http.Get(dfile)
if err != nil {
return "", nil, errors.Wrapf(err, "error getting %q", dfile)
return "", nil, err
}
if resp.ContentLength == 0 {
resp.Body.Close()
@ -237,7 +237,7 @@ func BuildDockerfiles(ctx context.Context, store storage.Store, options BuildOpt
logrus.Debugf("reading local Dockerfile %q", dfile)
contents, err := os.Open(dfile)
if err != nil {
return "", nil, errors.Wrapf(err, "error reading %q", dfile)
return "", nil, err
}
dinfo, err = contents.Stat()
if err != nil {
@ -246,7 +246,7 @@ func BuildDockerfiles(ctx context.Context, store storage.Store, options BuildOpt
}
if dinfo.Mode().IsRegular() && dinfo.Size() == 0 {
contents.Close()
return "", nil, errors.Wrapf(err, "no contents in %q", dfile)
return "", nil, errors.Errorf("no contents in %q", dfile)
}
data = contents
}
@ -265,7 +265,7 @@ func BuildDockerfiles(ctx context.Context, store storage.Store, options BuildOpt
mainNode, err := imagebuilder.ParseDockerfile(dockerfiles[0])
if err != nil {
return "", nil, errors.Wrapf(err, "error parsing main Dockerfile")
return "", nil, errors.Wrapf(err, "error parsing main Dockerfile: %s", dockerfiles[0])
}
warnOnUnsetBuildArgs(mainNode, options.Args)
@ -273,7 +273,7 @@ func BuildDockerfiles(ctx context.Context, store storage.Store, options BuildOpt
for _, d := range dockerfiles[1:] {
additionalNode, err := imagebuilder.ParseDockerfile(d)
if err != nil {
return "", nil, errors.Wrapf(err, "error parsing additional Dockerfile")
return "", nil, errors.Wrapf(err, "error parsing additional Dockerfile %s", d)
}
mainNode.Children = append(mainNode.Children, additionalNode.Children...)
}

View File

@ -90,7 +90,7 @@ func getSymbolicLink(path string) (string, error) {
symPath = filepath.Join(symPath, p)
isSymlink, resolvedPath, err := hasSymlink(symPath)
if err != nil {
return "", errors.Wrapf(err, "error checking symlink for %q", symPath)
return "", err
}
// if isSymlink is true, check if resolvedPath is potentially another symlink
// keep doing this till resolvedPath is not a symlink and isSymlink is false
@ -102,7 +102,7 @@ func getSymbolicLink(path string) (string, error) {
}
isSymlink, resolvedPath, err = hasSymlink(resolvedPath)
if err != nil {
return "", errors.Wrapf(err, "error checking symlink for %q", resolvedPath)
return "", err
}
symLinksResolved++
}
@ -121,14 +121,14 @@ func hasSymlink(path string) (bool, string, error) {
if err != nil {
if os.IsNotExist(err) {
if err = os.MkdirAll(path, 0755); err != nil {
return false, "", errors.Wrapf(err, "error ensuring volume path %q exists", path)
return false, "", err
}
info, err = os.Lstat(path)
if err != nil {
return false, "", errors.Wrapf(err, "error running lstat on %q", path)
return false, "", err
}
} else {
return false, path, errors.Wrapf(err, "error get stat of path %q", path)
return false, path, err
}
}
@ -140,7 +140,7 @@ func hasSymlink(path string) (bool, string, error) {
// Read the symlink to get what it points to
targetDir, err := os.Readlink(path)
if err != nil {
return false, "", errors.Wrapf(err, "error reading link %q", path)
return false, "", err
}
// if the symlink points to a relative path, prepend the path till now to the resolved path
if !filepath.IsAbs(targetDir) {

View File

@ -81,7 +81,7 @@ func (s *StageExecutor) Preserve(path string) error {
// except ensure that it exists.
archivedPath := filepath.Join(s.mountPoint, path)
if err := os.MkdirAll(archivedPath, 0755); err != nil {
return errors.Wrapf(err, "error ensuring volume path %q exists", archivedPath)
return errors.Wrapf(err, "error ensuring volume path exists")
}
if err := s.volumeCacheInvalidate(path); err != nil {
return errors.Wrapf(err, "error ensuring volume path %q is preserved", archivedPath)
@ -110,13 +110,13 @@ func (s *StageExecutor) Preserve(path string) error {
st, err := os.Stat(archivedPath)
if os.IsNotExist(err) {
if err = os.MkdirAll(archivedPath, 0755); err != nil {
return errors.Wrapf(err, "error ensuring volume path %q exists", archivedPath)
return errors.Wrapf(err, "error ensuring volume path exists")
}
st, err = os.Stat(archivedPath)
}
if err != nil {
logrus.Debugf("error reading info about %q: %v", archivedPath, err)
return errors.Wrapf(err, "error reading info about volume path %q", archivedPath)
return err
}
s.volumeCacheInfo[path] = st
if !s.volumes.Add(path) {
@ -152,7 +152,7 @@ func (s *StageExecutor) Preserve(path string) error {
if os.IsNotExist(err) {
continue
}
return errors.Wrapf(err, "error removing %q", s.volumeCache[cachedPath])
return err
}
delete(s.volumeCache, cachedPath)
}
@ -173,7 +173,7 @@ func (s *StageExecutor) volumeCacheInvalidate(path string) error {
if os.IsNotExist(err) {
continue
}
return errors.Wrapf(err, "error removing volume cache %q", s.volumeCache[cachedPath])
return err
}
archivedPath := filepath.Join(s.mountPoint, cachedPath)
logrus.Debugf("invalidated volume cache for %q from %q", archivedPath, s.volumeCache[cachedPath])
@ -193,15 +193,15 @@ func (s *StageExecutor) volumeCacheSave() error {
continue
}
if !os.IsNotExist(err) {
return errors.Wrapf(err, "error checking for cache of %q in %q", archivedPath, cacheFile)
return err
}
if err := os.MkdirAll(archivedPath, 0755); err != nil {
return errors.Wrapf(err, "error ensuring volume path %q exists", archivedPath)
return errors.Wrapf(err, "error ensuring volume path exists")
}
logrus.Debugf("caching contents of volume %q in %q", archivedPath, cacheFile)
cache, err := os.Create(cacheFile)
if err != nil {
return errors.Wrapf(err, "error creating archive at %q", cacheFile)
return err
}
defer cache.Close()
rc, err := archive.Tar(archivedPath, archive.Uncompressed)
@ -224,14 +224,14 @@ func (s *StageExecutor) volumeCacheRestore() error {
logrus.Debugf("restoring contents of volume %q from %q", archivedPath, cacheFile)
cache, err := os.Open(cacheFile)
if err != nil {
return errors.Wrapf(err, "error opening archive at %q", cacheFile)
return err
}
defer cache.Close()
if err := os.RemoveAll(archivedPath); err != nil {
return errors.Wrapf(err, "error clearing volume path %q", archivedPath)
return err
}
if err := os.MkdirAll(archivedPath, 0755); err != nil {
return errors.Wrapf(err, "error recreating volume path %q", archivedPath)
return err
}
err = archive.Untar(cache, archivedPath, nil)
if err != nil {
@ -239,7 +239,7 @@ func (s *StageExecutor) volumeCacheRestore() error {
}
if st, ok := s.volumeCacheInfo[cachedPath]; ok {
if err := os.Chmod(archivedPath, st.Mode()); err != nil {
return errors.Wrapf(err, "error restoring permissions on %q", archivedPath)
return err
}
uid := 0
gid := 0
@ -248,10 +248,10 @@ func (s *StageExecutor) volumeCacheRestore() error {
gid = util.GID(st)
}
if err := os.Chown(archivedPath, uid, gid); err != nil {
return errors.Wrapf(err, "error setting ownership on %q", archivedPath)
return err
}
if err := os.Chtimes(archivedPath, st.ModTime(), st.ModTime()); err != nil {
return errors.Wrapf(err, "error restoring datestamps on %q", archivedPath)
return err
}
}
}

View File

@ -40,7 +40,7 @@ func downloadToDirectory(url, dir string) error {
logrus.Debugf("extracting %q to %q", url, dir)
resp, err := http.Get(url)
if err != nil {
return errors.Wrapf(err, "error getting %q", url)
return err
}
defer resp.Body.Close()
if resp.ContentLength == 0 {
@ -49,12 +49,12 @@ func downloadToDirectory(url, dir string) error {
if err := chrootarchive.Untar(resp.Body, dir, nil); err != nil {
resp1, err := http.Get(url)
if err != nil {
return errors.Wrapf(err, "error getting %q", url)
return err
}
defer resp1.Body.Close()
body, err := ioutil.ReadAll(resp1.Body)
if err != nil {
return errors.Wrapf(err, "Failed to read %q", url)
return err
}
dockerfile := filepath.Join(dir, "Dockerfile")
// Assume this is a Dockerfile

View File

@ -63,18 +63,18 @@ func setChildProcess() error {
func (b *Builder) Run(command []string, options RunOptions) error {
p, err := ioutil.TempDir("", Package)
if err != nil {
return errors.Wrapf(err, "run: error creating temporary directory under %q", os.TempDir())
return err
}
// On some hosts like AH, /tmp is a symlink and we need an
// absolute path.
path, err := filepath.EvalSymlinks(p)
if err != nil {
return errors.Wrapf(err, "run: error evaluating %q for symbolic links", p)
return err
}
logrus.Debugf("using %q to hold bundle data", path)
defer func() {
if err2 := os.RemoveAll(path); err2 != nil {
logrus.Errorf("error removing %q: %v", path, err2)
logrus.Error(err2)
}
}()
@ -167,7 +167,7 @@ func (b *Builder) Run(command []string, options RunOptions) error {
logrus.Debugf("ensuring working directory %q exists", filepath.Join(mountPoint, spec.Process.Cwd))
if err = os.MkdirAll(filepath.Join(mountPoint, spec.Process.Cwd), 0755); err != nil && !os.IsExist(err) {
return errors.Wrapf(err, "error ensuring working directory %q exists", spec.Process.Cwd)
return err
}
// Set the seccomp configuration using the specified profile name. Some syscalls are
@ -213,7 +213,7 @@ func (b *Builder) Run(command []string, options RunOptions) error {
if _, ok := bindFiles["/run/.containerenv"]; !ok {
// Empty string for now, but we may consider populating this later
containerenvPath := filepath.Join(path, "/run/.containerenv")
if err = os.MkdirAll(filepath.Dir(containerenvPath), 0755); err != nil && !os.IsExist(err) {
if err = os.MkdirAll(filepath.Dir(containerenvPath), 0755); err != nil {
return err
}
emptyFile, err := os.Create(containerenvPath)
@ -222,7 +222,7 @@ func (b *Builder) Run(command []string, options RunOptions) error {
}
emptyFile.Close()
if err := label.Relabel(containerenvPath, b.MountLabel, false); err != nil {
return errors.Wrapf(err, "error relabeling %q in container %q", containerenvPath, b.ContainerID)
return err
}
bindFiles["/run/.containerenv"] = containerenvPath
@ -330,35 +330,35 @@ func runSetupBuiltinVolumes(mountLabel, mountPoint, containerDir string, builtin
// If we need to, initialize the volume path's initial contents.
if _, err := os.Stat(volumePath); err != nil {
if !os.IsNotExist(err) {
return nil, errors.Wrapf(err, "failed to stat %q for volume %q", volumePath, volume)
return nil, err
}
logrus.Debugf("setting up built-in volume at %q", volumePath)
if err = os.MkdirAll(volumePath, 0755); err != nil {
return nil, errors.Wrapf(err, "error creating directory %q for volume %q", volumePath, volume)
return nil, err
}
if err = label.Relabel(volumePath, mountLabel, false); err != nil {
return nil, errors.Wrapf(err, "error relabeling directory %q for volume %q", volumePath, volume)
return nil, err
}
initializeVolume = true
}
stat, err := os.Stat(srcPath)
if err != nil {
if !os.IsNotExist(err) {
return nil, errors.Wrapf(err, "failed to stat %q for volume %q", srcPath, volume)
return nil, err
}
if err = idtools.MkdirAllAndChownNew(srcPath, 0755, hostOwner); err != nil {
return nil, errors.Wrapf(err, "error creating directory %q for volume %q", srcPath, volume)
return nil, err
}
if stat, err = os.Stat(srcPath); err != nil {
return nil, errors.Wrapf(err, "failed to stat %q for volume %q", srcPath, volume)
return nil, err
}
}
if initializeVolume {
if err = os.Chmod(volumePath, stat.Mode().Perm()); err != nil {
return nil, errors.Wrapf(err, "failed to chmod %q for volume %q", volumePath, volume)
return nil, err
}
if err = os.Chown(volumePath, int(stat.Sys().(*syscall.Stat_t).Uid), int(stat.Sys().(*syscall.Stat_t).Gid)); err != nil {
return nil, errors.Wrapf(err, "error chowning directory %q for volume %q", volumePath, volume)
return nil, err
}
if err = extractWithTar(mountPoint, srcPath, volumePath); err != nil && !os.IsNotExist(errors.Cause(err)) {
return nil, errors.Wrapf(err, "error populating directory %q for volume %q using contents of %q", volumePath, volume, srcPath)
@ -514,11 +514,11 @@ func (b *Builder) setupMounts(mountPoint string, spec *specs.Spec, bundlePath st
func (b *Builder) addNetworkConfig(rdir, hostPath string, chownOpts *idtools.IDPair, dnsServers, dnsSearch, dnsOptions []string, namespaceOptions NamespaceOptions) (string, error) {
stat, err := os.Stat(hostPath)
if err != nil {
return "", errors.Wrapf(err, "error statting %q for container %q", hostPath, b.ContainerID)
return "", err
}
contents, err := ioutil.ReadFile(hostPath)
if err != nil {
return "", errors.Wrapf(err, "unable to read %s", hostPath)
return "", err
}
search := resolvconf.GetSearchDomains(contents)
@ -572,13 +572,12 @@ func (b *Builder) addNetworkConfig(rdir, hostPath string, chownOpts *idtools.IDP
gid = chownOpts.GID
}
if err = os.Chown(cfile, uid, gid); err != nil {
return "", errors.Wrapf(err, "error chowning file %q for container %q", cfile, b.ContainerID)
return "", err
}
if err := label.Relabel(cfile, b.MountLabel, false); err != nil {
return "", errors.Wrapf(err, "error relabeling %q in container %q", cfile, b.ContainerID)
return "", err
}
return cfile, nil
}
@ -587,13 +586,13 @@ func (b *Builder) generateHosts(rdir, hostname string, addHosts []string, chownO
hostPath := "/etc/hosts"
stat, err := os.Stat(hostPath)
if err != nil {
return "", errors.Wrapf(err, "error statting %q for container %q", hostPath, b.ContainerID)
return "", err
}
hosts := bytes.NewBufferString("# Generated by Buildah\n")
orig, err := ioutil.ReadFile(hostPath)
if err != nil {
return "", errors.Wrapf(err, "unable to read %s", hostPath)
return "", err
}
hosts.Write(orig)
for _, host := range addHosts {
@ -626,10 +625,10 @@ func (b *Builder) generateHosts(rdir, hostname string, addHosts []string, chownO
gid = chownOpts.GID
}
if err = os.Chown(cfile, uid, gid); err != nil {
return "", errors.Wrapf(err, "error chowning file %q for container %q", cfile, b.ContainerID)
return "", err
}
if err := label.Relabel(cfile, b.MountLabel, false); err != nil {
return "", errors.Wrapf(err, "error relabeling %q in container %q", cfile, b.ContainerID)
return "", err
}
return cfile, nil
@ -678,7 +677,7 @@ func runUsingRuntime(isolation Isolation, options RunOptions, configureNetwork b
return 1, errors.Wrapf(err, "error encoding configuration %#v as json", spec)
}
if err = ioutils.AtomicWriteFile(filepath.Join(bundlePath, "config.json"), specbytes, 0600); err != nil {
return 1, errors.Wrapf(err, "error storing runtime configuration in %q", filepath.Join(bundlePath, "config.json"))
return 1, errors.Wrapf(err, "error storing runtime configuration")
}
logrus.Debugf("config = %v", string(specbytes))
@ -800,7 +799,7 @@ func runUsingRuntime(isolation Isolation, options RunOptions, configureNetwork b
// Make sure we read the container's exit status when it exits.
pidValue, err := ioutil.ReadFile(pidFile)
if err != nil {
return 1, errors.Wrapf(err, "error reading pid from %q", pidFile)
return 1, err
}
pid, err := strconv.Atoi(strings.TrimSpace(string(pidValue)))
if err != nil {
@ -951,7 +950,7 @@ func runCollectOutput(fds, closeBeforeReadingFds []int) string {
func setupRootlessNetwork(pid int) (teardown func(), err error) {
slirp4netns, err := exec.LookPath("slirp4netns")
if err != nil {
return nil, errors.Wrapf(err, "cannot find slirp4netns")
return nil, err
}
rootlessSlirpSyncR, rootlessSlirpSyncW, err := os.Pipe()
@ -1568,7 +1567,7 @@ func setupNamespaces(g *generate.Generator, namespaceOptions NamespaceOptions, i
p := filepath.Join("/proc/sys", strings.Replace(name, ".", "/", -1))
_, err := os.Stat(p)
if err != nil && !os.IsNotExist(err) {
return false, nil, false, errors.Wrapf(err, "cannot stat %s", p)
return false, nil, false, err
}
if err == nil {
g.AddLinuxSysctl(name, val)
@ -1706,12 +1705,12 @@ func (b *Builder) runSetupVolumeMounts(mountLabel string, volumeMounts []string,
}
if foundz {
if err := label.Relabel(host, mountLabel, true); err != nil {
return specs.Mount{}, errors.Wrapf(err, "relabeling %q failed", host)
return specs.Mount{}, err
}
}
if foundZ {
if err := label.Relabel(host, mountLabel, false); err != nil {
return specs.Mount{}, errors.Wrapf(err, "relabeling %q failed", host)
return specs.Mount{}, err
}
}
if foundO {
@ -1988,7 +1987,7 @@ func setupRootlessSpecChanges(spec *specs.Spec, bundleDir string, shmSize string
emptyDir := filepath.Join(bundleDir, "empty")
if err := os.Mkdir(emptyDir, 0); err != nil {
return errors.Wrapf(err, "error creating %q", emptyDir)
return err
}
// Replace /sys with a read-only bind mount.

View File

@ -1787,10 +1787,16 @@ _EOF
@test "bud with specified context should succeed if context contains existing Dockerfile" {
DIR=$(mktemp -d)
touch "$DIR"/Dockerfile
echo "FROM alpine" > "$DIR"/Dockerfile
run_buildah 0 bud --signature-policy ${TESTSDIR}/policy.json "$DIR"/Dockerfile
}
@test "bud with specified context should fail if context contains empty Dockerfile" {
DIR=$(mktemp -d)
touch "$DIR"/Dockerfile
run_buildah 125 bud --signature-policy ${TESTSDIR}/policy.json "$DIR"/Dockerfile
}
@test "bud-no-change" {
_prefetch alpine
parent=alpine

View File

@ -189,7 +189,7 @@ func IsContainer(id string, store storage.Store) (bool, error) {
if os.IsNotExist(err) {
return false, nil
}
return false, errors.Wrapf(err, "error stating %q", filepath.Join(cdir, stateFile))
return false, err
}
return true, nil
}