diff --git a/aws/internal/service/sfn/waiter/status.go b/aws/internal/service/sfn/waiter/status.go new file mode 100644 index 00000000000..3d2f510e478 --- /dev/null +++ b/aws/internal/service/sfn/waiter/status.go @@ -0,0 +1,28 @@ +package waiter + +import ( + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/sfn" + "github.com/hashicorp/terraform-plugin-sdk/helper/resource" +) + +// StateMachineStatus fetches the Operation and its Status +func StateMachineStatus(conn *sfn.SFN, stateMachineArn string) resource.StateRefreshFunc { + return func() (interface{}, string, error) { + input := &sfn.DescribeStateMachineInput{ + StateMachineArn: aws.String(stateMachineArn), + } + + output, err := conn.DescribeStateMachine(input) + + if err != nil { + return nil, "", err + } + + if output == nil { + return nil, "", nil + } + + return output, aws.StringValue(output.Status), nil + } +} diff --git a/aws/internal/service/sfn/waiter/waiter.go b/aws/internal/service/sfn/waiter/waiter.go new file mode 100644 index 00000000000..f50f4d981e8 --- /dev/null +++ b/aws/internal/service/sfn/waiter/waiter.go @@ -0,0 +1,31 @@ +package waiter + +import ( + "time" + + "github.com/aws/aws-sdk-go/service/sfn" + "github.com/hashicorp/terraform-plugin-sdk/helper/resource" +) + +const ( + // Maximum amount of time to wait for an Operation to return Success + StateMachineDeleteTimeout = 5 * time.Minute +) + +// StateMachineDeleted waits for an Operation to return Success +func StateMachineDeleted(conn *sfn.SFN, stateMachineArn string) (*sfn.DescribeStateMachineOutput, error) { + stateConf := &resource.StateChangeConf{ + Pending: []string{sfn.StateMachineStatusActive, sfn.StateMachineStatusDeleting}, + Target: []string{}, + Refresh: StateMachineStatus(conn, stateMachineArn), + Timeout: StateMachineDeleteTimeout, + } + + outputRaw, err := stateConf.WaitForState() + + if output, ok := outputRaw.(*sfn.DescribeStateMachineOutput); ok { + return output, err + } + + return nil, err +} diff --git a/aws/resource_aws_sfn_state_machine.go b/aws/resource_aws_sfn_state_machine.go index 71f9e9fd109..af8f8d0e6ad 100644 --- a/aws/resource_aws_sfn_state_machine.go +++ b/aws/resource_aws_sfn_state_machine.go @@ -6,12 +6,12 @@ import ( "time" "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/aws/awserr" "github.com/aws/aws-sdk-go/service/sfn" "github.com/hashicorp/terraform-plugin-sdk/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/helper/validation" "github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/sfn/waiter" ) func resourceAwsSfnStateMachine() *schema.Resource { @@ -54,6 +54,10 @@ func resourceAwsSfnStateMachine() *schema.Resource { Computed: true, }, "tags": tagsSchema(), + "arn": { + Type: schema.TypeString, + Computed: true, + }, }, } } @@ -69,17 +73,21 @@ func resourceAwsSfnStateMachineCreate(d *schema.ResourceData, meta interface{}) Tags: keyvaluetags.New(d.Get("tags").(map[string]interface{})).IgnoreAws().SfnTags(), } - var activity *sfn.CreateStateMachineOutput + var stateMachine *sfn.CreateStateMachineOutput err := resource.Retry(5*time.Minute, func() *resource.RetryError { var err error - activity, err = conn.CreateStateMachine(params) + stateMachine, err = conn.CreateStateMachine(params) if err != nil { // Note: the instance may be in a deleting mode, hence the retry // when creating the step function. This can happen when we are // updating the resource (since there is no update API call). - if awsErr, ok := err.(awserr.Error); ok && awsErr.Code() == "StateMachineDeleting" { + if isAWSErr(err, sfn.ErrCodeStateMachineDeleting, "") { + return resource.RetryableError(err) + } + //This is done to deal with IAM eventual consistency + if isAWSErr(err, "AccessDeniedException", "") { return resource.RetryableError(err) } @@ -89,14 +97,14 @@ func resourceAwsSfnStateMachineCreate(d *schema.ResourceData, meta interface{}) return nil }) if isResourceTimeoutError(err) { - activity, err = conn.CreateStateMachine(params) + stateMachine, err = conn.CreateStateMachine(params) } if err != nil { return fmt.Errorf("Error creating Step Function State Machine: %s", err) } - d.SetId(*activity.StateMachineArn) + d.SetId(aws.StringValue(stateMachine.StateMachineArn)) return resourceAwsSfnStateMachineRead(d, meta) } @@ -112,15 +120,15 @@ func resourceAwsSfnStateMachineRead(d *schema.ResourceData, meta interface{}) er }) if err != nil { - if awserr, ok := err.(awserr.Error); ok { - if awserr.Code() == "NotFoundException" || awserr.Code() == "StateMachineDoesNotExist" { - d.SetId("") - return nil - } + if isAWSErr(err, sfn.ErrCodeStateMachineDoesNotExist, "") { + log.Printf("[WARN] SFN State Machine (%s) not found, removing from state", d.Id()) + d.SetId("") + return nil } return err } + d.Set("arn", sm.StateMachineArn) d.Set("definition", sm.Definition) d.Set("name", sm.Name) d.Set("role_arn", sm.RoleArn) @@ -146,21 +154,23 @@ func resourceAwsSfnStateMachineRead(d *schema.ResourceData, meta interface{}) er func resourceAwsSfnStateMachineUpdate(d *schema.ResourceData, meta interface{}) error { conn := meta.(*AWSClient).sfnconn - params := &sfn.UpdateStateMachineInput{ - StateMachineArn: aws.String(d.Id()), - Definition: aws.String(d.Get("definition").(string)), - RoleArn: aws.String(d.Get("role_arn").(string)), - } + if d.HasChanges("definition", "role_arn") { + params := &sfn.UpdateStateMachineInput{ + StateMachineArn: aws.String(d.Id()), + Definition: aws.String(d.Get("definition").(string)), + RoleArn: aws.String(d.Get("role_arn").(string)), + } - _, err := conn.UpdateStateMachine(params) + _, err := conn.UpdateStateMachine(params) - log.Printf("[DEBUG] Updating Step Function State Machine: %#v", params) + log.Printf("[DEBUG] Updating Step Function State Machine: %#v", params) - if err != nil { - if isAWSErr(err, "StateMachineDoesNotExist", "State Machine Does Not Exist") { - return fmt.Errorf("Error updating Step Function State Machine: %s", err) + if err != nil { + if isAWSErr(err, sfn.ErrCodeStateMachineDoesNotExist, "State Machine Does Not Exist") { + return fmt.Errorf("Error updating Step Function State Machine: %s", err) + } + return err } - return err } if d.HasChange("tags") { @@ -187,5 +197,12 @@ func resourceAwsSfnStateMachineDelete(d *schema.ResourceData, meta interface{}) return fmt.Errorf("Error deleting SFN state machine: %s", err) } + if _, err := waiter.StateMachineDeleted(conn, d.Id()); err != nil { + if isAWSErr(err, sfn.ErrCodeStateMachineDoesNotExist, "") { + return nil + } + return fmt.Errorf("error waiting for SFN State Machine (%s) deletion: %w", d.Id(), err) + } + return nil } diff --git a/aws/resource_aws_sfn_state_machine_test.go b/aws/resource_aws_sfn_state_machine_test.go index beec17c2dbd..663b6c94bf2 100644 --- a/aws/resource_aws_sfn_state_machine_test.go +++ b/aws/resource_aws_sfn_state_machine_test.go @@ -6,7 +6,6 @@ import ( "testing" "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/aws/awserr" "github.com/aws/aws-sdk-go/service/sfn" "github.com/hashicorp/terraform-plugin-sdk/helper/acctest" "github.com/hashicorp/terraform-plugin-sdk/helper/resource" @@ -14,7 +13,10 @@ import ( ) func TestAccAWSSfnStateMachine_createUpdate(t *testing.T) { - name := acctest.RandString(10) + var sm sfn.DescribeStateMachineOutput + resourceName := "aws_sfn_state_machine.test" + roleResourceName := "aws_iam_role.iam_for_sfn" + rName := acctest.RandomWithPrefix("tf-acc-test") resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -22,34 +24,43 @@ func TestAccAWSSfnStateMachine_createUpdate(t *testing.T) { CheckDestroy: testAccCheckAWSSfnStateMachineDestroy, Steps: []resource.TestStep{ { - Config: testAccAWSSfnStateMachineConfig(name, 5), + Config: testAccAWSSfnStateMachineConfig(rName, 5), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSSfnExists("aws_sfn_state_machine.foo"), - resource.TestCheckResourceAttr("aws_sfn_state_machine.foo", "status", sfn.StateMachineStatusActive), - resource.TestCheckResourceAttrSet("aws_sfn_state_machine.foo", "name"), - resource.TestCheckResourceAttrSet("aws_sfn_state_machine.foo", "creation_date"), - resource.TestCheckResourceAttrSet("aws_sfn_state_machine.foo", "definition"), - resource.TestMatchResourceAttr("aws_sfn_state_machine.foo", "definition", regexp.MustCompile(`.*\"MaxAttempts\": 5.*`)), - resource.TestCheckResourceAttrSet("aws_sfn_state_machine.foo", "role_arn"), + testAccCheckAWSSfnExists(resourceName, &sm), + testAccMatchResourceAttrRegionalARN(resourceName, "arn", "states", regexp.MustCompile(`stateMachine:.+`)), + resource.TestCheckResourceAttr(resourceName, "status", sfn.StateMachineStatusActive), + resource.TestCheckResourceAttr(resourceName, "name", rName), + resource.TestCheckResourceAttrSet(resourceName, "creation_date"), + resource.TestCheckResourceAttrSet(resourceName, "definition"), + resource.TestMatchResourceAttr(resourceName, "definition", regexp.MustCompile(`.*\"MaxAttempts\": 5.*`)), + resource.TestCheckResourceAttrPair(resourceName, "role_arn", roleResourceName, "arn"), ), }, { - Config: testAccAWSSfnStateMachineConfig(name, 10), + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + { + Config: testAccAWSSfnStateMachineConfig(rName, 10), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSSfnExists("aws_sfn_state_machine.foo"), - resource.TestCheckResourceAttr("aws_sfn_state_machine.foo", "status", sfn.StateMachineStatusActive), - resource.TestCheckResourceAttrSet("aws_sfn_state_machine.foo", "name"), - resource.TestCheckResourceAttrSet("aws_sfn_state_machine.foo", "creation_date"), - resource.TestMatchResourceAttr("aws_sfn_state_machine.foo", "definition", regexp.MustCompile(`.*\"MaxAttempts\": 10.*`)), - resource.TestCheckResourceAttrSet("aws_sfn_state_machine.foo", "role_arn"), + testAccCheckAWSSfnExists(resourceName, &sm), + testAccMatchResourceAttrRegionalARN(resourceName, "arn", "states", regexp.MustCompile(`stateMachine:.+`)), + resource.TestCheckResourceAttr(resourceName, "status", sfn.StateMachineStatusActive), + resource.TestCheckResourceAttr(resourceName, "name", rName), + resource.TestCheckResourceAttrSet(resourceName, "creation_date"), + resource.TestMatchResourceAttr(resourceName, "definition", regexp.MustCompile(`.*\"MaxAttempts\": 10.*`)), + resource.TestCheckResourceAttrPair(resourceName, "role_arn", roleResourceName, "arn"), ), }, }, }) } -func TestAccAWSSfnStateMachine_Tags(t *testing.T) { - name := acctest.RandString(10) +func TestAccAWSSfnStateMachine_tags(t *testing.T) { + var sm sfn.DescribeStateMachineOutput + resourceName := "aws_sfn_state_machine.test" + rName := acctest.RandomWithPrefix("tf-acc-test") resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -57,35 +68,62 @@ func TestAccAWSSfnStateMachine_Tags(t *testing.T) { CheckDestroy: testAccCheckAWSSfnStateMachineDestroy, Steps: []resource.TestStep{ { - Config: testAccAWSSfnStateMachineConfigTags1(name, "key1", "value1"), + Config: testAccAWSSfnStateMachineConfigTags1(rName, "key1", "value1"), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSSfnExists(resourceName, &sm), + resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), + resource.TestCheckResourceAttr(resourceName, "tags.key1", "value1"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + { + Config: testAccAWSSfnStateMachineConfigTags2(rName, "key1", "value1updated", "key2", "value2"), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSSfnExists("aws_sfn_state_machine.foo"), - resource.TestCheckResourceAttr("aws_sfn_state_machine.foo", "tags.%", "1"), - resource.TestCheckResourceAttr("aws_sfn_state_machine.foo", "tags.key1", "value1"), + testAccCheckAWSSfnExists(resourceName, &sm), + resource.TestCheckResourceAttr(resourceName, "tags.%", "2"), + resource.TestCheckResourceAttr(resourceName, "tags.key1", "value1updated"), + resource.TestCheckResourceAttr(resourceName, "tags.key2", "value2"), ), }, { - Config: testAccAWSSfnStateMachineConfigTags2(name, "key1", "value1updated", "key2", "value2"), + Config: testAccAWSSfnStateMachineConfigTags1(rName, "key2", "value2"), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSSfnExists("aws_sfn_state_machine.foo"), - resource.TestCheckResourceAttr("aws_sfn_state_machine.foo", "tags.%", "2"), - resource.TestCheckResourceAttr("aws_sfn_state_machine.foo", "tags.key1", "value1updated"), - resource.TestCheckResourceAttr("aws_sfn_state_machine.foo", "tags.key2", "value2"), + testAccCheckAWSSfnExists(resourceName, &sm), + resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), + resource.TestCheckResourceAttr(resourceName, "tags.key2", "value2"), ), }, + }, + }) +} + +func TestAccAWSSfnStateMachine_disappears(t *testing.T) { + var sm sfn.DescribeStateMachineOutput + resourceName := "aws_sfn_state_machine.test" + rName := acctest.RandomWithPrefix("tf-acc-test") + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSSfnStateMachineDestroy, + Steps: []resource.TestStep{ { - Config: testAccAWSSfnStateMachineConfigTags1(name, "key2", "value2"), + Config: testAccAWSSfnStateMachineConfig(rName, 5), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSSfnExists("aws_sfn_state_machine.foo"), - resource.TestCheckResourceAttr("aws_sfn_state_machine.foo", "tags.%", "1"), - resource.TestCheckResourceAttr("aws_sfn_state_machine.foo", "tags.key2", "value2"), + testAccCheckAWSSfnExists(resourceName, &sm), + testAccCheckResourceDisappears(testAccProvider, resourceAwsSfnStateMachine(), resourceName), ), + ExpectNonEmptyPlan: true, }, }, }) } -func testAccCheckAWSSfnExists(n string) resource.TestCheckFunc { +func testAccCheckAWSSfnExists(n string, sm *sfn.DescribeStateMachineOutput) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[n] if !ok { @@ -98,11 +136,17 @@ func testAccCheckAWSSfnExists(n string) resource.TestCheckFunc { conn := testAccProvider.Meta().(*AWSClient).sfnconn - _, err := conn.DescribeStateMachine(&sfn.DescribeStateMachineInput{ + resp, err := conn.DescribeStateMachine(&sfn.DescribeStateMachineInput{ StateMachineArn: aws.String(rs.Primary.ID), }) - return err + if err != nil { + return err + } + + *sm = *resp + + return nil } } @@ -119,50 +163,25 @@ func testAccCheckAWSSfnStateMachineDestroy(s *terraform.State) error { }) if err != nil { - if wserr, ok := err.(awserr.Error); ok && wserr.Code() == "StateMachineDoesNotExist" { - return nil + if isAWSErr(err, sfn.ErrCodeStateMachineDoesNotExist, "") { + continue } - return err } - if out != nil && *out.Status != sfn.StateMachineStatusDeleting { + if out != nil && aws.StringValue(out.Status) != sfn.StateMachineStatusDeleting { return fmt.Errorf("Expected AWS Step Function State Machine to be destroyed, but was still found") } - return nil + return err } - return fmt.Errorf("Default error in Step Function Test") + return nil } -func testAccAWSSfnStateMachineConfig(rName string, rMaxAttempts int) string { +func testAccAWSSfnStateMachineConfigBase(rName string) string { return fmt.Sprintf(` -data "aws_partition" "current" {} - -data "aws_region" "current" {} - -resource "aws_iam_role_policy" "iam_policy_for_lambda" { - name = "iam_policy_for_lambda_%s" - role = "${aws_iam_role.iam_for_lambda.id}" - - policy = <