diff --git a/cmd/thv-operator/pkg/filtering/doc.go b/cmd/thv-operator/pkg/filtering/doc.go deleted file mode 100644 index dc31cd92e..000000000 --- a/cmd/thv-operator/pkg/filtering/doc.go +++ /dev/null @@ -1,65 +0,0 @@ -// Package filtering provides server filtering capabilities for MCPRegistry resources. -// -// This package implements a comprehensive filtering system that allows MCPRegistry -// controllers to selectively include or exclude servers based on name patterns -// and tags. The filtering system supports both include and exclude rules with -// exclude taking precedence over include. -// -// # Architecture -// -// The filtering system consists of three main components: -// -// - NameFilter: Handles server name filtering using glob patterns -// - TagFilter: Handles tag-based filtering using exact string matching -// - FilterService: Coordinates both name and tag filtering -// -// # Name Filtering -// -// Name filtering uses Go's filepath.Match for glob pattern matching, supporting -// wildcards like '*', '?', and character classes '[...]'. Examples: -// -// - "postgres-*" matches "postgres-server", "postgres-client" -// - "db?" matches "db1", "db2" but not "database" -// - "server[1-3]" matches "server1", "server2", "server3" -// -// # Tag Filtering -// -// Tag filtering uses exact string matching against server tags. A server is -// included if any of its tags match any include tag, and excluded if any of -// its tags match any exclude tag. -// -// # Filtering Logic -// -// Both name and tag filters follow the same precedence rules: -// -// 1. If exclude patterns/tags are specified and match -> exclude (precedence) -// 2. If include patterns/tags are specified and match -> include -// 3. If include patterns/tags are specified but no match -> exclude -// 4. If only exclude patterns/tags specified and no match -> include -// 5. If no filters specified -> include (default behavior) -// -// For a server to be included in the final registry, it must pass BOTH -// name and tag filtering (logical AND). -// -// # Usage Example -// -// service := NewDefaultFilterService() -// filter := &mcpv1alpha1.RegistryFilter{ -// NameFilters: &mcpv1alpha1.NameFilter{ -// Include: []string{"postgres-*", "mysql-*"}, -// Exclude: []string{"*-experimental"}, -// }, -// Tags: &mcpv1alpha1.TagFilter{ -// Include: []string{"database", "sql"}, -// Exclude: []string{"deprecated"}, -// }, -// } -// -// filteredRegistry, err := service.ApplyFilters(ctx, originalRegistry, filter) -// -// # Detailed Logging -// -// The filtering system provides detailed logging with specific reasons for -// inclusion or exclusion decisions, making it easy to debug filtering -// configurations and understand filtering behavior. -package filtering diff --git a/cmd/thv-operator/pkg/filtering/filter_service.go b/cmd/thv-operator/pkg/filtering/filter_service.go deleted file mode 100644 index 114c3e953..000000000 --- a/cmd/thv-operator/pkg/filtering/filter_service.go +++ /dev/null @@ -1,182 +0,0 @@ -package filtering - -import ( - "context" - "fmt" - "strings" - - "sigs.k8s.io/controller-runtime/pkg/log" - - mcpv1alpha1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1alpha1" - regtypes "github.com/stacklok/toolhive/pkg/registry/types" -) - -// FilterService coordinates name and tag filtering to apply registry filters -type FilterService interface { - // ApplyFilters filters the registry based on MCPRegistry filter configuration - ApplyFilters(ctx context.Context, reg *regtypes.Registry, filter *mcpv1alpha1.RegistryFilter) (*regtypes.Registry, error) -} - -// DefaultFilterService implements filtering coordination using name and tag filters -type DefaultFilterService struct { - nameFilter NameFilter - tagFilter TagFilter -} - -// NewDefaultFilterService creates a new DefaultFilterService with default filter implementations -func NewDefaultFilterService() *DefaultFilterService { - return &DefaultFilterService{ - nameFilter: NewDefaultNameFilter(), - tagFilter: NewDefaultTagFilter(), - } -} - -// NewFilterService creates a new DefaultFilterService with custom filter implementations -func NewFilterService(nameFilter NameFilter, tagFilter TagFilter) *DefaultFilterService { - return &DefaultFilterService{ - nameFilter: nameFilter, - tagFilter: tagFilter, - } -} - -// ApplyFilters filters the registry based on MCPRegistry filter configuration -// -// The filtering process: -// 1. If no filter is specified, return the original registry unchanged -// 2. Create a new registry with the same metadata but empty server maps -// 3. For each server (both container and remote), apply name and tag filtering -// 4. Only include servers that pass both name and tag filters -// 5. Return the filtered registry -func (s *DefaultFilterService) ApplyFilters( - ctx context.Context, - reg *regtypes.Registry, - filter *mcpv1alpha1.RegistryFilter) (*regtypes.Registry, error) { - ctxLogger := log.FromContext(ctx) - - // If no filter is specified, return original registry - if filter == nil { - ctxLogger.Info("No filter specified, returning original registry") - return reg, nil - } - - ctxLogger.Info("Applying registry filters", - "originalServerCount", len(reg.Servers), - "originalRemoteServerCount", len(reg.RemoteServers)) - - // Create a new filtered registry with same metadata - filteredRegistry := ®types.Registry{ - Version: reg.Version, - LastUpdated: reg.LastUpdated, - Servers: make(map[string]*regtypes.ImageMetadata), - RemoteServers: make(map[string]*regtypes.RemoteServerMetadata), - Groups: reg.Groups, // Groups are not filtered for now - } - - // Extract filter criteria - var nameInclude, nameExclude, tagInclude, tagExclude []string - if filter.NameFilters != nil { - nameInclude = filter.NameFilters.Include - nameExclude = filter.NameFilters.Exclude - } - if filter.Tags != nil { - tagInclude = filter.Tags.Include - tagExclude = filter.Tags.Exclude - } - - includedCount := 0 - excludedCount := 0 - - // Filter container servers - for serverName, serverMetadata := range reg.Servers { - included, reason := s.shouldIncludeServerWithReason( - serverName, - serverMetadata.Tags, - nameInclude, - nameExclude, - tagInclude, - tagExclude, - ) - if included { - filteredRegistry.Servers[serverName] = serverMetadata - includedCount++ - ctxLogger.Info("Including container server", - "name", serverName, - "tags", serverMetadata.Tags, - "reason", reason) - } else { - excludedCount++ - ctxLogger.Info("Excluding container server", - "name", serverName, - "tags", serverMetadata.Tags, - "reason", reason) - } - } - - // Filter remote servers - for serverName, serverMetadata := range reg.RemoteServers { - included, reason := s.shouldIncludeServerWithReason( - serverName, - serverMetadata.Tags, - nameInclude, - nameExclude, - tagInclude, - tagExclude, - ) - if included { - filteredRegistry.RemoteServers[serverName] = serverMetadata - includedCount++ - ctxLogger.Info("Including remote server", - "name", serverName, - "tags", serverMetadata.Tags, - "reason", reason) - } else { - excludedCount++ - ctxLogger.Info("Excluding remote server", - "name", serverName, - "tags", serverMetadata.Tags, - "reason", reason) - } - } - - ctxLogger.Info("Registry filtering completed", - "includedServers", includedCount, - "excludedServers", excludedCount, - "filteredServerCount", len(filteredRegistry.Servers), - "filteredRemoteServerCount", len(filteredRegistry.RemoteServers)) - - return filteredRegistry, nil -} - -// shouldIncludeServerWithReason determines if a server should be included and provides detailed reasoning -// Both name and tag filters must pass for a server to be included -func (s *DefaultFilterService) shouldIncludeServerWithReason( - serverName string, - serverTags []string, - nameInclude, nameExclude, tagInclude, tagExclude []string) (bool, string) { - // Apply name filtering first - nameIncluded, nameReason := s.nameFilter.ShouldInclude(serverName, nameInclude, nameExclude) - if !nameIncluded { - return false, fmt.Sprintf("name filter: %s", nameReason) - } - - // Apply tag filtering - tagIncluded, tagReason := s.tagFilter.ShouldInclude(serverTags, tagInclude, tagExclude) - if !tagIncluded { - return false, fmt.Sprintf("tag filter: %s", tagReason) - } - - // Both filters passed - determine the inclusion reason - inclusionReasons := []string{} - if len(nameInclude) > 0 || len(nameExclude) > 0 { - inclusionReasons = append(inclusionReasons, fmt.Sprintf("name filter: %s", nameReason)) - } - if len(tagInclude) > 0 || len(tagExclude) > 0 { - inclusionReasons = append(inclusionReasons, fmt.Sprintf("tag filter: %s", tagReason)) - } - - if len(inclusionReasons) == 0 { - return true, "no filters specified, default include" - } - - return true, "passed all filters: " + strings.Join(inclusionReasons, " AND ") -} diff --git a/cmd/thv-operator/pkg/filtering/filter_service_test.go b/cmd/thv-operator/pkg/filtering/filter_service_test.go deleted file mode 100644 index 146a647c9..000000000 --- a/cmd/thv-operator/pkg/filtering/filter_service_test.go +++ /dev/null @@ -1,431 +0,0 @@ -package filtering - -import ( - "context" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - - mcpv1alpha1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1alpha1" - regtypes "github.com/stacklok/toolhive/pkg/registry/types" -) - -func TestNewDefaultFilterService(t *testing.T) { - t.Parallel() - - service := NewDefaultFilterService() - assert.NotNil(t, service) - assert.NotNil(t, service.nameFilter) - assert.NotNil(t, service.tagFilter) -} - -func TestNewFilterService(t *testing.T) { - t.Parallel() - - nameFilter := NewDefaultNameFilter() - tagFilter := NewDefaultTagFilter() - - service := NewFilterService(nameFilter, tagFilter) - assert.NotNil(t, service) - assert.Equal(t, nameFilter, service.nameFilter) - assert.Equal(t, tagFilter, service.tagFilter) -} - -func TestDefaultFilterService_ApplyFilters_NoFilter(t *testing.T) { - t.Parallel() - - service := NewDefaultFilterService() - ctx := context.Background() - - // Create test registry - originalRegistry := ®types.Registry{ - Version: "1.0.0", - LastUpdated: "2023-01-01T00:00:00Z", - Servers: map[string]*regtypes.ImageMetadata{ - "postgres": { - BaseServerMetadata: regtypes.BaseServerMetadata{ - Name: "postgres", - Tags: []string{"database", "sql"}, - }, - Image: "postgres:latest", - }, - }, - RemoteServers: map[string]*regtypes.RemoteServerMetadata{ - "web-api": { - BaseServerMetadata: regtypes.BaseServerMetadata{ - Name: "web-api", - Tags: []string{"web", "api"}, - }, - URL: "https://example.com", - }, - }, - } - - // Apply no filter - result, err := service.ApplyFilters(ctx, originalRegistry, nil) - - require.NoError(t, err) - assert.Equal(t, originalRegistry, result, "No filter should return original registry") -} - -func TestDefaultFilterService_ApplyFilters_NameFiltering(t *testing.T) { - t.Parallel() - - service := NewDefaultFilterService() - ctx := context.Background() - - tests := []struct { - name string - nameInclude []string - nameExclude []string - expectedServerNames []string - expectedRemoteServerNames []string - }{ - { - name: "include postgres pattern", - nameInclude: []string{"postgres-*"}, - nameExclude: []string{}, - expectedServerNames: []string{"postgres-server"}, - expectedRemoteServerNames: []string{}, - }, - { - name: "exclude experimental pattern", - nameInclude: []string{}, - nameExclude: []string{"*-experimental"}, - expectedServerNames: []string{"postgres-server", "mysql-server"}, - expectedRemoteServerNames: []string{"web-api"}, - }, - { - name: "include with exclude precedence", - nameInclude: []string{"*-server"}, - nameExclude: []string{"*-experimental"}, - expectedServerNames: []string{"postgres-server", "mysql-server"}, - expectedRemoteServerNames: []string{}, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - - // Create test registry - originalRegistry := ®types.Registry{ - Version: "1.0.0", - LastUpdated: "2023-01-01T00:00:00Z", - Servers: map[string]*regtypes.ImageMetadata{ - "postgres-server": { - BaseServerMetadata: regtypes.BaseServerMetadata{ - Name: "postgres-server", - Tags: []string{"database"}, - }, - Image: "postgres:latest", - }, - "mysql-server": { - BaseServerMetadata: regtypes.BaseServerMetadata{ - Name: "mysql-server", - Tags: []string{"database"}, - }, - Image: "mysql:latest", - }, - "redis-experimental": { - BaseServerMetadata: regtypes.BaseServerMetadata{ - Name: "redis-experimental", - Tags: []string{"cache"}, - }, - Image: "redis:latest", - }, - }, - RemoteServers: map[string]*regtypes.RemoteServerMetadata{ - "web-api": { - BaseServerMetadata: regtypes.BaseServerMetadata{ - Name: "web-api", - Tags: []string{"web"}, - }, - URL: "https://example.com", - }, - "admin-experimental": { - BaseServerMetadata: regtypes.BaseServerMetadata{ - Name: "admin-experimental", - Tags: []string{"admin"}, - }, - URL: "https://admin.example.com", - }, - }, - } - - filter := &mcpv1alpha1.RegistryFilter{ - NameFilters: &mcpv1alpha1.NameFilter{ - Include: tt.nameInclude, - Exclude: tt.nameExclude, - }, - } - - result, err := service.ApplyFilters(ctx, originalRegistry, filter) - - require.NoError(t, err) - assert.Equal(t, originalRegistry.Version, result.Version) - assert.Equal(t, originalRegistry.LastUpdated, result.LastUpdated) - - // Check filtered servers - assert.Len(t, result.Servers, len(tt.expectedServerNames)) - for _, expectedName := range tt.expectedServerNames { - assert.Contains(t, result.Servers, expectedName) - } - - // Check filtered remote servers - assert.Len(t, result.RemoteServers, len(tt.expectedRemoteServerNames)) - for _, expectedName := range tt.expectedRemoteServerNames { - assert.Contains(t, result.RemoteServers, expectedName) - } - }) - } -} - -func TestDefaultFilterService_ApplyFilters_TagFiltering(t *testing.T) { - t.Parallel() - - service := NewDefaultFilterService() - ctx := context.Background() - - tests := []struct { - name string - tagInclude []string - tagExclude []string - expectedServerNames []string - expectedRemoteServerNames []string - }{ - { - name: "include database tags", - tagInclude: []string{"database"}, - tagExclude: []string{}, - expectedServerNames: []string{"postgres-server", "mysql-server"}, - expectedRemoteServerNames: []string{}, - }, - { - name: "exclude deprecated tags", - tagInclude: []string{}, - tagExclude: []string{"deprecated"}, - expectedServerNames: []string{"postgres-server", "redis-server"}, - expectedRemoteServerNames: []string{"web-api"}, - }, - { - name: "include with exclude precedence", - tagInclude: []string{"database"}, - tagExclude: []string{"deprecated"}, - expectedServerNames: []string{"postgres-server"}, - expectedRemoteServerNames: []string{}, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - - // Create test registry - originalRegistry := ®types.Registry{ - Version: "1.0.0", - LastUpdated: "2023-01-01T00:00:00Z", - Servers: map[string]*regtypes.ImageMetadata{ - "postgres-server": { - BaseServerMetadata: regtypes.BaseServerMetadata{ - Name: "postgres-server", - Tags: []string{"database", "sql"}, - }, - Image: "postgres:latest", - }, - "mysql-server": { - BaseServerMetadata: regtypes.BaseServerMetadata{ - Name: "mysql-server", - Tags: []string{"database", "deprecated"}, - }, - Image: "mysql:latest", - }, - "redis-server": { - BaseServerMetadata: regtypes.BaseServerMetadata{ - Name: "redis-server", - Tags: []string{"cache"}, - }, - Image: "redis:latest", - }, - }, - RemoteServers: map[string]*regtypes.RemoteServerMetadata{ - "web-api": { - BaseServerMetadata: regtypes.BaseServerMetadata{ - Name: "web-api", - Tags: []string{"web", "api"}, - }, - URL: "https://example.com", - }, - "legacy-api": { - BaseServerMetadata: regtypes.BaseServerMetadata{ - Name: "legacy-api", - Tags: []string{"web", "deprecated"}, - }, - URL: "https://legacy.example.com", - }, - }, - } - - filter := &mcpv1alpha1.RegistryFilter{ - Tags: &mcpv1alpha1.TagFilter{ - Include: tt.tagInclude, - Exclude: tt.tagExclude, - }, - } - - result, err := service.ApplyFilters(ctx, originalRegistry, filter) - - require.NoError(t, err) - - // Check filtered servers - assert.Len(t, result.Servers, len(tt.expectedServerNames)) - for _, expectedName := range tt.expectedServerNames { - assert.Contains(t, result.Servers, expectedName) - } - - // Check filtered remote servers - assert.Len(t, result.RemoteServers, len(tt.expectedRemoteServerNames)) - for _, expectedName := range tt.expectedRemoteServerNames { - assert.Contains(t, result.RemoteServers, expectedName) - } - }) - } -} - -func TestDefaultFilterService_ApplyFilters_CombinedFiltering(t *testing.T) { - t.Parallel() - - service := NewDefaultFilterService() - ctx := context.Background() - - // Create test registry - originalRegistry := ®types.Registry{ - Version: "1.0.0", - LastUpdated: "2023-01-01T00:00:00Z", - Servers: map[string]*regtypes.ImageMetadata{ - "postgres-server": { - BaseServerMetadata: regtypes.BaseServerMetadata{ - Name: "postgres-server", - Tags: []string{"database", "sql"}, - }, - Image: "postgres:latest", - }, - "postgres-experimental": { - BaseServerMetadata: regtypes.BaseServerMetadata{ - Name: "postgres-experimental", - Tags: []string{"database", "experimental"}, - }, - Image: "postgres:experimental", - }, - "web-server": { - BaseServerMetadata: regtypes.BaseServerMetadata{ - Name: "web-server", - Tags: []string{"web", "api"}, - }, - Image: "nginx:latest", - }, - }, - RemoteServers: map[string]*regtypes.RemoteServerMetadata{ - "database-api": { - BaseServerMetadata: regtypes.BaseServerMetadata{ - Name: "database-api", - Tags: []string{"database", "api"}, - }, - URL: "https://db.example.com", - }, - }, - } - - filter := &mcpv1alpha1.RegistryFilter{ - NameFilters: &mcpv1alpha1.NameFilter{ - Include: []string{"postgres-*", "*-api"}, - Exclude: []string{"*-experimental"}, - }, - Tags: &mcpv1alpha1.TagFilter{ - Include: []string{"database"}, - Exclude: []string{}, - }, - } - - result, err := service.ApplyFilters(ctx, originalRegistry, filter) - - require.NoError(t, err) - - // Should include: - // - postgres-server: matches postgres-* pattern AND has database tag - // - database-api: matches *-api pattern AND has database tag - // Should exclude: - // - postgres-experimental: excluded by *-experimental pattern (exclude takes precedence) - // - web-server: matches no name patterns - assert.Len(t, result.Servers, 1) - assert.Contains(t, result.Servers, "postgres-server") - - assert.Len(t, result.RemoteServers, 1) - assert.Contains(t, result.RemoteServers, "database-api") -} - -func TestDefaultFilterService_ApplyFilters_EmptyRegistry(t *testing.T) { - t.Parallel() - - service := NewDefaultFilterService() - ctx := context.Background() - - // Create empty registry - originalRegistry := ®types.Registry{ - Version: "1.0.0", - LastUpdated: "2023-01-01T00:00:00Z", - Servers: make(map[string]*regtypes.ImageMetadata), - RemoteServers: make(map[string]*regtypes.RemoteServerMetadata), - } - - filter := &mcpv1alpha1.RegistryFilter{ - NameFilters: &mcpv1alpha1.NameFilter{ - Include: []string{"postgres-*"}, - }, - } - - result, err := service.ApplyFilters(ctx, originalRegistry, filter) - - require.NoError(t, err) - assert.Len(t, result.Servers, 0) - assert.Len(t, result.RemoteServers, 0) - assert.Equal(t, originalRegistry.Version, result.Version) - assert.Equal(t, originalRegistry.LastUpdated, result.LastUpdated) -} - -func TestDefaultFilterService_ApplyFilters_PreservesMetadata(t *testing.T) { - t.Parallel() - - service := NewDefaultFilterService() - ctx := context.Background() - - // Create registry with groups - groups := []*regtypes.Group{ - { - Name: "test-group", - Description: "Test group", - }, - } - - originalRegistry := ®types.Registry{ - Version: "1.0.0", - LastUpdated: "2023-01-01T00:00:00Z", - Servers: make(map[string]*regtypes.ImageMetadata), - RemoteServers: make(map[string]*regtypes.RemoteServerMetadata), - Groups: groups, - } - - filter := &mcpv1alpha1.RegistryFilter{ - NameFilters: &mcpv1alpha1.NameFilter{ - Include: []string{"*"}, - }, - } - - result, err := service.ApplyFilters(ctx, originalRegistry, filter) - - require.NoError(t, err) - assert.Equal(t, originalRegistry.Version, result.Version) - assert.Equal(t, originalRegistry.LastUpdated, result.LastUpdated) - assert.Equal(t, originalRegistry.Groups, result.Groups) -} diff --git a/cmd/thv-operator/pkg/filtering/name_filter.go b/cmd/thv-operator/pkg/filtering/name_filter.go deleted file mode 100644 index 761533752..000000000 --- a/cmd/thv-operator/pkg/filtering/name_filter.go +++ /dev/null @@ -1,65 +0,0 @@ -package filtering - -import ( - "fmt" - "path/filepath" -) - -// NameFilter handles name-based filtering using glob patterns -type NameFilter interface { - // ShouldInclude determines if a server name should be included based on include/exclude patterns - // Returns (shouldInclude bool, reason string) - ShouldInclude(name string, include, exclude []string) (bool, string) -} - -// DefaultNameFilter implements name filtering using Go's filepath.Match for glob patterns -type DefaultNameFilter struct{} - -// NewDefaultNameFilter creates a new DefaultNameFilter -func NewDefaultNameFilter() *DefaultNameFilter { - return &DefaultNameFilter{} -} - -// ShouldInclude determines if a server name should be included based on include/exclude patterns -// -// Logic: -// 1. If exclude patterns are specified and name matches any exclude pattern -> exclude (exclude takes precedence) -// 2. If include patterns are specified and name matches any include pattern -> include -// 3. If include patterns are specified and name doesn't match any -> exclude -// 4. If only exclude patterns are specified (no include) and name doesn't match exclude -> include -// 5. If no patterns are specified -> include (default behavior) -func (*DefaultNameFilter) ShouldInclude(name string, include, exclude []string) (bool, string) { - // Check exclude patterns first (exclude takes precedence) - if len(exclude) > 0 { - for _, pattern := range exclude { - matches, err := filepath.Match(pattern, name) - if err != nil { - return false, fmt.Sprintf("invalid exclude pattern '%s': %v", pattern, err) - } - if matches { - return false, fmt.Sprintf("excluded by pattern '%s'", pattern) - } - } - } - - // If include patterns are specified, name must match at least one - if len(include) > 0 { - for _, pattern := range include { - matches, err := filepath.Match(pattern, name) - if err != nil { - return false, fmt.Sprintf("invalid include pattern '%s': %v", pattern, err) - } - if matches { - return true, fmt.Sprintf("included by pattern '%s'", pattern) - } - } - // Include patterns specified but no match found - return false, fmt.Sprintf("no match found in include patterns %v", include) - } - - // No include patterns specified (or empty), and didn't match exclude patterns - if len(exclude) > 0 { - return true, fmt.Sprintf("no match in exclude patterns %v", exclude) - } - return true, "no name filters specified" -} diff --git a/cmd/thv-operator/pkg/filtering/name_filter_test.go b/cmd/thv-operator/pkg/filtering/name_filter_test.go deleted file mode 100644 index de392ccf1..000000000 --- a/cmd/thv-operator/pkg/filtering/name_filter_test.go +++ /dev/null @@ -1,236 +0,0 @@ -package filtering - -import ( - "testing" - - "github.com/stretchr/testify/assert" -) - -func TestNewDefaultNameFilter(t *testing.T) { - t.Parallel() - - filter := NewDefaultNameFilter() - assert.NotNil(t, filter) - assert.IsType(t, &DefaultNameFilter{}, filter) -} - -func TestDefaultNameFilter_ShouldInclude(t *testing.T) { - t.Parallel() - - filter := NewDefaultNameFilter() - - tests := []struct { - name string - serverName string - include []string - exclude []string - expected bool - reason string - }{ - // No filters specified - default include - { - name: "no filters - should include", - serverName: "any-server", - include: []string{}, - exclude: []string{}, - expected: true, - reason: "no filters means default include", - }, - { - name: "nil filters - should include", - serverName: "any-server", - include: nil, - exclude: nil, - expected: true, - reason: "nil filters means default include", - }, - // Include-only patterns - { - name: "exact match include", - serverName: "postgres-server", - include: []string{"postgres-server"}, - exclude: []string{}, - expected: true, - reason: "exact match should be included", - }, - { - name: "glob pattern include match", - serverName: "postgres-server", - include: []string{"postgres-*"}, - exclude: []string{}, - expected: true, - reason: "glob pattern match should be included", - }, - { - name: "multiple include patterns - first match", - serverName: "postgres-server", - include: []string{"postgres-*", "mysql-*"}, - exclude: []string{}, - expected: true, - reason: "should match first pattern", - }, - { - name: "multiple include patterns - second match", - serverName: "mysql-server", - include: []string{"postgres-*", "mysql-*"}, - exclude: []string{}, - expected: true, - reason: "should match second pattern", - }, - { - name: "include pattern no match", - serverName: "redis-server", - include: []string{"postgres-*", "mysql-*"}, - exclude: []string{}, - expected: false, - reason: "no pattern match should exclude", - }, - // Exclude-only patterns - { - name: "exclude pattern match", - serverName: "server-experimental", - include: []string{}, - exclude: []string{"*-experimental", "*-deprecated"}, - expected: false, - reason: "exclude pattern match should exclude", - }, - { - name: "exclude pattern no match", - serverName: "stable-server", - include: []string{}, - exclude: []string{"*-experimental", "*-deprecated"}, - expected: true, - reason: "no exclude pattern match should include", - }, - // Both include and exclude patterns - exclude takes precedence - { - name: "exclude takes precedence over include", - serverName: "postgres-experimental", - include: []string{"postgres-*"}, - exclude: []string{"*-experimental"}, - expected: false, - reason: "exclude should take precedence over include", - }, - { - name: "include match with non-matching exclude", - serverName: "postgres-stable", - include: []string{"postgres-*"}, - exclude: []string{"*-experimental"}, - expected: true, - reason: "include match with no exclude match should include", - }, - { - name: "no include match with non-matching exclude", - serverName: "redis-stable", - include: []string{"postgres-*"}, - exclude: []string{"*-experimental"}, - expected: false, - reason: "no include match should exclude even if exclude doesn't match", - }, - // Complex glob patterns - { - name: "question mark wildcard", - serverName: "db1", - include: []string{"db?"}, - exclude: []string{}, - expected: true, - reason: "question mark should match single character", - }, - { - name: "question mark wildcard no match", - serverName: "database", - include: []string{"db?"}, - exclude: []string{}, - expected: false, - reason: "question mark should not match multiple characters", - }, - { - name: "character class match", - serverName: "db1", - include: []string{"db[0-9]"}, - exclude: []string{}, - expected: true, - reason: "character class should match digit", - }, - { - name: "character class no match", - serverName: "dba", - include: []string{"db[0-9]"}, - exclude: []string{}, - expected: false, - reason: "character class should not match letter", - }, - // Edge cases - { - name: "empty server name", - serverName: "", - include: []string{"*"}, - exclude: []string{}, - expected: true, - reason: "wildcard should match empty string", - }, - { - name: "empty pattern in include", - serverName: "server", - include: []string{""}, - exclude: []string{}, - expected: false, - reason: "empty pattern should only match empty string", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - - result, reason := filter.ShouldInclude(tt.serverName, tt.include, tt.exclude) - assert.Equal(t, tt.expected, result, tt.reason) - if !result { - assert.NotEmpty(t, reason, "Reason should be provided when excluding") - } - }) - } -} - -func TestDefaultNameFilter_ShouldInclude_InvalidGlobPatterns(t *testing.T) { - t.Parallel() - - filter := NewDefaultNameFilter() - - // Test with malformed glob patterns - should handle gracefully - tests := []struct { - name string - serverName string - include []string - exclude []string - expected bool - expectsReason string - }{ - { - name: "malformed bracket pattern in include", - serverName: "test", - include: []string{"["}, - exclude: []string{}, - expected: false, - expectsReason: "invalid include pattern", - }, - { - name: "malformed bracket pattern in exclude", - serverName: "test", - include: []string{}, - exclude: []string{"["}, - expected: false, - expectsReason: "invalid exclude pattern", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - - result, reason := filter.ShouldInclude(tt.serverName, tt.include, tt.exclude) - assert.Equal(t, tt.expected, result) - assert.Contains(t, reason, tt.expectsReason, "Reason should contain expected error message") - }) - } -} diff --git a/cmd/thv-operator/pkg/filtering/tag_filter.go b/cmd/thv-operator/pkg/filtering/tag_filter.go deleted file mode 100644 index 1b78a98ee..000000000 --- a/cmd/thv-operator/pkg/filtering/tag_filter.go +++ /dev/null @@ -1,58 +0,0 @@ -package filtering - -import "fmt" - -// TagFilter handles tag-based filtering using exact string matching -type TagFilter interface { - // ShouldInclude determines if a server with given tags should be included based on include/exclude tag lists - // Returns (shouldInclude bool, reason string) - ShouldInclude(tags []string, include, exclude []string) (bool, string) -} - -// DefaultTagFilter implements tag filtering using exact string matching -type DefaultTagFilter struct{} - -// NewDefaultTagFilter creates a new DefaultTagFilter -func NewDefaultTagFilter() *DefaultTagFilter { - return &DefaultTagFilter{} -} - -// ShouldInclude determines if a server with given tags should be included based on include/exclude tag lists -// -// Logic: -// 1. If exclude tags are specified and any server tag matches any exclude tag -> exclude (exclude takes precedence) -// 2. If include tags are specified and any server tag matches any include tag -> include -// 3. If include tags are specified and no server tags match any include tag -> exclude -// 4. If only exclude tags are specified (no include) and no server tags match exclude -> include -// 5. If no tag filters are specified -> include (default behavior) -func (*DefaultTagFilter) ShouldInclude(tags []string, include, exclude []string) (bool, string) { - // Check exclude tags first (exclude takes precedence) - if len(exclude) > 0 { - for _, serverTag := range tags { - for _, excludeTag := range exclude { - if serverTag == excludeTag { - return false, fmt.Sprintf("excluded by tag '%s'", excludeTag) - } - } - } - } - - // If include tags are specified, at least one server tag must match - if len(include) > 0 { - for _, serverTag := range tags { - for _, includeTag := range include { - if serverTag == includeTag { - return true, fmt.Sprintf("included by tag '%s'", includeTag) - } - } - } - // Include tags specified but no match found - return false, fmt.Sprintf("no matching tags found in include list %v (server tags: %v)", include, tags) - } - - // No include tags specified (or empty), and didn't match exclude tags - if len(exclude) > 0 { - return true, fmt.Sprintf("no matching tags in exclude list %v (server tags: %v)", exclude, tags) - } - return true, "no tag filters specified" -} diff --git a/cmd/thv-operator/pkg/filtering/tag_filter_test.go b/cmd/thv-operator/pkg/filtering/tag_filter_test.go deleted file mode 100644 index 4ed710ecc..000000000 --- a/cmd/thv-operator/pkg/filtering/tag_filter_test.go +++ /dev/null @@ -1,233 +0,0 @@ -package filtering - -import ( - "testing" - - "github.com/stretchr/testify/assert" -) - -func TestNewDefaultTagFilter(t *testing.T) { - t.Parallel() - - filter := NewDefaultTagFilter() - assert.NotNil(t, filter) - assert.IsType(t, &DefaultTagFilter{}, filter) -} - -func TestDefaultTagFilter_ShouldInclude(t *testing.T) { - t.Parallel() - - filter := NewDefaultTagFilter() - - tests := []struct { - name string - tags []string - include []string - exclude []string - expected bool - reason string - }{ - // No filters specified - default include - { - name: "no filters - should include", - tags: []string{"database", "sql"}, - include: []string{}, - exclude: []string{}, - expected: true, - reason: "no filters means default include", - }, - { - name: "nil filters - should include", - tags: []string{"database", "sql"}, - include: nil, - exclude: nil, - expected: true, - reason: "nil filters means default include", - }, - { - name: "empty tags with no filters", - tags: []string{}, - include: []string{}, - exclude: []string{}, - expected: true, - reason: "empty tags with no filters should include", - }, - { - name: "nil tags with no filters", - tags: nil, - include: []string{}, - exclude: []string{}, - expected: true, - reason: "nil tags with no filters should include", - }, - // Include-only filters - { - name: "single tag matches include", - tags: []string{"database"}, - include: []string{"database"}, - exclude: []string{}, - expected: true, - reason: "matching tag should be included", - }, - { - name: "multiple tags, one matches include", - tags: []string{"database", "web", "api"}, - include: []string{"database"}, - exclude: []string{}, - expected: true, - reason: "any matching tag should include", - }, - { - name: "multiple include tags, first matches", - tags: []string{"database"}, - include: []string{"database", "web"}, - exclude: []string{}, - expected: true, - reason: "first include match should include", - }, - { - name: "multiple include tags, second matches", - tags: []string{"web"}, - include: []string{"database", "web"}, - exclude: []string{}, - expected: true, - reason: "second include match should include", - }, - { - name: "no tag matches include", - tags: []string{"cache", "storage"}, - include: []string{"database", "web"}, - exclude: []string{}, - expected: false, - reason: "no matching include tag should exclude", - }, - { - name: "empty tags with include filters", - tags: []string{}, - include: []string{"database"}, - exclude: []string{}, - expected: false, - reason: "empty tags should not match include filters", - }, - // Exclude-only filters - { - name: "single tag matches exclude", - tags: []string{"deprecated"}, - include: []string{}, - exclude: []string{"deprecated"}, - expected: false, - reason: "matching exclude tag should exclude", - }, - { - name: "multiple tags, one matches exclude", - tags: []string{"database", "deprecated", "sql"}, - include: []string{}, - exclude: []string{"deprecated"}, - expected: false, - reason: "any matching exclude tag should exclude", - }, - { - name: "no tag matches exclude", - tags: []string{"database", "sql"}, - include: []string{}, - exclude: []string{"deprecated", "experimental"}, - expected: true, - reason: "no matching exclude tag should include", - }, - { - name: "empty tags with exclude filters", - tags: []string{}, - include: []string{}, - exclude: []string{"deprecated"}, - expected: true, - reason: "empty tags should not match exclude filters", - }, - // Both include and exclude filters - exclude takes precedence - { - name: "exclude takes precedence over include", - tags: []string{"database", "deprecated"}, - include: []string{"database"}, - exclude: []string{"deprecated"}, - expected: false, - reason: "exclude should take precedence over include", - }, - { - name: "include match with non-matching exclude", - tags: []string{"database", "stable"}, - include: []string{"database"}, - exclude: []string{"deprecated"}, - expected: true, - reason: "include match with no exclude match should include", - }, - { - name: "no include match with non-matching exclude", - tags: []string{"cache", "stable"}, - include: []string{"database"}, - exclude: []string{"deprecated"}, - expected: false, - reason: "no include match should exclude even if exclude doesn't match", - }, - { - name: "multiple excludes, first matches", - tags: []string{"database", "deprecated"}, - include: []string{"database"}, - exclude: []string{"deprecated", "experimental"}, - expected: false, - reason: "first exclude match should take precedence", - }, - { - name: "multiple excludes, second matches", - tags: []string{"database", "experimental"}, - include: []string{"database"}, - exclude: []string{"deprecated", "experimental"}, - expected: false, - reason: "second exclude match should take precedence", - }, - // Case sensitivity - { - name: "case sensitive exact match required", - tags: []string{"Database"}, - include: []string{"database"}, - exclude: []string{}, - expected: false, - reason: "case should matter for tag matching", - }, - { - name: "case sensitive exclude", - tags: []string{"Deprecated"}, - include: []string{}, - exclude: []string{"deprecated"}, - expected: true, - reason: "case should matter for exclude matching", - }, - // Complex scenarios - { - name: "many tags with complex filters", - tags: []string{"database", "sql", "postgresql", "backend", "stable"}, - include: []string{"database", "web"}, - exclude: []string{"deprecated", "experimental"}, - expected: true, - reason: "complex scenario with include match and no exclude match", - }, - { - name: "many tags with exclude match", - tags: []string{"database", "sql", "postgresql", "backend", "experimental"}, - include: []string{"database", "web"}, - exclude: []string{"deprecated", "experimental"}, - expected: false, - reason: "exclude should override include even with many tags", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - - result, reason := filter.ShouldInclude(tt.tags, tt.include, tt.exclude) - assert.Equal(t, tt.expected, result, tt.reason) - if !result { - assert.NotEmpty(t, reason, "Reason should be provided when excluding") - } - }) - } -} diff --git a/cmd/thv-operator/pkg/sync/manager.go b/cmd/thv-operator/pkg/sync/manager.go index 3683b53ce..768f1ece1 100644 --- a/cmd/thv-operator/pkg/sync/manager.go +++ b/cmd/thv-operator/pkg/sync/manager.go @@ -14,7 +14,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/log" mcpv1alpha1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1alpha1" - "github.com/stacklok/toolhive/cmd/thv-operator/pkg/filtering" "github.com/stacklok/toolhive/cmd/thv-operator/pkg/mcpregistrystatus" "github.com/stacklok/toolhive/cmd/thv-operator/pkg/sources" ) @@ -122,7 +121,6 @@ type DefaultSyncManager struct { scheme *runtime.Scheme sourceHandlerFactory sources.SourceHandlerFactory storageManager sources.StorageManager - filterService filtering.FilterService dataChangeDetector DataChangeDetector manualSyncChecker ManualSyncChecker automaticSyncChecker AutomaticSyncChecker @@ -136,7 +134,6 @@ func NewDefaultSyncManager(k8sClient client.Client, scheme *runtime.Scheme, scheme: scheme, sourceHandlerFactory: sourceHandlerFactory, storageManager: storageManager, - filterService: filtering.NewDefaultFilterService(), dataChangeDetector: &DefaultDataChangeDetector{sourceHandlerFactory: sourceHandlerFactory}, manualSyncChecker: &DefaultManualSyncChecker{}, automaticSyncChecker: &DefaultAutomaticSyncChecker{}, @@ -409,53 +406,9 @@ func (s *DefaultSyncManager) fetchAndProcessRegistryData( "format", fetchResult.Format, "hash", fetchResult.Hash) - // Apply filtering if configured - if err := s.applyFilteringIfConfigured(ctx, mcpRegistry, fetchResult); err != nil { - return nil, err - } - return fetchResult, nil } -// applyFilteringIfConfigured applies filtering to fetch result if registry has filter configuration -func (s *DefaultSyncManager) applyFilteringIfConfigured( - ctx context.Context, - mcpRegistry *mcpv1alpha1.MCPRegistry, - fetchResult *sources.FetchResult) *mcpregistrystatus.Error { - ctxLogger := log.FromContext(ctx) - - if mcpRegistry.Spec.Filter != nil { - ctxLogger.Info("Applying registry filters", - "hasNameFilters", mcpRegistry.Spec.Filter.NameFilters != nil, - "hasTagFilters", mcpRegistry.Spec.Filter.Tags != nil) - - filteredRegistry, err := s.filterService.ApplyFilters(ctx, fetchResult.Registry, mcpRegistry.Spec.Filter) - if err != nil { - ctxLogger.Error(err, "Registry filtering failed") - return &mcpregistrystatus.Error{ - Err: err, - Message: fmt.Sprintf("Filtering failed: %v", err), - ConditionType: mcpv1alpha1.ConditionSyncSuccessful, - ConditionReason: conditionReasonFetchFailed, - } - } - - // Update fetch result with filtered data - originalServerCount := fetchResult.ServerCount - fetchResult.Registry = filteredRegistry - fetchResult.ServerCount = len(filteredRegistry.Servers) + len(filteredRegistry.RemoteServers) - - ctxLogger.Info("Registry filtering completed", - "originalServerCount", originalServerCount, - "filteredServerCount", fetchResult.ServerCount, - "serversFiltered", originalServerCount-fetchResult.ServerCount) - } else { - ctxLogger.Info("No filtering configured, using original registry data") - } - - return nil -} - // storeRegistryData stores the registry data using the storage manager func (s *DefaultSyncManager) storeRegistryData( ctx context.Context, diff --git a/cmd/thv-operator/pkg/sync/manager_test.go b/cmd/thv-operator/pkg/sync/manager_test.go index e9358984a..c3cd594cd 100644 --- a/cmd/thv-operator/pkg/sync/manager_test.go +++ b/cmd/thv-operator/pkg/sync/manager_test.go @@ -339,135 +339,6 @@ func TestDefaultSyncManager_PerformSync(t *testing.T) { expectedServerCount: intPtr(0), // 0 servers in the registry data validateConditions: false, }, - { - name: "successful sync with name filtering", - mcpRegistry: &mcpv1alpha1.MCPRegistry{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-registry", - Namespace: "test-namespace", - UID: types.UID("test-uid"), - }, - Spec: mcpv1alpha1.MCPRegistrySpec{ - Source: mcpv1alpha1.MCPRegistrySource{ - Type: mcpv1alpha1.RegistrySourceTypeConfigMap, - Format: mcpv1alpha1.RegistryFormatToolHive, - ConfigMap: &mcpv1alpha1.ConfigMapSource{ - Name: "source-configmap", - Key: "registry.json", - }, - }, - Filter: &mcpv1alpha1.RegistryFilter{ - NameFilters: &mcpv1alpha1.NameFilter{ - Include: []string{"test-*"}, - Exclude: []string{"*-excluded"}, - }, - }, - }, - Status: mcpv1alpha1.MCPRegistryStatus{ - Phase: mcpv1alpha1.MCPRegistryPhasePending, - }, - }, - sourceConfigMap: &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: "source-configmap", - Namespace: "test-namespace", - }, - Data: map[string]string{ - "registry.json": `{"version": "1.0.0", "last_updated": "2023-01-01T00:00:00Z", "servers": {"test-server": {"name": "test-server", "description": "Test server", "tier": "Community", "status": "Active", "transport": "stdio", "tools": ["test_tool"], "image": "test/image:latest"}, "excluded-server": {"name": "excluded-server", "description": "Excluded server", "tier": "Community", "status": "Active", "transport": "stdio", "tools": ["test_tool"], "image": "test/image:latest"}, "other-server": {"name": "other-server", "description": "Other server", "tier": "Community", "status": "Active", "transport": "stdio", "tools": ["test_tool"], "image": "test/image:latest"}}, "remoteServers": {}}`, - }, - }, - expectedError: false, - expectedPhase: mcpv1alpha1.MCPRegistryPhasePending, - expectedServerCount: intPtr(1), // 1 server after filtering (test-server matches include, others excluded/don't match) - validateConditions: false, - }, - { - name: "successful sync with tag filtering", - mcpRegistry: &mcpv1alpha1.MCPRegistry{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-registry", - Namespace: "test-namespace", - UID: types.UID("test-uid"), - }, - Spec: mcpv1alpha1.MCPRegistrySpec{ - Source: mcpv1alpha1.MCPRegistrySource{ - Type: mcpv1alpha1.RegistrySourceTypeConfigMap, - Format: mcpv1alpha1.RegistryFormatToolHive, - ConfigMap: &mcpv1alpha1.ConfigMapSource{ - Name: "source-configmap", - Key: "registry.json", - }, - }, - Filter: &mcpv1alpha1.RegistryFilter{ - Tags: &mcpv1alpha1.TagFilter{ - Include: []string{"database"}, - Exclude: []string{"deprecated"}, - }, - }, - }, - Status: mcpv1alpha1.MCPRegistryStatus{ - Phase: mcpv1alpha1.MCPRegistryPhasePending, - }, - }, - sourceConfigMap: &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: "source-configmap", - Namespace: "test-namespace", - }, - Data: map[string]string{ - "registry.json": `{"version": "1.0.0", "last_updated": "2023-01-01T00:00:00Z", "servers": {"db-server": {"name": "db-server", "description": "Database server", "tier": "Community", "status": "Active", "transport": "stdio", "tools": ["db_tool"], "tags": ["database", "sql"], "image": "db/image:latest"}, "old-db-server": {"name": "old-db-server", "description": "Old database server", "tier": "Community", "status": "Active", "transport": "stdio", "tools": ["db_tool"], "tags": ["database", "deprecated"], "image": "db/image:old"}, "web-server": {"name": "web-server", "description": "Web server", "tier": "Community", "status": "Active", "transport": "stdio", "tools": ["web_tool"], "tags": ["web"], "image": "web/image:latest"}}, "remoteServers": {}}`, - }, - }, - expectedError: false, - expectedPhase: mcpv1alpha1.MCPRegistryPhasePending, - expectedServerCount: intPtr(1), // 1 server after filtering (db-server has "database" tag, old-db-server excluded by "deprecated", web-server doesn't have "database") - validateConditions: false, - }, - { - name: "successful sync with combined name and tag filtering", - mcpRegistry: &mcpv1alpha1.MCPRegistry{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-registry", - Namespace: "test-namespace", - UID: types.UID("test-uid"), - }, - Spec: mcpv1alpha1.MCPRegistrySpec{ - Source: mcpv1alpha1.MCPRegistrySource{ - Type: mcpv1alpha1.RegistrySourceTypeConfigMap, - Format: mcpv1alpha1.RegistryFormatToolHive, - ConfigMap: &mcpv1alpha1.ConfigMapSource{ - Name: "source-configmap", - Key: "registry.json", - }, - }, - Filter: &mcpv1alpha1.RegistryFilter{ - NameFilters: &mcpv1alpha1.NameFilter{ - Include: []string{"prod-*"}, - }, - Tags: &mcpv1alpha1.TagFilter{ - Include: []string{"production"}, - Exclude: []string{"experimental"}, - }, - }, - }, - Status: mcpv1alpha1.MCPRegistryStatus{ - Phase: mcpv1alpha1.MCPRegistryPhasePending, - }, - }, - sourceConfigMap: &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: "source-configmap", - Namespace: "test-namespace", - }, - Data: map[string]string{ - "registry.json": `{"version": "1.0.0", "last_updated": "2023-01-01T00:00:00Z", "servers": {"prod-db": {"name": "prod-db", "description": "Production database", "tier": "Official", "status": "Active", "transport": "stdio", "tools": ["db_tool"], "tags": ["database", "production"], "image": "db/image:prod"}, "prod-web": {"name": "prod-web", "description": "Production web server", "tier": "Official", "status": "Active", "transport": "stdio", "tools": ["web_tool"], "tags": ["web", "production"], "image": "web/image:prod"}, "prod-experimental": {"name": "prod-experimental", "description": "Experimental prod server", "tier": "Community", "status": "Active", "transport": "stdio", "tools": ["exp_tool"], "tags": ["production", "experimental"], "image": "exp/image:latest"}, "dev-db": {"name": "dev-db", "description": "Development database", "tier": "Community", "status": "Active", "transport": "stdio", "tools": ["db_tool"], "tags": ["database", "development"], "image": "db/image:dev"}}, "remoteServers": {}}`, - }, - }, - expectedError: false, - expectedPhase: mcpv1alpha1.MCPRegistryPhasePending, - expectedServerCount: intPtr(2), // 2 servers after filtering (prod-db and prod-web match name pattern and have "production" tag, prod-experimental excluded by "experimental", dev-db doesn't match "prod-*") - validateConditions: false, - }, } for _, tt := range tests { diff --git a/cmd/thv-operator/test-integration/mcp-registry/registry_filtering_test.go b/cmd/thv-operator/test-integration/mcp-registry/registry_filtering_test.go deleted file mode 100644 index 317f7a9ac..000000000 --- a/cmd/thv-operator/test-integration/mcp-registry/registry_filtering_test.go +++ /dev/null @@ -1,408 +0,0 @@ -package operator_test - -import ( - "context" - - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - corev1 "k8s.io/api/core/v1" - - mcpv1alpha1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1alpha1" -) - -var _ = Describe("MCPRegistry Filtering", Label("k8s", "registry"), func() { - var ( - ctx context.Context - registryHelper *MCPRegistryTestHelper - configMapHelper *ConfigMapTestHelper - statusHelper *StatusTestHelper - timingHelper *TimingTestHelper - k8sHelper *K8sResourceTestHelper - testNamespace string - ) - - BeforeEach(func() { - ctx = context.Background() - testNamespace = createTestNamespace(ctx) - - // Initialize helpers - registryHelper = NewMCPRegistryTestHelper(ctx, k8sClient, testNamespace) - configMapHelper = NewConfigMapTestHelper(ctx, k8sClient, testNamespace) - statusHelper = NewStatusTestHelper(ctx, k8sClient, testNamespace) - timingHelper = NewTimingTestHelper(ctx, k8sClient) - k8sHelper = NewK8sResourceTestHelper(ctx, k8sClient, testNamespace) - }) - - AfterEach(func() { - // Clean up test resources - Expect(registryHelper.CleanupRegistries()).To(Succeed()) - Expect(configMapHelper.CleanupConfigMaps()).To(Succeed()) - deleteTestNamespace(ctx, testNamespace) - }) - - Context("Name-based filtering", func() { - var configMap *corev1.ConfigMap - - BeforeEach(func() { - // Create ConfigMap with multiple servers for filtering tests - configMap = configMapHelper.NewConfigMapBuilder("filter-test-config"). - WithToolHiveRegistry("registry.json", []RegistryServer{ - { - Name: "production-server", - Description: "Production server", - Tier: "Official", - Status: "Active", - Transport: "stdio", - Tools: []string{"prod_tool"}, - Image: "test/prod:1.0.0", - Tags: []string{"production", "stable"}, - }, - { - Name: "test-server-alpha", - Description: "Test server alpha", - Tier: "Community", - Status: "Active", - Transport: "streamable-http", - Tools: []string{"test_tool_alpha"}, - Image: "test/alpha:1.0.0", - Tags: []string{"testing", "experimental"}, - }, - { - Name: "test-server-beta", - Description: "Test server beta", - Tier: "Community", - Status: "Active", - Transport: "stdio", - Tools: []string{"test_tool_beta"}, - Image: "test/beta:1.0.0", - Tags: []string{"testing", "beta"}, - }, - { - Name: "dev-server", - Description: "Development server", - Tier: "Community", - Status: "Active", - Transport: "sse", - Tools: []string{"dev_tool"}, - Image: "test/dev:1.0.0", - Tags: []string{"development", "unstable"}, - }, - }). - Create(configMapHelper) - }) - - It("should apply name include filters correctly", func() { - // Create registry with name include filter - registry := registryHelper.NewRegistryBuilder("name-include-test"). - WithConfigMapSource(configMap.Name, "registry.json"). - WithNameIncludeFilter([]string{"production-*", "dev-*"}). - Create(registryHelper) - - // Wait for registry initialization - registryHelper.WaitForRegistryInitialization(registry.Name, timingHelper, statusHelper) - - // Verify filtering applied - should include only production-server and dev-server - updatedRegistry, err := registryHelper.GetRegistry(registry.Name) - Expect(err).NotTo(HaveOccurred()) - - Expect(updatedRegistry.Status.SyncStatus).NotTo(BeNil()) - Expect(updatedRegistry.Status.SyncStatus.Phase).To(Equal(mcpv1alpha1.SyncPhaseComplete)) - Expect(updatedRegistry.Status.SyncStatus.ServerCount).To(Equal(2)) // Only production-server and dev-server - - // Verify storage contains filtered content - storageConfigMapName := updatedRegistry.Status.StorageRef.ConfigMapRef.Name - storageConfigMap, err := k8sHelper.GetConfigMap(storageConfigMapName) - Expect(err).NotTo(HaveOccurred()) - - filteredData := storageConfigMap.Data["registry.json"] - Expect(filteredData).To(ContainSubstring("production-server")) - Expect(filteredData).To(ContainSubstring("dev-server")) - Expect(filteredData).NotTo(ContainSubstring("test-server-alpha")) - Expect(filteredData).NotTo(ContainSubstring("test-server-beta")) - }) - - It("should apply name exclude filters correctly", func() { - // Create registry with name exclude filter - registry := registryHelper.NewRegistryBuilder("name-exclude-test"). - WithConfigMapSource(configMap.Name, "registry.json"). - WithNameExcludeFilter([]string{"test-*"}). - Create(registryHelper) - - // Wait for registry initialization - registryHelper.WaitForRegistryInitialization(registry.Name, timingHelper, statusHelper) - - // Verify filtering applied - should exclude test-server-alpha and test-server-beta - updatedRegistry, err := registryHelper.GetRegistry(registry.Name) - Expect(err).NotTo(HaveOccurred()) - - Expect(updatedRegistry.Status.SyncStatus).NotTo(BeNil()) - Expect(updatedRegistry.Status.SyncStatus.Phase).To(Equal(mcpv1alpha1.SyncPhaseComplete)) - Expect(updatedRegistry.Status.SyncStatus.ServerCount).To(Equal(2)) // Only production-server and dev-server - - // Verify storage contains filtered content - storageConfigMapName := updatedRegistry.Status.StorageRef.ConfigMapRef.Name - storageConfigMap, err := k8sHelper.GetConfigMap(storageConfigMapName) - Expect(err).NotTo(HaveOccurred()) - - filteredData := storageConfigMap.Data["registry.json"] - Expect(filteredData).To(ContainSubstring("production-server")) - Expect(filteredData).To(ContainSubstring("dev-server")) - Expect(filteredData).NotTo(ContainSubstring("test-server-alpha")) - Expect(filteredData).NotTo(ContainSubstring("test-server-beta")) - }) - - It("should apply both name include and exclude filters correctly", func() { - // Create registry with both include and exclude filters - registry := registryHelper.NewRegistryBuilder("name-include-exclude-test"). - WithConfigMapSource(configMap.Name, "registry.json"). - WithNameIncludeFilter([]string{"*-server*"}). // Include all servers - WithNameExcludeFilter([]string{"test-*", "dev-*"}). // Exclude test and dev servers - Create(registryHelper) - - // Wait for registry initialization - registryHelper.WaitForRegistryInitialization(registry.Name, timingHelper, statusHelper) - - // Verify filtering applied - should only include production-server - updatedRegistry, err := registryHelper.GetRegistry(registry.Name) - Expect(err).NotTo(HaveOccurred()) - - Expect(updatedRegistry.Status.SyncStatus).NotTo(BeNil()) - Expect(updatedRegistry.Status.SyncStatus.Phase).To(Equal(mcpv1alpha1.SyncPhaseComplete)) - Expect(updatedRegistry.Status.SyncStatus.ServerCount).To(Equal(1)) // Only production-server - - // Verify storage contains filtered content - storageConfigMapName := updatedRegistry.Status.StorageRef.ConfigMapRef.Name - storageConfigMap, err := k8sHelper.GetConfigMap(storageConfigMapName) - Expect(err).NotTo(HaveOccurred()) - - filteredData := storageConfigMap.Data["registry.json"] - Expect(filteredData).To(ContainSubstring("production-server")) - Expect(filteredData).NotTo(ContainSubstring("test-server-alpha")) - Expect(filteredData).NotTo(ContainSubstring("test-server-beta")) - Expect(filteredData).NotTo(ContainSubstring("dev-server")) - }) - }) - - Context("Tag-based filtering", func() { - var configMap *corev1.ConfigMap - - BeforeEach(func() { - // Create ConfigMap with servers having different tags - configMap = configMapHelper.NewConfigMapBuilder("tag-filter-config"). - WithToolHiveRegistry("registry.json", []RegistryServer{ - { - Name: "stable-server", - Description: "Stable production server", - Tier: "Official", - Status: "Active", - Transport: "stdio", - Tools: []string{"stable_tool"}, - Image: "test/stable:1.0.0", - Tags: []string{"production", "stable", "verified"}, - }, - { - Name: "beta-server", - Description: "Beta testing server", - Tier: "Community", - Status: "Active", - Transport: "streamable-http", - Tools: []string{"beta_tool"}, - Image: "test/beta:1.0.0", - Tags: []string{"testing", "beta"}, - }, - { - Name: "experimental-server", - Description: "Experimental server", - Tier: "Community", - Status: "Active", - Transport: "stdio", - Tools: []string{"experimental_tool"}, - Image: "test/experimental:1.0.0", - Tags: []string{"experimental", "unstable"}, - }, - }). - Create(configMapHelper) - }) - - It("should apply tag include filters correctly", func() { - // Create registry with tag include filter - registry := registryHelper.NewRegistryBuilder("tag-include-test"). - WithConfigMapSource(configMap.Name, "registry.json"). - WithTagIncludeFilter([]string{"production", "testing"}). - Create(registryHelper) - - // Wait for registry initialization - registryHelper.WaitForRegistryInitialization(registry.Name, timingHelper, statusHelper) - - // Verify filtering applied - should include stable-server and beta-server - updatedRegistry, err := registryHelper.GetRegistry(registry.Name) - Expect(err).NotTo(HaveOccurred()) - - Expect(updatedRegistry.Status.SyncStatus).NotTo(BeNil()) - Expect(updatedRegistry.Status.SyncStatus.Phase).To(Equal(mcpv1alpha1.SyncPhaseComplete)) - Expect(updatedRegistry.Status.SyncStatus.ServerCount).To(Equal(2)) // stable-server and beta-server - - // Verify storage contains filtered content - storageConfigMapName := updatedRegistry.Status.StorageRef.ConfigMapRef.Name - storageConfigMap, err := k8sHelper.GetConfigMap(storageConfigMapName) - Expect(err).NotTo(HaveOccurred()) - - filteredData := storageConfigMap.Data["registry.json"] - Expect(filteredData).To(ContainSubstring("stable-server")) - Expect(filteredData).To(ContainSubstring("beta-server")) - Expect(filteredData).NotTo(ContainSubstring("experimental-server")) - }) - - It("should apply tag exclude filters correctly", func() { - // Create registry with tag exclude filter - registry := registryHelper.NewRegistryBuilder("tag-exclude-test"). - WithConfigMapSource(configMap.Name, "registry.json"). - WithTagExcludeFilter([]string{"experimental", "unstable"}). - Create(registryHelper) - - // Wait for registry initialization - registryHelper.WaitForRegistryInitialization(registry.Name, timingHelper, statusHelper) - - // Verify filtering applied - should exclude experimental-server - updatedRegistry, err := registryHelper.GetRegistry(registry.Name) - Expect(err).NotTo(HaveOccurred()) - - Expect(updatedRegistry.Status.SyncStatus).NotTo(BeNil()) - Expect(updatedRegistry.Status.SyncStatus.Phase).To(Equal(mcpv1alpha1.SyncPhaseComplete)) - Expect(updatedRegistry.Status.SyncStatus.ServerCount).To(Equal(2)) // stable-server and beta-server - - // Verify storage contains filtered content - storageConfigMapName := updatedRegistry.Status.StorageRef.ConfigMapRef.Name - storageConfigMap, err := k8sHelper.GetConfigMap(storageConfigMapName) - Expect(err).NotTo(HaveOccurred()) - - filteredData := storageConfigMap.Data["registry.json"] - Expect(filteredData).To(ContainSubstring("stable-server")) - Expect(filteredData).To(ContainSubstring("beta-server")) - Expect(filteredData).NotTo(ContainSubstring("experimental-server")) - }) - }) - - Context("Filter updates", func() { - var configMap *corev1.ConfigMap - var registry *mcpv1alpha1.MCPRegistry - - BeforeEach(func() { - // Create ConfigMap with multiple servers - configMap = configMapHelper.NewConfigMapBuilder("update-filter-config"). - WithToolHiveRegistry("registry.json", []RegistryServer{ - { - Name: "server-alpha", - Description: "Server alpha", - Tier: "Community", - Status: "Active", - Transport: "stdio", - Tools: []string{"alpha_tool"}, - Image: "test/alpha:1.0.0", - Tags: []string{"alpha", "testing"}, - }, - { - Name: "server-beta", - Description: "Server beta", - Tier: "Community", - Status: "Active", - Transport: "streamable-http", - Tools: []string{"beta_tool"}, - Image: "test/beta:1.0.0", - Tags: []string{"beta", "testing"}, - }, - { - Name: "server-prod", - Description: "Production server", - Tier: "Official", - Status: "Active", - Transport: "stdio", - Tools: []string{"prod_tool"}, - Image: "test/prod:1.0.0", - Tags: []string{"production", "stable"}, - }, - }). - Create(configMapHelper) - - // Create registry without any sync policy (manual sync only) - registry = registryHelper.NewRegistryBuilder("filter-update-test"). - WithConfigMapSource(configMap.Name, "registry.json"). - WithNameIncludeFilter([]string{"server-alpha", "server-beta"}). // Initially include alpha and beta - Create(registryHelper) - - // Wait for initial sync - registryHelper.WaitForRegistryInitialization(registry.Name, timingHelper, statusHelper) - }) - - It("should update storage content when filters are changed", func() { - // Verify initial filtering - should have 2 servers - updatedRegistry, err := registryHelper.GetRegistry(registry.Name) - Expect(err).NotTo(HaveOccurred()) - Expect(updatedRegistry.Status.SyncStatus.ServerCount).To(Equal(2)) - - // Get initial storage content - initialStorageConfigMapName := updatedRegistry.Status.StorageRef.ConfigMapRef.Name - initialStorageConfigMap, err := k8sHelper.GetConfigMap(initialStorageConfigMapName) - Expect(err).NotTo(HaveOccurred()) - initialData := initialStorageConfigMap.Data["registry.json"] - Expect(initialData).To(ContainSubstring("server-alpha")) - Expect(initialData).To(ContainSubstring("server-beta")) - Expect(initialData).NotTo(ContainSubstring("server-prod")) - - By("updating the filter to include all servers") - // Update registry filter to include all servers - updatedRegistry.Spec.Filter.NameFilters.Include = []string{"*"} - Expect(registryHelper.UpdateRegistry(updatedRegistry)).To(Succeed()) - - // Wait for sync to complete with new filter - timingHelper.WaitForControllerReconciliation(func() interface{} { - currentRegistry, err := registryHelper.GetRegistry(registry.Name) - if err != nil { - return false - } - return currentRegistry.Status.SyncStatus.ServerCount == 3 // All 3 servers now included - }).Should(BeTrue(), "Registry should sync with updated filter") - - By("verifying storage content reflects the filter change") - // Verify updated storage content - finalRegistry, err := registryHelper.GetRegistry(registry.Name) - Expect(err).NotTo(HaveOccurred()) - finalStorageConfigMapName := finalRegistry.Status.StorageRef.ConfigMapRef.Name - finalStorageConfigMap, err := k8sHelper.GetConfigMap(finalStorageConfigMapName) - Expect(err).NotTo(HaveOccurred()) - finalData := finalStorageConfigMap.Data["registry.json"] - Expect(finalData).To(ContainSubstring("server-alpha")) - Expect(finalData).To(ContainSubstring("server-beta")) - Expect(finalData).To(ContainSubstring("server-prod")) // Now included - - By("updating the filter to exclude beta and alpha servers") - // Update filter again to exclude alpha and beta - finalRegistry.Spec.Filter.NameFilters = &mcpv1alpha1.NameFilter{ - Include: []string{"*"}, - Exclude: []string{"*-alpha", "*-beta"}, - } - Expect(registryHelper.UpdateRegistry(finalRegistry)).To(Succeed()) - - // Wait for sync to complete with new exclusion filter - timingHelper.WaitForControllerReconciliation(func() interface{} { - currentRegistry, err := registryHelper.GetRegistry(registry.Name) - if err != nil { - return false - } - return currentRegistry.Status.SyncStatus.ServerCount == 1 // Only server-prod - }).Should(BeTrue(), "Registry should sync with updated exclusion filter") - - By("verifying final storage content reflects the exclusion") - // Verify final storage content - endRegistry, err := registryHelper.GetRegistry(registry.Name) - Expect(err).NotTo(HaveOccurred()) - endStorageConfigMapName := endRegistry.Status.StorageRef.ConfigMapRef.Name - endStorageConfigMap, err := k8sHelper.GetConfigMap(endStorageConfigMapName) - Expect(err).NotTo(HaveOccurred()) - endData := endStorageConfigMap.Data["registry.json"] - Expect(endData).NotTo(ContainSubstring("server-alpha")) - Expect(endData).NotTo(ContainSubstring("server-beta")) - Expect(endData).To(ContainSubstring("server-prod")) // Only this remains - }) - }) -})