Skip to content

Commit

Permalink
fix(aws-logs): remove validation of retentionInDays for unresolved to…
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
nmoutschen authored Mar 17, 2020
1 parent 38c8f34 commit 43a3420
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 3 deletions.
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-logs/lib/log-group.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import * as cloudwatch from '@aws-cdk/aws-cloudwatch';
import * as iam from '@aws-cdk/aws-iam';
import { Construct, IResource, RemovalPolicy, Resource, Stack } from '@aws-cdk/core';
import { Construct, IResource, RemovalPolicy, Resource, Stack, Token } from '@aws-cdk/core';
import { LogStream } from './log-stream';
import { CfnLogGroup } from './logs.generated';
import { MetricFilter } from './metric-filter';
Expand Down Expand Up @@ -352,7 +352,7 @@ export class LogGroup extends LogGroupBase {
if (retentionInDays === undefined) { retentionInDays = RetentionDays.TWO_YEARS; }
if (retentionInDays === Infinity || retentionInDays === RetentionDays.INFINITE) { retentionInDays = undefined; }

if (retentionInDays !== undefined && retentionInDays <= 0) {
if (retentionInDays !== undefined && !Token.isUnresolved(retentionInDays) && retentionInDays <= 0) {
throw new Error(`retentionInDays must be positive, got ${retentionInDays}`);
}

Expand Down
22 changes: 21 additions & 1 deletion packages/@aws-cdk/aws-logs/test/test.loggroup.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { expect, haveResource, matchTemplate } from '@aws-cdk/assert';
import * as iam from '@aws-cdk/aws-iam';
import { RemovalPolicy, Stack } from '@aws-cdk/core';
import { CfnParameter, RemovalPolicy, Stack } from '@aws-cdk/core';
import { Test } from 'nodeunit';
import { LogGroup, RetentionDays } from '../lib';

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

'unresolved retention'(test: Test) {
// GIVEN
const stack = new Stack();
const parameter = new CfnParameter(stack, "RetentionInDays", { default: 30, type: "Number" });

// WHEN
new LogGroup(stack, 'LogGroup', {
retention: parameter.valueAsNumber
});

// THEN
expect(stack).to(haveResource('AWS::Logs::LogGroup', {
RetentionInDays: {
Ref: "RetentionInDays"
}
}));

test.done();
},

'will delete log group if asked to'(test: Test) {
// GIVEN
const stack = new Stack();
Expand Down

0 comments on commit 43a3420

Please sign in to comment.