Skip to content

Commit

Permalink
fix(logs): fix infinite retention for jsii users (aws#3250)
Browse files Browse the repository at this point in the history
The retention period was still accepting the value `Infinity` for
"infinite retention", even though the property has been changed to an
enum instead of `number`.

Even though this apparently works for TypeScript customers, jsii
customers will not be able to pass in a number where an enum value
is expected, and so won't be able to configure infinite retention.

Add an `INFINITE` value to the enum.
  • Loading branch information
rix0rrr authored Jul 12, 2019
1 parent f68411c commit 0b1ea76
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 4 deletions.
13 changes: 9 additions & 4 deletions packages/@aws-cdk/aws-logs/lib/log-group.ts
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,12 @@ export enum RetentionDays {
/**
* 10 years
*/
TEN_YEARS = 3653
TEN_YEARS = 3653,

/**
* Retain logs forever
*/
INFINITE = 9999,
}

/**
Expand All @@ -276,9 +281,9 @@ export interface LogGroupProps {
/**
* How long, in days, the log contents will be retained.
*
* To retain all logs, set this value to Infinity.
* To retain all logs, set this value to RetentionDays.INFINITE.
*
* @default RetentionDays.TwoYears
* @default RetentionDays.TWO_YEARS
*/
readonly retention?: RetentionDays;

Expand Down Expand Up @@ -328,7 +333,7 @@ export class LogGroup extends LogGroupBase {

let retentionInDays = props.retention;
if (retentionInDays === undefined) { retentionInDays = RetentionDays.TWO_YEARS; }
if (retentionInDays === Infinity) { retentionInDays = undefined; }
if (retentionInDays === Infinity || retentionInDays === RetentionDays.INFINITE) { retentionInDays = undefined; }

if (retentionInDays !== undefined && retentionInDays <= 0) {
throw new Error(`retentionInDays must be positive, got ${retentionInDays}`);
Expand Down
26 changes: 26 additions & 0 deletions packages/@aws-cdk/aws-logs/test/test.loggroup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,32 @@ export = {

// WHEN
new LogGroup(stack, 'LogGroup', {
retention: RetentionDays.INFINITE,
});

// THEN
expect(stack).to(matchTemplate({
Resources: {
LogGroupF5B46931: {
Type: "AWS::Logs::LogGroup",
DeletionPolicy: "Retain",
UpdateReplacePolicy: "Retain"
}
}
}));

test.done();
},

'infinite retention via legacy method'(test: Test) {
// GIVEN
const stack = new Stack();

// WHEN
new LogGroup(stack, 'LogGroup', {
// Don't know why TypeScript doesn't complain about passing Infinity to
// something where an enum is expected, but better keep this behavior for
// existing clients.
retention: Infinity
});

Expand Down

0 comments on commit 0b1ea76

Please sign in to comment.