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.

Sign up for GitHub

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

Jump to bottom

AWS::ECR::Repository - Request to support Force Removal of Non-empty ECR repositories #515

Closed
heiba opened this issue Jun 7, 2020 · 34 comments
Closed

AWS::ECR::Repository - Request to support Force Removal of Non-empty ECR repositories #515

heiba opened this issue Jun 7, 2020 · 34 comments
Labels
compute EC2, ECR, ECS, EKS, Lambda, Batch, Elastic Beanstalk, Serverless Application Repository

Comments

@heiba
Copy link

heiba commented Jun 7, 2020 •
edited
Loading

  1. Title -> AWS::ECR::Repository - Request to support Force Removal of Non-empty ECR repositories

  2. Scope of request -> AWS::ECR::Repository supports deleting an empty ECR repository but doesn't support force deleting a non-empty ECR repository.

  3. Expected behavior -> In Delete, it should allow an option to force delete a non-empty ECR repository and delete all Images in the ECR repository.

  4. Test case recommendation (optional) -> it should help me delete an ECR repository given a force flag is supplied; it shouldn't delete a non-empty repository if the force flag is not provided.

  5. Links to existing API doc (optional) -> https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-ecr-repository.html

  6. Category tag (optional) -> Containers

  7. Any additional context (optional)

This is needed for CDK to support a removal policy which can force delete a non-empty ECR repository.
aws/aws-cdk#2765
https://docs.aws.amazon.com/cdk/api/latest/docs/aws-ecr-readme.html
https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_core.RemovalPolicy.html
@heiba heiba mentioned this issue Jun 7, 2020
Closed
@luiseduardocolon luiseduardocolon added the compute EC2, ECR, ECS, EKS, Lambda, Batch, Elastic Beanstalk, Serverless Application Repository label Jun 10, 2020
@rsahs
Copy link

rsahs commented Oct 12, 2020

any further development with this feature request??

@Bermpje
Copy link

Bermpje commented Oct 27, 2020

I would also like to know if there is any further development.

@rsahs
Copy link

rsahs commented Jan 6, 2021

Is this being looked at by AWS yet?

@kirintwn kirintwn mentioned this issue Jan 20, 2021
Closed
2 tasks
@fimbulvetr
Copy link

Really looking forward to this. Not many CF resources error out like this one. For instance you can delete a secret from cloudformation, and you can delete a codebuild report group (#390)

@heiba
Copy link
Author

heiba commented Apr 2, 2021

@luiseduardocolon Any chance we can have someone look at this request please ?

@bradg-aws
Copy link

Thank you for opening this. We now have this issue under consideration. We will get back to you once we have more updates.

@heiba
Copy link
Author

heiba commented May 30, 2021 •
edited
Loading

Thanks @bradg-aws, any chance you can update us on this please ? CDK team are blocked on this.

@rsahs
Copy link

rsahs commented Jun 28, 2021

@bradgr anything further regarding updates on this??

@heiba
Copy link
Author

heiba commented Aug 6, 2021 •
edited
Loading

@bradg-aws Hello Brad, thank you for taking this request into consideration. Could you possibly let us know whether it's in progress or has it been postponed ?

@kirintwn kirintwn mentioned this issue Aug 7, 2021
Closed
@alexs20
Copy link

alexs20 commented Feb 17, 2022

Hi! Any updates?

@heiba
Copy link
Author

heiba commented Feb 23, 2022

@bradg-aws Any updates or inputs you can provide us regarding CloudFormation force removal of non-empty ECR repos ?

@bradg-aws
Copy link

Hi,
We still have this under consideration, but I don't have an ETA for you at this time. I'll update here when I can.
Thanks

@rittneje
Copy link

@bradg-aws Did this feature actually get released? I don't see anything mentioned in the docs. Is there anything special that needs to be done to tell CloudFormation to force delete? Or will it always force delete now? https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-ecr-repository.html

@prerna-p prerna-p reopened this Mar 28, 2022
@bradg-aws
Copy link

This feature has not been released. Issue has been reopened.

@bharathiaws
Copy link

@bradg-aws I am interested in this feature too, Do we have any ETA for this feature ? Thanks in Advance

@akshay-kas
Copy link

Can we please have this feature and an ETA ?
Thanks !

@Conaire
Copy link

Conaire commented Mar 6, 2023

ETA please :)

@tapestryfrank
Copy link

Just stopping by for another monthly request for the ETA of this feature release. Any updates?

@ryan-wendel
Copy link

How is there not a removal policy for this? wow.

@paololazzari
Copy link

If you are using CloudFormation, you may use a lambda-backed custom resource to empty the ECR repository before deletion. You can find an example here: https://stackoverflow.com/a/76184754/3390419

@AlHanouf-BH
Copy link

can you let us know when this feature will be released?

@rittneje
Copy link

@bradg-aws Did this feature actually get released?

@alexs20
Copy link

alexs20 commented Jun 21, 2023

@rittneje Do you see a commit linked to this thread? I do not see. That means they just closed it to do not see it.

@paololazzari
Copy link

This bot is malfunctioning

@AkifRafique AkifRafique reopened this Jun 21, 2023
@jlbutler
Copy link

jlbutler commented Jul 3, 2023

Looks to me that Paolo is absolutely correct - the bot has been doing some weird things here.

@CloudChoom do you have any ideas on what might be happening with this bot? Thanks in advance!

To customers, apologies that these status updates flipping back and forth have been confusing. To clarify, this hasn't shipped, nor is it coming soon. We have discussed it previously, so still 'under consideration' at this time. We may consider closing it, but wanted to clarify here before that.

From ECR's perspective, we have leaned toward being conservative and not enabling automated deletion of repositories with content. This follows deletion practice for S3 buckets and other similar resources.

We are open to hearing more. We understand that not all customers use CDK and it's not ideal to run a Lambda to empty a repo just so you can delete it. We don't have an analog to aws s3 rm --recursive, that might be something we'd be able to consider if that would meet the need.

@rittneje
Copy link

rittneje commented Jul 4, 2023

@jlbutler Even in CDK, having to empty the ECR repo before deleting it really sucks. Writing custom resources for CloudFormation properly is very hard, and even the CDK team gets it wrong. In addition, due to a design flaw in CloudFormation, during stack deletion if the CR lambda function fails to empty the ECR repo, CloudFormation will proceed with deleting its IAM role anyway, which means that subsequent stack deletion attempts will always fail and manual intervention is required. (We see this happen a lot with our CR for purging S3 buckets.)

From ECR's perspective, we have leaned toward being conservative and not enabling automated deletion of repositories with content. This follows deletion practice for S3 buckets and other similar resources.

To be frank, this comes across like the ECR team thinks they know better than the customers trying to use their product. Clearly nobody asking for this feature is concerned about the content loss. Refusing to add the ability via CloudFormation does not remove the need, it just forces everyone to implement their own (potentially buggy/dangerous) workaround.

We don't have an analog to aws s3 rm --recursive, that might be something we'd be able to consider if that would meet the need.

That would not meet the need. We'd all still need to write a custom resource to go make the call. Also, I'm fairly certain aws s3 rm --recursive is a CLI-only feature; with the SDKs you have to go read all the objects out and delete them in a loop.

@alexs20
Copy link

alexs20 commented Jul 4, 2023

From ECR's perspective, we have leaned toward being conservative and not enabling automated deletion of repositories with content. This follows deletion practice for S3 buckets and other similar resources.

Nobody asks to break previous rules and do "automated deletion" as a new, default behavior
Just add one extra mode to DeletionPolicy, something like ForceNonEmpty and make AWS::ECR::Repository to honor it.
No new flag - no new behavior.

I really do not see what the problem here except for the lack of desire to implement it

@jlbutler
Copy link

jlbutler commented Jul 5, 2023

Thanks for the additional input @alexs20 and @rittneje.

Not defending, but I do want to clarify a few things. This isn't a lack of desire to implement something; every time it comes up in discussion, we are stuck on whether we should enable this mode, and conservative leanings win out. That said... with the issue being 3 years old, we could have made that much more clear and driven resolution sooner. That's on us.

Also, I definitely didn't mean to imply we know better than our customers - that's why we are open to hearing more. I was just trying (poorly) to explain our thinking so far on this. No one has denied anything - which is why we have a 3 year old issue still open. We are just trying to do the right thing, while prioritizing the biggest issues. Again, thanks for the additional input here, it helps a ton.

We'll see what we can do, and will update back here when we have either concrete reasons for not doing this, or a plan for doing it. Either way, we'll get it resolved (soon) so it doesn't just linger on.

@benbridts
Copy link

@jlbutler An approach that might balance both concerns is adding a EmptyOnRemoval (cloudformation-only) property.
If this defaults to False, and needs to be set explicitly (and in a separate deploy) it:

  • will make it much harder to have accidental deletion
  • won't need changes on how DeletionPolicy works in CloudFormation (although this might be something to signal to the CloudFormation team to work on)
  • Allow customers to use parameters to flip this depending on environment, if that's desired.

I generally think that Resources should "just delete", because you don't want customers to think that all resources will prevent deletion if they are not empty (most won't), so they adopt approaches that will protect all stateful resources, not just implementation-specific things like S3 Buckets.

At least with the current CloudFormation features. I would welcome a stack-level "protect stateful resources" flag

@jlbutler
Copy link

@benbridts thanks for the recommendation! We had had an internal chat earlier and landed at a similar idea, but your property name is better :)

We'll get started on this, no firm date due to review windows, testing, etc. Will keep you updated here, thanks again all.

@rittneje
Copy link

@jlbutler In case it helps, for prior art you can look at PendingWindowInDays on AWS::KMS::Key, which in a similar vein of "properties that only matter during deletion/replacement."

@bradg-aws
Copy link

ECR Repository resource now includes the EmptyOnDelete property to support force delete of repositories.
If true, deleting the repository force deletes the contents of the repository. If false, the repository must be empty before attempting to delete it.

Please see public docs: https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-ecr-repository.html

@Conaire
Copy link

Conaire commented Aug 22, 2023

Amazing, thanks!

@WaelA WaelA closed this as completed Aug 22, 2023
@rittneje rittneje mentioned this issue Aug 22, 2023
Open
@heiba
Copy link
Author

heiba commented Aug 23, 2023 •
edited
Loading

Thank you so much @bradg-aws for announcing this amazing news!

@markmansur markmansur mentioned this issue Dec 2, 2023
Merged
mergify bot pushed a commit to aws/aws-cdk that referenced this issue Dec 19, 2023
@markmansur
feat(ecr): Add emptyOnDelete CloudFormation property to Repository L2…
a175da8 
… construct (#28233)

Added `emptyOnDelete` prop to the ecr `Repository` construct. `emptyOndelete` is supported by CloudFormation 
See here: https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-ecr-repository.html#cfn-ecr-repository-emptyondelete


I've also deprecated the `autoDeleteImages` prop that deployed a custom resource. According to #24572 this was added before CloudFormation added the `EmptyOnDelete` property here aws-cloudformation/cloudformation-coverage-roadmap#515

Closes #28196 

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@corymhall corymhall mentioned this issue May 17, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
compute EC2, ECR, ECS, EKS, Lambda, Batch, Elastic Beanstalk, Serverless Application Repository
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests