Update golangci-lint and fix lint errors (#4656)

Signed-off-by: karamaru-alpha <[email protected]>

.golangci.yml

@@ -9,23 +9,23 @@ run:
 linters:
   disable-all: true
   enable:
-    - prealloc
+    - depguard
+    - exportloopref
+    - gocritic
+    - goimports
+    - gosimple
     - ineffassign
-    - goerr113
     - misspell
-    - errcheck
-    - gosimple
+    - prealloc
     - staticcheck
-    - gosec
-    - gocritic
-    - unparam
-    - deadcode
-    - unconvert
-    - typecheck
     - stylecheck
-    - exportloopref
-    - depguard
-    - goimports
+    - typecheck
+    - unconvert
+    - unparam
+    # TODO: Enable these linters
+    # - errcheck
+    # - goerr113
+    # - gosec
 
 issues:
   exclude-rules:
@@ -38,10 +38,15 @@ output:
 
 linters-settings:
   depguard:
-    list-type: blacklist
-    include-go-root: true
-    packages-with-error-message:
-      - sync/atomic: "Use go.uber.org/atomic instead of sync/atomic"
-      - io/ioutil: "Use corresponding 'os' or 'io' functions instead."
+    rules:
+      main:
+        deny:
+          - pkg: "sync/atomic"
+            desc: "Use go.uber.org/atomic instead of sync/atomic."
+          - pkg: "io/ioutil"
+            desc: "Use corresponding 'os' or 'io' functions instead."
+  gocritic:
+    disabled-checks:
+      - appendAssign
   goimports:
     local-prefixes: github.com/pipe-cd/pipecd

Makefile

@@ -149,7 +149,7 @@ run/site:
 
 .PHONY: lint/go
 lint/go: FIX ?= false
-lint/go: VERSION ?= sha256:78d1bbd01a9886a395dc8374218a6c0b7b67694e725dd76f0c8ac1de411b85e8 #v1.46.2
+lint/go: VERSION ?= sha256:fb70c9b2e6d0763141f057abcafde7f88d5e4bb3b5882d6b14bc79382f04481c #v1.55.2
 lint/go: FLAGS ?= --rm --platform linux/amd64 -e GOCACHE=/repo/.cache/go-build -e GOLANGCI_LINT_CACHE=/repo/.cache/golangci-lint -v ${PWD}:/repo -w /repo -it
 lint/go:
 ifeq ($(FIX),true)

pkg/app/ops/deploymentchaincontroller/controller.go

@@ -88,9 +88,7 @@ func (d *DeploymentChainController) Run(ctx context.Context) error {
 				d.logger.Error("failed while sync controller updaters", zap.Error(err))
 			}
 		case <-syncDeploymentChainsTicker.C:
-			if err := d.syncDeploymentChains(ctx); err != nil {
-				d.logger.Error("failed while sync deployment chains", zap.Error(err))
-			}
+			d.syncDeploymentChains(ctx)
 		}
 	}
 }
@@ -130,7 +128,7 @@ func (d *DeploymentChainController) syncUpdaters(ctx context.Context) error {
 	return nil
 }
 
-func (d *DeploymentChainController) syncDeploymentChains(ctx context.Context) error {
+func (d *DeploymentChainController) syncDeploymentChains(ctx context.Context) {
 	var (
 		updatersNum = len(d.updaters)
 		updaterCh   = make(chan *updater, updatersNum)
@@ -162,8 +160,6 @@ func (d *DeploymentChainController) syncDeploymentChains(ctx context.Context) er
 
 	d.logger.Info("waiting for all updaters to finish")
 	wg.Wait()
-
-	return nil
 }
 
 func listNotCompletedDeploymentChain(ctx context.Context, dcs deploymentChainStore) ([]*model.DeploymentChain, error) {

pkg/app/piped/appconfigreporter/appconfigreporter.go

@@ -167,10 +167,7 @@ func (r *Reporter) updateRegisteredApps(ctx context.Context, headCommits map[str
 	outOfSyncRegisteredApps := make([]*model.ApplicationInfo, 0)
 	for repoID, repo := range r.gitRepos {
 		headCommit := headCommits[repoID]
-		rs, err := r.findOutOfSyncRegisteredApps(repo.GetPath(), repoID, headCommit)
-		if err != nil {
-			return err
-		}
+		rs := r.findOutOfSyncRegisteredApps(repo.GetPath(), repoID, headCommit)
 		r.logger.Info(fmt.Sprintf("found out %d valid registered applications that config has been changed in repository %q", len(rs), repoID))
 		outOfSyncRegisteredApps = append(outOfSyncRegisteredApps, rs...)
 	}
@@ -233,7 +230,7 @@ func (r *Reporter) updateUnregisteredApps(ctx context.Context) error {
 }
 
 // findOutOfSyncRegisteredApps finds out registered application info that should be updated in the given git repository.
-func (r *Reporter) findOutOfSyncRegisteredApps(repoPath, repoID, headCommit string) ([]*model.ApplicationInfo, error) {
+func (r *Reporter) findOutOfSyncRegisteredApps(repoPath, repoID, headCommit string) []*model.ApplicationInfo {
 	// Compare the apps registered on Control-plane with the latest config file
 	// and return only the ones that have been changed.
 	apps := make([]*model.ApplicationInfo, 0)
@@ -275,7 +272,7 @@ func (r *Reporter) findOutOfSyncRegisteredApps(repoPath, repoID, headCommit stri
 		appCfg.Id = app.Id
 		apps = append(apps, appCfg)
 	}
-	return apps, nil
+	return apps
 }
 
 func (r *Reporter) isSynced(appInfo *model.ApplicationInfo, app *model.Application) bool {

pkg/app/piped/appconfigreporter/appconfigreporter_test.go

@@ -157,8 +157,7 @@ spec:
 	}
 	for _, tc := range testcases {
 		t.Run(tc.name, func(t *testing.T) {
-			got, err := tc.reporter.findOutOfSyncRegisteredApps(tc.args.repoPath, tc.args.repoID, "not-existed-head-commit")
-			assert.Equal(t, tc.wantErr, err != nil)
+			got := tc.reporter.findOutOfSyncRegisteredApps(tc.args.repoPath, tc.args.repoID, "not-existed-head-commit")
 			assert.Equal(t, tc.want, got)
 		})
 	}

pkg/app/piped/cmd/piped/piped.go

@@ -696,17 +696,14 @@ func (p *piped) sendPipedMeta(ctx context.Context, client pipedservice.Client, c
 	}
 
 	// Configure secret management.
-	if sm := cfg.SecretManagement; sm != nil {
-		switch sm.Type {
-		case model.SecretManagementTypeKeyPair:
-			publicKey, err := sm.KeyPair.LoadPublicKey()
-			if err != nil {
-				return fmt.Errorf("failed to read public key for secret management (%w)", err)
-			}
-			req.SecretEncryption = &model.Piped_SecretEncryption{
-				Type:      sm.Type.String(),
-				PublicKey: string(publicKey),
-			}
+	if sm := cfg.SecretManagement; sm != nil && sm.Type == model.SecretManagementTypeKeyPair {
+		publicKey, err := sm.KeyPair.LoadPublicKey()
+		if err != nil {
+			return fmt.Errorf("failed to read public key for secret management (%w)", err)
+		}
+		req.SecretEncryption = &model.Piped_SecretEncryption{
+			Type:      sm.Type.String(),
+			PublicKey: string(publicKey),
 		}
 	}
 	if req.SecretEncryption == nil {

pkg/app/piped/controller/controller.go

@@ -288,7 +288,7 @@ func (c *controller) checkCommands() {
 }
 
 // syncPlanners adds new planner for newly PENDING deployments.
-func (c *controller) syncPlanners(ctx context.Context) error {
+func (c *controller) syncPlanners(ctx context.Context) {
 	// Remove stale planners from the recently completed list.
 	for id, t := range c.donePlanners {
 		if time.Since(t) >= plannerStaleDuration {
@@ -324,7 +324,7 @@ func (c *controller) syncPlanners(ctx context.Context) error {
 	// Add missing planners.
 	pendings := c.deploymentLister.ListPendings()
 	if len(pendings) == 0 {
-		return nil
+		return
 	}
 
 	c.logger.Info(fmt.Sprintf("there are %d pending deployments for planning", len(pendings)),
@@ -426,8 +426,6 @@ func (c *controller) syncPlanners(ctx context.Context) error {
 			)
 		}
 	}
-
-	return nil
 }
 
 func (c *controller) startNewPlanner(ctx context.Context, d *model.Deployment) (*planner, error) {
@@ -506,7 +504,7 @@ func (c *controller) startNewPlanner(ctx context.Context, d *model.Deployment) (
 
 // syncSchedulers adds new scheduler for newly PLANNED/RUNNING deployments
 // as well as removes the schedulers for the completed deployments.
-func (c *controller) syncSchedulers(ctx context.Context) error {
+func (c *controller) syncSchedulers(ctx context.Context) {
 	// Update the most recent successful commit hashes.
 	for id, s := range c.schedulers {
 		if !s.IsDone() {
@@ -556,7 +554,7 @@ func (c *controller) syncSchedulers(ctx context.Context) error {
 	targets := append(runnings, planneds...)
 
 	if len(targets) == 0 {
-		return nil
+		return
 	}
 
 	c.logger.Info(fmt.Sprintf("there are %d planned/running deployments for scheduling", len(targets)),
@@ -589,8 +587,6 @@ func (c *controller) syncSchedulers(ctx context.Context) error {
 			zap.Int("count", len(c.schedulers)),
 		)
 	}
-
-	return nil
 }
 
 // startNewScheduler creates and starts running a new scheduler

pkg/app/piped/driftdetector/cloudrun/detector.go

@@ -122,7 +122,7 @@ func (d *detector) ProviderName() string {
 	return d.provider.Name
 }
 
-func (d *detector) check(ctx context.Context) error {
+func (d *detector) check(ctx context.Context) {
 	appsByRepo := d.listGroupedApplication()
 
 	for repoID, apps := range appsByRepo {
@@ -168,7 +168,6 @@ func (d *detector) check(ctx context.Context) error {
 			}
 		}
 	}
-	return nil
 }
 
 func (d *detector) cloneGitRepository(ctx context.Context, repoID string) (git.Repo, error) {

pkg/app/piped/driftdetector/kubernetes/detector.go

@@ -120,7 +120,7 @@ func (d *detector) Run(ctx context.Context) error {
 	}
 }
 
-func (d *detector) check(ctx context.Context) error {
+func (d *detector) check(ctx context.Context) {
 	appsByRepo := d.listGroupedApplication()
 
 	for repoID, apps := range appsByRepo {
@@ -171,8 +171,6 @@ func (d *detector) check(ctx context.Context) error {
 			}
 		}
 	}
-
-	return nil
 }
 
 func (d *detector) checkApplication(ctx context.Context, app *model.Application, repo git.Repo, headCommit git.Commit) error {

pkg/app/piped/driftdetector/terraform/detector.go

@@ -121,7 +121,7 @@ func (d *detector) Run(ctx context.Context) error {
 	}
 }
 
-func (d *detector) check(ctx context.Context) error {
+func (d *detector) check(ctx context.Context) {
 	appsByRepo := d.listGroupedApplication()
 
 	for repoID, apps := range appsByRepo {
@@ -167,8 +167,6 @@ func (d *detector) check(ctx context.Context) error {
 			}
 		}
 	}
-
-	return nil
 }
 
 func (d *detector) checkApplication(ctx context.Context, app *model.Application, repo git.Repo, headCommit git.Commit) error {

pkg/app/piped/executor/analysis/analysis.go

@@ -116,7 +116,7 @@ func (e *Executor) Execute(sig executor.StopSignal) model.StageStatus {
 					continue
 				}
 				status = model.StageStatus_STAGE_SKIPPED
-				// Stop the context to cancel all running analysises.
+				// Stop the context to cancel all running analyses.
 				cancel()
 				return
 			case <-doneCh:

pkg/app/piped/executor/kubernetes/kubernetes.go

@@ -308,7 +308,7 @@ func deleteResources(ctx context.Context, ag applierGetter, resources []provider
 }
 
 func findManifests(kind, name string, manifests []provider.Manifest) []provider.Manifest {
-	var out []provider.Manifest
+	out := make([]provider.Manifest, 0, len(manifests))
 	for _, m := range manifests {
 		if m.Key.Kind != kind {
 			continue
@@ -322,7 +322,7 @@ func findManifests(kind, name string, manifests []provider.Manifest) []provider.
 }
 
 func findConfigMapManifests(manifests []provider.Manifest) []provider.Manifest {
-	var out []provider.Manifest
+	out := make([]provider.Manifest, 0, len(manifests))
 	for _, m := range manifests {
 		if !m.Key.IsConfigMap() {
 			continue
@@ -333,7 +333,7 @@ func findConfigMapManifests(manifests []provider.Manifest) []provider.Manifest {
 }
 
 func findSecretManifests(manifests []provider.Manifest) []provider.Manifest {
-	var out []provider.Manifest
+	out := make([]provider.Manifest, 0, len(manifests))
 	for _, m := range manifests {
 		if !m.Key.IsSecret() {
 			continue

pkg/app/piped/executor/kubernetes/kubernetes_test.go

@@ -1018,7 +1018,7 @@ func TestPatchManifests(t *testing.T) {
 
 	patcher := func(m provider.Manifest, cfg config.K8sResourcePatch) (*provider.Manifest, error) {
 		out := m
-		out.Key.Namespace = out.Key.Namespace + "+"
+		out.Key.Namespace = fmt.Sprintf("%s+", out.Key.Namespace)
 		return &out, nil
 	}
 

pkg/app/piped/executor/kubernetes/traffic.go

@@ -226,7 +226,7 @@ func findIstioVirtualServiceManifests(manifests []provider.Manifest, ref config.
 		return nil, fmt.Errorf("support only %q kind for VirtualService reference", istioVirtualServiceKind)
 	}
 
-	var out []provider.Manifest
+	out := make([]provider.Manifest, 0, len(manifests))
 	for _, m := range manifests {
 		if !strings.HasPrefix(m.Key.APIVersion, istioNetworkingAPIVersionPrefix) {
 			continue

pkg/app/piped/livestatereporter/cloudrun/report.go

@@ -96,7 +96,7 @@ func (r *reporter) ProviderName() string {
 	return r.provider.Name
 }
 
-func (r *reporter) flushSnapshots(ctx context.Context) error {
+func (r *reporter) flushSnapshots(ctx context.Context) {
 	apps := r.appLister.ListByPlatformProvider(r.provider.Name)
 	for _, app := range apps {
 		state, ok := r.stateGetter.GetState(app.Id)
@@ -130,5 +130,4 @@ func (r *reporter) flushSnapshots(ctx context.Context) error {
 		r.snapshotVersions[app.Id] = state.Version
 		r.logger.Info(fmt.Sprintf("successfully reported application live state for application: %s", app.Id))
 	}
-	return nil
 }

pkg/app/piped/livestatereporter/kubernetes/reporter.go

@@ -109,7 +109,7 @@ func (r *reporter) Run(ctx context.Context) error {
 	}
 }
 
-func (r *reporter) flushSnapshots(ctx context.Context) error {
+func (r *reporter) flushSnapshots(ctx context.Context) {
 	// TODO: In the future, maybe we should apply worker model for this or
 	// send multiple application states in one request.
 	apps := r.appLister.ListByPlatformProvider(r.provider.Name)
@@ -145,7 +145,6 @@ func (r *reporter) flushSnapshots(ctx context.Context) error {
 		r.snapshotVersions[app.Id] = state.Version
 		r.logger.Info(fmt.Sprintf("successfully reported application live state for application: %s", app.Id))
 	}
-	return nil
 }
 
 func (r *reporter) flushEvents(ctx context.Context) error {

pkg/app/piped/livestatestore/kubernetes/appnodes.go

@@ -84,7 +84,7 @@ func (a *appNodes) addManagingResource(uid string, key provider.ResourceKey, obj
 	}, true
 }
 
-func (a *appNodes) deleteManagingResource(uid string, key provider.ResourceKey, now time.Time) (model.KubernetesResourceStateEvent, bool) {
+func (a *appNodes) deleteManagingResource(uid string, _ provider.ResourceKey, now time.Time) (model.KubernetesResourceStateEvent, bool) {
 	a.mu.Lock()
 	n, ok := a.managingNodes[uid]
 	if !ok {
@@ -138,7 +138,7 @@ func (a *appNodes) addDependedResource(uid string, key provider.ResourceKey, obj
 	}, true
 }
 
-func (a *appNodes) deleteDependedResource(uid string, key provider.ResourceKey, now time.Time) (model.KubernetesResourceStateEvent, bool) {
+func (a *appNodes) deleteDependedResource(uid string, _ provider.ResourceKey, now time.Time) (model.KubernetesResourceStateEvent, bool) {
 	a.mu.Lock()
 	n, ok := a.dependedNodes[uid]
 	if !ok {

pkg/app/piped/livestatestore/kubernetes/reflector.go

@@ -150,7 +150,7 @@ type reflector struct {
 	logger                *zap.Logger
 }
 
-func (r *reflector) start(ctx context.Context) error {
+func (r *reflector) start(_ context.Context) error {
 	matcher := newResourceMatcher(r.config.AppStateInformer)
 
 	// Use discovery to discover APIs supported by the Kubernetes API server.

pkg/app/piped/notifier/slack.go

@@ -350,6 +350,7 @@ func makeSlackDate(unix int64) string {
 	return fmt.Sprintf("<!date^%d^{date_num} {time_secs}|date>", unix)
 }
 
+// nolint:unparam
 func truncateText(text string, max int) string {
 	if len(text) <= max {
 		return text

pkg/app/piped/planner/kubernetes/kubernetes.go

@@ -341,7 +341,7 @@ func findWorkloadManifests(manifests []provider.Manifest, refs []config.K8sResou
 }
 
 func findManifests(kind, name string, manifests []provider.Manifest) []provider.Manifest {
-	var out []provider.Manifest
+	out := make([]provider.Manifest, 0, len(manifests))
 	for _, m := range manifests {
 		if m.Key.Kind != kind {
 			continue