chore: update linters (#217)

* chore: update linters

Signed-off-by: Sertac Ozercan <sozercan@gmail.com>

* Apply suggestions from code review

Co-authored-by: Anish Ramasekar <anish.ramasekar@gmail.com>
Signed-off-by: Sertaç Özercan <852750+sozercan@users.noreply.github.com>

* Update .golangci.yml

Co-authored-by: Anish Ramasekar <anish.ramasekar@gmail.com>
Signed-off-by: Sertaç Özercan <852750+sozercan@users.noreply.github.com>

* fix formatting

Signed-off-by: Sertac Ozercan <sozercan@gmail.com>

---------

Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
Signed-off-by: Sertaç Özercan <852750+sozercan@users.noreply.github.com>
Co-authored-by: Anish Ramasekar <anish.ramasekar@gmail.com>

.golangci.yml

@@ -1,12 +1,29 @@
 run:
   timeout: 5m
 
+linters-settings:
+  gocritic:
+    enabled-tags:
+    - performance
+  lll:
+    line-length: 200
+  misspell:
+    locale: US
+  staticcheck:
+    go: "1.20"
+
 linters:
   disable-all: true
   enable:
+    - errcheck
+    - exportloopref
+    - forcetypeassert
     - goconst
+    - gocritic
     - gocyclo
+    - godot
     - gofmt
+    - gofumpt
     - goimports
     - gosec
     - gosimple
@@ -17,4 +34,6 @@ linters:
     - prealloc
     - revive
     - staticcheck
+    - typecheck
     - unused
+    - whitespace

cmd/server/main.go

@@ -62,7 +62,10 @@ func main() {
 	}
 
 	if *versionInfo {
-		version.PrintVersion()
+		if err := version.PrintVersion(); err != nil {
+			klog.ErrorS(err, "failed to print version")
+			os.Exit(1)
+		}
 		os.Exit(0)
 	}
 
@@ -117,7 +120,12 @@ func main() {
 	pb.RegisterKeyManagementServiceServer(s, kmsServer)
 
 	klog.InfoS("Listening for connections", "addr", listener.Addr().String())
-	go s.Serve(listener)
+	go func() {
+		if err := s.Serve(listener); err != nil {
+			klog.ErrorS(err, "failed to serve")
+			os.Exit(1)
+		}
+	}()
 
 	healthz := &plugin.HealthZ{
 		KMSServer: kmsServer,

pkg/auth/auth.go

@@ -23,7 +23,7 @@ import (
 	"k8s.io/klog/v2"
 )
 
-// GetKeyvaultToken() returns token for Keyvault endpoint
+// GetKeyvaultToken() returns token for Keyvault endpoint.
 func GetKeyvaultToken(config *config.AzureConfig, env *azure.Environment, resource string, proxyMode bool) (authorizer autorest.Authorizer, err error) {
 	servicePrincipalToken, err := GetServicePrincipalToken(config, env.ActiveDirectoryEndpoint, resource, proxyMode)
 	if err != nil {
@@ -33,7 +33,7 @@ func GetKeyvaultToken(config *config.AzureConfig, env *azure.Environment, resour
 	return authorizer, nil
 }
 
-// GetServicePrincipalToken creates a new service principal token based on the configuration
+// GetServicePrincipalToken creates a new service principal token based on the configuration.
 func GetServicePrincipalToken(config *config.AzureConfig, aadEndpoint, resource string, proxyMode bool) (adal.OAuthTokenProvider, error) {
 	oauthConfig, err := adal.NewOAuthConfig(aadEndpoint, config.TenantID)
 	if err != nil {
@@ -106,7 +106,7 @@ func GetServicePrincipalToken(config *config.AzureConfig, aadEndpoint, resource
 	return nil, fmt.Errorf("no credentials provided for accessing keyvault")
 }
 
-// ParseAzureEnvironment returns azure environment by name
+// ParseAzureEnvironment returns azure environment by name.
 func ParseAzureEnvironment(cloudName string) (*azure.Environment, error) {
 	var env azure.Environment
 	var err error
@@ -119,7 +119,7 @@ func ParseAzureEnvironment(cloudName string) (*azure.Environment, error) {
 }
 
 // decodePkcs12 decodes a PKCS#12 client certificate by extracting the public certificate and
-// the private RSA key
+// the private RSA key.
 func decodePkcs12(pkcs []byte, password string) (*x509.Certificate, *rsa.PrivateKey, error) {
 	privateKey, certificate, err := pkcs12.Decode(pkcs, password)
 	if err != nil {
@@ -133,13 +133,13 @@ func decodePkcs12(pkcs []byte, password string) (*x509.Certificate, *rsa.Private
 	return certificate, rsaPrivateKey, nil
 }
 
-// redactClientCredentials applies regex to a sensitive string and return the redacted value
+// redactClientCredentials applies regex to a sensitive string and return the redacted value.
 func redactClientCredentials(sensitiveString string) string {
-	r, _ := regexp.Compile(`^(\S{4})(\S|\s)*(\S{4})$`)
+	r := regexp.MustCompile(`^(\S{4})(\S|\s)*(\S{4})$`)
 	return r.ReplaceAllString(sensitiveString, "$1##### REDACTED #####$3")
 }
 
-// addTargetTypeHeader adds the target header if proxy mode is enabled
+// addTargetTypeHeader adds the target header if proxy mode is enabled.
 func addTargetTypeHeader(spt *adal.ServicePrincipalToken) *adal.ServicePrincipalToken {
 	spt.SetSender(autorest.CreateSender(
 		(func() autorest.SendDecorator {

pkg/config/azure_config.go

@@ -8,7 +8,7 @@ import (
 	"k8s.io/klog/v2"
 )
 
-// AzureConfig is representing /etc/kubernetes/azure.json
+// AzureConfig is representing /etc/kubernetes/azure.json.
 type AzureConfig struct {
 	Cloud                       string `json:"cloud" yaml:"cloud"`
 	TenantID                    string `json:"tenantId" yaml:"tenantId"`
@@ -20,7 +20,7 @@ type AzureConfig struct {
 	AADClientCertPassword       string `json:"aadClientCertPassword" yaml:"aadClientCertPassword"`
 }
 
-// GetAzureConfig returns configs in the azure.json cloud provider file
+// GetAzureConfig returns configs in the azure.json cloud provider file.
 func GetAzureConfig(configFile string) (config *AzureConfig, err error) {
 	cfg := AzureConfig{}
 

pkg/metrics/exporter.go

@@ -11,7 +11,7 @@ const (
 	prometheusExporter = "prometheus"
 )
 
-// InitMetricsExporter initializes new exporter
+// InitMetricsExporter initializes new exporter.
 func InitMetricsExporter(metricsBackend, metricsAddress string) error {
 	exporter := strings.ToLower(metricsBackend)
 	klog.InfoS("metrics backend", "exporter", exporter)

pkg/metrics/prometheus_exporter.go

@@ -18,7 +18,8 @@ func initPrometheusExporter(metricsAddress string) error {
 	exporter, err := prometheus.InstallNewPipeline(prometheus.Config{
 		DefaultHistogramBoundaries: []float64{
 			0.1, 0.2, 0.3, 0.4, 0.5, 1, 1.5, 2, 2.5, 3.0, 5.0, 10.0, 15.0, 30.0,
-		}},
+		},
+	},
 	)
 	if err != nil {
 		return fmt.Errorf("failed to register prometheus exporter: %v", err)

pkg/metrics/stats_reporter.go

@@ -14,32 +14,30 @@ const (
 	statusTypeKey        = "status"
 	operationTypeKey     = "operation"
 	kmsRequestMetricName = "kms_request"
-	// ErrorStatusTypeValue sets status tag to "error"
+	// ErrorStatusTypeValue sets status tag to "error".
 	ErrorStatusTypeValue = "error"
-	// SuccessStatusTypeValue sets status tag to "success"
+	// SuccessStatusTypeValue sets status tag to "success".
 	SuccessStatusTypeValue = "success"
-	// EncryptOperationTypeValue sets operation tag to "encrypt"
+	// EncryptOperationTypeValue sets operation tag to "encrypt".
 	EncryptOperationTypeValue = "encrypt"
-	// DecryptOperationTypeValue sets operation tag to "decrypt"
+	// DecryptOperationTypeValue sets operation tag to "decrypt".
 	DecryptOperationTypeValue = "decrypt"
-	// GrpcOperationTypeValue sets operation tag to "grpc"
+	// GrpcOperationTypeValue sets operation tag to "grpc".
 	GrpcOperationTypeValue = "grpc"
 )
 
-var (
-	kmsRequest metric.Float64ValueRecorder
-)
+var kmsRequest metric.Float64ValueRecorder
 
 type reporter struct {
 	meter metric.Meter
 }
 
-// StatsReporter reports metrics
+// StatsReporter reports metrics.
 type StatsReporter interface {
 	ReportRequest(ctx context.Context, operationType, status string, duration float64, errors ...string)
 }
 
-// NewStatsReporter instantiates otel reporter
+// NewStatsReporter instantiates otel reporter.
 func NewStatsReporter() StatsReporter {
 	meter := global.Meter(instrumentationName)
 

pkg/plugin/healthz.go

@@ -33,7 +33,7 @@ type HealthZ struct {
 	RPCTimeout     time.Duration
 }
 
-// Serve creates the http handler for serving health requests
+// Serve creates the http handler for serving health requests.
 func (h *HealthZ) Serve() {
 	serveMux := http.NewServeMux()
 	serveMux.HandleFunc(h.HealthCheckURL.EscapedPath(), h.ServeHTTP)
@@ -85,7 +85,10 @@ func (h *HealthZ) ServeHTTP(w http.ResponseWriter, _ *http.Request) {
 		return
 	}
 	w.WriteHeader(http.StatusOK)
-	w.Write([]byte("ok"))
+	if _, err = w.Write([]byte("ok")); err != nil {
+		http.Error(w, err.Error(), http.StatusInternalServerError)
+		return
+	}
 	klog.V(5).Info("Completed health check")
 }
 

pkg/plugin/healthz_test.go

@@ -144,7 +144,9 @@ func setupFakeKMSServer(socketPath string) (*KeyManagementServiceServer, *mockke
 	}
 	s := grpc.NewServer()
 	pb.RegisterKeyManagementServiceServer(s, fakeKMSServer)
-	go s.Serve(listener)
+	go func() {
+		_ = s.Serve(listener)
+	}()
 
 	return fakeKMSServer, kvClient, nil
 }

pkg/plugin/keyvault.go

@@ -24,7 +24,7 @@ import (
 	"k8s.io/klog/v2"
 )
 
-// Client interface for interacting with Keyvault
+// Client interface for interacting with Keyvault.
 type Client interface {
 	Encrypt(ctx context.Context, cipher []byte) ([]byte, error)
 	Decrypt(ctx context.Context, plain []byte) ([]byte, error)
@@ -40,14 +40,15 @@ type keyVaultClient struct {
 	azureEnvironment *azure.Environment
 }
 
-// NewKeyVaultClient returns a new key vault client to use for kms operations
+// NewKeyVaultClient returns a new key vault client to use for kms operations.
 func newKeyVaultClient(
 	config *config.AzureConfig,
 	vaultName, keyName, keyVersion string,
 	proxyMode bool,
 	proxyAddress string,
 	proxyPort int,
-	managedHSM bool) (*keyVaultClient, error) {
+	managedHSM bool,
+) (*keyVaultClient, error) {
 	// Sanitize vaultName, keyName, keyVersion. (https://github.com/Azure/kubernetes-kms/issues/85)
 	vaultName = utils.SanitizeString(vaultName)
 	keyName = utils.SanitizeString(keyName)

pkg/plugin/server.go

@@ -50,7 +50,7 @@ func New(pc *Config) (*KeyManagementServiceServer, error) {
 	}, nil
 }
 
-// Version of kms
+// Version of kms.
 func (s *KeyManagementServiceServer) Version(_ context.Context, _ *k8spb.VersionRequest) (*k8spb.VersionResponse, error) {
 	return &k8spb.VersionResponse{
 		Version:        version.APIVersion,
@@ -59,7 +59,7 @@ func (s *KeyManagementServiceServer) Version(_ context.Context, _ *k8spb.Version
 	}, nil
 }
 
-// Encrypt message
+// Encrypt message.
 func (s *KeyManagementServiceServer) Encrypt(ctx context.Context, request *k8spb.EncryptRequest) (*k8spb.EncryptResponse, error) {
 	start := time.Now()
 
@@ -84,7 +84,7 @@ func (s *KeyManagementServiceServer) Encrypt(ctx context.Context, request *k8spb
 	return &k8spb.EncryptResponse{Cipher: cipher}, nil
 }
 
-// Decrypt message
+// Decrypt message.
 func (s *KeyManagementServiceServer) Decrypt(ctx context.Context, request *k8spb.DecryptRequest) (*k8spb.DecryptResponse, error) {
 	start := time.Now()
 

pkg/plugin/server_test.go

@@ -6,6 +6,7 @@
 package plugin
 
 import (
+	"bytes"
 	"context"
 	"fmt"
 	"testing"
@@ -54,7 +55,7 @@ func TestEncrypt(t *testing.T) {
 			if err != test.err {
 				t.Fatalf("expected err: %v, got: %v", test.err, err)
 			}
-			if string(out.GetCipher()) != string(test.output) {
+			if !bytes.Equal(out.GetCipher(), test.output) {
 				t.Fatalf("expected out: %v, got: %v", test.output, out)
 			}
 		})
@@ -98,7 +99,7 @@ func TestDecrypt(t *testing.T) {
 			if err != test.err {
 				t.Fatalf("expected err: %v, got: %v", test.err, err)
 			}
-			if string(out.GetPlain()) != string(test.output) {
+			if !bytes.Equal(out.GetPlain(), test.output) {
 				t.Fatalf("expected out: %v, got: %v", test.output, out)
 			}
 		})

pkg/utils/grpc.go

@@ -12,7 +12,7 @@ import (
 	"k8s.io/klog/v2"
 )
 
-// ParseEndpoint returns unix socket's protocol and address
+// ParseEndpoint returns unix socket's protocol and address.
 func ParseEndpoint(ep string) (string, string, error) {
 	if strings.HasPrefix(strings.ToLower(ep), "unix://") {
 		s := strings.SplitN(ep, "://", 2)

pkg/utils/sanitize.go

@@ -2,7 +2,7 @@ package utils
 
 import "strings"
 
-// SanitizeString returns a string that does not have white spaces and double quotes
+// SanitizeString returns a string that does not have white spaces and double quotes.
 func SanitizeString(s string) string {
 	return strings.TrimSpace(strings.Trim(strings.TrimSpace(s), "\""))
 }

pkg/version/version.go

@@ -7,17 +7,17 @@ import (
 )
 
 var (
-	// BuildDate is the date when the binary was built
+	// BuildDate is the date when the binary was built.
 	BuildDate string
-	// GitCommit is the commit hash when the binary was built
+	// GitCommit is the commit hash when the binary was built.
 	GitCommit string
-	// BuildVersion is the version of the KMS binary
+	// BuildVersion is the version of the KMS binary.
 	BuildVersion string
 	APIVersion   = "v1beta1"
 	Runtime      = "Microsoft AzureKMS"
 )
 
-// PrintVersion prints the current KMS plugin version
+// PrintVersion prints the current KMS plugin version.
 func PrintVersion() (err error) {
 	pv := struct {
 		BuildVersion string

pkg/version/version_test.go

@@ -25,7 +25,7 @@ func TestPrintVersion(t *testing.T) {
 	// copy the output in a separate goroutine so printing can't block indefinitely
 	go func() {
 		var buf bytes.Buffer
-		io.Copy(&buf, r)
+		_, _ = io.Copy(&buf, r)
 		outC <- strings.TrimSpace(buf.String())
 	}()
 
@@ -52,6 +52,5 @@ func TestGetUserAgent(t *testing.T) {
 	expectedUserAgent := fmt.Sprintf("k8s-kms-keyvault/version (%s/%s) hash/Now", runtime.GOOS, runtime.GOARCH)
 	if !strings.EqualFold(userAgent, expectedUserAgent) {
 		t.Fatalf("string doesn't match, expected %s, got %s", expectedUserAgent, userAgent)
-
 	}
 }

tests/client/client_test.go

@@ -1,6 +1,7 @@
 package test
 
 import (
+	"bytes"
 	"fmt"
 	"net"
 	"testing"
@@ -53,7 +54,6 @@ func TestEncryptDecrypt(t *testing.T) {
 
 	for _, tc := range cases {
 		t.Run(tc.name, func(t *testing.T) {
-
 			encryptRequest := k8spb.EncryptRequest{Version: version, Plain: tc.want}
 			encryptResponse, err := client.Encrypt(context.Background(), &encryptRequest)
 			if err != nil {
@@ -62,7 +62,7 @@ func TestEncryptDecrypt(t *testing.T) {
 
 			decryptRequest := k8spb.DecryptRequest{Version: version, Cipher: encryptResponse.Cipher}
 			decryptResponse, err := client.Decrypt(context.Background(), &decryptRequest)
-			if string(decryptResponse.Plain) != string(tc.want) {
+			if !bytes.Equal(decryptResponse.Plain, tc.want) {
 				t.Fatalf("Expected secret, but got %s - %v", string(decryptResponse.Plain), err)
 			}
 		})
@@ -85,7 +85,6 @@ func TestVersion(t *testing.T) {
 
 	for _, tc := range cases {
 		t.Run(tc.name, func(t *testing.T) {
-
 			request := &k8spb.VersionRequest{Version: tc.want}
 			response, err := client.Version(context.Background(), request)
 			if err != nil {