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

Fix for not accepting the past dates as expiration for bucket lifecycle rules #8633

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

achouhan09
Copy link
Member

@achouhan09 achouhan09 commented Dec 27, 2024

Explain the changes

  1. Added a fix for not accepting past dates as expiration for bucket lifecycle rules also will reject if expiration date is not in correct format.

Issues: Fixed #xxx / Gap #xxx

  1. Fixed: NSFS | S3 | Lifecycle: Expiration Date in the past gets accepted #8551

Testing Instructions:

  • Tests added

@achouhan09 achouhan09 requested review from liranmauda, shirady, naveenpaul1, a team and Neon-White and removed request for a team December 27, 2024 15:09
@achouhan09 achouhan09 requested a review from romayalon December 30, 2024 12:05
@achouhan09 achouhan09 changed the title NSFS | Fix for not accepting the past dates as expiration for bucket lifecycle rules Fix for not accepting the past dates as expiration for bucket lifecycle rules Dec 30, 2024
@romayalon
Copy link
Contributor

Hey @achouhan09

  1. Can you please link to the Issues section the bug you are fixing?
  2. Please add tests

@achouhan09
Copy link
Member Author

Hi @romayalon
Added issue link and tests. Thanks

@@ -44,6 +44,7 @@ exports.empty_filter_marker_lifecycle_configuration = empty_filter_marker_lifecy

function date_lifecycle_configuration(Bucket, Key) {
const now = new Date(Date.now());
now.setDate(now.getDate() + 1); // Increases date by 1 day; past/current dates can't be set
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you change the name of the variable? as it is not "now" anymore... (I saw it in more places).

Comment on lines +546 to +550
await s3.putBucketLifecycleConfiguration(putLifecycleParams)
.catch(error => {
assert(error.code === 'InvalidArgument', 'Expected InvalidArgument: expiration date in the past');
console.log('Expected error received, expiration date in the past');
});
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a special reason for using the promise chain here?

@@ -63,8 +64,31 @@ function date_lifecycle_configuration(Bucket, Key) {
}
exports.date_lifecycle_configuration = date_lifecycle_configuration;

function past_date_lifecycle_configuration(Bucket, Key) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you reuse the previous definition of date_lifecycle_configuration and make the change you need on this function's output?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NSFS | S3 | Lifecycle: Expiration Date in the past gets accepted
3 participants