From b627ed840bdca0e1321e58c84599d3e09077e34d Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Mon, 31 Oct 2022 16:18:20 -0400 Subject: [PATCH 01/13] r/aws_sfn_activity: Add 'FindActivityByARN'. Acceptance test output: % make testacc TESTARGS='-run=TestAccSFNActivity_' PKG=sfn ACCTEST_PARALLELISM=3 ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./internal/service/sfn/... -v -count 1 -parallel 3 -run=TestAccSFNActivity_ -timeout 180m === RUN TestAccSFNActivity_basic === PAUSE TestAccSFNActivity_basic === RUN TestAccSFNActivity_tags === PAUSE TestAccSFNActivity_tags === CONT TestAccSFNActivity_basic === CONT TestAccSFNActivity_tags --- PASS: TestAccSFNActivity_basic (19.98s) === CONT TestAccSFNActivity_tags testing_new.go:84: Error running post-test destroy, there may be dangling resources: Step Function Activity still exists: arn:aws:states:us-west-2:067819342479:activity:tf-acc-test-4936253122607672332 --- FAIL: TestAccSFNActivity_tags (44.45s) FAIL FAIL github.com/hashicorp/terraform-provider-aws/internal/service/sfn 48.442s FAIL make: *** [testacc] Error 1 --- internal/service/sfn/activity.go | 123 ++++++++++++++++---------- internal/service/sfn/activity_test.go | 67 ++++++-------- 2 files changed, 104 insertions(+), 86 deletions(-) diff --git a/internal/service/sfn/activity.go b/internal/service/sfn/activity.go index 0e4b35f93ed..1c3e90a3320 100644 --- a/internal/service/sfn/activity.go +++ b/internal/service/sfn/activity.go @@ -6,12 +6,14 @@ 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/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" "github.com/hashicorp/terraform-provider-aws/internal/conns" tftags "github.com/hashicorp/terraform-provider-aws/internal/tags" + "github.com/hashicorp/terraform-provider-aws/internal/tfresource" "github.com/hashicorp/terraform-provider-aws/internal/verify" ) @@ -21,22 +23,22 @@ func ResourceActivity() *schema.Resource { Read: resourceActivityRead, Update: resourceActivityUpdate, Delete: resourceActivityDelete, + Importer: &schema.ResourceImporter{ State: schema.ImportStatePassthrough, }, Schema: map[string]*schema.Schema{ + "creation_date": { + Type: schema.TypeString, + Computed: true, + }, "name": { Type: schema.TypeString, Required: true, ForceNew: true, ValidateFunc: validation.StringLenBetween(0, 80), }, - - "creation_date": { - Type: schema.TypeString, - Computed: true, - }, "tags": tftags.TagsSchema(), "tags_all": tftags.TagsSchemaComputed(), }, @@ -49,32 +51,20 @@ func resourceActivityCreate(d *schema.ResourceData, meta interface{}) error { conn := meta.(*conns.AWSClient).SFNConn defaultTagsConfig := meta.(*conns.AWSClient).DefaultTagsConfig tags := defaultTagsConfig.MergeTags(tftags.New(d.Get("tags").(map[string]interface{}))) - log.Print("[DEBUG] Creating Step Function Activity") - params := &sfn.CreateActivityInput{ - Name: aws.String(d.Get("name").(string)), + name := d.Get("name").(string) + input := &sfn.CreateActivityInput{ + Name: aws.String(name), Tags: Tags(tags.IgnoreAWS()), } - activity, err := conn.CreateActivity(params) + output, err := conn.CreateActivity(input) + if err != nil { - return fmt.Errorf("Error creating Step Function Activity: %s", err) + return fmt.Errorf("creating Step Function Activity (%s): %w", name, err) } - d.SetId(aws.StringValue(activity.ActivityArn)) - - return resourceActivityRead(d, meta) -} - -func resourceActivityUpdate(d *schema.ResourceData, meta interface{}) error { - conn := meta.(*conns.AWSClient).SFNConn - - if d.HasChange("tags_all") { - o, n := d.GetChange("tags_all") - if err := UpdateTags(conn, d.Id(), o, n); err != nil { - return fmt.Errorf("error updating tags: %s", err) - } - } + d.SetId(aws.StringValue(output.ActivityArn)) return resourceActivityRead(d, meta) } @@ -84,58 +74,99 @@ func resourceActivityRead(d *schema.ResourceData, meta interface{}) error { defaultTagsConfig := meta.(*conns.AWSClient).DefaultTagsConfig ignoreTagsConfig := meta.(*conns.AWSClient).IgnoreTagsConfig - log.Printf("[DEBUG] Reading Step Function Activity: %s", d.Id()) + output, err := FindActivityByARN(conn, d.Id()) - sm, err := conn.DescribeActivity(&sfn.DescribeActivityInput{ - ActivityArn: aws.String(d.Id()), - }) - if err != nil { - if awsErr, ok := err.(awserr.Error); ok && awsErr.Code() == "ActivityDoesNotExist" { - d.SetId("") - return nil - } - return err + if !d.IsNewResource() && tfresource.NotFound(err) { + log.Printf("[WARN] Step Functions Activity (%s) not found, removing from state", d.Id()) + d.SetId("") + return nil } - d.Set("name", sm.Name) - - if err := d.Set("creation_date", sm.CreationDate.Format(time.RFC3339)); err != nil { - log.Printf("[DEBUG] Error setting creation_date: %s", err) + if err != nil { + return fmt.Errorf("reading Step Functions Activity (%s): %w", d.Id(), err) } + d.Set("creation_date", output.CreationDate.Format(time.RFC3339)) + d.Set("name", output.Name) + tags, err := ListTags(conn, d.Id()) if err != nil { - return fmt.Errorf("error listing tags for SFN Activity (%s): %s", d.Id(), err) + return fmt.Errorf("listing tags for Step Functions Activity (%s): %w", d.Id(), err) } tags = tags.IgnoreAWS().IgnoreConfig(ignoreTagsConfig) //lintignore:AWSR002 if err := d.Set("tags", tags.RemoveDefaultConfig(defaultTagsConfig).Map()); err != nil { - return fmt.Errorf("error setting tags: %w", err) + return fmt.Errorf("setting tags: %w", err) } if err := d.Set("tags_all", tags.Map()); err != nil { - return fmt.Errorf("error setting tags_all: %w", err) + return fmt.Errorf("setting tags_all: %w", err) } return nil } +func resourceActivityUpdate(d *schema.ResourceData, meta interface{}) error { + conn := meta.(*conns.AWSClient).SFNConn + + if d.HasChange("tags_all") { + o, n := d.GetChange("tags_all") + + if err := UpdateTags(conn, d.Id(), o, n); err != nil { + return fmt.Errorf("updating Step Function Activity (%s) tags: %w", d.Id(), err) + } + } + + return resourceActivityRead(d, meta) +} + func resourceActivityDelete(d *schema.ResourceData, meta interface{}) error { conn := meta.(*conns.AWSClient).SFNConn - log.Printf("[DEBUG] Deleting Step Functions Activity: %s", d.Id()) - input := &sfn.DeleteActivityInput{ + log.Printf("[DEBUG] Deleting Step Functions Activity: %s", d.Id()) + _, err := conn.DeleteActivity(&sfn.DeleteActivityInput{ ActivityArn: aws.String(d.Id()), + }) + + if err != nil { + return fmt.Errorf("deleting Step Functions Activity (%s): %w", d.Id(), err) } - _, err := conn.DeleteActivity(input) + _, err = tfresource.RetryUntilNotFound(1*time.Minute, func() (interface{}, error) { + return FindActivityByARN(conn, d.Id()) + }) if err != nil { - return fmt.Errorf("Error deleting SFN Activity: %s", err) + return fmt.Errorf("waiting for Step Functions Activity (%s) delete: %w", d.Id(), err) } return nil } + +func FindActivityByARN(conn *sfn.SFN, arn string) (*sfn.DescribeActivityOutput, error) { + input := &sfn.DescribeActivityInput{ + ActivityArn: aws.String(arn), + } + + output, err := conn.DescribeActivity(input) + + if tfawserr.ErrCodeEquals(err, sfn.ErrCodeActivityDoesNotExist) { + return nil, &resource.NotFoundError{ + LastError: err, + LastRequest: input, + } + } + + if err != nil { + return nil, err + } + + if output == nil || output.CreationDate == nil { + return nil, tfresource.NewEmptyResultError(input) + } + + return output, nil +} diff --git a/internal/service/sfn/activity_test.go b/internal/service/sfn/activity_test.go index f3a51d6d78a..e30ca3aad8a 100644 --- a/internal/service/sfn/activity_test.go +++ b/internal/service/sfn/activity_test.go @@ -3,20 +3,19 @@ package sfn_test import ( "fmt" "testing" - "time" - "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/sfn" - "github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr" sdkacctest "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" "github.com/hashicorp/terraform-provider-aws/internal/acctest" "github.com/hashicorp/terraform-provider-aws/internal/conns" + tfsfn "github.com/hashicorp/terraform-provider-aws/internal/service/sfn" + "github.com/hashicorp/terraform-provider-aws/internal/tfresource" ) func TestAccSFNActivity_basic(t *testing.T) { - name := sdkacctest.RandString(10) + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resourceName := "aws_sfn_activity.test" resource.ParallelTest(t, resource.TestCase{ @@ -26,11 +25,12 @@ func TestAccSFNActivity_basic(t *testing.T) { CheckDestroy: testAccCheckActivityDestroy, Steps: []resource.TestStep{ { - Config: testAccActivityConfig_basic(name), + Config: testAccActivityConfig_basic(rName), Check: resource.ComposeTestCheckFunc( testAccCheckActivityExists(resourceName), - resource.TestCheckResourceAttr(resourceName, "name", name), resource.TestCheckResourceAttrSet(resourceName, "creation_date"), + resource.TestCheckResourceAttr(resourceName, "name", rName), + resource.TestCheckResourceAttr(resourceName, "tags.%", "0"), ), }, { @@ -43,7 +43,7 @@ func TestAccSFNActivity_basic(t *testing.T) { } func TestAccSFNActivity_tags(t *testing.T) { - name := sdkacctest.RandString(10) + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resourceName := "aws_sfn_activity.test" resource.ParallelTest(t, resource.TestCase{ @@ -53,7 +53,7 @@ func TestAccSFNActivity_tags(t *testing.T) { CheckDestroy: testAccCheckActivityDestroy, Steps: []resource.TestStep{ { - Config: testAccActivityConfig_basicTags1(name, "key1", "value1"), + Config: testAccActivityConfig_basicTags1(rName, "key1", "value1"), Check: resource.ComposeTestCheckFunc( testAccCheckActivityExists(resourceName), resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), @@ -66,7 +66,7 @@ func TestAccSFNActivity_tags(t *testing.T) { ImportStateVerify: true, }, { - Config: testAccActivityConfig_basicTags2(name, "key1", "value1updated", "key2", "value2"), + Config: testAccActivityConfig_basicTags2(rName, "key1", "value1updated", "key2", "value2"), Check: resource.ComposeTestCheckFunc( testAccCheckActivityExists(resourceName), resource.TestCheckResourceAttr(resourceName, "tags.%", "2"), @@ -75,7 +75,7 @@ func TestAccSFNActivity_tags(t *testing.T) { ), }, { - Config: testAccActivityConfig_basicTags1(name, "key2", "value2"), + Config: testAccActivityConfig_basicTags1(rName, "key2", "value2"), Check: resource.ComposeTestCheckFunc( testAccCheckActivityExists(resourceName), resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), @@ -94,14 +94,12 @@ func testAccCheckActivityExists(n string) resource.TestCheckFunc { } if rs.Primary.ID == "" { - return fmt.Errorf("No Step Function ID set") + return fmt.Errorf("No Step Function Activity ID set") } conn := acctest.Provider.Meta().(*conns.AWSClient).SFNConn - _, err := conn.DescribeActivity(&sfn.DescribeActivityInput{ - ActivityArn: aws.String(rs.Primary.ID), - }) + _, err := tfsfn.FindActivityByARN(conn, rs.Primary.ID) return err } @@ -115,37 +113,26 @@ func testAccCheckActivityDestroy(s *terraform.State) error { continue } - // Retrying as Read after Delete is not always consistent - retryErr := resource.Retry(1*time.Minute, func() *resource.RetryError { - var err error + _, err := tfsfn.FindActivityByARN(conn, rs.Primary.ID) - _, err = conn.DescribeActivity(&sfn.DescribeActivityInput{ - ActivityArn: aws.String(rs.Primary.ID), - }) - - if tfawserr.ErrCodeEquals(err, sfn.ErrCodeActivityDoesNotExist) { - return nil - } - - if err != nil { - return resource.NonRetryableError(err) - } + if tfresource.NotFound(err) { + continue + } - // If there are no errors, the removal failed - // and the object is not yet removed. - return resource.RetryableError(fmt.Errorf("Expected AWS Step Function Activity to be destroyed, but was still found, retrying")) - }) + if err != nil { + return err + } - return retryErr + return fmt.Errorf("Step Function Activity still exists: %s", rs.Primary.ID) } - return fmt.Errorf("Default error in Step Function Test") + return nil } func testAccActivityConfig_basic(rName string) string { return fmt.Sprintf(` resource "aws_sfn_activity" "test" { - name = "%s" + name = %[1]q } `, rName) } @@ -153,10 +140,10 @@ resource "aws_sfn_activity" "test" { func testAccActivityConfig_basicTags1(rName, tag1Key, tag1Value string) string { return fmt.Sprintf(` resource "aws_sfn_activity" "test" { - name = "%s" + name = %[1]q tags = { - %q = %q + %[2]q = %[3]q } } `, rName, tag1Key, tag1Value) @@ -165,11 +152,11 @@ resource "aws_sfn_activity" "test" { func testAccActivityConfig_basicTags2(rName, tag1Key, tag1Value, tag2Key, tag2Value string) string { return fmt.Sprintf(` resource "aws_sfn_activity" "test" { - name = "%s" + name = %[1]q tags = { - %q = %q - %q = %q + %[2]q = %[3]q + %[4]q = %[5]q } } `, rName, tag1Key, tag1Value, tag2Key, tag2Value) From cd8159641bf8527a95f386a4b1957ee84d555fc9 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 1 Nov 2022 07:55:37 -0400 Subject: [PATCH 02/13] r/aws_sfn_activity: Restore retry in 'testAccCheckActivityDestroy'. Acceptance test output: % make testacc TESTARGS='-run=TestAccSFNActivity_' PKG=sfn ACCTEST_PARALLELISM=3 ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./internal/service/sfn/... -v -count 1 -parallel 3 -run=TestAccSFNActivity_ -timeout 180m === RUN TestAccSFNActivity_basic === PAUSE TestAccSFNActivity_basic === RUN TestAccSFNActivity_tags === PAUSE TestAccSFNActivity_tags === CONT TestAccSFNActivity_basic === CONT TestAccSFNActivity_tags --- PASS: TestAccSFNActivity_tags (69.59s) --- PASS: TestAccSFNActivity_basic (74.98s) PASS ok github.com/hashicorp/terraform-provider-aws/internal/service/sfn 79.335s --- internal/service/sfn/activity.go | 8 -------- internal/service/sfn/activity_test.go | 22 ++++++++++++++-------- 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/internal/service/sfn/activity.go b/internal/service/sfn/activity.go index 1c3e90a3320..02a474f9724 100644 --- a/internal/service/sfn/activity.go +++ b/internal/service/sfn/activity.go @@ -135,14 +135,6 @@ func resourceActivityDelete(d *schema.ResourceData, meta interface{}) error { return fmt.Errorf("deleting Step Functions Activity (%s): %w", d.Id(), err) } - _, err = tfresource.RetryUntilNotFound(1*time.Minute, func() (interface{}, error) { - return FindActivityByARN(conn, d.Id()) - }) - - if err != nil { - return fmt.Errorf("waiting for Step Functions Activity (%s) delete: %w", d.Id(), err) - } - return nil } diff --git a/internal/service/sfn/activity_test.go b/internal/service/sfn/activity_test.go index e30ca3aad8a..5f5c864366c 100644 --- a/internal/service/sfn/activity_test.go +++ b/internal/service/sfn/activity_test.go @@ -3,6 +3,7 @@ package sfn_test import ( "fmt" "testing" + "time" "github.com/aws/aws-sdk-go/service/sfn" sdkacctest "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest" @@ -113,17 +114,22 @@ func testAccCheckActivityDestroy(s *terraform.State) error { continue } - _, err := tfsfn.FindActivityByARN(conn, rs.Primary.ID) + // Retrying as Read after Delete is not always consistent. + err := resource.Retry(1*time.Minute, func() *resource.RetryError { + _, err := tfsfn.FindActivityByARN(conn, rs.Primary.ID) - if tfresource.NotFound(err) { - continue - } + if tfresource.NotFound(err) { + return nil + } - if err != nil { - return err - } + if err != nil { + return resource.NonRetryableError(err) + } + + return resource.RetryableError(fmt.Errorf("Step Function Activity still exists: %s", rs.Primary.ID)) + }) - return fmt.Errorf("Step Function Activity still exists: %s", rs.Primary.ID) + return err } return nil From 96dc7204d066fb7f5e08db2cd5547c647b625e1c Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 1 Nov 2022 08:04:21 -0400 Subject: [PATCH 03/13] r/aws_swf_domain: Restore retry in 'testAccCheckDomainDestroy'. Acceptance test output: % SWF_DOMAIN_TESTING_ENABLED=1 make testacc TESTARGS='-run=TestAccSWFDomain_' PKG=swf ACCTEST_PARALLELISM=3 ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./internal/service/swf/... -v -count 1 -parallel 3 -run=TestAccSWFDomain_ -timeout 180m === RUN TestAccSWFDomain_basic === PAUSE TestAccSWFDomain_basic === RUN TestAccSWFDomain_nameGenerated === PAUSE TestAccSWFDomain_nameGenerated === RUN TestAccSWFDomain_namePrefix === PAUSE TestAccSWFDomain_namePrefix === RUN TestAccSWFDomain_tags === PAUSE TestAccSWFDomain_tags === RUN TestAccSWFDomain_description === PAUSE TestAccSWFDomain_description === CONT TestAccSWFDomain_basic === CONT TestAccSWFDomain_tags === CONT TestAccSWFDomain_description --- PASS: TestAccSWFDomain_description (20.60s) === CONT TestAccSWFDomain_namePrefix --- PASS: TestAccSWFDomain_basic (21.20s) === CONT TestAccSWFDomain_nameGenerated --- PASS: TestAccSWFDomain_namePrefix (18.86s) --- PASS: TestAccSWFDomain_nameGenerated (18.78s) --- PASS: TestAccSWFDomain_tags (46.25s) PASS ok github.com/hashicorp/terraform-provider-aws/internal/service/swf 50.504s --- internal/service/swf/domain.go | 9 --------- internal/service/swf/domain_test.go | 22 ++++++++++++++-------- 2 files changed, 14 insertions(+), 17 deletions(-) diff --git a/internal/service/swf/domain.go b/internal/service/swf/domain.go index 1a3705df787..ec289c101b3 100644 --- a/internal/service/swf/domain.go +++ b/internal/service/swf/domain.go @@ -5,7 +5,6 @@ import ( "fmt" "log" "strconv" - "time" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/swf" @@ -176,14 +175,6 @@ func resourceDomainDelete(ctx context.Context, d *schema.ResourceData, meta inte return diag.Errorf("deleting SWF Domain (%s): %s", d.Id(), err) } - _, err = tfresource.RetryUntilNotFoundContext(ctx, 1*time.Minute, func() (interface{}, error) { - return FindDomainByName(ctx, conn, d.Id()) - }) - - if err != nil { - return diag.Errorf("waiting for SWF Domain (%s) delete: %s", d.Id(), err) - } - return nil } diff --git a/internal/service/swf/domain_test.go b/internal/service/swf/domain_test.go index 45cf9175ea1..05f5b30ca50 100644 --- a/internal/service/swf/domain_test.go +++ b/internal/service/swf/domain_test.go @@ -6,6 +6,7 @@ import ( "os" "regexp" "testing" + "time" "github.com/aws/aws-sdk-go/service/swf" sdkacctest "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest" @@ -202,17 +203,22 @@ func testAccCheckDomainDestroy(s *terraform.State) error { continue } - _, err := tfswf.FindDomainByName(context.Background(), conn, rs.Primary.ID) + // Retrying as Read after Delete is not always consistent. + err := resource.Retry(2*time.Minute, func() *resource.RetryError { + _, err := tfswf.FindDomainByName(context.Background(), conn, rs.Primary.ID) - if tfresource.NotFound(err) { - continue - } + if tfresource.NotFound(err) { + return nil + } - if err != nil { - return err - } + if err != nil { + return resource.NonRetryableError(err) + } + + return resource.RetryableError(fmt.Errorf("SWF Domain still exists: %s", rs.Primary.ID)) + }) - return fmt.Errorf("SWF Domain still exists: %s", rs.Primary.ID) + return err } return nil From d1694d86686959c28297cd80dbaa680fac58e2d1 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 1 Nov 2022 08:12:49 -0400 Subject: [PATCH 04/13] r/aws_sfn_activity: Switch to 'WithoutTimeout' CRUD handlers (#15090). Acceptance test output: % make testacc TESTARGS='-run=TestAccSFNActivity_' PKG=sfn ACCTEST_PARALLELISM=3 ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./internal/service/sfn/... -v -count 1 -parallel 3 -run=TestAccSFNActivity_ -timeout 180m === RUN TestAccSFNActivity_basic === PAUSE TestAccSFNActivity_basic === RUN TestAccSFNActivity_tags === PAUSE TestAccSFNActivity_tags === CONT TestAccSFNActivity_basic === CONT TestAccSFNActivity_tags --- PASS: TestAccSFNActivity_basic (19.31s) --- PASS: TestAccSFNActivity_tags (69.13s) PASS ok github.com/hashicorp/terraform-provider-aws/internal/service/sfn 73.319s --- internal/service/sfn/activity.go | 51 ++++++++++++++------------- internal/service/sfn/activity_test.go | 5 +-- 2 files changed, 29 insertions(+), 27 deletions(-) diff --git a/internal/service/sfn/activity.go b/internal/service/sfn/activity.go index 02a474f9724..725faa5e716 100644 --- a/internal/service/sfn/activity.go +++ b/internal/service/sfn/activity.go @@ -1,13 +1,14 @@ package sfn import ( - "fmt" + "context" "log" "time" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/sfn" "github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr" + "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" @@ -19,10 +20,10 @@ import ( func ResourceActivity() *schema.Resource { return &schema.Resource{ - Create: resourceActivityCreate, - Read: resourceActivityRead, - Update: resourceActivityUpdate, - Delete: resourceActivityDelete, + CreateWithoutTimeout: resourceActivityCreate, + ReadWithoutTimeout: resourceActivityRead, + UpdateWithoutTimeout: resourceActivityUpdate, + DeleteWithoutTimeout: resourceActivityDelete, Importer: &schema.ResourceImporter{ State: schema.ImportStatePassthrough, @@ -47,7 +48,7 @@ func ResourceActivity() *schema.Resource { } } -func resourceActivityCreate(d *schema.ResourceData, meta interface{}) error { +func resourceActivityCreate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { conn := meta.(*conns.AWSClient).SFNConn defaultTagsConfig := meta.(*conns.AWSClient).DefaultTagsConfig tags := defaultTagsConfig.MergeTags(tftags.New(d.Get("tags").(map[string]interface{}))) @@ -58,23 +59,23 @@ func resourceActivityCreate(d *schema.ResourceData, meta interface{}) error { Tags: Tags(tags.IgnoreAWS()), } - output, err := conn.CreateActivity(input) + output, err := conn.CreateActivityWithContext(ctx, input) if err != nil { - return fmt.Errorf("creating Step Function Activity (%s): %w", name, err) + return diag.Errorf("creating Step Function Activity (%s): %s", name, err) } d.SetId(aws.StringValue(output.ActivityArn)) - return resourceActivityRead(d, meta) + return resourceActivityRead(ctx, d, meta) } -func resourceActivityRead(d *schema.ResourceData, meta interface{}) error { +func resourceActivityRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { conn := meta.(*conns.AWSClient).SFNConn defaultTagsConfig := meta.(*conns.AWSClient).DefaultTagsConfig ignoreTagsConfig := meta.(*conns.AWSClient).IgnoreTagsConfig - output, err := FindActivityByARN(conn, d.Id()) + output, err := FindActivityByARN(ctx, conn, d.Id()) if !d.IsNewResource() && tfresource.NotFound(err) { log.Printf("[WARN] Step Functions Activity (%s) not found, removing from state", d.Id()) @@ -83,67 +84,67 @@ func resourceActivityRead(d *schema.ResourceData, meta interface{}) error { } if err != nil { - return fmt.Errorf("reading Step Functions Activity (%s): %w", d.Id(), err) + return diag.Errorf("reading Step Functions Activity (%s): %s", d.Id(), err) } d.Set("creation_date", output.CreationDate.Format(time.RFC3339)) d.Set("name", output.Name) - tags, err := ListTags(conn, d.Id()) + tags, err := ListTagsWithContext(ctx, conn, d.Id()) if err != nil { - return fmt.Errorf("listing tags for Step Functions Activity (%s): %w", d.Id(), err) + return diag.Errorf("listing tags for Step Functions Activity (%s): %s", d.Id(), err) } tags = tags.IgnoreAWS().IgnoreConfig(ignoreTagsConfig) //lintignore:AWSR002 if err := d.Set("tags", tags.RemoveDefaultConfig(defaultTagsConfig).Map()); err != nil { - return fmt.Errorf("setting tags: %w", err) + return diag.Errorf("setting tags: %s", err) } if err := d.Set("tags_all", tags.Map()); err != nil { - return fmt.Errorf("setting tags_all: %w", err) + return diag.Errorf("setting tags_all: %s", err) } return nil } -func resourceActivityUpdate(d *schema.ResourceData, meta interface{}) error { +func resourceActivityUpdate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { conn := meta.(*conns.AWSClient).SFNConn if d.HasChange("tags_all") { o, n := d.GetChange("tags_all") - if err := UpdateTags(conn, d.Id(), o, n); err != nil { - return fmt.Errorf("updating Step Function Activity (%s) tags: %w", d.Id(), err) + if err := UpdateTagsWithContext(ctx, conn, d.Id(), o, n); err != nil { + return diag.Errorf("updating Step Function Activity (%s) tags: %s", d.Id(), err) } } - return resourceActivityRead(d, meta) + return resourceActivityRead(ctx, d, meta) } -func resourceActivityDelete(d *schema.ResourceData, meta interface{}) error { +func resourceActivityDelete(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { conn := meta.(*conns.AWSClient).SFNConn log.Printf("[DEBUG] Deleting Step Functions Activity: %s", d.Id()) - _, err := conn.DeleteActivity(&sfn.DeleteActivityInput{ + _, err := conn.DeleteActivityWithContext(ctx, &sfn.DeleteActivityInput{ ActivityArn: aws.String(d.Id()), }) if err != nil { - return fmt.Errorf("deleting Step Functions Activity (%s): %w", d.Id(), err) + return diag.Errorf("deleting Step Functions Activity (%s): %s", d.Id(), err) } return nil } -func FindActivityByARN(conn *sfn.SFN, arn string) (*sfn.DescribeActivityOutput, error) { +func FindActivityByARN(ctx context.Context, conn *sfn.SFN, arn string) (*sfn.DescribeActivityOutput, error) { input := &sfn.DescribeActivityInput{ ActivityArn: aws.String(arn), } - output, err := conn.DescribeActivity(input) + output, err := conn.DescribeActivityWithContext(ctx, input) if tfawserr.ErrCodeEquals(err, sfn.ErrCodeActivityDoesNotExist) { return nil, &resource.NotFoundError{ diff --git a/internal/service/sfn/activity_test.go b/internal/service/sfn/activity_test.go index 5f5c864366c..661181e68db 100644 --- a/internal/service/sfn/activity_test.go +++ b/internal/service/sfn/activity_test.go @@ -1,6 +1,7 @@ package sfn_test import ( + "context" "fmt" "testing" "time" @@ -100,7 +101,7 @@ func testAccCheckActivityExists(n string) resource.TestCheckFunc { conn := acctest.Provider.Meta().(*conns.AWSClient).SFNConn - _, err := tfsfn.FindActivityByARN(conn, rs.Primary.ID) + _, err := tfsfn.FindActivityByARN(context.Background(), conn, rs.Primary.ID) return err } @@ -116,7 +117,7 @@ func testAccCheckActivityDestroy(s *terraform.State) error { // Retrying as Read after Delete is not always consistent. err := resource.Retry(1*time.Minute, func() *resource.RetryError { - _, err := tfsfn.FindActivityByARN(conn, rs.Primary.ID) + _, err := tfsfn.FindActivityByARN(context.Background(), conn, rs.Primary.ID) if tfresource.NotFound(err) { return nil From 1769f7094d984f69744fe1b1c6d40ce3305664eb Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 1 Nov 2022 08:26:51 -0400 Subject: [PATCH 05/13] d/aws_sfn_activity: Switch to 'WithoutTimeout' CRUD handlers (#15090). Acceptance test output: % make testacc TESTARGS='-run=TestAccSFNActivityDataSource_' PKG=sfn ACCTEST_PARALLELISM=3 ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./internal/service/sfn/... -v -count 1 -parallel 3 -run=TestAccSFNActivityDataSource_ -timeout 180m === RUN TestAccSFNActivityDataSource_basic === PAUSE TestAccSFNActivityDataSource_basic === CONT TestAccSFNActivityDataSource_basic --- PASS: TestAccSFNActivityDataSource_basic (26.80s) PASS ok github.com/hashicorp/terraform-provider-aws/internal/service/sfn 30.968s --- internal/service/sfn/activity_data_source.go | 96 +++++++++---------- .../service/sfn/activity_data_source_test.go | 47 ++++----- 2 files changed, 61 insertions(+), 82 deletions(-) diff --git a/internal/service/sfn/activity_data_source.go b/internal/service/sfn/activity_data_source.go index 69288fe1f9c..d8197289b1a 100644 --- a/internal/service/sfn/activity_data_source.go +++ b/internal/service/sfn/activity_data_source.go @@ -1,22 +1,22 @@ package sfn import ( - "fmt" - "log" + "context" "time" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/sfn" + "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-provider-aws/internal/conns" ) func DataSourceActivity() *schema.Resource { return &schema.Resource{ - Read: dataSourceActivityRead, + ReadWithoutTimeout: dataSourceActivityRead, Schema: map[string]*schema.Schema{ - "name": { + "arn": { Type: schema.TypeString, Computed: true, Optional: true, @@ -25,7 +25,11 @@ func DataSourceActivity() *schema.Resource { "name", }, }, - "arn": { + "creation_date": { + Type: schema.TypeString, + Computed: true, + }, + "name": { Type: schema.TypeString, Computed: true, Optional: true, @@ -34,75 +38,61 @@ func DataSourceActivity() *schema.Resource { "name", }, }, - "creation_date": { - Type: schema.TypeString, - Computed: true, - }, }, } } -func dataSourceActivityRead(d *schema.ResourceData, meta interface{}) error { - client := meta.(*conns.AWSClient) - conn := client.SFNConn - log.Print("[DEBUG] Reading Step Function Activity") +func dataSourceActivityRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { + conn := meta.(*conns.AWSClient).SFNConn - if nm, ok := d.GetOk("name"); ok { - name := nm.(string) - var acts []*sfn.ActivityListItem + if v, ok := d.GetOk("name"); ok { + name := v.(string) + var activities []*sfn.ActivityListItem - err := conn.ListActivitiesPages(&sfn.ListActivitiesInput{}, func(page *sfn.ListActivitiesOutput, lastPage bool) bool { - for _, a := range page.Activities { - if name == aws.StringValue(a.Name) { - acts = append(acts, a) + err := conn.ListActivitiesPagesWithContext(ctx, &sfn.ListActivitiesInput{}, func(page *sfn.ListActivitiesOutput, lastPage bool) bool { + if page == nil { + return !lastPage + } + + for _, v := range page.Activities { + if name == aws.StringValue(v.Name) { + activities = append(activities, v) } } + return !lastPage }) if err != nil { - return fmt.Errorf("Error listing activities: %w", err) + return diag.Errorf("listing Step Function Activities: %s", err) } - if len(acts) == 0 { - return fmt.Errorf("No activity found with name %s in this region", name) + if n := len(activities); n == 0 { + return diag.Errorf("no Step Function Activities matched") + } else if n > 1 { + return diag.Errorf("%d Step Function Activities matched; use additional constraints to reduce matches to a single Activity", n) } - if len(acts) > 1 { - return fmt.Errorf("Found more than 1 activity with name %s in this region", name) - } - - act := acts[0] - - d.SetId(aws.StringValue(act.ActivityArn)) - d.Set("name", act.Name) - d.Set("arn", act.ActivityArn) - if err := d.Set("creation_date", act.CreationDate.Format(time.RFC3339)); err != nil { - log.Printf("[DEBUG] Error setting creation_date: %s", err) - } - } + activity := activities[0] - if rnm, ok := d.GetOk("arn"); ok { - arn := rnm.(string) - params := &sfn.DescribeActivityInput{ - ActivityArn: aws.String(arn), - } + arn := aws.StringValue(activity.ActivityArn) + d.SetId(arn) + d.Set("arn", arn) + d.Set("creation_date", activity.CreationDate.Format(time.RFC3339)) + d.Set("name", activity.Name) + } else if v, ok := d.GetOk("arn"); ok { + arn := v.(string) + activity, err := FindActivityByARN(ctx, conn, arn) - act, err := conn.DescribeActivity(params) if err != nil { - return fmt.Errorf("Error describing activities: %w", err) + return diag.Errorf("reading Step Functions Activity (%s): %s", arn, err) } - if act == nil { - return fmt.Errorf("No activity found with arn %s in this region", arn) - } - - d.SetId(aws.StringValue(act.ActivityArn)) - d.Set("name", act.Name) - d.Set("arn", act.ActivityArn) - if err := d.Set("creation_date", act.CreationDate.Format(time.RFC3339)); err != nil { - log.Printf("[DEBUG] Error setting creation_date: %s", err) - } + arn = aws.StringValue(activity.ActivityArn) + d.SetId(arn) + d.Set("arn", arn) + d.Set("creation_date", activity.CreationDate.Format(time.RFC3339)) + d.Set("name", activity.Name) } return nil diff --git a/internal/service/sfn/activity_data_source_test.go b/internal/service/sfn/activity_data_source_test.go index 1abab8e100d..ef44c2f2946 100644 --- a/internal/service/sfn/activity_data_source_test.go +++ b/internal/service/sfn/activity_data_source_test.go @@ -10,10 +10,11 @@ import ( "github.com/hashicorp/terraform-provider-aws/internal/acctest" ) -func TestAccSFNActivityDataSource_StepFunctions_basic(t *testing.T) { +func TestAccSFNActivityDataSource_basic(t *testing.T) { rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resourceName := "aws_sfn_activity.test" - dataName := "data.aws_sfn_activity.test" + dataSource1Name := "data.aws_sfn_activity.by_name" + dataSource2Name := "data.aws_sfn_activity.by_arn" resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(t) }, @@ -21,45 +22,33 @@ func TestAccSFNActivityDataSource_StepFunctions_basic(t *testing.T) { ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, Steps: []resource.TestStep{ { - Config: testAccActivityDataSourceConfig_checkARN(rName), - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttrPair(resourceName, "id", dataName, "id"), - resource.TestCheckResourceAttrPair(resourceName, "creation_date", dataName, "creation_date"), - resource.TestCheckResourceAttrPair(resourceName, "name", dataName, "name"), - ), - }, - { - Config: testAccActivityDataSourceConfig_checkName(rName), - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttrPair(resourceName, "id", dataName, "id"), - resource.TestCheckResourceAttrPair(resourceName, "creation_date", dataName, "creation_date"), - resource.TestCheckResourceAttrPair(resourceName, "name", dataName, "name"), + Config: testAccActivityDataSourceConfig_basic(rName), + Check: resource.ComposeAggregateTestCheckFunc( + resource.TestCheckResourceAttrPair(resourceName, "id", dataSource1Name, "arn"), + resource.TestCheckResourceAttrPair(resourceName, "creation_date", dataSource1Name, "creation_date"), + resource.TestCheckResourceAttrPair(resourceName, "name", dataSource1Name, "name"), + + resource.TestCheckResourceAttrPair(resourceName, "id", dataSource2Name, "arn"), + resource.TestCheckResourceAttrPair(resourceName, "creation_date", dataSource2Name, "creation_date"), + resource.TestCheckResourceAttrPair(resourceName, "name", dataSource2Name, "name"), ), }, }, }) } -func testAccActivityDataSourceConfig_checkARN(rName string) string { +func testAccActivityDataSourceConfig_basic(rName string) string { return fmt.Sprintf(` resource aws_sfn_activity "test" { - name = "%s" + name = %[1]q } -data aws_sfn_activity "test" { - arn = aws_sfn_activity.test.id -} -`, rName) -} - -func testAccActivityDataSourceConfig_checkName(rName string) string { - return fmt.Sprintf(` -resource aws_sfn_activity "test" { - name = "%s" +data aws_sfn_activity "by_name" { + name = aws_sfn_activity.test.name } -data aws_sfn_activity "test" { - name = aws_sfn_activity.test.name +data aws_sfn_activity "by_arn" { + arn = aws_sfn_activity.test.id } `, rName) } From 7d1bdc13054019fbf96899b66828c2edace7577b Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 1 Nov 2022 08:54:25 -0400 Subject: [PATCH 06/13] r/aws_sfn_state_machine: Tidy up error messages. Acceptance test output: % make testacc TESTARGS='-run=TestAccSFNStateMachine_' PKG=sfn ACCTEST_PARALLELISM=3 ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./internal/service/sfn/... -v -count 1 -parallel 3 -run=TestAccSFNStateMachine_ -timeout 180m === RUN TestAccSFNStateMachine_createUpdate === PAUSE TestAccSFNStateMachine_createUpdate === RUN TestAccSFNStateMachine_expressUpdate === PAUSE TestAccSFNStateMachine_expressUpdate === RUN TestAccSFNStateMachine_standardUpdate === PAUSE TestAccSFNStateMachine_standardUpdate === RUN TestAccSFNStateMachine_tags === PAUSE TestAccSFNStateMachine_tags === RUN TestAccSFNStateMachine_tracing === PAUSE TestAccSFNStateMachine_tracing === RUN TestAccSFNStateMachine_disappears === PAUSE TestAccSFNStateMachine_disappears === RUN TestAccSFNStateMachine_expressLogging === PAUSE TestAccSFNStateMachine_expressLogging === CONT TestAccSFNStateMachine_createUpdate === CONT TestAccSFNStateMachine_tracing === CONT TestAccSFNStateMachine_standardUpdate --- PASS: TestAccSFNStateMachine_createUpdate (101.37s) === CONT TestAccSFNStateMachine_tags --- PASS: TestAccSFNStateMachine_standardUpdate (102.78s) === CONT TestAccSFNStateMachine_expressLogging --- PASS: TestAccSFNStateMachine_tracing (107.31s) === CONT TestAccSFNStateMachine_expressUpdate --- PASS: TestAccSFNStateMachine_expressLogging (127.65s) === CONT TestAccSFNStateMachine_disappears --- PASS: TestAccSFNStateMachine_tags (132.07s) --- PASS: TestAccSFNStateMachine_expressUpdate (127.97s) --- PASS: TestAccSFNStateMachine_disappears (84.75s) PASS ok github.com/hashicorp/terraform-provider-aws/internal/service/sfn 319.585s --- internal/acctest/acctest.go | 13 +- internal/service/sfn/activity.go | 4 +- internal/service/sfn/activity_data_source.go | 6 +- internal/service/sfn/activity_test.go | 4 +- internal/service/sfn/find.go | 36 ---- internal/service/sfn/state_machine.go | 176 +++++++++++-------- internal/service/sfn/state_machine_test.go | 20 +-- internal/service/sfn/status.go | 24 --- internal/service/sfn/wait.go | 31 ---- 9 files changed, 123 insertions(+), 191 deletions(-) delete mode 100644 internal/service/sfn/find.go delete mode 100644 internal/service/sfn/status.go delete mode 100644 internal/service/sfn/wait.go diff --git a/internal/acctest/acctest.go b/internal/acctest/acctest.go index 25008e4683b..0f2f4d52ae4 100644 --- a/internal/acctest/acctest.go +++ b/internal/acctest/acctest.go @@ -1780,18 +1780,9 @@ func ConfigLatestAmazonLinux2HVMEBSARM64AMI() string { } func ConfigLambdaBase(policyName, roleName, sgName string) string { - return fmt.Sprintf(` + return ConfigCompose(ConfigAvailableAZsNoOptIn(), fmt.Sprintf(` data "aws_partition" "current" {} -data "aws_availability_zones" "available" { - state = "available" - - filter { - name = "opt-in-status" - values = ["opt-in-not-required"] - } -} - resource "aws_iam_role_policy" "iam_policy_for_lambda" { name = "%s" role = aws_iam_role.iam_for_lambda.id @@ -1912,7 +1903,7 @@ resource "aws_security_group" "sg_for_lambda" { cidr_blocks = ["0.0.0.0/0"] } } -`, policyName, roleName, sgName) +`, policyName, roleName, sgName)) } func ConfigVPCWithSubnets(rName string, subnetCount int) string { diff --git a/internal/service/sfn/activity.go b/internal/service/sfn/activity.go index 725faa5e716..1d20ded6074 100644 --- a/internal/service/sfn/activity.go +++ b/internal/service/sfn/activity.go @@ -62,7 +62,7 @@ func resourceActivityCreate(ctx context.Context, d *schema.ResourceData, meta in output, err := conn.CreateActivityWithContext(ctx, input) if err != nil { - return diag.Errorf("creating Step Function Activity (%s): %s", name, err) + return diag.Errorf("creating Step Functions Activity (%s): %s", name, err) } d.SetId(aws.StringValue(output.ActivityArn)) @@ -117,7 +117,7 @@ func resourceActivityUpdate(ctx context.Context, d *schema.ResourceData, meta in o, n := d.GetChange("tags_all") if err := UpdateTagsWithContext(ctx, conn, d.Id(), o, n); err != nil { - return diag.Errorf("updating Step Function Activity (%s) tags: %s", d.Id(), err) + return diag.Errorf("updating Step Functions Activity (%s) tags: %s", d.Id(), err) } } diff --git a/internal/service/sfn/activity_data_source.go b/internal/service/sfn/activity_data_source.go index d8197289b1a..f6d043825e2 100644 --- a/internal/service/sfn/activity_data_source.go +++ b/internal/service/sfn/activity_data_source.go @@ -64,13 +64,13 @@ func dataSourceActivityRead(ctx context.Context, d *schema.ResourceData, meta in }) if err != nil { - return diag.Errorf("listing Step Function Activities: %s", err) + return diag.Errorf("listing Step Functions Activities: %s", err) } if n := len(activities); n == 0 { - return diag.Errorf("no Step Function Activities matched") + return diag.Errorf("no Step Functions Activities matched") } else if n > 1 { - return diag.Errorf("%d Step Function Activities matched; use additional constraints to reduce matches to a single Activity", n) + return diag.Errorf("%d Step Functions Activities matched; use additional constraints to reduce matches to a single Activity", n) } activity := activities[0] diff --git a/internal/service/sfn/activity_test.go b/internal/service/sfn/activity_test.go index 661181e68db..e44f76b355b 100644 --- a/internal/service/sfn/activity_test.go +++ b/internal/service/sfn/activity_test.go @@ -96,7 +96,7 @@ func testAccCheckActivityExists(n string) resource.TestCheckFunc { } if rs.Primary.ID == "" { - return fmt.Errorf("No Step Function Activity ID set") + return fmt.Errorf("No Step Functions Activity ID set") } conn := acctest.Provider.Meta().(*conns.AWSClient).SFNConn @@ -127,7 +127,7 @@ func testAccCheckActivityDestroy(s *terraform.State) error { return resource.NonRetryableError(err) } - return resource.RetryableError(fmt.Errorf("Step Function Activity still exists: %s", rs.Primary.ID)) + return resource.RetryableError(fmt.Errorf("Step Functions Activity still exists: %s", rs.Primary.ID)) }) return err diff --git a/internal/service/sfn/find.go b/internal/service/sfn/find.go deleted file mode 100644 index 071c78774f8..00000000000 --- a/internal/service/sfn/find.go +++ /dev/null @@ -1,36 +0,0 @@ -package sfn - -import ( - "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/service/sfn" - "github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr" - "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" -) - -func FindStateMachineByARN(conn *sfn.SFN, arn string) (*sfn.DescribeStateMachineOutput, error) { - input := &sfn.DescribeStateMachineInput{ - StateMachineArn: aws.String(arn), - } - - output, err := conn.DescribeStateMachine(input) - - if tfawserr.ErrCodeEquals(err, sfn.ErrCodeStateMachineDoesNotExist) { - return nil, &resource.NotFoundError{ - LastError: err, - LastRequest: input, - } - } - - if err != nil { - return nil, err - } - - if output == nil { - return nil, &resource.NotFoundError{ - Message: "Empty result", - LastRequest: input, - } - } - - return output, nil -} diff --git a/internal/service/sfn/state_machine.go b/internal/service/sfn/state_machine.go index dae130ee0fa..2fe3d79fb69 100644 --- a/internal/service/sfn/state_machine.go +++ b/internal/service/sfn/state_machine.go @@ -23,6 +23,7 @@ func ResourceStateMachine() *schema.Resource { Read: resourceStateMachineRead, Update: resourceStateMachineUpdate, Delete: resourceStateMachineDelete, + Importer: &schema.ResourceImporter{ State: schema.ImportStatePassthrough, }, @@ -32,18 +33,15 @@ func ResourceStateMachine() *schema.Resource { Type: schema.TypeString, Computed: true, }, - "creation_date": { Type: schema.TypeString, Computed: true, }, - "definition": { Type: schema.TypeString, Required: true, ValidateFunc: validation.StringLenBetween(0, 1024*1024), // 1048576 }, - "logging_configuration": { Type: schema.TypeList, Optional: true, @@ -68,36 +66,23 @@ func ResourceStateMachine() *schema.Resource { }, DiffSuppressFunc: verify.SuppressMissingOptionalConfigurationBlock, }, - "name": { Type: schema.TypeString, Required: true, ForceNew: true, ValidateFunc: validStateMachineName, }, - "role_arn": { Type: schema.TypeString, Required: true, ValidateFunc: verify.ValidARN, }, - "status": { Type: schema.TypeString, Computed: true, }, - "tags": tftags.TagsSchema(), "tags_all": tftags.TagsSchemaComputed(), - - "type": { - Type: schema.TypeString, - Optional: true, - ForceNew: true, - Default: sfn.StateMachineTypeStandard, - ValidateFunc: validation.StringInSlice(sfn.StateMachineType_Values(), false), - }, - "tracing_configuration": { Type: schema.TypeList, Optional: true, @@ -113,6 +98,13 @@ func ResourceStateMachine() *schema.Resource { }, DiffSuppressFunc: verify.SuppressMissingOptionalConfigurationBlock, }, + "type": { + Type: schema.TypeString, + Optional: true, + ForceNew: true, + Default: sfn.StateMachineTypeStandard, + ValidateFunc: validation.StringInSlice(sfn.StateMachineType_Values(), false), + }, }, CustomizeDiff: verify.SetTagsDiff, @@ -141,42 +133,19 @@ func resourceStateMachineCreate(d *schema.ResourceData, meta interface{}) error input.TracingConfiguration = expandTracingConfiguration(v.([]interface{})[0].(map[string]interface{})) } - var output *sfn.CreateStateMachineOutput - - log.Printf("[DEBUG] Creating Step Function State Machine: %s", input) - err := resource.Retry(stateMachineCreatedTimeout, func() *resource.RetryError { - var err error - - output, err = conn.CreateStateMachine(input) - - // 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 tfawserr.ErrCodeEquals(err, sfn.ErrCodeStateMachineDeleting) { - return resource.RetryableError(err) - } - - // This is done to deal with IAM eventual consistency - if tfawserr.ErrCodeEquals(err, "AccessDeniedException") { - return resource.RetryableError(err) - } - - if err != nil { - return resource.NonRetryableError(err) - } - - return nil - }) - - if tfresource.TimedOut(err) { - output, err = conn.CreateStateMachine(input) - } + // This is done to deal with IAM eventual consistency. + // 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). + outputRaw, err := tfresource.RetryWhenAWSErrCodeEquals(stateMachineCreatedTimeout, func() (interface{}, error) { + return conn.CreateStateMachine(input) + }, sfn.ErrCodeStateMachineDeleting, "AccessDeniedException") if err != nil { - return fmt.Errorf("error creating Step Function State Machine (%s): %w", name, err) + return fmt.Errorf("creating Step Functions State Machine (%s): %w", name, err) } - d.SetId(aws.StringValue(output.StateMachineArn)) + d.SetId(aws.StringValue(outputRaw.(*sfn.CreateStateMachineOutput).StateMachineArn)) return resourceStateMachineRead(d, meta) } @@ -189,13 +158,13 @@ func resourceStateMachineRead(d *schema.ResourceData, meta interface{}) error { output, err := FindStateMachineByARN(conn, d.Id()) if !d.IsNewResource() && tfresource.NotFound(err) { - log.Printf("[WARN] Step Function State Machine (%s) not found, removing from state", d.Id()) + log.Printf("[WARN] Step Functions State Machine (%s) not found, removing from state", d.Id()) d.SetId("") return nil } if err != nil { - return fmt.Errorf("error reading Step Function State Machine (%s): %w", d.Id(), err) + return fmt.Errorf("error reading Step Functions State Machine (%s): %w", d.Id(), err) } d.Set("arn", output.StateMachineArn) @@ -205,11 +174,6 @@ func resourceStateMachineRead(d *schema.ResourceData, meta interface{}) error { d.Set("creation_date", nil) } d.Set("definition", output.Definition) - d.Set("name", output.Name) - d.Set("role_arn", output.RoleArn) - d.Set("type", output.Type) - d.Set("status", output.Status) - if output.LoggingConfiguration != nil { if err := d.Set("logging_configuration", []interface{}{flattenLoggingConfiguration(output.LoggingConfiguration)}); err != nil { return fmt.Errorf("error setting logging_configuration: %w", err) @@ -217,7 +181,9 @@ func resourceStateMachineRead(d *schema.ResourceData, meta interface{}) error { } else { d.Set("logging_configuration", nil) } - + d.Set("name", output.Name) + d.Set("role_arn", output.RoleArn) + d.Set("status", output.Status) if output.TracingConfiguration != nil { if err := d.Set("tracing_configuration", []interface{}{flattenTracingConfiguration(output.TracingConfiguration)}); err != nil { return fmt.Errorf("error setting tracing_configuration: %w", err) @@ -225,26 +191,27 @@ func resourceStateMachineRead(d *schema.ResourceData, meta interface{}) error { } else { d.Set("tracing_configuration", nil) } + d.Set("type", output.Type) tags, err := ListTags(conn, d.Id()) - if err != nil { - if tfawserr.ErrCodeEquals(err, "UnknownOperationException") { - return nil - } + if tfawserr.ErrCodeEquals(err, "UnknownOperationException") { + return nil + } - return fmt.Errorf("error listing tags for Step Function State Machine (%s): %w", d.Id(), err) + if err != nil { + return fmt.Errorf("listing tags for Step Functions State Machine (%s): %w", d.Id(), err) } tags = tags.IgnoreAWS().IgnoreConfig(ignoreTagsConfig) //lintignore:AWSR002 if err := d.Set("tags", tags.RemoveDefaultConfig(defaultTagsConfig).Map()); err != nil { - return fmt.Errorf("error setting tags: %w", err) + return fmt.Errorf("setting tags: %w", err) } if err := d.Set("tags_all", tags.Map()); err != nil { - return fmt.Errorf("error setting tags_all: %w", err) + return fmt.Errorf("setting tags_all: %w", err) } return nil @@ -256,9 +223,9 @@ func resourceStateMachineUpdate(d *schema.ResourceData, meta interface{}) error if d.HasChangesExcept("tags", "tags_all") { // "You must include at least one of definition or roleArn or you will receive a MissingRequiredParameter error" input := &sfn.UpdateStateMachineInput{ - StateMachineArn: aws.String(d.Id()), Definition: aws.String(d.Get("definition").(string)), RoleArn: aws.String(d.Get("role_arn").(string)), + StateMachineArn: aws.String(d.Id()), } if d.HasChange("logging_configuration") { @@ -273,11 +240,10 @@ func resourceStateMachineUpdate(d *schema.ResourceData, meta interface{}) error } } - log.Printf("[DEBUG] Updating Step Function State Machine: %s", input) _, err := conn.UpdateStateMachine(input) if err != nil { - return fmt.Errorf("error updating Step Function State Machine (%s): %w", d.Id(), err) + return fmt.Errorf("updating Step Functions State Machine (%s): %w", d.Id(), err) } // Handle eventual consistency after update. @@ -293,21 +259,22 @@ func resourceStateMachineUpdate(d *schema.ResourceData, meta interface{}) error d.HasChange("tracing_configuration.0.enabled") && output.TracingConfiguration != nil && aws.BoolValue(output.TracingConfiguration.Enabled) != d.Get("tracing_configuration.0.enabled").(bool) || d.HasChange("logging_configuration.0.include_execution_data") && output.LoggingConfiguration != nil && aws.BoolValue(output.LoggingConfiguration.IncludeExecutionData) != d.Get("logging_configuration.0.include_execution_data").(bool) || d.HasChange("logging_configuration.0.level") && output.LoggingConfiguration != nil && aws.StringValue(output.LoggingConfiguration.Level) != d.Get("logging_configuration.0.level").(string) { - return resource.RetryableError(fmt.Errorf("Step Function State Machine (%s) eventual consistency", d.Id())) + return resource.RetryableError(fmt.Errorf("Step Functions State Machine (%s) eventual consistency", d.Id())) } return nil }) - if tfresource.TimedOut(err) { - return fmt.Errorf("timed out waiting for Step Function State Machine (%s) update", d.Id()) + if err != nil { + return fmt.Errorf("waiting for Step Functions State Machine (%s) update: %w", d.Id(), err) } } if d.HasChange("tags_all") { o, n := d.GetChange("tags_all") + if err := UpdateTags(conn, d.Id(), o, n); err != nil { - return fmt.Errorf("error updating tags: %w", err) + return fmt.Errorf("updating Step Functions State Machine (%s) tags: %w", d.Id(), err) } } @@ -317,21 +284,86 @@ func resourceStateMachineUpdate(d *schema.ResourceData, meta interface{}) error func resourceStateMachineDelete(d *schema.ResourceData, meta interface{}) error { conn := meta.(*conns.AWSClient).SFNConn + log.Printf("[DEBUG] Deleting Step Functions State Machine: %s", d.Id()) _, err := conn.DeleteStateMachine(&sfn.DeleteStateMachineInput{ StateMachineArn: aws.String(d.Id()), }) if err != nil { - return fmt.Errorf("error deleting Step Function State Machine (%s): %s", d.Id(), err) + return fmt.Errorf("deleting Step Functions State Machine (%s): %w", d.Id(), err) } if _, err := waitStateMachineDeleted(conn, d.Id()); err != nil { - return fmt.Errorf("error waiting for Step Function State Machine (%s) deletion: %w", d.Id(), err) + return fmt.Errorf("waiting for Step Functions State Machine (%s) delete: %w", d.Id(), err) } return nil } +func FindStateMachineByARN(conn *sfn.SFN, arn string) (*sfn.DescribeStateMachineOutput, error) { + input := &sfn.DescribeStateMachineInput{ + StateMachineArn: aws.String(arn), + } + + output, err := conn.DescribeStateMachine(input) + + if tfawserr.ErrCodeEquals(err, sfn.ErrCodeStateMachineDoesNotExist) { + return nil, &resource.NotFoundError{ + LastError: err, + LastRequest: input, + } + } + + if err != nil { + return nil, err + } + + if output == nil { + return nil, tfresource.NewEmptyResultError(input) + } + + return output, nil +} + +func statusStateMachine(conn *sfn.SFN, stateMachineArn string) resource.StateRefreshFunc { + return func() (interface{}, string, error) { + output, err := FindStateMachineByARN(conn, stateMachineArn) + + if tfresource.NotFound(err) { + return nil, "", nil + } + + if err != nil { + return nil, "", err + } + + return output, aws.StringValue(output.Status), nil + } +} + +const ( + stateMachineCreatedTimeout = 5 * time.Minute + stateMachineDeletedTimeout = 5 * time.Minute + stateMachineUpdatedTimeout = 1 * time.Minute +) + +func waitStateMachineDeleted(conn *sfn.SFN, stateMachineArn string) (*sfn.DescribeStateMachineOutput, error) { + stateConf := &resource.StateChangeConf{ + Pending: []string{sfn.StateMachineStatusActive, sfn.StateMachineStatusDeleting}, + Target: []string{}, + Refresh: statusStateMachine(conn, stateMachineArn), + Timeout: stateMachineDeletedTimeout, + } + + outputRaw, err := stateConf.WaitForState() + + if output, ok := outputRaw.(*sfn.DescribeStateMachineOutput); ok { + return output, err + } + + return nil, err +} + func expandLoggingConfiguration(tfMap map[string]interface{}) *sfn.LoggingConfiguration { if tfMap == nil { return nil diff --git a/internal/service/sfn/state_machine_test.go b/internal/service/sfn/state_machine_test.go index f1b81077a19..d2bad8ac954 100644 --- a/internal/service/sfn/state_machine_test.go +++ b/internal/service/sfn/state_machine_test.go @@ -311,7 +311,7 @@ func testAccCheckExists(n string, v *sfn.DescribeStateMachineOutput) resource.Te } if rs.Primary.ID == "" { - return fmt.Errorf("No Step Function State Machine ID is set") + return fmt.Errorf("No Step Functions State Machine ID is set") } conn := acctest.Provider.Meta().(*conns.AWSClient).SFNConn @@ -346,13 +346,13 @@ func testAccCheckStateMachineDestroy(s *terraform.State) error { return err } - return fmt.Errorf("Step Function State Machine %s still exists", rs.Primary.ID) + return fmt.Errorf("Step Functions State Machine %s still exists", rs.Primary.ID) } return nil } -func testAccStateMachineBaseConfig(rName string) string { +func testAccStateMachineConfig_base(rName string) string { return fmt.Sprintf(` resource "aws_iam_role_policy" "for_lambda" { name = "%[1]s-lambda" @@ -453,7 +453,7 @@ EOF } func testAccStateMachineConfig_basic(rName string, rMaxAttempts int) string { - return acctest.ConfigCompose(testAccStateMachineBaseConfig(rName), fmt.Sprintf(` + return acctest.ConfigCompose(testAccStateMachineConfig_base(rName), fmt.Sprintf(` resource "aws_sfn_state_machine" "test" { name = %[1]q role_arn = aws_iam_role.for_sfn.arn @@ -486,7 +486,7 @@ EOF } func testAccStateMachineConfig_tags1(rName, tag1Key, tag1Value string) string { - return acctest.ConfigCompose(testAccStateMachineBaseConfig(rName), fmt.Sprintf(` + return acctest.ConfigCompose(testAccStateMachineConfig_base(rName), fmt.Sprintf(` resource "aws_sfn_state_machine" "test" { name = %[1]q role_arn = aws_iam_role.for_sfn.arn @@ -523,7 +523,7 @@ EOF } func testAccStateMachineConfig_tags2(rName, tag1Key, tag1Value, tag2Key, tag2Value string) string { - return acctest.ConfigCompose(testAccStateMachineBaseConfig(rName), fmt.Sprintf(` + return acctest.ConfigCompose(testAccStateMachineConfig_base(rName), fmt.Sprintf(` resource "aws_sfn_state_machine" "test" { name = %[1]q role_arn = aws_iam_role.for_sfn.arn @@ -561,7 +561,7 @@ EOF } func testAccStateMachineConfig_typed(rName, rType string, rMaxAttempts int) string { - return acctest.ConfigCompose(testAccStateMachineBaseConfig(rName), fmt.Sprintf(` + return acctest.ConfigCompose(testAccStateMachineConfig_base(rName), fmt.Sprintf(` resource "aws_sfn_state_machine" "test" { name = %[1]q role_arn = aws_iam_role.for_sfn.arn @@ -593,7 +593,7 @@ EOF } func testAccStateMachineConfig_expressLogConfiguration(rName string, rLevel string) string { - return acctest.ConfigCompose(testAccStateMachineBaseConfig(rName), fmt.Sprintf(` + return acctest.ConfigCompose(testAccStateMachineConfig_base(rName), fmt.Sprintf(` resource "aws_cloudwatch_log_group" "test" { name = %[1]q } @@ -637,7 +637,7 @@ EOF } func testAccStateMachineConfig_tracingEnable(rName string) string { - return acctest.ConfigCompose(testAccStateMachineBaseConfig(rName), fmt.Sprintf(` + return acctest.ConfigCompose(testAccStateMachineConfig_base(rName), fmt.Sprintf(` resource "aws_sfn_state_machine" "test" { name = %[1]q role_arn = aws_iam_role.for_sfn.arn @@ -674,7 +674,7 @@ EOF } func testAccStateMachineConfig_tracingDisable(rName string) string { - return acctest.ConfigCompose(testAccStateMachineBaseConfig(rName), fmt.Sprintf(` + return acctest.ConfigCompose(testAccStateMachineConfig_base(rName), fmt.Sprintf(` resource "aws_sfn_state_machine" "test" { name = %[1]q role_arn = aws_iam_role.for_sfn.arn diff --git a/internal/service/sfn/status.go b/internal/service/sfn/status.go deleted file mode 100644 index be16920c435..00000000000 --- a/internal/service/sfn/status.go +++ /dev/null @@ -1,24 +0,0 @@ -package sfn - -import ( - "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/service/sfn" - "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" - "github.com/hashicorp/terraform-provider-aws/internal/tfresource" -) - -func statusStateMachine(conn *sfn.SFN, stateMachineArn string) resource.StateRefreshFunc { - return func() (interface{}, string, error) { - output, err := FindStateMachineByARN(conn, stateMachineArn) - - if tfresource.NotFound(err) { - return nil, "", nil - } - - if err != nil { - return nil, "", err - } - - return output, aws.StringValue(output.Status), nil - } -} diff --git a/internal/service/sfn/wait.go b/internal/service/sfn/wait.go deleted file mode 100644 index df407238cd8..00000000000 --- a/internal/service/sfn/wait.go +++ /dev/null @@ -1,31 +0,0 @@ -package sfn - -import ( - "time" - - "github.com/aws/aws-sdk-go/service/sfn" - "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" -) - -const ( - stateMachineCreatedTimeout = 5 * time.Minute - stateMachineDeletedTimeout = 5 * time.Minute - stateMachineUpdatedTimeout = 1 * time.Minute -) - -func waitStateMachineDeleted(conn *sfn.SFN, stateMachineArn string) (*sfn.DescribeStateMachineOutput, error) { - stateConf := &resource.StateChangeConf{ - Pending: []string{sfn.StateMachineStatusActive, sfn.StateMachineStatusDeleting}, - Target: []string{}, - Refresh: statusStateMachine(conn, stateMachineArn), - Timeout: stateMachineDeletedTimeout, - } - - outputRaw, err := stateConf.WaitForState() - - if output, ok := outputRaw.(*sfn.DescribeStateMachineOutput); ok { - return output, err - } - - return nil, err -} From 83ed520f811ffce01720f04ce3da75e04d79db02 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 1 Nov 2022 09:06:46 -0400 Subject: [PATCH 07/13] r/aws_sfn_state_machine: Switch to 'WithoutTimeout' CRUD handlers (#15090). Acceptance test output: % make testacc TESTARGS='-run=TestAccSFNStateMachine_' PKG=sfn ACCTEST_PARALLELISM=3 ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./internal/service/sfn/... -v -count 1 -parallel 3 -run=TestAccSFNStateMachine_ -timeout 180m === RUN TestAccSFNStateMachine_createUpdate === PAUSE TestAccSFNStateMachine_createUpdate === RUN TestAccSFNStateMachine_expressUpdate === PAUSE TestAccSFNStateMachine_expressUpdate === RUN TestAccSFNStateMachine_standardUpdate === PAUSE TestAccSFNStateMachine_standardUpdate === RUN TestAccSFNStateMachine_tags === PAUSE TestAccSFNStateMachine_tags === RUN TestAccSFNStateMachine_tracing === PAUSE TestAccSFNStateMachine_tracing === RUN TestAccSFNStateMachine_disappears === PAUSE TestAccSFNStateMachine_disappears === RUN TestAccSFNStateMachine_expressLogging === PAUSE TestAccSFNStateMachine_expressLogging === CONT TestAccSFNStateMachine_createUpdate === CONT TestAccSFNStateMachine_tracing === CONT TestAccSFNStateMachine_expressLogging --- PASS: TestAccSFNStateMachine_createUpdate (102.69s) === CONT TestAccSFNStateMachine_disappears --- PASS: TestAccSFNStateMachine_tracing (107.93s) === CONT TestAccSFNStateMachine_standardUpdate --- PASS: TestAccSFNStateMachine_expressLogging (109.37s) === CONT TestAccSFNStateMachine_tags --- PASS: TestAccSFNStateMachine_standardUpdate (71.73s) === CONT TestAccSFNStateMachine_expressUpdate --- PASS: TestAccSFNStateMachine_disappears (84.48s) --- PASS: TestAccSFNStateMachine_expressUpdate (62.61s) --- PASS: TestAccSFNStateMachine_tags (135.87s) PASS ok github.com/hashicorp/terraform-provider-aws/internal/service/sfn 249.652s --- internal/service/sfn/state_machine.go | 80 +++++++++++----------- internal/service/sfn/state_machine_test.go | 5 +- 2 files changed, 44 insertions(+), 41 deletions(-) diff --git a/internal/service/sfn/state_machine.go b/internal/service/sfn/state_machine.go index 2fe3d79fb69..d0c6e817c69 100644 --- a/internal/service/sfn/state_machine.go +++ b/internal/service/sfn/state_machine.go @@ -1,6 +1,7 @@ package sfn import ( + "context" "fmt" "log" "time" @@ -8,6 +9,7 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/sfn" "github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr" + "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" @@ -19,10 +21,10 @@ import ( func ResourceStateMachine() *schema.Resource { return &schema.Resource{ - Create: resourceStateMachineCreate, - Read: resourceStateMachineRead, - Update: resourceStateMachineUpdate, - Delete: resourceStateMachineDelete, + CreateWithoutTimeout: resourceStateMachineCreate, + ReadWithoutTimeout: resourceStateMachineRead, + UpdateWithoutTimeout: resourceStateMachineUpdate, + DeleteWithoutTimeout: resourceStateMachineDelete, Importer: &schema.ResourceImporter{ State: schema.ImportStatePassthrough, @@ -111,7 +113,7 @@ func ResourceStateMachine() *schema.Resource { } } -func resourceStateMachineCreate(d *schema.ResourceData, meta interface{}) error { +func resourceStateMachineCreate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { conn := meta.(*conns.AWSClient).SFNConn defaultTagsConfig := meta.(*conns.AWSClient).DefaultTagsConfig tags := defaultTagsConfig.MergeTags(tftags.New(d.Get("tags").(map[string]interface{}))) @@ -137,25 +139,25 @@ func resourceStateMachineCreate(d *schema.ResourceData, meta interface{}) error // 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). - outputRaw, err := tfresource.RetryWhenAWSErrCodeEquals(stateMachineCreatedTimeout, func() (interface{}, error) { - return conn.CreateStateMachine(input) + outputRaw, err := tfresource.RetryWhenAWSErrCodeEqualsContext(ctx, stateMachineCreatedTimeout, func() (interface{}, error) { + return conn.CreateStateMachineWithContext(ctx, input) }, sfn.ErrCodeStateMachineDeleting, "AccessDeniedException") if err != nil { - return fmt.Errorf("creating Step Functions State Machine (%s): %w", name, err) + return diag.Errorf("creating Step Functions State Machine (%s): %s", name, err) } d.SetId(aws.StringValue(outputRaw.(*sfn.CreateStateMachineOutput).StateMachineArn)) - return resourceStateMachineRead(d, meta) + return resourceStateMachineRead(ctx, d, meta) } -func resourceStateMachineRead(d *schema.ResourceData, meta interface{}) error { +func resourceStateMachineRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { conn := meta.(*conns.AWSClient).SFNConn defaultTagsConfig := meta.(*conns.AWSClient).DefaultTagsConfig ignoreTagsConfig := meta.(*conns.AWSClient).IgnoreTagsConfig - output, err := FindStateMachineByARN(conn, d.Id()) + output, err := FindStateMachineByARN(ctx, conn, d.Id()) if !d.IsNewResource() && tfresource.NotFound(err) { log.Printf("[WARN] Step Functions State Machine (%s) not found, removing from state", d.Id()) @@ -164,7 +166,7 @@ func resourceStateMachineRead(d *schema.ResourceData, meta interface{}) error { } if err != nil { - return fmt.Errorf("error reading Step Functions State Machine (%s): %w", d.Id(), err) + return diag.Errorf("error reading Step Functions State Machine (%s): %s", d.Id(), err) } d.Set("arn", output.StateMachineArn) @@ -176,7 +178,7 @@ func resourceStateMachineRead(d *schema.ResourceData, meta interface{}) error { d.Set("definition", output.Definition) if output.LoggingConfiguration != nil { if err := d.Set("logging_configuration", []interface{}{flattenLoggingConfiguration(output.LoggingConfiguration)}); err != nil { - return fmt.Errorf("error setting logging_configuration: %w", err) + return diag.Errorf("error setting logging_configuration: %s", err) } } else { d.Set("logging_configuration", nil) @@ -186,38 +188,38 @@ func resourceStateMachineRead(d *schema.ResourceData, meta interface{}) error { d.Set("status", output.Status) if output.TracingConfiguration != nil { if err := d.Set("tracing_configuration", []interface{}{flattenTracingConfiguration(output.TracingConfiguration)}); err != nil { - return fmt.Errorf("error setting tracing_configuration: %w", err) + return diag.Errorf("error setting tracing_configuration: %s", err) } } else { d.Set("tracing_configuration", nil) } d.Set("type", output.Type) - tags, err := ListTags(conn, d.Id()) + tags, err := ListTagsWithContext(ctx, conn, d.Id()) if tfawserr.ErrCodeEquals(err, "UnknownOperationException") { return nil } if err != nil { - return fmt.Errorf("listing tags for Step Functions State Machine (%s): %w", d.Id(), err) + return diag.Errorf("listing tags for Step Functions State Machine (%s): %s", d.Id(), err) } tags = tags.IgnoreAWS().IgnoreConfig(ignoreTagsConfig) //lintignore:AWSR002 if err := d.Set("tags", tags.RemoveDefaultConfig(defaultTagsConfig).Map()); err != nil { - return fmt.Errorf("setting tags: %w", err) + return diag.Errorf("setting tags: %s", err) } if err := d.Set("tags_all", tags.Map()); err != nil { - return fmt.Errorf("setting tags_all: %w", err) + return diag.Errorf("setting tags_all: %s", err) } return nil } -func resourceStateMachineUpdate(d *schema.ResourceData, meta interface{}) error { +func resourceStateMachineUpdate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { conn := meta.(*conns.AWSClient).SFNConn if d.HasChangesExcept("tags", "tags_all") { @@ -240,15 +242,15 @@ func resourceStateMachineUpdate(d *schema.ResourceData, meta interface{}) error } } - _, err := conn.UpdateStateMachine(input) + _, err := conn.UpdateStateMachineWithContext(ctx, input) if err != nil { - return fmt.Errorf("updating Step Functions State Machine (%s): %w", d.Id(), err) + return diag.Errorf("updating Step Functions State Machine (%s): %s", d.Id(), err) } // Handle eventual consistency after update. - err = resource.Retry(stateMachineUpdatedTimeout, func() *resource.RetryError { - output, err := FindStateMachineByARN(conn, d.Id()) + err = resource.RetryContext(ctx, stateMachineUpdatedTimeout, func() *resource.RetryError { + output, err := FindStateMachineByARN(ctx, conn, d.Id()) if err != nil { return resource.NonRetryableError(err) @@ -266,46 +268,46 @@ func resourceStateMachineUpdate(d *schema.ResourceData, meta interface{}) error }) if err != nil { - return fmt.Errorf("waiting for Step Functions State Machine (%s) update: %w", d.Id(), err) + return diag.Errorf("waiting for Step Functions State Machine (%s) update: %s", d.Id(), err) } } if d.HasChange("tags_all") { o, n := d.GetChange("tags_all") - if err := UpdateTags(conn, d.Id(), o, n); err != nil { - return fmt.Errorf("updating Step Functions State Machine (%s) tags: %w", d.Id(), err) + if err := UpdateTagsWithContext(ctx, conn, d.Id(), o, n); err != nil { + return diag.Errorf("updating Step Functions State Machine (%s) tags: %s", d.Id(), err) } } - return resourceStateMachineRead(d, meta) + return resourceStateMachineRead(ctx, d, meta) } -func resourceStateMachineDelete(d *schema.ResourceData, meta interface{}) error { +func resourceStateMachineDelete(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { conn := meta.(*conns.AWSClient).SFNConn log.Printf("[DEBUG] Deleting Step Functions State Machine: %s", d.Id()) - _, err := conn.DeleteStateMachine(&sfn.DeleteStateMachineInput{ + _, err := conn.DeleteStateMachineWithContext(ctx, &sfn.DeleteStateMachineInput{ StateMachineArn: aws.String(d.Id()), }) if err != nil { - return fmt.Errorf("deleting Step Functions State Machine (%s): %w", d.Id(), err) + return diag.Errorf("deleting Step Functions State Machine (%s): %s", d.Id(), err) } - if _, err := waitStateMachineDeleted(conn, d.Id()); err != nil { - return fmt.Errorf("waiting for Step Functions State Machine (%s) delete: %w", d.Id(), err) + if _, err := waitStateMachineDeleted(ctx, conn, d.Id()); err != nil { + return diag.Errorf("waiting for Step Functions State Machine (%s) delete: %s", d.Id(), err) } return nil } -func FindStateMachineByARN(conn *sfn.SFN, arn string) (*sfn.DescribeStateMachineOutput, error) { +func FindStateMachineByARN(ctx context.Context, conn *sfn.SFN, arn string) (*sfn.DescribeStateMachineOutput, error) { input := &sfn.DescribeStateMachineInput{ StateMachineArn: aws.String(arn), } - output, err := conn.DescribeStateMachine(input) + output, err := conn.DescribeStateMachineWithContext(ctx, input) if tfawserr.ErrCodeEquals(err, sfn.ErrCodeStateMachineDoesNotExist) { return nil, &resource.NotFoundError{ @@ -325,9 +327,9 @@ func FindStateMachineByARN(conn *sfn.SFN, arn string) (*sfn.DescribeStateMachine return output, nil } -func statusStateMachine(conn *sfn.SFN, stateMachineArn string) resource.StateRefreshFunc { +func statusStateMachine(ctx context.Context, conn *sfn.SFN, stateMachineArn string) resource.StateRefreshFunc { return func() (interface{}, string, error) { - output, err := FindStateMachineByARN(conn, stateMachineArn) + output, err := FindStateMachineByARN(ctx, conn, stateMachineArn) if tfresource.NotFound(err) { return nil, "", nil @@ -347,15 +349,15 @@ const ( stateMachineUpdatedTimeout = 1 * time.Minute ) -func waitStateMachineDeleted(conn *sfn.SFN, stateMachineArn string) (*sfn.DescribeStateMachineOutput, error) { +func waitStateMachineDeleted(ctx context.Context, conn *sfn.SFN, stateMachineArn string) (*sfn.DescribeStateMachineOutput, error) { stateConf := &resource.StateChangeConf{ Pending: []string{sfn.StateMachineStatusActive, sfn.StateMachineStatusDeleting}, Target: []string{}, - Refresh: statusStateMachine(conn, stateMachineArn), + Refresh: statusStateMachine(ctx, conn, stateMachineArn), Timeout: stateMachineDeletedTimeout, } - outputRaw, err := stateConf.WaitForState() + outputRaw, err := stateConf.WaitForStateContext(ctx) if output, ok := outputRaw.(*sfn.DescribeStateMachineOutput); ok { return output, err diff --git a/internal/service/sfn/state_machine_test.go b/internal/service/sfn/state_machine_test.go index d2bad8ac954..d04dc62ce37 100644 --- a/internal/service/sfn/state_machine_test.go +++ b/internal/service/sfn/state_machine_test.go @@ -1,6 +1,7 @@ package sfn_test import ( + "context" "fmt" "regexp" "testing" @@ -316,7 +317,7 @@ func testAccCheckExists(n string, v *sfn.DescribeStateMachineOutput) resource.Te conn := acctest.Provider.Meta().(*conns.AWSClient).SFNConn - output, err := tfsfn.FindStateMachineByARN(conn, rs.Primary.ID) + output, err := tfsfn.FindStateMachineByARN(context.Background(), conn, rs.Primary.ID) if err != nil { return err @@ -336,7 +337,7 @@ func testAccCheckStateMachineDestroy(s *terraform.State) error { continue } - _, err := tfsfn.FindStateMachineByARN(conn, rs.Primary.ID) + _, err := tfsfn.FindStateMachineByARN(context.Background(), conn, rs.Primary.ID) if tfresource.NotFound(err) { continue From 94fef809b12c37655737fe695ca08cab43685649 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 1 Nov 2022 09:14:45 -0400 Subject: [PATCH 08/13] r/aws_sfn_activity: Add 'TestAccSFNActivity_disappears'. Acceptance test output: % make testacc TESTARGS='-run=TestAccSFNActivity_disappears' PKG=sfn ACCTEST_PARALLELISM=3 ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./internal/service/sfn/... -v -count 1 -parallel 3 -run=TestAccSFNActivity_disappears -timeout 180m === RUN TestAccSFNActivity_disappears === PAUSE TestAccSFNActivity_disappears === CONT TestAccSFNActivity_disappears --- PASS: TestAccSFNActivity_disappears (23.30s) PASS ok github.com/hashicorp/terraform-provider-aws/internal/service/sfn 37.425s --- internal/service/sfn/activity_test.go | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/internal/service/sfn/activity_test.go b/internal/service/sfn/activity_test.go index e44f76b355b..c3b15c3c60b 100644 --- a/internal/service/sfn/activity_test.go +++ b/internal/service/sfn/activity_test.go @@ -44,6 +44,28 @@ func TestAccSFNActivity_basic(t *testing.T) { }) } +func TestAccSFNActivity_disappears(t *testing.T) { + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_sfn_activity.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t) }, + ErrorCheck: acctest.ErrorCheck(t, sfn.EndpointsID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckActivityDestroy, + Steps: []resource.TestStep{ + { + Config: testAccActivityConfig_basic(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckActivityExists(resourceName), + acctest.CheckResourceDisappears(acctest.Provider, tfsfn.ResourceActivity(), resourceName), + ), + ExpectNonEmptyPlan: true, + }, + }, + }) +} + func TestAccSFNActivity_tags(t *testing.T) { rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resourceName := "aws_sfn_activity.test" From 3df305f9a0e64ca27f2e4ccaa566e1a679e2ed4c Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 1 Nov 2022 09:27:54 -0400 Subject: [PATCH 09/13] d/aws_sfn_state_machine: Switch to 'WithoutTimeout' CRUD handlers (#15090). Acceptance test output: % make testacc TESTARGS='-run=TestAccSFNStateMachineDataSource_' PKG=sfn ACCTEST_PARALLELISM=3 ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./internal/service/sfn/... -v -count 1 -parallel 3 -run=TestAccSFNStateMachineDataSource_ -timeout 180m === RUN TestAccSFNStateMachineDataSource_basic === PAUSE TestAccSFNStateMachineDataSource_basic === CONT TestAccSFNStateMachineDataSource_basic --- PASS: TestAccSFNStateMachineDataSource_basic (67.16s) PASS ok github.com/hashicorp/terraform-provider-aws/internal/service/sfn 71.512s --- internal/service/sfn/state_machine.go | 6 +- .../service/sfn/state_machine_data_source.go | 78 +++++++++---------- .../sfn/state_machine_data_source_test.go | 46 +---------- 3 files changed, 46 insertions(+), 84 deletions(-) diff --git a/internal/service/sfn/state_machine.go b/internal/service/sfn/state_machine.go index d0c6e817c69..3309710ddc6 100644 --- a/internal/service/sfn/state_machine.go +++ b/internal/service/sfn/state_machine.go @@ -166,7 +166,7 @@ func resourceStateMachineRead(ctx context.Context, d *schema.ResourceData, meta } if err != nil { - return diag.Errorf("error reading Step Functions State Machine (%s): %s", d.Id(), err) + return diag.Errorf("reading Step Functions State Machine (%s): %s", d.Id(), err) } d.Set("arn", output.StateMachineArn) @@ -178,7 +178,7 @@ func resourceStateMachineRead(ctx context.Context, d *schema.ResourceData, meta d.Set("definition", output.Definition) if output.LoggingConfiguration != nil { if err := d.Set("logging_configuration", []interface{}{flattenLoggingConfiguration(output.LoggingConfiguration)}); err != nil { - return diag.Errorf("error setting logging_configuration: %s", err) + return diag.Errorf("setting logging_configuration: %s", err) } } else { d.Set("logging_configuration", nil) @@ -188,7 +188,7 @@ func resourceStateMachineRead(ctx context.Context, d *schema.ResourceData, meta d.Set("status", output.Status) if output.TracingConfiguration != nil { if err := d.Set("tracing_configuration", []interface{}{flattenTracingConfiguration(output.TracingConfiguration)}); err != nil { - return diag.Errorf("error setting tracing_configuration: %s", err) + return diag.Errorf("setting tracing_configuration: %s", err) } } else { d.Set("tracing_configuration", nil) diff --git a/internal/service/sfn/state_machine_data_source.go b/internal/service/sfn/state_machine_data_source.go index e461ef06003..7597cf77ee6 100644 --- a/internal/service/sfn/state_machine_data_source.go +++ b/internal/service/sfn/state_machine_data_source.go @@ -1,29 +1,26 @@ package sfn import ( - "fmt" - "log" + "context" "time" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/sfn" + "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-provider-aws/internal/conns" ) func DataSourceStateMachine() *schema.Resource { return &schema.Resource{ - Read: dataSourceStateMachineRead, + ReadWithoutTimeout: dataSourceStateMachineRead, + Schema: map[string]*schema.Schema{ - "name": { - Type: schema.TypeString, - Required: true, - }, "arn": { Type: schema.TypeString, Computed: true, }, - "role_arn": { + "creation_date": { Type: schema.TypeString, Computed: true, }, @@ -31,11 +28,15 @@ func DataSourceStateMachine() *schema.Resource { Type: schema.TypeString, Computed: true, }, - "status": { + "name": { + Type: schema.TypeString, + Required: true, + }, + "role_arn": { Type: schema.TypeString, Computed: true, }, - "creation_date": { + "status": { Type: schema.TypeString, Computed: true, }, @@ -43,51 +44,50 @@ func DataSourceStateMachine() *schema.Resource { } } -func dataSourceStateMachineRead(d *schema.ResourceData, meta interface{}) error { +func dataSourceStateMachineRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { conn := meta.(*conns.AWSClient).SFNConn - params := &sfn.ListStateMachinesInput{} - log.Printf("[DEBUG] Reading Step Function State Machine: %s", d.Id()) - target := d.Get("name") + name := d.Get("name").(string) var arns []string - err := conn.ListStateMachinesPages(params, func(page *sfn.ListStateMachinesOutput, lastPage bool) bool { - for _, sm := range page.StateMachines { - if aws.StringValue(sm.Name) == target { - arns = append(arns, aws.StringValue(sm.StateMachineArn)) + err := conn.ListStateMachinesPages(&sfn.ListStateMachinesInput{}, func(page *sfn.ListStateMachinesOutput, lastPage bool) bool { + if page == nil { + return !lastPage + } + + for _, v := range page.StateMachines { + if aws.StringValue(v.Name) == name { + arns = append(arns, aws.StringValue(v.StateMachineArn)) } } - return true + + return !lastPage }) if err != nil { - return fmt.Errorf("Error listing state machines: %w", err) + return diag.Errorf("listing Step Functions State Machines: %s", err) } - if len(arns) == 0 { - return fmt.Errorf("No state machine with name %q found in this region.", target) - } - if len(arns) > 1 { - return fmt.Errorf("Multiple state machines with name %q found in this region.", target) + if n := len(arns); n == 0 { + return diag.Errorf("no Step Functions State Machines matched") + } else if n > 1 { + return diag.Errorf("%d Step Functions State Machines matched; use additional constraints to reduce matches to a single State Machine", n) } - sm, err := conn.DescribeStateMachine(&sfn.DescribeStateMachineInput{ - StateMachineArn: aws.String(arns[0]), - }) - if err != nil { - return fmt.Errorf("error describing SFN State Machine (%s): %w", arns[0], err) - } + arn := arns[0] + output, err := FindStateMachineByARN(ctx, conn, arn) - d.Set("definition", sm.Definition) - d.Set("name", sm.Name) - d.Set("arn", sm.StateMachineArn) - d.Set("role_arn", sm.RoleArn) - d.Set("status", sm.Status) - if err := d.Set("creation_date", sm.CreationDate.Format(time.RFC3339)); err != nil { - log.Printf("[DEBUG] Error setting creation_date: %s", err) + if err != nil { + return diag.Errorf("reading Step Functions State Machine (%s): %s", arn, err) } - d.SetId(arns[0]) + d.SetId(arn) + d.Set("arn", output.StateMachineArn) + d.Set("creation_date", output.CreationDate.Format(time.RFC3339)) + d.Set("definition", output.Definition) + d.Set("name", output.Name) + d.Set("role_arn", output.RoleArn) + d.Set("status", output.Status) return nil } diff --git a/internal/service/sfn/state_machine_data_source_test.go b/internal/service/sfn/state_machine_data_source_test.go index e08cdd612f5..a7e00376598 100644 --- a/internal/service/sfn/state_machine_data_source_test.go +++ b/internal/service/sfn/state_machine_data_source_test.go @@ -1,7 +1,6 @@ package sfn_test import ( - "fmt" "testing" "github.com/aws/aws-sdk-go/service/sfn" @@ -11,7 +10,7 @@ import ( ) func TestAccSFNStateMachineDataSource_basic(t *testing.T) { - rName := sdkacctest.RandString(5) + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) dataSourceName := "data.aws_sfn_state_machine.test" resourceName := "aws_sfn_state_machine.test" @@ -22,7 +21,7 @@ func TestAccSFNStateMachineDataSource_basic(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccStateMachineDataSourceConfig_basic(rName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( resource.TestCheckResourceAttrPair(resourceName, "id", dataSourceName, "arn"), resource.TestCheckResourceAttrPair(resourceName, "creation_date", dataSourceName, "creation_date"), resource.TestCheckResourceAttrPair(resourceName, "definition", dataSourceName, "definition"), @@ -36,46 +35,9 @@ func TestAccSFNStateMachineDataSource_basic(t *testing.T) { } func testAccStateMachineDataSourceConfig_basic(rName string) string { - return fmt.Sprintf(` -data "aws_region" "current" {} - -resource "aws_iam_role" "iam_for_sfn" { - name = "iam_for_sfn_%s" - - assume_role_policy = < Date: Tue, 1 Nov 2022 09:39:55 -0400 Subject: [PATCH 10/13] SFN: Add sweepers. Acceptance test output: % make sweep SWEEPARGS=-sweep-run=aws_sfn_activity,aws_sfn_state_machine WARNING: This will destroy infrastructure. Use only in development accounts. go test ./internal/sweep -v -tags=sweep -sweep=us-west-2,us-east-1,us-east-2 -sweep-run=aws_sfn_activity,aws_sfn_state_machine -timeout 60m 2022/11/01 09:38:06 [DEBUG] Running Sweepers for region (us-west-2): 2022/11/01 09:38:06 [DEBUG] Running Sweeper (aws_sfn_activity) in region (us-west-2) 2022/11/01 09:38:06 [INFO] Retrieved credentials from "EnvConfigCredentials" 2022/11/01 09:38:06 [DEBUG] Trying to get account information via sts:GetCallerIdentity 2022/11/01 09:38:06 [DEBUG] Trying to get account information via sts:GetCallerIdentity 2022/11/01 09:38:07 [DEBUG] Waiting for state to become: [success] 2022/11/01 09:38:07 [DEBUG] Waiting for state to become: [success] 2022/11/01 09:38:07 [DEBUG] Deleting Step Functions Activity: arn:aws:states:us-west-2:123456789012:activity:8insfll7bk 2022/11/01 09:38:07 [DEBUG] Waiting for state to become: [success] 2022/11/01 09:38:07 [DEBUG] Deleting Step Functions Activity: arn:aws:states:us-west-2:123456789012:activity:tqkwct7ni8 2022/11/01 09:38:07 [DEBUG] Deleting Step Functions Activity: arn:aws:states:us-west-2:123456789012:activity:z8emyrl6 2022/11/01 09:38:08 [DEBUG] Completed Sweeper (aws_sfn_activity) in region (us-west-2) in 2.039239611s 2022/11/01 09:38:08 [DEBUG] Running Sweeper (aws_sfn_state_machine) in region (us-west-2) 2022/11/01 09:38:08 [DEBUG] Waiting for state to become: [success] 2022/11/01 09:38:08 [DEBUG] Deleting Step Functions State Machine: arn:aws:states:us-west-2:123456789012:stateMachine:tf-acc-test-2233972156310038186 2022/11/01 09:38:08 [DEBUG] Waiting for state to become: [] 2022/11/01 09:38:08 [TRACE] Waiting 200ms before next try 2022/11/01 09:38:09 [TRACE] Waiting 400ms before next try 2022/11/01 09:38:09 [TRACE] Waiting 800ms before next try 2022/11/01 09:38:10 [TRACE] Waiting 1.6s before next try 2022/11/01 09:38:12 [TRACE] Waiting 3.2s before next try 2022/11/01 09:38:15 [TRACE] Waiting 6.4s before next try 2022/11/01 09:38:22 [TRACE] Waiting 10s before next try 2022/11/01 09:38:32 [TRACE] Waiting 10s before next try 2022/11/01 09:38:42 [DEBUG] Completed Sweeper (aws_sfn_state_machine) in region (us-west-2) in 33.905723812s 2022/11/01 09:38:42 Completed Sweepers for region (us-west-2) in 35.945206544s 2022/11/01 09:38:42 Sweeper Tests for region (us-west-2) ran successfully: - aws_sfn_activity - aws_sfn_state_machine 2022/11/01 09:38:42 [DEBUG] Running Sweepers for region (us-east-1): 2022/11/01 09:38:42 [DEBUG] Running Sweeper (aws_sfn_activity) in region (us-east-1) 2022/11/01 09:38:42 [INFO] Retrieved credentials from "EnvConfigCredentials" 2022/11/01 09:38:42 [DEBUG] Trying to get account information via sts:GetCallerIdentity 2022/11/01 09:38:42 [DEBUG] Trying to get account information via sts:GetCallerIdentity 2022/11/01 09:38:43 [DEBUG] Completed Sweeper (aws_sfn_activity) in region (us-east-1) in 781.40589ms 2022/11/01 09:38:43 [DEBUG] Running Sweeper (aws_sfn_state_machine) in region (us-east-1) 2022/11/01 09:38:43 [DEBUG] Completed Sweeper (aws_sfn_state_machine) in region (us-east-1) in 40.949297ms 2022/11/01 09:38:43 Completed Sweepers for region (us-east-1) in 822.40173ms 2022/11/01 09:38:43 Sweeper Tests for region (us-east-1) ran successfully: - aws_sfn_state_machine - aws_sfn_activity 2022/11/01 09:38:43 [DEBUG] Running Sweepers for region (us-east-2): 2022/11/01 09:38:43 [DEBUG] Running Sweeper (aws_sfn_activity) in region (us-east-2) 2022/11/01 09:38:43 [INFO] Retrieved credentials from "EnvConfigCredentials" 2022/11/01 09:38:43 [DEBUG] Trying to get account information via sts:GetCallerIdentity 2022/11/01 09:38:43 [DEBUG] Trying to get account information via sts:GetCallerIdentity 2022/11/01 09:38:43 [DEBUG] Completed Sweeper (aws_sfn_activity) in region (us-east-2) in 664.725939ms 2022/11/01 09:38:43 [DEBUG] Running Sweeper (aws_sfn_state_machine) in region (us-east-2) 2022/11/01 09:38:43 [DEBUG] Completed Sweeper (aws_sfn_state_machine) in region (us-east-2) in 51.677755ms 2022/11/01 09:38:43 Completed Sweepers for region (us-east-2) in 716.453528ms 2022/11/01 09:38:43 Sweeper Tests for region (us-east-2) ran successfully: - aws_sfn_activity - aws_sfn_state_machine ok github.com/hashicorp/terraform-provider-aws/internal/sweep 42.533s --- internal/service/sfn/sweep.go | 113 ++++++++++++++++++++++++++++++++++ internal/sweep/sweep_test.go | 1 + 2 files changed, 114 insertions(+) create mode 100644 internal/service/sfn/sweep.go diff --git a/internal/service/sfn/sweep.go b/internal/service/sfn/sweep.go new file mode 100644 index 00000000000..5eaf8e148bd --- /dev/null +++ b/internal/service/sfn/sweep.go @@ -0,0 +1,113 @@ +//go:build sweep +// +build sweep + +package sfn + +import ( + "fmt" + "log" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/sfn" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" + "github.com/hashicorp/terraform-provider-aws/internal/conns" + "github.com/hashicorp/terraform-provider-aws/internal/sweep" +) + +func init() { + resource.AddTestSweepers("aws_sfn_activity", &resource.Sweeper{ + Name: "aws_sfn_activity", + F: sweepActivities, + }) + + resource.AddTestSweepers("aws_sfn_state_machine", &resource.Sweeper{ + Name: "aws_sfn_state_machine", + F: sweepStateMachines, + }) +} + +func sweepActivities(region string) error { + client, err := sweep.SharedRegionalSweepClient(region) + if err != nil { + return fmt.Errorf("error getting client: %s", err) + } + conn := client.(*conns.AWSClient).SFNConn + input := &sfn.ListActivitiesInput{} + sweepResources := make([]sweep.Sweepable, 0) + + err = conn.ListActivitiesPages(input, func(page *sfn.ListActivitiesOutput, lastPage bool) bool { + if page == nil { + return !lastPage + } + + for _, v := range page.Activities { + r := ResourceActivity() + d := r.Data(nil) + d.SetId(aws.StringValue(v.ActivityArn)) + + sweepResources = append(sweepResources, sweep.NewSweepResource(r, d, client)) + } + + return !lastPage + }) + + if sweep.SkipSweepError(err) { + log.Printf("[WARN] Skipping Step Functions Activity sweep for %s: %s", region, err) + return nil + } + + if err != nil { + return fmt.Errorf("error listing Step Functions Activities (%s): %w", region, err) + } + + err = sweep.SweepOrchestrator(sweepResources) + + if err != nil { + return fmt.Errorf("error sweeping Step Functions Activities (%s): %w", region, err) + } + + return nil +} + +func sweepStateMachines(region string) error { + client, err := sweep.SharedRegionalSweepClient(region) + if err != nil { + return fmt.Errorf("error getting client: %s", err) + } + conn := client.(*conns.AWSClient).SFNConn + input := &sfn.ListStateMachinesInput{} + sweepResources := make([]sweep.Sweepable, 0) + + err = conn.ListStateMachinesPages(input, func(page *sfn.ListStateMachinesOutput, lastPage bool) bool { + if page == nil { + return !lastPage + } + + for _, v := range page.StateMachines { + r := ResourceStateMachine() + d := r.Data(nil) + d.SetId(aws.StringValue(v.StateMachineArn)) + + sweepResources = append(sweepResources, sweep.NewSweepResource(r, d, client)) + } + + return !lastPage + }) + + if sweep.SkipSweepError(err) { + log.Printf("[WARN] Skipping Step Functions State Machine sweep for %s: %s", region, err) + return nil + } + + if err != nil { + return fmt.Errorf("error listing Step Functions State Machines (%s): %w", region, err) + } + + err = sweep.SweepOrchestrator(sweepResources) + + if err != nil { + return fmt.Errorf("error sweeping Step Functions State Machines (%s): %w", region, err) + } + + return nil +} diff --git a/internal/sweep/sweep_test.go b/internal/sweep/sweep_test.go index 173e2df796f..2df023824f4 100644 --- a/internal/sweep/sweep_test.go +++ b/internal/sweep/sweep_test.go @@ -122,6 +122,7 @@ import ( _ "github.com/hashicorp/terraform-provider-aws/internal/service/servicecatalog" _ "github.com/hashicorp/terraform-provider-aws/internal/service/servicediscovery" _ "github.com/hashicorp/terraform-provider-aws/internal/service/ses" + _ "github.com/hashicorp/terraform-provider-aws/internal/service/sfn" _ "github.com/hashicorp/terraform-provider-aws/internal/service/simpledb" _ "github.com/hashicorp/terraform-provider-aws/internal/service/sns" _ "github.com/hashicorp/terraform-provider-aws/internal/service/sqs" From 8bb27d08e4a4108c882b8948abb7f6efa6b351fe Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 1 Nov 2022 11:25:55 -0400 Subject: [PATCH 11/13] r/aws_sfn_state_machine: Add 'name_prefix'. Acceptance test output: % make testacc TESTARGS='-run=TestAccSFNStateMachine_' PKG=sfn ACCTEST_PARALLELISM=3 ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./internal/service/sfn/... -v -count 1 -parallel 3 -run=TestAccSFNStateMachine_ -timeout 180m === RUN TestAccSFNStateMachine_createUpdate === PAUSE TestAccSFNStateMachine_createUpdate === RUN TestAccSFNStateMachine_expressUpdate === PAUSE TestAccSFNStateMachine_expressUpdate === RUN TestAccSFNStateMachine_standardUpdate === PAUSE TestAccSFNStateMachine_standardUpdate === RUN TestAccSFNStateMachine_nameGenerated === PAUSE TestAccSFNStateMachine_nameGenerated === RUN TestAccSFNStateMachine_namePrefix === PAUSE TestAccSFNStateMachine_namePrefix === RUN TestAccSFNStateMachine_tags === PAUSE TestAccSFNStateMachine_tags === RUN TestAccSFNStateMachine_tracing === PAUSE TestAccSFNStateMachine_tracing === RUN TestAccSFNStateMachine_disappears === PAUSE TestAccSFNStateMachine_disappears === RUN TestAccSFNStateMachine_expressLogging === PAUSE TestAccSFNStateMachine_expressLogging === CONT TestAccSFNStateMachine_createUpdate === CONT TestAccSFNStateMachine_tags === CONT TestAccSFNStateMachine_disappears --- PASS: TestAccSFNStateMachine_createUpdate (83.80s) === CONT TestAccSFNStateMachine_expressLogging --- PASS: TestAccSFNStateMachine_disappears (86.70s) === CONT TestAccSFNStateMachine_tracing --- PASS: TestAccSFNStateMachine_expressLogging (62.77s) === CONT TestAccSFNStateMachine_nameGenerated --- PASS: TestAccSFNStateMachine_tags (154.33s) === CONT TestAccSFNStateMachine_namePrefix --- PASS: TestAccSFNStateMachine_tracing (126.12s) === CONT TestAccSFNStateMachine_standardUpdate --- PASS: TestAccSFNStateMachine_nameGenerated (66.47s) === CONT TestAccSFNStateMachine_expressUpdate --- PASS: TestAccSFNStateMachine_namePrefix (64.44s) --- PASS: TestAccSFNStateMachine_expressUpdate (68.11s) --- PASS: TestAccSFNStateMachine_standardUpdate (68.53s) PASS ok github.com/hashicorp/terraform-provider-aws/internal/service/sfn 285.690s --- internal/service/sfn/state_machine.go | 29 ++++- internal/service/sfn/state_machine_test.go | 122 ++++++++++++++++++ internal/service/sfn/validate.go | 19 --- internal/service/sfn/validate_test.go | 51 -------- .../docs/r/sfn_state_machine.html.markdown | 3 +- 5 files changed, 148 insertions(+), 76 deletions(-) delete mode 100644 internal/service/sfn/validate.go delete mode 100644 internal/service/sfn/validate_test.go diff --git a/internal/service/sfn/state_machine.go b/internal/service/sfn/state_machine.go index 3309710ddc6..aff9d9b757c 100644 --- a/internal/service/sfn/state_machine.go +++ b/internal/service/sfn/state_machine.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "log" + "regexp" "time" "github.com/aws/aws-sdk-go/aws" @@ -14,6 +15,7 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" "github.com/hashicorp/terraform-provider-aws/internal/conns" + "github.com/hashicorp/terraform-provider-aws/internal/create" tftags "github.com/hashicorp/terraform-provider-aws/internal/tags" "github.com/hashicorp/terraform-provider-aws/internal/tfresource" "github.com/hashicorp/terraform-provider-aws/internal/verify" @@ -69,10 +71,26 @@ func ResourceStateMachine() *schema.Resource { DiffSuppressFunc: verify.SuppressMissingOptionalConfigurationBlock, }, "name": { - Type: schema.TypeString, - Required: true, - ForceNew: true, - ValidateFunc: validStateMachineName, + Type: schema.TypeString, + Optional: true, + Computed: true, + ForceNew: true, + ConflictsWith: []string{"name_prefix"}, + ValidateFunc: validation.All( + validation.StringLenBetween(1, 80), + validation.StringMatch(regexp.MustCompile(`^[a-zA-Z0-9-_]+$`), "the name should only contain 0-9, A-Z, a-z, - and _"), + ), + }, + "name_prefix": { + Type: schema.TypeString, + Optional: true, + Computed: true, + ForceNew: true, + ConflictsWith: []string{"name"}, + ValidateFunc: validation.All( + validation.StringLenBetween(1, 80-resource.UniqueIDSuffixLength), + validation.StringMatch(regexp.MustCompile(`^[a-zA-Z0-9-_]+$`), "the name should only contain 0-9, A-Z, a-z, - and _"), + ), }, "role_arn": { Type: schema.TypeString, @@ -118,7 +136,7 @@ func resourceStateMachineCreate(ctx context.Context, d *schema.ResourceData, met defaultTagsConfig := meta.(*conns.AWSClient).DefaultTagsConfig tags := defaultTagsConfig.MergeTags(tftags.New(d.Get("tags").(map[string]interface{}))) - name := d.Get("name").(string) + name := create.Name(d.Get("name").(string), d.Get("name_prefix").(string)) input := &sfn.CreateStateMachineInput{ Definition: aws.String(d.Get("definition").(string)), Name: aws.String(name), @@ -184,6 +202,7 @@ func resourceStateMachineRead(ctx context.Context, d *schema.ResourceData, meta d.Set("logging_configuration", nil) } d.Set("name", output.Name) + d.Set("name_prefix", create.NamePrefixFromName(aws.StringValue(output.Name))) d.Set("role_arn", output.RoleArn) d.Set("status", output.Status) if output.TracingConfiguration != nil { diff --git a/internal/service/sfn/state_machine_test.go b/internal/service/sfn/state_machine_test.go index d04dc62ce37..cc297e9238e 100644 --- a/internal/service/sfn/state_machine_test.go +++ b/internal/service/sfn/state_machine_test.go @@ -35,6 +35,7 @@ func TestAccSFNStateMachine_createUpdate(t *testing.T) { acctest.CheckResourceAttrRegionalARN(resourceName, "arn", "states", fmt.Sprintf("stateMachine:%s", rName)), resource.TestCheckResourceAttr(resourceName, "status", sfn.StateMachineStatusActive), resource.TestCheckResourceAttr(resourceName, "name", rName), + resource.TestCheckResourceAttr(resourceName, "name_prefix", ""), resource.TestCheckResourceAttrSet(resourceName, "creation_date"), resource.TestCheckResourceAttrSet(resourceName, "definition"), resource.TestMatchResourceAttr(resourceName, "definition", regexp.MustCompile(`.*\"MaxAttempts\": 5.*`)), @@ -158,6 +159,62 @@ func TestAccSFNStateMachine_standardUpdate(t *testing.T) { }) } +func TestAccSFNStateMachine_nameGenerated(t *testing.T) { + var sm sfn.DescribeStateMachineOutput + resourceName := "aws_sfn_state_machine.test" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t) }, + ErrorCheck: acctest.ErrorCheck(t, sfn.EndpointsID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckStateMachineDestroy, + Steps: []resource.TestStep{ + { + Config: testAccStateMachineConfig_nameGenerated(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckExists(resourceName, &sm), + acctest.CheckResourceAttrNameGenerated(resourceName, "name"), + resource.TestCheckResourceAttr(resourceName, "name_prefix", resource.UniqueIdPrefix), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + +func TestAccSFNStateMachine_namePrefix(t *testing.T) { + var sm sfn.DescribeStateMachineOutput + resourceName := "aws_sfn_state_machine.test" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t) }, + ErrorCheck: acctest.ErrorCheck(t, sfn.EndpointsID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckStateMachineDestroy, + Steps: []resource.TestStep{ + { + Config: testAccStateMachineConfig_namePrefix(rName, "tf-acc-test-prefix-"), + Check: resource.ComposeTestCheckFunc( + testAccCheckExists(resourceName, &sm), + acctest.CheckResourceAttrNameFromPrefix(resourceName, "name", "tf-acc-test-prefix-"), + resource.TestCheckResourceAttr(resourceName, "name_prefix", "tf-acc-test-prefix-"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + func TestAccSFNStateMachine_tags(t *testing.T) { var sm sfn.DescribeStateMachineOutput resourceName := "aws_sfn_state_machine.test" @@ -486,6 +543,71 @@ EOF `, rName, rMaxAttempts)) } +func testAccStateMachineConfig_nameGenerated(rName string) string { + return acctest.ConfigCompose(testAccStateMachineConfig_base(rName), ` +resource "aws_sfn_state_machine" "test" { + role_arn = aws_iam_role.for_sfn.arn + + definition = < 80 { - errors = append(errors, fmt.Errorf("%q cannot be longer than 80 characters", k)) - } - - if !regexp.MustCompile(`^[a-zA-Z0-9-_]+$`).MatchString(value) { - errors = append(errors, fmt.Errorf( - "%q must be composed with only these characters [a-zA-Z0-9-_]: %v", k, value)) - } - return -} diff --git a/internal/service/sfn/validate_test.go b/internal/service/sfn/validate_test.go deleted file mode 100644 index a00f4492153..00000000000 --- a/internal/service/sfn/validate_test.go +++ /dev/null @@ -1,51 +0,0 @@ -package sfn - -import ( - "strings" - "testing" -) - -func TestValidStateMachineName(t *testing.T) { - validTypes := []string{ - "foo", - "BAR", - "FooBar123", - "FooBar123Baz-_", - } - - invalidTypes := []string{ - "foo bar", - "foo", - "foo{bar}", - "foo[bar]", - "foo*bar", - "foo?bar", - "foo#bar", - "foo%bar", - "foo\bar", - "foo^bar", - "foo|bar", - "foo~bar", - "foo$bar", - "foo&bar", - "foo,bar", - "foo:bar", - "foo;bar", - "foo/bar", - strings.Repeat("W", 81), // length > 80 - } - - for _, v := range validTypes { - _, errors := validStateMachineName(v, "name") - if len(errors) != 0 { - t.Fatalf("%q should be a valid Step Function State Machine name: %v", v, errors) - } - } - - for _, v := range invalidTypes { - _, errors := validStateMachineName(v, "name") - if len(errors) == 0 { - t.Fatalf("%q should not be a valid Step Function State Machine name", v) - } - } -} diff --git a/website/docs/r/sfn_state_machine.html.markdown b/website/docs/r/sfn_state_machine.html.markdown index c001979d533..cfd7404757f 100644 --- a/website/docs/r/sfn_state_machine.html.markdown +++ b/website/docs/r/sfn_state_machine.html.markdown @@ -101,7 +101,8 @@ The following arguments are supported: * `definition` - (Required) The [Amazon States Language](https://docs.aws.amazon.com/step-functions/latest/dg/concepts-amazon-states-language.html) definition of the state machine. * `logging_configuration` - (Optional) Defines what execution history events are logged and where they are logged. The `logging_configuration` parameter is only valid when `type` is set to `EXPRESS`. Defaults to `OFF`. For more information see [Logging Express Workflows](https://docs.aws.amazon.com/step-functions/latest/dg/cw-logs.html) and [Log Levels](https://docs.aws.amazon.com/step-functions/latest/dg/cloudwatch-log-level.html) in the AWS Step Functions User Guide. -* `name` - (Required) The name of the state machine. To enable logging with CloudWatch Logs, the name should only contain `0`-`9`, `A`-`Z`, `a`-`z`, `-` and `_`. +* `name` - (Optional) The name of the state machine. The name should only contain `0`-`9`, `A`-`Z`, `a`-`z`, `-` and `_`. If omitted, Terraform will assign a random, unique name. +* `name_prefix` - (Optional) Creates a unique name beginning with the specified prefix. Conflicts with `name`. * `role_arn` - (Required) The Amazon Resource Name (ARN) of the IAM role to use for this state machine. * `tags` - (Optional) Key-value map of resource tags. If configured with a provider [`default_tags` configuration block](https://registry.terraform.io/providers/hashicorp/aws/latest/docs#default_tags-configuration-block) present, tags with matching keys will overwrite those defined at the provider-level. * `tracing_configuration` - (Optional) Selects whether AWS X-Ray tracing is enabled. From 3e4005586a20ba06ac2eb59c0616ce2f7aaa25fd Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 1 Nov 2022 11:29:43 -0400 Subject: [PATCH 12/13] Add CHANGELOG entry. --- .changelog/27574.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/27574.txt diff --git a/.changelog/27574.txt b/.changelog/27574.txt new file mode 100644 index 00000000000..a6339f40738 --- /dev/null +++ b/.changelog/27574.txt @@ -0,0 +1,3 @@ +```release-note:enhancement +resource/aws_sfn_state_machine: Add `name_prefix` argument +``` \ No newline at end of file From 532b45f04002eb9dc870fe1e5d287df6e47da38d Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 1 Nov 2022 11:32:51 -0400 Subject: [PATCH 13/13] Fix semgrep 'helper-schema-resource-Retry-without-TimeoutError-check'. --- internal/service/sfn/state_machine.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/service/sfn/state_machine.go b/internal/service/sfn/state_machine.go index aff9d9b757c..ea1a3f5f132 100644 --- a/internal/service/sfn/state_machine.go +++ b/internal/service/sfn/state_machine.go @@ -268,7 +268,7 @@ func resourceStateMachineUpdate(ctx context.Context, d *schema.ResourceData, met } // Handle eventual consistency after update. - err = resource.RetryContext(ctx, stateMachineUpdatedTimeout, func() *resource.RetryError { + err = resource.RetryContext(ctx, stateMachineUpdatedTimeout, func() *resource.RetryError { // nosemgrep:ci.helper-schema-resource-Retry-without-TimeoutError-check output, err := FindStateMachineByARN(ctx, conn, d.Id()) if err != nil {