resource/ecs_service: fix crash when importing non-existing service

[loivis]

Fix #3671

One thing unsure though. Do we test such scenario as import error?

[bflad]

Do we test such scenario as import error?

Not currently for most (any?) resources, but maybe we should given scenarios like this. 😄 I was able to generate the panic on master:

make testacc TEST=./aws TESTARGS='-run=TestAccAWSEcsService_basicImport'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSEcsService_basicImport -timeout 120m
=== RUN   TestAccAWSEcsService_basicImport
panic: runtime error: index out of range

goroutine 350 [running]:
github.com/terraform-providers/terraform-provider-aws/aws.resourceAwsEcsServiceRead.func1(0xc4204c2190)
	/Users/bflad/go/src/github.com/terraform-providers/terraform-provider-aws/aws/resource_aws_ecs_service.go:364 +0x4b7
...
FAIL	github.com/terraform-providers/terraform-provider-aws/aws	11.893s
make: *** [testacc] Error 1

With an additional TestStep:

			// Test non-existent resource import
			{
				ResourceName:      resourceName,
				ImportStateId:     fmt.Sprintf("%s/nonexistent", clusterName),
				ImportState:       true,
				ImportStateVerify: false,
			},

Assuming this works, I can add this to the PR so we can have verification, otherwise the fix looks fine. 👍

[loivis]

👍Please feel free to add the verification

[bflad]

Huzzah! 😂

With naive implementation above:

make testacc TEST=./aws TESTARGS='-run=TestAccAWSEcsService_importBasic'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSEcsService_importBasic -timeout 120m
=== RUN   TestAccAWSEcsService_importBasic
--- FAIL: TestAccAWSEcsService_importBasic (43.50s)
	testing.go:513: Step 2 error: 1 error(s) occurred:

		* aws_ecs_service.jenkins (import id: tf-acc-cluster-svc-0oezofzf/nonexistent): 1 error(s) occurred:

		* import aws_ecs_service.jenkins result: nonexistent: aws_ecs_service.jenkins: No ECS service found: "nonexistent"
FAIL
FAIL	github.com/terraform-providers/terraform-provider-aws/aws	43.535s

Modified:

			// Test non-existent resource import
			{
				ResourceName:      resourceName,
				ImportStateId:     fmt.Sprintf("%s/nonexistent", clusterName),
				ImportState:       true,
				ImportStateVerify: false,
				ExpectError:       regexp.MustCompile(`No ECS service found`),
			},

Returns:

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

[bflad]

LGTM, thanks! 🚀

 31 tests passed (all tests)
=== RUN   TestAccAWSEcsDataSource_ecsTaskDefinition
--- PASS: TestAccAWSEcsDataSource_ecsTaskDefinition (16.76s)
=== RUN   TestAccAWSEcsService_withPlacementConstraints_emptyExpression
--- PASS: TestAccAWSEcsService_withPlacementConstraints_emptyExpression (17.10s)
=== RUN   TestAccAWSEcsCluster_basic
--- PASS: TestAccAWSEcsCluster_basic (19.20s)
=== RUN   TestAccAWSEcsCluster_importBasic
--- PASS: TestAccAWSEcsCluster_importBasic (20.78s)
=== RUN   TestAccAWSEcsTaskDefinition_withScratchVolume
--- PASS: TestAccAWSEcsTaskDefinition_withScratchVolume (6.78s)
=== RUN   TestAccAWSEcsService_withDeploymentValues
--- PASS: TestAccAWSEcsService_withDeploymentValues (26.49s)
=== RUN   TestAccAWSEcsService_withEcsClusterName
--- PASS: TestAccAWSEcsService_withEcsClusterName (26.46s)
=== RUN   TestAccAWSEcsTaskDefinition_basic
--- PASS: TestAccAWSEcsTaskDefinition_basic (10.70s)
=== RUN   TestAccAWSEcsTaskDefinition_constraint
--- PASS: TestAccAWSEcsTaskDefinition_constraint (7.69s)
=== RUN   TestAccAWSEcsService_withFamilyAndRevision
--- PASS: TestAccAWSEcsService_withFamilyAndRevision (35.47s)
=== RUN   TestAccAWSEcsDataSource_ecsContainerDefinition
--- PASS: TestAccAWSEcsDataSource_ecsContainerDefinition (36.20s)
=== RUN   TestAccAWSEcsTaskDefinition_withTaskRoleArn
--- PASS: TestAccAWSEcsTaskDefinition_withTaskRoleArn (10.71s)
=== RUN   TestAccAWSEcsTaskDefinition_withNetworkMode
--- PASS: TestAccAWSEcsTaskDefinition_withNetworkMode (12.51s)
=== RUN   TestAccAWSEcsDataSource_ecsCluster
--- PASS: TestAccAWSEcsDataSource_ecsCluster (39.48s)
=== RUN   TestAccAWSEcsService_withARN
--- PASS: TestAccAWSEcsService_withARN (40.15s)
=== RUN   TestAccAWSEcsTaskDefinition_changeVolumesForcesNewResource
--- PASS: TestAccAWSEcsTaskDefinition_changeVolumesForcesNewResource (14.10s)
=== RUN   TestAccAWSEcsTaskDefinition_Fargate
--- PASS: TestAccAWSEcsTaskDefinition_Fargate (7.41s)
=== RUN   TestAccAWSEcsService_withUnnormalizedPlacementStrategy
--- PASS: TestAccAWSEcsService_withUnnormalizedPlacementStrategy (43.30s)
=== RUN   TestAccAWSEcsTaskDefinition_arrays
--- PASS: TestAccAWSEcsTaskDefinition_arrays (9.26s)
=== RUN   TestAccAWSEcsService_withRenamedCluster
--- PASS: TestAccAWSEcsService_withRenamedCluster (45.18s)
=== RUN   TestAccAWSEcsService_withPlacementConstraints
--- PASS: TestAccAWSEcsService_withPlacementConstraints (46.70s)
=== RUN   TestAccAWSEcsTaskDefinition_ExecutionRole
--- PASS: TestAccAWSEcsTaskDefinition_ExecutionRole (12.90s)
=== RUN   TestAccAWSEcsService_importBasic
--- PASS: TestAccAWSEcsService_importBasic (49.70s)
=== RUN   TestAccAWSEcsService_withPlacementStrategy
--- PASS: TestAccAWSEcsService_withPlacementStrategy (52.23s)
=== RUN   TestAccAWSEcsTaskDefinition_withEcsService
--- PASS: TestAccAWSEcsTaskDefinition_withEcsService (70.24s)
=== RUN   TestAccAWSEcsService_withLaunchTypeEC2AndNetworkConfiguration
--- PASS: TestAccAWSEcsService_withLaunchTypeEC2AndNetworkConfiguration (81.10s)
=== RUN   TestAccAWSEcsService_withIamRole
--- PASS: TestAccAWSEcsService_withIamRole (123.60s)
=== RUN   TestAccAWSEcsService_withAlb
--- PASS: TestAccAWSEcsService_withAlb (225.05s)
=== RUN   TestAccAWSEcsService_healthCheckGracePeriodSeconds
--- PASS: TestAccAWSEcsService_healthCheckGracePeriodSeconds (231.43s)
=== RUN   TestAccAWSEcsService_withLbChanges
--- PASS: TestAccAWSEcsService_withLbChanges (245.53s)
=== RUN   TestAccAWSEcsService_withLaunchTypeFargate
--- PASS: TestAccAWSEcsService_withLaunchTypeFargate (247.18s)

[bflad]

This has been released in version 1.11.0 of the AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

[ghost]

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

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