Fix remaining linter warnings and add gocritic

.golangci.json

@@ -14,6 +14,7 @@
       "errorlint",
       "funlen",
       "ginkgolinter",
+      "gocritic",
       "gocyclo",
       "gosec",
       "govet",

benchmarks/cmd/dataset.go

@@ -88,12 +88,12 @@ func calculate(sample *parser.Sample, dsPop Dataset, scores scoresByXP) {
 			}
 
 			if _, ok := sample.Experiments[experiment]; !ok {
-				//fmt.Printf("missing experiment %s\n", name)
+				// fmt.Printf("missing experiment %s\n", name)
 				continue
 			}
 
 			if _, ok := sample.Experiments[experiment].Measurements[measurement]; !ok {
-				//fmt.Printf("missing measurement %s for experiments %s\n", measurement, name)
+				// fmt.Printf("missing measurement %s for experiments %s\n", measurement, name)
 				continue
 			}
 
@@ -104,7 +104,7 @@ func calculate(sample *parser.Sample, dsPop Dataset, scores scoresByXP) {
 			// calculate zscore
 			m := sample.Experiments[experiment].Measurements[measurement]
 			zscore := stat.StdScore(m.Value, mean, stddev)
-			//fmt.Printf("zscore %s - %s %v %v %v\n", experiment, measurement, m, mean, zscore)
+			// fmt.Printf("zscore %s - %s %v %v %v\n", experiment, measurement, m, mean, zscore)
 
 			// store in dsPop
 			sg.Mean = mean
@@ -125,7 +125,7 @@ func calculate(sample *parser.Sample, dsPop Dataset, scores scoresByXP) {
 		avg := stat.Mean(xp.ZScores, xp.Weights)
 		xp.MeanZScore = avg
 		scores[name] = xp
-		//fmt.Printf("%s %v %v %v\n", name, avg, xp.ZScores, xp.Weights)
+		// fmt.Printf("%s %v %v %v\n", name, avg, xp.ZScores, xp.Weights)
 	}
 
 }

benchmarks/cmd/report.go

@@ -195,11 +195,12 @@ func newTableZScore(rows map[string]Row) *table.Table {
 			r.AppendCell(table.C(fmt.Sprintf("%.02fs", row.StdDevDuration)))
 			r.AppendCell(table.C(fmt.Sprintf("%.02f", row.ZScore)))
 		}
-		if row.ZScore < 0 {
+		switch {
+		case row.ZScore < 0:
 			r.AppendCell(table.C("better"))
-		} else if row.ZScore > 0 {
+		case row.ZScore > 0:
 			r.AppendCell(table.C("worse"))
-		} else {
+		default:
 			r.AppendCell(table.C(""))
 		}
 

benchmarks/record/record.go

@@ -172,11 +172,12 @@ func Nodes(ctx context.Context, experiment *gm.Experiment) {
 		for _, image := range node.Status.Images {
 			name := ""
 			// in k3d, the first image name contains the hash, not the tag
-			if len(image.Names) == 0 {
+			switch {
+			case len(image.Names) == 0:
 				continue
-			} else if len(image.Names) > 1 {
+			case len(image.Names) > 1:
 				name = image.Names[1]
-			} else {
+			default:
 				name = image.Names[0]
 			}
 			images[name] = struct{}{}

integrationtests/cli/apply/apply_online_test.go

@@ -35,7 +35,7 @@ var _ = Describe("Fleet apply online", Label("online"), func() {
 	)
 
 	JustBeforeEach(func() {
-		//Setting up all the needed mocked interfaces for the test
+		// Setting up all the needed mocked interfaces for the test
 		ctrl = gomock.NewController(GinkgoT())
 		clientMock = mocks.NewMockK8sClient(ctrl)
 		clientMock.EXPECT().Get(
@@ -68,7 +68,7 @@ var _ = Describe("Fleet apply online", Label("online"), func() {
 		BeforeEach(func() {
 			name = "labels_update"
 			dirs = []string{cli.AssetsPath + "labels_update"}
-			//bundle in the cluster
+			// bundle in the cluster
 			oldBundle = &fleet.Bundle{
 				ObjectMeta: metav1.ObjectMeta{
 					Labels: map[string]string{
@@ -129,7 +129,7 @@ data:
 		BeforeEach(func() {
 			name = "labels_update"
 			dirs = []string{cli.AssetsPath + "labels_update"}
-			//bundle in the cluster
+			// bundle in the cluster
 			oldBundle = &fleet.Bundle{
 				ObjectMeta: metav1.ObjectMeta{
 					Namespace: "foo",
@@ -160,7 +160,7 @@ data:
 			ts := metav1.NewTime(time.Now())
 			name = "labels_update"
 			dirs = []string{cli.AssetsPath + "labels_update"}
-			//bundle in the cluster
+			// bundle in the cluster
 			oldBundle = &fleet.Bundle{
 				ObjectMeta: metav1.ObjectMeta{
 					Namespace:         "foo",
@@ -224,7 +224,7 @@ data:
 		BeforeEach(func() {
 			name = "labels_update"
 			dirs = []string{cli.AssetsPath + "labels_update"}
-			//bundle in the cluster
+			// bundle in the cluster
 			oldBundle = &fleet.Bundle{
 				ObjectMeta: metav1.ObjectMeta{
 					Namespace: "foo",

integrationtests/cli/helpers.go

@@ -71,10 +71,8 @@ func IsResourcePresentInBundle(resourcePath string, resources []v1alpha1.BundleR
 			if resource.Content == resourceFileEncoded {
 				return true, nil
 			}
-		} else {
-			if strings.ReplaceAll(resource.Content, "\n", "") == strings.ReplaceAll(string(resourceFile), "\n", "") {
-				return true, nil
-			}
+		} else if strings.ReplaceAll(resource.Content, "\n", "") == strings.ReplaceAll(string(resourceFile), "\n", "") {
+			return true, nil
 		}
 	}
 

integrationtests/controller/bundle/userid_logging_test.go

@@ -72,9 +72,7 @@ var _ = Describe("Bundle UserID logging", func() {
 			return bundle.Status.ObservedGeneration
 		}).Should(BeNumerically(">", 0))
 
-		Eventually(func() string {
-			return logsBuffer.String()
-		}).Should(ContainSubstring(bundle.Name))
+		Eventually(logsBuffer.String).Should(ContainSubstring(bundle.Name))
 	}
 
 	When("Bundle has user ID label", func() {

integrationtests/controller/bundledeployment/userid_logging_test.go

@@ -63,9 +63,7 @@ var _ = Describe("UserID logging", func() {
 		})
 
 		It("logs userID in reconciliation", func() {
-			Eventually(func() string {
-				return logsBuffer.String()
-			}).Should(ContainSubstring(bd.Name))
+			Eventually(logsBuffer.String).Should(ContainSubstring(bd.Name))
 
 			logs := logsBuffer.String()
 			bdLogs := utils.ExtractResourceLogs(logs, bd.Name)
@@ -117,9 +115,7 @@ var _ = Describe("UserID logging", func() {
 		})
 
 		It("does not log userID in reconciliation", func() {
-			Eventually(func() string {
-				return logsBuffer.String()
-			}).Should(ContainSubstring(bd.Name))
+			Eventually(logsBuffer.String).Should(ContainSubstring(bd.Name))
 
 			logs := logsBuffer.String()
 			bdLogs := utils.ExtractResourceLogs(logs, bd.Name)

integrationtests/gitjob/controller/userid_logging_test.go

@@ -55,9 +55,7 @@ var _ = Describe("GitRepo UserID logging", func() {
 		})
 
 		It("includes userID in log output", func() {
-			Eventually(func() string {
-				return logsBuffer.String()
-			}, timeout).Should(Or(
+			Eventually(logsBuffer.String, timeout).Should(Or(
 				ContainSubstring(`"userID":"`+userID+`"`),
 				ContainSubstring(`"userID": "`+userID+`"`),
 			))
@@ -74,9 +72,7 @@ var _ = Describe("GitRepo UserID logging", func() {
 		})
 
 		It("does not include userID in log output", func() {
-			Eventually(func() string {
-				return logsBuffer.String()
-			}, timeout).Should(ContainSubstring(gitrepo.Name))
+			Eventually(logsBuffer.String, timeout).Should(ContainSubstring(gitrepo.Name))
 
 			logs := logsBuffer.String()
 			gitrepoLogs := utils.ExtractResourceLogs(logs, gitrepo.Name)

integrationtests/helmops/controller/userid_logging_test.go

@@ -60,9 +60,7 @@ var _ = Describe("HelmOp UserID logging", func() {
 		})
 
 		It("includes userID in log output", func() {
-			Eventually(func() string {
-				return logsBuffer.String()
-			}, timeout).Should(Or(
+			Eventually(logsBuffer.String, timeout).Should(Or(
 				ContainSubstring(`"userID":"`+userID+`"`),
 				ContainSubstring(`"userID": "`+userID+`"`),
 			))
@@ -79,9 +77,7 @@ var _ = Describe("HelmOp UserID logging", func() {
 		})
 
 		It("does not include userID in log output", func() {
-			Eventually(func() string {
-				return logsBuffer.String()
-			}, timeout).Should(ContainSubstring(helmop.Name))
+			Eventually(logsBuffer.String, timeout).Should(ContainSubstring(helmop.Name))
 
 			logs := logsBuffer.String()
 			helmopLogs := utils.ExtractResourceLogs(logs, helmop.Name)

internal/bundlereader/charturl.go

@@ -72,7 +72,7 @@ func ChartVersion(ctx context.Context, location fleet.HelmOptions, a Auth) (stri
 	}
 
 	if !strings.HasSuffix(location.Repo, "/") {
-		location.Repo = location.Repo + "/"
+		location.Repo += "/"
 	}
 
 	chart, err := getHelmChartVersion(ctx, location, a)
@@ -104,7 +104,7 @@ func chartURL(ctx context.Context, location fleet.HelmOptions, auth Auth, isHelm
 	}
 
 	if !strings.HasSuffix(location.Repo, "/") {
-		location.Repo = location.Repo + "/"
+		location.Repo += "/"
 	}
 
 	chart, err := getHelmChartVersion(ctx, location, auth)

internal/bundlereader/helm_test.go

@@ -85,7 +85,7 @@ func checksumPrefix(helm *fleet.HelmOptions) string {
 	if helm == nil {
 		return "none"
 	}
-	return fmt.Sprintf(".chart/%x", sha256.Sum256([]byte(helm.Chart + ":" + helm.Repo + ":" + helm.Version)[:]))
+	return fmt.Sprintf(".chart/%x", sha256.Sum256([]byte(helm.Chart+":"+helm.Repo+":"+helm.Version)))
 }
 
 func createChartDir(dir string) error {

internal/bundlereader/loaddirectory.go

@@ -110,9 +110,7 @@ func (xt *ignoreTree) findNode(path string, isDir bool, nodesRoute []*ignoreTree
 
 	for _, c := range xt.children {
 		if steps := c.findNode(path, isDir, nodesRoute); steps != nil {
-			crossed := append(nodesRoute, steps...)
-
-			return crossed
+			return append(nodesRoute, steps...)
 		}
 	}
 

internal/bundlereader/loaddirectory_test.go

@@ -584,10 +584,8 @@ func TestGetContent(t *testing.T) {
 			files, err := bundlereader.GetContent(context.Background(), root, c.source, "", c.auth, false, ignoreApplyConfigs)
 			if c.expectedErr == nil {
 				require.NoError(t, err)
-			} else {
-				if !c.expectedErr.Match([]byte(err.Error())) {
-					assert.Failf(t, "expected error to match", "expected: %s, got: %s", c.expectedErr.String(), err.Error())
-				}
+			} else if !c.expectedErr.Match([]byte(err.Error())) {
+				assert.Failf(t, "expected error to match", "expected: %s, got: %s", c.expectedErr.String(), err.Error())
 			}
 
 			assert.Len(t, files, len(c.expectedFiles))

internal/bundlereader/resources.go

@@ -258,7 +258,7 @@ func checksum(helm *fleet.HelmOptions) string {
 	if helm == nil {
 		return "none"
 	}
-	return fmt.Sprintf(".chart/%x", sha256.Sum256([]byte(helm.Chart + ":" + helm.Repo + ":" + helm.Version)[:]))
+	return fmt.Sprintf(".chart/%x", sha256.Sum256([]byte(helm.Chart+":"+helm.Repo+":"+helm.Version)))
 }
 
 // loadDirectories loads all resources from a bundle's directories

internal/cmd/agent/deployer/deployer.go

@@ -121,7 +121,8 @@ func (d *Deployer) helmdeploy(ctx context.Context, logger logr.Logger, bd *fleet
 		m   *manifest.Manifest
 		err error
 	)
-	if bd.Spec.OCIContents {
+	switch {
+	case bd.Spec.OCIContents:
 		oci := ocistorage.NewOCIWrapper()
 		secretID := client.ObjectKey{Name: manifestID, Namespace: bd.Namespace}
 		opts, err := ocistorage.ReadOptsFromSecret(ctx, d.upstreamClient, secretID)
@@ -142,12 +143,12 @@ func (d *Deployer) helmdeploy(ctx context.Context, logger logr.Logger, bd *fleet
 		if actualID != manifestID {
 			return "", fmt.Errorf("invalid or corrupt manifest. Expecting id: %q, got %q", manifestID, actualID)
 		}
-	} else if bd.Spec.HelmChartOptions != nil {
+	case bd.Spec.HelmChartOptions != nil:
 		m, err = bundlereader.GetManifestFromHelmChart(ctx, d.upstreamClient, bd)
 		if err != nil {
 			return "", err
 		}
-	} else {
+	default:
 		m, err = d.lookup.Get(ctx, d.upstreamClient, manifestID)
 		if err != nil {
 			return "", err

internal/cmd/agent/deployer/desiredset/style.go

@@ -36,11 +36,12 @@ func getMergeStyle(gvk schema.GroupVersionKind) (types.PatchType, strategicpatch
 
 	versionedObject, err := scheme.Scheme.New(gvk)
 
-	if runtime.IsNotRegisteredError(err) || gvk.Kind == "CustomResourceDefinition" {
+	switch {
+	case runtime.IsNotRegisteredError(err) || gvk.Kind == "CustomResourceDefinition":
 		patchType = types.MergePatchType
-	} else if err != nil {
+	case err != nil:
 		return patchType, nil, err
-	} else {
+	default:
 		patchType = types.StrategicMergePatchType
 		lookupPatchMeta, err = strategicpatch.NewPatchMetaFromStruct(versionedObject)
 		if err != nil {

internal/cmd/agent/deployer/internal/diff/diff.go

@@ -76,36 +76,35 @@ func Diff(config, live *unstructured.Unstructured, opts ...Option) (*DiffResult,
 	orig, err := GetLastAppliedConfigAnnotation(live)
 	if err != nil {
 		o.log.V(1).Info(fmt.Sprintf("Failed to get last applied configuration: %v", err))
-	} else {
-		if orig != nil && config != nil {
-			Normalize(orig, opts...)
-			dr, err := ThreeWayDiff(orig, config, live)
-			if err == nil {
-				return dr, nil
-			}
-			o.log.V(1).Info(fmt.Sprintf("three-way diff calculation failed: %v. Falling back to two-way diff", err))
+	} else if orig != nil && config != nil {
+		Normalize(orig, opts...)
+		dr, err := ThreeWayDiff(orig, config, live)
+		if err == nil {
+			return dr, nil
 		}
+		o.log.V(1).Info(fmt.Sprintf("three-way diff calculation failed: %v. Falling back to two-way diff", err))
 	}
 	return TwoWayDiff(config, live)
 }
 
 // TwoWayDiff performs a three-way diff and uses specified config as a recently applied config
 func TwoWayDiff(config, live *unstructured.Unstructured) (*DiffResult, error) {
-	if live != nil && config != nil {
+	switch {
+	case live != nil && config != nil:
 		return ThreeWayDiff(config, config.DeepCopy(), live)
-	} else if live != nil {
+	case live != nil:
 		liveData, err := json.Marshal(live)
 		if err != nil {
 			return nil, err
 		}
 		return &DiffResult{Modified: false, NormalizedLive: liveData, PredictedLive: []byte("null")}, nil
-	} else if config != nil {
+	case config != nil:
 		predictedLiveData, err := json.Marshal(config.Object)
 		if err != nil {
 			return nil, err
 		}
 		return &DiffResult{Modified: true, NormalizedLive: []byte("null"), PredictedLive: predictedLiveData}, nil
-	} else {
+	default:
 		return nil, errors.New("both live and config are null objects")
 	}
 }
@@ -417,11 +416,12 @@ func Normalize(un *unstructured.Unstructured, opts ...Option) {
 	unstructured.RemoveNestedField(un.Object, "metadata", "creationTimestamp")
 
 	gvk := un.GroupVersionKind()
-	if gvk.Group == "" && gvk.Kind == "Secret" {
+	switch {
+	case gvk.Group == "" && gvk.Kind == "Secret":
 		NormalizeSecret(un, opts...)
-	} else if gvk.Group == "rbac.authorization.k8s.io" && (gvk.Kind == "ClusterRole" || gvk.Kind == "Role") {
+	case gvk.Group == "rbac.authorization.k8s.io" && (gvk.Kind == "ClusterRole" || gvk.Kind == "Role"):
 		normalizeRole(un, o)
-	} else if gvk.Group == "" && gvk.Kind == "Endpoints" {
+	case gvk.Group == "" && gvk.Kind == "Endpoints":
 		normalizeEndpoint(un, o)
 	}
 
@@ -488,7 +488,7 @@ func normalizeEndpoint(un *unstructured.Unstructured, o options) {
 	if gvk.Group != "" || gvk.Kind != "Endpoints" {
 		return
 	}
-	//nolint: staticcheck // Endpoints is deprecated but still supported; see fleet#3760.
+	//nolint:staticcheck // Endpoints is deprecated but still supported; see fleet#3760.
 	var ep corev1.Endpoints
 	err := runtime.DefaultUnstructuredConverter.FromUnstructured(un.Object, &ep)
 	if err != nil {
@@ -629,7 +629,7 @@ func HideSecretData(target *unstructured.Unstructured, live *unstructured.Unstru
 			replacement, ok := valToReplacement[val]
 			if !ok {
 				replacement = nextReplacement
-				nextReplacement = nextReplacement + "++++"
+				nextReplacement += "++++"
 				valToReplacement[val] = replacement
 			}
 			data[k] = replacement

internal/cmd/agent/deployer/internal/diff/kubernetes_vendor/pkg/api/v1/endpoints/util.go

@@ -63,7 +63,7 @@ func hashObject(hasher hash.Hash, obj interface{}) []byte {
 	return hasher.Sum(nil)
 }
 
-//nolint:staticcheck // EndpointSubset is deprecated but still supported; see fleet#3760.
+//nolint:staticcheck,nolintlint // EndpointSubset is deprecated but still supported; see fleet#3760.
 type subsetsByHash []v1.EndpointSubset
 
 func (sl subsetsByHash) Len() int      { return len(sl) }