[Bug]: atRestEncryptionEnabled type changed to string from boolean

[ShamilMagamadov]

Is there an existing issue for this?

• I have searched the existing issues

Affected Resource(s)

• elasticache.aws.upbound.io/v1beta2 - ReplicationGroup

Resource MRs required to reproduce the bug

No response

Steps to Reproduce

Upgrading existing upbound-provider-aws-elasticache from v.1.18.3 to v.1.19.0

What happened?

RedisCluster is False due to in the new version v.1.19.0, the type of atRestEncryptionEnabled is changed from boolean to string. It is expected to be handled as a boolean.

Relevant Error Output Snippet

Crossplane Version

v1.16.0

Provider Version

1.19.0

Kubernetes Version

Client Version: v1.31.0 Kustomize Version: v5.4.2

Kubernetes Distribution

EKS

Additional Info

No response

[chlunde]

Related change: hashicorp/terraform-provider-aws#40514

[alexinthesky]

Is testing not supposed to catch this 🤔

[jeanduplessis]

@alexinthesky The relevant Terraform provider update took place in this PR.

The resource was tested with Uptest, however, this field is not present in the resource’s example manifest, so it was not validated during the testing.

It is often impossible to test for all configuration fields in example manifests, and we generally rely on testing the basic configuration. Once an issue like this appears, we add additional example manifests/or update the existing one for testing to ensure it is caught going forward. You'll understand that having exhaustive example manifests for almost 1000 resources and all their configuration permutations in this provider is a very big task.

Another challenge is that the TF provider introduced a breaking API change in a minor release with no indication of a breaking API in their release notes.

We'll evaluate how to better detect these potential problems and communicate them clearly to end users in our release notes.

[chlunde]

@jeanduplessis release notes, and using the crddiff output for the provider release notes, would be nice!

@jeanduplessis but I also wonder if you considered, or if we should consider, overriding the type in cases like this?

[pierluigilenoci]

If the terraform-provider-aws introduces a breaking change, updating the provider version cannot be treated as a minor change because it also impacts the Crossplane provider. It is also crucial to communicate strategies for resolving, migrating, or mitigating the issue.

[jeanduplessis]

@pierluigilenoci, you are right, and in an ideal world, it would work like that. That said, if we had to do a major version change every time the Terraform provider made a breaking change, we would have been at version 6 of this provider already. Unfortunately, this happens way more frequently than one would expect. We have made big efforts to hide these sorts of breaking changes in the past and will continue to try to do so. It slipped through in this instance and wasn't intentional - we're sorry for the impact it had on you.

We have updated the release notes to call this out for the v1.19 release and will take care to ensure this doesn't happen again.

[pierluigilenoci]

@jeanduplessis If the provider's versioning adheres to a semantic versioning protocol, I don't see any issues with changing the major version whenever necessary.

It would also be helpful if, in addition to the release notes, instructions on how to recover from any issues were provided. This should be in addition to the option of removing resources created with previous versions of the provider.