Skip to content

Lifecycle changes - Allow all actions in put lifecycle configuration #8424

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

Merged
merged 1 commit into from
Oct 1, 2024

Conversation

dannyzaken
Copy link
Member

@dannyzaken dannyzaken commented Oct 1, 2024

Explain the changes

  • although not all of the actions are implemented in the lifecycle bg worker, we still want to store the provided configuration and not fail it
  • in NC case, the lifecycle work is performed outside of noobaa, so we want to provide all fields
  • In addition, for now, allow passing the deprecated Prefix field. This commit leaves a gap regarding validation of the configuration, which should be more aligned with what AWS allows

Issues: Fixed #xxx / Gap #xxx

  1. Fixes Bucket lifecycle rules should allow top level Prefix and NoncurrentVersionExpiration and AbortIncompleteMultipartUpload elements #8341
  2. NSFS | S3 | Versioning: PutBucketLifecycleConfiguration on version-enabled bucket fails with ParamValidationError #8376
  3. opened Gap s3_put_bucket_lifecycle validations should be more compatible with AWS #8426

Testing Instructions:

  • Doc added/updated
  • Tests added

@dannyzaken dannyzaken marked this pull request as draft October 1, 2024 07:56
@dannyzaken dannyzaken force-pushed the danny-fixes branch 2 times, most recently from 43270f2 to fcf3e1d Compare October 1, 2024 08:41
@pull-request-size pull-request-size bot added size/L and removed size/M labels Oct 1, 2024
dbg.error('ExpiredObjectDeleteMarker is not implemented, expiration:', expiration);
throw new S3Error(S3Error.NotImplemented);
expiration.ExpiredObjectDeleteMarker.length === 1 &&
(/true/i).test(expiration.ExpiredObjectDeleteMarker[0])) {
Copy link
Contributor

Choose a reason for hiding this comment

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

worth putting the regexp in the global scope? due to #7110

if (rule.Transition) {
dbg.error('Transition is not implemented, rule:', rule);
throw new S3Error(S3Error.NotImplemented);
if (rule.AbortIncompleteMultipartUpload && rule.AbortIncompleteMultipartUpload.length === 1) {
Copy link
Contributor

Choose a reason for hiding this comment

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

style, maybe worth in all the conditions make the conditions easier to read by using ? check -
if (rule?.AbortIncompleteMultipartUpload.length === 1)

throw new S3Error(S3Error.NotImplemented);


if (rule.Expiration && rule.Expiration.length === 1) {
Copy link
Contributor

Choose a reason for hiding this comment

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

expiration is still required in the schema, we probably want to remove it if it's not required anymore
same for filter

Copy link
Member Author

Choose a reason for hiding this comment

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

Filter is still required. I will remove expiration. thanks

@dannyzaken dannyzaken force-pushed the danny-fixes branch 2 times, most recently from 387380b to 84a7a68 Compare October 1, 2024 10:19
@dannyzaken dannyzaken marked this pull request as ready for review October 1, 2024 10:31
* although not all of the actions are implemented in the lifecycle bg worker, we still want to store the provided configuration and not fail it
* in NC case, the lifecycle work is performed outside of noobaa, so we want to provide all fields
* In addition, for now, allow passing the deprecated Prefix field. This commit leaves a gap regarding validation of the configuration, which should be more aligned with what AWS allows

Signed-off-by: Danny Zaken <[email protected]>
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.

Bucket lifecycle rules should allow top level Prefix and NoncurrentVersionExpiration and AbortIncompleteMultipartUpload elements
2 participants