diff --git a/.golangci.yml b/.golangci.yml index 2f13ded006..fb126d3078 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -2,3 +2,7 @@ run: concurrency: 6 timeout: 5m + +linters: + enable: + - errorlint diff --git a/copy/copy.go b/copy/copy.go index 996a4e2d7a..83fe4768c9 100644 --- a/copy/copy.go +++ b/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)) } }() diff --git a/directory/directory_dest.go b/directory/directory_dest.go index c9b3903185..0ec22d0976 100644 --- a/directory/directory_dest.go +++ b/directory/directory_dest.go @@ -1,6 +1,7 @@ package directory import ( + "bytes" "context" "errors" "fmt" @@ -8,6 +9,7 @@ import ( "os" "path/filepath" "runtime" + "syscall" "github.com/containers/image/v5/internal/imagedestination/impl" "github.com/containers/image/v5/internal/imagedestination/stubs" @@ -15,7 +17,6 @@ import ( "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 -} diff --git a/directory/directory_transport_test.go b/directory/directory_transport_test.go index 9b6d63b99e..a31a6e6f81 100644 --- a/directory/directory_transport_test.go +++ b/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() } diff --git a/docker/archive/writer.go b/docker/archive/writer.go index 11f797c007..6b3e9c598f 100644 --- a/docker/archive/writer.go +++ b/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() { diff --git a/docker/body_reader.go b/docker/body_reader.go index 7d66ef6bc0..e2f004f5a2 100644 --- a/docker/body_reader.go +++ b/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) diff --git a/docker/distribution_error.go b/docker/distribution_error.go index 0a0064576a..c60f3f9c34 100644 --- a/docker/distribution_error.go +++ b/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 diff --git a/docker/docker_image_src.go b/docker/docker_image_src.go index c8f6ba3055..bcfc21ceb6 100644 --- a/docker/docker_image_src.go +++ b/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 diff --git a/docker/errors.go b/docker/errors.go index 4392f9d182..d41c04626e 100644 --- a/docker/errors.go +++ b/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 { diff --git a/docker/internal/tarfile/reader.go b/docker/internal/tarfile/reader.go index 362657596e..59d8ff859f 100644 --- a/docker/internal/tarfile/reader.go +++ b/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() diff --git a/docker/reference/reference_test.go b/docker/reference/reference_test.go index ce1a11ddcb..02da500011 100644 --- a/docker/reference/reference_test.go +++ b/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 { diff --git a/manifest/docker_schema1.go b/manifest/docker_schema1.go index 222aa896ee..b74a1e240d 100644 --- a/manifest/docker_schema1.go +++ b/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 } diff --git a/openshift/openshift-copies.go b/openshift/openshift-copies.go index fff586bee6..0856396e05 100644 --- a/openshift/openshift-copies.go +++ b/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)) } } } diff --git a/pkg/blobcache/blobcache_test.go b/pkg/blobcache/blobcache_test.go index fa20d003b4..95138a768c 100644 --- a/pkg/blobcache/blobcache_test.go +++ b/pkg/blobcache/blobcache_test.go @@ -163,14 +163,9 @@ func TestBlobCache(t *testing.T) { t.Fatalf("error closing source image: %v", err) } // Check that the cache was populated. - cache, err := os.Open(cacheDir) + cachedNames, err := os.ReadDir(cacheDir) if err != nil { - t.Fatalf("error opening cache directory %q: %v", cacheDir, err) - } - defer cache.Close() - cachedNames, err := cache.Readdirnames(-1) - if err != nil { - t.Fatalf("error reading contents of cache directory %q: %v", cacheDir, err) + t.Fatal(err) } // Expect a layer blob, a config blob, and the manifest. expected := 3 @@ -182,12 +177,13 @@ func TestBlobCache(t *testing.T) { t.Fatalf("expected %d items in cache directory %q, got %d: %v", expected, cacheDir, len(cachedNames), cachedNames) } // Check that the blobs were all correctly stored. - for _, cachedName := range cachedNames { + for _, de := range cachedNames { + cachedName := de.Name() if digest.Digest(cachedName).Validate() == nil { cacheMember := filepath.Join(cacheDir, cachedName) cacheMemberBytes, err := os.ReadFile(cacheMember) if err != nil { - t.Fatalf("error reading cache member %q: %v", cacheMember, err) + t.Fatal(err) } if digest.FromBytes(cacheMemberBytes).String() != cachedName { t.Fatalf("cache member %q was stored incorrectly!", cacheMember) @@ -196,16 +192,12 @@ func TestBlobCache(t *testing.T) { } // Clear out anything in the source directory that probably isn't a manifest, so that we'll // have to depend on the cached copies of some of the blobs. - srcNameDir, err := os.Open(srcdir) - if err != nil { - t.Fatalf("error opening source directory %q: %v", srcdir, err) - } - defer srcNameDir.Close() - srcNames, err := srcNameDir.Readdirnames(-1) + srcNames, err := os.ReadDir(srcdir) if err != nil { - t.Fatalf("error reading contents of source directory %q: %v", srcdir, err) + t.Fatal(err) } - for _, name := range srcNames { + for _, de := range srcNames { + name := de.Name() if !strings.HasPrefix(name, "manifest") { os.Remove(filepath.Join(srcdir, name)) } diff --git a/pkg/cli/sigstore/params/sigstore.go b/pkg/cli/sigstore/params/sigstore.go index 0151b9acb0..ea51658b09 100644 --- a/pkg/cli/sigstore/params/sigstore.go +++ b/pkg/cli/sigstore/params/sigstore.go @@ -64,7 +64,7 @@ func ParseFile(path string) (*SigningParameterFile, error) { var res SigningParameterFile source, err := os.ReadFile(path) if err != nil { - return nil, fmt.Errorf("reading %q: %w", path, err) + return nil, err } dec := yaml.NewDecoder(bytes.NewReader(source)) dec.KnownFields(true) diff --git a/pkg/compression/compression.go b/pkg/compression/compression.go index b83a257e4f..5299fbf32a 100644 --- a/pkg/compression/compression.go +++ b/pkg/compression/compression.go @@ -112,7 +112,7 @@ func DetectCompressionFormat(input io.Reader) (Algorithm, DecompressorFunc, io.R buffer := [8]byte{} n, err := io.ReadAtLeast(input, buffer[:], len(buffer)) - if err != nil && err != io.EOF && err != io.ErrUnexpectedEOF { + if err != nil && err != io.EOF && err != io.ErrUnexpectedEOF { //nolint:errorlint // TODO remove this (https://github.com/polyfloyd/go-errorlint/pull/83). // This is a “real” error. We could just ignore it this time, process the data we have, and hope that the source will report the same error again. // Instead, fail immediately with the original error cause instead of a possibly secondary/misleading error returned later. return Algorithm{}, nil, nil, err diff --git a/sif/src.go b/sif/src.go index f8bf310344..096fe9aefc 100644 --- a/sif/src.go +++ b/sif/src.go @@ -42,7 +42,7 @@ type sifImageSource struct { func getBlobInfo(path string) (digest.Digest, int64, error) { f, err := os.Open(path) if err != nil { - return "", -1, fmt.Errorf("opening %q for reading: %w", path, err) + return "", -1, err } defer f.Close() @@ -186,7 +186,7 @@ func (s *sifImageSource) GetBlob(ctx context.Context, info types.BlobInfo, cache case s.layerDigest: reader, err := os.Open(s.layerFile) if err != nil { - return nil, -1, fmt.Errorf("opening %q: %w", s.layerFile, err) + return nil, -1, err } return reader, s.layerSize, nil default: diff --git a/signature/internal/json.go b/signature/internal/json.go index f9efafb8e1..3ab10de489 100644 --- a/signature/internal/json.go +++ b/signature/internal/json.go @@ -61,7 +61,7 @@ func ParanoidUnmarshalJSONObject(data []byte, fieldResolver func(string) any) er return JSONFormatError(err.Error()) } } - if _, err := dec.Token(); err != io.EOF { + if _, err := dec.Token(); err != io.EOF { //nolint:errorlint // TODO remove this (https://github.com/polyfloyd/go-errorlint/pull/82). return JSONFormatError("Unexpected data after JSON object") } return nil diff --git a/signature/internal/rekor_set.go b/signature/internal/rekor_set.go index e79c91cf99..10cc0c3d74 100644 --- a/signature/internal/rekor_set.go +++ b/signature/internal/rekor_set.go @@ -11,6 +11,7 @@ import ( "encoding/hex" "encoding/json" "encoding/pem" + "errors" "fmt" "time" @@ -40,29 +41,30 @@ type UntrustedRekorPayload struct { // A compile-time check that UntrustedRekorSET implements json.Unmarshaler var _ json.Unmarshaler = (*UntrustedRekorSET)(nil) -// UnmarshalJSON implements the json.Unmarshaler interface -func (s *UntrustedRekorSET) UnmarshalJSON(data []byte) error { - err := s.strictUnmarshalJSON(data) - if err != nil { - if formatErr, ok := err.(JSONFormatError); ok { - err = NewInvalidSignatureError(formatErr.Error()) - } +// ConvertFormatError converts JSONFormatError to NewInvalidSignatureError. +// All other errors are returned as is. +func ConvertFormatError(err error) error { + var formatErr JSONFormatError + if errors.As(err, &formatErr) { + return NewInvalidSignatureError(formatErr.Error()) } return err } -// strictUnmarshalJSON is UnmarshalJSON, except that it may return the internal JSONFormatError error type. -// Splitting it into a separate function allows us to do the JSONFormatError → InvalidSignatureError in a single place, the caller. -func (s *UntrustedRekorSET) strictUnmarshalJSON(data []byte) error { - return ParanoidUnmarshalJSONObjectExactFields(data, map[string]any{ - "SignedEntryTimestamp": &s.UntrustedSignedEntryTimestamp, - "Payload": &s.UntrustedPayload, - }) +// UnmarshalJSON implements the json.Unmarshaler interface +func (s *UntrustedRekorSET) UnmarshalJSON(data []byte) error { + return ConvertFormatError(ParanoidUnmarshalJSONObjectExactFields(data, + map[string]any{ + "SignedEntryTimestamp": &s.UntrustedSignedEntryTimestamp, + "Payload": &s.UntrustedPayload, + })) } // A compile-time check that UntrustedRekorSET and *UntrustedRekorSET implements json.Marshaler -var _ json.Marshaler = UntrustedRekorSET{} -var _ json.Marshaler = (*UntrustedRekorSET)(nil) +var ( + _ json.Marshaler = UntrustedRekorSET{} + _ json.Marshaler = (*UntrustedRekorSET)(nil) +) // MarshalJSON implements the json.Marshaler interface. func (s UntrustedRekorSET) MarshalJSON() ([]byte, error) { @@ -77,29 +79,20 @@ var _ json.Unmarshaler = (*UntrustedRekorPayload)(nil) // UnmarshalJSON implements the json.Unmarshaler interface func (p *UntrustedRekorPayload) UnmarshalJSON(data []byte) error { - err := p.strictUnmarshalJSON(data) - if err != nil { - if formatErr, ok := err.(JSONFormatError); ok { - err = NewInvalidSignatureError(formatErr.Error()) - } - } - return err -} - -// strictUnmarshalJSON is UnmarshalJSON, except that it may return the internal JSONFormatError error type. -// Splitting it into a separate function allows us to do the JSONFormatError → InvalidSignatureError in a single place, the caller. -func (p *UntrustedRekorPayload) strictUnmarshalJSON(data []byte) error { - return ParanoidUnmarshalJSONObjectExactFields(data, map[string]any{ - "body": &p.Body, - "integratedTime": &p.IntegratedTime, - "logIndex": &p.LogIndex, - "logID": &p.LogID, - }) + return ConvertFormatError(ParanoidUnmarshalJSONObjectExactFields(data, + map[string]any{ + "body": &p.Body, + "integratedTime": &p.IntegratedTime, + "logIndex": &p.LogIndex, + "logID": &p.LogID, + })) } // A compile-time check that UntrustedRekorPayload and *UntrustedRekorPayload implements json.Marshaler -var _ json.Marshaler = UntrustedRekorPayload{} -var _ json.Marshaler = (*UntrustedRekorPayload)(nil) +var ( + _ json.Marshaler = UntrustedRekorPayload{} + _ json.Marshaler = (*UntrustedRekorPayload)(nil) +) // MarshalJSON implements the json.Marshaler interface. func (p UntrustedRekorPayload) MarshalJSON() ([]byte, error) { @@ -177,7 +170,6 @@ func VerifyRekorSET(publicKey *ecdsa.PublicKey, unverifiedRekorSET []byte, unver } if hashedRekordV001.Signature.PublicKey == nil { return time.Time{}, NewInvalidSignatureError(`Missing "signature.publicKey" field in hashedrekord`) - } rekorKeyOrCertPEM, rest := pem.Decode(hashedRekordV001.Signature.PublicKey.Content) if rekorKeyOrCertPEM == nil { @@ -232,7 +224,6 @@ func VerifyRekorSET(publicKey *ecdsa.PublicKey, unverifiedRekorSET []byte, unver rekorPayloadHash, err := hex.DecodeString(*hashedRekordV001.Data.Hash.Value) if err != nil { return time.Time{}, NewInvalidSignatureError(fmt.Sprintf(`Invalid "data.hash.value" field in hashedrekord: %v`, err)) - } unverifiedPayloadHash := sha256.Sum256(unverifiedPayloadBytes) if !bytes.Equal(rekorPayloadHash, unverifiedPayloadHash[:]) { diff --git a/signature/internal/sigstore_payload.go b/signature/internal/sigstore_payload.go index a2609c954b..650afe5cbc 100644 --- a/signature/internal/sigstore_payload.go +++ b/signature/internal/sigstore_payload.go @@ -47,8 +47,10 @@ func NewUntrustedSigstorePayload(dockerManifestDigest digest.Digest, dockerRefer } // A compile-time check that UntrustedSigstorePayload and *UntrustedSigstorePayload implements json.Marshaler -var _ json.Marshaler = UntrustedSigstorePayload{} -var _ json.Marshaler = (*UntrustedSigstorePayload)(nil) +var ( + _ json.Marshaler = UntrustedSigstorePayload{} + _ json.Marshaler = (*UntrustedSigstorePayload)(nil) +) // MarshalJSON implements the json.Marshaler interface. func (s UntrustedSigstorePayload) MarshalJSON() ([]byte, error) { @@ -79,13 +81,7 @@ var _ json.Unmarshaler = (*UntrustedSigstorePayload)(nil) // UnmarshalJSON implements the json.Unmarshaler interface func (s *UntrustedSigstorePayload) UnmarshalJSON(data []byte) error { - err := s.strictUnmarshalJSON(data) - if err != nil { - if formatErr, ok := err.(JSONFormatError); ok { - err = NewInvalidSignatureError(formatErr.Error()) - } - } - return err + return ConvertFormatError(s.strictUnmarshalJSON(data)) } // strictUnmarshalJSON is UnmarshalJSON, except that it may return the internal JSONFormatError error type. diff --git a/signature/policy_reference_match.go b/signature/policy_reference_match.go index 48dbfbbde5..390957b02b 100644 --- a/signature/policy_reference_match.go +++ b/signature/policy_reference_match.go @@ -136,7 +136,7 @@ func (prm *prmRemapIdentity) remapReferencePrefix(ref reference.Named) (referenc newNamedRef := strings.Replace(refString, prm.Prefix, prm.SignedPrefix, 1) newParsedRef, err := reference.ParseNamed(newNamedRef) if err != nil { - return nil, fmt.Errorf(`error rewriting reference from %q to %q: %v`, refString, newNamedRef, err) + return nil, fmt.Errorf(`error rewriting reference from %q to %q: %w`, refString, newNamedRef, err) } return newParsedRef, nil } diff --git a/signature/sigstore/signer.go b/signature/sigstore/signer.go index fb825ada9d..64455dced2 100644 --- a/signature/sigstore/signer.go +++ b/signature/sigstore/signer.go @@ -25,7 +25,7 @@ func WithPrivateKeyFile(file string, passphrase []byte) Option { privateKeyPEM, err := os.ReadFile(file) if err != nil { - return fmt.Errorf("reading private key from %s: %w", file, err) + return fmt.Errorf("reading private key: %w", err) } signerVerifier, err := loadPrivateKey(privateKeyPEM, passphrase) if err != nil { diff --git a/signature/simple.go b/signature/simple.go index 30df997d86..657935accd 100644 --- a/signature/simple.go +++ b/signature/simple.go @@ -105,13 +105,7 @@ var _ json.Unmarshaler = (*untrustedSignature)(nil) // UnmarshalJSON implements the json.Unmarshaler interface func (s *untrustedSignature) UnmarshalJSON(data []byte) error { - err := s.strictUnmarshalJSON(data) - if err != nil { - if formatErr, ok := err.(internal.JSONFormatError); ok { - err = internal.NewInvalidSignatureError(formatErr.Error()) - } - } - return err + return internal.ConvertFormatError(s.strictUnmarshalJSON(data)) } // strictUnmarshalJSON is UnmarshalJSON, except that it may return the internal.JSONFormatError error type. diff --git a/storage/storage_dest.go b/storage/storage_dest.go index 842a3ab068..4cc3cc1ad3 100644 --- a/storage/storage_dest.go +++ b/storage/storage_dest.go @@ -239,7 +239,7 @@ func (s *storageImageDestination) putBlobToPendingFile(stream io.Reader, blobinf filename := s.computeNextBlobCacheFile() file, err := os.OpenFile(filename, os.O_CREATE|os.O_TRUNC|os.O_WRONLY|os.O_EXCL, 0600) if err != nil { - return private.UploadedBlob{}, fmt.Errorf("creating temporary file %q: %w", filename, err) + return private.UploadedBlob{}, err } defer file.Close() counter := ioutils.NewWriteCounter(file) @@ -301,11 +301,10 @@ func (f *zstdFetcher) GetBlobAt(chunks []chunked.ImageSourceChunk) (chan io.Read newChunks = append(newChunks, i) } rc, errs, err := f.chunkAccessor.GetBlobAt(f.ctx, f.blobInfo, newChunks) - if _, ok := err.(private.BadPartialRequestError); ok { + if _, ok := err.(private.BadPartialRequestError); ok { //nolint:errorlint err = chunked.ErrBadRequest{} } return rc, errs, err - } // PutBlobPartial attempts to create a blob using the data that is already present @@ -714,9 +713,9 @@ func (s *storageImageDestination) getConfigBlob(info types.BlobInfo) ([]byte, er } // Assume it's a file, since we're only calling this from a place that expects to read files. if filename, ok := s.lockProtected.filenames[info.Digest]; ok { - contents, err2 := os.ReadFile(filename) - if err2 != nil { - return nil, fmt.Errorf(`reading blob from file %q: %w`, filename, err2) + contents, err := os.ReadFile(filename) + if err != nil { + return nil, fmt.Errorf(`reading blob: %w`, err) } return contents, nil } @@ -1009,7 +1008,7 @@ func (s *storageImageDestination) createNewLayer(index int, layerDigest digest.D file, err := os.OpenFile(filename, os.O_CREATE|os.O_TRUNC|os.O_WRONLY|os.O_EXCL, 0o600) if err != nil { diff.Close() - return nil, fmt.Errorf("creating temporary file %q: %w", filename, err) + return nil, err } // Copy the data to the file. // TODO: This can take quite some time, and should ideally be cancellable using @@ -1052,7 +1051,7 @@ func (s *storageImageDestination) createNewLayer(index int, layerDigest digest.D // Read the cached blob and use it as a diff. file, err := os.Open(filename) if err != nil { - return nil, fmt.Errorf("opening file %q: %w", filename, err) + return nil, err } defer file.Close() // Build the new layer using the diff, regardless of where it came from. diff --git a/storage/storage_reference.go b/storage/storage_reference.go index 2a1099f679..3706e7c53c 100644 --- a/storage/storage_reference.go +++ b/storage/storage_reference.go @@ -37,7 +37,7 @@ func newReference(transport storageTransport, named reference.Named, id string) } if id != "" { if err := validateImageID(id); err != nil { - return nil, fmt.Errorf("invalid ID value %q: %v: %w", id, err, ErrInvalidReference) + return nil, fmt.Errorf("invalid ID value %q: %w: %w", id, err, ErrInvalidReference) } } // We take a copy of the transport, which contains a pointer to the diff --git a/tarball/tarball_src.go b/tarball/tarball_src.go index 18d4cc2d29..b15d9de479 100644 --- a/tarball/tarball_src.go +++ b/tarball/tarball_src.go @@ -73,7 +73,7 @@ func (r *tarballReference) NewImageSource(ctx context.Context, sys *types.System reader = file fileinfo, err := file.Stat() if err != nil { - return nil, fmt.Errorf("error reading size of %q: %w", filename, err) + return nil, err } blobTime = fileinfo.ModTime() blob = tarballBlob{ @@ -103,7 +103,7 @@ func (r *tarballReference) NewImageSource(ctx context.Context, sys *types.System } // TODO: This can take quite some time, and should ideally be cancellable using ctx.Done(). if _, err := io.Copy(io.Discard, reader); err != nil { - return nil, fmt.Errorf("error reading %q: %v", filename, err) + return nil, fmt.Errorf("error reading %q: %w", filename, err) } if uncompressed != nil { uncompressed.Close() @@ -152,7 +152,7 @@ func (r *tarballReference) NewImageSource(ctx context.Context, sys *types.System // Encode and digest the image configuration blob. configBytes, err := json.Marshal(&config) if err != nil { - return nil, fmt.Errorf("error generating configuration blob for %q: %v", strings.Join(r.filenames, separator), err) + return nil, fmt.Errorf("error generating configuration blob for %q: %w", strings.Join(r.filenames, separator), err) } configID := digest.Canonical.FromBytes(configBytes) blobs[configID] = tarballBlob{ @@ -177,7 +177,7 @@ func (r *tarballReference) NewImageSource(ctx context.Context, sys *types.System // Encode the manifest. manifestBytes, err := json.Marshal(&manifest) if err != nil { - return nil, fmt.Errorf("error generating manifest for %q: %v", strings.Join(r.filenames, separator), err) + return nil, fmt.Errorf("error generating manifest for %q: %w", strings.Join(r.filenames, separator), err) } // Return the image. diff --git a/tarball/tarball_transport.go b/tarball/tarball_transport.go index 63d835530b..5e1d635032 100644 --- a/tarball/tarball_transport.go +++ b/tarball/tarball_transport.go @@ -38,13 +38,13 @@ func (t *tarballTransport) ParseReference(reference string) (types.ImageReferenc if filename == "-" { stdin, err = io.ReadAll(os.Stdin) if err != nil { - return nil, fmt.Errorf("error buffering stdin: %v", err) + return nil, fmt.Errorf("error buffering stdin: %w", err) } continue } f, err := os.Open(filename) if err != nil { - return nil, fmt.Errorf("error opening %q: %v", filename, err) + return nil, err } f.Close() }