Skip to content

Commit b16c03c

Browse files
committed
Improve Docker tunnel start & stop process
This avoids unnecessary pulls (very slow, it turns out), stops the tunnel from being created before it's needed, and ensures it's properly cleaned up (wasn't before, due to a missing 'await')
1 parent 02f37d5 commit b16c03c

File tree

5 files changed

+66
-11
lines changed

5 files changed

+66
-11
lines changed

src/interceptors/docker/docker-commands.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,12 @@ export const getDockerHostIp = (
7070
}
7171
}
7272

73+
export function isImageAvailable(docker: Docker, name: string) {
74+
return docker.getImage(name).inspect()
75+
.then(() => true)
76+
.catch(() => false);
77+
}
78+
7379
export function isInterceptedContainer(container: Docker.ContainerInspectInfo, port: string | number) {
7480
return container.Config.Labels[DOCKER_CONTAINER_LABEL] === port.toString();
7581
}

src/interceptors/docker/docker-interception-services.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,9 @@ export async function startDockerInterceptionServices(
3636
httpsConfig: { certPath: string, certContent: string },
3737
ruleParameters: { [key: `docker-tunnel-proxy-${number}`]: ProxySettingCallback }
3838
) {
39+
// Prepare (pull) the tunnel image, but we don't actually start the tunnel itself until some
40+
// Docker interception happens while HTTP Toolkit is running - e.g. proxy use, container attach,
41+
// or an intercepted container connecting to a network.
3942
prepareDockerTunnel();
4043
const networkMonitor = monitorDockerNetworkAliases(proxyPort);
4144

src/interceptors/docker/docker-networking.ts

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import { reportError } from '../../error-tracking';
88

99
import { DOCKER_HOST_HOSTNAME, isInterceptedContainer } from './docker-commands';
1010
import { isDockerAvailable } from './docker-interception-services';
11-
import { ensureDockerTunnelRunning, updateDockerTunnelledNetworks } from './docker-tunnel-proxy';
11+
import { updateDockerTunnelledNetworks } from './docker-tunnel-proxy';
1212
import { getDnsServer } from '../../dns-server';
1313

1414
interface DockerEvent {
@@ -93,16 +93,19 @@ export async function monitorDockerNetworkAliases(proxyPort: number): Promise<Do
9393
reportError(e);
9494
});
9595

96-
ensureDockerTunnelRunning(proxyPort);
9796
const dnsServer = await getDnsServer(proxyPort);
9897

9998
const networkMonitor = new DockerNetworkMonitor(docker, proxyPort, stream);
100-
mobx.autorun(() =>
101-
updateDockerTunnelledNetworks(proxyPort, networkMonitor.interceptedNetworks)
102-
.catch(console.warn)
103-
);
104-
mobx.autorun(() =>
105-
dnsServer.setHosts(networkMonitor.aliasIpMap)
99+
100+
// We update DNS immediately, and on all changes:
101+
mobx.autorun(() => dnsServer.setHosts(networkMonitor.aliasIpMap));
102+
103+
// We update tunnel _only_ once something is actually intercepted - once interceptedNetworks changes.
104+
// We don't want to create the tunnel container unless Docker interception is actually used.
105+
mobx.reaction(
106+
() => networkMonitor.interceptedNetworks,
107+
(interceptedNetworks) => updateDockerTunnelledNetworks(proxyPort, interceptedNetworks)
108+
.catch(console.warn)
106109
);
107110

108111
dockerNetworkMonitors[proxyPort] = networkMonitor;

src/interceptors/docker/docker-tunnel-proxy.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import * as Docker from 'dockerode';
33
import * as semver from 'semver';
44
import { Mutex } from 'async-mutex';
55

6-
import { DOCKER_HOST_HOSTNAME } from './docker-commands';
6+
import { DOCKER_HOST_HOSTNAME, isImageAvailable } from './docker-commands';
77
import { isDockerAvailable } from './docker-interception-services';
88

99
const DOCKER_TUNNEL_IMAGE = "httptoolkit/docker-socks-tunnel:v1.1.0";
@@ -22,7 +22,8 @@ export async function prepareDockerTunnel() {
2222

2323
await containerMutex.runExclusive(async () => {
2424
const docker = new Docker();
25-
await docker.pull(DOCKER_TUNNEL_IMAGE).catch(console.warn);
25+
if (await isImageAvailable(docker, DOCKER_TUNNEL_IMAGE)) return;
26+
else await docker.pull(DOCKER_TUNNEL_IMAGE).catch(console.warn);
2627
});
2728
}
2829

@@ -234,7 +235,7 @@ export async function refreshDockerTunnelPortCache(proxyPort: number): Promise<n
234235
export async function stopDockerTunnel(proxyPort: number | 'all'): Promise<void> {
235236
const docker = new Docker();
236237

237-
containerMutex.runExclusive(async () => {
238+
await containerMutex.runExclusive(async () => {
238239
const containers = await docker.listContainers({
239240
all: true,
240241
filters: JSON.stringify({

test/interceptors/docker-terminal-interception.spec.ts

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { expect } from 'chai';
88

99
import { setupTest } from './interceptor-test-utils';
1010
import { spawnToResult } from '../../src/util/process-management';
11+
import { delay } from '../../src/util/promise';
1112

1213
import { getTerminalEnvVars } from '../../src/interceptors/terminal/terminal-env-overrides';
1314

@@ -325,12 +326,53 @@ Successfully built <hash>
325326

326327
await stopDockerInterceptionServices(server.port, ruleParams);
327328

329+
// Intercepted container is removed:
328330
const inspectResultAfterShutdown: unknown = await docker.getContainer(interceptedContainerId).inspect().catch(e => e);
329331
expect(inspectResultAfterShutdown).to.be.instanceOf(Error)
330332
expect((inspectResultAfterShutdown as Error).message).to.include("no such container");
331333

334+
// Unintercepted container is not touched:
332335
const uninterceptedContainerAfterShutdown = await docker.getContainer(uninterceptedContainerId).inspect();
333336
expect(uninterceptedContainerAfterShutdown.Config.Image).to.equal('node:14');
334337
});
335338

339+
it("should start & clean up tunnel automatically", async () => {
340+
const { server, httpsConfig } = await testSetup;
341+
const docker = new Docker();
342+
343+
const terminalEnvOverrides = getTerminalEnvVars(server.port, httpsConfig, process.env, {}, {
344+
dockerEnabled: true
345+
});
346+
347+
// Tunnel doesn't start preemptively:
348+
await delay(500);
349+
let tunnel = await docker.getContainer(`httptoolkit-docker-tunnel-${server.port}`).inspect().catch(e => e);
350+
expect(tunnel).to.be.instanceOf(Error)
351+
expect((tunnel as Error).message).to.include("no such container");
352+
353+
// Then launch an intercepted container:
354+
const interceptedResult = await spawnToResult(
355+
'docker', ['create', 'node:14'],
356+
{ env: { ...process.env, ...terminalEnvOverrides } },
357+
true
358+
);
359+
360+
expect(interceptedResult.exitCode).to.equal(0);
361+
expect(interceptedResult.stderr).to.equal('');
362+
363+
await delay(100); // There can be a brief delay in CI in starting the tunnel itself
364+
365+
// Tunnel is now running:
366+
tunnel = await docker.getContainer(`httptoolkit-docker-tunnel-${server.port}`).inspect();
367+
expect(tunnel.Config.Image.split(':')[0]).to.equal('httptoolkit/docker-socks-tunnel');
368+
369+
// Then shut everything down:
370+
await stopDockerInterceptionServices(server.port, ruleParams);
371+
372+
// Tunnel is automatically removed:
373+
tunnel = await docker.getContainer(`httptoolkit-docker-tunnel-${server.port}`).inspect().catch(e => e);
374+
expect(tunnel).to.be.instanceOf(Error)
375+
expect((tunnel as Error).message).to.include("no such container");
376+
});
377+
336378
});

0 commit comments

Comments
 (0)