Merge pull request #18 from jgwest/add-gosec-golangcilint

Add golangci-lint and gosec to Makefile

.github/workflows/lint.yaml

@@ -1,20 +1,21 @@
-name: Integration tests
+name: Lint code
 on:
   pull_request:
     branches:
-      - 'master'
-      - 'release-*'
+      - '*'     
 
 jobs:
   lint_code:
-    name: Run golangci-lint on PR
+    name: Run golangci-lint and gosec on PR
     runs-on: ubuntu-latest
     steps:
       - name: Checkout code
         uses: actions/checkout@v2
-      - name: Run golangci-lint
-        uses: golangci/golangci-lint-action@v3
-        with:
-          version: v1.52.2
-          args: --timeout 5m --exclude SA5011
-          only-new-issues: true
+      - name: "run gosec"
+        run: |
+          cd $GITHUB_WORKSPACE
+          make gosec
+      - name: "run golangci-lint"
+        run: |
+          cd $GITHUB_WORKSPACE
+          make lint

Makefile

@@ -262,3 +262,38 @@ catalog-build: opm ## Build a catalog image.
 .PHONY: catalog-push
 catalog-push: ## Push a catalog image.
 	$(MAKE) docker-push IMG=$(CATALOG_IMG)
+
+
+
+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 gosec locally if necessary.
+	$(call go-get-tool,$(GOLANGCI_LINT),github.com/golangci/golangci-lint/cmd/golangci-lint@latest)
+
+
+.PHONY: lint
+lint: golangci_lint
+	$(GOLANGCI_LINT) --version
+	GOMAXPROCS=2 $(GOLANGCI_LINT) run --fix --verbose --timeout 300s
+
+.PHONY: gosec
+gosec: go_sec
+	$(GO_SEC) ./...
+
+# 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
+

controllers/default.go

@@ -14,7 +14,7 @@ const (
 	// deployment, service, role, rolebinding and serviceaccount.
 	DefaultArgoRolloutsResourceName = "argo-rollouts"
 	// DefaultRolloutsNotificationSecretName is the default name for rollout controller secret resource.
-	DefaultRolloutsNotificationSecretName = "argo-rollouts-notification-secret"
+	DefaultRolloutsNotificationSecretName = "argo-rollouts-notification-secret" // #nosec G101
 	// DefaultRolloutsServiceSelectorKey is key used by selector
 	DefaultRolloutsSelectorKey = "app.kubernetes.io/name"
 

controllers/resources.go

@@ -124,7 +124,10 @@ func (r *RolloutManagerReconciler) reconcileRolloutsRoleBinding(cr *rolloutsApi.
 	// Reconcile if the role already exists and modified.
 	if !reflect.DeepEqual(expectedRoleBinding.Subjects, actualRoleBinding.Subjects) {
 		actualRoleBinding.Subjects = expectedRoleBinding.Subjects
-		r.Client.Update(context.TODO(), actualRoleBinding)
+
+		if err := r.Client.Update(context.TODO(), actualRoleBinding); err != nil {
+			return err
+		}
 	}
 
 	return nil
@@ -311,6 +314,10 @@ func (r *RolloutManagerReconciler) reconcileRolloutsSecrets(cr *rolloutsApi.Roll
 }
 
 // Deletes rollout resources when the corresponding rollout CR is deleted.
+//
+// TODO: Remove the nolint:all once this function is called
+//
+//nolint:unused
 func (r *RolloutManagerReconciler) deleteRolloutResources(cr *rolloutsApi.RolloutManager) error {
 	if cr.DeletionTimestamp != nil {
 		log.Info(fmt.Sprintf("Argo Rollout resource in %s namespace is deleted, Deleting the Argo Rollout workloads",

controllers/resources_test.go

@@ -8,7 +8,6 @@ import (
 	"github.com/stretchr/testify/assert"
 	corev1 "k8s.io/api/core/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
-	v1 "k8s.io/api/rbac/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
@@ -43,37 +42,37 @@ func TestReconcileRolloutManager_verifyRolloutsResources(t *testing.T) {
 		t.Fatalf("failed to find the rollouts serviceaccount: %#v\n", err)
 	}
 
-	role := &v1.Role{}
+	role := &rbacv1.Role{}
 	if err = r.Client.Get(context.TODO(), types.NamespacedName{
 		Name:      DefaultArgoRolloutsResourceName,
 		Namespace: testNamespace,
 	}, role); err != nil {
 		t.Fatalf("failed to find the rollouts role: %#v\n", err)
 	}
 
-	rolebinding := &v1.RoleBinding{}
+	rolebinding := &rbacv1.RoleBinding{}
 	if err = r.Client.Get(context.TODO(), types.NamespacedName{
 		Name:      DefaultArgoRolloutsResourceName,
 		Namespace: testNamespace,
 	}, rolebinding); err != nil {
 		t.Fatalf("failed to find the rollouts rolebinding: %#v\n", err)
 	}
 
-	aggregateToAdminClusterRole := &v1.ClusterRole{}
+	aggregateToAdminClusterRole := &rbacv1.ClusterRole{}
 	if err = r.Client.Get(context.TODO(), types.NamespacedName{
 		Name: "argo-rollouts-aggregate-to-admin",
 	}, aggregateToAdminClusterRole); err != nil {
 		t.Fatalf("failed to find the aggregateToAdmin ClusterRole: %#v\n", err)
 	}
 
-	aggregateToEditClusterRole := &v1.ClusterRole{}
+	aggregateToEditClusterRole := &rbacv1.ClusterRole{}
 	if err = r.Client.Get(context.TODO(), types.NamespacedName{
 		Name: "argo-rollouts-aggregate-to-edit",
 	}, aggregateToEditClusterRole); err != nil {
 		t.Fatalf("failed to find the aggregateToEdit ClusterRole: %#v\n", err)
 	}
 
-	aggregateToViewClusterRole := &v1.ClusterRole{}
+	aggregateToViewClusterRole := &rbacv1.ClusterRole{}
 	if err = r.Client.Get(context.TODO(), types.NamespacedName{
 		Name: "argo-rollouts-aggregate-to-view",
 	}, aggregateToViewClusterRole); err != nil {
@@ -145,7 +144,8 @@ func TestReconcileRolloutManager_CleanUp(t *testing.T) {
 		t.Fatal("reconcile requeued request")
 	}
 
-	r.Client.Delete(context.TODO(), a)
+	err = r.Client.Delete(context.TODO(), a)
+	assert.NoError(t, err)
 
 	// check if rollouts resources are deleted
 	tt := []struct {
@@ -172,7 +172,7 @@ func TestReconcileRolloutManager_CleanUp(t *testing.T) {
 		},
 		{
 			fmt.Sprintf("RoleBinding %s", DefaultArgoRolloutsResourceName),
-			&v1.RoleBinding{
+			&rbacv1.RoleBinding{
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      DefaultArgoRolloutsResourceName,
 					Namespace: a.Namespace,

controllers/utils.go

@@ -10,7 +10,6 @@ import (
 
 	corev1 "k8s.io/api/core/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
-	v1 "k8s.io/api/rbac/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -40,7 +39,7 @@ func fetchObject(client client.Client, namespace string, name string, obj client
 
 // getPolicyRules returns the policy rules for Argo Rollouts Role.
 func getPolicyRules() []rbacv1.PolicyRule {
-	return []v1.PolicyRule{
+	return []rbacv1.PolicyRule{
 		{
 			APIGroups: []string{
 				"argoproj.io",
@@ -355,8 +354,8 @@ func getPolicyRules() []rbacv1.PolicyRule {
 }
 
 // Returns PolicyRules for the Cluster Role argo-rollouts-aggregate-to-admin
-func getAggregateToAdminPolicyRules() []v1.PolicyRule {
-	return []v1.PolicyRule{
+func getAggregateToAdminPolicyRules() []rbacv1.PolicyRule {
+	return []rbacv1.PolicyRule{
 		{
 			APIGroups: []string{
 				"argoproj.io",
@@ -385,8 +384,8 @@ func getAggregateToAdminPolicyRules() []v1.PolicyRule {
 }
 
 // Returns PolicyRules for the Cluster Role argo-rollouts-aggregate-to-edit
-func getAggregateToEditPolicyRules() []v1.PolicyRule {
-	return []v1.PolicyRule{
+func getAggregateToEditPolicyRules() []rbacv1.PolicyRule {
+	return []rbacv1.PolicyRule{
 		{
 			APIGroups: []string{
 				"argoproj.io",
@@ -415,8 +414,8 @@ func getAggregateToEditPolicyRules() []v1.PolicyRule {
 }
 
 // Returns PolicyRules for the Cluster Role argo-rollouts-aggregate-to-view
-func getAggregateToViewPolicyRules() []v1.PolicyRule {
-	return []v1.PolicyRule{
+func getAggregateToViewPolicyRules() []rbacv1.PolicyRule {
+	return []rbacv1.PolicyRule{
 		{
 			APIGroups: []string{
 				"argoproj.io",