Fix a number of problems found by staticcheck

Some of these problems are minor (unused vars), but others are real bugs (ignored errors).

Co-authored-by: Matt Keeler <mkeeler@users.noreply.github.com>

agent/agent_endpoint.go

@@ -914,7 +914,7 @@ func (s *HTTPServer) AgentRegisterService(resp http.ResponseWriter, req *http.Re
 	// the catalog endpoint so it helps ensure the sync will work properly.
 	if err := ns.Validate(); err != nil {
 		resp.WriteHeader(http.StatusBadRequest)
-		fmt.Fprintf(resp, err.Error())
+		fmt.Fprint(resp, err.Error())
 		return nil, nil
 	}
 

agent/agent_endpoint_test.go

@@ -3683,7 +3683,7 @@ func testAgent_RegisterServiceDeregisterService_Sidecar(t *testing.T, extraHCL s
 				require.Nil(obj)
 
 				svcs := a.State.Services(nil)
-				svc, ok = svcs[structs.NewServiceID(tt.wantNS.ID, nil)]
+				_, ok = svcs[structs.NewServiceID(tt.wantNS.ID, nil)]
 				if tt.wantSidecarIDLeftAfterDereg {
 					require.True(ok, "removed non-sidecar service at "+tt.wantNS.ID)
 				} else {

agent/cache-types/service_checks_test.go

@@ -188,8 +188,8 @@ func (m *mockAgent) LocalState() *local.State {
 	return m.state
 }
 
-func (m *mockAgent) LocalBlockingQuery(alwaysBlock bool, hash string, wait time.Duration,
+func (m *mockAgent) LocalBlockingQuery(_ bool, _ string, _ time.Duration,
-	fn func(ws memdb.WatchSet) (string, interface{}, error)) (string, interface{}, error) {
+	_ func(ws memdb.WatchSet) (string, interface{}, error)) (string, interface{}, error) {
 
 	hash, err := hashChecks(m.checks)
 	if err != nil {

agent/consul/leader_routine_manager.go

@@ -48,7 +48,7 @@ func (m *LeaderRoutineManager) IsRunning(name string) bool {
 }
 
 func (m *LeaderRoutineManager) Start(name string, routine LeaderRoutine) error {
-	return m.StartWithContext(nil, name, routine)
+	return m.StartWithContext(context.TODO(), name, routine)
 }
 
 func (m *LeaderRoutineManager) StartWithContext(parentCtx context.Context, name string, routine LeaderRoutine) error {

agent/consul/prepared_query/template.go

@@ -116,13 +116,9 @@ func (ct *CompiledTemplate) Render(name string, source structs.QuerySource) (*st
 		return nil, fmt.Errorf("Failed to copy query")
 	}
 
-	// Run the regular expression, if provided. We execute on a copy here
-	// to avoid internal lock contention because we expect this to be called
-	// from multiple goroutines.
 	var matches []string
 	if ct.re != nil {
-		re := ct.re.Copy()
+		matches = ct.re.FindStringSubmatch(name)
-		matches = re.FindStringSubmatch(name)
 	}
 
 	// Create a safe match function that can't fail at run time. It will

agent/consul/state/txn.go

@@ -217,11 +217,14 @@ func (s *Store) txnService(tx *memdb.Txn, idx uint64, op *structs.TxnServiceOp)
 	case api.ServiceGet:
 		entry, err = s.getNodeServiceTxn(tx, op.Node, op.Service.ID, &op.Service.EnterpriseMeta)
 		if entry == nil && err == nil {
-			err = fmt.Errorf("service %q on node %q doesn't exist", op.Service.ID, op.Node)
+			return nil, fmt.Errorf("service %q on node %q doesn't exist", op.Service.ID, op.Node)
 		}
 
 	case api.ServiceSet:
 		err = s.ensureServiceTxn(tx, idx, op.Node, &op.Service)
+		if err != nil {
+			return nil, err
+		}
 		entry, err = s.getNodeServiceTxn(tx, op.Node, op.Service.ID, &op.Service.EnterpriseMeta)
 
 	case api.ServiceCAS:
@@ -240,11 +243,11 @@ func (s *Store) txnService(tx *memdb.Txn, idx uint64, op *structs.TxnServiceOp)
 		var ok bool
 		ok, err = s.deleteServiceCASTxn(tx, idx, op.Service.ModifyIndex, op.Node, op.Service.ID, &op.Service.EnterpriseMeta)
 		if !ok && err == nil {
-			err = fmt.Errorf("failed to delete service %q on node %q, index is stale", op.Service.ID, op.Node)
+			return nil, fmt.Errorf("failed to delete service %q on node %q, index is stale", op.Service.ID, op.Node)
 		}
 
 	default:
-		err = fmt.Errorf("unknown Service verb %q", op.Verb)
+		return nil, fmt.Errorf("unknown Service verb %q", op.Verb)
 	}
 	if err != nil {
 		return nil, err

agent/coordinate_endpoint_test.go

@@ -236,7 +236,7 @@ func TestCoordinate_Node(t *testing.T) {
 	// Make sure we get a 404 with no coordinates.
 	req, _ := http.NewRequest("GET", "/v1/coordinate/node/foo?dc=dc1", nil)
 	resp := httptest.NewRecorder()
-	obj, err := a.srv.CoordinateNode(resp, req)
+	_, err := a.srv.CoordinateNode(resp, req)
 	if err != nil {
 		t.Fatalf("err: %v", err)
 	}
@@ -285,7 +285,7 @@ func TestCoordinate_Node(t *testing.T) {
 	// Query back and check the nodes are present.
 	req, _ = http.NewRequest("GET", "/v1/coordinate/node/foo?dc=dc1", nil)
 	resp = httptest.NewRecorder()
-	obj, err = a.srv.CoordinateNode(resp, req)
+	obj, err := a.srv.CoordinateNode(resp, req)
 	if err != nil {
 		t.Fatalf("err: %v", err)
 	}
@@ -303,7 +303,7 @@ func TestCoordinate_Node(t *testing.T) {
 	// Filter on a nonexistent node segment
 	req, _ = http.NewRequest("GET", "/v1/coordinate/node/foo?segment=nope", nil)
 	resp = httptest.NewRecorder()
-	obj, err = a.srv.CoordinateNode(resp, req)
+	_, err = a.srv.CoordinateNode(resp, req)
 	if err != nil {
 		t.Fatalf("err: %v", err)
 	}
@@ -331,7 +331,7 @@ func TestCoordinate_Node(t *testing.T) {
 	// Make sure the empty filter works
 	req, _ = http.NewRequest("GET", "/v1/coordinate/node/foo?segment=", nil)
 	resp = httptest.NewRecorder()
-	obj, err = a.srv.CoordinateNode(resp, req)
+	_, err = a.srv.CoordinateNode(resp, req)
 	if err != nil {
 		t.Fatalf("err: %v", err)
 	}

agent/dns_test.go

@@ -116,11 +116,11 @@ func TestRecursorAddr(t *testing.T) {
 	if addr != "[2001:4860:4860::8888]:53" {
 		t.Fatalf("bad: %v", addr)
 	}
-	addr, err = recursorAddr("1.2.3.4::53")
+	_, err = recursorAddr("1.2.3.4::53")
 	if err == nil || !strings.Contains(err.Error(), "too many colons in address") {
 		t.Fatalf("err: %v", err)
 	}
-	addr, err = recursorAddr("2001:4860:4860::8888:::53")
+	_, err = recursorAddr("2001:4860:4860::8888:::53")
 	if err == nil || !strings.Contains(err.Error(), "too many colons in address") {
 		t.Fatalf("err: %v", err)
 	}
@@ -282,7 +282,7 @@ func TestDNS_NodeLookup(t *testing.T) {
 	require.Equal(t, "127.0.0.1", aRec.A.String())
 	require.Equal(t, uint32(0), aRec.Hdr.Ttl)
 
-	txt, ok = in.Answer[1].(*dns.TXT)
+	_, ok = in.Answer[1].(*dns.TXT)
 	require.True(t, ok, "Second answer is not a TXT record")
 
 	// lookup a non-existing node, we should receive a SOA

agent/http.go

@@ -496,7 +496,7 @@ func (s *HTTPServer) wrap(handler endpoint, methods []string) http.HandlerFunc {
 				fmt.Fprint(resp, err.Error())
 			case isNotFound(err):
 				resp.WriteHeader(http.StatusNotFound)
-				fmt.Fprintf(resp, err.Error())
+				fmt.Fprint(resp, err.Error())
 			case isTooManyRequests(err):
 				resp.WriteHeader(http.StatusTooManyRequests)
 				fmt.Fprint(resp, err.Error())

agent/http_test.go

@@ -323,11 +323,11 @@ func TestHTTPAPI_Ban_Nonprintable_Characters(t *testing.T) {
 	a := NewTestAgent(t, "")
 	defer a.Shutdown()
 
-	req, err := http.NewRequest("GET", "/v1/kv/bad\x00ness", nil)
+	_, err := http.NewRequest("GET", "/v1/kv/bad\x00ness", nil)
 	if err == nil {
 		t.Fatal("expected error")
 	}
-	req, err = http.NewRequest("GET", "/v1/kv/bad%00ness", nil)
+	req, err := http.NewRequest("GET", "/v1/kv/bad%00ness", nil)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -342,11 +342,11 @@ func TestHTTPAPI_Allow_Nonprintable_Characters_With_Flag(t *testing.T) {
 	a := NewTestAgent(t, "disable_http_unprintable_char_filter = true")
 	defer a.Shutdown()
 
-	req, err := http.NewRequest("GET", "/v1/kv/bad\x00ness", nil)
+	_, err := http.NewRequest("GET", "/v1/kv/bad\x00ness", nil)
 	if err == nil {
 		t.Fatal("expected error")
 	}
-	req, err = http.NewRequest("GET", "/v1/kv/bad%00ness", nil)
+	req, err := http.NewRequest("GET", "/v1/kv/bad%00ness", nil)
 	if err != nil {
 		t.Fatal(err)
 	}

agent/kvs_endpoint_test.go

@@ -416,7 +416,7 @@ func TestKVSEndpoint_GET_Raw(t *testing.T) {
 
 	req, _ = http.NewRequest("GET", "/v1/kv/test?raw", nil)
 	resp = httptest.NewRecorder()
-	obj, err = a.srv.KVSEndpoint(resp, req)
+	_, err = a.srv.KVSEndpoint(resp, req)
 	if err != nil {
 		t.Fatalf("err: %v", err)
 	}

agent/router/manager.go

@@ -378,16 +378,17 @@ func (m *Manager) RebalanceServers() {
 			"number_of_servers", len(l.servers),
 			"active_server", l.servers[0].String(),
 		)
-	} else {
-		// reconcileServerList failed because Serf removed the server
-		// that was at the front of the list that had successfully
-		// been Ping'ed.  Between the Ping and reconcile, a Serf
-		// event had shown up removing the node.
-		//
-		// Instead of doing any heroics, "freeze in place" and
-		// continue to use the existing connection until the next
-		// rebalance occurs.
 	}
+	// else {
+	// reconcileServerList failed because Serf removed the server
+	// that was at the front of the list that had successfully
+	// been Ping'ed.  Between the Ping and reconcile, a Serf
+	// event had shown up removing the node.
+	//
+	// Instead of doing any heroics, "freeze in place" and
+	// continue to use the existing connection until the next
+	// rebalance occurs.
+	// }
 }
 
 // reconcileServerList returns true when the first server in serverList

agent/session_endpoint_test.go

@@ -14,6 +14,7 @@ import (
 	"github.com/hashicorp/consul/sdk/testutil/retry"
 	"github.com/hashicorp/consul/testrpc"
 	"github.com/hashicorp/consul/types"
+	"github.com/stretchr/testify/require"
 )
 
 func verifySession(t *testing.T, r *retry.R, a *TestAgent, want structs.Session) {
@@ -715,9 +716,11 @@ func TestSessionsForNode(t *testing.T) {
 		if !ok {
 			t.Fatalf("should work")
 		}
-		if len(respObj) != 10 {
+		respIDs := make([]string, 0, len(ids))
-			t.Fatalf("bad: %v", respObj)
+		for _, session := range respObj {
+			respIDs = append(respIDs, session.ID)
 		}
+		require.ElementsMatch(t, ids, respIDs)
 	})
 }
 

agent/ui_endpoint_test.go

@@ -36,7 +36,7 @@ func TestUiIndex(t *testing.T) {
 
 	// Create file
 	path := filepath.Join(a.Config.UIDir, "my-file")
-	if err := ioutil.WriteFile(path, []byte("test"), 777); err != nil {
+	if err := ioutil.WriteFile(path, []byte("test"), 0777); err != nil {
 		t.Fatalf("err: %v", err)
 	}
 

agent/xds/endpoints.go

@@ -3,6 +3,7 @@ package xds
 import (
 	"errors"
 	"fmt"
+
 	envoy "github.com/envoyproxy/go-control-plane/envoy/api/v2"
 	envoycore "github.com/envoyproxy/go-control-plane/envoy/api/v2/core"
 	envoyendpoint "github.com/envoyproxy/go-control-plane/envoy/api/v2/endpoint"

command/connect/envoy/envoy.go

@@ -551,7 +551,7 @@ func (c *cmd) grpcAddress(httpCfg *api.Config) (GRPC, error) {
 	} else {
 		// Parse as host:port with option http prefix
 		grpcAddr = strings.TrimPrefix(addr, "http://")
-		grpcAddr = strings.TrimPrefix(addr, "https://")
+		grpcAddr = strings.TrimPrefix(grpcAddr, "https://")
 
 		var err error
 		var host string