From b4a15bb919964db018608a9384bd118bdc797916 Mon Sep 17 00:00:00 2001 From: Chris Burns <29541485+ChrisJBurns@users.noreply.github.com> Date: Fri, 10 Apr 2026 00:49:34 +0100 Subject: [PATCH] Query registry API instead of ConfigMap for image validation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The image validation logic used GetStorageName() to look up a {name}-registry-storage ConfigMap that was only created by the legacy typed config path. With the configYAML path, this ConfigMap does not exist, causing enforceServers to silently degrade — the lookup returns NotFound and checkImageInRegistry returns false. Replace the ConfigMap-based image lookup with HTTP queries to the registry API service at Status.URL (/v0.1/servers endpoint). This checks OCI package identifiers in the server list, which works correctly with both legacy and configYAML paths. Fixes #4717 Co-Authored-By: Claude Opus 4.6 (1M context) --- .../pkg/validation/image_validation.go | 142 ++-- .../pkg/validation/image_validation_test.go | 642 +++++++----------- 2 files changed, 347 insertions(+), 437 deletions(-) diff --git a/cmd/thv-operator/pkg/validation/image_validation.go b/cmd/thv-operator/pkg/validation/image_validation.go index b6a2027ed5..f51f03be90 100644 --- a/cmd/thv-operator/pkg/validation/image_validation.go +++ b/cmd/thv-operator/pkg/validation/image_validation.go @@ -9,13 +9,15 @@ import ( "encoding/json" "errors" "fmt" + "net/http" + "net/url" + "time" - corev1 "k8s.io/api/core/v1" - k8serr "k8s.io/apimachinery/pkg/api/errors" + v0 "github.com/modelcontextprotocol/registry/pkg/api/v0" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/log" - regtypes "github.com/stacklok/toolhive-core/registry/types" mcpv1alpha1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1alpha1" ) @@ -68,17 +70,21 @@ func (*AlwaysAllowValidator) ValidateImage(_ context.Context, _ string, _ metav1 func NewImageValidator(k8sClient client.Client, namespace string, validation ImageValidation) ImageValidator { if validation == ImageValidationRegistryEnforcing { return &RegistryEnforcingValidator{ - client: k8sClient, - namespace: namespace, + client: k8sClient, + namespace: namespace, + httpClient: &http.Client{Timeout: 10 * time.Second}, } } return &AlwaysAllowValidator{} } -// RegistryEnforcingValidator provides validation against MCPRegistry resources +// RegistryEnforcingValidator provides validation against MCPRegistry resources. +// It queries the registry API service (via HTTP) to check whether an image +// exists in a registry's OCI packages. type RegistryEnforcingValidator struct { - client client.Client - namespace string + client client.Client + namespace string + httpClient *http.Client } // ValidateImage checks if an image should be validated and if it exists in registries @@ -191,7 +197,8 @@ func (*RegistryEnforcingValidator) getEnforcingRegistries( return enforcingRegistries } -// checkImageInRegistry checks if an image exists in a specific MCPRegistry +// checkImageInRegistry checks if an image exists in a specific MCPRegistry by +// querying the registry API service at the URL stored in the MCPRegistry status. func (v *RegistryEnforcingValidator) checkImageInRegistry( ctx context.Context, mcpRegistry *mcpv1alpha1.MCPRegistry, @@ -202,56 +209,101 @@ func (v *RegistryEnforcingValidator) checkImageInRegistry( return false, nil } - // Get the ConfigMap containing the registry data - configMapName := mcpRegistry.GetStorageName() - configMap := &corev1.ConfigMap{} - if err := v.client.Get(ctx, client.ObjectKey{ - Name: configMapName, - Namespace: v.namespace, - }, configMap); err != nil { - if k8serr.IsNotFound(err) { - // ConfigMap not found, registry data not available - return false, nil - } - return false, fmt.Errorf("failed to get ConfigMap %s: %w", configMapName, err) + // Get the registry API URL from status + registryURL := mcpRegistry.Status.URL + if registryURL == "" { + return false, nil } - // Get the registry data from the ConfigMap - registryData, exists := configMap.Data["registry.json"] - if !exists { - // No registry data in ConfigMap - return false, nil + // Query the registry API for all servers + servers, err := v.listRegistryServers(ctx, registryURL) + if err != nil { + return false, fmt.Errorf("failed to query registry API at %s: %w", registryURL, err) } - // Parse the registry data - var reg regtypes.Registry - if err := json.Unmarshal([]byte(registryData), ®); err != nil { - // Invalid registry data - return false, fmt.Errorf("failed to parse registry data: %w", err) + return findImageInServers(servers, image), nil +} + +// listRegistryServers queries the registry API to fetch all servers, handling pagination. +func (v *RegistryEnforcingValidator) listRegistryServers( + ctx context.Context, + registryURL string, +) ([]v0.ServerResponse, error) { + var allServers []v0.ServerResponse + cursor := "" + + for { + servers, nextCursor, err := v.fetchRegistryPage(ctx, registryURL, cursor) + if err != nil { + return nil, err + } + + allServers = append(allServers, servers...) + + if nextCursor == "" { + break + } + cursor = nextCursor + + // Safety limit to prevent infinite loops + if len(allServers) > 10000 { + return nil, fmt.Errorf("exceeded maximum server limit (10000)") + } } - // Search for the image in this registry - return findImageInRegistry(®, image), nil + return allServers, nil } -// findImageInRegistry searches for an image in a registry -func findImageInRegistry(reg *regtypes.Registry, image string) bool { - // Check top-level servers - for _, server := range reg.Servers { - if server.Image == image { - return true +// fetchRegistryPage fetches a single page of servers from the registry API. +func (v *RegistryEnforcingValidator) fetchRegistryPage( + ctx context.Context, + registryURL string, + cursor string, +) ([]v0.ServerResponse, string, error) { + params := url.Values{} + params.Set("limit", "100") + if cursor != "" { + params.Set("cursor", cursor) + } + + endpoint := fmt.Sprintf("%s/v0.1/servers?%s", registryURL, params.Encode()) + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, endpoint, nil) + if err != nil { + return nil, "", fmt.Errorf("failed to create request: %w", err) + } + + resp, err := v.httpClient.Do(req) + if err != nil { + return nil, "", fmt.Errorf("failed to query registry: %w", err) + } + defer func() { + if closeErr := resp.Body.Close(); closeErr != nil { + log.FromContext(ctx).V(1).Info("Failed to close response body", "error", closeErr) } + }() + + if resp.StatusCode != http.StatusOK { + return nil, "", fmt.Errorf("registry API returned status %d", resp.StatusCode) + } + + var listResp v0.ServerListResponse + if err := json.NewDecoder(resp.Body).Decode(&listResp); err != nil { + return nil, "", fmt.Errorf("failed to decode registry API response: %w", err) } - // Check servers in groups - // TODO: check with Rado or Ria, is this needed? - for _, group := range reg.Groups { - for _, server := range group.Servers { - if server.Image == image { + return listResp.Servers, listResp.Metadata.NextCursor, nil +} + +// findImageInServers searches for an OCI image in the servers returned by the registry API. +func findImageInServers(servers []v0.ServerResponse, image string) bool { + for i := range servers { + for j := range servers[i].Server.Packages { + if servers[i].Server.Packages[j].RegistryType == "oci" && + servers[i].Server.Packages[j].Identifier == image { return true } } } - return false } diff --git a/cmd/thv-operator/pkg/validation/image_validation_test.go b/cmd/thv-operator/pkg/validation/image_validation_test.go index 76ede4408d..dba131f17e 100644 --- a/cmd/thv-operator/pkg/validation/image_validation_test.go +++ b/cmd/thv-operator/pkg/validation/image_validation_test.go @@ -5,17 +5,20 @@ package validation import ( "context" + "encoding/json" "fmt" + "net/http" + "net/http/httptest" "testing" + v0 "github.com/modelcontextprotocol/registry/pkg/api/v0" + "github.com/modelcontextprotocol/registry/pkg/model" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "sigs.k8s.io/controller-runtime/pkg/client/fake" - regtypes "github.com/stacklok/toolhive-core/registry/types" mcpv1alpha1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1alpha1" ) @@ -58,7 +61,6 @@ func TestNewImageValidator(t *testing.T) { t.Parallel() scheme := runtime.NewScheme() require.NoError(t, mcpv1alpha1.AddToScheme(scheme)) - require.NoError(t, corev1.AddToScheme(scheme)) fakeClient := fake.NewClientBuilder().WithScheme(scheme).Build() @@ -112,56 +114,75 @@ func TestNewImageValidator(t *testing.T) { } } +// newRegistryAPIServer creates an httptest.Server that serves the registry API +// /v0.1/servers endpoint, returning the provided servers in the response. +func newRegistryAPIServer(t *testing.T, servers []v0.ServerResponse) *httptest.Server { + t.Helper() + return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + resp := v0.ServerListResponse{ + Servers: servers, + Metadata: v0.Metadata{Count: len(servers)}, + } + w.Header().Set("Content-Type", "application/json") + err := json.NewEncoder(w).Encode(resp) + require.NoError(t, err) + })) +} + +// newErrorRegistryAPIServer creates an httptest.Server that returns the specified status code. +func newErrorRegistryAPIServer(statusCode int) *httptest.Server { + return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(statusCode) + })) +} + +// makeOCIServerResponse creates a v0.ServerResponse with an OCI package for the given image. +func makeOCIServerResponse(name, image string) v0.ServerResponse { + return v0.ServerResponse{ + Server: v0.ServerJSON{ + Name: name, + Packages: []model.Package{ + { + RegistryType: "oci", + Identifier: image, + Transport: model.Transport{ + Type: "stdio", + }, + }, + }, + }, + } +} + func TestRegistryEnforcingValidator_ValidateImage(t *testing.T) { t.Parallel() scheme := runtime.NewScheme() require.NoError(t, mcpv1alpha1.AddToScheme(scheme)) - require.NoError(t, corev1.AddToScheme(scheme)) ctx := context.Background() - // Test registry data - registryDataWithImage := `{ - "version": "1.0", - "servers": { - "test-server": { - "name": "test-server", - "image": "docker.io/toolhive/test:v1.0.0", - "description": "Test server" - }, - "another-server": { - "name": "another-server", - "image": "docker.io/toolhive/another:latest", - "description": "Another server" - } - }, - "groups": [ - { - "name": "group1", - "description": "Test group", - "servers": { - "group-server": { - "name": "group-server", - "image": "docker.io/toolhive/group:v2.0.0", - "description": "Group server" - } - } - } - ] - }` + // Registry API servers with test data + serversWithImage := []v0.ServerResponse{ + makeOCIServerResponse("io.toolhive/test-server", "docker.io/toolhive/test:v1.0.0"), + makeOCIServerResponse("io.toolhive/another-server", "docker.io/toolhive/another:latest"), + } - emptyRegistryData := `{ - "version": "1.0", - "servers": {} - }` + emptyServers := []v0.ServerResponse{} + + // registryServerData maps registry names to the server data they should return. + // Registries not in this map use serversWithImage by default. + registryServerData := map[string][]v0.ServerResponse{ + "empty-registry": emptyServers, + "enforcing-registry": emptyServers, + } tests := []struct { name string namespace string image string registries []runtime.Object - configMaps []runtime.Object + apiServers map[string]*httptest.Server // registry name -> test server expectedValid bool expectedError bool expectedErrorMsg string @@ -207,51 +228,11 @@ func TestRegistryEnforcingValidator_ValidateImage(t *testing.T) { }, Status: mcpv1alpha1.MCPRegistryStatus{ Phase: mcpv1alpha1.MCPRegistryPhaseReady, + // URL will be set from apiServers below }, }, }, - configMaps: []runtime.Object{ - &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-registry-registry-storage", - Namespace: "test-namespace", - }, - Data: map[string]string{ - "registry.json": registryDataWithImage, - }, - }, - }, - expectedValid: true, - }, - { - name: "enforcing registry with image in group - validation passes", - namespace: "test-namespace", - image: "docker.io/toolhive/group:v2.0.0", - registries: []runtime.Object{ - &mcpv1alpha1.MCPRegistry{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-registry", - Namespace: "test-namespace", - }, - Spec: mcpv1alpha1.MCPRegistrySpec{ - EnforceServers: true, - }, - Status: mcpv1alpha1.MCPRegistryStatus{ - Phase: mcpv1alpha1.MCPRegistryPhaseReady, - }, - }, - }, - configMaps: []runtime.Object{ - &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-registry-registry-storage", - Namespace: "test-namespace", - }, - Data: map[string]string{ - "registry.json": registryDataWithImage, - }, - }, - }, + apiServers: map[string]*httptest.Server{"test-registry": nil}, // placeholder, set in test expectedValid: true, }, { @@ -272,17 +253,7 @@ func TestRegistryEnforcingValidator_ValidateImage(t *testing.T) { }, }, }, - configMaps: []runtime.Object{ - &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-registry-registry-storage", - Namespace: "test-namespace", - }, - Data: map[string]string{ - "registry.json": registryDataWithImage, - }, - }, - }, + apiServers: map[string]*httptest.Server{"test-registry": nil}, expectedValid: false, expectedError: true, expectedErrorMsg: "not found in enforced registries", @@ -294,7 +265,7 @@ func TestRegistryEnforcingValidator_ValidateImage(t *testing.T) { registries: []runtime.Object{ &mcpv1alpha1.MCPRegistry{ ObjectMeta: metav1.ObjectMeta{ - Name: "test-registry", + Name: "empty-registry", Namespace: "test-namespace", }, Spec: mcpv1alpha1.MCPRegistrySpec{ @@ -305,17 +276,7 @@ func TestRegistryEnforcingValidator_ValidateImage(t *testing.T) { }, }, }, - configMaps: []runtime.Object{ - &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-registry-registry-storage", - Namespace: "test-namespace", - }, - Data: map[string]string{ - "registry.json": emptyRegistryData, - }, - }, - }, + apiServers: map[string]*httptest.Server{"empty-registry": nil}, expectedValid: false, expectedError: true, expectedErrorMsg: "not found in enforced registries", @@ -372,38 +333,19 @@ func TestRegistryEnforcingValidator_ValidateImage(t *testing.T) { }, }, }, - configMaps: []runtime.Object{ - &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: "enforcing-registry-registry-storage", - Namespace: "test-namespace", - }, - Data: map[string]string{ - "registry.json": emptyRegistryData, - }, - }, - &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: "non-enforcing-registry-registry-storage", - Namespace: "test-namespace", - }, - Data: map[string]string{ - "registry.json": registryDataWithImage, - }, - }, - }, + apiServers: map[string]*httptest.Server{"enforcing-registry": nil}, expectedValid: false, expectedError: true, expectedErrorMsg: "not found in enforced registries", }, { - name: "missing ConfigMap - enforcing registry without ConfigMap should fail", + name: "enforcing registry with no URL - skips that registry", namespace: "test-namespace", image: "docker.io/toolhive/test:v1.0.0", registries: []runtime.Object{ &mcpv1alpha1.MCPRegistry{ ObjectMeta: metav1.ObjectMeta{ - Name: "registry-without-configmap", + Name: "registry-no-url", Namespace: "test-namespace", }, Spec: mcpv1alpha1.MCPRegistrySpec{ @@ -411,83 +353,7 @@ func TestRegistryEnforcingValidator_ValidateImage(t *testing.T) { }, Status: mcpv1alpha1.MCPRegistryStatus{ Phase: mcpv1alpha1.MCPRegistryPhaseReady, - }, - }, - &mcpv1alpha1.MCPRegistry{ - ObjectMeta: metav1.ObjectMeta{ - Name: "registry-with-configmap", - Namespace: "test-namespace", - }, - Spec: mcpv1alpha1.MCPRegistrySpec{ - EnforceServers: false, - }, - Status: mcpv1alpha1.MCPRegistryStatus{ - Phase: mcpv1alpha1.MCPRegistryPhaseReady, - }, - }, - }, - configMaps: []runtime.Object{ - &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: "registry-with-configmap-registry-storage", - Namespace: "test-namespace", - }, - Data: map[string]string{ - "registry.json": registryDataWithImage, - }, - }, - }, - expectedValid: false, - expectedError: true, - expectedErrorMsg: "not found in enforced registries", - }, - { - name: "invalid JSON in ConfigMap - enforcing registry with invalid JSON should fail", - namespace: "test-namespace", - image: "docker.io/toolhive/test:v1.0.0", - registries: []runtime.Object{ - &mcpv1alpha1.MCPRegistry{ - ObjectMeta: metav1.ObjectMeta{ - Name: "registry-invalid-json", - Namespace: "test-namespace", - }, - Spec: mcpv1alpha1.MCPRegistrySpec{ - EnforceServers: true, - }, - Status: mcpv1alpha1.MCPRegistryStatus{ - Phase: mcpv1alpha1.MCPRegistryPhaseReady, - }, - }, - &mcpv1alpha1.MCPRegistry{ - ObjectMeta: metav1.ObjectMeta{ - Name: "registry-valid-json", - Namespace: "test-namespace", - }, - Spec: mcpv1alpha1.MCPRegistrySpec{ - EnforceServers: false, - }, - Status: mcpv1alpha1.MCPRegistryStatus{ - Phase: mcpv1alpha1.MCPRegistryPhaseReady, - }, - }, - }, - configMaps: []runtime.Object{ - &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: "registry-invalid-json-registry-storage", - Namespace: "test-namespace", - }, - Data: map[string]string{ - "registry.json": "not-valid-json{", - }, - }, - &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: "registry-valid-json-registry-storage", - Namespace: "test-namespace", - }, - Data: map[string]string{ - "registry.json": registryDataWithImage, + URL: "", // No URL set }, }, }, @@ -500,19 +366,49 @@ func TestRegistryEnforcingValidator_ValidateImage(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { t.Parallel() - // Build fake client with test objects - var objs []runtime.Object - objs = append(objs, tt.registries...) - objs = append(objs, tt.configMaps...) + // Create test API servers for registries that have apiServers entries + var testServers []*httptest.Server + for name, ts := range tt.apiServers { + if ts == nil { + // Look up data for this registry, defaulting to serversWithImage + data := serversWithImage + if d, ok := registryServerData[name]; ok { + data = d + } + ts = newRegistryAPIServer(t, data) + tt.apiServers[name] = ts + testServers = append(testServers, ts) + } + } + defer func() { + for _, ts := range testServers { + ts.Close() + } + }() + + // Set Status.URL on registries that have API servers + for i, obj := range tt.registries { + reg, ok := obj.(*mcpv1alpha1.MCPRegistry) + if !ok { + continue + } + if ts, exists := tt.apiServers[reg.Name]; exists { + reg.Status.URL = ts.URL + tt.registries[i] = reg + } + } + + // Build fake client with test objects fakeClient := fake.NewClientBuilder(). WithScheme(scheme). - WithRuntimeObjects(objs...). + WithRuntimeObjects(tt.registries...). Build() validator := &RegistryEnforcingValidator{ - client: fakeClient, - namespace: tt.namespace, + client: fakeClient, + namespace: tt.namespace, + httpClient: http.DefaultClient, } // Create empty metadata for test (original behavior) @@ -527,7 +423,7 @@ func TestRegistryEnforcingValidator_ValidateImage(t *testing.T) { } else { // Validation should fail if tt.expectedError { - assert.Error(t, err) + require.Error(t, err) if tt.expectedErrorMsg != "" { assert.Contains(t, err.Error(), tt.expectedErrorMsg) } @@ -544,27 +440,20 @@ func TestCheckImageInRegistry(t *testing.T) { scheme := runtime.NewScheme() require.NoError(t, mcpv1alpha1.AddToScheme(scheme)) - require.NoError(t, corev1.AddToScheme(scheme)) ctx := context.Background() - registryData := `{ - "version": "1.0", - "servers": { - "test-server": { - "name": "test-server", - "image": "docker.io/toolhive/test:v1.0.0", - "description": "Test server" - } - } - }` + servers := []v0.ServerResponse{ + makeOCIServerResponse("io.toolhive/test-server", "docker.io/toolhive/test:v1.0.0"), + } tests := []struct { name string mcpRegistry *mcpv1alpha1.MCPRegistry - configMap *corev1.ConfigMap + apiServer func(t *testing.T) *httptest.Server image string expectedFound bool + expectedError bool }{ { name: "registry not ready - returns false", @@ -581,7 +470,7 @@ func TestCheckImageInRegistry(t *testing.T) { expectedFound: false, }, { - name: "ConfigMap not found - returns false", + name: "empty URL - returns false", mcpRegistry: &mcpv1alpha1.MCPRegistry{ ObjectMeta: metav1.ObjectMeta{ Name: "test-registry", @@ -589,13 +478,14 @@ func TestCheckImageInRegistry(t *testing.T) { }, Status: mcpv1alpha1.MCPRegistryStatus{ Phase: mcpv1alpha1.MCPRegistryPhaseReady, + URL: "", }, }, image: "docker.io/toolhive/test:v1.0.0", expectedFound: false, }, { - name: "registry data not in ConfigMap - returns false", + name: "image found in registry - returns true", mcpRegistry: &mcpv1alpha1.MCPRegistry{ ObjectMeta: metav1.ObjectMeta{ Name: "test-registry", @@ -605,20 +495,15 @@ func TestCheckImageInRegistry(t *testing.T) { Phase: mcpv1alpha1.MCPRegistryPhaseReady, }, }, - configMap: &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-registry-registry-storage", - Namespace: "test-namespace", - }, - Data: map[string]string{ - "other-key": "some-data", - }, + apiServer: func(t *testing.T) *httptest.Server { + t.Helper() + return newRegistryAPIServer(t, servers) }, image: "docker.io/toolhive/test:v1.0.0", - expectedFound: false, + expectedFound: true, }, { - name: "image found in registry - returns true", + name: "image not found in registry - returns false", mcpRegistry: &mcpv1alpha1.MCPRegistry{ ObjectMeta: metav1.ObjectMeta{ Name: "test-registry", @@ -628,20 +513,15 @@ func TestCheckImageInRegistry(t *testing.T) { Phase: mcpv1alpha1.MCPRegistryPhaseReady, }, }, - configMap: &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-registry-registry-storage", - Namespace: "test-namespace", - }, - Data: map[string]string{ - "registry.json": registryData, - }, + apiServer: func(t *testing.T) *httptest.Server { + t.Helper() + return newRegistryAPIServer(t, servers) }, - image: "docker.io/toolhive/test:v1.0.0", - expectedFound: true, + image: "docker.io/toolhive/missing:v1.0.0", + expectedFound: false, }, { - name: "image not found in registry - returns false", + name: "API returns error - returns error", mcpRegistry: &mcpv1alpha1.MCPRegistry{ ObjectMeta: metav1.ObjectMeta{ Name: "test-registry", @@ -651,129 +531,143 @@ func TestCheckImageInRegistry(t *testing.T) { Phase: mcpv1alpha1.MCPRegistryPhaseReady, }, }, - configMap: &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-registry-registry-storage", - Namespace: "test-namespace", - }, - Data: map[string]string{ - "registry.json": registryData, - }, + apiServer: func(_ *testing.T) *httptest.Server { + return newErrorRegistryAPIServer(http.StatusInternalServerError) }, - image: "docker.io/toolhive/missing:v1.0.0", + image: "docker.io/toolhive/test:v1.0.0", expectedFound: false, + expectedError: true, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { t.Parallel() - var objs []runtime.Object - if tt.configMap != nil { - objs = append(objs, tt.configMap) + + // Set up API server if provided + if tt.apiServer != nil { + ts := tt.apiServer(t) + defer ts.Close() + tt.mcpRegistry.Status.URL = ts.URL } fakeClient := fake.NewClientBuilder(). WithScheme(scheme). - WithRuntimeObjects(objs...). Build() validator := &RegistryEnforcingValidator{ - client: fakeClient, - namespace: "test-namespace", + client: fakeClient, + namespace: "test-namespace", + httpClient: http.DefaultClient, } found, err := validator.checkImageInRegistry(ctx, tt.mcpRegistry, tt.image) - assert.NoError(t, err) + if tt.expectedError { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } assert.Equal(t, tt.expectedFound, found) }) } } -func TestFindImageInRegistry(t *testing.T) { +func TestFindImageInServers(t *testing.T) { t.Parallel() tests := []struct { name string - registry *regtypes.Registry + servers []v0.ServerResponse image string expected bool }{ { - name: "finds image in top-level servers", - registry: ®types.Registry{ - Servers: map[string]*regtypes.ImageMetadata{ - "server1": { - Image: "docker.io/toolhive/test:v1.0.0", - }, - "server2": { - Image: "docker.io/toolhive/other:v2.0.0", - }, - }, + name: "finds image in OCI package", + servers: []v0.ServerResponse{ + makeOCIServerResponse("io.toolhive/server1", "docker.io/toolhive/test:v1.0.0"), + makeOCIServerResponse("io.toolhive/server2", "docker.io/toolhive/other:v2.0.0"), }, image: "docker.io/toolhive/test:v1.0.0", expected: true, }, { - name: "finds image in group servers", - registry: ®types.Registry{ - Servers: map[string]*regtypes.ImageMetadata{}, - Groups: []*regtypes.Group{ - { - Name: "group1", - Servers: map[string]*regtypes.ImageMetadata{ - "group-server": { - Image: "docker.io/toolhive/group:v1.0.0", - }, - }, - }, - }, + name: "does not find missing image", + servers: []v0.ServerResponse{ + makeOCIServerResponse("io.toolhive/server1", "docker.io/toolhive/test:v1.0.0"), }, - image: "docker.io/toolhive/group:v1.0.0", - expected: true, + image: "docker.io/toolhive/missing:v1.0.0", + expected: false, }, { - name: "does not find missing image", - registry: ®types.Registry{ - Servers: map[string]*regtypes.ImageMetadata{ - "server1": { - Image: "docker.io/toolhive/test:v1.0.0", - }, - }, - Groups: []*regtypes.Group{ - { - Name: "group1", - Servers: map[string]*regtypes.ImageMetadata{ - "group-server": { - Image: "docker.io/toolhive/group:v1.0.0", + name: "handles empty server list", + servers: []v0.ServerResponse{}, + image: "docker.io/toolhive/test:v1.0.0", + expected: false, + }, + { + name: "handles nil server list", + servers: nil, + image: "docker.io/toolhive/test:v1.0.0", + expected: false, + }, + { + name: "ignores non-OCI packages", + servers: []v0.ServerResponse{ + { + Server: v0.ServerJSON{ + Name: "io.toolhive/npm-server", + Packages: []model.Package{ + { + RegistryType: "npm", + Identifier: "docker.io/toolhive/test:v1.0.0", }, }, }, }, }, - image: "docker.io/toolhive/missing:v1.0.0", + image: "docker.io/toolhive/test:v1.0.0", expected: false, }, { - name: "handles empty registry", - registry: ®types.Registry{ - Servers: map[string]*regtypes.ImageMetadata{}, + name: "handles server with no packages", + servers: []v0.ServerResponse{ + { + Server: v0.ServerJSON{ + Name: "io.toolhive/no-packages", + }, + }, }, image: "docker.io/toolhive/test:v1.0.0", expected: false, }, { - name: "handles nil maps", - registry: ®types.Registry{}, + name: "finds image among multiple packages", + servers: []v0.ServerResponse{ + { + Server: v0.ServerJSON{ + Name: "io.toolhive/multi-pkg", + Packages: []model.Package{ + { + RegistryType: "npm", + Identifier: "@toolhive/server", + }, + { + RegistryType: "oci", + Identifier: "docker.io/toolhive/test:v1.0.0", + }, + }, + }, + }, + }, image: "docker.io/toolhive/test:v1.0.0", - expected: false, + expected: true, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { t.Parallel() - result := findImageInRegistry(tt.registry, tt.image) + result := findImageInServers(tt.servers, tt.image) assert.Equal(t, tt.expected, result) }) } @@ -784,21 +678,14 @@ func TestRegistryEnforcingValidator_ValidateImageWithRegistryLabel(t *testing.T) scheme := runtime.NewScheme() require.NoError(t, mcpv1alpha1.AddToScheme(scheme)) - require.NoError(t, corev1.AddToScheme(scheme)) ctx := context.Background() - // Test registry data - registryDataWithImage := `{ - "version": "1.0", - "servers": { - "test-server": { - "name": "test-server", - "image": "docker.io/toolhive/test:v1.0.0", - "description": "Test server" - } - } - }` + serversWithImage := []v0.ServerResponse{ + makeOCIServerResponse("io.toolhive/test-server", "docker.io/toolhive/test:v1.0.0"), + } + + emptyServers := []v0.ServerResponse{} tests := []struct { name string @@ -806,7 +693,7 @@ func TestRegistryEnforcingValidator_ValidateImageWithRegistryLabel(t *testing.T) image string metadata metav1.ObjectMeta registries []runtime.Object - configMaps []runtime.Object + apiServers map[string]*httptest.Server expectedValid bool expectedError bool expectedErrorMsg string @@ -846,17 +733,7 @@ func TestRegistryEnforcingValidator_ValidateImageWithRegistryLabel(t *testing.T) }, }, }, - configMaps: []runtime.Object{ - &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: "target-registry-registry-storage", - Namespace: "test-namespace", - }, - Data: map[string]string{ - "registry.json": registryDataWithImage, - }, - }, - }, + apiServers: map[string]*httptest.Server{"target-registry": nil}, expectedValid: true, }, { @@ -907,17 +784,7 @@ func TestRegistryEnforcingValidator_ValidateImageWithRegistryLabel(t *testing.T) }, }, }, - configMaps: []runtime.Object{ - &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: "target-registry-registry-storage", - Namespace: "test-namespace", - }, - Data: map[string]string{ - "registry.json": registryDataWithImage, - }, - }, - }, + apiServers: map[string]*httptest.Server{"target-registry": nil}, expectedValid: false, expectedError: true, expectedErrorMsg: "not found in specified registry", @@ -984,26 +851,7 @@ func TestRegistryEnforcingValidator_ValidateImageWithRegistryLabel(t *testing.T) }, }, }, - configMaps: []runtime.Object{ - &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: "empty-registry-registry-storage", - Namespace: "test-namespace", - }, - Data: map[string]string{ - "registry.json": `{"version": "1.0", "servers": {}}`, - }, - }, - &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: "registry-with-image-registry-storage", - Namespace: "test-namespace", - }, - Data: map[string]string{ - "registry.json": registryDataWithImage, - }, - }, - }, + apiServers: map[string]*httptest.Server{"empty-registry": nil, "registry-with-image": nil}, expectedValid: false, expectedError: true, expectedErrorMsg: "not found in specified registry \"empty-registry\"", @@ -1039,26 +887,7 @@ func TestRegistryEnforcingValidator_ValidateImageWithRegistryLabel(t *testing.T) }, }, }, - configMaps: []runtime.Object{ - &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: "registry1-registry-storage", - Namespace: "test-namespace", - }, - Data: map[string]string{ - "registry.json": `{"version": "1.0", "servers": {}}`, - }, - }, - &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: "registry2-registry-storage", - Namespace: "test-namespace", - }, - Data: map[string]string{ - "registry.json": registryDataWithImage, - }, - }, - }, + apiServers: map[string]*httptest.Server{"registry1": nil, "registry2": nil}, expectedValid: true, }, } @@ -1066,19 +895,48 @@ func TestRegistryEnforcingValidator_ValidateImageWithRegistryLabel(t *testing.T) for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { t.Parallel() - // Build fake client with test objects - var objs []runtime.Object - objs = append(objs, tt.registries...) - objs = append(objs, tt.configMaps...) + // Create test API servers + var testServers []*httptest.Server + for name, ts := range tt.apiServers { + if ts == nil { + data := serversWithImage + if name == "empty-registry" { + data = emptyServers + } + ts = newRegistryAPIServer(t, data) + tt.apiServers[name] = ts + testServers = append(testServers, ts) + } + } + defer func() { + for _, ts := range testServers { + ts.Close() + } + }() + + // Set Status.URL on registries that have API servers + for i, obj := range tt.registries { + reg, ok := obj.(*mcpv1alpha1.MCPRegistry) + if !ok { + continue + } + if ts, exists := tt.apiServers[reg.Name]; exists { + reg.Status.URL = ts.URL + tt.registries[i] = reg + } + } + + // Build fake client with test objects fakeClient := fake.NewClientBuilder(). WithScheme(scheme). - WithRuntimeObjects(objs...). + WithRuntimeObjects(tt.registries...). Build() validator := &RegistryEnforcingValidator{ - client: fakeClient, - namespace: tt.namespace, + client: fakeClient, + namespace: tt.namespace, + httpClient: http.DefaultClient, } err := validator.ValidateImage(ctx, tt.image, tt.metadata) @@ -1091,7 +949,7 @@ func TestRegistryEnforcingValidator_ValidateImageWithRegistryLabel(t *testing.T) } else { // Validation should fail if tt.expectedError { - assert.Error(t, err) + require.Error(t, err) if tt.expectedErrorMsg != "" { assert.Contains(t, err.Error(), tt.expectedErrorMsg) }