Skip to content

Commit

Permalink
Fix empty secret namespace issue in reconciler
Browse files Browse the repository at this point in the history
fixed empty secret namespace issue in reconcile when
git proivder is not github e.g. gitea, gitlab etc.
added E2E tests as well to confirm behaviour.

https://issues.redhat.com/browse/SRVKP-5897

Signed-off-by: Zaki Shaikh <[email protected]>
  • Loading branch information
zakisk authored and chmouel committed Sep 10, 2024
1 parent 5173774 commit 77e61ea
Show file tree
Hide file tree
Showing 3 changed files with 82 additions and 1 deletion.
8 changes: 7 additions & 1 deletion pkg/reconciler/reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -234,14 +234,20 @@ func (r *Reconciler) updatePipelineRunToInProgress(ctx context.Context, logger *
if event.InstallationID > 0 {
event.Provider.WebhookSecret, _ = pac.GetCurrentNSWebhookSecret(ctx, r.kinteract, r.run)
} else {
// secretNS is needed when git provider is other than Github.
secretNS := repo.GetNamespace()
if repo.Spec.GitProvider != nil && repo.Spec.GitProvider.Secret == nil && r.globalRepo != nil && r.globalRepo.Spec.GitProvider != nil && r.globalRepo.Spec.GitProvider.Secret != nil {
secretNS = r.globalRepo.GetNamespace()
}

secretFromRepo := pac.SecretFromRepository{
K8int: r.kinteract,
Config: detectedProvider.GetConfig(),
Event: event,
Repo: repo,
WebhookType: pacInfo.WebhookType,
Logger: logger,
Namespace: r.secretNS,
Namespace: secretNS,
}
if err := secretFromRepo.Get(ctx); err != nil {
return fmt.Errorf("cannot get secret from repository: %w", err)
Expand Down
40 changes: 40 additions & 0 deletions test/gitea_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -735,6 +735,46 @@ func verifyProvinance(t *testing.T, topts *tgitea.TestOpts, expectedOutput, cNam
assert.NilError(t, err)
}

// TestGiteaGlobalRepoConcurrencyLimit tests the concurrency_limit feature of the PipelineRun.
// It fetches the PipelineRun definition from the default branch of the repository
// as configured on the git platform (e.g., main).
// In this test, the concurrency_limit is enabled using a global repository instead of a local repository.
func TestGiteaGlobalRepoConcurrencyLimit(t *testing.T) {
numPipelines := 10
yamlFiles := map[string]string{}
for i := 0; i < numPipelines; i++ {
yamlFiles[fmt.Sprintf(".tekton/pr%d.yaml", i)] = "testdata/pipelinerun.yaml"
}
topts := &tgitea.TestOpts{
TargetEvent: triggertype.PullRequest.String(),
YAMLFiles: yamlFiles,
CheckForNumberStatus: numPipelines,
CheckForStatus: "success",
}

tgitea.VerifyConcurrency(t, topts, github.Int(2))
}

// TestGiteaGlobalAndLocalRepoConcurrencyLimit verifies the concurrency_limit feature of the PipelineRun,
// ensuring that when concurrency_limit is defined in both global and local repository,
// the local repository limit takes precedence. This end-to-end test confirms that behavior.
func TestGiteaGlobalAndLocalRepoConcurrencyLimit(t *testing.T) {
numPipelines := 10
yamlFiles := map[string]string{}
for i := 0; i < numPipelines; i++ {
yamlFiles[fmt.Sprintf(".tekton/pr%d.yaml", i)] = "testdata/pipelinerun.yaml"
}
topts := &tgitea.TestOpts{
TargetEvent: triggertype.PullRequest.String(),
YAMLFiles: yamlFiles,
CheckForNumberStatus: numPipelines,
ConcurrencyLimit: github.Int(3),
CheckForStatus: "success",
}

tgitea.VerifyConcurrency(t, topts, github.Int(2))
}

func TestGiteaPushToTagGreedy(t *testing.T) {
topts := &tgitea.TestOpts{
SkipEventsCheck: true,
Expand Down
35 changes: 35 additions & 0 deletions test/pkg/gitea/test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/v1alpha1"
"github.com/openshift-pipelines/pipelines-as-code/pkg/formatting"
"github.com/openshift-pipelines/pipelines-as-code/pkg/params"
"github.com/openshift-pipelines/pipelines-as-code/pkg/params/info"
pgitea "github.com/openshift-pipelines/pipelines-as-code/pkg/provider/gitea"
"github.com/openshift-pipelines/pipelines-as-code/test/pkg/cctx"
tlogs "github.com/openshift-pipelines/pipelines-as-code/test/pkg/logs"
Expand Down Expand Up @@ -546,3 +547,37 @@ func GetStandardParams(t *testing.T, topts *TestOpts, eventType string) (repoURL

return repoURL, sourceURL, sourceBranch, targetBranch
}

func VerifyConcurrency(t *testing.T, topts *TestOpts, globalRepoConcurrencyLimit *int) {
t.Helper()
ctx := context.Background()
topts.ParamsRun, topts.Opts, topts.GiteaCNX, _ = Setup(ctx)
assert.NilError(t, topts.ParamsRun.Clients.NewClients(ctx, &topts.ParamsRun.Info))
topts.TargetRefName = names.SimpleNameGenerator.RestrictLengthWithRandomSuffix("pac-e2e-test")
topts.TargetNS = topts.TargetRefName
ctx, err := cctx.GetControllerCtxInfo(ctx, topts.ParamsRun)
assert.NilError(t, err)
assert.NilError(t, pacrepo.CreateNS(ctx, topts.TargetNS, topts.ParamsRun))
globalNs, _, err := params.GetInstallLocation(ctx, topts.ParamsRun)
assert.NilError(t, err)
ctx = info.StoreNS(ctx, globalNs)

err = CreateCRD(ctx, topts,
v1alpha1.RepositorySpec{
ConcurrencyLimit: globalRepoConcurrencyLimit,
},
true)
assert.NilError(t, err)

defer (func() {
if os.Getenv("TEST_NOCLEANUP") != "true" {
topts.ParamsRun.Clients.Log.Infof("Cleaning up global repo %s in %s", info.DefaultGlobalRepoName, globalNs)
err = topts.ParamsRun.Clients.PipelineAsCode.PipelinesascodeV1alpha1().Repositories(globalNs).Delete(
context.Background(), info.DefaultGlobalRepoName, metav1.DeleteOptions{})
assert.NilError(t, err)
}
})()

_, f := TestPR(t, topts)
defer f()
}

0 comments on commit 77e61ea

Please sign in to comment.