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

Return error if the vault secrets app resource is not found. #619

Merged
merged 3 commits into from
Oct 6, 2023

Conversation

atheken
Copy link
Contributor

@atheken atheken commented Oct 2, 2023

🛠️ Description

The provider crashes when reading Vault Secrets App resources that no longer exist because the Read method of the resource_vault_secrets_app does not currently return early on error. (I encountered this when doing a terraform destroy with a resource that had already been manually deleted). This also appears to be a bug in the Update method.

Stack trace from the terraform-provider-hcp_v0.72.0_x5 plugin:

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x2 addr=0x40 pc=0x102a99cd8]

goroutine 100 [running]:
github.com/hashicorp/terraform-provider-hcp/internal/provider.(*resourceVaultsecretsApp).Read(0x14000898020, {0x102f5d240, 0x140006137a0}, {{{{0x102f61f58, 0x14000354ed0}, {0x102db9ec0, 0x14000354ae0}}, {0x102f63398, 0x140005de230}}, 0x14000898030, ...}, ...)
	github.com/hashicorp/terraform-provider-hcp/internal/provider/resource_vault_secrets_app.go:134 +0x228
github.com/hashicorp/terraform-plugin-framework/internal/fwserver.(*Server).ReadResource(0x140000cc000, {0x102f5d240, 0x140006137a0}, 0x14000613830, 0x140008bd580)
	github.com/hashicorp/terraform-plugin-framework@v1.4.0/internal/fwserver/server_readresource.go:101 +0x508
github.com/hashicorp/terraform-plugin-framework/internal/proto5server.(*Server).ReadResource(0x140000cc000, {0x102f5d240?, 0x14000613650?}, 0x140000a7a40)
	github.com/hashicorp/terraform-plugin-framework@v1.4.0/internal/proto5server/server_readresource.go:56 +0x314
github.com/hashicorp/terraform-plugin-mux/tf5muxserver.(*muxServer).ReadResource(0x102f5d198?, {0x102f5d240?, 0x140006132c0?}, 0x140000a7a40)
	github.com/hashicorp/terraform-plugin-mux@v0.12.0/tf5muxserver/mux_server_ReadResource.go:35 +0x188
github.com/hashicorp/terraform-plugin-go/tfprotov5/tf5server.(*server).ReadResource(0x1400057d0e0, {0x102f5d240?, 0x140006125a0?}, 0x1400075c120)
	github.com/hashicorp/terraform-plugin-go@v0.19.0/tfprotov5/tf5server/server.go:789 +0x3e8
github.com/hashicorp/terraform-plugin-go/tfprotov5/internal/tfplugin5._Provider_ReadResource_Handler({0x102f0b880?, 0x1400057d0e0}, {0x102f5d240, 0x140006125a0}, 0x14000576700, 0x0)
	github.com/hashicorp/terraform-plugin-go@v0.19.0/tfprotov5/internal/tfplugin5/tfplugin5_grpc.pb.go:431 +0x170
google.golang.org/grpc.(*Server).processUnaryRPC(0x140002e6000, {0x102f622f8, 0x140005c29c0}, 0x140008a2000, 0x140002f8210, 0x10380ab68, 0x0)
	google.golang.org/grpc@v1.58.2/server.go:1376 +0xbe0
google.golang.org/grpc.(*Server).handleStream(0x140002e6000, {0x102f622f8, 0x140005c29c0}, 0x140008a2000, 0x0)
	google.golang.org/grpc@v1.58.2/server.go:1753 +0x82c
google.golang.org/grpc.(*Server).serveStreams.func1.1()
	google.golang.org/grpc@v1.58.2/server.go:998 +0x84
created by google.golang.org/grpc.(*Server).serveStreams.func1
	google.golang.org/grpc@v1.58.2/server.go:996 +0x16c

Error: The terraform-provider-hcp_v0.72.0_x5 plugin crashed!

🏗️ Acceptance tests

I looked into writing a test for this, but based on my understanding of the test harnesses, the terraform test helper .Test handles both the apply and the destroy. To properly verify the patch, the resource needs to be removed out-of-band of the terraform runs (this could probably be done as a final step of the Steps array, using the client, if I understand how the provider and test helpers are structured).

I wanted to get feedback on whether this patch is worthwhile, as well as any input on how you'd like to see it tested, before I got into the weeds on it.

@hashicorp-cla
Copy link

hashicorp-cla commented Oct 2, 2023

CLA assistant check
All committers have signed the CLA.

@atheken atheken marked this pull request as ready for review October 2, 2023 20:29
@atheken atheken requested a review from a team as a code owner October 2, 2023 20:29
@aidan-mundy aidan-mundy added bug Something isn't working go Pull requests that update Go code labels Oct 3, 2023
@AnPucel AnPucel self-assigned this Oct 4, 2023
@AnPucel
Copy link
Contributor

AnPucel commented Oct 6, 2023

Thanks for the PR and the catch! This was just an oversight. I think this is straightforward enough and falls outside the realm of the acceptance tests for now. I've also manually verified the changes.

@AnPucel AnPucel merged commit bef2e17 into hashicorp:main Oct 6, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working go Pull requests that update Go code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants