aws / aws-cdk

The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code
https://aws.amazon.com/cdk
Apache License 2.0
11.42k stars 3.8k forks source link

aws_secretsmanager: RotationSchedule cron expression support #24062

Open vobarian opened 1 year ago

vobarian commented 1 year ago

Describe the feature

Secrets Manager and CloudFormation support using a cron schedule expression for secret rotation:

Maybe I'm missing it, but I cannot find equivalent functionality in CDK. It looks like only setting a rotation period is supported:

Request: Support cron schedule expressions in RotationSchedule, SecretRotation, and Secret#addRotationSchedule.

See: https://github.com/aws/aws-cdk/discussions/19980

Use Case

I would like to control more precisely when secret rotation happens, and this functionality exists in the underlying service but is not exposed in CDK.

Proposed Solution

Similar to the CloudFormation resource, a schedule property could be added to the relevant props interfaces alongside automaticallyAfter. The type of this property would be a flexible schedule object similar to aws_events.Schedule.

Unfortunately, this results in an awkward interface where one but not both of the schedule or automaticallyAfter properties is required. A better solution would be to have a single property that supports either rate or cron expression type. For example automaticallyAfter could accept Duration | Schedule. However then the name of that prop would be awkward. Unfortunately I can't think of a great way to express that interface in a backwards-compatible way. The ideal would be to just have a schedule property without automaticallyAfter but that would not be backwards-compatible.

Other Information

Use CfnRotationSchedule L1 as a workaround. Unfortunately it requires manually setting up the permissions for the rotation function as well.

Acknowledgements

CDK version used

2.63.2

Environment details (OS name and version, etc.)

macOS 12.6.1

PritamSangani commented 1 year ago

Another approach to the proposed solution is to expose 2 new props in the RotationSchedule construct.

One called windowDuration (of type Duration) the other called schedule (of type Schedule).

windowDuration would map to the duration prop in the RotationRules property within the L1 construct and schedule would map to scheduleExpression.

Looking at the docs for the L1 construct, it seems you can't pass both the automaticallyAfter and the scheduleExpression prop so there would also need to be a check to make sure both props are not passed.

rpawlaszek commented 8 months ago

For now you can use property overrides. So in this case one can write:

    declare const schedule: Schedule;

    const cfnSchedule = rotationSchedule.node.defaultChild as CfnRotationSchedule;
    cfnSchedule.addPropertyOverride('RotationRules.ScheduleExpression', schedule.expressionString);
    cfnSchedule.addPropertyDeletionOverride('RotationRules.AutomaticallyAfterDays');

And the solution could be simply adding the other parameter schedule with a check if both automaticallyAfter and schedule exist that would add an error annotation if they do.

rpawlaszek commented 8 months ago

Or a yet another approach. As both things are down below RotationRules and it's either-or there could be a property rotationRule

and a class with two static methods onSchedule(schedule: Schedule, rotationWindow?: Duration); and automaticallyAfter(days: Duration; rotationWindow?: Duration); and the usage could be like:

declare const stack: Stack;
declare const everySixHours: Schedule;
declare const threeDays: Duration;

const rotation1Schedule = new RotationSchedule(stack, 'Rotation1Schedule', {
    ...
    rotationRule: RotationRule.onSchedule(everySixHours)
});

const rotation2Schedule = new RotationSchedule(stack, 'Rotation1Schedule', {
    ...
    rotationRule: RotationRule.automaticallyAfter(threeDays)
});

It would then remove the ambiguity.