Aurora DBInstance .spec.forProvider field is modified and makes the CR invalid

[drewwells]

Crossplane is setting spec.forProvider.{masterUsername,kmsKeyID,port} on a spec that is disallowed when those fields are set.

What happened?

Crossplane attempts to recreate a deleted aurora instance and fails because the forProvider block was modified to an invalid state. kmsKeyID, masterusername and port were populated.

For example, these errors are seen:

The requested DB Instance will be a member of a DB Cluster. Set master user name for the DB Cluster.

create failed: cannot create DBInstance in AWS: InvalidParameterCombination:
      The requested DB Instance will be a member of a DB Cluster. Set database endpoint
      port number for the DB Cluster.\n\tstatus code: 400

How can we reproduce it?

Provision an aurora dbcluster and dbinstance
Delete the dbinstance from AWS console
Trigger crossplane to reconcile the dbinstance which is missing an aws resource now
Crossplane reports confusing errors that the dbcluster is missing fields port, masterUsername, and kmsKeyID. It's actually because the dbinstance now includes those fields.

What environment did it happen in?

provider-aws version: 0.46.0
crossplane: 1.17.0

[MisterMX]

Crossplane does not support external deletion of resources. The deletion process should always be triggered from within Crossplane.

If there is a way to prevent this error from happening without breaking existing implementations feel free to open a pull request.

[drewwells]

Do you know why the provider is modifying the spec fields? Its changing fields to make the spec configuration invalid Drew On October 22, 2024, Anthony Sottile ***@***.***> wrote: Crossplane does not support external deletion of resources. The deletion process should always be triggered from within Crossplane. If there is a way to prevent this error from happening without breaking existing implementations feel free to open a pull request. — Reply to this email directly, view it on GitHub <#2106 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAB2ME2R34JQOW35D4NXV7TZ4ZFUVAVCNFSM6AAAAABP73RLCKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMRZGI2TMNRQG4> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[MisterMX]

The controller intentionally late-initializes all unset, optional properties with the values retrieved from AWS. I haven't looked into the code specifically but I think you issue is caused by the external deletion of the DBInstance which the controller does not seem to support.

[drewwells]

Those properties can't be set, they have to be left blank. Repo steps are pretty easy.

Create an aurora dbinstance CR
Save the modified spec properties, rename it (metadata.name)
Create it again
You will need to remove the 3 properties specified in the description to create a new dbinstance object

[MisterMX]

Now I'm not sure anymore if I understand the problem correctly. If you think there is an issue with how the controller builds and sends the creation / update request to AWS feel free to open a PR with a fix.

[drewwells]

Okay, is there a dummy's guide to get started on developing for provider-aws? The issue is spec fields get populated as the provider reconciles the object. This is a pretty universal problem with this provider though. Controller should stick to updating the status field (in my opinion). Especially in cases like this one, when the object becomes invalid by updates from the controller.

…

-Drew On October 31, 2024, Anthony Sottile ***@***.***> wrote: Now I'm not sure anymore if I understand the problem correctly. If you think there is an issue with how the controller builds and sends the creation / update request to AWS feel free to open a PR with a fix. — Reply to this email directly, view it on GitHub <#2106 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAB2ME4TJQXGEVZJIJNEEF3Z6IK6PAVCNFSM6AAAAABP73RLCKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDINBZGY3DSOBRGU> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[MisterMX]

Every controller populates empty, optional fields with the state of the resource it receives from the external API (here: AWS). That is common across most Crossplane provider and we won't change that.

However, each controller can decide individually which field should be set in the request it sends to AWS. Given the case above: If a single-instance specific field should not be part of the response, we can remove it from the creation or update request.

[drewwells]

That's what I was hoping to hear. So the provider should zero out those 3 fields before sending a create/update request. I wasn't sure what the expected operation here was. On November 4, 2024, Anthony Sottile ***@***.***> wrote: Every controller populates empty, optional fields with the state of the resource it receives from the external API (here: AWS). That is common across most Crossplane provider and we won't change that. However, each controller can decide individually which field should be set in the request it sends to AWS. Given the case above: If a single-instance specific field should not be part of the response, we can remove it from the creation or update request. — Reply to this email directly, view it on GitHub <#2106 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAB2ME7W3KJSBLEJBAGQEQ3Z66QKZAVCNFSM6AAAAABP73RLCKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDINJVGIYDOMZUGM> . You are receiving this because you authored the thread.Message ID: ***@***.***>