From 21785e2526e8c9498e2a9c905d81dd9c5554ad9b Mon Sep 17 00:00:00 2001 From: go-to-k <24818752+go-to-k@users.noreply.github.com> Date: Tue, 17 Oct 2023 15:46:01 +0900 Subject: [PATCH 1/9] docs(secretsmanager): doc when automaticallyAfter for RotationSchedule is 0 is wrong --- .../aws-cdk-lib/aws-secretsmanager/lib/rotation-schedule.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/aws-cdk-lib/aws-secretsmanager/lib/rotation-schedule.ts b/packages/aws-cdk-lib/aws-secretsmanager/lib/rotation-schedule.ts index b2460667599f1..6188a8ba82f8c 100644 --- a/packages/aws-cdk-lib/aws-secretsmanager/lib/rotation-schedule.ts +++ b/packages/aws-cdk-lib/aws-secretsmanager/lib/rotation-schedule.ts @@ -37,7 +37,7 @@ export interface RotationScheduleOptions { * Specifies the number of days after the previous rotation before * Secrets Manager triggers the next automatic rotation. * - * A value of zero will disable automatic rotation - `Duration.days(0)`. + * A value of zero (`Duration.days(0)`) will not to create RotationRules. * * @default Duration.days(30) */ From 324066e6d86c206f2e65ecd49594c205ff87e2bd Mon Sep 17 00:00:00 2001 From: go-to-k <24818752+go-to-k@users.noreply.github.com> Date: Wed, 18 Oct 2023 00:47:09 +0900 Subject: [PATCH 2/9] add validation for if automaticallyAfter > 1000 --- .../lib/rotation-schedule.ts | 5 ++++- .../test/rotation-schedule.test.ts | 18 ++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/packages/aws-cdk-lib/aws-secretsmanager/lib/rotation-schedule.ts b/packages/aws-cdk-lib/aws-secretsmanager/lib/rotation-schedule.ts index 6188a8ba82f8c..dc698f5a7688d 100644 --- a/packages/aws-cdk-lib/aws-secretsmanager/lib/rotation-schedule.ts +++ b/packages/aws-cdk-lib/aws-secretsmanager/lib/rotation-schedule.ts @@ -125,7 +125,10 @@ export class RotationSchedule extends Resource { } let automaticallyAfterDays: number | undefined = undefined; - if (props.automaticallyAfter?.toMilliseconds() !== 0) { + if (props.automaticallyAfter && props.automaticallyAfter.toDays() > 1000) { + throw new Error(`automaticallyAfter must not greater than 1000 days, got ${props.automaticallyAfter.toDays()} days`); + } + if (!props.automaticallyAfter || props.automaticallyAfter.toMilliseconds() !== 0) { automaticallyAfterDays = props.automaticallyAfter?.toDays() || 30; } diff --git a/packages/aws-cdk-lib/aws-secretsmanager/test/rotation-schedule.test.ts b/packages/aws-cdk-lib/aws-secretsmanager/test/rotation-schedule.test.ts index d406b8830dda1..13abd72c4557d 100644 --- a/packages/aws-cdk-lib/aws-secretsmanager/test/rotation-schedule.test.ts +++ b/packages/aws-cdk-lib/aws-secretsmanager/test/rotation-schedule.test.ts @@ -651,3 +651,21 @@ test('rotation schedule should have a dependency on lambda permissions', () => { ], }); }); + +test('automaticallyAfter must not greater than 1000 days', () => { + // GIVEN + const secret = new secretsmanager.Secret(stack, 'Secret'); + const rotationLambda = new lambda.Function(stack, 'Lambda', { + runtime: lambda.Runtime.NODEJS_LATEST, + code: lambda.Code.fromInline('export.handler = event => event;'), + handler: 'index.handler', + }); + + // WHEN + // THEN + expect(() => new secretsmanager.RotationSchedule(stack, 'RotationSchedule', { + secret, + rotationLambda, + automaticallyAfter: Duration.days(1001), + })).toThrow(/automaticallyAfter must not greater than 1000 days, got 1001 days/); +}); \ No newline at end of file From 0f8bbf32b6936612c1e3e2838bec4571ac2c126c Mon Sep 17 00:00:00 2001 From: go-to-k <24818752+go-to-k@users.noreply.github.com> Date: Wed, 18 Oct 2023 11:14:36 +0900 Subject: [PATCH 3/9] tweak for error message --- .../aws-cdk-lib/aws-secretsmanager/lib/rotation-schedule.ts | 2 +- .../aws-secretsmanager/test/rotation-schedule.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/aws-cdk-lib/aws-secretsmanager/lib/rotation-schedule.ts b/packages/aws-cdk-lib/aws-secretsmanager/lib/rotation-schedule.ts index dc698f5a7688d..e54dad900ea3f 100644 --- a/packages/aws-cdk-lib/aws-secretsmanager/lib/rotation-schedule.ts +++ b/packages/aws-cdk-lib/aws-secretsmanager/lib/rotation-schedule.ts @@ -126,7 +126,7 @@ export class RotationSchedule extends Resource { let automaticallyAfterDays: number | undefined = undefined; if (props.automaticallyAfter && props.automaticallyAfter.toDays() > 1000) { - throw new Error(`automaticallyAfter must not greater than 1000 days, got ${props.automaticallyAfter.toDays()} days`); + throw new Error(`automaticallyAfter must not be greater than 1000 days, got ${props.automaticallyAfter.toDays()} days`); } if (!props.automaticallyAfter || props.automaticallyAfter.toMilliseconds() !== 0) { automaticallyAfterDays = props.automaticallyAfter?.toDays() || 30; diff --git a/packages/aws-cdk-lib/aws-secretsmanager/test/rotation-schedule.test.ts b/packages/aws-cdk-lib/aws-secretsmanager/test/rotation-schedule.test.ts index 13abd72c4557d..7966fce559cdd 100644 --- a/packages/aws-cdk-lib/aws-secretsmanager/test/rotation-schedule.test.ts +++ b/packages/aws-cdk-lib/aws-secretsmanager/test/rotation-schedule.test.ts @@ -652,7 +652,7 @@ test('rotation schedule should have a dependency on lambda permissions', () => { }); }); -test('automaticallyAfter must not greater than 1000 days', () => { +test('automaticallyAfter must not be greater than 1000 days', () => { // GIVEN const secret = new secretsmanager.Secret(stack, 'Secret'); const rotationLambda = new lambda.Function(stack, 'Lambda', { From d0dd6d0de6ee5acd43b3ae68af1993a52ed39ae2 Mon Sep 17 00:00:00 2001 From: go-to-k <24818752+go-to-k@users.noreply.github.com> Date: Wed, 18 Oct 2023 11:18:30 +0900 Subject: [PATCH 4/9] tweak for a doc --- .../aws-cdk-lib/aws-secretsmanager/lib/rotation-schedule.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/aws-cdk-lib/aws-secretsmanager/lib/rotation-schedule.ts b/packages/aws-cdk-lib/aws-secretsmanager/lib/rotation-schedule.ts index e54dad900ea3f..4e3a6dab3467a 100644 --- a/packages/aws-cdk-lib/aws-secretsmanager/lib/rotation-schedule.ts +++ b/packages/aws-cdk-lib/aws-secretsmanager/lib/rotation-schedule.ts @@ -37,7 +37,7 @@ export interface RotationScheduleOptions { * Specifies the number of days after the previous rotation before * Secrets Manager triggers the next automatic rotation. * - * A value of zero (`Duration.days(0)`) will not to create RotationRules. + * A value of zero (`Duration.days(0)`) will not create RotationRules. * * @default Duration.days(30) */ From 51a4a12cdb7aa9437414e9002f0f0cf4da2baca8 Mon Sep 17 00:00:00 2001 From: go-to-k <24818752+go-to-k@users.noreply.github.com> Date: Wed, 18 Oct 2023 11:21:07 +0900 Subject: [PATCH 5/9] tweak for a test --- .../aws-secretsmanager/test/rotation-schedule.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/aws-cdk-lib/aws-secretsmanager/test/rotation-schedule.test.ts b/packages/aws-cdk-lib/aws-secretsmanager/test/rotation-schedule.test.ts index 7966fce559cdd..81d14021be962 100644 --- a/packages/aws-cdk-lib/aws-secretsmanager/test/rotation-schedule.test.ts +++ b/packages/aws-cdk-lib/aws-secretsmanager/test/rotation-schedule.test.ts @@ -667,5 +667,5 @@ test('automaticallyAfter must not be greater than 1000 days', () => { secret, rotationLambda, automaticallyAfter: Duration.days(1001), - })).toThrow(/automaticallyAfter must not greater than 1000 days, got 1001 days/); + })).toThrow(/automaticallyAfter must not be greater than 1000 days, got 1001 days/); }); \ No newline at end of file From 7be4cee0497d64ad768a7b12e5d02f6a279b07fb Mon Sep 17 00:00:00 2001 From: go-to-k <24818752+go-to-k@users.noreply.github.com> Date: Wed, 18 Oct 2023 17:54:01 +0900 Subject: [PATCH 6/9] drop without docs --- .../lib/rotation-schedule.ts | 5 +---- .../test/rotation-schedule.test.ts | 18 ------------------ 2 files changed, 1 insertion(+), 22 deletions(-) diff --git a/packages/aws-cdk-lib/aws-secretsmanager/lib/rotation-schedule.ts b/packages/aws-cdk-lib/aws-secretsmanager/lib/rotation-schedule.ts index 4e3a6dab3467a..7002dc99ee6ad 100644 --- a/packages/aws-cdk-lib/aws-secretsmanager/lib/rotation-schedule.ts +++ b/packages/aws-cdk-lib/aws-secretsmanager/lib/rotation-schedule.ts @@ -125,10 +125,7 @@ export class RotationSchedule extends Resource { } let automaticallyAfterDays: number | undefined = undefined; - if (props.automaticallyAfter && props.automaticallyAfter.toDays() > 1000) { - throw new Error(`automaticallyAfter must not be greater than 1000 days, got ${props.automaticallyAfter.toDays()} days`); - } - if (!props.automaticallyAfter || props.automaticallyAfter.toMilliseconds() !== 0) { + if (props.automaticallyAfter?.toMilliseconds() !== 0) { automaticallyAfterDays = props.automaticallyAfter?.toDays() || 30; } diff --git a/packages/aws-cdk-lib/aws-secretsmanager/test/rotation-schedule.test.ts b/packages/aws-cdk-lib/aws-secretsmanager/test/rotation-schedule.test.ts index 81d14021be962..1dad96d28d881 100644 --- a/packages/aws-cdk-lib/aws-secretsmanager/test/rotation-schedule.test.ts +++ b/packages/aws-cdk-lib/aws-secretsmanager/test/rotation-schedule.test.ts @@ -650,22 +650,4 @@ test('rotation schedule should have a dependency on lambda permissions', () => { 'LambdaInvokeN0a2GKfZP0JmDqDEVhhu6A0TUv3NyNbk4YMFKNc69846677', ], }); -}); - -test('automaticallyAfter must not be greater than 1000 days', () => { - // GIVEN - const secret = new secretsmanager.Secret(stack, 'Secret'); - const rotationLambda = new lambda.Function(stack, 'Lambda', { - runtime: lambda.Runtime.NODEJS_LATEST, - code: lambda.Code.fromInline('export.handler = event => event;'), - handler: 'index.handler', - }); - - // WHEN - // THEN - expect(() => new secretsmanager.RotationSchedule(stack, 'RotationSchedule', { - secret, - rotationLambda, - automaticallyAfter: Duration.days(1001), - })).toThrow(/automaticallyAfter must not be greater than 1000 days, got 1001 days/); }); \ No newline at end of file From eb2392dcb97be0ebbd22f30ed5bf056dad112e24 Mon Sep 17 00:00:00 2001 From: go-to-k <24818752+go-to-k@users.noreply.github.com> Date: Wed, 18 Oct 2023 17:55:09 +0900 Subject: [PATCH 7/9] add comments for maximum value --- .../aws-cdk-lib/aws-secretsmanager/lib/rotation-schedule.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/aws-cdk-lib/aws-secretsmanager/lib/rotation-schedule.ts b/packages/aws-cdk-lib/aws-secretsmanager/lib/rotation-schedule.ts index 7002dc99ee6ad..8363bc0ce547f 100644 --- a/packages/aws-cdk-lib/aws-secretsmanager/lib/rotation-schedule.ts +++ b/packages/aws-cdk-lib/aws-secretsmanager/lib/rotation-schedule.ts @@ -37,6 +37,8 @@ export interface RotationScheduleOptions { * Specifies the number of days after the previous rotation before * Secrets Manager triggers the next automatic rotation. * + * The maximum value is 1000 days. + * * A value of zero (`Duration.days(0)`) will not create RotationRules. * * @default Duration.days(30) From c161bfbb6806e3d6ccceaf20c90667bb488ccb91 Mon Sep 17 00:00:00 2001 From: go-to-k <24818752+go-to-k@users.noreply.github.com> Date: Wed, 18 Oct 2023 17:56:09 +0900 Subject: [PATCH 8/9] tweak --- .../aws-secretsmanager/test/rotation-schedule.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/aws-cdk-lib/aws-secretsmanager/test/rotation-schedule.test.ts b/packages/aws-cdk-lib/aws-secretsmanager/test/rotation-schedule.test.ts index 1dad96d28d881..d406b8830dda1 100644 --- a/packages/aws-cdk-lib/aws-secretsmanager/test/rotation-schedule.test.ts +++ b/packages/aws-cdk-lib/aws-secretsmanager/test/rotation-schedule.test.ts @@ -650,4 +650,4 @@ test('rotation schedule should have a dependency on lambda permissions', () => { 'LambdaInvokeN0a2GKfZP0JmDqDEVhhu6A0TUv3NyNbk4YMFKNc69846677', ], }); -}); \ No newline at end of file +}); From 23e34222bd80f57c62341f95730503c4e3b04cd5 Mon Sep 17 00:00:00 2001 From: go-to-k <24818752+go-to-k@users.noreply.github.com> Date: Wed, 18 Oct 2023 18:04:30 +0900 Subject: [PATCH 9/9] change comments --- .../aws-cdk-lib/aws-secretsmanager/lib/rotation-schedule.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/aws-cdk-lib/aws-secretsmanager/lib/rotation-schedule.ts b/packages/aws-cdk-lib/aws-secretsmanager/lib/rotation-schedule.ts index 8363bc0ce547f..7002dc99ee6ad 100644 --- a/packages/aws-cdk-lib/aws-secretsmanager/lib/rotation-schedule.ts +++ b/packages/aws-cdk-lib/aws-secretsmanager/lib/rotation-schedule.ts @@ -37,8 +37,6 @@ export interface RotationScheduleOptions { * Specifies the number of days after the previous rotation before * Secrets Manager triggers the next automatic rotation. * - * The maximum value is 1000 days. - * * A value of zero (`Duration.days(0)`) will not create RotationRules. * * @default Duration.days(30)