adding golangci linting to project and correcting all errors found

Signed-off-by: Adam D. Cornett <adc@redhat.com>
redhat-openshift-ecosystem · May 22, 2023 · ca2a077 · ca2a077
1 parent f9d62a3
commit ca2a077
Show file tree

Hide file tree

Showing 14 changed files with 173 additions and 69 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -39,5 +39,17 @@ jobs:
     - name: Manifests
       run: make manifests && git diff --exit-code
 
+    - name: Tidy
+      run: make tidy
+
+    - name: Vet
+      run: make vet
+
+    - name: Format
+      run: make fmt
+
+    - name: Run golangci linting checks
+      run: make lint
+
     - name: Test
       run: make test
diff --git a/.golangci.yaml b/.golangci.yaml
@@ -0,0 +1,67 @@
+run:
+  # Default timeout is 1m, up to give more room
+  timeout: 4m
+
+linters:
+  enable:
+  - asciicheck
+  - deadcode
+  - depguard
+  - gofumpt
+  - goimports
+  - importas
+  - revive
+  - misspell
+  - stylecheck
+  - tparallel
+  - unconvert
+  - unparam
+  - whitespace
+
+linters-settings:
+  importas:
+    alias:
+    - pkg: k8s.io/api/core/v1
+      alias: corev1
+    - pkg: k8s.io/api/rbac/v1
+      alias: rbacv1
+    - pkg: k8s.io/apimachinery/pkg/apis/meta/v1
+      alias: metav1
+    - pkg: k8s.io/apimachinery/pkg/api/errors
+      alias: apierrors
+    - pkg: sigs.k8s.io/controller-runtime
+      alias: ctrl
+    - pkg: sigs.k8s.io/controller-runtime/pkg/log
+      alias: logf
+    - pkg: k8s.io/apimachinery/pkg/util/runtime
+      alias: utilruntime
+    - pkg: k8s.io/client-go/kubernetes/scheme
+      alias: clientgoscheme
+    - pkg: k8s.io/apimachinery/pkg/util/yaml
+      alias: yamlutil
+    - pkg: github.com/operator-framework/api/pkg/operators/v1alpha1
+      alias: operatorsv1a1
+    - pkg: github.com/openshift/api/image/v1
+      alias: imagev1
+    - pkg: github.com/openshift/api/security/v1
+      alias: securityv1
+    - pkg: github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1
+      alias: tekton
+    - pkg: github.com/redhat-openshift-ecosystem/operator-certification-operator/api/v1alpha1
+      alias: certv1alpha1
+
+  revive:
+    rules:
+    - name: dot-imports
+      severity: warning
+      disabled: true
+  stylecheck:
+    dot-import-whitelist:
+      - github.com/onsi/gomega
+      - github.com/onsi/ginkgo
+      - github.com/onsi/ginkgo/v2
+  goimports:
+    local-prefixes: github.com/redhat-openshift-ecosystem/operator-certification-operator
+
+output:
+  format: tab
diff --git a/Makefile b/Makefile
@@ -111,6 +111,10 @@ fmt: ## Run go fmt against code.
 vet: ## Run go vet against code.
 	go vet ./...
 
+.PHONY: lint
+lint: golangci-lint ## Run golangci-lint linter checks.
+	$(GOLANGCI_LINT) run
+
 .PHONY: test
 test: manifests generate fmt vet envtest ## Run tests.
 	KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)" go test ./... -coverprofile cover.out
@@ -281,3 +285,17 @@ test-sanity: tidy vet fmt
 tidy:
 	go mod tidy
 	git diff --exit-code
+
+GOLANGCI_LINT = $(shell pwd)/bin/golangci-lint
+GOLANGCI_LINT_VERSION ?= v1.52.2
+golangci-lint: $(GOLANGCI_LINT)
+$(GOLANGCI_LINT):
+	$(call go-install-tool,$(GOLANGCI_LINT),github.com/golangci/golangci-lint/cmd/golangci-lint@$(GOLANGCI_LINT_VERSION))
+
+# go-get-tool will 'go get' any package $2 and install it to $1.
+PROJECT_DIR := $(shell dirname $(abspath $(lastword $(MAKEFILE_LIST))))
+define go-install-tool
+@[ -f $(1) ] || { \
+GOBIN=$(PROJECT_DIR)/bin go install $(2) ;\
+}
+endef
diff --git a/api/v1alpha1/groupversion_info.go b/api/v1alpha1/groupversion_info.go
@@ -15,8 +15,8 @@ limitations under the License.
 */
 
 // Package v1alpha1 contains API Schema definitions for the certification v1alpha1 API group
-//+kubebuilder:object:generate=true
-//+groupName=certification.redhat.com
+// +kubebuilder:object:generate=true
+// +groupName=certification.redhat.com
 package v1alpha1
 
 import (

diff --git a/controllers/operatorpipeline_controller.go b/controllers/operatorpipeline_controller.go
@@ -19,11 +19,12 @@ package controllers
 import (
 	"context"
 
+	"github.com/redhat-openshift-ecosystem/operator-certification-operator/api/v1alpha1"
+	"github.com/redhat-openshift-ecosystem/operator-certification-operator/internal/reconcilers"
+
 	"github.com/go-logr/logr"
 	imagev1 "github.com/openshift/api/image/v1"
 	securityv1 "github.com/openshift/api/security/v1"
-	certv1alpha1 "github.com/redhat-openshift-ecosystem/operator-certification-operator/api/v1alpha1"
-	"github.com/redhat-openshift-ecosystem/operator-certification-operator/internal/reconcilers"
 	tekton "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
 	corev1 "k8s.io/api/core/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
@@ -61,7 +62,7 @@ func (r *OperatorPipelineReconciler) Reconcile(ctx context.Context, req ctrl.Req
 	reqLogger := logf.FromContext(ctx, "Request.Namespace", req.Namespace, "Request.Name", req.Name)
 	reqLogger.Info("Reconciling OperatorPipeline")
 
-	currentPipeline := &certv1alpha1.OperatorPipeline{}
+	currentPipeline := &v1alpha1.OperatorPipeline{}
 	err := r.Client.Get(ctx, req.NamespacedName, currentPipeline)
 	if err != nil {
 		if errors.IsNotFound(err) {
@@ -77,7 +78,7 @@ func (r *OperatorPipelineReconciler) Reconcile(ctx context.Context, req ctrl.Req
 	isOperatorPipelineMarkedToBeDeleted := currentPipeline.GetDeletionTimestamp() != nil
 	if isOperatorPipelineMarkedToBeDeleted {
 		if controllerutil.ContainsFinalizer(currentPipeline, operatorPipelineFinalizer) {
-			namespacePipelines := &certv1alpha1.OperatorPipelineList{}
+			namespacePipelines := &v1alpha1.OperatorPipelineList{}
 
 			// creating listOptions inorder to know the number of OperatorPipeline resources in the given namespace
 			// if last CR in namespace we can remove the ClusterRoleBinding associated with the namespace
@@ -94,7 +95,7 @@ func (r *OperatorPipelineReconciler) Reconcile(ctx context.Context, req ctrl.Req
 				}
 			}
 
-			clusterPipelines := &certv1alpha1.OperatorPipelineList{}
+			clusterPipelines := &v1alpha1.OperatorPipelineList{}
 			// not creating listOptions since we want to know the total number of OperatorPipelines in the entire cluster
 			// if last CR in cluster we can remove the SCC and ClusterRole
 			if err := r.Client.List(ctx, clusterPipelines); err != nil {
@@ -209,7 +210,7 @@ func (r *OperatorPipelineReconciler) deleteClusterRoleBinding(ctx context.Contex
 // SetupWithManager sets up the controller with the Manager.
 func (r *OperatorPipelineReconciler) SetupWithManager(mgr ctrl.Manager) error {
 	return ctrl.NewControllerManagedBy(mgr).
-		For(&certv1alpha1.OperatorPipeline{}).
+		For(&v1alpha1.OperatorPipeline{}).
 		Owns(&corev1.Secret{}).
 		Owns(&imagev1.ImageStream{}).
 		Owns(&tekton.Pipeline{}).

diff --git a/controllers/suite_test.go b/controllers/suite_test.go
@@ -20,6 +20,8 @@ import (
 	"path/filepath"
 	"testing"
 
+	"github.com/redhat-openshift-ecosystem/operator-certification-operator/api/v1alpha1"
+
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 	"k8s.io/client-go/kubernetes/scheme"
@@ -29,8 +31,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/envtest/printer"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/log/zap"
-
-	certificationv1alpha1 "github.com/redhat-openshift-ecosystem/operator-certification-operator/api/v1alpha1"
 	//+kubebuilder:scaffold:imports
 )
 
@@ -66,7 +66,7 @@ var _ = BeforeSuite(func() {
 	Expect(err).NotTo(HaveOccurred())
 	Expect(cfg).NotTo(BeNil())
 
-	err = certificationv1alpha1.AddToScheme(scheme.Scheme)
+	err = v1alpha1.AddToScheme(scheme.Scheme)
 	Expect(err).NotTo(HaveOccurred())
 
 	//+kubebuilder:scaffold:scheme

diff --git a/internal/errors/errors.go b/internal/errors/errors.go
@@ -2,6 +2,8 @@ package errors
 
 import "errors"
 
-var ErrSecretNotFound = errors.New("could not find existing secret")
-var ErrInvalidSecret = errors.New("the secret does not contain a valid key")
-var ErrGitRepoPathNotSpecified = errors.New("the GIT_REPO_PATH environment variable was not specified")
+var (
+	ErrSecretNotFound          = errors.New("could not find existing secret")
+	ErrInvalidSecret           = errors.New("the secret does not contain a valid key")
+	ErrGitRepoPathNotSpecified = errors.New("the GIT_REPO_PATH environment variable was not specified")
+)
diff --git a/internal/reconcilers/certified_image_stream.go b/internal/reconcilers/certified_image_stream.go
@@ -3,10 +3,11 @@ package reconcilers
 import (
 	"context"
 
+	"github.com/redhat-openshift-ecosystem/operator-certification-operator/api/v1alpha1"
+	"github.com/redhat-openshift-ecosystem/operator-certification-operator/internal/objects"
+
 	"github.com/go-logr/logr"
 	imagev1 "github.com/openshift/api/image/v1"
-	certv1alpha1 "github.com/redhat-openshift-ecosystem/operator-certification-operator/api/v1alpha1"
-	"github.com/redhat-openshift-ecosystem/operator-certification-operator/internal/objects"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -33,7 +34,7 @@ func NewCertifiedImageStreamReconciler(client client.Client, log logr.Logger, sc
 }
 
 // reconcileCertifiedImageStream will ensure that the certified operator ImageStream is present and up to date.
-func (r *CertifiedImageStreamReconciler) Reconcile(ctx context.Context, pipeline *certv1alpha1.OperatorPipeline) (bool, error) {
+func (r *CertifiedImageStreamReconciler) Reconcile(ctx context.Context, pipeline *v1alpha1.OperatorPipeline) (bool, error) {
 	key := types.NamespacedName{
 		Namespace: pipeline.Namespace,
 		Name:      certifiedIndex,

diff --git a/internal/reconcilers/marketplace_image_stream.go b/internal/reconcilers/marketplace_image_stream.go
@@ -3,10 +3,11 @@ package reconcilers
 import (
 	"context"
 
+	"github.com/redhat-openshift-ecosystem/operator-certification-operator/api/v1alpha1"
+	"github.com/redhat-openshift-ecosystem/operator-certification-operator/internal/objects"
+
 	"github.com/go-logr/logr"
 	imagev1 "github.com/openshift/api/image/v1"
-	certv1alpha1 "github.com/redhat-openshift-ecosystem/operator-certification-operator/api/v1alpha1"
-	"github.com/redhat-openshift-ecosystem/operator-certification-operator/internal/objects"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
@@ -32,7 +33,7 @@ func NewMarketplaceImageStreamReconciler(client client.Client, log logr.Logger,
 }
 
 // reconcileMarketplaceImageStream will ensure that the Red Hat Marketplace ImageStream is present and up to date.
-func (r *MarketplaceImageStreamReconciler) Reconcile(ctx context.Context, pipeline *certv1alpha1.OperatorPipeline) (bool, error) {
+func (r *MarketplaceImageStreamReconciler) Reconcile(ctx context.Context, pipeline *v1alpha1.OperatorPipeline) (bool, error) {
 	key := types.NamespacedName{
 		Namespace: pipeline.Namespace,
 		Name:      marketplaceIndex,

diff --git a/internal/reconcilers/pipeline_dependencies.go b/internal/reconcilers/pipeline_dependencies.go
@@ -6,21 +6,21 @@ import (
 	"os"
 	"path/filepath"
 
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"sigs.k8s.io/yaml"
+	"github.com/redhat-openshift-ecosystem/operator-certification-operator/api/v1alpha1"
 
 	"github.com/go-git/go-git/v5"
 	"github.com/go-logr/logr"
 	securityv1 "github.com/openshift/api/security/v1"
-	certv1alpha1 "github.com/redhat-openshift-ecosystem/operator-certification-operator/api/v1alpha1"
 	tekton "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
 	rbacv1 "k8s.io/api/rbac/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
 	yamlutil "k8s.io/apimachinery/pkg/util/yaml"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
+	"sigs.k8s.io/yaml"
 )
 
 const (
@@ -54,7 +54,7 @@ func NewPipeDependenciesReconciler(client client.Client, log logr.Logger, scheme
 	}
 }
 
-func (r *PipelineDependenciesReconciler) Reconcile(ctx context.Context, pipeline *certv1alpha1.OperatorPipeline) (bool, error) {
+func (r *PipelineDependenciesReconciler) Reconcile(ctx context.Context, pipeline *v1alpha1.OperatorPipeline) (bool, error) {
 	// Cloning operator-pipelines project to retrieve pipelines and tasks
 	// yaml manifests that need to be applied beforehand
 	// ref: https://github.com/redhat-openshift-ecosystem/certification-releases/blob/main/4.9/ga/ci-pipeline.md#step-6---install-the-certification-pipeline-and-dependencies-into-the-cluster
@@ -110,7 +110,6 @@ func (r *PipelineDependenciesReconciler) Reconcile(ctx context.Context, pipeline
 	tmpFile, err := r.modifyAndSaveTempClusterRoleBinding(ctx, filepath.Join(gitPath, baseManifestsPath, clusterRoleBindingYml), pipeline, new(rbacv1.ClusterRoleBinding))
 	if err != nil {
 		return true, err
-
 	}
 	defer os.Remove(tmpFile)
 
@@ -121,7 +120,7 @@ func (r *PipelineDependenciesReconciler) Reconcile(ctx context.Context, pipeline
 	return false, nil
 }
 
-func (r *PipelineDependenciesReconciler) applyOrDeletePipeline(ctx context.Context, pipeline *certv1alpha1.OperatorPipeline, applyManifest bool, yamlPath string) error {
+func (r *PipelineDependenciesReconciler) applyOrDeletePipeline(ctx context.Context, pipeline *v1alpha1.OperatorPipeline, applyManifest bool, yamlPath string) error {
 	if applyManifest {
 		return r.applyManifests(ctx, yamlPath, pipeline, new(tekton.Pipeline), false)
 	}
@@ -169,7 +168,7 @@ func (r *PipelineDependenciesReconciler) applyManifests(ctx context.Context, fil
 		if !errors.IsNotFound(err) {
 			return err
 		}
-		controllerutil.SetControllerReference(owner, obj, r.Scheme)
+		_ = controllerutil.SetControllerReference(owner, obj, r.Scheme)
 		if err := r.Client.Create(ctx, obj); err != nil {
 			log.Error(err, fmt.Sprintf("failed to create pipeline resource for file: %s", fileName))
 			return err

diff --git a/internal/reconcilers/pipeline_git_repo.go b/internal/reconcilers/pipeline_git_repo.go
@@ -6,11 +6,12 @@ import (
 	"os"
 	"path/filepath"
 
+	"github.com/redhat-openshift-ecosystem/operator-certification-operator/api/v1alpha1"
+	"github.com/redhat-openshift-ecosystem/operator-certification-operator/internal/errors"
+
 	"github.com/go-git/go-git/v5"
 	"github.com/go-git/go-git/v5/plumbing"
 	"github.com/go-logr/logr"
-	certv1alpha1 "github.com/redhat-openshift-ecosystem/operator-certification-operator/api/v1alpha1"
-	"github.com/redhat-openshift-ecosystem/operator-certification-operator/internal/errors"
 	"k8s.io/apimachinery/pkg/runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
@@ -33,7 +34,7 @@ func NewPipelineGitRepoReconciler(client client.Client, log logr.Logger, scheme
 	}
 }
 
-func (r *PipelineGitRepoReconciler) Reconcile(ctx context.Context, pipeline *certv1alpha1.OperatorPipeline) (bool, error) {
+func (r *PipelineGitRepoReconciler) Reconcile(_ context.Context, pipeline *v1alpha1.OperatorPipeline) (bool, error) {
 	log := r.Log.WithName("gitrepo")
 	gitMount, ok := os.LookupEnv("GIT_REPO_PATH")
 	if !ok {