chore: extend linters to tests and fix some of them

.golangci.yml

@@ -1,7 +1,7 @@
 version: "2"
 
 run:
-  tests: false
+  tests: true
   timeout: 10m
 
 linters: 
@@ -19,6 +19,9 @@ linters:
     govet:
       enable:
         - nilness
+    revive:
+      enable-default-rules: true
+      max-open-files: 2048
   exclusions:
     generated: lax
     presets:
@@ -28,34 +31,22 @@ linters:
       - std-error-handling
     rules:
       - linters:
+          - ineffassign
+          - misspell
           - revive
-        text: stutters
-      - linters:
-          - revive
-        text: empty-block
-      - linters:
-          - revive
-        text: superfluous-else
-      - linters:
-          - revive
-        text: unused-parameter
-      - linters:
-          - revive
-        text: redefines-builtin-id
-      - linters:
-          - revive
-        text: if-return
+          - unused
+        path: .*_test.go
     paths:
       - .*\.pb\.go$
+    warn-unused: true
 
 formatters:
   enable:
     - gofmt
     - goimports
   exclusions:
     generated: lax
-    paths:
-      - .*\.pb\.go$
+    warn-unused: true
 
 issues:
   max-issues-per-linter: 0

agent/agent_test.go

@@ -7,6 +7,7 @@ import (
 	"fmt"
 	"net"
 	"os"
+	"slices"
 	"sync"
 	"testing"
 	"time"
@@ -587,8 +588,8 @@ func agentTestEnv(t *testing.T, nodeChangeCh chan *NodeChanges, tlsChangeCh chan
 		testCA:         tc,
 		cleanup: func() {
 			// go in reverse order
-			for i := len(cleanup) - 1; i >= 0; i-- {
-				cleanup[i]()
+			for _, v := range slices.Backward(cleanup) {
+				v()
 			}
 		},
 		remotes: fr,

agent/storage_test.go

@@ -4,6 +4,7 @@ import (
 	"math/rand"
 	"os"
 	"path/filepath"
+	"slices"
 	"sort"
 	"testing"
 
@@ -187,8 +188,8 @@ func storageTestEnv(t *testing.T) (*bolt.DB, func()) {
 	assert.NoError(t, InitDB(db))
 	return db, func() {
 		// iterate in reverse so it works like defer
-		for i := len(cleanup) - 1; i >= 0; i-- {
-			cleanup[i]()
+		for _, v := range slices.Backward(cleanup) {
+			v()
 		}
 	}
 }

ca/certificates_test.go

@@ -560,7 +560,7 @@ type nonSigningCAServer struct {
 	tc               *cautils.TestCA
 	server           *grpc.Server
 	addr             string
-	nodeStatusCalled int64
+	nodeStatusCalled atomic.Int64
 }
 
 func newNonSigningCAServer(t *testing.T, tc *cautils.TestCA) *nonSigningCAServer {
@@ -593,7 +593,7 @@ func (n *nonSigningCAServer) getConnBroker() *connectionbroker.Broker {
 
 // only returns the status in the store
 func (n *nonSigningCAServer) NodeCertificateStatus(ctx context.Context, request *api.NodeCertificateStatusRequest) (*api.NodeCertificateStatusResponse, error) {
-	atomic.AddInt64(&n.nodeStatusCalled, 1)
+	n.nodeStatusCalled.Add(1)
 	for {
 		var node *api.Node
 		n.tc.MemoryStore.View(func(tx store.ReadTx) {
@@ -696,7 +696,7 @@ func TestGetRemoteSignedCertificateWithPending(t *testing.T) {
 
 	// wait for the calls to NodeCertificateStatus to begin on the first signing server before we start timing
 	require.NoError(t, testutils.PollFuncWithTimeout(nil, func() error {
-		if atomic.LoadInt64(&fakeCAServer.nodeStatusCalled) == 0 {
+		if fakeCAServer.nodeStatusCalled.Load() == 0 {
 			return fmt.Errorf("waiting for NodeCertificateStatus to be called")
 		}
 		return nil
@@ -712,7 +712,7 @@ func TestGetRemoteSignedCertificateWithPending(t *testing.T) {
 	case <-time.After(2500 * time.Millisecond):
 		// good, it's still polling so we can proceed with the test
 	}
-	require.True(t, atomic.LoadInt64(&fakeCAServer.nodeStatusCalled) > 1, "expected NodeCertificateStatus to have been polled more than once")
+	require.True(t, fakeCAServer.nodeStatusCalled.Load() > 1, "expected NodeCertificateStatus to have been polled more than once")
 
 	// Directly update the status of the store
 	err = tc.MemoryStore.Update(func(tx store.Tx) error {
@@ -836,7 +836,7 @@ func TestGetRemoteSignedCertificateConnectionErrors(t *testing.T) {
 
 	// wait for the calls to NodeCertificateStatus to begin on the first signing server
 	require.NoError(t, testutils.PollFuncWithTimeout(nil, func() error {
-		if atomic.LoadInt64(&fakeSigningServers[0].nodeStatusCalled) == 0 {
+		if fakeSigningServers[0].nodeStatusCalled.Load() == 0 {
 			return fmt.Errorf("waiting for NodeCertificateStatus to be called")
 		}
 		return nil
@@ -854,7 +854,7 @@ func TestGetRemoteSignedCertificateConnectionErrors(t *testing.T) {
 
 	// wait for the calls to NodeCertificateStatus to begin on the second signing server
 	require.NoError(t, testutils.PollFuncWithTimeout(nil, func() error {
-		if atomic.LoadInt64(&fakeSigningServers[1].nodeStatusCalled) == 0 {
+		if fakeSigningServers[1].nodeStatusCalled.Load() == 0 {
 			return fmt.Errorf("waiting for NodeCertificateStatus to be called")
 		}
 		return nil

ca/keyutils/keyutils_test.go

@@ -50,12 +50,12 @@ aMbljbOLAjpZS3/VnQteab4=
 
 func TestIsPKCS8(t *testing.T) {
 	// Check PKCS8 keys
-	assert.True(t, IsPKCS8([]byte(decryptedPKCS8Block.Bytes)))
-	assert.True(t, IsPKCS8([]byte(encryptedPKCS8Block.Bytes)))
+	assert.True(t, IsPKCS8(decryptedPKCS8Block.Bytes))
+	assert.True(t, IsPKCS8(encryptedPKCS8Block.Bytes))
 
 	// Check PKCS1 keys
-	assert.False(t, IsPKCS8([]byte(decryptedPKCS1Block.Bytes)))
-	assert.False(t, IsPKCS8([]byte(encryptedPKCS1Block.Bytes)))
+	assert.False(t, IsPKCS8(decryptedPKCS1Block.Bytes))
+	assert.False(t, IsPKCS8(encryptedPKCS1Block.Bytes))
 }
 
 func TestIsEncryptedPEMBlock(t *testing.T) {

manager/controlapi/service_test.go

@@ -350,11 +350,11 @@ func TestValidateContainerSpec(t *testing.T) {
 				Image: "image",
 				Healthcheck: &api.HealthConfig{
 					Test:          []string{"curl 127.0.0.1:3000"},
-					Interval:      gogotypes.DurationProto(time.Duration(-1 * time.Second)), // invalid negative duration
-					Timeout:       gogotypes.DurationProto(time.Duration(-1 * time.Second)), // invalid negative duration
-					Retries:       -1,                                                       // invalid negative integer
-					StartPeriod:   gogotypes.DurationProto(time.Duration(-1 * time.Second)), // invalid negative duration
-					StartInterval: gogotypes.DurationProto(time.Duration(-1 * time.Second)), // invalid negative duration
+					Interval:      gogotypes.DurationProto(-1 * time.Second), // invalid negative duration
+					Timeout:       gogotypes.DurationProto(-1 * time.Second), // invalid negative duration
+					Retries:       -1,                                        // invalid negative integer
+					StartPeriod:   gogotypes.DurationProto(-1 * time.Second), // invalid negative duration
+					StartInterval: gogotypes.DurationProto(-1 * time.Second), // invalid negative duration
 				},
 			},
 		},
@@ -397,11 +397,11 @@ func TestValidateContainerSpec(t *testing.T) {
 				},
 				Healthcheck: &api.HealthConfig{
 					Test:          []string{"curl 127.0.0.1:3000"},
-					Interval:      gogotypes.DurationProto(time.Duration(1 * time.Second)),
-					Timeout:       gogotypes.DurationProto(time.Duration(3 * time.Second)),
+					Interval:      gogotypes.DurationProto(1 * time.Second),
+					Timeout:       gogotypes.DurationProto(3 * time.Second),
 					Retries:       5,
-					StartPeriod:   gogotypes.DurationProto(time.Duration(1 * time.Second)),
-					StartInterval: gogotypes.DurationProto(time.Duration(1 * time.Second)),
+					StartPeriod:   gogotypes.DurationProto(1 * time.Second),
+					StartInterval: gogotypes.DurationProto(1 * time.Second),
 				},
 			},
 		},
@@ -526,19 +526,19 @@ func TestValidateServiceSpecJobsDifference(t *testing.T) {
 func TestValidateRestartPolicy(t *testing.T) {
 	bad := []*api.RestartPolicy{
 		{
-			Delay:  gogotypes.DurationProto(time.Duration(-1 * time.Second)),
-			Window: gogotypes.DurationProto(time.Duration(-1 * time.Second)),
+			Delay:  gogotypes.DurationProto(-1 * time.Second),
+			Window: gogotypes.DurationProto(-1 * time.Second),
 		},
 		{
-			Delay:  gogotypes.DurationProto(time.Duration(20 * time.Second)),
-			Window: gogotypes.DurationProto(time.Duration(-4 * time.Second)),
+			Delay:  gogotypes.DurationProto(20 * time.Second),
+			Window: gogotypes.DurationProto(-4 * time.Second),
 		},
 	}
 
 	good := []*api.RestartPolicy{
 		{
-			Delay:  gogotypes.DurationProto(time.Duration(10 * time.Second)),
-			Window: gogotypes.DurationProto(time.Duration(1 * time.Second)),
+			Delay:  gogotypes.DurationProto(10 * time.Second),
+			Window: gogotypes.DurationProto(1 * time.Second),
 		},
 	}
 
@@ -557,15 +557,15 @@ func TestValidateUpdate(t *testing.T) {
 	bad := []*api.UpdateConfig{
 		{Delay: -1 * time.Second},
 		{Delay: -1000 * time.Second},
-		{Monitor: gogotypes.DurationProto(time.Duration(-1 * time.Second))},
-		{Monitor: gogotypes.DurationProto(time.Duration(-1000 * time.Second))},
+		{Monitor: gogotypes.DurationProto(-1 * time.Second)},
+		{Monitor: gogotypes.DurationProto(-1000 * time.Second)},
 		{MaxFailureRatio: -0.1},
 		{MaxFailureRatio: 1.1},
 	}
 
 	good := []*api.UpdateConfig{
 		{Delay: time.Second},
-		{Monitor: gogotypes.DurationProto(time.Duration(time.Second))},
+		{Monitor: gogotypes.DurationProto(time.Second)},
 		{MaxFailureRatio: 0.5},
 	}
 
@@ -595,15 +595,15 @@ func TestCreateService(t *testing.T) {
 	// test port conflicts
 	spec = createSpec("name2", "image", 1)
 	spec.Endpoint = &api.EndpointSpec{Ports: []*api.PortConfig{
-		{PublishedPort: uint32(9000), TargetPort: uint32(9000), Protocol: api.PortConfig_Protocol(api.ProtocolTCP)},
+		{PublishedPort: uint32(9000), TargetPort: uint32(9000), Protocol: api.ProtocolTCP},
 	}}
 	r, err = ts.Client.CreateService(context.Background(), &api.CreateServiceRequest{Spec: spec})
 	assert.NoError(t, err)
 	assert.NotEmpty(t, r.Service.ID)
 
 	spec2 := createSpec("name3", "image", 1)
 	spec2.Endpoint = &api.EndpointSpec{Ports: []*api.PortConfig{
-		{PublishedPort: uint32(9000), TargetPort: uint32(9000), Protocol: api.PortConfig_Protocol(api.ProtocolTCP)},
+		{PublishedPort: uint32(9000), TargetPort: uint32(9000), Protocol: api.ProtocolTCP},
 	}}
 	_, err = ts.Client.CreateService(context.Background(), &api.CreateServiceRequest{Spec: spec2})
 	assert.Error(t, err)
@@ -612,62 +612,62 @@ func TestCreateService(t *testing.T) {
 	// test no port conflicts when no publish port is specified
 	spec3 := createSpec("name4", "image", 1)
 	spec3.Endpoint = &api.EndpointSpec{Ports: []*api.PortConfig{
-		{TargetPort: uint32(9000), Protocol: api.PortConfig_Protocol(api.ProtocolTCP)},
+		{TargetPort: uint32(9000), Protocol: api.ProtocolTCP},
 	}}
 	r, err = ts.Client.CreateService(context.Background(), &api.CreateServiceRequest{Spec: spec3})
 	assert.NoError(t, err)
 	assert.NotEmpty(t, r.Service.ID)
 	spec4 := createSpec("name5", "image", 1)
 	spec4.Endpoint = &api.EndpointSpec{Ports: []*api.PortConfig{
-		{TargetPort: uint32(9001), Protocol: api.PortConfig_Protocol(api.ProtocolTCP)},
+		{TargetPort: uint32(9001), Protocol: api.ProtocolTCP},
 	}}
 	_, err = ts.Client.CreateService(context.Background(), &api.CreateServiceRequest{Spec: spec4})
 	assert.NoError(t, err)
 
 	// ensure no port conflict when different protocols are used
 	spec = createSpec("name6", "image", 1)
 	spec.Endpoint = &api.EndpointSpec{Ports: []*api.PortConfig{
-		{PublishedPort: uint32(9100), TargetPort: uint32(9100), Protocol: api.PortConfig_Protocol(api.ProtocolTCP)},
+		{PublishedPort: uint32(9100), TargetPort: uint32(9100), Protocol: api.ProtocolTCP},
 	}}
 	r, err = ts.Client.CreateService(context.Background(), &api.CreateServiceRequest{Spec: spec})
 	assert.NoError(t, err)
 	assert.NotEmpty(t, r.Service.ID)
 
 	spec2 = createSpec("name7", "image", 1)
 	spec2.Endpoint = &api.EndpointSpec{Ports: []*api.PortConfig{
-		{PublishedPort: uint32(9100), TargetPort: uint32(9100), Protocol: api.PortConfig_Protocol(api.ProtocolUDP)},
+		{PublishedPort: uint32(9100), TargetPort: uint32(9100), Protocol: api.ProtocolUDP},
 	}}
 	_, err = ts.Client.CreateService(context.Background(), &api.CreateServiceRequest{Spec: spec2})
 	assert.NoError(t, err)
 
 	// ensure no port conflict when host ports overlap
 	spec = createSpec("name8", "image", 1)
 	spec.Endpoint = &api.EndpointSpec{Ports: []*api.PortConfig{
-		{PublishMode: api.PublishModeHost, PublishedPort: uint32(9101), TargetPort: uint32(9101), Protocol: api.PortConfig_Protocol(api.ProtocolTCP)},
+		{PublishMode: api.PublishModeHost, PublishedPort: uint32(9101), TargetPort: uint32(9101), Protocol: api.ProtocolTCP},
 	}}
 	r, err = ts.Client.CreateService(context.Background(), &api.CreateServiceRequest{Spec: spec})
 	assert.NoError(t, err)
 	assert.NotEmpty(t, r.Service.ID)
 
 	spec2 = createSpec("name9", "image", 1)
 	spec2.Endpoint = &api.EndpointSpec{Ports: []*api.PortConfig{
-		{PublishMode: api.PublishModeHost, PublishedPort: uint32(9101), TargetPort: uint32(9101), Protocol: api.PortConfig_Protocol(api.ProtocolTCP)},
+		{PublishMode: api.PublishModeHost, PublishedPort: uint32(9101), TargetPort: uint32(9101), Protocol: api.ProtocolTCP},
 	}}
 	_, err = ts.Client.CreateService(context.Background(), &api.CreateServiceRequest{Spec: spec2})
 	assert.NoError(t, err)
 
 	// ensure port conflict when host ports overlaps with ingress port (host port first)
 	spec = createSpec("name10", "image", 1)
 	spec.Endpoint = &api.EndpointSpec{Ports: []*api.PortConfig{
-		{PublishMode: api.PublishModeHost, PublishedPort: uint32(9102), TargetPort: uint32(9102), Protocol: api.PortConfig_Protocol(api.ProtocolTCP)},
+		{PublishMode: api.PublishModeHost, PublishedPort: uint32(9102), TargetPort: uint32(9102), Protocol: api.ProtocolTCP},
 	}}
 	r, err = ts.Client.CreateService(context.Background(), &api.CreateServiceRequest{Spec: spec})
 	assert.NoError(t, err)
 	assert.NotEmpty(t, r.Service.ID)
 
 	spec2 = createSpec("name11", "image", 1)
 	spec2.Endpoint = &api.EndpointSpec{Ports: []*api.PortConfig{
-		{PublishMode: api.PublishModeIngress, PublishedPort: uint32(9102), TargetPort: uint32(9102), Protocol: api.PortConfig_Protocol(api.ProtocolTCP)},
+		{PublishMode: api.PublishModeIngress, PublishedPort: uint32(9102), TargetPort: uint32(9102), Protocol: api.ProtocolTCP},
 	}}
 	_, err = ts.Client.CreateService(context.Background(), &api.CreateServiceRequest{Spec: spec2})
 	assert.Error(t, err)
@@ -676,15 +676,15 @@ func TestCreateService(t *testing.T) {
 	// ensure port conflict when host ports overlaps with ingress port (ingress port first)
 	spec = createSpec("name12", "image", 1)
 	spec.Endpoint = &api.EndpointSpec{Ports: []*api.PortConfig{
-		{PublishMode: api.PublishModeIngress, PublishedPort: uint32(9103), TargetPort: uint32(9103), Protocol: api.PortConfig_Protocol(api.ProtocolTCP)},
+		{PublishMode: api.PublishModeIngress, PublishedPort: uint32(9103), TargetPort: uint32(9103), Protocol: api.ProtocolTCP},
 	}}
 	r, err = ts.Client.CreateService(context.Background(), &api.CreateServiceRequest{Spec: spec})
 	assert.NoError(t, err)
 	assert.NotEmpty(t, r.Service.ID)
 
 	spec2 = createSpec("name13", "image", 1)
 	spec2.Endpoint = &api.EndpointSpec{Ports: []*api.PortConfig{
-		{PublishMode: api.PublishModeHost, PublishedPort: uint32(9103), TargetPort: uint32(9103), Protocol: api.PortConfig_Protocol(api.ProtocolTCP)},
+		{PublishMode: api.PublishModeHost, PublishedPort: uint32(9103), TargetPort: uint32(9103), Protocol: api.ProtocolTCP},
 	}}
 	_, err = ts.Client.CreateService(context.Background(), &api.CreateServiceRequest{Spec: spec2})
 	assert.Error(t, err)
@@ -1009,7 +1009,7 @@ func TestUpdateService(t *testing.T) {
 	// test port conflicts
 	spec2 := createSpec("name2", "image", 1)
 	spec2.Endpoint = &api.EndpointSpec{Ports: []*api.PortConfig{
-		{PublishedPort: uint32(9000), TargetPort: uint32(9000), Protocol: api.PortConfig_Protocol(api.ProtocolTCP)},
+		{PublishedPort: uint32(9000), TargetPort: uint32(9000), Protocol: api.ProtocolTCP},
 	}}
 	_, err = ts.Client.CreateService(context.Background(), &api.CreateServiceRequest{Spec: spec2})
 	assert.NoError(t, err)
@@ -1019,7 +1019,7 @@ func TestUpdateService(t *testing.T) {
 	assert.NoError(t, err)
 
 	spec3.Endpoint = &api.EndpointSpec{Ports: []*api.PortConfig{
-		{PublishedPort: uint32(9000), TargetPort: uint32(9000), Protocol: api.PortConfig_Protocol(api.ProtocolTCP)},
+		{PublishedPort: uint32(9000), TargetPort: uint32(9000), Protocol: api.ProtocolTCP},
 	}}
 	_, err = ts.Client.UpdateService(context.Background(), &api.UpdateServiceRequest{
 		ServiceID:      rs.Service.ID,
@@ -1029,7 +1029,7 @@ func TestUpdateService(t *testing.T) {
 	assert.Error(t, err)
 	assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err))
 	spec3.Endpoint = &api.EndpointSpec{Ports: []*api.PortConfig{
-		{PublishedPort: uint32(9001), TargetPort: uint32(9000), Protocol: api.PortConfig_Protocol(api.ProtocolTCP)},
+		{PublishedPort: uint32(9001), TargetPort: uint32(9000), Protocol: api.ProtocolTCP},
 	}}
 	_, err = ts.Client.UpdateService(context.Background(), &api.UpdateServiceRequest{
 		ServiceID:      rs.Service.ID,

manager/controlapi/volume_test.go

@@ -368,7 +368,7 @@ func TestUpdateVolumeInvalidFields(t *testing.T) {
 			name: "Secrets",
 			apply: func(spec *api.VolumeSpec) {
 				spec.Secrets = []*api.VolumeSecret{
-					&api.VolumeSecret{Key: "mykey", Secret: "mysecret"},
+					{Key: "mykey", Secret: "mysecret"},
 				}
 			},
 		}, {