Skip to content

Commit

Permalink
fix(s3): remove restriction of creating lifecycle rule for noncurrent…
Browse files Browse the repository at this point in the history
… objects when bucket versionining is not set up (#22803)

fixes #22392

----

### All Submissions:

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

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/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/main/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*
  • Loading branch information
clueleaf committed Nov 7, 2022
1 parent ddee5cf commit b20a6b4
Show file tree
Hide file tree
Showing 9 changed files with 12 additions and 53 deletions.
8 changes: 0 additions & 8 deletions packages/@aws-cdk/aws-s3/lib/bucket.ts
Expand Up @@ -1766,7 +1766,6 @@ export class Bucket extends BucketBase {
protected disallowPublicAccess?: boolean;
private accessControl?: BucketAccessControl;
private readonly lifecycleRules: LifecycleRule[] = [];
private readonly versioned?: boolean;
private readonly eventBridgeEnabled?: boolean;
private readonly metrics: BucketMetrics[] = [];
private readonly cors: CorsRule[] = [];
Expand Down Expand Up @@ -1807,7 +1806,6 @@ export class Bucket extends BucketBase {

resource.applyRemovalPolicy(props.removalPolicy);

this.versioned = props.versioned;
this.encryptionKey = encryptionKey;
this.eventBridgeEnabled = props.eventBridgeEnabled;

Expand Down Expand Up @@ -1872,12 +1870,6 @@ export class Bucket extends BucketBase {
* @param rule The rule to add
*/
public addLifecycleRule(rule: LifecycleRule) {
if ((rule.noncurrentVersionExpiration !== undefined
|| (rule.noncurrentVersionTransitions && rule.noncurrentVersionTransitions.length > 0))
&& !this.versioned) {
throw new Error("Cannot use 'noncurrent' rules on a nonversioned bucket");
}

this.lifecycleRules.push(rule);
}

Expand Down
@@ -1,15 +1,15 @@
{
"version": "20.0.0",
"version": "21.0.0",
"files": {
"679bf529d9a3ed82ef264cdba57192bff86674a9ce36d45db3ec5c344ae364c0": {
"928a24cbf7a16b7c4d15a9ceca38a2f7f016495b93db9bdf0160accc3d13876c": {
"source": {
"path": "aws-cdk-s3.template.json",
"packaging": "file"
},
"destinations": {
"current_account-current_region": {
"bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}",
"objectKey": "679bf529d9a3ed82ef264cdba57192bff86674a9ce36d45db3ec5c344ae364c0.json",
"objectKey": "928a24cbf7a16b7c4d15a9ceca38a2f7f016495b93db9bdf0160accc3d13876c.json",
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}"
}
}
Expand Down
Expand Up @@ -13,9 +13,6 @@
"Status": "Enabled"
}
]
},
"VersioningConfiguration": {
"Status": "Enabled"
}
},
"UpdateReplacePolicy": "Retain",
Expand Down
@@ -1 +1 @@
{"version":"20.0.0"}
{"version":"21.0.0"}
@@ -1,5 +1,5 @@
{
"version": "20.0.0",
"version": "21.0.0",
"testCases": {
"integ.lifecycle-expiration": {
"stacks": [
Expand Down
@@ -1,5 +1,5 @@
{
"version": "20.0.0",
"version": "21.0.0",
"artifacts": {
"Tree": {
"type": "cdk:tree",
Expand All @@ -23,7 +23,7 @@
"validateOnSynth": false,
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-deploy-role-${AWS::AccountId}-${AWS::Region}",
"cloudFormationExecutionRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-cfn-exec-role-${AWS::AccountId}-${AWS::Region}",
"stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/679bf529d9a3ed82ef264cdba57192bff86674a9ce36d45db3ec5c344ae364c0.json",
"stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/928a24cbf7a16b7c4d15a9ceca38a2f7f016495b93db9bdf0160accc3d13876c.json",
"requiresBootstrapStackVersion": 6,
"bootstrapStackVersionSsmParameter": "/cdk-bootstrap/hnb659fds/version",
"additionalDependencies": [
Expand Down
Expand Up @@ -9,7 +9,7 @@
"path": "Tree",
"constructInfo": {
"fqn": "constructs.Construct",
"version": "10.1.85"
"version": "10.1.140"
}
},
"aws-cdk-s3": {
Expand All @@ -36,9 +36,6 @@
"status": "Enabled"
}
]
},
"versioningConfiguration": {
"status": "Enabled"
}
}
},
Expand All @@ -55,14 +52,14 @@
}
},
"constructInfo": {
"fqn": "constructs.Construct",
"version": "10.1.85"
"fqn": "@aws-cdk/core.Stack",
"version": "0.0.0"
}
}
},
"constructInfo": {
"fqn": "constructs.Construct",
"version": "10.1.85"
"fqn": "@aws-cdk/core.App",
"version": "0.0.0"
}
}
}
Expand Up @@ -10,7 +10,6 @@ new Bucket(stack, 'MyBucket', {
noncurrentVersionExpiration: Duration.days(30),
noncurrentVersionsToRetain: 123,
}],
versioned: true,
});

app.synth();
26 changes: 0 additions & 26 deletions packages/@aws-cdk/aws-s3/test/rules.test.ts
Expand Up @@ -96,28 +96,6 @@ describe('rules', () => {
});
});

test('Noncurrent rule on nonversioned bucket fails', () => {
// GIVEN
const stack = new Stack();

// WHEN: Fail because of lack of versioning
expect(() => {
new Bucket(stack, 'Bucket1', {
lifecycleRules: [{
noncurrentVersionExpiration: Duration.days(10),
}],
});
}).toThrow();

// WHEN: Succeeds because versioning is enabled
new Bucket(stack, 'Bucket2', {
versioned: true,
lifecycleRules: [{
noncurrentVersionExpiration: Duration.days(10),
}],
});
});

test('Bucket with expiredObjectDeleteMarker', () => {
// GIVEN
const stack = new Stack();
Expand Down Expand Up @@ -146,7 +124,6 @@ describe('rules', () => {

// WHEN: Noncurrent version to retain available
new Bucket(stack, 'Bucket1', {
versioned: true,
lifecycleRules: [{
noncurrentVersionExpiration: Duration.days(10),
noncurrentVersionTransitions: [
Expand Down Expand Up @@ -185,7 +162,6 @@ describe('rules', () => {

// WHEN: Noncurrent version to retain not set
new Bucket(stack, 'Bucket1', {
versioned: true,
lifecycleRules: [{
noncurrentVersionExpiration: Duration.days(10),
noncurrentVersionTransitions: [
Expand Down Expand Up @@ -222,7 +198,6 @@ describe('rules', () => {

// WHEN: Noncurrent version to retain available
new Bucket(stack, 'Bucket1', {
versioned: true,
lifecycleRules: [{
noncurrentVersionExpiration: Duration.days(10),
noncurrentVersionsToRetain: 1,
Expand Down Expand Up @@ -261,7 +236,6 @@ describe('rules', () => {

// WHEN: Noncurrent version to retain not set
new Bucket(stack, 'Bucket1', {
versioned: true,
lifecycleRules: [{
noncurrentVersionExpiration: Duration.days(10),
noncurrentVersionTransitions: [
Expand Down

0 comments on commit b20a6b4

Please sign in to comment.