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

improve validation of provider RPC responses #31047

Closed
abergmeier opened this issue May 13, 2022 · 6 comments · Fixed by #31184
Closed

improve validation of provider RPC responses #31047

abergmeier opened this issue May 13, 2022 · 6 comments · Fixed by #31184

Comments

@abergmeier
Copy link

Terraform Version

Terraform v1.1.9
on linux_amd64

Terraform Configuration Files

resource "buildx_instance" "foo" {
  name = "test-basic"
  driver {
    name = "docker-container"
  }
  bootstrap = true
}

resource "buildx_built" "foo" {
  file    = "testdata/Containerfile"
  context = "."
  output {
    docker {
      dest = "image.docker"
    }
  }
  instance = buildx_instance.foo.name
}

Debug Output

See https://gist.github.com/abergmeier/fe55ad498453c30c85dde63aca2aa54b

Expected Behavior

It should not have crashed.

Actual Behavior

It crashes.

Steps to Reproduce

CGO_ENABLED=0 TF_ACC=1 TF_LOG=trace go test ./internal/resources/builttest -v

References

@abergmeier abergmeier added bug new new issue not yet triaged labels May 13, 2022
@jbardin
Copy link
Member

jbardin commented May 13, 2022

Hi @abergmeier,

Thanks for reporting the issue. While Terraform might be able to better validate the plugin return values and prevent the crash, the problem ultimately looks like an invalid schema returned by the provider.

Without any provider code to see, it's hard to determine what may be producing the invalid schema. If by "Framework Provider" you mean that this is using the terraform-plugin-framework, then I would start by filing an issue there with a reproduction case.

Thanks!

@jbardin jbardin added core and removed new new issue not yet triaged labels May 13, 2022
@jbardin jbardin changed the title Crash with Framework Provider improve validation of provider RPC responses May 13, 2022
@abergmeier
Copy link
Author

the problem ultimately looks like an invalid schema returned by the provider.

Well personally I would be expecting Terraform to be stable enough to not crash upon whatever the provider is doing "wrong" or even better give a proper error message in those cases.

@jbardin
Copy link
Member

jbardin commented May 13, 2022

@abergmeier, yes, both sides should be responsible for the correct protocol handling. I'm leaving this open as an issue to improve the validation in the Terraform CLI, but the fact that an invalid data structure was returned by the framework is possibly a bug there as well.

@abergmeier
Copy link
Author

So looking at CLI code it should be as easy as adding a check whether Block is nil and then gracefully producing an error.
Also if this check is omitted often in the codebase maybe a general sweep across usage of Protobuf types would be good so these kind of errors get squashed.

@bflad
Copy link
Contributor

bflad commented May 17, 2022

@jbardin additional details here: hashicorp/terraform-plugin-framework#326 (comment)

Outside the missing nil checks in the internal/plugin6/convert code, the GetProviderSchema receiving code should probably also ensure there was no error diagnostics from the provider before proceeding with conversion logic of potentially incomplete/invalid schemas.

bflad added a commit that referenced this issue Jun 3, 2022
Reference: #31047

Prevent potential panics and immediately return provider-defined errors diagnostics.

Previously:

```
--- FAIL: TestGRPCProvider_GetSchema_ResponseErrorDiagnostic (0.00s)
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
	panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x70 pc=0x17fa752]

goroutine 13 [running]:
testing.tRunner.func1.2({0x191a100, 0x2236330})
	/usr/local/Cellar/go/1.18.2/libexec/src/testing/testing.go:1389 +0x24e
testing.tRunner.func1()
	/usr/local/Cellar/go/1.18.2/libexec/src/testing/testing.go:1392 +0x39f
panic({0x191a100, 0x2236330})
	/usr/local/Cellar/go/1.18.2/libexec/src/runtime/panic.go:838 +0x207
github.com/hashicorp/terraform/internal/plugin6/convert.ProtoToConfigSchema(0x0)
	/Users/bflad/src/github.com/hashicorp/terraform/internal/plugin6/convert/schema.go:110 +0x52
github.com/hashicorp/terraform/internal/plugin6/convert.ProtoToProviderSchema(...)
	/Users/bflad/src/github.com/hashicorp/terraform/internal/plugin6/convert/schema.go:98
github.com/hashicorp/terraform/internal/plugin6.(*GRPCProvider).GetProviderSchema(0xc00004a200)
	/Users/bflad/src/github.com/hashicorp/terraform/internal/plugin6/grpc_provider.go:152 +0x29a
github.com/hashicorp/terraform/internal/plugin6.TestGRPCProvider_GetSchema_ResponseErrorDiagnostic(0x0?)
	/Users/bflad/src/github.com/hashicorp/terraform/internal/plugin6/grpc_provider_test.go:158 +0x265
testing.tRunner(0xc0001031e0, 0x1a733d8)
	/usr/local/Cellar/go/1.18.2/libexec/src/testing/testing.go:1439 +0x102
created by testing.(*T).Run
	/usr/local/Cellar/go/1.18.2/libexec/src/testing/testing.go:1486 +0x35f
```

Previously:

```
--- FAIL: TestGRPCProvider_GetSchema_ResponseErrorDiagnostic (0.00s)
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
	panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x70 pc=0x18a2732]

goroutine 7 [running]:
testing.tRunner.func1.2({0x1a5e720, 0x250be50})
	/usr/local/Cellar/go/1.18.2/libexec/src/testing/testing.go:1389 +0x24e
testing.tRunner.func1()
	/usr/local/Cellar/go/1.18.2/libexec/src/testing/testing.go:1392 +0x39f
panic({0x1a5e720, 0x250be50})
	/usr/local/Cellar/go/1.18.2/libexec/src/runtime/panic.go:838 +0x207
github.com/hashicorp/terraform/internal/plugin/convert.ProtoToConfigSchema(0x0)
	/Users/bflad/src/github.com/hashicorp/terraform/internal/plugin/convert/schema.go:104 +0x52
github.com/hashicorp/terraform/internal/plugin/convert.ProtoToProviderSchema(...)
	/Users/bflad/src/github.com/hashicorp/terraform/internal/plugin/convert/schema.go:92
github.com/hashicorp/terraform/internal/plugin.(*GRPCProvider).GetProviderSchema(0xc00004a600)
	/Users/bflad/src/github.com/hashicorp/terraform/internal/plugin/grpc_provider.go:149 +0x29a
github.com/hashicorp/terraform/internal/plugin.TestGRPCProvider_GetSchema_ResponseErrorDiagnostic(0x0?)
	/Users/bflad/src/github.com/hashicorp/terraform/internal/plugin/grpc_provider_test.go:130 +0x265
testing.tRunner(0xc0001031e0, 0x1be9500)
	/usr/local/Cellar/go/1.18.2/libexec/src/testing/testing.go:1439 +0x102
created by testing.(*T).Run
	/usr/local/Cellar/go/1.18.2/libexec/src/testing/testing.go:1486 +0x35f
```
@github-actions
Copy link

github-actions bot commented Jul 7, 2022

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 have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 7, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants