From b551d0d5ddbe6121f0938a200d9b9f31b0f10388 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 6 Jan 2023 17:03:54 -0500 Subject: [PATCH 1/5] dms/repl_subnet_grp: Fix inconsistent result after apply --- .../service/dms/replication_subnet_group.go | 20 ++++- ...dms_replication_subnet_group.html.markdown | 78 ++++++++++++++++--- 2 files changed, 85 insertions(+), 13 deletions(-) diff --git a/internal/service/dms/replication_subnet_group.go b/internal/service/dms/replication_subnet_group.go index 78297d27de45..6a362ca1a6f9 100644 --- a/internal/service/dms/replication_subnet_group.go +++ b/internal/service/dms/replication_subnet_group.go @@ -3,6 +3,7 @@ package dms import ( "fmt" "log" + "time" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/arn" @@ -11,10 +12,12 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-provider-aws/internal/conns" + "github.com/hashicorp/terraform-provider-aws/internal/create" "github.com/hashicorp/terraform-provider-aws/internal/flex" 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" + "github.com/hashicorp/terraform-provider-aws/names" ) func ResourceReplicationSubnetGroup() *schema.Resource { @@ -24,6 +27,12 @@ func ResourceReplicationSubnetGroup() *schema.Resource { Update: resourceReplicationSubnetGroupUpdate, Delete: resourceReplicationSubnetGroupDelete, + Timeouts: &schema.ResourceTimeout{ + Create: schema.DefaultTimeout(15 * time.Minute), + Update: schema.DefaultTimeout(15 * time.Minute), + Delete: schema.DefaultTimeout(15 * time.Minute), + }, + Importer: &schema.ResourceImporter{ State: schema.ImportStatePassthrough, }, @@ -47,6 +56,7 @@ func ResourceReplicationSubnetGroup() *schema.Resource { Type: schema.TypeSet, Elem: &schema.Schema{Type: schema.TypeString}, Set: schema.HashString, + MinItems: 2, Required: true, }, "tags": tftags.TagsSchema(), @@ -61,6 +71,10 @@ func ResourceReplicationSubnetGroup() *schema.Resource { } } +const ( + ResNameReplicationSubnetGroup = "Replication Subnet Group" +) + func resourceReplicationSubnetGroupCreate(d *schema.ResourceData, meta interface{}) error { conn := meta.(*conns.AWSClient).DMSConn() defaultTagsConfig := meta.(*conns.AWSClient).DefaultTagsConfig @@ -93,10 +107,14 @@ func resourceReplicationSubnetGroupCreate(d *schema.ResourceData, meta interface _, err = conn.CreateReplicationSubnetGroup(request) if err != nil { - return err + return create.Error(names.DMS, create.ErrActionCreating, ResNameReplicationSubnetGroup, d.Get("replication_subnet_group_id").(string), err) } } + if err != nil { + return create.Error(names.DMS, create.ErrActionCreating, ResNameReplicationSubnetGroup, d.Get("replication_subnet_group_id").(string), err) + } + d.SetId(d.Get("replication_subnet_group_id").(string)) return resourceReplicationSubnetGroupRead(d, meta) } diff --git a/website/docs/r/dms_replication_subnet_group.html.markdown b/website/docs/r/dms_replication_subnet_group.html.markdown index b3f0efb95a94..48e1b26eec83 100644 --- a/website/docs/r/dms_replication_subnet_group.html.markdown +++ b/website/docs/r/dms_replication_subnet_group.html.markdown @@ -10,36 +10,82 @@ description: |- Provides a DMS (Data Migration Service) replication subnet group resource. DMS replication subnet groups can be created, updated, deleted, and imported. +~> **Note:** AWS requires a special IAM role called `dms-vpc-role` when using this resource. See the example below to create it as part of your configuration. + ## Example Usage +### Basic + ```terraform # Create a new replication subnet group -resource "aws_dms_replication_subnet_group" "test" { - replication_subnet_group_description = "Test replication subnet group" - replication_subnet_group_id = "test-dms-replication-subnet-group-tf" +resource "aws_dms_replication_subnet_group" "example" { + replication_subnet_group_description = "Example replication subnet group" + replication_subnet_group_id = "example-dms-replication-subnet-group-tf" subnet_ids = [ "subnet-12345678", + "subnet-12345679", ] tags = { - Name = "test" + Name = "example" } } ``` -## Argument Reference +### Creating special IAM role -The following arguments are supported: +If your account does not already include the `dms-vpc-role` IAM role, you will need to create it to allow DMS to manage subnets in the VPC. -* `replication_subnet_group_description` - (Required) The description for the subnet group. -* `replication_subnet_group_id` - (Required) The name for the replication subnet group. This value is stored as a lowercase string. +```terraform +resource "aws_iam_role" "dms-vpc-role" { + name = "dms-vpc-role" + description = "Allows DMS to manage VPC" + assume_role_policy = jsonencode({ + Version = "2012-10-17" + Statement = [ + { + Effect = "Allow" + Principal = { + Service = "dms.amazonaws.com" + } + Action = "sts:AssumeRole" + }, + ] + }) +} - - Must contain no more than 255 alphanumeric characters, periods, spaces, underscores, or hyphens. - - Must not be "default". +resource "aws_iam_role_policy_attachment" "example" { + role = aws_iam_role.dms-vpc-role.name + policy_arn = "arn:aws:iam::aws:policy/service-role/AmazonDMSVPCManagementRole" +} -* `subnet_ids` - (Required) A list of the EC2 subnet IDs for the subnet group. -* `tags` - (Optional) A map of tags to assign to the resource. 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. +resource "aws_dms_replication_subnet_group" "example" { + replication_subnet_group_description = "Example" + replication_subnet_group_id = "example-id" + + subnet_ids = [ + "subnet-12345678", + "subnet-12345679", + ] + + tags = { + Name = "example-id" + } + + # explicit depends_on is needed since this resource doesn't reference the role or policy attachment + depends_on = [aws_iam_role_policy_attachment.example] +} +``` + +## Argument Reference + +The following arguments are supported: + +* `replication_subnet_group_description` - (Required) Description for the subnet group. +* `replication_subnet_group_id` - (Required) Name for the replication subnet group. This value is stored as a lowercase string. It must contain no more than 255 alphanumeric characters, periods, spaces, underscores, or hyphens and cannot be `default`. +* `subnet_ids` - (Required) List of at least 2 EC2 subnet IDs for the subnet group. The subnets must cover at least 2 availability zones. +* `tags` - (Optional) Map of tags to assign to the resource. 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. ## Attributes Reference @@ -48,6 +94,14 @@ In addition to all arguments above, the following attributes are exported: * `tags_all` - A map of tags assigned to the resource, including those inherited from the provider [`default_tags` configuration block](https://registry.terraform.io/providers/hashicorp/aws/latest/docs#default_tags-configuration-block). * `vpc_id` - The ID of the VPC the subnet group is in. +## Timeouts + +[Configuration options](https://developer.hashicorp.com/terraform/language/resources/syntax#operation-timeouts): + +- `create` - (Default `15m`) +- `update` - (Default `15m`) +- `delete` - (Default `15m`) + ## Import Replication subnet groups can be imported using the `replication_subnet_group_id`, e.g., From 17422a1a13690eab1725a9dc8926791b4f6a46c0 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 6 Jan 2023 17:11:36 -0500 Subject: [PATCH 2/5] Add changelog --- .changelog/28748.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/28748.txt diff --git a/.changelog/28748.txt b/.changelog/28748.txt new file mode 100644 index 000000000000..da486919881a --- /dev/null +++ b/.changelog/28748.txt @@ -0,0 +1,3 @@ +```release-note:bug +resource/aws_dms_replication_subnet_group: Fix error ("Provider produced inconsistent result") when an error is encountered during creation +``` \ No newline at end of file From c191f7f94d1460d633293a9cb8947e8cac992a8f Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 6 Jan 2023 17:11:53 -0500 Subject: [PATCH 3/5] Clean up errors --- internal/service/dms/replication_subnet_group.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/internal/service/dms/replication_subnet_group.go b/internal/service/dms/replication_subnet_group.go index 6a362ca1a6f9..f21b5126e2f2 100644 --- a/internal/service/dms/replication_subnet_group.go +++ b/internal/service/dms/replication_subnet_group.go @@ -133,7 +133,7 @@ func resourceReplicationSubnetGroupRead(d *schema.ResourceData, meta interface{} }, }) if err != nil { - return err + return create.Error(names.DMS, create.ErrActionReading, ResNameReplicationSubnetGroup, d.Id(), err) } if len(response.ReplicationSubnetGroups) == 0 { d.SetId("") @@ -153,24 +153,24 @@ func resourceReplicationSubnetGroupRead(d *schema.ResourceData, meta interface{} err = resourceReplicationSubnetGroupSetState(d, response.ReplicationSubnetGroups[0]) if err != nil { - return err + return create.Error(names.DMS, create.ErrActionReading, ResNameReplicationSubnetGroup, d.Id(), err) } tags, err := ListTags(conn, arn) if err != nil { - return fmt.Errorf("error listing tags for DMS Replication Subnet Group (%s): %s", arn, err) + return create.Error(names.DMS, create.ErrActionReading, ResNameReplicationSubnetGroup, 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 create.SettingError(names.DMS, ResNameReplicationSubnetGroup, d.Id(), "tags", err) } if err := d.Set("tags_all", tags.Map()); err != nil { - return fmt.Errorf("error setting tags_all: %w", err) + return create.SettingError(names.DMS, ResNameReplicationSubnetGroup, d.Id(), "tags_all", err) } return nil @@ -195,7 +195,7 @@ func resourceReplicationSubnetGroupUpdate(d *schema.ResourceData, meta interface o, n := d.GetChange("tags_all") if err := UpdateTags(conn, arn, o, n); err != nil { - return fmt.Errorf("error updating DMS Replication Subnet Group (%s) tags: %s", arn, err) + return create.Error(names.DMS, create.ErrActionUpdating, ResNameReplicationSubnetGroup, d.Id(), err) } } @@ -203,7 +203,7 @@ func resourceReplicationSubnetGroupUpdate(d *schema.ResourceData, meta interface _, err := conn.ModifyReplicationSubnetGroup(request) if err != nil { - return err + return create.Error(names.DMS, create.ErrActionUpdating, ResNameReplicationSubnetGroup, d.Id(), err) } return resourceReplicationSubnetGroupRead(d, meta) @@ -219,7 +219,7 @@ func resourceReplicationSubnetGroupDelete(d *schema.ResourceData, meta interface log.Printf("[DEBUG] DMS delete replication subnet group: %#v", request) _, err := conn.DeleteReplicationSubnetGroup(request) - return err + return create.Error(names.DMS, create.ErrActionDeleting, ResNameReplicationSubnetGroup, d.Id(), err) } func resourceReplicationSubnetGroupSetState(d *schema.ResourceData, group *dms.ReplicationSubnetGroup) error { From 1bb0e46deb4329bb4591f72c8cc306aaba7d0c87 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 6 Jan 2023 17:24:54 -0500 Subject: [PATCH 4/5] Fix for when nil error --- internal/service/dms/replication_subnet_group.go | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/internal/service/dms/replication_subnet_group.go b/internal/service/dms/replication_subnet_group.go index f21b5126e2f2..080ad92a05bc 100644 --- a/internal/service/dms/replication_subnet_group.go +++ b/internal/service/dms/replication_subnet_group.go @@ -132,9 +132,17 @@ func resourceReplicationSubnetGroupRead(d *schema.ResourceData, meta interface{} }, }, }) + + if !d.IsNewResource() && tfresource.NotFound(err) { + create.LogNotFoundRemoveState(names.DMS, create.ErrActionReading, ResNameReplicationSubnetGroup, d.Id()) + d.SetId("") + return nil + } + if err != nil { return create.Error(names.DMS, create.ErrActionReading, ResNameReplicationSubnetGroup, d.Id(), err) } + if len(response.ReplicationSubnetGroups) == 0 { d.SetId("") return nil @@ -219,7 +227,12 @@ func resourceReplicationSubnetGroupDelete(d *schema.ResourceData, meta interface log.Printf("[DEBUG] DMS delete replication subnet group: %#v", request) _, err := conn.DeleteReplicationSubnetGroup(request) - return create.Error(names.DMS, create.ErrActionDeleting, ResNameReplicationSubnetGroup, d.Id(), err) + + if err != nil { + return create.Error(names.DMS, create.ErrActionDeleting, ResNameReplicationSubnetGroup, d.Id(), err) + } + + return nil } func resourceReplicationSubnetGroupSetState(d *schema.ResourceData, group *dms.ReplicationSubnetGroup) error { From b8442042b933929e1a64ee0ec02254673c6f661e Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 6 Jan 2023 23:19:56 -0500 Subject: [PATCH 5/5] Fix always return nil --- .../service/dms/replication_subnet_group.go | 31 +++++++------------ 1 file changed, 12 insertions(+), 19 deletions(-) diff --git a/internal/service/dms/replication_subnet_group.go b/internal/service/dms/replication_subnet_group.go index 080ad92a05bc..fbdc1ea4e454 100644 --- a/internal/service/dms/replication_subnet_group.go +++ b/internal/service/dms/replication_subnet_group.go @@ -159,11 +159,20 @@ func resourceReplicationSubnetGroupRead(d *schema.ResourceData, meta interface{} }.String() d.Set("replication_subnet_group_arn", arn) - err = resourceReplicationSubnetGroupSetState(d, response.ReplicationSubnetGroups[0]) - if err != nil { - return create.Error(names.DMS, create.ErrActionReading, ResNameReplicationSubnetGroup, d.Id(), err) + group := response.ReplicationSubnetGroups[0] + + d.SetId(aws.StringValue(group.ReplicationSubnetGroupIdentifier)) + + subnet_ids := []string{} + for _, subnet := range group.Subnets { + subnet_ids = append(subnet_ids, aws.StringValue(subnet.SubnetIdentifier)) } + d.Set("replication_subnet_group_description", group.ReplicationSubnetGroupDescription) + d.Set("replication_subnet_group_id", group.ReplicationSubnetGroupIdentifier) + d.Set("subnet_ids", subnet_ids) + d.Set("vpc_id", group.VpcId) + tags, err := ListTags(conn, arn) if err != nil { @@ -234,19 +243,3 @@ func resourceReplicationSubnetGroupDelete(d *schema.ResourceData, meta interface return nil } - -func resourceReplicationSubnetGroupSetState(d *schema.ResourceData, group *dms.ReplicationSubnetGroup) error { - d.SetId(aws.StringValue(group.ReplicationSubnetGroupIdentifier)) - - subnet_ids := []string{} - for _, subnet := range group.Subnets { - subnet_ids = append(subnet_ids, aws.StringValue(subnet.SubnetIdentifier)) - } - - d.Set("replication_subnet_group_description", group.ReplicationSubnetGroupDescription) - d.Set("replication_subnet_group_id", group.ReplicationSubnetGroupIdentifier) - d.Set("subnet_ids", subnet_ids) - d.Set("vpc_id", group.VpcId) - - return nil -}