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.

Sign up for GitHub

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

Jump to bottom

retentionInDays for logs.LogGroup using a CfnParameter throws an error #6690

Closed
nmoutschen opened this issue Mar 12, 2020 · 1 comment · Fixed by #6727
Closed

retentionInDays for logs.LogGroup using a CfnParameter throws an error #6690

nmoutschen opened this issue Mar 12, 2020 · 1 comment · Fixed by #6727
Assignees
@rix0rrr
Labels
@aws-cdk/aws-logs Related to Amazon CloudWatch Logs bug This issue is a bug. good first issue Related to contributions. See CONTRIBUTING.md in-progress This issue is being actively worked on. p2

Comments

@nmoutschen
Copy link
Contributor

When using the retention property in logs.LogGroup and provide the value of a parameter, it throws an error that the number is negative: retentionInDays must be positive, got -1.888154589708755e+289. This exception seems to be thrown by the LogGroup.

Here is the CfnParameter:

const retentionInDays = new cdk.CfnParameter(this, "RetentionInDays", { default: 30, type: "Number" });

The log group:

new logs.LogGroup(this, "PreAuthLogGroup", {
  logGroupName: "/aws/lambda/"+preAuthFunction.functionName,
  retention: retentionInDays.valueAsNumber
});

Reproduction Steps

  1. Create a stack with the two resources mentioned above.
  2. Run cdk synth.

Error Log

retentionInDays must be positive, got -1.888154589708755e+289

Environment

  • CLI Version : 1.27.0
  • Framework Version: 1.27.0
  • OS : MacOS
  • Language : Typescript

Other

This is 🐛 Bug Report
@nmoutschen nmoutschen added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Mar 12, 2020
@SomayaB SomayaB added the @aws-cdk/aws-logs Related to Amazon CloudWatch Logs label Mar 12, 2020
@rix0rrr
Copy link
Contributor

rix0rrr commented Mar 13, 2020

Validation shouldn't happen if the input is a Token.isUnresolved().

@rix0rrr rix0rrr added good first issue Related to contributions. See CONTRIBUTING.md p2 and removed needs-triage This issue or PR still needs to be triaged. labels Mar 13, 2020
@nmoutschen nmoutschen mentioned this issue Mar 14, 2020
Merged
@SomayaB SomayaB added the in-progress This issue is being actively worked on. label Mar 16, 2020
@mergify mergify bot closed this as completed in #6727 Mar 17, 2020
mergify bot pushed a commit that referenced this issue Mar 17, 2020
@nmoutschen
fix(aws-logs): remove validation of retentionInDays for unresolved to…
43a3420 
…kens (#6727)

Prevents performing validation on the retention property for LogGroups
if the token is unresolved (e.g. a CloudFormation Parameter). Otherwise,
running the command `cdk synth` could fail with the following error
message:

```
retentionInDays must be positive, got -1.888154589708755e+289
```

The new unit test creates a LogGroup with a retention property that
refers to a CloudFormation parameter to ensure that the stack
synthesizes properly.

fixes #6690
@snyk-bot snyk-bot mentioned this issue Apr 10, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@rix0rrr rix0rrr

Labels
@aws-cdk/aws-logs Related to Amazon CloudWatch Logs bug This issue is a bug. good first issue Related to contributions. See CONTRIBUTING.md in-progress This issue is being actively worked on. p2
Projects
None yet
Milestone
No milestone
3 participants
@rix0rrr @nmoutschen @SomayaB