From 7e93a58fa6e53758d611de031dbf984a4f1a88ed Mon Sep 17 00:00:00 2001
From: Nicolas Moutschen <nicolas.moutschen@gmail.com>
Date: Sat, 14 Mar 2020 21:48:40 +0100
Subject: [PATCH] fix(aws-logs): remove validation of retentionInDays for
 unresolved tokens

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
---
 packages/@aws-cdk/aws-logs/lib/log-group.ts   |  4 ++--
 .../@aws-cdk/aws-logs/test/test.loggroup.ts   | 22 ++++++++++++++++++-
 2 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/packages/@aws-cdk/aws-logs/lib/log-group.ts b/packages/@aws-cdk/aws-logs/lib/log-group.ts
index 1d6d3ce95101e..1ab8a9f5a5211 100644
--- a/packages/@aws-cdk/aws-logs/lib/log-group.ts
+++ b/packages/@aws-cdk/aws-logs/lib/log-group.ts
@@ -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';
@@ -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}`);
     }
 
diff --git a/packages/@aws-cdk/aws-logs/test/test.loggroup.ts b/packages/@aws-cdk/aws-logs/test/test.loggroup.ts
index 9592e0a88bac6..00687f3fe1c62 100644
--- a/packages/@aws-cdk/aws-logs/test/test.loggroup.ts
+++ b/packages/@aws-cdk/aws-logs/test/test.loggroup.ts
@@ -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';
 
@@ -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();