chore: enable and fix several rules from staticcheck

.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:

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!")
 		}
 	}

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
 	}

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)

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)
 			}

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 {

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")

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,15 +1095,15 @@ 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"])
 		assert.Equal(t, 1, t1Assignments["id3"])
 		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")
 	}
 }

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
 }

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)
 		}
 	}

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

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 (

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")
 	}

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{

volumequeue/queue.go

@@ -208,5 +208,4 @@ func (vq *VolumeQueue) Stop() {
 	for _, entry := range vq.outstanding {
 		entry.cancel()
 	}
-	return
 }