From c7a98aa706379c4e5c79ea675c7f333192677971 Mon Sep 17 00:00:00 2001 From: Christoph Mewes Date: Sat, 7 May 2022 00:17:04 +0200 Subject: [PATCH] :seedling: enable errorlint and unused linters (#1890) * use %w instead of %v for wrapping errors * do not use == or != to compare errors * remove unused code * fix formatting --- .golangci.yml | 2 ++ pkg/cache/multi_namespace_cache.go | 2 +- pkg/client/config/config.go | 2 +- pkg/client/fake/client.go | 6 +++--- pkg/client/namespaced_client.go | 16 ++++++++-------- pkg/envtest/crd.go | 3 ++- pkg/envtest/webhook.go | 18 +++++++++--------- pkg/finalizer/finalizer.go | 2 +- pkg/internal/testing/certs/tinyca.go | 12 ++++++------ pkg/internal/testing/process/arguments_test.go | 6 ------ pkg/manager/internal.go | 9 +++++---- pkg/manager/manager.go | 2 +- pkg/manager/manager_test.go | 6 ------ pkg/source/source.go | 2 +- pkg/webhook/server.go | 6 +++--- tools/setup-envtest/remote/client.go | 5 +++-- tools/setup-envtest/store/store.go | 2 +- .../workflows/workflows_testutils_test.go | 9 --------- 18 files changed, 47 insertions(+), 63 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 68be34b063..7e47f53136 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -7,6 +7,7 @@ linters: - depguard - dogsled - errcheck + - errorlint - exportloopref - goconst - gocritic @@ -34,6 +35,7 @@ linters: - typecheck - unconvert - unparam + - unused - varcheck - whitespace diff --git a/pkg/cache/multi_namespace_cache.go b/pkg/cache/multi_namespace_cache.go index dc29651b01..47a5bb3b3f 100644 --- a/pkg/cache/multi_namespace_cache.go +++ b/pkg/cache/multi_namespace_cache.go @@ -55,7 +55,7 @@ func MultiNamespacedCacheBuilder(namespaces []string) NewCacheFunc { // create a cache for cluster scoped resources gCache, err := New(config, opts) if err != nil { - return nil, fmt.Errorf("error creating global cache %v", err) + return nil, fmt.Errorf("error creating global cache: %w", err) } for _, ns := range namespaces { diff --git a/pkg/client/config/config.go b/pkg/client/config/config.go index da87f2bd43..fd27724127 100644 --- a/pkg/client/config/config.go +++ b/pkg/client/config/config.go @@ -123,7 +123,7 @@ func loadConfig(context string) (*rest.Config, error) { if _, ok := os.LookupEnv("HOME"); !ok { u, err := user.Current() if err != nil { - return nil, fmt.Errorf("could not get current user: %v", err) + return nil, fmt.Errorf("could not get current user: %w", err) } loadingRules.Precedence = append(loadingRules.Precedence, filepath.Join(u.HomeDir, clientcmd.RecommendedHomeDir, clientcmd.RecommendedFileName)) } diff --git a/pkg/client/fake/client.go b/pkg/client/fake/client.go index cabc4ee7cf..00d5b3bfe2 100644 --- a/pkg/client/fake/client.go +++ b/pkg/client/fake/client.go @@ -215,7 +215,7 @@ func (t versionedTracker) Add(obj runtime.Object) error { func (t versionedTracker) Create(gvr schema.GroupVersionResource, obj runtime.Object, ns string) error { accessor, err := meta.Accessor(obj) if err != nil { - return fmt.Errorf("failed to get accessor for object: %v", err) + return fmt.Errorf("failed to get accessor for object: %w", err) } if accessor.GetName() == "" { return apierrors.NewInvalid( @@ -269,7 +269,7 @@ func convertFromUnstructuredIfNecessary(s *runtime.Scheme, o runtime.Object) (ru func (t versionedTracker) Update(gvr schema.GroupVersionResource, obj runtime.Object, ns string) error { accessor, err := meta.Accessor(obj) if err != nil { - return fmt.Errorf("failed to get accessor for object: %v", err) + return fmt.Errorf("failed to get accessor for object: %w", err) } if accessor.GetName() == "" { @@ -315,7 +315,7 @@ func (t versionedTracker) Update(gvr schema.GroupVersionResource, obj runtime.Ob } intResourceVersion, err := strconv.ParseUint(oldAccessor.GetResourceVersion(), 10, 64) if err != nil { - return fmt.Errorf("can not convert resourceVersion %q to int: %v", oldAccessor.GetResourceVersion(), err) + return fmt.Errorf("can not convert resourceVersion %q to int: %w", oldAccessor.GetResourceVersion(), err) } intResourceVersion++ accessor.SetResourceVersion(strconv.FormatUint(intResourceVersion, 10)) diff --git a/pkg/client/namespaced_client.go b/pkg/client/namespaced_client.go index 557598727c..45a694543e 100644 --- a/pkg/client/namespaced_client.go +++ b/pkg/client/namespaced_client.go @@ -56,7 +56,7 @@ func (n *namespacedClient) RESTMapper() meta.RESTMapper { func (n *namespacedClient) Create(ctx context.Context, obj Object, opts ...CreateOption) error { isNamespaceScoped, err := objectutil.IsAPINamespaced(obj, n.Scheme(), n.RESTMapper()) if err != nil { - return fmt.Errorf("error finding the scope of the object: %v", err) + return fmt.Errorf("error finding the scope of the object: %w", err) } objectNamespace := obj.GetNamespace() @@ -74,7 +74,7 @@ func (n *namespacedClient) Create(ctx context.Context, obj Object, opts ...Creat func (n *namespacedClient) Update(ctx context.Context, obj Object, opts ...UpdateOption) error { isNamespaceScoped, err := objectutil.IsAPINamespaced(obj, n.Scheme(), n.RESTMapper()) if err != nil { - return fmt.Errorf("error finding the scope of the object: %v", err) + return fmt.Errorf("error finding the scope of the object: %w", err) } objectNamespace := obj.GetNamespace() @@ -92,7 +92,7 @@ func (n *namespacedClient) Update(ctx context.Context, obj Object, opts ...Updat func (n *namespacedClient) Delete(ctx context.Context, obj Object, opts ...DeleteOption) error { isNamespaceScoped, err := objectutil.IsAPINamespaced(obj, n.Scheme(), n.RESTMapper()) if err != nil { - return fmt.Errorf("error finding the scope of the object: %v", err) + return fmt.Errorf("error finding the scope of the object: %w", err) } objectNamespace := obj.GetNamespace() @@ -110,7 +110,7 @@ func (n *namespacedClient) Delete(ctx context.Context, obj Object, opts ...Delet func (n *namespacedClient) DeleteAllOf(ctx context.Context, obj Object, opts ...DeleteAllOfOption) error { isNamespaceScoped, err := objectutil.IsAPINamespaced(obj, n.Scheme(), n.RESTMapper()) if err != nil { - return fmt.Errorf("error finding the scope of the object: %v", err) + return fmt.Errorf("error finding the scope of the object: %w", err) } if isNamespaceScoped { @@ -123,7 +123,7 @@ func (n *namespacedClient) DeleteAllOf(ctx context.Context, obj Object, opts ... func (n *namespacedClient) Patch(ctx context.Context, obj Object, patch Patch, opts ...PatchOption) error { isNamespaceScoped, err := objectutil.IsAPINamespaced(obj, n.Scheme(), n.RESTMapper()) if err != nil { - return fmt.Errorf("error finding the scope of the object: %v", err) + return fmt.Errorf("error finding the scope of the object: %w", err) } objectNamespace := obj.GetNamespace() @@ -141,7 +141,7 @@ func (n *namespacedClient) Patch(ctx context.Context, obj Object, patch Patch, o func (n *namespacedClient) Get(ctx context.Context, key ObjectKey, obj Object) error { isNamespaceScoped, err := objectutil.IsAPINamespaced(obj, n.Scheme(), n.RESTMapper()) if err != nil { - return fmt.Errorf("error finding the scope of the object: %v", err) + return fmt.Errorf("error finding the scope of the object: %w", err) } if isNamespaceScoped { if key.Namespace != "" && key.Namespace != n.namespace { @@ -179,7 +179,7 @@ func (nsw *namespacedClientStatusWriter) Update(ctx context.Context, obj Object, isNamespaceScoped, err := objectutil.IsAPINamespaced(obj, nsw.namespacedclient.Scheme(), nsw.namespacedclient.RESTMapper()) if err != nil { - return fmt.Errorf("error finding the scope of the object: %v", err) + return fmt.Errorf("error finding the scope of the object: %w", err) } objectNamespace := obj.GetNamespace() @@ -198,7 +198,7 @@ func (nsw *namespacedClientStatusWriter) Patch(ctx context.Context, obj Object, isNamespaceScoped, err := objectutil.IsAPINamespaced(obj, nsw.namespacedclient.Scheme(), nsw.namespacedclient.RESTMapper()) if err != nil { - return fmt.Errorf("error finding the scope of the object: %v", err) + return fmt.Errorf("error finding the scope of the object: %w", err) } objectNamespace := obj.GetNamespace() diff --git a/pkg/envtest/crd.go b/pkg/envtest/crd.go index 27e08475ec..dfa2e716ea 100644 --- a/pkg/envtest/crd.go +++ b/pkg/envtest/crd.go @@ -20,6 +20,7 @@ import ( "bufio" "bytes" "context" + "errors" "fmt" "io" "io/ioutil" @@ -437,7 +438,7 @@ func readDocuments(fp string) ([][]byte, error) { // Read document doc, err := reader.Read() if err != nil { - if err == io.EOF { + if errors.Is(err, io.EOF) { break } diff --git a/pkg/envtest/webhook.go b/pkg/envtest/webhook.go index 8552d3ba61..79dac3fbe7 100644 --- a/pkg/envtest/webhook.go +++ b/pkg/envtest/webhook.go @@ -117,7 +117,7 @@ func (o *WebhookInstallOptions) generateHostPort() (string, error) { if o.LocalServingPort == 0 { port, host, err := addr.Suggest(o.LocalServingHost) if err != nil { - return "", fmt.Errorf("unable to grab random port for serving webhooks on: %v", err) + return "", fmt.Errorf("unable to grab random port for serving webhooks on: %w", err) } o.LocalServingPort = port o.LocalServingHost = host @@ -180,7 +180,7 @@ func WaitForWebhooks(config *rest.Config, h := hook gvk, err := apiutil.GVKForObject(h, scheme.Scheme) if err != nil { - return fmt.Errorf("unable to get gvk for MutatingWebhookConfiguration %s: %v", hook.GetName(), err) + return fmt.Errorf("unable to get gvk for MutatingWebhookConfiguration %s: %w", hook.GetName(), err) } if _, ok := waitingFor[gvk]; !ok { @@ -193,7 +193,7 @@ func WaitForWebhooks(config *rest.Config, h := hook gvk, err := apiutil.GVKForObject(h, scheme.Scheme) if err != nil { - return fmt.Errorf("unable to get gvk for ValidatingWebhookConfiguration %s: %v", hook.GetName(), err) + return fmt.Errorf("unable to get gvk for ValidatingWebhookConfiguration %s: %w", hook.GetName(), err) } if _, ok := waitingFor[gvk]; !ok { @@ -257,31 +257,31 @@ func (p *webhookPoller) poll() (done bool, err error) { func (o *WebhookInstallOptions) setupCA() error { hookCA, err := certs.NewTinyCA() if err != nil { - return fmt.Errorf("unable to set up webhook CA: %v", err) + return fmt.Errorf("unable to set up webhook CA: %w", err) } names := []string{"localhost", o.LocalServingHost, o.LocalServingHostExternalName} hookCert, err := hookCA.NewServingCert(names...) if err != nil { - return fmt.Errorf("unable to set up webhook serving certs: %v", err) + return fmt.Errorf("unable to set up webhook serving certs: %w", err) } localServingCertsDir, err := ioutil.TempDir("", "envtest-serving-certs-") o.LocalServingCertDir = localServingCertsDir if err != nil { - return fmt.Errorf("unable to create directory for webhook serving certs: %v", err) + return fmt.Errorf("unable to create directory for webhook serving certs: %w", err) } certData, keyData, err := hookCert.AsBytes() if err != nil { - return fmt.Errorf("unable to marshal webhook serving certs: %v", err) + return fmt.Errorf("unable to marshal webhook serving certs: %w", err) } if err := ioutil.WriteFile(filepath.Join(localServingCertsDir, "tls.crt"), certData, 0640); err != nil { //nolint:gosec - return fmt.Errorf("unable to write webhook serving cert to disk: %v", err) + return fmt.Errorf("unable to write webhook serving cert to disk: %w", err) } if err := ioutil.WriteFile(filepath.Join(localServingCertsDir, "tls.key"), keyData, 0640); err != nil { //nolint:gosec - return fmt.Errorf("unable to write webhook serving key to disk: %v", err) + return fmt.Errorf("unable to write webhook serving key to disk: %w", err) } o.LocalServingCAData = certData diff --git a/pkg/finalizer/finalizer.go b/pkg/finalizer/finalizer.go index 1f627f8c49..10c5645dbe 100644 --- a/pkg/finalizer/finalizer.go +++ b/pkg/finalizer/finalizer.go @@ -64,7 +64,7 @@ func (f finalizers) Finalize(ctx context.Context, obj client.Object) (Result, er // object (e.g. it may set a condition and need a status update). res.Updated = res.Updated || finalizerRes.Updated res.StatusUpdated = res.StatusUpdated || finalizerRes.StatusUpdated - errList = append(errList, fmt.Errorf("finalizer %q failed: %v", key, err)) + errList = append(errList, fmt.Errorf("finalizer %q failed: %w", key, err)) } else { // If the finalizer succeeds, we remove the finalizer from the primary // object's metadata, so we know it will need an update. diff --git a/pkg/internal/testing/certs/tinyca.go b/pkg/internal/testing/certs/tinyca.go index 55b044c5b3..b4188237e6 100644 --- a/pkg/internal/testing/certs/tinyca.go +++ b/pkg/internal/testing/certs/tinyca.go @@ -64,7 +64,7 @@ func (k CertPair) AsBytes() (cert []byte, key []byte, err error) { rawKeyData, err := x509.MarshalPKCS8PrivateKey(k.Key) if err != nil { - return nil, nil, fmt.Errorf("unable to encode private key: %v", err) + return nil, nil, fmt.Errorf("unable to encode private key: %w", err) } key = pem.EncodeToMemory(&pem.Block{ @@ -95,12 +95,12 @@ func newPrivateKey() (crypto.Signer, error) { func NewTinyCA() (*TinyCA, error) { caPrivateKey, err := newPrivateKey() if err != nil { - return nil, fmt.Errorf("unable to generate private key for CA: %v", err) + return nil, fmt.Errorf("unable to generate private key for CA: %w", err) } caCfg := certutil.Config{CommonName: "envtest-environment", Organization: []string{"envtest"}} caCert, err := certutil.NewSelfSignedCACert(caCfg, caPrivateKey) if err != nil { - return nil, fmt.Errorf("unable to generate certificate for CA: %v", err) + return nil, fmt.Errorf("unable to generate certificate for CA: %w", err) } return &TinyCA{ @@ -115,7 +115,7 @@ func (c *TinyCA) makeCert(cfg certutil.Config) (CertPair, error) { key, err := newPrivateKey() if err != nil { - return CertPair{}, fmt.Errorf("unable to create private key: %v", err) + return CertPair{}, fmt.Errorf("unable to create private key: %w", err) } serial := new(big.Int).Set(c.nextSerial) @@ -140,12 +140,12 @@ func (c *TinyCA) makeCert(cfg certutil.Config) (CertPair, error) { certRaw, err := x509.CreateCertificate(crand.Reader, &template, c.CA.Cert, key.Public(), c.CA.Key) if err != nil { - return CertPair{}, fmt.Errorf("unable to create certificate: %v", err) + return CertPair{}, fmt.Errorf("unable to create certificate: %w", err) } cert, err := x509.ParseCertificate(certRaw) if err != nil { - return CertPair{}, fmt.Errorf("generated invalid certificate, could not parse: %v", err) + return CertPair{}, fmt.Errorf("generated invalid certificate, could not parse: %w", err) } return CertPair{ diff --git a/pkg/internal/testing/process/arguments_test.go b/pkg/internal/testing/process/arguments_test.go index 2534eb8446..386a3ec52f 100644 --- a/pkg/internal/testing/process/arguments_test.go +++ b/pkg/internal/testing/process/arguments_test.go @@ -225,12 +225,6 @@ var _ = Describe("Arguments Templates", func() { }) }) -type plainDefaults map[string][]string - -func (d plainDefaults) DefaultArgs() map[string][]string { - return d -} - var _ = Describe("Arguments", func() { Context("when appending", func() { It("should copy from defaults when appending for the first time", func() { diff --git a/pkg/manager/internal.go b/pkg/manager/internal.go index 6fb59abdc4..5b22c628f9 100644 --- a/pkg/manager/internal.go +++ b/pkg/manager/internal.go @@ -457,21 +457,21 @@ func (cm *controllerManager) Start(ctx context.Context) (err error) { // between conversion webhooks and the cache sync (usually initial list) which causes the webhooks // to never start because no cache can be populated. if err := cm.runnables.Webhooks.Start(cm.internalCtx); err != nil { - if err != wait.ErrWaitTimeout { + if !errors.Is(err, wait.ErrWaitTimeout) { return err } } // Start and wait for caches. if err := cm.runnables.Caches.Start(cm.internalCtx); err != nil { - if err != wait.ErrWaitTimeout { + if !errors.Is(err, wait.ErrWaitTimeout) { return err } } // Start the non-leaderelection Runnables after the cache has synced. if err := cm.runnables.Others.Start(cm.internalCtx); err != nil { - if err != wait.ErrWaitTimeout { + if !errors.Is(err, wait.ErrWaitTimeout) { return err } } @@ -587,7 +587,7 @@ func (cm *controllerManager) engageStopProcedure(stopComplete <-chan struct{}) e }() <-cm.shutdownCtx.Done() - if err := cm.shutdownCtx.Err(); err != nil && err != context.Canceled { + if err := cm.shutdownCtx.Err(); err != nil && !errors.Is(err, context.Canceled) { if errors.Is(err, context.DeadlineExceeded) { if cm.gracefulShutdownTimeout > 0 { return fmt.Errorf("failed waiting for all runnables to end within grace period of %s: %w", cm.gracefulShutdownTimeout, err) @@ -597,6 +597,7 @@ func (cm *controllerManager) engageStopProcedure(stopComplete <-chan struct{}) e // For any other error, return the error. return err } + return nil } diff --git a/pkg/manager/manager.go b/pkg/manager/manager.go index f6c4d6f144..428cb15a12 100644 --- a/pkg/manager/manager.go +++ b/pkg/manager/manager.go @@ -533,7 +533,7 @@ func defaultHealthProbeListener(addr string) (net.Listener, error) { ln, err := net.Listen("tcp", addr) if err != nil { - return nil, fmt.Errorf("error listening on %s: %v", addr, err) + return nil, fmt.Errorf("error listening on %s: %w", addr, err) } return ln, nil } diff --git a/pkg/manager/manager_test.go b/pkg/manager/manager_test.go index 2d78872579..7962ed9eb7 100644 --- a/pkg/manager/manager_test.go +++ b/pkg/manager/manager_test.go @@ -1806,12 +1806,6 @@ type startSignalingInformer struct { cache.Cache } -func (c *startSignalingInformer) started() bool { - c.mu.Lock() - defer c.mu.Unlock() - return c.wasStarted -} - func (c *startSignalingInformer) Start(ctx context.Context) error { c.mu.Lock() c.wasStarted = true diff --git a/pkg/source/source.go b/pkg/source/source.go index ad18c9eedb..241c582eff 100644 --- a/pkg/source/source.go +++ b/pkg/source/source.go @@ -181,7 +181,7 @@ func (ks *Kind) WaitForSync(ctx context.Context) error { return err case <-ctx.Done(): ks.startCancel() - if ctx.Err() == context.Canceled { + if errors.Is(ctx.Err(), context.Canceled) { return nil } return errors.New("timed out waiting for cache to be synced") diff --git a/pkg/webhook/server.go b/pkg/webhook/server.go index 364d0f902e..20e8fea282 100644 --- a/pkg/webhook/server.go +++ b/pkg/webhook/server.go @@ -243,7 +243,7 @@ func (s *Server) Start(ctx context.Context) error { certPool := x509.NewCertPool() clientCABytes, err := ioutil.ReadFile(filepath.Join(s.CertDir, s.ClientCAName)) if err != nil { - return fmt.Errorf("failed to read client CA cert: %v", err) + return fmt.Errorf("failed to read client CA cert: %w", err) } ok := certPool.AppendCertsFromPEM(clientCABytes) @@ -305,11 +305,11 @@ func (s *Server) StartedChecker() healthz.Checker { d := &net.Dialer{Timeout: 10 * time.Second} conn, err := tls.DialWithDialer(d, "tcp", net.JoinHostPort(s.Host, strconv.Itoa(s.Port)), config) if err != nil { - return fmt.Errorf("webhook server is not reachable: %v", err) + return fmt.Errorf("webhook server is not reachable: %w", err) } if err := conn.Close(); err != nil { - return fmt.Errorf("webhook server is not reachable: closing connection: %v", err) + return fmt.Errorf("webhook server is not reachable: closing connection: %w", err) } return nil diff --git a/tools/setup-envtest/remote/client.go b/tools/setup-envtest/remote/client.go index 00e4840813..be82532583 100644 --- a/tools/setup-envtest/remote/client.go +++ b/tools/setup-envtest/remote/client.go @@ -8,6 +8,7 @@ import ( "crypto/md5" //nolint:gosec "encoding/base64" "encoding/json" + "errors" "fmt" "io" "net/http" @@ -160,7 +161,7 @@ func (c *Client) GetVersion(ctx context.Context, version versions.Concrete, plat checksum := md5.New() //nolint:gosec for cont := true; cont; { amt, err := resp.Body.Read(buf) - if err != nil && err != io.EOF { + if err != nil && !errors.Is(err, io.EOF) { return fmt.Errorf("unable read next chunk of %s: %w", itemName, err) } if amt > 0 { @@ -170,7 +171,7 @@ func (c *Client) GetVersion(ctx context.Context, version versions.Concrete, plat return fmt.Errorf("unable write next chunk of %s: %w", itemName, err) } } - cont = amt > 0 && err != io.EOF + cont = amt > 0 && !errors.Is(err, io.EOF) } sum := base64.StdEncoding.EncodeToString(checksum.Sum(nil)) diff --git a/tools/setup-envtest/store/store.go b/tools/setup-envtest/store/store.go index df6652bd3a..e6f258e4ac 100644 --- a/tools/setup-envtest/store/store.go +++ b/tools/setup-envtest/store/store.go @@ -182,7 +182,7 @@ func (s *Store) Add(ctx context.Context, item Item, contents io.Reader) (resErr return err } } - if err != nil && err != io.EOF { + if err != nil && !errors.Is(err, io.EOF) { return fmt.Errorf("unable to finish un-tar-ing the downloaded archive: %w", err) } log.V(1).Info("unpacked archive") diff --git a/tools/setup-envtest/workflows/workflows_testutils_test.go b/tools/setup-envtest/workflows/workflows_testutils_test.go index a20cf1a2ac..c50b6f50ae 100644 --- a/tools/setup-envtest/workflows/workflows_testutils_test.go +++ b/tools/setup-envtest/workflows/workflows_testutils_test.go @@ -84,15 +84,6 @@ var ( } ) -type platformVer struct { - versions.Platform - versions.Concrete -} - -func (p platformVer) String() string { - return p.Platform.BaseName(p.Concrete) -} - type item struct { meta bucketObject contents []byte