feat: enable linting (#19)

* fix: fix linting errors

* feat: add linting to prow

* fix: fix failing test

* fix: increase the timeout of the linter to 2 mins

* fix: only run helm chart checks if helm chart has changed

* fix: add resources to prow pod

.prow.yaml

@@ -16,10 +16,32 @@ presubmits:
             - make
           args:
             - test
+  - name: lint-credstash-operator
+    decorate: true
+    always_run: true
+    skip_report: false
+    clone_uri: "[email protected]:ouzi-dev/credstash-operator.git"
+    max_concurrency: 1
+    trigger: "(?m)lint( please)?"
+    rerun_command: "lint"
+    spec:
+      containers:
+        - name: "lint"
+          imagePullPolicy: IfNotPresent
+          image: quay.io/ouzi/go-builder:1.13.7
+          command:
+            - make
+          args:
+            - lint
+          resources:
+            requests:
+              memory: "2Gi"
+              cpu: 1
   - name: helm-chart-lint
     context: helm-chart-lint
     decorate: true
-    always_run: true
+    run_if_changed: 'deploy/helm/'
+    always_run: false
     skip_report: false
     clone_uri: "[email protected]:ouzi-dev/credstash-operator.git"
     max_concurrency: 1
@@ -37,7 +59,8 @@ presubmits:
   - name: helm-chart-validate
     context: helm-chart-validate
     decorate: true
-    always_run: true
+    run_if_changed: 'deploy/helm/'
+    always_run: false
     skip_report: false
     clone_uri: "[email protected]:ouzi-dev/credstash-operator.git"
     max_concurrency: 1
@@ -55,7 +78,8 @@ presubmits:
   - name: helm-chart-package
     context: helm-chart-package
     decorate: true
-    always_run: true
+    run_if_changed: 'deploy/helm/'
+    always_run: false
     skip_report: false
     clone_uri: "[email protected]:ouzi-dev/credstash-operator.git"
     max_concurrency: 1

Makefile

@@ -100,7 +100,16 @@ fmt:
 	find . -name '*.go' -not -wholename './vendor/*' | while read -r file; do gofmt -w -s "$$file"; goimports -w "$$file"; done
 
 lint:
-	golangci-lint run --enable-all -D gochecknoglobals -D gochecknoinits -D dupl ./...
+	golangci-lint run \
+	--enable-all \
+	-D gochecknoglobals \
+	-D gochecknoinits \
+	-D dupl \
+	--skip-files \
+	pkg/apis/credstash/v1alpha1/zz_generated.deepcopy.go,\
+	pkg/apis/credstash/v1alpha1/zz_generated.openapi.go \
+	--timeout 2m \
+	./...
 
 .DEFAULT_GOAL := build
 

cmd/manager/main.go

@@ -45,7 +45,11 @@ var (
 var log = logf.Log.WithName("cmd")
 
 func init() {
-	pflag.StringVar(&flags.SelectorLabelValue, "selector-label", "", "If provided the controller will only process CRDs that have the provided label")
+	pflag.StringVar(
+		&flags.SelectorLabelValue,
+		"selector-label",
+		"",
+		"If provided the controller will only process CRDs that have the provided label")
 }
 
 func printVersion() {
@@ -55,6 +59,7 @@ func printVersion() {
 	log.Info(fmt.Sprintf("Version of operator-sdk: %v", sdkVersion.Version))
 }
 
+//nolint funlen
 func main() {
 	// Add the zap logger flag set to the CLI. The flag set must
 	// be added before calling pflag.Parse().
@@ -162,13 +167,24 @@ func addMetrics(ctx context.Context, cfg *rest.Config, namespace string) {
 			log.Info("Skipping CR metrics server creation; not running in a cluster.")
 			return
 		}
+
 		log.Info("Could not generate and serve custom resource metrics", "error", err.Error())
 	}
 
 	// Add to the below struct any other metrics ports you want to expose.
 	servicePorts := []v1.ServicePort{
-		{Port: metricsPort, Name: metrics.OperatorPortName, Protocol: v1.ProtocolTCP, TargetPort: intstr.IntOrString{Type: intstr.Int, IntVal: metricsPort}},
-		{Port: operatorMetricsPort, Name: metrics.CRPortName, Protocol: v1.ProtocolTCP, TargetPort: intstr.IntOrString{Type: intstr.Int, IntVal: operatorMetricsPort}},
+		{
+			Port:       metricsPort,
+			Name:       metrics.OperatorPortName,
+			Protocol:   v1.ProtocolTCP,
+			TargetPort: intstr.IntOrString{Type: intstr.Int, IntVal: metricsPort},
+		},
+		{
+			Port:       operatorMetricsPort,
+			Name:       metrics.CRPortName,
+			Protocol:   v1.ProtocolTCP,
+			TargetPort: intstr.IntOrString{Type: intstr.Int, IntVal: operatorMetricsPort},
+		},
 	}
 
 	// Create Service object to expose the metrics port(s).
@@ -180,6 +196,7 @@ func addMetrics(ctx context.Context, cfg *rest.Config, namespace string) {
 	// CreateServiceMonitors will automatically create the prometheus-operator ServiceMonitor resources
 	// necessary to configure Prometheus to scrape metrics from this operator.
 	services := []*v1.Service{service}
+
 	_, err = metrics.CreateServiceMonitors(cfg, namespace, services)
 	if err != nil {
 		log.Info("Could not create ServiceMonitor object", "error", err.Error())
@@ -212,5 +229,6 @@ func serveCRMetrics(cfg *rest.Config) error {
 	if err != nil {
 		return err
 	}
+
 	return nil
 }

pkg/aws/credentials.go

@@ -6,13 +6,14 @@ import (
 	"github.com/aws/aws-sdk-go/aws/session"
 )
 
-// Gets the aws session to use for looking up credstash secrets
-func GetAwsSession(region string, awsAccessKeyId string, awsSecretAccessKey string) (*session.Session, error) {
+const awsSdkMaxRetries = 5
 
-	if awsAccessKeyId == "" || awsSecretAccessKey == "" {
+// Gets the aws session to use for looking up credstash secrets
+func GetAwsSession(region string, awsAccessKeyID string, awsSecretAccessKey string) (*session.Session, error) {
+	if awsAccessKeyID == "" || awsSecretAccessKey == "" {
 		config := aws.Config{
 			Region:     aws.String(region),
-			MaxRetries: aws.Int(5),
+			MaxRetries: aws.Int(awsSdkMaxRetries),
 		}
 
 		sess, err := session.NewSessionWithOptions(session.Options{
@@ -25,20 +26,18 @@ func GetAwsSession(region string, awsAccessKeyId string, awsSecretAccessKey stri
 		}
 
 		return sess, nil
-
 	}
 
 	sess, err := session.NewSession(&aws.Config{
 		Region:      aws.String(region),
-		Credentials: credentials.NewStaticCredentials(awsAccessKeyId, awsSecretAccessKey, ""),
+		Credentials: credentials.NewStaticCredentials(awsAccessKeyID, awsSecretAccessKey, ""),
 	})
 
 	if err != nil {
 		return nil, err
 	}
 
 	return sess, nil
-
 }
 
 // Gets the aws session to use for looking up credstash secrets falling back to the environment config

pkg/controller/controller.go

@@ -14,5 +14,6 @@ func AddToManager(m manager.Manager) error {
 			return err
 		}
 	}
+
 	return nil
 }

pkg/controller/credstashsecret/credstashsecret_controller.go

@@ -46,13 +46,15 @@ const LabelNameForSelector = "operatorInstance"
 
 var log = logf.Log.WithName("controller_credstashsecret")
 
-// Add creates a new CredstashSecret Controller and adds it to the Manager. The Manager will set fields on the Controller
+// Add creates a new CredstashSecret Controller and adds it to the Manager.
+// The Manager will set fields on the Controller
 // and Start it when the Manager is Started.
 func Add(mgr manager.Manager) error {
 	reconciler, err := newReconciler(mgr)
 	if err != nil {
 		return err
 	}
+
 	return add(mgr, reconciler)
 }
 
@@ -116,6 +118,7 @@ type ReconcileCredstashSecret struct {
 // Note:
 // The Controller will requeue the Request to be processed again if the returned error is non-nil or
 // Result.Requeue is true, otherwise upon completion it will remove the work from the queue.
+//nolint funlen
 func (r *ReconcileCredstashSecret) Reconcile(request reconcile.Request) (reconcile.Result, error) {
 	reqLogger := log.WithValues("Namespace", request.Namespace, "Name", request.Name)
 	reqLogger.Info("Reconciling CredstashSecret")
@@ -167,7 +170,12 @@ func (r *ReconcileCredstashSecret) Reconcile(request reconcile.Request) (reconci
 				},
 			}
 
-			reqLogger.Info("Deleting old secret since name has changed", "Secret.Namespace", secretToDelete.Namespace, "Secret.Name", secretToDelete.Name)
+			reqLogger.Info(
+				"Deleting old secret since name has changed",
+				"Secret.Namespace",
+				secretToDelete.Namespace,
+				"Secret.Name",
+				secretToDelete.Name)
 
 			err = r.client.Delete(context.TODO(), secretToDelete)
 			if err != nil {
@@ -190,17 +198,27 @@ func (r *ReconcileCredstashSecret) Reconcile(request reconcile.Request) (reconci
 
 	// Secret is out of date with credstash data
 	if !reflect.DeepEqual(secret.Data, found.Data) {
-		reqLogger.Info("Updating Secret because contents have changed", "Secret.Namespace", secret.Namespace, "Secret.Name", secret.Name)
+		reqLogger.Info(
+			"Updating Secret because contents have changed",
+			"Secret.Namespace",
+			secret.Namespace,
+			"Secret.Name",
+			secret.Name)
+
 		err = r.client.Update(context.TODO(), secret)
 		if err != nil {
 			return reconcile.Result{}, err
-		} else {
-			return reconcile.Result{}, nil
 		}
+		return reconcile.Result{}, nil
 	}
 
 	// Secret already exists - don't requeue
-	reqLogger.Info("Skip reconcile: Secret already exists and is up to date", "Secret.Namespace", found.Namespace, "Secret.Name", found.Name)
+	reqLogger.Info(
+		"Skip reconcile: Secret already exists and is up to date",
+		"Secret.Namespace",
+		found.Namespace,
+		"Secret.Name",
+		found.Name)
 	return reconcile.Result{}, nil
 }
 
@@ -229,6 +247,7 @@ func (r *ReconcileCredstashSecret) secretForCR(cr *credstashv1alpha1.CredstashSe
 }
 
 // setupPredicateFuncs makes sure that we only watch resources that match the correct label selector that we want
+// nolint funlen
 func setupPredicateFuncs() predicate.Funcs {
 	return predicate.Funcs{
 		CreateFunc: func(e event.CreateEvent) bool {

pkg/controller/credstashsecret/credstashsecret_controller_test.go

@@ -19,11 +19,10 @@ import (
 	"k8s.io/client-go/kubernetes/scheme"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
-	logf "sigs.k8s.io/controller-runtime/pkg/runtime/log"
 )
 
 const (
-	errorString    = "an error has occured"
+	errorString    = "an error has occurred"
 	name           = "credstashCR"
 	namespace      = "credstash"
 	credstashKey   = "key1"
@@ -258,16 +257,14 @@ var tests = []testReconcileItem{
 	},
 }
 
+//nolint funlen
 func TestReconcileCredstashSecret_Reconcile(t *testing.T) {
-
 	for _, testData := range tests {
 		t.Run(testData.testName, func(t *testing.T) {
 			ctrl := gomock.NewController(t)
 			defer ctrl.Finish()
 
 			mockCredstashSecretGetter := mocks.NewMockSecretGetter(ctrl)
-			// Set the logger to development mode for verbose logs.
-			logf.SetLogger(logf.ZapLogger(true))
 
 			// Register operator types with the runtime scheme.
 			s := scheme.Scheme

pkg/credstash/credstash_secret_getter.go

@@ -24,12 +24,15 @@ var log = logf.Log.WithName("credstashsecret_getter")
 func NewSecretGetter(awsSession *session.Session) SecretGetter {
 	unicreds.SetKMSConfig(awsSession.Config)
 	unicreds.SetDynamoDBConfig(awsSession.Config)
+
 	return &secretGetter{}
 }
 
-func (h *secretGetter) GetCredstashSecretsForCredstashSecretDefs(credstashDefs []v1alpha1.CredstashSecretDef) (map[string][]byte, error) {
+func (h *secretGetter) GetCredstashSecretsForCredstashSecretDefs(
+	credstashDefs []v1alpha1.CredstashSecretDef) (map[string][]byte, error) {
 	ecryptionContext := unicreds.NewEncryptionContextValue()
 	secretsMap := map[string][]byte{}
+
 	for _, v := range credstashDefs {
 		table := v.Table
 		if table == "" {
@@ -43,6 +46,7 @@ func (h *secretGetter) GetCredstashSecretsForCredstashSecretDefs(credstashDefs [
 					"Secret.Key", v.Key, "Secret.Version", "latest", "Secret.Table", table)
 				return nil, err
 			}
+
 			secretsMap[v.Key] = []byte(creds.Secret)
 		} else {
 			formattedVersion, err := formatCredstashVersion(v.Version)
@@ -58,6 +62,7 @@ func (h *secretGetter) GetCredstashSecretsForCredstashSecretDefs(credstashDefs [
 					"Secret.Key", v.Key, "Secret.Version", formattedVersion, "Secret.Table", table)
 				return nil, err
 			}
+
 			secretsMap[v.Key] = []byte(creds.Secret)
 		}
 	}

pkg/credstash/util.go

@@ -1,7 +1,6 @@
 package credstash
 
 import (
-	"errors"
 	"fmt"
 	"strconv"
 )
@@ -21,14 +20,14 @@ func formatCredstashVersion(inputVersion string) (string, error) {
 
 	// version is too longßß
 	if len(inputVersion) > credstashVersionLength {
-		return "", errors.New(
-			fmt.Sprintf("Version string is longer than supported. Maximum length is %d characters",
-				credstashVersionLength))
+		return "", fmt.Errorf("version string is longer than supported. Maximum length is %d characters",
+			credstashVersionLength)
 	}
 
 	// pad version with leading zeros until we reach credstashVersionLength
 	// format becomes something like %019s which means pad the string until there's 19 0s
 	format := fmt.Sprintf("%s%ds", "%0", credstashVersionLength)
 	newVersion := fmt.Sprintf(format, inputVersion)
+
 	return newVersion, nil
 }

pkg/credstash/util_test.go

@@ -32,7 +32,7 @@ func TestCredstashSecretGetter_GetCredstashSecretsForCredstashSecretDefs(t *test
 		{
 			inputVersion:      "00000000000000000000001",
 			expectedVersion:   "",
-			expectedErrorText: "Version string is longer than supported.",
+			expectedErrorText: "version string is longer than supported.",
 		},
 		{
 			inputVersion:      "this is not a number",
@@ -44,6 +44,7 @@ func TestCredstashSecretGetter_GetCredstashSecretsForCredstashSecretDefs(t *test
 	for _, v := range tests {
 		actualVersion, actualError := formatCredstashVersion(v.inputVersion)
 		assert.Equal(t, v.expectedVersion, actualVersion)
+
 		if actualError != nil {
 			assert.Contains(t, actualError.Error(), v.expectedErrorText)
 		}