Added further lint work (#1303)

* Added furhter lint work Signed-off-by: Yuri Sa <[email protected]> * Fixed param ghosted vars Signed-off-by: Yuri Sa <[email protected]> * add missing line Signed-off-by: Yuri Sa <[email protected]> Signed-off-by: Yuri Sa <[email protected]>
open-telemetry · Dec 7, 2022 · c7c4b5f · c7c4b5f
1 parent 3a9621a
commit c7c4b5f
Show file tree

Hide file tree

Showing 27 changed files with 207 additions and 147 deletions.
diff --git a/.golangci.yaml b/.golangci.yaml
@@ -1,5 +1,8 @@
 run:
+  concurrency: 4
   timeout: 5m
+  issues-exit-code: 1
+  tests: true
 
 # all available settings of specific linters
 linters-settings:
@@ -11,8 +14,42 @@ linters-settings:
     suggest-new: true
   misspell:
     locale: US
+    ignore-words:
+      - cancelled
+      - metre
+      - meter
+      - metres
+      - kilometre
+      - kilometres
   govet:
-    disable-all: true
+    # report about shadowed variables
+    check-shadowing: true
+
+    # settings per analyzer
+    settings:
+      printf: # analyzer name, run `go tool vet help` to see all analyzers
+        funcs: # run `go tool vet help printf` to see available settings for `printf` analyzer
+          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Infof
+          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Warnf
+          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Errorf
+          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Fatalf
+
+    enable-all: true
+    # TODO: Enable this and fix the alignment issues.
+    disable:
+      - fieldalignment
+  gofmt:
+    simplify: true
+  revive:
+    min-confidence: 0.8
+
+  depguard:
+    list-type: denylist
+    include-go-root: true
+    packages-with-error-message:
+      # See https://github.com/open-telemetry/opentelemetry-collector/issues/5200 for rationale
+      - sync/atomic: "Use go.uber.org/atomic instead of sync/atomic"
+      - github.com/pkg/errors: "Use 'errors' or 'fmt' instead of github.com/pkg/errors"
 
 linters:
   enable:
@@ -29,4 +66,7 @@ linters:
     - staticcheck
     - ineffassign
     - typecheck
-    - unparam
+    - unparam
+    - depguard
+    - errcheck
+    - errorlint
diff --git a/apis/v1alpha1/opentelemetrycollector_webhook.go b/apis/v1alpha1/opentelemetrycollector_webhook.go
@@ -132,7 +132,7 @@ func (r *OpenTelemetryCollector) validateCRDSpec() error {
 	if r.Spec.TargetAllocator.Enabled {
 		_, err := ta.ConfigToPromConfig(r.Spec.Config)
 		if err != nil {
-			return fmt.Errorf("the OpenTelemetry Spec Prometheus configuration is incorrect, %s", err)
+			return fmt.Errorf("the OpenTelemetry Spec Prometheus configuration is incorrect, %w", err)
 		}
 	}
 

diff --git a/cmd/otel-allocator/config/config.go b/cmd/otel-allocator/config/config.go
@@ -15,6 +15,7 @@
 package config
 
 import (
+	"errors"
 	"flag"
 	"fmt"
 	"io/fs"
@@ -115,7 +116,8 @@ func ParseCLI() (CLIConfig, error) {
 	clusterConfig, err := clientcmd.BuildConfigFromFlags("", *kubeconfigPath)
 	cLIConf.KubeConfigFilePath = *kubeconfigPath
 	if err != nil {
-		if _, ok := err.(*fs.PathError); !ok {
+		pathError := &fs.PathError{}
+		if ok := errors.As(err, &pathError); !ok {
 			return CLIConfig{}, err
 		}
 		clusterConfig, err = rest.InClusterConfig()

diff --git a/cmd/otel-allocator/main.go b/cmd/otel-allocator/main.go
@@ -17,6 +17,7 @@ package main
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"net/http"
 	"net/url"
 	"os"
@@ -91,9 +92,9 @@ func main() {
 		os.Exit(1)
 	}
 	defer func() {
-		err := watcher.Close()
-		if err != nil {
-			log.Error(err, "failed to close watcher")
+		watcherErr := watcher.Close()
+		if watcherErr != nil {
+			log.Error(watcherErr, "failed to close watcher")
 		}
 	}()
 
@@ -115,7 +116,7 @@ func main() {
 	signal.Notify(interrupts, os.Interrupt, syscall.SIGTERM)
 
 	go func() {
-		if err := srv.Start(); err != http.ErrServerClosed {
+		if err := srv.Start(); !errors.Is(err, http.ErrServerClosed) {
 			setupLog.Error(err, "Can't start the server")
 		}
 	}()
@@ -139,7 +140,7 @@ func main() {
 				}
 				srv = newServer(log, allocator, discoveryManager, k8sclient, cliConf.ListenAddr)
 				go func() {
-					if err := srv.Start(); err != http.ErrServerClosed {
+					if err := srv.Start(); !errors.Is(err, http.ErrServerClosed) {
 						setupLog.Error(err, "Can't restart the server")
 					}
 				}()

diff --git a/controllers/suite_test.go b/controllers/suite_test.go
@@ -28,6 +28,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/client-go/kubernetes/scheme"
+	"k8s.io/client-go/rest"
 	"k8s.io/client-go/util/retry"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -43,6 +44,8 @@ var (
 	testScheme *runtime.Scheme = scheme.Scheme
 	ctx        context.Context
 	cancel     context.CancelFunc
+	err        error
+	cfg        *rest.Config
 )
 
 func TestMain(m *testing.M) {
@@ -55,13 +58,13 @@ func TestMain(m *testing.M) {
 			Paths: []string{filepath.Join("..", "config", "webhook")},
 		},
 	}
-	cfg, err := testEnv.Start()
+	cfg, err = testEnv.Start()
 	if err != nil {
 		fmt.Printf("failed to start testEnv: %v", err)
 		os.Exit(1)
 	}
 
-	if err := v1alpha1.AddToScheme(testScheme); err != nil {
+	if err = v1alpha1.AddToScheme(testScheme); err != nil {
 		fmt.Printf("failed to register scheme: %v", err)
 		os.Exit(1)
 	}
@@ -75,20 +78,20 @@ func TestMain(m *testing.M) {
 
 	// start webhook server using Manager
 	webhookInstallOptions := &testEnv.WebhookInstallOptions
-	mgr, err := ctrl.NewManager(cfg, ctrl.Options{
+	mgr, mgrErr := ctrl.NewManager(cfg, ctrl.Options{
 		Scheme:             testScheme,
 		Host:               webhookInstallOptions.LocalServingHost,
 		Port:               webhookInstallOptions.LocalServingPort,
 		CertDir:            webhookInstallOptions.LocalServingCertDir,
 		LeaderElection:     false,
 		MetricsBindAddress: "0",
 	})
-	if err != nil {
-		fmt.Printf("failed to start webhook server: %v", err)
+	if mgrErr != nil {
+		fmt.Printf("failed to start webhook server: %v", mgrErr)
 		os.Exit(1)
 	}
 
-	if err := (&v1alpha1.OpenTelemetryCollector{}).SetupWebhookWithManager(mgr); err != nil {
+	if err = (&v1alpha1.OpenTelemetryCollector{}).SetupWebhookWithManager(mgr); err != nil {
 		fmt.Printf("failed to SetupWebhookWithManager: %v", err)
 		os.Exit(1)
 	}
@@ -119,9 +122,9 @@ func TestMain(m *testing.M) {
 			return true
 		}, func() error {
 			// #nosec G402
-			conn, err := tls.DialWithDialer(dialer, "tcp", addrPort, &tls.Config{InsecureSkipVerify: true})
-			if err != nil {
-				return err
+			conn, tlsErr := tls.DialWithDialer(dialer, "tcp", addrPort, &tls.Config{InsecureSkipVerify: true})
+			if tlsErr != nil {
+				return tlsErr
 			}
 			_ = conn.Close()
 			return nil

diff --git a/internal/webhookhandler/webhookhandler_suite_test.go b/internal/webhookhandler/webhookhandler_suite_test.go
@@ -28,6 +28,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/client-go/kubernetes/scheme"
+	"k8s.io/client-go/rest"
 	"k8s.io/client-go/util/retry"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -43,6 +44,8 @@ var (
 	testScheme *runtime.Scheme = scheme.Scheme
 	ctx        context.Context
 	cancel     context.CancelFunc
+	err        error
+	cfg        *rest.Config
 )
 
 func TestMain(m *testing.M) {
@@ -55,13 +58,13 @@ func TestMain(m *testing.M) {
 			Paths: []string{filepath.Join("..", "..", "config", "webhook")},
 		},
 	}
-	cfg, err := testEnv.Start()
+	cfg, err = testEnv.Start()
 	if err != nil {
 		fmt.Printf("failed to start testEnv: %v", err)
 		os.Exit(1)
 	}
 
-	if err := v1alpha1.AddToScheme(testScheme); err != nil {
+	if err = v1alpha1.AddToScheme(testScheme); err != nil {
 		fmt.Printf("failed to register scheme: %v", err)
 		os.Exit(1)
 	}
@@ -75,20 +78,20 @@ func TestMain(m *testing.M) {
 
 	// start webhook server using Manager
 	webhookInstallOptions := &testEnv.WebhookInstallOptions
-	mgr, err := ctrl.NewManager(cfg, ctrl.Options{
+	mgr, mgrErr := ctrl.NewManager(cfg, ctrl.Options{
 		Scheme:             testScheme,
 		Host:               webhookInstallOptions.LocalServingHost,
 		Port:               webhookInstallOptions.LocalServingPort,
 		CertDir:            webhookInstallOptions.LocalServingCertDir,
 		LeaderElection:     false,
 		MetricsBindAddress: "0",
 	})
-	if err != nil {
-		fmt.Printf("failed to start webhook server: %v", err)
+	if mgrErr != nil {
+		fmt.Printf("failed to start webhook server: %v", mgrErr)
 		os.Exit(1)
 	}
 
-	if err := (&v1alpha1.OpenTelemetryCollector{}).SetupWebhookWithManager(mgr); err != nil {
+	if err = (&v1alpha1.OpenTelemetryCollector{}).SetupWebhookWithManager(mgr); err != nil {
 		fmt.Printf("failed to SetupWebhookWithManager: %v", err)
 		os.Exit(1)
 	}
@@ -119,9 +122,9 @@ func TestMain(m *testing.M) {
 			return true
 		}, func() error {
 			// #nosec G402
-			conn, err := tls.DialWithDialer(dialer, "tcp", addrPort, &tls.Config{InsecureSkipVerify: true})
-			if err != nil {
-				return err
+			conn, tlsErr := tls.DialWithDialer(dialer, "tcp", addrPort, &tls.Config{InsecureSkipVerify: true})
+			if tlsErr != nil {
+				return tlsErr
 			}
 			_ = conn.Close()
 			return nil

diff --git a/internal/webhookhandler/webhookhandler_test.go b/internal/webhookhandler/webhookhandler_test.go
@@ -131,8 +131,8 @@ func TestShouldInjectSidecar(t *testing.T) {
 			}()
 
 			for i := range tt.otelcols {
-				err := k8sClient.Create(context.Background(), &tt.otelcols[i])
-				require.NoError(t, err)
+				clientErr := k8sClient.Create(context.Background(), &tt.otelcols[i])
+				require.NoError(t, clientErr)
 			}
 
 			encoded, err := json.Marshal(tt.pod)
@@ -154,8 +154,8 @@ func TestShouldInjectSidecar(t *testing.T) {
 			require.NoError(t, err)
 
 			injector := NewWebhookHandler(cfg, logger, k8sClient, []PodMutator{sidecar.NewMutator(logger, cfg, k8sClient)})
-			err = injector.InjectDecoder(decoder)
-			require.NoError(t, err)
+			injectErr := injector.InjectDecoder(decoder)
+			require.NoError(t, injectErr)
 
 			// test
 			res := injector.Handle(context.Background(), req)
@@ -354,8 +354,8 @@ func TestPodShouldNotBeChanged(t *testing.T) {
 			}()
 
 			for i := range tt.otelcols {
-				err := k8sClient.Create(context.Background(), &tt.otelcols[i])
-				require.NoError(t, err)
+				clientErr := k8sClient.Create(context.Background(), &tt.otelcols[i])
+				require.NoError(t, clientErr)
 			}
 
 			encoded, err := json.Marshal(tt.pod)

diff --git a/pkg/collector/adapters/config_to_probe.go b/pkg/collector/adapters/config_to_probe.go
@@ -49,22 +49,22 @@ const (
 
 // ConfigToContainerProbe converts the incoming configuration object into a container probe or returns an error.
 func ConfigToContainerProbe(config map[interface{}]interface{}) (*corev1.Probe, error) {
-	serviceProperty, ok := config["service"]
-	if !ok {
+	serviceProperty, withService := config["service"]
+	if !withService {
 		return nil, errNoService
 	}
-	service, ok := serviceProperty.(map[interface{}]interface{})
-	if !ok {
+	service, withSvcProperty := serviceProperty.(map[interface{}]interface{})
+	if !withSvcProperty {
 		return nil, errServiceNotAMap
 	}
 
-	serviceExtensionsProperty, ok := service["extensions"]
-	if !ok {
+	serviceExtensionsProperty, withExtension := service["extensions"]
+	if !withExtension {
 		return nil, errNoServiceExtensions
 	}
 
-	serviceExtensions, ok := serviceExtensionsProperty.([]interface{})
-	if !ok {
+	serviceExtensions, withExtProperty := serviceExtensionsProperty.([]interface{})
+	if !withExtProperty {
 		return nil, errServiceExtensionsNotSlice
 	}
 	healthCheckServiceExtensions := make([]string, 0)

diff --git a/pkg/collector/adapters/config_validate.go b/pkg/collector/adapters/config_validate.go
@@ -34,29 +34,29 @@ func GetEnabledReceivers(_ logr.Logger, config map[interface{}]interface{}) map[
 	for recvID := range receivers {
 
 		//Safe Cast
-		receiverID, ok := recvID.(string)
-		if !ok {
+		receiverID, withReceiver := recvID.(string)
+		if !withReceiver {
 			return nil
 		}
 		//Getting all receivers present in the receivers section and setting them to false.
 		availableReceivers[receiverID] = false
 	}
 
-	cfgService, ok := config["service"].(map[interface{}]interface{})
-	if !ok {
+	cfgService, withService := config["service"].(map[interface{}]interface{})
+	if !withService {
 		return nil
 	}
 
-	pipeline, ok := cfgService["pipelines"].(map[interface{}]interface{})
-	if !ok {
+	pipeline, withPipeline := cfgService["pipelines"].(map[interface{}]interface{})
+	if !withPipeline {
 		return nil
 	}
 	availablePipelines := map[string]bool{}
 
 	for pipID := range pipeline {
 		//Safe Cast
-		pipelineID, ok := pipID.(string)
-		if !ok {
+		pipelineID, existsPipeline := pipID.(string)
+		if !existsPipeline {
 			return nil
 		}
 		//Getting all the available pipelines.
@@ -66,8 +66,8 @@ func GetEnabledReceivers(_ logr.Logger, config map[interface{}]interface{}) map[
 	if len(pipeline) > 0 {
 		for pipelineID, pipelineCfg := range pipeline {
 			//Safe Cast
-			pipelineV, ok := pipelineID.(string)
-			if !ok {
+			pipelineV, withPipelineCfg := pipelineID.(string)
+			if !withPipelineCfg {
 				continue
 			}
 			//Condition will get information if there are multiple configured pipelines.