Merge branch 'main' into conroy/prlint1

packages/aws-cdk-lib/aws-sqs/lib/queue.ts

@@ -1,7 +1,7 @@
 import { Construct } from 'constructs';
 import { IQueue, QueueAttributes, QueueBase, QueueEncryption } from './queue-base';
 import { CfnQueue } from './sqs.generated';
-import { validateProps } from './validate-props';
+import { validateQueueProps, validateRedriveAllowPolicy } from './validate-queue-props';
 import * as iam from '../../aws-iam';
 import * as kms from '../../aws-kms';
 import { Duration, RemovalPolicy, Stack, Token, ArnFormat, Annotations } from '../../core';
@@ -383,20 +383,10 @@ export class Queue extends QueueBase {
       physicalName: props.queueName,
     });
 
-    validateProps(this, props);
+    validateQueueProps(this, props);
 
     if (props.redriveAllowPolicy) {
-      const { redrivePermission, sourceQueues } = props.redriveAllowPolicy;
-      if (redrivePermission === RedrivePermission.BY_QUEUE) {
-        if (!sourceQueues || sourceQueues.length === 0) {
-          throw new ValidationError('At least one source queue must be specified when RedrivePermission is set to \'byQueue\'', this);
-        }
-        if (sourceQueues && sourceQueues.length > 10) {
-          throw new ValidationError('Up to 10 sourceQueues can be specified. Set RedrivePermission to \'allowAll\' to specify more', this);
-        }
-      } else if (redrivePermission && sourceQueues) {
-        throw new ValidationError('sourceQueues cannot be configured when RedrivePermission is set to \'allowAll\' or \'denyAll\'', this);
-      }
+      validateRedriveAllowPolicy(this, props.redriveAllowPolicy);
     }
 
     const redrivePolicy = props.deadLetterQueue

packages/aws-cdk-lib/aws-sqs/lib/validate-queue-props.ts

@@ -0,0 +1,61 @@
+import { Construct } from 'constructs';
+import { Queue, QueueProps, RedriveAllowPolicy, RedrivePermission } from './index';
+import { Token } from '../../core';
+import { validateAllProps, ValidationRule } from '../../core/lib/helpers-internal';
+
+function validateRange(value: number | undefined, minValue: number, maxValue: number): boolean {
+  return value !== undefined && !Token.isUnresolved(value) && (value < minValue || value > maxValue);
+}
+
+const queueValidationRules: ValidationRule<QueueProps>[] = [
+  {
+    condition: (props) => validateRange(props.deliveryDelay?.toSeconds(), 0, 900),
+    message: (props) => `delivery delay must be between 0 and 900 seconds, but ${props.deliveryDelay?.toSeconds()} was provided`,
+  },
+  {
+    condition: (props) => validateRange(props.maxMessageSizeBytes, 1_024, 262_144),
+    message: (props) => `maximum message size must be between 1,024 and 262,144 bytes, but ${props.maxMessageSizeBytes} was provided`,
+  },
+  {
+    condition: (props) => validateRange(props.retentionPeriod?.toSeconds(), 60, 1_209_600),
+    message: (props) => `message retention period must be between 60 and 1,209,600 seconds, but ${props.retentionPeriod?.toSeconds()} was provided`,
+  },
+  {
+    condition: (props) => validateRange(props.receiveMessageWaitTime?.toSeconds(), 0, 20),
+    message: (props) => `receive wait time must be between 0 and 20 seconds, but ${props.receiveMessageWaitTime?.toSeconds()} was provided`,
+  },
+  {
+    condition: (props) => validateRange(props.visibilityTimeout?.toSeconds(), 0, 43_200),
+    message: (props) => `visibility timeout must be between 0 and 43,200 seconds, but ${props.visibilityTimeout?.toSeconds()} was provided`,
+  },
+  {
+    condition: (props) => validateRange(props.deadLetterQueue?.maxReceiveCount, 1, Number.MAX_SAFE_INTEGER),
+    message: (props) => `dead letter target maximum receive count must be 1 or more, but ${props.deadLetterQueue?.maxReceiveCount} was provided`,
+  },
+];
+
+const redriveValidationRules: ValidationRule<RedriveAllowPolicy>[] = [
+  {
+    condition: ({ redrivePermission, sourceQueues }) =>
+      redrivePermission === RedrivePermission.BY_QUEUE && (!sourceQueues || sourceQueues.length === 0),
+    message: () => 'At least one source queue must be specified when RedrivePermission is set to \'byQueue\'',
+  },
+  {
+    condition: ({ redrivePermission, sourceQueues }) =>
+      !!(redrivePermission === RedrivePermission.BY_QUEUE && sourceQueues && sourceQueues.length > 10),
+    message: () => 'Up to 10 sourceQueues can be specified. Set RedrivePermission to \'allowAll\' to specify more',
+  },
+  {
+    condition: ({ redrivePermission, sourceQueues }) =>
+      !!((redrivePermission === RedrivePermission.ALLOW_ALL || redrivePermission === RedrivePermission.DENY_ALL) && sourceQueues),
+    message: () => 'sourceQueues cannot be configured when RedrivePermission is set to \'allowAll\' or \'denyAll\'',
+  },
+];
+
+export function validateQueueProps(scope: Construct, props: QueueProps) {
+  validateAllProps(scope, Queue.name, props, queueValidationRules);
+}
+
+export function validateRedriveAllowPolicy(scope: Construct, policy: RedriveAllowPolicy) {
+  validateAllProps(scope, Queue.name, policy, redriveValidationRules);
+}

packages/aws-cdk-lib/aws-sqs/test/sqs.test.ts

@@ -3,6 +3,7 @@ import * as iam from '../../aws-iam';
 import * as kms from '../../aws-kms';
 import { CfnParameter, Duration, Stack, App, Token } from '../../core';
 import * as sqs from '../lib';
+import { validateRedriveAllowPolicy } from '../lib/validate-queue-props';
 
 /* eslint-disable quote-props */
 
@@ -62,18 +63,29 @@ test('with a dead letter queue', () => {
   expect(queue.deadLetterQueue).toEqual(dlqProps);
 });
 
+test('multiple prop validation errors are presented to the user (out-of-range retentionPeriod and deliveryDelay)', () => {
+  // GIVEN
+  const stack = new Stack();
+
+  // THEN
+  expect(() => new sqs.Queue(stack, 'MyQueue', {
+    retentionPeriod: Duration.seconds(30),
+    deliveryDelay: Duration.minutes(16),
+  })).toThrow('Queue initialization failed due to the following validation error(s):\n- delivery delay must be between 0 and 900 seconds, but 960 was provided\n- message retention period must be between 60 and 1,209,600 seconds, but 30 was provided');
+});
+
 test('message retention period must be between 1 minute to 14 days', () => {
   // GIVEN
   const stack = new Stack();
 
   // THEN
   expect(() => new sqs.Queue(stack, 'MyQueue', {
     retentionPeriod: Duration.seconds(30),
-  })).toThrow(/message retention period must be 60 seconds or more/);
+  })).toThrow('Queue initialization failed due to the following validation error(s):\n- message retention period must be between 60 and 1,209,600 seconds, but 30 was provided');
 
   expect(() => new sqs.Queue(stack, 'AnotherQueue', {
     retentionPeriod: Duration.days(15),
-  })).toThrow(/message retention period must be 1209600 seconds or less/);
+  })).toThrow('Queue initialization failed due to the following validation error(s):\n- message retention period must be between 60 and 1,209,600 seconds, but 1296000 was provided');
 });
 
 test('message retention period can be provided as a parameter', () => {
@@ -155,6 +167,65 @@ test('addToPolicy will automatically create a policy for this queue', () => {
   });
 });
 
+describe('validateRedriveAllowPolicy', () => {
+  test('does not throw for valid policy', () => {
+    // GIVEN
+    const stack = new Stack();
+
+    // WHEN
+    const redriveAllowPolicy = { redrivePermission: sqs.RedrivePermission.ALLOW_ALL };
+
+    // THEN
+    expect(() => validateRedriveAllowPolicy(stack, redriveAllowPolicy)).not.toThrow();
+  });
+
+  test('throws when sourceQueues is provided with ALLOW_ALL permission', () => {
+    // GIVEN
+    const stack = new Stack();
+
+    // WHEN
+    const sourceQueue = new sqs.Queue(stack, 'SourceQueue');
+    const redriveAllowPolicy = {
+      redrivePermission: sqs.RedrivePermission.ALLOW_ALL,
+      sourceQueues: [sourceQueue],
+    };
+
+    // THEN
+    expect(() => validateRedriveAllowPolicy(stack, redriveAllowPolicy))
+      .toThrow("Queue initialization failed due to the following validation error(s):\n- sourceQueues cannot be configured when RedrivePermission is set to 'allowAll' or 'denyAll'");
+  });
+
+  test('throws when sourceQueues is not provided with BY_QUEUE permission', () => {
+    // GIVEN
+    const stack = new Stack();
+
+    // WHEN
+    const redriveAllowPolicy = {
+      redrivePermission: sqs.RedrivePermission.BY_QUEUE,
+    };
+
+    // THEN
+    expect(() => validateRedriveAllowPolicy(stack, redriveAllowPolicy))
+      .toThrow("Queue initialization failed due to the following validation error(s):\n- At least one source queue must be specified when RedrivePermission is set to 'byQueue'");
+  });
+
+  test('throws when more than 10 sourceQueues are provided', () => {
+    // GIVEN
+    const stack = new Stack();
+
+    // WHEN
+    const sourceQueues = Array(11).fill(null).map((_, i) => new sqs.Queue(stack, `SourceQueue${i}`));
+    const redriveAllowPolicy = {
+      redrivePermission: sqs.RedrivePermission.BY_QUEUE,
+      sourceQueues,
+    };
+
+    // THEN
+    expect(() => validateRedriveAllowPolicy(stack, redriveAllowPolicy))
+      .toThrow("Queue initialization failed due to the following validation error(s):\n- Up to 10 sourceQueues can be specified. Set RedrivePermission to 'allowAll' to specify more");
+  });
+});
+
 describe('export and import', () => {
   test('importing works correctly', () => {
     // GIVEN

tools/@aws-cdk/prlint/lint.ts

@@ -411,7 +411,7 @@ function validateTitlePrefix(pr: GitHubPr): TestResult {
 }
 
 /**
- * Check that the PR title uses the typical convention for package names.
+ * Check that the PR title uses the typical convention for package names, and is lowercase.
  *
  * For example, "fix(s3)" is preferred over "fix(aws-s3)".
  */
@@ -428,7 +428,17 @@ function validateTitleScope(pr: GitHubPr): TestResult {
   if (m && !scopesExemptFromThisRule.includes(m[2])) {
     result.assessFailure(
       !!(m[2] && m[3]),
-      `The title of the pull request should omit 'aws-' from the name of modified packages. Use '${m[3]}' instead of '${m[2]}'.`,
+      `The title scope of the pull request should omit 'aws-' from the name of modified packages. Use '${m[3]}' instead of '${m[2]}'.`,
+    );
+  }
+
+  // Title scope is lowercase
+  const scopeRe = /^\w+\(([\w_-]+)\)?: /; // Isolate the scope
+  const scope = scopeRe.exec(pr.title);
+  if (scope && scope[1]) {
+    result.assessFailure(
+      scope[1] !== scope[1].toLocaleLowerCase(),
+      `The title scope of the pull request should be entirely in lowercase. Use '${scope[1].toLocaleLowerCase()}' instead.`,
     );
   }
   return result;

tools/@aws-cdk/prlint/test/lint.test.ts

@@ -142,7 +142,25 @@ describe('commit message format', () => {
       },
     };
     const prLinter = configureMock(issue, undefined);
-    await expect(legacyValidatePullRequestTarget(prLinter)).rejects.toThrow(/The title of the pull request should omit 'aws-' from the name of modified packages. Use 's3' instead of 'aws-s3'./);
+    await expect(legacyValidatePullRequestTarget(prLinter)).rejects.toThrow(/The title scope of the pull request should omit 'aws-' from the name of modified packages. Use 's3' instead of 'aws-s3'./);
+  });
+
+  test('invalid scope with capital letters', async () => {
+    const issue = {
+      number: 1,
+      title: 'fix(S3): some title',
+      body: '',
+      labels: [{ name: 'pr-linter/exempt-test' }, { name: 'pr-linter/exempt-integ-test' }],
+      user: {
+        login: 'author',
+      },
+    };
+    const prLinter = configureMock(issue, undefined);
+    await expect(prLinter.validatePullRequestTarget()).resolves.toEqual(expect.objectContaining({
+      requestChanges: expect.objectContaining({
+        failures: ["The title scope of the pull request should be entirely in lowercase. Use 's3' instead."],
+      }),
+    }));
   });
 
   test('valid scope with aws- in summary and body', async () => {