From 42aebccee6e43aba898d378f6e625f948c8209bd Mon Sep 17 00:00:00 2001 From: Zaki Shaikh Date: Mon, 13 Apr 2026 14:51:54 +0530 Subject: [PATCH 1/2] refactor: move client setup to its own package this moves the git provider client setup logic to its own package gitclient so that we can call it from github package. at the moment if we call client setup from github it creates import cycle. Signed-off-by: Zaki Shaikh --- pkg/adapter/sinker.go | 3 ++- pkg/cli/webhook/secret.go | 12 ++++++------ pkg/cli/webhook/secret_test.go | 4 ++-- pkg/cmd/tknpac/webhook/add.go | 4 ++-- pkg/cmd/tknpac/webhook/update-token.go | 4 ++-- pkg/{pipelineascode => gitclient}/client_setup.go | 7 ++++--- .../client_setup_test.go | 2 +- pkg/pipelineascode/match.go | 3 ++- pkg/provider/gitlab/parse_payload.go | 4 ++-- pkg/reconciler/reconciler.go | 9 ++++----- pkg/secrets/{secrets.go => pipelinerun_secrets.go} | 0 .../{secrets_test.go => pipelinerun_secrets_test.go} | 0 pkg/{pipelineascode => secrets}/secret.go | 2 +- pkg/{pipelineascode => secrets}/secret_test.go | 2 +- 14 files changed, 29 insertions(+), 27 deletions(-) rename pkg/{pipelineascode => gitclient}/client_setup.go (95%) rename pkg/{pipelineascode => gitclient}/client_setup_test.go (99%) rename pkg/secrets/{secrets.go => pipelinerun_secrets.go} (100%) rename pkg/secrets/{secrets_test.go => pipelinerun_secrets_test.go} (100%) rename pkg/{pipelineascode => secrets}/secret.go (99%) rename pkg/{pipelineascode => secrets}/secret_test.go (99%) diff --git a/pkg/adapter/sinker.go b/pkg/adapter/sinker.go index 35a6f2a93d..26b2314f44 100644 --- a/pkg/adapter/sinker.go +++ b/pkg/adapter/sinker.go @@ -8,6 +8,7 @@ import ( "github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/v1alpha1" "github.com/openshift-pipelines/pipelines-as-code/pkg/kubeinteraction" + "github.com/openshift-pipelines/pipelines-as-code/pkg/gitclient" "github.com/openshift-pipelines/pipelines-as-code/pkg/matcher" "github.com/openshift-pipelines/pipelines-as-code/pkg/params" "github.com/openshift-pipelines/pipelines-as-code/pkg/params/info" @@ -149,7 +150,7 @@ func (s *sinker) findMatchingRepository(ctx context.Context) (*v1alpha1.Reposito // Centralizing this here ensures consistent behavior across all events and enables early // optimizations like skip-CI detection before expensive processing. func (s *sinker) setupClient(ctx context.Context, repo *v1alpha1.Repository) error { - return pipelineascode.SetupAuthenticatedClient( + return gitclient.SetupAuthenticatedClient( ctx, s.vcx, s.kint, diff --git a/pkg/cli/webhook/secret.go b/pkg/cli/webhook/secret.go index 4b8b2a7fe0..f231985062 100644 --- a/pkg/cli/webhook/secret.go +++ b/pkg/cli/webhook/secret.go @@ -5,7 +5,7 @@ import ( "fmt" "github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/v1alpha1" - "github.com/openshift-pipelines/pipelines-as-code/pkg/pipelineascode" + "github.com/openshift-pipelines/pipelines-as-code/pkg/secrets" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -16,8 +16,8 @@ func (w *Options) createWebhookSecret(ctx context.Context, response *response) e Name: w.RepositoryName, }, Data: map[string][]byte{ - pipelineascode.DefaultGitProviderSecretKey: []byte(response.PersonalAccessToken), - pipelineascode.DefaultGitProviderWebhookSecretKey: []byte(response.WebhookSecret), + secrets.DefaultGitProviderSecretKey: []byte(response.PersonalAccessToken), + secrets.DefaultGitProviderWebhookSecretKey: []byte(response.WebhookSecret), }, }, metav1.CreateOptions{}) if err != nil { @@ -33,7 +33,7 @@ func (w *Options) updateWebhookSecret(ctx context.Context, response *response) e if err != nil { return err } - secretInfo.Data[pipelineascode.DefaultGitProviderWebhookSecretKey] = []byte(response.WebhookSecret) + secretInfo.Data[secrets.DefaultGitProviderWebhookSecretKey] = []byte(response.WebhookSecret) _, err = w.Run.Clients.Kube.CoreV1().Secrets(w.RepositoryNamespace).Update(ctx, secretInfo, metav1.UpdateOptions{}) if err != nil { @@ -57,11 +57,11 @@ func (w *Options) updateRepositoryCR(ctx context.Context, res *response) error { repo.Spec.GitProvider.Secret = &v1alpha1.Secret{ Name: w.RepositoryName, - Key: pipelineascode.DefaultGitProviderSecretKey, + Key: secrets.DefaultGitProviderSecretKey, } repo.Spec.GitProvider.WebhookSecret = &v1alpha1.Secret{ Name: w.RepositoryName, - Key: pipelineascode.DefaultGitProviderWebhookSecretKey, + Key: secrets.DefaultGitProviderWebhookSecretKey, } if res.UserName != "" { diff --git a/pkg/cli/webhook/secret_test.go b/pkg/cli/webhook/secret_test.go index b6b9f33f3e..c2c4cf93e6 100644 --- a/pkg/cli/webhook/secret_test.go +++ b/pkg/cli/webhook/secret_test.go @@ -11,7 +11,7 @@ import ( "github.com/openshift-pipelines/pipelines-as-code/pkg/params/clients" "github.com/openshift-pipelines/pipelines-as-code/pkg/params/info" "github.com/openshift-pipelines/pipelines-as-code/pkg/params/settings" - "github.com/openshift-pipelines/pipelines-as-code/pkg/pipelineascode" + "github.com/openshift-pipelines/pipelines-as-code/pkg/secrets" testclient "github.com/openshift-pipelines/pipelines-as-code/pkg/test/clients" "github.com/openshift-pipelines/pipelines-as-code/pkg/test/logger" "gotest.tools/v3/assert" @@ -35,7 +35,7 @@ func TestWebHookSecret(t *testing.T) { Namespace: repoNS, }, Data: map[string][]byte{ - pipelineascode.DefaultGitProviderSecretKey: []byte("somethingsomething"), + secrets.DefaultGitProviderSecretKey: []byte("somethingsomething"), }, }, }, diff --git a/pkg/cmd/tknpac/webhook/add.go b/pkg/cmd/tknpac/webhook/add.go index 39ff184cd6..a2c037a1e9 100644 --- a/pkg/cmd/tknpac/webhook/add.go +++ b/pkg/cmd/tknpac/webhook/add.go @@ -11,7 +11,7 @@ import ( "github.com/openshift-pipelines/pipelines-as-code/pkg/cmd/tknpac/completion" "github.com/openshift-pipelines/pipelines-as-code/pkg/params" "github.com/openshift-pipelines/pipelines-as-code/pkg/params/settings" - "github.com/openshift-pipelines/pipelines-as-code/pkg/pipelineascode" + "github.com/openshift-pipelines/pipelines-as-code/pkg/secrets" "github.com/spf13/cobra" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -117,7 +117,7 @@ func add(ctx context.Context, opts *cli.PacCliOpts, run *params.Run, ioStreams * gitProviderSecretKey := repo.Spec.GitProvider.Secret.Key if gitProviderSecretKey == "" { - gitProviderSecretKey = pipelineascode.DefaultGitProviderSecretKey + gitProviderSecretKey = secrets.DefaultGitProviderSecretKey } tokenData, ok := secretData.Data[gitProviderSecretKey] diff --git a/pkg/cmd/tknpac/webhook/update-token.go b/pkg/cmd/tknpac/webhook/update-token.go index c180708595..5197b54f04 100644 --- a/pkg/cmd/tknpac/webhook/update-token.go +++ b/pkg/cmd/tknpac/webhook/update-token.go @@ -10,7 +10,7 @@ import ( "github.com/openshift-pipelines/pipelines-as-code/pkg/cli/prompt" "github.com/openshift-pipelines/pipelines-as-code/pkg/cmd/tknpac/completion" "github.com/openshift-pipelines/pipelines-as-code/pkg/params" - "github.com/openshift-pipelines/pipelines-as-code/pkg/pipelineascode" + "github.com/openshift-pipelines/pipelines-as-code/pkg/secrets" "github.com/spf13/cobra" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -110,7 +110,7 @@ func update(ctx context.Context, opts *cli.PacCliOpts, run *params.Run, ioStream gitProviderSecretKey := repo.Spec.GitProvider.Secret.Key if gitProviderSecretKey == "" { - gitProviderSecretKey = pipelineascode.DefaultGitProviderSecretKey + gitProviderSecretKey = secrets.DefaultGitProviderSecretKey } secretData.Data[gitProviderSecretKey] = []byte(personalAccessToken) diff --git a/pkg/pipelineascode/client_setup.go b/pkg/gitclient/client_setup.go similarity index 95% rename from pkg/pipelineascode/client_setup.go rename to pkg/gitclient/client_setup.go index dc57f21558..684a3eda18 100644 --- a/pkg/pipelineascode/client_setup.go +++ b/pkg/gitclient/client_setup.go @@ -1,4 +1,4 @@ -package pipelineascode +package gitclient import ( "context" @@ -11,6 +11,7 @@ import ( "github.com/openshift-pipelines/pipelines-as-code/pkg/params" "github.com/openshift-pipelines/pipelines-as-code/pkg/params/info" "github.com/openshift-pipelines/pipelines-as-code/pkg/provider" + "github.com/openshift-pipelines/pipelines-as-code/pkg/secrets" "github.com/openshift-pipelines/pipelines-as-code/pkg/provider/github" semconv "go.opentelemetry.io/otel/semconv/v1.40.0" "go.opentelemetry.io/otel/trace" @@ -49,10 +50,10 @@ func SetupAuthenticatedClient( // GitHub Apps use controller secret, not Repository git_provider if event.InstallationID > 0 { logger.Debugf("setupAuthenticatedClient: github app installation id=%d, using controller webhook secret", event.InstallationID) - event.Provider.WebhookSecret, _ = GetCurrentNSWebhookSecret(ctx, kint, run) + event.Provider.WebhookSecret, _ = secrets.GetCurrentNSWebhookSecret(ctx, kint, run) } else { // Non-GitHub App providers use git_provider section in Repository spec - scm := SecretFromRepository{ + scm := secrets.SecretFromRepository{ K8int: kint, Config: vcx.GetConfig(), Event: event, diff --git a/pkg/pipelineascode/client_setup_test.go b/pkg/gitclient/client_setup_test.go similarity index 99% rename from pkg/pipelineascode/client_setup_test.go rename to pkg/gitclient/client_setup_test.go index 867edcb699..9340c3b555 100644 --- a/pkg/pipelineascode/client_setup_test.go +++ b/pkg/gitclient/client_setup_test.go @@ -1,4 +1,4 @@ -package pipelineascode +package gitclient import ( "context" diff --git a/pkg/pipelineascode/match.go b/pkg/pipelineascode/match.go index 9500346366..dc50437dfc 100644 --- a/pkg/pipelineascode/match.go +++ b/pkg/pipelineascode/match.go @@ -11,6 +11,7 @@ import ( apipac "github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/keys" "github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/v1alpha1" pacerrors "github.com/openshift-pipelines/pipelines-as-code/pkg/errors" + "github.com/openshift-pipelines/pipelines-as-code/pkg/gitclient" "github.com/openshift-pipelines/pipelines-as-code/pkg/matcher" "github.com/openshift-pipelines/pipelines-as-code/pkg/opscomments" "github.com/openshift-pipelines/pipelines-as-code/pkg/params/triggertype" @@ -75,7 +76,7 @@ func (p *PacRun) verifyRepoAndUser(ctx context.Context) (*v1alpha1.Repository, e // but we call it here as a safety net for edge cases (e.g., tests calling Run() directly, // or if the early setup in sinker failed/was skipped). The call is idempotent. // SetupAuthenticatedClient will merge global repo settings after determining secret namespace. - err = SetupAuthenticatedClient(ctx, p.vcx, p.k8int, p.run, p.event, repo, p.globalRepo, p.pacInfo, p.logger) + err = gitclient.SetupAuthenticatedClient(ctx, p.vcx, p.k8int, p.run, p.event, repo, p.globalRepo, p.pacInfo, p.logger) if err != nil { return repo, err } diff --git a/pkg/provider/gitlab/parse_payload.go b/pkg/provider/gitlab/parse_payload.go index c10cc8431d..7e91f9a5c7 100644 --- a/pkg/provider/gitlab/parse_payload.go +++ b/pkg/provider/gitlab/parse_payload.go @@ -13,7 +13,7 @@ import ( "github.com/openshift-pipelines/pipelines-as-code/pkg/params" "github.com/openshift-pipelines/pipelines-as-code/pkg/params/info" "github.com/openshift-pipelines/pipelines-as-code/pkg/params/triggertype" - "github.com/openshift-pipelines/pipelines-as-code/pkg/pipelineascode" + "github.com/openshift-pipelines/pipelines-as-code/pkg/secrets" "github.com/openshift-pipelines/pipelines-as-code/pkg/provider" gitlab "gitlab.com/gitlab-org/api/client-go" @@ -205,7 +205,7 @@ func (v *Provider) initGitLabClient(ctx context.Context, event *info.Event) (*in return event, err } - scm := pipelineascode.SecretFromRepository{ + scm := secrets.SecretFromRepository{ K8int: kubeInterface, Config: v.GetConfig(), Event: event, diff --git a/pkg/reconciler/reconciler.go b/pkg/reconciler/reconciler.go index e7cf7a93f2..bd9a715aad 100644 --- a/pkg/reconciler/reconciler.go +++ b/pkg/reconciler/reconciler.go @@ -30,7 +30,6 @@ import ( "github.com/openshift-pipelines/pipelines-as-code/pkg/params" "github.com/openshift-pipelines/pipelines-as-code/pkg/params/info" "github.com/openshift-pipelines/pipelines-as-code/pkg/params/settings" - pac "github.com/openshift-pipelines/pipelines-as-code/pkg/pipelineascode" prmetrics "github.com/openshift-pipelines/pipelines-as-code/pkg/pipelinerunmetrics" "github.com/openshift-pipelines/pipelines-as-code/pkg/provider" providerstatus "github.com/openshift-pipelines/pipelines-as-code/pkg/provider/status" @@ -286,9 +285,9 @@ func (r *Reconciler) reportFinalStatus(ctx context.Context, logger *zap.SugaredL r.run.Clients.ConsoleUI().SetParams(maptemplate) if event.InstallationID > 0 { - event.Provider.WebhookSecret, _ = pac.GetCurrentNSWebhookSecret(ctx, r.kinteract, r.run) + event.Provider.WebhookSecret, _ = secrets.GetCurrentNSWebhookSecret(ctx, r.kinteract, r.run) } else { - secretFromRepo := pac.SecretFromRepository{ + secretFromRepo := secrets.SecretFromRepository{ K8int: r.kinteract, Config: provider.GetConfig(), Event: event, @@ -424,7 +423,7 @@ func (r *Reconciler) initGitProviderClient(ctx context.Context, logger *zap.Suga // installation ID indicates Github App installation if event.InstallationID > 0 { - event.Provider.WebhookSecret, _ = pac.GetCurrentNSWebhookSecret(ctx, r.kinteract, r.run) + event.Provider.WebhookSecret, _ = secrets.GetCurrentNSWebhookSecret(ctx, r.kinteract, r.run) } else { // secretNS is needed when git provider is other than Github App. secretNS := repo.GetNamespace() @@ -435,7 +434,7 @@ func (r *Reconciler) initGitProviderClient(ctx context.Context, logger *zap.Suga repo.Spec.Merge(r.globalRepo.Spec) } - secretFromRepo := pac.SecretFromRepository{ + secretFromRepo := secrets.SecretFromRepository{ K8int: r.kinteract, Config: detectedProvider.GetConfig(), Event: event, diff --git a/pkg/secrets/secrets.go b/pkg/secrets/pipelinerun_secrets.go similarity index 100% rename from pkg/secrets/secrets.go rename to pkg/secrets/pipelinerun_secrets.go diff --git a/pkg/secrets/secrets_test.go b/pkg/secrets/pipelinerun_secrets_test.go similarity index 100% rename from pkg/secrets/secrets_test.go rename to pkg/secrets/pipelinerun_secrets_test.go diff --git a/pkg/pipelineascode/secret.go b/pkg/secrets/secret.go similarity index 99% rename from pkg/pipelineascode/secret.go rename to pkg/secrets/secret.go index 1a73e1d18a..1ec3c948ec 100644 --- a/pkg/pipelineascode/secret.go +++ b/pkg/secrets/secret.go @@ -1,4 +1,4 @@ -package pipelineascode +package secrets import ( "context" diff --git a/pkg/pipelineascode/secret_test.go b/pkg/secrets/secret_test.go similarity index 99% rename from pkg/pipelineascode/secret_test.go rename to pkg/secrets/secret_test.go index 0fcdc48fa1..ad95dfcc08 100644 --- a/pkg/pipelineascode/secret_test.go +++ b/pkg/secrets/secret_test.go @@ -1,4 +1,4 @@ -package pipelineascode +package secrets import ( "fmt" From 76bb854c561e7c02731a27c7f49467996666f312 Mon Sep 17 00:00:00 2001 From: Zaki Shaikh Date: Tue, 14 Apr 2026 11:13:41 +0530 Subject: [PATCH 2/2] fix(github-webhook): /ok-to-test is not triggering CI on PRs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When an unauthorized user opens a pull request on a repository configured with Pipelines-as-Code using GitHub webhook integration, commenting /ok-to-test as an admin does not trigger the CI pipeline. This happens because the GitHub client (ghClient) is never initialized for webhook-based issue comment events — the client setup only ran for GitHub App events during payload parsing. Root cause: - In the issue_comment handler, the code checked if ghClient was nil and returned an error, but for webhook integrations the client is legitimately nil at that point since webhooks authenticate differently from GitHub Apps. - The PR number was being extracted by parsing the HTML URL string instead of reading it directly from the event object. - The webhook request payload and headers were not being preserved on the event object, which is needed for webhook signature validation. Changes: - pkg/provider/github/parse_payload.go: - Add initGitHubWebhookClient() to initialize the provider client for webhook-based events using gitclient.SetupAuthenticatedClient - Preserve request headers and payload on the event object early in ParsePayload so they are available for webhook signature validation - Reorder handleIssueCommentEvent to match the repository first, then lazily initialize the GitHub client if nil (webhook case), before fetching the pull request details - Use event.GetIssue().GetNumber() directly instead of parsing the PR number from the HTML URL string - Remove the early ghClient nil check that blocked webhook events - pkg/provider/github/github.go: - Move GitHub App token scoping logic from gitclient into SetClient, keeping provider-specific concerns within the provider package - pkg/gitclient/client_setup.go: - Remove GitHub App token scoping (moved to provider) - Add global repository lookup when globalRepo is nil, so webhook-based flows can resolve credentials from the global repository configuration - Replace github provider import with metav1 for the Get call - pkg/provider/github/parse_payload_test.go: - Remove test cases that asserted ghClient nil was an error (no longer applicable) - Remove test for invalid PR URL parsing (PR number now read from event) - Add Number field to IssueCommentEvent test fixtures - pkg/provider/github/acl_test.go: - Add html_url and number to issue comment test payload to match new handleIssueCommentEvent flow that sets URL and PR number from the event object - pkg/provider/github/github_test.go: - Add Logger, pacInfo, and repo with Settings to SetClient test to support token scoping moved into SetClient - pkg/gitclient/client_setup_test.go: - Add GlobalRepository and Namespace to test seed data to match new global repo lookup - pkg/pipelineascode/pipelineascode_test.go: - Add GlobalRepository and Kube namespace to Run.Info to match new global repo lookup in SetupAuthenticatedClient - pkg/reconciler/reconciler_test.go: - Add Logger to Provider in reconciler test to support token scoping logging in SetClient Signed-off-by: Zaki Shaikh --- pkg/adapter/sinker.go | 2 +- pkg/adapter/sinker_test.go | 80 +---------- pkg/gitclient/client_setup.go | 28 ++-- pkg/gitclient/client_setup_test.go | 155 +++++++++++++++++++++- pkg/pipelineascode/pipelineascode_test.go | 6 +- pkg/provider/github/acl_test.go | 2 +- pkg/provider/github/github.go | 13 ++ pkg/provider/github/github_test.go | 12 +- pkg/provider/github/parse_payload.go | 59 +++++--- pkg/provider/github/parse_payload_test.go | 85 ++++++++---- pkg/provider/gitlab/parse_payload.go | 2 +- pkg/reconciler/reconciler_test.go | 3 +- 12 files changed, 301 insertions(+), 146 deletions(-) diff --git a/pkg/adapter/sinker.go b/pkg/adapter/sinker.go index 26b2314f44..86ce1b2613 100644 --- a/pkg/adapter/sinker.go +++ b/pkg/adapter/sinker.go @@ -7,8 +7,8 @@ import ( "net/http" "github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/v1alpha1" - "github.com/openshift-pipelines/pipelines-as-code/pkg/kubeinteraction" "github.com/openshift-pipelines/pipelines-as-code/pkg/gitclient" + "github.com/openshift-pipelines/pipelines-as-code/pkg/kubeinteraction" "github.com/openshift-pipelines/pipelines-as-code/pkg/matcher" "github.com/openshift-pipelines/pipelines-as-code/pkg/params" "github.com/openshift-pipelines/pipelines-as-code/pkg/params/info" diff --git a/pkg/adapter/sinker_test.go b/pkg/adapter/sinker_test.go index 0f6ef25058..aa255e39ce 100644 --- a/pkg/adapter/sinker_test.go +++ b/pkg/adapter/sinker_test.go @@ -105,18 +105,6 @@ func TestSetupClientGitHubAppVsOther(t *testing.T) { wantErr: false, wantRepositoryIDsCount: 0, // No extra repos }, - { - name: "GitHub App with extra repos - IDs should be populated", - installationID: 12345, - hasGitProvider: false, - extraReposConfigured: true, - extraRepoInstallIDs: map[string]int64{ - "another/one": 789, - "andanother/two": 10112, - }, - wantErr: false, - wantRepositoryIDsCount: 2, // Should have 2 extra repo IDs - }, { name: "Non-GitHub App requires git_provider", installationID: 0, @@ -162,37 +150,11 @@ func TestSetupClientGitHubAppVsOther(t *testing.T) { } } - // Setup extra repos if configured - extraRepos := []*v1alpha1.Repository{} - if tt.extraReposConfigured { - repo.Spec.Settings = &v1alpha1.Settings{ - GithubAppTokenScopeRepos: []string{}, - } - for repoName := range tt.extraRepoInstallIDs { - repo.Spec.Settings.GithubAppTokenScopeRepos = append( - repo.Spec.Settings.GithubAppTokenScopeRepos, - repoName, - ) - // Create matching repository CRs for extra repos - extraRepo := testnewrepo.NewRepo(testnewrepo.RepoTestcreationOpts{ - Name: repoName, - URL: "https://github.com/" + repoName, - InstallNamespace: "default", - }) - extraRepos = append(extraRepos, extraRepo) - } - } - - // Create test data with all repositories - allRepos := append([]*v1alpha1.Repository{repo}, extraRepos...) - run := setupTestData(t, allRepos) + run := setupTestData(t, []*v1alpha1.Repository{repo}) // Create a tracking provider to verify behavior trackingProvider := &trackingProviderImpl{ - TestProviderImp: testprovider.TestProviderImp{AllowIT: true}, - createTokenCalled: false, - repositoryIDs: []int64{}, - extraRepoInstallIDs: tt.extraRepoInstallIDs, + TestProviderImp: testprovider.TestProviderImp{AllowIT: true}, } trackingProvider.SetLogger(log) @@ -229,32 +191,6 @@ func TestSetupClientGitHubAppVsOther(t *testing.T) { } else { assert.NilError(t, err, "unexpected error: %v", err) } - - // For GitHub Apps with extra repos, verify CreateToken was called - // and repository IDs were populated - if tt.extraReposConfigured && !tt.wantErr { - assert.Assert(t, trackingProvider.createTokenCalled, - "CreateToken should have been called for extra repos") - - // Verify all expected repo IDs are present - for repoName, expectedID := range tt.extraRepoInstallIDs { - found := false - for _, id := range trackingProvider.repositoryIDs { - if id == expectedID { - found = true - break - } - } - assert.Assert(t, found, - "Repository ID %d for %s not found in provider.RepositoryIDs: %v", - expectedID, repoName, trackingProvider.repositoryIDs) - } - - assert.Equal(t, len(trackingProvider.repositoryIDs), tt.wantRepositoryIDsCount, - "Expected %d repository IDs, got %d: %v", - tt.wantRepositoryIDsCount, len(trackingProvider.repositoryIDs), - trackingProvider.repositoryIDs) - } }) } } @@ -262,19 +198,9 @@ func TestSetupClientGitHubAppVsOther(t *testing.T) { // trackingProviderImpl wraps TestProviderImp to track CreateToken calls and repository IDs. type trackingProviderImpl struct { testprovider.TestProviderImp - createTokenCalled bool - repositoryIDs []int64 - extraRepoInstallIDs map[string]int64 } -func (t *trackingProviderImpl) CreateToken(_ context.Context, repositories []string, _ *info.Event) (string, error) { - t.createTokenCalled = true - // Simulate adding repository IDs like the real CreateToken does - for _, repo := range repositories { - if id, ok := t.extraRepoInstallIDs[repo]; ok { - t.repositoryIDs = append(t.repositoryIDs, id) - } - } +func (t *trackingProviderImpl) CreateToken(_ context.Context, _ []string, _ *info.Event) (string, error) { return "fake-token", nil } diff --git a/pkg/gitclient/client_setup.go b/pkg/gitclient/client_setup.go index 684a3eda18..9f3bc47175 100644 --- a/pkg/gitclient/client_setup.go +++ b/pkg/gitclient/client_setup.go @@ -12,10 +12,10 @@ import ( "github.com/openshift-pipelines/pipelines-as-code/pkg/params/info" "github.com/openshift-pipelines/pipelines-as-code/pkg/provider" "github.com/openshift-pipelines/pipelines-as-code/pkg/secrets" - "github.com/openshift-pipelines/pipelines-as-code/pkg/provider/github" semconv "go.opentelemetry.io/otel/semconv/v1.40.0" "go.opentelemetry.io/otel/trace" "go.uber.org/zap" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) // SetupAuthenticatedClient sets up the authenticated VCS client with proper token scoping. @@ -33,6 +33,19 @@ func SetupAuthenticatedClient( pacInfo *info.PacOpts, logger *zap.SugaredLogger, ) error { + if globalRepo == nil && + run != nil && + run.Info.Controller != nil && + run.Info.Kube != nil && + run.Info.Kube.Namespace != "" && + run.Info.Controller.GlobalRepository != "" { + var err error + if globalRepo, err = run.Clients.PipelineAsCode.PipelinesascodeV1alpha1().Repositories(run.Info.Kube.Namespace).Get( + ctx, run.Info.Controller.GlobalRepository, metav1.GetOptions{}, + ); err != nil { + logger.Errorf("cannot get global repository: %v", err) + } + } // Determine secret namespace BEFORE merging repos // This preserves the ability to detect when credentials come from global repo secretNS := repo.GetNamespace() @@ -97,18 +110,5 @@ is that what you want? make sure you use -n when generating the secret, eg: echo } logger.Debugf("setupAuthenticatedClient: provider client initialized") - // Handle GitHub App token scoping for both global and repo-level configuration - if event.InstallationID > 0 { - logger.Debugf("setupAuthenticatedClient: scoping github app token") - token, err := github.ScopeTokenToListOfRepos(ctx, vcx, pacInfo, repo, run, event, eventEmitter, logger) - if err != nil { - return fmt.Errorf("failed to scope token: %w", err) - } - // If Global and Repo level configurations are not provided then lets not override the provider token. - if token != "" { - event.Provider.Token = token - } - } - return nil } diff --git a/pkg/gitclient/client_setup_test.go b/pkg/gitclient/client_setup_test.go index 9340c3b555..f09faa91aa 100644 --- a/pkg/gitclient/client_setup_test.go +++ b/pkg/gitclient/client_setup_test.go @@ -113,7 +113,11 @@ func seedTestData(ctx context.Context, t *testing.T, repos []*v1alpha1.Repositor }, Info: info.Info{ Controller: &info.ControllerInfo{ - Secret: "pipelines-as-code-secret", + Secret: "pipelines-as-code-secret", + GlobalRepository: "global-repo", + }, + Kube: &info.KubeOpts{ + Namespace: "default", }, }, } @@ -339,6 +343,155 @@ func TestSetupAuthenticatedClientRepositoryConfig(t *testing.T) { } } +// TestSetupAuthenticatedClientGlobalRepoAutoFetch tests the auto-fetch behavior +// when globalRepo is nil. +func TestSetupAuthenticatedClientGlobalRepoAutoFetch(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + globalRepoExist bool + passGlobalRepo bool + wantErr bool + description string + }{ + { + name: "globalRepo nil and exists in API is fetched and merged", + globalRepoExist: true, + passGlobalRepo: false, + wantErr: false, + description: "When globalRepo is nil and exists in the API, it should be fetched automatically", + }, + { + name: "globalRepo nil and does not exist in API continues without error", + globalRepoExist: false, + passGlobalRepo: false, + wantErr: false, + description: "When globalRepo is nil and does not exist in the API, function should continue without error", + }, + { + name: "globalRepo provided skips API fetch", + globalRepoExist: false, + passGlobalRepo: true, + wantErr: false, + description: "When globalRepo is provided, no API fetch should happen", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + ctx, log := setupTestContext(t) + + repo := createTestRepository(true) + repos := []*v1alpha1.Repository{repo} + + // If global repo should exist in the API, seed it + var globalRepo *v1alpha1.Repository + if tt.globalRepoExist { + globalRepo = testnewrepo.NewRepo(testnewrepo.RepoTestcreationOpts{ + Name: "global-repo", + URL: "https://github.com/global/repo", + InstallNamespace: "default", + }) + globalRepo.Spec.GitProvider = &v1alpha1.GitProvider{ + URL: "https://github.com", + Secret: &v1alpha1.Secret{ + Name: "global-secret", + Key: "token", + }, + } + repos = append(repos, globalRepo) + } + + run := seedTestData(ctx, t, repos) + testProvider := createTestProvider(log) + + kint := createTestKintMock(map[string]string{ + "test-secret": "test-token", + "global-secret": "global-token", + }) + + event := createTestEvent("pull_request", 0, "") + pacInfo := createTestPacInfo() + + // Determine what to pass as globalRepo parameter + var globalRepoParam *v1alpha1.Repository + if tt.passGlobalRepo { + globalRepoParam = &v1alpha1.Repository{ + ObjectMeta: metav1.ObjectMeta{ + Name: "provided-global-repo", + Namespace: "default", + }, + } + } + + err := SetupAuthenticatedClient(ctx, testProvider, kint, run, event, repo, globalRepoParam, pacInfo, log) + + if tt.wantErr { + assert.Assert(t, err != nil, "%s: expected error but got nil", tt.description) + } else { + assert.NilError(t, err, "%s: %v", tt.description, err) + } + }) + } +} + +// TestSetupAuthenticatedClientGlobalRepoMergesSettings verifies that when globalRepo +// is auto-fetched, its settings are properly merged into the local repo. +func TestSetupAuthenticatedClientGlobalRepoMergesSettings(t *testing.T) { + t.Parallel() + + ctx, log := setupTestContext(t) + + // Local repo with git_provider URL but no secret (secret comes from global) + repo := testnewrepo.NewRepo(testnewrepo.RepoTestcreationOpts{ + Name: "test-repo", + URL: "https://github.com/owner/repo", + InstallNamespace: "default", + }) + repo.Spec.GitProvider = &v1alpha1.GitProvider{ + URL: "https://github.com", + } + + // Global repo with git_provider credentials + globalRepo := testnewrepo.NewRepo(testnewrepo.RepoTestcreationOpts{ + Name: "global-repo", + URL: "https://github.com/global/repo", + InstallNamespace: "default", + }) + globalRepo.Spec.GitProvider = &v1alpha1.GitProvider{ + URL: "https://github.com", + Secret: &v1alpha1.Secret{ + Name: "global-secret", + Key: "token", + }, + WebhookSecret: &v1alpha1.Secret{ + Name: "global-secret", + Key: "webhook.secret", + }, + } + + run := seedTestData(ctx, t, []*v1alpha1.Repository{repo, globalRepo}) + testProvider := createTestProvider(log) + + kint := createTestKintMock(map[string]string{ + "global-secret": "global-token-value", + }) + + event := createTestEvent("pull_request", 0, "") + pacInfo := createTestPacInfo() + + // Pass nil globalRepo so it gets auto-fetched + err := SetupAuthenticatedClient(ctx, testProvider, kint, run, event, repo, nil, pacInfo, log) + + assert.NilError(t, err, "should succeed with auto-fetched global repo providing credentials") + // After merge, the repo should have secret from global repo + assert.Assert(t, repo.Spec.GitProvider.Secret != nil, "secret should be merged from global repo") + assert.Equal(t, "global-token-value", event.Provider.Token, "token should come from global repo secret") +} + // TestSetupAuthenticatedClient_WebhookValidation tests webhook secret validation. func TestSetupAuthenticatedClientWebhookValidation(t *testing.T) { t.Parallel() diff --git a/pkg/pipelineascode/pipelineascode_test.go b/pkg/pipelineascode/pipelineascode_test.go index 6397af7790..0e5c8f4417 100644 --- a/pkg/pipelineascode/pipelineascode_test.go +++ b/pkg/pipelineascode/pipelineascode_test.go @@ -610,7 +610,11 @@ func TestRun(t *testing.T) { }, }, Controller: &info.ControllerInfo{ - Secret: info.DefaultPipelinesAscodeSecretName, + Secret: info.DefaultPipelinesAscodeSecretName, + GlobalRepository: "global-repo", + }, + Kube: &info.KubeOpts{ + Namespace: repo.InstallNamespace, }, }, } diff --git a/pkg/provider/github/acl_test.go b/pkg/provider/github/acl_test.go index de70722138..d815415b80 100644 --- a/pkg/provider/github/acl_test.go +++ b/pkg/provider/github/acl_test.go @@ -539,7 +539,7 @@ func TestOkToTestCommentSHA(t *testing.T) { pacInfo: pacopts, } - payload := fmt.Sprintf(`{"action": "created", "repository": {"name": "repo", "owner": {"login": "owner"}}, "sender": {"login": %q}, "issue": {"pull_request": {"html_url": "https://github.com/owner/repo/pull/1"}}, "comment": {"body": %q}}`, + payload := fmt.Sprintf(`{"action": "created", "repository": {"name": "repo", "owner": {"login": "owner"}, "html_url": "http://url.com/owner/repo/1"}, "sender": {"login": %q}, "issue": {"number": 1, "pull_request": {"html_url": "https://github.com/owner/repo/pull/1"}}, "comment": {"body": %q}}`, tt.runevent.Sender, tt.commentBody) _, err := gprovider.ParsePayload(ctx, gprovider.Run, &http.Request{Header: http.Header{"X-Github-Event": []string{"issue_comment"}}}, payload) if (err != nil) != tt.wantErr { diff --git a/pkg/provider/github/github.go b/pkg/provider/github/github.go index 8580e65e11..a261ff4c7c 100644 --- a/pkg/provider/github/github.go +++ b/pkg/provider/github/github.go @@ -343,6 +343,19 @@ func (v *Provider) SetClient(ctx context.Context, run *params.Run, event *info.E } } + // Handle GitHub App token scoping for both global and repo-level configuration + if event.InstallationID > 0 { + v.Logger.Debugf("setupAuthenticatedClient: scoping github app token") + token, err := ScopeTokenToListOfRepos(ctx, v, v.pacInfo, repo, run, event, v.eventEmitter, v.Logger) + if err != nil { + return fmt.Errorf("failed to scope token: %w", err) + } + // If Global and Repo level configurations are not provided then lets not override the provider token. + if token != "" { + event.Provider.Token = token + } + } + return nil } diff --git a/pkg/provider/github/github_test.go b/pkg/provider/github/github_test.go index 241da4b49e..f8414f3bc2 100644 --- a/pkg/provider/github/github_test.go +++ b/pkg/provider/github/github_test.go @@ -1124,8 +1124,16 @@ func TestGithubSetClient(t *testing.T) { Log: testLog, }, } - v := Provider{} - err := v.SetClient(ctx, fakeRun, tt.event, nil, nil) + v := Provider{ + Logger: testLog, + pacInfo: &info.PacOpts{}, + } + repo := &v1alpha1.Repository{ + Spec: v1alpha1.RepositorySpec{ + Settings: &v1alpha1.Settings{}, + }, + } + err := v.SetClient(ctx, fakeRun, tt.event, repo, nil) assert.NilError(t, err) assert.Equal(t, tt.expectedURL, *v.APIURL) assert.Equal(t, "https", v.Client().BaseURL.Scheme) diff --git a/pkg/provider/github/parse_payload.go b/pkg/provider/github/parse_payload.go index 7300e86159..72b556c9d3 100644 --- a/pkg/provider/github/parse_payload.go +++ b/pkg/provider/github/parse_payload.go @@ -1,6 +1,7 @@ package github import ( + "bytes" "context" "encoding/json" "errors" @@ -16,6 +17,8 @@ import ( "github.com/google/go-github/v84/github" "github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/keys" "github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/v1alpha1" + "github.com/openshift-pipelines/pipelines-as-code/pkg/gitclient" + "github.com/openshift-pipelines/pipelines-as-code/pkg/kubeinteraction" "github.com/openshift-pipelines/pipelines-as-code/pkg/opscomments" "github.com/openshift-pipelines/pipelines-as-code/pkg/params" "github.com/openshift-pipelines/pipelines-as-code/pkg/params/info" @@ -98,6 +101,15 @@ func (v *Provider) GetAppToken(ctx context.Context, kube kubernetes.Interface, g return token, err } +func (v *Provider) initGitHubWebhookClient(ctx context.Context, event *info.Event, repo *v1alpha1.Repository) error { + kint, err := kubeinteraction.NewKubernetesInteraction(v.Run) + if err != nil { + return err + } + + return gitclient.SetupAuthenticatedClient(ctx, v, kint, v.Run, event, repo, nil, v.pacInfo, v.Logger) +} + func (v *Provider) parseEventType(request *http.Request, event *info.Event) error { event.EventType = request.Header.Get("X-GitHub-Event") if event.EventType == "" { @@ -156,6 +168,10 @@ func (v *Provider) ParsePayload(ctx context.Context, run *params.Run, request *h // Only apply for GitHub provider since we do fancy token creation at payload parsing v.Run = run event := info.NewEvent() + event.Request = &info.Request{ + Header: request.Header, + Payload: bytes.TrimSpace([]byte(payload)), + } systemNS := info.GetNS(ctx) if err := v.parseEventType(request, event); err != nil { return nil, err @@ -328,6 +344,8 @@ func (v *Provider) processEvent(ctx context.Context, event *info.Event, eventInt var err error processedEvent = info.NewEvent() + // need this for validating the webhook signature + processedEvent.Request = event.Request switch gitEvent := eventInt.(type) { case *github.CheckRunEvent: @@ -349,11 +367,7 @@ func (v *Provider) processEvent(ctx context.Context, event *info.Event, eventInt } return v.handleCheckSuites(ctx, gitEvent) case *github.IssueCommentEvent: - if v.ghClient == nil { - return nil, fmt.Errorf("no github client has been initialized, " + - "exiting... (hint: did you forget setting a secret on your repo?)") - } - processedEvent, err = v.handleIssueCommentEvent(ctx, gitEvent) + processedEvent, err = v.handleIssueCommentEvent(ctx, gitEvent, processedEvent) if err != nil { return nil, err } @@ -593,41 +607,42 @@ const ( errSHANotMatch = "the SHA provided in the `/ok-to-test` comment (`%s`) does not match the pull request's HEAD SHA (`%s`)" ) -func (v *Provider) handleIssueCommentEvent(ctx context.Context, event *github.IssueCommentEvent) (*info.Event, error) { +func (v *Provider) handleIssueCommentEvent(ctx context.Context, event *github.IssueCommentEvent, runevent *info.Event) (*info.Event, error) { action := "recheck" - runevent := info.NewEvent() runevent.Organization = event.GetRepo().GetOwner().GetLogin() runevent.Repository = event.GetRepo().GetName() runevent.Sender = event.GetSender().GetLogin() + runevent.URL = event.GetRepo().GetHTMLURL() // Always set the trigger target as pull_request on issue comment events runevent.TriggerTarget = triggertype.PullRequest if !event.GetIssue().IsPullRequest() { return info.NewEvent(), fmt.Errorf("issue comment is not coming from a pull_request") } v.userType = event.GetSender().GetType() + runevent.PullRequestNumber = event.GetIssue().GetNumber() - // We are getting the full URL so we have to get the last part to get the PR number, - // we don\'t have to care about URL query string/hash and other stuff because - // that comes up from the API. - var err error - runevent.PullRequestNumber, err = convertPullRequestURLtoNumber(event.GetIssue().GetPullRequestLinks().GetHTMLURL()) - if err != nil { - return info.NewEvent(), err + repo, repoErr := MatchEventURLRepo(ctx, v.Run, runevent, "") + if repoErr != nil { + return nil, repoErr + } + if repo == nil { + return nil, fmt.Errorf("no repository found matching URL: %s", runevent.URL) } - v.Logger.Infof("issue_comment: pipelinerun %s on %s/%s#%d has been requested", action, runevent.Organization, runevent.Repository, runevent.PullRequestNumber) - processedEvent, err := v.getPullRequest(ctx, runevent) - if err != nil { - return nil, err + // if v.ghClient is nil, then it means that it's Github webhook and client is not initialized yet + // because we initialize the client above only for github app events. + if v.ghClient == nil { + err := v.initGitHubWebhookClient(ctx, runevent, repo) + if err != nil { + return nil, fmt.Errorf("cannot initialize GitHub webhook client: %w", err) + } } - repo, err := MatchEventURLRepo(ctx, v.Run, processedEvent, "") + v.Logger.Infof("issue_comment: pipelinerun %s on %s/%s#%d has been requested", action, runevent.Organization, runevent.Repository, runevent.PullRequestNumber) + processedEvent, err := v.getPullRequest(ctx, runevent) if err != nil { return nil, err } - if repo == nil { - return nil, fmt.Errorf("no repository found matching URL: %s", processedEvent.URL) - } gitOpsCommentPrefix := provider.GetGitOpsCommentPrefix(repo) diff --git a/pkg/provider/github/parse_payload_test.go b/pkg/provider/github/parse_payload_test.go index 386f30758b..064e2b98bf 100644 --- a/pkg/provider/github/parse_payload_test.go +++ b/pkg/provider/github/parse_payload_test.go @@ -462,6 +462,7 @@ func TestParsePayLoad(t *testing.T) { objectType string gitopscommentprefix string wantRepoCRError bool + wantRequestSet bool }{ { name: "bad/unknown event", @@ -498,13 +499,6 @@ func TestParsePayLoad(t *testing.T) { triggerTarget: "pull_request", payloadEventStruct: github.CheckRunEvent{Action: github.Ptr("created")}, }, - { - name: "bad/issue comment retest only with github apps", - wantErrString: "no github client has been initialized", - eventType: "issue_comment", - triggerTarget: "pull_request", - payloadEventStruct: github.IssueCommentEvent{Action: github.Ptr("created"), Repo: sampleRepo}, - }, { name: "bad/issue comment not coming from pull request", eventType: "issue_comment", @@ -513,22 +507,6 @@ func TestParsePayLoad(t *testing.T) { payloadEventStruct: github.IssueCommentEvent{Action: github.Ptr("created"), Issue: &github.Issue{}, Repo: sampleRepo}, wantErrString: "issue comment is not coming from a pull_request", }, - { - name: "bad/issue comment invalid pullrequest", - eventType: "issue_comment", - triggerTarget: "pull_request", - githubClient: true, - payloadEventStruct: github.IssueCommentEvent{ - Action: github.Ptr("created"), - Issue: &github.Issue{ - PullRequestLinks: &github.PullRequestLinks{ - HTMLURL: github.Ptr("/bad"), - }, - }, - Repo: sampleRepo, - }, - wantErrString: "bad pull request number", - }, { name: "bad/rerequest error fetching PR", githubClient: true, @@ -755,11 +733,13 @@ func TestParsePayLoad(t *testing.T) { PullRequestLinks: &github.PullRequestLinks{ HTMLURL: github.Ptr("/666"), }, + Number: github.Ptr(666), }, Repo: sampleRepo, }, - muxReplies: map[string]any{"/repos/owner/reponame/pulls/666": samplePR}, - shaRet: "samplePRsha", + muxReplies: map[string]any{"/repos/owner/reponame/pulls/666": samplePR}, + shaRet: "samplePRsha", + wantRequestSet: true, }, { name: "good/pull request", @@ -799,6 +779,7 @@ func TestParsePayLoad(t *testing.T) { PullRequestLinks: &github.PullRequestLinks{ HTMLURL: github.Ptr("/777"), }, + Number: github.Ptr(777), }, Repo: sampleRepo, Comment: &github.IssueComment{ @@ -820,6 +801,7 @@ func TestParsePayLoad(t *testing.T) { PullRequestLinks: &github.PullRequestLinks{ HTMLURL: github.Ptr("/777"), }, + Number: github.Ptr(777), }, Repo: sampleRepo, Comment: &github.IssueComment{ @@ -842,6 +824,7 @@ func TestParsePayLoad(t *testing.T) { PullRequestLinks: &github.PullRequestLinks{ HTMLURL: github.Ptr("/777"), }, + Number: github.Ptr(777), }, Repo: sampleRepo, Comment: &github.IssueComment{ @@ -864,6 +847,7 @@ func TestParsePayLoad(t *testing.T) { PullRequestLinks: &github.PullRequestLinks{ HTMLURL: github.Ptr("/777"), }, + Number: github.Ptr(777), }, Repo: sampleRepo, Comment: &github.IssueComment{ @@ -886,6 +870,7 @@ func TestParsePayLoad(t *testing.T) { PullRequestLinks: &github.PullRequestLinks{ HTMLURL: github.Ptr("/999"), }, + Number: github.Ptr(999), }, Repo: sampleRepo, Comment: &github.IssueComment{ @@ -906,6 +891,7 @@ func TestParsePayLoad(t *testing.T) { PullRequestLinks: &github.PullRequestLinks{ HTMLURL: github.Ptr("/888"), }, + Number: github.Ptr(888), }, Repo: sampleRepo, Comment: &github.IssueComment{ @@ -1360,6 +1346,41 @@ func TestParsePayLoad(t *testing.T) { skipPushEventForPRCommits: true, muxReplies: map[string]any{"/repos/owner/pushRepo/commits/SHAPush/pulls": sampleGhPRs}, }, + { + name: "good/issue comment without ghClient initializes webhook client", + eventType: "issue_comment", + triggerTarget: "pull_request", + githubClient: false, + payloadEventStruct: github.IssueCommentEvent{ + Action: github.Ptr("created"), + Issue: &github.Issue{ + PullRequestLinks: &github.PullRequestLinks{ + HTMLURL: github.Ptr("/666"), + }, + Number: github.Ptr(666), + }, + Repo: sampleRepo, + }, + wantErrString: "cannot initialize GitHub webhook client", + }, + { + name: "bad/issue comment no matching repo", + eventType: "issue_comment", + triggerTarget: "pull_request", + githubClient: true, + payloadEventStruct: github.IssueCommentEvent{ + Action: github.Ptr("created"), + Issue: &github.Issue{ + PullRequestLinks: &github.PullRequestLinks{ + HTMLURL: github.Ptr("/666"), + }, + Number: github.Ptr(666), + }, + Repo: sampleRepo, + }, + wantRepoCRError: true, + wantErrString: "no repository found matching URL", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -1397,6 +1418,15 @@ func TestParsePayLoad(t *testing.T) { Log: logger, Kube: stdata.Kube, }, + Info: info.Info{ + Controller: &info.ControllerInfo{ + Secret: "pipelines-as-code-secret", + GlobalRepository: "global-repo", + }, + Kube: &info.KubeOpts{ + Namespace: "default", + }, + }, } for key, value := range tt.muxReplies { @@ -1521,6 +1551,11 @@ func TestParsePayLoad(t *testing.T) { if tt.targetCancelPipelinerun != "" { assert.Equal(t, tt.targetCancelPipelinerun, ret.TargetCancelPipelineRun) } + if tt.wantRequestSet { + assert.Assert(t, ret.Request != nil, "Request should be set on returned event") + assert.Equal(t, ret.Request.Header.Get("X-GitHub-Event"), tt.eventType) + assert.Assert(t, len(ret.Request.Payload) > 0, "Payload should be set on returned event") + } assert.Equal(t, tt.triggerTarget, string(ret.TriggerTarget)) }) } diff --git a/pkg/provider/gitlab/parse_payload.go b/pkg/provider/gitlab/parse_payload.go index 7e91f9a5c7..37a2e5ebc4 100644 --- a/pkg/provider/gitlab/parse_payload.go +++ b/pkg/provider/gitlab/parse_payload.go @@ -13,8 +13,8 @@ import ( "github.com/openshift-pipelines/pipelines-as-code/pkg/params" "github.com/openshift-pipelines/pipelines-as-code/pkg/params/info" "github.com/openshift-pipelines/pipelines-as-code/pkg/params/triggertype" - "github.com/openshift-pipelines/pipelines-as-code/pkg/secrets" "github.com/openshift-pipelines/pipelines-as-code/pkg/provider" + "github.com/openshift-pipelines/pipelines-as-code/pkg/secrets" gitlab "gitlab.com/gitlab-org/api/client-go" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" diff --git a/pkg/reconciler/reconciler_test.go b/pkg/reconciler/reconciler_test.go index 00da0ced48..e2f1d00cf2 100644 --- a/pkg/reconciler/reconciler_test.go +++ b/pkg/reconciler/reconciler_test.go @@ -72,7 +72,8 @@ func TestReconcilerReconcileKind(t *testing.T) { defer teardown() vcx := &ghprovider.Provider{ - Token: github.Ptr("None"), + Token: github.Ptr("None"), + Logger: fakelogger, } vcx.SetGithubClient(fakeclient)