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 issue during setting .WithLifecycleConfiguration (#656) #722

Merged

Conversation

MarDipp
Copy link

@MarDipp MarDipp commented Dec 5, 2022

Fixes issue Error during setting .WithLifecycleConfiguration #656.

The XML node NoncurrentVersionExpiration was empty because internal properties are not serialized.

@ebozduman
Copy link
Collaborator

Reviewing...

Copy link
Collaborator

@ebozduman ebozduman left a comment

Choose a reason for hiding this comment

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

I ran with @peterczabanski 's configuration given in the issue tobe fixed, #656, and SetBucketLifecycleAsync api still fails the same way with the same error message.
Your fix looks good, but it seems like the fix needs more changes.

One thing I noticed is; minio-go has 2 properties for NoncurrentVersionExpiration

<NoncurrentVersionExpiration>     
            <NewerNoncurrentVersions>5</NewerNoncurrentVersions>
            <NoncurrentDays>365</NoncurrentDays>    
</NoncurrentVersionExpiration> 

i.e NewerNoncurrentVersions is missing.

@MarDipp
Copy link
Author

MarDipp commented Dec 15, 2022

I ran with @peterczabanski 's configuration given in the issue tobe fixed, #656, and SetBucketLifecycleAsync api still fails the same way with the same error message. Your fix looks good, but it seems like the fix needs more changes.

One thing I noticed is; minio-go has 2 properties for NoncurrentVersionExpiration

<NoncurrentVersionExpiration>     
            <NewerNoncurrentVersions>5</NewerNoncurrentVersions>
            <NoncurrentDays>365</NoncurrentDays>    
</NoncurrentVersionExpiration> 

i.e NewerNoncurrentVersions is missing.

The remaining problem with @peterczabanski 's configuration is the part Filter = null. The Filter Node must not be missing, even if it is null. To apply the rule to all files an empty prefix can be used: Filter = new RuleFilter() { Prefix = String.Empty }

This configuration will work:
SetBucketLifecycleArgs args = new SetBucketLifecycleArgs() .WithBucket(bucketName) .WithLifecycleConfiguration( new LifecycleConfiguration(new List<LifecycleRule>(new[] { new LifecycleRule { AbortIncompleteMultipartUploadObject = null, ID = "Delete imagines after 31 days", Expiration = null, TransitionObject = null, Filter = new RuleFilter() { Prefix = String.Empty }, NoncurrentVersionExpirationObject = new NoncurrentVersionExpiration(31), NoncurrentVersionTransitionObject = null, Status = LifecycleRule.LIFECYCLE_RULE_STATUS_ENABLED } })));

The empty Filter node is removed in utils.RemoveNamespaceInXML. I don't think this function should be changed only to allow missing RuleFilter objects.

@ebozduman
Copy link
Collaborator

@MarDipp ,

This configuration will work:
SetBucketLifecycleArgs args = new SetBucketLifecycleArgs() .WithBucket(bucketName) .WithLifecycleConfiguration( new LifecycleConfiguration(new List(new[] { new LifecycleRule { AbortIncompleteMultipartUploadObject = null, ID = "Delete imagines after 31 days", Expiration = null, TransitionObject = null, Filter = new RuleFilter() { Prefix = String.Empty }, NoncurrentVersionExpirationObject = new NoncurrentVersionExpiration(31), NoncurrentVersionTransitionObject = null, Status = LifecycleRule.LIFECYCLE_RULE_STATUS_ENABLED } })));

Yes. This configuration works.
But we have to fix the null value issue for the Filter tag. Otherwise, it needs to be documented and it sounds pretty bad to me.

We also need to add the missing NewerNoncurrentVersions tag in NoncurrentVersionExpiration.

@MarDipp
Copy link
Author

MarDipp commented Dec 19, 2022

...
Yes. This configuration works. But we have to fix the null value issue for the Filter tag. Otherwise, it needs to be documented and it sounds pretty bad to me.

We also need to add the missing NewerNoncurrentVersions tag in NoncurrentVersionExpiration.

I added the property NewerNoncurrentVersions like in minio-go.
Additionally it is ensured that the filter is never null. Now @peterczabanski 's configuration works without change.

Copy link
Collaborator

@ebozduman ebozduman left a comment

Choose a reason for hiding this comment

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

I think there is one last issue:

NoncurrentVersionExpirationObject has become a mandatory tag and it has to be defined.
Try it with a null value and with no NoncurrentVersionExpirationObject definition in your Rule.

@MarDipp
Copy link
Author

MarDipp commented Dec 19, 2022

I think there is one last issue:

NoncurrentVersionExpirationObject has become a mandatory tag and it has to be defined. Try it with a null value and with no NoncurrentVersionExpirationObject definition in your Rule.

Have you added an other condition?

I don´t think NoncurrentVersionExpirationObject is mandatory. But you need either Expiration, TransitionObject, NoncurrentVersionExpirationObject or NoncurrentVersionTransitionObject filled. If all of these objects are missing, an XML error is thrown. Here is the code of the Rule Validation

This configuration has no NoncurrentVersionExpirationObject, but works anyway:
SetBucketLifecycleArgs setPolicyArgs = new SetBucketLifecycleArgs().WithBucket(bucketName).WithLifecycleConfiguration(new LifecycleConfiguration(new List<LifecycleRule>(new[] { new LifecycleRule { AbortIncompleteMultipartUploadObject = null, ID = "Test", Expiration = new Expiration(DateTime.UtcNow.AddYears(10)), TransitionObject = null, Filter = null, NoncurrentVersionExpirationObject = null, NoncurrentVersionTransitionObject = null, Status = LifecycleRule.LIFECYCLE_RULE_STATUS_ENABLED } })));

@ebozduman
Copy link
Collaborator

I see. At this point in the code, there is only one required tag, "Status" and the following is one of the minimum configs:

LifecycleConfiguration lfc = new LifecycleConfiguration(
    new List<LifecycleRule>(new[]
    { new LifecycleRule
        { 
            NoncurrentVersionExpirationObject = new NoncurrentVersionExpiration(31, 5), 
Glacier Flexible Retrieval"), 
            Status = LifecycleRule.LIFECYCLE_RULE_STATUS_ENABLED
        }
    }
));

while the following config fails:

LifecycleConfiguration lfc = new LifecycleConfiguration(
    new List<LifecycleRule>(new[]
    { new LifecycleRule
        { 
            Status = LifecycleRule.LIFECYCLE_RULE_STATUS_ENABLED
        }
    }
));

I guess, at least one of the tags, either transitions or expirations, needs to be provided with values.

So, I guess we can stop here.

LGTM

@ebozduman ebozduman merged commit 717563c into minio:master Dec 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants