github/skopeo
1
0
Fork 0
mirror of https://github.com/containers/skopeo.git synced 2025-08-30 21:25:11 +00:00
Code Issues Projects Releases Wiki Activity

Merge pull request #1698 from mtrmac/pkg_errors

Browse Source 
Error handling cleanups, and drop pkg/errors
This commit is contained in:
Daniel J Walsh 2022-07-01 07:02:23 -04:00 committed by GitHub
commit f75d570709
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 105 additions and 60 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
cmd/skopeo/copy.go
View File

@ -130,7 +130,7 @@ func (opts *copyOptions) run(args []string, stdout io.Writer) (retErr error) {
}
defer func() {
if err := policyContext.Destroy(); err != nil {
retErr = fmt.Errorf("(error tearing down policy context: %v): %w", err, retErr)
retErr = noteCloseFailure(retErr, "tearing down policy context", err)
}
}()

24
cmd/skopeo/inspect.go
View File

@ -2,6 +2,7 @@ package main
import (
"encoding/json"
"errors"
"fmt"
"io"
"os"
@ -18,7 +19,6 @@ import (
"github.com/containers/image/v5/types"
"github.com/containers/skopeo/cmd/skopeo/inspect"
v1 "github.com/opencontainers/image-spec/specs-go/v1"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
"github.com/spf13/cobra"
)
@ -100,12 +100,12 @@ func (opts *inspectOptions) run(args []string, stdout io.Writer) (retErr error)
src, err = parseImageSource(ctx, opts.image, imageName)
return err
}, opts.retryOpts); err != nil {
return errors.Wrapf(err, "Error parsing image name %q", imageName)
return fmt.Errorf("Error parsing image name %q: %w", imageName, err)
}
defer func() {
if err := src.Close(); err != nil {
retErr = errors.Wrapf(retErr, fmt.Sprintf("(could not close image: %v) ", err))
retErr = noteCloseFailure(retErr, "closing image", err)
}
}()
@ -113,13 +113,13 @@ func (opts *inspectOptions) run(args []string, stdout io.Writer) (retErr error)
rawManifest, _, err = src.GetManifest(ctx, nil)
return err
}, opts.retryOpts); err != nil {
return errors.Wrapf(err, "Error retrieving manifest for image")
return fmt.Errorf("Error retrieving manifest for image: %w", err)
}
if opts.raw && !opts.config {
_, err := stdout.Write(rawManifest)
if err != nil {
return fmt.Errorf("Error writing manifest to standard output: %v", err)
return fmt.Errorf("Error writing manifest to standard output: %w", err)
}
return nil
@ -127,7 +127,7 @@ func (opts *inspectOptions) run(args []string, stdout io.Writer) (retErr error)
img, err := image.FromUnparsedImage(ctx, sys, image.UnparsedInstance(src, nil))
if err != nil {
return errors.Wrapf(err, "Error parsing manifest for image")
return fmt.Errorf("Error parsing manifest for image: %w", err)
}
if opts.config && opts.raw {
@ -136,11 +136,11 @@ func (opts *inspectOptions) run(args []string, stdout io.Writer) (retErr error)
configBlob, err = img.ConfigBlob(ctx)
return err
}, opts.retryOpts); err != nil {
return errors.Wrapf(err, "Error reading configuration blob")
return fmt.Errorf("Error reading configuration blob: %w", err)
}
_, err = stdout.Write(configBlob)
if err != nil {
return errors.Wrapf(err, "Error writing configuration blob to standard output")
return fmt.Errorf("Error writing configuration blob to standard output: %w", err)
}
return nil
} else if opts.config {
@ -149,7 +149,7 @@ func (opts *inspectOptions) run(args []string, stdout io.Writer) (retErr error)
config, err = img.OCIConfig(ctx)
return err
}, opts.retryOpts); err != nil {
return errors.Wrapf(err, "Error reading OCI-formatted configuration data")
return fmt.Errorf("Error reading OCI-formatted configuration data: %w", err)
}
if report.IsJSON(opts.format) || opts.format == "" {
var out []byte
@ -163,7 +163,7 @@ func (opts *inspectOptions) run(args []string, stdout io.Writer) (retErr error)
err = printTmpl(row, data)
}
if err != nil {
return errors.Wrapf(err, "Error writing OCI-formatted configuration data to standard output")
return fmt.Errorf("Error writing OCI-formatted configuration data to standard output: %w", err)
}
return nil
}
@ -190,7 +190,7 @@ func (opts *inspectOptions) run(args []string, stdout io.Writer) (retErr error)
}
outputData.Digest, err = manifest.Digest(rawManifest)
if err != nil {
return errors.Wrapf(err, "Error computing manifest digest")
return fmt.Errorf("Error computing manifest digest: %w", err)
}
if dockerRef := img.Reference().DockerReference(); dockerRef != nil {
outputData.Name = dockerRef.Name()
@ -208,7 +208,7 @@ func (opts *inspectOptions) run(args []string, stdout io.Writer) (retErr error)
// In addition, AWS ECR rejects it with 403 (Forbidden) if the "ecr:ListImages"
// action is not allowed.
if !strings.Contains(err.Error(), "401") && !strings.Contains(err.Error(), "403") {
return errors.Wrapf(err, "Error determining repository tags")
return fmt.Errorf("Error determining repository tags: %w", err)
}
logrus.Warnf("Registry disallows tag list retrieval; skipping")
}

10
cmd/skopeo/layers.go
View File

@ -1,6 +1,7 @@
package main
import (
"errors"
"fmt"
"io"
"os"
@ -12,7 +13,6 @@ import (
"github.com/containers/image/v5/pkg/blobinfocache"
"github.com/containers/image/v5/types"
"github.com/opencontainers/go-digest"
"github.com/pkg/errors"
"github.com/spf13/cobra"
)
@ -79,14 +79,14 @@ func (opts *layersOptions) run(args []string, stdout io.Writer) (retErr error) {
return err
}, opts.retryOpts); err != nil {
if closeErr := rawSource.Close(); closeErr != nil {
return errors.Wrapf(err, " (close error: %v)", closeErr)
return fmt.Errorf("%w (closing image source: %v)", err, closeErr)
}
return err
}
defer func() {
if err := src.Close(); err != nil {
retErr = errors.Wrapf(retErr, " (close error: %v)", err)
retErr = noteCloseFailure(retErr, "closing image", err)
}
}()
@ -136,7 +136,7 @@ func (opts *layersOptions) run(args []string, stdout io.Writer) (retErr error) {
defer func() {
if err := dest.Close(); err != nil {
retErr = errors.Wrapf(retErr, " (close error: %v)", err)
retErr = noteCloseFailure(retErr, "closing destination", err)
}
}()
@ -153,7 +153,7 @@ func (opts *layersOptions) run(args []string, stdout io.Writer) (retErr error) {
}
if _, err := dest.PutBlob(ctx, r, types.BlobInfo{Digest: bd.digest, Size: blobSize}, cache, bd.isConfig); err != nil {
if closeErr := r.Close(); closeErr != nil {
return errors.Wrapf(err, " (close error: %v)", closeErr)
return fmt.Errorf("%w (close error: %v)", err, closeErr)
}
return err
}

8
cmd/skopeo/list_tags.go
View File

@ -3,6 +3,7 @@ package main
import (
"context"
"encoding/json"
"errors"
"fmt"
"io"
"sort"
@ -14,7 +15,6 @@ import (
"github.com/containers/image/v5/docker/reference"
"github.com/containers/image/v5/transports/alltransports"
"github.com/containers/image/v5/types"
"github.com/pkg/errors"
"github.com/spf13/cobra"
)
@ -81,12 +81,12 @@ See skopeo-list-tags(1) section "REPOSITORY NAMES" for the expected format
// Would really love to not have this, but needed to enforce tag-less and digest-less names
func parseDockerRepositoryReference(refString string) (types.ImageReference, error) {
if !strings.HasPrefix(refString, docker.Transport.Name()+"://") {
return nil, errors.Errorf("docker: image reference %s does not start with %s://", refString, docker.Transport.Name())
return nil, fmt.Errorf("docker: image reference %s does not start with %s://", refString, docker.Transport.Name())
}
parts := strings.SplitN(refString, ":", 2)
if len(parts) != 2 {
return nil, errors.Errorf(`Invalid image name "%s", expected colon-separated transport:reference`, refString)
return nil, fmt.Errorf(`Invalid image name "%s", expected colon-separated transport:reference`, refString)
}
ref, err := reference.ParseNormalizedNamed(strings.TrimPrefix(parts[1], "//"))
@ -108,7 +108,7 @@ func listDockerTags(ctx context.Context, sys *types.SystemContext, imgRef types.
tags, err := docker.GetRepositoryTags(ctx, sys, imgRef)
if err != nil {
return ``, nil, fmt.Errorf("Error listing repository tags: %v", err)
return ``, nil, fmt.Errorf("Error listing repository tags: %w", err)
}
return repositoryName, tags, nil
}

66
cmd/skopeo/sync.go
View File

@ -2,6 +2,7 @@ package main
import (
"context"
"errors"
"fmt"
"io"
"io/fs"
@ -21,7 +22,6 @@ import (
"github.com/containers/image/v5/transports"
"github.com/containers/image/v5/types"
"github.com/opencontainers/go-digest"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
"github.com/spf13/cobra"
"gopkg.in/yaml.v2"
@ -146,7 +146,7 @@ func newSourceConfig(yamlFile string) (sourceConfig, error) {
}
err = yaml.Unmarshal(source, &cfg)
if err != nil {
return cfg, errors.Wrapf(err, "Failed to unmarshal %q", yamlFile)
return cfg, fmt.Errorf("Failed to unmarshal %q: %w", yamlFile, err)
}
return cfg, nil
}
@ -158,7 +158,7 @@ func parseRepositoryReference(input string) (reference.Named, error) {
return nil, err
}
if !reference.IsNameOnly(ref) {
return nil, errors.Errorf("input names a reference, not a repository")
return nil, errors.New("input names a reference, not a repository")
}
return ref, nil
}
@ -176,24 +176,24 @@ func destinationReference(destination string, transport string) (types.ImageRefe
case directory.Transport.Name():
_, err := os.Stat(destination)
if err == nil {
return nil, errors.Errorf("Refusing to overwrite destination directory %q", destination)
return nil, fmt.Errorf("Refusing to overwrite destination directory %q", destination)
}
if !os.IsNotExist(err) {
return nil, errors.Wrap(err, "Destination directory could not be used")
return nil, fmt.Errorf("Destination directory could not be used: %w", err)
}
// the directory holding the image must be created here
if err = os.MkdirAll(destination, 0755); err != nil {
return nil, errors.Wrapf(err, "Error creating directory for image %s", destination)
return nil, fmt.Errorf("Error creating directory for image %s: %w", destination, err)
}
imageTransport = directory.Transport
default:
return nil, errors.Errorf("%q is not a valid destination transport", transport)
return nil, fmt.Errorf("%q is not a valid destination transport", transport)
}
logrus.Debugf("Destination for transport %q: %s", transport, destination)
destRef, err := imageTransport.ParseReference(destination)
if err != nil {
return nil, errors.Wrapf(err, "Cannot obtain a valid image reference for transport %q and reference %q", imageTransport.Name(), destination)
return nil, fmt.Errorf("Cannot obtain a valid image reference for transport %q and reference %q: %w", imageTransport.Name(), destination, err)
}
return destRef, nil
@ -213,16 +213,16 @@ func getImageTags(ctx context.Context, sysCtx *types.SystemContext, repoRef refe
return nil, err // Should never happen for a reference with tag and no digest
}
tags, err := docker.GetRepositoryTags(ctx, sysCtx, dockerRef)
switch err := err.(type) {
case nil:
break
case docker.ErrUnauthorizedForCredentials:
// Some registries may decide to block the "list all tags" endpoint.
// Gracefully allow the sync to continue in this case.
logrus.Warnf("Registry disallows tag list retrieval: %s", err)
default:
return tags, errors.Wrapf(err, "Error determining repository tags for image %s", name)
if err != nil {
var unauthorizedForCredentials docker.ErrUnauthorizedForCredentials
if errors.As(err, &unauthorizedForCredentials) {
// Some registries may decide to block the "list all tags" endpoint.
// Gracefully allow the sync to continue in this case.
logrus.Warnf("Registry disallows tag list retrieval: %s", err)
tags = nil
} else {
return nil, fmt.Errorf("Error determining repository tags for image %s: %w", name, err)
}
}
return tags, nil
@ -242,11 +242,11 @@ func imagesToCopyFromRepo(sys *types.SystemContext, repoRef reference.Named) ([]
for _, tag := range tags {
taggedRef, err := reference.WithTag(repoRef, tag)
if err != nil {
return nil, errors.Wrapf(err, "Error creating a reference for repository %s and tag %q", repoRef.Name(), tag)
return nil, fmt.Errorf("Error creating a reference for repository %s and tag %q: %w", repoRef.Name(), tag, err)
}
ref, err := docker.NewReference(taggedRef)
if err != nil {
return nil, errors.Wrapf(err, "Cannot obtain a valid image reference for transport %q and reference %s", docker.Transport.Name(), taggedRef.String())
return nil, fmt.Errorf("Cannot obtain a valid image reference for transport %q and reference %s: %w", docker.Transport.Name(), taggedRef.String(), err)
}
sourceReferences = append(sourceReferences, ref)
}
@ -267,7 +267,7 @@ func imagesToCopyFromDir(dirPath string) ([]types.ImageReference, error) {
dirname := filepath.Dir(path)
ref, err := directory.Transport.ParseReference(dirname)
if err != nil {
return errors.Wrapf(err, "Cannot obtain a valid image reference for transport %q and reference %q", directory.Transport.Name(), dirname)
return fmt.Errorf("Cannot obtain a valid image reference for transport %q and reference %q: %w", directory.Transport.Name(), dirname, err)
}
sourceReferences = append(sourceReferences, ref)
return filepath.SkipDir
@ -277,7 +277,7 @@ func imagesToCopyFromDir(dirPath string) ([]types.ImageReference, error) {
if err != nil {
return sourceReferences,
errors.Wrapf(err, "Error walking the path %q", dirPath)
fmt.Errorf("Error walking the path %q: %w", dirPath, err)
}
return sourceReferences, nil
@ -435,7 +435,7 @@ func imagesToCopy(source string, transport string, sourceCtx *types.SystemContex
}
named, err := reference.ParseNormalizedNamed(source) // May be a repository or an image.
if err != nil {
return nil, errors.Wrapf(err, "Cannot obtain a valid image reference for transport %q and reference %q", docker.Transport.Name(), source)
return nil, fmt.Errorf("Cannot obtain a valid image reference for transport %q and reference %q: %w", docker.Transport.Name(), source, err)
}
imageTagged := !reference.IsNameOnly(named)
logrus.WithFields(logrus.Fields{
@ -445,7 +445,7 @@ func imagesToCopy(source string, transport string, sourceCtx *types.SystemContex
if imageTagged {
srcRef, err := docker.NewReference(named)
if err != nil {
return nil, errors.Wrapf(err, "Cannot obtain a valid image reference for transport %q and reference %q", docker.Transport.Name(), named.String())
return nil, fmt.Errorf("Cannot obtain a valid image reference for transport %q and reference %q: %w", docker.Transport.Name(), named.String(), err)
}
desc.ImageRefs = []types.ImageReference{srcRef}
} else {
@ -454,7 +454,7 @@ func imagesToCopy(source string, transport string, sourceCtx *types.SystemContex
return descriptors, err
}
if len(desc.ImageRefs) == 0 {
return descriptors, errors.Errorf("No images to sync found in %q", source)
return descriptors, fmt.Errorf("No images to sync found in %q", source)
}
}
descriptors = append(descriptors, desc)
@ -465,7 +465,7 @@ func imagesToCopy(source string, transport string, sourceCtx *types.SystemContex
}
if _, err := os.Stat(source); err != nil {
return descriptors, errors.Wrap(err, "Invalid source directory specified")
return descriptors, fmt.Errorf("Invalid source directory specified: %w", err)
}
desc.DirBasePath = source
var err error
@ -474,7 +474,7 @@ func imagesToCopy(source string, transport string, sourceCtx *types.SystemContex
return descriptors, err
}
if len(desc.ImageRefs) == 0 {
return descriptors, errors.Errorf("No images to sync found in %q", source)
return descriptors, fmt.Errorf("No images to sync found in %q", source)
}
descriptors = append(descriptors, desc)
@ -493,7 +493,7 @@ func imagesToCopy(source string, transport string, sourceCtx *types.SystemContex
descs, err := imagesToCopyFromRegistry(registryName, registryConfig, *sourceCtx)
if err != nil {
return descriptors, errors.Wrapf(err, "Failed to retrieve list of images from registry %q", registryName)
return descriptors, fmt.Errorf("Failed to retrieve list of images from registry %q: %w", registryName, err)
}
descriptors = append(descriptors, descs...)
}
@ -510,11 +510,11 @@ func (opts *syncOptions) run(args []string, stdout io.Writer) (retErr error) {
policyContext, err := opts.global.getPolicyContext()
if err != nil {
return errors.Wrapf(err, "Error loading trust policy")
return fmt.Errorf("Error loading trust policy: %w", err)
}
defer func() {
if err := policyContext.Destroy(); err != nil {
retErr = fmt.Errorf("(error tearing down policy context: %v): %w", err, retErr)
retErr = noteCloseFailure(retErr, "tearing down policy context", err)
}
}()
@ -532,14 +532,14 @@ func (opts *syncOptions) run(args []string, stdout io.Writer) (retErr error) {
return errors.New("A source transport must be specified")
}
if !contains(opts.source, []string{docker.Transport.Name(), directory.Transport.Name(), "yaml"}) {
return errors.Errorf("%q is not a valid source transport", opts.source)
return fmt.Errorf("%q is not a valid source transport", opts.source)
}
if len(opts.destination) == 0 {
return errors.New("A destination transport must be specified")
}
if !contains(opts.destination, []string{docker.Transport.Name(), directory.Transport.Name()}) {
return errors.Errorf("%q is not a valid destination transport", opts.destination)
return fmt.Errorf("%q is not a valid destination transport", opts.destination)
}
if opts.source == opts.destination && opts.source == directory.Transport.Name() {
@ -642,7 +642,7 @@ func (opts *syncOptions) run(args []string, stdout io.Writer) (retErr error) {
return err
}, opts.retryOpts); err != nil {
if !opts.keepGoing {
return errors.Wrapf(err, "Error copying ref %q", transports.ImageName(ref))
return fmt.Errorf("Error copying ref %q: %w", transports.ImageName(ref), err)
}
// log the error, keep a note that there was a failure and move on to the next
// image ref

5
cmd/skopeo/unshare_linux.go
View File

@ -1,9 +1,10 @@
package main
import (
"fmt"
"github.com/containers/image/v5/transports/alltransports"
"github.com/containers/storage/pkg/unshare"
"github.com/pkg/errors"
"github.com/syndtr/gocapability/capability"
)
@ -22,7 +23,7 @@ func maybeReexec() error {
// if we already have the capabilities we need.
capabilities, err := capability.NewPid(0)
if err != nil {
return errors.Wrapf(err, "error reading the current capabilities sets")
return fmt.Errorf("error reading the current capabilities sets: %w", err)
}
for _, cap := range neededCapabilities {
if !capabilities.Get(capability.EFFECTIVE, cap) {

26
cmd/skopeo/utils.go
View File

@ -2,6 +2,7 @@ package main
import (
"context"
"errors"
"fmt"
"io"
"os"
@ -14,7 +15,6 @@ import (
"github.com/containers/image/v5/transports/alltransports"
"github.com/containers/image/v5/types"
imgspecv1 "github.com/opencontainers/image-spec/specs-go/v1"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
"github.com/spf13/cobra"
"github.com/spf13/pflag"
@ -25,6 +25,27 @@ type errorShouldDisplayUsage struct {
error
}
// noteCloseFailure returns (possibly-nil) err modified to account for (non-nil) closeErr.
// The error for closeErr is annotated with description (which is not a format string)
// Typical usage:
//
// defer func() {
// if err := something.Close(); err != nil {
// returnedErr = noteCloseFailure(returnedErr, "closing something", err)
// }
// }
func noteCloseFailure(err error, description string, closeErr error) error {
// We don’t accept a Closer() and close it ourselves because signature.PolicyContext has .Destroy(), not .Close().
// This also makes it harder for a caller to do
// defer noteCloseFailure(returnedErr, …)
// which doesn’t use the right value of returnedErr, and doesn’t update it.
if err == nil {
return fmt.Errorf("%s: %w", description, closeErr)
}
// In this case we prioritize the primary error for use with %w; closeErr is usually less relevant, or might be a consequence of the primary erorr.
return fmt.Errorf("%w (%s: %v)", err, description, closeErr)
}
// commandAction intermediates between the RunE interface and the real handler,
// primarily to ensure that cobra.Command is not available to the handler, which in turn
// makes sure that the cmd.Flags() etc. flag access functions are not used,
@ -33,7 +54,8 @@ type errorShouldDisplayUsage struct {
func commandAction(handler func(args []string, stdout io.Writer) error) func(cmd *cobra.Command, args []string) error {
return func(c *cobra.Command, args []string) error {
err := handler(args, c.OutOrStdout())
if _, ok := err.(errorShouldDisplayUsage); ok {
var shouldDisplayUsage errorShouldDisplayUsage
if errors.As(err, &shouldDisplayUsage) {
return c.Help()
}
return err

22
cmd/skopeo/utils_test.go
View File

@ -1,6 +1,7 @@
package main
import (
"errors"
"testing"
"github.com/containers/image/v5/manifest"
@ -12,6 +13,27 @@ import (
"github.com/stretchr/testify/require"
)
func TestNoteCloseFailure(t *testing.T) {
const description = "description"
mainErr := errors.New("main")
closeErr := errors.New("closing")
// Main success, closing failed
res := noteCloseFailure(nil, description, closeErr)
require.NotNil(t, res)
assert.Contains(t, res.Error(), description)
assert.Contains(t, res.Error(), closeErr.Error())
// Both main and closing failed
res = noteCloseFailure(mainErr, description, closeErr)
require.NotNil(t, res)
assert.Contains(t, res.Error(), mainErr.Error())
assert.Contains(t, res.Error(), description)
assert.Contains(t, res.Error(), closeErr.Error())
assert.ErrorIs(t, res, mainErr)
}
// fakeGlobalOptions creates globalOptions and sets it according to flags.
func fakeGlobalOptions(t *testing.T, flags []string) (*globalOptions, *cobra.Command) {
app, opts := createApp()

2
go.mod
View File

@ -11,7 +11,6 @@ require (
github.com/opencontainers/go-digest v1.0.0
github.com/opencontainers/image-spec v1.0.3-0.20211202193544-a5463b7f9c84
github.com/opencontainers/image-tools v1.0.0-rc3
github.com/pkg/errors v0.9.1
github.com/sirupsen/logrus v1.8.1
github.com/spf13/cobra v1.5.0
github.com/spf13/pflag v1.0.5
@ -68,6 +67,7 @@ require (
github.com/opencontainers/runtime-spec v1.0.3-0.20210326190908-1c3f411f0417 // indirect
github.com/opencontainers/selinux v1.10.1 // indirect
github.com/ostreedev/ostree-go v0.0.0-20210805093236-719684c64e4f // indirect
github.com/pkg/errors v0.9.1 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
github.com/proglottis/gpgme v0.1.2 // indirect
github.com/prometheus/client_golang v1.11.1 // indirect