search

labd / terraform-provider-commercetools

Terraform provider for commercetools
https://registry.terraform.io/providers/labd/commercetools/latest/docs
Mozilla Public License 2.0
64 stars 68 forks source link

New TaxRates are being created by ignoring existing TaxRates #462

Open sebin-vincent-ayata opened 7 months ago

sebin-vincent-ayata commented 7 months ago

It came to our attention that when any updates are made on tax rates within a tax category manually, CommerceTools (CT) changes the ID of the Tax Rate itself. Consequently, when Terraform attempts to locate a tax category rate using the saved ID in it's statefile, discrepancies arise due to the manual update of taxRates. As a result, Terraform attempts to create a new tax category rate because as far as terraform is concerned the existing tax category rate got deleted somehow. You can view a demonstration of this scenario in the following video [link].

I checked this with commercetools support to clarify if this is an expected behaviour from commercetools or is it a bug. According to them, this is an expected behaviour because this is actually a replace action rather than update action so it is expected to have a change in the Id. image

Their recommendation is to use Tax Rate Key instead of Tax Rate Id, as the earlier remains same during a Tax Rate update. image

demeyerthom commented 7 months ago

Hi @sebin-vincent-ayata thanks for reaching out!

Hmm, thanks for looking into that. That is indeed not the behaviour we would want here. However I am also loathe to switch to using the key as identifier here because this would conflict with how we treat all the other resources. This might introduce bugs down the line if we ever refer to tax rates by id elsewhere.

I will do some testing and see If I can come up with a different solution

demeyerthom commented 5 months ago

@sebin-vincent-ayata I did some research into this, and I came to the conclusion that using the key is probably not the best way of doing this. The key is optional in commercetools, so we run the risk that this gets removed manually, which is equally a bug-risk. I also don't want to fill in the key for the user with something the provider can understand, as developers might use the key to have a stable reference to use in their code (which is the intention for this field).

So my alternative solution is to implement https://github.com/labd/terraform-provider-commercetools/issues/291, and see if I can ensure a smooth migration between the old system and the new. This keeps the data model closer to the commercetools API, and we can do the required checks internally in the resource