Enable lint check for comments (#2023)

Commit 1: Enable lint check for comments

Part of #217. Follow up from #1982 and #2018.

A subsequent commit will fix the ci failure.

Commit 2: Address all comment-related linter errors.

This change addresses all comment-related linter errors by doing the
following:
- Add comments to exported symbols
- Make some exported symbols private
- Recommend via TODOs that some exported symbols should should move or
  be removed

This PR does not:
- Modify, move, or remove any code
- Modify existing comments

Signed-off-by: Andrew Seigner <siggy@buoyant.io>

bin/lint

@@ -12,8 +12,8 @@ cd "$rootdir"
 # install golint from vendor
 go install ./vendor/golang.org/x/lint/golint
 
-# use `go list` to exclude packages in vendor, ignore uncommented warnings
-out=$(go list ./... | xargs golint | grep -v 'should have comment') || true
+# use `go list` to exclude packages in vendor
+out=$(go list ./... | xargs golint) || true
 
 if [ -n "$out" ]; then
   echo "$out"

controller/api/proxy/client.go

@@ -5,6 +5,8 @@ import (
 	"google.golang.org/grpc"
 )
 
+// NewClient creates a new gRPC client to the Destination service.
+// TODO: consider moving this into destination-client, or removing altogether.
 func NewClient(addr string) (pb.DestinationClient, *grpc.ClientConn, error) {
 	conn, err := grpc.Dial(addr, grpc.WithInsecure())
 	if err != nil {

controller/api/public/client.go

@@ -190,6 +190,8 @@ func newClient(apiURL *url.URL, httpClientToUse *http.Client, controlPlaneNamesp
 	}, nil
 }
 
+// NewInternalClient creates a new Public API client intended to run inside a
+// Kubernetes cluster.
 func NewInternalClient(controlPlaneNamespace string, kubeAPIHost string) (pb.ApiClient, error) {
 	apiURL, err := url.Parse(fmt.Sprintf("http://%s/", kubeAPIHost))
 	if err != nil {
@@ -199,6 +201,8 @@ func NewInternalClient(controlPlaneNamespace string, kubeAPIHost string) (pb.Api
 	return newClient(apiURL, http.DefaultClient, controlPlaneNamespace)
 }
 
+// NewExternalClient creates a new Public API client intended to run from
+// outside a Kubernetes cluster.
 func NewExternalClient(controlPlaneNamespace string, kubeAPI *k8s.KubernetesAPI) (pb.ApiClient, error) {
 	apiURL, err := kubeAPI.URLFor(controlPlaneNamespace, "/services/linkerd-controller-api:http/proxy/")
 	if err != nil {

controller/api/public/grpc_server.go

@@ -39,10 +39,10 @@ type podReport struct {
 
 const (
 	podQuery                   = "max(process_start_time_seconds{%s}) by (pod, namespace)"
-	K8sClientSubsystemName     = "kubernetes"
-	K8sClientCheckDescription  = "control plane can talk to Kubernetes"
-	PromClientSubsystemName    = "prometheus"
-	PromClientCheckDescription = "control plane can talk to Prometheus"
+	k8sClientSubsystemName     = "kubernetes"
+	k8sClientCheckDescription  = "control plane can talk to Kubernetes"
+	promClientSubsystemName    = "prometheus"
+	promClientCheckDescription = "control plane can talk to Prometheus"
 )
 
 func newGrpcServer(
@@ -190,8 +190,8 @@ func (s *grpcServer) ListPods(ctx context.Context, req *pb.ListPodsRequest) (*pb
 
 func (s *grpcServer) SelfCheck(ctx context.Context, in *healthcheckPb.SelfCheckRequest) (*healthcheckPb.SelfCheckResponse, error) {
 	k8sClientCheck := &healthcheckPb.CheckResult{
-		SubsystemName:    K8sClientSubsystemName,
-		CheckDescription: K8sClientCheckDescription,
+		SubsystemName:    k8sClientSubsystemName,
+		CheckDescription: k8sClientCheckDescription,
 		Status:           healthcheckPb.CheckStatus_OK,
 	}
 	_, err := s.k8sAPI.Pod().Lister().List(labels.Everything())
@@ -201,8 +201,8 @@ func (s *grpcServer) SelfCheck(ctx context.Context, in *healthcheckPb.SelfCheckR
 	}
 
 	promClientCheck := &healthcheckPb.CheckResult{
-		SubsystemName:    PromClientSubsystemName,
-		CheckDescription: PromClientCheckDescription,
+		SubsystemName:    promClientSubsystemName,
+		CheckDescription: promClientCheckDescription,
 		Status:           healthcheckPb.CheckStatus_OK,
 	}
 	_, err = s.queryProm(ctx, fmt.Sprintf(podQuery, ""))

controller/api/public/grpc_server_test.go

@@ -167,7 +167,7 @@ spec:
 			}
 
 			fakeGrpcServer := newGrpcServer(
-				&MockProm{Res: exp.promRes},
+				&mockProm{Res: exp.promRes},
 				tap.NewTapClient(nil),
 				k8sAPI,
 				"linkerd",
@@ -248,7 +248,7 @@ metadata:
 			}
 
 			fakeGrpcServer := newGrpcServer(
-				&MockProm{},
+				&mockProm{},
 				tap.NewTapClient(nil),
 				k8sAPI,
 				"linkerd",

controller/api/public/http_server.go

@@ -239,6 +239,7 @@ func fullURLPathFor(method string) string {
 	return apiRoot + apiPrefix + method
 }
 
+// NewServer creates a Public API HTTP server.
 func NewServer(
 	addr string,
 	prometheusClient promApi.Client,

controller/api/public/stat_summary_test.go

@@ -771,7 +771,7 @@ status:
 
 		for _, exp := range expectations {
 			fakeGrpcServer := newGrpcServer(
-				&MockProm{Res: exp.mockPromResponse},
+				&mockProm{Res: exp.mockPromResponse},
 				tap.NewTapClient(nil),
 				k8sAPI,
 				"linkerd",
@@ -795,7 +795,7 @@ status:
 			t.Fatalf("NewFakeAPI returned an error: %s", err)
 		}
 		fakeGrpcServer := newGrpcServer(
-			&MockProm{Res: model.Vector{}},
+			&mockProm{Res: model.Vector{}},
 			tap.NewTapClient(nil),
 			k8sAPI,
 			"linkerd",

controller/api/public/test_helper.go

@@ -18,6 +18,7 @@ import (
 	"google.golang.org/grpc"
 )
 
+// MockAPIClient satisfies the Public API's gRPC interface.
 type MockAPIClient struct {
 	ErrorToReturn                  error
 	VersionInfoToReturn            *pb.VersionInfo
@@ -30,45 +31,53 @@ type MockAPIClient struct {
 	APITapByResourceClientToReturn pb.Api_TapByResourceClient
 }
 
+// StatSummary provides a mock of a Public API method.
 func (c *MockAPIClient) StatSummary(ctx context.Context, in *pb.StatSummaryRequest, opts ...grpc.CallOption) (*pb.StatSummaryResponse, error) {
 	return c.StatSummaryResponseToReturn, c.ErrorToReturn
 }
 
+// TopRoutes provides a mock of a Public API method.
 func (c *MockAPIClient) TopRoutes(ctx context.Context, in *pb.TopRoutesRequest, opts ...grpc.CallOption) (*pb.TopRoutesResponse, error) {
 	return c.TopRoutesResponseToReturn, c.ErrorToReturn
 }
 
+// Version provides a mock of a Public API method.
 func (c *MockAPIClient) Version(ctx context.Context, in *pb.Empty, opts ...grpc.CallOption) (*pb.VersionInfo, error) {
 	return c.VersionInfoToReturn, c.ErrorToReturn
 }
 
+// ListPods provides a mock of a Public API method.
 func (c *MockAPIClient) ListPods(ctx context.Context, in *pb.ListPodsRequest, opts ...grpc.CallOption) (*pb.ListPodsResponse, error) {
 	return c.ListPodsResponseToReturn, c.ErrorToReturn
 }
 
+// ListServices provides a mock of a Public API method.
 func (c *MockAPIClient) ListServices(ctx context.Context, in *pb.ListServicesRequest, opts ...grpc.CallOption) (*pb.ListServicesResponse, error) {
 	return c.ListServicesResponseToReturn, c.ErrorToReturn
 }
 
+// Tap provides a mock of a Public API method.
 func (c *MockAPIClient) Tap(ctx context.Context, in *pb.TapRequest, opts ...grpc.CallOption) (pb.Api_TapClient, error) {
 	return c.APITapClientToReturn, c.ErrorToReturn
 }
 
+// TapByResource provides a mock of a Public API method.
 func (c *MockAPIClient) TapByResource(ctx context.Context, in *pb.TapByResourceRequest, opts ...grpc.CallOption) (pb.Api_TapByResourceClient, error) {
 	return c.APITapByResourceClientToReturn, c.ErrorToReturn
 }
 
+// SelfCheck provides a mock of a Public API method.
 func (c *MockAPIClient) SelfCheck(ctx context.Context, in *healthcheckPb.SelfCheckRequest, _ ...grpc.CallOption) (*healthcheckPb.SelfCheckResponse, error) {
 	return c.SelfCheckResponseToReturn, c.ErrorToReturn
 }
 
-type MockAPITapClient struct {
+type mockAPITapClient struct {
 	TapEventsToReturn []pb.TapEvent
 	ErrorsToReturn    []error
 	grpc.ClientStream
 }
 
-func (a *MockAPITapClient) Recv() (*pb.TapEvent, error) {
+func (a *mockAPITapClient) Recv() (*pb.TapEvent, error) {
 	var eventPopped pb.TapEvent
 	var errorPopped error
 	if len(a.TapEventsToReturn) == 0 && len(a.ErrorsToReturn) == 0 {
@@ -84,12 +93,14 @@ func (a *MockAPITapClient) Recv() (*pb.TapEvent, error) {
 	return &eventPopped, errorPopped
 }
 
+// MockAPITapByResourceClient satisfies the TapByResourceClient gRPC interface.
 type MockAPITapByResourceClient struct {
 	TapEventsToReturn []pb.TapEvent
 	ErrorsToReturn    []error
 	grpc.ClientStream
 }
 
+// Recv satisfies the TapByResourceClient.Recv() gRPC method.
 func (a *MockAPITapByResourceClient) Recv() (*pb.TapEvent, error) {
 	var eventPopped pb.TapEvent
 	var errorPopped error
@@ -110,37 +121,41 @@ func (a *MockAPITapByResourceClient) Recv() (*pb.TapEvent, error) {
 // Prometheus client
 //
 
-type MockProm struct {
+type mockProm struct {
 	Res             model.Value
 	QueriesExecuted []string // expose the queries our Mock Prometheus receives, to test query generation
 	rwLock          sync.Mutex
 }
 
+// PodCounts is a test helper struct that is used for representing data in a
+// StatTable.PodGroup.Row.
 type PodCounts struct {
 	MeshedPods  uint64
 	RunningPods uint64
 	FailedPods  uint64
 }
 
-func (m *MockProm) Query(ctx context.Context, query string, ts time.Time) (model.Value, error) {
+func (m *mockProm) Query(ctx context.Context, query string, ts time.Time) (model.Value, error) {
 	m.rwLock.Lock()
 	defer m.rwLock.Unlock()
 	m.QueriesExecuted = append(m.QueriesExecuted, query)
 	return m.Res, nil
 }
-func (m *MockProm) QueryRange(ctx context.Context, query string, r v1.Range) (model.Value, error) {
+func (m *mockProm) QueryRange(ctx context.Context, query string, r v1.Range) (model.Value, error) {
 	m.rwLock.Lock()
 	defer m.rwLock.Unlock()
 	m.QueriesExecuted = append(m.QueriesExecuted, query)
 	return m.Res, nil
 }
-func (m *MockProm) LabelValues(ctx context.Context, label string) (model.LabelValues, error) {
+func (m *mockProm) LabelValues(ctx context.Context, label string) (model.LabelValues, error) {
 	return nil, nil
 }
-func (m *MockProm) Series(ctx context.Context, matches []string, startTime time.Time, endTime time.Time) ([]model.LabelSet, error) {
+func (m *mockProm) Series(ctx context.Context, matches []string, startTime time.Time, endTime time.Time) ([]model.LabelSet, error) {
 	return nil, nil
 }
 
+// GenStatSummaryResponse generates a mock Public API StatSummaryResponse
+// object.
 func GenStatSummaryResponse(resName, resType string, resNs []string, counts *PodCounts, basicStats bool) pb.StatSummaryResponse {
 	rows := []*pb.StatTable_PodGroup_Row{}
 	for _, ns := range resNs {
@@ -192,6 +207,7 @@ func GenStatSummaryResponse(resName, resType string, resNs []string, counts *Pod
 	return resp
 }
 
+// GenTopRoutesResponse generates a mock Public API TopRoutesResponse object.
 func GenTopRoutesResponse(routes []string, counts []uint64) pb.TopRoutesResponse {
 	rows := []*pb.RouteTable_Row{}
 	for i, route := range routes {
@@ -228,13 +244,13 @@ type expectedStatRPC struct {
 	expectedPrometheusQueries []string    // queries we expect public-api to issue to prometheus
 }
 
-func newMockGrpcServer(exp expectedStatRPC) (*MockProm, *grpcServer, error) {
+func newMockGrpcServer(exp expectedStatRPC) (*mockProm, *grpcServer, error) {
 	k8sAPI, err := k8s.NewFakeAPI("", exp.k8sConfigs...)
 	if err != nil {
 		return nil, nil, err
 	}
 
-	mockProm := &MockProm{Res: exp.mockPromResponse}
+	mockProm := &mockProm{Res: exp.mockPromResponse}
 	fakeGrpcServer := newGrpcServer(
 		mockProm,
 		tap.NewTapClient(nil),
@@ -248,7 +264,7 @@ func newMockGrpcServer(exp expectedStatRPC) (*MockProm, *grpcServer, error) {
 	return mockProm, fakeGrpcServer, nil
 }
 
-func (exp expectedStatRPC) verifyPromQueries(mockProm *MockProm) error {
+func (exp expectedStatRPC) verifyPromQueries(mockProm *mockProm) error {
 	// if exp.expectedPrometheusQueries is an empty slice we still wanna check no queries were executed.
 	if exp.expectedPrometheusQueries != nil {
 		sort.Strings(exp.expectedPrometheusQueries)

controller/api/util/api_utils.go

@@ -49,7 +49,7 @@ var (
 )
 
 // StatsBaseRequestParams contains parameters that are used to build requests
-// for metrics data.  This includes requests to StatSummary and TopRoutes
+// for metrics data.  This includes requests to StatSummary and TopRoutes.
 type StatsBaseRequestParams struct {
 	TimeWindow    string
 	Namespace     string
@@ -58,6 +58,8 @@ type StatsBaseRequestParams struct {
 	AllNamespaces bool
 }
 
+// StatsSummaryRequestParams contains parameters that are used to build
+// StatSummary requests.
 type StatsSummaryRequestParams struct {
 	StatsBaseRequestParams
 	ToNamespace   string
@@ -69,12 +71,16 @@ type StatsSummaryRequestParams struct {
 	SkipStats     bool
 }
 
+// TopRoutesRequestParams contains parameters that are used to build TopRoutes
+// requests.
 type TopRoutesRequestParams struct {
 	StatsBaseRequestParams
 	To    string
 	ToAll bool
 }
 
+// TapRequestParams contains parameters that are used to build a
+// TapByResourceRequest.
 type TapRequestParams struct {
 	Resource    string
 	Namespace   string
@@ -119,6 +125,8 @@ func GRPCError(err error) error {
 	return err
 }
 
+// BuildStatSummaryRequest builds a Public API StatSummaryRequest from a
+// StatsSummaryRequestParams.
 func BuildStatSummaryRequest(p StatsSummaryRequestParams) (*pb.StatSummaryRequest, error) {
 	window := defaultMetricTimeWindow
 	if p.TimeWindow != "" {
@@ -206,6 +214,8 @@ func BuildStatSummaryRequest(p StatsSummaryRequestParams) (*pb.StatSummaryReques
 	return statRequest, nil
 }
 
+// BuildTopRoutesRequest builds a Public API TopRoutesRequest from a
+// TopRoutesRequestParams.
 func BuildTopRoutesRequest(p TopRoutesRequestParams) (*pb.TopRoutesRequest, error) {
 	window := defaultMetricTimeWindow
 	if p.TimeWindow != "" {
@@ -372,6 +382,8 @@ func buildResource(namespace string, resType string, name string) (pb.Resource,
 	}, nil
 }
 
+// BuildTapByResourceRequest builds a Public API TapByResourceRequest from a
+// TapRequestParams.
 func BuildTapByResourceRequest(params TapRequestParams) (*pb.TapByResourceRequest, error) {
 	target, err := BuildResource(params.Namespace, params.Resource)
 	if err != nil {
@@ -535,6 +547,8 @@ func routeLabels(event *pb.TapEvent) string {
 	return out
 }
 
+// RenderTapEvent renders a Public API TapEvent to a string.
+// TODO: consider moving this into cli/cmd/tap.go.
 func RenderTapEvent(event *pb.TapEvent, resource string) string {
 	dst := dst(event)
 	src := src(event)
@@ -634,6 +648,8 @@ func RenderTapEvent(event *pb.TapEvent, resource string) string {
 	}
 }
 
+// GetRequestRate calculates request rate from Public API BasicStats.
+// TODO: consider moving this into `/cli/cmd`.
 func GetRequestRate(stats *pb.BasicStats, timeWindow string) float64 {
 	success := stats.SuccessCount
 	failure := stats.FailureCount
@@ -645,6 +661,8 @@ func GetRequestRate(stats *pb.BasicStats, timeWindow string) float64 {
 	return float64(success+failure) / windowLength.Seconds()
 }
 
+// GetSuccessRate calculates success rate from Public API BasicStats.
+// TODO: consider moving this into `/cli/cmd`.
 func GetSuccessRate(stats *pb.BasicStats) float64 {
 	success := stats.SuccessCount
 	failure := stats.FailureCount
@@ -655,6 +673,9 @@ func GetSuccessRate(stats *pb.BasicStats) float64 {
 	return float64(success) / float64(success+failure)
 }
 
+// GetPercentTLS calculates the percent of traffic that is TLS, from Public API
+// BasicStats.
+// TODO: consider moving this into `/cli/cmd/stat.go`.
 func GetPercentTLS(stats *pb.BasicStats) float64 {
 	reqTotal := stats.SuccessCount + stats.FailureCount
 	if reqTotal == 0 {

controller/ca/ca.go

@@ -48,6 +48,7 @@ type CA struct {
 	nextSerialNumber uint64
 }
 
+// CertificateAndPrivateKey encapsulates a certificate / private key pair.
 type CertificateAndPrivateKey struct {
 	// The ASN.1 DER-encoded (binary, not PEM) certificate.
 	Certificate []byte