diff --git a/.ci/Dockerfile.depend b/.ci/Dockerfile.depend index a66dc5df..9662e522 100644 --- a/.ci/Dockerfile.depend +++ b/.ci/Dockerfile.depend @@ -11,7 +11,7 @@ RUN apt-get update \ && DEBIAN_FRONTEND=noninteractive apt-get install -y -qq \ apt-utils wget cmake curl git -ENV GOVERSION=1.21.3 +ENV GOVERSION=1.22.3 ENV GOROOT="/root/.go" ENV GOPATH="/root/go" ENV PATH=$GOROOT/bin:$PATH @@ -22,7 +22,7 @@ RUN mkdir -p "${GOROOT}" &&\ RUN wget -nv "https://dl.google.com/go/go${GOVERSION}.linux-amd64.tar.gz" -O "/tmp/go.tar.gz" && \ tar -C "${GOROOT}" --strip-components=1 -xzf "/tmp/go.tar.gz" && \ rm -f "/tmp/go.tar.gz" && \ - curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.54.2 + curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.61.0 # Get modules used by the source code COPY . /vpp-dataplane diff --git a/.golangci.yml b/.golangci.yml index 2c6f1d8f..a0353830 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,6 +1,30 @@ -run: - timeout: 5m +linters-settings: + errcheck: + check-type-assertions: true + goconst: + min-len: 2 + min-occurrences: 3 + gocritic: + enabled-tags: + - diagnostic + - experimental + - opinionated + - performance + - style + govet: + shadow: + # Whether to be strict about shadowing; can be noisy. + # Default: false + strict: true + nolintlint: + require-explanation: true + require-specific: true linters: + # Disable all linters. + # Default: false + # disable-all: true + # Enable specific linter + # https://golangci-lint.run/usage/linters/#enabled-by-default enable: - errcheck - gofmt @@ -12,6 +36,58 @@ linters: - staticcheck - typecheck - unused + # Run only fast linters from enabled linters set (first run won't be fast) + # Default: false + fast: true +run: + # The default concurrency value is the number of available CPU. + concurrency: 4 + # Timeout for analysis, e.g. 30s, 5m. + # Default: 1m + timeout: 5m + # Include test files or not. + # Default: true + tests: false + # Which dirs to skip: issues from them won't be reported. + # Can use regexp here: `generated.*`, regexp is applied on full path, + # including the path prefix if one is set. + # Default value is empty list, + # but default dirs are skipped independently of this option's value (see skip-dirs-use-default). + # "/" will be replaced by current OS file path separator to properly work on Windows. + # skip-dirs: + # - src/external_libs + # - autogenerated_by_my_lib + # Enables skipping of directories: + # - vendor$, third_party$, testdata$, examples$, Godeps$, builtin$ + # Default: true + # skip-dirs-use-default: false + # Which files to skip: they will be analyzed, but issues from them won't be reported. + # Default value is empty list, + # but there is no need to include all autogenerated files, + # we confidently recognize autogenerated files. + # If it's not please let us know. + # "/" will be replaced by current OS file path separator to properly work on Windows. + # skip-files: + # - ".*\\.my\\.go$" + # - lib/bad.go + # If set we pass it to "go list -mod={option}". From "go help modules": + # If invoked with -mod=readonly, the go command is disallowed from the implicit + # automatic updating of go.mod described above. Instead, it fails when any changes + # to go.mod are needed. This setting is most useful to check that go.mod does + # not need updates, such as in a continuous integration and testing system. + # If invoked with -mod=vendor, the go command assumes that the vendor + # directory holds the correct copies of dependencies and ignores + # the dependency descriptions in go.mod. + # + # Allowed values: readonly|vendor|mod + # By default, it isn't set. + # modules-download-mode: readonly + # Allow multiple parallel golangci-lint instances running. + # If false (default) - golangci-lint acquires file lock on start. + allow-parallel-runners: false output: - format: tab - sort-results: true + formats: + - format: tab + path: stderr + sort-results: true + diff --git a/calico-vpp-agent/cni/cni_server.go b/calico-vpp-agent/cni/cni_server.go index 02facdb2..c55da461 100644 --- a/calico-vpp-agent/cni/cni_server.go +++ b/calico-vpp-agent/cni/cni_server.go @@ -550,10 +550,18 @@ func (s *Server) ServeCNI(t *tomb.Tomb) error { case common.NetsSynced: netsSynced <- true case common.NetAddedOrUpdated: - netDef := event.New.(*watchers.NetworkDefinition) + netDef, ok := event.New.(*watchers.NetworkDefinition) + if !ok { + s.log.Errorf("event.New is not a *watchers.NetworkDefinition %v", event.New) + continue + } s.networkDefinitions.Store(netDef.Name, netDef) case common.NetDeleted: - netDef := event.Old.(*watchers.NetworkDefinition) + netDef, ok := event.Old.(*watchers.NetworkDefinition) + if !ok { + s.log.Errorf("event.Old is not a *watchers.NetworkDefinition %v", event.Old) + continue + } s.networkDefinitions.Delete(netDef.Name) } } diff --git a/calico-vpp-agent/connectivity/connectivity_server.go b/calico-vpp-agent/connectivity/connectivity_server.go index dbddd98b..d2d13d6e 100644 --- a/calico-vpp-agent/connectivity/connectivity_server.go +++ b/calico-vpp-agent/connectivity/connectivity_server.go @@ -153,26 +153,44 @@ func (s *ConnectivityServer) ServeConnectivity(t *tomb.Tomb) error { /* Note: we will only receive events we ask for when registering the chan */ switch evt.Type { case common.NetAddedOrUpdated: - new := evt.New.(*watchers.NetworkDefinition) + new, ok := evt.New.(*watchers.NetworkDefinition) + if !ok { + s.log.Errorf("evt.New is not a *watchers.NetworkDefinition %v", evt.New) + } s.networks[new.Vni] = *new case common.NetDeleted: - old := evt.Old.(*watchers.NetworkDefinition) + old, ok := evt.Old.(*watchers.NetworkDefinition) + if !ok { + s.log.Errorf("evt.Old is not a *watchers.NetworkDefinition %v", evt.Old) + } delete(s.networks, old.Vni) case common.ConnectivityAdded: - new := evt.New.(*common.NodeConnectivity) + new, ok := evt.New.(*common.NodeConnectivity) + if !ok { + s.log.Errorf("evt.New is not a *common.NodeConnectivity %v", evt.New) + } err := s.UpdateIPConnectivity(new, false /* isWithdraw */) if err != nil { s.log.Errorf("Error while adding connectivity %s", err) } case common.ConnectivityDeleted: - old := evt.Old.(*common.NodeConnectivity) + old, ok := evt.Old.(*common.NodeConnectivity) + if !ok { + s.log.Errorf("evt.Old is not a *common.NodeConnectivity %v", evt.Old) + } err := s.UpdateIPConnectivity(old, true /* isWithdraw */) if err != nil { s.log.Errorf("Error while deleting connectivity %s", err) } case common.WireguardPublicKeyChanged: - old, _ := evt.Old.(*common.NodeWireguardPublicKey) - new, _ := evt.New.(*common.NodeWireguardPublicKey) + old, ok := evt.Old.(*common.NodeWireguardPublicKey) + if !ok { + s.log.Errorf("evt.Old is not a *common.NodeWireguardPublicKey %v", evt.Old) + } + new, ok := evt.New.(*common.NodeWireguardPublicKey) + if !ok { + s.log.Errorf("evt.New is not a *common.NodeWireguardPublicKey %v", evt.New) + } s.providers[WIREGUARD].(*WireguardProvider).nodesToWGPublicKey[new.Name] = new.WireguardPublicKey change := common.GetStringChangeType(old.WireguardPublicKey, new.WireguardPublicKey) if change != common.ChangeSame { @@ -180,8 +198,14 @@ func (s *ConnectivityServer) ServeConnectivity(t *tomb.Tomb) error { s.updateAllIPConnectivity() } case common.PeerNodeStateChanged: - old, _ := evt.Old.(*common.LocalNodeSpec) - new, _ := evt.New.(*common.LocalNodeSpec) + old, ok := evt.Old.(*common.LocalNodeSpec) + if !ok { + s.log.Errorf("evt.Old is not a *common.LocalNodeSpec %v", evt.Old) + } + new, ok := evt.New.(*common.LocalNodeSpec) + if !ok { + s.log.Errorf("evt.New is not a *common.LocalNodeSpec %v", evt.New) + } if old != nil { if old.IPv4Address != nil { delete(s.nodeByAddr, old.IPv4Address.IP.String()) @@ -199,8 +223,14 @@ func (s *ConnectivityServer) ServeConnectivity(t *tomb.Tomb) error { } } case common.FelixConfChanged: - old, _ := evt.Old.(*felixConfig.Config) - new, _ := evt.New.(*felixConfig.Config) + old, ok := evt.Old.(*felixConfig.Config) + if !ok { + s.log.Errorf("evt.Old is not a *felixConfig.Config %v", evt.Old) + } + new, ok := evt.New.(*felixConfig.Config) + if !ok { + s.log.Errorf("evt.Old is not a *felixConfig.Config %v", evt.New) + } if new == nil || old == nil { /* First/last update, do nothing more */ continue @@ -217,13 +247,19 @@ func (s *ConnectivityServer) ServeConnectivity(t *tomb.Tomb) error { s.log.Infof("connectivity(upd) ipamConf Changed") s.updateAllIPConnectivity() case common.SRv6PolicyAdded: - new := evt.New.(*common.NodeConnectivity) + new, ok := evt.New.(*common.NodeConnectivity) + if !ok { + s.log.Errorf("evt.New is not a *common.NodeConnectivity %v", evt.New) + } err := s.UpdateSRv6Policy(new, false /* isWithdraw */) if err != nil { s.log.Errorf("Error while adding SRv6 Policy %s", err) } case common.SRv6PolicyDeleted: - old := evt.Old.(*common.NodeConnectivity) + old, ok := evt.Old.(*common.NodeConnectivity) + if !ok { + s.log.Errorf("evt.Old is not a *common.NodeConnectivity %v", evt.Old) + } err := s.UpdateSRv6Policy(old, true /* isWithdraw */) if err != nil { s.log.Errorf("Error while deleting SRv6 Policy %s", err) diff --git a/calico-vpp-agent/connectivity/srv6.go b/calico-vpp-agent/connectivity/srv6.go index a489ab3e..e9ed7144 100644 --- a/calico-vpp-agent/connectivity/srv6.go +++ b/calico-vpp-agent/connectivity/srv6.go @@ -182,7 +182,10 @@ func (p *SRv6Provider) AddConnectivity(cn *common.NodeConnectivity) (err error) } else if p.isSRv6TunnelInfoFromBGP(cn) && cn.Custom != nil { // getting SRv6 tunnel data from BGP // storing info in nodePolices - policyData := cn.Custom.(*common.SRv6Tunnel) + policyData, ok := cn.Custom.(*common.SRv6Tunnel) + if !ok { + return fmt.Errorf("cn.Custom is not a (*common.SRv6Tunnel) %v", cn.Custom) + } nodeip = policyData.Dst.String() if p.nodePolices[policyData.Dst.String()] == nil { p.nodePolices[policyData.Dst.String()] = &NodeToPolicies{ diff --git a/calico-vpp-agent/policy/policy_server.go b/calico-vpp-agent/policy/policy_server.go index 73b156d9..64188dd6 100644 --- a/calico-vpp-agent/policy/policy_server.go +++ b/calico-vpp-agent/policy/policy_server.go @@ -336,13 +336,22 @@ func (s *Server) handlePolicyServerEvents(evt common.CalicoVppEvent) error { /* Note: we will only receive events we ask for when registering the chan */ switch evt.Type { case common.NetAddedOrUpdated: - netDef := evt.New.(*watchers.NetworkDefinition) + netDef, ok := evt.New.(*watchers.NetworkDefinition) + if !ok { + return fmt.Errorf("evt.New is not a (*watchers.NetworkDefinition) %v", evt.New) + } s.networkDefinitions[netDef.Name] = netDef case common.NetDeleted: - netDef := evt.Old.(*watchers.NetworkDefinition) + netDef, ok := evt.Old.(*watchers.NetworkDefinition) + if !ok { + return fmt.Errorf("evt.Old is not a (*watchers.NetworkDefinition) %v", evt.Old) + } delete(s.networkDefinitions, netDef.Name) case common.PodAdded: - podSpec := evt.New.(*storage.LocalPodSpec) + podSpec, ok := evt.New.(*storage.LocalPodSpec) + if !ok { + return fmt.Errorf("evt.New is not a (*storage.LocalPodSpec) %v", evt.New) + } swIfIndex := podSpec.TunTapSwIfIndex if swIfIndex == vpplink.InvalidID { swIfIndex = podSpec.MemifSwIfIndex @@ -354,7 +363,10 @@ func (s *Server) handlePolicyServerEvents(evt common.CalicoVppEvent) error { Network: podSpec.NetworkName, }, swIfIndex, podSpec.InterfaceName, podSpec.GetContainerIps()) case common.PodDeleted: - podSpec := evt.Old.(*storage.LocalPodSpec) + podSpec, ok := evt.Old.(*storage.LocalPodSpec) + if !ok { + return fmt.Errorf("evt.Old is not a (*storage.LocalPodSpec) %v", evt.Old) + } if podSpec != nil { s.WorkloadRemoved(&WorkloadEndpointID{ OrchestratorID: podSpec.OrchestratorID, @@ -364,7 +376,10 @@ func (s *Server) handlePolicyServerEvents(evt common.CalicoVppEvent) error { }, podSpec.GetContainerIps()) } case common.TunnelAdded: - swIfIndex := evt.New.(uint32) + swIfIndex, ok := evt.New.(uint32) + if !ok { + return fmt.Errorf("evt.New not a uint32 %v", evt.New) + } s.tunnelSwIfIndexesLock.Lock() s.tunnelSwIfIndexes[swIfIndex] = true @@ -388,7 +403,10 @@ func (s *Server) handlePolicyServerEvents(evt common.CalicoVppEvent) error { case common.TunnelDeleted: var pending bool - swIfIndex := evt.Old.(uint32) + swIfIndex, ok := evt.Old.(uint32) + if !ok { + return fmt.Errorf("evt.Old not a uint32 %v", evt.Old) + } s.tunnelSwIfIndexesLock.Lock() delete(s.tunnelSwIfIndexes, swIfIndex) diff --git a/calico-vpp-agent/prometheus/prometheus.go b/calico-vpp-agent/prometheus/prometheus.go index a45dba98..3d81b382 100644 --- a/calico-vpp-agent/prometheus/prometheus.go +++ b/calico-vpp-agent/prometheus/prometheus.go @@ -17,6 +17,7 @@ package prometheus import ( "context" + "fmt" "net/http" "strconv" "strings" @@ -145,7 +146,10 @@ func (s *Server) exportMetricsForStat(names []string, sta adapter.StatEntry, ifN } s.lock.Lock() if sta.Type == adapter.SimpleCounterVector { - values := sta.Data.(adapter.SimpleCounterStat) + values, ok := sta.Data.(adapter.SimpleCounterStat) + if !ok { + return fmt.Errorf("sta.Data is not a (adapter.SimpleCounterStat), %v", sta.Data) + } for worker := range values { for ifIdx := range values[worker] { if string(ifNames[ifIdx]) != "" { @@ -157,7 +161,10 @@ func (s *Server) exportMetricsForStat(names []string, sta adapter.StatEntry, ifN } } else if sta.Type == adapter.CombinedCounterVector { metric.MetricDescriptor.Unit = units[k] - values := sta.Data.(adapter.CombinedCounterStat) + values, ok := sta.Data.(adapter.CombinedCounterStat) + if !ok { + return fmt.Errorf("sta.Data is not a (adapter.CombinedCounterStat), %v", sta.Data) + } for worker := range values { for ifIdx := range values[worker] { if string(ifNames[ifIdx]) != "" { @@ -220,7 +227,11 @@ func (s *Server) ServePrometheus(t *tomb.Tomb) error { evt := <-s.channel switch evt.Type { case common.PodAdded: - podSpec := evt.New.(*storage.LocalPodSpec) + podSpec, ok := evt.New.(*storage.LocalPodSpec) + if !ok { + s.log.Errorf("evt.New is not a *storage.LocalPodSpec %v", evt.New) + continue + } s.lock.Lock() if podSpec.TunTapSwIfIndex == vpplink.INVALID_SW_IF_INDEX { s.podInterfacesBySwifIndex[podSpec.MemifSwIfIndex] = *podSpec @@ -231,7 +242,11 @@ func (s *Server) ServePrometheus(t *tomb.Tomb) error { s.lock.Unlock() case common.PodDeleted: s.lock.Lock() - podSpec := evt.Old.(*storage.LocalPodSpec) + podSpec, ok := evt.Old.(*storage.LocalPodSpec) + if !ok { + s.log.Errorf("evt.Old is not a *storage.LocalPodSpec %v", evt.Old) + continue + } initialPod := s.podInterfacesByKey[podSpec.Key()] delete(s.podInterfacesByKey, initialPod.Key()) if podSpec.TunTapSwIfIndex == vpplink.INVALID_SW_IF_INDEX { diff --git a/calico-vpp-agent/routing/bgp_watcher.go b/calico-vpp-agent/routing/bgp_watcher.go index 7f6f54d2..fda8124f 100644 --- a/calico-vpp-agent/routing/bgp_watcher.go +++ b/calico-vpp-agent/routing/bgp_watcher.go @@ -470,19 +470,28 @@ func (w *Server) WatchBGPPath(t *tomb.Tomb) error { /* Note: we will only receive events we ask for when registering the chan */ switch evt.Type { case common.LocalPodAddressAdded: - networkPod := evt.New.(cni.NetworkPod) + networkPod, ok := evt.New.(cni.NetworkPod) + if !ok { + return fmt.Errorf("evt.New is not a (cni.NetworkPod) %v", evt.New) + } err := w.announceLocalAddress(networkPod.ContainerIP, networkPod.NetworkVni) if err != nil { return err } case common.LocalPodAddressDeleted: - networkPod := evt.Old.(cni.NetworkPod) + networkPod, ok := evt.Old.(cni.NetworkPod) + if !ok { + return fmt.Errorf("evt.Old is not a (cni.NetworkPod) %v", evt.Old) + } err := w.withdrawLocalAddress(networkPod.ContainerIP, networkPod.NetworkVni) if err != nil { return err } case common.BGPPathAdded: - path := evt.New.(*bgpapi.Path) + path, ok := evt.New.(*bgpapi.Path) + if !ok { + return fmt.Errorf("evt.New is not a (*bgpapi.Path) %v", evt.New) + } _, err = w.BGPServer.AddPath(context.Background(), &bgpapi.AddPathRequest{ TableType: bgpapi.TableType_GLOBAL, Path: path, @@ -491,7 +500,10 @@ func (w *Server) WatchBGPPath(t *tomb.Tomb) error { return err } case common.BGPPathDeleted: - path := evt.Old.(*bgpapi.Path) + path, ok := evt.Old.(*bgpapi.Path) + if !ok { + return fmt.Errorf("evt.Old is not a (*bgpapi.Path) %v", evt.Old) + } err = w.BGPServer.DeletePath(context.Background(), &bgpapi.DeletePathRequest{ TableType: bgpapi.TableType_GLOBAL, Path: path, @@ -500,7 +512,10 @@ func (w *Server) WatchBGPPath(t *tomb.Tomb) error { return err } case common.BGPDefinedSetAdded: - ps := evt.New.(*bgpapi.DefinedSet) + ps, ok := evt.New.(*bgpapi.DefinedSet) + if !ok { + return fmt.Errorf("evt.New is not a (*bgpapi.DefinedSet) %v", evt.New) + } err := w.BGPServer.AddDefinedSet( context.Background(), &bgpapi.AddDefinedSetRequest{DefinedSet: ps}, @@ -509,7 +524,10 @@ func (w *Server) WatchBGPPath(t *tomb.Tomb) error { return err } case common.BGPDefinedSetDeleted: - ps := evt.Old.(*bgpapi.DefinedSet) + ps, ok := evt.Old.(*bgpapi.DefinedSet) + if !ok { + return fmt.Errorf("evt.Old is not a (*bgpapi.DefinedSet) %v", evt.Old) + } err := w.BGPServer.DeleteDefinedSet( context.Background(), &bgpapi.DeleteDefinedSetRequest{DefinedSet: ps, All: false}, @@ -518,7 +536,10 @@ func (w *Server) WatchBGPPath(t *tomb.Tomb) error { return err } case common.BGPPeerAdded: - localPeer := evt.New.(*watchers.LocalBGPPeer) + localPeer, ok := evt.New.(*watchers.LocalBGPPeer) + if !ok { + return fmt.Errorf("evt.New is not a (*watchers.LocalBGPPeer) %v", evt.New) + } peer := localPeer.Peer filters := localPeer.BGPFilterNames // create a neighbor set to apply filter only on specific peer using a global policy @@ -550,7 +571,10 @@ func (w *Server) WatchBGPPath(t *tomb.Tomb) error { localPeer.NeighborSet = neighborSet w.bgpPeers[peer.Conf.NeighborAddress] = localPeer case common.BGPPeerDeleted: - addr := evt.New.(string) + addr, ok := evt.New.(string) + if !ok { + return fmt.Errorf("evt.New is not a (string) %v", evt.New) + } w.log.Infof("bgp(del) neighbor=%s", addr) err = w.cleanUpPeerFilters(addr) if err != nil { @@ -569,13 +593,19 @@ func (w *Server) WatchBGPPath(t *tomb.Tomb) error { } delete(w.bgpPeers, addr) case common.BGPPeerUpdated: - oldFilters := evt.Old.(*watchers.LocalBGPPeer).BGPFilterNames - localPeer := evt.New.(*watchers.LocalBGPPeer) + oldPeer, ok := evt.Old.(*watchers.LocalBGPPeer) + if !ok { + return fmt.Errorf("evt.Old is not (*watchers.LocalBGPPeer) %v", evt.Old) + } + localPeer, ok := evt.New.(*watchers.LocalBGPPeer) + if !ok { + return fmt.Errorf("evt.New is not (*watchers.LocalBGPPeer %v", evt.New) + } peer := localPeer.Peer filters := localPeer.BGPFilterNames w.log.Infof("bgp(upd) neighbor=%s", peer.Conf.NeighborAddress) var BGPPolicies map[string]*watchers.ImpExpPol - if !watchers.CompareStringSlices(localPeer.BGPFilterNames, oldFilters) { // update filters + if !watchers.CompareStringSlices(localPeer.BGPFilterNames, oldPeer.BGPFilterNames) { // update filters err = w.cleanUpPeerFilters(peer.Conf.NeighborAddress) if err != nil { return errors.Wrapf(err, "error cleaning peer filters up") @@ -597,7 +627,10 @@ func (w *Server) WatchBGPPath(t *tomb.Tomb) error { localPeer.BGPPolicies = BGPPolicies w.bgpPeers[peer.Conf.NeighborAddress] = localPeer case common.BGPFilterAddedOrUpdated: - filter := evt.New.(calicov3.BGPFilter) + filter, ok := evt.New.(calicov3.BGPFilter) + if !ok { + return fmt.Errorf("evt.New is not (calicov3.BGPFilter) %v", evt.New) + } w.log.Infof("bgp(add/upd) filter: %s", filter.Name) w.bgpFilters[filter.Name] = &filter // If this filter is already used in gobgp, delete old policies if any and recreate them @@ -626,7 +659,10 @@ func (w *Server) WatchBGPPath(t *tomb.Tomb) error { } } case common.BGPFilterDeleted: // supposed to rely on user to never delete a used bgpfilter - filter := evt.Old.(calicov3.BGPFilter) + filter, ok := evt.Old.(calicov3.BGPFilter) + if !ok { + return fmt.Errorf("evt.Old is not (calicov3.BGPFilter) %v", evt.Old) + } w.log.Infof("bgp(del) filter deleted: %s", filter.Name) delete(w.bgpFilters, filter.Name) } diff --git a/calico-vpp-agent/services/service_server.go b/calico-vpp-agent/services/service_server.go index aa20a690..2ea8a18e 100644 --- a/calico-vpp-agent/services/service_server.go +++ b/calico-vpp-agent/services/service_server.go @@ -16,6 +16,7 @@ package services import ( + "fmt" "net" "strconv" "strings" @@ -195,7 +196,10 @@ func NewServiceServer(vpp *vpplink.VppLink, k8sclient *kubernetes.Clientset, log DeleteFunc: func(obj interface{}) { service, ok := obj.(*v1.Service) if !ok { - service = obj.(cache.DeletedFinalStateUnknown).Obj.(*v1.Service) + service, ok = obj.(cache.DeletedFinalStateUnknown).Obj.(*v1.Service) + if !ok { + panic(fmt.Sprintf("obj.(cache.DeletedFinalStateUnknown).Obj not a (*v1.Service) %v", obj)) + } } server.deleteServiceByName(serviceID(&service.ObjectMeta)) }, @@ -220,7 +224,10 @@ func NewServiceServer(vpp *vpplink.VppLink, k8sclient *kubernetes.Clientset, log DeleteFunc: func(obj interface{}) { ep, ok := obj.(*v1.Endpoints) if !ok { - ep = obj.(cache.DeletedFinalStateUnknown).Obj.(*v1.Endpoints) + ep, ok = obj.(cache.DeletedFinalStateUnknown).Obj.(*v1.Endpoints) + if !ok { + panic(fmt.Sprintf("obj.(cache.DeletedFinalStateUnknown).Obj not a (*v1.Endpoints) %v", obj)) + } } server.deleteServiceByName(serviceID(&ep.ObjectMeta)) }, diff --git a/calico-vpp-agent/watchers/net_watcher.go b/calico-vpp-agent/watchers/net_watcher.go index 050816f0..257f103b 100644 --- a/calico-vpp-agent/watchers/net_watcher.go +++ b/calico-vpp-agent/watchers/net_watcher.go @@ -178,13 +178,21 @@ func (w *NetWatcher) WatchNetworks(t *tomb.Tomb) error { } switch update.Type { case watch.Added: - net := update.Object.(*networkv3.Network) + net, ok := update.Object.(*networkv3.Network) + if !ok { + w.log.Errorf("update.Object is not *networkv3.Network, %v", net) + continue + } err := w.OnNetAdded(net) if err != nil { w.log.Error(err) } case watch.Deleted: - oldNet := update.Object.(*networkv3.Network) + oldNet, ok := update.Object.(*networkv3.Network) + if !ok { + w.log.Errorf("update.Object is not *networkv3.Network, %v", update.Object) + continue + } err := w.OnNetDeleted(oldNet.Name) if err != nil { w.log.Error(err) @@ -203,13 +211,21 @@ func (w *NetWatcher) WatchNetworks(t *tomb.Tomb) error { } switch update.Type { case watch.Added: - nad := update.Object.(*netv1.NetworkAttachmentDefinition) + nad, ok := update.Object.(*netv1.NetworkAttachmentDefinition) + if !ok { + w.log.Errorf("update.Object is not *NetworkAttachmentDefinition, %v", update.Object) + continue + } err := w.onNadAdded(nad) if err != nil { w.log.Error(err) } case watch.Deleted: - nad := update.Object.(*netv1.NetworkAttachmentDefinition) + nad, ok := update.Object.(*netv1.NetworkAttachmentDefinition) + if !ok { + w.log.Errorf("update.Object is not *NetworkAttachmentDefinition, %v", update.Object) + continue + } err := w.onNadDeleted(nad) if err != nil { w.log.Error(err) diff --git a/calico-vpp-agent/watchers/prefix_watcher.go b/calico-vpp-agent/watchers/prefix_watcher.go index be5a76ea..2df4cafe 100644 --- a/calico-vpp-agent/watchers/prefix_watcher.go +++ b/calico-vpp-agent/watchers/prefix_watcher.go @@ -126,8 +126,16 @@ func (w *PrefixWatcher) getAssignedPrefixes() ([]string, error) { } for _, block := range blockList.KVPairs { w.log.Debugf("Found assigned prefix: %+v", block) - key := block.Key.(model.BlockAffinityKey) - value := block.Value.(*model.BlockAffinity) + key, ok := block.Key.(model.BlockAffinityKey) + if !ok { + w.log.Errorf("block.Key is not a model.BlockAffinity %v", block.Key) + continue + } + value, ok := block.Value.(*model.BlockAffinity) + if !ok { + w.log.Errorf("block.Value is not a model.BlockAffinity %v", block.Value) + continue + } if value.State == model.StateConfirmed && !value.Deleted { ps = append(ps, key.CIDR.String()) } diff --git a/calico-vpp-agent/watchers/uplink_route_watcher.go b/calico-vpp-agent/watchers/uplink_route_watcher.go index 27e4656d..0b497fd4 100644 --- a/calico-vpp-agent/watchers/uplink_route_watcher.go +++ b/calico-vpp-agent/watchers/uplink_route_watcher.go @@ -240,7 +240,11 @@ func (r *RouteWatcher) WatchRoutes(t *tomb.Tomb) error { case event := <-r.eventChan: switch event.Type { case common.NetDeleted: - netDef := event.Old.(*NetworkDefinition) + netDef, ok := event.Old.(*NetworkDefinition) + if !ok { + r.log.Errorf("event.Old is not a (*NetworkDefinition) %v", event.Old) + goto restart + } key := netDef.Range routes, err := r.getNetworkRoute(key, netDef.PhysicalNetworkName) if err != nil { @@ -255,7 +259,11 @@ func (r *RouteWatcher) WatchRoutes(t *tomb.Tomb) error { } } case common.NetAddedOrUpdated: - netDef := event.New.(*NetworkDefinition) + netDef, ok := event.New.(*NetworkDefinition) + if !ok { + r.log.Errorf("event.New is not a (*NetworkDefinition) %v", event.New) + goto restart + } key := netDef.Range routes, err := r.getNetworkRoute(key, netDef.PhysicalNetworkName) if err != nil { @@ -270,8 +278,16 @@ func (r *RouteWatcher) WatchRoutes(t *tomb.Tomb) error { } } case common.IpamConfChanged: - old, _ := event.Old.(*proto.IPAMPool) - new, _ := event.New.(*proto.IPAMPool) + old, ok := event.Old.(*proto.IPAMPool) + if !ok { + r.log.Errorf("event.Old is not a (*proto.IPAMPool) %v", event.Old) + goto restart + } + new, ok := event.New.(*proto.IPAMPool) + if !ok { + r.log.Errorf("event.New is not a (*proto.IPAMPool) %v", event.New) + goto restart + } r.log.Debugf("Received IPAM config update in route watcher old:%+v new:%+v", old, new) if new == nil { key := old.Cidr diff --git a/go.mod b/go.mod index 23514e7b..34a7f364 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/projectcalico/vpp-dataplane/v3 -go 1.21 +go 1.22.3 require ( github.com/calico-vpp/vpplink v0.0.0-20240117140938-62e485f48c6d diff --git a/multinet-monitor/watcher.go b/multinet-monitor/watcher.go index 1ae2b2af..39ed1d11 100644 --- a/multinet-monitor/watcher.go +++ b/multinet-monitor/watcher.go @@ -97,7 +97,11 @@ func watching(podWatcher, svcWatcher, nadWatcher watch.Interface) { } switch update.Type { case watch.Added: - nad := update.Object.(*netv1.NetworkAttachmentDefinition) + nad, ok := update.Object.(*netv1.NetworkAttachmentDefinition) + if !ok { + log.Errorf("update.Object is not a (*netv1.NetworkAttachmentDefinition), %v", update.Object) + continue + } var nadConfig nadv1.NetConfList err := json.Unmarshal([]byte(nad.Spec.Config), &nadConfig) if err != nil { @@ -110,7 +114,11 @@ func watching(podWatcher, svcWatcher, nadWatcher watch.Interface) { case watch.Modified: log.Warn("net attach def modified: not supported") case watch.Deleted: - nad := update.Object.(*netv1.NetworkAttachmentDefinition) + nad, ok := update.Object.(*netv1.NetworkAttachmentDefinition) + if !ok { + log.Errorf("update.Object is not a (*netv1.NetworkAttachmentDefinition), %v", update.Object) + continue + } var nadConfig nadv1.NetConfList err := json.Unmarshal([]byte(nad.Spec.Config), &nadConfig) if err != nil { @@ -128,7 +136,11 @@ func watching(podWatcher, svcWatcher, nadWatcher watch.Interface) { } switch update.Type { case watch.Added: - pod := update.Object.(*v1.Pod) + pod, ok := update.Object.(*v1.Pod) + if !ok { + log.Errorf("update.Object is not a (*v1.Pod), %v", update.Object) + continue + } podNadIf, inNetwork := pod.Annotations["k8s.v1.cni.cncf.io/networks"] if inNetwork { podNad := strings.Split(podNadIf, "@")[0] @@ -137,9 +149,13 @@ func watching(podWatcher, svcWatcher, nadWatcher watch.Interface) { addPod(pod, podNad) } case watch.Deleted: - pod := update.Object.(*v1.Pod) + pod, ok := update.Object.(*v1.Pod) + if !ok { + log.Errorf("update.Object is not a (*v1.Pod), %v", update.Object) + continue + } if _, found := currentPodList[pod.Name]; found { - log.Infof("POD (del) %s", update.Object.(*v1.Pod).Name) + log.Infof("POD (del) %s", pod.Name) delete(currentPodList, pod.Name) deletePod(pod) } @@ -152,20 +168,28 @@ func watching(podWatcher, svcWatcher, nadWatcher watch.Interface) { } switch update.Type { case watch.Added: - service := update.Object.(*v1.Service) + service, ok := update.Object.(*v1.Service) + if !ok { + log.Errorf("update.Object is not a (*v1.Service), %v", update.Object) + continue + } if service.Spec.Selector == nil { selector, selectorFound := service.Annotations["extensions.projectcalico.org/selector"] network, networkFound := service.Annotations["extensions.projectcalico.org/network"] if selectorFound && networkFound { log.Infof("SVC (add) %s", service.Name) currentSvcList[service.Name] = service - addService(update.Object.(*v1.Service), selector, network) + addService(service, selector, network) } } case watch.Deleted: - service := update.Object.(*v1.Service) + service, ok := update.Object.(*v1.Service) + if !ok { + log.Errorf("update.Object is not a (*v1.Service), %v", update.Object) + continue + } if _, found := currentSvcList[service.Name]; found { - log.Infof("SVC (del) %s", update.Object.(*v1.Service).Name) + log.Infof("SVC (del) %s", service.Name) delete(currentSvcList, service.Name) delete(currentEpList, service.Name) } diff --git a/scripts/replay-trace/replay_trace.go b/scripts/replay-trace/replay_trace.go index 8eb8f4b9..4592c196 100644 --- a/scripts/replay-trace/replay_trace.go +++ b/scripts/replay-trace/replay_trace.go @@ -95,7 +95,11 @@ func newReplyTypeForRequest(req api.Message) (api.Message, error) { if !found { return nil, fmt.Errorf("No reply for %s", req.GetMessageName()) } - reply = reflect.New(reflect.TypeOf(reply).Elem()).Interface().(api.Message) + obj := reflect.New(reflect.TypeOf(reply).Elem()).Interface() + reply, ok := obj.(api.Message) + if !ok { + return reply, fmt.Errorf("reply type is not api.Message %v", obj) + } return reply, nil } @@ -362,7 +366,11 @@ func main() { continue } - msg = reflect.New(reflect.TypeOf(msg).Elem()).Interface().(api.Message) + msg, ok := reflect.New(reflect.TypeOf(msg).Elem()).Interface().(api.Message) + if !ok { + continue + } + if _, ok := msg.(*core.ControlPing); ok { continue } diff --git a/vpplink/helpers.go b/vpplink/helpers.go index 042a88dc..55b6767d 100644 --- a/vpplink/helpers.go +++ b/vpplink/helpers.go @@ -65,8 +65,8 @@ func (call *CleanupCall) Execute() { } /* If only one value is returned, we assume this is an error and log it */ if !ret[0].IsNil() { - err := ret[0].Interface().(error) - log.Errorf("Cleanup errored : %s", err) + err, ok := ret[0].Interface().(error) + log.Errorf("Cleanup errored : %s, %t", err, ok) } } @@ -101,8 +101,8 @@ func (v *VppLink) Retry(sleepBtwRetries time.Duration, retries int, f interface{ for i := 0; i < retries; i++ { ret := reflect.ValueOf(f).Call(vargs)[0] if !ret.IsNil() { - err := ret.Interface().(error) - log.Warnf("Try [%d] errored : %s", i, err) + err, ok := ret.Interface().(error) + log.Warnf("Try [%d] errored : %s, %t", i, err, ok) time.Sleep(sleepBtwRetries) } else { return nil diff --git a/vpplink/stats.go b/vpplink/stats.go index 8cda806b..225aa3c6 100644 --- a/vpplink/stats.go +++ b/vpplink/stats.go @@ -33,7 +33,10 @@ func GetInterfaceStats(sc *statsclient.StatsClient) (ifNames adapter.NameStat, d if len(dumpStatsNames) == 0 { return nil, nil, fmt.Errorf("no interfaces available: %w", err) } - ifNames = dumpStatsNames[0].Data.(adapter.NameStat) + ifNames, ok := dumpStatsNames[0].Data.(adapter.NameStat) + if !ok { + return nil, nil, fmt.Errorf("dumpStatsNames[0].Data. is not an adapter.NameStat: %v", dumpStatsNames[0].Data) + } dumpStats, err = sc.DumpStats("/if/") if err != nil { diff --git a/vpplink/types/capo.go b/vpplink/types/capo.go index 663dc8da..eddff2be 100644 --- a/vpplink/types/capo.go +++ b/vpplink/types/capo.go @@ -215,7 +215,11 @@ func StrableListToString(prefix string, arg interface{}) string { m = v.Addr().MethodByName("String") } ret := m.Call(make([]reflect.Value, 0))[0] - s += ret.Interface().(string) + retStr, ok := ret.Interface().(string) + if !ok { + panic(fmt.Sprintf("Call to String() did not output a string %v", ret)) + } + s += retStr } return s + "]" }