diff --git a/cmd/thv-operator/pkg/controllerutil/resources.go b/cmd/thv-operator/pkg/controllerutil/resources.go index aa913e3ad4..abedfad873 100644 --- a/cmd/thv-operator/pkg/controllerutil/resources.go +++ b/cmd/thv-operator/pkg/controllerutil/resources.go @@ -174,13 +174,6 @@ func CreateProxyServiceName(resourceName string) string { return fmt.Sprintf("mcp-%s-proxy", resourceName) } -// CreateProxyServiceURL generates the full cluster-local service URL -// Shared between MCPServer and MCPRemoteProxy -func CreateProxyServiceURL(resourceName, namespace string, port int32) string { - serviceName := CreateProxyServiceName(resourceName) - return fmt.Sprintf("http://%s.%s.svc.cluster.local:%d", serviceName, namespace, port) -} - // ProxyRunnerServiceAccountName generates the service account name for the proxy runner // Shared between MCPServer and MCPRemoteProxy func ProxyRunnerServiceAccountName(resourceName string) string { diff --git a/cmd/thv-operator/pkg/registryapi/podtemplatespec.go b/cmd/thv-operator/pkg/registryapi/podtemplatespec.go index ad45083cb8..e1da5e4060 100644 --- a/cmd/thv-operator/pkg/registryapi/podtemplatespec.go +++ b/cmd/thv-operator/pkg/registryapi/podtemplatespec.go @@ -31,11 +31,6 @@ type PodTemplateSpecBuilder struct { defaultSpec *corev1.PodTemplateSpec } -// NewPodTemplateSpecBuilder creates a new PodTemplateSpecBuilder with an empty template. -func NewPodTemplateSpecBuilder() *PodTemplateSpecBuilder { - return NewPodTemplateSpecBuilderFrom(nil) -} - // NewPodTemplateSpecBuilderFrom creates a new PodTemplateSpecBuilder with a user-provided template. // The user template is deep-copied to avoid mutating the original. // Options applied via Apply() act as defaults - Build() will merge them with user values, diff --git a/cmd/thv-operator/pkg/registryapi/podtemplatespec_test.go b/cmd/thv-operator/pkg/registryapi/podtemplatespec_test.go index 65c7e7fd6e..cf2d0f497e 100644 --- a/cmd/thv-operator/pkg/registryapi/podtemplatespec_test.go +++ b/cmd/thv-operator/pkg/registryapi/podtemplatespec_test.go @@ -717,7 +717,7 @@ func TestNewPodTemplateSpecBuilderFrom_MergeOnBuild(t *testing.T) { assert.Equal(t, "default-value", result.Labels["default-label"]) }) - t.Run("nil user template behaves like NewPodTemplateSpecBuilder", func(t *testing.T) { + t.Run("nil user template behaves like an empty builder", func(t *testing.T) { t.Parallel() builder := NewPodTemplateSpecBuilderFrom(nil) diff --git a/cmd/thv-operator/test-integration/mcp-registry/configmap_helpers.go b/cmd/thv-operator/test-integration/mcp-registry/configmap_helpers.go index 5902719b74..87dde71efd 100644 --- a/cmd/thv-operator/test-integration/mcp-registry/configmap_helpers.go +++ b/cmd/thv-operator/test-integration/mcp-registry/configmap_helpers.go @@ -12,7 +12,6 @@ import ( "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -73,21 +72,6 @@ func (h *ConfigMapTestHelper) NewConfigMapBuilder(name string) *ConfigMapBuilder } } -// WithLabel adds a label to the ConfigMap -func (cb *ConfigMapBuilder) WithLabel(key, value string) *ConfigMapBuilder { - if cb.configMap.Labels == nil { - cb.configMap.Labels = make(map[string]string) - } - cb.configMap.Labels[key] = value - return cb -} - -// WithData adds arbitrary data to the ConfigMap -func (cb *ConfigMapBuilder) WithData(key, value string) *ConfigMapBuilder { - cb.configMap.Data[key] = value - return cb -} - // WithToolHiveRegistry adds ToolHive format registry data func (cb *ConfigMapBuilder) WithToolHiveRegistry(key string, servers []RegistryServer) *ConfigMapBuilder { // Convert slice to map using server names as keys @@ -151,21 +135,6 @@ func (h *ConfigMapTestHelper) CreateSampleToolHiveRegistry(name string) *corev1. Create(h) } -// GetConfigMap retrieves a ConfigMap by name -func (h *ConfigMapTestHelper) GetConfigMap(name string) (*corev1.ConfigMap, error) { - cm := &corev1.ConfigMap{} - err := h.Client.Get(h.Context, types.NamespacedName{ - Namespace: h.Namespace, - Name: name, - }, cm) - return cm, err -} - -// UpdateConfigMap updates an existing ConfigMap -func (h *ConfigMapTestHelper) UpdateConfigMap(configMap *corev1.ConfigMap) error { - return h.Client.Update(h.Context, configMap) -} - // DeleteConfigMap deletes a ConfigMap by name func (h *ConfigMapTestHelper) DeleteConfigMap(name string) error { cm := &corev1.ConfigMap{ diff --git a/cmd/thv-operator/test-integration/mcp-registry/k8s_helpers.go b/cmd/thv-operator/test-integration/mcp-registry/k8s_helpers.go index 16e3c723b7..196e1a578d 100644 --- a/cmd/thv-operator/test-integration/mcp-registry/k8s_helpers.go +++ b/cmd/thv-operator/test-integration/mcp-registry/k8s_helpers.go @@ -48,16 +48,6 @@ func (h *K8sResourceTestHelper) GetService(name string) (*corev1.Service, error) return service, err } -// GetConfigMap retrieves a configmap by name -func (h *K8sResourceTestHelper) GetConfigMap(name string) (*corev1.ConfigMap, error) { - configMap := &corev1.ConfigMap{} - err := h.k8sClient.Get(h.ctx, types.NamespacedName{ - Namespace: h.namespace, - Name: name, - }, configMap) - return configMap, err -} - // DeploymentExists checks if a deployment exists func (h *K8sResourceTestHelper) DeploymentExists(name string) bool { _, err := h.GetDeployment(name) @@ -69,18 +59,3 @@ func (h *K8sResourceTestHelper) ServiceExists(name string) bool { _, err := h.GetService(name) return err == nil } - -// IsDeploymentReady checks if a deployment is ready (all replicas available) -func (h *K8sResourceTestHelper) IsDeploymentReady(name string) bool { - deployment, err := h.GetDeployment(name) - if err != nil { - return false - } - - // Check if deployment has at least one replica and all are available - if deployment.Spec.Replicas == nil || *deployment.Spec.Replicas == 0 { - return false - } - - return deployment.Status.ReadyReplicas == *deployment.Spec.Replicas -} diff --git a/cmd/thv-operator/test-integration/mcp-registry/registry_helpers.go b/cmd/thv-operator/test-integration/mcp-registry/registry_helpers.go index 279fe46829..aa0a1b7a98 100644 --- a/cmd/thv-operator/test-integration/mcp-registry/registry_helpers.go +++ b/cmd/thv-operator/test-integration/mcp-registry/registry_helpers.go @@ -8,7 +8,6 @@ import ( "encoding/json" "fmt" "strings" - "time" "github.com/onsi/ginkgo/v2" "github.com/onsi/gomega" @@ -112,16 +111,6 @@ func (rb *RegistryBuilder) WithAPISource(endpoint string) *RegistryBuilder { return rb } -// WithRegistryName sets the name for the source config -func (rb *RegistryBuilder) WithRegistryName(name string) *RegistryBuilder { - rb.config.SourceName = name - // Recalculate file path if this is a file source - if rb.config.SourceType == sourceTypeFile { - rb.config.FilePath = fmt.Sprintf("/config/registry/%s/registry.json", name) - } - return rb -} - // WithSyncPolicy configures the sync policy interval for the source func (rb *RegistryBuilder) WithSyncPolicy(interval string) *RegistryBuilder { rb.config.SyncInterval = interval @@ -146,30 +135,6 @@ func (rb *RegistryBuilder) WithLabel(key, value string) *RegistryBuilder { return rb } -// WithNameIncludeFilter sets name include patterns for filtering on the source -func (rb *RegistryBuilder) WithNameIncludeFilter(patterns []string) *RegistryBuilder { - rb.config.NameInclude = patterns - return rb -} - -// WithNameExcludeFilter sets name exclude patterns for filtering on the source -func (rb *RegistryBuilder) WithNameExcludeFilter(patterns []string) *RegistryBuilder { - rb.config.NameExclude = patterns - return rb -} - -// WithTagIncludeFilter sets tag include patterns for filtering on the source -func (rb *RegistryBuilder) WithTagIncludeFilter(tags []string) *RegistryBuilder { - rb.config.TagInclude = tags - return rb -} - -// WithTagExcludeFilter sets tag exclude patterns for filtering on the source -func (rb *RegistryBuilder) WithTagExcludeFilter(tags []string) *RegistryBuilder { - rb.config.TagExclude = tags - return rb -} - // Build returns the constructed MCPRegistry with configYAML generated from the builder config. func (rb *RegistryBuilder) Build() *mcpv1beta1.MCPRegistry { configYAML := rb.buildConfigYAML() @@ -315,21 +280,6 @@ func writeStringList(b *strings.Builder, label string, items []string) { } } -// CreateBasicConfigMapRegistry creates a simple MCPRegistry with ConfigMap source -func (h *MCPRegistryTestHelper) CreateBasicConfigMapRegistry(name, configMapName string) *mcpv1beta1.MCPRegistry { - return h.NewRegistryBuilder(name). - WithConfigMapSource(configMapName, "registry.json"). - WithSyncPolicy("1h"). - Create(h) -} - -// CreateManualSyncRegistry creates an MCPRegistry with manual sync only -func (h *MCPRegistryTestHelper) CreateManualSyncRegistry(name, configMapName string) *mcpv1beta1.MCPRegistry { - return h.NewRegistryBuilder(name). - WithConfigMapSource(configMapName, "registry.json"). - Create(h) -} - // GetRegistry retrieves an MCPRegistry by name func (h *MCPRegistryTestHelper) GetRegistry(name string) (*mcpv1beta1.MCPRegistry, error) { registry := &mcpv1beta1.MCPRegistry{} @@ -345,14 +295,6 @@ func (h *MCPRegistryTestHelper) UpdateRegistry(registry *mcpv1beta1.MCPRegistry) return h.Client.Update(h.Context, registry) } -// PatchRegistry patches an MCPRegistry with the given patch -func (h *MCPRegistryTestHelper) PatchRegistry(name string, patch client.Patch) error { - registry := &mcpv1beta1.MCPRegistry{} - registry.Name = name - registry.Namespace = h.Namespace - return h.Client.Patch(h.Context, registry, patch) -} - // DeleteRegistry deletes an MCPRegistry by name func (h *MCPRegistryTestHelper) DeleteRegistry(name string) error { registry := &mcpv1beta1.MCPRegistry{ @@ -364,54 +306,6 @@ func (h *MCPRegistryTestHelper) DeleteRegistry(name string) error { return h.Client.Delete(h.Context, registry) } -// TriggerManualSync adds the manual sync annotation to trigger a sync -func (h *MCPRegistryTestHelper) TriggerManualSync(name string) error { - registry, err := h.GetRegistry(name) - if err != nil { - return err - } - - if registry.Annotations == nil { - registry.Annotations = make(map[string]string) - } - registry.Annotations["toolhive.stacklok.dev/manual-sync"] = fmt.Sprintf("%d", time.Now().Unix()) - - return h.UpdateRegistry(registry) -} - -// GetRegistryStatus returns the current status of an MCPRegistry -func (h *MCPRegistryTestHelper) GetRegistryStatus(name string) (*mcpv1beta1.MCPRegistryStatus, error) { - registry, err := h.GetRegistry(name) - if err != nil { - return nil, err - } - return ®istry.Status, nil -} - -// GetRegistryPhase returns the current phase of an MCPRegistry -func (h *MCPRegistryTestHelper) GetRegistryPhase(name string) (mcpv1beta1.MCPRegistryPhase, error) { - status, err := h.GetRegistryStatus(name) - if err != nil { - return "", err - } - return status.Phase, nil -} - -// GetRegistryCondition returns a specific condition from the registry status -func (h *MCPRegistryTestHelper) GetRegistryCondition(name, conditionType string) (*metav1.Condition, error) { - status, err := h.GetRegistryStatus(name) - if err != nil { - return nil, err - } - - for _, condition := range status.Conditions { - if condition.Type == conditionType { - return &condition, nil - } - } - return nil, fmt.Errorf("condition %s not found", conditionType) -} - // ListRegistries returns all MCPRegistries in the namespace func (h *MCPRegistryTestHelper) ListRegistries() (*mcpv1beta1.MCPRegistryList, error) { registryList := &mcpv1beta1.MCPRegistryList{} diff --git a/cmd/thv-operator/test-integration/mcp-registry/status_helpers.go b/cmd/thv-operator/test-integration/mcp-registry/status_helpers.go index b8763088df..1cba5ee771 100644 --- a/cmd/thv-operator/test-integration/mcp-registry/status_helpers.go +++ b/cmd/thv-operator/test-integration/mcp-registry/status_helpers.go @@ -11,7 +11,6 @@ import ( "github.com/onsi/ginkgo/v2" "github.com/onsi/gomega" "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" mcpv1beta1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1beta1" @@ -51,44 +50,3 @@ func (h *StatusTestHelper) WaitForPhaseAny(registryName string, }, timeout, time.Second).Should(gomega.BeElementOf(expectedPhases), "MCPRegistry %s should reach one of phases %v", registryName, expectedPhases) } - -// WaitForCondition waits for a specific condition to have the expected status -func (h *StatusTestHelper) WaitForCondition(registryName, conditionType string, - expectedStatus metav1.ConditionStatus, timeout time.Duration) { - gomega.Eventually(func() metav1.ConditionStatus { - condition, err := h.registryHelper.GetRegistryCondition(registryName, conditionType) - if err != nil { - return metav1.ConditionUnknown - } - return condition.Status - }, timeout, time.Second).Should(gomega.Equal(expectedStatus), - "MCPRegistry %s should have condition %s with status %s", registryName, conditionType, expectedStatus) -} - -// WaitForConditionReason waits for a condition to have a specific reason -func (h *StatusTestHelper) WaitForConditionReason(registryName, conditionType, expectedReason string, timeout time.Duration) { - gomega.Eventually(func() string { - condition, err := h.registryHelper.GetRegistryCondition(registryName, conditionType) - if err != nil { - return "" - } - return condition.Reason - }, timeout, time.Second).Should(gomega.Equal(expectedReason), - "MCPRegistry %s condition %s should have reason %s", registryName, conditionType, expectedReason) -} - -// WaitForSyncCompletion waits for a sync operation to complete (either success or failure) -func (h *StatusTestHelper) WaitForSyncCompletion(registryName string, timeout time.Duration) { - gomega.Eventually(func() bool { - registry, err := h.registryHelper.GetRegistry(registryName) - if err != nil { - return false - } - - // Check if sync is no longer in progress - phase := registry.Status.Phase - return phase == mcpv1beta1.MCPRegistryPhaseReady || - phase == mcpv1beta1.MCPRegistryPhaseFailed - }, timeout, time.Second).Should(gomega.BeTrue(), - "MCPRegistry %s sync operation should complete", registryName) -} diff --git a/cmd/thv-operator/test-integration/mcp-registry/timing_helpers.go b/cmd/thv-operator/test-integration/mcp-registry/timing_helpers.go index d1ce59da1e..db6867b0ca 100644 --- a/cmd/thv-operator/test-integration/mcp-registry/timing_helpers.go +++ b/cmd/thv-operator/test-integration/mcp-registry/timing_helpers.go @@ -49,104 +49,7 @@ const ( SlowPollingInterval = 5 * time.Second ) -// EventuallyWithTimeout runs an Eventually check with custom timeout and polling -func (*TimingTestHelper) EventuallyWithTimeout(assertion func() interface{}, - timeout, polling time.Duration) gomega.AsyncAssertion { - return gomega.Eventually(assertion, timeout, polling) -} - -// ConsistentlyWithTimeout runs a Consistently check with custom timeout and polling -func (*TimingTestHelper) ConsistentlyWithTimeout(assertion func() interface{}, - duration, polling time.Duration) gomega.AsyncAssertion { - return gomega.Consistently(assertion, duration, polling) -} - -// WaitForResourceCreation waits for a resource to be created with quick timeout -func (*TimingTestHelper) WaitForResourceCreation(assertion func() interface{}) gomega.AsyncAssertion { - return gomega.Eventually(assertion, QuickTimeout, FastPollingInterval) -} - // WaitForControllerReconciliation waits for controller to reconcile changes func (*TimingTestHelper) WaitForControllerReconciliation(assertion func() interface{}) gomega.AsyncAssertion { return gomega.Eventually(assertion, MediumTimeout, DefaultPollingInterval) } - -// WaitForSyncOperation waits for a sync operation to complete -func (*TimingTestHelper) WaitForSyncOperation(assertion func() interface{}) gomega.AsyncAssertion { - return gomega.Eventually(assertion, LongTimeout, DefaultPollingInterval) -} - -// WaitForComplexOperation waits for complex multi-step operations -func (*TimingTestHelper) WaitForComplexOperation(assertion func() interface{}) gomega.AsyncAssertion { - return gomega.Eventually(assertion, ExtraLongTimeout, SlowPollingInterval) -} - -// EnsureStableState ensures a condition remains stable for a period -func (*TimingTestHelper) EnsureStableState(assertion func() interface{}, duration time.Duration) gomega.AsyncAssertion { - return gomega.Consistently(assertion, duration, DefaultPollingInterval) -} - -// EnsureQuickStability ensures a condition remains stable for a short period -func (h *TimingTestHelper) EnsureQuickStability(assertion func() interface{}) gomega.AsyncAssertion { - return h.EnsureStableState(assertion, 5*time.Second) -} - -// TimeoutConfig represents timeout configuration for different scenarios -type TimeoutConfig struct { - Timeout time.Duration - PollingInterval time.Duration - Description string -} - -// GetTimeoutForOperation returns appropriate timeout configuration for different operation types -func (*TimingTestHelper) GetTimeoutForOperation(operationType string) TimeoutConfig { - switch operationType { - case "create": - return TimeoutConfig{ - Timeout: QuickTimeout, - PollingInterval: FastPollingInterval, - Description: "Resource creation", - } - case "reconcile": - return TimeoutConfig{ - Timeout: MediumTimeout, - PollingInterval: DefaultPollingInterval, - Description: "Controller reconciliation", - } - case "sync": - return TimeoutConfig{ - Timeout: LongTimeout, - PollingInterval: DefaultPollingInterval, - Description: "Sync operation", - } - case "complex": - return TimeoutConfig{ - Timeout: ExtraLongTimeout, - PollingInterval: SlowPollingInterval, - Description: "Complex operation", - } - case "delete": - return TimeoutConfig{ - Timeout: MediumTimeout, - PollingInterval: DefaultPollingInterval, - Description: "Resource deletion", - } - case "status-update": - return TimeoutConfig{ - Timeout: MediumTimeout, - PollingInterval: FastPollingInterval, - Description: "Status update", - } - default: - return TimeoutConfig{ - Timeout: MediumTimeout, - PollingInterval: DefaultPollingInterval, - Description: "Default operation", - } - } -} - -// WaitWithCustomTimeout waits with custom timeout configuration -func (*TimingTestHelper) WaitWithCustomTimeout(assertion func() interface{}, config TimeoutConfig) gomega.AsyncAssertion { - return gomega.Eventually(assertion, config.Timeout, config.PollingInterval) -} diff --git a/pkg/api/server.go b/pkg/api/server.go index faea31edc1..441804ead0 100644 --- a/pkg/api/server.go +++ b/pkg/api/server.go @@ -142,50 +142,6 @@ func (b *ServerBuilder) WithOtelEnabled(enabled bool) *ServerBuilder { return b } -// WithMiddleware adds middleware to the server -func (b *ServerBuilder) WithMiddleware(mw ...func(http.Handler) http.Handler) *ServerBuilder { - b.middlewares = append(b.middlewares, mw...) - return b -} - -// WithRoute adds a custom route to the server -func (b *ServerBuilder) WithRoute(prefix string, handler http.Handler) *ServerBuilder { - b.customRoutes[prefix] = handler - return b -} - -// WithContainerRuntime sets the container runtime -func (b *ServerBuilder) WithContainerRuntime(containerRuntime runtime.Runtime) *ServerBuilder { - b.containerRuntime = containerRuntime - return b -} - -// WithClientManager sets the client manager -func (b *ServerBuilder) WithClientManager(manager client.Manager) *ServerBuilder { - b.clientManager = manager - return b -} - -// WithWorkloadManager sets the workload manager -func (b *ServerBuilder) WithWorkloadManager(manager workloads.Manager) *ServerBuilder { - b.workloadManager = manager - return b -} - -// WithGroupManager sets the group manager -func (b *ServerBuilder) WithGroupManager(manager groups.Manager) *ServerBuilder { - b.groupManager = manager - return b -} - -// WithSkillManager sets the skill service manager. -// The caller is responsible for closing any underlying resources -// when providing an external skill service. -func (b *ServerBuilder) WithSkillManager(manager skills.SkillService) *ServerBuilder { - b.skillManager = manager - return b -} - // Build creates and configures the HTTP router func (b *ServerBuilder) Build(ctx context.Context) (*chi.Mux, error) { r := chi.NewRouter() @@ -822,41 +778,3 @@ func GenerateNonce() (string, error) { } return hex.EncodeToString(b), nil } - -// Serve starts the server on the given address and serves the API. -// It is assumed that the caller sets up appropriate signal handling. -// If isUnixSocket is true, address is treated as a UNIX socket path. -// If oidcConfig is provided, OIDC authentication will be enabled for all API endpoints. -// Serve is a convenience wrapper that builds and starts the API server. -// For callers that need to configure OTEL or other builder options not exposed -// here, use NewServerBuilder and NewServer directly. -func Serve( - ctx context.Context, - address string, - isUnixSocket bool, - debugMode bool, - enableDocs bool, - oidcConfig *auth.TokenValidatorConfig, - middlewares ...func(http.Handler) http.Handler, -) error { - nonce, err := GenerateNonce() - if err != nil { - return err - } - - builder := NewServerBuilder(). - WithAddress(address). - WithUnixSocket(isUnixSocket). - WithDebugMode(debugMode). - WithDocs(enableDocs). - WithNonce(nonce). - WithOIDCConfig(oidcConfig). - WithMiddleware(middlewares...) - - server, err := NewServer(ctx, builder) - if err != nil { - return err - } - - return server.Start(ctx) -} diff --git a/pkg/auth/token.go b/pkg/auth/token.go index 8b5913f302..1f519ddc46 100644 --- a/pkg/auth/token.go +++ b/pkg/auth/token.go @@ -491,22 +491,6 @@ func discoverOIDCConfiguration( return &doc, nil } -// NewTokenValidatorConfig creates a new TokenValidatorConfig with the provided parameters -func NewTokenValidatorConfig(issuer, audience, jwksURL, clientID string, clientSecret string) *TokenValidatorConfig { - // Only create a config if at least one parameter is provided - if issuer == "" && audience == "" && jwksURL == "" && clientID == "" && clientSecret == "" { - return nil - } - - return &TokenValidatorConfig{ - Issuer: issuer, - Audience: audience, - JWKSURL: jwksURL, - ClientID: clientID, - ClientSecret: clientSecret, - } -} - // registerIntrospectionProviders creates and configures the provider registry // for token introspection based on the configuration. func registerIntrospectionProviders(config TokenValidatorConfig, clientSecret string) (*Registry, error) { diff --git a/pkg/client/config.go b/pkg/client/config.go index e76cc04974..e78cdfa660 100644 --- a/pkg/client/config.go +++ b/pkg/client/config.go @@ -1149,15 +1149,6 @@ func (cm *ClientManager) FindClientConfig(clientType ClientApp) (*ConfigFile, er return configFile, nil } -// FindRegisteredClientConfigs finds all registered client configs and creates them if they don't exist. -func FindRegisteredClientConfigs(ctx context.Context) ([]ConfigFile, error) { - manager, err := NewClientManager() - if err != nil { - return nil, err - } - return manager.FindRegisteredClientConfigs(ctx) -} - // FindRegisteredClientConfigs finds all registered client configs using this manager's dependencies func (cm *ClientManager) FindRegisteredClientConfigs(ctx context.Context) ([]ConfigFile, error) { clientStatuses, err := cm.GetClientStatus(ctx) diff --git a/pkg/container/kubernetes/client.go b/pkg/container/kubernetes/client.go index a4638c4c28..7bb0a4d049 100644 --- a/pkg/container/kubernetes/client.go +++ b/pkg/container/kubernetes/client.go @@ -108,11 +108,6 @@ func NewClient(_ context.Context) (*Client, error) { return NewClientWithConfig(clientset, config), nil } -// IsAvailable checks if kubernetes is available -func IsAvailable() bool { - return k8s.IsAvailable() -} - // NewClientWithConfig creates a new container client with a provided config // This is primarily used for testing with fake clients func NewClientWithConfig(clientset kubernetes.Interface, config *rest.Config) *Client { diff --git a/pkg/container/runtime/registry.go b/pkg/container/runtime/registry.go index 5c2053259b..52f4394acb 100644 --- a/pkg/container/runtime/registry.go +++ b/pkg/container/runtime/registry.go @@ -110,21 +110,6 @@ func RegisterRuntime(info *Info) { DefaultRegistry.Register(info) } -// GetRegisteredRuntime returns the Info from the DefaultRegistry for the given name. -func GetRegisteredRuntime(name string) *Info { - return DefaultRegistry.Get(name) -} - -// IsRuntimeRegistered returns true if a runtime is registered in the DefaultRegistry. -func IsRuntimeRegistered(name string) bool { - return DefaultRegistry.IsRegistered(name) -} - -// RegisteredRuntimes returns all runtimes from the DefaultRegistry. -func RegisteredRuntimes() []*Info { - return DefaultRegistry.All() -} - // RegisteredRuntimesByPriority returns all runtimes from the DefaultRegistry // sorted by priority. func RegisteredRuntimesByPriority() []*Info { diff --git a/pkg/json/any.go b/pkg/json/any.go index e055629569..47a4dbb49e 100644 --- a/pkg/json/any.go +++ b/pkg/json/any.go @@ -12,7 +12,6 @@ import ( "fmt" "gopkg.in/yaml.v3" - "k8s.io/apimachinery/pkg/runtime" ) // Data stores JSON-compatible data of type T. It supports both JSON and YAML @@ -143,11 +142,6 @@ type Any = Data[any] // +kubebuilder:validation:Type=object type Map = Data[map[string]any] -// NewData creates a Data[T] from a value. -func NewData[T any](v T) Data[T] { - return Data[T]{Value: v} -} - // NewAny creates an Any (Data[any]) from a value. // This is a convenience function for tests and programmatic use. func NewAny(v any) Any { @@ -159,42 +153,6 @@ func NewMap(m map[string]any) Map { return Map{Value: m} } -// MustParse parses a JSON string into an Any. -// This is a convenience function for tests. Panics if parsing fails. -func MustParse(jsonStr string) Any { - var v any - if err := stdjson.Unmarshal([]byte(jsonStr), &v); err != nil { - panic(fmt.Sprintf("json.MustParse: failed to parse JSON: %v", err)) - } - return Any{Value: v} -} - -// FromRawExtension creates an Any from runtime.RawExtension. -// Returns an error if the JSON cannot be unmarshaled. -func FromRawExtension(ext runtime.RawExtension) (Any, error) { - if len(ext.Raw) == 0 { - return Any{}, nil - } - var v any - if err := stdjson.Unmarshal(ext.Raw, &v); err != nil { - return Any{}, fmt.Errorf("failed to unmarshal RawExtension: %w", err) - } - return Any{Value: v}, nil -} - -// MapFromRawExtension creates a Map from runtime.RawExtension. -// Returns an error if the JSON cannot be unmarshaled. -func MapFromRawExtension(ext runtime.RawExtension) (Map, error) { - if len(ext.Raw) == 0 { - return Map{}, nil - } - var v map[string]any - if err := stdjson.Unmarshal(ext.Raw, &v); err != nil { - return Map{}, fmt.Errorf("failed to unmarshal RawExtension as map: %w", err) - } - return Map{Value: v}, nil -} - // ToMap returns the data as a map[string]any. // This is a convenience method for Any types. // Returns nil if there is no data or if the data is not a map. diff --git a/pkg/k8s/client.go b/pkg/k8s/client.go index 336b8195fa..aee1c0ea8e 100644 --- a/pkg/k8s/client.go +++ b/pkg/k8s/client.go @@ -7,7 +7,6 @@ import ( "fmt" "k8s.io/apimachinery/pkg/runtime" - "k8s.io/client-go/dynamic" "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" "sigs.k8s.io/controller-runtime/pkg/client" @@ -77,32 +76,3 @@ func newControllerRuntimeClientWithConfig(config *rest.Config, scheme *runtime.S return k8sClient, nil } - -// NewDynamicClient creates a new dynamic client for working with arbitrary resources. -// Use this when you need to work with resources without compile-time type information, -// such as discovering resources at runtime or working with unstructured data. -func NewDynamicClient() (dynamic.Interface, error) { - config, err := GetConfig() - if err != nil { - return nil, fmt.Errorf("failed to get kubernetes config: %w", err) - } - - return newDynamicClientWithConfig(config) -} - -// newDynamicClientWithConfig is the internal implementation for creating a dynamic client -func newDynamicClientWithConfig(config *rest.Config) (dynamic.Interface, error) { - dynamicClient, err := dynamic.NewForConfig(config) - if err != nil { - return nil, fmt.Errorf("failed to create dynamic client: %w", err) - } - - return dynamicClient, nil -} - -// IsAvailable checks if Kubernetes is available by attempting to create a client -// and verifying connectivity. -func IsAvailable() bool { - _, _, err := NewClient() - return err == nil -} diff --git a/pkg/k8s/client_test.go b/pkg/k8s/client_test.go index 683e2f3efd..b240037a5d 100644 --- a/pkg/k8s/client_test.go +++ b/pkg/k8s/client_test.go @@ -123,20 +123,6 @@ func TestNewControllerRuntimeClientWithConfig(t *testing.T) { } } -func TestNewDynamicClientWithConfig(t *testing.T) { - t.Parallel() - - t.Run("creates dynamic client from valid config", func(t *testing.T) { - t.Parallel() - - config := createTestConfig(t) - client, err := newDynamicClientWithConfig(config) - - assert.NoError(t, err) - assert.NotNil(t, client) - }) -} - func TestClientTypeCompatibility(t *testing.T) { t.Parallel() diff --git a/pkg/labels/labels.go b/pkg/labels/labels.go index 5da8db81c5..d1ffc7ec09 100644 --- a/pkg/labels/labels.go +++ b/pkg/labels/labels.go @@ -113,16 +113,6 @@ func GetPort(labels map[string]string) (int, error) { return port, nil } -// GetGroup gets the group name from labels -func GetGroup(labels map[string]string) string { - return labels[LabelGroup] -} - -// SetGroup sets the group name in labels -func SetGroup(labels map[string]string, groupName string) { - labels[LabelGroup] = groupName -} - // IsAuxiliaryWorkload checks if a workload is an auxiliary workload (like inspector) // Auxiliary workloads don't follow standard workload management patterns and don't use proxy processes func IsAuxiliaryWorkload(labels map[string]string) bool { diff --git a/pkg/networking/port.go b/pkg/networking/port.go index b6bf5f3e03..6e35d08248 100644 --- a/pkg/networking/port.go +++ b/pkg/networking/port.go @@ -61,41 +61,6 @@ func IsAvailable(port int) bool { return true } -// IsIPv6Available checks if IPv6 is available on the system -// by looking for IPv6 addresses on network interfaces -func IsIPv6Available() bool { - interfaces, err := net.Interfaces() - if err != nil { - return false - } - - for _, iface := range interfaces { - if iface.Flags&net.FlagUp == 0 { - // Interface is down - continue - } - - addrs, err := iface.Addrs() - if err != nil { - continue - } - - for _, addr := range addrs { - ipNet, ok := addr.(*net.IPNet) - if !ok { - continue - } - - if ipNet.IP.To4() == nil && !ipNet.IP.IsLoopback() { - // This is an IPv6 address and not a loopback - return true - } - } - } - - return false -} - // FindAvailable finds an available port func FindAvailable() int { for i := 0; i < MaxAttempts; i++ { diff --git a/pkg/oauthproto/tokenexchange/exchange.go b/pkg/oauthproto/tokenexchange/exchange.go index b06088ed7c..5f3018b539 100644 --- a/pkg/oauthproto/tokenexchange/exchange.go +++ b/pkg/oauthproto/tokenexchange/exchange.go @@ -71,29 +71,12 @@ type exchangeRequest struct { ActingParty *actingParty } -// String implements fmt.Stringer for exchangeRequest, redacting sensitive tokens. -func (r exchangeRequest) String() string { - actorToken := "" - if r.ActingParty != nil { - actorToken = oauthproto.Redact(r.ActingParty.ActorToken) - } - - return fmt.Sprintf("exchangeRequest{GrantType: %s, Audience: %s, Resource: %s, Scope: %v, SubjectToken: %s, ActorToken: %s}", - r.GrantType, r.Audience, r.Resource, r.Scope, oauthproto.Redact(r.SubjectToken), actorToken) -} - // clientAuthentication represents OAuth client credentials for token exchange. type clientAuthentication struct { ClientID string ClientSecret string } -// String implements fmt.Stringer for clientAuthentication, redacting the client secret. -func (c clientAuthentication) String() string { - return fmt.Sprintf("clientAuthentication{ClientID: %s, ClientSecret: %s}", - c.ClientID, oauthproto.Redact(c.ClientSecret)) -} - // ExchangeConfig holds the configuration for token exchange. type ExchangeConfig struct { // TokenURL is the OAuth 2.0 token endpoint URL diff --git a/pkg/runner/config_builder.go b/pkg/runner/config_builder.go index 897d091272..97d9ebd5b5 100644 --- a/pkg/runner/config_builder.go +++ b/pkg/runner/config_builder.go @@ -181,14 +181,6 @@ func WithName(name string) RunConfigBuilderOption { } } -// WithMiddlewareConfig sets the middleware configuration -func WithMiddlewareConfig(middlewareConfig []types.MiddlewareConfig) RunConfigBuilderOption { - return func(b *runConfigBuilder) error { - b.config.MiddlewareConfigs = middlewareConfig - return nil - } -} - // WithCmdArgs sets the command arguments func WithCmdArgs(args []string) RunConfigBuilderOption { return func(b *runConfigBuilder) error { diff --git a/pkg/runner/permissions.go b/pkg/runner/permissions.go index a97f5b0eda..d524699938 100644 --- a/pkg/runner/permissions.go +++ b/pkg/runner/permissions.go @@ -4,52 +4,16 @@ package runner import ( - "encoding/json" "fmt" "log/slog" "os" "path/filepath" "strings" - - "github.com/stacklok/toolhive-core/permissions" ) // This was moved from the CLI to allow it to be shared with the lifecycle manager. // It will likely be moved elsewhere in a future PR. -// CreatePermissionProfileFile creates a temporary file with the permission profile -func CreatePermissionProfileFile(serverName string, permProfile *permissions.Profile) (string, error) { - tempFile, err := os.CreateTemp("", fmt.Sprintf("toolhive-%s-permissions-*.json", serverName)) - if err != nil { - return "", fmt.Errorf("failed to create temporary file: %w", err) - } - defer func() { - if err := tempFile.Close(); err != nil { - // Non-fatal: temp file cleanup failure - slog.Warn("Failed to close temp file", "error", err) - } - }() - - // Get the temporary file path - permProfilePath := tempFile.Name() - - // Serialize the permission profile to JSON - permProfileJSON, err := json.Marshal(permProfile) - if err != nil { - return "", fmt.Errorf("failed to serialize permission profile: %w", err) - } - - // Write the permission profile to the temporary file - if _, err := tempFile.Write(permProfileJSON); err != nil { - return "", fmt.Errorf("failed to write permission profile to file: %w", err) - } - - //nolint:gosec // G706: path is a temp file created by us - slog.Debug("Wrote permission profile to temporary file", "path", permProfilePath) - - return permProfilePath, nil -} - // CleanupTempPermissionProfile removes a temporary permission profile file if it was created by toolhive func CleanupTempPermissionProfile(permissionProfilePath string) error { if permissionProfilePath == "" { diff --git a/pkg/server/discovery/discovery.go b/pkg/server/discovery/discovery.go index 04790e19ca..b00ecddfc2 100644 --- a/pkg/server/discovery/discovery.go +++ b/pkg/server/discovery/discovery.go @@ -64,12 +64,6 @@ func WriteServerInfo(info *ServerInfo) error { return writeServerInfoTo(defaultDiscoveryDir(), info) } -// ReadServerInfo reads and parses the server discovery file. -// Returns os.ErrNotExist if the file does not exist. -func ReadServerInfo() (*ServerInfo, error) { - return readServerInfoFrom(defaultDiscoveryDir()) -} - // RemoveServerInfo removes the server discovery file. // It is a no-op if the file does not exist. func RemoveServerInfo() error { diff --git a/pkg/state/runconfig.go b/pkg/state/runconfig.go index 2bb1a29008..ea1fc91c7d 100644 --- a/pkg/state/runconfig.go +++ b/pkg/state/runconfig.go @@ -121,41 +121,6 @@ func LoadRunConfig[T any](ctx context.Context, name string, readJSONFunc ReadJSO return readJSONFunc(reader) } -// ReadRunConfigJSON deserializes a run configuration from JSON read from the provided reader -// This is a generic JSON deserializer for any type that can be unmarshalled from JSON -func ReadRunConfigJSON[T any](r io.Reader) (*T, error) { - var config T - decoder := json.NewDecoder(r) - if err := decoder.Decode(&config); err != nil { - return nil, err - } - return &config, nil -} - -// LoadRunConfigOfType loads a run configuration of a specific type T from the state store -func LoadRunConfigOfType[T any](ctx context.Context, name string) (*T, error) { - return LoadRunConfig(ctx, name, ReadRunConfigJSON[T]) -} - -// RunConfigReadJSONFunc defines the function signature for reading a RunConfig from JSON -// This allows us to accept the runner.ReadJSON function without creating a circular dependency -type RunConfigReadJSONFunc func(r io.Reader) (interface{}, error) - -// LoadRunConfigWithFunc loads a run configuration using a provided read function -func LoadRunConfigWithFunc(ctx context.Context, name string, readFunc RunConfigReadJSONFunc) (interface{}, error) { - reader, err := LoadRunConfigJSON(ctx, name) - if err != nil { - return nil, err - } - defer func() { - if err := reader.Close(); err != nil { - slog.Warn("Failed to close reader", "error", err) - } - }() - - return readFunc(reader) -} - // ReadJSON deserializes JSON from the provided reader into a generic interface // This function is moved from the runner package to avoid circular dependencies func ReadJSON(r io.Reader, target interface{}) error { diff --git a/pkg/transport/proxy/transparent/pinger.go b/pkg/transport/proxy/transparent/pinger.go index 9144bff464..52a8d58417 100644 --- a/pkg/transport/proxy/transparent/pinger.go +++ b/pkg/transport/proxy/transparent/pinger.go @@ -26,11 +26,6 @@ const ( DefaultPingerTimeout = 5 * time.Second ) -// NewMCPPinger creates a new MCP pinger for transparent proxies -func NewMCPPinger(targetURL string) healthcheck.MCPPinger { - return NewMCPPingerWithTimeout(targetURL, DefaultPingerTimeout) -} - // NewMCPPingerWithTimeout creates a new MCP pinger with a custom timeout func NewMCPPingerWithTimeout(targetURL string, timeout time.Duration) healthcheck.MCPPinger { if timeout <= 0 { diff --git a/pkg/vmcp/aggregator/discoverer.go b/pkg/vmcp/aggregator/discoverer.go index d50f409ed7..682fa4d003 100644 --- a/pkg/vmcp/aggregator/discoverer.go +++ b/pkg/vmcp/aggregator/discoverer.go @@ -122,16 +122,6 @@ func NewBackendDiscoverer( return NewUnifiedBackendDiscoverer(workloadDiscoverer, groupsManager, authConfig), nil } -// NewBackendDiscovererWithManager creates a unified BackendDiscoverer with a pre-configured -// WorkloadDiscoverer. This is useful for testing or when you already have a workload manager. -func NewBackendDiscovererWithManager( - workloadManager workloads.Discoverer, - groupsManager groups.Manager, - authConfig *config.OutgoingAuthConfig, -) BackendDiscoverer { - return NewUnifiedBackendDiscoverer(workloadManager, groupsManager, authConfig) -} - // Discover finds all backend workloads in the specified group. // Returns all accessible backends with their health status marked based on workload status. // The groupRef is the group name (e.g., "engineering-team"). diff --git a/pkg/vmcp/composer/workflow_errors.go b/pkg/vmcp/composer/workflow_errors.go index a16fdbcaf4..d51d94f88e 100644 --- a/pkg/vmcp/composer/workflow_errors.go +++ b/pkg/vmcp/composer/workflow_errors.go @@ -39,44 +39,6 @@ var ( ErrToolCallFailed = errors.New("tool call failed") ) -// WorkflowError wraps workflow execution errors with context. -type WorkflowError struct { - // WorkflowID is the workflow execution ID. - WorkflowID string - - // StepID is the step that caused the error (if applicable). - StepID string - - // Message is the error message. - Message string - - // Cause is the underlying error. - Cause error -} - -// Error implements the error interface. -func (e *WorkflowError) Error() string { - if e.StepID != "" { - return fmt.Sprintf("workflow %s, step %s: %s: %v", e.WorkflowID, e.StepID, e.Message, e.Cause) - } - return fmt.Sprintf("workflow %s: %s: %v", e.WorkflowID, e.Message, e.Cause) -} - -// Unwrap returns the underlying error for errors.Is and errors.As. -func (e *WorkflowError) Unwrap() error { - return e.Cause -} - -// NewWorkflowError creates a new workflow error. -func NewWorkflowError(workflowID, stepID, message string, cause error) *WorkflowError { - return &WorkflowError{ - WorkflowID: workflowID, - StepID: stepID, - Message: message, - Cause: cause, - } -} - // ValidationError wraps workflow validation errors. type ValidationError struct { // Field is the field that failed validation. diff --git a/test/e2e/api_helpers.go b/test/e2e/api_helpers.go index ad8c4cc5f6..7b509a2032 100644 --- a/test/e2e/api_helpers.go +++ b/test/e2e/api_helpers.go @@ -7,7 +7,6 @@ package e2e import ( "context" "fmt" - "io" "net/http" "os" "os/exec" @@ -196,20 +195,6 @@ func (s *Server) Get(path string) (*http.Response, error) { return s.httpClient.Do(req) // #nosec G704 -- baseURL is the local test server URL } -// GetWithHeaders performs a GET request with custom headers. -func (s *Server) GetWithHeaders(path string, headers map[string]string) (*http.Response, error) { - req, err := http.NewRequestWithContext(s.ctx, http.MethodGet, s.baseURL+path, nil) - if err != nil { - return nil, err - } - - for key, value := range headers { - req.Header.Set(key, value) - } - - return s.httpClient.Do(req) // #nosec G704 -- baseURL is the local test server URL -} - // BaseURL returns the base URL of the API server. func (s *Server) BaseURL() string { return s.baseURL @@ -242,14 +227,3 @@ func StartServer(config *ServerConfig) *Server { return server } - -// ExpectStatus reads the response body and asserts the status code, -// including the response body in the failure message for debugging. -// The response body is consumed and closed; callers must not read it again. -func ExpectStatus(resp *http.Response, expected int) { - body, _ := io.ReadAll(resp.Body) - //nolint:errcheck,gosec // This is just a test - resp.Body.Close() - ExpectWithOffset(1, resp.StatusCode).To(Equal(expected), - fmt.Sprintf("Response body: %s", string(body))) -} diff --git a/test/e2e/cimd_auth_helpers_test.go b/test/e2e/cimd_auth_helpers_test.go index 6af96eda2d..cc66cd47ad 100644 --- a/test/e2e/cimd_auth_helpers_test.go +++ b/test/e2e/cimd_auth_helpers_test.go @@ -82,11 +82,6 @@ func (s *cimdMockAuthServer) IssuerURL() string { return s.server.URL } -// ResourceMetadataURL returns the RFC 9728 resource metadata URL for this server. -func (s *cimdMockAuthServer) ResourceMetadataURL() string { - return fmt.Sprintf("%s/.well-known/mcp-resource", s.server.URL) -} - // WaitForAuthRequest blocks until an authorization request arrives or the timeout // elapses. func (s *cimdMockAuthServer) WaitForAuthRequest(timeout time.Duration) (cimdAuthRequest, error) { @@ -105,13 +100,6 @@ func (s *cimdMockAuthServer) DcrWasCalled() bool { return s.dcrCalled } -// LastClientID returns the most recent client_id seen in /oauth/authorize. -func (s *cimdMockAuthServer) LastClientID() string { - s.mu.Lock() - defer s.mu.Unlock() - return s.lastClientID -} - // handleDiscovery serves the OIDC discovery document. It sets // client_id_metadata_document_supported based on the server's configuration. func (s *cimdMockAuthServer) handleDiscovery(w http.ResponseWriter, _ *http.Request) { diff --git a/test/e2e/helpers.go b/test/e2e/helpers.go index 6e9d6c4566..cbf40af20d 100644 --- a/test/e2e/helpers.go +++ b/test/e2e/helpers.go @@ -73,12 +73,6 @@ func (c *THVCommand) WithEnv(env ...string) *THVCommand { return c } -// WithDir sets the working directory for the command -func (c *THVCommand) WithDir(dir string) *THVCommand { - c.dir = dir - return c -} - // WithStdin sets the stdin input for the command func (c *THVCommand) WithStdin(stdin string) *THVCommand { c.stdin = stdin diff --git a/test/e2e/llm_gateway_mock.go b/test/e2e/llm_gateway_mock.go index 67c4fd5c60..f7fb27f208 100644 --- a/test/e2e/llm_gateway_mock.go +++ b/test/e2e/llm_gateway_mock.go @@ -71,15 +71,6 @@ func NewLLMGatewayMock(port int) (*LLMGatewayMock, error) { return m, nil } -// NewLLMGatewayMockHTTP creates a mock LLM gateway that serves plain HTTP. -// Useful in e2e tests where the thv subprocess cannot easily be configured to -// trust a self-signed certificate. Call Start to begin serving. -func NewLLMGatewayMockHTTP(port int) *LLMGatewayMock { - m := &LLMGatewayMock{port: port, useTLS: false} - m.server = m.newServer() - return m -} - func (m *LLMGatewayMock) newServer() *http.Server { mux := http.NewServeMux() mux.HandleFunc("/v1/models", m.handleModels) @@ -155,25 +146,6 @@ func (m *LLMGatewayMock) CertPEM() []byte { return m.certPEM } -// TLSClientConfig returns a *tls.Config that trusts the mock gateway's -// self-signed certificate. Useful for building a custom *http.Client in tests. -func (m *LLMGatewayMock) TLSClientConfig() (*tls.Config, error) { - pool := x509.NewCertPool() - if !pool.AppendCertsFromPEM(m.certPEM) { - return nil, fmt.Errorf("failed to parse mock gateway certificate") - } - return &tls.Config{RootCAs: pool, MinVersion: tls.VersionTLS12}, nil //nolint:gosec // test-only; min version set -} - -// Requests returns a copy of all requests received so far, in order. -func (m *LLMGatewayMock) Requests() []GatewayRequest { - m.mu.Lock() - defer m.mu.Unlock() - out := make([]GatewayRequest, len(m.requests)) - copy(out, m.requests) - return out -} - // LastBearerToken returns the Bearer token from the most recent request, or // empty string if no requests have been received or none carried a token. func (m *LLMGatewayMock) LastBearerToken() string { diff --git a/test/e2e/mcp_client_helpers.go b/test/e2e/mcp_client_helpers.go index c2874b99c1..f08b8a2e10 100644 --- a/test/e2e/mcp_client_helpers.go +++ b/test/e2e/mcp_client_helpers.go @@ -151,19 +151,6 @@ func (h *MCPClientHelper) CallTool( return h.client.CallTool(ctx, request) } -// ListResources lists all available resources from the MCP server -func (h *MCPClientHelper) ListResources(ctx context.Context) (*mcp.ListResourcesResult, error) { - request := mcp.ListResourcesRequest{} - return h.client.ListResources(ctx, request) -} - -// ReadResource reads a specific resource -func (h *MCPClientHelper) ReadResource(ctx context.Context, uri string) (*mcp.ReadResourceResult, error) { - request := mcp.ReadResourceRequest{} - request.Params.URI = uri - return h.client.ReadResource(ctx, request) -} - // Ping sends a ping to test connectivity func (h *MCPClientHelper) Ping(ctx context.Context) error { return h.client.Ping(ctx) @@ -194,21 +181,6 @@ func (h *MCPClientHelper) ExpectToolCall( return result } -// ExpectResourceExists verifies that a resource with the given URI exists -func (h *MCPClientHelper) ExpectResourceExists(ctx context.Context, uri string) { - resources, err := h.ListResources(ctx) - ExpectWithOffset(1, err).ToNot(HaveOccurred(), "Should be able to list resources") - - found := false - for _, resource := range resources.Resources { - if resource.URI == uri { - found = true - break - } - } - ExpectWithOffset(1, found).To(BeTrue(), fmt.Sprintf("Resource '%s' should exist", uri)) -} - // WaitForMCPServerReady waits for an MCP server to be ready and responsive func WaitForMCPServerReady(config *TestConfig, serverURL string, mode string, timeout time.Duration) error { ctx, cancel := context.WithTimeout(context.Background(), timeout) @@ -270,47 +242,3 @@ func extractServerNameFromURL(serverURL string) string { } return "unknown" } - -// TestMCPServerBasicFunctionality tests basic MCP server functionality -func TestMCPServerBasicFunctionality(config *TestConfig, serverURL string) error { - ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) - defer cancel() - - // Create MCP client - mcpClient, err := NewMCPClientForSSE(config, serverURL) - if err != nil { - return fmt.Errorf("failed to create MCP client: %w", err) - } - defer func() { - // Error ignored in test cleanup - the test may have already closed the connection - _ = mcpClient.Close() - }() - - // Initialize the connection - if err := mcpClient.Initialize(ctx); err != nil { - return fmt.Errorf("failed to initialize MCP connection: %w", err) - } - - // Test ping - if err := mcpClient.Ping(ctx); err != nil { - return fmt.Errorf("ping failed: %w", err) - } - - // List tools - tools, err := mcpClient.ListTools(ctx) - if err != nil { - return fmt.Errorf("failed to list tools: %w", err) - } - - if len(tools.Tools) == 0 { - return fmt.Errorf("no tools available from MCP server") - } - - // List resources (if supported) - // Note: Not all MCP servers support resources, so we don't fail on this - if _, err := mcpClient.ListResources(ctx); err != nil { - GinkgoWriter.Printf("Note: Server does not support resources: %v\n", err) - } - - return nil -} diff --git a/test/e2e/oidc_mock.go b/test/e2e/oidc_mock.go index 629cdba8ab..af670d9f62 100644 --- a/test/e2e/oidc_mock.go +++ b/test/e2e/oidc_mock.go @@ -450,11 +450,6 @@ func (m *OIDCMockServer) Stop() error { return nil } -// GetBaseURL returns the base URL of the mock server -func (m *OIDCMockServer) GetBaseURL() string { - return fmt.Sprintf("http://localhost:%d", m.port) -} - // EnableAutoComplete enables automatic OAuth flow completion for testing func (m *OIDCMockServer) EnableAutoComplete() { m.autoComplete = true diff --git a/test/e2e/stateless_proxy_test.go b/test/e2e/stateless_proxy_test.go index 3b9c64baf8..fd0f85f8ce 100644 --- a/test/e2e/stateless_proxy_test.go +++ b/test/e2e/stateless_proxy_test.go @@ -257,7 +257,3 @@ func (m *statelessMockMCPServer) Stop() { func (m *statelessMockMCPServer) GetCount() int32 { return m.postHits.Load() } - -func (m *statelessMockMCPServer) GotGET() bool { - return m.gotGET.Load() -} diff --git a/test/e2e/thv-operator/virtualmcp/helpers.go b/test/e2e/thv-operator/virtualmcp/helpers.go index d91e314b72..29bf374a78 100644 --- a/test/e2e/thv-operator/virtualmcp/helpers.go +++ b/test/e2e/thv-operator/virtualmcp/helpers.go @@ -5,7 +5,6 @@ package virtualmcp import ( - "bytes" "context" "encoding/json" "errors" @@ -258,167 +257,6 @@ func WaitForCondition( // OIDC Testing Helpers -// DeployMockOIDCServerHTTP deploys a mock OIDC server with HTTP (for testing) -func DeployMockOIDCServerHTTP(ctx context.Context, c client.Client, namespace, serverName string) { - deployment := &appsv1.Deployment{ - ObjectMeta: metav1.ObjectMeta{ - Name: serverName, - Namespace: namespace, - Labels: map[string]string{"app": serverName}, - }, - Spec: appsv1.DeploymentSpec{ - Replicas: int32Ptr(1), - Selector: &metav1.LabelSelector{ - MatchLabels: map[string]string{"app": serverName}, - }, - Template: corev1.PodTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{"app": serverName}, - }, - Spec: corev1.PodSpec{ - Containers: []corev1.Container{ - { - Name: "mock-oidc", - Image: images.PythonImage, - Command: []string{"sh", "-c"}, - Args: []string{MockOIDCServerHTTPScript}, - Ports: []corev1.ContainerPort{ - {ContainerPort: 80, Name: "http"}, - }, - }, - }, - }, - }, - }, - } - gomega.Expect(c.Create(ctx, deployment)).To(gomega.Succeed()) - - service := &corev1.Service{ - ObjectMeta: metav1.ObjectMeta{ - Name: serverName, - Namespace: namespace, - }, - Spec: corev1.ServiceSpec{ - Selector: map[string]string{"app": serverName}, - Ports: []corev1.ServicePort{ - { - Port: 80, - Protocol: corev1.ProtocolTCP, - }, - }, - }, - } - gomega.Expect(c.Create(ctx, service)).To(gomega.Succeed()) - - gomega.Eventually(func() bool { - dep := &appsv1.Deployment{} - err := c.Get(ctx, types.NamespacedName{Name: serverName, Namespace: namespace}, dep) - return err == nil && dep.Status.ReadyReplicas > 0 - }, 3*time.Minute, 1*time.Second).Should(gomega.BeTrue(), "Mock OIDC server should be ready") -} - -// DeployInstrumentedBackendServer deploys a backend server that logs all headers -func DeployInstrumentedBackendServer(ctx context.Context, c client.Client, namespace, serverName string) { - deployment := &appsv1.Deployment{ - ObjectMeta: metav1.ObjectMeta{ - Name: serverName, - Namespace: namespace, - Labels: map[string]string{"app": serverName}, - }, - Spec: appsv1.DeploymentSpec{ - Replicas: int32Ptr(1), - Selector: &metav1.LabelSelector{ - MatchLabels: map[string]string{"app": serverName}, - }, - Template: corev1.PodTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{"app": serverName}, - }, - Spec: corev1.PodSpec{ - Containers: []corev1.Container{ - { - Name: "instrumented-backend", - Image: images.PythonImage, - Command: []string{"sh", "-c"}, - Args: []string{InstrumentedBackendScript}, - Ports: []corev1.ContainerPort{ - {ContainerPort: 8080, Name: "http"}, - }, - }, - }, - }, - }, - }, - } - gomega.Expect(c.Create(ctx, deployment)).To(gomega.Succeed()) - - service := &corev1.Service{ - ObjectMeta: metav1.ObjectMeta{ - Name: serverName, - Namespace: namespace, - }, - Spec: corev1.ServiceSpec{ - Selector: map[string]string{"app": serverName}, - Ports: []corev1.ServicePort{ - { - Port: 8080, - Protocol: corev1.ProtocolTCP, - }, - }, - }, - } - gomega.Expect(c.Create(ctx, service)).To(gomega.Succeed()) - - gomega.Eventually(func() bool { - dep := &appsv1.Deployment{} - err := c.Get(ctx, types.NamespacedName{Name: serverName, Namespace: namespace}, dep) - return err == nil && dep.Status.ReadyReplicas > 0 - }, 3*time.Minute, 1*time.Second).Should(gomega.BeTrue(), "Instrumented backend should be ready") -} - -// CleanupMockServer cleans up a mock server deployment, service, and optionally its TLS secret -func CleanupMockServer(ctx context.Context, c client.Client, namespace, serverName, tlsSecretName string) { - _ = c.Delete(ctx, &appsv1.Deployment{ - ObjectMeta: metav1.ObjectMeta{Name: serverName, Namespace: namespace}, - }) - _ = c.Delete(ctx, &corev1.Service{ - ObjectMeta: metav1.ObjectMeta{Name: serverName, Namespace: namespace}, - }) - if tlsSecretName != "" { - _ = c.Delete(ctx, &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{Name: tlsSecretName, Namespace: namespace}, - }) - } -} - -// GetPodLogsForDeployment returns logs from pods for a deployment (for debugging) -func GetPodLogsForDeployment(ctx context.Context, c client.Client, namespace, deploymentName string) string { - pods := &corev1.PodList{} - listOpts := []client.ListOption{ - client.InNamespace(namespace), - client.MatchingLabels{"app": deploymentName}, - } - - err := c.List(ctx, pods, listOpts...) - if err != nil || len(pods.Items) == 0 { - return fmt.Sprintf("No pods found for deployment %s", deploymentName) - } - - pod := pods.Items[0] - if len(pod.Spec.Containers) == 0 { - return fmt.Sprintf("No containers found in pod %s", pod.Name) - } - - // Get logs from the first container - containerName := pod.Spec.Containers[0].Name - logs, err := getPodLogs(ctx, namespace, pod.Name, containerName, false) - if err != nil { - return fmt.Sprintf("Failed to get logs for pod %s: %v", pod.Name, err) - } - - return logs -} - // GetPodLogs returns logs from a specific pod and container func GetPodLogs(ctx context.Context, podName, namespace, containerName string) (string, error) { logs, err := getPodLogs(ctx, namespace, podName, containerName, false) @@ -432,34 +270,6 @@ func int32Ptr(i int32) *int32 { return &i } -// GetMCPServerDeployment retrieves the deployment for an MCPServer by name. -// MCPServer deployments use the same name as the MCPServer resource. -func GetMCPServerDeployment(ctx context.Context, c client.Client, serverName, namespace string) *appsv1.Deployment { - deployment := &appsv1.Deployment{} - err := c.Get(ctx, types.NamespacedName{ - Name: serverName, - Namespace: namespace, - }, deployment) - if err != nil { - return nil - } - return deployment -} - -// GetMCPServerStatefulSet retrieves the StatefulSet for an MCPServer by name. -// MCPServer StatefulSets use the same name as the MCPServer resource for the workload pods. -func GetMCPServerStatefulSet(ctx context.Context, c client.Client, serverName, namespace string) *appsv1.StatefulSet { - statefulset := &appsv1.StatefulSet{} - err := c.Get(ctx, types.NamespacedName{ - Name: serverName, - Namespace: namespace, - }, statefulset) - if err != nil { - return nil - } - return statefulset -} - // WaitForPodDeletion waits for a pod to be fully deleted from the cluster. // This is useful in AfterAll cleanup to ensure pods are gone before tests repeat. func WaitForPodDeletion(ctx context.Context, c client.Client, name, namespace string, timeout, pollingInterval time.Duration) { @@ -517,36 +327,6 @@ func GetServiceStats(ctx context.Context, c client.Client, namespace, serviceNam return logs, nil } -// GetMockOIDCStats queries the /stats endpoint of the mock OIDC server -func GetMockOIDCStats(ctx context.Context, c client.Client, namespace, serviceName string) (map[string]int, error) { - logs, err := GetServiceStats(ctx, c, namespace, serviceName, 80) - if err != nil { - return nil, err - } - - // Parse JSON response - check if discovery_requests field exists - stats := make(map[string]int) - if len(logs) > 0 && bytes.Contains([]byte(logs), []byte("discovery_requests")) { - stats["discovery_requests"] = 1 // Simplified - just check if field exists - } - return stats, nil -} - -// GetInstrumentedBackendStats queries the /stats endpoint of the instrumented backend -func GetInstrumentedBackendStats(ctx context.Context, c client.Client, namespace, serviceName string) (map[string]int, error) { - logs, err := GetServiceStats(ctx, c, namespace, serviceName, 8080) - if err != nil { - return nil, err - } - - // Parse JSON response - check if bearer_token_requests field exists - stats := make(map[string]int) - if len(logs) > 0 && bytes.Contains([]byte(logs), []byte("bearer_token_requests")) { - stats["bearer_token_requests"] = 1 // Simplified - just check if field exists and > 0 - } - return stats, nil -} - // GetMockOAuth2Stats queries the /stats endpoint of the mock OAuth2 server (port 8080) // and returns the number of client_credentials grant requests recorded so far. func GetMockOAuth2Stats(ctx context.Context, c client.Client, namespace, serviceName string) (int, error) { diff --git a/test/e2e/thv-operator/virtualmcp/wait_for_tools_helpers.go b/test/e2e/thv-operator/virtualmcp/wait_for_tools_helpers.go index 437c3e7207..9699e21fc2 100644 --- a/test/e2e/thv-operator/virtualmcp/wait_for_tools_helpers.go +++ b/test/e2e/thv-operator/virtualmcp/wait_for_tools_helpers.go @@ -138,29 +138,6 @@ func ToolsContainAll(tools []mcp.Tool, expectedNames ...string) error { return nil } -// ToolsContainSubstring checks if the tool list contains at least one tool whose -// name contains each of the given substrings. Returns an error if any substring -// has no matching tool. -func ToolsContainSubstring(tools []mcp.Tool, substrings ...string) error { - var missing []string - for _, sub := range substrings { - found := false - for _, t := range tools { - if strings.Contains(t.Name, sub) { - found = true - break - } - } - if !found { - missing = append(missing, sub) - } - } - if len(missing) > 0 { - return fmt.Errorf("no tools matching substrings %v; got %v", missing, toolNames(tools)) - } - return nil -} - // ToolsHavePrefix checks if there is at least one tool with each of the given prefixes. // Returns an error listing missing prefixes, or nil if all are found. func ToolsHavePrefix(tools []mcp.Tool, prefixes ...string) error { diff --git a/test/integration/authserver/helpers/authserver.go b/test/integration/authserver/helpers/authserver.go index 54b34ba779..f4939f445f 100644 --- a/test/integration/authserver/helpers/authserver.go +++ b/test/integration/authserver/helpers/authserver.go @@ -31,27 +31,6 @@ type authServerConfig struct { baselineClientScopes []string } -// WithIssuer sets the issuer URL. -func WithIssuer(issuer string) AuthServerOption { - return func(c *authServerConfig) { - c.issuer = issuer - } -} - -// WithUpstreams sets the upstream IDP configurations. -func WithUpstreams(upstreams []authserver.UpstreamRunConfig) AuthServerOption { - return func(c *authServerConfig) { - c.upstreams = upstreams - } -} - -// WithAllowedAudiences sets the allowed resource audiences. -func WithAllowedAudiences(audiences []string) AuthServerOption { - return func(c *authServerConfig) { - c.allowedAudiences = audiences - } -} - // WithSigningKey sets the signing key configuration. func WithSigningKey(cfg *authserver.SigningKeyRunConfig) AuthServerOption { return func(c *authServerConfig) { @@ -59,20 +38,6 @@ func WithSigningKey(cfg *authserver.SigningKeyRunConfig) AuthServerOption { } } -// WithHMACSecrets sets the HMAC secret file paths. -func WithHMACSecrets(files []string) AuthServerOption { - return func(c *authServerConfig) { - c.hmacSecretFiles = files - } -} - -// WithTokenLifespans sets the token lifespan configuration. -func WithTokenLifespans(cfg *authserver.TokenLifespanRunConfig) AuthServerOption { - return func(c *authServerConfig) { - c.tokenLifespans = cfg - } -} - // WithScopesSupported sets the supported scopes. func WithScopesSupported(scopes []string) AuthServerOption { return func(c *authServerConfig) { diff --git a/test/integration/authserver/helpers/http_client.go b/test/integration/authserver/helpers/http_client.go index 53eb674870..550be402e1 100644 --- a/test/integration/authserver/helpers/http_client.go +++ b/test/integration/authserver/helpers/http_client.go @@ -174,8 +174,3 @@ func (c *OAuthClient) RegisterClient(clientMetadata map[string]interface{}) (map return result, resp.StatusCode, nil } - -// Get performs a GET request to the specified path. -func (c *OAuthClient) Get(path string) (*http.Response, error) { - return c.httpClient.Get(c.baseURL + path) -} diff --git a/test/integration/authserver/helpers/mock_upstream.go b/test/integration/authserver/helpers/mock_upstream.go index 9c88da2c89..825a83e32d 100644 --- a/test/integration/authserver/helpers/mock_upstream.go +++ b/test/integration/authserver/helpers/mock_upstream.go @@ -25,27 +25,6 @@ type MockUpstreamIDP struct { // MockUpstreamOption is a functional option for configuring the mock upstream. type MockUpstreamOption func(*MockUpstreamIDP) -// WithAuthorizeHandler sets a custom authorization endpoint handler. -func WithAuthorizeHandler(h func(w http.ResponseWriter, r *http.Request)) MockUpstreamOption { - return func(m *MockUpstreamIDP) { - m.AuthorizeHandler = h - } -} - -// WithTokenHandler sets a custom token endpoint handler. -func WithTokenHandler(h func(w http.ResponseWriter, r *http.Request)) MockUpstreamOption { - return func(m *MockUpstreamIDP) { - m.TokenHandler = h - } -} - -// WithUserInfoHandler sets a custom userinfo endpoint handler. -func WithUserInfoHandler(h func(w http.ResponseWriter, r *http.Request)) MockUpstreamOption { - return func(m *MockUpstreamIDP) { - m.UserInfoHandler = h - } -} - // NewMockUpstreamIDP creates a mock upstream IDP for testing. // The server is automatically started and will be ready when this function returns. func NewMockUpstreamIDP(tb testing.TB, opts ...MockUpstreamOption) *MockUpstreamIDP {