From 5fbebdf2c7d1336a5a931c2321090790adaca63e Mon Sep 17 00:00:00 2001 From: Chmouel Boudjnah Date: Mon, 3 Nov 2025 17:57:09 +0100 Subject: [PATCH 1/4] fix: prevent nil pointer panic in EventEmitter Add nil check for Kubernetes client in EmitMessage to prevent panic when client is uninitialized. This fixes a crash that occurs during error handling in GitLab provider when 401 errors are encountered. The EventEmitter now gracefully handles three nil scenarios: - nil repo: skips event creation, continues logging - nil logger: skips logging, continues event creation - nil client: skips event creation, continues logging Added comprehensive test coverage for nil client scenario to match existing tests for nil repo and nil logger cases. Fixes crash: runtime error: invalid memory address or nil pointer dereference at pkg/events/emit.go:36 Jira: https://issues.redhat.com/browse/SRVKP-8910 Assisted-by: Claude-Sonnet-4.5 (via Claude Code) Signed-off-by: Chmouel Boudjnah --- pkg/events/emit.go | 2 +- pkg/events/emit_test.go | 29 +++++++++++++++++++++++++++-- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/pkg/events/emit.go b/pkg/events/emit.go index b059842021..fec1a70d32 100644 --- a/pkg/events/emit.go +++ b/pkg/events/emit.go @@ -31,7 +31,7 @@ func (e *EventEmitter) SetLogger(logger *zap.SugaredLogger) { } func (e *EventEmitter) EmitMessage(repo *v1alpha1.Repository, loggerLevel zapcore.Level, reason, message string) { - if repo != nil { + if repo != nil && e.client != nil { event := makeEvent(repo, loggerLevel, reason, message) if _, err := e.client.CoreV1().Events(event.Namespace).Create(context.Background(), event, metav1.CreateOptions{}); err != nil { if e.logger != nil { diff --git a/pkg/events/emit_test.go b/pkg/events/emit_test.go index 9bce2eafba..5a1a51dc05 100644 --- a/pkg/events/emit_test.go +++ b/pkg/events/emit_test.go @@ -13,6 +13,7 @@ import ( "gotest.tools/v3/assert" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes" rtesting "knative.dev/pkg/reconciler/testing" ) @@ -28,6 +29,7 @@ func TestEventEmitter_EmitMessage(t *testing.T) { expectEvent bool expectedType string useNilLogger bool + useNilClient bool }{ { name: "repo exists", @@ -82,6 +84,21 @@ func TestEventEmitter_EmitMessage(t *testing.T) { expectedType: v1.EventTypeWarning, useNilLogger: true, }, + { + name: "nil client doesn't cause panic", + repo: &v1alpha1.Repository{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-repo", + Namespace: "test-ns", + }, + Spec: v1alpha1.RepositorySpec{}, + }, + message: "info-message", + logLevel: zap.InfoLevel, + expectEvent: false, + expectedType: v1.EventTypeNormal, + useNilClient: true, + }, } for _, tt := range tests { @@ -97,8 +114,16 @@ func TestEventEmitter_EmitMessage(t *testing.T) { logger = fakelogger } - // emit event - this should not panic even with nil logger - NewEventEmitter(stdata.Kube, logger).EmitMessage(tt.repo, tt.logLevel, tt.reason, tt.message) + // Use nil client for specific test case + var client kubernetes.Interface + if tt.useNilClient { + client = nil + } else { + client = stdata.Kube + } + + // emit event - this should not panic even with nil logger or nil client + NewEventEmitter(client, logger).EmitMessage(tt.repo, tt.logLevel, tt.reason, tt.message) if tt.expectEvent { events, err := stdata.Kube.CoreV1().Events(tt.repo.Namespace).List(context.Background(), metav1.ListOptions{}) From ac8e45ccbce63db99fd414ff2476cf80c094f976 Mon Sep 17 00:00:00 2001 From: Chmouel Boudjnah Date: Thu, 30 Oct 2025 16:47:00 +0100 Subject: [PATCH 2/4] refactor: Migrate build tags from +build to //go:build directives Converted deprecated `// +build` directives to the modern `//go:build` format across several files. This standardized the build constraint declarations. Signed-off-by: Chmouel Boudjnah --- .../v1alpha1/zz_generated.deepcopy.go | 1 - test/bitbucket_cloud_pullrequest_test.go | 1 - ...ucket_datacenter_dynamic_variables_test.go | 1 - .../bitbucket_datacenter_pull_request_test.go | 1 - test/bitbucket_datacenter_push_test.go | 1 - test/gitea_results_annotation_test.go | 1 - test/github_config_maxkeepruns_test.go | 1 - test/github_incoming_test.go | 1 - ...pullrequest_concurrency_multiplepr_test.go | 1 - test/github_pullrequest_concurrency_test.go | 1 - test/github_pullrequest_oktotest_test.go | 1 - ...thub_pullrequest_privaterepository_test.go | 1 - test/github_pullrequest_rerequest_test.go | 1 - test/github_pullrequest_retest_test.go | 1 - test/github_pullrequest_test_comment_test.go | 1 - test/github_push_retest_test.go | 1 - test/github_push_test.go | 1 - ...en_to_list_of_private_public_repos_test.go | 1 - test/github_tkn_pac_cli_test.go | 1 - test/gitlab_delete_tag_event_test.go | 1 - test/gitlab_incoming_webhook_test.go | 1 - test/gitlab_oktotest_thread_reply_test.go | 94 +++++++++++++++++++ test/gitlab_push_gitops_command_test.go | 1 - test/invalid_event_test.go | 1 - test/repo_validation_test.go | 1 - test/repository_webhook_test.go | 1 - 26 files changed, 94 insertions(+), 25 deletions(-) create mode 100644 test/gitlab_oktotest_thread_reply_test.go diff --git a/pkg/apis/pipelinesascode/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/pipelinesascode/v1alpha1/zz_generated.deepcopy.go index 1ff9bcc9ef..34682e2156 100644 --- a/pkg/apis/pipelinesascode/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/pipelinesascode/v1alpha1/zz_generated.deepcopy.go @@ -1,5 +1,4 @@ //go:build !ignore_autogenerated -// +build !ignore_autogenerated /* Copyright Red Hat diff --git a/test/bitbucket_cloud_pullrequest_test.go b/test/bitbucket_cloud_pullrequest_test.go index d863360043..c618ed8e79 100644 --- a/test/bitbucket_cloud_pullrequest_test.go +++ b/test/bitbucket_cloud_pullrequest_test.go @@ -1,5 +1,4 @@ //go:build e2e -// +build e2e package test diff --git a/test/bitbucket_datacenter_dynamic_variables_test.go b/test/bitbucket_datacenter_dynamic_variables_test.go index eb06b23584..8388c5762d 100644 --- a/test/bitbucket_datacenter_dynamic_variables_test.go +++ b/test/bitbucket_datacenter_dynamic_variables_test.go @@ -1,5 +1,4 @@ //go:build e2e -// +build e2e package test diff --git a/test/bitbucket_datacenter_pull_request_test.go b/test/bitbucket_datacenter_pull_request_test.go index f7acea18e7..c46e49cf79 100644 --- a/test/bitbucket_datacenter_pull_request_test.go +++ b/test/bitbucket_datacenter_pull_request_test.go @@ -1,5 +1,4 @@ //go:build e2e -// +build e2e package test diff --git a/test/bitbucket_datacenter_push_test.go b/test/bitbucket_datacenter_push_test.go index a8e442cc61..fa77c997b0 100644 --- a/test/bitbucket_datacenter_push_test.go +++ b/test/bitbucket_datacenter_push_test.go @@ -1,5 +1,4 @@ //go:build e2e -// +build e2e package test diff --git a/test/gitea_results_annotation_test.go b/test/gitea_results_annotation_test.go index 27bd8207f0..b0e0282276 100644 --- a/test/gitea_results_annotation_test.go +++ b/test/gitea_results_annotation_test.go @@ -1,5 +1,4 @@ //go:build e2e -// +build e2e package test diff --git a/test/github_config_maxkeepruns_test.go b/test/github_config_maxkeepruns_test.go index 4238129964..d53c26b335 100644 --- a/test/github_config_maxkeepruns_test.go +++ b/test/github_config_maxkeepruns_test.go @@ -1,5 +1,4 @@ //go:build e2e -// +build e2e package test diff --git a/test/github_incoming_test.go b/test/github_incoming_test.go index 4c1bc6b2b0..d7d9101764 100644 --- a/test/github_incoming_test.go +++ b/test/github_incoming_test.go @@ -1,5 +1,4 @@ //go:build e2e -// +build e2e package test diff --git a/test/github_pullrequest_concurrency_multiplepr_test.go b/test/github_pullrequest_concurrency_multiplepr_test.go index 247d27f9f1..c58b931751 100644 --- a/test/github_pullrequest_concurrency_multiplepr_test.go +++ b/test/github_pullrequest_concurrency_multiplepr_test.go @@ -1,5 +1,4 @@ //go:build e2e -// +build e2e package test diff --git a/test/github_pullrequest_concurrency_test.go b/test/github_pullrequest_concurrency_test.go index bf2b67e7fe..1a5f7b348a 100644 --- a/test/github_pullrequest_concurrency_test.go +++ b/test/github_pullrequest_concurrency_test.go @@ -1,5 +1,4 @@ //go:build e2e -// +build e2e package test diff --git a/test/github_pullrequest_oktotest_test.go b/test/github_pullrequest_oktotest_test.go index 13eb6b0198..8559ff2c4c 100644 --- a/test/github_pullrequest_oktotest_test.go +++ b/test/github_pullrequest_oktotest_test.go @@ -1,5 +1,4 @@ //go:build e2e -// +build e2e package test diff --git a/test/github_pullrequest_privaterepository_test.go b/test/github_pullrequest_privaterepository_test.go index 5736d82f4d..ea258f7409 100644 --- a/test/github_pullrequest_privaterepository_test.go +++ b/test/github_pullrequest_privaterepository_test.go @@ -1,5 +1,4 @@ //go:build e2e -// +build e2e package test diff --git a/test/github_pullrequest_rerequest_test.go b/test/github_pullrequest_rerequest_test.go index 5746ae6a0f..bc696d8c6a 100644 --- a/test/github_pullrequest_rerequest_test.go +++ b/test/github_pullrequest_rerequest_test.go @@ -1,5 +1,4 @@ //go:build e2e -// +build e2e package test diff --git a/test/github_pullrequest_retest_test.go b/test/github_pullrequest_retest_test.go index f34df2ac3f..b6f3f4ab2b 100644 --- a/test/github_pullrequest_retest_test.go +++ b/test/github_pullrequest_retest_test.go @@ -1,5 +1,4 @@ //go:build e2e -// +build e2e package test diff --git a/test/github_pullrequest_test_comment_test.go b/test/github_pullrequest_test_comment_test.go index d3b36dd3d8..b3f9c147ae 100644 --- a/test/github_pullrequest_test_comment_test.go +++ b/test/github_pullrequest_test_comment_test.go @@ -1,5 +1,4 @@ //go:build e2e -// +build e2e package test diff --git a/test/github_push_retest_test.go b/test/github_push_retest_test.go index 280826917b..97e28dfe90 100644 --- a/test/github_push_retest_test.go +++ b/test/github_push_retest_test.go @@ -1,5 +1,4 @@ //go:build e2e -// +build e2e package test diff --git a/test/github_push_test.go b/test/github_push_test.go index 4562b9036c..f1622027f6 100644 --- a/test/github_push_test.go +++ b/test/github_push_test.go @@ -1,5 +1,4 @@ //go:build e2e -// +build e2e package test diff --git a/test/github_scope_token_to_list_of_private_public_repos_test.go b/test/github_scope_token_to_list_of_private_public_repos_test.go index 8ee55a0338..cb76c68e38 100644 --- a/test/github_scope_token_to_list_of_private_public_repos_test.go +++ b/test/github_scope_token_to_list_of_private_public_repos_test.go @@ -1,5 +1,4 @@ //go:build e2e -// +build e2e package test diff --git a/test/github_tkn_pac_cli_test.go b/test/github_tkn_pac_cli_test.go index 9a99bb01e0..19f86f9b12 100644 --- a/test/github_tkn_pac_cli_test.go +++ b/test/github_tkn_pac_cli_test.go @@ -1,5 +1,4 @@ //go:build e2e -// +build e2e package test diff --git a/test/gitlab_delete_tag_event_test.go b/test/gitlab_delete_tag_event_test.go index 1afef6ff05..9f618f28a7 100644 --- a/test/gitlab_delete_tag_event_test.go +++ b/test/gitlab_delete_tag_event_test.go @@ -1,5 +1,4 @@ //go:build e2e -// +build e2e package test diff --git a/test/gitlab_incoming_webhook_test.go b/test/gitlab_incoming_webhook_test.go index d4a89e044e..ddd8f5e48d 100644 --- a/test/gitlab_incoming_webhook_test.go +++ b/test/gitlab_incoming_webhook_test.go @@ -1,5 +1,4 @@ //go:build e2e -// +build e2e package test diff --git a/test/gitlab_oktotest_thread_reply_test.go b/test/gitlab_oktotest_thread_reply_test.go new file mode 100644 index 0000000000..2ee9a7ae5e --- /dev/null +++ b/test/gitlab_oktotest_thread_reply_test.go @@ -0,0 +1,94 @@ +//go:build e2e + +package test + +import ( + "context" + "net/http" + "testing" + + "github.com/openshift-pipelines/pipelines-as-code/test/pkg/cctx" + tgitlab "github.com/openshift-pipelines/pipelines-as-code/test/pkg/gitlab" + "github.com/openshift-pipelines/pipelines-as-code/test/pkg/payload" + "github.com/openshift-pipelines/pipelines-as-code/test/pkg/scm" + twait "github.com/openshift-pipelines/pipelines-as-code/test/pkg/wait" + "github.com/tektoncd/pipeline/pkg/names" + clientGitlab "gitlab.com/gitlab-org/api/client-go" + "gotest.tools/v3/assert" +) + +// TestGitlabOpsCommentInThreadReply verifies that a /test command placed +// in a reply within a discussion thread on a Merge Request is honored. +func TestGitlabOpsCommentInThreadReply(t *testing.T) { + targetNS := names.SimpleNameGenerator.RestrictLengthWithRandomSuffix("pac-e2e-ns") + ctx := context.Background() + runcnx, opts, glprovider, err := tgitlab.Setup(ctx) + assert.NilError(t, err) + ctx, err = cctx.GetControllerCtxInfo(ctx, runcnx) + assert.NilError(t, err) + runcnx.Clients.Log.Info("Testing GitLab /test in thread replies") + + projectinfo, resp, err := glprovider.Client().Projects.GetProject(opts.ProjectID, nil) + assert.NilError(t, err) + if resp != nil && resp.StatusCode == http.StatusNotFound { + t.Errorf("Repository %s not found in %s", opts.Organization, opts.Repo) + } + + // Create Repository CRD + err = tgitlab.CreateCRD(ctx, projectinfo, runcnx, opts, targetNS, nil) + assert.NilError(t, err) + + // Create a basic PipelineRun + entries, err := payload.GetEntries(map[string]string{ + ".tekton/pipelinerun.yaml": "testdata/pipelinerun.yaml", + }, targetNS, projectinfo.DefaultBranch, "pull_request", map[string]string{}) + assert.NilError(t, err) + + // Create a branch with files and open a merge request + targetRefName := names.SimpleNameGenerator.RestrictLengthWithRandomSuffix("pac-e2e-test") + gitCloneURL, err := scm.MakeGitCloneURL(projectinfo.WebURL, opts.UserName, opts.Password) + assert.NilError(t, err) + commitTitle := "Committing files from test on " + targetRefName + scmOpts := &scm.Opts{ + GitURL: gitCloneURL, + CommitTitle: commitTitle, + Log: runcnx.Clients.Log, + WebURL: projectinfo.WebURL, + TargetRefName: targetRefName, + BaseRefName: projectinfo.DefaultBranch, + } + _ = scm.PushFilesToRefGit(t, scmOpts, entries) + mrTitle := "TestMergeRequest - " + targetRefName + mrID, err := tgitlab.CreateMR(glprovider.Client(), opts.ProjectID, targetRefName, projectinfo.DefaultBranch, mrTitle) + assert.NilError(t, err) + defer tgitlab.TearDown(ctx, t, runcnx, glprovider, mrID, targetRefName, targetNS, opts.ProjectID) + + // Create a discussion thread with an initial note + disc, _, err := glprovider.Client().Discussions.CreateMergeRequestDiscussion(opts.ProjectID, mrID, &clientGitlab.CreateMergeRequestDiscussionOptions{ + Body: clientGitlab.Ptr("random initial note"), + }) + assert.NilError(t, err) + + // Wait for repository status to reflect a successful run triggered by the comment + waitOpts := twait.Opts{ + RepoName: targetNS, + Namespace: targetNS, + MinNumberStatus: 1, + PollTimeout: twait.DefaultTimeout, + TargetSHA: "", + } + _, err = twait.UntilRepositoryUpdated(ctx, runcnx.Clients, waitOpts) + assert.NilError(t, err) + + runcnx.Clients.Log.Info("Updating discussion with /test comment in a reply thread") + // Add a reply to the discussion containing /test + _, _, err = glprovider.Client().Discussions.AddMergeRequestDiscussionNote(opts.ProjectID, mrID, disc.ID, &clientGitlab.AddMergeRequestDiscussionNoteOptions{ + Body: clientGitlab.Ptr("/test"), + }) + assert.NilError(t, err) + waitOpts.MinNumberStatus = 2 + _, err = twait.UntilRepositoryUpdated(ctx, runcnx.Clients, waitOpts) + assert.NilError(t, err) + + runcnx.Clients.Log.Info("Repository status updated after /test comment") +} diff --git a/test/gitlab_push_gitops_command_test.go b/test/gitlab_push_gitops_command_test.go index 871df20de3..0baee2f1e7 100644 --- a/test/gitlab_push_gitops_command_test.go +++ b/test/gitlab_push_gitops_command_test.go @@ -1,5 +1,4 @@ //go:build e2e -// +build e2e package test diff --git a/test/invalid_event_test.go b/test/invalid_event_test.go index e4885eebab..c183950dc7 100644 --- a/test/invalid_event_test.go +++ b/test/invalid_event_test.go @@ -1,5 +1,4 @@ //go:build e2e -// +build e2e package test diff --git a/test/repo_validation_test.go b/test/repo_validation_test.go index d69f3cf78f..1956382cd7 100644 --- a/test/repo_validation_test.go +++ b/test/repo_validation_test.go @@ -1,5 +1,4 @@ //go:build e2e -// +build e2e package test diff --git a/test/repository_webhook_test.go b/test/repository_webhook_test.go index 0ff689a3d5..46f7df1d23 100644 --- a/test/repository_webhook_test.go +++ b/test/repository_webhook_test.go @@ -1,5 +1,4 @@ //go:build e2e -// +build e2e package test From 3725c385ab1f72080fa60f4885875b3e235f045a Mon Sep 17 00:00:00 2001 From: Akshay Pant Date: Wed, 22 Oct 2025 11:50:27 +0530 Subject: [PATCH 3/4] chore(goreleaser): use binaries field in homebrew Replace deprecated binary field with binaries array syntax in homebrew_casks configuration to resolve GoReleaser deprecation warning. Signed-off-by: Akshay Pant --- .goreleaser.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.goreleaser.yml b/.goreleaser.yml index 17a28f573f..02410041dc 100644 --- a/.goreleaser.yml +++ b/.goreleaser.yml @@ -86,7 +86,7 @@ homebrew_casks: - formula: git homepage: "https://pipelinesascode.com" description: tkn-pac - A command line interface for interacting with Pipelines as Code - binary: "tkn-pac" + binaries: ["tkn-pac"] custom_block: | zsh_completion = "#{staged_path}/_tkn-pac" bash_completion = "#{staged_path}/tkn-pac.bash" From 8195b9d24fb2957656f0b4cf89eea02b37e14d70 Mon Sep 17 00:00:00 2001 From: Zaki Shaikh Date: Fri, 7 Nov 2025 17:51:44 +0530 Subject: [PATCH 4/4] fix: delete go build tag for CI Signed-off-by: Zaki Shaikh --- test/gitea_params_test.go | 1 - test/gitlab_merge_request_test.go | 1 - 2 files changed, 2 deletions(-) diff --git a/test/gitea_params_test.go b/test/gitea_params_test.go index 368914c275..c85c39498f 100644 --- a/test/gitea_params_test.go +++ b/test/gitea_params_test.go @@ -1,5 +1,4 @@ //go:build e2e -// +build e2e package test diff --git a/test/gitlab_merge_request_test.go b/test/gitlab_merge_request_test.go index 2feb6b8379..71cf645e6b 100644 --- a/test/gitlab_merge_request_test.go +++ b/test/gitlab_merge_request_test.go @@ -1,5 +1,4 @@ //go:build e2e -// +build e2e package test