diff --git a/.golangci.yml b/.golangci.yml index 86b2b2336..50e7106f4 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -156,4 +156,5 @@ issues: linters: - gochecknoinits - goerr113 + - unparam - wrapcheck diff --git a/coredns/gateway/controller.go b/coredns/gateway/controller.go index b080e92bf..2e67e3642 100644 --- a/coredns/gateway/controller.go +++ b/coredns/gateway/controller.go @@ -121,7 +121,7 @@ func (c *Controller) Stop() { logger.Infof("Gateway status Controller stopped") } -func (c *Controller) processNextGateway(key, name, ns string) (bool, error) { +func (c *Controller) processNextGateway(key, _, _ string) (bool, error) { obj, exists, err := c.store.GetByKey(key) if err != nil { // requeue the item to work on later. @@ -181,6 +181,7 @@ func (c *Controller) updateClusterStatusMap(connections []interface{}) { if newMap == nil { newMap = copyMap(currentMap) } + delete(newMap, clusterID) } } @@ -250,6 +251,7 @@ func (c *Controller) getCheckedClientset(client dynamic.Interface) (dynamic.Reso // First check if the Submariner resource is present. gvr, _ := schema.ParseResourceArg("submariners.v1alpha1.submariner.io") list, err := client.Resource(*gvr).Namespace(v1.NamespaceAll).List(context.TODO(), metav1.ListOptions{}) + if apierrors.IsNotFound(err) || meta.IsNoMatchError(err) || (err == nil && len(list.Items) == 0) { return nil, apierrors.NewNotFound(gvr.GroupResource(), "") } diff --git a/coredns/loadbalancer/smooth_weighted_round_robin_test.go b/coredns/loadbalancer/smooth_weighted_round_robin_test.go index db244a151..ead1b264e 100644 --- a/coredns/loadbalancer/smooth_weighted_round_robin_test.go +++ b/coredns/loadbalancer/smooth_weighted_round_robin_test.go @@ -34,6 +34,9 @@ type server struct { weight int64 } +//nolint:gosec // This is only used to shuffle the list of resolvers +var randgen = rand.New(rand.NewSource(time.Now().UnixNano())) + var _ = Describe("Smooth Weighted RR", func() { // Global Vars var servers []server @@ -123,20 +126,18 @@ var _ = Describe("Smooth Weighted RR", func() { {name: "server2", weight: 1}, {name: "server3", weight: 1}, } - rand.Seed(time.Now().UnixNano()) servers = []server{ {name: "server1", weight: randInt()}, {name: "server2", weight: randInt()}, {name: "server3", weight: randInt()}, } - roundRobinServers = []server{ {name: "server1", weight: 1}, {name: "server2", weight: 1}, {name: "server3", weight: 1}, } - rand.Shuffle(len(servers), func(i, j int) { servers[i], servers[j] = servers[j], servers[i] }) + randgen.Shuffle(len(servers), func(i, j int) { servers[i], servers[j] = servers[j], servers[i] }) }) When("first created", func() { @@ -165,7 +166,7 @@ var _ = Describe("Smooth Weighted RR", func() { When("a nil is added", func() { It("should return an error", func() { err := lb.Add(nil, 100) - Expect(err).ToNot(BeNil()) + Expect(err).To(HaveOccurred()) validateEmptyLBState() }) }) @@ -173,7 +174,7 @@ var _ = Describe("Smooth Weighted RR", func() { When("an item is added with a negative weight", func() { It("should return an error", func() { err := lb.Add(servers[0], -100) - Expect(err).ToNot(BeNil()) + Expect(err).To(HaveOccurred()) validateEmptyLBState() }) }) @@ -193,7 +194,7 @@ var _ = Describe("Smooth Weighted RR", func() { s := servers[0] addServer(s) err := lb.Add(s.name, s.weight) - Expect(err).ToNot(BeNil()) + Expect(err).To(HaveOccurred()) Expect(lb.ItemCount()).To(Equal(1)) }) }) @@ -202,7 +203,7 @@ var _ = Describe("Smooth Weighted RR", func() { It("should balance between them in an equal manner", func() { for _, s := range roundRobinServers { err := lb.Add(s.name, s.weight) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) } for i := 0; i < 100; i++ { for _, s := range roundRobinServers { @@ -253,7 +254,7 @@ var _ = Describe("Smooth Weighted RR", func() { newServer := server{name: "server4", weight: 1} smoothTestingServers = append(smoothTestingServers, newServer) err := lb.Add(newServer.name, newServer.weight) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) Expect(lb.Next().(string)).To(Equal(smoothTestingServers[0].name)) Expect(lb.Next().(string)).To(Equal(smoothTestingServers[2].name)) diff --git a/coredns/main.go b/coredns/main.go index 43b508e69..5e84cad22 100644 --- a/coredns/main.go +++ b/coredns/main.go @@ -117,6 +117,7 @@ var ( func init() { flag.BoolVar(&showVersion, "subm-version", showVersion, "Show version") + dnsserver.Directives = directives } diff --git a/coredns/plugin/handler_test.go b/coredns/plugin/handler_test.go index ddeeac855..b05d8f7d6 100644 --- a/coredns/plugin/handler_test.go +++ b/coredns/plugin/handler_test.go @@ -84,7 +84,7 @@ type FailingResponseWriter struct { errorMsg string } -func (w *FailingResponseWriter) WriteMsg(m *dns.Msg) error { +func (w *FailingResponseWriter) WriteMsg(_ *dns.Msg) error { return errors.New(w.errorMsg) } @@ -317,6 +317,7 @@ func testWithFallback() { m := new(dns.Msg) m.SetRcode(r, dns.RcodeBadCookie) _ = w.WriteMsg(m) + return dns.RcodeBadCookie, nil }) @@ -991,7 +992,6 @@ func (t *handlerTestDriver) executeTestCase(rec *dnstest.Recorder, tc test.Case) } } -//nolint:unparam // `name` always receives `service1'. func newServiceImport(namespace, name string, siType mcsv1a1.ServiceImportType) *mcsv1a1.ServiceImport { return &mcsv1a1.ServiceImport{ ObjectMeta: metav1.ObjectMeta{ @@ -1004,9 +1004,8 @@ func newServiceImport(namespace, name string, siType mcsv1a1.ServiceImportType) } } -//nolint:unparam // `namespace` always receives `namespace1`. -func newEndpointSlice(namespace, name, clusterID string, ports []mcsv1a1.ServicePort, - endpoints ...discovery.Endpoint) *discovery.EndpointSlice { +func newEndpointSlice(namespace, name, clusterID string, ports []mcsv1a1.ServicePort, endpoints ...discovery.Endpoint, +) *discovery.EndpointSlice { epPorts := make([]discovery.EndpointPort, len(ports)) for i := range ports { epPorts[i] = discovery.EndpointPort{ diff --git a/coredns/plugin/record.go b/coredns/plugin/record.go index bbdbe9052..b5bc8b349 100644 --- a/coredns/plugin/record.go +++ b/coredns/plugin/record.go @@ -54,11 +54,13 @@ func (lh *Lighthouse) createSRVRecords(dnsrecords []resolver.DNSRecord, state *r reqPorts = dnsRecord.Ports } else { log.Debugf("Requested port %q, protocol %q for SRV", pReq.port, pReq.protocol) + for _, port := range dnsRecord.Ports { name := strings.ToLower(port.Name) protocol := strings.ToLower(string(port.Protocol)) log.Debugf("Checking port %q, protocol %q", name, protocol) + if name == pReq.port && protocol == pReq.protocol { reqPorts = append(reqPorts, port) } diff --git a/coredns/plugin/setup.go b/coredns/plugin/setup.go index 2d99a7fa3..c5fe54b1e 100644 --- a/coredns/plugin/setup.go +++ b/coredns/plugin/setup.go @@ -49,7 +49,8 @@ var ( buildKubeConfigFunc = clientcmd.BuildConfigFromFlags newDynamicClient = func(c *rest.Config) (dynamic.Interface, error) { - return dynamic.NewForConfig(c) + client, err := dynamic.NewForConfig(c) + return client, errors.Wrap(err, "error creating a dynamic client") } restMapper meta.RESTMapper @@ -109,7 +110,7 @@ func lighthouseParse(c *caddy.Controller) (*Lighthouse, error) { resolverController := resolver.NewController(lh.Resolver) - err = resolverController.Start(watcher.Config{ + err = resolverController.Start(&watcher.Config{ RestConfig: cfg, Client: localClient, RestMapper: restMapper, @@ -121,6 +122,7 @@ func lighthouseParse(c *caddy.Controller) (*Lighthouse, error) { c.OnShutdown(func() error { gwController.Stop() resolverController.Stop() + return nil }) @@ -136,8 +138,10 @@ func lighthouseParse(c *caddy.Controller) (*Lighthouse, error) { for i, str := range lh.Zones { hosts := plugin.Host(str).NormalizeExact() if hosts == nil { - c.Errf("Failed to normalize zone %q", str) + log.Infof("Failed to normalize zone %q", str) + lh.Zones[i] = "" + continue } diff --git a/coredns/plugin/setup_internal_test.go b/coredns/plugin/setup_internal_test.go index 66f8c5775..9eaa581ba 100644 --- a/coredns/plugin/setup_internal_test.go +++ b/coredns/plugin/setup_internal_test.go @@ -40,7 +40,7 @@ import ( type fakeHandler struct{} -func (f *fakeHandler) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (int, error) { +func (f *fakeHandler) ServeDNS(_ context.Context, _ dns.ResponseWriter, _ *dns.Msg) (int, error) { return dns.RcodeSuccess, nil } @@ -151,6 +151,7 @@ func testCorrectConfig() { c := caddy.NewTestController("dns", config) lh, err := lighthouseParse(c) Expect(err).NotTo(HaveOccurred()) + setupErr := setupLighthouse(c) // For coverage Expect(setupErr).NotTo(HaveOccurred()) Expect(lh.Fall).Should(Equal(fall.F{})) diff --git a/coredns/resolver/controller.go b/coredns/resolver/controller.go index c30dae6f2..1483e6560 100644 --- a/coredns/resolver/controller.go +++ b/coredns/resolver/controller.go @@ -28,7 +28,6 @@ import ( discovery "k8s.io/api/discovery/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" - k8slabels "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" logf "sigs.k8s.io/controller-runtime/pkg/log" mcsv1a1 "sigs.k8s.io/mcs-api/pkg/apis/v1alpha1" @@ -42,14 +41,14 @@ type controller struct { stopCh chan struct{} } -func NewController(r *Interface) *controller { +func NewController(r *Interface) *controller { //nolint:revive // Returning a private type doesn't cause problems here. return &controller{ resolver: r, stopCh: make(chan struct{}), } } -func (c *controller) Start(config watcher.Config) error { +func (c *controller) Start(config *watcher.Config) error { logger.Infof("Starting Resolver Controller") config.ResourceConfigs = []watcher.ResourceConfig{ @@ -78,7 +77,7 @@ func (c *controller) Start(config watcher.Config) error { var err error - c.resourceWatcher, err = watcher.New(&config) + c.resourceWatcher, err = watcher.New(config) if err != nil { return errors.Wrap(err, "error creating the resource watcher") } @@ -111,13 +110,14 @@ func (c *controller) onEndpointSliceCreateOrUpdate(obj runtime.Object, _ int) bo } func (c *controller) getAllEndpointSlices(forEPS *discovery.EndpointSlice) []*discovery.EndpointSlice { - list := c.resourceWatcher.ListResources(&discovery.EndpointSlice{}, k8slabels.SelectorFromSet(map[string]string{ + list := c.resourceWatcher.ListResources(&discovery.EndpointSlice{}, labels.SelectorFromSet(map[string]string{ constants.LabelSourceNamespace: forEPS.Labels[constants.LabelSourceNamespace], mcsv1a1.LabelServiceName: forEPS.Labels[mcsv1a1.LabelServiceName], constants.MCSLabelSourceCluster: forEPS.Labels[constants.MCSLabelSourceCluster], })) var epSlices []*discovery.EndpointSlice + for i := range list { eps := list[i].(*discovery.EndpointSlice) if !isOnBroker(eps) && !isLegacyEndpointSlice(eps) { diff --git a/coredns/resolver/controller_test.go b/coredns/resolver/controller_test.go index a06cfc64d..0dba3b8cd 100644 --- a/coredns/resolver/controller_test.go +++ b/coredns/resolver/controller_test.go @@ -21,11 +21,10 @@ package resolver_test import ( "context" - "github.com/submariner-io/lighthouse/coredns/constants" - . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "github.com/submariner-io/admiral/pkg/syncer/test" + "github.com/submariner-io/lighthouse/coredns/constants" "github.com/submariner-io/lighthouse/coredns/resolver" discovery "k8s.io/api/discovery/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" diff --git a/coredns/resolver/endpoint_slice.go b/coredns/resolver/endpoint_slice.go index c6d9972df..c5927471a 100644 --- a/coredns/resolver/endpoint_slice.go +++ b/coredns/resolver/endpoint_slice.go @@ -24,9 +24,7 @@ import ( "strconv" "strings" - utilruntime "k8s.io/apimachinery/pkg/util/runtime" - "k8s.io/apimachinery/pkg/util/sets" - + "github.com/pkg/errors" "github.com/submariner-io/admiral/pkg/resource" "github.com/submariner-io/lighthouse/coredns/constants" discovery "k8s.io/api/discovery/v1" @@ -34,6 +32,8 @@ import ( "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" + "k8s.io/apimachinery/pkg/util/sets" mcsv1a1 "sigs.k8s.io/mcs-api/pkg/apis/v1alpha1" ) @@ -95,7 +95,8 @@ func (i *Interface) PutEndpointSlices(endpointSlices ...*discovery.EndpointSlice return false } -func (i *Interface) putClusterIPEndpointSlice(key, clusterID string, endpointSlice *discovery.EndpointSlice, serviceInfo *serviceInfo) bool { +func (i *Interface) putClusterIPEndpointSlice(key, clusterID string, endpointSlice *discovery.EndpointSlice, serviceInfo *serviceInfo, +) bool { _, found := endpointSlice.Labels[constants.LabelIsHeadless] if !found { // This is a legacy pre-0.15 EndpointSlice. @@ -167,8 +168,8 @@ func (i *Interface) putHeadlessEndpointSlices(key, clusterID string, endpointSli switch { case endpoint.Hostname != nil && *endpoint.Hostname != "": hostname = *endpoint.Hostname - case endpoint.TargetRef != nil && strings.ToLower((*endpoint.TargetRef).Kind) == "pod": - hostname = (*endpoint.TargetRef).Name + case endpoint.TargetRef != nil && strings.EqualFold(endpoint.TargetRef.Kind, "pod"): + hostname = endpoint.TargetRef.Name } for _, address := range endpoint.Addresses { @@ -220,7 +221,7 @@ func (i *Interface) getLocalEndpointSlices(forEPS *discovery.EndpointSlice) ([]* }).String(), }) if err != nil { - return nil, err + return nil, errors.Wrapf(err, "error retrieving the endpointslices in namespace %s", forEPS.Labels[constants.LabelSourceNamespace]) } if len(list.Items) == 0 { @@ -229,6 +230,7 @@ func (i *Interface) getLocalEndpointSlices(forEPS *discovery.EndpointSlice) ([]* } epSlices := make([]*discovery.EndpointSlice, len(list.Items)) + for i := range list.Items { epSlice := &discovery.EndpointSlice{} err := runtime.DefaultUnstructuredConverter.FromUnstructured(list.Items[i].Object, epSlice) diff --git a/coredns/resolver/headless_service_test.go b/coredns/resolver/headless_service_test.go index 8b2c7d1da..871fc5b4e 100644 --- a/coredns/resolver/headless_service_test.go +++ b/coredns/resolver/headless_service_test.go @@ -48,6 +48,7 @@ func testHeadlessService() { BeforeEach(func() { t.resolver.PutServiceImport(newHeadlessAggregatedServiceImport(namespace1, service1)) + annotations = nil endpointSlice = nil }) diff --git a/coredns/resolver/resolver.go b/coredns/resolver/resolver.go index 998cacad5..ac308f7f2 100644 --- a/coredns/resolver/resolver.go +++ b/coredns/resolver/resolver.go @@ -30,7 +30,7 @@ func New(clusterStatus ClusterStatus, client dynamic.Interface) *Interface { } } -func (i *Interface) GetDNSRecords(namespace, name, clusterID, hostname string) (records []DNSRecord, isHeadless bool, found bool) { +func (i *Interface) GetDNSRecords(namespace, name, clusterID, hostname string) (records []DNSRecord, isHeadless, found bool) { i.mutex.RLock() defer i.mutex.RUnlock() @@ -49,6 +49,7 @@ func (i *Interface) GetDNSRecords(namespace, name, clusterID, hostname string) ( } records, found = i.getHeadlessRecords(serviceInfo, clusterID, hostname) + return records, true, found } diff --git a/coredns/resolver/resolver_suite_test.go b/coredns/resolver/resolver_suite_test.go index 5cbebfa3a..5b906937a 100644 --- a/coredns/resolver/resolver_suite_test.go +++ b/coredns/resolver/resolver_suite_test.go @@ -145,7 +145,7 @@ func newTestDriver() *testDriver { t.resolver = resolver.New(t.clusterStatus, client) controller := resolver.NewController(t.resolver) - Expect(controller.Start(watcher.Config{ + Expect(controller.Start(&watcher.Config{ RestMapper: restMapper, Client: client, })).To(Succeed()) diff --git a/test/e2e/discovery/headless_services.go b/test/e2e/discovery/headless_services.go index 104f8577f..97675909e 100644 --- a/test/e2e/discovery/headless_services.go +++ b/test/e2e/discovery/headless_services.go @@ -317,7 +317,7 @@ func RunHeadlessDiscoveryClusterNameTest(f *lhframework.Framework) { clusterBName, false, true, true) } -//nolint:gocognit,unparam // This really isn't that complex and would be awkward to refactor. +//nolint:gocognit // This really isn't that complex and would be awkward to refactor. func verifyHeadlessSRVRecordsWithDig(f *framework.Framework, cluster framework.ClusterIndex, service *corev1.Service, targetPod *corev1.PodList, hostNameList, domains []string, clusterName string, withPort, withcluster, shouldContain bool, ) { diff --git a/test/e2e/discovery/service_discovery.go b/test/e2e/discovery/service_discovery.go index 014d854a9..6964c7b1d 100644 --- a/test/e2e/discovery/service_discovery.go +++ b/test/e2e/discovery/service_discovery.go @@ -556,7 +556,6 @@ func RunServicesClusterAvailabilityMultiClusterTest(f *lhframework.Framework) { checkedDomains, "", false) } -//nolint:unparam // Keep srcCluster as a parameter for consistency and possible future extensions func verifySRVWithDig(f *framework.Framework, srcCluster framework.ClusterIndex, service *corev1.Service, targetPod *corev1.PodList, domains []string, clusterName string, withPort, shouldContain bool, ) { diff --git a/test/e2e/discovery/statefulsets.go b/test/e2e/discovery/statefulsets.go index ff974bce2..d6aee2a2b 100644 --- a/test/e2e/discovery/statefulsets.go +++ b/test/e2e/discovery/statefulsets.go @@ -192,7 +192,6 @@ func RunSSPodsAvailabilityTest(f *lhframework.Framework) { f.AwaitAggregatedServiceImport(framework.ClusterA, nginxServiceClusterB, 0) } -//nolint:unparam // `targetCluster` always receives `framework.ClusterA`. func verifyEndpointSlices(f *lhframework.Framework, targetCluster framework.ClusterIndex, netshootPodList *corev1.PodList, endpointSlices *discovery.EndpointSliceList, service *corev1.Service, verifyCount int, shouldContain bool, localClusterName string, ) {