🌱 Update golangci-lint to v1.60 and add configuration (#48)

* Add golangci-lint linters

* Remove unused function arguments

* Add configuration file

* Address issues

* Add more linters

* Address issues

* Address issues

* Use 0.5

* Address issues

* Fix float comparison

* Use InDelta

.github/workflows/ci.yaml

@@ -27,8 +27,9 @@ jobs:
         go-version: '1.23.4'
         cache: false
     - name: golangci-lint
-      uses: golangci/golangci-lint-action@v3
+      uses: golangci/golangci-lint-action@v6
       with:
+        version: v1.60
         args: --timeout=10m
   # https://docs.github.com/en/actions/automating-builds-and-tests/building-and-testing-go
   test:
@@ -62,13 +63,13 @@ jobs:
         # https://docs.docker.com/build/ci/github-actions/export-docker/
         load: true
     - name: Setup Helm
-      uses: azure/setup-helm@v3
+      uses: azure/setup-helm@v4
       with:
-        version: v3.12.1
+        version: v3.16.4
     - name: Helm lint
       run: helm lint --strict ./charts/cost-manager
     - name: Install kind
-      uses: helm/kind-action@v1.8.0
+      uses: helm/kind-action@v1
       with:
         install_only: true
     - name: Run E2E tests

.golangci.yml

@@ -0,0 +1,16 @@
+linters:
+  disable-all: true
+  enable:
+  - errcheck
+  - gofmt
+  - goimports
+  - gosec
+  - gosimple
+  - govet
+  - ineffassign
+  - misspell
+  - staticcheck
+  - stylecheck
+  - testifylint
+  - unused
+  - whitespace

e2e/main_test.go

@@ -90,7 +90,7 @@ func setup(ctx context.Context, image string) error {
 	}
 
 	// Install Prometheus Operator and Prometheus
-	err = installPrometheus(ctx, image)
+	err = installPrometheus()
 	if err != nil {
 		return err
 	}
@@ -224,7 +224,7 @@ podMonitor:
 	return kubernetes.WaitUntilDeploymentAvailable(ctx, kubeClient, "cost-manager", "cost-manager")
 }
 
-func installPrometheus(ctx context.Context, image string) (rerr error) {
+func installPrometheus() (rerr error) {
 	// Add prometheus-community Helm repository:
 	// https://github.com/prometheus-community/helm-charts/tree/main/charts/kube-prometheus-stack#get-helm-repository-info
 	err := runCommand("helm", "repo", "add", "prometheus-community", "https://prometheus-community.github.io/helm-charts")

e2e/pod_safe_to_evict_annotator_test.go

@@ -18,17 +18,17 @@ func TestPodSafeToEvictAnnotator(t *testing.T) {
 	ctx := context.Background()
 
 	kubeClient, _, err := kubernetes.NewClient()
-	require.Nil(t, err)
+	require.NoError(t, err)
 
 	// Wait until all Pods have expected safe-to-evict annotation
 	for {
 		success, err := allPodsHaveExpectedSafeToEvictAnnotation(ctx, kubeClient)
-		require.Nil(t, err)
+		require.NoError(t, err)
 		if success {
 			// Make sure condition still holds after 2 seconds
 			time.Sleep(2 * time.Second)
 			stillSuccess, err := allPodsHaveExpectedSafeToEvictAnnotation(ctx, kubeClient)
-			require.Nil(t, err)
+			require.NoError(t, err)
 			require.True(t, stillSuccess)
 			break
 		}

e2e/prometheus_alerts_test.go

@@ -28,31 +28,31 @@ func TestPrometheusAlerts(t *testing.T) {
 	ctx := context.Background()
 
 	kubeClient, restConfig, err := kubernetes.NewClient()
-	require.Nil(t, err)
+	require.NoError(t, err)
 
 	// Port forward to Prometheus and create client using local forwarded port
 	pod, err := kubernetes.WaitForAnyReadyPod(ctx, kubeClient, client.InNamespace("monitoring"), client.MatchingLabels{"app.kubernetes.io/name": "prometheus"})
-	require.Nil(t, err)
+	require.NoError(t, err)
 	forwardedPort, stop, err := kubernetes.PortForward(ctx, restConfig, pod.Namespace, pod.Name, 9090)
-	require.Nil(t, err)
+	require.NoError(t, err)
 	defer func() {
 		err := stop()
-		require.Nil(t, err)
+		require.NoError(t, err)
 	}()
 	prometheusAddress := fmt.Sprintf("http://127.0.0.1:%d", forwardedPort)
 	prometheusClient, err := api.NewClient(api.Config{
 		Address: prometheusAddress,
 	})
-	require.Nil(t, err)
+	require.NoError(t, err)
 	prometheusAPI := prometheusv1.NewAPI(prometheusClient)
 
 	t.Log("Waiting for cost-manager alerts to be registered with Prometheus...")
 	costManagerPrometheusRule := &monitoringv1.PrometheusRule{}
 	err = kubeClient.Get(ctx, client.ObjectKey{Name: "cost-manager", Namespace: "cost-manager"}, costManagerPrometheusRule)
-	require.Nil(t, err)
+	require.NoError(t, err)
 	for {
 		prometheusRules, err := prometheusAPI.Rules(ctx)
-		require.Nil(t, err)
+		require.NoError(t, err)
 		if prometheusHasAllCostManagerAlerts(costManagerPrometheusRule, prometheusRules) {
 			break
 		}
@@ -63,7 +63,7 @@ func TestPrometheusAlerts(t *testing.T) {
 	// Wait for Prometheus to scrape cost-manager
 	for {
 		results, _, err := prometheusAPI.Query(ctx, `up{job="cost-manager",namespace="cost-manager"}`, time.Now())
-		require.Nil(t, err)
+		require.NoError(t, err)
 		if len(results.(model.Vector)) > 0 {
 			break
 		}
@@ -72,7 +72,7 @@ func TestPrometheusAlerts(t *testing.T) {
 
 	t.Logf("Ensuring all Prometheus alerts remain inactive for %s...", prometheusAlertsInactiveDuration)
 	err = waitForAllPrometheusAlertsToRemainInactive(ctx, prometheusAPI)
-	require.Nil(t, err)
+	require.NoError(t, err)
 	t.Logf("All Prometheus alerts remained inactive for %s!", prometheusAlertsInactiveDuration)
 }
 

e2e/spot_migrator_test.go

@@ -32,28 +32,28 @@ func TestSpotMigrator(t *testing.T) {
 	ctx := context.Background()
 
 	kubeClient, restConfig, err := kubernetes.NewClient()
-	require.Nil(t, err)
+	require.NoError(t, err)
 
 	// Port forward to Prometheus and create client using local forwarded port
 	pod, err := kubernetes.WaitForAnyReadyPod(ctx, kubeClient, client.InNamespace("monitoring"), client.MatchingLabels{"app.kubernetes.io/name": "prometheus"})
-	require.Nil(t, err)
+	require.NoError(t, err)
 	forwardedPort, stop, err := kubernetes.PortForward(ctx, restConfig, pod.Namespace, pod.Name, 9090)
-	require.Nil(t, err)
+	require.NoError(t, err)
 	defer func() {
 		err := stop()
-		require.Nil(t, err)
+		require.NoError(t, err)
 	}()
 	prometheusAddress := fmt.Sprintf("http://127.0.0.1:%d", forwardedPort)
 	prometheusClient, err := api.NewClient(api.Config{
 		Address: prometheusAddress,
 	})
-	require.Nil(t, err)
+	require.NoError(t, err)
 	prometheusAPI := prometheusv1.NewAPI(prometheusClient)
 
 	t.Log("Waiting for the failure metric to be scraped by Prometheus...")
 	for {
 		results, _, err := prometheusAPI.Query(ctx, `sum(cost_manager_spot_migrator_operation_failure_total{job="cost-manager",namespace="cost-manager"})`, time.Now())
-		require.Nil(t, err)
+		require.NoError(t, err)
 		if len(results.(model.Vector)) == 1 {
 			break
 		}
@@ -75,21 +75,21 @@ func TestSpotMigrator(t *testing.T) {
 			},
 		},
 	})
-	require.Nil(t, err)
+	require.NoError(t, err)
 	nodeList := &corev1.NodeList{}
 	err = kubeClient.List(ctx, nodeList, client.MatchingLabelsSelector{Selector: workerNodeSelector})
-	require.Nil(t, err)
-	require.Greater(t, len(nodeList.Items), 0)
+	require.NoError(t, err)
+	require.NotEmpty(t, nodeList.Items)
 	nodeName := nodeList.Items[0].Name
 
 	// Deploy a workload to the worker Node
 	namespaceName := test.GenerateResourceName(t)
 	namespace := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: namespaceName}}
 	err = kubeClient.Create(ctx, namespace)
-	require.Nil(t, err)
+	require.NoError(t, err)
 	deploymentName := namespaceName
 	deployment, err := test.GenerateDeployment(namespaceName, deploymentName)
-	require.Nil(t, err)
+	require.NoError(t, err)
 	deployment.Spec.Template.Spec.NodeSelector = map[string]string{"kubernetes.io/hostname": nodeName}
 	deployment.Spec.Template.Spec.Tolerations = []corev1.Toleration{
 		{
@@ -100,10 +100,10 @@ func TestSpotMigrator(t *testing.T) {
 		},
 	}
 	err = kubeClient.Create(ctx, deployment)
-	require.Nil(t, err)
+	require.NoError(t, err)
 	t.Logf("Waiting for Deployment %s/%s to become available...", deployment.Namespace, deployment.Name)
 	err = kubernetes.WaitUntilDeploymentAvailable(ctx, kubeClient, namespaceName, deploymentName)
-	require.Nil(t, err)
+	require.NoError(t, err)
 	t.Logf("Deployment %s/%s is available!", deployment.Namespace, deployment.Name)
 
 	// Create PodDisruptionBudget...
@@ -121,7 +121,7 @@ func TestSpotMigrator(t *testing.T) {
 		},
 	}
 	err = kubeClient.Create(ctx, pdb)
-	require.Nil(t, err)
+	require.NoError(t, err)
 	// ...and wait until it's blocking eviction
 	pdbName := pdb.Name
 	listerWatcher := kubernetes.NewListerWatcher(ctx, kubeClient, &policyv1.PodDisruptionBudgetList{}, &client.ListOptions{Namespace: pdb.Namespace})
@@ -133,15 +133,15 @@ func TestSpotMigrator(t *testing.T) {
 		return pdb.Name == pdbName && pdb.Status.DisruptionsAllowed == 0 && pdb.Generation == pdb.Status.ObservedGeneration, nil
 	}
 	_, err = watch.UntilWithSync(ctx, listerWatcher, &policyv1.PodDisruptionBudget{}, nil, condition)
-	require.Nil(t, err)
+	require.NoError(t, err)
 
 	// Label worker Node as an on-demand Node to give spot-migrator something to drain
 	node := &corev1.Node{}
 	err = kubeClient.Get(ctx, types.NamespacedName{Name: nodeName}, node)
-	require.Nil(t, err)
+	require.NoError(t, err)
 	patch := []byte(fmt.Sprintf(`{"metadata":{"labels":{"%s":"false"}}}`, cloudproviderfake.SpotInstanceLabelKey))
 	err = kubeClient.Patch(ctx, node, client.RawPatch(types.StrategicMergePatchType, patch))
-	require.Nil(t, err)
+	require.NoError(t, err)
 
 	// Wait for the Node to be marked as unschedulable. This should not take any longer than 2
 	// minutes since spot-migrator is configured with a 1 minute migration interval
@@ -157,7 +157,7 @@ func TestSpotMigrator(t *testing.T) {
 		return node.Name == nodeName && node.Spec.Unschedulable, nil
 	}
 	_, err = watch.UntilWithSync(ctxWithTimeout, listerWatcher, &corev1.Node{}, nil, condition)
-	require.Nil(t, err)
+	require.NoError(t, err)
 	t.Logf("Node %s marked as unschedulable!", nodeName)
 
 	// Make sure that the PodDisruptionBudget blocks eviction
@@ -170,11 +170,11 @@ func TestSpotMigrator(t *testing.T) {
 
 	// Delete PodDisruptionBudget...
 	err = kubeClient.Delete(ctx, pdb)
-	require.Nil(t, err)
+	require.NoError(t, err)
 	// ...and wait for the Deployment to become unavailable
 	t.Logf("Waiting for Deployment %s/%s to become unavailable...", deployment.Namespace, deployment.Name)
 	err = kubernetes.WaitUntilDeploymentUnavailable(ctx, kubeClient, namespaceName, deploymentName)
-	require.Nil(t, err)
+	require.NoError(t, err)
 	t.Logf("Deployment %s/%s is unavailable!", deployment.Namespace, deployment.Name)
 
 	// Delete Node; typically this would be done by the node controller but we simulate it here:
@@ -183,17 +183,17 @@ func TestSpotMigrator(t *testing.T) {
 	// or waiting for Node deletion are discovered due to the failure metric being incremented
 	time.Sleep(10 * time.Second)
 	err = kubeClient.Delete(ctx, node)
-	require.Nil(t, err)
+	require.NoError(t, err)
 
 	// Delete Namespace
 	err = kubeClient.Delete(ctx, namespace)
-	require.Nil(t, err)
+	require.NoError(t, err)
 
 	// Make sure that the failure metric was never incremented
 	results, _, err := prometheusAPI.Query(ctx, `sum(sum_over_time(cost_manager_spot_migrator_operation_failure_total{job="cost-manager",namespace="cost-manager"}[1h]))`, time.Now())
-	require.Nil(t, err)
-	require.Equal(t, 1, len(results.(model.Vector)))
-	require.True(t, results.(model.Vector)[0].Value == 0, "spot-migrator failure metric was incremented!")
+	require.NoError(t, err)
+	require.Len(t, results.(model.Vector), 1)
+	require.InDelta(t, 0, float64(results.(model.Vector)[0].Value), 0.5, "spot-migrator failure metric was incremented!")
 
 	// Finally, we verify that all control plane Nodes are still schedulable
 	controlPlaneNodeSelector, err := metav1.LabelSelectorAsSelector(&metav1.LabelSelector{
@@ -204,11 +204,11 @@ func TestSpotMigrator(t *testing.T) {
 			},
 		},
 	})
-	require.Nil(t, err)
+	require.NoError(t, err)
 	nodeList = &corev1.NodeList{}
 	err = kubeClient.List(ctx, nodeList, client.MatchingLabelsSelector{Selector: controlPlaneNodeSelector})
-	require.Nil(t, err)
-	require.Greater(t, len(nodeList.Items), 0)
+	require.NoError(t, err)
+	require.NotEmpty(t, nodeList.Items)
 	for _, node := range nodeList.Items {
 		require.False(t, node.Spec.Unschedulable)
 	}

hack/notes/notes.go

@@ -53,6 +53,7 @@ func run(from, to string) error {
 		}
 	}
 
+	//#nosec G204
 	cmd := exec.Command("git", "rev-list", fmt.Sprintf("%s..%s", from, to), "--pretty=format:%B")
 	out, err := cmd.CombinedOutput()
 	if err != nil {
@@ -112,6 +113,7 @@ func run(from, to string) error {
 }
 
 func previousTag(to string) (string, error) {
+	//#nosec G204
 	cmd := exec.Command("git", "describe", "--abbrev=0", "--tags", fmt.Sprintf("%s^", to))
 	out, err := cmd.CombinedOutput()
 	if err != nil {

pkg/cloudprovider/cloud_provider_test.go

@@ -9,5 +9,5 @@ import (
 
 func TestNewCloudProvider(t *testing.T) {
 	_, err := NewCloudProvider(context.Background(), "")
-	require.NotNil(t, err)
+	require.Error(t, err)
 }

pkg/cloudprovider/gcp/cloud_provider_test.go

@@ -77,7 +77,7 @@ func TestIsSpotInstance(t *testing.T) {
 		t.Run(name, func(t *testing.T) {
 			cloudProvider := &CloudProvider{}
 			isSpotInstance, err := cloudProvider.IsSpotInstance(context.Background(), test.node)
-			require.Nil(t, err)
+			require.NoError(t, err)
 			require.Equal(t, test.isSpotInstance, isSpotInstance)
 		})
 	}

pkg/cloudprovider/gcp/compute_test.go

@@ -24,6 +24,6 @@ func TestGetManagedInstanceGroupFromInstance(t *testing.T) {
 		},
 	}
 	managedInstanceGroupName, err := getManagedInstanceGroupFromInstance(instance)
-	require.Nil(t, err)
+	require.NoError(t, err)
 	require.Equal(t, "my-managed-instance-group", managedInstanceGroupName)
 }

pkg/cloudprovider/gcp/provider_test.go

@@ -9,7 +9,7 @@ import (
 func TestParseProviderID(t *testing.T) {
 	providerID := "gce://my-project/my-zone/my-instance"
 	project, zone, instanceName, err := parseProviderID(providerID)
-	require.Nil(t, err)
+	require.NoError(t, err)
 	require.Equal(t, "my-project", project)
 	require.Equal(t, "my-zone", zone)
 	require.Equal(t, "my-instance", instanceName)

pkg/config/config_test.go

@@ -105,10 +105,10 @@ foo: bar
 		t.Run(name, func(t *testing.T) {
 			config, err := decode(test.configData)
 			if test.valid {
-				require.Nil(t, err)
+				require.NoError(t, err)
 				require.Equal(t, test.config, config)
 			} else {
-				require.NotNil(t, err)
+				require.Error(t, err)
 			}
 		})
 	}
@@ -140,9 +140,9 @@ func TestValidate(t *testing.T) {
 		t.Run(name, func(t *testing.T) {
 			err := validate(test.config)
 			if test.valid {
-				require.Nil(t, err)
+				require.NoError(t, err)
 			} else {
-				require.NotNil(t, err)
+				require.Error(t, err)
 			}
 		})
 	}