Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unable to use CfnParameter valueAsNumber in SQS Queue #7126

Closed
amwill04 opened this issue Apr 1, 2020 · 5 comments · Fixed by #8252
Closed

Unable to use CfnParameter valueAsNumber in SQS Queue #7126

amwill04 opened this issue Apr 1, 2020 · 5 comments · Fixed by #8252
Assignees
Labels
@aws-cdk/core Related to core CDK functionality bug This issue is a bug. in-progress This issue is being actively worked on. p1

Comments

@amwill04
Copy link

amwill04 commented Apr 1, 2020

If trying to use a CfnParameter with a SQS Queue you end up with the following error:

message retention period must be 60 seconds or more, but -1.8881545897087585e+289 was provided

Reproduction Steps

        const redemptionPeriod = new CfnParameter(this, 'RedendtionPeriod', {
            description: 'SQS redention period in seconds 60 - 1209600 [14400]',
            type: 'Number',
            default: 14400,
            minValue: 60,
            maxValue: 1209600,
            constraintDescription: 'Must be between 60 and 1209600',
        });

        return new Queue(this, 'Queue', {
            queueName: 'queue',
            retentionPeriod: Duration.seconds(redemptionPeriod.valueAsNumber), // This pass typechecking but will at best fail and at worst pass with the incorrect number.
            deliveryDelay: Duration.seconds(0),
            maxMessageSizeBytes: 1024,
            receiveMessageWaitTime: Duration.seconds(20),
            visibilityTimeout: Duration.seconds(60),
        });

Error Log

message retention period must be 60 seconds or more, but -1.8881545897087585e+289 was provided

Environment

  • **CLI Version :1.31.0
  • **Framework Version:1.31.0
  • **OS :MacOS
  • **Language :Typescript

This is 🐛 Bug Report

@amwill04 amwill04 added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Apr 1, 2020
@SomayaB SomayaB added the @aws-cdk/aws-sqs Related to Amazon Simple Queue Service label Apr 3, 2020
@MrArnoldPalmer
Copy link
Contributor

MrArnoldPalmer commented Apr 6, 2020

@shivlaks what is the right way to do this?

@eladb
Copy link
Contributor

eladb commented Apr 6, 2020

Interesting! I think Durarion.seconds should be token-aware and just pass through the token to toSeconds and fail in all other conversions.

@amwill04
Copy link
Author

amwill04 commented Apr 6, 2020

@eladb it only seems to fail only with SQS. It seems to work fine with other resources such as Lambda.

@shivlaks
Copy link
Contributor

shivlaks commented Apr 6, 2020

@amwill04 I think it works for Lambda because of an oversight where the retry parameter doesn't seem to have any validation for it.

i.e. the range for retry should be between 3 and 900 seconds, but the following will build and synth successfully (although it will fail during deployment because it's an invalid timeout value)

const handler = new lambda.Function(stack, 'handler', {
      handler: 'index.handler',
      code: lambda.Code.fromInline('boom'),
      runtime: lambda.Runtime.NODEJS_10_X,
      timeout: 901
    });

SQS on the other hand validates the props that are set (which contain a token) and it's erroring out there

@amwill04
Copy link
Author

amwill04 commented Apr 8, 2020

@shivlaks does that mean with the way token are encoded when using valueAsNumber it means we lose the ability to validate at synthesis? As the example you provide shows?

@shivlaks shivlaks added p1 @aws-cdk/core Related to core CDK functionality and removed @aws-cdk/aws-sqs Related to Amazon Simple Queue Service needs-triage This issue or PR still needs to be triaged. labels Apr 12, 2020
shivlaks added a commit that referenced this issue May 28, 2020
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
shivlaks added a commit that referenced this issue May 28, 2020
… properties

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
@SomayaB SomayaB added the in-progress This issue is being actively worked on. label Jun 1, 2020
@mergify mergify bot closed this as completed in #8252 Jun 1, 2020
mergify bot pushed a commit that referenced this issue Jun 1, 2020
… 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*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/core Related to core CDK functionality bug This issue is a bug. in-progress This issue is being actively worked on. p1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants