From 68886758670c815fbb899a62caf7fae6231babce Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 23 Jul 2024 09:54:45 -0400 Subject: [PATCH 1/5] Add 'TestAccECSService_VolumeConfiguration_throughputTypeChange'. --- internal/service/ecs/service_test.go | 146 +++++++++++++-------------- 1 file changed, 73 insertions(+), 73 deletions(-) diff --git a/internal/service/ecs/service_test.go b/internal/service/ecs/service_test.go index 6d1d9521a35..64c29563e0e 100644 --- a/internal/service/ecs/service_test.go +++ b/internal/service/ecs/service_test.go @@ -362,6 +362,53 @@ func TestAccECSService_VolumeConfiguration_update(t *testing.T) { }) } +// https://github.com/hashicorp/terraform-provider-aws/issues/38475 +func TestAccECSService_VolumeConfiguration_throughputTypeChange(t *testing.T) { + ctx := acctest.Context(t) + var service awstypes.Service + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_ecs_service.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, names.ECSServiceID), + CheckDestroy: testAccCheckServiceDestroy(ctx), + Steps: []resource.TestStep{ + { + ExternalProviders: map[string]resource.ExternalProvider{ + "aws": { + Source: "hashicorp/aws", + VersionConstraint: "5.58.0", + }, + }, + Config: testAccServiceConfig_volumeConfiguration_gp3(rName), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckServiceExists(ctx, resourceName, &service), + resource.TestCheckResourceAttr(resourceName, "volume_configuration.#", acctest.Ct1), + resource.TestCheckResourceAttr(resourceName, "volume_configuration.0.name", "vol1"), + resource.TestCheckResourceAttr(resourceName, "volume_configuration.0.managed_ebs_volume.#", acctest.Ct1), + resource.TestCheckResourceAttr(resourceName, "volume_configuration.0.managed_ebs_volume.0.size_in_gb", "10"), + resource.TestCheckResourceAttr(resourceName, "volume_configuration.0.managed_ebs_volume.0.throughput", ""), + resource.TestCheckResourceAttr(resourceName, "volume_configuration.0.managed_ebs_volume.0.volume_type", "gp3"), + ), + }, + { + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + Config: testAccServiceConfig_volumeConfiguration_gp3(rName), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckServiceExists(ctx, resourceName, &service), + resource.TestCheckResourceAttr(resourceName, "volume_configuration.#", acctest.Ct1), + resource.TestCheckResourceAttr(resourceName, "volume_configuration.0.name", "vol1"), + resource.TestCheckResourceAttr(resourceName, "volume_configuration.0.managed_ebs_volume.#", acctest.Ct1), + resource.TestCheckResourceAttr(resourceName, "volume_configuration.0.managed_ebs_volume.0.size_in_gb", "10"), + resource.TestCheckResourceAttr(resourceName, "volume_configuration.0.managed_ebs_volume.0.throughput", "0"), + resource.TestCheckResourceAttr(resourceName, "volume_configuration.0.managed_ebs_volume.0.volume_type", "gp3"), + ), + }, + }, + }) +} + func TestAccECSService_familyAndRevision(t *testing.T) { ctx := acctest.Context(t) var service awstypes.Service @@ -2144,7 +2191,7 @@ resource "aws_ecs_service" "test" { `, rName)) } -func testAccServiceConfig_volumeConfiguration_basic(rName string) string { +func testAccServiceConfig_baseVolumeConfiguration(rName string) string { return fmt.Sprintf(` data "aws_caller_identity" "current" {} data "aws_partition" "current" {} @@ -2218,7 +2265,11 @@ resource "aws_iam_role_policy" "ecs_service" { } EOF } +`, rName) +} +func testAccServiceConfig_volumeConfiguration_basic(rName string) string { + return acctest.ConfigCompose(testAccServiceConfig_baseVolumeConfiguration(rName), fmt.Sprintf(` resource "aws_ecs_service" "test" { name = %[1]q cluster = aws_ecs_cluster.test.id @@ -2235,84 +2286,33 @@ resource "aws_ecs_service" "test" { depends_on = [aws_iam_role_policy.ecs_service] } -`, rName) +`, rName)) } func testAccServiceConfig_volumeConfiguration_update(rName, volumeType string, size int) string { - return fmt.Sprintf(` -data "aws_caller_identity" "current" {} -data "aws_partition" "current" {} - -resource "aws_ecs_cluster" "test" { - name = %[1]q -} - -resource "aws_ecs_task_definition" "test" { - family = %[1]q - - container_definitions = < Date: Tue, 23 Jul 2024 10:33:22 -0400 Subject: [PATCH 2/5] r/aws_ecs_service: Add StateUpgrader to convert 'volume_configuration.managed_ebs_volume.throughput' from string to int. --- internal/service/ecs/service.go | 506 ++++++++++++++++++++++++++++++++ 1 file changed, 506 insertions(+) diff --git a/internal/service/ecs/service.go b/internal/service/ecs/service.go index 3d47a704c44..3f2684dc1dd 100644 --- a/internal/service/ecs/service.go +++ b/internal/service/ecs/service.go @@ -8,6 +8,7 @@ import ( "fmt" "log" "math" + "strconv" "strings" "time" @@ -37,6 +38,479 @@ import ( // @SDKResource("aws_ecs_service", name="Service") // @Tags(identifierAttribute="id") func resourceService() *schema.Resource { + // Resource with v0 schema (provider v5.58.0). + resourceV0 := &schema.Resource{ + Schema: map[string]*schema.Schema{ + "alarms": { + Type: schema.TypeList, + Optional: true, + MaxItems: 1, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "alarm_names": { + Type: schema.TypeSet, + Required: true, + Elem: &schema.Schema{ + Type: schema.TypeString, + }, + }, + "enable": { + Type: schema.TypeBool, + Required: true, + }, + "rollback": { + Type: schema.TypeBool, + Required: true, + }, + }, + }, + }, + names.AttrCapacityProviderStrategy: { + Type: schema.TypeSet, + Optional: true, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "base": { + Type: schema.TypeInt, + Optional: true, + }, + "capacity_provider": { + Type: schema.TypeString, + Required: true, + }, + names.AttrWeight: { + Type: schema.TypeInt, + Optional: true, + }, + }, + }, + }, + "cluster": { + Type: schema.TypeString, + Optional: true, + Computed: true, + ForceNew: true, + }, + "deployment_circuit_breaker": { + Type: schema.TypeList, + Optional: true, + MaxItems: 1, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "enable": { + Type: schema.TypeBool, + Required: true, + }, + "rollback": { + Type: schema.TypeBool, + Required: true, + }, + }, + }, + }, + "deployment_controller": { + Type: schema.TypeList, + Optional: true, + MaxItems: 1, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + names.AttrType: { + Type: schema.TypeString, + ForceNew: true, + Optional: true, + Default: awstypes.DeploymentControllerTypeEcs, + }, + }, + }, + }, + "deployment_maximum_percent": { + Type: schema.TypeInt, + Optional: true, + Default: 200, + }, + "deployment_minimum_healthy_percent": { + Type: schema.TypeInt, + Optional: true, + Default: 100, + }, + "desired_count": { + Type: schema.TypeInt, + Optional: true, + }, + "enable_ecs_managed_tags": { + Type: schema.TypeBool, + Optional: true, + Default: false, + }, + "enable_execute_command": { + Type: schema.TypeBool, + Optional: true, + Default: false, + }, + "force_new_deployment": { + Type: schema.TypeBool, + Optional: true, + }, + "health_check_grace_period_seconds": { + Type: schema.TypeInt, + Optional: true, + }, + "iam_role": { + Type: schema.TypeString, + ForceNew: true, + Optional: true, + Computed: true, + }, + "launch_type": { + Type: schema.TypeString, + ForceNew: true, + Optional: true, + Computed: true, + }, + "load_balancer": { + Type: schema.TypeSet, + Optional: true, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "container_name": { + Type: schema.TypeString, + Required: true, + }, + "container_port": { + Type: schema.TypeInt, + Required: true, + }, + "elb_name": { + Type: schema.TypeString, + Optional: true, + }, + "target_group_arn": { + Type: schema.TypeString, + Optional: true, + ValidateFunc: verify.ValidARN, + }, + }, + }, + }, + names.AttrName: { + Type: schema.TypeString, + Required: true, + ForceNew: true, + }, + names.AttrNetworkConfiguration: { + Type: schema.TypeList, + Optional: true, + MaxItems: 1, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "assign_public_ip": { + Type: schema.TypeBool, + Optional: true, + Default: false, + }, + names.AttrSecurityGroups: { + Type: schema.TypeSet, + Optional: true, + Elem: &schema.Schema{Type: schema.TypeString}, + }, + names.AttrSubnets: { + Type: schema.TypeSet, + Required: true, + Elem: &schema.Schema{Type: schema.TypeString}, + }, + }, + }, + }, + "ordered_placement_strategy": { + Type: schema.TypeList, + Optional: true, + MaxItems: 5, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + names.AttrField: { + Type: schema.TypeString, + Optional: true, + }, + names.AttrType: { + Type: schema.TypeString, + Required: true, + }, + }, + }, + }, + "placement_constraints": { + Type: schema.TypeSet, + Optional: true, + MaxItems: 10, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + names.AttrExpression: { + Type: schema.TypeString, + Optional: true, + }, + names.AttrType: { + Type: schema.TypeString, + Required: true, + }, + }, + }, + }, + "platform_version": { + Type: schema.TypeString, + Optional: true, + Computed: true, + }, + names.AttrPropagateTags: { + Type: schema.TypeString, + Optional: true, + }, + "scheduling_strategy": { + Type: schema.TypeString, + Optional: true, + ForceNew: true, + Default: awstypes.SchedulingStrategyReplica, + }, + "service_connect_configuration": { + Type: schema.TypeList, + Optional: true, + MaxItems: 1, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + names.AttrEnabled: { + Type: schema.TypeBool, + Required: true, + }, + "log_configuration": { + Type: schema.TypeList, + Optional: true, + MaxItems: 1, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "log_driver": { + Type: schema.TypeString, + Required: true, + }, + "options": { + Type: schema.TypeMap, + Optional: true, + Computed: true, + Elem: &schema.Schema{Type: schema.TypeString}, + }, + "secret_option": { + Type: schema.TypeList, + Optional: true, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + names.AttrName: { + Type: schema.TypeString, + Required: true, + }, + "value_from": { + Type: schema.TypeString, + Required: true, + }, + }, + }, + }, + }, + }, + }, + names.AttrNamespace: { + Type: schema.TypeString, + Optional: true, + }, + "service": { + Type: schema.TypeList, + Optional: true, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "client_alias": { + Type: schema.TypeList, + Optional: true, + MaxItems: 1, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + names.AttrDNSName: { + Type: schema.TypeString, + Optional: true, + }, + names.AttrPort: { + Type: schema.TypeInt, + Required: true, + }, + }, + }, + }, + "discovery_name": { + Type: schema.TypeString, + Optional: true, + }, + "ingress_port_override": { + Type: schema.TypeInt, + Optional: true, + }, + "port_name": { + Type: schema.TypeString, + Required: true, + }, + names.AttrTimeout: { + Type: schema.TypeList, + Optional: true, + MaxItems: 1, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "idle_timeout_seconds": { + Type: schema.TypeInt, + Optional: true, + }, + "per_request_timeout_seconds": { + Type: schema.TypeInt, + Optional: true, + }, + }, + }, + }, + "tls": { + Type: schema.TypeList, + Optional: true, + MaxItems: 1, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "issuer_cert_authority": { + Type: schema.TypeList, + Required: true, + MaxItems: 1, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "aws_pca_authority_arn": { + Type: schema.TypeString, + Required: true, + }, + }, + }, + }, + names.AttrKMSKey: { + Type: schema.TypeString, + Optional: true, + }, + names.AttrRoleARN: { + Type: schema.TypeString, + Optional: true, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + "service_registries": { + Type: schema.TypeList, + Optional: true, + MaxItems: 1, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "container_name": { + Type: schema.TypeString, + Optional: true, + }, + "container_port": { + Type: schema.TypeInt, + Optional: true, + }, + names.AttrPort: { + Type: schema.TypeInt, + Optional: true, + }, + "registry_arn": { + Type: schema.TypeString, + Required: true, + }, + }, + }, + }, + names.AttrTags: tftags.TagsSchema(), + names.AttrTagsAll: tftags.TagsSchemaComputed(), + "task_definition": { + Type: schema.TypeString, + Optional: true, + }, + names.AttrTriggers: { + Type: schema.TypeMap, + Optional: true, + Computed: true, + Elem: &schema.Schema{Type: schema.TypeString}, + }, + "wait_for_steady_state": { + Type: schema.TypeBool, + Optional: true, + Default: false, + }, + "volume_configuration": { + Type: schema.TypeList, + Optional: true, + MaxItems: 1, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + names.AttrName: { + Type: schema.TypeString, + Required: true, + }, + "managed_ebs_volume": { + Type: schema.TypeList, + Required: true, + MaxItems: 1, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + names.AttrEncrypted: { + Type: schema.TypeBool, + Optional: true, + Default: true, + }, + "file_system_type": { + Type: schema.TypeString, + Optional: true, + Default: awstypes.TaskFilesystemTypeXfs, + }, + names.AttrIOPS: { + Type: schema.TypeInt, + Optional: true, + }, + names.AttrKMSKeyID: { + Type: schema.TypeString, + Optional: true, + }, + names.AttrRoleARN: { + Type: schema.TypeString, + Required: true, + }, + "size_in_gb": { + Type: schema.TypeInt, + Optional: true, + }, + names.AttrSnapshotID: { + Type: schema.TypeString, + Optional: true, + }, + names.AttrThroughput: { + Type: schema.TypeString, + Optional: true, + }, + names.AttrVolumeType: { + Type: schema.TypeString, + Optional: true, + }, + }, + }, + }, + }, + }, + }, + }, + } + return &schema.Resource{ CreateWithoutTimeout: resourceServiceCreate, ReadWithoutTimeout: resourceServiceRead, @@ -580,6 +1054,38 @@ func resourceService() *schema.Resource { }, }, + SchemaVersion: 1, + StateUpgraders: []schema.StateUpgrader{ + { + Type: resourceV0.CoreConfigSchema().ImpliedType(), + Upgrade: func(ctx context.Context, rawState map[string]interface{}, meta interface{}) (map[string]interface{}, error) { + // Convert volume_configuration.managed_ebs_volume.throughput from string to int. + if v, ok := rawState["volume_configuration"].([]interface{}); ok && len(v) > 0 && v[0] != nil { + tfMap := v[0].(map[string]interface{}) + if v, ok := tfMap["managed_ebs_volume"].([]interface{}); ok && len(v) > 0 && v[0] != nil { + tfMap := v[0].(map[string]interface{}) + if v, ok := tfMap["throughput"]; ok { + if v, ok := v.(string); ok { + if v == "" { + tfMap["throughput"] = 0 + } else { + if v, err := strconv.Atoi(v); err == nil { + tfMap["throughput"] = v + } else { + return nil, err + } + } + } + } + } + } + + return rawState, nil + }, + Version: 0, + }, + }, + CustomizeDiff: customdiff.Sequence( verify.SetTagsDiff, capacityProviderStrategyCustomizeDiff, From 1afc5b73b80141005fc901875157a9533edc0d3c Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 23 Jul 2024 10:35:54 -0400 Subject: [PATCH 3/5] Add CHANGELOG entry. --- .changelog/#####.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/#####.txt diff --git a/.changelog/#####.txt b/.changelog/#####.txt new file mode 100644 index 00000000000..25751afe633 --- /dev/null +++ b/.changelog/#####.txt @@ -0,0 +1,3 @@ +```release-note:bug +resource/aws_ecs_service: Fix `error marshaling prior state: a number is required` when upgrading from v5.58.0 to v5.59.0 +``` \ No newline at end of file From 56d2d0e85c28a8b6cad0a503ad57ef21c940cea9 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 23 Jul 2024 10:37:46 -0400 Subject: [PATCH 4/5] Correct CHANGELOG entry file name. --- .changelog/{#####.txt => 38490.txt} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename .changelog/{#####.txt => 38490.txt} (100%) diff --git a/.changelog/#####.txt b/.changelog/38490.txt similarity index 100% rename from .changelog/#####.txt rename to .changelog/38490.txt From d6a819ab028bee5c21c867923ccc6759766b72a7 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 23 Jul 2024 10:55:25 -0400 Subject: [PATCH 5/5] Run 'make fix-constants PKG=ecs'. --- internal/service/ecs/service.go | 6 +++--- internal/service/ecs/service_test.go | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/internal/service/ecs/service.go b/internal/service/ecs/service.go index 3f2684dc1dd..3753afd229c 100644 --- a/internal/service/ecs/service.go +++ b/internal/service/ecs/service.go @@ -1064,13 +1064,13 @@ func resourceService() *schema.Resource { tfMap := v[0].(map[string]interface{}) if v, ok := tfMap["managed_ebs_volume"].([]interface{}); ok && len(v) > 0 && v[0] != nil { tfMap := v[0].(map[string]interface{}) - if v, ok := tfMap["throughput"]; ok { + if v, ok := tfMap[names.AttrThroughput]; ok { if v, ok := v.(string); ok { if v == "" { - tfMap["throughput"] = 0 + tfMap[names.AttrThroughput] = 0 } else { if v, err := strconv.Atoi(v); err == nil { - tfMap["throughput"] = v + tfMap[names.AttrThroughput] = v } else { return nil, err } diff --git a/internal/service/ecs/service_test.go b/internal/service/ecs/service_test.go index 64c29563e0e..33c0265211a 100644 --- a/internal/service/ecs/service_test.go +++ b/internal/service/ecs/service_test.go @@ -387,7 +387,7 @@ func TestAccECSService_VolumeConfiguration_throughputTypeChange(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "volume_configuration.#", acctest.Ct1), resource.TestCheckResourceAttr(resourceName, "volume_configuration.0.name", "vol1"), resource.TestCheckResourceAttr(resourceName, "volume_configuration.0.managed_ebs_volume.#", acctest.Ct1), - resource.TestCheckResourceAttr(resourceName, "volume_configuration.0.managed_ebs_volume.0.size_in_gb", "10"), + resource.TestCheckResourceAttr(resourceName, "volume_configuration.0.managed_ebs_volume.0.size_in_gb", acctest.Ct10), resource.TestCheckResourceAttr(resourceName, "volume_configuration.0.managed_ebs_volume.0.throughput", ""), resource.TestCheckResourceAttr(resourceName, "volume_configuration.0.managed_ebs_volume.0.volume_type", "gp3"), ), @@ -400,8 +400,8 @@ func TestAccECSService_VolumeConfiguration_throughputTypeChange(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "volume_configuration.#", acctest.Ct1), resource.TestCheckResourceAttr(resourceName, "volume_configuration.0.name", "vol1"), resource.TestCheckResourceAttr(resourceName, "volume_configuration.0.managed_ebs_volume.#", acctest.Ct1), - resource.TestCheckResourceAttr(resourceName, "volume_configuration.0.managed_ebs_volume.0.size_in_gb", "10"), - resource.TestCheckResourceAttr(resourceName, "volume_configuration.0.managed_ebs_volume.0.throughput", "0"), + resource.TestCheckResourceAttr(resourceName, "volume_configuration.0.managed_ebs_volume.0.size_in_gb", acctest.Ct10), + resource.TestCheckResourceAttr(resourceName, "volume_configuration.0.managed_ebs_volume.0.throughput", acctest.Ct0), resource.TestCheckResourceAttr(resourceName, "volume_configuration.0.managed_ebs_volume.0.volume_type", "gp3"), ), },