Add more linters and fix linter errors (#81)

* Add more linters and fix linter errors

* Specify golangci-lint version

* Fix punctuations at the end of error messages

* More punctuation fixes

* Specify the version of staticcheck

Co-authored-by: Wonkun Kim <[email protected]>

Author: wongni

.golangci.yml

@@ -17,12 +17,16 @@ linters-settings:
       WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
       See the License for the specific language governing permissions and
       limitations under the License.
+  gocyclo:
+    min-complexity: 15
 
 linters:
   enable:
     - gosec
     - goheader
     - revive
+    - gocyclo
+    - misspell
 
 run:
   issues-exit-code: 1

Makefile

@@ -114,10 +114,11 @@ vet: ## Run go vet on the whole project.
 	go vet ./...
 
 .PHONY: lint
-lint: bin/golangci-lint generate-mocks ## Run linting for the project.
+lint: bin/golangci-lint bin/staticcheck generate-mocks ## Run linting for the project.
 	go fmt ./...
 	go vet ./...
 	golangci-lint run -v --timeout 360s ./...
+	staticcheck ./...
 	@ # The below string of commands checks that ginkgo isn't present in the controllers.
 	@(grep ginkgo ${PROJECT_DIR}/controllers/cloudstack*_controller.go && \
 		echo "Remove ginkgo from controllers. This is probably an artifact of testing." \
@@ -137,11 +138,13 @@ undeploy: bin/kustomize ## Undeploy controller from the K8s cluster specified in
 ##@ Binaries
 
 .PHONY: binaries
-binaries: bin/controller-gen bin/kustomize bin/ginkgo bin/golangci-lint bin/mockgen bin/kubectl ## Locally install all needed bins.
+binaries: bin/controller-gen bin/kustomize bin/ginkgo bin/golangci-lint bin/staticcheck bin/mockgen bin/kubectl ## Locally install all needed bins.
 bin/controller-gen: ## Install controller-gen to bin.
 	GOBIN=$(PROJECT_DIR)/bin go install sigs.k8s.io/controller-tools/cmd/[email protected]
 bin/golangci-lint: ## Install golangci-lint to bin.
-	GOBIN=$(PROJECT_DIR)/bin go install github.com/golangci/golangci-lint/cmd/[email protected]
+	GOBIN=$(PROJECT_DIR)/bin go install github.com/golangci/golangci-lint/cmd/[email protected]
+bin/staticcheck: ## Install staticcheck to bin.
+	GOBIN=$(PROJECT_DIR)/bin go install honnef.co/go/tools/cmd/[email protected]
 bin/ginkgo: ## Install ginkgo to bin.
 	GOBIN=$(PROJECT_DIR)/bin go install github.com/onsi/ginkgo/[email protected]
 bin/mockgen:

config/crd/bases/infrastructure.cluster.x-k8s.io_cloudstackmachinestatecheckers.yaml

@@ -35,8 +35,7 @@ spec:
           metadata:
             type: object
           spec:
-            description: CloudStackMachineStateCheckerSpec defines the desired state
-              of CloudStackMachineStateChecker
+            description: CloudStackMachineStateCheckerSpec
             properties:
               instanceID:
                 description: CloudStack machine instance ID

controllers/cloudstackcluster_controller.go

@@ -126,7 +126,7 @@ func (r *CloudStackClusterReconciliationRunner) SetFailureDomains() (ctrl.Result
 	return ctrl.Result{}, nil
 }
 
-// ReconcileDelete cleans up resources used by the cluster and finaly removes the CloudStackCluster's finalizers.
+// ReconcileDelete cleans up resources used by the cluster and finally removes the CloudStackCluster's finalizers.
 func (r *CloudStackClusterReconciliationRunner) ReconcileDelete() (ctrl.Result, error) {
 	r.Log.Info("Deleting CloudStackCluster.")
 	if res, err := r.GetZones(r.Zones)(); r.ShouldReturn(res, err) {
@@ -171,7 +171,7 @@ func (reconciler *CloudStackClusterReconciler) SetupWithManager(mgr ctrl.Manager
 			},
 		).Build(reconciler)
 	if err != nil {
-		return errors.Wrap(err, "building CloudStackCluster controller:")
+		return errors.Wrap(err, "building CloudStackCluster controller")
 	}
 
 	// Add a watch on CAPI Cluster objects for unpause and ready events.
@@ -187,5 +187,5 @@ func (reconciler *CloudStackClusterReconciler) SetupWithManager(mgr ctrl.Manager
 			},
 			DeleteFunc: func(e event.DeleteEvent) bool { return false },
 			CreateFunc: func(e event.CreateEvent) bool { return false }})
-	return errors.Wrap(err, "building CloudStackCluster controller:")
+	return errors.Wrap(err, "building CloudStackCluster controller")
 }

controllers/cloudstackisolatednetwork_controller.go

@@ -77,17 +77,17 @@ func (r *CloudStackIsoNetReconciliationRunner) Reconcile() (retRes ctrl.Result,
 	// Set endpoint of CloudStackCluster if it is not currently set. (uses patcher to do so)
 	csClusterPatcher, err := patch.NewHelper(r.CSCluster, r.K8sClient)
 	if err != nil {
-		return r.ReturnWrappedError(retErr, "setting up CloudStackCluster patcher:")
+		return r.ReturnWrappedError(retErr, "setting up CloudStackCluster patcher")
 	}
 	if err := r.CSUser.GetOrCreateIsolatedNetwork(r.Zone, r.ReconciliationSubject, r.CSCluster); err != nil {
 		return ctrl.Result{}, err
 	}
 	// Tag the created network.
 	if err := r.CSUser.AddClusterTag(cloud.ResourceTypeNetwork, r.ReconciliationSubject.Spec.ID, r.CSCluster); err != nil {
-		return ctrl.Result{}, errors.Wrapf(err, "tagging network with id %s:", r.ReconciliationSubject.Spec.ID)
+		return ctrl.Result{}, errors.Wrapf(err, "tagging network with id %s", r.ReconciliationSubject.Spec.ID)
 	}
 	if err := csClusterPatcher.Patch(r.RequestCtx, r.CSCluster); err != nil {
-		return r.ReturnWrappedError(err, "patching endpoint update to CloudStackCluster:")
+		return r.ReturnWrappedError(err, "patching endpoint update to CloudStackCluster")
 	}
 
 	r.ReconciliationSubject.Status.Ready = true

controllers/cloudstackmachine_controller.go

@@ -35,7 +35,6 @@ import (
 
 	infrav1 "github.com/aws/cluster-api-provider-cloudstack/api/v1beta1"
 	"github.com/aws/cluster-api-provider-cloudstack/controllers/utils"
-	csCtrlrUtils "github.com/aws/cluster-api-provider-cloudstack/controllers/utils"
 	"github.com/aws/cluster-api-provider-cloudstack/pkg/cloud"
 	capiv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 )
@@ -49,7 +48,7 @@ import (
 
 // CloudStackMachineReconciliationRunner is a ReconciliationRunner with extensions specific to CloudStack machine reconciliation.
 type CloudStackMachineReconciliationRunner struct {
-	csCtrlrUtils.ReconciliationRunner
+	utils.ReconciliationRunner
 	ReconciliationSubject *infrav1.CloudStackMachine
 	CAPIMachine           *capiv1.Machine
 	StateChecker          *infrav1.CloudStackMachineStateChecker
@@ -61,7 +60,7 @@ type CloudStackMachineReconciliationRunner struct {
 
 // CloudStackMachineReconciler reconciles a CloudStackMachine object
 type CloudStackMachineReconciler struct {
-	csCtrlrUtils.ReconcilerBase
+	utils.ReconcilerBase
 }
 
 // Initialize a new CloudStackMachine reconciliation runner with concrete types and initialized member fields.
@@ -75,7 +74,7 @@ func NewCSMachineReconciliationRunner() *CloudStackMachineReconciliationRunner {
 	r.AffinityGroup = &infrav1.CloudStackAffinityGroup{}
 	r.FailureDomain = &infrav1.CloudStackZone{}
 	// Setup the base runner. Initializes pointers and links reconciliation methods.
-	r.ReconciliationRunner = csCtrlrUtils.NewRunner(r, r.ReconciliationSubject)
+	r.ReconciliationRunner = utils.NewRunner(r, r.ReconciliationSubject)
 	return r
 }
 
@@ -112,9 +111,9 @@ func (r *CloudStackMachineReconciliationRunner) ConsiderAffinity() (ctrl.Result,
 		return ctrl.Result{}, nil
 	}
 
-	agName, err := csCtrlrUtils.GenerateAffinityGroupName(*r.ReconciliationSubject, r.CAPIMachine)
+	agName, err := utils.GenerateAffinityGroupName(*r.ReconciliationSubject, r.CAPIMachine)
 	if err != nil {
-		r.Log.Info("getting affinity group name:", err)
+		r.Log.Info("getting affinity group name", err)
 	}
 
 	if res, err := r.GetOrCreateAffinityGroup(agName, r.ReconciliationSubject.Spec.Affinity, r.AffinityGroup)(); r.ShouldReturn(res, err) {
@@ -241,7 +240,7 @@ func (r *CloudStackMachineReconciliationRunner) GetOrCreateMachineStateChecker()
 		Status:     infrav1.CloudStackMachineStateCheckerStatus{Ready: false},
 	}
 
-	if err := r.K8sClient.Create(r.RequestCtx, csMachineStateChecker); err != nil && !csCtrlrUtils.ContainsAlreadyExistsSubstring(err) {
+	if err := r.K8sClient.Create(r.RequestCtx, csMachineStateChecker); err != nil && !utils.ContainsAlreadyExistsSubstring(err) {
 		return r.ReturnWrappedError(err, "error encountered when creating CloudStackMachineStateChecker")
 	}
 

controllers/cloudstackzone_controller.go

@@ -82,11 +82,11 @@ func (r *CloudStackZoneReconciliationRunner) Reconcile() (retRes ctrl.Result, re
 	r.Log.V(1).Info("Reconciling CloudStackZone.", "zoneSpec", r.ReconciliationSubject.Spec)
 	// Start by purely data fetching information about the zone and specified network.
 	if err := r.CSUser.ResolveZone(r.ReconciliationSubject); err != nil {
-		return ctrl.Result{}, errors.Wrap(err, "resolving CloudStack zone information:")
+		return ctrl.Result{}, errors.Wrap(err, "resolving CloudStack zone information")
 	}
 	if err := r.CSUser.ResolveNetworkForZone(r.ReconciliationSubject); err != nil &&
 		!csCtrlrUtils.ContainsNoMatchSubstring(err) {
-		return ctrl.Result{}, errors.Wrap(err, "resolving Cloudstack network information:")
+		return ctrl.Result{}, errors.Wrap(err, "resolving Cloudstack network information")
 	}
 
 	// Address Isolated Networks.

controllers/utils/affinity_group.go

@@ -67,7 +67,7 @@ func (r *ReconciliationRunner) GetOrCreateAffinityGroup(name string, affinityTyp
 		}
 
 		if err := r.K8sClient.Create(r.RequestCtx, ag); err != nil && !ContainsAlreadyExistsSubstring(err) {
-			return r.ReturnWrappedError(err, "creating affinity group CRD:")
+			return r.ReturnWrappedError(err, "creating affinity group CRD")
 		}
 		return ctrl.Result{}, nil
 	}

controllers/utils/base_reconciler.go

@@ -146,7 +146,7 @@ func (r *ReconciliationRunner) GetCAPICluster() (ctrl.Result, error) {
 		Name:      name,
 	}
 	if err := r.K8sClient.Get(r.RequestCtx, key, r.CAPICluster); err != nil {
-		return ctrl.Result{}, errors.Wrapf(client.IgnoreNotFound(err), "getting CAPI Cluster %s:", name)
+		return ctrl.Result{}, errors.Wrapf(client.IgnoreNotFound(err), "getting CAPI Cluster %s", name)
 	} else if r.CAPICluster.Name == "" {
 		return r.RequeueWithMessage("Cluster not fetched.")
 	}
@@ -167,7 +167,7 @@ func (r *ReconciliationRunner) GetCSCluster() (ctrl.Result, error) {
 		Name:      name,
 	}
 	err := r.K8sClient.Get(r.RequestCtx, key, r.CSCluster)
-	return ctrl.Result{}, errors.Wrapf(client.IgnoreNotFound(err), "getting CAPI Cluster %s:", name)
+	return ctrl.Result{}, errors.Wrapf(client.IgnoreNotFound(err), "getting CAPI Cluster %s", name)
 }
 
 // CheckOwnedCRDsForReadiness queries for the readiness of CRDs of GroupVersionKind passed.
@@ -181,7 +181,7 @@ func (r *ReconciliationRunner) CheckOwnedCRDsForReadiness(gvks ...schema.GroupVe
 			potentiallyOnwedObjs.SetGroupVersionKind(gvk)
 			err := r.K8sClient.List(r.RequestCtx, potentiallyOnwedObjs)
 			if err != nil {
-				return ctrl.Result{}, errors.Wrapf(err, "requesting owned objects with gvk %s:", gvk)
+				return ctrl.Result{}, errors.Wrapf(err, "requesting owned objects with gvk %s", gvk)
 			}
 
 			// Filter objects not actually owned by reconciliation subject via owner reference UID.
@@ -199,10 +199,10 @@ func (r *ReconciliationRunner) CheckOwnedCRDsForReadiness(gvks ...schema.GroupVe
 			// Check that found objects are ready.
 			for _, owned := range ownedObjs {
 				if ready, found, err := unstructured.NestedBool(owned.Object, "status", "ready"); err != nil {
-					return ctrl.Result{}, errors.Wrapf(err, "parsing ready for object %s:", owned)
+					return ctrl.Result{}, errors.Wrapf(err, "parsing ready for object %s", owned)
 				} else if !found || !ready {
 					if name, found, err := unstructured.NestedString(owned.Object, "metadata", "name"); err != nil {
-						return ctrl.Result{}, errors.Wrapf(err, "parsing name for object %s:", owned)
+						return ctrl.Result{}, errors.Wrapf(err, "parsing name for object %s", owned)
 					} else if !found {
 						return r.RequeueWithMessage(
 							fmt.Sprintf(
@@ -257,14 +257,14 @@ func (r *ReconciliationRunner) RequeueIfCloudStackClusterNotReady() (ctrl.Result
 func (r *ReconciliationRunner) SetupPatcher() (res ctrl.Result, retErr error) {
 	r.Log.V(1).Info("Setting up patcher.")
 	r.Patcher, retErr = patch.NewHelper(r.ReconciliationSubject, r.K8sClient)
-	return res, errors.Wrapf(retErr, "setting up patcher:")
+	return res, errors.Wrapf(retErr, "setting up patcher")
 }
 
 // PatchChangesBackToAPI patches changes to the ReconciliationSubject back to the appropriate API.
 func (r *ReconciliationRunner) PatchChangesBackToAPI() (res ctrl.Result, retErr error) {
 	r.Log.V(1).Info("Patching changes back to api.")
 	err := r.Patcher.Patch(r.RequestCtx, r.ReconciliationSubject)
-	return res, errors.Wrapf(err, "patching reconciliation subject:")
+	return res, errors.Wrapf(err, "patching reconciliation subject")
 }
 
 // RequeueWithMessage is a convenience method to log requeue message and then return a result with RequeueAfter set.
@@ -358,7 +358,7 @@ func (r *ReconciliationRunner) GetReconciliationSubject() (res ctrl.Result, rete
 		r.SetReturnEarly()
 	}
 	if err != nil {
-		return ctrl.Result{}, errors.Wrap(err, "fetching reconciliation subject:")
+		return ctrl.Result{}, errors.Wrap(err, "fetching reconciliation subject")
 	}
 	return r.SetupPatcher()
 }

controllers/utils/isolated_network.go

@@ -40,7 +40,7 @@ func (r *ReconciliationRunner) GenerateIsolatedNetwork(name string) CloudStackRe
 		csIsoNet.Spec.ControlPlaneEndpoint.Port = r.CSCluster.Spec.ControlPlaneEndpoint.Port
 
 		if err := r.K8sClient.Create(r.RequestCtx, csIsoNet); err != nil && !ContainsAlreadyExistsSubstring(err) {
-			return r.ReturnWrappedError(err, "creating isolated network CRD:")
+			return r.ReturnWrappedError(err, "creating isolated network CRD")
 		}
 		return ctrl.Result{}, nil
 	}