chore: more lint (#387)

Try to slowly converge towards what exists on argo-cd.

Signed-off-by: Blake Pettersson <[email protected]>

Author: blakepettersson

.golangci.yml

@@ -9,19 +9,29 @@ formatters:
 
 issues:
   max-issues-per-linter: 0
+
   max-same-issues: 0
 
 linters:
   enable:
     - errorlint
+    - exptostd
+    - gocritic
     - gomodguard
+    - govet
+    - importas
     - misspell
-    - testifylint
+    - staticcheck
     - thelper
+    - tparallel
+    - unparam
     - usestdlibvars
+    - usetesting
     - whitespace
   disable:
-    - staticcheck
+    - perfsprint  # Performance optimizations cause many changes
+    - testifylint # Test assertion order changes cause large diffs
+    - revive      # Too many style changes (265+ issues)
 
   exclusions:
     rules:
@@ -39,12 +49,23 @@ linters:
 
   settings:
     gocritic:
+      enable-all: true
+      # Most of these should probably be enabled one-by-one.
       disabled-checks:
         - appendAssign
-        - assignOp
+        - appendCombine # Leave disabled, multi-line assigns can be more readable.
+        - assignOp # Leave disabled, assign operations can be more confusing than helpful.
+        - commentedOutCode
+        - deferInLoop
         - exitAfterDefer
-        - mapKey
-        - typeSwitchVar
+        - exposedSyncMutex
+        - hugeParam
+        - importShadow
+        - paramTypeCombine # Leave disabled, there are too many failures to be worth fixing.
+        - rangeValCopy
+        - tooManyResultsChecker
+        - unnamedResult
+        - whyNoLint
 
     gomodguard:
       blocked:
@@ -62,24 +83,38 @@ linters:
               recommendations:
                 - errors
 
+    govet:
+      disable:
+        - fieldalignment
+        - shadow
+      enable-all: true
+
     importas:
       alias:
         - pkg: github.com/golang-jwt/jwt/v5
           alias: jwtgo
+
         - pkg: k8s.io/api/apps/v1
           alias: appsv1
+
         - pkg: k8s.io/api/core/v1
           alias: corev1
+
         - pkg: k8s.io/api/rbac/v1
           alias: rbacv1
+
         - pkg: k8s.io/apimachinery/pkg/api/errors
           alias: apierrors
+
         - pkg: k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1
           alias: apiextensionsv1
+
         - pkg: k8s.io/apimachinery/pkg/apis/meta/v1
           alias: metav1
+
         - pkg: k8s.io/client-go/informers/core/v1
           alias: informersv1
+
         - pkg: errors
           alias: stderrors
 
@@ -102,52 +137,93 @@ linters:
       # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md
       rules:
         - name: bool-literal-in-expr
+
         - name: blank-imports
           disabled: true
+
         - name: context-as-argument
           arguments:
             - allowTypesBefore: '*testing.T,testing.TB'
+
         - name: context-keys-type
           disabled: true
+
         - name: dot-imports
           disabled: true
+
         - name: duplicated-imports
+
+        - name: early-return
+          arguments:
+            - preserveScope
+
         - name: empty-block
           disabled: true
+
         - name: error-naming
           disabled: true
+
         - name: error-return
+
         - name: error-strings
           disabled: true
+
         - name: errorf
+
         - name: identical-branches
+
         - name: if-return
+
         - name: increment-decrement
+
         - name: indent-error-flow
           arguments:
             - preserveScope
+
         - name: modifies-parameter
+
         - name: optimize-operands-order
+
         - name: range
+
         - name: receiver-naming
+
         - name: redefines-builtin-id
           disabled: true
+
         - name: redundant-import-alias
+
         - name: superfluous-else
           arguments:
             - preserveScope
+
         - name: time-equal
+
         - name: time-naming
           disabled: true
+
         - name: unexported-return
           disabled: true
+
         - name: unnecessary-stmt
+
         - name: unreachable-code
+
         - name: unused-parameter
+
         - name: use-any
+
         - name: useless-break
+
         - name: var-declaration
 
+        - name: var-naming
+          arguments:
+            - - ID
+            - - VM
+            - - skipPackageNameChecks: true
+                upperCaseConst: true
+
     staticcheck:
       checks:
         - all
@@ -157,17 +233,18 @@ linters:
 
     testifylint:
       enable-all: true
+
       disable:
         - go-require
-        - expected-actual
-        - require-error
-        - len
-        - empty
-        - error-nil
-        - bool-compare
+
+    unused:
+      field-writes-are-uses: false
+      exported-fields-are-used: false
 
     usetesting:
       os-mkdir-temp: false
+
 output:
   show-stats: false
+
 version: "2"

examples/certmanager/cli/main.go

@@ -8,7 +8,7 @@ import (
 	"github.com/argoproj/notifications-engine/pkg/cmd"
 	"github.com/argoproj/notifications-engine/pkg/services"
 
-	v1 "k8s.io/api/core/v1"
+	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 
 	_ "k8s.io/client-go/plugin/pkg/client/auth/gcp"
@@ -20,7 +20,7 @@ func main() {
 	}, api.Settings{
 		ConfigMapName: "cert-manager-notifications-cm",
 		SecretName:    "cert-manager-notifications-secret",
-		InitGetVars: func(cfg *api.Config, configMap *v1.ConfigMap, secret *v1.Secret) (api.GetVars, error) {
+		InitGetVars: func(cfg *api.Config, configMap *corev1.ConfigMap, secret *corev1.Secret) (api.GetVars, error) {
 			return func(obj map[string]interface{}, dest services.Destination) map[string]interface{} {
 				return map[string]interface{}{"cert": obj}
 			}, nil

examples/certmanager/controller/main.go

@@ -12,7 +12,7 @@ import (
 	"github.com/argoproj/notifications-engine/pkg/services"
 
 	"github.com/spf13/cobra"
-	v1 "k8s.io/api/core/v1"
+	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -59,7 +59,7 @@ func main() {
 			notificationsFactory := api.NewFactory(api.Settings{
 				ConfigMapName: "cert-manager-notifications-cm",
 				SecretName:    "cert-manager-notifications-secret",
-				InitGetVars: func(cfg *api.Config, configMap *v1.ConfigMap, secret *v1.Secret) (api.GetVars, error) {
+				InitGetVars: func(cfg *api.Config, configMap *corev1.ConfigMap, secret *corev1.Secret) (api.GetVars, error) {
 					return func(obj map[string]interface{}, dest services.Destination) map[string]interface{} {
 						return map[string]interface{}{"cert": obj}
 					}, nil

pkg/api/config.go

@@ -11,7 +11,7 @@ import (
 
 	log "github.com/sirupsen/logrus"
 	yaml3 "gopkg.in/yaml.v3"
-	v1 "k8s.io/api/core/v1"
+	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/fields"
 	"sigs.k8s.io/yaml"
 )
@@ -57,7 +57,7 @@ func (cfg Config) GetGlobalDestinations(labels map[string]string) services.Desti
 	return dests
 }
 
-var keyPattern = regexp.MustCompile(`[$][\w-_]+`)
+var keyPattern = regexp.MustCompile(`[$][\w-]+`)
 
 // replaceStringSecret checks if given string is a secret key reference ( starts with $ ) and returns corresponding value from provided map
 func replaceStringSecret(val string, secretValues map[string][]byte) string {
@@ -72,7 +72,7 @@ func replaceStringSecret(val string, secretValues map[string][]byte) string {
 }
 
 // ParseConfig retrieves Config from given ConfigMap and Secret
-func ParseConfig(configMap *v1.ConfigMap, secret *v1.Secret) (*Config, error) {
+func ParseConfig(configMap *corev1.ConfigMap, secret *corev1.Secret) (*Config, error) {
 	cfg := Config{
 		Services:               map[string]ServiceFactory{},
 		Triggers:               map[string][]triggers.Condition{},
@@ -106,11 +106,12 @@ func ParseConfig(configMap *v1.ConfigMap, secret *v1.Secret) (*Config, error) {
 			name := ""
 			serviceType := ""
 			parts := strings.Split(k, ".")
-			if len(parts) == 3 {
+			switch len(parts) {
+			case 3:
 				serviceType, name = parts[1], parts[2]
-			} else if len(parts) == 2 {
+			case 2:
 				serviceType, name = parts[1], parts[1]
-			} else {
+			default:
 				return nil, fmt.Errorf("invalid service key; expected 'service.<type>(.<name>)' but got '%s'", k)
 			}
 
@@ -141,7 +142,7 @@ func ParseConfig(configMap *v1.ConfigMap, secret *v1.Secret) (*Config, error) {
 	return &cfg, nil
 }
 
-func replaceServiceConfigSecrets(inputYaml string, secret *v1.Secret) ([]byte, error) {
+func replaceServiceConfigSecrets(inputYaml string, secret *corev1.Secret) ([]byte, error) {
 	var node yaml3.Node
 	err := yaml3.Unmarshal([]byte(inputYaml), &node)
 	if err != nil {

pkg/api/config_test.go

@@ -7,16 +7,16 @@ import (
 	"github.com/argoproj/notifications-engine/pkg/subscriptions"
 
 	"github.com/stretchr/testify/assert"
-	v1 "k8s.io/api/core/v1"
+	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/labels"
 )
 
 var (
-	emptySecret = &v1.Secret{Data: map[string][]byte{}}
+	emptySecret = &corev1.Secret{Data: map[string][]byte{}}
 )
 
 func TestParseConfig_Services(t *testing.T) {
-	cfg, err := ParseConfig(&v1.ConfigMap{Data: map[string]string{
+	cfg, err := ParseConfig(&corev1.ConfigMap{Data: map[string]string{
 		"service.slack": `
 token: my-token
 `}}, emptySecret)
@@ -29,7 +29,7 @@ token: my-token
 }
 
 func TestParseConfig_Templates(t *testing.T) {
-	cfg, err := ParseConfig(&v1.ConfigMap{Data: map[string]string{
+	cfg, err := ParseConfig(&corev1.ConfigMap{Data: map[string]string{
 		"template.my-template": `
 message: hello world
 `}}, emptySecret)
@@ -44,7 +44,7 @@ message: hello world
 }
 
 func TestParseConfig_DefaultServiceTriggers(t *testing.T) {
-	cfg, err := ParseConfig(&v1.ConfigMap{Data: map[string]string{
+	cfg, err := ParseConfig(&corev1.ConfigMap{Data: map[string]string{
 		"defaultTriggers.slack": `
 - trigger-a
 - trigger-b
@@ -85,7 +85,7 @@ headers:
     value: Bearer $secret-value
 `
 
-	secrets := v1.Secret{
+	secrets := corev1.Secret{
 		Data: map[string][]byte{
 			"endpoint":     []byte("https://example.com"),
 			"secret-value": []byte("token"),
@@ -111,7 +111,7 @@ apiKeys:
   second-team: $second-team-secret
 `
 
-	secrets := v1.Secret{
+	secrets := corev1.Secret{
 		Data: map[string][]byte{
 			"first-team-secret":  []byte("first-token"),
 			"second-team-secret": []byte("second-token"),
@@ -136,7 +136,7 @@ privateKey: $github-privateKey
 installationID: 67890
 `
 
-	secrets := v1.Secret{
+	secrets := corev1.Secret{
 		Data: map[string][]byte{
 			"github-privateKey": []byte("A\nValue\nOn\nMultiple\nLines"),
 		},
@@ -159,7 +159,7 @@ installationID: 67890
 }
 
 func TestParseConfig_DefaultTriggers(t *testing.T) {
-	cfg, err := ParseConfig(&v1.ConfigMap{
+	cfg, err := ParseConfig(&corev1.ConfigMap{
 		Data: map[string]string{
 			"defaultTriggers": `[trigger1, trigger2]`,
 		},
@@ -172,7 +172,7 @@ func TestParseConfig_DefaultTriggers(t *testing.T) {
 }
 
 func TestParseConfig_Subscriptions(t *testing.T) {
-	cfg, err := ParseConfig(&v1.ConfigMap{
+	cfg, err := ParseConfig(&corev1.ConfigMap{
 		Data: map[string]string{
 			"subscriptions": `
 - selector: test=true