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

Update clear key logic #64

Merged
merged 3 commits into from
Oct 15, 2024
Merged

Update clear key logic #64

merged 3 commits into from
Oct 15, 2024

Conversation

lbaquerofierro
Copy link
Contributor

Update clear key logic, delete keys after prevRotationDate + KEY_LIFE…SPAN (48H), except last uploaded key. This creates a 48h grace period for old keys after a key has being rotated

src/utils/keyRotation.ts Outdated Show resolved Hide resolved
@lbaquerofierro lbaquerofierro force-pushed the lina/update-key-cleanup branch 11 times, most recently from 09cccbb to bb052f2 Compare October 14, 2024 16:10
Copy link
Contributor

@thibmeu thibmeu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the rotation logic seems ok to me. hopefully the tests confirm this

src/index.ts Outdated Show resolved Hide resolved
src/index.ts Show resolved Hide resolved
src/utils/keyRotation.ts Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
src/index.ts Show resolved Hide resolved
Key rotation and clearance tests have a lot of copy paste, making them
hard to review and update. This commit moves them to `each` format.
Copy link
Contributor

@thibmeu thibmeu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change looks good for me. thanks for pushing!

src/index.ts Outdated Show resolved Hide resolved
@thibmeu thibmeu merged commit 91d3caa into main Oct 15, 2024
5 checks passed
@thibmeu thibmeu deleted the lina/update-key-cleanup branch October 15, 2024 12:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants