Skip to content

Commit 256c0bd

Browse files
committed
feat(webhook): support multi-region SQS dispatch in runner webhook
Derive the SQS client region from the queue URL instead of always using AWS_REGION. This prevents cross-region SignatureDoesNotMatch failures when the webhook sends messages to queues outside the Lambda region. Cache traced SQS clients by region to avoid recreating clients for repeated sends, while still creating separate clients for different queue regions. Fall back to AWS_REGION when the queue URL cannot be parsed, and fall back to the SDK default region resolution when no region is available. Expand unit coverage for queue URL region parsing, AWS_REGION fallback, missing-region behavior, same-region client reuse, and per-region client separation.
1 parent 1d57199 commit 256c0bd

2 files changed

Lines changed: 137 additions & 24 deletions

File tree

Lines changed: 106 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,37 +1,54 @@
11
import { SendMessageCommandInput } from '@aws-sdk/client-sqs';
2-
import { sendActionRequest } from '.';
3-
import { describe, it, expect, afterEach, vi } from 'vitest';
2+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
3+
4+
const { mockSqsClients, sqsConstructorSpy, tracedClients, logger } = vi.hoisted(() => ({
5+
mockSqsClients: [] as Array<{ sendMessage: ReturnType<typeof vi.fn> }>,
6+
sqsConstructorSpy: vi.fn(),
7+
tracedClients: [] as unknown[],
8+
logger: { debug: vi.fn() },
9+
}));
10+
11+
function MockSQS(this: unknown, config?: unknown) {
12+
sqsConstructorSpy(config);
13+
const client = {
14+
sendMessage: vi.fn().mockResolvedValue({}),
15+
};
16+
mockSqsClients.push(client);
17+
return client;
18+
}
419

5-
const mockSQS = {
6-
sendMessage: vi.fn(() => {
7-
return {};
8-
}),
9-
};
1020
vi.mock('@aws-sdk/client-sqs', () => ({
11-
SQS: vi.fn().mockImplementation(function () {
12-
return mockSQS;
21+
SQS: vi.fn(MockSQS),
22+
}));
23+
24+
vi.mock('@aws-github-runner/aws-powertools-util', () => ({
25+
createChildLogger: vi.fn(() => logger),
26+
getTracedAWSV3Client: vi.fn((client: unknown) => {
27+
tracedClients.push(client);
28+
return client;
1329
}),
1430
}));
15-
vi.mock('@aws-github-runner/aws-ssm-util');
31+
32+
const cleanEnv = process.env;
1633

1734
describe('Test sending message to SQS.', () => {
18-
const queueUrl = 'https://sqs.eu-west-1.amazonaws.com/123456789/queued-builds';
19-
const message = {
20-
eventType: 'type',
21-
id: 0,
22-
installationId: 0,
23-
repositoryName: 'test',
24-
repositoryOwner: 'owner',
25-
queueId: queueUrl,
26-
queueFifo: false,
27-
repoOwnerType: 'Organization',
28-
};
35+
beforeEach(() => {
36+
vi.resetModules();
37+
vi.clearAllMocks();
38+
mockSqsClients.length = 0;
39+
tracedClients.length = 0;
40+
process.env = { ...cleanEnv };
41+
});
2942

3043
afterEach(() => {
31-
vi.clearAllMocks();
44+
process.env = { ...cleanEnv };
3245
});
3346

3447
it('no fifo queue', async () => {
48+
const queueUrl = 'https://sqs.eu-west-1.amazonaws.com/123456789/queued-builds';
49+
const message = createMessage(queueUrl);
50+
const { sendActionRequest } = await import('.');
51+
3552
// Arrange
3653
const sqsMessage: SendMessageCommandInput = {
3754
QueueUrl: queueUrl,
@@ -42,7 +59,73 @@ describe('Test sending message to SQS.', () => {
4259
const result = sendActionRequest(message);
4360

4461
// Assert
45-
expect(mockSQS.sendMessage).toHaveBeenCalledWith(sqsMessage);
62+
expect(sqsConstructorSpy).toHaveBeenCalledWith({ region: 'eu-west-1' });
63+
expect(mockSqsClients[0].sendMessage).toHaveBeenCalledWith(sqsMessage);
64+
expect(tracedClients).toHaveLength(1);
65+
expect(logger.debug).toHaveBeenCalledTimes(1);
4666
await expect(result).resolves.not.toThrow();
4767
});
68+
69+
it('falls back to AWS_REGION when the queue url is invalid', async () => {
70+
process.env.AWS_REGION = 'us-east-2';
71+
const { sendActionRequest } = await import('.');
72+
73+
await sendActionRequest(createMessage('not-a-valid-url'));
74+
75+
expect(sqsConstructorSpy).toHaveBeenCalledTimes(1);
76+
expect(sqsConstructorSpy).toHaveBeenCalledWith({ region: 'us-east-2' });
77+
expect(mockSqsClients[0].sendMessage).toHaveBeenCalledTimes(1);
78+
expect(tracedClients).toHaveLength(1);
79+
});
80+
81+
it('creates a client without an explicit region when no region can be resolved', async () => {
82+
delete process.env.AWS_REGION;
83+
const { sendActionRequest } = await import('.');
84+
85+
await sendActionRequest(createMessage('not-a-valid-url'));
86+
87+
expect(sqsConstructorSpy).toHaveBeenCalledTimes(1);
88+
expect(sqsConstructorSpy).toHaveBeenCalledWith({});
89+
expect(mockSqsClients[0].sendMessage).toHaveBeenCalledTimes(1);
90+
expect(tracedClients).toHaveLength(1);
91+
});
92+
93+
it('reuses the same client for multiple queues in the same region', async () => {
94+
const { sendActionRequest } = await import('.');
95+
96+
await sendActionRequest(createMessage('https://sqs.us-east-1.amazonaws.com/123456789/queue-a'));
97+
await sendActionRequest(createMessage('https://sqs.us-east-1.amazonaws.com/123456789/queue-b'));
98+
99+
expect(sqsConstructorSpy).toHaveBeenCalledTimes(1);
100+
expect(sqsConstructorSpy).toHaveBeenCalledWith({ region: 'us-east-1' });
101+
expect(mockSqsClients[0].sendMessage).toHaveBeenCalledTimes(2);
102+
expect(tracedClients).toHaveLength(1);
103+
});
104+
105+
it('creates a separate client per region', async () => {
106+
const { sendActionRequest } = await import('.');
107+
108+
await sendActionRequest(createMessage('https://sqs.us-east-1.amazonaws.com/123456789/queue-a'));
109+
await sendActionRequest(createMessage('https://sqs.eu-west-1.amazonaws.com/123456789/queue-b'));
110+
111+
expect(sqsConstructorSpy).toHaveBeenCalledTimes(2);
112+
expect(sqsConstructorSpy).toHaveBeenNthCalledWith(1, { region: 'us-east-1' });
113+
expect(sqsConstructorSpy).toHaveBeenNthCalledWith(2, { region: 'eu-west-1' });
114+
expect(mockSqsClients).toHaveLength(2);
115+
expect(mockSqsClients[0].sendMessage).toHaveBeenCalledTimes(1);
116+
expect(mockSqsClients[1].sendMessage).toHaveBeenCalledTimes(1);
117+
expect(tracedClients).toHaveLength(2);
118+
});
48119
});
120+
121+
function createMessage(queueId: string) {
122+
return {
123+
eventType: 'type',
124+
id: 0,
125+
installationId: 0,
126+
repositoryName: 'test',
127+
repositoryOwner: 'owner',
128+
queueId,
129+
repoOwnerType: 'Organization',
130+
};
131+
}

lambdas/functions/webhook/src/sqs/index.ts

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import { createChildLogger, getTracedAWSV3Client } from '@aws-github-runner/aws-
44

55
const logger = createChildLogger('sqs');
66

7+
const sqsClientsByRegion = new Map<string, SQS>();
8+
79
export interface ActionRequestMessage {
810
id: number;
911
eventType: string;
@@ -32,7 +34,8 @@ export interface GithubWorkflowEvent {
3234
}
3335

3436
export const sendActionRequest = async (message: ActionRequestMessage): Promise<void> => {
35-
const sqs = getTracedAWSV3Client(new SQS({ region: process.env.AWS_REGION }));
37+
const region = getRegionFromQueueUrl(message.queueId) ?? process.env.AWS_REGION;
38+
const sqs = getSqsClient(region);
3639

3740
const sqsMessage: SendMessageCommandInput = {
3841
QueueUrl: message.queueId,
@@ -43,3 +46,30 @@ export const sendActionRequest = async (message: ActionRequestMessage): Promise<
4346

4447
await sqs.sendMessage(sqsMessage);
4548
};
49+
50+
function getSqsClient(region: string | undefined): SQS {
51+
if (!region) {
52+
return getTracedAWSV3Client(new SQS({}));
53+
}
54+
55+
const cached = sqsClientsByRegion.get(region);
56+
if (cached) {
57+
return cached;
58+
}
59+
60+
const client = getTracedAWSV3Client(new SQS({ region }));
61+
sqsClientsByRegion.set(region, client);
62+
return client;
63+
}
64+
65+
function getRegionFromQueueUrl(queueUrl: string): string | undefined {
66+
try {
67+
const url = new URL(queueUrl);
68+
const parts = url.hostname.split('.');
69+
if (parts.length >= 3 && parts[0] === 'sqs') {
70+
return parts[1];
71+
}
72+
} catch {}
73+
74+
return undefined;
75+
}

0 commit comments

Comments
 (0)