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

(S3): Cannot specify number of noncurrent versions to retain in LifeCycle rule without transitioning the StorageClass #19784

Closed
KomaGR opened this issue Apr 6, 2022 · 4 comments · Fixed by #20348
Assignees
Labels
@aws-cdk/aws-s3 Related to Amazon S3 effort/small Small work item – less than a day of effort feature/enhancement A new API to make things easier or more intuitive. A catch-all for general feature requests. feature-request A feature should be added or improved. good first issue Related to contributions. See CONTRIBUTING.md p1

Comments

@KomaGR
Copy link

KomaGR commented Apr 6, 2022

Describe the bug

Using the lifecycleRules configuration of a versioned bucket, I cannot specify how many noncurrent versions to retain without triggering any storage transition. (I want the objects to stay in STANDARD tier but that is also not available under s3.StorageClass.)

What I'm trying to achieve
I want to delete all object versions older than one day but I want to keep the five latest versions for each object (in the same storage tier). I don't want the older versions to transition to another tier since I also want to delete them after a single day if, i.e., another batch of 5 versions comes through.

Expected Behavior

The Console seems to allow exactly what I'm going after:
image

Current Behavior

Currently, the option for how many noncurrent versions to retain noncurrentVersionsToRetain is under the noncurrentVersionTransitions configuration option, which forces to specify transitionAfter and storageClass. The Console UI (see above) suggests that I would not need to specify these and only need specify noncurrentVersionExpiration.

{
    enabled: true,
    prefix: "usr_",
    noncurrentVersionExpiration: Duration.days(1),
    expiredObjectDeleteMarker: true,
    abortIncompleteMultipartUploadAfter: Duration.days(1),
    noncurrentVersionTransitions: [
        {
            transitionAfter: Duration.days(1),
            noncurrentVersionsToRetain: 5,
            storageClass: /* Side issue: There is no s3.StorageClass.STANDARD */
        }
    ]
}

Reproduction Steps

Here is the Object I'm trying to create with the noncurrentVersionTransitions config option commented out:

new s3.Bucket(this, 'bugBucket', {
            versioned: true,
            lifecycleRules: [
                {
                    enabled: true,
                    prefix: "prefix",
                    noncurrentVersionExpiration: Duration.days(1),
                    expiredObjectDeleteMarker: true,
                    abortIncompleteMultipartUploadAfter: Duration.days(1),
//                    noncurrentVersionTransitions: [
//                        {
//                            transitionAfter: Duration.days(1),
//                            noncurrentVersionsToRetain: 5,
//                            storageClass: /*  */
//                        }
//                    ]
                }
            ]
        });

Possible Solution

Judging by the UI, it looks like noncurrentVersionsToRetain should be a property of the Lifecycle rule and not nested into noncurrentVersionTransitions.

Additional Information/Context

No response

CDK CLI Version

2.19.0 (build e0d3e62)

Framework Version

[email protected]

Node.js Version

v16.13.2

OS

Ubuntu 20.04

Language

Typescript

Language Version

No response

Other information

No response

@KomaGR KomaGR added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Apr 6, 2022
@github-actions github-actions bot added the @aws-cdk/aws-s3 Related to Amazon S3 label Apr 6, 2022
@NGL321 NGL321 added feature-request A feature should be added or improved. feature/enhancement A new API to make things easier or more intuitive. A catch-all for general feature requests. p2 p1 effort/small Small work item – less than a day of effort and removed bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. p2 labels Apr 7, 2022
@NGL321
Copy link
Contributor

NGL321 commented Apr 7, 2022

Hey @KomaGR,

Thank you for reporting this. I apologize for contradicting my previous comment, but after a bit more digging I believe that this would be impossible to implement without a request to the S3 team. In the L1 construct this is represented through a JSON structure that would be submitted to the S3 API, so I looked into the S3 documentation itself and found that retention is always declared through the NoncurrentVersionTransition action element. Please do let me know if you think this is incorrect and I can dig a little deeper

@NGL321 NGL321 added the closing-soon This issue will automatically close in 4 days unless further comments are made. label Apr 7, 2022
@NGL321 NGL321 self-assigned this Apr 7, 2022
@NGL321 NGL321 added response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. and removed closing-soon This issue will automatically close in 4 days unless further comments are made. labels Apr 7, 2022
@KomaGR
Copy link
Author

KomaGR commented Apr 7, 2022

Hi @NGL321,

thanks for taking a look at this. I dug a bit deeper in the links you shared and I think there might be more to this.

It looks like the L1 construct supports retaining a specific number of versions for both expiration and transition actions:

    noncurrentVersionExpiration: {
      noncurrentDays: 123,

      // the properties below are optional
      newerNoncurrentVersions: 123,    // <- This field
    },
    noncurrentVersionTransition: {
      storageClass: 'storageClass',
      transitionInDays: 123,

      // the properties below are optional
      newerNoncurrentVersions: 123,    // <- This field
    },

The way I interpret this is that the user can:

  1. use noncurrentVersionTransition.newerNoncurrentVersions to retain a number of versions to not be not transitioned by the rule, and
  2. use noncurrentVersionExpiration.newerNoncurrentVersions to retain a number of versions to not be not expired by the rule.

The realization that both these support a newerNoncurrentVersions option is confirmed by the linked documentation page where under both "NoncurrentVersionTransition action element" and "NoncurrentVersionExpiration action element" it says:

In addition to the number of days, you can also provide a maximum number of noncurrent versions to retain.

(The point is that the above is found two separate times in the doc page.)

Setting these two retention numbers separately is possible to do from the Console.

In conclusion

The noncurrentVersionTransitions.noncurrentVersionsToRetain option seems to not be misplaced. At the same time, it looks like there needs to be an option for the expiration that goes with noncurrentVersionExpiration. Name could be noncurrentVersionsToRetain if not nested (or noncurrentVersionsToNotExpire if it needs to be different for clarity). Alternatively, you can make noncurrentVersionExpiration a nested configuration (similar to noncurrentVersionTransitions) to reflect the underlying L1 construct options.

Example with expiration only (what I'm trying to achieve)

Since I have done this through the Console as a demonstration, here is what the Lifecycle rule configuration JSON looks like when I set the number of versions to retain not-expired:

{
    "Rules": [
        {
            "Expiration": {
                "ExpiredObjectDeleteMarker": true
            },
            "ID": "<redacted>",
            "Filter": {
                "Prefix": "prefix"
            },
            "Status": "Enabled",
            "NoncurrentVersionExpiration": {
                "NoncurrentDays": 1,
                "NewerNoncurrentVersions": 5
            },
            "AbortIncompleteMultipartUpload": {
                "DaysAfterInitiation": 1
            }
        }
    ]
}

and here is the interpretation of this from the Console:
image

Example with transition and then expiration

Here is what the configuration looks like with both transition and expiration configured (had to increase the days due to STANDARD-IA constraints):

{
    "Rules": [
        {
            "Expiration": {
                "ExpiredObjectDeleteMarker": true
            },
            "ID": "<redacted>",
            "Filter": {
                "Prefix": "prefix"
            },
            "Status": "Enabled",
            "NoncurrentVersionTransitions": [
                {
                    "NoncurrentDays": 30,
                    "StorageClass": "STANDARD_IA",
                    "NewerNoncurrentVersions": 5
                }
            ],
            "NoncurrentVersionExpiration": {
                "NoncurrentDays": 31,
                "NewerNoncurrentVersions": 5
            },
            "AbortIncompleteMultipartUpload": {
                "DaysAfterInitiation": 1
            }
        }
    ]
}

and here is the interpretation of this from the Console:
image

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Apr 7, 2022
@rix0rrr
Copy link
Contributor

rix0rrr commented Apr 20, 2022

We need a noncurrentVersionsToRetain property, which renders into the CloudFormation NoncurrentVersionExpiration object.

PRs welcome.

@rix0rrr rix0rrr added the good first issue Related to contributions. See CONTRIBUTING.md label Apr 20, 2022
@mergify mergify bot closed this as completed in #20348 May 19, 2022
mergify bot pushed a commit that referenced this issue May 19, 2022
…#20348)

Fixes #19784

----

### All Submissions:

* [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [x] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/master/INTEGRATION_TESTS.md)?
	* [x] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

wphilipw pushed a commit to wphilipw/aws-cdk that referenced this issue May 23, 2022
…aws#20348)

Fixes aws#19784

----

### All Submissions:

* [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [x] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/master/INTEGRATION_TESTS.md)?
	* [x] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-s3 Related to Amazon S3 effort/small Small work item – less than a day of effort feature/enhancement A new API to make things easier or more intuitive. A catch-all for general feature requests. feature-request A feature should be added or improved. good first issue Related to contributions. See CONTRIBUTING.md p1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants