chore: enable gocritic linter and fix errors

Signed-off-by: Anish Ramasekar <anish.ramasekar@gmail.com>

.golangci.yml

@@ -1,10 +1,20 @@
 run:
   timeout: 5m
-  go: '1.20'
-  skip-dirs: []
+  go: "1.20"
   skip-files:
   - "zz_generated.*\\.go$"
 
+linters-settings:
+  gocritic:
+    enabled-tags:
+    - performance
+  lll:
+    line-length: 200
+  misspell:
+    locale: US
+  staticcheck:
+    go: "1.20"
+
 linters:
   disable-all: true
   enable:
@@ -14,6 +24,7 @@ linters:
     - errorlint
     - exhaustive
     - goconst
+    - gocritic
     - gocyclo
     - gofmt
     - goimports
@@ -29,6 +40,7 @@ linters:
     - stylecheck
     - typecheck
     - unused
+    - whitespace
   # Run with --fast=false for more extensive checks (enables all linters)
   fast: true
 

pkg/k8s/token/token_manager.go

@@ -58,7 +58,8 @@ func NewManager(c clientset.Interface) *Manager {
 			if err != nil {
 				return
 			}
-			for _, resource := range resources.APIResources {
+			for idx := range resources.APIResources {
+				resource := &resources.APIResources[idx]
 				if resource.Name == "serviceaccounts/token" {
 					supported = true
 					return

pkg/k8s/token/token_manager_test.go

@@ -325,7 +325,7 @@ func TestDeleteServiceAccountToken(t *testing.T) {
 					shouldFail: true,
 				},
 				{
-					//exactly the same with last one, besides it will success
+					// exactly the same with last one, besides it will success
 					name:      "fake-name-4",
 					namespace: "fake-namespace-4",
 					tr: authenticationv1.TokenRequest{
@@ -519,7 +519,7 @@ func TestKeyFunc(t *testing.T) {
 				tr: &authenticationv1.TokenRequest{
 					Spec: authenticationv1.TokenRequestSpec{
 						Audiences: []string{"foo1", "foo2"},
-						//everthing is same besides ExpirationSeconds
+						// everything is same besides ExpirationSeconds
 						ExpirationSeconds: getInt64Point(2001),
 						BoundObjectRef: &authenticationv1.BoundObjectReference{
 							Kind: "pod",
@@ -559,7 +559,7 @@ func TestKeyFunc(t *testing.T) {
 						ExpirationSeconds: getInt64Point(2000),
 						BoundObjectRef: &authenticationv1.BoundObjectReference{
 							Kind: "pod",
-							//everthing is same besides BoundObjectRef.Name
+							// everything is same besides BoundObjectRef.Name
 							Name: "diff-pod",
 							UID:  "foo-uid",
 						},
@@ -577,7 +577,7 @@ func TestKeyFunc(t *testing.T) {
 			for _, tru := range c.trus {
 				mgr.set(getKeyFunc(tru), &authenticationv1.TokenRequest{
 					Status: authenticationv1.TokenRequestStatus{
-						//make sure the token cache would not be cleaned by token manager clenaup func
+						// make sure the token cache would not be cleaned by token manager cleanup func
 						ExpirationTimestamp: metav1.Time{Time: mgr.clock.Now().Add(50 * time.Minute)},
 					},
 				})
@@ -589,7 +589,6 @@ func TestKeyFunc(t *testing.T) {
 			}
 		})
 	}
-
 }
 
 func getTokenRequest() *authenticationv1.TokenRequest {

pkg/rotation/reconciler.go

@@ -51,6 +51,7 @@ import (
 	"k8s.io/client-go/tools/record"
 	"k8s.io/client-go/util/workqueue"
 	"k8s.io/klog/v2"
+	"monis.app/mlog"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
@@ -123,6 +124,12 @@ func NewReconciler(client client.Reader,
 
 // Run starts the rotation reconciler
 func (r *Reconciler) Run(stopCh <-chan struct{}) {
+	if err := r.runErr(stopCh); err != nil {
+		mlog.Fatal(err)
+	}
+}
+
+func (r *Reconciler) runErr(stopCh <-chan struct{}) error {
 	defer r.queue.ShutDown()
 	klog.InfoS("starting rotation reconciler", "rotationPollInterval", r.rotationPollInterval)
 
@@ -131,7 +138,7 @@ func (r *Reconciler) Run(stopCh <-chan struct{}) {
 
 	if err := r.secretStore.Run(stopCh); err != nil {
 		klog.ErrorS(err, "failed to run informers for rotation reconciler")
-		os.Exit(1)
+		return err
 	}
 
 	// TODO (aramase) consider adding more workers to process reconcile concurrently
@@ -142,7 +149,7 @@ func (r *Reconciler) Run(stopCh <-chan struct{}) {
 	for {
 		select {
 		case <-stopCh:
-			return
+			return nil
 		case <-ticker.C:
 			// The spc pod status informer is configured to do a filtered list watch of spc pod statuses
 			// labeled for the same node as the driver. LIST will only return the filtered results.

pkg/secrets-store/provider_client.go

@@ -193,7 +193,7 @@ func (p *PluginClientBuilder) Cleanup() {
 // HealthCheck enables periodic healthcheck for configured provider clients by making
 // a Version() RPC call. If the provider healthcheck fails, we log an error.
 //
-// This method blocks until the parent context is cancelled during termination.
+// This method blocks until the parent context is canceled during termination.
 func (p *PluginClientBuilder) HealthCheck(ctx context.Context, interval time.Duration) {
 	ticker := time.NewTicker(interval)
 	defer ticker.Stop()

pkg/secrets-store/server.go

@@ -32,6 +32,7 @@ import (
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/status"
 	"k8s.io/klog/v2"
+	"monis.app/mlog"
 )
 
 // Defines Non blocking GRPC server interfaces
@@ -60,7 +61,11 @@ type nonBlockingGRPCServer struct {
 // Start the server
 func (s *nonBlockingGRPCServer) Start(ctx context.Context, endpoint string, ids csi.IdentityServer, cs csi.ControllerServer, ns csi.NodeServer) {
 	s.wg.Add(1)
-	go s.serve(ctx, endpoint, ids, cs, ns)
+	go func() {
+		if err := s.serve(ctx, endpoint, ids, cs, ns); err != nil {
+			mlog.Fatal(err)
+		}
+	}()
 }
 
 // Wait for the server to stop
@@ -78,7 +83,7 @@ func (s *nonBlockingGRPCServer) ForceStop() {
 	s.server.Stop()
 }
 
-func (s *nonBlockingGRPCServer) serve(ctx context.Context, endpoint string, ids csi.IdentityServer, cs csi.ControllerServer, ns csi.NodeServer) {
+func (s *nonBlockingGRPCServer) serve(ctx context.Context, endpoint string, ids csi.IdentityServer, cs csi.ControllerServer, ns csi.NodeServer) error {
 	defer s.wg.Done()
 
 	proto, addr, err := parseEndpoint(endpoint)
@@ -92,14 +97,14 @@ func (s *nonBlockingGRPCServer) serve(ctx context.Context, endpoint string, ids
 		}
 		if err := os.Remove(addr); err != nil && !os.IsNotExist(err) {
 			klog.ErrorS(err, "failed to remove unix domain socket", "address", addr)
-			os.Exit(1)
+			return err
 		}
 	}
 
 	listener, err := net.Listen(proto, addr)
 	if err != nil {
 		klog.ErrorS(err, "failed to listen", "proto", proto, "address", addr)
-		os.Exit(1)
+		return err
 	}
 	defer listener.Close()
 
@@ -129,6 +134,7 @@ func (s *nonBlockingGRPCServer) serve(ctx context.Context, endpoint string, ids
 
 	<-ctx.Done()
 	server.GracefulStop()
+	return nil
 }
 
 func parseEndpoint(ep string) (string, string, error) {

pkg/secrets-store/utils.go

@@ -137,7 +137,7 @@ func createOrUpdateSecretProviderClassPodStatus(ctx context.Context, c client.Cl
 		if !apierrors.IsNotFound(err) {
 			return err
 		}
-		// the secret provider class pod status could be missing in the cache because it was labelled with a different node
+		// the secret provider class pod status could be missing in the cache because it was labeled with a different node
 		// label, so we need to get it from the API server
 		if err = reader.Get(ctx, client.ObjectKey{Name: spcpsName, Namespace: namespace}, spcps); err != nil {
 			return err

pkg/util/fileutil/atomic_writer_test.go

@@ -402,13 +402,14 @@ IAAAAAAAsDyZDwU=`
 
 		writer := &AtomicWriter{targetDir: targetDir, logContext: "-test-"}
 		err = writer.Write(tc.payload)
-		if err != nil && tc.success {
+		switch {
+		case err != nil && tc.success:
 			t.Errorf("%v: unexpected error writing payload: %v", tc.name, err)
 			continue
-		} else if err == nil && !tc.success {
+		case err == nil && !tc.success:
 			t.Errorf("%v: unexpected success", tc.name)
 			continue
-		} else if err != nil {
+		case err != nil:
 			continue
 		}
 
@@ -906,7 +907,6 @@ func TestValidatePayload(t *testing.T) {
 				t.Errorf("%v: unexpected payload paths: %v is not equal to %v", tc.name, realPaths, tc.expected)
 			}
 		}
-
 	}
 }
 

pkg/util/k8sutil/volume.go

@@ -25,7 +25,8 @@ import (
 // SPCVolume finds the Secret Provider Class volume from a Pod, or returns nil
 // if a volume could not be found.
 func SPCVolume(pod *corev1.Pod, spcName string) *corev1.Volume {
-	for i, vol := range pod.Spec.Volumes {
+	for idx := range pod.Spec.Volumes {
+		vol := &pod.Spec.Volumes[idx]
 		if vol.CSI == nil {
 			continue
 		}
@@ -35,7 +36,7 @@ func SPCVolume(pod *corev1.Pod, spcName string) *corev1.Volume {
 		if vol.CSI.VolumeAttributes["secretProviderClass"] != spcName {
 			continue
 		}
-		return &pod.Spec.Volumes[i]
+		return vol
 	}
 	return nil
 }

pkg/util/secretutil/secret.go

@@ -219,7 +219,7 @@ func GetSecretData(secretObjData []*secretsstorev1.SecretObjectData, secretType
 func GetSHAFromSecret(data map[string][]byte) (string, error) {
 	var values []string
 	for k, v := range data {
-		values = append(values, k+"="+string(v[:]))
+		values = append(values, k+"="+string(v))
 	}
 	// sort the values to always obtain a deterministic SHA for
 	// same content in different order

pkg/util/secretutil/secret_test.go

@@ -78,7 +78,7 @@ Katu6uOQ6tjRyEbx1/vXXPV7Peztr9/8daMeIAdbAoGBAOYRJ1CzMYQKjWF32Uas
 7hhQxyH1QI4nV56Dryq7l/UWun2pfwNLZFqOHD3qm05aznzNKvk9aHAsOPFfUUXO
 7sp0Ge5FLMSw1uMNnutcVcMz37KAY2fOoE2xoLM4DU/H2NqDjeGCsOsU1ReRS1vB
 J+42JGwBdLV99ruYKVKOWPh4
------END PRIVATE KEY-----	
+-----END PRIVATE KEY-----
 `
 	certPEM = `-----BEGIN CERTIFICATE-----
 MIIDOTCCAiGgAwIBAgIJAP0J5Z7N0Y5fMA0GCSqGSIb3DQEBCwUAMDMxFzAVBgNV
@@ -383,7 +383,7 @@ func createTestFile(tmpDir, fileName string) (string, error) {
 		if err != nil {
 			return filePath, err
 		}
-		_, err = f.Write([]byte("test"))
+		_, err = f.WriteString("test")
 		if err != nil {
 			return filePath, err
 		}

test/e2eprovider/e2e_provider.go

@@ -30,13 +30,20 @@ import (
 	"sigs.k8s.io/secrets-store-csi-driver/test/e2eprovider/server"
 
 	"k8s.io/klog/v2"
+	"monis.app/mlog"
 )
 
 var (
 	endpoint = flag.String("endpoint", "unix:///tmp/e2e-provider.sock", "CSI provier gRPC endpoint")
 )
 
 func main() {
+	if err := mainErr(); err != nil {
+		mlog.Fatal(err)
+	}
+}
+
+func mainErr() error {
 	klog.InitFlags(nil)
 	defer klog.Flush()
 
@@ -48,18 +55,18 @@ func main() {
 	mockProviderServer, err := server.NewE2EProviderServer(*endpoint)
 	if err != nil {
 		klog.ErrorS(err, "failed to get new mock e2e provider server")
-		os.Exit(1)
+		return err
 	}
 
 	if err := os.Remove(mockProviderServer.GetSocketPath()); err != nil && !os.IsNotExist(err) {
 		klog.ErrorS(err, "failed to remove unix domain socket", "socketPath", mockProviderServer.GetSocketPath())
-		os.Exit(1)
+		return err
 	}
 
 	err = mockProviderServer.Start()
 	if err != nil {
 		klog.ErrorS(err, "failed to start mock e2e provider server")
-		os.Exit(1)
+		return err
 	}
 
 	// endpoint to enable rotation response.
@@ -83,4 +90,5 @@ func main() {
 	// gracefully stop the grpc server
 	klog.InfoS("terminating the e2e provider server")
 	mockProviderServer.Stop()
+	return nil
 }

test/e2eprovider/go.mod

@@ -8,20 +8,40 @@ require (
 	github.com/google/go-cmp v0.5.8
 	google.golang.org/grpc v1.47.0
 	k8s.io/klog/v2 v2.80.1
+	monis.app/mlog v0.0.2
 	sigs.k8s.io/secrets-store-csi-driver v0.0.0-00010101000000-000000000000
 	sigs.k8s.io/yaml v1.3.0
 )
 
 require (
+	github.com/davecgh/go-spew v1.1.1 // indirect
 	github.com/go-logr/logr v1.2.3 // indirect
+	github.com/go-logr/zapr v1.2.3 // indirect
+	github.com/gogo/protobuf v1.3.2 // indirect
 	github.com/golang/protobuf v1.5.2 // indirect
-	github.com/kr/text v0.2.0 // indirect
-	github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e // indirect
+	github.com/google/gofuzz v1.2.0 // indirect
+	github.com/inconshreveable/mousetrap v1.0.1 // indirect
+	github.com/json-iterator/go v1.1.12 // indirect
+	github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
+	github.com/modern-go/reflect2 v1.0.2 // indirect
+	github.com/pmezard/go-difflib v1.0.0 // indirect
+	github.com/spf13/cobra v1.6.1 // indirect
+	github.com/spf13/pflag v1.0.5 // indirect
+	github.com/stretchr/testify v1.8.1 // indirect
+	go.uber.org/atomic v1.10.0 // indirect
+	go.uber.org/multierr v1.8.0 // indirect
+	go.uber.org/zap v1.24.0 // indirect
 	golang.org/x/net v0.7.0 // indirect
 	golang.org/x/sys v0.5.0 // indirect
 	golang.org/x/text v0.7.0 // indirect
 	google.golang.org/genproto v0.0.0-20220502173005-c8bf987b8c21 // indirect
 	google.golang.org/protobuf v1.28.0 // indirect
-	gopkg.in/check.v1 v1.0.0-20200227125254-8fa46927fb4f // indirect
+	gopkg.in/inf.v0 v0.9.1 // indirect
 	gopkg.in/yaml.v2 v2.4.0 // indirect
+	gopkg.in/yaml.v3 v3.0.1 // indirect
+	k8s.io/apimachinery v0.25.0 // indirect
+	k8s.io/component-base v0.25.0 // indirect
+	k8s.io/utils v0.0.0-20221128185143-99ec85e7a448 // indirect
+	sigs.k8s.io/json v0.0.0-20220713155537-f223a00ba0e2 // indirect
+	sigs.k8s.io/structured-merge-diff/v4 v4.2.3 // indirect
 )