Skip to content

Commit 106ea35

Browse files
yroblataskbotCopilot
authored
feat: use different port for ingress proxy (#783)
Co-authored-by: taskbot <[email protected]> Co-authored-by: Copilot <[email protected]>
1 parent 7b55692 commit 106ea35

File tree

10 files changed

+108
-58
lines changed

10 files changed

+108
-58
lines changed

‎cmd/thv/app/inspector.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ func inspectorCmdFunc(cmd *cobra.Command, args []string) error {
106106

107107
labelsMap := map[string]string{}
108108
labels.AddStandardLabels(labelsMap, "inspector", "inspector", string(types.TransportTypeInspector), inspectorUIPort)
109-
_, err = rt.DeployWorkload(
109+
_, _, err = rt.DeployWorkload(
110110
ctx,
111111
processedImage,
112112
"inspector",

‎pkg/container/docker/client.go

Lines changed: 65 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"io"
1010
"os"
1111
"path/filepath"
12+
"strconv"
1213
"strings"
1314
"time"
1415

@@ -26,6 +27,7 @@ import (
2627
"github.com/stacklok/toolhive/pkg/container/runtime"
2728
lb "github.com/stacklok/toolhive/pkg/labels"
2829
"github.com/stacklok/toolhive/pkg/logger"
30+
"github.com/stacklok/toolhive/pkg/networking"
2931
"github.com/stacklok/toolhive/pkg/permissions"
3032
)
3133

@@ -320,6 +322,43 @@ func addEgressEnvVars(envVars map[string]string, egressContainerName string) map
320322
return envVars
321323
}
322324

325+
func (c *Client) createIngressContainer(ctx context.Context, containerName string, upstreamPort int, attachStdio bool,
326+
externalEndpointsConfig map[string]*network.EndpointSettings) (int, error) {
327+
squidPort, err := networking.FindOrUsePort(upstreamPort + 1)
328+
if err != nil {
329+
return 0, fmt.Errorf("failed to find or use port %d: %v", squidPort, err)
330+
}
331+
squidExposedPorts := map[string]struct{}{
332+
fmt.Sprintf("%d/tcp", squidPort): {},
333+
}
334+
squidPortBindings := map[string][]runtime.PortBinding{
335+
fmt.Sprintf("%d/tcp", squidPort): {
336+
{
337+
HostIP: "127.0.0.1",
338+
HostPort: fmt.Sprintf("%d", squidPort),
339+
},
340+
},
341+
}
342+
ingressContainerName := fmt.Sprintf("%s-ingress", containerName)
343+
_, err = createIngressSquidContainer(
344+
ctx,
345+
c,
346+
containerName,
347+
ingressContainerName,
348+
attachStdio,
349+
upstreamPort,
350+
squidPort,
351+
squidExposedPorts,
352+
externalEndpointsConfig,
353+
squidPortBindings,
354+
)
355+
if err != nil {
356+
return 0, fmt.Errorf("failed to create ingress container: %v", err)
357+
}
358+
return squidPort, nil
359+
360+
}
361+
323362
// DeployWorkload creates and starts a workload.
324363
// It configures the workload based on the provided permission profile and transport type.
325364
// If options is nil, default options will be used.
@@ -334,11 +373,11 @@ func (c *Client) DeployWorkload(
334373
transportType string,
335374
options *runtime.DeployWorkloadOptions,
336375
isolateNetwork bool,
337-
) (string, error) {
376+
) (string, int, error) {
338377
// Get permission config from profile
339378
permissionConfig, err := c.getPermissionConfigFromProfile(permissionProfile, transportType)
340379
if err != nil {
341-
return "", fmt.Errorf("failed to get permission config: %w", err)
380+
return "", 0, fmt.Errorf("failed to get permission config: %w", err)
342381
}
343382

344383
// Determine if we should attach stdio
@@ -365,7 +404,7 @@ func (c *Client) DeployWorkload(
365404
lb.AddNetworkLabels(internalNetworkLabels, networkName)
366405
err := c.createNetwork(ctx, networkName, internalNetworkLabels, true)
367406
if err != nil {
368-
return "", fmt.Errorf("failed to create internal network: %v", err)
407+
return "", 0, fmt.Errorf("failed to create internal network: %v", err)
369408
}
370409

371410
// create dns container
@@ -375,26 +414,23 @@ func (c *Client) DeployWorkload(
375414
additionalDNS = dnsContainerIP
376415
}
377416
if err != nil {
378-
return "", fmt.Errorf("failed to create dns container: %v", err)
417+
return "", 0, fmt.Errorf("failed to create dns container: %v", err)
379418
}
380419

381420
// create egress container
382421
egressContainerName := fmt.Sprintf("%s-egress", name)
383-
egressExposedPorts := map[string]struct{}{
384-
"3128/tcp": {},
385-
}
386422
_, err = createEgressSquidContainer(
387423
ctx,
388424
c,
389425
name,
390426
egressContainerName,
391427
attachStdio,
392-
egressExposedPorts,
428+
nil,
393429
externalEndpointsConfig,
394430
permissionProfile.Network,
395431
)
396432
if err != nil {
397-
return "", fmt.Errorf("failed to create egress container: %v", err)
433+
return "", 0, fmt.Errorf("failed to create egress container: %v", err)
398434
}
399435

400436
envVars = addEgressEnvVars(envVars, egressContainerName)
@@ -418,28 +454,34 @@ func (c *Client) DeployWorkload(
418454
isolateNetwork,
419455
)
420456
if err != nil {
421-
return "", fmt.Errorf("failed to create mcp container: %v", err)
457+
return "", 0, fmt.Errorf("failed to create mcp container: %v", err)
422458
}
423459

424460
// now create ingress container
461+
var firstPort string
462+
if len(options.ExposedPorts) == 0 {
463+
return "", 0, fmt.Errorf("no exposed ports specified in options.ExposedPorts")
464+
}
465+
for port := range options.ExposedPorts {
466+
firstPort = port
467+
468+
// need to strip the protocol
469+
firstPort = strings.Split(firstPort, "/")[0]
470+
break // take only the first one
471+
}
472+
firstPortInt, err := strconv.Atoi(firstPort)
473+
if err != nil {
474+
return "", 0, fmt.Errorf("failed to convert port %s to int: %v", firstPort, err)
475+
}
476+
425477
if isolateNetwork {
426-
ingressContainerName := fmt.Sprintf("%s-ingress", name)
427-
_, err = createIngressSquidContainer(
428-
ctx,
429-
c,
430-
name,
431-
ingressContainerName,
432-
attachStdio,
433-
options.ExposedPorts,
434-
externalEndpointsConfig,
435-
options.PortBindings,
436-
)
478+
firstPortInt, err = c.createIngressContainer(ctx, name, firstPortInt, attachStdio, externalEndpointsConfig)
437479
if err != nil {
438-
return "", fmt.Errorf("failed to create ingress container: %v", err)
480+
return "", 0, fmt.Errorf("failed to create ingress container: %v", err)
439481
}
440482
}
441483

442-
return containerId, nil
484+
return containerId, firstPortInt, nil
443485
}
444486

445487
// ListWorkloads lists workloads

‎pkg/container/docker/squid.go

Lines changed: 21 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,13 @@ func createIngressSquidContainer(
2525
containerName string,
2626
squidContainerName string,
2727
attachStdio bool,
28+
upstreamPort int,
29+
squidPort int,
2830
exposedPorts map[string]struct{},
2931
endpointsConfig map[string]*network.EndpointSettings,
3032
portBindings map[string][]runtime.PortBinding,
3133
) (string, error) {
32-
squidConfPath, err := createTempIngressSquidConf(containerName, exposedPorts)
34+
squidConfPath, err := createTempIngressSquidConf(containerName, upstreamPort, squidPort)
3335
if err != nil {
3436
return "", fmt.Errorf("failed to create temporary squid.conf: %v", err)
3537
}
@@ -259,13 +261,13 @@ func getSquidImage() string {
259261

260262
func createTempIngressSquidConf(
261263
serverHostname string,
262-
ingressPorts map[string]struct{},
264+
upstreamPort int,
265+
squidPort int,
263266
) (string, error) {
264267
var sb strings.Builder
265268

266269
sb.WriteString(
267-
"http_port 3128\n" +
268-
"visible_hostname " + serverHostname + "-ingress\n" +
270+
"visible_hostname " + serverHostname + "-ingress\n" +
269271
"access_log stdio:/var/log/squid/access.log squid\n" +
270272
"pid_filename /tmp/squid.pid\n" +
271273
"# Disable memory and disk caching\n" +
@@ -277,7 +279,7 @@ func createTempIngressSquidConf(
277279
"cache_dir null /tmp\n" +
278280
"cache_store_log none\n\n")
279281

280-
writeIngressProxyConfig(&sb, ingressPorts, serverHostname)
282+
writeIngressProxyConfig(&sb, serverHostname, upstreamPort, squidPort)
281283
sb.WriteString("http_access deny all\n")
282284

283285
tmpFile, err := os.CreateTemp("", "squid-*.conf")
@@ -298,19 +300,18 @@ func createTempIngressSquidConf(
298300
return tmpFile.Name(), nil
299301
}
300302

301-
func writeIngressProxyConfig(sb *strings.Builder, ingressPorts map[string]struct{}, serverHostname string) {
302-
for port := range ingressPorts {
303-
portNum := strings.Split(port, "/")[0]
304-
sb.WriteString(
305-
"\n# Reverse proxy setup for port " + portNum + "\n" +
306-
"http_port " + portNum + " accel defaultsite=" + serverHostname + "\n" +
307-
"cache_peer " + serverHostname + " parent " + portNum + " 0 no-query originserver name=origin_" +
308-
portNum + " connect-timeout=5 connect-fail-limit=5\n" +
309-
"acl site_" + portNum + " dstdomain " + serverHostname + "\n" +
310-
"acl local_dst dst 127.0.0.1\n" +
311-
"acl local_domain dstdomain localhost\n" +
312-
"http_access allow site_" + portNum + "\n" +
313-
"http_access allow local_dst\n" +
314-
"http_access allow local_domain\n")
315-
}
303+
func writeIngressProxyConfig(sb *strings.Builder, serverHostname string, upstreamPort int, squidPort int) {
304+
portNum := strconv.Itoa(upstreamPort)
305+
squidPortNum := strconv.Itoa(squidPort)
306+
sb.WriteString(
307+
"\n# Reverse proxy setup for port " + portNum + "\n" +
308+
"http_port 0.0.0.0:" + squidPortNum + " accel defaultsite=" + serverHostname + "\n" +
309+
"cache_peer " + serverHostname + " parent " + portNum + " 0 no-query originserver name=origin_" +
310+
portNum + " connect-timeout=5 connect-fail-limit=5\n" +
311+
"acl site_" + portNum + " dstdomain " + serverHostname + "\n" +
312+
"acl local_dst dst 127.0.0.1\n" +
313+
"acl local_domain dstdomain localhost\n" +
314+
"http_access allow site_" + portNum + "\n" +
315+
"http_access allow local_dst\n" +
316+
"http_access allow local_domain\n")
316317
}

‎pkg/container/kubernetes/client.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,7 @@ func (c *Client) DeployWorkload(ctx context.Context,
259259
transportType string,
260260
options *runtime.DeployWorkloadOptions,
261261
_ bool,
262-
) (string, error) {
262+
) (string, int, error) {
263263
namespace := getCurrentNamespace()
264264
containerLabels["app"] = containerName
265265
containerLabels["toolhive"] = "true"
@@ -280,7 +280,7 @@ func (c *Client) DeployWorkload(ctx context.Context,
280280
var err error
281281
podTemplateSpec, err = applyPodTemplatePatch(podTemplateSpec, options.K8sPodTemplatePatch)
282282
if err != nil {
283-
return "", fmt.Errorf("failed to apply pod template patch: %w", err)
283+
return "", 0, fmt.Errorf("failed to apply pod template patch: %w", err)
284284
}
285285
}
286286

@@ -298,7 +298,7 @@ func (c *Client) DeployWorkload(ctx context.Context,
298298
options,
299299
)
300300
if err != nil {
301-
return "", err
301+
return "", 0, err
302302
}
303303

304304
// Create an apply configuration for the statefulset
@@ -321,7 +321,7 @@ func (c *Client) DeployWorkload(ctx context.Context,
321321
Force: true,
322322
})
323323
if err != nil {
324-
return "", fmt.Errorf("failed to apply statefulset: %v", err)
324+
return "", 0, fmt.Errorf("failed to apply statefulset: %v", err)
325325
}
326326

327327
logger.Infof("Applied statefulset %s", createdStatefulSet.Name)
@@ -330,7 +330,7 @@ func (c *Client) DeployWorkload(ctx context.Context,
330330
// Create a headless service for SSE transport
331331
err := c.createHeadlessService(ctx, containerName, namespace, containerLabels, options)
332332
if err != nil {
333-
return "", fmt.Errorf("failed to create headless service: %v", err)
333+
return "", 0, fmt.Errorf("failed to create headless service: %v", err)
334334
}
335335
}
336336

@@ -341,10 +341,10 @@ func (c *Client) DeployWorkload(ctx context.Context,
341341
}
342342
err = waitFunc(ctx, c.client, namespace, createdStatefulSet.Name)
343343
if err != nil {
344-
return createdStatefulSet.Name, fmt.Errorf("statefulset applied but failed to become ready: %w", err)
344+
return createdStatefulSet.Name, 0, fmt.Errorf("statefulset applied but failed to become ready: %w", err)
345345
}
346346

347-
return createdStatefulSet.Name, nil
347+
return createdStatefulSet.Name, 0, nil
348348
}
349349

350350
// GetWorkloadInfo implements runtime.Runtime.

‎pkg/container/kubernetes/client_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ func TestCreateContainerWithPodTemplatePatch(t *testing.T) {
171171
options.K8sPodTemplatePatch = tc.k8sPodTemplatePatch
172172

173173
// Deploy the workload
174-
containerID, err := client.DeployWorkload(
174+
containerID, _, err := client.DeployWorkload(
175175
context.Background(),
176176
"test-image",
177177
"test-container",
@@ -667,7 +667,7 @@ func TestCreateContainerWithMCP(t *testing.T) {
667667
}
668668

669669
// Deploy the workload
670-
containerID, err := client.DeployWorkload(
670+
containerID, _, err := client.DeployWorkload(
671671
context.Background(),
672672
tc.image,
673673
"test-container",

‎pkg/container/runtime/types.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ type Runtime interface {
8181
transportType string,
8282
options *DeployWorkloadOptions,
8383
isolateNetwork bool,
84-
) (string, error)
84+
) (string, int, error)
8585

8686
// ListWorkloads lists all deployed workloads managed by this runtime.
8787
// Returns information about each workload including its components,

‎pkg/runner/config_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,8 @@ func (*mockRuntime) DeployWorkload(
3232
_ string,
3333
_ *rt.DeployWorkloadOptions,
3434
_ bool,
35-
) (string, error) {
36-
return "container-id", nil
35+
) (string, int, error) {
36+
return "container-id", 8080, nil
3737
}
3838

3939
func (*mockRuntime) StartContainer(_ context.Context, _ string) error {

‎pkg/transport/sse.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ func (t *SSETransport) Setup(ctx context.Context, runtime rt.Runtime, containerN
144144

145145
// Create the container
146146
logger.Infof("Deploying workload %s from image %s...", containerName, image)
147-
containerID, err := t.runtime.DeployWorkload(
147+
containerID, exposedPort, err := t.runtime.DeployWorkload(
148148
ctx,
149149
image,
150150
containerName,
@@ -169,6 +169,9 @@ func (t *SSETransport) Setup(ctx context.Context, runtime rt.Runtime, containerN
169169
t.targetHost = containerOptions.SSEHeadlessServiceName
170170
}
171171

172+
// also override the exposed port, in case we need it via ingress
173+
t.targetPort = exposedPort
174+
172175
return nil
173176
}
174177

‎pkg/transport/stdio.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ func (t *StdioTransport) Setup(
110110

111111
// Create the container
112112
logger.Infof("Deploying workload %s from image %s...", containerName, image)
113-
containerID, err := t.runtime.DeployWorkload(
113+
containerID, _, err := t.runtime.DeployWorkload(
114114
ctx,
115115
image,
116116
containerName,

‎pkg/workloads/manager.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,10 @@ func (*defaultManager) RunWorkloadDetached(runConfig *runner.RunConfig) error {
228228
detachedArgs = append(detachedArgs, "--debug")
229229
}
230230

231+
if runConfig.IsolateNetwork {
232+
detachedArgs = append(detachedArgs, "--isolate-network")
233+
}
234+
231235
// Use Name if available
232236
if runConfig.Name != "" {
233237
detachedArgs = append(detachedArgs, "--name", runConfig.Name)

0 commit comments

Comments
 (0)