fixing some of the missed/ignored errors (#446)

egorikas · web-flow · commit a490967ba3ae · 2025-09-17T16:55:19.000+02:00
* adding closer function
* handling some missed errors
* imports formatting
diff --git a/build-index/cmd/cmd.go b/build-index/cmd/cmd.go
@@ -32,6 +32,7 @@ import (
 	"github.com/uber/kraken/metrics"
 	"github.com/uber/kraken/nginx"
 	"github.com/uber/kraken/origin/blobclient"
+	"github.com/uber/kraken/utils/closers"
 	"github.com/uber/kraken/utils/configutil"
 	"github.com/uber/kraken/utils/log"
 
@@ -116,7 +117,10 @@ func Run(flags *Flags, opts ...Option) {
 		log.SetGlobalLogger(overrides.logger.Sugar())
 	} else {
 		zlog := log.ConfigureLogger(config.ZapLogging)
-		defer zlog.Sync()
+		defer func() {
+			// if flushing logs fails, we can't do anything about the error.
+			_ = zlog.Sync() //nolint:errcheck
+		}()
 	}
 
 	stats := overrides.metrics
@@ -126,7 +130,7 @@ func Run(flags *Flags, opts ...Option) {
 			log.Fatalf("Failed to init metrics: %s", err)
 		}
 		stats = s
-		defer closer.Close()
+		defer closers.Close(closer)
 	}
 
 	go metrics.EmitVersion(stats)
diff --git a/build-index/tagserver/server.go b/build-index/tagserver/server.go
@@ -40,6 +40,7 @@ import (
 	"github.com/uber/kraken/utils/httputil"
 	"github.com/uber/kraken/utils/listener"
 	"github.com/uber/kraken/utils/log"
+	"go.uber.org/zap"
 
 	"github.com/go-chi/chi"
 	chimiddleware "github.com/go-chi/chi/middleware"
@@ -142,20 +143,30 @@ func (s *Server) ListenAndServe() error {
 }
 
 func (s *Server) healthHandler(w http.ResponseWriter, r *http.Request) error {
-	fmt.Fprintln(w, "OK")
+	_, err := fmt.Fprintln(w, "OK")
+	if err != nil {
+		log.Desugar().Error("Health check write failed", zap.Error(err))
+		return handler.Errorf("write health check: %s", err)
+	}
 	return nil
 }
 
 func (s *Server) readinessCheckHandler(w http.ResponseWriter, r *http.Request) error {
 	err := s.backends.CheckReadiness()
 	if err != nil {
+		log.Desugar().Error("Backends readiness check failed", zap.Error(err))
 		return handler.Errorf("not ready to serve traffic: %s", err).Status(http.StatusServiceUnavailable)
 	}
 	err = s.localOriginClient.CheckReadiness()
 	if err != nil {
+		log.Desugar().Error("Origin readiness check failed", zap.Error(err))
 		return handler.Errorf("not ready to serve traffic: %s", err).Status(http.StatusServiceUnavailable)
 	}
-	fmt.Fprintln(w, "OK")
+	_, err = fmt.Fprintln(w, "OK")
+	if err != nil {
+		log.Desugar().Error("Readiness check write failed", zap.Error(err))
+		return handler.Errorf("write readiness check: %s", err)
+	}
 	return nil
 }
 
diff --git a/core/digester_test.go b/core/digester_test.go
@@ -40,7 +40,8 @@ func TestFromBytes(t *testing.T) {
 	require := require.New(t)
 
 	d := NewDigester()
-	d.FromBytes([]byte(_testStr))
+	_, err := d.FromBytes([]byte(_testStr))
+	require.NoError(err)
 
 	hexDigest := d.Digest().Hex()
 	require.NoError(ValidateSHA256(hexDigest))
@@ -52,7 +53,8 @@ func TestFromReader(t *testing.T) {
 
 	d := NewDigester()
 	r := strings.NewReader(_testStr)
-	d.FromReader(r)
+	_, err := d.FromReader(r)
+	require.NoError(err)
 
 	hexDigest := d.Digest().Hex()
 	require.NoError(ValidateSHA256(hexDigest))
diff --git a/lib/backend/gcsbackend/client.go b/lib/backend/gcsbackend/client.go
@@ -20,6 +20,8 @@ import (
 	"io"
 	"path"
 
+	"github.com/uber/kraken/utils/closers"
+
 	"github.com/uber-go/tally"
 	"github.com/uber/kraken/core"
 	"github.com/uber/kraken/lib/backend"
@@ -245,7 +247,7 @@ func (g *GCSImpl) Download(objectName string, w io.Writer) (int64, error) {
 		}
 		return 0, err
 	}
-	defer rc.Close()
+	defer closers.Close(rc)
 
 	r, err := io.CopyN(w, rc, int64(g.config.BufferGuard))
 	if err != nil && err != io.EOF {
diff --git a/lib/backend/httpbackend/http.go b/lib/backend/httpbackend/http.go
@@ -20,6 +20,8 @@ import (
 	"io"
 	"time"
 
+	"github.com/uber/kraken/utils/closers"
+
 	"github.com/uber-go/tally"
 	"github.com/uber/kraken/core"
 	"github.com/uber/kraken/lib/backend"
@@ -105,7 +107,7 @@ func (c *Client) Download(namespace, name string, dst io.Writer) error {
 		}
 		return err
 	}
-	defer resp.Body.Close()
+	defer closers.Close(resp.Body)
 	if _, err := io.Copy(dst, resp.Body); err != nil {
 		return fmt.Errorf("copy: %s", err)
 	}
diff --git a/lib/backend/httpbackend/http_test.go b/lib/backend/httpbackend/http_test.go
@@ -68,7 +68,8 @@ func TestHttpDownloadFileNotFound(t *testing.T) {
 	r := chi.NewRouter()
 	r.Get("/data/{blob}", func(w http.ResponseWriter, req *http.Request) {
 		w.WriteHeader(http.StatusNotFound)
-		w.Write([]byte("file not found"))
+		_, err := w.Write([]byte("file not found"))
+		require.NoError(err)
 	})
 	addr, stop := testutil.StartServer(r)
 	defer stop()
diff --git a/lib/backend/registrybackend/blobclient.go b/lib/backend/registrybackend/blobclient.go
@@ -20,6 +20,8 @@ import (
 	"net/http"
 	"strconv"
 
+	"github.com/uber/kraken/utils/closers"
+
 	"github.com/uber-go/tally"
 	"github.com/uber/kraken/core"
 	"github.com/uber/kraken/lib/backend"
@@ -145,7 +147,7 @@ func (c *BlobClient) downloadHelper(namespace, name, query string, dst io.Writer
 		}
 		return fmt.Errorf("get blob: %s", err)
 	}
-	defer resp.Body.Close()
+	defer closers.Close(resp.Body)
 
 	if _, err := io.Copy(dst, resp.Body); err != nil {
 		return fmt.Errorf("copy: %s", err)
diff --git a/lib/backend/registrybackend/blobclient_test.go b/lib/backend/registrybackend/blobclient_test.go
@@ -108,7 +108,8 @@ func TestBlobDownloadFileNotFound(t *testing.T) {
 	r := chi.NewRouter()
 	r.Get(fmt.Sprintf("/v2/%s/blobs/{blob}", namespace), func(w http.ResponseWriter, req *http.Request) {
 		w.WriteHeader(http.StatusNotFound)
-		w.Write([]byte("file not found"))
+		_, err := w.Write([]byte("file not found"))
+		require.NoError(err)
 	})
 	r.Head(fmt.Sprintf("/v2/%s/blobs/{blob}", namespace), func(w http.ResponseWriter, req *http.Request) {
 		w.WriteHeader(http.StatusNotFound)
diff --git a/lib/backend/testfs/client.go b/lib/backend/testfs/client.go
@@ -26,8 +26,8 @@ import (
 	"github.com/uber/kraken/lib/backend"
 	"github.com/uber/kraken/lib/backend/backenderrors"
 	"github.com/uber/kraken/lib/backend/namepath"
+	"github.com/uber/kraken/utils/closers"
 	"github.com/uber/kraken/utils/httputil"
-
 	"go.uber.org/zap"
 	"gopkg.in/yaml.v2"
 )
@@ -127,7 +127,7 @@ func (c *Client) Download(namespace, name string, dst io.Writer) error {
 		}
 		return err
 	}
-	defer resp.Body.Close()
+	defer closers.Close(resp.Body)
 	if _, err := io.Copy(dst, resp.Body); err != nil {
 		return fmt.Errorf("copy: %s", err)
 	}
@@ -150,7 +150,7 @@ func (c *Client) List(prefix string, opts ...backend.ListOption) (*backend.ListR
 	if err != nil {
 		return nil, err
 	}
-	defer resp.Body.Close()
+	defer closers.Close(resp.Body)
 	var paths []string
 	if err := json.NewDecoder(resp.Body).Decode(&paths); err != nil {
 		return nil, fmt.Errorf("json: %s", err)
diff --git a/lib/backend/testfs/server.go b/lib/backend/testfs/server.go
@@ -24,6 +24,8 @@ import (
 	"strings"
 	"sync"
 
+	"github.com/uber/kraken/utils/log"
+
 	"github.com/uber/kraken/utils/handler"
 
 	"github.com/go-chi/chi"
@@ -57,11 +59,17 @@ func (s *Server) Handler() http.Handler {
 
 // Cleanup cleans up the underlying directory of s.
 func (s *Server) Cleanup() {
-	os.RemoveAll(s.dir)
+	err := os.RemoveAll(s.dir)
+	if err != nil {
+		log.Error(err)
+	}
 }
 
 func (s *Server) healthHandler(w http.ResponseWriter, r *http.Request) {
-	w.Write([]byte("OK"))
+	_, err := w.Write([]byte("OK"))
+	if err != nil {
+		log.Error(err)
+	}
 }
 
 func (s *Server) statHandler(w http.ResponseWriter, r *http.Request) error {
diff --git a/lib/containerruntime/containerd/client.go b/lib/containerruntime/containerd/client.go
@@ -17,6 +17,8 @@ import (
 	"context"
 	"fmt"
 
+	"github.com/uber/kraken/utils/closers"
+
 	"github.com/containerd/containerd"
 )
 
@@ -38,7 +40,7 @@ func (c *Impl) PullImage(ctx context.Context, ns, repo, tag string) error {
 	if err != nil {
 		return fmt.Errorf("new containerd client: %s", err)
 	}
-	defer client.Close()
+	defer closers.Close(client)
 
 	_, err = client.Pull(ctx, fmt.Sprintf("%s/%s:%s", c.registry, repo, tag), containerd.WithPullUnpack)
 	if err != nil {
diff --git a/lib/dockerregistry/blobs.go b/lib/dockerregistry/blobs.go
@@ -21,6 +21,8 @@ import (
 	"os"
 	"time"
 
+	"github.com/uber/kraken/utils/closers"
+
 	"github.com/uber/kraken/lib/dockerregistry/transfer"
 	"github.com/uber/kraken/lib/store"
 
@@ -86,7 +88,7 @@ func (b *blobs) getContent(ctx context.Context, path string) ([]byte, error) {
 	if err != nil {
 		return nil, err
 	}
-	defer r.Close()
+	defer closers.Close(r)
 	return io.ReadAll(r)
 }
 
diff --git a/lib/dockerregistry/manifests.go b/lib/dockerregistry/manifests.go
@@ -18,6 +18,8 @@ import (
 	"strings"
 	"time"
 
+	"github.com/uber/kraken/utils/closers"
+
 	"github.com/uber-go/tally"
 	"github.com/uber/kraken/lib/store"
 
@@ -117,13 +119,17 @@ func (t *manifests) getDigest(path string, subtype PathSubType) ([]byte, error)
 	if err != nil {
 		return nil, fmt.Errorf("transferer download: %w", err)
 	}
-	defer blob.Close()
+	defer closers.Close(blob)
 
 	// Only verify if we haven't already verified this (repo, digest) combination
 	cacheKey := fmt.Sprintf("%s:%s", repo, digest.String())
 	shouldEmitMetrics := !t.verifiedCache.Has(cacheKey)
 
-	_, _ = t.verify(path, repo, digest, blob, shouldEmitMetrics)
+	// Signature verification is currently not enforced: errors from t.verify are ignored.
+	// This is intentional because verification enforcement is planned for a future release.
+	// Risks: manifests may be accepted without verification, which could allow untrusted content.
+	// TODO: Remove error ignoring and enforce verification once the feature is activated.
+	_, _ = t.verify(path, repo, digest, blob, shouldEmitMetrics) //nolint:errcheck
 
 	if shouldEmitMetrics {
 		t.verifiedCache.Add(cacheKey)
diff --git a/lib/dockerregistry/storage_driver_test.go b/lib/dockerregistry/storage_driver_test.go
@@ -160,20 +160,20 @@ func TestStorageDriverWriter(t *testing.T) {
 	content := []byte("this is a test for upload writer")
 	for _, tc := range testCases {
 		t.Run(fmt.Sprintf("GetWriter %s", tc.input), func(t *testing.T) {
-			require := require.New(t)
 			w, err := sd.Writer(contextFixture(), tc.input, false)
-			require.Equal(tc.err, err)
+			require.Equal(t, tc.err, err)
 			if err != nil {
 				return
 			}
-			w.Write(content)
-			w.Close()
+			_, err = w.Write(content)
+			require.NoError(t, err)
+			require.NoError(t, w.Close())
 			r, err := sd.Reader(contextFixture(), tc.input, 0)
-			require.NoError(err)
-			defer r.Close()
+			require.NoError(t, err)
 			data, err := io.ReadAll(r)
-			require.NoError(err)
-			require.Equal(content, data)
+			require.NoError(t, err)
+			require.Equal(t, content, data)
+			require.NoError(t, r.Close())
 		})
 	}
 }
diff --git a/lib/dockerregistry/testutils_test.go b/lib/dockerregistry/testutils_test.go
@@ -73,8 +73,16 @@ func (d *testDriver) setup() (*KrakenStorageDriver, testImageUploadBundle) {
 	if err != nil {
 		log.Panic(err)
 	}
-	defer writer.Close()
-	writer.Write([]byte(uploadContent))
+	defer func() {
+		err := writer.Close()
+		if err != nil {
+			log.Panic(err)
+		}
+	}()
+	_, err = writer.Write([]byte(uploadContent))
+	if err != nil {
+		log.Panic(err)
+	}
 
 	config := core.NewBlobFixture()
 	layer1 := core.NewBlobFixture()
diff --git a/mocks/io/mock_closer.go b/mocks/io/mock_closer.go
diff --git a/origin/blobclient/client.go b/origin/blobclient/client.go
diff --git a/proxy/proxyserver/server_test.go b/proxy/proxyserver/server_test.go
diff --git a/utils/closers/closer.go b/utils/closers/closer.go
diff --git a/utils/closers/closer_test.go b/utils/closers/closer_test.go
diff --git a/utils/log/log.go b/utils/log/log.go

Original file line number	Diff line number	Diff line change
`@@ -20,6 +20,8 @@ import (`
`20`	`20`	`"io"`
`21`	`21`	`"time"`
`22`	`22`
	`23`	`+ "github.com/uber/kraken/utils/closers"`
	`24`	`+`
`23`	`25`	`"github.com/uber-go/tally"`
`24`	`26`	`"github.com/uber/kraken/core"`
`25`	`27`	`"github.com/uber/kraken/lib/backend"`
`@@ -105,7 +107,7 @@ func (c *Client) Download(namespace, name string, dst io.Writer) error {`
`105`	`107`	`}`
`106`	`108`	`return err`
`107`	`109`	`}`
`108`		`- defer resp.Body.Close()`
	`110`	`+ defer closers.Close(resp.Body)`
`109`	`111`	`if _, err := io.Copy(dst, resp.Body); err != nil {`
`110`	`112`	`return fmt.Errorf("copy: %s", err)`
`111`	`113`	`}`
Original file line number	Diff line number	Diff line change
`@@ -26,8 +26,8 @@ import (`
`26`	`26`	`"github.com/uber/kraken/lib/backend"`
`27`	`27`	`"github.com/uber/kraken/lib/backend/backenderrors"`
`28`	`28`	`"github.com/uber/kraken/lib/backend/namepath"`
	`29`	`+ "github.com/uber/kraken/utils/closers"`
`29`	`30`	`"github.com/uber/kraken/utils/httputil"`
`30`		`-`
`31`	`31`	`"go.uber.org/zap"`
`32`	`32`	`"gopkg.in/yaml.v2"`
`33`	`33`	`)`
`@@ -127,7 +127,7 @@ func (c *Client) Download(namespace, name string, dst io.Writer) error {`
`127`	`127`	`}`
`128`	`128`	`return err`
`129`	`129`	`}`
`130`		`- defer resp.Body.Close()`
	`130`	`+ defer closers.Close(resp.Body)`
`131`	`131`	`if _, err := io.Copy(dst, resp.Body); err != nil {`
`132`	`132`	`return fmt.Errorf("copy: %s", err)`
`133`	`133`	`}`
`@@ -150,7 +150,7 @@ func (c Client) List(prefix string, opts ...backend.ListOption) (backend.ListR`
`150`	`150`	`if err != nil {`
`151`	`151`	`return nil, err`
`152`	`152`	`}`
`153`		`- defer resp.Body.Close()`
	`153`	`+ defer closers.Close(resp.Body)`
`154`	`154`	`var paths []string`
`155`	`155`	`if err := json.NewDecoder(resp.Body).Decode(&paths); err != nil {`
`156`	`156`	`return nil, fmt.Errorf("json: %s", err)`