Upgrade golangci-lint to v1.18 (istio#16965)

.golangci.yml

@@ -7,7 +7,7 @@
 
 service:
   # When updating this, also update bin/linters.sh accordingly
-  golangci-lint-version: 1.16.x # use the fixed version to not introduce new linters unexpectedly
+  golangci-lint-version: 1.18.x # use the fixed version to not introduce new linters unexpectedly
 run:
   # timeout for analysis, e.g. 30s, 5m, default is 1m
   deadline: 20m
@@ -42,6 +42,8 @@ linters:
     - nakedret
     - prealloc
     - scopelint
+    - funlen
+    - bodyclose
   fast: false
 
 linters-settings:
@@ -74,6 +76,8 @@ linters-settings:
     # Default is to use a neutral variety of English.
     # Setting locale to US will correct the British spelling of 'colour' to 'color'.
     locale: US
+    ignore-words:
+    - cancelled
   lll:
     # max line length, lines longer will be reported. Default is 120.
     # '\t' is counted as 1 character by default, and can be changed with the tab-width option
@@ -142,9 +146,9 @@ linters-settings:
       - unslice
       - valSwap
       - weakCond
-      - yodaStyleExpr
 
       # Unused
+      # - yodaStyleExpr
       # - appendAssign
       # - commentFormatting
       # - emptyStringTest

galley/pkg/config/event/router.go

@@ -131,6 +131,6 @@ func AddToRouter(r Router, source collection.Name, handler Handler) Router {
 		return s
 
 	default:
-		panic(fmt.Sprintf("unkown Router: %T", v))
+		panic(fmt.Sprintf("unknown Router: %T", v))
 	}
 }

galley/pkg/config/processing/session.go

@@ -188,7 +188,7 @@ func (s *session) terminate() {
 	scope.Processing.Debug("session.terminate: stopping sources...")
 	s.options.Source.Stop()
 
-	scope.Processing.Debug("session.terminate: signalling session termination...")
+	scope.Processing.Debug("session.terminate: signaling session termination...")
 	s.mu.Lock()
 	if s.doneCh != nil {
 		close(s.doneCh)

galley/pkg/runtime/distributor.go

@@ -124,7 +124,6 @@ func (d *InMemoryDistributor) deleteListener(l *listenerEntry) {
 	for i := len(d.listeners) - 1; i >= 0; i-- {
 		if d.listeners[i] == l {
 			d.listeners = append(d.listeners[:i], d.listeners[i+1:]...)
-			l = nil
 			return
 		}
 	}

galley/pkg/runtime/publish/strategy_test.go

@@ -68,7 +68,7 @@ func TestStrategy_OnTimer(t *testing.T) {
 	default:
 	}
 	if published {
-		t.Fatal("strategy shouldn't have signalled to publish")
+		t.Fatal("strategy shouldn't have signaled to publish")
 	}
 }
 
@@ -96,7 +96,7 @@ func TestStrategy_OnTimer_Quiesce(t *testing.T) {
 	default:
 	}
 	if !published {
-		t.Fatal("strategy should have signalled to publish")
+		t.Fatal("strategy should have signaled to publish")
 	}
 }
 
@@ -126,7 +126,7 @@ func TestStrategy_OnTimer_MaxTimeout(t *testing.T) {
 		default:
 		}
 		if published {
-			t.Fatal("strategy should have not signalled to publish")
+			t.Fatal("strategy should have not signaled to publish")
 		}
 
 	}
@@ -142,7 +142,7 @@ func TestStrategy_OnTimer_MaxTimeout(t *testing.T) {
 	default:
 	}
 	if !published {
-		t.Fatal("strategy should have signalled to publish")
+		t.Fatal("strategy should have signaled to publish")
 	}
 }
 

galley/tools/gen-meta/main.go

@@ -124,7 +124,7 @@ func readMetadata(path string) (*metadata, error) {
 	var m metadata
 
 	if err = yaml.Unmarshal(b, &m); err != nil {
-		return nil, fmt.Errorf("error marshalling input file: %v", err)
+		return nil, fmt.Errorf("error marshaling input file: %v", err)
 	}
 
 	// Auto-complete listkind fields with defaults.

galley/tools/mcpc/main.go

@@ -126,7 +126,7 @@ func (u *updater) printLongChange(ch *sink.Change) {
 
 		b, err := json.MarshalIndent(o, "  ", "  ")
 		if err != nil {
-			fmt.Printf("  Marshalling error: %v", err)
+			fmt.Printf("  Marshaling error: %v", err)
 		} else {
 			fmt.Printf("%s\n", string(b))
 		}

istioctl/pkg/util/handlers/handlers_test.go

@@ -150,7 +150,7 @@ func TestHandleNamespaces(t *testing.T) {
 			}
 			if !tt.wantError {
 				if err != nil {
-					t.Fatalf("unexpected error %v occured", err)
+					t.Fatalf("unexpected error %v occurred", err)
 				}
 			}
 			if gotNs != tt.wantNamespace {

mixer/adapter/bypass/bypass.go

@@ -90,7 +90,7 @@ func (b *builder) Validate() (ce *adapter.ConfigErrors) {
 
 	anyTypes, err := b.getInferredTypes()
 	if err != nil {
-		ce = ce.Appendf("infrerred_types", "Error marshalling to any: %v", err)
+		ce = ce.Appendf("infrerred_types", "Error marshaling to any: %v", err)
 		return
 	}
 

mixer/pkg/protobuf/yaml/dynamic/handler_test.go

@@ -352,15 +352,6 @@ func TestStaticBag(t *testing.T) {
 	b.Done()
 }
 
-func TestBuildHandler_ConnectError(t *testing.T) {
-	/*
-		h, err := BuildHandler("spy", &attributeV1beta1.Connection{Address: ""}, false, []*adapter.DynamicInstance{}, false,)
-		if err != nil {
-			t.Fatalf("unable to build handler: %v", err)
-		}
-		h.Close()*/
-}
-
 func TestHandlerTimeout(t *testing.T) {
 	args := spy.DefaultArgs()
 	args.Behavior.HandleMetricResult = &v1beta1.ReportResult{}

mixer/pkg/runtime/lang/lang.go

@@ -50,7 +50,7 @@ const (
 	COMPAT
 )
 
-var langVar = env.RegisterStringVar("ISTIO_LANG", "", "Selects the attribute expression langauge runtime for Mixer.")
+var langVar = env.RegisterStringVar("ISTIO_LANG", "", "Selects the attribute expression language runtime for Mixer.")
 
 // GetLanguageRuntime reads an override from a resource annotation
 func GetLanguageRuntime(annotations map[string]string) LanguageRuntime {

pilot/pkg/config/kube/ingress/controller.go

@@ -150,9 +150,6 @@ func (c *controller) RegisterEventHandler(typ string, f func(model.Config, model
 		// TODO: This works well for Add and Delete events, but not so for Update:
 		// An updated ingress may also trigger an Add or Delete for one of its constituent sub-rules.
 		switch typ {
-		case schemas.Gateway.Type:
-			//config, _ := ConvertIngressV1alpha3(*ingress, c.domainSuffix)
-			//f(config, event)
 		case schemas.VirtualService.Type:
 			f(model.Config{}, event)
 		}

pilot/pkg/networking/core/v1alpha3/listener.go

@@ -1438,43 +1438,6 @@ func (configgen *ConfigGeneratorImpl) buildSidecarOutboundListenerForPortOrUDS(n
 	}
 }
 
-// TODO(silentdai): duplicate with listener_builder.go. Remove this one once split is verified.
-func (configgen *ConfigGeneratorImpl) generateManagementListeners(node *model.Proxy, noneMode bool,
-	env *model.Environment, listeners []*xdsapi.Listener) []*xdsapi.Listener {
-	// Do not generate any management port listeners if the user has specified a SidecarScope object
-	// with ingress listeners. Specifying the ingress listener implies that the user wants
-	// to only have those specific listeners and nothing else, in the inbound path.
-	generateManagementListeners := true
-	if node.SidecarScope.HasCustomIngressListeners || noneMode {
-		generateManagementListeners = false
-	}
-	if generateManagementListeners {
-		// Let ServiceDiscovery decide which IP and Port are used for management if
-		// there are multiple IPs
-		mgmtListeners := make([]*xdsapi.Listener, 0)
-		for _, ip := range node.IPAddresses {
-			managementPorts := env.ManagementPorts(ip)
-			management := buildSidecarInboundMgmtListeners(node, env, managementPorts, ip)
-			mgmtListeners = append(mgmtListeners, management...)
-		}
-
-		// If management listener port and service port are same, bad things happen
-		// when running in kubernetes, as the probes stop responding. So, append
-		// non overlapping listeners only.
-		for i := range mgmtListeners {
-			m := mgmtListeners[i]
-			l := util.GetByAddress(listeners, *m.Address)
-			if l != nil {
-				log.Warnf("Omitting listener for management address %s due to collision with service listener %s",
-					m.Name, l.Name)
-				continue
-			}
-			listeners = append(listeners, m)
-		}
-	}
-	return listeners
-}
-
 // onVirtualOutboundListener calls the plugin API for the outbound virtual listener
 func (configgen *ConfigGeneratorImpl) onVirtualOutboundListener(env *model.Environment,
 	node *model.Proxy,

pilot/pkg/proxy/envoy/v2/ads.go

@@ -432,7 +432,7 @@ func (s *DiscoveryServer) StreamAggregatedResources(stream ads.AggregatedDiscove
 				}
 
 				for _, cn := range clusters {
-					s.addEdsCon(cn, con.ConID, con)
+					s.getOrAddEdsCluster(cn, con.ConID, con)
 				}
 
 				con.Clusters = clusters

pilot/pkg/proxy/envoy/v2/eds.go

@@ -757,25 +757,6 @@ func hasOutlierDetection(push *model.PushContext, proxy *model.Proxy, clusterNam
 	return false
 }
 
-// addEdsCon will track the eds connection with clusters, for optimized event-based push and debug
-func (s *DiscoveryServer) addEdsCon(clusterName string, node string, connection *XdsConnection) {
-
-	s.getOrAddEdsCluster(clusterName, node, connection)
-	// TODO: left the code here so we can skip sending the already-sent clusters.
-	// See comments in ads - envoy keeps adding one cluster to the list (this seems new
-	// previous version sent all the clusters from CDS in bulk).
-
-	//c.mutex.Lock()
-	//existing := c.EdsClients[node]
-	//c.mutex.Unlock()
-	//
-	//// May replace an existing connection: this happens when Envoy adds more clusters
-	//// one by one, creating new grpc requests each time it adds one more cluster.
-	//if existing != nil {
-	//	log.Warnf("Replacing existing connection %s %s old: %s", clusterName, node, existing.ConID)
-	//}
-}
-
 // getEdsCluster returns a cluster.
 func (s *DiscoveryServer) getEdsCluster(clusterName string) *EdsCluster {
 	// separate method only to have proper lock.
@@ -784,6 +765,7 @@ func (s *DiscoveryServer) getEdsCluster(clusterName string) *EdsCluster {
 	return edsClusters[clusterName]
 }
 
+// getOrAddEdsCluster will track the eds connection with clusters, for optimized event-based push and debug
 func (s *DiscoveryServer) getOrAddEdsCluster(clusterName, node string, connection *XdsConnection) *EdsCluster {
 	edsClusterMutex.Lock()
 	defer edsClusterMutex.Unlock()

pilot/pkg/serviceregistry/consul/controller.go

@@ -290,7 +290,7 @@ func (c *Controller) getServices() (map[string][]string, error) {
 func (c *Controller) getCatalogService(name string, q *api.QueryOptions) ([]*api.CatalogService, error) {
 	endpoints, _, err := c.client.Catalog().Service(name, "", q)
 	if err != nil {
-		log.Warnf("Could not retrieve service catalogue from consul: %v", err)
+		log.Warnf("Could not retrieve service catalog from consul: %v", err)
 		return nil, err
 	}
 

pilot/pkg/serviceregistry/consul/monitor.go

@@ -123,7 +123,7 @@ func (m *consulMonitor) updateInstanceRecord() {
 	for name := range svcs {
 		endpoints, _, err := m.discovery.Catalog().Service(name, "", nil)
 		if err != nil {
-			log.Warnf("Could not retrieve service catalogue from consul: %v", err)
+			log.Warnf("Could not retrieve service catalog from consul: %v", err)
 			continue
 		}
 		instances = append(instances, endpoints...)

pkg/test/framework/components/mixer/native.go

@@ -125,24 +125,6 @@ func newNative(ctx resource.Context, config Config) (Instance, error) {
 		policyService:    client,
 	}
 
-	//// Update the mesh with the mixer address
-	//port := n.client.server.Addr().(*net.TCPAddr).Port
-	//mixerAddr := fmt.Sprintf("%s.%s:%d", localServiceName, service.FullyQualifiedDomainName, port)
-	//env.Mesh.MixerCheckServer = mixerAddr
-	//env.Mesh.MixerReportServer = mixerAddr
-	//
-	//// Add a service entry for Mixer.
-	//_, err = env.ServiceManager.Create(localServiceName, "", model.PortList{
-	//	&model.Port{
-	//		Name:     grpcPortName,
-	//		Protocol: protocol.GRPC,
-	//		Port:     port,
-	//	},
-	//})
-	if err != nil {
-		return nil, err
-	}
-
 	return n, nil
 }
 

pkg/test/framework/components/policybackend/client.go

@@ -90,7 +90,7 @@ func (c *client) ExpectReportJSON(t test.Failer, expected ...string) {
 		for _, r := range reports {
 			as, err := m.MarshalToString(r)
 			if err != nil {
-				t.Fatalf("Failed marshalling to string: %v", err)
+				t.Fatalf("Failed marshaling to string: %v", err)
 			}
 			actual = append(actual, as)
 		}

scripts/run_golangci.sh

@@ -33,8 +33,8 @@ then
 fi
 
 # if you want to update this version, also change the version number in .golangci.yml
-GOLANGCI_VERSION="v1.16.0"
+GOLANGCI_VERSION="v1.18.0"
 curl -sfL https://install.goreleaser.com/github.com/golangci/golangci-lint.sh | sh -s -- -b "$GOPATH"/bin "$GOLANGCI_VERSION"
 golangci-lint --version
 # For tuning and when switching versions PLEASE REFERENCE: https://github.com/istio/istio/issues/14888
-env GOGC=25 golangci-lint run ${FIX} -j 8 -v ./...
+env GOGC=25 golangci-lint run ${FIX} -v ./...

tests/integration/pilot/locality/main_test.go

@@ -240,15 +240,10 @@ func sendTraffic(from echo.Instance, host string) error {
 	if len(resp) != sendCount {
 		return fmt.Errorf("%s->%s expected %d responses, received %d", from.Config().Service, host, sendCount, len(resp))
 	}
-	numFailed := 0
 	for i, r := range resp {
 		if match := bHostnameMatcher.FindString(r.Hostname); len(match) == 0 {
-			numFailed++
 			return fmt.Errorf("%s->%s request[%d] made to unexpected service: %s", from.Config().Service, host, i, r.Hostname)
 		}
 	}
-	if numFailed > 0 {
-		return fmt.Errorf("%s->%s total requests to unexpected service=%d/%d", from.Config().Service, host, numFailed, len(resp))
-	}
 	return nil
 }

tests/integration/security/sds_ingress/util/util.go

@@ -310,7 +310,7 @@ func WaitUntilGatewaySdsStatsGE(t *testing.T, ing ingress.Instance, expectedUpda
 	var err error
 	for {
 		if time.Since(start) > timeout {
-			return fmt.Errorf("sds stats does not meet expection in %v: Expected %v, Last stats: %v",
+			return fmt.Errorf("sds stats does not meet expectation in %v: Expected %v, Last stats: %v",
 				timeout, expectedUpdates, sdsUpdates)
 		}
 		sdsUpdates, err = GetStatsByName(t, ing, "listener.0.0.0.0_443.server_ssl_socket_factory.ssl_context_update_by_sds")
@@ -334,7 +334,7 @@ func WaitUntilGatewayActiveListenerStatsGE(t *testing.T, ing ingress.Instance, e
 	var err error
 	for {
 		if time.Since(start) > timeout {
-			return fmt.Errorf("active listener stats does not meet expection in %v: Expected %v, "+
+			return fmt.Errorf("active listener stats does not meet expectation in %v: Expected %v, "+
 				"Last stats: %v", timeout, expectedListeners, activeListeners)
 		}
 		activeListeners, err = GetStatsByName(t, ing, "listener_manager.total_listeners_active")

tests/util/kube_utils.go

@@ -600,8 +600,6 @@ func PodExec(n, pod, container, command string, muteOutput bool, kubeconfig stri
 
 // CreateTLSSecret creates a secret from the provided cert and key files
 func CreateTLSSecret(secretName, n, keyFile, certFile string, kubeconfig string) (string, error) {
-	//cmd := fmt.Sprintf("kubectl create secret tls %s -n %s --key %s --cert %s", secretName, n, keyFile, certFile)
-	//return Shell(cmd)
 	return Shell("kubectl create secret tls %s -n %s --key %s --cert %s --kubeconfig=%s", secretName, n, keyFile, certFile, kubeconfig)
 }