Revert repo list on namespace and add indexers

- Remove namespace-only filter from webhook conflict detection
- Add field indexes on spec.git.repo and spec.git.branch for optimization
- Add comprehensive test coverage for indexing functions
- Condense and improve webhook query comments

Signed-off-by: Fiachra Corcoran <fiachra.corcoran@est.tech>

Author: efiacor

controllers/repositories/pkg/controllers/repository/config.go

@@ -15,12 +15,16 @@
 package repository
 
 import (
+	"context"
 	"flag"
+	"fmt"
 	"time"
 
+	configapi "github.com/kptdev/porch/api/porchconfig/v1alpha1"
 	"github.com/kptdev/porch/controllers/repositories/pkg/webhooks"
 	cachetypes "github.com/kptdev/porch/pkg/cache/types"
 	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 )
 
@@ -91,10 +95,34 @@ func (r *RepositoryReconciler) validateConfig() {
 }
 
 // Init wires runtime dependencies that require the manager.
-// It registers the Repository validating webhook on the shared webhook server.
+// It registers the Repository validating webhook on the shared webhook server
+// and sets up field indexes for efficient conflict detection queries.
 func (r *RepositoryReconciler) Init(mgr ctrl.Manager) error {
 	log := ctrl.Log.WithName(r.Name())
 
+	// Set up field indexes for Repository conflict detection
+	// These indexes allow efficient querying by git location without listing all repositories
+	ctx := context.Background()
+	if err := mgr.GetFieldIndexer().IndexField(ctx, &configapi.Repository{}, "spec.git.repo", func(o client.Object) []string {
+		repository := o.(*configapi.Repository)
+		if repository.Spec.Git == nil || repository.Spec.Git.Repo == "" {
+			return nil
+		}
+		return []string{repository.Spec.Git.Repo}
+	}); err != nil {
+		return fmt.Errorf("error indexing Repository by git.repo: %w", err)
+	}
+
+	if err := mgr.GetFieldIndexer().IndexField(ctx, &configapi.Repository{}, "spec.git.branch", func(o client.Object) []string {
+		repository := o.(*configapi.Repository)
+		if repository.Spec.Git == nil || repository.Spec.Git.Branch == "" {
+			return nil
+		}
+		return []string{repository.Spec.Git.Branch}
+	}); err != nil {
+		return fmt.Errorf("error indexing Repository by git.branch: %w", err)
+	}
+
 	// Register Repository validating webhook.
 	// The validator implements admission.Handler interface via its Handle method.
 	// Use GetAPIReader() for strong consistency during admission validation.

controllers/repositories/pkg/controllers/repository/config_test.go

@@ -21,8 +21,10 @@ import (
 	"testing"
 	"time"
 
+	configapi "github.com/kptdev/porch/api/porchconfig/v1alpha1"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 	"sigs.k8s.io/controller-runtime/pkg/healthz"
@@ -308,11 +310,20 @@ func TestValidateConfig(t *testing.T) {
 	}
 }
 
+// fakeFieldIndexer is a minimal field.Indexer for testing that stores index functions but doesn't actually index.
+type fakeFieldIndexer struct{}
+
+func (f *fakeFieldIndexer) IndexField(ctx context.Context, obj client.Object, field string, fn client.IndexerFunc) error {
+	// No-op for testing - just accept the index function registration
+	return nil
+}
+
 // fakeManager is a minimal manager.Manager for unit testing Init().
-// Only GetClient(), GetAPIReader(), and GetWebhookServer() are implemented; all other methods will panic if called.
+// Only GetClient(), GetAPIReader(), GetWebhookServer(), and GetFieldIndexer() are implemented; all other methods will panic if called.
 type fakeManager struct {
 	manager.Manager
-	client client.Client
+	client  client.Client
+	indexer *fakeFieldIndexer
 }
 
 func (f *fakeManager) GetClient() client.Client {
@@ -327,6 +338,10 @@ func (f *fakeManager) GetWebhookServer() webhook.Server {
 	return &fakeWebhookServer{}
 }
 
+func (f *fakeManager) GetFieldIndexer() client.FieldIndexer {
+	return f.indexer
+}
+
 // fakeWebhookServer is a minimal webhook.Server for testing.
 type fakeWebhookServer struct{}
 
@@ -365,7 +380,10 @@ func TestInit(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			reconciler := &RepositoryReconciler{}
-			mgr := &fakeManager{client: fake.NewClientBuilder().Build()}
+			mgr := &fakeManager{
+				client:  fake.NewClientBuilder().Build(),
+				indexer: &fakeFieldIndexer{},
+			}
 
 			err := reconciler.Init(mgr)
 
@@ -377,3 +395,153 @@ func TestInit(t *testing.T) {
 		})
 	}
 }
+
+// TestGitRepoIndexingFunction tests the git.repo index function used in Init
+func TestGitRepoIndexingFunction(t *testing.T) {
+	indexFunc := func(o client.Object) []string {
+		repository := o.(*configapi.Repository)
+		if repository.Spec.Git == nil || repository.Spec.Git.Repo == "" {
+			return nil
+		}
+		return []string{repository.Spec.Git.Repo}
+	}
+
+	tests := []struct {
+		name     string
+		repo     *configapi.Repository
+		expected []string
+	}{
+		{
+			name: "git repository with valid URL",
+			repo: &configapi.Repository{
+				ObjectMeta: metav1.ObjectMeta{Name: "repo1", Namespace: "default"},
+				Spec: configapi.RepositorySpec{
+					Git: &configapi.GitRepository{
+						Repo: "http://gitea.local/org/repo.git",
+					},
+				},
+			},
+			expected: []string{"http://gitea.local/org/repo.git"},
+		},
+		{
+			name: "OCI repository (nil Git)",
+			repo: &configapi.Repository{
+				ObjectMeta: metav1.ObjectMeta{Name: "repo-oci", Namespace: "default"},
+				Spec: configapi.RepositorySpec{
+					Type: configapi.RepositoryTypeOCI,
+				},
+			},
+			expected: nil,
+		},
+		{
+			name: "git repository with empty URL",
+			repo: &configapi.Repository{
+				ObjectMeta: metav1.ObjectMeta{Name: "repo2", Namespace: "default"},
+				Spec: configapi.RepositorySpec{
+					Git: &configapi.GitRepository{Repo: ""},
+				},
+			},
+			expected: nil,
+		},
+		{
+			name: "repository with git spec but empty URL",
+			repo: &configapi.Repository{
+				ObjectMeta: metav1.ObjectMeta{Name: "repo3", Namespace: "default"},
+				Spec: configapi.RepositorySpec{
+					Git: &configapi.GitRepository{
+						Repo: "https://github.com/example/pkg.git",
+					},
+				},
+			},
+			expected: []string{"https://github.com/example/pkg.git"},
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			result := indexFunc(tc.repo)
+			assert.Equal(t, tc.expected, result)
+		})
+	}
+}
+
+// TestGitBranchIndexingFunction tests the git.branch index function used in Init
+func TestGitBranchIndexingFunction(t *testing.T) {
+	indexFunc := func(o client.Object) []string {
+		repository := o.(*configapi.Repository)
+		if repository.Spec.Git == nil || repository.Spec.Git.Branch == "" {
+			return nil
+		}
+		return []string{repository.Spec.Git.Branch}
+	}
+
+	tests := []struct {
+		name     string
+		repo     *configapi.Repository
+		expected []string
+	}{
+		{
+			name: "repository with main branch",
+			repo: &configapi.Repository{
+				ObjectMeta: metav1.ObjectMeta{Name: "repo1", Namespace: "default"},
+				Spec: configapi.RepositorySpec{
+					Git: &configapi.GitRepository{
+						Branch: "main",
+					},
+				},
+			},
+			expected: []string{"main"},
+		},
+		{
+			name: "repository with develop branch",
+			repo: &configapi.Repository{
+				ObjectMeta: metav1.ObjectMeta{Name: "repo2", Namespace: "default"},
+				Spec: configapi.RepositorySpec{
+					Git: &configapi.GitRepository{
+						Branch: "develop",
+					},
+				},
+			},
+			expected: []string{"develop"},
+		},
+		{
+			name: "OCI repository (nil Git)",
+			repo: &configapi.Repository{
+				ObjectMeta: metav1.ObjectMeta{Name: "repo-oci", Namespace: "default"},
+				Spec:       configapi.RepositorySpec{},
+			},
+			expected: nil,
+		},
+		{
+			name: "repository with release branch",
+			repo: &configapi.Repository{
+				ObjectMeta: metav1.ObjectMeta{Name: "repo3", Namespace: "prod"},
+				Spec: configapi.RepositorySpec{
+					Git: &configapi.GitRepository{
+						Branch: "release-v1.0",
+					},
+				},
+			},
+			expected: []string{"release-v1.0"},
+		},
+		{
+			name: "repository with feature branch",
+			repo: &configapi.Repository{
+				ObjectMeta: metav1.ObjectMeta{Name: "repo4", Namespace: "staging"},
+				Spec: configapi.RepositorySpec{
+					Git: &configapi.GitRepository{
+						Branch: "feature/new-feature",
+					},
+				},
+			},
+			expected: []string{"feature/new-feature"},
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			result := indexFunc(tc.repo)
+			assert.Equal(t, tc.expected, result)
+		})
+	}
+}

controllers/repositories/pkg/webhooks/repository_webhook.go

@@ -80,8 +80,22 @@ func (v *RepositoryValidator) handleCreateOrUpdate(ctx context.Context, req admi
 	// NOTE: Immutability checks (URL, branch, directory) are handled by CEL validation in the CRD.
 	// This webhook only performs complex cross-resource conflict detection that CEL cannot do.
 
+	// Only Git repositories need conflict detection (OCI repos don't have the same multi-namespace issues)
+	if attempted.Spec.Git == nil {
+		logger.V(3).Info("repository is OCI type, skipping conflict check",
+			"namespace", attempted.Namespace, "name", attempted.Name)
+		return admission.Allowed("OCI repositories do not require conflict detection")
+	}
+
+	// Query repositories with matching git location (repo + branch).
+	// Field indexes optimize this query to O(1) lookups when available.
 	var repoList configapi.RepositoryList
-	opts := []client.ListOption{client.InNamespace(attempted.Namespace)}
+	opts := []client.ListOption{
+		client.MatchingFields{
+			"spec.git.repo":   attempted.Spec.Git.Repo,
+			"spec.git.branch": attempted.Spec.Git.Branch,
+		},
+	}
 	if err := v.client.List(ctx, &repoList, opts...); err != nil {
 		logger.Error(err, "failed to list repositories for conflict check")
 		return admission.Errored(http.StatusInternalServerError,
@@ -120,7 +134,14 @@ func NormalizeURL(url string) string {
 //  1. Same URL, branch, and directory in the same namespace
 //  2. Root directory conflicts with any subdirectory under the same URL and branch
 //  3. Nested directory conflicts (one path is a prefix of another)
+//
+// Returns false if either repository doesn't have a Git spec (no conflict possible).
 func IsConflict(existing, attempted *configapi.Repository) bool {
+	// Only Git repositories can conflict with each other
+	if existing.Spec.Git == nil || attempted.Spec.Git == nil {
+		return false
+	}
+
 	existingURL := NormalizeURL(existing.Spec.Git.Repo)
 	attemptedURL := NormalizeURL(attempted.Spec.Git.Repo)