From acdc3db10f505455007a72e792645987725a8711 Mon Sep 17 00:00:00 2001 From: Matthieu MOREL Date: Wed, 29 Jul 2026 08:08:36 +0200 Subject: [PATCH] chore: extend linters to tests and fix some of them Signed-off-by: Matthieu MOREL --- .golangci.yml | 29 ++++----- agent/agent_test.go | 5 +- agent/storage_test.go | 5 +- ca/certificates_test.go | 12 ++-- ca/keyutils/keyutils_test.go | 8 +-- manager/controlapi/service_test.go | 66 ++++++++++----------- manager/controlapi/volume_test.go | 2 +- manager/keymanager/keymanager.go | 8 +-- manager/orchestrator/update/updater_test.go | 2 +- 9 files changed, 65 insertions(+), 72 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 9add6fc6f9..75092ea788 100644 --- a/.golangci.yml +++ b/.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,25 +31,14 @@ 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: @@ -54,8 +46,7 @@ formatters: - goimports exclusions: generated: lax - paths: - - .*\.pb\.go$ + warn-unused: true issues: max-issues-per-linter: 0 diff --git a/agent/agent_test.go b/agent/agent_test.go index 441f0d3277..6be0b421fb 100644 --- a/agent/agent_test.go +++ b/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, diff --git a/agent/storage_test.go b/agent/storage_test.go index 0ef01dacb2..1d9d570410 100644 --- a/agent/storage_test.go +++ b/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() } } } diff --git a/ca/certificates_test.go b/ca/certificates_test.go index 9543578920..4a4115a149 100644 --- a/ca/certificates_test.go +++ b/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 diff --git a/ca/keyutils/keyutils_test.go b/ca/keyutils/keyutils_test.go index d0b0d455a7..c031845ada 100644 --- a/ca/keyutils/keyutils_test.go +++ b/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) { diff --git a/manager/controlapi/service_test.go b/manager/controlapi/service_test.go index 737dc7f3b7..acef1d9add 100644 --- a/manager/controlapi/service_test.go +++ b/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,7 +595,7 @@ 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) @@ -603,7 +603,7 @@ func TestCreateService(t *testing.T) { 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,14 +612,14 @@ 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) @@ -627,7 +627,7 @@ func TestCreateService(t *testing.T) { // 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) @@ -635,7 +635,7 @@ func TestCreateService(t *testing.T) { 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) @@ -643,7 +643,7 @@ func TestCreateService(t *testing.T) { // 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) @@ -651,7 +651,7 @@ func TestCreateService(t *testing.T) { 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) @@ -659,7 +659,7 @@ func TestCreateService(t *testing.T) { // 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) @@ -667,7 +667,7 @@ func TestCreateService(t *testing.T) { 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,7 +676,7 @@ 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) @@ -684,7 +684,7 @@ func TestCreateService(t *testing.T) { 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, diff --git a/manager/controlapi/volume_test.go b/manager/controlapi/volume_test.go index 8c6a237a35..f3f65579b2 100644 --- a/manager/controlapi/volume_test.go +++ b/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"}, } }, }, { diff --git a/manager/keymanager/keymanager.go b/manager/keymanager/keymanager.go index df59d27922..f0b260788a 100644 --- a/manager/keymanager/keymanager.go +++ b/manager/keymanager/keymanager.go @@ -150,13 +150,13 @@ func (k *KeyManager) rotateKey(ctx context.Context) error { // agents to communicate without disruption on key change. for subsys, keys := range subsysKeys { if len(keys) == keyringSize { - min := 0 + minimum := 0 for i, key := range keys[1:] { - if key.LamportTime < keys[min].LamportTime { - min = i + if key.LamportTime < keys[minimum].LamportTime { + minimum = i } } - keys = append(keys[0:min], keys[min+1:]...) + keys = append(keys[0:minimum], keys[minimum+1:]...) } keys = append(keys, k.allocateKey(ctx, subsys)) subsysKeys[subsys] = keys diff --git a/manager/orchestrator/update/updater_test.go b/manager/orchestrator/update/updater_test.go index 55a9038e7e..1753af481d 100644 --- a/manager/orchestrator/update/updater_test.go +++ b/manager/orchestrator/update/updater_test.go @@ -573,7 +573,7 @@ func TestUpdaterTaskTimeout(t *testing.T) { err := s.Update(func(tx store.Tx) error { assert.NoError(t, store.CreateService(tx, service)) for i := range instances { - task := orchestrator.NewTask(nil, service, uint64(i), "") + task := orchestrator.NewTask(nil, service, i, "") task.Status.State = api.TaskStateRunning assert.NoError(t, store.CreateTask(tx, task)) }