Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Panic if previously registered directory is no longer registered. #11814

Closed
linuxman79 opened this issue Jan 30, 2020 · 7 comments · Fixed by #11837
Closed

Panic if previously registered directory is no longer registered. #11814

linuxman79 opened this issue Jan 30, 2020 · 7 comments · Fixed by #11837
Labels
bug Addresses a defect in current functionality. crash Results from or addresses a Terraform crash or kernel panic. service/workspaces Issues and PRs that pertain to the workspaces service.
Milestone

Comments

@linuxman79
Copy link

If a directory is registered with workspaces (via terraform config) and that config is then removed, the directory is not unregistered. If you manually unregister the directory via the AWS console, terraform operations fail on refresh with RPC errors. If the directory is manually re-registered with workspaces, terraform will refresh again. It appears that the provider is not checking the length of the Directories list returned by the API before trying to index into it. Here's the relevant snippet from the log:

2020-01-30T10:10:45.573-0500 [DEBUG] plugin.terraform-provider-aws_v2.45.0_x4: 2020/01/30 10:10:45 [DEBUG] [aws-sdk-go] DEBUG: Response workspaces/DescribeWorkspaceDirectories Details:
2020-01-30T10:10:45.573-0500 [DEBUG] plugin.terraform-provider-aws_v2.45.0_x4: ---[ RESPONSE ]--------------------------------------
2020-01-30T10:10:45.573-0500 [DEBUG] plugin.terraform-provider-aws_v2.45.0_x4: HTTP/1.1 200 OK
2020-01-30T10:10:45.573-0500 [DEBUG] plugin.terraform-provider-aws_v2.45.0_x4: Connection: close
2020-01-30T10:10:45.573-0500 [DEBUG] plugin.terraform-provider-aws_v2.45.0_x4: Content-Length: 18
2020-01-30T10:10:45.573-0500 [DEBUG] plugin.terraform-provider-aws_v2.45.0_x4: Content-Type: application/x-amz-json-1.1
2020-01-30T10:10:45.573-0500 [DEBUG] plugin.terraform-provider-aws_v2.45.0_x4: Date: Thu, 30 Jan 2020 15:10:45 GMT
2020-01-30T10:10:45.573-0500 [DEBUG] plugin.terraform-provider-aws_v2.45.0_x4: X-Amzn-Requestid: f0b3c2b7-b716-4d5b-80d5-519657f6e98e
2020-01-30T10:10:45.573-0500 [DEBUG] plugin.terraform-provider-aws_v2.45.0_x4:
2020-01-30T10:10:45.573-0500 [DEBUG] plugin.terraform-provider-aws_v2.45.0_x4:
2020-01-30T10:10:45.573-0500 [DEBUG] plugin.terraform-provider-aws_v2.45.0_x4: -----------------------------------------------------
2020-01-30T10:10:45.573-0500 [DEBUG] plugin.terraform-provider-aws_v2.45.0_x4: 2020/01/30 10:10:45 [DEBUG] [aws-sdk-go] {"Directories":[]}
2020-01-30T10:10:45.576-0500 [DEBUG] plugin.terraform-provider-aws_v2.45.0_x4: panic: runtime error: index out of range [0] with length 0

This occurs with versions 2.45 and 2.46 of the provider along with 0.12.18 and 0.12.20 of terraform.

I have the complete log, but it has some data in it that I'm not comfortable sharing here. If more of the log is needed than is provided, please let me know.

Thanks,
Ben

@ghost ghost added bug Addresses a defect in current functionality. crash Results from or addresses a Terraform crash or kernel panic. labels Jan 30, 2020
@github-actions github-actions bot added the needs-triage Waiting for first response or review from a maintainer. label Jan 30, 2020
@ewbankkit
Copy link
Contributor

@linuxman79 Thanks for reporting this.

It looks like it's this code
https://github.com/terraform-providers/terraform-provider-aws/blob/26f1b90ac37ccf31f1e703c081e7162b46167ec0/aws/resource_aws_workspaces_directory.go#L144-L150
in resourceAwsWorkspacesDirectoryRead() that causes the crash.
Similar code
https://github.com/terraform-providers/terraform-provider-aws/blob/26f1b90ac37ccf31f1e703c081e7162b46167ec0/aws/resource_aws_workspaces_directory.go#L249-L257
in workspacesDirectoryRefreshStateFunc() checks for an empty list of directories explicitly.
We should have the same logic (ideally call workspacesDirectoryRefreshStateFunc() and handle response) in resourceAwsWorkspacesDirectoryRead().
An _disappears acceptance test case should catch this.

@ewbankkit
Copy link
Contributor

ewbankkit commented Jan 31, 2020

As expected, a _disappears acceptance test reproduces:

$ make testacc TEST=./aws/ TESTARGS='-run=TestAccAwsWorkspacesDirectory/disappears'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -count 1 -parallel 20 -run=TestAccAwsWorkspacesDirectory/disappears -timeout 120m
=== RUN   TestAccAwsWorkspacesDirectory
=== RUN   TestAccAwsWorkspacesDirectory/disappears
panic: runtime error: index out of range [0] with length 0

goroutine 1403 [running]:
github.com/terraform-providers/terraform-provider-aws/aws.resourceAwsWorkspacesDirectoryRead(0xc0003eb570, 0x517aec0, 0xc000386a00, 0xc0003eb570, 0x0)
	/home/kit/wrk/src/github.com/terraform-providers/terraform-provider-aws/aws/resource_aws_workspaces_directory.go:150 +0x5a5
github.com/hashicorp/terraform-plugin-sdk/helper/schema.(*Resource).RefreshWithoutUpgrade(0xc0007caf00, 0xc000ee3a40, 0x517aec0, 0xc000386a00, 0xc000aaacc0, 0x0, 0x0)
	/home/kit/wrk/pkg/mod/github.com/hashicorp/[email protected]/helper/schema/resource.go:455 +0x119
github.com/hashicorp/terraform-plugin-sdk/internal/helper/plugin.(*GRPCProviderServer).ReadResource(0xc000dcd658, 0x739b4e0, 0xc00163ca20, 0xc000ee38b0, 0xc000dcd658, 0xc00163ca20, 0xc000bd8a80)
	/home/kit/wrk/pkg/mod/github.com/hashicorp/[email protected]/internal/helper/plugin/grpc_provider.go:525 +0x3d8
github.com/hashicorp/terraform-plugin-sdk/internal/tfplugin5._Provider_ReadResource_Handler(0x6359200, 0xc000dcd658, 0x739b4e0, 0xc00163ca20, 0xc000da3f20, 0x0, 0x739b4e0, 0xc00163ca20, 0xc0008ac6e0, 0x150)
	/home/kit/wrk/pkg/mod/github.com/hashicorp/[email protected]/internal/tfplugin5/tfplugin5.pb.go:3153 +0x217
google.golang.org/grpc.(*Server).processUnaryRPC(0xc0016ef4a0, 0x73d5a20, 0xc001642000, 0xc000b8f900, 0xc001918240, 0xaee0210, 0x0, 0x0, 0x0)
	/home/kit/wrk/pkg/mod/google.golang.org/[email protected]/server.go:995 +0x460
google.golang.org/grpc.(*Server).handleStream(0xc0016ef4a0, 0x73d5a20, 0xc001642000, 0xc000b8f900, 0x0)
	/home/kit/wrk/pkg/mod/google.golang.org/[email protected]/server.go:1275 +0xd97
google.golang.org/grpc.(*Server).serveStreams.func1.1(0xc00086ce30, 0xc0016ef4a0, 0x73d5a20, 0xc001642000, 0xc000b8f900)
	/home/kit/wrk/pkg/mod/google.golang.org/[email protected]/server.go:710 +0xbb
created by google.golang.org/grpc.(*Server).serveStreams.func1
	/home/kit/wrk/pkg/mod/google.golang.org/[email protected]/server.go:708 +0xa1
FAIL	github.com/terraform-providers/terraform-provider-aws/aws	421.024s
FAIL
GNUmakefile:24: recipe for target 'testacc' failed
make: *** [testacc] Error 1

@linuxman79
Copy link
Author

@ewbankkit, thanks for your work on this. Please let me know if there's anything I can do to help. I'm not a Go programmer, but I can test in my environment if needed.

Ben

@ewbankkit
Copy link
Contributor

@linuxman79 Thanks for the offer. The newly added test case should be sufficient.

@bflad bflad added service/workspaces Issues and PRs that pertain to the workspaces service. and removed needs-triage Waiting for first response or review from a maintainer. labels Feb 11, 2020
@bflad bflad added this to the v2.51.0 milestone Feb 23, 2020
@bflad
Copy link
Contributor

bflad commented Feb 23, 2020

The fix for this has been merged and will release with version 2.51.0 of the Terraform AWS Provider, end of this week. Thanks to @ewbankkit for the implementation. 👍

@ghost
Copy link

ghost commented Feb 28, 2020

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

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template for triage. Thanks!

@ghost
Copy link

ghost commented Mar 27, 2020

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!

@ghost ghost locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Addresses a defect in current functionality. crash Results from or addresses a Terraform crash or kernel panic. service/workspaces Issues and PRs that pertain to the workspaces service.
Projects
None yet
3 participants