Skip to content

Commit

Permalink
🌱 enable errorlint and unused linters (#1890)
Browse files Browse the repository at this point in the history
* use %w instead of %v for wrapping errors

* do not use == or != to compare errors

* remove unused code

* fix formatting
  • Loading branch information
xrstf committed May 6, 2022
1 parent 236ad94 commit c7a98aa
Show file tree
Hide file tree
Showing 18 changed files with 47 additions and 63 deletions.
2 changes: 2 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ linters:
- depguard
- dogsled
- errcheck
- errorlint
- exportloopref
- goconst
- gocritic
Expand Down Expand Up @@ -34,6 +35,7 @@ linters:
- typecheck
- unconvert
- unparam
- unused
- varcheck
- whitespace

Expand Down
2 changes: 1 addition & 1 deletion pkg/cache/multi_namespace_cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion pkg/client/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/client/fake/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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() == "" {
Expand Down Expand Up @@ -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))
Expand Down
16 changes: 8 additions & 8 deletions pkg/client/namespaced_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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()
Expand All @@ -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()
Expand All @@ -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 {
Expand All @@ -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()
Expand All @@ -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 {
Expand Down Expand Up @@ -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()
Expand All @@ -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()
Expand Down
3 changes: 2 additions & 1 deletion pkg/envtest/crd.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"bufio"
"bytes"
"context"
"errors"
"fmt"
"io"
"io/ioutil"
Expand Down Expand Up @@ -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
}

Expand Down
18 changes: 9 additions & 9 deletions pkg/envtest/webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion pkg/finalizer/finalizer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
12 changes: 6 additions & 6 deletions pkg/internal/testing/certs/tinyca.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand All @@ -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)
Expand All @@ -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{
Expand Down
6 changes: 0 additions & 6 deletions pkg/internal/testing/process/arguments_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
9 changes: 5 additions & 4 deletions pkg/manager/internal.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand Down Expand Up @@ -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)
Expand All @@ -597,6 +597,7 @@ func (cm *controllerManager) engageStopProcedure(stopComplete <-chan struct{}) e
// For any other error, return the error.
return err
}

return nil
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/manager/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
6 changes: 0 additions & 6 deletions pkg/manager/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion pkg/source/source.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
6 changes: 3 additions & 3 deletions pkg/webhook/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
Loading

0 comments on commit c7a98aa

Please sign in to comment.