Go linting pre commit hook

.golangci.yml

@@ -0,0 +1,40 @@
+---
+#########################
+#########################
+## Golang Linter rules ##
+#########################
+#########################
+
+# configure golangci-lint
+# see https://github.com/golangci/golangci-lint/blob/master/.golangci.example.yml
+issues:
+  exclude-rules:
+    - path: _test\.go
+      linters:
+      - dupl
+      - gosec
+      - goconst
+      - golint
+      text: "underscore"
+linters:
+  enable:
+    - govet
+    - errcheck
+    - staticcheck
+    - unused
+    - gosimple
+    - structcheck
+    - varcheck
+    - ineffassign
+    - deadcode
+    - typecheck
+    - rowserrcheck
+    - gosec
+    - unconvert
+
+run:
+  modules-download-mode: 
+  # timeout for analysis, e.g. 30s, 5m, default is 1m
+  timeout: 5m
+  # default concurrency is a available CPU number
+  concurrency: 4

cmd/manager/main.go

@@ -12,13 +12,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/manager/signals"
 )
 
-// Change below variables to serve metrics on different host or port.
-var (
-	metricsHost               = "0.0.0.0"
-	metricsPort         int32 = 8383
-	operatorMetricsPort int32 = 8686
-)
-
 func configureLogger() (*zap.Logger, error) {
 	// TODO: configure non development logger
 	logger, err := zap.NewDevelopment()

cmd/prestop/main.go

@@ -18,8 +18,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
-var logger *zap.SugaredLogger
-
 const (
 	agentStatusFilePathEnv = "AGENT_STATUS_FILEPATH"
 	logFilePathEnv         = "PRE_STOP_HOOK_LOG_PATH"
@@ -107,31 +105,30 @@ func waitForAgentHealthStatus() (agenthealth.Health, error) {
 	defer ticker.Stop()
 
 	totalTime := time.Duration(0)
-	for {
-		select {
-		case <-ticker.C:
-			if totalTime > pollingDuration {
-				return agenthealth.Health{}, fmt.Errorf("agenth health status not ready after waiting %s", pollingDuration.String())
-			}
-			totalTime += pollingInterval
-
-			health, err := getAgentHealthStatus()
-			if err != nil {
-				return agenthealth.Health{}, err
-			}
-
-			status, ok := health.Healthiness[getHostname()]
-			if !ok {
-				return agenthealth.Health{}, fmt.Errorf("couldn't find status for hostname %s", getHostname())
-			}
-
-			// We determine if the file has been updated by checking if the process is not in goal state.
-			// As the agent is currently executing a plan, the process should not be in goal state.
-			if !status.IsInGoalState {
-				return health, nil
-			}
+	for range ticker.C {
+		if totalTime > pollingDuration {
+			break
+		}
+		totalTime += pollingInterval
+
+		health, err := getAgentHealthStatus()
+		if err != nil {
+			return agenthealth.Health{}, err
+		}
+
+		status, ok := health.Healthiness[getHostname()]
+		if !ok {
+			return agenthealth.Health{}, fmt.Errorf("couldn't find status for hostname %s", getHostname())
+		}
+
+		// We determine if the file has been updated by checking if the process is not in goal state.
+		// As the agent is currently executing a plan, the process should not be in goal state.
+		if !status.IsInGoalState {
+			return health, nil
 		}
 	}
+	return agenthealth.Health{}, fmt.Errorf("agenth health status not ready after waiting %s", pollingDuration.String())
+
 }
 
 // getAgentHealthStatus returns an instance of agenthealth.Health read

pkg/agenthealth/agenthealth.go

@@ -11,8 +11,8 @@ type Health struct {
 
 type ProcessHealth struct {
 	IsInGoalState   bool  `json:"IsInGoalState"`
-	LastMongoUpTime int64 `json:"LastMongoUpTime"`
 	ExpectedToBeUp  bool  `json:"ExpectedToBeUp"`
+	LastMongoUpTime int64 `json:"LastMongoUpTime"`
 }
 
 type MmsDirectorStatus struct {

pkg/automationconfig/automation_config.go

@@ -8,8 +8,8 @@ type ProcessType string
 
 const (
 	Mongod                ProcessType = "mongod"
-	DefaultMongoDBDataDir             = "/data"
-	DefaultAgentLogPath               = "/var/log/mongodb-mms-automation"
+	DefaultMongoDBDataDir string      = "/data"
+	DefaultAgentLogPath   string      = "/var/log/mongodb-mms-automation"
 )
 
 type Auth struct {
@@ -182,7 +182,7 @@ type ClientCertificateMode string
 
 const (
 	ClientCertificateModeOptional ClientCertificateMode = "OPTIONAL"
-	ClientCertificateModeRequired                       = "REQUIRED"
+	ClientCertificateModeRequired ClientCertificateMode = "REQUIRED"
 )
 
 type SSL struct {

pkg/automationconfig/automation_config_builder.go

@@ -15,8 +15,6 @@ const (
 type Builder struct {
 	processes         []Process
 	replicaSets       []ReplicaSet
-	version           int
-	auth              Auth
 	members           int
 	domain            string
 	name              string
@@ -133,10 +131,10 @@ func (b *Builder) Build() (AutomationConfig, error) {
 				ProtocolVersion: "1",
 			},
 		},
-		Versions: b.versions,
+		Versions:     b.versions,
 		ToolsVersion: b.toolsVersion,
-		Options:  Options{DownloadBase: "/var/lib/mongodb-mms-automation"},
-		Auth:     DisabledAuth(),
+		Options:      Options{DownloadBase: "/var/lib/mongodb-mms-automation"},
+		Auth:         DisabledAuth(),
 		SSL: SSL{
 			ClientCertificateMode: ClientCertificateModeOptional,
 		},
@@ -163,8 +161,8 @@ func (b *Builder) Build() (AutomationConfig, error) {
 		return AutomationConfig{}, err
 	}
 
-	if bytes.Compare(newAcBytes, currentAcBytes) != 0 {
-		currentAc.Version += 1
+	if !bytes.Equal(newAcBytes, currentAcBytes) {
+		currentAc.Version++
 	}
 	return currentAc, nil
 }

pkg/controller/mongodb/mongodb_controller.go

@@ -60,8 +60,8 @@ const (
 
 	tlsCAMountPath     = "/var/lib/tls/ca/"
 	tlsCACertName      = "ca.crt"
-	tlsSecretMountPath = "/var/lib/tls/secret/"
-	tlsSecretCertName  = "tls.crt"
+	tlsSecretMountPath = "/var/lib/tls/secret/" //nolint
+	tlsSecretCertName  = "tls.crt"              //nolint
 	tlsSecretKeyName   = "tls.key"
 	tlsServerMountPath = "/var/lib/tls/server/"
 	tlsServerFileName  = "server.pem"
@@ -72,6 +72,8 @@ const (
 	// TLSRolledOutKey indicates if TLS has been fully rolled out
 	tLSRolledOutAnnotationKey      = "mongodb.com/v1.tlsRolledOut"
 	hasLeftReadyStateAnnotationKey = "mongodb.com/v1.hasLeftReadyStateAnnotationKey"
+
+	trueAnnotation = "true"
 )
 
 // Add creates a new MongoDB Controller and adds it to the Manager. The Manager will set fields on the Controller
@@ -261,14 +263,14 @@ func (r *ReplicaSetReconciler) isStatefulSetReady(mdb mdbv1.MongoDB, existingSta
 	if existingStatefulSet.Spec.UpdateStrategy.Type == appsv1.OnDeleteStatefulSetStrategyType && !isReady {
 		r.log.Info("StatefulSet has left ready state, version upgrade in progress")
 		annotations := map[string]string{
-			hasLeftReadyStateAnnotationKey: "true",
+			hasLeftReadyStateAnnotationKey: trueAnnotation,
 		}
 		if err := r.setAnnotations(mdb.NamespacedName(), annotations); err != nil {
 			return false, fmt.Errorf("failed setting %s annotation to true: %s", hasLeftReadyStateAnnotationKey, err)
 		}
 	}
 
-	hasPerformedUpgrade := mdb.Annotations[hasLeftReadyStateAnnotationKey] == "true"
+	hasPerformedUpgrade := mdb.Annotations[hasLeftReadyStateAnnotationKey] == trueAnnotation
 	r.log.Infow("StatefulSet Readiness", "isReady", isReady, "hasPerformedUpgrade", hasPerformedUpgrade, "areEqual", areEqual)
 
 	if existingStatefulSet.Spec.UpdateStrategy.Type == appsv1.OnDeleteStatefulSetStrategyType {

pkg/controller/mongodb/mongodb_tls.go

@@ -87,7 +87,7 @@ func (r *ReplicaSetReconciler) completeTLSRollout(mdb mdbv1.MongoDB) error {
 
 	r.log.Debug("Completing TLS rollout")
 
-	mdb.Annotations[tLSRolledOutAnnotationKey] = "true"
+	mdb.Annotations[tLSRolledOutAnnotationKey] = trueAnnotation
 	if err := r.ensureAutomationConfig(mdb); err != nil {
 		return fmt.Errorf("error updating automation config after TLS rollout: %+v", err)
 	}

pkg/controller/mongodb/reconcilliation_assertions.go

@@ -1,10 +1,11 @@
 package mongodb
 
 import (
-	"github.com/stretchr/testify/assert"
-	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 	"testing"
 	"time"
+
+	"github.com/stretchr/testify/assert"
+	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 )
 
 func assertReconciliationSuccessful(t *testing.T, result reconcile.Result, err error) {

pkg/controller/mongodb/replicaset_controller_test.go

@@ -155,7 +155,7 @@ func TestChangingVersion_ResultsInRollingUpdateStrategyType(t *testing.T) {
 	sts.Status.UpdatedReplicas = 1
 	sts.Status.ReadyReplicas = 2
 	err = mgrClient.Update(context.TODO(), &sts)
-
+	assert.NoError(t, err)
 	_ = mgrClient.Get(context.TODO(), mdb.NamespacedName(), &sts)
 
 	// the request is requeued as the agents are still doing the upgrade
@@ -169,6 +169,7 @@ func TestChangingVersion_ResultsInRollingUpdateStrategyType(t *testing.T) {
 	sts.Status.UpdatedReplicas = 3
 	sts.Status.ReadyReplicas = 3
 	err = mgrClient.Update(context.TODO(), &sts)
+	assert.NoError(t, err)
 
 	// reconcilliation is successful
 	res, err = r.Reconcile(reconcile.Request{NamespacedName: types.NamespacedName{Namespace: mdb.Namespace, Name: mdb.Name}})
@@ -241,7 +242,7 @@ func TestAutomationConfig_versionIsBumpedOnChange(t *testing.T) {
 	assert.NoError(t, err)
 	assert.Equal(t, 1, currentAc.Version)
 
-	mdb.Spec.Members += 1
+	mdb.Spec.Members++
 	makeStatefulSetReady(mgr.GetClient(), mdb)
 
 	_ = mgr.GetClient().Update(context.TODO(), &mdb)
@@ -287,7 +288,8 @@ func TestStatefulSet_IsCorrectlyConfiguredWithTLS(t *testing.T) {
 			"tls.key": []byte("KEY"),
 		},
 	}
-	mgr.GetClient().Create(context.TODO(), &secret)
+	err := mgr.GetClient().Create(context.TODO(), &secret)
+	assert.NoError(t, err)
 
 	configMap := corev1.ConfigMap{
 		ObjectMeta: metav1.ObjectMeta{
@@ -298,7 +300,8 @@ func TestStatefulSet_IsCorrectlyConfiguredWithTLS(t *testing.T) {
 			"ca.crt": "CERT",
 		},
 	}
-	mgr.GetClient().Create(context.TODO(), &configMap)
+	err = mgr.GetClient().Create(context.TODO(), &configMap)
+	assert.NoError(t, err)
 
 	r := newReconciler(mgr, mockManifestProvider(mdb.Spec.Version))
 	res, err := r.Reconcile(reconcile.Request{NamespacedName: types.NamespacedName{Namespace: mdb.Namespace, Name: mdb.Name}})

pkg/kube/client/client.go

@@ -2,7 +2,6 @@ package client
 
 import (
 	"context"
-	"reflect"
 
 	"github.com/mongodb/mongodb-kubernetes-operator/pkg/kube/configmap"
 	"github.com/mongodb/mongodb-kubernetes-operator/pkg/kube/secret"
@@ -147,12 +146,3 @@ func (c client) DeleteStatefulSet(objectKey k8sClient.ObjectKey) error {
 	}
 	return c.Delete(context.TODO(), &sts)
 }
-
-func namespacedNameFromObject(obj runtime.Object) types.NamespacedName {
-	ns := reflect.ValueOf(obj).Elem().FieldByName("Namespace").String()
-	name := reflect.ValueOf(obj).Elem().FieldByName("Name").String()
-	return types.NamespacedName{
-		Name:      name,
-		Namespace: ns,
-	}
-}

pkg/kube/client/mocked_client_test.go

@@ -2,12 +2,14 @@ package client
 
 import (
 	"context"
+
+	"testing"
+
 	"github.com/mongodb/mongodb-kubernetes-operator/pkg/kube/configmap"
 	"github.com/mongodb/mongodb-kubernetes-operator/pkg/kube/service"
 	"github.com/stretchr/testify/assert"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/types"
-	"testing"
 )
 
 func TestMockedClient(t *testing.T) {

pkg/kube/configmap/configmap_test.go

@@ -46,7 +46,7 @@ func TestReadKey(t *testing.T) {
 	assert.Equal(t, "value2", value)
 	assert.NoError(t, err)
 
-	value, err = ReadKey(getter, "key3", nsName("namespace", "name"))
+	_, err = ReadKey(getter, "key3", nsName("namespace", "name"))
 	assert.Error(t, err)
 }
 

pkg/kube/secret/secret_builder.go

@@ -8,7 +8,6 @@ import (
 type builder struct {
 	data            map[string][]byte
 	labels          map[string]string
-	stringData      map[string]string
 	name            string
 	namespace       string
 	ownerReferences []metav1.OwnerReference

pkg/kube/secret/secret_test.go

@@ -46,7 +46,7 @@ func TestReadKey(t *testing.T) {
 	assert.Equal(t, "value2", value)
 	assert.NoError(t, err)
 
-	value, err = ReadKey(getter, "key3", nsName("namespace", "name"))
+	_, err = ReadKey(getter, "key3", nsName("namespace", "name"))
 	assert.Error(t, err)
 }
 

pkg/kube/service/service_builder.go

@@ -12,7 +12,6 @@ type builder struct {
 	serviceType           corev1.ServiceType
 	servicePort           corev1.ServicePort
 	labels                map[string]string
-	port                  int32
 	loadBalancerIP        string
 	publishNotReady       bool
 	ownerReferences       []metav1.OwnerReference

scripts/ci/download.go

@@ -51,7 +51,7 @@ func downloadFile(opts downloadOptions) error {
 }
 
 func fetchFile(filePath, url string) error {
-	resp, err := http.Get(url)
+	resp, err := http.Get(url) //nolint
 	if err != nil {
 		return fmt.Errorf("error getting url: %s", err)
 	}