Enable gosec/golangci-lint, then fix reported errors (#927)

Signed-off-by: Jonathan West <jonwest@redhat.com>

Author: jgwest

.github/workflows/codegen.yaml

@@ -58,3 +58,22 @@ jobs:
       - name: Ensure there is no diff in manifests
         run: |
           git diff --ignore-matching-lines='.*createdAt:.*' --exit-code -- .
+
+  gosec:
+    name: Ensure that code passes gosec and golint
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout code
+        uses: actions/checkout@v4
+      - name: Setup Golang
+        uses: actions/setup-go@v5
+        with:
+          go-version-file: 'go.mod'
+
+      - name: "Ensure code passes 'go-sec' - run 'make gosec' to identify issues"
+        run: |
+          make gosec
+
+      - name: "Ensure code passes 'golangci-lint' - run 'make lint' to identify issues"
+        run: |
+          make lint 

.golangci.yml

@@ -0,0 +1,23 @@
+version: "2"
+linters:
+  enable:
+    - ginkgolinter   # https://github.com/nunnatsa/ginkgolinter
+  exclusions:
+    generated: lax
+    presets:
+      - comments
+      - common-false-positives
+      - legacy
+      - std-error-handling
+    paths:
+      - third_party$
+      - builtin$
+      - examples$
+
+formatters:
+  exclusions:
+    generated: lax
+    paths:
+      - third_party$
+      - builtin$
+      - examples$

Makefile

@@ -329,7 +329,6 @@ endif
 endif
 
 
-
 # A comma-separated list of bundle images (e.g. make catalog-build BUNDLE_IMGS=example.com/operator-bundle:v0.1.0,example.com/operator-bundle:v0.2.0).
 # These images MUST exist in a registry and be pull-able.
 BUNDLE_IMGS ?= $(BUNDLE_IMG)
@@ -353,3 +352,37 @@ catalog-build: opm ## Build a catalog image.
 .PHONY: catalog-push
 catalog-push: ## Push a catalog image.
 	$(MAKE) docker-push IMG=$(CATALOG_IMG)
+
+
+.PHONY: gosec
+gosec: go_sec
+	$(GO_SEC) --exclude-dir "hack/upgrade-rollouts-manager"  ./... 
+
+.PHONY: lint
+lint: golangci_lint
+	$(GOLANGCI_LINT) --version
+	GOMAXPROCS=2 $(GOLANGCI_LINT) run --fix --verbose --timeout 300s
+
+
+GO_SEC = $(shell pwd)/bin/gosec
+go_sec: ## Download gosec locally if necessary.
+	$(call go-get-tool,$(GO_SEC),github.com/securego/gosec/v2/cmd/gosec@latest)
+
+GOLANGCI_LINT = $(shell pwd)/bin/golangci-lint
+golangci_lint: ## Download golangci-lint locally if necessary.
+	$(call go-get-tool,$(GOLANGCI_LINT),github.com/golangci/golangci-lint/v2/cmd/golangci-lint@latest)
+
+
+# go-get-tool will 'go install' any package $2 and install it to $1.
+PROJECT_DIR := $(shell dirname $(abspath $(lastword $(MAKEFILE_LIST))))
+define go-get-tool
+@[ -f $(1) ] || { \
+set -e ;\
+TMP_DIR=$$(mktemp -d) ;\
+cd $$TMP_DIR ;\
+go mod init tmp ;\
+echo "Downloading $(2)" ;\
+GOBIN=$(PROJECT_DIR)/bin go install $(2) ;\
+rm -rf $$TMP_DIR ;\
+}
+endef

cmd/main.go

@@ -47,7 +47,7 @@ import (
 	monitoringv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1"
 	crdv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	"k8s.io/apimachinery/pkg/labels"
-	"k8s.io/apimachinery/pkg/runtime"
+
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
 	ctrl "sigs.k8s.io/controller-runtime"
@@ -57,8 +57,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/manager"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
 
-	"github.com/argoproj-labs/argocd-operator/controllers/argocd"
-
 	pipelinesv1alpha1 "github.com/redhat-developer/gitops-operator/api/v1alpha1"
 	"github.com/redhat-developer/gitops-operator/common"
 	"github.com/redhat-developer/gitops-operator/controllers"
@@ -70,7 +68,7 @@ import (
 )
 
 var (
-	scheme   = runtime.NewScheme()
+	scheme   = k8sruntime.NewScheme()
 	setupLog = ctrl.Log.WithName("setup")
 )
 
@@ -262,7 +260,7 @@ func main() {
 		os.Exit(1)
 	}
 
-	argocd.Register(openshift.ReconcilerHook)
+	argocdprovisioner.Register(openshift.ReconcilerHook)
 
 	setupLog.Info("starting manager")
 	if err := mgr.Start(ctrl.SetupSignalHandler()); err != nil {

controllers/argocd/argocd_test.go

@@ -74,7 +74,9 @@ func TestArgoCD(t *testing.T) {
 			v1.ResourceCPU:    resourcev1.MustParse("500m"),
 		},
 	}
-	assert.DeepEqual(t, testArgoCD.Spec.Grafana.Resources, testGrafanaResources)
+
+	//lint:ignore SA1019 known to be deprecated
+	assert.DeepEqual(t, testArgoCD.Spec.Grafana.Resources, testGrafanaResources) //nolint:staticcheck // SA1019: We must test deprecated fields.
 
 	testHAResources := &v1.ResourceRequirements{
 		Requests: v1.ResourceList{

controllers/argocd/openshift/clusterconfig_test.go

@@ -97,29 +97,18 @@ func makeTestDeployment() *appsv1.Deployment {
 	}
 }
 
-func makeTestRoleBinding() *rbacv1.RoleBinding {
-	return &rbacv1.RoleBinding{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      testArgoCDName,
-			Namespace: testNamespace,
-		},
-		Subjects: []rbacv1.Subject{},
-		RoleRef:  rbacv1.RoleRef{},
-	}
-}
-
 func newStatefulSetWithSuffix(suffix string, component string, cr *argoapp.ArgoCD) *appsv1.StatefulSet {
 	return newStatefulSetWithName(fmt.Sprintf("%s-%s", cr.Name, suffix), component, cr)
 }
 
 func newStatefulSetWithName(name string, component string, cr *argoapp.ArgoCD) *appsv1.StatefulSet {
 	ss := newStatefulSet(cr)
-	ss.ObjectMeta.Name = name
+	ss.Name = name
 
-	lbls := ss.ObjectMeta.Labels
+	lbls := ss.Labels
 	lbls[common.ArgoCDKeyName] = name
 	lbls[common.ArgoCDKeyComponent] = component
-	ss.ObjectMeta.Labels = lbls
+	ss.Labels = lbls
 
 	return ss
 }

controllers/argocd/openshift/openshift.go

@@ -31,15 +31,16 @@ func ReconcilerHook(cr *argoapp.ArgoCD, v interface{}, hint string) error {
 	logv := log.WithValues("ArgoCD Namespace", cr.Namespace, "ArgoCD Name", cr.Name)
 	switch o := v.(type) {
 	case *rbacv1.ClusterRole:
-		if o.ObjectMeta.Name == argocd.GenerateUniqueResourceName("argocd-application-controller", cr) {
+		if o.Name == argocd.GenerateUniqueResourceName("argocd-application-controller", cr) {
 			logv.Info("configuring openshift cluster config policy rules")
 			o.Rules = policyRulesForClusterConfig()
 		}
 	case *appsv1.Deployment:
-		if o.ObjectMeta.Name == cr.ObjectMeta.Name+"-redis" {
+		switch o.Name {
+		case cr.Name + "-redis":
 			logv.Info("configuring openshift redis")
 			o.Spec.Template.Spec.Containers[0].Args = append(getArgsForRedhatRedis(), o.Spec.Template.Spec.Containers[0].Args...)
-		} else if o.ObjectMeta.Name == cr.ObjectMeta.Name+"-redis-ha-haproxy" {
+		case cr.Name + "-redis-ha-haproxy":
 			logv.Info("configuring openshift redis haproxy")
 			o.Spec.Template.Spec.Containers[0].Command = append(getCommandForRedhatRedisHaProxy(), o.Spec.Template.Spec.Containers[0].Command...)
 			version := hint
@@ -55,13 +56,14 @@ func ReconcilerHook(cr *argoapp.ArgoCD, v interface{}, hint string) error {
 			}
 		}
 	case *appsv1.StatefulSet:
-		if o.ObjectMeta.Name == cr.ObjectMeta.Name+"-redis-ha-server" {
+		if o.Name == cr.Name+"-redis-ha-server" {
 			logv.Info("configuring openshift redis-ha-server stateful set")
 			for index := range o.Spec.Template.Spec.Containers {
-				if o.Spec.Template.Spec.Containers[index].Name == "redis" {
+				switch o.Spec.Template.Spec.Containers[index].Name {
+				case "redis":
 					o.Spec.Template.Spec.Containers[index].Args = getArgsForRedhatHaRedisServer()
 					o.Spec.Template.Spec.Containers[index].Command = []string{}
-				} else if o.Spec.Template.Spec.Containers[index].Name == "sentinel" {
+				case "sentinel":
 					o.Spec.Template.Spec.Containers[index].Args = getArgsForRedhatHaRedisSentinel()
 					o.Spec.Template.Spec.Containers[index].Command = []string{}
 				}
@@ -70,12 +72,12 @@ func ReconcilerHook(cr *argoapp.ArgoCD, v interface{}, hint string) error {
 			o.Spec.Template.Spec.InitContainers[0].Command = []string{}
 		}
 	case *corev1.Secret:
-		if allowedNamespace(cr.ObjectMeta.Namespace, os.Getenv("ARGOCD_CLUSTER_CONFIG_NAMESPACES")) {
+		if allowedNamespace(cr.Namespace, os.Getenv("ARGOCD_CLUSTER_CONFIG_NAMESPACES")) {
 			logv.Info("configuring cluster secret with empty namespaces to allow cluster resources")
 			delete(o.Data, "namespaces")
 		}
 	case *rbacv1.Role:
-		if o.ObjectMeta.Name == cr.Name+"-"+"argocd-application-controller" {
+		if o.Name == cr.Name+"-"+"argocd-application-controller" {
 			logv.Info("configuring policy rule for Application Controller")
 
 			// can move this to somewhere common eventually, maybe init()

controllers/argocd/openshift/openshift_test.go

@@ -107,13 +107,13 @@ func TestReconcileArgoCD_reconcileRedisDeployment(t *testing.T) {
 	a := makeTestArgoCD()
 	testDeployment := makeTestDeployment()
 
-	testDeployment.ObjectMeta.Name = a.Name + "-" + "redis"
+	testDeployment.Name = a.Name + "-" + "redis"
 	want := append(getArgsForRedhatRedis(), testDeployment.Spec.Template.Spec.Containers[0].Args...)
 
 	assert.NoError(t, ReconcilerHook(a, testDeployment, ""))
 	assert.Equal(t, testDeployment.Spec.Template.Spec.Containers[0].Args, want)
 
-	testDeployment.ObjectMeta.Name = a.Name + "-" + "not-redis"
+	testDeployment.Name = a.Name + "-" + "not-redis"
 	want = testDeployment.Spec.Template.Spec.Containers[0].Args
 
 	assert.NoError(t, ReconcilerHook(a, testDeployment, ""))
@@ -124,7 +124,7 @@ func TestReconcileArgoCD_reconcileRedisHaProxyDeployment(t *testing.T) {
 	a := makeTestArgoCD()
 	testDeployment := makeTestDeployment()
 
-	testDeployment.ObjectMeta.Name = a.Name + "-redis-ha-haproxy"
+	testDeployment.Name = a.Name + "-redis-ha-haproxy"
 	testDeployment.Spec.Template.Spec.Containers[0].SecurityContext = &corev1.SecurityContext{
 		Capabilities: &corev1.Capabilities{},
 	}
@@ -141,7 +141,7 @@ func TestReconcileArgoCD_reconcileRedisHaProxyDeployment(t *testing.T) {
 	assert.Equal(t, wantc, *testDeployment.Spec.Template.Spec.Containers[0].SecurityContext.Capabilities)
 
 	testDeployment = makeTestDeployment()
-	testDeployment.ObjectMeta.Name = a.Name + "-redis-ha-haproxy"
+	testDeployment.Name = a.Name + "-redis-ha-haproxy"
 	testDeployment.Spec.Template.Spec.Containers[0].SecurityContext = &corev1.SecurityContext{
 		Capabilities: &corev1.Capabilities{},
 	}
@@ -150,7 +150,7 @@ func TestReconcileArgoCD_reconcileRedisHaProxyDeployment(t *testing.T) {
 	assert.Nil(t, testDeployment.Spec.Template.Spec.Containers[0].SecurityContext.Capabilities)
 
 	testDeployment = makeTestDeployment()
-	testDeployment.ObjectMeta.Name = a.Name + "-" + "not-redis-ha-haproxy"
+	testDeployment.Name = a.Name + "-" + "not-redis-ha-haproxy"
 	want = testDeployment.Spec.Template.Spec.Containers[0].Command
 
 	assert.NoError(t, ReconcilerHook(a, testDeployment, ""))

controllers/argocd_controller_test.go

@@ -118,7 +118,8 @@ func TestReconcile_delete_consolelink(t *testing.T) {
 		t.Run(test.name, func(t *testing.T) {
 			reconcileArgoCD, fakeClient := newFakeReconcileArgoCD(argoCDRoute, consoleLink)
 			consoleLink := newConsoleLink("https://test.com", "Cluster Argo CD")
-			fakeClient.Create(context.TODO(), consoleLink)
+			err := fakeClient.Create(context.TODO(), consoleLink)
+			assert.NilError(t, err)
 
 			if test.setEnvVarFunc != nil {
 				test.setEnvVarFunc(t, test.envVar)

controllers/argocd_metrics_controller_test.go

@@ -223,8 +223,8 @@ func TestReconcile_add_service_monitors(t *testing.T) {
 		assert.Assert(t, is.Len(serviceMonitor.OwnerReferences, 1))
 		assert.Equal(t, serviceMonitor.OwnerReferences[0].Kind, argocdKind)
 		assert.Equal(t, serviceMonitor.OwnerReferences[0].Name, tc.instanceName)
-		assert.Equal(t, len(serviceMonitor.ObjectMeta.Labels), 1)
-		assert.Equal(t, serviceMonitor.ObjectMeta.Labels["release"], "prometheus-operator")
+		assert.Equal(t, len(serviceMonitor.Labels), 1)
+		assert.Equal(t, serviceMonitor.Labels["release"], "prometheus-operator")
 		assert.Equal(t, len(serviceMonitor.Spec.Selector.MatchLabels), 1)
 		assert.Equal(t, serviceMonitor.Spec.Selector.MatchLabels["app.kubernetes.io/name"], matchLabel)
 		assert.Equal(t, len(serviceMonitor.Spec.Endpoints), 1)
@@ -238,8 +238,8 @@ func TestReconcile_add_service_monitors(t *testing.T) {
 		assert.Assert(t, is.Len(serviceMonitor.OwnerReferences, 1))
 		assert.Equal(t, serviceMonitor.OwnerReferences[0].Kind, argocdKind)
 		assert.Equal(t, serviceMonitor.OwnerReferences[0].Name, tc.instanceName)
-		assert.Equal(t, len(serviceMonitor.ObjectMeta.Labels), 1)
-		assert.Equal(t, serviceMonitor.ObjectMeta.Labels["release"], "prometheus-operator")
+		assert.Equal(t, len(serviceMonitor.Labels), 1)
+		assert.Equal(t, serviceMonitor.Labels["release"], "prometheus-operator")
 		assert.Equal(t, len(serviceMonitor.Spec.Selector.MatchLabels), 1)
 		assert.Equal(t, serviceMonitor.Spec.Selector.MatchLabels["app.kubernetes.io/name"], matchLabel)
 		assert.Equal(t, len(serviceMonitor.Spec.Endpoints), 1)
@@ -253,8 +253,8 @@ func TestReconcile_add_service_monitors(t *testing.T) {
 		assert.Assert(t, is.Len(serviceMonitor.OwnerReferences, 1))
 		assert.Equal(t, serviceMonitor.OwnerReferences[0].Kind, argocdKind)
 		assert.Equal(t, serviceMonitor.OwnerReferences[0].Name, tc.instanceName)
-		assert.Equal(t, len(serviceMonitor.ObjectMeta.Labels), 1)
-		assert.Equal(t, serviceMonitor.ObjectMeta.Labels["release"], "prometheus-operator")
+		assert.Equal(t, len(serviceMonitor.Labels), 1)
+		assert.Equal(t, serviceMonitor.Labels["release"], "prometheus-operator")
 		assert.Equal(t, len(serviceMonitor.Spec.Selector.MatchLabels), 1)
 		assert.Equal(t, serviceMonitor.Spec.Selector.MatchLabels["app.kubernetes.io/name"], matchLabel)
 		assert.Equal(t, len(serviceMonitor.Spec.Endpoints), 1)
@@ -390,7 +390,7 @@ func TestReconciler_add_dashboard(t *testing.T) {
 			err = r.Client.Get(context.TODO(), types.NamespacedName{Name: name, Namespace: dashboardNamespace}, dashboard)
 			assert.NilError(t, err)
 
-			assert.Assert(t, dashboard.ObjectMeta.Labels["console.openshift.io/dashboard"] == "true")
+			assert.Assert(t, dashboard.Labels["console.openshift.io/dashboard"] == "true")
 			assert.Assert(t, dashboard.Data[entry.Name()] == string(content))
 		}
 	}