chore: bump golanglint-ci (#383)

There's a major upgrade of it. This is an initial upgrade to make the
changes as minimal as possible.

Signed-off-by: Blake Pettersson <[email protected]>
Co-authored-by: Michael Crenshaw <[email protected]>
Signed-off-by: Afzal Ansari <[email protected]>

Author: 2 people

.github/workflows/ci.yaml

@@ -25,10 +25,11 @@ jobs:
       - name: Checkout code
         uses: actions/checkout@v3
       - name: Run golangci-lint
-        uses: golangci/golangci-lint-action@v3
+        uses: golangci/golangci-lint-action@4afd733a84b1f43292c63897423277bb7f4313a9 # v8.0.0
         with:
-          version: v1.64.5
-          args: --timeout 5m
+          # renovate: datasource=go packageName=github.com/golangci/golangci-lint versioning=regex:^v(?<major>\d+)\.(?<minor>\d+)\.(?<patch>\d+)?$
+          version: v2.2.1
+          args: --timeout 5m --verbose
   test:
     runs-on: ubuntu-latest
     steps:

.golangci.yml

@@ -1,7 +1,173 @@
-linters-settings:
-  goimports:
-    local-prefixes: github.com/argoproj/notifications-engine
+formatters:
+  enable:
+    - goimports
+
+  settings:
+    goimports:
+      local-prefixes:
+        - github.com/argoproj/notifications-engine
+
+issues:
+  max-issues-per-linter: 0
+  max-same-issues: 0
 
 linters:
   enable:
-    - goimports
+    - errorlint
+    - gomodguard
+    - misspell
+    - testifylint
+    - thelper
+    - usestdlibvars
+    - whitespace
+  disable:
+    - staticcheck
+
+  exclusions:
+    rules:
+      - linters:
+          - unparam
+        path: (.+)_test\.go
+
+    presets:
+      - comments
+      - common-false-positives
+      - legacy
+      - std-error-handling
+
+    warn-unused: true
+
+  settings:
+    gocritic:
+      disabled-checks:
+        - appendAssign
+        - assignOp
+        - exitAfterDefer
+        - mapKey
+        - typeSwitchVar
+
+    gomodguard:
+      blocked:
+        modules:
+          - github.com/golang-jwt/jwt/v4:
+              recommendations:
+                - github.com/golang-jwt/jwt/v5
+
+          - github.com/imdario/mergo:
+              recommendations:
+                - dario.cat/mergo
+              reason: '`github.com/imdario/mergo` has been renamed.'
+
+          - github.com/pkg/errors:
+              recommendations:
+                - errors
+
+    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
+
+    nolintlint:
+      require-specific: true
+
+    perfsprint:
+      # Optimizes even if it requires an int or uint type cast.
+      int-conversion: true
+      # Optimizes into `err.Error()` even if it is only equivalent for non-nil errors.
+      err-error: true
+      # Optimizes `fmt.Errorf`.
+      errorf: true
+      # Optimizes `fmt.Sprintf` with only one argument.
+      sprintf1: true
+      # Optimizes into strings concatenation.
+      strconcat: true
+
+    revive:
+      # 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: 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
+
+    staticcheck:
+      checks:
+        - all
+        - -SA5011
+        - -ST1003
+        - -ST1016
+
+    testifylint:
+      enable-all: true
+      disable:
+        - go-require
+        - expected-actual
+        - require-error
+        - len
+        - empty
+        - error-nil
+        - bool-compare
+
+    usetesting:
+      os-mkdir-temp: false
+output:
+  show-stats: false
+version: "2"

pkg/api/config.go

@@ -99,7 +99,7 @@ func ParseConfig(configMap *v1.ConfigMap, secret *v1.Secret) (*Config, error) {
 			name := strings.Join(parts[1:], ".")
 			template := services.Notification{}
 			if err := yaml.Unmarshal([]byte(v), &template); err != nil {
-				return nil, fmt.Errorf("failed to unmarshal template %s: %v", name, err)
+				return nil, fmt.Errorf("failed to unmarshal template %s: %w", name, err)
 			}
 			cfg.Templates[name] = template
 		case strings.HasPrefix(k, "service."):
@@ -116,7 +116,7 @@ func ParseConfig(configMap *v1.ConfigMap, secret *v1.Secret) (*Config, error) {
 
 			optsData, err := replaceServiceConfigSecrets(v, secret)
 			if err != nil {
-				return nil, fmt.Errorf("failed to render service configuration %s: %v", serviceType, err)
+				return nil, fmt.Errorf("failed to render service configuration %s: %w", serviceType, err)
 			}
 
 			cfg.Services[name] = func() (services.NotificationService, error) {
@@ -126,14 +126,14 @@ func ParseConfig(configMap *v1.ConfigMap, secret *v1.Secret) (*Config, error) {
 			name := strings.Join(parts[1:], ".")
 			var trigger []triggers.Condition
 			if err := yaml.Unmarshal([]byte(v), &trigger); err != nil {
-				return nil, fmt.Errorf("failed to unmarshal trigger %s: %v", name, err)
+				return nil, fmt.Errorf("failed to unmarshal trigger %s: %w", name, err)
 			}
 			cfg.Triggers[name] = trigger
 		case strings.HasPrefix(k, "defaultTriggers."):
 			name := strings.Join(parts[1:], ".")
 			var defaultTriggers []string
 			if err := yaml.Unmarshal([]byte(v), &defaultTriggers); err != nil {
-				return nil, fmt.Errorf("failed to unmarshal default trigger %s: %v", name, err)
+				return nil, fmt.Errorf("failed to unmarshal default trigger %s: %w", name, err)
 			}
 			cfg.ServiceDefaultTriggers[name] = defaultTriggers
 		}

pkg/api/factory.go

@@ -178,7 +178,6 @@ func (f *apiFactory) getApiFromNamespace(namespace string) (API, error) {
 		return nil, err
 	}
 	return f.getApiFromConfigmapAndSecret(cm, secret)
-
 }
 
 func (f *apiFactory) getApiFromConfigmapAndSecret(cm *v1.ConfigMap, secret *v1.Secret) (API, error) {

pkg/cmd/context.go

@@ -3,6 +3,7 @@ package cmd
 import (
 	"bytes"
 	"context"
+	"errors"
 	"fmt"
 	"io"
 	"os"
@@ -43,18 +44,18 @@ func splitYAML(yamlData []byte) ([]*unstructured.Unstructured, error) {
 	for {
 		ext := runtime.RawExtension{}
 		if err := d.Decode(&ext); err != nil {
-			if err == io.EOF {
+			if errors.Is(err, io.EOF) {
 				break
 			}
-			return objs, fmt.Errorf("failed to unmarshal manifest: %v", err)
+			return objs, fmt.Errorf("failed to unmarshal manifest: %w", err)
 		}
 		ext.Raw = bytes.TrimSpace(ext.Raw)
 		if len(ext.Raw) == 0 || bytes.Equal(ext.Raw, []byte("null")) {
 			continue
 		}
 		u := &unstructured.Unstructured{}
 		if err := yaml.Unmarshal(ext.Raw, u); err != nil {
-			return objs, fmt.Errorf("failed to unmarshal manifest: %v", err)
+			return objs, fmt.Errorf("failed to unmarshal manifest: %w", err)
 		}
 		objs = append(objs, u)
 	}

pkg/controller/controller.go

@@ -205,7 +205,7 @@ func (c *notificationController) processResourceWithAPI(api api.API, resource v1
 		res, err := api.RunTrigger(trigger, un.Object)
 		if err != nil {
 			logEntry.Errorf("Failed to execute condition of trigger %s: %v using the configuration in namespace %s", trigger, err, apiNamespace)
-			eventSequence.addWarning(fmt.Errorf("failed to execute condition of trigger %s: %v using the configuration in namespace %s", trigger, err, apiNamespace))
+			eventSequence.addWarning(fmt.Errorf("failed to execute condition of trigger %s: %w using the configuration in namespace %s", trigger, err, apiNamespace))
 		}
 		logEntry.Infof("Trigger %s result: %v", trigger, res)
 
@@ -234,7 +234,7 @@ func (c *notificationController) processResourceWithAPI(api api.API, resource v1
 							to, resource.GetNamespace(), resource.GetName(), err, apiNamespace)
 						notificationsState.SetAlreadyNotified(c.isSelfServiceConfigureApi(api), apiNamespace, trigger, cr, to, false)
 						c.metricsRegistry.IncDeliveriesCounter(trigger, to.Service, false)
-						eventSequence.addError(fmt.Errorf("failed to deliver notification %s to %s: %v using the configuration in namespace %s", trigger, to, err, apiNamespace))
+						eventSequence.addError(fmt.Errorf("failed to deliver notification %s to %s: %w using the configuration in namespace %s", trigger, to, err, apiNamespace))
 					} else {
 						logEntry.Debugf("Notification %s was sent using the configuration in namespace %s", to.Recipient, apiNamespace)
 						c.metricsRegistry.IncDeliveriesCounter(trigger, to.Service, true)
@@ -374,18 +374,18 @@ func (c *notificationController) processResource(api api.API, resource v1.Object
 		})
 		if err != nil {
 			logEntry.Errorf("Failed to marshal resource patch: %v", err)
-			eventSequence.addWarning(fmt.Errorf("failed to marshal annotations patch %v", err))
+			eventSequence.addWarning(fmt.Errorf("failed to marshal annotations patch %w", err))
 			return
 		}
 		resource, err = c.client.Namespace(resource.GetNamespace()).Patch(context.Background(), resource.GetName(), types.MergePatchType, patchData, v1.PatchOptions{})
 		if err != nil {
 			logEntry.Errorf("Failed to patch resource: %v", err)
-			eventSequence.addWarning(fmt.Errorf("failed to patch resource annotations %v", err))
+			eventSequence.addWarning(fmt.Errorf("failed to patch resource annotations %w", err))
 			return
 		}
 		if err := c.informer.GetStore().Update(resource); err != nil {
 			logEntry.Warnf("Failed to store update resource in informer: %v", err)
-			eventSequence.addWarning(fmt.Errorf("failed to store update resource in informer: %v", err))
+			eventSequence.addWarning(fmt.Errorf("failed to store update resource in informer: %w", err))
 			return
 		}
 	}

pkg/controller/controller_test.go

@@ -66,6 +66,7 @@ func newResource(name string, modifiers ...func(app *unstructured.Unstructured))
 }
 
 func newController(t *testing.T, ctx context.Context, client dynamic.Interface, opts ...Opts) (*notificationController, *mocks.MockAPI, error) {
+	t.Helper()
 	mockCtrl := gomock.NewController(t)
 	go func() {
 		<-ctx.Done()
@@ -98,6 +99,7 @@ func newController(t *testing.T, ctx context.Context, client dynamic.Interface,
 }
 
 func newControllerWithNamespaceSupport(t *testing.T, ctx context.Context, client dynamic.Interface, opts ...Opts) (*notificationController, map[string]notificationApi.API, error) {
+	t.Helper()
 	mockCtrl := gomock.NewController(t)
 	go func() {
 		<-ctx.Done()
@@ -340,7 +342,7 @@ func TestWithEventCallback(t *testing.T) {
 			description: "EventCallback should be invoked with non-nil error on send failure",
 			sendErr:     errors.New("this is a send error"),
 			expectedErrors: []error{
-				errors.New("failed to deliver notification my-trigger to {mock recipient}: this is a send error using the configuration in namespace "),
+				fmt.Errorf("failed to deliver notification my-trigger to {mock recipient}: %w using the configuration in namespace ", errors.New("this is a send error")),
 			},
 		},
 		{
@@ -483,5 +485,4 @@ func TestProcessItemsWithSelfService(t *testing.T) {
 		assert.Equal(t, expectedDeliveries[i].Trigger, event.Trigger)
 		assert.Equal(t, expectedDeliveries[i].Destination, event.Destination)
 	}
-
 }

pkg/services/alertmanager.go

@@ -245,7 +245,7 @@ func (s alertmanagerService) sendOneTarget(ctx context.Context, target string, r
 
 	data, err := io.ReadAll(response.Body)
 	if err != nil {
-		return fmt.Errorf("unable to read response data: %v", err)
+		return fmt.Errorf("unable to read response data: %w", err)
 	}
 
 	if response.StatusCode != http.StatusOK {

pkg/services/awssqs.go

@@ -72,7 +72,6 @@ func (s awsSqsService) sendMessageInput(queueUrl *string, notif Notification) *s
 		MessageBody:  aws.String(notif.Message),
 		DelaySeconds: 10,
 	}
-
 }
 func (s awsSqsService) getQueueInput(dest Destination) *sqs.GetQueueUrlInput {
 	result := &sqs.GetQueueUrlInput{}

pkg/services/awssqs_test.go

@@ -48,7 +48,6 @@ func TestGetTemplater_AwsSqs(t *testing.T) {
 }
 
 func TestSend_AwsSqs(t *testing.T) {
-
 	// Overriding methods inside, so service.Send could be called.
 	saveGetQueueURL := GetQueueURL
 	saveSendMsg := SendMsg