Skip to content

Commit 321b520

Browse files
committed
Fix support for remote registry entries in prerunner
Signed-off-by: Juan Antonio Osorio <ozz@stacklok.com>
1 parent b68de41 commit 321b520

File tree

2 files changed

+167
-4
lines changed

2 files changed

+167
-4
lines changed

pkg/runner/prerun_transformer.go

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -188,17 +188,21 @@ func (t *PreRunTransformer) transformFromRegistry(
188188

189189
if server.IsRemote() {
190190
// Remote server from registry
191-
logger.Infof("Found remote server in registry: %s", src.ServerName)
191+
remoteServerMetadata, ok := server.(*registry.RemoteServerMetadata)
192+
if !ok {
193+
return nil, fmt.Errorf("server marked as remote but is not RemoteServerMetadata type")
194+
}
195+
logger.Infof("Found remote server in registry: %s -> %s", src.ServerName, remoteServerMetadata.URL)
192196
return t.buildRunConfigFromSource(
193-
src.ServerName, // imageURL (server name)
194-
server, // serverMetadata
197+
"", // imageURL (empty for remote servers - no Docker image)
198+
server, // serverMetadata
195199
runFlags,
196200
cmdArgs,
197201
debugMode,
198202
validatedHost,
199203
oidcConfig,
200204
telemetryConfig,
201-
"", // remoteURL (will be set from metadata)
205+
remoteServerMetadata.URL, // remoteURL from registry metadata
202206
)
203207
}
204208

Lines changed: 159 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,159 @@
1+
package runner
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/assert"
7+
"github.com/stretchr/testify/require"
8+
9+
"github.com/stacklok/toolhive/pkg/registry"
10+
)
11+
12+
func TestRemoteServerMetadata_IsRemote(t *testing.T) {
13+
t.Parallel()
14+
15+
// Test that RemoteServerMetadata correctly identifies as remote
16+
remoteServer := &registry.RemoteServerMetadata{
17+
BaseServerMetadata: registry.BaseServerMetadata{
18+
Name: "test-remote-server",
19+
Description: "Test remote server",
20+
Transport: "sse",
21+
Tools: []string{"test-tool"},
22+
Tier: "test",
23+
Status: "active",
24+
},
25+
URL: "https://example.com/mcp",
26+
}
27+
28+
assert.True(t, remoteServer.IsRemote(), "RemoteServerMetadata should identify as remote")
29+
assert.Equal(t, "https://example.com/mcp", remoteServer.URL)
30+
assert.Equal(t, "test-remote-server", remoteServer.GetName())
31+
assert.Equal(t, "sse", remoteServer.GetTransport())
32+
}
33+
34+
func TestImageMetadata_IsRemote(t *testing.T) {
35+
t.Parallel()
36+
37+
// Test that ImageMetadata correctly identifies as not remote
38+
containerServer := &registry.ImageMetadata{
39+
BaseServerMetadata: registry.BaseServerMetadata{
40+
Name: "test-container-server",
41+
Description: "Test container server",
42+
Transport: "stdio",
43+
Tools: []string{"test-tool"},
44+
Tier: "test",
45+
Status: "active",
46+
},
47+
Image: "docker.io/test/server:latest",
48+
}
49+
50+
assert.False(t, containerServer.IsRemote(), "ImageMetadata should not identify as remote")
51+
assert.Equal(t, "docker.io/test/server:latest", containerServer.Image)
52+
assert.Equal(t, "test-container-server", containerServer.GetName())
53+
assert.Equal(t, "stdio", containerServer.GetTransport())
54+
}
55+
56+
func TestPreRunConfigType_Constants(t *testing.T) {
57+
t.Parallel()
58+
59+
// Test that the PreRunConfigType constants are defined correctly
60+
assert.Equal(t, PreRunConfigType("registry"), PreRunConfigTypeRegistry)
61+
assert.Equal(t, PreRunConfigType("container_image"), PreRunConfigTypeContainerImage)
62+
assert.Equal(t, PreRunConfigType("protocol_scheme"), PreRunConfigTypeProtocolScheme)
63+
assert.Equal(t, PreRunConfigType("remote_url"), PreRunConfigTypeRemoteURL)
64+
assert.Equal(t, PreRunConfigType("config_file"), PreRunConfigTypeConfigFile)
65+
}
66+
67+
func TestRegistrySource_Structure(t *testing.T) {
68+
t.Parallel()
69+
70+
// Test the RegistrySource structure
71+
registrySource := &RegistrySource{
72+
ServerName: "test-server",
73+
IsRemote: true,
74+
}
75+
76+
assert.Equal(t, "test-server", registrySource.ServerName)
77+
assert.True(t, registrySource.IsRemote)
78+
79+
// Test with container server
80+
containerSource := &RegistrySource{
81+
ServerName: "container-server",
82+
IsRemote: false,
83+
}
84+
85+
assert.Equal(t, "container-server", containerSource.ServerName)
86+
assert.False(t, containerSource.IsRemote)
87+
}
88+
89+
func TestPreRunConfig_Structure(t *testing.T) {
90+
t.Parallel()
91+
92+
// Test PreRunConfig structure for registry source
93+
preConfig := &PreRunConfig{
94+
Type: PreRunConfigTypeRegistry,
95+
Source: "test-server",
96+
ParsedSource: &RegistrySource{
97+
ServerName: "test-server",
98+
IsRemote: true,
99+
},
100+
Metadata: map[string]interface{}{
101+
"discovered": true,
102+
},
103+
}
104+
105+
assert.Equal(t, PreRunConfigTypeRegistry, preConfig.Type)
106+
assert.Equal(t, "test-server", preConfig.Source)
107+
108+
registrySource, ok := preConfig.ParsedSource.(*RegistrySource)
109+
require.True(t, ok, "ParsedSource should be a RegistrySource")
110+
assert.Equal(t, "test-server", registrySource.ServerName)
111+
assert.True(t, registrySource.IsRemote)
112+
113+
assert.Equal(t, true, preConfig.Metadata["discovered"])
114+
}
115+
116+
// Test the type assertion logic that was fixed in the transformer
117+
func TestRemoteServerTypeAssertion(t *testing.T) {
118+
t.Parallel()
119+
120+
// Test successful type assertion
121+
var serverMetadata registry.ServerMetadata = &registry.RemoteServerMetadata{
122+
BaseServerMetadata: registry.BaseServerMetadata{
123+
Name: "remote-server",
124+
},
125+
URL: "https://example.com/mcp",
126+
}
127+
128+
if serverMetadata.IsRemote() {
129+
remoteServer, ok := serverMetadata.(*registry.RemoteServerMetadata)
130+
require.True(t, ok, "Should be able to cast to RemoteServerMetadata")
131+
assert.Equal(t, "https://example.com/mcp", remoteServer.URL)
132+
}
133+
134+
// Test failed type assertion (this is what our fix handles)
135+
var invalidRemoteServer registry.ServerMetadata = &mockInvalidRemoteServer{}
136+
137+
if invalidRemoteServer.IsRemote() {
138+
_, ok := invalidRemoteServer.(*registry.RemoteServerMetadata)
139+
assert.False(t, ok, "Should not be able to cast invalid server to RemoteServerMetadata")
140+
// This is the error case our fix handles
141+
}
142+
}
143+
144+
// Mock server that claims to be remote but isn't RemoteServerMetadata
145+
// This tests the error case we fixed in the transformer
146+
type mockInvalidRemoteServer struct{}
147+
148+
func (*mockInvalidRemoteServer) GetName() string { return "invalid-remote" }
149+
func (*mockInvalidRemoteServer) GetDescription() string { return "Invalid remote server" }
150+
func (*mockInvalidRemoteServer) GetTier() string { return "test" }
151+
func (*mockInvalidRemoteServer) GetStatus() string { return "active" }
152+
func (*mockInvalidRemoteServer) GetTransport() string { return "sse" }
153+
func (*mockInvalidRemoteServer) GetTools() []string { return []string{"test-tool"} }
154+
func (*mockInvalidRemoteServer) GetMetadata() *registry.Metadata { return nil }
155+
func (*mockInvalidRemoteServer) GetRepositoryURL() string { return "" }
156+
func (*mockInvalidRemoteServer) GetTags() []string { return nil }
157+
func (*mockInvalidRemoteServer) GetCustomMetadata() map[string]any { return nil }
158+
func (*mockInvalidRemoteServer) IsRemote() bool { return true } // Claims to be remote
159+
func (*mockInvalidRemoteServer) GetEnvVars() []*registry.EnvVar { return nil }

0 commit comments

Comments
 (0)