Enable more linters. (istio#12751)

- Flip on a couple more linters

- Fix a bazzilion warnings produced by these linters,
along with many warnings produced by other not-yet-enabled
linters.

- Fix pkg/version so the tests compile on Mac. This broke a while
back, preventing the linter from running to completion on the Mac.

.golangci.yml

@@ -116,10 +116,12 @@ linters:
     - gosimple
     - interfacer
     - lll
+    - maligned
     - misspell
     - staticcheck
     - structcheck
     - ineffassign
+    - typecheck
     - unconvert
     - unparam
     - unused
@@ -128,7 +130,6 @@ linters:
   disable:
     - gocritic
     - prealloc
-    - typecheck
     - megacheck
     - nakedret
     - scopelint
@@ -138,7 +139,6 @@ linters:
       # start here to enable back all gometalinter linters
     - errcheck
     - goconst
-    - maligned
   disable-all: false
   presets:
     - bugs

galley/pkg/crd/validation/webhook_test.go

@@ -537,9 +537,9 @@ func TestServe(t *testing.T) {
 		name            string
 		body            []byte
 		contentType     string
-		wantAllowed     bool
 		wantStatusCode  int
-		allowedResponse bool //
+		wantAllowed     bool
+		allowedResponse bool
 	}{
 		{
 			name:            "valid",

galley/pkg/server/args.go

@@ -43,27 +43,18 @@ type Args struct {
 	// Address to use for Galley's gRPC API.
 	APIAddress string
 
-	// Enables gRPC-level tracing
-	EnableGRPCTracing bool
-
 	// Maximum size of individual received gRPC messages
 	MaxReceivedMessageSize uint
 
 	// Maximum number of outstanding RPCs per connection
 	MaxConcurrentStreams uint
 
-	// Insecure gRPC service is used for the MCP server. CertificateFile and KeyFile is ignored.
-	Insecure bool
-
 	// The credential options to use for MCP.
 	CredentialOptions *creds.Options
 
 	// The introspection options to use
 	IntrospectionOptions *ctrlz.Options
 
-	// Enable galley server mode
-	EnableServer bool
-
 	// AccessListFile is the YAML file that specifies ids of the allowed mTLS peers.
 	AccessListFile string
 
@@ -79,11 +70,6 @@ type Args struct {
 	// DNS Domain suffix to use while constructing Ingress based resources.
 	DomainSuffix string
 
-	// DisableResourceReadyCheck disables the CRD readiness check. This
-	// allows Galley to start when not all supported CRD are
-	// registered with the kube-apiserver.
-	DisableResourceReadyCheck bool
-
 	// SinkAddress should be set to the address of a MCP Resource
 	// Sink service that Galley will dial out to. Leaving empty disables
 	// sink.
@@ -96,6 +82,20 @@ type Args struct {
 	// SinkMeta list of key=values to attach as gRPC stream metadata to
 	// outgoing Sink connections.
 	SinkMeta []string
+
+	// Enables gRPC-level tracing
+	EnableGRPCTracing bool
+
+	// Insecure gRPC service is used for the MCP server. CertificateFile and KeyFile is ignored.
+	Insecure bool
+
+	// Enable galley server mode
+	EnableServer bool
+
+	// DisableResourceReadyCheck disables the CRD readiness check. This
+	// allows Galley to start when not all supported CRD are
+	// registered with the kube-apiserver.
+	DisableResourceReadyCheck bool
 }
 
 // DefaultArgs allocates an Args struct initialized with Mixer's default configuration.

galley/pkg/server/callout_test.go

@@ -36,19 +36,19 @@ func TestCallout(t *testing.T) {
 	if len(co.metadata) != 2 || co.metadata[0] != "foo" || co.metadata[1] != "bar" {
 		t.Errorf("Callout meta not set: %v", co.metadata)
 	}
-	co, err = newCallout("foo", "NONE", []string{"foo"}, &source.Options{})
+	_, err = newCallout("foo", "NONE", []string{"foo"}, &source.Options{})
 	if err == nil {
 		t.Error("did not error with malformed metadata, no =")
 	}
-	co, err = newCallout("foo", "NONE", []string{"foo="}, &source.Options{})
+	_, err = newCallout("foo", "NONE", []string{"foo="}, &source.Options{})
 	if err == nil {
 		t.Error("did not error with malformed metadata, no value")
 	}
-	co, err = newCallout("foo", "NONE", []string{"=foo"}, &source.Options{})
+	_, err = newCallout("foo", "NONE", []string{"=foo"}, &source.Options{})
 	if err == nil {
 		t.Error("did not error with malformed metadata, no key")
 	}
-	co, err = newCallout("foo", "NONE", []string{"="}, &source.Options{})
+	_, err = newCallout("foo", "NONE", []string{"="}, &source.Options{})
 	if err == nil {
 		t.Error("did not error with malformed metadata, no key or value")
 	}

galley/pkg/server/server.go

@@ -157,8 +157,7 @@ func newServer(a *Args, p patchTable) (*Server, error) {
 	grpcOptions = append(grpcOptions, grpc.MaxRecvMsgSize(int(a.MaxReceivedMessageSize)))
 
 	s.stopCh = make(chan struct{})
-	var checker server.AuthChecker
-	checker = server.NewAllowAllChecker()
+	var checker server.AuthChecker = server.NewAllowAllChecker()
 	if !a.Insecure {
 		checker, err = watchAccessList(s.stopCh, a.AccessListFile)
 		if err != nil {

galley/pkg/source/fs/source.go

@@ -90,7 +90,7 @@ func (s *source) readFiles(root string) map[fileResourceKey]*fileResource {
 			return err
 		}
 		result := s.readFile(path, info, true)
-		if result != nil && len(result) != 0 {
+		if len(result) != 0 {
 			for _, r := range result {
 				results[r.newKey()] = r
 			}
@@ -303,32 +303,25 @@ func (s *source) Start(handler resource.EventHandler) error {
 					fi, err := os.Stat(ev.Name)
 					if err != nil {
 						log.Scope.Warnf("error occurs for watching %s", ev.Name)
-					} else {
-						if fi.Mode().IsDir() {
-							log.Scope.Debugf("add watcher for new folder %s", ev.Name)
-							_ = s.watcher.Watch(ev.Name)
-						} else {
-							newData := s.readFile(ev.Name, fi, true)
-							if newData != nil && len(newData) != 0 {
-								s.processAddOrUpdate(ev.Name, newData)
-							}
-						}
+					} else if fi.Mode().IsDir() {
+						log.Scope.Debugf("add watcher for new folder %s", ev.Name)
+						_ = s.watcher.Watch(ev.Name)
+					} else if newData := s.readFile(ev.Name, fi, true); len(newData) != 0 {
+						s.processAddOrUpdate(ev.Name, newData)
 					}
 				} else if ev.IsModify() {
 					fi, err := os.Stat(ev.Name)
 					if err != nil {
 						log.Scope.Warnf("error occurs for watching %s", ev.Name)
+					} else if fi.Mode().IsDir() {
+						_ = s.watcher.RemoveWatch(ev.Name)
 					} else {
-						if fi.Mode().IsDir() {
-							_ = s.watcher.RemoveWatch(ev.Name)
+						newData := s.readFile(ev.Name, fi, false)
+						if len(newData) != 0 {
+							s.processPartialDelete(ev.Name, newData)
+							s.processAddOrUpdate(ev.Name, newData)
 						} else {
-							newData := s.readFile(ev.Name, fi, false)
-							if newData != nil && len(newData) != 0 {
-								s.processPartialDelete(ev.Name, newData)
-								s.processAddOrUpdate(ev.Name, newData)
-							} else {
-								s.processDelete(ev.Name)
-							}
+							s.processDelete(ev.Name)
 						}
 					}
 				}

galley/pkg/source/kube/dynamic/converter/converter_test.go

@@ -527,8 +527,8 @@ func TestKubeIngressResource(t *testing.T) {
 func TestShouldProcessIngress(t *testing.T) {
 	istio := model.DefaultMeshConfig().IngressClass
 	cases := []struct {
-		ingressMode   meshcfg.MeshConfig_IngressControllerMode
 		ingressClass  string
+		ingressMode   meshcfg.MeshConfig_IngressControllerMode
 		shouldProcess bool
 	}{
 		{ingressMode: meshcfg.MeshConfig_DEFAULT, ingressClass: "nginx", shouldProcess: false},

galley/pkg/source/kube/schema/check/check_test.go

@@ -119,7 +119,6 @@ func TestFindSupportedResourceSchemas(t *testing.T) {
 	cases := []struct {
 		name    string
 		missing map[int]bool
-		want    map[int]bool
 	}{
 		{
 			name: "all present",

istioctl/cmd/istioctl/handlers_test.go

@@ -15,7 +15,6 @@
 package main
 
 import (
-	"fmt"
 	"strings"
 	"testing"
 )
@@ -47,7 +46,7 @@ func TestInferPodInfo(t *testing.T) {
 		},
 	}
 	for _, tt := range tests {
-		t.Run(fmt.Sprintf("%s", strings.Split(tt.proxyName, ".")[0]), func(t *testing.T) {
+		t.Run(strings.Split(tt.proxyName, ".")[0], func(t *testing.T) {
 			gotPodName, gotNamespace := inferPodInfo(tt.proxyName, tt.namespace)
 			if gotPodName != tt.wantPodName || gotNamespace != tt.wantNamespace {
 				t.Errorf("unexpected podName and namespace: wanted %v %v got %v %v", tt.wantPodName, tt.wantNamespace, gotPodName, gotNamespace)

istioctl/pkg/util/configdump/cluster_test.go

@@ -34,10 +34,10 @@ func setupWrapper(t *testing.T) *Wrapper {
 func TestWrapper_GetClusterConfigDump(t *testing.T) {
 	tests := []struct {
 		name                    string
-		noConfigs               bool
-		noCluster               bool
 		wantVersion             string
 		wantStatic, wantDynamic int
+		noConfigs               bool
+		noCluster               bool
 		wantErr                 bool
 	}{
 		{
@@ -89,9 +89,9 @@ func TestWrapper_GetClusterConfigDump(t *testing.T) {
 func TestWrapper_GetDynamicClusterDump(t *testing.T) {
 	tests := []struct {
 		name                                string
+		wantStatic, wantDynamic             int
 		noCluster                           bool
 		stripVersion, wantVersion, wantLast bool
-		wantStatic, wantDynamic             int
 		wantErr                             bool
 	}{
 		{

istioctl/pkg/util/configdump/listener_test.go

@@ -23,10 +23,10 @@ import (
 func TestWrapper_GetListenerConfigDump(t *testing.T) {
 	tests := []struct {
 		name                    string
-		noConfigs               bool
-		noListener              bool
 		wantVersion             string
 		wantStatic, wantDynamic int
+		noConfigs               bool
+		noListener              bool
 		wantErr                 bool
 	}{
 		{
@@ -79,9 +79,9 @@ func TestWrapper_GetListenerConfigDump(t *testing.T) {
 func TestWrapper_GetDynamicListenerDump(t *testing.T) {
 	tests := []struct {
 		name                                string
+		wantStatic, wantDynamic             int
 		noListener                          bool
 		stripVersion, wantVersion, wantLast bool
-		wantStatic, wantDynamic             int
 		wantErr                             bool
 	}{
 		{

istioctl/pkg/util/configdump/route_test.go

@@ -23,9 +23,9 @@ import (
 func TestWrapper_GetRouteConfigDump(t *testing.T) {
 	tests := []struct {
 		name                    string
+		wantStatic, wantDynamic int
 		noConfigs               bool
 		noRoute                 bool
-		wantStatic, wantDynamic int
 		wantErr                 bool
 	}{
 		{
@@ -74,9 +74,9 @@ func TestWrapper_GetRouteConfigDump(t *testing.T) {
 func TestWrapper_GetDynamicRouteDump(t *testing.T) {
 	tests := []struct {
 		name                                string
+		wantStatic, wantDynamic             int
 		noRoute                             bool
 		stripVersion, wantVersion, wantLast bool
-		wantStatic, wantDynamic             int
 		wantErr                             bool
 	}{
 		{

istioctl/pkg/validate/validate_test.go

@@ -202,8 +202,6 @@ func TestValidateCommand(t *testing.T) {
 	cases := []struct {
 		name      string
 		args      []string
-		stdin     *bytes.Buffer
-		in        []string
 		wantError bool
 	}{
 		{

istioctl/pkg/writer/compare/cluster_test.go

@@ -28,7 +28,6 @@ func TestComparator_ClusterDiff(t *testing.T) {
 		envoy           []byte
 		pilot           map[string][]byte
 		wantClusterDump bool
-		wantMatch       bool
 		wantDiff        string
 	}{
 		{

istioctl/pkg/writer/compare/listener_test.go

@@ -27,9 +27,8 @@ func TestComparator_ListenerDiff(t *testing.T) {
 		name             string
 		envoy            []byte
 		pilot            map[string][]byte
-		wantListenerDump bool
-		wantMatch        bool
 		wantDiff         string
+		wantListenerDump bool
 	}{
 		{
 			name:     "prints a diff",

istioctl/pkg/writer/compare/route_test.go

@@ -28,9 +28,8 @@ func TestComparator_RouteDiff(t *testing.T) {
 		name          string
 		envoy         []byte
 		pilot         map[string][]byte
-		wantRouteDump bool
-		wantMatch     bool
 		wantDiff      string
+		wantRouteDump bool
 	}{
 		{
 			name:     "prints a diff",