changes for gocritic linter (clusterlink-net#269)

* changes for gocritic
* enable gocritic features

---------

Signed-off-by: Etai Lev Ran <[email protected]>

.golangci.yaml

@@ -76,10 +76,10 @@ linters-settings:
   gocritic:
     enabled-tags:
       - diagnostic
-      # - experimental
-      # - opinionated
+      - experimental
+      - opinionated
       - performance
-      # - style
+      - style
 
   errcheck:
     # change some error checks which are disabled by default

cmd/cl-controlplane/app/server.go

@@ -129,21 +129,21 @@ func (o *Options) Run() error {
 
 	storeManager := kv.NewManager(kvStore)
 
-	// initiailze platform
-	var platform platform.Platform
+	// initialize platform
+	var pp platform.Platform
 	switch o.Platform {
 	case platformUnknown:
-		platform = unknown.NewPlatform()
+		pp = unknown.NewPlatform()
 	case platformK8S:
-		platform, err = k8s.NewPlatform()
+		pp, err = k8s.NewPlatform()
 		if err != nil {
 			return err
 		}
 	default:
 		return fmt.Errorf("unknown platform type: %s", o.Platform)
 	}
 
-	cp, err := controlplane.NewInstance(parsedCertData, storeManager, platform)
+	cp, err := controlplane.NewInstance(parsedCertData, storeManager, pp)
 	if err != nil {
 		return err
 	}

pkg/controlplane/client.go

@@ -133,11 +133,9 @@ func (c *client) getHeartbeat() error {
 	var retErr error
 	// copy peer clients array aside
 	peerClients := make([]*jsonapi.Client, len(c.clients))
-	{
-		c.lock.RLock()
-		defer c.lock.RUnlock()
-		copy(peerClients, c.clients)
-	}
+	c.lock.RLock()
+	copy(peerClients, c.clients)
+	c.lock.RUnlock()
 
 	for _, client := range peerClients {
 		serverResp, err := client.Get(api.HeartbeatPath)

pkg/controlplane/instance.go

@@ -641,7 +641,7 @@ func (cp *Instance) generateJWK() error {
 }
 
 // NewInstance returns a new controlplane instance.
-func NewInstance(peerTLS *util.ParsedCertData, storeManager store.Manager, platform platform.Platform) (*Instance, error) {
+func NewInstance(peerTLS *util.ParsedCertData, storeManager store.Manager, pp platform.Platform) (*Instance, error) {
 	logger := logrus.WithField("component", "controlplane")
 
 	peers, err := cpstore.NewPeers(storeManager)
@@ -692,7 +692,7 @@ func NewInstance(peerTLS *util.ParsedCertData, storeManager store.Manager, platf
 		xdsManager:    newXDSManager(),
 		ports:         newPortManager(),
 		policyDecider: policyengine.NewPolicyHandler(),
-		platform:      platform,
+		platform:      pp,
 		initialized:   false,
 		logger:        logger,
 	}

pkg/controlplane/server/http/api.go

@@ -108,7 +108,7 @@ func peerToAPI(peer *store.Peer) *api.Peer {
 	return &api.Peer{
 		Name:   peer.Name,
 		Spec:   peer.PeerSpec,
-		Status: api.PeerStatus{}, // TODO
+		Status: api.PeerStatus{},
 	}
 }
 
@@ -252,8 +252,7 @@ func importToAPI(imp *store.Import) *api.Import {
 		Name: imp.Name,
 		Spec: imp.ImportSpec,
 		Status: api.ImportStatus{
-			Listener: api.Endpoint{
-				Host: "", // TODO
+			Listener: api.Endpoint{ // Endpoint.Host is not set
 				Port: imp.Port,
 			},
 		},

pkg/dataplane/client/fetcher.go

@@ -93,16 +93,16 @@ func (f *fetcher) Run() error {
 	}
 }
 
-func newFetcher(ctx context.Context, conn *grpc.ClientConn, resourceType string, dataplane *server.Dataplane) (*fetcher, error) {
-	client := client.NewADSClient(ctx, &core.Node{Id: dataplane.ID}, resourceType)
-	err := client.InitConnect(conn)
+func newFetcher(ctx context.Context, conn *grpc.ClientConn, resourceType string, dp *server.Dataplane) (*fetcher, error) {
+	cl := client.NewADSClient(ctx, &core.Node{Id: dp.ID}, resourceType)
+	err := cl.InitConnect(conn)
 	if err != nil {
 		return nil, err
 	}
 	return &fetcher{
-		client:       client,
+		client:       cl,
 		resourceType: resourceType,
-		dataplane:    dataplane,
+		dataplane:    dp,
 		logger:       logrus.WithField("component", "fetcher.xds.client"),
 	}, nil
 }

pkg/dataplane/server/dataplane.go

@@ -62,21 +62,21 @@ func (d *Dataplane) GetClusterHost(name string) (string, error) {
 }
 
 // AddCluster adds a cluster to the map.
-func (d *Dataplane) AddCluster(cluster *cluster.Cluster) {
-	d.clusters[cluster.Name] = cluster
+func (d *Dataplane) AddCluster(c *cluster.Cluster) {
+	d.clusters[c.Name] = c
 }
 
 // AddListener adds a listener to the map.
-func (d *Dataplane) AddListener(listener *listener.Listener) {
-	listenerName := strings.TrimPrefix(listener.Name, api.ImportListenerPrefix)
+func (d *Dataplane) AddListener(ln *listener.Listener) {
+	listenerName := strings.TrimPrefix(ln.Name, api.ImportListenerPrefix)
 	if _, ok := d.listeners[listenerName]; ok {
 		return
 	}
-	d.listeners[listenerName] = listener
+	d.listeners[listenerName] = ln
 	go func() {
 		d.CreateListener(listenerName,
-			listener.Address.GetSocketAddress().GetAddress(),
-			listener.Address.GetSocketAddress().GetPortValue())
+			ln.Address.GetSocketAddress().GetAddress(),
+			ln.Address.GetSocketAddress().GetPortValue())
 	}()
 }
 

pkg/dataplane/server/forwarder.go

@@ -142,7 +142,7 @@ func (f *forwarder) run() {
 	f.closeConnections()
 }
 
-func newForwarder(workloadConn net.Conn, peerConn net.Conn) *forwarder {
+func newForwarder(workloadConn, peerConn net.Conn) *forwarder {
 	return &forwarder{
 		workloadConn: workloadConn,
 		peerConn:     peerConn,

pkg/dataplane/server/listener.go

@@ -86,9 +86,10 @@ func (d *Dataplane) serveEgressConnections(name string, listener net.Listener) e
 	}
 }
 
-func (d *Dataplane) getEgressAuth(name, sourceIP string) (string, string, error) {
+// getEgressAuth returns the target cluster and authorization token for the outgoing connection.
+func (d *Dataplane) getEgressAuth(name, sourceIP string) (string, string, error) { //nolint:gocritic // unnamedResult
 	url := "https://" + d.controlplaneTarget + api.DataplaneEgressAuthorizationPath
-	egressAuthReq, err := http.NewRequest(http.MethodPost, url, nil)
+	egressAuthReq, err := http.NewRequest(http.MethodPost, url, http.NoBody)
 	if err != nil {
 		return "", "", err
 	}

pkg/dataplane/server/server.go

@@ -175,7 +175,7 @@ func (d *Dataplane) initiateEgressConnection(targetCluster, authToken string, ap
 		},
 	}
 
-	egressReq, err := http.NewRequest(http.MethodPost, url, nil)
+	egressReq, err := http.NewRequest(http.MethodPost, url, http.NoBody)
 	if err != nil {
 		return err
 	}

pkg/k8s/kubernetes/kubeinformer.go

@@ -132,7 +132,7 @@ func (k *kubeData) GetInfoApp(app string) (*Info, error) {
 }
 
 // Get Ip and key(prefix of label) and return pod label.
-func (k *kubeData) GetLabel(ip string, key string) (string, error) {
+func (k *kubeData) GetLabel(ip, key string) (string, error) {
 	if info, ok := k.fetchInformers(ip); ok {
 		// Owner data might be discovered after the owned, so we fetch it
 		// at the last moment
@@ -395,7 +395,7 @@ func (k *kubeData) initInformers(client kubernetes.Interface) error {
 }
 
 // CreateService Add support to create a service/NodePort for a target port.
-func (k *kubeData) CreateService(serviceName string, port int, targetPort int, namespace, svcAppName string) error {
+func (k *kubeData) CreateService(serviceName string, port, targetPort int, namespace, svcAppName string) error {
 	var selectorMap map[string]string
 	if svcAppName != "" {
 		selectorMap = map[string]string{"app": svcAppName}

pkg/platform/k8s/platform.go

@@ -132,7 +132,7 @@ func NewPlatform() (*Platform, error) {
 	if err != nil {
 		return nil, err
 	}
-	podReconciler, err := NewPodReconciler(&manager)
+	podReconciler, err := NewPodReconciler(manager)
 	if err != nil {
 		return nil, err
 	}

pkg/platform/k8s/pod_reconciler.go

@@ -109,16 +109,16 @@ func (r *PodReconciler) GetLabelsFromIP(ip string) map[string]string {
 }
 
 // setupWithManager setup PodReconciler for all the pods.
-func (r *PodReconciler) setupWithManager(mgr *ctrl.Manager) error {
-	return ctrl.NewControllerManagedBy(*mgr).
+func (r *PodReconciler) setupWithManager(mgr ctrl.Manager) error {
+	return ctrl.NewControllerManagedBy(mgr).
 		For(&corev1.Pod{}).
 		Complete(r)
 }
 
 // NewPodReconciler creates pod reconciler for monitoring pods in the cluster.
-func NewPodReconciler(mgr *ctrl.Manager) (*PodReconciler, error) {
+func NewPodReconciler(mgr ctrl.Manager) (*PodReconciler, error) {
 	logger := logrus.WithField("component", "platform.k8s.podReconciler")
-	r := CreatePodReconciler((*mgr).GetClient(), logger)
+	r := CreatePodReconciler(mgr.GetClient(), logger)
 
 	if err := r.setupWithManager(mgr); err != nil {
 		return nil, err

pkg/platform/k8s/pod_reconciler_test.go

@@ -40,10 +40,10 @@ const (
 func TestPodReconciler(t *testing.T) {
 	// Test setup
 	logger := logrus.WithField("component", "podReconciler")
-	client, err := getFakeClient()
+	clnt, err := getFakeClient()
 	require.NoError(t, err)
 	ctx := context.Background()
-	podReconciler := k8s.CreatePodReconciler(client, logger)
+	podReconciler := k8s.CreatePodReconciler(clnt, logger)
 
 	req := ctrl.Request{NamespacedName: types.NamespacedName{
 		Name:      TestPodName,

pkg/policyengine/loadBalancer.go

@@ -46,23 +46,23 @@ type ServiceState struct {
 
 type LoadBalancer struct {
 	ServiceMap      map[string][]string                 // Service to Peers
-	Scheme          map[string](map[string]LBScheme)    // PolicyMap [serviceDst][serviceSrc]Policy
+	Scheme          map[string]map[string]LBScheme      // PolicyMap [serviceDst][serviceSrc]Policy
 	ServiceStateMap map[string]map[string]*ServiceState // State of policy Per destination and source
 }
 
 func NewLoadBalancer() *LoadBalancer {
 	lb := &LoadBalancer{
 		ServiceMap:      make(map[string][]string),
-		Scheme:          make(map[string](map[string]LBScheme)),
-		ServiceStateMap: make(map[string](map[string]*ServiceState)),
+		Scheme:          make(map[string]map[string]LBScheme),
+		ServiceStateMap: make(map[string]map[string]*ServiceState),
 	}
 
 	lb.Scheme[event.Wildcard] = map[string]LBScheme{event.Wildcard: Random} // default policy
 	lb.ServiceStateMap[event.Wildcard] = map[string]*ServiceState{event.Wildcard: {}}
 	return lb
 }
 
-func (lB *LoadBalancer) AddToServiceMap(serviceDst string, peer string) {
+func (lB *LoadBalancer) AddToServiceMap(serviceDst, peer string) {
 	if peers, ok := lB.ServiceMap[serviceDst]; ok {
 		_, exist := exists(peers, peer)
 		if !exist {

pkg/store/kv/manager.go

@@ -26,6 +26,6 @@ func (m *Manager) GetObjectStore(name string, sampleObject any) store.ObjectStor
 }
 
 // NewManager returns a new manager.
-func NewManager(store Store) *Manager {
-	return &Manager{store: store}
+func NewManager(s Store) *Manager {
+	return &Manager{store: s}
 }

pkg/store/kv/store.go

@@ -123,14 +123,14 @@ func (s *ObjectStore) GetAll() ([]any, error) {
 }
 
 // NewObjectStore returns a new object store backed by a KV-store.
-func NewObjectStore(name string, store Store, sampleObject any) *ObjectStore {
+func NewObjectStore(name string, s Store, sampleObject any) *ObjectStore {
 	logger := logrus.WithFields(logrus.Fields{
 		"component": "store.kv.object-store",
 		"name":      name,
 	})
 
 	return &ObjectStore{
-		store:      store,
+		store:      s,
 		keyPrefix:  name + ".",
 		objectType: reflect.TypeOf(sampleObject),
 		logger:     logger,

pkg/util/log/log.go

@@ -29,7 +29,7 @@ const (
 )
 
 // SetLog sets logrus logger (format, file, level).
-func SetLog(logLevel string, logFileName string) (*os.File, error) {
+func SetLog(logLevel, logFileName string) (*os.File, error) {
 	var f *os.File
 	if logFileName != "" {
 		usr, err := user.Current()
@@ -39,13 +39,13 @@ func SetLog(logLevel string, logFileName string) (*os.File, error) {
 		logFileFullPath := path.Join(usr.HomeDir, logFileName)
 		createLogFolder(logFileFullPath)
 
-		f, err := os.OpenFile(logFileFullPath, os.O_APPEND|os.O_CREATE|os.O_RDWR, 0o600)
+		logfile, err := os.OpenFile(logFileFullPath, os.O_APPEND|os.O_CREATE|os.O_RDWR, 0o600)
 		fmt.Printf("Creating log file: %v\n", logFileFullPath)
 		if err != nil {
 			return nil, fmt.Errorf("error opening log file: %w", err)
 		}
 		// assign it to the standard logger
-		logrus.SetOutput(f)
+		logrus.SetOutput(logfile)
 	}
 
 	// Set logrus.

pkg/utils/netutils/netutils.go

@@ -37,7 +37,7 @@ var (
 )
 
 // GetConnIP returns the connection's local IP and port.
-func GetConnIP(c net.Conn) (string, string) {
+func GetConnIP(c net.Conn) (string, string) { //nolint:gocritic // unnamedResult: named in comment
 	s := strings.Split(c.LocalAddr().String(), ":")
 	ip := s[0]
 	port := s[1]

tests/e2e/k8s/util/clusterlink.go

@@ -122,7 +122,7 @@ func (c *ClusterLink) RestartDataplane() error {
 
 // Access a cluster service.
 func (c *ClusterLink) AccessService(
-	client func(*KindCluster, *Service) (string, error),
+	clientFn func(*KindCluster, *Service) (string, error),
 	service *Service, allowRetry bool, expectedError error,
 ) (string, error) {
 	if service.Namespace == "" {
@@ -132,7 +132,7 @@ func (c *ClusterLink) AccessService(
 	var data string
 	var err error
 	for t := time.Now(); time.Since(t) < time.Second*60; time.Sleep(time.Millisecond * 500) {
-		data, err = client(c.cluster, service)
+		data, err = clientFn(c.cluster, service)
 		if errors.Is(err, expectedError) || !allowRetry {
 			break
 		}

tests/e2e/k8s/util/kind.go

@@ -378,10 +378,10 @@ func (c *KindCluster) ExposeNodeport(service *Service) (uint16, error) {
 		return 0, fmt.Errorf("expected a single port service, but got: %v", k8sService.Spec.Ports)
 	}
 
-	services := c.nodeportServices[service.Namespace]
-	if services == nil {
-		services = &map[string]*v1.Service{}
-		c.nodeportServices[service.Namespace] = services
+	svcs := c.nodeportServices[service.Namespace]
+	if svcs == nil {
+		svcs = &map[string]*v1.Service{}
+		c.nodeportServices[service.Namespace] = svcs
 	}
 
 	nodeportService := &v1.Service{
@@ -399,7 +399,7 @@ func (c *KindCluster) ExposeNodeport(service *Service) (uint16, error) {
 		},
 	}
 
-	cachedService := (*services)[service.Name]
+	cachedService := (*svcs)[service.Name]
 	cacheMiss := true
 	switch {
 	case cachedService == nil:
@@ -427,7 +427,7 @@ func (c *KindCluster) ExposeNodeport(service *Service) (uint16, error) {
 		}
 
 		cachedService = nodeportService
-		(*services)[service.Name] = cachedService
+		(*svcs)[service.Name] = cachedService
 	}
 
 	return uint16(cachedService.Spec.Ports[0].NodePort), nil