Skip to content

Commit 86c4f20

Browse files
yroblataskbotCopilot
authored
feat: only isolate container network when desired (#762)
Co-authored-by: taskbot <[email protected]> Co-authored-by: Copilot <[email protected]>
1 parent 5012f59 commit 86c4f20

File tree

14 files changed

+58
-37
lines changed

14 files changed

+58
-37
lines changed

cmd/thv/app/inspector.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,7 @@ func inspectorCmdFunc(cmd *cobra.Command, args []string) error {
114114
&permissions.Profile{}, // Empty profile as we don't need special permissions
115115
string(types.TransportTypeInspector),
116116
options,
117+
false, // Do not isolate network
117118
)
118119
if err != nil {
119120
return fmt.Errorf("failed to create inspector container: %v", err)

cmd/thv/app/run.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,9 @@ var (
9494
runOtelHeaders []string
9595
runOtelInsecure bool
9696
runOtelEnablePrometheusMetricsPath bool
97+
98+
// Network isolation flag
99+
runIsolateNetwork bool
97100
)
98101

99102
func init() {
@@ -192,6 +195,9 @@ func init() {
192195
"Disable TLS verification for OpenTelemetry endpoint")
193196
runCmd.Flags().BoolVar(&runOtelEnablePrometheusMetricsPath, "otel-enable-prometheus-metrics-path", false,
194197
"Enable Prometheus-style /metrics endpoint on the main transport port")
198+
runCmd.Flags().BoolVar(&runIsolateNetwork, "isolate-network", false,
199+
"Isolate the container network from the host (default: false)")
200+
195201
}
196202

197203
func runCmdFunc(cmd *cobra.Command, args []string) error {
@@ -252,6 +258,7 @@ func runCmdFunc(cmd *cobra.Command, args []string) error {
252258
runOtelHeaders,
253259
runOtelInsecure,
254260
runOtelEnablePrometheusMetricsPath,
261+
runIsolateNetwork,
255262
)
256263

257264
// Set the Kubernetes pod template patch if provided

docs/cli/thv_run.md

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/api/v1/workloads.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,7 @@ func (s *WorkloadRoutes) createWorkload(w http.ResponseWriter, r *http.Request)
258258
nil, // otelHeaders - not exposed through API yet
259259
false, // otelInsecure - not exposed through API yet
260260
false, // otelEnablePrometheusMetricsPath - not exposed through API yet
261+
false, // isolateNetwork - not exposed through API yet
261262
)
262263

263264
// TODO: De-dupe from `configureRunConfig` in `cmd/thv/app/run_common.go`.

pkg/container/docker/client.go

Lines changed: 28 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -628,28 +628,10 @@ func (c *Client) createIngressContainer(ctx context.Context, containerName strin
628628
return egressContainerId, nil
629629
}
630630

631-
func (c *Client) createContainerNetworks(ctx context.Context, internalNetworkName string, externalNetworkName string) error {
632-
internalNetworkLabels := map[string]string{}
633-
lb.AddNetworkLabels(internalNetworkLabels, internalNetworkName)
634-
err := c.createNetwork(ctx, internalNetworkName, internalNetworkLabels, true)
635-
if err != nil {
636-
return fmt.Errorf("failed to create internal network: %v", err)
637-
}
638-
639-
externalNetworkLabels := map[string]string{}
640-
lb.AddNetworkLabels(externalNetworkLabels, externalNetworkName)
641-
err = c.createNetwork(ctx, externalNetworkName, externalNetworkLabels, false)
642-
if err != nil {
643-
// just log the error and continue
644-
logger.Warnf("failed to create external network %q: %v", externalNetworkName, err)
645-
}
646-
return nil
647-
}
648-
649631
func (c *Client) createMcpContainer(ctx context.Context, name string, networkName string, image string, command []string,
650632
envVars map[string]string, labels map[string]string, attachStdio bool, permissionConfig *runtime.PermissionConfig,
651633
additionalDNS string, exposedPorts map[string]struct{}, portBindings map[string][]runtime.PortBinding,
652-
isMcpWorkload bool) (string, error) {
634+
isolateNetwork bool) (string, error) {
653635
// Create container configuration
654636
config := &container.Config{
655637
Image: image,
@@ -690,11 +672,10 @@ func (c *Client) createMcpContainer(ctx context.Context, name string, networkNam
690672
}
691673

692674
// create mcp container
693-
internalEndpointsConfig := map[string]*network.EndpointSettings{
694-
networkName: {},
695-
}
696-
697-
if !isMcpWorkload {
675+
internalEndpointsConfig := map[string]*network.EndpointSettings{}
676+
if isolateNetwork {
677+
internalEndpointsConfig[networkName] = &network.EndpointSettings{}
678+
} else {
698679
// for other workloads such as inspector, add to external network
699680
internalEndpointsConfig["toolhive-external"] = &network.EndpointSettings{}
700681
}
@@ -735,9 +716,8 @@ func (c *Client) DeployWorkload(
735716
permissionProfile *permissions.Profile,
736717
transportType string,
737718
options *runtime.DeployWorkloadOptions,
719+
isolateNetwork bool,
738720
) (string, error) {
739-
// check if we are an mcp workload
740-
isMcpWorkload := name != "inspector"
741721
// Get permission config from profile
742722
permissionConfig, err := c.getPermissionConfigFromProfile(permissionProfile, transportType)
743723
if err != nil {
@@ -748,17 +728,29 @@ func (c *Client) DeployWorkload(
748728
attachStdio := options == nil || options.AttachStdio
749729

750730
// create networks
751-
networkName := fmt.Sprintf("toolhive-%s-internal", name)
752-
err = c.createContainerNetworks(ctx, networkName, "toolhive-external")
753-
if err != nil {
754-
return "", fmt.Errorf("failed to create container networks: %v", err)
755-
}
756731
var additionalDNS string
732+
networkName := fmt.Sprintf("toolhive-%s-internal", name)
733+
757734
externalEndpointsConfig := map[string]*network.EndpointSettings{
758735
networkName: {},
759736
"toolhive-external": {},
760737
}
761-
if isMcpWorkload {
738+
externalNetworkLabels := map[string]string{}
739+
lb.AddNetworkLabels(externalNetworkLabels, "toolhive-external")
740+
err = c.createNetwork(ctx, "toolhive-external", externalNetworkLabels, false)
741+
if err != nil {
742+
// just log the error and continue
743+
logger.Warnf("failed to create external network %q: %v", "toolhive-external", err)
744+
}
745+
746+
if isolateNetwork {
747+
internalNetworkLabels := map[string]string{}
748+
lb.AddNetworkLabels(internalNetworkLabels, networkName)
749+
err := c.createNetwork(ctx, networkName, internalNetworkLabels, true)
750+
if err != nil {
751+
return "", fmt.Errorf("failed to create internal network: %v", err)
752+
}
753+
762754
// create dns container
763755
dnsContainerName := fmt.Sprintf("%s-dns", name)
764756
_, dnsContainerIP, err := c.createDnsContainer(ctx, dnsContainerName, attachStdio, networkName, externalEndpointsConfig)
@@ -781,16 +773,18 @@ func (c *Client) DeployWorkload(
781773
}
782774

783775
envVars = addExtraEnvVars(envVars, egressContainerName)
776+
} else {
777+
networkName = ""
784778
}
785779

786780
containerId, err := c.createMcpContainer(ctx, name, networkName, image, command, envVars, labels, attachStdio,
787-
permissionConfig, additionalDNS, options.ExposedPorts, options.PortBindings, isMcpWorkload)
781+
permissionConfig, additionalDNS, options.ExposedPorts, options.PortBindings, isolateNetwork)
788782
if err != nil {
789783
return "", fmt.Errorf("failed to create mcp container: %v", err)
790784
}
791785

792786
// now create ingress container
793-
if isMcpWorkload {
787+
if isolateNetwork {
794788
ingressContainerName := fmt.Sprintf("%s-ingress", name)
795789
_, err = c.createIngressContainer(ctx, name, ingressContainerName, attachStdio, options.PortBindings,
796790
options.ExposedPorts, externalEndpointsConfig)

pkg/container/kubernetes/client.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,8 @@ func (c *Client) DeployWorkload(ctx context.Context,
259259
containerLabels map[string]string,
260260
_ *permissions.Profile, // TODO: Implement permission profile support for Kubernetes
261261
transportType string,
262-
options *runtime.DeployWorkloadOptions) (string, error) {
262+
options *runtime.DeployWorkloadOptions,
263+
_ bool) (string, error) {
263264
namespace := getCurrentNamespace()
264265
containerLabels["app"] = containerName
265266
containerLabels["toolhive"] = "true"

pkg/container/kubernetes/client_test.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,7 @@ func TestCreateContainerWithPodTemplatePatch(t *testing.T) {
181181
nil,
182182
"stdio",
183183
options,
184+
false,
184185
)
185186

186187
// Check that there was no error
@@ -676,6 +677,7 @@ func TestCreateContainerWithMCP(t *testing.T) {
676677
nil,
677678
tc.transportType,
678679
tc.options,
680+
false,
679681
)
680682

681683
// Check that there was no error

pkg/container/runtime/types.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ type Runtime interface {
8181
permissionProfile *permissions.Profile,
8282
transportType string,
8383
options *DeployWorkloadOptions,
84+
isolateNetwork bool,
8485
) (string, error)
8586

8687
// ListWorkloads lists all deployed workloads managed by this runtime.

pkg/runner/config.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,9 @@ type RunConfig struct {
104104

105105
// Runtime is the container runtime to use (not serialized)
106106
Runtime rt.Runtime `json:"-" yaml:"-"`
107+
108+
// IsolateNetwork indicates whether to isolate the network for the container
109+
IsolateNetwork bool `json:"isolate_network,omitempty" yaml:"isolate_network,omitempty"`
107110
}
108111

109112
// WriteJSON serializes the RunConfig to JSON and writes it to the provided writer
@@ -155,6 +158,7 @@ func NewRunConfigFromFlags(
155158
otelHeaders []string,
156159
otelInsecure bool,
157160
otelEnablePrometheusMetricsPath bool,
161+
isolateNetwork bool,
158162
) *RunConfig {
159163
// Ensure default values for host and targetHost
160164
if host == "" {
@@ -177,6 +181,7 @@ func NewRunConfigFromFlags(
177181
ContainerLabels: make(map[string]string),
178182
EnvVars: make(map[string]string),
179183
Host: host,
184+
IsolateNetwork: isolateNetwork,
180185
}
181186

182187
// If enable audit is true and no audit config path is provided, use default config

pkg/runner/config_test.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ func (*mockRuntime) DeployWorkload(
3131
_ *permissions.Profile,
3232
_ string,
3333
_ *rt.DeployWorkloadOptions,
34+
_ bool,
3435
) (string, error) {
3536
return "container-id", nil
3637
}
@@ -879,6 +880,7 @@ func TestNewRunConfigFromFlags(t *testing.T) {
879880
nil, // otelHeaders
880881
false, // otelInsecure
881882
false, // otelEnablePrometheusMetricsPath
883+
false, // isolatedNetwork
882884
)
883885

884886
assert.NotNil(t, config, "NewRunConfigFromFlags should return a non-nil config")

0 commit comments

Comments
 (0)