fix(webhook): prevent duplicate repository CR on trailing slash

The webhook admission controller's checkIfRepoExist used exact
string equality to compare repository URLs. This allowed users
to create a second Repository CR with the same URL by simply
appending a trailing slash (e.g. https://github.com/org/repo
vs https://github.com/org/repo/), bypassing the uniqueness
validation.

  - pkg/webhook/validation.go:
    - Normalize URLs with strings.TrimSuffix before comparison
      in checkIfRepoExist to treat trailing slash variants as
      identical

  - pkg/webhook/validation_test.go:
    - Add test case for rejecting repository URL with trailing
      slash when a matching repo already exists

  - test/repository_webhook_test.go:
    - Add e2e test TestOthersRepositoryCreationWithTrailingSlash
      to verify the webhook rejects duplicate repos created via
      trailing slash bypass

Signed-off-by: Zaki Shaikh <zashaikh@redhat.com>

Author: zakisk

pkg/webhook/validation.go

@@ -90,7 +90,7 @@ func checkIfRepoExist(pac pac.RepositoryLister, repo *v1alpha1.Repository, ns st
 	}
 	for i := len(repositories) - 1; i >= 0; i-- {
 		repoFromCluster := repositories[i]
-		if repoFromCluster.Spec.URL == repo.Spec.URL &&
+		if strings.TrimRight(strings.TrimSpace(repoFromCluster.Spec.URL), "/") == strings.TrimRight(strings.TrimSpace(repo.Spec.URL), "/") &&
 			(repoFromCluster.Name != repo.Name || repoFromCluster.Namespace != repo.Namespace) {
 			return true, nil
 		}

pkg/webhook/validation_test.go

@@ -171,6 +171,26 @@ func TestReconcilerAdmit(t *testing.T) {
 			}),
 			allowed: true,
 		},
+		{
+			name: "reject repository URL with trailing slash",
+			repo: testnewrepo.NewRepo(testnewrepo.RepoTestcreationOpts{
+				Name:             "test-run",
+				InstallNamespace: "namespace",
+				URL:              "https://pac.test/already/installed/",
+			}),
+			allowed: false,
+			result:  "repository already exists with URL: https://pac.test/already/installed/",
+		},
+		{
+			name: "reject repository URL with multiple trailing slashes",
+			repo: testnewrepo.NewRepo(testnewrepo.RepoTestcreationOpts{
+				Name:             "test-run",
+				InstallNamespace: "namespace",
+				URL:              "https://pac.test/already/installed//////",
+			}),
+			allowed: false,
+			result:  "repository already exists with URL: https://pac.test/already/installed//////",
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

test/repository_webhook_test.go

@@ -46,3 +46,37 @@ func TestOthersRepositoryCreation(t *testing.T) {
 	assert.Assert(t, err != nil)
 	assert.Equal(t, err.Error(), "admission webhook \"validation.pipelinesascode.tekton.dev\" denied the request: repository already exists with URL: https://pac.test/pac/app")
 }
+
+func TestOthersRepositoryCreationWithTrailingSlash(t *testing.T) {
+	ctx := context.TODO()
+	ctx, runcnx, _, _, err := ghtest.Setup(ctx, false, false)
+	assert.NilError(t, err)
+
+	targetNs := names.SimpleNameGenerator.RestrictLengthWithRandomSuffix("test-repo")
+	repo := &v1alpha1.Repository{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: targetNs,
+		},
+		Spec: v1alpha1.RepositorySpec{
+			URL: "https://pac.test/pac/app",
+		},
+	}
+
+	defer repository.NSTearDown(ctx, t, runcnx, targetNs)
+	err = repository.CreateNS(ctx, targetNs, runcnx)
+	assert.NilError(t, err)
+	err = repository.CreateRepo(ctx, targetNs, runcnx, repo)
+	assert.NilError(t, err)
+
+	// create a new cr with same git url
+	targetNsNew := names.SimpleNameGenerator.RestrictLengthWithRandomSuffix("test-repo-new")
+	repo.Name = "test-repo-new"
+	repo.Spec.URL = "https://pac.test/pac/app/"
+
+	defer repository.NSTearDown(ctx, t, runcnx, targetNsNew)
+	err = repository.CreateNS(ctx, targetNsNew, runcnx)
+	assert.NilError(t, err)
+	err = repository.CreateRepo(ctx, targetNsNew, runcnx, repo)
+	assert.Assert(t, err != nil)
+	assert.Equal(t, err.Error(), "admission webhook \"validation.pipelinesascode.tekton.dev\" denied the request: repository already exists with URL: https://pac.test/pac/app/")
+}