Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ integTest(
withDefaultFixture(async (fixture) => {
const output = await fixture.cdk(['cli-telemetry', '--disable'], { verboseLevel: 3 });

// Check the trace that telemetry was not executed successfully
expect(output).not.toContain('Telemetry Sent Successfully');
// Check the trace that telemetry was never handed to a sender
expect(output).not.toContain('Telemetry dispatched');

// Check the trace that endpoint telemetry was never connected
expect(output).toContain('Endpoint Telemetry NOT connected');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ integTest(
verboseLevel: 3, // trace mode
});

// Check the trace that telemetry was executed successfully
expect(deployOutput).toContain('Telemetry Sent Successfully');
// Check the trace that telemetry was handed to the detached sender that delivers it
expect(deployOutput).toContain('Telemetry dispatched');

const json = fs.readJSONSync(telemetryFile);
expect(json).toEqual([
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ integTest(
modEnv: { DYNAMIC_LAMBDA_PROPERTY_VALUE: 'updated' },
});

// Check the trace that telemetry was executed successfully
expect(deployOutput).toContain('Telemetry Sent Successfully');
// Check the trace that telemetry was handed to the detached sender that delivers it
expect(deployOutput).toContain('Telemetry dispatched');

const json = fs.readJSONSync(telemetryFile);
expect(json).toEqual(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ integTest(
}, // trace mode
);

// Check the trace that telemetry was executed successfully
expect(synthOutput).toContain('Telemetry Sent Successfully');
// Check the trace that telemetry was handed to the detached sender that delivers it
expect(synthOutput).toContain('Telemetry dispatched');

const json = fs.readJSONSync(telemetryFile);
expect(json).toEqual([
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ integTest(

expect(output).toContain('This is an error');

// Check the trace that telemetry was executed successfully despite error in synth
expect(output).toContain('Telemetry Sent Successfully');
// Check the trace that telemetry was handed to the detached sender despite the error in synth
expect(output).toContain('Telemetry dispatched');

const json = fs.readJSONSync(telemetryFile);
expect(json).toEqual([
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ integTest(
{ verboseLevel: 3 }, // trace mode
);

// Check the trace that telemetry was executed successfully
expect(synthOutput).toContain('Telemetry Sent Successfully');
// Check the trace that telemetry was handed to the detached sender that delivers it
expect(synthOutput).toContain('Telemetry dispatched');

const json = fs.readJSONSync(telemetryFile);
expect(json).toEqual([
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
import * as net from 'node:net';
import type { AddressInfo } from 'node:net';
import { integTest, withDefaultFixture } from '../../lib';

/**
* Telemetry is delivered by a detached child process, so the CLI must not wait for the POST.
*
* The endpoint here is a black hole: a TCP listener that accepts the connection and then never
* writes a byte, so anything talking to it hangs until its own timeout. Before the sender was
* detached, the flush at the end of the invocation blocked on exactly that, which is why this
* asserts on wall-clock time rather than on output.
*/
integTest(
'cdk synth does not wait for the telemetry endpoint',
withDefaultFixture(async (fixture) => {
const sockets: net.Socket[] = [];
const blackHole = net.createServer((socket) => {
// Accept and hold. Never respond, never close.
sockets.push(socket);
});
await new Promise<void>((ok) => blackHole.listen(0, '127.0.0.1', ok));
const port = (blackHole.address() as AddressInfo).port;

try {
// Baseline: the same synth with telemetry switched off entirely.
const disabledStart = Date.now();
await fixture.cdkSynth({
options: [fixture.fullStackName('test-1')],
modEnv: { CDK_DISABLE_CLI_TELEMETRY: 'true' },
});
const disabledMs = Date.now() - disabledStart;

// The same synth, with telemetry pointed at the black hole.
const blackHoleStart = Date.now();
await fixture.cdkSynth({
options: [fixture.fullStackName('test-1')],
modEnv: { TELEMETRY_ENDPOINT: `https://127.0.0.1:${port}/metrics` },
});
const blackHoleMs = Date.now() - blackHoleStart;

const overhead = blackHoleMs - disabledMs;
fixture.log(`synth with telemetry disabled: ${disabledMs}ms, pointed at a black hole: ${blackHoleMs}ms (overhead ${overhead}ms)`);

// The detached sender is what hangs on the black hole, not us. The headroom is generous
// because CI machines are noisy; what this rules out is the CLI blocking on the request
// timeout, which shows up as whole seconds.
expect(overhead).toBeLessThan(2000);
} finally {
for (const socket of sockets) {
socket.destroy();
}
await new Promise<void>((ok) => blackHole.close(() => ok()));
}
}),
);
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
import * as https from 'node:https';
import type { AddressInfo } from 'node:net';
import * as mockttp from 'mockttp';
import { integTest, withDefaultFixture } from '../../lib';
import { startProxyServer } from '../../lib/proxy';

/**
* Telemetry has to keep working for users behind a corporate proxy.
*
* This matters more than it looks. The POST is made by a detached child process that has no access
* to the parent's `proxy-agent` instance -- it only has Node built-ins -- so it re-implements HTTP
* CONNECT tunnelling and has to be handed the proxy URL and CA bundle explicitly. This test proves
* that hand-off end to end against the same TLS-terminating proxy the other proxy tests use, whose
* certificate is signed by a throwaway CA that is not in any system trust store.
*
* `TELEMETRY_ENDPOINT` is pointed at a local server rather than the real one, so the test neither
* needs egress to production nor posts real telemetry from CI. What is under test is the CLI ->
* proxy hop: that the child opened a CONNECT tunnel and completed a TLS handshake against a
* certificate it could only have verified using the forwarded CA. The proxy -> endpoint hop is
* deliberately out of scope (the proxy will not trust the local server's self-signed certificate,
* which does not matter -- the proxy records the decrypted request either way).
*/
integTest(
'telemetry is delivered through a configured proxy',
withDefaultFixture(async (fixture) => {
// Stand-in for the telemetry endpoint. Never actually serves a response to the proxy; it only
// needs to occupy a port so the CONNECT target is real.
const { key, cert } = await mockttp.generateCACertificate();
const endpointServer = https.createServer({ key, cert }, (_req, res) => {
res.writeHead(200, { 'content-type': 'application/json' });
res.end('{"ok":true}');
});
await new Promise<void>((ok) => endpointServer.listen(0, '127.0.0.1', ok));
const endpointPort = (endpointServer.address() as AddressInfo).port;
const telemetryEndpoint = `https://localhost:${endpointPort}/metrics`;

const proxyServer = await startProxyServer();
try {
const output = await fixture.cdkSynth({
options: [
fixture.fullStackName('test-1'),
'--proxy', proxyServer.url,
'--ca-bundle-path', proxyServer.certPath,
],
modEnv: {
CDK_HOME: fixture.integTestDir,
TELEMETRY_ENDPOINT: telemetryEndpoint,
},
verboseLevel: 3, // trace
});

// The parent reports the hand-off, not the delivery.
expect(output).toContain('Telemetry dispatched');

// Delivery happens after the CLI exits, so poll rather than asserting immediately.
const telemetryRequest = await waitFor(
async () => {
const requests = await proxyServer.getSeenRequests();
return requests.find((req) => req.url.includes(`localhost:${endpointPort}`));
},
30_000,
);

expect(telemetryRequest).toBeDefined();
expect(telemetryRequest!.method).toBe('POST');

// The proxy terminates TLS, so we can read the decrypted body and confirm the child sent a
// well-formed batch (and therefore that both the proxy URL and the CA made it across).
const body = JSON.parse(telemetryRequest!.body.buffer.toString('utf-8'));
expect(Array.isArray(body.events)).toBe(true);
expect(body.events.length).toBeGreaterThan(0);
expect(body.events[0]).toEqual(expect.objectContaining({
identifiers: expect.objectContaining({ sessionId: expect.anything() }),
}));
} finally {
await proxyServer.stop();
await new Promise<void>((ok) => endpointServer.close(() => ok()));
}
}),
);

/**
* Poll `fn` until it returns something truthy, or give up after `timeoutMs`.
*/
async function waitFor<A>(fn: () => Promise<A | undefined>, timeoutMs: number): Promise<A | undefined> {
const deadline = Date.now() + timeoutMs;
while (Date.now() < deadline) {
const result = await fn();
if (result) {
return result;
}
await new Promise((ok) => setTimeout(ok, 500));
}
return undefined;
}
14 changes: 14 additions & 0 deletions packages/aws-cdk/bin/cdk
Original file line number Diff line number Diff line change
@@ -1,4 +1,18 @@
#!/usr/bin/env node
// Publish our own location so the CLI can respawn us as a detached telemetry sender.
// This is the only place that knows it reliably; process.argv[1] may be a .bin symlink,
// the `cdk` alias package's wrapper, or an embedding script.
process.env.CDK_CLI_BIN_PATH = __filename;

// That detached sender is this same script with a flag. Dispatch before requiring the CLI,
// whose bundle costs ~600ms to load and which the sender does not need.
if (process.env.CDK_TELEMETRY_SENDER === '1') {
require("../lib/cli/telemetry/sender").main();
// Relies on the CommonJS module wrapper (modules are functions); would be a SyntaxError if this
// file ever became native ESM.
return;
}

// source maps must be enabled before importing files
process.setSourceMapsEnabled(true);
const { cli } = require("../lib");
Expand Down
7 changes: 4 additions & 3 deletions packages/aws-cdk/lib/cli/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,13 +116,14 @@ export async function exec(args: string[], synthesizer?: Synthesizer): Promise<n
});

// Always create and use ProxyAgent to support configuration via env vars
const proxyAgent = await new ProxyAgentProvider(ioHelper).create({
proxyAddress: configuration.settings.get(['proxy']),
const proxyUrl: string | undefined = configuration.settings.get(['proxy']);
const { agent: proxyAgent, caCert } = await new ProxyAgentProvider(ioHelper).create({
proxyAddress: proxyUrl,
caBundlePath: configuration.settings.get(['caBundlePath']),
});

try {
await ioHost.startTelemetry(argv, configuration.context, proxyAgent);
await ioHost.startTelemetry(argv, configuration.context, { proxyUrl, caCert });
} catch (e: any) {
await ioHost.asIoHelper().defaults.trace(`Telemetry instantiation failed: ${e.message}`);
}
Expand Down
31 changes: 28 additions & 3 deletions packages/aws-cdk/lib/cli/io-host/cli-io-host.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import type { Agent } from 'node:https';
import * as util from 'node:util';
import { RequireApproval } from '@aws-cdk/cloud-assembly-schema';
import { ToolkitError } from '@aws-cdk/toolkit-lib';
Expand All @@ -9,6 +8,7 @@ import type { IoHelper, ActivityPrinterProps, IActivityPrinter, IoMessageMaker,
import { asIoHelper, IO, isMessageRelevantForLevel, CurrentActivityPrinter, HistoryActivityPrinter } from '../../../lib/api-private';
import type { Context } from '../../api/context';
import { StackActivityProgress } from '../../commands/deploy';
import { cliBinPath } from '../telemetry/cli-bin-path';
import { canCollectTelemetry } from '../telemetry/collect-telemetry';
import { cdkCliErrorName } from '../telemetry/error';
import type { EventResult } from '../telemetry/messages';
Expand Down Expand Up @@ -37,6 +37,29 @@ type CliAction =
| 'cli-telemetry'
| 'none';

/**
* How telemetry should reach the network.
*
* The endpoint sink does not make the request itself -- it hands off to a detached child process
* that only has Node built-ins available. That child cannot be given an `Agent`, so the proxy and
* certificate configuration have to travel as plain data instead.
*/
export interface TelemetryNetworkOptions {
/**
* Proxy configured via `--proxy` or the `proxy` setting.
*
* @default - the sender resolves it from the environment
*/
readonly proxyUrl?: string;

/**
* Contents of the CA bundle configured via `--ca-bundle-path` or `AWS_CA_BUNDLE`.
*
* @default - only the system trust store
*/
readonly caCert?: string;
}

export interface CliIoHostProps {
/**
* The initial Toolkit action the hosts starts with.
Expand Down Expand Up @@ -376,7 +399,7 @@ export class CliIoHost implements IIoHost, ObservableIoHost {
this.routeStackActivityToPrinter();
}

public async startTelemetry(args: any, context: Context, proxyAgent?: Agent) {
public async startTelemetry(args: any, context: Context, network: TelemetryNetworkOptions = {}) {
// eslint-disable-next-line @typescript-eslint/no-require-imports
const config = require('../cli-type-registry.json');
const validCommands = Object.keys(config.commands);
Expand Down Expand Up @@ -407,8 +430,10 @@ export class CliIoHost implements IIoHost, ObservableIoHost {
try {
sinks.push(new EndpointTelemetrySink({
ioHost: this,
agent: proxyAgent,
endpoint: telemetryEndpoint,
binCdkPath: cliBinPath(),
proxyUrl: network.proxyUrl,
caCert: network.caCert,
}));
await this.asIoHelper().defaults.trace('Endpoint Telemetry connected');
} catch (e: any) {
Expand Down
35 changes: 30 additions & 5 deletions packages/aws-cdk/lib/cli/proxy-agent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,24 +21,49 @@ interface ProxyAgentOptions {
readonly caBundlePath?: string;
}

/**
* The proxy configuration resolved for this invocation.
*/
export interface ResolvedProxyAgent {
/**
* The agent to pass to anything making HTTPS requests in this process.
*/
readonly agent: ProxyAgent;

/**
* Contents of the resolved CA bundle, if one was configured.
*
* Exposed because the detached telemetry sender cannot use `agent` -- it runs in another process
* and only has Node built-ins -- so it needs the certificate itself.
*
* @default - no CA bundle was configured
*/
readonly caCert?: string;
}

export class ProxyAgentProvider {
private readonly ioHelper: IoHelper;

public constructor(ioHelper: IoHelper) {
this.ioHelper = ioHelper;
}

public async create(options: ProxyAgentOptions) {
public async create(options: ProxyAgentOptions): Promise<ResolvedProxyAgent> {
// Force it to use the proxy provided through the command line.
// Otherwise, let the ProxyAgent auto-detect the proxy using environment variables.
const getProxyForUrl = options.proxyAddress != null
? () => Promise.resolve(options.proxyAddress!)
: undefined;

return new ProxyAgent({
ca: await this.tryGetCACert(options.caBundlePath),
getProxyForUrl,
});
const caCert = await this.tryGetCACert(options.caBundlePath);

return {
agent: new ProxyAgent({
ca: caCert,
getProxyForUrl,
}),
caCert,
};
}

private async tryGetCACert(bundlePath?: string) {
Expand Down
Loading
Loading