Merge pull request #258 from grafana/golangci-1-53-3

Upgrade to golangci-lint v1.53.3

.golangci.yml

@@ -1,4 +1,4 @@
-# v1.49.0
+# v1.53.3
 # Please don't remove the first line. It is used in CI to determine the golangci version
 run:
   deadline: 5m
@@ -57,8 +57,6 @@ linters-settings:
     min-occurrences: 4
   govet:
     check-shadowing: true
-  maligned:
-    suggest-new: true
   gosec:
     excludes:
       - G107 # Http request made with variable url
@@ -100,4 +98,11 @@ linters:
     - usestdlibvars
     - nosprintfhostport
     - nonamedreturns
+    # Deprecated linters as of 1.53.3
+    - structcheck
+    - varcheck
+    - deadcode
+    - ifshort
+    - nosnakecase
+    - depguard # Dependency whitelist, needs to be configured
   fast: false

Makefile

@@ -41,8 +41,10 @@ e2e:
 format:
 	go fmt ./...
 
+# Running with -buildvcs=false works around the issue of `go list all` failing when git, which runs as root inside
+# the container, refuses to operate on the disruptor source tree as it is not owned by the same user (root).
 lint:
-	docker run --rm -v $(work_dir):/disruptor -w /disruptor golangci/golangci-lint:$(golangci_version) golangci-lint run
+	docker run --rm -v $(work_dir):/disruptor -w /disruptor -e GOFLAGS=-buildvcs=false golangci/golangci-lint:$(golangci_version) golangci-lint run
 
 test:
 	go test -race  ./...

pkg/agent/agent_test.go

@@ -15,7 +15,7 @@ import (
 type FakeProtocolDisruptor struct{}
 
 // Apply implements the Apply method from the protocol Disruptor interface
-func (d *FakeProtocolDisruptor) Apply(ctx context.Context, duration time.Duration) error {
+func (d *FakeProtocolDisruptor) Apply(_ context.Context, duration time.Duration) error {
 	time.Sleep(duration)
 	return nil
 }

pkg/agent/protocol/grpc/handler.go

@@ -51,7 +51,7 @@ func contains(list []string, target string) bool {
 
 // handles requests from the client. If selected for error injection, returns an error,
 // otherwise, forwards to the server transparently
-func (h *handler) streamHandler(srv interface{}, serverStream grpc.ServerStream) error {
+func (h *handler) streamHandler(_ interface{}, serverStream grpc.ServerStream) error {
 	fullMethodName, ok := grpc.MethodFromServerStream(serverStream)
 	if !ok {
 		return status.Errorf(codes.Internal, "ServerTransportStream not exists in context")

pkg/agent/protocol/http/proxy_test.go

@@ -21,7 +21,7 @@ type fakeHTTPClient struct {
 	body    []byte
 }
 
-func (f *fakeHTTPClient) Do(req *http.Request) (*http.Response, error) {
+func (f *fakeHTTPClient) Do(_ *http.Request) (*http.Response, error) {
 	resp := &http.Response{
 		Proto:         "HTTP/1.1",
 		ProtoMajor:    1,

pkg/api/api.go

@@ -17,8 +17,8 @@ import (
 	"go.k6.io/k6/js/common"
 )
 
-//nolint:unparam   // TODO: call directly Convert from API methods
-func convertValue(rt *goja.Runtime, value goja.Value, target interface{}) error {
+// TODO: call directly Convert from API methods
+func convertValue(_ *goja.Runtime, value goja.Value, target interface{}) error {
 	return Convert(value.Export(), target)
 }
 

pkg/api/validation.go

@@ -11,7 +11,8 @@ import (
 func IsCompatible(actual interface{}, expected interface{}) error {
 	actualType := reflect.TypeOf(actual)
 	expectedType := reflect.TypeOf(expected)
-	compatible := false
+
+	var compatible bool
 	switch expectedType.Kind() {
 	case reflect.Map:
 		compatible = actualType.Kind() == reflect.Map

pkg/disruptors/controller.go

@@ -102,7 +102,7 @@ func (c *agentController) ExecCommand(ctx context.Context, cmd []string) error {
 }
 
 // Visit allows executing a different command on each target returned by a visiting function
-func (c *agentController) Visit(ctx context.Context, visitor func(corev1.Pod) ([]string, error)) error {
+func (c *agentController) Visit(_ context.Context, visitor func(corev1.Pod) ([]string, error)) error {
 	var wg sync.WaitGroup
 	// ensure errors channel has enough space to avoid blocking gorutines
 	errors := make(chan error, len(c.targets))
@@ -136,7 +136,7 @@ func (c *agentController) Visit(ctx context.Context, visitor func(corev1.Pod) ([
 }
 
 // Targets retrieves the list of names of the target pods
-func (c *agentController) Targets(ctx context.Context) ([]string, error) {
+func (c *agentController) Targets(_ context.Context) ([]string, error) {
 	names := []string{}
 	for _, p := range c.targets {
 		names = append(names, p.Name)
@@ -146,7 +146,7 @@ func (c *agentController) Targets(ctx context.Context) ([]string, error) {
 
 // NewAgentController creates a new controller for a list of target pods
 func NewAgentController(
-	ctx context.Context,
+	_ context.Context,
 	helper helpers.PodHelper,
 	namespace string,
 	targets []corev1.Pod,

pkg/disruptors/pod_test.go

@@ -22,24 +22,24 @@ type fakeAgentController struct {
 	executor  *runtime.FakeExecutor
 }
 
-func (f *fakeAgentController) Targets(ctx context.Context) ([]string, error) {
+func (f *fakeAgentController) Targets(_ context.Context) ([]string, error) {
 	names := []string{}
 	for _, p := range f.targets {
 		names = append(names, p.Name)
 	}
 	return names, nil
 }
 
-func (f *fakeAgentController) InjectDisruptorAgent(ctx context.Context) error {
+func (f *fakeAgentController) InjectDisruptorAgent(_ context.Context) error {
 	return nil
 }
 
-func (f *fakeAgentController) ExecCommand(ctx context.Context, cmd []string) error {
+func (f *fakeAgentController) ExecCommand(_ context.Context, cmd []string) error {
 	_, err := f.executor.Exec(cmd[0], cmd[1:]...)
 	return err
 }
 
-func (f *fakeAgentController) Visit(ctx context.Context, visitor func(corev1.Pod) ([]string, error)) error {
+func (f *fakeAgentController) Visit(_ context.Context, visitor func(corev1.Pod) ([]string, error)) error {
 	for _, t := range f.targets {
 		cmd, err := visitor(t)
 		if err != nil {

pkg/runtime/fake.go

@@ -167,12 +167,12 @@ func NewFakeSignal() *FakeSignal {
 }
 
 // Notify implements Signal's interface Notify method
-func (f *FakeSignal) Notify(signals ...os.Signal) <-chan os.Signal {
+func (f *FakeSignal) Notify(_ ...os.Signal) <-chan os.Signal {
 	return f.channel
 }
 
 // Reset implements Signal's interface Reset method. It is noop.
-func (f *FakeSignal) Reset(signals ...os.Signal) {
+func (f *FakeSignal) Reset(_ ...os.Signal) {
 	// noop
 }
 

pkg/runtime/lock_test.go

@@ -87,7 +87,7 @@ func Test_Acquire(t *testing.T) {
 					return
 				}
 
-				_, err = lockFile.Write([]byte(tc.ownerPid))
+				_, err = lockFile.WriteString(tc.ownerPid)
 				if err != nil {
 					t.Errorf("error in test setup: %v", err)
 					return
@@ -170,7 +170,7 @@ func Test_Release(t *testing.T) {
 					return
 				}
 
-				_, err = lockFile.Write([]byte(tc.ownerPid))
+				_, err = lockFile.WriteString(tc.ownerPid)
 				if err != nil {
 					t.Errorf("error in test setup: %v", err)
 					return

pkg/testutils/e2e/checks/checks.go

@@ -59,7 +59,7 @@ type GrpcCheck struct {
 }
 
 // Verify verifies a HTTPCheck
-func (c HTTPCheck) Verify(k kubernetes.Kubernetes, ingress string, namespace string) error {
+func (c HTTPCheck) Verify(_ kubernetes.Kubernetes, ingress string, _ string) error {
 	time.Sleep(c.Delay)
 
 	url := fmt.Sprintf("http://%s", ingress)
@@ -85,7 +85,7 @@ func (c HTTPCheck) Verify(k kubernetes.Kubernetes, ingress string, namespace str
 }
 
 // Verify verifies a GrpcServiceCheck
-func (c GrpcCheck) Verify(k kubernetes.Kubernetes, ingress string, namespace string) error {
+func (c GrpcCheck) Verify(_ kubernetes.Kubernetes, ingress string, _ string) error {
 	time.Sleep(c.Delay)
 
 	client, err := dynamic.NewClientWithDialOptions(

pkg/testutils/e2e/kubectl/kubectl.go

@@ -62,7 +62,7 @@ func NewFromKubeconfig(ctx context.Context, kubeconfig string) (*Client, error)
 }
 
 // NewForConfig returns a new Client using a rest.Config
-func NewForConfig(ctx context.Context, config *rest.Config) (*Client, error) {
+func NewForConfig(_ context.Context, config *rest.Config) (*Client, error) {
 	dynamic, err := dynamic.NewForConfig(config)
 	if err != nil {
 		return nil, err

pkg/testutils/grpc/dynamic/dynamic.go

@@ -63,7 +63,6 @@ func (c *Client) Connect(ctx context.Context) error {
 	rc := grpcreflect.NewClientAuto(ctx, conn)
 	defer rc.Reset()
 
-	//nolint:contextcheck   // linter expects this function to be passed a context!?
 	desc, err := rc.ResolveService(c.service)
 	if err != nil {
 		return err