Add force_delete optional parameter to resource_aws_ecr_repository

[yeforriak]

Community Note

• Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
• Please do not leave "+1" comments, they generate extra noise for pull request followers and do not help prioritize the request

Closes #9911

Release note for CHANGELOG:

ENHANCEMENTS:
* resource/resource_aws_ecr_repository supports now force_delete, It can be set to false to avoid accidentally deleting the repository if it still contains images. ([#9911](https://github.com/terraform-providers/terraform-provider-aws/issues/9911))

Output from acceptance testing:
It is hard to test since at least one image needs to be pushed to the ECR repo. I manually tested the changes.

> terraform apply
aws_ecr_repository.repository: Refreshing state... (ID: victor)

An execution plan has been generated and is shown below.
Resource actions are indicated with the following symbols:
  ~ update in-place

Terraform will perform the following actions:

  ~ aws_ecr_repository.repository
      force_delete: "true" => "false"


Plan: 0 to add, 1 to change, 0 to destroy.

Do you want to perform these actions?
  Terraform will perform the actions described above.
  Only 'yes' will be accepted to approve.

  Enter a value: yes

aws_ecr_repository.repository: Modifying... (ID: victor)
  force_delete: "true" => "false"
aws_ecr_repository.repository: Modifications complete after 0s (ID: victor)

Apply complete! Resources: 0 added, 1 changed, 0 destroyed.

> terraform destroy
aws_ecr_repository.repository: Refreshing state... (ID: victor)

An execution plan has been generated and is shown below.
Resource actions are indicated with the following symbols:
  - destroy

Terraform will perform the following actions:

  - aws_ecr_repository.repository


Plan: 0 to add, 0 to change, 1 to destroy.

Do you really want to destroy?
  Terraform will destroy all your managed infrastructure, as shown above.
  There is no undo. Only 'yes' will be accepted to confirm.

  Enter a value: yes

aws_ecr_repository.repository: Destroying... (ID: victor)

Error: Error applying plan:

1 error(s) occurred:

* aws_ecr_repository.repository (destroy): 1 error(s) occurred:

* aws_ecr_repository.repository: error ECR repository not empty, consider using force_delete: RepositoryNotEmptyException: The repository with name 'victor' in registry with id '...' cannot be deleted because it still contains images
	status code: 400, request id: c09a3fa8-e184-4fd8-9242-c7fe41c385a4

Terraform does not automatically rollback in the face of errors.
Instead, your Terraform state file has been partially updated with
any resources that successfully completed. Please address the error
above and apply again to incrementally change your infrastructure.

[wjam]

Is there any chance of getting this change reviewed?
@bflad @appilon

[zhelding]

Pull request #21306 has significantly refactored the AWS Provider codebase. As a result, most PRs opened prior to the refactor now have merge conflicts that must be resolved before proceeding.

Specifically, PR #21306 relocated the code for all AWS resources and data sources from a single aws directory to a large number of separate directories in internal/service, each corresponding to a particular AWS service. This separation of code has also allowed for us to simplify the names of underlying functions -- while still avoiding namespace collisions.

We recognize that many pull requests have been open for some time without yet being addressed by our maintainers. Therefore, we want to make it clear that resolving these conflicts in no way affects the prioritization of a particular pull request. Once a pull request has been prioritized for review, the necessary changes will be made by a maintainer -- either directly or in collaboration with the pull request author.

For a more complete description of this refactor, including examples of how old filepaths and function names correspond to their new counterparts: please refer to issue #20000.

For a quick guide on how to amend your pull request to resolve the merge conflicts resulting from this refactor and bring it in line with our new code patterns: please refer to our Service Package Refactor Pull Request Guide.

[gdavison]

Thanks for the PR, @yeforriak. I've rebased this onto the current provider code.

--- PASS: TestAccECRRepository_basic (52.46s)
--- PASS: TestAccECRRepository_immutability (52.78s)
--- PASS: TestAccECRRepository_tags (70.18s)
--- PASS: TestAccECRRepository_Encryption_kms (87.12s)
--- PASS: TestAccECRRepository_Encryption_aes256 (89.73s)
--- PASS: TestAccECRRepository_Image_scanning (99.69s)

[github-actions]

This functionality has been released in v4.22.0 of the Terraform AWS Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

[pete-fl0]

Hi @gdavison,

This was kind of a breaking change in my use case, as I was depending on the previous default behaviour.

The new default for force_delete is false, whereas before it was true. This has resulted in deletion failures for me, and is not what #9911 asked for:

This parameter should default to true for backwards compatibility.

I now have to update (many) configurations adding the new argument, run an apply, then rerun the destroy.
Not the end of the world by any means, but I've now lost a bit of faith in the auto-install latest minor version of the provider.

Appreciate your feedback on why the original intent wasn't followed, how this was missed in tests, and if you plan on changing the default value back to true.

[github-actions]

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.