terraform-provider-commercetools icon indicating copy to clipboard operation
terraform-provider-commercetools copied to clipboard

Add state transitions resource

Open RaniSputnik opened this issue 5 years ago • 1 comments

This PR introduces a new resource commercetools_state_transitions and removes the transitions property from commercetools_state. This pull request fixes #86, the limitations with the existing attribute based approach are discussed there.

TODO

  • [x] Add state transitions lifecycle
  • [x] Import state transitions resource
  • [x] Update state resource with mutex
  • [x] Handle deletion race conditions
  • [x] Update documentation
  • [x] Remove commercetools_state resource's transitions attribute
  • [x] Remove redundant StateGetWithID call to CT API on commercetools_state update
  • [x] ~Investigate multiple resource import for commercetools_state~ Will tackle this in a separate PR, this one's on the larger side already.

Possible contention points

Removing the version attribute from state

The commercetools_state resource has a version attribute which I don't think makes sense any more. It's possible for this version attribute to fall out of sync due to changes in a related commercetools_state_transitions resource.

  1. commercetools_state is created, version is stored in TF state as version=1
  2. commercetools_state_transitions are created, updating the commercetools_state and increasing the version in Commerce Tools state to version=2
  3. The versions between CT and TF now no longer match (TF still thinks version=1)

Because of this I have chosen to remove the version attribute, I can't think of any use cases that require it, but it would be good to discuss this change further.

Multiple state transitions resources for a single state

I was wondering whether there was a way to guard against multiple state transitions resources referencing the same state in Commerce Tools. ie. is there any way to return an error to the user in the following scenario:

resource "commercetools_state" "state_a" { /* omitted */ }

resource "commercetools_state_transitions" "state_a" {
	from = commercetools_state.test_state_a.id
	to   = [ /* etc. */ ]
}

resource "commercetools_state_transitions" "state_a_duplicate" {
	from = commercetools_state.test_state_a.id
	to   = [ /* etc. */ ]
}

Seems like this is more of a design problem for Terraform core, as individual resources don't know about other resources of the same type. There is an open issue in the Terraform repo about this topic https://github.com/hashicorp/terraform/issues/18444

RaniSputnik avatar Jun 13 '20 07:06 RaniSputnik

This PR seems to be related to some of the limitations described in this https://github.com/labd/terraform-provider-commercetools/pull/74#issuecomment-514628845 and https://github.com/labd/terraform-provider-commercetools/issues/86.

@davidweterings, any chance you can have look and provide feedback on this solution?

etiennetremel avatar Feb 02 '22 11:02 etiennetremel

I completely forgot that this PR existed when I implemented the new state transition resource. And somehow I implemented it almost exactly like this PR so I must have seen it before :-)

Sorry for not merging your PR and continuing your work instead of reinventing it. And again thanks for the work

mvantellingen avatar Oct 04 '22 12:10 mvantellingen