Skip to content

Commit

Permalink
fix(sqs): unable to use CfnParameter 'valueAsNumber' to specify queue…
Browse files Browse the repository at this point in the history
… properties (#8252)

validation that was being performed was not taking into account that tokens
could be provided for these parameters. added a check and some tests to
allow parameters to be supplied.

Fixes #7126

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
shivlaks authored Jun 1, 2020
1 parent 7d60681 commit 8ec405f
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 2 deletions.
3 changes: 2 additions & 1 deletion packages/@aws-cdk/aws-sqs/lib/validate-props.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { Token } from '@aws-cdk/core';
import { QueueProps } from './index';

export function validateProps(props: QueueProps) {
Expand All @@ -10,7 +11,7 @@ export function validateProps(props: QueueProps) {
}

function validateRange(label: string, value: number | undefined, minValue: number, maxValue: number, unit?: string) {
if (value === undefined) { return; }
if (value === undefined || Token.isUnresolved(value)) { return; }
const unitSuffix = unit ? ` ${unit}` : '';
if (value < minValue) { throw new Error(`${label} must be ${minValue}${unitSuffix} or more, but ${value} was provided`); }
if (value > maxValue) { throw new Error(`${label} must be ${maxValue}${unitSuffix} of less, but ${value} was provided`); }
Expand Down
54 changes: 53 additions & 1 deletion packages/@aws-cdk/aws-sqs/test/test.sqs.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { expect, haveResource } from '@aws-cdk/assert';
import * as iam from '@aws-cdk/aws-iam';
import * as kms from '@aws-cdk/aws-kms';
import { Duration, Stack } from '@aws-cdk/core';
import { CfnParameter, Duration, Stack } from '@aws-cdk/core';
import { Test } from 'nodeunit';
import * as sqs from '../lib';

Expand Down Expand Up @@ -54,6 +54,58 @@ export = {
test.done();
},

'message retention period must be between 1 minute to 14 days'(test: Test) {
// GIVEN
const stack = new Stack();

// THEN
test.throws(() => new sqs.Queue(stack, 'MyQueue', {
retentionPeriod: Duration.seconds(30),
}), /message retention period must be 60 seconds or more/);

test.throws(() => new sqs.Queue(stack, 'AnotherQueue', {
retentionPeriod: Duration.days(15),
}), /message retention period must be 1209600 seconds of less/);

test.done();
},

'message retention period can be provided as a parameter'(test: Test) {
// GIVEN
const stack = new Stack();
const parameter = new CfnParameter(stack, 'my-retention-period', {
type: 'Number',
default: 30,
});

// WHEN
new sqs.Queue(stack, 'MyQueue', {
retentionPeriod: Duration.seconds(parameter.valueAsNumber),
});

// THEN
expect(stack).toMatch({
'Parameters': {
'myretentionperiod': {
'Type': 'Number',
'Default': 30,
},
},
'Resources': {
'MyQueueE6CA6235': {
'Type': 'AWS::SQS::Queue',
'Properties': {
'MessageRetentionPeriod': {
'Ref': 'myretentionperiod',
},
},
},
},
});

test.done();
},

'addToPolicy will automatically create a policy for this queue'(test: Test) {
const stack = new Stack();
const queue = new sqs.Queue(stack, 'MyQueue');
Expand Down

0 comments on commit 8ec405f

Please sign in to comment.