enable staticcheck (linkerd#8037)

Closes linkerd#7881 This makes the rest of the necessary fixes to satisfy the `staticcheck` lint. The only class of lints that are being skipped are those related to deprecated tap code. There was some discussion on the original change started by @adleong about if this _actually_ deprecated [here](linkerd#3240 (comment)); it doesn't look like we every came back around to fully removing it but I don't think it should be a blocker for enabling the lint right now. Signed-off-by: Kevin Leimkuhler <[email protected]>
yc185050 · Mar 10, 2022 · fc2032f · fc2032f
1 parent 310ef6b
commit fc2032f
Show file tree

Hide file tree

Showing 20 changed files with 29 additions and 26 deletions.
diff --git a/.golangci.yml b/.golangci.yml
@@ -27,6 +27,7 @@ linters:
   - misspell
   - nakedret
   - revive
+  - staticcheck
   - typecheck
   - unconvert
   - unparam
@@ -43,7 +44,6 @@ linters:
   # - prealloc
   # - stylecheck
   disable:
-  - staticcheck
   - structcheck
 
 issues:

diff --git a/cli/cmd/identity.go b/cli/cmd/identity.go
@@ -195,7 +195,7 @@ func getCertResponse(url string, pod corev1.Pod) ([]*x509.Certificate, error) {
 	if err != nil {
 		return nil, err
 	}
-	connURL := strings.Trim(url, "http://")
+	connURL := strings.TrimPrefix(url, "http://")
 	conn, err := tls.Dial("tcp", connURL, &tls.Config{
 		// We want to connect directly to a proxy port to dump its certificate. We don't necessarily
 		// want to verify the server's certificate, since this is purely for diagnostics and may be

diff --git a/controller/api/destination/client.go b/controller/api/destination/client.go
@@ -7,6 +7,7 @@ import (
 	"github.com/linkerd/linkerd2/pkg/k8s"
 	"go.opencensus.io/plugin/ocgrpc"
 	"google.golang.org/grpc"
+	"google.golang.org/grpc/credentials/insecure"
 )
 
 const (
@@ -17,7 +18,7 @@ const (
 // NewClient creates a client for the control plane Destination API that
 // implements the Destination service.
 func NewClient(addr string) (pb.DestinationClient, *grpc.ClientConn, error) {
-	conn, err := grpc.Dial(addr, grpc.WithInsecure(), grpc.WithStatsHandler(&ocgrpc.ClientHandler{}))
+	conn, err := grpc.Dial(addr, grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithStatsHandler(&ocgrpc.ClientHandler{}))
 	if err != nil {
 		return nil, nil, err
 	}

diff --git a/controller/api/destination/profile_translator_test.go b/controller/api/destination/profile_translator_test.go
@@ -3,12 +3,12 @@ package destination
 import (
 	"testing"
 
-	"github.com/golang/protobuf/proto"
 	"github.com/golang/protobuf/ptypes/duration"
 	pb "github.com/linkerd/linkerd2-proxy-api/go/destination"
 	httpPb "github.com/linkerd/linkerd2-proxy-api/go/http_types"
 	sp "github.com/linkerd/linkerd2/controller/gen/apis/serviceprofile/v1alpha2"
 	logging "github.com/sirupsen/logrus"
+	"google.golang.org/protobuf/proto"
 )
 
 var (

diff --git a/pkg/addr/addr_test.go b/pkg/addr/addr_test.go
@@ -4,10 +4,10 @@ import (
 	"fmt"
 	"testing"
 
-	"github.com/golang/protobuf/proto"
 	pb "github.com/linkerd/linkerd2-proxy-api/go/net"
 	proxy "github.com/linkerd/linkerd2-proxy-api/go/net"
 	l5dNetPb "github.com/linkerd/linkerd2/controller/gen/common/net"
+	"google.golang.org/protobuf/proto"
 )
 
 func TestPublicAddressToString(t *testing.T) {

diff --git a/pkg/identity/service.go b/pkg/identity/service.go
@@ -15,13 +15,13 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 
-	"github.com/golang/protobuf/ptypes"
 	pb "github.com/linkerd/linkerd2-proxy-api/go/identity"
 	"github.com/linkerd/linkerd2/pkg/tls"
 	log "github.com/sirupsen/logrus"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
+	"google.golang.org/protobuf/types/known/timestamppb"
 )
 
 const (
@@ -237,12 +237,7 @@ func (svc *Service) Certify(ctx context.Context, req *pb.CertifyRequest) (*pb.Ce
 		//nolint:gocritic
 		log.Fatal("the issuer provided a certificate without key material")
 	}
-
-	validUntil, err := ptypes.TimestampProto(crt.Certificate.NotAfter)
-	if err != nil {
-		log.Errorf("invalid expiry time: %s", err)
-		return nil, status.Error(codes.Internal, err.Error())
-	}
+	validUntil := timestamppb.New(crt.Certificate.NotAfter)
 
 	hasher := sha256.New()
 	hasher.Write(crts[0])

diff --git a/pkg/protohttp/protohttp.go b/pkg/protohttp/protohttp.go
@@ -8,12 +8,12 @@ import (
 	"io"
 	"net/http"
 
-	"github.com/golang/protobuf/proto"
 	"github.com/linkerd/linkerd2/pkg/k8s"
 	"github.com/linkerd/linkerd2/pkg/util"
 	metricsPb "github.com/linkerd/linkerd2/viz/metrics-api/gen/viz"
 	log "github.com/sirupsen/logrus"
 	"google.golang.org/grpc/status"
+	"google.golang.org/protobuf/proto"
 	kerrors "k8s.io/apimachinery/pkg/api/errors"
 )
 

diff --git a/pkg/protohttp/protohttp_test.go b/pkg/protohttp/protohttp_test.go
@@ -13,10 +13,10 @@ import (
 	"strings"
 	"testing"
 
-	"github.com/golang/protobuf/proto"
 	metricsPb "github.com/linkerd/linkerd2/viz/metrics-api/gen/viz"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
+	"google.golang.org/protobuf/proto"
 	kerrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 )

diff --git a/viz/cmd/top.go b/viz/cmd/top.go
@@ -11,7 +11,6 @@ import (
 	"strings"
 	"time"
 
-	"github.com/golang/protobuf/ptypes"
 	"github.com/linkerd/linkerd2/pkg/addr"
 	pkgcmd "github.com/linkerd/linkerd2/pkg/cmd"
 	"github.com/linkerd/linkerd2/pkg/healthcheck"
@@ -597,10 +596,12 @@ func newRow(req topRequest) (tableRow, error) {
 		destination = pod
 	}
 
-	latency, err := ptypes.Duration(req.rspEnd.GetSinceRequestInit())
+	err := req.rspEnd.GetSinceRequestInit().CheckValid()
 	if err != nil {
 		return tableRow{}, fmt.Errorf("error parsing duration %v: %w", req.rspEnd.GetSinceRequestInit(), err)
 	}
+	latency := req.rspEnd.GetSinceRequestInit().AsDuration()
+
 	// TODO: Once tap events have a classification field, we should use that field
 	// instead of determining success here.
 	success := req.rspInit.GetHttpStatus() < 500

diff --git a/viz/metrics-api/client/client.go b/viz/metrics-api/client/client.go
@@ -8,13 +8,13 @@ import (
 	"net/http"
 	"net/url"
 
-	"github.com/golang/protobuf/proto"
 	"github.com/linkerd/linkerd2/pkg/k8s"
 	"github.com/linkerd/linkerd2/pkg/protohttp"
 	pb "github.com/linkerd/linkerd2/viz/metrics-api/gen/viz"
 	log "github.com/sirupsen/logrus"
 	"go.opencensus.io/plugin/ochttp"
 	"google.golang.org/grpc"
+	"google.golang.org/protobuf/proto"
 )
 
 const (

diff --git a/viz/metrics-api/edges_test.go b/viz/metrics-api/edges_test.go
@@ -6,10 +6,10 @@ import (
 	"fmt"
 	"testing"
 
-	"github.com/golang/protobuf/proto"
 	pkgK8s "github.com/linkerd/linkerd2/pkg/k8s"
 	pb "github.com/linkerd/linkerd2/viz/metrics-api/gen/viz"
 	"github.com/prometheus/common/model"
+	"google.golang.org/protobuf/proto"
 )
 
 const (

diff --git a/viz/metrics-api/http_server_test.go b/viz/metrics-api/http_server_test.go
@@ -7,9 +7,9 @@ import (
 	"net/http"
 	"testing"
 
-	"github.com/golang/protobuf/proto"
 	vizClient "github.com/linkerd/linkerd2/viz/metrics-api/client"
 	pb "github.com/linkerd/linkerd2/viz/metrics-api/gen/viz"
+	"google.golang.org/protobuf/proto"
 )
 
 type mockServer struct {

diff --git a/viz/metrics-api/stat_summary.go b/viz/metrics-api/stat_summary.go
@@ -7,12 +7,12 @@ import (
 	"sort"
 	"strings"
 
-	proto "github.com/golang/protobuf/proto"
 	"github.com/linkerd/linkerd2/pkg/k8s"
 	pb "github.com/linkerd/linkerd2/viz/metrics-api/gen/viz"
 	vizutil "github.com/linkerd/linkerd2/viz/pkg/util"
 	"github.com/prometheus/common/model"
 	log "github.com/sirupsen/logrus"
+	"google.golang.org/protobuf/proto"
 	corev1 "k8s.io/api/core/v1"
 	kerrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"

diff --git a/viz/metrics-api/stat_summary_test.go b/viz/metrics-api/stat_summary_test.go
@@ -6,12 +6,12 @@ import (
 	"sort"
 	"testing"
 
-	"github.com/golang/protobuf/proto"
 	"github.com/linkerd/linkerd2/controller/k8s"
 	pkgK8s "github.com/linkerd/linkerd2/pkg/k8s"
 	"github.com/linkerd/linkerd2/pkg/prometheus"
 	pb "github.com/linkerd/linkerd2/viz/metrics-api/gen/viz"
 	"github.com/prometheus/common/model"
+	"google.golang.org/protobuf/proto"
 )
 
 type statSumExpected struct {

diff --git a/viz/metrics-api/top_routes_test.go b/viz/metrics-api/top_routes_test.go
@@ -7,10 +7,10 @@ import (
 	"sort"
 	"testing"
 
-	"github.com/golang/protobuf/proto"
 	pkgK8s "github.com/linkerd/linkerd2/pkg/k8s"
 	pb "github.com/linkerd/linkerd2/viz/metrics-api/gen/viz"
 	"github.com/prometheus/common/model"
+	"google.golang.org/protobuf/proto"
 )
 
 // deployment/books

diff --git a/viz/tap/api/client.go b/viz/tap/api/client.go
@@ -3,11 +3,12 @@ package api
 import (
 	pb "github.com/linkerd/linkerd2/viz/tap/gen/tap"
 	"google.golang.org/grpc"
+	"google.golang.org/grpc/credentials/insecure"
 )
 
 // NewClient creates a client for the control-plane's Tap service.
 func NewClient(addr string) (pb.TapClient, *grpc.ClientConn, error) {
-	conn, err := grpc.Dial(addr, grpc.WithInsecure())
+	conn, err := grpc.Dial(addr, grpc.WithTransportCredentials(insecure.NewCredentials()))
 	if err != nil {
 		return nil, nil, err
 	}

diff --git a/viz/tap/api/grpc_server.go b/viz/tap/api/grpc_server.go
@@ -26,6 +26,7 @@ import (
 	log "github.com/sirupsen/logrus"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/credentials/insecure"
 	"google.golang.org/grpc/metadata"
 	"google.golang.org/grpc/status"
 	corev1 "k8s.io/api/core/v1"
@@ -50,6 +51,8 @@ var (
 )
 
 // Tap is deprecated, use TapByResource.
+// This API endpoint is marked as deprecated but it's still used.
+//nolint:staticcheck
 func (s *GRPCTapServer) Tap(req *tapPb.TapRequest, stream tapPb.Tap_TapServer) error {
 	return status.Error(codes.Unimplemented, "Tap is deprecated, use TapByResource")
 }
@@ -300,7 +303,7 @@ func buildExtractHTTP(extract *tapPb.TapByResourceRequest_Extract_Http) *proxy.O
 func (s *GRPCTapServer) tapProxy(ctx context.Context, maxRps float32, match *proxy.ObserveRequest_Match, extract *proxy.ObserveRequest_Extract, addr string, events chan *tapPb.TapEvent) {
 	tapAddr := fmt.Sprintf("%s:%d", addr, s.tapPort)
 	log.Infof("Establishing tap on %s", tapAddr)
-	conn, err := grpc.DialContext(ctx, tapAddr, grpc.WithInsecure())
+	conn, err := grpc.DialContext(ctx, tapAddr, grpc.WithTransportCredentials(insecure.NewCredentials()))
 	if err != nil {
 		log.Error(err)
 		return

diff --git a/viz/tap/api/handlers.go b/viz/tap/api/handlers.go
@@ -171,6 +171,8 @@ func (h *handler) handleTap(w http.ResponseWriter, req *http.Request, p httprout
 	}
 
 	serverStream := serverStream{w: flushableWriter, req: req, log: h.log}
+	// This API endpoint is marked as deprecated but it's still used.
+	//nolint:staticcheck
 	err = h.grpcTapServer.TapByResource(&tapReq, &serverStream)
 	if err != nil {
 		h.log.Error(err)

diff --git a/viz/tap/pkg/reader.go b/viz/tap/pkg/reader.go
@@ -9,11 +9,11 @@ import (
 	"net/http"
 	"net/url"
 
-	"github.com/golang/protobuf/proto"
 	"github.com/linkerd/linkerd2/pkg/k8s"
 	"github.com/linkerd/linkerd2/pkg/protohttp"
 	pb "github.com/linkerd/linkerd2/viz/tap/gen/tap"
 	log "github.com/sirupsen/logrus"
+	"google.golang.org/protobuf/proto"
 )
 
 const (

diff --git a/web/srv/api_handlers.go b/web/srv/api_handlers.go
@@ -304,7 +304,7 @@ func (h *handler) handleAPITap(w http.ResponseWriter, req *http.Request, p httpr
 
 			json, err := pbMarshaler.Marshal(&event)
 			if err != nil {
-				websocketError(ws, websocket.CloseInternalServerErr, err)
+				websocketError(ws, websocket.CloseUnsupportedData, err)
 				break
 			}
 			buf := new(bytes.Buffer)