From 79e1ca31d06a12529388aa9e1e7fb5e0bba28be7 Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Tue, 12 Oct 2021 11:59:03 -0400 Subject: [PATCH] helper/schema: Prevent panic with missing InstanceDiff in ReadDataSource Reference: https://github.com/hashicorp/terraform-plugin-sdk/issues/813 Previously: ``` --- FAIL: TestReadDataSource/optional-no-id (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=0x18 pc=0x12f3f64] goroutine 20 [running]: testing.tRunner.func1.2({0x1418300, 0x17d34d0}) /usr/local/Cellar/go/1.17.1/libexec/src/testing/testing.go:1209 +0x24e testing.tRunner.func1() /usr/local/Cellar/go/1.17.1/libexec/src/testing/testing.go:1212 +0x218 panic({0x1418300, 0x17d34d0}) /usr/local/Cellar/go/1.17.1/libexec/src/runtime/panic.go:1038 +0x215 github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*GRPCProviderServer).ReadDataSource(0xc00013ce58, {0x1583cb0, 0xc000128008}, 0xc0001626a0) /Users/bflad/src/github.com/hashicorp/terraform-plugin-sdk/helper/schema/grpc_provider.go:1126 +0x304 github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.TestReadDataSource.func9(0xc0002124e0) /Users/bflad/src/github.com/hashicorp/terraform-plugin-sdk/helper/schema/grpc_provider_test.go:1381 +0x6b ``` --- helper/schema/grpc_provider.go | 6 +- helper/schema/grpc_provider_test.go | 415 ++++++++++++++++++++++++++++ 2 files changed, 420 insertions(+), 1 deletion(-) diff --git a/helper/schema/grpc_provider.go b/helper/schema/grpc_provider.go index 0695fc648d6..4ed1253decf 100644 --- a/helper/schema/grpc_provider.go +++ b/helper/schema/grpc_provider.go @@ -1123,7 +1123,11 @@ func (s *GRPCProviderServer) ReadDataSource(ctx context.Context, req *tfprotov5. return resp, nil } - diff.RawConfig = configVal + // Not setting RawConfig here is okay, as ResourceData.GetRawConfig() + // will return a NullVal of the schema if there is no InstanceDiff. + if diff != nil { + diff.RawConfig = configVal + } // now we can get the new complete data source newInstanceState, diags := res.ReadDataApply(ctx, diff, s.provider.Meta()) diff --git a/helper/schema/grpc_provider_test.go b/helper/schema/grpc_provider_test.go index 9fbf8d30f5f..dac6b28c238 100644 --- a/helper/schema/grpc_provider_test.go +++ b/helper/schema/grpc_provider_test.go @@ -1006,6 +1006,401 @@ func TestApplyResourceChange_bigint(t *testing.T) { } } +func TestReadDataSource(t *testing.T) { + t.Parallel() + + testCases := map[string]struct { + server *GRPCProviderServer + req *tfprotov5.ReadDataSourceRequest + expected *tfprotov5.ReadDataSourceResponse + }{ + "missing-set-id": { + server: NewGRPCProviderServer(&Provider{ + DataSourcesMap: map[string]*Resource{ + "test": { + SchemaVersion: 1, + Schema: map[string]*Schema{ + "id": { + Type: TypeString, + Computed: true, + }, + }, + ReadContext: func(ctx context.Context, d *ResourceData, meta interface{}) diag.Diagnostics { + return nil + }, + }, + }, + }), + req: &tfprotov5.ReadDataSourceRequest{ + TypeName: "test", + Config: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + }), + cty.NullVal(cty.Object(map[string]cty.Type{ + "id": cty.String, + })), + ), + }, + }, + expected: &tfprotov5.ReadDataSourceResponse{ + State: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.NullVal(cty.String), + }), + ), + }, + }, + }, + "empty": { + server: NewGRPCProviderServer(&Provider{ + DataSourcesMap: map[string]*Resource{ + "test": { + SchemaVersion: 1, + Schema: map[string]*Schema{ + "id": { + Type: TypeString, + Computed: true, + }, + }, + ReadContext: func(ctx context.Context, d *ResourceData, meta interface{}) diag.Diagnostics { + d.SetId("test-id") + return nil + }, + }, + }, + }), + req: &tfprotov5.ReadDataSourceRequest{ + TypeName: "test", + Config: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.EmptyObject, + cty.NullVal(cty.EmptyObject), + ), + }, + }, + expected: &tfprotov5.ReadDataSourceResponse{ + State: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("test-id"), + }), + ), + }, + }, + }, + "null-object": { + server: NewGRPCProviderServer(&Provider{ + DataSourcesMap: map[string]*Resource{ + "test": { + SchemaVersion: 1, + Schema: map[string]*Schema{ + "id": { + Type: TypeString, + Computed: true, + }, + }, + ReadContext: func(ctx context.Context, d *ResourceData, meta interface{}) diag.Diagnostics { + d.SetId("test-id") + return nil + }, + }, + }, + }), + req: &tfprotov5.ReadDataSourceRequest{ + TypeName: "test", + Config: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + }), + cty.NullVal(cty.Object(map[string]cty.Type{ + "id": cty.String, + })), + ), + }, + }, + expected: &tfprotov5.ReadDataSourceResponse{ + State: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("test-id"), + }), + ), + }, + }, + }, + "computed-id": { + server: NewGRPCProviderServer(&Provider{ + DataSourcesMap: map[string]*Resource{ + "test": { + SchemaVersion: 1, + Schema: map[string]*Schema{ + "id": { + Type: TypeString, + Computed: true, + }, + }, + ReadContext: func(ctx context.Context, d *ResourceData, meta interface{}) diag.Diagnostics { + d.SetId("test-id") + return nil + }, + }, + }, + }), + req: &tfprotov5.ReadDataSourceRequest{ + TypeName: "test", + Config: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.NullVal(cty.String), + }), + ), + }, + }, + expected: &tfprotov5.ReadDataSourceResponse{ + State: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("test-id"), + }), + ), + }, + }, + }, + "optional-computed-id": { + server: NewGRPCProviderServer(&Provider{ + DataSourcesMap: map[string]*Resource{ + "test": { + SchemaVersion: 1, + Schema: map[string]*Schema{ + "id": { + Type: TypeString, + Optional: true, + Computed: true, + }, + }, + ReadContext: func(ctx context.Context, d *ResourceData, meta interface{}) diag.Diagnostics { + d.SetId("test-id") + return nil + }, + }, + }, + }), + req: &tfprotov5.ReadDataSourceRequest{ + TypeName: "test", + Config: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.NullVal(cty.String), + }), + ), + }, + }, + expected: &tfprotov5.ReadDataSourceResponse{ + State: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("test-id"), + }), + ), + }, + }, + }, + "optional-no-id": { + server: NewGRPCProviderServer(&Provider{ + DataSourcesMap: map[string]*Resource{ + "test": { + SchemaVersion: 1, + Schema: map[string]*Schema{ + "test": { + Type: TypeString, + Optional: true, + }, + }, + ReadContext: func(ctx context.Context, d *ResourceData, meta interface{}) diag.Diagnostics { + d.SetId("test-id") + return nil + }, + }, + }, + }), + req: &tfprotov5.ReadDataSourceRequest{ + TypeName: "test", + Config: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "test": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.NullVal(cty.String), + "test": cty.NullVal(cty.String), + }), + ), + }, + }, + expected: &tfprotov5.ReadDataSourceResponse{ + State: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "test": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("test-id"), + "test": cty.NullVal(cty.String), + }), + ), + }, + }, + }, + "required-id": { + server: NewGRPCProviderServer(&Provider{ + DataSourcesMap: map[string]*Resource{ + "test": { + SchemaVersion: 1, + Schema: map[string]*Schema{ + "id": { + Type: TypeString, + Required: true, + }, + }, + ReadContext: func(ctx context.Context, d *ResourceData, meta interface{}) diag.Diagnostics { + d.SetId("test-id") + return nil + }, + }, + }, + }), + req: &tfprotov5.ReadDataSourceRequest{ + TypeName: "test", + Config: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("test-id"), + }), + ), + }, + }, + expected: &tfprotov5.ReadDataSourceResponse{ + State: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("test-id"), + }), + ), + }, + }, + }, + "required-no-id": { + server: NewGRPCProviderServer(&Provider{ + DataSourcesMap: map[string]*Resource{ + "test": { + SchemaVersion: 1, + Schema: map[string]*Schema{ + "test": { + Type: TypeString, + Required: true, + }, + }, + ReadContext: func(ctx context.Context, d *ResourceData, meta interface{}) diag.Diagnostics { + d.SetId("test-id") + return nil + }, + }, + }, + }), + req: &tfprotov5.ReadDataSourceRequest{ + TypeName: "test", + Config: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "test": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.NullVal(cty.String), + "test": cty.StringVal("test-string"), + }), + ), + }, + }, + expected: &tfprotov5.ReadDataSourceResponse{ + State: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "test": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("test-id"), + "test": cty.StringVal("test-string"), + }), + ), + }, + }, + }, + } + + for name, testCase := range testCases { + name, testCase := name, testCase + + t.Run(name, func(t *testing.T) { + t.Parallel() + + resp, err := testCase.server.ReadDataSource(context.Background(), testCase.req) + + if err != nil { + t.Fatal(err) + } + + if diff := cmp.Diff(resp, testCase.expected, valueComparer); diff != "" { + ty := testCase.server.getDatasourceSchemaBlock("test").ImpliedType() + + if resp != nil && resp.State != nil { + t.Logf("resp.State.MsgPack: %s", mustMsgpackUnmarshal(ty, resp.State.MsgPack)) + } + + if testCase.expected != nil && testCase.expected.State != nil { + t.Logf("expected: %s", mustMsgpackUnmarshal(ty, testCase.expected.State.MsgPack)) + } + + t.Error(diff) + } + }) + } +} + func TestPrepareProviderConfig(t *testing.T) { for _, tc := range []struct { Name string @@ -2086,3 +2481,23 @@ func Test_pathToAttributePath_noSteps(t *testing.T) { t.Errorf("Expected nil attribute path, got %+v", res) } } + +func mustMsgpackMarshal(ty cty.Type, val cty.Value) []byte { + result, err := msgpack.Marshal(val, ty) + + if err != nil { + panic(fmt.Sprintf("cannot marshal msgpack: %s\n\ntype: %v\n\nvalue: %v", err, ty, val)) + } + + return result +} + +func mustMsgpackUnmarshal(ty cty.Type, b []byte) cty.Value { + result, err := msgpack.Unmarshal(b, ty) + + if err != nil { + panic(fmt.Sprintf("cannot unmarshal msgpack: %s\n\ntype: %v\n\nvalue: %v", err, ty, b)) + } + + return result +}