feat(aws-s3): orphan buckets by default (#1273)

rix0rrr · web-flow · commit 2eb47ad9c857 · 2018-12-06T10:06:06.000+01:00
Switch the default RemovalPolicy of a Bucket to Orphan. This reduces friction for users with stacks that contain Buckets. In the 99% of cases right now, any activity on a Bucket will cause the stack to fail to delete, and customers are left wondering how to fix this situation (see #1269 for example). It seems better to default to the frictionless case, and have people opt-in to hard deletes if they know for a fact the operation is going to work.
diff --git a/packages/@aws-cdk/app-delivery/test/integ.cicd.ts b/packages/@aws-cdk/app-delivery/test/integ.cicd.ts
@@ -8,7 +8,9 @@ const app = new cdk.App();
 
 const stack = new cdk.Stack(app, 'CICD');
 const pipeline = new code.Pipeline(stack, 'CodePipeline', {
-  artifactBucket: new s3.Bucket(stack, 'ArtifactBucket'),
+  artifactBucket: new s3.Bucket(stack, 'ArtifactBucket', {
+    removalPolicy: cdk.RemovalPolicy.Destroy
+  })
 });
 const source = new code.GitHubSourceAction(stack, 'GitHub', {
   stage: pipeline.addStage('Source'),
diff --git a/packages/@aws-cdk/aws-cloudfront/test/integ.cloudfront-alias-target.ts b/packages/@aws-cdk/aws-cloudfront/test/integ.cloudfront-alias-target.ts
@@ -9,7 +9,9 @@ const stack = new cdk.Stack(app, 'aws-cdk-cloudfront');
 
 const zone = new route53.PublicHostedZone(stack, 'HostedZone', { zoneName: 'test.public' });
 
-const sourceBucket = new s3.Bucket(stack, 'Bucket');
+const sourceBucket = new s3.Bucket(stack, 'Bucket', {
+  removalPolicy: cdk.RemovalPolicy.Destroy
+});
 
 const distribution = new cloudfront.CloudFrontWebDistribution(stack, 'MyDistribution', {
   originConfigs: [
diff --git a/packages/@aws-cdk/aws-cloudfront/test/integ.cloudfront-bucket-logging.expected.json b/packages/@aws-cdk/aws-cloudfront/test/integ.cloudfront-bucket-logging.expected.json
@@ -70,7 +70,8 @@
       }
     },
     "AnAmazingWebsiteProbably2LoggingBucket222F7CE9": {
-      "Type": "AWS::S3::Bucket"
+      "Type": "AWS::S3::Bucket",
+      "DeletionPolicy": "Retain"
     },
     "AnAmazingWebsiteProbably2CFDistribution7C1CCD12": {
       "Type": "AWS::CloudFront::Distribution",
@@ -138,4 +139,4 @@
       }
     }
   }
-}
+}
diff --git a/packages/@aws-cdk/aws-cloudfront/test/integ.cloudfront-bucket-logging.ts b/packages/@aws-cdk/aws-cloudfront/test/integ.cloudfront-bucket-logging.ts
@@ -6,7 +6,9 @@ const app = new cdk.App();
 
 const stack = new cdk.Stack(app, 'aws-cdk-cloudfront-custom');
 
-const loggingBucket = new s3.Bucket(stack, 'Bucket');
+const loggingBucket = new s3.Bucket(stack, 'Bucket', {
+  removalPolicy: cdk.RemovalPolicy.Destroy
+});
 
 new cloudfront.CloudFrontWebDistribution(stack, 'AnAmazingWebsiteProbably', {
   originConfigs: [
diff --git a/packages/@aws-cdk/aws-cloudfront/test/integ.cloudfront-ipv6-disabled.ts b/packages/@aws-cdk/aws-cloudfront/test/integ.cloudfront-ipv6-disabled.ts
@@ -7,7 +7,9 @@ const app = new cdk.App();
 
 const stack = new cdk.Stack(app, 'aws-cdk-cloudfront');
 
-const sourceBucket = new s3.Bucket(stack, 'Bucket');
+const sourceBucket = new s3.Bucket(stack, 'Bucket', {
+  removalPolicy: cdk.RemovalPolicy.Destroy
+});
 
 new cloudfront.CloudFrontWebDistribution(stack, 'MyDistribution', {
   originConfigs: [
diff --git a/packages/@aws-cdk/aws-cloudfront/test/integ.cloudfront.ts b/packages/@aws-cdk/aws-cloudfront/test/integ.cloudfront.ts
@@ -7,7 +7,9 @@ const app = new cdk.App();
 
 const stack = new cdk.Stack(app, 'aws-cdk-cloudfront');
 
-const sourceBucket = new s3.Bucket(stack, 'Bucket');
+const sourceBucket = new s3.Bucket(stack, 'Bucket', {
+  removalPolicy: cdk.RemovalPolicy.Destroy
+});
 
 new cloudfront.CloudFrontWebDistribution(stack, 'MyDistribution', {
   originConfigs: [
diff --git a/packages/@aws-cdk/aws-cloudfront/test/test.basic.ts b/packages/@aws-cdk/aws-cloudfront/test/test.basic.ts
@@ -117,7 +117,8 @@ export = {
     expect(stack).toMatch({
       "Resources": {
         "Bucket83908E77": {
-        "Type": "AWS::S3::Bucket"
+        "Type": "AWS::S3::Bucket",
+        "DeletionPolicy": "Retain",
         },
         "AnAmazingWebsiteProbablyCFDistribution47E3983B": {
         "Type": "AWS::CloudFront::Distribution",
@@ -191,7 +192,8 @@ export = {
     expect(stack).toMatch({
       "Resources": {
         "Bucket83908E77": {
-        "Type": "AWS::S3::Bucket"
+        "Type": "AWS::S3::Bucket",
+        "DeletionPolicy": "Retain",
         },
         "AnAmazingWebsiteProbablyCFDistribution47E3983B": {
         "Type": "AWS::CloudFront::Distribution",
diff --git a/packages/@aws-cdk/aws-codebuild/test/integ.caching.ts b/packages/@aws-cdk/aws-codebuild/test/integ.caching.ts
@@ -7,7 +7,9 @@ const app = new cdk.App();
 
 const stack = new cdk.Stack(app, 'aws-cdk-codebuild');
 
-const bucket = new s3.Bucket(stack, 'CacheBucket');
+const bucket = new s3.Bucket(stack, 'CacheBucket', {
+  removalPolicy: cdk.RemovalPolicy.Destroy
+});
 
 new codebuild.Project(stack, 'MyProject', {
   cacheBucket: bucket,
diff --git a/packages/@aws-cdk/aws-codebuild/test/integ.project-bucket.ts b/packages/@aws-cdk/aws-codebuild/test/integ.project-bucket.ts
@@ -7,7 +7,9 @@ const app = new cdk.App();
 
 const stack = new cdk.Stack(app, 'aws-cdk-codebuild');
 
-const bucket = new s3.Bucket(stack, 'MyBucket');
+const bucket = new s3.Bucket(stack, 'MyBucket', {
+  removalPolicy: cdk.RemovalPolicy.Destroy
+});
 
 new codebuild.Project(stack, 'MyProject', {
   source: new codebuild.S3BucketSource({
diff --git a/packages/@aws-cdk/aws-codebuild/test/integ.project-secondary-sources-artifacts.ts b/packages/@aws-cdk/aws-codebuild/test/integ.project-secondary-sources-artifacts.ts
@@ -6,7 +6,9 @@ const app = new cdk.App();
 
 const stack = new cdk.Stack(app, 'aws-cdk-codebuild-secondary-sources-artifacts');
 
-const bucket = new s3.Bucket(stack, 'MyBucket');
+const bucket = new s3.Bucket(stack, 'MyBucket', {
+  removalPolicy: cdk.RemovalPolicy.Destroy
+});
 
 new codebuild.Project(stack, 'MyProject', {
   buildSpec: {
diff --git a/packages/@aws-cdk/aws-codebuild/test/test.codebuild.ts b/packages/@aws-cdk/aws-codebuild/test/test.codebuild.ts
@@ -307,7 +307,8 @@ export = {
       expect(stack).toMatch({
         "Resources": {
         "MyBucketF68F3FF0": {
-          "Type": "AWS::S3::Bucket"
+          "Type": "AWS::S3::Bucket",
+          "DeletionPolicy": "Retain"
         },
         "MyProjectRole9BBE5233": {
           "Type": "AWS::IAM::Role",
diff --git a/packages/@aws-cdk/aws-codepipeline/test/integ.lambda-pipeline.ts b/packages/@aws-cdk/aws-codepipeline/test/integ.lambda-pipeline.ts
@@ -12,6 +12,7 @@ const pipeline = new codepipeline.Pipeline(stack, 'Pipeline');
 const sourceStage = new codepipeline.Stage(pipeline, 'Source', { pipeline });
 const bucket = new s3.Bucket(stack, 'PipelineBucket', {
   versioned: true,
+  removalPolicy: cdk.RemovalPolicy.Destroy,
 });
 new s3.PipelineSourceAction(stack, 'Source', {
   stage: sourceStage,
diff --git a/packages/@aws-cdk/aws-codepipeline/test/integ.pipeline-cfn-cross-region.ts b/packages/@aws-cdk/aws-codepipeline/test/integ.pipeline-cfn-cross-region.ts
@@ -14,6 +14,7 @@ const stack = new cdk.Stack(app, 'aws-cdk-codepipeline-cloudformation-cross-regi
 
 const bucket = new s3.Bucket(stack, 'MyBucket', {
   versioned: true,
+  removalPolicy: cdk.RemovalPolicy.Destroy,
 });
 
 const pipeline = new codepipeline.Pipeline(stack, 'MyPipeline', {
diff --git a/packages/@aws-cdk/aws-codepipeline/test/integ.pipeline-cfn.ts b/packages/@aws-cdk/aws-codepipeline/test/integ.pipeline-cfn.ts
@@ -14,6 +14,7 @@ const pipeline = new codepipeline.Pipeline(stack, 'Pipeline');
 const sourceStage = new codepipeline.Stage(pipeline, 'Source', { pipeline });
 const bucket = new s3.Bucket(stack, 'PipelineBucket', {
   versioned: true,
+  removalPolicy: cdk.RemovalPolicy.Destroy,
 });
 const source = new s3.PipelineSourceAction(stack, 'Source', {
   stage: sourceStage,
diff --git a/packages/@aws-cdk/aws-codepipeline/test/integ.pipeline-code-build-multiple-inputs-outputs.ts b/packages/@aws-cdk/aws-codepipeline/test/integ.pipeline-code-build-multiple-inputs-outputs.ts
@@ -13,6 +13,7 @@ const repository = new codecommit.Repository(stack, 'MyRepo', {
 });
 const bucket = new s3.Bucket(stack, 'MyBucket', {
   versioned: true,
+  removalPolicy: cdk.RemovalPolicy.Destroy,
 });
 
 const pipeline = new codepipeline.Pipeline(stack, 'Pipeline', {
diff --git a/packages/@aws-cdk/aws-codepipeline/test/integ.pipeline-code-deploy.ts b/packages/@aws-cdk/aws-codepipeline/test/integ.pipeline-code-deploy.ts
@@ -23,6 +23,7 @@ const deploymentGroup = new codedeploy.ServerDeploymentGroup(stack, 'CodeDeployG
 
 const bucket = new s3.Bucket(stack, 'CodeDeployPipelineIntegTest', {
   versioned: true,
+  removalPolicy: cdk.RemovalPolicy.Destroy,
 });
 
 const pipeline = new codepipeline.Pipeline(stack, 'Pipeline', {
diff --git a/packages/@aws-cdk/aws-codepipeline/test/integ.pipeline-ecr-source.ts b/packages/@aws-cdk/aws-codepipeline/test/integ.pipeline-ecr-source.ts
@@ -7,7 +7,9 @@ const app = new cdk.App();
 
 const stack = new cdk.Stack(app, 'aws-cdk-codepipeline-ecr-source');
 
-const bucket = new s3.Bucket(stack, 'MyBucket');
+const bucket = new s3.Bucket(stack, 'MyBucket', {
+  removalPolicy: cdk.RemovalPolicy.Destroy,
+});
 const pipeline = new codepipeline.Pipeline(stack, 'MyPipeline', {
   artifactBucket: bucket,
 });
diff --git a/packages/@aws-cdk/aws-lambda-event-sources/test/integ.s3.ts b/packages/@aws-cdk/aws-lambda-event-sources/test/integ.s3.ts
@@ -8,7 +8,9 @@ class S3EventSourceTest extends cdk.Stack {
     super(parent, id);
 
     const fn = new TestFunction(this, 'F');
-    const bucket = new s3.Bucket(this, 'B');
+    const bucket = new s3.Bucket(this, 'B', {
+      removalPolicy: cdk.RemovalPolicy.Destroy
+    });
 
     fn.addEventSource(new S3EventSource(bucket, {
       events: [ s3.EventType.ObjectCreated ],
@@ -19,4 +21,4 @@ class S3EventSourceTest extends cdk.Stack {
 
 const app = new cdk.App();
 new S3EventSourceTest(app, 'lambda-event-source-s3');
-app.run();
+app.run();
diff --git a/packages/@aws-cdk/aws-lambda/test/integ.bucket-notifications.ts b/packages/@aws-cdk/aws-lambda/test/integ.bucket-notifications.ts
@@ -6,15 +6,19 @@ const app = new cdk.App();
 
 const stack = new cdk.Stack(app, 'lambda-bucket-notifications');
 
-const bucketA = new s3.Bucket(stack, 'MyBucket');
+const bucketA = new s3.Bucket(stack, 'MyBucket', {
+  removalPolicy: cdk.RemovalPolicy.Destroy
+});
 
 const fn = new lambda.Function(stack, 'MyFunction', {
   runtime: lambda.Runtime.NodeJS610,
   handler: 'index.handler',
   code: lambda.Code.inline(`exports.handler = ${handler.toString()}`)
 });
 
-const bucketB = new s3.Bucket(stack, 'YourBucket');
+const bucketB = new s3.Bucket(stack, 'YourBucket', {
+  removalPolicy: cdk.RemovalPolicy.Destroy
+});
 
 bucketA.onObjectCreated(fn, { suffix: '.png' });
 bucketB.onEvent(s3.EventType.ObjectRemoved, fn);
diff --git a/packages/@aws-cdk/aws-s3-deployment/test/integ.bucket-deployment.expected.json b/packages/@aws-cdk/aws-s3-deployment/test/integ.bucket-deployment.expected.json
@@ -343,7 +343,8 @@
       ]
     },
     "Destination281A09BDF": {
-      "Type": "AWS::S3::Bucket"
+      "Type": "AWS::S3::Bucket",
+      "DeletionPolicy": "Retain"
     },
     "DeployWithPrefixCustomResource9CF7C694": {
       "Type": "Custom::CDKBucketDeployment",
diff --git a/packages/@aws-cdk/aws-s3-deployment/test/integ.bucket-deployment.ts b/packages/@aws-cdk/aws-s3-deployment/test/integ.bucket-deployment.ts
@@ -10,6 +10,7 @@ class TestBucketDeployment extends cdk.Stack {
     const destinationBucket = new s3.Bucket(this, 'Destination', {
       websiteIndexDocument: 'index.html',
       publicReadAccess: true,
+      removalPolicy: cdk.RemovalPolicy.Destroy
     });
 
     new s3deploy.BucketDeployment(this, 'DeployMe', {
@@ -33,4 +34,4 @@ const app = new cdk.App();
 
 new TestBucketDeployment(app, 'test-bucket-deployments-1');
 
-app.run();
+app.run();
diff --git a/packages/@aws-cdk/aws-s3/lib/bucket.ts b/packages/@aws-cdk/aws-s3/lib/bucket.ts
@@ -367,7 +367,7 @@ export interface BucketProps {
   /**
    * Policy to apply when the bucket is removed from this stack.
    *
-   * @default By default, the bucket will be destroyed if it is removed from the stack.
+   * @default The bucket will be orphaned
    */
   removalPolicy?: cdk.RemovalPolicy;
 
@@ -435,7 +435,7 @@ export class Bucket extends BucketRef {
       websiteConfiguration: this.renderWebsiteConfiguration(props)
     });
 
-    cdk.applyRemovalPolicy(resource, props.removalPolicy);
+    cdk.applyRemovalPolicy(resource, props.removalPolicy !== undefined ? props.removalPolicy : cdk.RemovalPolicy.Orphan);
 
     this.versioned = props.versioned;
     this.encryptionKey = encryptionKey;
diff --git a/packages/@aws-cdk/aws-s3/test/integ.bucket.domain-name.ts b/packages/@aws-cdk/aws-s3/test/integ.bucket.domain-name.ts
@@ -6,7 +6,9 @@ class TestStack extends cdk.Stack {
     super(parent, id);
 
     /// !show
-    const bucket = new s3.Bucket(this, 'MyBucket');
+    const bucket = new s3.Bucket(this, 'MyBucket', {
+      removalPolicy: cdk.RemovalPolicy.Destroy
+    });
     const bucket2 = s3.Bucket.import(this, "MyBucket2", {
       bucketArn: "arn:aws:s3:::my-bucket-test"
     });
diff --git a/packages/@aws-cdk/aws-s3/test/integ.bucket.ts b/packages/@aws-cdk/aws-s3/test/integ.bucket.ts
@@ -8,11 +8,13 @@ const app = new cdk.App();
 const stack = new cdk.Stack(app, 'aws-cdk-s3');
 
 const bucket = new s3.Bucket(stack, 'MyBucket', {
-  encryption: s3.BucketEncryption.Kms
+  encryption: s3.BucketEncryption.Kms,
+  removalPolicy: cdk.RemovalPolicy.Destroy
 });
 
 const otherwiseEncryptedBucket = new s3.Bucket(stack, 'MyOtherBucket', {
-  encryption: s3.BucketEncryption.S3Managed
+  encryption: s3.BucketEncryption.S3Managed,
+  removalPolicy: cdk.RemovalPolicy.Destroy
 });
 
 const user = new iam.User(stack, 'MyUser');
diff --git a/packages/@aws-cdk/aws-s3/test/integ.bucket.url.lit.ts b/packages/@aws-cdk/aws-s3/test/integ.bucket.url.lit.ts
@@ -6,7 +6,9 @@ class TestStack extends cdk.Stack {
     super(parent, id);
 
     /// !show
-    const bucket = new s3.Bucket(this, 'MyBucket');
+    const bucket = new s3.Bucket(this, 'MyBucket', {
+      removalPolicy: cdk.RemovalPolicy.Destroy
+    });
 
     new cdk.Output(this, 'BucketURL', { value: bucket.bucketUrl });
     new cdk.Output(this, 'ObjectURL', { value: bucket.urlForObject('myfolder/myfile.txt') });
diff --git a/packages/@aws-cdk/aws-s3/test/integ.lifecycle.ts b/packages/@aws-cdk/aws-s3/test/integ.lifecycle.ts
@@ -1,4 +1,4 @@
-import { App, Stack } from '@aws-cdk/cdk';
+import { App, RemovalPolicy, Stack } from '@aws-cdk/cdk';
 import { Bucket } from '../lib';
 
 const app = new App();
@@ -9,7 +9,8 @@ const stack = new Stack(app, 'aws-cdk-s3');
 new Bucket(stack, 'MyBucket', {
   lifecycleRules: [{
     expirationDate: new Date('2019-10-01')
-  }]
+  }],
+  removalPolicy: RemovalPolicy.Destroy
 });
 
 app.run();
diff --git a/packages/@aws-cdk/aws-s3/test/integ.notifications.ts b/packages/@aws-cdk/aws-s3/test/integ.notifications.ts
@@ -7,14 +7,18 @@ const app = new cdk.App();
 
 const stack = new Stack(app, 'test-3');
 
-const bucket = new s3.Bucket(stack, 'Bucket');
+const bucket = new s3.Bucket(stack, 'Bucket', {
+  removalPolicy: cdk.RemovalPolicy.Destroy
+});
 const topic = new Topic(stack, 'Topic');
 const topic3 = new Topic(stack, 'Topic3');
 
 bucket.onEvent(s3.EventType.ObjectCreatedPut, topic);
 bucket.onEvent(s3.EventType.ObjectRemoved, topic3, { prefix: 'home/myusername/' });
 
-const bucket2 = new s3.Bucket(stack, 'Bucket2');
+const bucket2 = new s3.Bucket(stack, 'Bucket2', {
+  removalPolicy: cdk.RemovalPolicy.Destroy
+});
 bucket2.onObjectRemoved(topic3, { prefix: 'foo' }, { suffix: 'foo/bar' });
 
 app.run();
diff --git a/packages/@aws-cdk/aws-s3/test/test.bucket.ts b/packages/@aws-cdk/aws-s3/test/test.bucket.ts
diff --git a/packages/@aws-cdk/aws-s3/test/test.notifications.ts b/packages/@aws-cdk/aws-s3/test/test.notifications.ts
diff --git a/packages/@aws-cdk/aws-sns/test/integ.sns-bucket-notifications.ts b/packages/@aws-cdk/aws-sns/test/integ.sns-bucket-notifications.ts
diff --git a/packages/@aws-cdk/aws-sqs/test/integ.bucket-notifications.ts b/packages/@aws-cdk/aws-sqs/test/integ.bucket-notifications.ts

Original file line number	Diff line number	Diff line change
`@@ -70,7 +70,8 @@`
`70`	`70`	`}`
`71`	`71`	`},`
`72`	`72`	`"AnAmazingWebsiteProbably2LoggingBucket222F7CE9": {`
`73`		`- "Type": "AWS::S3::Bucket"`
	`73`	`+ "Type": "AWS::S3::Bucket",`
	`74`	`+ "DeletionPolicy": "Retain"`
`74`	`75`	`},`
`75`	`76`	`"AnAmazingWebsiteProbably2CFDistribution7C1CCD12": {`
`76`	`77`	`"Type": "AWS::CloudFront::Distribution",`
`@@ -138,4 +139,4 @@`
`138`	`139`	`}`
`139`	`140`	`}`
`140`	`141`	`}`
`141`		`-}`
	`142`	`+}`