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

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

Closed
1 of 5 tasks
AlexCheema opened this issue Jul 12, 2019 · 33 comments · Fixed by #12090
Closed
1 of 5 tasks

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

AlexCheema opened this issue Jul 12, 2019 · 33 comments · Fixed by #12090
Assignees
Labels
@aws-cdk/aws-s3 Related to Amazon S3 effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. in-progress This issue is being actively worked on. p2

Comments

@AlexCheema
Copy link
Contributor

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

    • 🪲 bug report
    • 🚀 feature request
    • 📚 construct library gap
    • ☎️ security issue or vulnerability => Please see policy
    • ❓ support request => Please see note at the top of this template.
  • What is the current behavior?
    If the current behavior is a 🪲bug🪲: 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.

@AlexCheema AlexCheema added the needs-triage This issue or PR still needs to be triaged. label Jul 12, 2019
@nmussy
Copy link
Contributor

nmussy commented Jul 12, 2019

Hey @AlexCheema,

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

readonly removalPolicy?: RemovalPolicy;

@Obirah
Copy link

Obirah commented Jul 12, 2019

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
Copy link
Contributor

nmussy commented Jul 12, 2019

Apologies. There's a comment related to this issue: #2526 (comment)

@AlexCheema
Copy link
Contributor Author

@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 NGL321 added package/cfn Related to the CFN layer (L1) feature-request A feature should be added or improved. no-cfn-supp @aws-cdk/aws-s3 Related to Amazon S3 and removed needs-triage This issue or PR still needs to be triaged. labels Jul 15, 2019
@NGL321
Copy link
Contributor

NGL321 commented Jul 15, 2019

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
Copy link
Contributor

schof commented Jul 26, 2019

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

@AlexCheema
Copy link
Contributor Author

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
Copy link
Contributor

eladb commented Aug 7, 2019

@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
Copy link
Contributor

schof commented Aug 7, 2019

@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.

@eladb eladb assigned eladb and unassigned eladb Aug 12, 2019
@pmarques
Copy link

pmarques commented Aug 19, 2019

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
Copy link
Contributor

schof commented Aug 20, 2019 via email

@NGL321 NGL321 added needs-cfn This issue is waiting on changes to CloudFormation before it can be addressed. and removed no-cfn-supp labels Aug 23, 2019
@eladb eladb removed package/cfn Related to the CFN layer (L1) needs-cfn This issue is waiting on changes to CloudFormation before it can be addressed. labels Aug 28, 2019
@eladb eladb assigned iliapolo and unassigned eladb Jan 23, 2020
@AlexCheema
Copy link
Contributor Author

👍

@Obirah
Copy link

Obirah commented May 21, 2020

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
Copy link

heiba commented Jun 1, 2020

@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 #2765. The difference is that ECR does have a force delete option in the CLI but not in CloudFormation.

@schof
Copy link
Contributor

schof commented Jun 1, 2020

@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!

@SomayaB SomayaB added the in-progress This issue is being actively worked on. label Aug 17, 2020
@iliapolo iliapolo added the p2 label Aug 29, 2020
@srinivasreddych
Copy link

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
Copy link
Contributor

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
Copy link

@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
Copy link

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
Copy link
Contributor

schof commented Dec 6, 2020

@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
Copy link

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

@aehrath-amazon
Copy link

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
Copy link

dnlopes commented Dec 9, 2020

@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
Copy link

@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
Copy link
Contributor

iliapolo commented Dec 9, 2020

@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
Copy link

@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
Copy link
Contributor

iliapolo commented Dec 9, 2020

@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
Copy link

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.

jogold added a commit to jogold/aws-cdk that referenced this issue Dec 15, 2020
Use the custom resource provider from core to delete objects in the
bucket. A bucket policy gives the correct permissions to the provider's
Lambda function role.

Credits to @Chriscbr for starting the work on this.

Closes aws#3297
Closes aws#9751
jogold added a commit to jogold/aws-cdk that referenced this issue Dec 15, 2020
Use the custom resource provider from core to delete objects in the
bucket. A bucket policy gives the correct permissions to the provider's
Lambda function role.

Credits to @Chriscbr for starting the work on this.

Closes aws#3297
Closes aws#9751
jogold added a commit to jogold/aws-cdk that referenced this issue Dec 15, 2020
Use the custom resource provider from core to delete objects in the
bucket. A bucket policy gives the correct permissions to the provider's
Lambda function role.

Credits to @Chriscbr for starting the work on this.

Closes aws#3297
Closes aws#9751
@mergify mergify bot closed this as completed in #12090 Dec 26, 2020
mergify bot pushed a commit that referenced this issue Dec 26, 2020
Use the custom resource provider from core to delete objects in the
bucket. A bucket policy gives the correct permissions to the provider's
Lambda function role.

Credits to @Chriscbr for starting the work on this.

Closes #3297
Closes #9751


----

*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.

flochaz pushed a commit to flochaz/aws-cdk that referenced this issue Jan 5, 2021
Use the custom resource provider from core to delete objects in the
bucket. A bucket policy gives the correct permissions to the provider's
Lambda function role.

Credits to @Chriscbr for starting the work on this.

Closes aws#3297
Closes aws#9751


----

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

@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

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/medium Medium work item – several days of effort feature-request A feature should be added or improved. in-progress This issue is being actively worked on. p2
Projects
None yet