From 1cdc132e9d8f8383b3587a07c377a07357ea8cb2 Mon Sep 17 00:00:00 2001 From: "joakim.wanggren@schibsted.com" Date: Fri, 17 Mar 2023 09:27:05 +0100 Subject: [PATCH 1/4] Fix getlift/lift#208, allow for batchSize above 10 --- docs/queue.md | 3 ++- src/constructs/aws/Queue.ts | 19 +++++++++++++- test/unit/queues.test.ts | 49 +++++++++++++++++++++++++++++++++++++ 3 files changed, 69 insertions(+), 2 deletions(-) diff --git a/docs/queue.md b/docs/queue.md index b6c69480..65124b47 100644 --- a/docs/queue.md +++ b/docs/queue.md @@ -360,7 +360,8 @@ By default, Lift configures Lambda to be invoked with 1 messages at a time. The Note you can use [partial batch failures](#partial-batch-failures) to avoid failing the whole batch. -It is possible to set the batch size between 1 and 10. +It is possible to set the batch size between 1 and 10 for FIFO queues and 10000 for regular queues. +For batch size over 10, [maxBatchingWindow](#maximum-batching-window) must be set. ### Max Concurrency diff --git a/src/constructs/aws/Queue.ts b/src/constructs/aws/Queue.ts index 5d74f86a..48f84e5c 100644 --- a/src/constructs/aws/Queue.ts +++ b/src/constructs/aws/Queue.ts @@ -43,7 +43,7 @@ const QUEUE_DEFINITION = { batchSize: { type: "number", minimum: 1, - maximum: 10, + maximum: 10000, }, maxBatchingWindow: { type: "number", @@ -167,6 +167,23 @@ export class Queue extends AwsConstruct { delay = Duration.seconds(configuration.delay); } + if (configuration.batchSize !== undefined) { + if (configuration.batchSize > 10 && configuration.fifo === true) { + throw new ServerlessError( + `Invalid configuration in 'constructs.${this.id}': 'batchSize' must be between 0 and 10 for FIFO queues, '${configuration.batchSize}' given.`, + "LIFT_INVALID_CONSTRUCT_CONFIGURATION" + ); + } + if (configuration.batchSize > 10 && !this.getMaximumBatchingWindow()) { + throw new ServerlessError( + `Invalid configuration in 'constructs.${ + this.id + }': 'maxBatchingWindow' must be greater than 0 for batchSize > 10, '${this.getMaximumBatchingWindow()}' given.`, + "LIFT_INVALID_CONSTRUCT_CONFIGURATION" + ); + } + } + let encryption = undefined; if (isNil(configuration.encryption) || configuration.encryption.length === 0) { encryption = {}; diff --git a/test/unit/queues.test.ts b/test/unit/queues.test.ts index 0a1b4a03..56c6d477 100644 --- a/test/unit/queues.test.ts +++ b/test/unit/queues.test.ts @@ -700,4 +700,53 @@ describe("queues", () => { ); } }); + + it("should throw if batch size is over 10 for FIFO queues", async () => { + expect.assertions(2); + + try { + await runServerless({ + fixture: "queues", + configExt: merge({}, pluginConfigExt, { + constructs: { + emails: { + batchSize: 100, + fifo: true, + }, + }, + }), + command: "package", + }); + } catch (error) { + expect(error).toBeInstanceOf(ServerlessError); + expect(error).toHaveProperty( + "message", + "Invalid configuration in 'constructs.emails': 'batchSize' must be between 0 and 10 for FIFO queues, '100' given." + ); + } + }); + + it("should throw if batch size is over 10 and maxBatchWindow is not set", async () => { + expect.assertions(2); + + try { + await runServerless({ + fixture: "queues", + configExt: merge({}, pluginConfigExt, { + constructs: { + emails: { + batchSize: 100, + }, + }, + }), + command: "package", + }); + } catch (error) { + expect(error).toBeInstanceOf(ServerlessError); + expect(error).toHaveProperty( + "message", + "Invalid configuration in 'constructs.emails': 'maxBatchingWindow' must be greater than 0 for batchSize > 10, '0' given." + ); + } + }); }); From cacb09bad5e5cca9f92558ff260c872a58220bd8 Mon Sep 17 00:00:00 2001 From: Chris Hanline Date: Mon, 5 Jun 2023 19:41:33 -0500 Subject: [PATCH 2/4] dlq alarm treat missing as not breaching --- src/constructs/aws/Queue.ts | 3 ++- test/unit/queues.test.ts | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/constructs/aws/Queue.ts b/src/constructs/aws/Queue.ts index 5d74f86a..fa4d0c47 100644 --- a/src/constructs/aws/Queue.ts +++ b/src/constructs/aws/Queue.ts @@ -3,7 +3,7 @@ import type { CfnQueue } from "aws-cdk-lib/aws-sqs"; import { Queue as CdkQueue, QueueEncryption } from "aws-cdk-lib/aws-sqs"; import type { FromSchema } from "json-schema-to-ts"; import type { CfnAlarm } from "aws-cdk-lib/aws-cloudwatch"; -import { Alarm, ComparisonOperator, Metric } from "aws-cdk-lib/aws-cloudwatch"; +import { Alarm, ComparisonOperator, Metric, TreatMissingData } from "aws-cdk-lib/aws-cloudwatch"; import { Subscription, SubscriptionProtocol, Topic } from "aws-cdk-lib/aws-sns"; import type { AlarmActionConfig } from "aws-cdk-lib/aws-cloudwatch/lib/alarm-action"; import type { Construct as CdkConstruct } from "constructs"; @@ -241,6 +241,7 @@ export class Queue extends AwsConstruct { // Alert as soon as we have 1 message in the DLQ threshold: 0, comparisonOperator: ComparisonOperator.GREATER_THAN_THRESHOLD, + treatMissingData: TreatMissingData.NOT_BREACHING, }); this.alarm.addAlarmAction({ bind(): AlarmActionConfig { diff --git a/test/unit/queues.test.ts b/test/unit/queues.test.ts index 0a1b4a03..09dfad1a 100644 --- a/test/unit/queues.test.ts +++ b/test/unit/queues.test.ts @@ -411,6 +411,7 @@ describe("queues", () => { Period: 60, Statistic: "Sum", Threshold: 0, + TreatMissingData: "notBreaching", }, }); expect(cfTemplate.Resources[computeLogicalId("emails", "AlarmTopic")]).toMatchObject({ From 8609c1edd6eaea316102350d2b122871631448a7 Mon Sep 17 00:00:00 2001 From: Aran Reeks Date: Fri, 9 Jun 2023 17:54:34 +0100 Subject: [PATCH 3/4] Prevent the block on redirects Far better way to handle the same solution. Plus it fixes the CI --- src/classes/cloudfrontFunctions.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/classes/cloudfrontFunctions.ts b/src/classes/cloudfrontFunctions.ts index c68afda0..d2f6672e 100644 --- a/src/classes/cloudfrontFunctions.ts +++ b/src/classes/cloudfrontFunctions.ts @@ -2,10 +2,7 @@ import ServerlessError from "../utils/error"; export function redirectToMainDomain(domains: string[] | undefined): string { if (domains === undefined || domains.length < 2) { - throw new ServerlessError( - `Invalid value in 'redirectToMainDomain': you must have at least 2 domains configured to enable redirection to the main domain.`, - "LIFT_INVALID_CONSTRUCT_CONFIGURATION" - ); + return ""; } const mainDomain = domains[0]; From e0d9a29583092dfd4595824319296cd290401126 Mon Sep 17 00:00:00 2001 From: Craig McNicholas <41394833+cmcnicholas@users.noreply.github.com> Date: Sun, 11 Jun 2023 10:58:07 +0100 Subject: [PATCH 4/4] fix: adds `dynamodb:ConditionCheckItem` permission to dynamodb construct Without this IAM permission, transacted writes in dynamodb using the `ConditionCheck` feature are disallowed. See: https://docs.aws.amazon.com/amazondynamodb/latest/APIReference/API_ConditionCheck.html https://docs.aws.amazon.com/amazondynamodb/latest/developerguide/transaction-apis-iam.html --- src/constructs/aws/DatabaseDynamoDBSingleTable.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/constructs/aws/DatabaseDynamoDBSingleTable.ts b/src/constructs/aws/DatabaseDynamoDBSingleTable.ts index 54e9c6ef..6be1f9a8 100644 --- a/src/constructs/aws/DatabaseDynamoDBSingleTable.ts +++ b/src/constructs/aws/DatabaseDynamoDBSingleTable.ts @@ -86,6 +86,7 @@ export class DatabaseDynamoDBSingleTable extends AwsConstruct { "dynamodb:DeleteItem", "dynamodb:BatchWriteItem", "dynamodb:UpdateItem", + "dynamodb:ConditionCheckItem", ], [this.table.tableArn, Stack.of(this).resolve(Fn.join("/", [this.table.tableArn, "index", "*"]))] ),