Add staticcheck github action, resolve existing lint

.github/workflows/static-check.yml

@@ -0,0 +1,26 @@
+name: static-check
+
+on:
+  pull_request:
+  push:
+    tags:
+      - 'v*'
+    branches:
+      - main
+
+jobs:
+  staticcheck:
+    name: staticcheck (project)
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v1
+        with:
+          fetch-depth: 1
+      - id: install_go
+        uses: WillAbides/[email protected]
+        with:
+          go-version: "1.20.x"
+      - uses: dominikh/[email protected]
+        with:
+          install-go: false
+          version: "2023.1.7"

client/httpclient_test.go

@@ -3,7 +3,7 @@ package client
 import (
 	"compress/gzip"
 	"context"
-	"io/ioutil"
+	"io"
 	"net/http"
 	"sync/atomic"
 	"testing"
@@ -22,10 +22,12 @@ func TestHTTPPolling(t *testing.T) {
 	srv := internal.StartMockServer(t)
 	var rcvCounter int64
 	srv.OnMessage = func(msg *protobufs.AgentToServer) *protobufs.ServerToAgent {
-		assert.EqualValues(t, rcvCounter, msg.SequenceNum)
-		if msg != nil {
-			atomic.AddInt64(&rcvCounter, 1)
+		if msg == nil {
+			t.Error("unexpected nil msg")
+			return nil
 		}
+		assert.EqualValues(t, rcvCounter, msg.SequenceNum)
+		atomic.AddInt64(&rcvCounter, 1)
 		return nil
 	}
 
@@ -63,7 +65,7 @@ func TestHTTPClientCompression(t *testing.T) {
 		assert.Equal(t, "gzip", r.Header.Get("Content-Encoding"))
 		reader, err := gzip.NewReader(r.Body)
 		assert.NoError(t, err)
-		body, err := ioutil.ReadAll(reader)
+		body, err := io.ReadAll(reader)
 		assert.NoError(t, err)
 		_ = r.Body.Close()
 		var response protobufs.AgentToServer
@@ -112,10 +114,12 @@ func TestHTTPClientSetPollingInterval(t *testing.T) {
 	srv := internal.StartMockServer(t)
 	var rcvCounter int64
 	srv.OnMessage = func(msg *protobufs.AgentToServer) *protobufs.ServerToAgent {
-		assert.EqualValues(t, rcvCounter, msg.SequenceNum)
-		if msg != nil {
-			atomic.AddInt64(&rcvCounter, 1)
+		if msg == nil {
+			t.Error("unexpected nil msg")
+			return nil
 		}
+		assert.EqualValues(t, rcvCounter, msg.SequenceNum)
+		atomic.AddInt64(&rcvCounter, 1)
 		return nil
 	}
 

client/internal/httpsender.go

@@ -110,7 +110,6 @@ func (h *HTTPSender) Run(
 			// This will make hasPendingMessage channel readable, so we will enter
 			// the case above on the next iteration of the loop.
 			h.ScheduleSend()
-			break
 
 		case <-ctx.Done():
 			return

client/internal/mockserver.go

@@ -40,8 +40,8 @@
 func newMockServer(t *testing.T) (*MockServer, *http.ServeMux) {
 	srv := &MockServer{
 		t:                t,
-		expectedHandlers: make(chan receivedMessageHandler, 0),
-		expectedComplete: make(chan struct{}, 0),
+		expectedHandlers: make(chan receivedMessageHandler),
+		expectedComplete: make(chan struct{}),
 	}
 
 	m := http.NewServeMux()
@@ -112,6 +112,11 @@
 
 func (m *MockServer) handlePlainHttp(w http.ResponseWriter, r *http.Request) {
 	msgBytes, err := io.ReadAll(r.Body)
+	if err != nil {
+		// request could not be read for some reason
+		http.Error(w, err.Error(), 500)
+		return
+	}
 
 	// We use alwaysRespond=true here because plain HTTP requests must always have
 	// a response.

client/internal/packagessyncer.go

@@ -104,7 +104,7 @@ func (s *packagesSyncer) doSync(ctx context.Context) {
 		s.logger.Errorf(ctx, "Package syncing failed: %V", err)
 		return
 	}
-	if bytes.Compare(hash, s.available.AllPackagesHash) == 0 {
+	if bytes.Equal(hash, s.available.AllPackagesHash) {
 		s.logger.Debugf(ctx, "All packages are already up to date.")
 		return
 	}
@@ -243,7 +243,7 @@ func (s *packagesSyncer) shouldDownloadFile(ctx context.Context, packageName str
 	} else {
 		// Compare the checksum of the file we have with what
 		// we are offered by the server.
-		if bytes.Compare(fileContentHash, file.ContentHash) != 0 {
+		if !bytes.Equal(fileContentHash, file.ContentHash) {
 			s.logger.Debugf(ctx, "Package %s: file hash mismatch, will download.", packageName)
 			return true, nil
 		}

client/wsclient.go

@@ -128,7 +128,7 @@ func (c *wsClient) SetPackageStatuses(statuses *protobufs.PackageStatuses) error
 // Try to connect once. Returns an error if connection fails and optional retryAfter
 // duration to indicate to the caller to retry after the specified time as instructed
 // by the Server.
-func (c *wsClient) tryConnectOnce(ctx context.Context) (err error, retryAfter sharedinternal.OptionalDuration) {
+func (c *wsClient) tryConnectOnce(ctx context.Context) (retryAfter sharedinternal.OptionalDuration, err error) {
 	var resp *http.Response
 	conn, resp, err := c.dialer.DialContext(ctx, c.url.String(), c.requestHeader)
 	if err != nil {
@@ -142,7 +142,7 @@ func (c *wsClient) tryConnectOnce(ctx context.Context) (err error, retryAfter sh
 				redirect, err := resp.Location()
 				if err != nil {
 					c.common.Logger.Errorf(ctx, "%d redirect, but no valid location: %s", resp.StatusCode, err)
-					return err, duration
+					return duration, err
 				}
 				// rewrite the scheme for the sake of tolerance
 				if redirect.Scheme == "http" {
@@ -157,9 +157,9 @@ func (c *wsClient) tryConnectOnce(ctx context.Context) (err error, retryAfter sh
 			} else {
 				c.common.Logger.Errorf(ctx, "Server responded with status=%v", resp.Status)
 			}
-			return err, duration
+			return duration, err
 		}
-		return err, sharedinternal.OptionalDuration{Defined: false}
+		return sharedinternal.OptionalDuration{Defined: false}, err
 	}
 
 	// Successfully connected.
@@ -170,7 +170,7 @@ func (c *wsClient) tryConnectOnce(ctx context.Context) (err error, retryAfter sh
 		c.common.Callbacks.OnConnect(ctx)
 	}
 
-	return nil, sharedinternal.OptionalDuration{Defined: false}
+	return sharedinternal.OptionalDuration{Defined: false}, nil
 }
 
 // Continuously try until connected. Will return nil when successfully
@@ -190,7 +190,7 @@ func (c *wsClient) ensureConnected(ctx context.Context) error {
 		select {
 		case <-timer.C:
 			{
-				if err, retryAfter := c.tryConnectOnce(ctx); err != nil {
+				if retryAfter, err := c.tryConnectOnce(ctx); err != nil {
 					c.lastInternalErr.Store(&err)
 					if errors.Is(err, context.Canceled) {
 						c.common.Logger.Debugf(ctx, "Client is stopped, will not try anymore.")

internal/certs.go

@@ -15,7 +15,6 @@
 	"encoding/pem"
 	"errors"
 	"fmt"
-	"io/ioutil"
 	"math/big"
 	"net"
 	"os"
@@ -79,19 +78,18 @@
 		InsecureSkipVerify: true,
 		ClientCAs:          caCertPool,
 	}
-	tlsConfig.BuildNameToCertificate()
 	return tlsConfig, nil
 }
 
 func CreateTLSCert(caCertPath, caKeyPath string) (*protobufs.TLSCertificate, error) {
 
 	// Load CA Cert.
-	caCertBytes, err := ioutil.ReadFile(caCertPath)
+	caCertBytes, err := os.ReadFile(caCertPath)
 	if err != nil {
 		return nil, fmt.Errorf("cannot read CA cert: %v", err)
 	}
 
-	caKeyBytes, err := ioutil.ReadFile(caKeyPath)
+	caKeyBytes, err := os.ReadFile(caKeyPath)
 	if err != nil {
 		return nil, fmt.Errorf("cannot read CA key: %v", err)
 	}

internal/retryafter_test.go

@@ -1,12 +1,13 @@
 package internal
 
 import (
-	"github.com/stretchr/testify/assert"
 	"math/rand"
 	"net/http"
 	"strconv"
 	"testing"
 	"time"
+
+	"github.com/stretchr/testify/assert"
 )
 
 func response503() *http.Response {
@@ -32,7 +33,6 @@ func assertDuration(t *testing.T, duration OptionalDuration, expected time.Durat
 
 func TestExtractRetryAfterHeaderDelaySeconds(t *testing.T) {
 	// Generate random n > 0 int
-	rand.Seed(time.Now().UnixNano())
 	retryIntervalSec := rand.Intn(9999)
 
 	// Generate a 503 status code response with Retry-After = n header
@@ -58,7 +58,6 @@ func TestExtractRetryAfterHeaderDelaySeconds(t *testing.T) {
 func TestExtractRetryAfterHeaderHttpDate(t *testing.T) {
 	// Generate a random n > 0 second duration
 	now := time.Now()
-	rand.Seed(now.UnixNano())
 	retryIntervalSec := rand.Intn(9999)
 	expectedDuration := time.Second * time.Duration(retryIntervalSec)
 

internal/wsmessage.go

@@ -11,6 +11,7 @@ import (
 // Message header is currently uint64 zero value.
 const wsMsgHeader = uint64(0)
 
+// DecodeWSMessage decodes a websocket message as bytes into a proto.Message.
 func DecodeWSMessage(bytes []byte, msg proto.Message) error {
 	// Message header is optional until the end of grace period that ends Feb 1, 2023.
 	// Check if the header is present.
@@ -23,9 +24,9 @@ func DecodeWSMessage(bytes []byte, msg proto.Message) error {
 		}
 		// Skip the header. It really is just a single zero byte for now.
 		bytes = bytes[n:]
-	} else {
-		// Old message format. No header present.
 	}
+	// If no header was present (the "if" check above), then this is the old
+	// message format. No header is present.
 
 	// Decode WebSocket message as a Protobuf message.
 	err := proto.Unmarshal(bytes, msg)

protobufshelpers/anyvaluehelpers.go

@@ -39,7 +39,7 @@ func IsEqualAnyValue(v1, v2 *protobufs.AnyValue) bool {
 
 	case *protobufs.AnyValue_BytesValue:
 		v2, ok := v2.Value.(*protobufs.AnyValue_BytesValue)
-		return ok && bytes.Compare(v1.BytesValue, v2.BytesValue) == 0
+		return ok && bytes.Equal(v1.BytesValue, v2.BytesValue)
 
 	case *protobufs.AnyValue_ArrayValue:
 		v2, ok := v2.Value.(*protobufs.AnyValue_ArrayValue)

server/serverimpl_test.go

@@ -628,6 +628,7 @@ func TestServerHonoursClientRequestContentEncoding(t *testing.T) {
 	require.NoError(t, err)
 
 	req, err := http.NewRequest("POST", "http://"+settings.ListenEndpoint+settings.ListenPath, bytes.NewReader(b))
+	require.NoError(t, err)
 	req.Header.Set(headerContentType, contentTypeProtobuf)
 	req.Header.Set(headerContentEncoding, contentEncodingGzip)
 	resp, err := hc.Do(req)
@@ -700,6 +701,7 @@ func TestServerHonoursAcceptEncoding(t *testing.T) {
 	b, err := proto.Marshal(&sendMsg)
 	require.NoError(t, err)
 	req, err := http.NewRequest("POST", "http://"+settings.ListenEndpoint+settings.ListenPath, bytes.NewReader(b))
+	require.NoError(t, err)
 	req.Header.Set(headerContentType, contentTypeProtobuf)
 	req.Header.Set(headerAcceptEncoding, contentEncodingGzip)
 	resp, err := hc.Do(req)