From 4bb63029553743eb7fddfbafeeb31c961dcc6c8d Mon Sep 17 00:00:00 2001 From: Etai Lev Ran Date: Thu, 18 Jan 2024 13:46:56 +0200 Subject: [PATCH] changes for gocritic linter (#269) * changes for gocritic * enable gocritic features --------- Signed-off-by: Etai Lev Ran --- .golangci.yaml | 6 +++--- cmd/cl-controlplane/app/server.go | 10 +++++----- pkg/controlplane/client.go | 8 +++----- pkg/controlplane/instance.go | 4 ++-- pkg/controlplane/server/http/api.go | 5 ++--- pkg/dataplane/client/fetcher.go | 10 +++++----- pkg/dataplane/server/dataplane.go | 14 +++++++------- pkg/dataplane/server/forwarder.go | 2 +- pkg/dataplane/server/listener.go | 5 +++-- pkg/dataplane/server/server.go | 2 +- pkg/k8s/kubernetes/kubeinformer.go | 4 ++-- pkg/platform/k8s/platform.go | 2 +- pkg/platform/k8s/pod_reconciler.go | 8 ++++---- pkg/platform/k8s/pod_reconciler_test.go | 4 ++-- pkg/policyengine/loadBalancer.go | 8 ++++---- pkg/store/kv/manager.go | 4 ++-- pkg/store/kv/store.go | 4 ++-- pkg/util/log/log.go | 6 +++--- pkg/utils/netutils/netutils.go | 2 +- tests/e2e/k8s/util/clusterlink.go | 4 ++-- tests/e2e/k8s/util/kind.go | 12 ++++++------ 21 files changed, 61 insertions(+), 63 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index c3850634..50be6ed2 100644 --- a/.golangci.yaml +++ b/.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 diff --git a/cmd/cl-controlplane/app/server.go b/cmd/cl-controlplane/app/server.go index a47eff86..f743b564 100644 --- a/cmd/cl-controlplane/app/server.go +++ b/cmd/cl-controlplane/app/server.go @@ -129,13 +129,13 @@ 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 } @@ -143,7 +143,7 @@ func (o *Options) Run() error { 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 } diff --git a/pkg/controlplane/client.go b/pkg/controlplane/client.go index ed042153..53531070 100644 --- a/pkg/controlplane/client.go +++ b/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) diff --git a/pkg/controlplane/instance.go b/pkg/controlplane/instance.go index affa4165..396de170 100644 --- a/pkg/controlplane/instance.go +++ b/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, } diff --git a/pkg/controlplane/server/http/api.go b/pkg/controlplane/server/http/api.go index c5c643f2..4c13489d 100644 --- a/pkg/controlplane/server/http/api.go +++ b/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, }, }, diff --git a/pkg/dataplane/client/fetcher.go b/pkg/dataplane/client/fetcher.go index c422ab07..0e12bf45 100644 --- a/pkg/dataplane/client/fetcher.go +++ b/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 } diff --git a/pkg/dataplane/server/dataplane.go b/pkg/dataplane/server/dataplane.go index b5e7516f..f744d877 100644 --- a/pkg/dataplane/server/dataplane.go +++ b/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()) }() } diff --git a/pkg/dataplane/server/forwarder.go b/pkg/dataplane/server/forwarder.go index 17aef071..e30c24f6 100644 --- a/pkg/dataplane/server/forwarder.go +++ b/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, diff --git a/pkg/dataplane/server/listener.go b/pkg/dataplane/server/listener.go index f30ae1a9..a85f25e3 100644 --- a/pkg/dataplane/server/listener.go +++ b/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 } diff --git a/pkg/dataplane/server/server.go b/pkg/dataplane/server/server.go index 3cb92dee..43526dc0 100644 --- a/pkg/dataplane/server/server.go +++ b/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 } diff --git a/pkg/k8s/kubernetes/kubeinformer.go b/pkg/k8s/kubernetes/kubeinformer.go index 56073e80..513c59c3 100644 --- a/pkg/k8s/kubernetes/kubeinformer.go +++ b/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} diff --git a/pkg/platform/k8s/platform.go b/pkg/platform/k8s/platform.go index e1d0d4e0..52e78ce1 100644 --- a/pkg/platform/k8s/platform.go +++ b/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 } diff --git a/pkg/platform/k8s/pod_reconciler.go b/pkg/platform/k8s/pod_reconciler.go index 1580b1d6..bfcd3ee1 100644 --- a/pkg/platform/k8s/pod_reconciler.go +++ b/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 diff --git a/pkg/platform/k8s/pod_reconciler_test.go b/pkg/platform/k8s/pod_reconciler_test.go index 856c874c..a22d899d 100644 --- a/pkg/platform/k8s/pod_reconciler_test.go +++ b/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, diff --git a/pkg/policyengine/loadBalancer.go b/pkg/policyengine/loadBalancer.go index 33392b8a..7da256a5 100644 --- a/pkg/policyengine/loadBalancer.go +++ b/pkg/policyengine/loadBalancer.go @@ -46,15 +46,15 @@ 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 @@ -62,7 +62,7 @@ func NewLoadBalancer() *LoadBalancer { 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 { diff --git a/pkg/store/kv/manager.go b/pkg/store/kv/manager.go index 568ef6da..c7c9fb9d 100644 --- a/pkg/store/kv/manager.go +++ b/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} } diff --git a/pkg/store/kv/store.go b/pkg/store/kv/store.go index db6cf55e..74a568ec 100644 --- a/pkg/store/kv/store.go +++ b/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, diff --git a/pkg/util/log/log.go b/pkg/util/log/log.go index 5dd52550..f3d3ee77 100644 --- a/pkg/util/log/log.go +++ b/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. diff --git a/pkg/utils/netutils/netutils.go b/pkg/utils/netutils/netutils.go index c85e2c9c..cfca886f 100755 --- a/pkg/utils/netutils/netutils.go +++ b/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] diff --git a/tests/e2e/k8s/util/clusterlink.go b/tests/e2e/k8s/util/clusterlink.go index 4172c9d9..95415565 100644 --- a/tests/e2e/k8s/util/clusterlink.go +++ b/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 } diff --git a/tests/e2e/k8s/util/kind.go b/tests/e2e/k8s/util/kind.go index f69ba5f3..2259d0b2 100644 --- a/tests/e2e/k8s/util/kind.go +++ b/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