Feature Request: Option to clear and delete S3 bucket on delete

AlexCheema commented 5 years ago

Note: for support questions, please first reference our documentation, then use Stackoverflow. This repository's issues are intended for feature requests and bug reports.

I'm submitting a ...
- [ ] :beetle: bug report
- [X] :rocket: feature request
- [ ] :books: construct library gap
- [ ] :phone: security issue or vulnerability => Please see policy
- [ ] :question: support request => Please see note at the top of this template.
What is the current behavior? If the current behavior is a :beetle:bug:beetle:: Please provide the steps to reproduce

S3 Buckets currently do not get deleted if they contain objects.

What is the expected behavior (or behavior of feature suggested)?

There should be an option to override this behaviour and force the S3 bucket to be cleared and deleted.

What is the motivation / use case for changing the behavior or adding this feature?

If you are doing frequent deploys with different stack names, you will eventually stack up many s3 buckets that have been left behind from previous deploys. Eventually, this hits the default limit of 100 S3 buckets per account and it is a nightmare to selectively delete the ones that you don't want.

nmussy commented 5 years ago

Hey @AlexCheema,

There's already a removalPolicy property available, which you can set to RemovalPolicy.DESTROY:

https://github.com/aws/aws-cdk/blob/f68411c672d05846048a72ff56f8ea82bc4faaae/packages/%40aws-cdk/aws-s3/lib/bucket.ts#L733

Obirah commented 5 years ago

The RemovalPolicy will only work if the Bucket is empty. I think, Alex also would like to see something like a flag that allows user to force the deletion of all bucket contents before it is deleted.

I would like that feature a lot as well, since we are working around this with a CustomResource + Lambda function right now.

nmussy commented 5 years ago

Apologies. There's a comment related to this issue: https://github.com/aws/aws-cdk/issues/2526#issuecomment-492687957

AlexCheema commented 5 years ago

@Obirah Yes, exactly. The RemovalPolicy only works if the bucket is empty. I imagine this is a source of frustration for a lot of people and this feature would save devs a lot of time, instead of having to write their own custom solution. @nmussy Great that it already exists. Perhaps this can be integrated into the standard library. A simple flag that can be specified at bucket creation indicating whether the bucket should be cleared and deleted would be great.

NGL321 commented 5 years ago

Hey @AlexCheema,

Unfortunately, @Obirah is correct. The DeletionPolicy attribute in Cloudformation will only delete if the bucket is completely empty. I understand the desire for this functionality, however we are somewhat limited by Cloudformation support.

Imo the fastest way to see movement here would be to put in a request to the Cloudformation team on their forums.

schof commented 5 years ago

Check out the custom CDK resource I wrote to address this specific purpose: @mobileposse/auto-delete-bucket

AlexCheema commented 5 years ago

Hey @AlexCheema,

Unfortunately, @Obirah is correct. The DeletionPolicy attribute in Cloudformation will only delete if the bucket is completely empty. I understand the desire for this functionality, however we are somewhat limited by Cloudformation support.

Imo the fastest way to see movement here would be to put in a request to the Cloudformation team on their forums.

Hey @NGL321,

Fortunately, CDK has support for custom resources which allows us to build things on top of the standard Cloudformation featureset. @schof already made something for it! My suggestion was to include this custom resource in CDK - perhaps there could be a flag on the bucket for this autoDelete.

eladb commented 5 years ago

@schof would you be interested to submit a PR to incorporate your support for bucket auto-deletion into the @aws-cdk/aws-s3 library?

schof commented 5 years ago

@eladb I'm open to it, but I'm a bit short on time right now with some pressing deadlines. I agree that this is a common use case and this functionality belongs in CDK proper. There's also one existing bug I've yet to look at in detail, which is that if you rename the bucket it will no longer auto-delete. That seems solvable though.

pmarques commented 5 years ago

I'm also been thinking on a solution for this problem. Right now we have custom resources everywhere with inline lambdas in our CF templates. I'm trying to make it transparent as possible for the developers, and by that I mean not having much boilerplate on their side (CDK or CF) and also avoiding to have extra resources in their stacks. I was also looking for some kind of hooks for CDK, like pre-deploy and post-deploy events that we can use to do things like:

Cleanup Resources: S3, Cloudwatch Groups, etc
Smoke tests and Route 53 changes for some Blue-Green deployments
possible other use cases.

I can't find anything about this in Issues/PRs, does anyone have any thoughts?

EDIT: seems to be discussed here: https://github.com/aws/aws-cdk/issues/922

schof commented 5 years ago

I would love to see post deploy hooks for a variety of reasons (unrelated to bucket cleanup which I've solved with @mobileposse/auto-delete-bucket)

On Mon, Aug 19, 2019 at 10:09 AM Patrick Marques notifications@github.com wrote:

I'm also been thinking on a solution for this problem. Right now we have custom resources everywhere with inline lambdas in our CF templates. I'm trying to make it transparent as possible for the developers, and by that I mean not having much boilerplate on their side (CDK or CF) and also avoiding to have extra resources in their stacks. I was also looking for some kind of hooks for CDK, like pre-deploy and post-deploy events that we can use to do things like:

Cleanup Resources: S3, Cloudwatch Groups, etc

Smoke tests and Route 53 changes for some Blue-Green deployments

possible other use cases.

I can't find anything about this in Issues/PRs, does anyone have any thoughts?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/aws/aws-cdk/issues/3297?email_source=notifications&email_token=AAAU4DQUEIPBOSCJ2NGJEXLQFKSRTA5CNFSM4ICCQ6QKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD4TB7DQ#issuecomment-522592142, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAU4DWC55FIK6CHXDVPOX3QFKSRTANCNFSM4ICCQ6QA .

AlexCheema commented 4 years ago

:+1:

iliapolo commented 4 years ago

@schof Are you still up for contributing a PR to implement this? :)

zoonderkins commented 4 years ago

need this feature 👍

schof commented 4 years ago

I don't think it would be hard to incorporate this into CDK but unfortunately I don't have the time right now to get up to speed with the build framework, test suite etc. necessary for a proper PR. We've come up with the idea, the code and proved the concept. Just need someone on the CDK team to take it the last mile. /cc @eladb

Obirah commented 4 years ago

There's also the AutoDeleteBucket construct: https://github.com/mobileposse/auto-delete-bucket/blob/master/README.md

Maybe parts of this approach could be incorporated into the CDK.

heiba commented 4 years ago

@schof Thanks for your great work on the custom CDK Resource you wrote @mobileposse/auto-delete-bucket which helps to resolve the limitation in CloudFormation regarding destroying non-empty S3 buckets.

Do you reckon we can do something similar for ECR ? We would need to build a Custom CDK Resource to force delete ECR repos with existing images https://github.com/aws/aws-cdk/issues/2765. The difference is that ECR does have a force delete option in the CLI but not in CloudFormation.

schof commented 4 years ago

@heiba If it can be done in a lambda, and in a reasonable period of time, then it can be done with a custom resource. Lambda automatically has the JS SDK available to it but if what you need is only in the CLI then you will have to webpack/parcel the stuff you need. The auto-delete project is a good working example of a custom resource and perhaps you can extend an existing ECR related resource like I did with S3. Good luck!

srinivasreddych commented 4 years ago

Hi Team. We are seeing this issue when CDK stacks are destroyed and leaves too many orphaned s3 buckets. I see the PR would help for future efficient cleanup, but i was curious if there is a pattern i can follow for cleaning up the existing orphaned S3 buckets across accounts. Thanks!

Dzhuneyt commented 4 years ago

We have the same problem in one of our projects. For this reason, I created a simple command line script that cleans up old orphaned S3 buckets that are no longer referred to by any CloudFormation stacks: https://gist.github.com/Dzhuneyt/53d57e1234cafb956791ddcc1ba66406

Hi Team. We are seeing this issue when CDK stacks are destroyed and leaves too many orphaned s3 buckets. I see the PR would help for future efficient cleanup, but i was curious if there is a pattern i can follow for cleaning up the existing orphaned S3 buckets across accounts. Thanks!

AlvinMengCao commented 3 years ago

@srinivasreddych To clean up orphaned buckets, what I am doing is, when sending the bucket deleting CFN update, also give the bucket a tag, so that later I can find all tagged resources and delete them with a script.

resourcegroupstaggingapi gives you the ability to search all resources within your account, with tag filter, service type filter and/or resource type filter.

aehrath-amazon commented 3 years ago

So I started with @mobileposse/auto-delete-bucket and modernized it to work with CDK 1.73.0, refactored deprecated methods and refactored the code to eliminate the node-fixture and axios dependencies. I got this working in my fork of that code and also tested it with code i use inside Amazon. Seems to work. I need someone from Amazon to run this by, so we can make sure open source, etc. is properly handled. Feel free to contact me for more info.

schof commented 3 years ago

@aehrath-amazon Does that mean you are going to officially add to CDK at some point? If so, please let me know if you need anything from me to make that happen. Thanks.

aehrath-amazon commented 3 years ago

That is the plan. I will follow up with AWS folks to see where this is at.

aehrath-amazon commented 3 years ago

After diving deep into the CDK guts, I just verified that this does what AutoDeleteBucket is trying to solve (tested with CDK 1.73.0):

bucket = new Bucket(this, 'myBucket', {
  removalPolicy: RemovalPolicy.DESTROY
});
new BucketDeployment(this, 'myInputData', {
  sources: [Source.asset('myDataFolder')],
  destinationBucket: bucket,
  retainOnDelete: false
});

Therefore I am dropping the AutoDeleteBucket work. Use this instead :)

dnlopes commented 3 years ago

@aehrath-amazon the RemovalPolicy.DESTROY only works when the bucket is empty afaik.

I'm also interested in this feature. My use case is that on our team we are constantly spinning up CodePipeline resources via CDK to build up test environments. When tests are done we teardown the pipelines but the artifacts bucket are left behind. We are constantly hitting the maximum number of buckets in the account.

Having a built-in solution for this use case would be good.

aehrath-amazon commented 3 years ago

@dnlopes Have you tried adding the BucketDeployment right after the bucket creation? You should be able to leave sources as an empty list of not specify it at all and it should do the right thing.

iliapolo commented 3 years ago

@aehrath-amazon Thats a nifty workaround :)

We do want to also provide native support for this, there is already an ongoing PR, its a little stalled at the moment so if anyone wants to pick this up that would be great.

aehrath-amazon commented 3 years ago

@iliapolo can we just add the model deployment object if auto delete is desired? Very little code change that way: bucket_constructor . . if(autodelete) { new BucketDeployment(this, 'AutoDeleteBucketDeployment', { sources: [], destinationBucket: this, retainOnDelete: false }); . . done! ?

iliapolo commented 3 years ago

@aehrath-amazon Unfortunately no because this creates a circular dependency between s3 and lambda. The implementation proposed in the PR uses a custom resource that is implemented using raw CloudFormation templates to avoid adding this dependency on lambda.

aehrath-amazon commented 3 years ago

Ah ok, good point. I forked the Posse solution previously and refactored the code to work with CDK 1.73.0 for my own use. I hope this PR though goes through soon. There definitely seems to be a need for a standardized solution.

github-actions[bot] commented 3 years ago

⚠️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.

rajshah001 commented 3 years ago

@AlexCheema

Now, you can delete the bucket as well as the contents of the Bucket which got created with the help of CloudFormation (i.e. cdk deploy)

You just have to add autoDeleteObjects: true parameter while creating the S3 object.

Here is the Sample TypeScript Code:

new s3.Bucket(this, 'MyFirstBucket', {
  versioned: true,
  removalPolicy: cdk.RemovalPolicy.DESTROY,
  autoDeleteObjects: true
});

Here is the reference link from the official AWS Documentation: https://docs.aws.amazon.com/cdk/api/latest/docs/aws-s3-readme.html#bucket-deletion

aws / aws-cdk

Feature Request: Option to clear and delete S3 bucket on delete #3297

⚠️COMMENT VISIBILITY WARNING⚠️