resource/aws_sfn_state_machine: Support Update State machine call

[Puneeth-n]

Resolves #2341

requires #2344 to be merged first

[Ninir]

Hi @Puneeth-n

As asked, just left a few comments to address before getting this merged in! :)

Thanks for the work! 🙂 🚀

[Ninir]

Can you move this one before 137 and log the overall params structure? as in:

log.Printf("[DEBUG] Updating Step Function State Machine: %#v", params)

[Ninir]

Can you change the 2 above line for something like:

if isAWSErr(err, "StateMachineDoesNotExist", "something") {
...
}

In place of something, you should write something that is sent back by the AWS API. This parameter is used by the function to check whether err.Message() contains the string you are passing.

[Ninir]

In place of these 2 lines, it would be preferable to return an error, as in:

return fmt.Errorf("Error updating Step Function State Machine: %s", err)

Setting an ID to empty removes it from the state, step done by the Read part, always happening before updating/deleting

[Puneeth-n]

@Ninir done :)

[Ninir]

Could you add an acceptance test with 2 steps (one for creation, the other one for update)? thanks :)

[Puneeth-n]

@Ninir I am not sure how I can check the sfn definition changes in the acceptance tests.

root@5f4b89633452:/go/src/github.com/terraform-providers/terraform-provider-aws# make testacc TESTARGS='-run=TestAccAWSSfnStateMachine_createUpdate'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test $(go list ./... |grep -v 'vendor') -v -run=TestAccAWSSfnStateMachine_createUpdate -timeout 120m
?   	github.com/terraform-providers/terraform-provider-aws	[no test files]
=== RUN   TestAccAWSSfnStateMachine_createUpdate
--- PASS: TestAccAWSSfnStateMachine_createUpdate (84.16s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	84.182s

[Ninir]

@Puneeth-n We could store the output and use it afterwards, as in https://github.com/terraform-providers/terraform-provider-aws/blob/master/aws/resource_aws_flow_log_test.go#L15-L29

We could then check if the ID changed, and hash the structure to ensure that the content changed too. Thoughts?

[Puneeth-n]

@Ninir I used regex to check the output. after the state machine was updated. what do you think about it?

[Ninir]

@Puneeth-n Depending on the regex written, this could be sufficient, nice idea :)

[Puneeth-n]

@Ninir I have already included it in the Acceptance tests. Let me know if you want any changes.

[Ninir]

Hey @Puneeth-n

This looks very good to me :)

make testacc TEST=./aws TESTARGS='-run=TestAccAWSSfnStateMachine_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSSfnStateMachine_ -timeout 120m
=== RUN   TestAccAWSSfnStateMachine_createUpdate
--- PASS: TestAccAWSSfnStateMachine_createUpdate (53.01s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	53.054s

Thanks a lot for the work and reactivity! 🚀 😄

[Puneeth-n]

@Ninir Thanks a lot :) Wanted to get this feature into mainline so that we can get rid of our fork. :)

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!