Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch to golang native error wrapping #1077

Merged
merged 1 commit into from
Jul 12, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ require (
github.com/opencontainers/runtime-spec v1.0.3-0.20210326190908-1c3f411f0417
github.com/opencontainers/runtime-tools v0.9.0
github.com/opencontainers/selinux v1.10.1
github.com/pkg/errors v0.9.1
github.com/pmezard/go-difflib v1.0.0
github.com/seccomp/libseccomp-golang v0.10.0
github.com/sirupsen/logrus v1.8.1
Expand Down Expand Up @@ -82,6 +81,7 @@ require (
github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
github.com/modern-go/reflect2 v1.0.2 // indirect
github.com/ostreedev/ostree-go v0.0.0-20210805093236-719684c64e4f // indirect
github.com/pkg/errors v0.9.1 // indirect
github.com/proglottis/gpgme v0.1.2 // indirect
github.com/prometheus/client_golang v1.11.1 // indirect
github.com/prometheus/client_model v0.2.0 // indirect
Expand Down
13 changes: 7 additions & 6 deletions libimage/copier.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package libimage

import (
"context"
"errors"
"fmt"
"io"
"os"
"strings"
Expand All @@ -17,7 +19,6 @@ import (
storageTransport "github.com/containers/image/v5/storage"
"github.com/containers/image/v5/types"
encconfig "github.com/containers/ocicrypt/config"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
)

Expand Down Expand Up @@ -343,12 +344,12 @@ func (c *copier) copy(ctx context.Context, source, destination types.ImageRefere
// Sanity checks for Buildah.
if sourceInsecure != nil && *sourceInsecure {
if c.systemContext.DockerInsecureSkipTLSVerify == types.OptionalBoolFalse {
return nil, errors.Errorf("can't require tls verification on an insecured registry")
return nil, fmt.Errorf("can't require tls verification on an insecured registry")
}
}
if destinationInsecure != nil && *destinationInsecure {
if c.systemContext.DockerInsecureSkipTLSVerify == types.OptionalBoolFalse {
return nil, errors.Errorf("can't require tls verification on an insecured registry")
return nil, fmt.Errorf("can't require tls verification on an insecured registry")
}
}

Expand Down Expand Up @@ -402,7 +403,7 @@ func checkRegistrySourcesAllows(dest types.ImageReference) (insecure *bool, err
AllowedRegistries []string `json:"allowedRegistries,omitempty"`
}
if err := json.Unmarshal([]byte(registrySources), &sources); err != nil {
return nil, errors.Wrapf(err, "error parsing $BUILD_REGISTRY_SOURCES (%q) as JSON", registrySources)
return nil, fmt.Errorf("error parsing $BUILD_REGISTRY_SOURCES (%q) as JSON: %w", registrySources, err)
}
blocked := false
if len(sources.BlockedRegistries) > 0 {
Expand All @@ -413,7 +414,7 @@ func checkRegistrySourcesAllows(dest types.ImageReference) (insecure *bool, err
}
}
if blocked {
return nil, errors.Errorf("registry %q denied by policy: it is in the blocked registries list (%s)", reference.Domain(dref), registrySources)
return nil, fmt.Errorf("registry %q denied by policy: it is in the blocked registries list (%s)", reference.Domain(dref), registrySources)
}
allowed := true
if len(sources.AllowedRegistries) > 0 {
Expand All @@ -425,7 +426,7 @@ func checkRegistrySourcesAllows(dest types.ImageReference) (insecure *bool, err
}
}
if !allowed {
return nil, errors.Errorf("registry %q denied by policy: not in allowed registries list (%s)", reference.Domain(dref), registrySources)
return nil, fmt.Errorf("registry %q denied by policy: not in allowed registries list (%s)", reference.Domain(dref), registrySources)
}

for _, inseureDomain := range sources.InsecureRegistries {
Expand Down
13 changes: 6 additions & 7 deletions libimage/filters.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (
filtersPkg "github.com/containers/common/pkg/filters"
"github.com/containers/common/pkg/timetype"
"github.com/containers/image/v5/docker/reference"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
)

Expand Down Expand Up @@ -102,7 +101,7 @@ func (r *Runtime) compileImageFilters(ctx context.Context, options *ListImagesOp
} else {
split = strings.SplitN(f, "=", 2)
if len(split) != 2 {
return nil, errors.Errorf("invalid image filter %q: must be in the format %q", f, "filter=value or filter!=value")
return nil, fmt.Errorf("invalid image filter %q: must be in the format %q", f, "filter=value or filter!=value")
}
}

Expand Down Expand Up @@ -186,7 +185,7 @@ func (r *Runtime) compileImageFilters(ctx context.Context, options *ListImagesOp
filter = filterBefore(until)

default:
return nil, errors.Errorf("unsupported image filter %q", key)
return nil, fmt.Errorf("unsupported image filter %q", key)
}
if negate {
filter = negateFilter(filter)
Expand All @@ -206,7 +205,7 @@ func negateFilter(f filterFunc) filterFunc {

func (r *Runtime) containers(duplicate map[string]string, key, value string, externalFunc IsExternalContainerFunc) error {
if exists, ok := duplicate[key]; ok && exists != value {
return errors.Errorf("specifying %q filter more than once with different values is not supported", key)
return fmt.Errorf("specifying %q filter more than once with different values is not supported", key)
}
duplicate[key] = value
switch value {
Expand Down Expand Up @@ -237,19 +236,19 @@ func (r *Runtime) until(value string) (time.Time, error) {
func (r *Runtime) time(key, value string) (*Image, error) {
img, _, err := r.LookupImage(value, nil)
if err != nil {
return nil, errors.Wrapf(err, "could not find local image for filter filter %q=%q", key, value)
return nil, fmt.Errorf("could not find local image for filter filter %q=%q: %w", key, value, err)
}
return img, nil
}

func (r *Runtime) bool(duplicate map[string]string, key, value string) (bool, error) {
if exists, ok := duplicate[key]; ok && exists != value {
return false, errors.Errorf("specifying %q filter more than once with different values is not supported", key)
return false, fmt.Errorf("specifying %q filter more than once with different values is not supported", key)
}
duplicate[key] = value
set, err := strconv.ParseBool(value)
if err != nil {
return false, errors.Wrapf(err, "non-boolean value %q for %s filter", key, value)
return false, fmt.Errorf("non-boolean value %q for %s filter: %w", key, value, err)
}
return set, nil
}
Expand Down
49 changes: 23 additions & 26 deletions libimage/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package libimage

import (
"context"
"errors"
"fmt"
"path/filepath"
"sort"
Expand All @@ -16,7 +17,6 @@ import (
"github.com/hashicorp/go-multierror"
"github.com/opencontainers/go-digest"
ociv1 "github.com/opencontainers/image-spec/specs-go/v1"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
)

Expand Down Expand Up @@ -54,7 +54,7 @@ func (i *Image) reload() error {
logrus.Tracef("Reloading image %s", i.ID())
img, err := i.runtime.store.Image(i.ID())
if err != nil {
return errors.Wrap(err, "reloading image")
return fmt.Errorf("reloading image: %w", err)
}
i.storageImage = img
i.cached.imageSource = nil
Expand All @@ -81,7 +81,7 @@ func (i *Image) isCorrupted(name string) error {
if name == "" {
name = i.ID()[:12]
}
return errors.Errorf("Image %s exists in local storage but may be corrupted (remove the image to resolve the issue): %v", name, err)
return fmt.Errorf("Image %s exists in local storage but may be corrupted (remove the image to resolve the issue): %v", name, err)
}
return nil
}
Expand Down Expand Up @@ -195,7 +195,7 @@ func (i *Image) Labels(ctx context.Context) (map[string]string, error) {
if err != nil {
isManifestList, listErr := i.IsManifestList(ctx)
if listErr != nil {
err = errors.Wrapf(err, "fallback error checking whether image is a manifest list: %v", err)
err = fmt.Errorf("fallback error checking whether image is a manifest list: %v: %w", err, err)
} else if isManifestList {
logrus.Debugf("Ignoring error: cannot return labels for manifest list or image index %s", i.ID())
return nil, nil
Expand Down Expand Up @@ -305,7 +305,7 @@ func (i *Image) removeContainers(options *RemoveImagesOptions) error {
for _, cID := range containers {
if err := i.runtime.store.DeleteContainer(cID); err != nil {
// If the container does not exist anymore, we're good.
if errors.Cause(err) != storage.ErrContainerUnknown {
if !errors.Is(err, storage.ErrContainerUnknown) {
multiE = multierror.Append(multiE, err)
}
}
Expand Down Expand Up @@ -361,7 +361,7 @@ func (i *Image) removeRecursive(ctx context.Context, rmMap map[string]*RemoveIma
logrus.Debugf("Removing image %s", i.ID())

if i.IsReadOnly() {
return processedIDs, errors.Errorf("cannot remove read-only image %q", i.ID())
return processedIDs, fmt.Errorf("cannot remove read-only image %q", i.ID())
}

if i.runtime.eventChannel != nil {
Expand All @@ -384,15 +384,12 @@ func (i *Image) removeRecursive(ctx context.Context, rmMap map[string]*RemoveIma
// have a closer look at the errors. On top, image removal should be
// tolerant toward corrupted images.
handleError := func(err error) error {
switch errors.Cause(err) {
case storage.ErrImageUnknown, storage.ErrNotAnImage, storage.ErrLayerUnknown:
// The image or layers of the image may already
// have been removed in which case we consider
// the image to be removed.
if errors.Is(err, storage.ErrImageUnknown) || errors.Is(err, storage.ErrNotAnImage) || errors.Is(err, storage.ErrLayerUnknown) {
// The image or layers of the image may already have been removed
// in which case we consider the image to be removed.
return nil
default:
return err
}
return err
}

// Calculate the size if requested. `podman-image-prune` likes to
Expand Down Expand Up @@ -421,11 +418,11 @@ func (i *Image) removeRecursive(ctx context.Context, rmMap map[string]*RemoveIma
byDigest := strings.HasPrefix(referencedBy, "sha256:")
if !options.Force {
if byID && numNames > 1 {
return processedIDs, errors.Errorf("unable to delete image %q by ID with more than one tag (%s): please force removal", i.ID(), i.Names())
return processedIDs, fmt.Errorf("unable to delete image %q by ID with more than one tag (%s): please force removal", i.ID(), i.Names())
} else if byDigest && numNames > 1 {
// FIXME - Docker will remove the digest but containers storage
// does not support that yet, so our hands are tied.
return processedIDs, errors.Errorf("unable to delete image %q by digest with more than one tag (%s): please force removal", i.ID(), i.Names())
return processedIDs, fmt.Errorf("unable to delete image %q by digest with more than one tag (%s): please force removal", i.ID(), i.Names())
}
}

Expand Down Expand Up @@ -509,16 +506,16 @@ var errTagDigest = errors.New("tag by digest not supported")
// storage. The name is normalized according to the rules of NormalizeName.
func (i *Image) Tag(name string) error {
if strings.HasPrefix(name, "sha256:") { // ambiguous input
return errors.Wrap(errTagDigest, name)
return fmt.Errorf("%s: %w", name, errTagDigest)
}

ref, err := NormalizeName(name)
if err != nil {
return errors.Wrapf(err, "normalizing name %q", name)
return fmt.Errorf("normalizing name %q: %w", name, err)
}

if _, isDigested := ref.(reference.Digested); isDigested {
return errors.Wrap(errTagDigest, name)
return fmt.Errorf("%s: %w", name, errTagDigest)
}

logrus.Debugf("Tagging image %s with %q", i.ID(), ref.String())
Expand Down Expand Up @@ -546,12 +543,12 @@ var errUntagDigest = errors.New("untag by digest not supported")
// of NormalizeName.
func (i *Image) Untag(name string) error {
if strings.HasPrefix(name, "sha256:") { // ambiguous input
return errors.Wrap(errUntagDigest, name)
return fmt.Errorf("%s: %w", name, errUntagDigest)
}

ref, err := NormalizeName(name)
if err != nil {
return errors.Wrapf(err, "normalizing name %q", name)
return fmt.Errorf("normalizing name %q: %w", name, err)
}

// FIXME: this is breaking Podman CI but must be re-enabled once
Expand All @@ -560,9 +557,9 @@ func (i *Image) Untag(name string) error {
//
// !!! Also make sure to re-enable the tests !!!
//
// if _, isDigested := ref.(reference.Digested); isDigested {
// return errors.Wrap(errUntagDigest, name)
// }
// if _, isDigested := ref.(reference.Digested); isDigested {
// return fmt.Errorf("%s: %w", name, errUntagDigest)
// }

name = ref.String()

Expand All @@ -582,7 +579,7 @@ func (i *Image) Untag(name string) error {
}

if !removedName {
return errors.Wrap(errTagUnknown, name)
return fmt.Errorf("%s: %w", name, errTagUnknown)
}

if err := i.runtime.store.SetNames(i.ID(), newNames); err != nil {
Expand Down Expand Up @@ -731,7 +728,7 @@ func (i *Image) Mount(ctx context.Context, mountOptions []string, mountLabel str
func (i *Image) Mountpoint() (string, error) {
mountedTimes, err := i.runtime.store.Mounted(i.TopLayer())
if err != nil || mountedTimes == 0 {
if errors.Cause(err) == storage.ErrLayerUnknown {
if errors.Is(err, storage.ErrLayerUnknown) {
// Can happen, Podman did it, but there's no
// explanation why.
err = nil
Expand Down Expand Up @@ -943,7 +940,7 @@ func getImageID(ctx context.Context, src types.ImageReference, sys *types.System
}()
imageDigest := newImg.ConfigInfo().Digest
if err = imageDigest.Validate(); err != nil {
return "", errors.Wrapf(err, "getting config info")
return "", fmt.Errorf("getting config info: %w", err)
}
return "@" + imageDigest.Encoded(), nil
}
Loading