diff --git a/cmd/thv-registry-api/api/v1/routes_test.go b/cmd/thv-registry-api/api/v1/routes_test.go index 8febd8d55..e77fccc60 100644 --- a/cmd/thv-registry-api/api/v1/routes_test.go +++ b/cmd/thv-registry-api/api/v1/routes_test.go @@ -257,6 +257,11 @@ func (*realisticRegistryProvider) GetSource() string { return "test:realistic-registry-data" } +// GetRegistryName implements RegistryDataProvider.GetRegistryName +func (*realisticRegistryProvider) GetRegistryName() string { + return "test-registry" +} + func TestHealthRouter(t *testing.T) { t.Parallel() ctrl := gomock.NewController(t) @@ -607,6 +612,11 @@ func (*fileBasedRegistryProvider) GetSource() string { return "embedded:pkg/registry/data/registry.json" } +// GetRegistryName implements RegistryDataProvider.GetRegistryName +func (*fileBasedRegistryProvider) GetRegistryName() string { + return "embedded-registry" +} + // TestRoutesWithRealData tests all routes using the embedded registry.json data // This provides integration-style testing with realistic MCP server configurations func TestRoutesWithRealData(t *testing.T) { diff --git a/cmd/thv-registry-api/app/serve.go b/cmd/thv-registry-api/app/serve.go index f9862ad08..81e7c6a70 100644 --- a/cmd/thv-registry-api/app/serve.go +++ b/cmd/thv-registry-api/app/serve.go @@ -27,7 +27,12 @@ var serveCmd = &cobra.Command{ Use: "serve", Short: "Start the registry API server", Long: `Start the registry API server to serve MCP registry data. -The server reads registry data from ConfigMaps and provides REST endpoints for clients.`, +The server can read registry data from either: +- ConfigMaps using --from-configmap flag (requires Kubernetes API access) +- Local files using --from-file flag (for mounted ConfigMaps) + +Both options require --registry-name to specify the registry identifier. +One of --from-configmap or --from-file must be specified.`, RunE: runServe, } @@ -41,15 +46,25 @@ const ( func init() { serveCmd.Flags().String("address", ":8080", "Address to listen on") - serveCmd.Flags().String("configmap", "", "ConfigMap name containing registry data") + serveCmd.Flags().String("from-configmap", "", "ConfigMap name containing registry data (mutually exclusive with --from-file)") + serveCmd.Flags().String("from-file", "", "File path to registry.json (mutually exclusive with --from-configmap)") + serveCmd.Flags().String("registry-name", "", "Registry name identifier (required)") err := viper.BindPFlag("address", serveCmd.Flags().Lookup("address")) if err != nil { logger.Fatalf("Failed to bind address flag: %v", err) } - err = viper.BindPFlag("configmap", serveCmd.Flags().Lookup("configmap")) + err = viper.BindPFlag("from-configmap", serveCmd.Flags().Lookup("from-configmap")) + if err != nil { + logger.Fatalf("Failed to bind from-configmap flag: %v", err) + } + err = viper.BindPFlag("from-file", serveCmd.Flags().Lookup("from-file")) + if err != nil { + logger.Fatalf("Failed to bind from-file flag: %v", err) + } + err = viper.BindPFlag("registry-name", serveCmd.Flags().Lookup("registry-name")) if err != nil { - logger.Fatalf("Failed to bind configmap flag: %v", err) + logger.Fatalf("Failed to bind registry-name flag: %v", err) } } @@ -68,48 +83,95 @@ func getKubernetesConfig() (*rest.Config, error) { return kubeConfig.ClientConfig() } -func runServe(_ *cobra.Command, _ []string) error { - ctx := context.Background() +// buildProviderConfig creates provider configuration based on command-line flags +func buildProviderConfig() (*service.RegistryProviderConfig, error) { + configMapName := viper.GetString("from-configmap") + filePath := viper.GetString("from-file") + registryName := viper.GetString("registry-name") - // Get configuration - address := viper.GetString("address") - configMapName := viper.GetString("configmap") + // Validate mutual exclusivity + if configMapName != "" && filePath != "" { + return nil, fmt.Errorf("--from-configmap and --from-file flags are mutually exclusive") + } + + // Require one of the flags + if configMapName == "" && filePath == "" { + return nil, fmt.Errorf("either --from-configmap or --from-file flag is required") + } - if configMapName == "" { - return fmt.Errorf("configmap flag is required") + // Require registry name + if registryName == "" { + return nil, fmt.Errorf("--registry-name flag is required") } - namespace := thvk8scli.GetCurrentNamespace() + if configMapName != "" { + config, err := getKubernetesConfig() + if err != nil { + return nil, fmt.Errorf("failed to create kubernetes config: %w", err) + } + + clientset, err := kubernetes.NewForConfig(config) + if err != nil { + return nil, fmt.Errorf("failed to create kubernetes client: %w", err) + } + + return &service.RegistryProviderConfig{ + Type: service.RegistryProviderTypeConfigMap, + ConfigMap: &service.ConfigMapProviderConfig{ + Name: configMapName, + Namespace: thvk8scli.GetCurrentNamespace(), + Clientset: clientset, + RegistryName: registryName, + }, + }, nil + } + + return &service.RegistryProviderConfig{ + Type: service.RegistryProviderTypeFile, + File: &service.FileProviderConfig{ + FilePath: filePath, + RegistryName: registryName, + }, + }, nil +} + +func runServe(_ *cobra.Command, _ []string) error { + ctx := context.Background() + address := viper.GetString("address") logger.Infof("Starting registry API server on %s", address) - logger.Infof("ConfigMap: %s, Namespace: %s", configMapName, namespace) - // Create Kubernetes client and providers - var registryProvider service.RegistryDataProvider - var deploymentProvider service.DeploymentProvider + providerConfig, err := buildProviderConfig() + if err != nil { + return fmt.Errorf("failed to build provider configuration: %w", err) + } - // Get Kubernetes config - config, err := getKubernetesConfig() + if err := providerConfig.Validate(); err != nil { + return fmt.Errorf("invalid provider configuration: %w", err) + } + + factory := service.NewRegistryProviderFactory() + registryProvider, err := factory.CreateProvider(providerConfig) if err != nil { - return fmt.Errorf("failed to create kubernetes config: %w", err) + return fmt.Errorf("failed to create registry provider: %w", err) } - // Create Kubernetes client - clientset, err := kubernetes.NewForConfig(config) + logger.Infof("Created registry data provider: %s", registryProvider.GetSource()) + + var deploymentProvider service.DeploymentProvider + config, err := getKubernetesConfig() if err != nil { - return fmt.Errorf("failed to create kubernetes client: %w", err) + return fmt.Errorf("failed to create kubernetes config for deployment provider: %w", err) } - // Create the Kubernetes-based registry data provider - registryProvider = service.NewK8sRegistryDataProvider(clientset, configMapName, namespace) - logger.Infof("Created Kubernetes registry data provider for ConfigMap %s/%s", namespace, configMapName) + // Use registry name from provider + registryName := registryProvider.GetRegistryName() - // Create the Kubernetes-based deployment provider - deploymentProvider, err = service.NewK8sDeploymentProvider(config, configMapName) + deploymentProvider, err = service.NewK8sDeploymentProvider(config, registryName) if err != nil { return fmt.Errorf("failed to create kubernetes deployment provider: %w", err) } - logger.Infof("Created Kubernetes deployment provider for registry: %s", configMapName) + logger.Infof("Created Kubernetes deployment provider for registry: %s", registryName) // Create the registry service svc, err := service.NewService(ctx, registryProvider, deploymentProvider) diff --git a/cmd/thv-registry-api/internal/service/file_provider.go b/cmd/thv-registry-api/internal/service/file_provider.go new file mode 100644 index 000000000..a23c1d3dd --- /dev/null +++ b/cmd/thv-registry-api/internal/service/file_provider.go @@ -0,0 +1,78 @@ +// Package service provides the business logic for the MCP registry API +package service + +import ( + "context" + "encoding/json" + "fmt" + "os" + "path/filepath" + + "github.com/stacklok/toolhive/pkg/registry" +) + +// FileRegistryDataProvider implements RegistryDataProvider using local file system. +// This implementation reads registry data from a mounted file instead of calling the Kubernetes API. +// It is designed to work with ConfigMaps mounted as volumes in Kubernetes deployments. +type FileRegistryDataProvider struct { + filePath string + registryName string +} + +// NewFileRegistryDataProvider creates a new file-based registry data provider. +// The filePath parameter should point to the registry.json file, typically mounted from a ConfigMap. +// The registryName parameter specifies the registry identifier for business logic purposes. +func NewFileRegistryDataProvider(filePath, registryName string) *FileRegistryDataProvider { + return &FileRegistryDataProvider{ + filePath: filePath, + registryName: registryName, + } +} + +// GetRegistryData implements RegistryDataProvider.GetRegistryData. +// It reads the registry.json file from the local filesystem and parses it into a Registry struct. +func (p *FileRegistryDataProvider) GetRegistryData(_ context.Context) (*registry.Registry, error) { + if p.filePath == "" { + return nil, fmt.Errorf("file path not configured") + } + + // Check if the file exists and is readable + if _, err := os.Stat(p.filePath); err != nil { + if os.IsNotExist(err) { + return nil, fmt.Errorf("registry file not found at %s: %w", p.filePath, err) + } + return nil, fmt.Errorf("cannot access registry file at %s: %w", p.filePath, err) + } + + // Read the file contents + data, err := os.ReadFile(p.filePath) + if err != nil { + return nil, fmt.Errorf("failed to read registry file at %s: %w", p.filePath, err) + } + + // Parse the JSON data + var reg registry.Registry + if err := json.Unmarshal(data, ®); err != nil { + return nil, fmt.Errorf("failed to parse registry data from file %s: %w", p.filePath, err) + } + + return ®, nil +} + +// GetSource implements RegistryDataProvider.GetSource. +// It returns a descriptive string indicating the file source. +func (p *FileRegistryDataProvider) GetSource() string { + if p.filePath == "" { + return "file:" + } + + // Clean the path for consistent display + cleanPath := filepath.Clean(p.filePath) + return fmt.Sprintf("file:%s", cleanPath) +} + +// GetRegistryName implements RegistryDataProvider.GetRegistryName. +// It returns the injected registry name identifier. +func (p *FileRegistryDataProvider) GetRegistryName() string { + return p.registryName +} diff --git a/cmd/thv-registry-api/internal/service/file_provider_test.go b/cmd/thv-registry-api/internal/service/file_provider_test.go new file mode 100644 index 000000000..84f39a7f7 --- /dev/null +++ b/cmd/thv-registry-api/internal/service/file_provider_test.go @@ -0,0 +1,242 @@ +package service + +import ( + "context" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/stacklok/toolhive/pkg/registry" +) + +func TestFileRegistryDataProvider_GetRegistryData(t *testing.T) { + t.Parallel() + ctx := context.Background() + + tests := []struct { + name string + fileContent string + wantErr bool + errContains string + validate func(*testing.T, *registry.Registry) + }{ + { + name: "valid registry file", + fileContent: `{ + "version": "1.0", + "last_updated": "2024-01-01T00:00:00Z", + "servers": { + "test-server": { + "name": "test-server", + "description": "A test server", + "image": "test:latest" + } + }, + "remote_servers": {} + }`, + wantErr: false, + validate: func(t *testing.T, reg *registry.Registry) { + t.Helper() + assert.Equal(t, "1.0", reg.Version) + assert.Equal(t, "2024-01-01T00:00:00Z", reg.LastUpdated) + assert.Len(t, reg.Servers, 1) + assert.Contains(t, reg.Servers, "test-server") + assert.Equal(t, "test-server", reg.Servers["test-server"].Name) + assert.Equal(t, "A test server", reg.Servers["test-server"].Description) + assert.Equal(t, "test:latest", reg.Servers["test-server"].Image) + }, + }, + { + name: "empty registry file", + fileContent: `{ + "version": "1.0", + "last_updated": "", + "servers": {}, + "remote_servers": {} + }`, + wantErr: false, + validate: func(t *testing.T, reg *registry.Registry) { + t.Helper() + assert.Equal(t, "1.0", reg.Version) + assert.Len(t, reg.Servers, 0) + assert.Len(t, reg.RemoteServers, 0) + }, + }, + { + name: "invalid JSON", + fileContent: `{invalid json}`, + wantErr: true, + errContains: "failed to parse registry data", + }, + { + name: "empty file", + fileContent: "", + wantErr: true, + errContains: "failed to parse registry data", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + // Create temporary file + tmpDir := t.TempDir() + tmpFile := filepath.Join(tmpDir, "registry.json") + + err := os.WriteFile(tmpFile, []byte(tt.fileContent), 0644) + require.NoError(t, err) + + // Create provider + provider := NewFileRegistryDataProvider(tmpFile, "test-registry") + + // Test GetRegistryData + result, err := provider.GetRegistryData(ctx) + + if tt.wantErr { + assert.Error(t, err) + if tt.errContains != "" { + assert.Contains(t, err.Error(), tt.errContains) + } + assert.Nil(t, result) + } else { + assert.NoError(t, err) + assert.NotNil(t, result) + if tt.validate != nil { + tt.validate(t, result) + } + } + }) + } +} + +func TestFileRegistryDataProvider_GetRegistryData_FileErrors(t *testing.T) { + t.Parallel() + ctx := context.Background() + + tests := []struct { + name string + filePath string + wantErr bool + errContains string + }{ + { + name: "empty file path", + filePath: "", + wantErr: true, + errContains: "file path not configured", + }, + { + name: "non-existent file", + filePath: "/non/existent/file.json", + wantErr: true, + errContains: "registry file not found", + }, + { + name: "directory instead of file", + filePath: t.TempDir(), + wantErr: true, + errContains: "failed to read registry file", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + provider := NewFileRegistryDataProvider(tt.filePath, "test-registry") + result, err := provider.GetRegistryData(ctx) + + if tt.wantErr { + assert.Error(t, err) + if tt.errContains != "" { + assert.Contains(t, err.Error(), tt.errContains) + } + assert.Nil(t, result) + } else { + assert.NoError(t, err) + assert.NotNil(t, result) + } + }) + } +} + +func TestFileRegistryDataProvider_GetSource(t *testing.T) { + t.Parallel() + tests := []struct { + name string + filePath string + expected string + }{ + { + name: "absolute path", + filePath: "/data/registry/registry.json", + expected: "file:/data/registry/registry.json", + }, + { + name: "relative path", + filePath: "./registry.json", + expected: "file:registry.json", // filepath.Clean removes ./ + }, + { + name: "empty path", + filePath: "", + expected: "file:", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + provider := NewFileRegistryDataProvider(tt.filePath, "test-registry") + result := provider.GetSource() + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestNewFileRegistryDataProvider(t *testing.T) { + t.Parallel() + filePath := "/test/path/registry.json" + registryName := "test-registry" + provider := NewFileRegistryDataProvider(filePath, registryName) + + assert.NotNil(t, provider) + assert.Equal(t, "file:/test/path/registry.json", provider.GetSource()) + assert.Equal(t, registryName, provider.GetRegistryName()) +} + +func TestFileRegistryDataProvider_GetRegistryName(t *testing.T) { + t.Parallel() + tests := []struct { + name string + filePath string + registryName string + }{ + { + name: "normal registry name", + filePath: "/data/registry/registry.json", + registryName: "my-custom-registry", + }, + { + name: "production registry", + filePath: "/path/to/file.json", + registryName: "production-registry", + }, + { + name: "empty registry name", + filePath: "/some/path.json", + registryName: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + provider := NewFileRegistryDataProvider(tt.filePath, tt.registryName) + result := provider.GetRegistryName() + assert.Equal(t, tt.registryName, result) + }) + } +} diff --git a/cmd/thv-registry-api/internal/service/k8s_provider.go b/cmd/thv-registry-api/internal/service/k8s_provider.go index 8c243a7c8..8b2e6f84d 100644 --- a/cmd/thv-registry-api/internal/service/k8s_provider.go +++ b/cmd/thv-registry-api/internal/service/k8s_provider.go @@ -41,15 +41,21 @@ type K8sRegistryDataProvider struct { client kubernetes.Interface configMapName string namespace string + registryName string } // NewK8sRegistryDataProvider creates a new Kubernetes-based registry data provider. -// It requires a Kubernetes client and the ConfigMap details where registry data is stored. -func NewK8sRegistryDataProvider(kubeClient kubernetes.Interface, configMapName, namespace string) *K8sRegistryDataProvider { +// It requires a Kubernetes client, the ConfigMap details where registry data is stored, +// and the registry name identifier for business logic purposes. +func NewK8sRegistryDataProvider( + kubeClient kubernetes.Interface, + configMapName, namespace, registryName string, +) *K8sRegistryDataProvider { return &K8sRegistryDataProvider{ client: kubeClient, configMapName: configMapName, namespace: namespace, + registryName: registryName, } } @@ -86,6 +92,12 @@ func (p *K8sRegistryDataProvider) GetSource() string { return fmt.Sprintf("configmap:%s/%s", p.namespace, p.configMapName) } +// GetRegistryName implements RegistryDataProvider.GetRegistryName. +// It returns the injected registry name identifier. +func (p *K8sRegistryDataProvider) GetRegistryName() string { + return p.registryName +} + // K8sDeploymentProvider implements DeploymentProvider using Kubernetes API. // This implementation queries Kubernetes MCPServer custom resources to find deployed MCP servers. type K8sDeploymentProvider struct { diff --git a/cmd/thv-registry-api/internal/service/mocks/mock_provider.go b/cmd/thv-registry-api/internal/service/mocks/mock_provider.go index 84b580464..ee6eafc52 100644 --- a/cmd/thv-registry-api/internal/service/mocks/mock_provider.go +++ b/cmd/thv-registry-api/internal/service/mocks/mock_provider.go @@ -57,6 +57,20 @@ func (mr *MockRegistryDataProviderMockRecorder) GetRegistryData(ctx any) *gomock return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetRegistryData", reflect.TypeOf((*MockRegistryDataProvider)(nil).GetRegistryData), ctx) } +// GetRegistryName mocks base method. +func (m *MockRegistryDataProvider) GetRegistryName() string { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetRegistryName") + ret0, _ := ret[0].(string) + return ret0 +} + +// GetRegistryName indicates an expected call of GetRegistryName. +func (mr *MockRegistryDataProviderMockRecorder) GetRegistryName() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetRegistryName", reflect.TypeOf((*MockRegistryDataProvider)(nil).GetRegistryName)) +} + // GetSource mocks base method. func (m *MockRegistryDataProvider) GetSource() string { m.ctrl.T.Helper() diff --git a/cmd/thv-registry-api/internal/service/mocks/mock_provider_factory.go b/cmd/thv-registry-api/internal/service/mocks/mock_provider_factory.go new file mode 100644 index 000000000..a1179b641 --- /dev/null +++ b/cmd/thv-registry-api/internal/service/mocks/mock_provider_factory.go @@ -0,0 +1,56 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: provider_factory.go +// +// Generated by this command: +// +// mockgen -destination=mocks/mock_provider_factory.go -package=mocks -source=provider_factory.go RegistryProviderFactory +// + +// Package mocks is a generated GoMock package. +package mocks + +import ( + reflect "reflect" + + service "github.com/stacklok/toolhive/cmd/thv-registry-api/internal/service" + gomock "go.uber.org/mock/gomock" +) + +// MockRegistryProviderFactory is a mock of RegistryProviderFactory interface. +type MockRegistryProviderFactory struct { + ctrl *gomock.Controller + recorder *MockRegistryProviderFactoryMockRecorder + isgomock struct{} +} + +// MockRegistryProviderFactoryMockRecorder is the mock recorder for MockRegistryProviderFactory. +type MockRegistryProviderFactoryMockRecorder struct { + mock *MockRegistryProviderFactory +} + +// NewMockRegistryProviderFactory creates a new mock instance. +func NewMockRegistryProviderFactory(ctrl *gomock.Controller) *MockRegistryProviderFactory { + mock := &MockRegistryProviderFactory{ctrl: ctrl} + mock.recorder = &MockRegistryProviderFactoryMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockRegistryProviderFactory) EXPECT() *MockRegistryProviderFactoryMockRecorder { + return m.recorder +} + +// CreateProvider mocks base method. +func (m *MockRegistryProviderFactory) CreateProvider(config *service.RegistryProviderConfig) (service.RegistryDataProvider, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "CreateProvider", config) + ret0, _ := ret[0].(service.RegistryDataProvider) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// CreateProvider indicates an expected call of CreateProvider. +func (mr *MockRegistryProviderFactoryMockRecorder) CreateProvider(config any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateProvider", reflect.TypeOf((*MockRegistryProviderFactory)(nil).CreateProvider), config) +} diff --git a/cmd/thv-registry-api/internal/service/provider.go b/cmd/thv-registry-api/internal/service/provider.go index ba9b370c6..629869936 100644 --- a/cmd/thv-registry-api/internal/service/provider.go +++ b/cmd/thv-registry-api/internal/service/provider.go @@ -20,6 +20,10 @@ type RegistryDataProvider interface { // GetSource returns a descriptive string about where the registry data comes from. // Examples: "configmap:namespace/name", "file:/path/to/registry.json", "remote:https://example.com/registry" GetSource() string + + // GetRegistryName returns the registry name/identifier for this provider. + // This name is used for business logic such as finding related Kubernetes resources. + GetRegistryName() string } // DeploymentProvider abstracts access to deployed MCP servers. diff --git a/cmd/thv-registry-api/internal/service/provider_factory.go b/cmd/thv-registry-api/internal/service/provider_factory.go new file mode 100644 index 000000000..1e6dcee90 --- /dev/null +++ b/cmd/thv-registry-api/internal/service/provider_factory.go @@ -0,0 +1,159 @@ +// Package service provides the business logic for the MCP registry API +package service + +import ( + "fmt" + + "k8s.io/client-go/kubernetes" +) + +// RegistryProviderType represents the type of registry data provider +type RegistryProviderType string + +const ( + // RegistryProviderTypeConfigMap represents a ConfigMap-based registry provider + RegistryProviderTypeConfigMap RegistryProviderType = "configmap" + // RegistryProviderTypeFile represents a file-based registry provider + RegistryProviderTypeFile RegistryProviderType = "file" +) + +// RegistryProviderConfig holds configuration for creating a registry data provider +type RegistryProviderConfig struct { + Type RegistryProviderType + ConfigMap *ConfigMapProviderConfig + File *FileProviderConfig +} + +// ConfigMapProviderConfig holds configuration for ConfigMap-based registry provider +type ConfigMapProviderConfig struct { + Name string + Namespace string + Clientset kubernetes.Interface + RegistryName string +} + +// FileProviderConfig holds configuration for file-based registry provider +type FileProviderConfig struct { + FilePath string + RegistryName string +} + +// Validate validates the RegistryProviderConfig +func (c *RegistryProviderConfig) Validate() error { + if c.Type == "" { + return fmt.Errorf("provider type is required") + } + + switch c.Type { + case RegistryProviderTypeConfigMap: + if c.ConfigMap == nil { + return fmt.Errorf("configmap configuration required for configmap provider") + } + return c.ConfigMap.Validate() + case RegistryProviderTypeFile: + if c.File == nil { + return fmt.Errorf("file configuration required for file provider") + } + return c.File.Validate() + default: + return fmt.Errorf("unsupported provider type: %s", c.Type) + } +} + +// Validate validates the ConfigMapProviderConfig +func (c *ConfigMapProviderConfig) Validate() error { + if c.Clientset == nil { + return fmt.Errorf("kubernetes clientset is required") + } + if c.Name == "" { + return fmt.Errorf("configmap name is required") + } + if c.Namespace == "" { + return fmt.Errorf("configmap namespace is required") + } + if c.RegistryName == "" { + return fmt.Errorf("registry name is required") + } + return nil +} + +// Validate validates the FileProviderConfig +func (c *FileProviderConfig) Validate() error { + if c.FilePath == "" { + return fmt.Errorf("file path is required") + } + if c.RegistryName == "" { + return fmt.Errorf("registry name is required") + } + return nil +} + +//go:generate mockgen -destination=mocks/mock_provider_factory.go -package=mocks -source=provider_factory.go RegistryProviderFactory + +// RegistryProviderFactory creates registry data providers based on configuration +type RegistryProviderFactory interface { + // CreateProvider creates a registry data provider based on the provided configuration + CreateProvider(config *RegistryProviderConfig) (RegistryDataProvider, error) +} + +// DefaultRegistryProviderFactory is the default implementation of RegistryProviderFactory +type DefaultRegistryProviderFactory struct{} + +// NewRegistryProviderFactory creates a new default registry provider factory +func NewRegistryProviderFactory() RegistryProviderFactory { + return &DefaultRegistryProviderFactory{} +} + +// CreateProvider implements RegistryProviderFactory.CreateProvider +func (f *DefaultRegistryProviderFactory) CreateProvider(config *RegistryProviderConfig) (RegistryDataProvider, error) { + if config == nil { + return nil, fmt.Errorf("registry provider config cannot be nil") + } + + switch config.Type { + case RegistryProviderTypeConfigMap: + if config.ConfigMap == nil { + return nil, fmt.Errorf("configmap configuration required for configmap provider type") + } + return f.createConfigMapProvider(config.ConfigMap) + + case RegistryProviderTypeFile: + if config.File == nil { + return nil, fmt.Errorf("file configuration required for file provider type") + } + return f.createFileProvider(config.File) + + default: + return nil, fmt.Errorf("unsupported registry provider type: %s", config.Type) + } +} + +// createConfigMapProvider creates a ConfigMap-based registry data provider +func (*DefaultRegistryProviderFactory) createConfigMapProvider(config *ConfigMapProviderConfig) (RegistryDataProvider, error) { + if config.Clientset == nil { + return nil, fmt.Errorf("kubernetes clientset is required for configmap provider") + } + if config.Name == "" { + return nil, fmt.Errorf("configmap name is required") + } + if config.Namespace == "" { + return nil, fmt.Errorf("configmap namespace is required") + } + if config.RegistryName == "" { + return nil, fmt.Errorf("registry name is required for configmap provider") + } + + return NewK8sRegistryDataProvider(config.Clientset, config.Name, config.Namespace, config.RegistryName), nil +} + +// createFileProvider creates a file-based registry data provider +func (*DefaultRegistryProviderFactory) createFileProvider(config *FileProviderConfig) (RegistryDataProvider, error) { + if config.FilePath == "" { + return nil, fmt.Errorf("file path is required for file provider") + } + if config.RegistryName == "" { + return nil, fmt.Errorf("registry name is required for file provider") + } + + return NewFileRegistryDataProvider(config.FilePath, config.RegistryName), nil +} diff --git a/cmd/thv-registry-api/internal/service/provider_factory_test.go b/cmd/thv-registry-api/internal/service/provider_factory_test.go new file mode 100644 index 000000000..21148fc16 --- /dev/null +++ b/cmd/thv-registry-api/internal/service/provider_factory_test.go @@ -0,0 +1,336 @@ +package service + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "k8s.io/client-go/kubernetes/fake" +) + +func TestRegistryProviderConfig_Validate(t *testing.T) { + t.Parallel() + tests := []struct { + name string + config *RegistryProviderConfig + wantErr bool + errContains string + }{ + { + name: "valid configmap provider", + config: &RegistryProviderConfig{ + Type: RegistryProviderTypeConfigMap, + ConfigMap: &ConfigMapProviderConfig{ + Name: "test-cm", + Namespace: "test-ns", + Clientset: fake.NewSimpleClientset(), + RegistryName: "test-registry", + }, + }, + wantErr: false, + }, + { + name: "valid file provider", + config: &RegistryProviderConfig{ + Type: RegistryProviderTypeFile, + File: &FileProviderConfig{ + FilePath: "/data/registry.json", + RegistryName: "test-registry", + }, + }, + wantErr: false, + }, + { + name: "empty provider type", + config: &RegistryProviderConfig{ + Type: "", + }, + wantErr: true, + errContains: "provider type is required", + }, + { + name: "configmap provider without config", + config: &RegistryProviderConfig{ + Type: RegistryProviderTypeConfigMap, + ConfigMap: nil, + }, + wantErr: true, + errContains: "configmap configuration required", + }, + { + name: "file provider without config", + config: &RegistryProviderConfig{ + Type: RegistryProviderTypeFile, + File: nil, + }, + wantErr: true, + errContains: "file configuration required", + }, + { + name: "unsupported provider type", + config: &RegistryProviderConfig{ + Type: "unsupported", + }, + wantErr: true, + errContains: "unsupported provider type", + }, + { + name: "configmap with missing clientset", + config: &RegistryProviderConfig{ + Type: RegistryProviderTypeConfigMap, + ConfigMap: &ConfigMapProviderConfig{ + Name: "test-cm", + Namespace: "test-ns", + Clientset: nil, + }, + }, + wantErr: true, + errContains: "kubernetes clientset is required", + }, + { + name: "configmap with empty name", + config: &RegistryProviderConfig{ + Type: RegistryProviderTypeConfigMap, + ConfigMap: &ConfigMapProviderConfig{ + Name: "", + Namespace: "test-ns", + Clientset: fake.NewSimpleClientset(), + }, + }, + wantErr: true, + errContains: "configmap name is required", + }, + { + name: "configmap with empty namespace", + config: &RegistryProviderConfig{ + Type: RegistryProviderTypeConfigMap, + ConfigMap: &ConfigMapProviderConfig{ + Name: "test-cm", + Namespace: "", + Clientset: fake.NewSimpleClientset(), + }, + }, + wantErr: true, + errContains: "configmap namespace is required", + }, + { + name: "file with empty path", + config: &RegistryProviderConfig{ + Type: RegistryProviderTypeFile, + File: &FileProviderConfig{ + FilePath: "", + }, + }, + wantErr: true, + errContains: "file path is required", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + err := tt.config.Validate() + + if tt.wantErr { + assert.Error(t, err) + if tt.errContains != "" { + assert.Contains(t, err.Error(), tt.errContains) + } + } else { + assert.NoError(t, err) + } + }) + } +} + +func TestDefaultRegistryProviderFactory_CreateProvider(t *testing.T) { + t.Parallel() + tmpDir := t.TempDir() + tmpFile := filepath.Join(tmpDir, "test-registry.json") + + // Create a test file + err := os.WriteFile(tmpFile, []byte(`{ + "version": "1.0", + "last_updated": "", + "servers": {}, + "remote_servers": {} + }`), 0644) + require.NoError(t, err) + + tests := []struct { + name string + config *RegistryProviderConfig + wantErr bool + errContains string + checkType func(*testing.T, RegistryDataProvider) + }{ + { + name: "create configmap provider", + config: &RegistryProviderConfig{ + Type: RegistryProviderTypeConfigMap, + ConfigMap: &ConfigMapProviderConfig{ + Name: "test-cm", + Namespace: "test-ns", + Clientset: fake.NewSimpleClientset(), + RegistryName: "test-registry", + }, + }, + wantErr: false, + checkType: func(t *testing.T, provider RegistryDataProvider) { + t.Helper() + assert.IsType(t, &K8sRegistryDataProvider{}, provider) + assert.Equal(t, "configmap:test-ns/test-cm", provider.GetSource()) + assert.Equal(t, "test-registry", provider.GetRegistryName()) + }, + }, + { + name: "create file provider", + config: &RegistryProviderConfig{ + Type: RegistryProviderTypeFile, + File: &FileProviderConfig{ + FilePath: tmpFile, + RegistryName: "test-file-registry", + }, + }, + wantErr: false, + checkType: func(t *testing.T, provider RegistryDataProvider) { + t.Helper() + assert.IsType(t, &FileRegistryDataProvider{}, provider) + assert.Equal(t, "file:"+tmpFile, provider.GetSource()) + assert.Equal(t, "test-file-registry", provider.GetRegistryName()) + }, + }, + { + name: "nil config", + config: nil, + wantErr: true, + errContains: "registry provider config cannot be nil", + }, + { + name: "invalid config - missing configmap", + config: &RegistryProviderConfig{ + Type: RegistryProviderTypeConfigMap, + ConfigMap: nil, + }, + wantErr: true, + errContains: "configmap configuration required", + }, + { + name: "invalid config - missing file", + config: &RegistryProviderConfig{ + Type: RegistryProviderTypeFile, + File: nil, + }, + wantErr: true, + errContains: "file configuration required", + }, + { + name: "invalid config - unsupported type", + config: &RegistryProviderConfig{ + Type: "unknown", + }, + wantErr: true, + errContains: "unsupported registry provider type", + }, + { + name: "configmap with missing clientset", + config: &RegistryProviderConfig{ + Type: RegistryProviderTypeConfigMap, + ConfigMap: &ConfigMapProviderConfig{ + Name: "test-cm", + Namespace: "test-ns", + Clientset: nil, + }, + }, + wantErr: true, + errContains: "kubernetes clientset is required", + }, + { + name: "file with empty path", + config: &RegistryProviderConfig{ + Type: RegistryProviderTypeFile, + File: &FileProviderConfig{ + FilePath: "", + }, + }, + wantErr: true, + errContains: "file path is required", + }, + } + + factory := NewRegistryProviderFactory() + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + provider, err := factory.CreateProvider(tt.config) + + if tt.wantErr { + assert.Error(t, err) + if tt.errContains != "" { + assert.Contains(t, err.Error(), tt.errContains) + } + assert.Nil(t, provider) + } else { + assert.NoError(t, err) + assert.NotNil(t, provider) + if tt.checkType != nil { + tt.checkType(t, provider) + } + } + }) + } +} + +func TestNewRegistryProviderFactory(t *testing.T) { + t.Parallel() + factory := NewRegistryProviderFactory() + assert.NotNil(t, factory) + assert.IsType(t, &DefaultRegistryProviderFactory{}, factory) +} + +func TestProviderTypes(t *testing.T) { + t.Parallel() + assert.Equal(t, "configmap", string(RegistryProviderTypeConfigMap)) + assert.Equal(t, "file", string(RegistryProviderTypeFile)) +} + +func TestK8sRegistryDataProvider_GetRegistryName(t *testing.T) { + t.Parallel() + tests := []struct { + name string + configMapName string + namespace string + registryName string + }{ + { + name: "registry name independent of configmap", + configMapName: "my-configmap", + namespace: "default", + registryName: "production-registry", + }, + { + name: "different registry name", + configMapName: "some-configmap", + namespace: "test-namespace", + registryName: "custom-registry", + }, + { + name: "empty registry name", + configMapName: "test-cm", + namespace: "default", + registryName: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + provider := NewK8sRegistryDataProvider(fake.NewSimpleClientset(), tt.configMapName, tt.namespace, tt.registryName) + result := provider.GetRegistryName() + assert.Equal(t, tt.registryName, result) + }) + } +}