diff --git a/.golangci.yml b/.golangci.yml index 9add6fc6f9..1ff60c67d1 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -13,12 +13,24 @@ linters: - misspell - modernize - revive + - staticcheck - unconvert - unused settings: govet: enable: - nilness + staticcheck: + checks: + - all + - -QF1008 # Omit embedded fields from selector expression; https://staticcheck.dev/docs/checks/#QF1008 + - -SA1019 # FIXME: Using a deprecated function, variable, constant or field; https://staticcheck.dev/docs/checks/#SA1019 + - -SA1029 # FIXME: Inappropriate key in call to context.WithValue; https://staticcheck.dev/docs/checks/#SA1029 + - -SA2001 # FIXME: Empty critical section; https://staticcheck.dev/docs/checks/#SA2001 + - -SA4005 # FIXME: Field assignment that will never be observed; https://staticcheck.dev/docs/checks/#SA4005 + - -SA4006 # FIXME: A value assigned to a variable is never read before being overwritten; https://staticcheck.dev/docs/checks/#SA4006 + - -SA4010 # FIXME: The result of append will never be observed anywhere; https://staticcheck.dev/docs/checks/#SA4010 + - -ST1005 # Incorrectly formatted error string; https://staticcheck.dev/docs/checks/#ST1005 exclusions: generated: lax presets: diff --git a/agent/exec/controller_test.go b/agent/exec/controller_test.go index ab83184f50..5334455d17 100644 --- a/agent/exec/controller_test.go +++ b/agent/exec/controller_test.go @@ -182,11 +182,12 @@ func TestReadyRunning(t *testing.T) { return nil } ctlr.WaitFn = func(ctx context.Context) error { - if ctlr.calls["Wait"] == 1 { + switch ctlr.calls["Wait"] { + case 1: return context.Canceled - } else if ctlr.calls["Wait"] == 2 { + case 2: return nil - } else { + default: panic("unexpected call!") } } @@ -298,11 +299,12 @@ func TestAlreadyStarted(t *testing.T) { return ErrTaskStarted } ctlr.WaitFn = func(ctx context.Context) error { - if ctlr.calls["Wait"] == 1 { + switch ctlr.calls["Wait"] { + case 1: return context.Canceled - } else if ctlr.calls["Wait"] == 2 { + case 2: return newExitError(1) - } else { + default: panic("unexpected call!") } } diff --git a/ca/config.go b/ca/config.go index 8311048173..6b4328c5c1 100644 --- a/ca/config.go +++ b/ca/config.go @@ -513,7 +513,7 @@ type CertificateRequestConfig struct { // CreateSecurityConfig creates a new key and cert for this node, either locally // or via a remote CA. -func (rootCA RootCA) CreateSecurityConfig(ctx context.Context, krw *KeyReadWriter, config CertificateRequestConfig) (*SecurityConfig, func() error, error) { +func (rca RootCA) CreateSecurityConfig(ctx context.Context, krw *KeyReadWriter, config CertificateRequestConfig) (*SecurityConfig, func() error, error) { ctx = log.WithModule(ctx, "tls") // Create a new random ID for this certificate @@ -524,13 +524,13 @@ func (rootCA RootCA) CreateSecurityConfig(ctx context.Context, krw *KeyReadWrite } proposedRole := ManagerRole - tlsKeyPair, issuerInfo, err := rootCA.IssueAndSaveNewCertificates(krw, cn, proposedRole, org) + tlsKeyPair, issuerInfo, err := rca.IssueAndSaveNewCertificates(krw, cn, proposedRole, org) switch errors.Cause(err) { case ErrNoValidSigner: config.RetryInterval = GetCertRetryInterval // Request certificate issuance from a remote CA. // Last argument is nil because at this point we don't have any valid TLS creds - tlsKeyPair, issuerInfo, err = rootCA.RequestAndSaveNewCertificates(ctx, krw, config) + tlsKeyPair, issuerInfo, err = rca.RequestAndSaveNewCertificates(ctx, krw, config) if err != nil { log.G(ctx).WithError(err).Error("failed to request and save new certificate") return nil, nil, err @@ -548,7 +548,7 @@ func (rootCA RootCA) CreateSecurityConfig(ctx context.Context, krw *KeyReadWrite return nil, nil, err } - secConfig, cleanup, err := NewSecurityConfig(&rootCA, krw, tlsKeyPair, issuerInfo) + secConfig, cleanup, err := NewSecurityConfig(&rca, krw, tlsKeyPair, issuerInfo) if err == nil { log.G(ctx).WithFields(log.Fields{ "node.id": secConfig.ClientTLSCreds.NodeID(), @@ -685,8 +685,8 @@ func NewClientTLSConfig(certs []tls.Certificate, rootCAPool *x509.CertPool, serv // NewClientTLSCredentials returns GRPC credentials for a TLS GRPC client, given a tls.Certificate // a PEM-Encoded root CA Certificate, and the name of the remote server the client wants to connect to. -func (rootCA *RootCA) NewClientTLSCredentials(cert *tls.Certificate, serverName string) (*MutableTLSCreds, error) { - tlsConfig, err := NewClientTLSConfig([]tls.Certificate{*cert}, rootCA.Pool, serverName) +func (rca *RootCA) NewClientTLSCredentials(cert *tls.Certificate, serverName string) (*MutableTLSCreds, error) { + tlsConfig, err := NewClientTLSConfig([]tls.Certificate{*cert}, rca.Pool, serverName) if err != nil { return nil, err } @@ -698,8 +698,8 @@ func (rootCA *RootCA) NewClientTLSCredentials(cert *tls.Certificate, serverName // NewServerTLSCredentials returns GRPC credentials for a TLS GRPC client, given a tls.Certificate // a PEM-Encoded root CA Certificate, and the name of the remote server the client wants to connect to. -func (rootCA *RootCA) NewServerTLSCredentials(cert *tls.Certificate) (*MutableTLSCreds, error) { - tlsConfig, err := NewServerTLSConfig([]tls.Certificate{*cert}, rootCA.Pool) +func (rca *RootCA) NewServerTLSCredentials(cert *tls.Certificate) (*MutableTLSCreds, error) { + tlsConfig, err := NewServerTLSConfig([]tls.Certificate{*cert}, rca.Pool) if err != nil { return nil, err } diff --git a/manager/dispatcher/dispatcher_test.go b/manager/dispatcher/dispatcher_test.go index 6f593ed792..f67a2c69fa 100644 --- a/manager/dispatcher/dispatcher_test.go +++ b/manager/dispatcher/dispatcher_test.go @@ -514,11 +514,12 @@ func TestAssignmentsSecretDriver(t *testing.T) { _, _, secretChanges, _ := splitChanges(resp.Changes) assert.Len(t, secretChanges, 2) for _, s := range secretChanges { - if s.ID == "driverSecret" { + switch s.ID { + case "driverSecret": assert.Equal(t, secretValue, s.Spec.Data) - } else if s.ID == "driverDoNotReuseSecret" { + case "driverDoNotReuseSecret": assert.Fail(t, "Secret with DoNotReuse==true should not retain its original ID in the assignment", "%s != %s", "driverDoNotReuseSecret", s.ID) - } else { + default: taskSpecificID := fmt.Sprintf("%s.%s", "driverDoNotReuseSecret", task.ID) assert.Equal(t, taskSpecificID, s.ID) assert.Equal(t, doNotReuseSecretValue, s.Spec.Data) diff --git a/manager/manager.go b/manager/manager.go index 8b14005641..470257e967 100644 --- a/manager/manager.go +++ b/manager/manager.go @@ -879,10 +879,11 @@ func (m *Manager) handleLeadershipEvents(ctx context.Context, leadershipCh chan } newState := leadershipEvent.(raft.LeadershipState) - if newState == raft.IsLeader { + switch newState { + case raft.IsLeader: m.becomeLeader(ctx) leaderMetric.Set(1) - } else if newState == raft.IsFollower { + case raft.IsFollower: m.becomeFollower() leaderMetric.Set(0) } diff --git a/manager/orchestrator/jobs/replicated/reconciler_test.go b/manager/orchestrator/jobs/replicated/reconciler_test.go index e4d5e03c16..f93ac49dd6 100644 --- a/manager/orchestrator/jobs/replicated/reconciler_test.go +++ b/manager/orchestrator/jobs/replicated/reconciler_test.go @@ -50,7 +50,7 @@ func (u uniqueSlotsMatcher) FailureMessage(_ any) string { } func (u uniqueSlotsMatcher) NegatedFailureMessage(_ any) string { - return fmt.Sprintf("expected tasks to have duplicate slots") + return "expected tasks to have duplicate slots" } func HaveUniqueSlots() types.GomegaMatcher { diff --git a/manager/orchestrator/restart/restart.go b/manager/orchestrator/restart/restart.go index a13340112b..0f729767f0 100644 --- a/manager/orchestrator/restart/restart.go +++ b/manager/orchestrator/restart/restart.go @@ -186,13 +186,9 @@ func (r *Supervisor) Restart(ctx context.Context, tx store.Tx, cluster *api.Clus } } - waitStop := true - // Normally we wait for the old task to stop running, but we skip this // if the old task is already dead or the node it's assigned to is down. - if (n != nil && n.Status.State == api.NodeStatus_DOWN) || t.Status.State > api.TaskStateRunning { - waitStop = false - } + waitStop := (n == nil || n.Status.State != api.NodeStatus_DOWN) && (t.Status.State <= api.TaskStateRunning) if err := store.CreateTask(tx, restartTask); err != nil { log.G(ctx).WithError(err).WithField("task.id", restartTask.ID).Error("task create failed") diff --git a/manager/scheduler/scheduler_test.go b/manager/scheduler/scheduler_test.go index 4059f5c6b9..becf69406e 100644 --- a/manager/scheduler/scheduler_test.go +++ b/manager/scheduler/scheduler_test.go @@ -483,11 +483,12 @@ func testHA(t *testing.T, useSpecVersion bool) { nodesWith3T1Tasks := 0 nodesWith4T1Tasks := 0 for nodeID, taskCount := range t1Assignments { - if taskCount == 3 { + switch taskCount { + case 3: nodesWith3T1Tasks++ - } else if taskCount == 4 { + case 4: nodesWith4T1Tasks++ - } else { + default: t.Fatalf("unexpected number of tasks %d on node %s", taskCount, nodeID) } } @@ -564,11 +565,12 @@ func testHA(t *testing.T, useSpecVersion bool) { nodesWith4T1Tasks = 0 nodesWith5T1Tasks := 0 for nodeID, taskCount := range t1Assignments { - if taskCount == 4 { + switch taskCount { + case 4: nodesWith4T1Tasks++ - } else if taskCount == 5 { + case 5: nodesWith5T1Tasks++ - } else { + default: t.Fatalf("unexpected number of tasks %d on node %s", taskCount, nodeID) } } @@ -1070,7 +1072,8 @@ func testMultiplePreferences(t *testing.T, useSpecVersion bool) { // The remaining 7 tasks should be spread across rack1 and rack2 of // az2. - if t1Assignments["id2"]+t1Assignments["id3"]+t1Assignments["id4"] == 4 { + switch t1Assignments["id2"] + t1Assignments["id3"] + t1Assignments["id4"] { + case 4: // If rack1 gets 4 and rack2 gets 3, then one of id[2-4] will have two // tasks and the others will have one. if t1Assignments["id2"] == 2 { @@ -1092,7 +1095,7 @@ func testMultiplePreferences(t *testing.T, useSpecVersion bool) { assert.Equal(t, 2, t1Assignments["id5"]) assert.Equal(t, 1, t1Assignments["id6"]) } - } else if t1Assignments["id2"]+t1Assignments["id3"]+t1Assignments["id4"] == 3 { + case 3: // If rack2 gets 4 and rack1 gets 3, then id[2-4] will each get // 1 task and id[5-6] will each get 2 tasks. assert.Equal(t, 1, t1Assignments["id2"]) @@ -1100,7 +1103,7 @@ func testMultiplePreferences(t *testing.T, useSpecVersion bool) { assert.Equal(t, 1, t1Assignments["id4"]) assert.Equal(t, 2, t1Assignments["id5"]) assert.Equal(t, 2, t1Assignments["id6"]) - } else { + default: t.Fatal("unexpected task layout") } } diff --git a/manager/state/raft/storage/walwrap.go b/manager/state/raft/storage/walwrap.go index cffcd485d1..fad95f1756 100644 --- a/manager/state/raft/storage/walwrap.go +++ b/manager/state/raft/storage/walwrap.go @@ -253,6 +253,6 @@ func ListWALs(dirpath string) ([]string, error) { } // Sort WAL filenames in lexical order - sort.Sort(sort.StringSlice(wals)) + sort.Strings(wals) return wals, nil } diff --git a/remotes/remotes_test.go b/remotes/remotes_test.go index c907059db0..590ca4c76c 100644 --- a/remotes/remotes_test.go +++ b/remotes/remotes_test.go @@ -285,7 +285,7 @@ func TestRemotesPractical(t *testing.T) { expected, delta := selections/len(peers), int(tolerance*float64(selections)) low, high := expected-delta, expected+delta for peer, count := range seen { - if !(count >= low && count <= high) { + if count < low || count > high { t.Fatalf("weighted selection not balanced: %v selected %v/%v, expected range %v, %v", peer, count, selections, low, high) } } @@ -317,7 +317,7 @@ func TestRemotesPractical(t *testing.T) { } } - if !(count >= low && count <= high) { + if count < low || count > high { t.Fatalf("weighted selection not balanced: %v selected %v/%v, expected range %v, %v", peer, count, selections, low, high) } } diff --git a/swarmd/cmd/swarm-rafttool/renewcert.go b/swarmd/cmd/swarm-rafttool/renewcert.go index 253adfe4f3..a2ae329afd 100644 --- a/swarmd/cmd/swarm-rafttool/renewcert.go +++ b/swarmd/cmd/swarm-rafttool/renewcert.go @@ -31,9 +31,9 @@ func renewCerts(swarmdir, unlockKey string) error { // We need to make sure when renewing that we provide the same CN (node ID), // OU (role), and org (swarm cluster ID) when getting a new certificate var ( - cn string = certificates[0].Subject.CommonName - ou string = certificates[0].Subject.OrganizationalUnit[0] - org string = certificates[0].Subject.Organization[0] + cn = certificates[0].Subject.CommonName + ou = certificates[0].Subject.OrganizationalUnit[0] + org = certificates[0].Subject.Organization[0] ) // Load up the raft data on disk diff --git a/swarmd/cmd/swarmctl/config/inspect.go b/swarmd/cmd/swarmctl/config/inspect.go index d4fa30d45f..7b3e928726 100644 --- a/swarmd/cmd/swarmctl/config/inspect.go +++ b/swarmd/cmd/swarmctl/config/inspect.go @@ -28,7 +28,7 @@ func printConfigSummary(config *api.Config) { common.FprintfIfNotEmpty(w, "Created\t: %s\n", gogotypes.TimestampString(config.Meta.CreatedAt)) fmt.Print(w, "Payload:\n\n") - fmt.Println(w, config.Spec.Data) + fmt.Println(w, string(config.Spec.Data)) } var ( diff --git a/swarmd/dockerexec/controller_test.go b/swarmd/dockerexec/controller_test.go index 9a7ff1e32f..1a6081e27c 100644 --- a/swarmd/dockerexec/controller_test.go +++ b/swarmd/dockerexec/controller_test.go @@ -14,7 +14,6 @@ import ( "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/container" - containertypes "github.com/docker/docker/api/types/container" "github.com/docker/docker/api/types/events" "github.com/docker/docker/api/types/network" gogotypes "github.com/gogo/protobuf/types" @@ -43,12 +42,12 @@ func TestControllerPrepare(t *testing.T) { panic("unexpected call of ImagePull") } - client.ContainerCreateFn = func(_ context.Context, cConfig *containertypes.Config, hConfig *containertypes.HostConfig, nConfig *network.NetworkingConfig, platform *v1.Platform, containerName string) (containertypes.CreateResponse, error) { + client.ContainerCreateFn = func(_ context.Context, cConfig *container.Config, hConfig *container.HostConfig, nConfig *network.NetworkingConfig, platform *v1.Platform, containerName string) (container.CreateResponse, error) { if reflect.DeepEqual(*cConfig, *config.config()) && reflect.DeepEqual(*hConfig, *config.hostConfig()) && reflect.DeepEqual(*nConfig, *config.networkingConfig()) && containerName == config.name() { - return containertypes.CreateResponse{ID: "container-id-" + task.ID}, nil + return container.CreateResponse{ID: "container-id-" + task.ID}, nil } panic("unexpected call to ContainerCreate") } @@ -73,11 +72,11 @@ func TestControllerPrepareAlreadyPrepared(t *testing.T) { panic("unexpected call of ImagePull") } - client.ContainerCreateFn = func(_ context.Context, cConfig *containertypes.Config, hostConfig *containertypes.HostConfig, networking *network.NetworkingConfig, platform *v1.Platform, containerName string) (containertypes.CreateResponse, error) { + client.ContainerCreateFn = func(_ context.Context, cConfig *container.Config, hostConfig *container.HostConfig, networking *network.NetworkingConfig, platform *v1.Platform, containerName string) (container.CreateResponse, error) { if reflect.DeepEqual(*cConfig, *config.config()) && reflect.DeepEqual(*networking, *config.networkingConfig()) && containerName == config.name() { - return containertypes.CreateResponse{}, fmt.Errorf("Conflict. The name") + return container.CreateResponse{}, fmt.Errorf("Conflict. The name") } panic("unexpected call of ContainerCreate") } diff --git a/swarmd/dockerexec/executor.go b/swarmd/dockerexec/executor.go index 1aa0e99132..754522693f 100644 --- a/swarmd/dockerexec/executor.go +++ b/swarmd/dockerexec/executor.go @@ -66,9 +66,10 @@ func (e *executor) Describe(ctx context.Context) (*api.NodeDescription, error) { for _, typ := range plgn.Config.Interface.Types { if typ.Prefix == "docker" && plgn.Enabled { plgnTyp := typ.Capability - if typ.Capability == "volumedriver" { + switch typ.Capability { + case "volumedriver": plgnTyp = "Volume" - } else if typ.Capability == "networkdriver" { + case "networkdriver": plgnTyp = "Network" } plugins[api.PluginDescription{ diff --git a/volumequeue/queue.go b/volumequeue/queue.go index 3e87471f79..1fc8cf64f8 100644 --- a/volumequeue/queue.go +++ b/volumequeue/queue.go @@ -208,5 +208,4 @@ func (vq *VolumeQueue) Stop() { for _, entry := range vq.outstanding { entry.cancel() } - return }