Skip to content

Commit 4066c4e

Browse files
npalmphilips-labs-pr|bot
and
philips-labs-pr|bot
authored
feat: remove deprecated bata feature workflow job queue (#4249)
## Description This PR removed the deprecated (beta / experimental) feature to publish the `workflow_job` event on an extra queue. ## Test - [x] default example - [x] multi runner ## Migration Migration when depended on the feature can be done as follow ### Enable eventbridge ```hcl module "runners" { ... eventbridge { enable = true} } ... ``` ### Add rule to forward events to a queue ```hcl resource "aws_cloudwatch_event_rule" "workflow_job_in_progress" { name = "workflow-job-in-progress" event_bus_name = modules.runners.webhook.eventbridge.name # The name of the event bus output by the module event_pattern = <<EOF { "detail-type": ["workflow_job"], "detail": { "action": ["in_progress"] } } EOF } resource "aws_sqs_queue" "workflow_job_in_progress" { name = "workflow_job_in_progress } resource "aws_sqs_queue_policy" "workflow_job_in_progress" { queue_url = aws_sqs_queue.workflow_job_in_progress.id policy = data.aws_iam_policy_document.sqs_policy.json } data "aws_iam_policy_document" "sqs_policy" { statement { sid = "AllowFromEventBridge" actions = ["sqs:SendMessage"] principals { type = "Service" identifiers = ["events.amazonaws.com"] } resources = [aws_sqs_queue.workflow_job_in_progress.arn] condition { test = "ArnEquals" variable = "aws:SourceArn" values = [aws_cloudwatch_event_rule.workflow_job_in_progress.arn] } } } ``` --------- Co-authored-by: philips-labs-pr|bot <philips-labs-pr[bot]@users.noreply.github.com>
1 parent 52ce9c1 commit 4066c4e

30 files changed

+57
-283
lines changed

.tflint.hcl

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
config {
22
format = "compact"
3-
module = true
3+
call_module_type = "local"
44
}
55

66
plugin "aws" {

README.md

-4
Original file line numberDiff line numberDiff line change
@@ -110,10 +110,8 @@ Talk to the forestkeepers in the `runners-channel` on Slack.
110110
|------|------|
111111
| [aws_sqs_queue.queued_builds](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/sqs_queue) | resource |
112112
| [aws_sqs_queue.queued_builds_dlq](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/sqs_queue) | resource |
113-
| [aws_sqs_queue.webhook_events_workflow_job_queue](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/sqs_queue) | resource |
114113
| [aws_sqs_queue_policy.build_queue_dlq_policy](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/sqs_queue_policy) | resource |
115114
| [aws_sqs_queue_policy.build_queue_policy](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/sqs_queue_policy) | resource |
116-
| [aws_sqs_queue_policy.webhook_events_workflow_job_queue_policy](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/sqs_queue_policy) | resource |
117115
| [random_string.random](https://registry.terraform.io/providers/hashicorp/random/latest/docs/resources/string) | resource |
118116
| [aws_iam_policy_document.deny_unsecure_transport](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/iam_policy_document) | data source |
119117

@@ -156,7 +154,6 @@ Talk to the forestkeepers in the `runners-channel` on Slack.
156154
| <a name="input_enable_ssm_on_runners"></a> [enable\_ssm\_on\_runners](#input\_enable\_ssm\_on\_runners) | Enable to allow access to the runner instances for debugging purposes via SSM. Note that this adds additional permissions to the runner instances. | `bool` | `false` | no |
157155
| <a name="input_enable_user_data_debug_logging_runner"></a> [enable\_user\_data\_debug\_logging\_runner](#input\_enable\_user\_data\_debug\_logging\_runner) | Option to enable debug logging for user-data, this logs all secrets as well. | `bool` | `false` | no |
158156
| <a name="input_enable_userdata"></a> [enable\_userdata](#input\_enable\_userdata) | Should the userdata script be enabled for the runner. Set this to false if you are using your own prebuilt AMI. | `bool` | `true` | no |
159-
| <a name="input_enable_workflow_job_events_queue"></a> [enable\_workflow\_job\_events\_queue](#input\_enable\_workflow\_job\_events\_queue) | Enabling this experimental feature will create a secondary SQS queue to which a copy of the workflow\_job event will be delivered. | `bool` | `false` | no |
160157
| <a name="input_eventbridge"></a> [eventbridge](#input\_eventbridge) | Enable the use of EventBridge by the module. By enabling this feature events will be put on the EventBridge by the webhook instead of directly dispatching to queues for scaling.<br/><br/> `enable`: Enable the EventBridge feature.<br/> `accept_events`: List can be used to only allow specific events to be putted on the EventBridge. By default all events, empty list will be be interpreted as all events. | <pre>object({<br/> enable = optional(bool, false)<br/> accept_events = optional(list(string), null)<br/> })</pre> | `{}` | no |
161158
| <a name="input_ghes_ssl_verify"></a> [ghes\_ssl\_verify](#input\_ghes\_ssl\_verify) | GitHub Enterprise SSL verification. Set to 'false' when custom certificate (chains) is used for GitHub Enterprise Server (insecure). | `bool` | `true` | no |
162159
| <a name="input_ghes_url"></a> [ghes\_url](#input\_ghes\_url) | GitHub Enterprise Server URL. Example: https://github.internal.co - DO NOT SET IF USING PUBLIC GITHUB | `string` | `null` | no |
@@ -251,7 +248,6 @@ Talk to the forestkeepers in the `runners-channel` on Slack.
251248
| <a name="input_webhook_lambda_s3_object_version"></a> [webhook\_lambda\_s3\_object\_version](#input\_webhook\_lambda\_s3\_object\_version) | S3 object version for webhook lambda function. Useful if S3 versioning is enabled on source bucket. | `string` | `null` | no |
252249
| <a name="input_webhook_lambda_timeout"></a> [webhook\_lambda\_timeout](#input\_webhook\_lambda\_timeout) | Time out of the webhook lambda in seconds. | `number` | `10` | no |
253250
| <a name="input_webhook_lambda_zip"></a> [webhook\_lambda\_zip](#input\_webhook\_lambda\_zip) | File location of the webhook lambda zip file. | `string` | `null` | no |
254-
| <a name="input_workflow_job_queue_configuration"></a> [workflow\_job\_queue\_configuration](#input\_workflow\_job\_queue\_configuration) | Configuration options for workflow job queue which is only applicable if the flag enable\_workflow\_job\_events\_queue is set to true. | <pre>object({<br/> delay_seconds = number<br/> visibility_timeout_seconds = number<br/> message_retention_seconds = number<br/> })</pre> | <pre>{<br/> "delay_seconds": null,<br/> "message_retention_seconds": null,<br/> "visibility_timeout_seconds": null<br/>}</pre> | no |
255251

256252
## Outputs
257253

docs/configuration.md

+46-10
Original file line numberDiff line numberDiff line change
@@ -332,23 +332,59 @@ resource "aws_iam_role_policy" "event_rule_firehose_role" {
332332

333333
### Queue to publish workflow job events
334334

335-
!!! warning "Deprecated
335+
!!! warning "Removed
336336

337-
This fearure will be removed since we introducing the EventBridge. Same functinallity can be implemented by adding a rule to the EventBridge to forward `workflow_job` events to the SQS queue.
337+
This feaTure will be removed since we introducing the EventBridge. Same functionality can be implemented by adding a rule to the EventBridge to forward `workflow_job` events to the SQS queue.
338338

339-
This queue is an experimental feature to allow you to receive a copy of the wokflow_jobs events sent by the GitHub App. This can be used to calculate a matrix or monitor the system.
339+
Below an example how you can sent all `workflow_job` with action `in_progress` to a SQS queue.
340340

341-
To enable the feature set `enable_workflow_job_events_queue = true`. Be aware though, this feature is experimental!
341+
```hcl
342342
343-
Messages received on the queue are using the same format as published by GitHub wrapped in a property `workflowJobEvent`.
343+
resource "aws_cloudwatch_event_rule" "workflow_job_in_progress" {
344+
name = "workflow-job-in-progress"
345+
event_bus_name = modules.runners.webhook.eventbridge.name # The name of the event bus output by the module
344346
345-
```
346-
export interface GithubWorkflowEvent {
347-
workflowJobEvent: WorkflowJobEvent;
347+
event_pattern = <<EOF
348+
{
349+
"detail-type": ["workflow_job"],
350+
"detail": {
351+
"action": ["in_progress"]
352+
}
353+
}
354+
EOF
355+
}
356+
357+
resource "aws_sqs_queue" "workflow_job_in_progress" {
358+
name = "workflow_job_in_progress
359+
}
360+
361+
resource "aws_sqs_queue_policy" "workflow_job_in_progress" {
362+
queue_url = aws_sqs_queue.workflow_job_in_progress.id
363+
policy = data.aws_iam_policy_document.sqs_policy.json
364+
}
365+
366+
data "aws_iam_policy_document" "sqs_policy" {
367+
statement {
368+
sid = "AllowFromEventBridge"
369+
actions = ["sqs:SendMessage"]
370+
371+
principals {
372+
type = "Service"
373+
identifiers = ["events.amazonaws.com"]
374+
}
375+
376+
resources = [aws_sqs_queue.workflow_job_in_progress.arn]
377+
378+
condition {
379+
test = "ArnEquals"
380+
variable = "aws:SourceArn"
381+
values = [aws_cloudwatch_event_rule.workflow_job_in_progress.arn]
382+
}
383+
}
348384
}
385+
349386
```
350387

351-
This extensible format allows more fields to be added if needed.
352-
You can configure the queue by setting properties to `workflow_job_events_queue_config`
388+
353389

354390
NOTE: By default, a runner AMI update requires a re-apply of this terraform config (the runner AMI ID is looked up by a terraform data source). To avoid this, you can use `ami_id_ssm_parameter_name` to have the scale-up lambda dynamically lookup the runner AMI ID from an SSM parameter at instance launch time. Said SSM parameter is managed outside of this module (e.g. by a runner AMI build workflow).

examples/default/main.tf

-2
Original file line numberDiff line numberDiff line change
@@ -89,8 +89,6 @@ module "runners" {
8989

9090
# override scaling down
9191
scale_down_schedule_expression = "cron(* * * * ? *)"
92-
# enable this flag to publish webhook events to workflow job queue
93-
# enable_workflow_job_events_queue = true
9492

9593
enable_user_data_debug_logging_runner = true
9694

examples/multi-runner/main.tf

-3
Original file line numberDiff line numberDiff line change
@@ -97,9 +97,6 @@ module "runners" {
9797
# runner_binaries_syncer_lambda_zip = "../lambdas-download/runner-binaries-syncer.zip"
9898
# runners_lambda_zip = "../lambdas-download/runners.zip"
9999

100-
# enable_workflow_job_events_queue = true
101-
# override delay of events in seconds
102-
103100
# Enable debug logging for the lambda functions
104101
# log_level = "debug"
105102

lambdas/functions/webhook/jest.config.ts

+2-3
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,14 @@
11
import type { Config } from 'jest';
2-
32
import defaultConfig from '../../jest.base.config';
43

54
const config: Config = {
65
...defaultConfig,
76
coverageThreshold: {
87
global: {
9-
statements: 99.58,
8+
statements: 100,
109
branches: 100,
1110
functions: 100,
12-
lines: 99.57,
11+
lines: 100,
1312
},
1413
},
1514
};

lambdas/functions/webhook/src/ConfigLoader.test.ts

+1-3
Original file line numberDiff line numberDiff line change
@@ -94,9 +94,8 @@ describe('ConfigLoader Tests', () => {
9494
describe('ConfigWebhook', () => {
9595
it('should load config successfully', async () => {
9696
process.env.REPOSITORY_ALLOW_LIST = '["repo1", "repo2"]';
97-
process.env.SQS_WORKFLOW_JOB_QUEUE = 'secondary-queue';
98-
process.env.PARAMETER_RUNNER_MATCHER_CONFIG_PATH = '/path/to/matcher/config';
9997
process.env.PARAMETER_GITHUB_APP_WEBHOOK_SECRET = '/path/to/webhook/secret';
98+
process.env.PARAMETER_RUNNER_MATCHER_CONFIG_PATH = '/path/to/matcher/config';
10099
const matcherConfig = [
101100
{
102101
id: '1',
@@ -121,7 +120,6 @@ describe('ConfigLoader Tests', () => {
121120
const config: ConfigWebhook = await ConfigWebhook.load();
122121

123122
expect(config.repositoryAllowList).toEqual(['repo1', 'repo2']);
124-
expect(config.workflowJobEventSecondaryQueue).toBe('secondary-queue');
125123
expect(config.matcherConfig).toEqual(matcherConfig);
126124
expect(config.webhookSecret).toBe('secret');
127125
});

lambdas/functions/webhook/src/ConfigLoader.ts

-2
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,6 @@ export class ConfigWebhook extends BaseConfig {
9595

9696
async loadConfig(): Promise<void> {
9797
this.loadEnvVar(process.env.REPOSITORY_ALLOW_LIST, 'repositoryAllowList', []);
98-
this.loadEnvVar(process.env.SQS_WORKFLOW_JOB_QUEUE, 'workflowJobEventSecondaryQueue', '');
9998

10099
await Promise.all([
101100
this.loadParameter(process.env.PARAMETER_RUNNER_MATCHER_CONFIG_PATH, 'matcherConfig'),
@@ -129,7 +128,6 @@ export class ConfigDispatcher extends BaseConfig {
129128

130129
async loadConfig(): Promise<void> {
131130
this.loadEnvVar(process.env.REPOSITORY_ALLOW_LIST, 'repositoryAllowList', []);
132-
this.loadEnvVar(process.env.SQS_WORKFLOW_JOB_QUEUE, 'workflowJobEventSecondaryQueue', '');
133131
await this.loadParameter(process.env.PARAMETER_RUNNER_MATCHER_CONFIG_PATH, 'matcherConfig');
134132

135133
validateRunnerMatcherConfig(this);

lambdas/functions/webhook/src/modules.d.ts

-1
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,5 @@ declare namespace NodeJS {
77
REPOSITORY_ALLOW_LIST: string;
88
RUNNER_LABELS: string;
99
ACCEPT_EVENTS: string;
10-
SQS_WORKFLOW_JOB_QUEUE: string;
1110
}
1211
}

lambdas/functions/webhook/src/runners/dispatch.test.ts

-7
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import { logger } from '@aws-github-runner/aws-powertools-util';
1414
jest.mock('../sqs');
1515
jest.mock('@aws-github-runner/aws-ssm-util');
1616

17-
const sendWebhookEventToWorkflowJobQueueMock = jest.mocked(sendActionRequest);
1817
const GITHUB_APP_WEBHOOK_SECRET = 'TEST_SECRET';
1918

2019
const cleanEnv = process.env;
@@ -56,7 +55,6 @@ describe('Dispatcher', () => {
5655
statusCode: 403,
5756
});
5857
expect(sendActionRequest).not.toHaveBeenCalled();
59-
expect(sendWebhookEventToWorkflowJobQueueMock).not.toHaveBeenCalled();
6058
});
6159

6260
it('should handle workflow_job events without installation id', async () => {
@@ -65,7 +63,6 @@ describe('Dispatcher', () => {
6563
const resp = await dispatch(event, 'workflow_job', config);
6664
expect(resp.statusCode).toBe(201);
6765
expect(sendActionRequest).toHaveBeenCalled();
68-
expect(sendWebhookEventToWorkflowJobQueueMock).toHaveBeenCalled();
6966
});
7067

7168
it('should handle workflow_job events from allow listed repositories', async () => {
@@ -74,7 +71,6 @@ describe('Dispatcher', () => {
7471
const resp = await dispatch(event, 'workflow_job', config);
7572
expect(resp.statusCode).toBe(201);
7673
expect(sendActionRequest).toHaveBeenCalled();
77-
expect(sendWebhookEventToWorkflowJobQueueMock).toHaveBeenCalled();
7874
});
7975

8076
it('should match labels', async () => {
@@ -108,7 +104,6 @@ describe('Dispatcher', () => {
108104
queueFifo: false,
109105
repoOwnerType: 'Organization',
110106
});
111-
expect(sendWebhookEventToWorkflowJobQueueMock).toHaveBeenCalled();
112107
});
113108

114109
it('should sort matcher with exact first.', async () => {
@@ -157,7 +152,6 @@ describe('Dispatcher', () => {
157152
queueFifo: false,
158153
repoOwnerType: 'Organization',
159154
});
160-
expect(sendWebhookEventToWorkflowJobQueueMock).toHaveBeenCalled();
161155
});
162156

163157
it('should not accept jobs where not all labels are supported (single matcher).', async () => {
@@ -181,7 +175,6 @@ describe('Dispatcher', () => {
181175
const resp = await dispatch(event, 'workflow_job', config);
182176
expect(resp.statusCode).toBe(202);
183177
expect(sendActionRequest).not.toHaveBeenCalled();
184-
expect(sendWebhookEventToWorkflowJobQueueMock).not.toHaveBeenCalled();
185178
});
186179
});
187180

lambdas/functions/webhook/src/runners/dispatch.ts

+2-5
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { createChildLogger } from '@aws-github-runner/aws-powertools-util';
22
import { WorkflowJobEvent } from '@octokit/webhooks-types';
33

44
import { Response } from '../lambda';
5-
import { RunnerMatcherConfig, sendActionRequest, sendWebhookEventToWorkflowJobQueue } from '../sqs';
5+
import { RunnerMatcherConfig, sendActionRequest } from '../sqs';
66
import ValidationError from '../ValidationError';
77
import { ConfigDispatcher, ConfigWebhook } from '../ConfigLoader';
88

@@ -15,10 +15,7 @@ export async function dispatch(
1515
): Promise<Response> {
1616
validateRepoInAllowList(event, config);
1717

18-
const result = await handleWorkflowJob(event, eventType, config.matcherConfig!);
19-
await sendWebhookEventToWorkflowJobQueue({ workflowJobEvent: event }, config);
20-
21-
return result;
18+
return await handleWorkflowJob(event, eventType, config.matcherConfig!);
2219
}
2320

2421
function validateRepoInAllowList(event: WorkflowJobEvent, config: ConfigDispatcher) {
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,5 @@
11
import { SendMessageCommandInput } from '@aws-sdk/client-sqs';
2-
3-
import { ActionRequestMessage, GithubWorkflowEvent, sendActionRequest, sendWebhookEventToWorkflowJobQueue } from '.';
4-
import workflowjob_event from '../../test/resources/github_workflowjob_event.json';
5-
import { getParameter } from '@aws-github-runner/aws-ssm-util';
6-
import { mocked } from 'jest-mock';
2+
import { ActionRequestMessage, sendActionRequest } from '.';
73

84
const mockSQS = {
95
sendMessage: jest.fn(() => {
@@ -15,9 +11,6 @@ jest.mock('@aws-sdk/client-sqs', () => ({
1511
}));
1612
jest.mock('@aws-github-runner/aws-ssm-util');
1713

18-
import { SQS } from '@aws-sdk/client-sqs';
19-
import { ConfigDispatcher, ConfigWebhook } from '../ConfigLoader';
20-
2114
describe('Test sending message to SQS.', () => {
2215
const queueUrl = 'https://sqs.eu-west-1.amazonaws.com/123456789/queued-builds';
2316
const message = {
@@ -72,62 +65,3 @@ describe('Test sending message to SQS.', () => {
7265
await expect(result).resolves.not.toThrow();
7366
});
7467
});
75-
76-
describe('Test sending message to SQS.', () => {
77-
const message: GithubWorkflowEvent = {
78-
workflowJobEvent: JSON.parse(JSON.stringify(workflowjob_event)),
79-
};
80-
const sqsMessage: SendMessageCommandInput = {
81-
QueueUrl: 'https://sqs.eu-west-1.amazonaws.com/123456789/webhook_events_workflow_job_queue',
82-
MessageBody: JSON.stringify(message),
83-
};
84-
beforeEach(() => {
85-
ConfigDispatcher.reset();
86-
const mockedGet = mocked(getParameter);
87-
mockedGet.mockResolvedValue('["abc"]');
88-
});
89-
afterEach(() => {
90-
jest.clearAllMocks();
91-
});
92-
93-
it('sends webhook events to workflow job queue', async () => {
94-
// Arrange
95-
process.env.SQS_WORKFLOW_JOB_QUEUE = sqsMessage.QueueUrl || '';
96-
const config: ConfigWebhook = await ConfigWebhook.load();
97-
98-
// Act
99-
const result = sendWebhookEventToWorkflowJobQueue(message, config);
100-
101-
// Assert
102-
expect(mockSQS.sendMessage).toHaveBeenCalledWith(sqsMessage);
103-
await expect(result).resolves.not.toThrow();
104-
});
105-
106-
it('Does not send webhook events to workflow job event copy queue when job queue is not in environment', async () => {
107-
// Arrange
108-
process.env.SQS_WORKFLOW_JOB_QUEUE = '';
109-
const config: ConfigDispatcher = await ConfigDispatcher.load();
110-
111-
// Act
112-
await sendWebhookEventToWorkflowJobQueue(message, config);
113-
114-
// Assert
115-
expect(SQS).not.toHaveBeenCalled();
116-
});
117-
118-
it('Catch the exception when even copy queue throws exception', async () => {
119-
// Arrange
120-
process.env.SQS_WORKFLOW_JOB_QUEUE = sqsMessage.QueueUrl || '';
121-
const config: ConfigDispatcher = await ConfigDispatcher.load();
122-
123-
const mockSQS = {
124-
sendMessage: jest.fn(() => {
125-
throw new Error();
126-
}),
127-
};
128-
jest.mock('aws-sdk', () => ({
129-
SQS: jest.fn().mockImplementation(() => mockSQS),
130-
}));
131-
await expect(sendWebhookEventToWorkflowJobQueue(message, config)).resolves.not.toThrow();
132-
});
133-
});

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

-24
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import { SQS, SendMessageCommandInput } from '@aws-sdk/client-sqs';
22
import { WorkflowJobEvent } from '@octokit/webhooks-types';
33
import { createChildLogger, getTracedAWSV3Client } from '@aws-github-runner/aws-powertools-util';
4-
import { ConfigDispatcher } from '../ConfigLoader';
54

65
const logger = createChildLogger('sqs');
76

@@ -49,26 +48,3 @@ export const sendActionRequest = async (message: ActionRequestMessage): Promise<
4948

5049
await sqs.sendMessage(sqsMessage);
5150
};
52-
53-
export async function sendWebhookEventToWorkflowJobQueue(
54-
message: GithubWorkflowEvent,
55-
config: ConfigDispatcher,
56-
): Promise<void> {
57-
if (!config.workflowJobEventSecondaryQueue) {
58-
return;
59-
}
60-
61-
const sqs = new SQS({ region: process.env.AWS_REGION });
62-
const sqsMessage: SendMessageCommandInput = {
63-
QueueUrl: String(config.workflowJobEventSecondaryQueue),
64-
MessageBody: JSON.stringify(message),
65-
};
66-
67-
logger.info(`Sending event to the workflow job queue: ${config.workflowJobEventSecondaryQueue}`);
68-
69-
try {
70-
await sqs.sendMessage(sqsMessage);
71-
} catch (e) {
72-
logger.warn(`Error in sending webhook events to workflow job queue: ${(e as Error).message}`);
73-
}
74-
}

0 commit comments

Comments
 (0)