ci(lint): add two more linters gocritic and errorlint (#69)

* ci(lint): add two more linters gocritic and errorlint

* fix

Author: zoumo

.golangci.yaml

@@ -11,37 +11,27 @@ linters:
     - unconvert
     - unparam
     - whitespace
+    - gocritic
+    - errorlint
   settings:
-    dupl:
-      threshold: 100
-    funlen:
-      lines: 100
-      statements: 50
-    gocritic:
-      disabled-checks:
-        - dupImport
-        - ifElseChain
-        - octalLiteral
-        - wrapperFunc
-      enabled-tags:
-        - diagnostic
-        - experimental
-        - opinionated
-        - performance
-        - style
-    gocyclo:
-      min-complexity: 15
     govet:
       enable:
         - shadow
     misspell:
       locale: US
-    revive:
-      confidence: 0.1
-      enable-all-rules: true
     staticcheck:
       checks:
        - "-QF1008"
+    gocritic:
+      enable-all: true
+      disabled-checks:
+        - emptyStringTest
+        - ifElseChain
+        - singleCaseSwitch
+        - hugeParam
+        - unnamedResult
+        - whyNoLint
+        - tooManyResultsChecker
   exclusions:
     generated: lax
     presets:
@@ -51,11 +41,8 @@ linters:
       - std-error-handling
     rules:
       - linters:
-          - dupl
-          - errcheck
-          - gocyclo
-          - gosec
-          - unparam
+        - errcheck
+        - unparam
         path: _test.go
       - linters:
           - govet
@@ -88,3 +75,4 @@ formatters:
 
 run:
   allow-parallel-runners: true
+  timeout: 10m

cache/cache.go

@@ -21,6 +21,7 @@ import (
 	"context"
 	"fmt"
 	"reflect"
+	"slices"
 	"strings"
 
 	apimeta "k8s.io/apimachinery/pkg/api/meta"
@@ -92,7 +93,7 @@ func (c *cacheSupportDisableDeepCopy) ListWithoutDeepCopy(ctx context.Context, l
 	}
 
 	indexer := sharedInformer.GetIndexer()
-	var objs []interface{}
+	var objs []any
 
 	switch {
 	case listOpts.FieldSelector != nil:
@@ -149,8 +150,8 @@ func (c *cacheSupportDisableDeepCopy) ListWithoutDeepCopy(ctx context.Context, l
 // objectTypeForListObject tries to find the runtime.Object and associated GVK
 // for a single object corresponding to the passed-in list type. We need them
 // because they are used as cache map key.
-func (ip *cacheSupportDisableDeepCopy) objectTypeForListObject(list client.ObjectList) (*schema.GroupVersionKind, runtime.Object, error) {
-	gvk, err := apiutil.GVKForObject(list, ip.scheme)
+func (c *cacheSupportDisableDeepCopy) objectTypeForListObject(list client.ObjectList) (*schema.GroupVersionKind, runtime.Object, error) {
+	gvk, err := apiutil.GVKForObject(list, c.scheme)
 	if err != nil {
 		return nil, nil, err
 	}
@@ -203,10 +204,10 @@ func RequiresExactMatch(sel fields.Selector) bool {
 	return true
 }
 
-func byIndexes(indexer clienttoolcache.Indexer, requires fields.Requirements, namespace string) ([]interface{}, error) {
+func byIndexes(indexer clienttoolcache.Indexer, requires fields.Requirements, namespace string) ([]any, error) {
 	var (
 		err  error
-		objs []interface{}
+		objs []any
 		vals []string
 	)
 	indexers := indexer.GetIndexers()
@@ -230,17 +231,14 @@ func byIndexes(indexer clienttoolcache.Indexer, requires fields.Requirements, na
 		if !exist {
 			return nil, fmt.Errorf("index with name %s does not exist", indexName)
 		}
-		filteredObjects := make([]interface{}, 0, len(objs))
+		filteredObjects := make([]any, 0, len(objs))
 		for _, obj := range objs {
 			vals, err = fn(obj)
 			if err != nil {
 				return nil, err
 			}
-			for _, val := range vals {
-				if val == indexedValue {
-					filteredObjects = append(filteredObjects, obj)
-					break
-				}
+			if slices.Contains(vals, indexedValue) {
+				filteredObjects = append(filteredObjects, obj)
 			}
 		}
 		if len(filteredObjects) == 0 {

cert/cert.go

@@ -24,13 +24,12 @@ import (
 	"net"
 	"time"
 
-	"github.com/zoumo/golib/cert"
 	certutil "github.com/zoumo/golib/cert"
 )
 
 type (
-	Config   = cert.Config
-	AltNames = cert.AltNames
+	Config   = certutil.Config
+	AltNames = certutil.AltNames
 )
 
 // ServingCerts is a set of serving certificates.
@@ -56,7 +55,7 @@ func (c *ServingCerts) Validate(host string) error {
 		return fmt.Errorf("CA certificate is empty")
 	}
 
-	tlsCert, err := cert.X509KeyPair(c.Cert, c.Key)
+	tlsCert, err := certutil.X509KeyPair(c.Cert, c.Key)
 	if err != nil {
 		return fmt.Errorf("invalid x509 keypair: %w", err)
 	}
@@ -99,7 +98,7 @@ func GenerateSelfSignedCerts(cfg Config) (*ServingCerts, error) {
 
 // GenerateSelfSignedCertKeyIfNotExist generates a self-signed certificate and
 // write them to the given path if not exist.
-func GenerateSelfSignedCertKeyIfNotExist(path string, cfg cert.Config) error {
+func GenerateSelfSignedCertKeyIfNotExist(path string, cfg Config) error {
 	fscerts, err := NewFSCertProvider(path, FSOptions{})
 	if err != nil {
 		return err

cert/error.go

@@ -26,7 +26,7 @@ import (
 var errNotFound = errors.New("not found")
 
 func newNotFound(name string, err error) error {
-	return fmt.Errorf("%s %w: %v", name, errNotFound, err)
+	return fmt.Errorf("%s %w: %w", name, errNotFound, err)
 }
 
 // IsNotFound returns true if certificate not found.

cert/fs.go

@@ -182,28 +182,28 @@ func (p *FSCertProvider) Overwrite(certs *ServingCerts) (bool, error) {
 	var updated bool
 	changed, err := p.writeFile(keyPath, certs.Key)
 	if err != nil {
-		return false, fmt.Errorf("failed to write key to %s: %v", keyPath, err)
+		return false, fmt.Errorf("failed to write key to %s: %w", keyPath, err)
 	}
 	updated = changed || updated
 
 	certPath := path.Join(p.path, p.CertName)
 	changed, err = p.writeFile(certPath, certs.Cert)
 	if err != nil {
-		return false, fmt.Errorf("failed to write cert to %s: %v", certPath, err)
+		return false, fmt.Errorf("failed to write cert to %s: %w", certPath, err)
 	}
 	updated = changed || updated
 
 	caKeyPath := path.Join(p.path, p.CAKeyName)
 	changed, err = p.writeFile(caKeyPath, certs.CAKey)
 	if err != nil {
-		return false, fmt.Errorf("failed to write ca key to %s: %v", caKeyPath, err)
+		return false, fmt.Errorf("failed to write ca key to %s: %w", caKeyPath, err)
 	}
 	updated = changed || updated
 
 	caCertPath := path.Join(p.path, p.CACertName)
 	changed, err = p.writeFile(caCertPath, certs.CACert)
 	if err != nil {
-		return false, fmt.Errorf("failed to write ca cert to %s: %v", caCertPath, err)
+		return false, fmt.Errorf("failed to write ca cert to %s: %w", caCertPath, err)
 	}
 	updated = changed || updated
 	return updated, nil

client/util.go

@@ -56,7 +56,7 @@ func UpdateOnConflict[T client.Object](
 	err = retry.RetryOnConflict(retry.DefaultBackoff, func() error {
 		if !first {
 			// refresh object
-			if innerErr := reader.Get(ctx, key, original); err != nil {
+			if innerErr := reader.Get(ctx, key, original); innerErr != nil {
 				return innerErr
 			}
 		} else {

condition/helper.go

@@ -60,6 +60,7 @@ func SetCondition(conditions []metav1.Condition, condition metav1.Condition) []m
 
 func RejectConditionByType(conditions []metav1.Condition, ctype string) []metav1.Condition {
 	result := []metav1.Condition{}
+
 	for i := range conditions {
 		c := conditions[i]
 		if c.Type == ctype {

controller/expectations/controller_expectation.go

@@ -54,7 +54,7 @@ const (
 // * Controllers that don't set expectations will get woken up for every matching controllee
 
 // ExpKeyFunc to parse out the key from a ControlleeExpectation
-var ExpKeyFunc = func(obj interface{}) (string, error) {
+var ExpKeyFunc = func(obj any) (string, error) {
 	if e, ok := obj.(ExpectationKey); ok {
 		return e.Key(), nil
 	}
@@ -138,13 +138,6 @@ func (r *ControllerExpectations) SatisfiedExpectations(controllerKey string) boo
 	return true
 }
 
-// TODO: Extend ExpirationCache to support explicit expiration.
-// TODO: Make this possible to disable in tests.
-// TODO: Support injection of clock.
-func (exp *ControlleeExpectations) isExpired() bool {
-	return clock.RealClock{}.Since(exp.timestamp) > ExpectationsTimeout
-}
-
 // SetExpectations registers new expectations for the given controller. Forgets existing expectations.
 func (r *ControllerExpectations) SetExpectations(controllerKey string, add, del int) error {
 	exp := &ControlleeExpectations{add: int64(add), del: int64(del), key: controllerKey, timestamp: clock.RealClock{}.Now()}
@@ -200,6 +193,11 @@ type ControlleeExpectations struct {
 	timestamp time.Time
 }
 
+// NewControllerExpectations returns a store for ControllerExpectations.
+func NewControllerExpectations() *ControllerExpectations {
+	return &ControllerExpectations{cache.NewStore(ExpKeyFunc)}
+}
+
 // Key implements ExpectationKey interface
 func (e *ControlleeExpectations) Key() string {
 	return e.key
@@ -222,7 +220,9 @@ func (e *ControlleeExpectations) GetExpectations() (int64, int64) {
 	return atomic.LoadInt64(&e.add), atomic.LoadInt64(&e.del)
 }
 
-// NewControllerExpectations returns a store for ControllerExpectations.
-func NewControllerExpectations() *ControllerExpectations {
-	return &ControllerExpectations{cache.NewStore(ExpKeyFunc)}
+// TODO: Extend ExpirationCache to support explicit expiration.
+// TODO: Make this possible to disable in tests.
+// TODO: Support injection of clock.
+func (exp *ControlleeExpectations) isExpired() bool {
+	return clock.RealClock{}.Since(exp.timestamp) > ExpectationsTimeout
 }

controller/history/revision_control.go

@@ -23,7 +23,6 @@ import (
 
 	"github.com/go-logr/logr"
 	"github.com/samber/lo"
-	apps "k8s.io/api/apps/v1"
 	appsv1 "k8s.io/api/apps/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -52,22 +51,22 @@ type RevisionControlInterface interface {
 	// error is not nil, creation failed. If the returned error is nil, the returned ControllerRevision has been
 	// created.
 	// Callers must make sure that collisionCount is not nil. An error is returned if it is.
-	CreateControllerRevision(ctx context.Context, parent metav1.Object, revision *apps.ControllerRevision, collisionCount *int32) (*apps.ControllerRevision, error)
+	CreateControllerRevision(ctx context.Context, parent metav1.Object, revision *appsv1.ControllerRevision, collisionCount *int32) (*appsv1.ControllerRevision, error)
 	// DeleteControllerRevision attempts to delete revision. If the returned error is not nil, deletion has failed.
-	DeleteControllerRevision(ctx context.Context, revision *apps.ControllerRevision) error
+	DeleteControllerRevision(ctx context.Context, revision *appsv1.ControllerRevision) error
 	// UpdateControllerRevision updates revision such that its Revision is equal to newRevision. Implementations
 	// may retry on conflict. If the returned error is nil, the update was successful and returned ControllerRevision
 	// is valid. If the returned error is not nil, the update failed and the returned ControllerRevision is invalid.
-	UpdateControllerRevision(ctx context.Context, revision *apps.ControllerRevision, newRevision int64) (*apps.ControllerRevision, error)
+	UpdateControllerRevision(ctx context.Context, revision *appsv1.ControllerRevision, newRevision int64) (*appsv1.ControllerRevision, error)
 	// AdoptControllerRevision attempts to adopt revision by adding a ControllerRef indicating that the parent
 	// Object of parentKind is the owner of revision. If revision is already owned, an error is returned. If the
 	// resource patch fails, an error is returned. If no error is returned, the returned ControllerRevision is
 	// valid.
-	AdoptControllerRevision(ctx context.Context, parent metav1.Object, parentKind schema.GroupVersionKind, revision *apps.ControllerRevision) (*apps.ControllerRevision, error)
+	AdoptControllerRevision(ctx context.Context, parent metav1.Object, parentKind schema.GroupVersionKind, revision *appsv1.ControllerRevision) (*appsv1.ControllerRevision, error)
 	// ReleaseControllerRevision attempts to release parent's ownership of revision by removing parent from the
 	// OwnerReferences of revision. If an error is returned, parent remains the owner of revision. If no error is
 	// returned, the returned ControllerRevision is valid.
-	ReleaseControllerRevision(ctx context.Context, parent metav1.Object, revision *apps.ControllerRevision) (*apps.ControllerRevision, error)
+	ReleaseControllerRevision(ctx context.Context, parent metav1.Object, revision *appsv1.ControllerRevision) (*appsv1.ControllerRevision, error)
 }
 
 var _ RevisionControlInterface = &realRevisionControl{}
@@ -103,7 +102,7 @@ func (h *realRevisionControl) ListControllerRevisions(ctx context.Context, paren
 	return result, nil
 }
 
-func (h *realRevisionControl) CreateControllerRevision(ctx context.Context, parent metav1.Object, revision *apps.ControllerRevision, collisionCount *int32) (*apps.ControllerRevision, error) {
+func (h *realRevisionControl) CreateControllerRevision(ctx context.Context, parent metav1.Object, revision *appsv1.ControllerRevision, collisionCount *int32) (*appsv1.ControllerRevision, error) {
 	if collisionCount == nil {
 		return nil, fmt.Errorf("collisionCount should not be nil")
 	}
@@ -126,7 +125,7 @@ func (h *realRevisionControl) CreateControllerRevision(ctx context.Context, pare
 		clone.Name = ControllerRevisionName(parent.GetName(), hash)
 		err = h.writer.Create(ctx, clone)
 		if errors.IsAlreadyExists(err) {
-			exists := &apps.ControllerRevision{}
+			exists := &appsv1.ControllerRevision{}
 			err := h.reader.Get(ctx, types.NamespacedName{Namespace: clone.Namespace, Name: clone.Name}, exists)
 			if err != nil {
 				return nil, err
@@ -151,7 +150,7 @@ func (h *realRevisionControl) CreateControllerRevision(ctx context.Context, pare
 func (h *realRevisionControl) UpdateControllerRevision(ctx context.Context, revision *appsv1.ControllerRevision, newRevision int64) (*appsv1.ControllerRevision, error) {
 	clone := revision.DeepCopy()
 	oldRevision := clone.Revision
-	changed, err := clientutil.UpdateOnConflict(ctx, h.reader, h.writer, clone, func(obj *apps.ControllerRevision) error {
+	changed, err := clientutil.UpdateOnConflict(ctx, h.reader, h.writer, clone, func(obj *appsv1.ControllerRevision) error {
 		oldRevision = obj.Revision
 		obj.Revision = newRevision
 		return nil
@@ -163,7 +162,7 @@ func (h *realRevisionControl) UpdateControllerRevision(ctx context.Context, revi
 	return clone, err
 }
 
-func (h *realRevisionControl) DeleteControllerRevision(ctx context.Context, revision *apps.ControllerRevision) error {
+func (h *realRevisionControl) DeleteControllerRevision(ctx context.Context, revision *appsv1.ControllerRevision) error {
 	err := h.writer.Delete(ctx, revision)
 	if err == nil {
 		logger := logr.FromContextOrDiscard(ctx)
@@ -173,7 +172,7 @@ func (h *realRevisionControl) DeleteControllerRevision(ctx context.Context, revi
 	return client.IgnoreNotFound(err)
 }
 
-func (h *realRevisionControl) AdoptControllerRevision(ctx context.Context, parent metav1.Object, parentKind schema.GroupVersionKind, revision *apps.ControllerRevision) (*apps.ControllerRevision, error) {
+func (h *realRevisionControl) AdoptControllerRevision(ctx context.Context, parent metav1.Object, parentKind schema.GroupVersionKind, revision *appsv1.ControllerRevision) (*appsv1.ControllerRevision, error) {
 	oldOwner := metav1.GetControllerOfNoCopy(revision)
 	newOwner := metav1.NewControllerRef(parent, parentKind)
 
@@ -194,7 +193,7 @@ func (h *realRevisionControl) AdoptControllerRevision(ctx context.Context, paren
 	return revision, err
 }
 
-func (h *realRevisionControl) ReleaseControllerRevision(ctx context.Context, parent metav1.Object, revision *apps.ControllerRevision) (*apps.ControllerRevision, error) {
+func (h *realRevisionControl) ReleaseControllerRevision(ctx context.Context, parent metav1.Object, revision *appsv1.ControllerRevision) (*appsv1.ControllerRevision, error) {
 	w := refmanager.NewOwnerRefWriter(h.writer)
 	err := w.Release(ctx, parent, revision)
 	return revision, err

controller/history/revision_control_test.go

@@ -123,7 +123,6 @@ func (s *revisionControlTestSuite) TestCreateRevisionsConflict1() {
 	s.EqualValues(1, collisionCount, "create revision conflict, collisionCount should be 1")
 	s.NotEqual(revision.Name, created.Name, "create revision conflict, revision name should be changed")
 	s.Equal(revision.Labels[ControllerRevisionHashLabel], created.Labels[ControllerRevisionHashLabel], "the hash label should be the same")
-	// s.True(strings.HasSuffix(created.Name, created.Labels[ControllerRevisionHashLabel]), "the suffix of revision name should be the same with hash")
 
 	list := &appsv1.ControllerRevisionList{}
 	err = s.client.List(context.Background(), list, client.InNamespace(s.testObj.Namespace))
@@ -146,7 +145,6 @@ func (s *revisionControlTestSuite) TestCreateRevisionsConflict2() {
 	s.EqualValues(1, collisionCount, "create revision conflict, collisionCount should be 1")
 	s.NotEqual(revision.Name, created.Name)
 	s.Equal(revision.Labels[ControllerRevisionHashLabel], created.Labels[ControllerRevisionHashLabel], "the hash label should be the same")
-	// s.True(strings.HasSuffix(created.Name, created.Labels[ControllerRevisionHashLabel]), "the suffix of revision name should be the same with hash")
 
 	list := &appsv1.ControllerRevisionList{}
 	err = s.client.List(context.Background(), list, client.InNamespace(s.testObj.Namespace))

controller/history/revision_test.go

@@ -20,7 +20,6 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
-	apps "k8s.io/api/apps/v1"
 	appsv1 "k8s.io/api/apps/v1"
 	"k8s.io/utils/ptr"
 )
@@ -78,8 +77,8 @@ func TestHashControllerRevisionRawData(t *testing.T) {
 func TestEqualRevision(t *testing.T) {
 	tests := []struct {
 		name string
-		lrv  *apps.ControllerRevision
-		rrv  *apps.ControllerRevision
+		lrv  *appsv1.ControllerRevision
+		rrv  *appsv1.ControllerRevision
 		want bool
 	}{
 		{