CI: enable errorlint

.golangci.yml

@@ -2,3 +2,7 @@
 run:
   concurrency: 6
   timeout: 5m
+
+linters:
+  enable:
+    - errorlint

copy/copy.go

@@ -200,11 +200,7 @@ func Image(ctx context.Context, policyContext *signature.PolicyContext, destRef,
 	dest := imagedestination.FromPublic(publicDest)
 	defer func() {
 		if err := dest.Close(); err != nil {
-			if retErr != nil {
-				retErr = fmt.Errorf(" (dest: %v): %w", err, retErr)
-			} else {
-				retErr = fmt.Errorf(" (dest: %v)", err)
-			}
+			retErr = errors.Join(retErr, fmt.Errorf("dest: %w", err))
 		}
 	}()
 
@@ -215,11 +211,7 @@ func Image(ctx context.Context, policyContext *signature.PolicyContext, destRef,
 	rawSource := imagesource.FromPublic(publicRawSource)
 	defer func() {
 		if err := rawSource.Close(); err != nil {
-			if retErr != nil {
-				retErr = fmt.Errorf(" (src: %v): %w", err, retErr)
-			} else {
-				retErr = fmt.Errorf(" (src: %v)", err)
-			}
+			retErr = errors.Join(retErr, fmt.Errorf("src: %w", err))
 		}
 	}()
 

directory/directory_dest.go

@@ -1,21 +1,22 @@
 package directory
 
 import (
+	"bytes"
 	"context"
 	"errors"
 	"fmt"
 	"io"
 	"os"
 	"path/filepath"
 	"runtime"
+	"syscall"
 
 	"github.com/containers/image/v5/internal/imagedestination/impl"
 	"github.com/containers/image/v5/internal/imagedestination/stubs"
 	"github.com/containers/image/v5/internal/private"
 	"github.com/containers/image/v5/internal/putblobdigest"
 	"github.com/containers/image/v5/internal/signature"
 	"github.com/containers/image/v5/types"
-	"github.com/containers/storage/pkg/fileutils"
 	"github.com/opencontainers/go-digest"
 	"github.com/sirupsen/logrus"
 )
@@ -54,49 +55,40 @@ func newImageDestination(sys *types.SystemContext, ref dirReference) (private.Im
 	// If directory exists check if it is empty
 	// if not empty, check whether the contents match that of a container image directory and overwrite the contents
 	// if the contents don't match throw an error
-	dirExists, err := pathExists(ref.resolvedPath)
-	if err != nil {
-		return nil, fmt.Errorf("checking for path %q: %w", ref.resolvedPath, err)
-	}
-	if dirExists {
-		isEmpty, err := isDirEmpty(ref.resolvedPath)
-		if err != nil {
+	dir, err := os.OpenFile(ref.resolvedPath, syscall.O_DIRECTORY|syscall.O_NOFOLLOW|os.O_RDONLY, 0)
+	switch {
+	case err == nil: // Directory exists.
+		contents, err := dir.Readdirnames(-1)
+		_ = dir.Close()
+		if err != nil { // Unexpected error.
 			return nil, err
 		}
-
-		if !isEmpty {
-			versionExists, err := pathExists(ref.versionPath())
-			if err != nil {
-				return nil, fmt.Errorf("checking if path exists %q: %w", ref.versionPath(), err)
-			}
-			if versionExists {
-				contents, err := os.ReadFile(ref.versionPath())
-				if err != nil {
-					return nil, err
-				}
-				// check if contents of version file is what we expect it to be
-				if string(contents) != version {
-					return nil, ErrNotContainerImageDir
+		if len(contents) > 0 { // Not empty.
+			// Check if contents of version file is what we expect it to be.
+			ver, err := os.ReadFile(ref.versionPath())
+			if err == nil && bytes.Equal(ver, []byte(version)) {
+				// Definitely an image directory. Reuse by removing all the old contents.
+				logrus.Debugf("overwriting existing container image directory %q", ref.resolvedPath)
+				for _, name := range contents {
+					if os.RemoveAll(filepath.Join(ref.resolvedPath, name)) != nil {
+						return nil, err
+					}
 				}
-			} else {
-				return nil, ErrNotContainerImageDir
-			}
-			// delete directory contents so that only one image is in the directory at a time
-			if err = removeDirContents(ref.resolvedPath); err != nil {
-				return nil, fmt.Errorf("erasing contents in %q: %w", ref.resolvedPath, err)
 			}
-			logrus.Debugf("overwriting existing container image directory %q", ref.resolvedPath)
 		}
-	} else {
-		// create directory if it doesn't exist
-		if err := os.MkdirAll(ref.resolvedPath, 0755); err != nil {
-			return nil, fmt.Errorf("unable to create directory %q: %w", ref.resolvedPath, err)
+	case errors.Is(err, os.ErrNotExist): // Directory does not exist; create it.
+		if err := os.MkdirAll(ref.resolvedPath, 0o755); err != nil {
+			return nil, err
 		}
+	default:
+		// Unexpected error.
+		return nil, err
 	}
-	// create version file
-	err = os.WriteFile(ref.versionPath(), []byte(version), 0644)
+
+	// Create version file.
+	err = os.WriteFile(ref.versionPath(), []byte(version), 0o644)
 	if err != nil {
-		return nil, fmt.Errorf("creating version file %q: %w", ref.versionPath(), err)
+		return nil, err
 	}
 
 	d := &dirImageDestination{
@@ -261,39 +253,3 @@ func (d *dirImageDestination) PutSignaturesWithFormat(ctx context.Context, signa
 func (d *dirImageDestination) Commit(context.Context, types.UnparsedImage) error {
 	return nil
 }
-
-// returns true if path exists
-func pathExists(path string) (bool, error) {
-	err := fileutils.Exists(path)
-	if err == nil {
-		return true, nil
-	}
-	if os.IsNotExist(err) {
-		return false, nil
-	}
-	return false, err
-}
-
-// returns true if directory is empty
-func isDirEmpty(path string) (bool, error) {
-	files, err := os.ReadDir(path)
-	if err != nil {
-		return false, err
-	}
-	return len(files) == 0, nil
-}
-
-// deletes the contents of a directory
-func removeDirContents(path string) error {
-	files, err := os.ReadDir(path)
-	if err != nil {
-		return err
-	}
-
-	for _, file := range files {
-		if err := os.RemoveAll(filepath.Join(path, file.Name())); err != nil {
-			return err
-		}
-	}
-	return nil
-}

directory/directory_transport_test.go

@@ -181,7 +181,7 @@ func TestReferenceNewImageSource(t *testing.T) {
 func TestReferenceNewImageDestination(t *testing.T) {
 	ref, _ := refToTempDir(t)
 	dest, err := ref.NewImageDestination(context.Background(), nil)
-	assert.NoError(t, err)
+	require.NoError(t, err)
 	defer dest.Close()
 }
 

docker/archive/writer.go

@@ -34,7 +34,7 @@ func NewWriter(sys *types.SystemContext, path string) (*Writer, error) {
 	// only in a different way. Either way, it’s up to the user to not have two writers to the same path.)
 	fh, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE, 0644)
 	if err != nil {
-		return nil, fmt.Errorf("opening file %q: %w", path, err)
+		return nil, err
 	}
 	succeeded := false
 	defer func() {

docker/body_reader.go

@@ -165,7 +165,7 @@ func (br *bodyReader) Read(p []byte) (int, error) {
 		}
 		res, err := br.c.makeRequest(br.ctx, http.MethodGet, br.path, headers, nil, v2Auth, nil)
 		if err != nil {
-			return n, fmt.Errorf("%w (while reconnecting: %v)", originalErr, err)
+			return n, fmt.Errorf("%w (while reconnecting: %w)", originalErr, err)
 		}
 		consumedBody := false
 		defer func() {
@@ -179,7 +179,7 @@ func (br *bodyReader) Read(p []byte) (int, error) {
 			// The recipient of an invalid Content-Range MUST NOT attempt to recombine the received content with a stored representation.
 			first, last, completeLength, err := parseContentRange(res)
 			if err != nil {
-				return n, fmt.Errorf("%w (after reconnecting, invalid Content-Range header: %v)", originalErr, err)
+				return n, fmt.Errorf("%w (after reconnecting, invalid Content-Range header: %w)", originalErr, err)
 			}
 			// We don’t handle responses that start at an unrequested offset, nor responses that terminate before the end of the full blob.
 			if first != br.offset || (completeLength != -1 && last+1 != completeLength) {
@@ -190,7 +190,7 @@ func (br *bodyReader) Read(p []byte) (int, error) {
 			return n, fmt.Errorf("%w (after reconnecting, server did not process a Range: header, status %d)", originalErr, http.StatusOK)
 		default:
 			err := registryHTTPResponseToError(res)
-			return n, fmt.Errorf("%w (after reconnecting, fetching blob: %v)", originalErr, err)
+			return n, fmt.Errorf("%w (after reconnecting, fetching blob: %w)", originalErr, err)
 		}
 
 		logrus.Debugf("Successfully reconnected to %s", redactedURL)

docker/distribution_error.go

@@ -99,7 +99,7 @@ func parseHTTPErrorResponse(statusCode int, r io.Reader) error {
 }
 
 func makeErrorList(err error) []error {
-	if errL, ok := err.(errcode.Errors); ok {
+	if errL, ok := err.(errcode.Errors); ok { //nolint:errorlint
 		return []error(errL)
 	}
 	return []error{err}
@@ -139,7 +139,7 @@ func handleErrorResponse(resp *http.Response) error {
 			}
 		}
 		err := parseHTTPErrorResponse(resp.StatusCode, resp.Body)
-		if uErr, ok := err.(*unexpectedHTTPResponseError); ok && resp.StatusCode == 401 {
+		if uErr, ok := err.(*unexpectedHTTPResponseError); ok && resp.StatusCode == 401 { //nolint:errorlint
 			return errcode.ErrorCodeUnauthorized.WithDetail(uErr.Response)
 		}
 		return err

docker/docker_image_src.go

@@ -186,7 +186,7 @@ func newImageSourceAttempt(ctx context.Context, sys *types.SystemContext, logica
 			cmd.Stdin = bytes.NewReader(acfD)
 			if err := cmd.Run(); err != nil {
 				var stderr string
-				if ee, ok := err.(*exec.ExitError); ok {
+				if ee, ok := err.(*exec.ExitError); ok { //nolint:errorlint
 					stderr = string(ee.Stderr)
 				}
 				logrus.Warnf("Failed to call additional-layer-store-auth-helper (stderr:%s): %v", stderr, err)
@@ -357,7 +357,7 @@ var multipartByteRangesRe = regexp.Delayed("multipart/byteranges; boundary=([A-Z
 func parseMediaType(contentType string) (string, map[string]string, error) {
 	mediaType, params, err := mime.ParseMediaType(contentType)
 	if err != nil {
-		if err == mime.ErrInvalidMediaParameter {
+		if err == mime.ErrInvalidMediaParameter { //nolint:errorlint // TODO remove this (https://github.com/polyfloyd/go-errorlint/pull/79).
 			// CloudFront returns an invalid MIME type, that contains an unquoted ":" in the boundary
 			// param, let's handle it here.
 			matches := multipartByteRangesRe.FindStringSubmatch(contentType)
@@ -798,7 +798,7 @@ func (n *bufferedNetworkReader) read(p []byte) (int, error) {
 	select {
 	case b = <-n.readyBuffer:
 		if b.err != nil {
-			if b.err != io.EOF {
+			if b.err != io.EOF { //nolint:errorlint
 				return b.len, b.err
 			}
 			n.gotEOF = true

docker/errors.go

@@ -56,7 +56,7 @@ func httpResponseToError(res *http.Response, context string) error {
 func registryHTTPResponseToError(res *http.Response) error {
 	err := handleErrorResponse(res)
 	// len(errs) == 0 should never be returned by handleErrorResponse; if it does, we don't modify it and let the caller report it as is.
-	if errs, ok := err.(errcode.Errors); ok && len(errs) > 0 {
+	if errs, ok := err.(errcode.Errors); ok && len(errs) > 0 { //nolint:errorlint
 		// The docker/distribution registry implementation almost never returns
 		// more than one error in the HTTP body; it seems there is only one
 		// possible instance, where the second error reports a cleanup failure
@@ -81,7 +81,7 @@ func registryHTTPResponseToError(res *http.Response) error {
 		}
 		err = errs[0]
 	}
-	switch e := err.(type) {
+	switch e := err.(type) { //nolint:errorlint
 	case *unexpectedHTTPResponseError:
 		response := string(e.Response)
 		if len(response) > 50 {

docker/internal/tarfile/reader.go

@@ -30,7 +30,7 @@ type Reader struct {
 func NewReaderFromFile(sys *types.SystemContext, path string) (*Reader, error) {
 	file, err := os.Open(path)
 	if err != nil {
-		return nil, fmt.Errorf("opening file %q: %w", path, err)
+		return nil, err
 	}
 	defer file.Close()
 

docker/reference/reference_test.go

@@ -182,7 +182,7 @@ func TestReferenceParse(t *testing.T) {
 		if testcase.err != nil {
 			if err == nil {
 				failf("missing expected error: %v", testcase.err)
-			} else if testcase.err != err {
+			} else if testcase.err != err { //nolint:errorlint
 				failf("mismatched error: got %v, expected %v", err, testcase.err)
 			}
 			continue
@@ -392,7 +392,7 @@ func TestSerialization(t *testing.T) {
 			if testcase.err == nil {
 				failf("error unmarshaling: %v", err)
 			}
-			if err != testcase.err {
+			if err != testcase.err { //nolint:errorlint
 				failf("wrong error, expected %v, got %v", testcase.err, err)
 			}
 
@@ -639,7 +639,7 @@ func TestParseNamed(t *testing.T) {
 		} else if err == nil && testcase.err != nil {
 			failf("parsing succeeded: expected error %v", testcase.err)
 			continue
-		} else if err != testcase.err {
+		} else if err != testcase.err { //nolint:errorlint
 			failf("unexpected error %v, expected %v", err, testcase.err)
 			continue
 		} else if err != nil {

manifest/docker_schema1.go

@@ -318,20 +318,20 @@ func (m *Schema1) ToSchema2Config(diffIDs []digest.Digest) ([]byte, error) {
 	// Add the history and rootfs information.
 	rootfs, err := json.Marshal(rootFS)
 	if err != nil {
-		return nil, fmt.Errorf("error encoding rootfs information %#v: %v", rootFS, err)
+		return nil, fmt.Errorf("error encoding rootfs information %#v: %w", rootFS, err)
 	}
 	rawRootfs := json.RawMessage(rootfs)
 	raw["rootfs"] = &rawRootfs
 	history, err := json.Marshal(convertedHistory)
 	if err != nil {
-		return nil, fmt.Errorf("error encoding history information %#v: %v", convertedHistory, err)
+		return nil, fmt.Errorf("error encoding history information %#v: %w", convertedHistory, err)
 	}
 	rawHistory := json.RawMessage(history)
 	raw["history"] = &rawHistory
 	// Encode the result.
 	config, err = json.Marshal(raw)
 	if err != nil {
-		return nil, fmt.Errorf("error re-encoding compat image config %#v: %v", s1, err)
+		return nil, fmt.Errorf("error re-encoding compat image config %#v: %w", s1, err)
 	}
 	return config, nil
 }

openshift/openshift-copies.go

@@ -285,7 +285,7 @@ func (config *directClientConfig) ConfirmUsable() error {
 	validationErrors = append(validationErrors, validateClusterInfo(config.getClusterName(), config.getCluster())...)
 	// when direct client config is specified, and our only error is that no server is defined, we should
 	// return a standard "no config" error
-	if len(validationErrors) == 1 && validationErrors[0] == errEmptyCluster {
+	if len(validationErrors) == 1 && validationErrors[0] == errEmptyCluster { //nolint:errorlint
 		return newErrConfigurationInvalid([]error{errEmptyConfig})
 	}
 	return newErrConfigurationInvalid(validationErrors)
@@ -365,7 +365,7 @@ func validateClusterInfo(clusterName string, clusterInfo clientcmdCluster) []err
 	if len(clusterInfo.CertificateAuthority) != 0 {
 		err := validateFileIsReadable(clusterInfo.CertificateAuthority)
 		if err != nil {
-			validationErrors = append(validationErrors, fmt.Errorf("unable to read certificate-authority %v for %v due to %v", clusterInfo.CertificateAuthority, clusterName, err))
+			validationErrors = append(validationErrors, fmt.Errorf("unable to read certificate-authority %v for %v due to %w", clusterInfo.CertificateAuthority, clusterName, err))
 		}
 	}
 
@@ -403,13 +403,13 @@ func validateAuthInfo(authInfoName string, authInfo clientcmdAuthInfo) []error {
 		if len(authInfo.ClientCertificate) != 0 {
 			err := validateFileIsReadable(authInfo.ClientCertificate)
 			if err != nil {
-				validationErrors = append(validationErrors, fmt.Errorf("unable to read client-cert %v for %v due to %v", authInfo.ClientCertificate, authInfoName, err))
+				validationErrors = append(validationErrors, fmt.Errorf("unable to read client-cert %v for %v due to %w", authInfo.ClientCertificate, authInfoName, err))
 			}
 		}
 		if len(authInfo.ClientKey) != 0 {
 			err := validateFileIsReadable(authInfo.ClientKey)
 			if err != nil {
-				validationErrors = append(validationErrors, fmt.Errorf("unable to read client-key %v for %v due to %v", authInfo.ClientKey, authInfoName, err))
+				validationErrors = append(validationErrors, fmt.Errorf("unable to read client-key %v for %v due to %w", authInfo.ClientKey, authInfoName, err))
 			}
 		}
 	}