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.

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

aws-secretsmanager: Cannot set rotation schedule to be less than a day #28261

Closed
wmattei opened this issue Dec 5, 2023 · 3 comments · Fixed by #28303
Closed

aws-secretsmanager: Cannot set rotation schedule to be less than a day #28261

wmattei opened this issue Dec 5, 2023 · 3 comments · Fixed by #28303
Labels
@aws-cdk/aws-secretsmanager Related to AWS Secrets Manager bug This issue is a bug. effort/medium Medium work item – several days of effort p2

Comments

@wmattei
Copy link

wmattei commented Dec 5, 2023

Describe the bug

AWS secrets manager documentation, says that "You can rotate a secret as often as every four hours". However if I want to do something like:

    new Secret(this, 'my-secret').addRotationSchedule('my-secret-rotation-schedule', {
        rotationLambda: myLambdaFunction,
        automaticallyAfter: Duration.hours(4),
    })

I will get the following error:

Error: '4 hours' cannot be converted into a whole number of days.
    at convert (/Users/wmattei/www/peek/peek-iac/node_modules/aws-cdk-lib/core/lib/duration.js:1:6197)
    at Duration.toDays (/Users/wmattei/www/peek/peek-iac/node_modules/aws-cdk-lib/core/lib/duration.js:1:3641)
    at new RotationSchedule (/Users/wmattei/www/peek/peek-iac/node_modules/aws-cdk-lib/aws-secretsmanager/lib/rotation-schedule.js:1:2264)
    at Secret.addRotationSchedule (/Users/wmattei/www/peek/peek-iac/node_modules/aws-cdk-lib/aws-secretsmanager/lib/secret.js:1:4727)

Expected Behavior

I would expect CDK to allow me to create schedules for 4 hours, as described in the documention.

Current Behavior

I can only rotation schedules for the minimum of 1 day.

Reproduction Steps

Try to synth this stack:

import { App, Stack, Duration } from "aws-cdk-lib";
import { Secret } from "aws-cdk-lib/aws-secretsmanager";
import { NodejsFunction } from "aws-cdk-lib/aws-lambda-nodejs";
export class MyStack extends Stack {
  constructor() {
    const app = new App();

    super(app, "MyStack");

    const myLambdaFunction = new NodejsFunction(this, "my-lambda-function", {
      entry: "path/to/my/lambda",
    });

    new Secret(this, "my-secret").addRotationSchedule(
      "my-secret-rotation-schedule",
      {
        rotationLambda: myLambdaFunction,
        automaticallyAfter: Duration.hours(4),
      }
    );
  }
}

Possible Solution

I suppose this is related to the fact that when checking if the provided duration is greater than 1000 days we are directly calling .toDays which validates wether the given value can be converted in a whole number of days.

Additional Information/Context

No response

CDK CLI Version

2.113.0 (build ccd534a)

Framework Version

No response

Node.js Version

v21.3.0

OS

MacOs

Language

TypeScript

Language Version

Version 5.1.6

Other information

No response

@wmattei wmattei added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Dec 5, 2023
@github-actions github-actions bot added the @aws-cdk/aws-secretsmanager Related to AWS Secrets Manager label Dec 5, 2023
@pahud
Copy link
Contributor

pahud commented Dec 5, 2023

I guess this is because current implementation is to satisfy AutomaticallyAfterDays which the unit is day but it's still possible to implement it with ScheduleExpression.

rotationRules = {
automaticallyAfterDays,
};

@pahud pahud added p2 effort/medium Medium work item – several days of effort and removed needs-triage This issue or PR still needs to be triaged. labels Dec 5, 2023
@wmattei
Copy link
Author

wmattei commented Dec 5, 2023

@pahud I would be happy to work on this at some point during this week.

Do you think the lib should internally convert it to ScheduleExpression, or export an option to the caller to provide automaticallyAfterDays | scheduleExpression

I see that there is already a feature request for this at #24062

mergify bot added a commit to lpizzinidev/aws-cdk that referenced this issue Dec 14, 2023
@mergify mergify bot closed this as completed in #28303 Dec 14, 2023
mergify bot pushed a commit that referenced this issue Dec 14, 2023
Allows to set hourly rotation up to 4 hours on secrets as per [official docs](https://docs.aws.amazon.com/secretsmanager/latest/userguide/rotate-secrets_managed.html).

Closes #28261.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-secretsmanager Related to AWS Secrets Manager bug This issue is a bug. effort/medium Medium work item – several days of effort p2
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants