Update golangci-lint to 2.0

- Update golangci-lint configuration with enabled linters and exclusion paths
- Adjust struct field accesses from status.Status.Conditions to
status.Conditions
- Simplify PipelineRun name references using ObjectMeta.Name shortcut
- Fix invalid URL test data and control character handling assertions
- Correct pagination header check in Gitea provider
- Update test assertions for PipelineRun GenerateName fields
- Add unknown status handling in GitHub retest verification

Signed-off-by: Chmouel Boudjnah <[email protected]>

Author: chmouel

.golangci.yml

@@ -1,132 +1,105 @@
-issues:
-  exclude-dirs:
-    - vendor
-    - pkg/provider/gitea/structs
-  exclude-rules:
-    # Exclude some linters from running on tests files.
-    - path: _test\.go
-      linters:
-        - gosec
-    # hopefully we can remove this when https://github.com/golangci/golangci-lint/issues/4697 is fixed
-    - path: pkg/resolve/resolve.go
-      text: "don't use `init` function"
+version: "2"
 run:
   build-tags:
     - e2e
-linters-settings:
-  gocritic:
-    disabled-checks:
-      - unlambda
-  errcheck:
-    exclude-functions:
-      - (*github.com/tektoncd/pipeline/vendor/go.uber.org/zap.SugaredLogger).Sync
-      - flag.Set
-      - logger.Sync
-      - fmt.Fprintf
-      - fmt.Fprintln
-      - (io.Closer).Close
-      - updateConfigMap
-  gofumpt:
-    extra-rules: true
 linters:
   enable:
     - asasalint
     - asciicheck
     - bidichk
     - bodyclose
-    #- containedctx
-    #- contextcheck
-    #- cyclop
+    - copyloopvar
     - decorder
-    #- depguard
     - dogsled
     - dupl
     - dupword
     - durationcheck
-    - errcheck
     - errchkjson
     - errname
     - errorlint
-    # - execinquery
     - exhaustive
-    #- exhaustruct
-    - copyloopvar
     - forbidigo
     - forcetypeassert
-    #- funlen
-    #- gci
     - ginkgolinter
     - gocheckcompilerdirectives
-    #- gochecknoglobals
     - gochecknoinits
     - gochecksumtype
-    #- gocognit
-    #- goconst
     - gocritic
-    #- gocyclo
     - godot
-    #- godox
-    #- goerr113
-    #- gofmt
-    - gofumpt
     - goheader
-    - goimports
-    #- gomnd
-    #- gomoddirectives
     - gomodguard
     - goprintffuncname
     - gosec
-    - gosimple
     - gosmopolitan
-    - govet
     - grouper
     - importas
-    #- inamedparam
-    #- interfacebloat
-    #- ireturn
-    #- lll
     - loggercheck
-    #- maintidx
     - makezero
     - mirror
     - misspell
-    #- musttag
     - nakedret
-    #- nestif
     - nilerr
-    #- nilnil
-    #- nlreturn
     - noctx
-    #- nolintlint
-    #- nonamedreturns
     - nosprintfhostport
-    #- paralleltest
-    #- perfsprint
     - prealloc
     - predeclared
     - promlinter
     - protogetter
     - reassign
     - revive
-    #- rowserrcheck
     - sloglint
-    #- sqlclosecheck
     - staticcheck
-    - stylecheck
     - tagalign
-    #- tagliatelle
     - testableexamples
-    # - testifylint
-    #- testpackage
-    #- thelper
     - tparallel
-    #- unconvert
     - unparam
-    - unused
     - usestdlibvars
-    #- varnamelen
-    #- wastedassign
     - whitespace
-    #- wrapcheck
-    #- wsl
     - zerologlint
+  settings:
+    errcheck:
+      exclude-functions:
+        - (*github.com/tektoncd/pipeline/vendor/go.uber.org/zap.SugaredLogger).Sync
+        - flag.Set
+        - logger.Sync
+        - fmt.Fprintf
+        - fmt.Fprintln
+        - (io.Closer).Close
+        - updateConfigMap
+    gocritic:
+      disabled-checks:
+        - unlambda
+  exclusions:
+    generated: lax
+    presets:
+      - comments
+      - common-false-positives
+      - legacy
+      - std-error-handling
+    rules:
+      - linters:
+          - gosec
+        path: _test\.go
+      - path: pkg/resolve/resolve.go
+        text: don't use `init` function
+    paths:
+      - vendor
+      - pkg/provider/gitea/structs
+      - third_party$
+      - builtin$
+      - examples$
+formatters:
+  enable:
+    - gofumpt
+    - goimports
+  settings:
+    gofumpt:
+      extra-rules: true
+  exclusions:
+    generated: lax
+    paths:
+      - vendor
+      - pkg/provider/gitea/structs
+      - third_party$
+      - builtin$
+      - examples$

pkg/cmd/tknpac/list/list.go

@@ -123,8 +123,8 @@ func formatStatus(status *v1alpha1.RepositoryRunStatus, cs *cli.ColorScheme, c c
 	}
 
 	reason := "UNKNOWN"
-	if len(status.Status.Conditions) > 0 {
-		reason = status.Status.Conditions[0].Reason
+	if len(status.Conditions) > 0 {
+		reason = status.Conditions[0].Reason
 	}
 	return fmt.Sprintf("%s\t%s", s, cs.HyperLink(cs.ColorStatus(reason), *status.LogURL))
 }

pkg/cmd/tknpac/logs/logs.go

@@ -191,7 +191,7 @@ func getPipelineRunsToRepo(ctx context.Context, lopt *logOption, repoName string
 		if lopt.limit > -1 && i > lopt.limit {
 			continue
 		}
-		ret = append(ret, fmt.Sprintf("%s %s %s", run.ObjectMeta.Name, label, formatting.Age(date, lopt.cw)))
+		ret = append(ret, fmt.Sprintf("%s %s %s", run.Name, label, formatting.Age(date, lopt.cw)))
 	}
 	return ret, nil
 }

pkg/provider/bitbucketserver/parse_payload_test.go

@@ -598,9 +598,8 @@ func TestParsePayload(t *testing.T) {
 					Sender:       "sender",
 					Organization: "PROJ",
 					Repository:   "repo",
-					//nolint: stylecheck
-					URL: "💢",
-					SHA: "abcd",
+					URL:          "\x03💢\x16",
+					SHA:          "abcd",
 				}, ""),
 			wantErrSubstr: "invalid control character",
 		},

pkg/provider/gitea/gitea.go

@@ -318,7 +318,7 @@ func (v *Provider) GetCommitInfo(_ context.Context, runevent *info.Event) error
 }
 
 func ShouldGetNextPage(resp *gitea.Response, currentPage int) (bool, int) {
-	val, exists := resp.Response.Header[http.CanonicalHeaderKey("x-pagecount")]
+	val, exists := resp.Header[http.CanonicalHeaderKey("x-pagecount")]
 	if !exists {
 		return false, 0
 	}

pkg/provider/gitlab/gitlab.go

@@ -329,7 +329,7 @@ func (v *Provider) getObject(fname, branch string, pid int) ([]byte, *gitlab.Res
 	if err != nil {
 		return []byte{}, resp, fmt.Errorf("failed to get filename from api %s dir: %w", fname, err)
 	}
-	if resp != nil && resp.Response.StatusCode == http.StatusNotFound {
+	if resp != nil && resp.StatusCode == http.StatusNotFound {
 		return []byte{}, resp, nil
 	}
 	return file, resp, nil

pkg/resolve/resolve_test.go

@@ -85,11 +85,11 @@ func TestPipelineRunPipelineMiddle(t *testing.T) {
 func TestGenerateName(t *testing.T) {
 	resolved, _, err := readTDfile(t, "pipelinerun-pipeline-task", true, true)
 	assert.NilError(t, err)
-	assert.Assert(t, resolved.ObjectMeta.GenerateName != "")
+	assert.Assert(t, resolved.GenerateName != "")
 
 	resolved, _, err = readTDfile(t, "with-generatename", true, true)
 	assert.NilError(t, err)
-	assert.Assert(t, resolved.ObjectMeta.GenerateName != "")
+	assert.Assert(t, resolved.GenerateName != "")
 }
 
 // TestPipelineBundlesSkipped effectively test conversion from beta1 to v1.

pkg/test/clients/clients.go

@@ -51,7 +51,7 @@ type Data struct {
 
 // SeedTestData returns Clients and Informers populated with the
 // given Data.
-func SeedTestData(t *testing.T, ctx context.Context, d Data) (Clients, Informers) { //nolint: golint, revive
+func SeedTestData(t *testing.T, ctx context.Context, d Data) (Clients, Informers) { //nolint: revive
 	c := Clients{
 		PipelineAsCode: fakepacclient.Get(ctx),
 		Kube:           fakekubeclient.Get(ctx),

test/github_pullrequest_retest_test.go

@@ -86,14 +86,54 @@ func TestGithubPullRequestGitOpsComments(t *testing.T) {
 			prNum:   4,
 		},
 	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			waitOpts := twait.Opts{
-				RepoName:        g.TargetNamespace,
-				Namespace:       g.TargetNamespace,
-				MinNumberStatus: tt.prNum,
-				PollTimeout:     twait.DefaultTimeout,
-				TargetSHA:       g.SHA,
+	assert.NilError(t, err)
+	err = twait.UntilPipelineRunCreated(ctx, g.Cnx.Clients, waitOpts)
+	assert.NilError(t, err)
+
+	g.Cnx.Clients.Log.Infof("/cancel pr-gitops-comment on Pull Request")
+	_, _, err = g.Provider.Client.Issues.CreateComment(ctx,
+		g.Options.Organization,
+		g.Options.Repo, g.PRNumber,
+		&github.IssueComment{Body: github.Ptr("/cancel pr-gitops-comment")})
+	assert.NilError(t, err)
+
+	waitOpts = twait.Opts{
+		RepoName:        g.TargetNamespace,
+		Namespace:       g.TargetNamespace,
+		MinNumberStatus: 3,
+		PollTimeout:     twait.DefaultTimeout,
+		TargetSHA:       g.SHA,
+	}
+	g.Cnx.Clients.Log.Info("Waiting for Repository to be updated")
+	_, err = twait.UntilRepositoryUpdated(ctx, g.Cnx.Clients, waitOpts)
+	assert.NilError(t, err)
+
+	g.Cnx.Clients.Log.Infof("Check if we have the repository set as succeeded")
+	repo, err := g.Cnx.Clients.PipelineAsCode.PipelinesascodeV1alpha1().Repositories(g.TargetNamespace).Get(ctx, g.TargetNamespace, metav1.GetOptions{})
+	assert.NilError(t, err)
+	assert.Equal(t, repo.Status[len(repo.Status)-1].Conditions[0].Status, corev1.ConditionFalse)
+
+	pruns, err = g.Cnx.Clients.Tekton.TektonV1().PipelineRuns(g.TargetNamespace).List(ctx, metav1.ListOptions{
+		LabelSelector: fmt.Sprintf("%s=%s", keys.SHA, g.SHA),
+	})
+	assert.NilError(t, err)
+	assert.Equal(t, len(pruns.Items), 3)
+
+	// go over all pruns check that at least one is canceled and the other two are succeeded
+	canceledCount := 0
+	succeededCount := 0
+	unknownCount := 0
+	for _, prun := range pruns.Items {
+		for _, condition := range prun.Status.Conditions {
+			if condition.Type == "Succeeded" {
+				switch condition.Status {
+				case corev1.ConditionFalse:
+					canceledCount++
+				case corev1.ConditionTrue:
+					succeededCount++
+				case corev1.ConditionUnknown:
+					unknownCount++
+				}
 			}
 			if tt.comment == "/cancel pr-gitops-comment" {
 				g.Cnx.Clients.Log.Info("/test pr-gitops-comment on Pull Request before canceling")
@@ -132,4 +172,7 @@ func TestGithubPullRequestGitOpsComments(t *testing.T) {
 			assert.Equal(t, len(pruns.Items), tt.prNum)
 		})
 	}
+	assert.Equal(t, canceledCount, 1, "should have one canceled PipelineRun")
+	assert.Equal(t, succeededCount, 2, "should have two succeeded PipelineRuns")
+	assert.Equal(t, unknownCount, 0, "should have zero unknown PipelineRuns: %+v", pruns.Items)
 }