Skip to content

Commit

Permalink
Return early on GetProviderSchema RPC responses with error diagnostics
Browse files Browse the repository at this point in the history 
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
```
  • Loading branch information
@bflad
bflad committed Jun 3, 2022
1 parent 472df96 commit 0b404f4
Show file tree
Hide file tree
Showing 4 changed files with 130 additions and 0 deletions.
4 changes: 4 additions & 0 deletions internal/plugin/grpc_provider.go
View file
Expand Up @@ -137,6 +137,10 @@ func (p *GRPCProvider) GetProviderSchema() (resp providers.GetProviderSchemaResp

resp.Diagnostics = resp.Diagnostics.Append(convert.ProtoToDiagnostics(protoResp.Diagnostics))

if resp.Diagnostics.HasErrors() {
return resp
}

if protoResp.Provider == nil {
resp.Diagnostics = resp.Diagnostics.Append(errors.New("missing provider schema"))
return resp
Expand Down
61 changes: 61 additions & 0 deletions internal/plugin/grpc_provider_test.go
View file
Expand Up @@ -39,6 +39,15 @@ func checkDiags(t *testing.T, d tfdiags.Diagnostics) {
}
}

// checkDiagsHasError ensures error diagnostics are present or fails the test.
func checkDiagsHasError(t *testing.T, d tfdiags.Diagnostics) {
t.Helper()

if !d.HasErrors() {
t.Fatal("expected error diagnostics")
}
}

func providerProtoSchema() *proto.GetProviderSchema_Response {
return &proto.GetProviderSchema_Response{
Provider: &proto.Schema{
Expand Down Expand Up @@ -92,6 +101,58 @@ func TestGRPCProvider_GetSchema(t *testing.T) {
checkDiags(t, resp.Diagnostics)
}

// Ensure that gRPC errors are returned early.
// Reference: https://github.com/hashicorp/terraform/issues/31047
func TestGRPCProvider_GetSchema_GRPCError(t *testing.T) {
ctrl := gomock.NewController(t)
client := mockproto.NewMockProviderClient(ctrl)

client.EXPECT().GetSchema(
gomock.Any(),
gomock.Any(),
gomock.Any(),
).Return(&proto.GetProviderSchema_Response{}, fmt.Errorf("test error"))

p := &GRPCProvider{
client: client,
}

resp := p.GetProviderSchema()

checkDiagsHasError(t, resp.Diagnostics)
}

// Ensure that provider error diagnostics are returned early.
// Reference: https://github.com/hashicorp/terraform/issues/31047
func TestGRPCProvider_GetSchema_ResponseErrorDiagnostic(t *testing.T) {
ctrl := gomock.NewController(t)
client := mockproto.NewMockProviderClient(ctrl)

client.EXPECT().GetSchema(
gomock.Any(),
gomock.Any(),
gomock.Any(),
).Return(&proto.GetProviderSchema_Response{
Diagnostics: []*proto.Diagnostic{
{
Severity: proto.Diagnostic_ERROR,
Summary: "error summary",
Detail: "error detail",
},
},
// Trigger potential panics
Provider: &proto.Schema{},
}, nil)

p := &GRPCProvider{
client: client,
}

resp := p.GetProviderSchema()

checkDiagsHasError(t, resp.Diagnostics)
}

func TestGRPCProvider_PrepareProviderConfig(t *testing.T) {
client := mockProviderClient(t)
p := &GRPCProvider{
Expand Down
4 changes: 4 additions & 0 deletions internal/plugin6/grpc_provider.go
View file
Expand Up @@ -144,6 +144,10 @@ func (p *GRPCProvider) GetProviderSchema() (resp providers.GetProviderSchemaResp

resp.Diagnostics = resp.Diagnostics.Append(convert.ProtoToDiagnostics(protoResp.Diagnostics))

if resp.Diagnostics.HasErrors() {
return resp
}

if protoResp.Provider == nil {
resp.Diagnostics = resp.Diagnostics.Append(errors.New("missing provider schema"))
return resp
Expand Down
61 changes: 61 additions & 0 deletions internal/plugin6/grpc_provider_test.go
View file
Expand Up @@ -46,6 +46,15 @@ func checkDiags(t *testing.T, d tfdiags.Diagnostics) {
}
}

// checkDiagsHasError ensures error diagnostics are present or fails the test.
func checkDiagsHasError(t *testing.T, d tfdiags.Diagnostics) {
t.Helper()

if !d.HasErrors() {
t.Fatal("expected error diagnostics")
}
}

func providerProtoSchema() *proto.GetProviderSchema_Response {
return &proto.GetProviderSchema_Response{
Provider: &proto.Schema{
Expand Down Expand Up @@ -99,6 +108,58 @@ func TestGRPCProvider_GetSchema(t *testing.T) {
checkDiags(t, resp.Diagnostics)
}

// Ensure that gRPC errors are returned early.
// Reference: https://github.com/hashicorp/terraform/issues/31047
func TestGRPCProvider_GetSchema_GRPCError(t *testing.T) {
ctrl := gomock.NewController(t)
client := mockproto.NewMockProviderClient(ctrl)

client.EXPECT().GetProviderSchema(
gomock.Any(),
gomock.Any(),
gomock.Any(),
).Return(&proto.GetProviderSchema_Response{}, fmt.Errorf("test error"))

p := &GRPCProvider{
client: client,
}

resp := p.GetProviderSchema()

checkDiagsHasError(t, resp.Diagnostics)
}

// Ensure that provider error diagnostics are returned early.
// Reference: https://github.com/hashicorp/terraform/issues/31047
func TestGRPCProvider_GetSchema_ResponseErrorDiagnostic(t *testing.T) {
ctrl := gomock.NewController(t)
client := mockproto.NewMockProviderClient(ctrl)

client.EXPECT().GetProviderSchema(
gomock.Any(),
gomock.Any(),
gomock.Any(),
).Return(&proto.GetProviderSchema_Response{
Diagnostics: []*proto.Diagnostic{
{
Severity: proto.Diagnostic_ERROR,
Summary: "error summary",
Detail: "error detail",
},
},
// Trigger potential panics
Provider: &proto.Schema{},
}, nil)

p := &GRPCProvider{
client: client,
}

resp := p.GetProviderSchema()

checkDiagsHasError(t, resp.Diagnostics)
}

func TestGRPCProvider_PrepareProviderConfig(t *testing.T) {
client := mockProviderClient(t)
p := &GRPCProvider{
Expand Down

0 comments on commit 0b404f4

Please sign in to comment.