fix(webhook): Don't log warning when secondary job queue is empty (#3942)

Right now the Terraform module is causing `${SQS_WORKFLOW_JOB_QUEUE}` to
be set to an empty string. Since we pass this through currently, and
explicitly check for `!== undefined` - not any falsy value - we end up
trying to send to an empty queue and logging a warning.

Doesn't break anything, but it's noisy in the logs.

Fix this by checking for any falsy value instead, and also using `||`
instead of `??` when setting the variable in the first place, so an
empty string ends up as `undefined`. Also, modify the testsuite to check
for the `SQS` being created at all, since that happens earlier on and
reproduces the failure.

This is a companion to #3943. This one stops the warning, and that one
fixes the original cause by not setting the env var in the first place.
Both could be merged, ideally.

Co-authored-by: Niek Palm <[email protected]>

Author: iainlane

Diff for: lambdas/functions/webhook/src/ConfigResolver.ts

@@ -29,7 +29,7 @@ export class Config {
       Config.matcherConfig = JSON.parse(matcherConfigVal) as Array<RunnerMatcherConfig>;
       logger.debug('Loaded queues config', { matcherConfig: Config.matcherConfig });
     }
-    const workflowJobEventSecondaryQueue = process.env.SQS_WORKFLOW_JOB_QUEUE ?? undefined;
+    const workflowJobEventSecondaryQueue = process.env.SQS_WORKFLOW_JOB_QUEUE || undefined;
     return new Config(repositoryAllowList, workflowJobEventSecondaryQueue);
   }
 

Diff for: lambdas/functions/webhook/src/sqs/index.test.ts

@@ -16,6 +16,8 @@ jest.mock('@aws-sdk/client-sqs', () => ({
 }));
 jest.mock('@terraform-aws-github-runner/aws-ssm-util');
 
+import { SQS } from '@aws-sdk/client-sqs';
+
 describe('Test sending message to SQS.', () => {
   const queueUrl = 'https://sqs.eu-west-1.amazonaws.com/123456789/queued-builds';
   const message = {
@@ -98,15 +100,27 @@ describe('Test sending message to SQS.', () => {
     expect(result).resolves;
   });
 
-  it('Does not send webhook events to workflow job event copy queue', async () => {
+  it('Does not send webhook events to workflow job event copy queue when job queue is not in environment', async () => {
+    // Arrange
+    delete process.env.SQS_WORKFLOW_JOB_QUEUE;
+    const config = await Config.load();
+
+    // Act
+    await sendWebhookEventToWorkflowJobQueue(message, config);
+
+    // Assert
+    expect(SQS).not.toHaveBeenCalled();
+  });
+
+  it('Does not send webhook events to workflow job event copy queue when job queue is set to empty string', async () => {
     // Arrange
     process.env.SQS_WORKFLOW_JOB_QUEUE = '';
     const config = await Config.load();
     // Act
     await sendWebhookEventToWorkflowJobQueue(message, config);
 
     // Assert
-    expect(mockSQS.sendMessage).not.toHaveBeenCalledWith(sqsMessage);
+    expect(SQS).not.toHaveBeenCalled();
   });
 
   it('Catch the exception when even copy queue throws exception', async () => {

Diff for: lambdas/functions/webhook/src/sqs/index.ts

@@ -50,17 +50,21 @@ export const sendActionRequest = async (message: ActionRequestMessage): Promise<
 };
 
 export async function sendWebhookEventToWorkflowJobQueue(message: GithubWorkflowEvent, config: Config): Promise<void> {
-  if (config.workflowJobEventSecondaryQueue != undefined) {
-    const sqs = new SQS({ region: process.env.AWS_REGION });
-    const sqsMessage: SendMessageCommandInput = {
-      QueueUrl: String(config.workflowJobEventSecondaryQueue),
-      MessageBody: JSON.stringify(message),
-    };
-    logger.debug(`Sending Webhook events to the workflow job queue: ${config.workflowJobEventSecondaryQueue}`);
-    try {
-      await sqs.sendMessage(sqsMessage);
-    } catch (e) {
-      logger.warn(`Error in sending webhook events to workflow job queue: ${(e as Error).message}`);
-    }
+  if (!config.workflowJobEventSecondaryQueue) {
+    return;
+  }
+
+  const sqs = new SQS({ region: process.env.AWS_REGION });
+  const sqsMessage: SendMessageCommandInput = {
+    QueueUrl: String(config.workflowJobEventSecondaryQueue),
+    MessageBody: JSON.stringify(message),
+  };
+
+  logger.debug(`Sending Webhook events to the workflow job queue: ${config.workflowJobEventSecondaryQueue}`);
+
+  try {
+    await sqs.sendMessage(sqsMessage);
+  } catch (e) {
+    logger.warn(`Error in sending webhook events to workflow job queue: ${(e as Error).message}`);
   }
 }