crossplane / crossplane/upjet

Deleting an MR with a pending RequiresNew change causes either a panic or recreation

Open
#420 0 comments 0 reactions 0 assignees View on GitHub
bug
Dominant language
Go
Stars
481
Forks
131
Avg merge
2d 1h
Merged PRs (30d)
11

Description

### What happened?
First reported in https://github.com/crossplane-contrib/provider-upjet-aws/issues/1360, but I've observed it with other MR kinds as well.

In the [Delete method](https://github.com/crossplane/upjet/blob/v1.4.1/pkg/controller/external_tfpluginsdk.go#L706-L723) for a tf sdk managed resource, upjet sets `n.instanceDiff.Destroy = true`, then invokes

```
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*Resource).Apply(0xc0045795e0,
github.com/hashicorp/terraform-plugin-sdk/v2@v2.33.0/helper/schema/resource.go:909
```
That Apply method is designed to handle both a "delete only" request, and a delete and recreate request. The delete implementation contains this code
```
// If we're only destroying, and not creating, then return
// now since we're done!
if !d.RequiresNew() {
return nil, diags
}
```
which in this case doesn't trigger if the observed instance diff has `RequiresNew = true` due to a change to an immutable-by-terraform field, or due to a perpetual-diff bug that creates such a change, like `https://github.com/crossplane-contrib/provider-upjet-aws/issues/1363`. So then the function proceeds to attempt to recreate the desired state. In some cases, this successfully deletes and then recreates the resource, and then the next run of the reconciliation loop deletes it again. In other cases, such as in the linked issue, it causes a panic due to some unexpected input.

### How can we reproduce it?
1. Apply the [example manifest for MultiRegionAccessPoint.s3control.aws.upbound.io/v1beta1](https://github.com/crossplane-contrib/provider-upjet-aws/blob/main/examples/s3control/v1beta1/multiregionaccesspoint.yaml)
2. Once the resource has been created and is in a loop of being unable to update a parameter which requires recreation, `kubectl delete` the managed resource.
3. The provider pod will delete the external resource from aws, panic, restart, and then successfully delete the managed resource on the second try (because it will see that the resource is gone from aws)

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.