Merge pull request #31184 from hashicorp/bflad-GetProviderSchema-earl…

…y-diag-return Return early on GetProviderSchema RPC responses with error diagnostics
hashicorp · Jun 6, 2022 · 82f47ca · 82f47ca
2 parents 472df96 + 0b404f4
commit 82f47ca
Show file tree

Hide file tree

Showing 4 changed files with 130 additions and 0 deletions.
diff --git a/internal/plugin/grpc_provider.go b/internal/plugin/grpc_provider.go
@@ -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

diff --git a/internal/plugin/grpc_provider_test.go b/internal/plugin/grpc_provider_test.go
@@ -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{
@@ -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{

diff --git a/internal/plugin6/grpc_provider.go b/internal/plugin6/grpc_provider.go
@@ -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

diff --git a/internal/plugin6/grpc_provider_test.go b/internal/plugin6/grpc_provider_test.go
@@ -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{
@@ -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{