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-cdk-lib/aws-ecr: No option available in CDK to set EmptyOnDelete property for ECR repo #28196

Closed
carlosahs opened this issue Nov 30, 2023 · 5 comments · Fixed by #28233
Closed

aws-cdk-lib/aws-ecr: No option available in CDK to set EmptyOnDelete property for ECR repo #28196

carlosahs opened this issue Nov 30, 2023 · 5 comments · Fixed by #28233
Labels
@aws-cdk/aws-ecr Related to Amazon Elastic Container Registry effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p2

Comments

@carlosahs
Copy link

Describe the issue

AWS CloudFormation documentation states that AWS::ECR::Repository resources have EmptyOnDelete property, but it's not possible to set such property in CDK using Repository construct.

Is expected EmptyOnDelete property not to be set in CDK?

Links
@carlosahs carlosahs added documentation This is a problem with documentation. needs-triage This issue or PR still needs to be triaged. labels Nov 30, 2023
@github-actions github-actions bot added the @aws-cdk/aws-ecr Related to Amazon Elastic Container Registry label Nov 30, 2023
@markmansur
Copy link
Contributor

markmansur commented Nov 30, 2023 •
edited

You're right it is supported by CloudFormation. The CfnRepository L1 construct supports emptyOnDelete as optional. See here: https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_ecr.CfnRepository.html#emptyondelete

Should be a fairly straight forward change here: https://github.com/aws/aws-cdk/blob/116b93324f5c2d961ed222c659b1b35ec71c57c7/packages/aws-cdk-lib/aws-ecr/lib/repository.ts#L701C16-L701C16

I can work on a PR

@markmansur
Copy link
Contributor

@carlosahs it seems CDK has implemented a custom resource that can empty the repository on deletion. You can enable it with the autoDeleteImages prop. Does this work for you?

I'm not sure why we have this if CloudFormation supports it, I'll have to dig deeper

@github-actions github-actions bot mentioned this issue Dec 1, 2023
Closed
@khushail khushail added the investigating This issue is being investigated and/or work is in progress to resolve the issue. label Dec 1, 2023
@khushail
Copy link
Contributor

khushail commented Dec 1, 2023

@carlosahs ,thanks for submitting this request.
@markmansur , as its supported by Cloudformation and L1 construct, it should be implementable. Contributions from community are welcome.

@khushail khushail added p2 effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. and removed needs-triage This issue or PR still needs to be triaged. investigating This issue is being investigated and/or work is in progress to resolve the issue. documentation This is a problem with documentation. labels Dec 1, 2023
@markmansur
Copy link
Contributor

markmansur commented Dec 1, 2023 •
edited

Thanks @khushail

What do you recommend we do with the autoDeleteImages custom resource? It doesn't make sense for both to co-exist IMO.

I can go ahead and start on the PR to add the prop 🙂

@markmansur markmansur mentioned this issue Dec 2, 2023
Merged
@mergify mergify bot closed this as completed in #28233 Dec 19, 2023
mergify bot pushed a commit 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*
@github-actions 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
@aws-cdk/aws-ecr Related to Amazon Elastic Container Registry effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p2
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat(ecr): Add emptyOnDelete CloudFormation property to Repository L2 construct markmansur/aws-cdk
4 participants
@markmansur @carlosahs @khushail and others