Skip to content

Commit 42d3ce1

Browse files
stephybunstephybun
authored
identity: add validation to check if a resource identity is null (#1513)
* add validation to check if a resource identity is null after an apply or a read * review comments
1 parent f982e31 commit 42d3ce1

File tree

2 files changed

+238
-0
lines changed

2 files changed

+238
-0
lines changed

‎helper/schema/grpc_provider.go‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"encoding/json"
99
"fmt"
1010
"strconv"
11+
"strings"
1112
"sync"
1213

1314
"github.com/hashicorp/go-cty/cty"
@@ -880,6 +881,7 @@ func (s *GRPCProviderServer) ReadResource(ctx context.Context, req *tfprotov5.Re
880881
resp.Diagnostics = convert.AppendProtoDiag(ctx, resp.Diagnostics, err)
881882
return resp, nil
882883
}
884+
883885
// Step 2: Turn cty.Value into flatmap representation
884886
identityAttrs := hcl2shim.FlatmapValueFromHCL2(currentIdentityVal)
885887
// Step 3: Well, set it in the instanceState
@@ -961,6 +963,22 @@ func (s *GRPCProviderServer) ReadResource(ctx context.Context, req *tfprotov5.Re
961963
return resp, nil
962964
}
963965

966+
isFullyNull := true
967+
for _, v := range newIdentityVal.AsValueMap() {
968+
if !v.IsNull() {
969+
isFullyNull = false
970+
break
971+
}
972+
}
973+
974+
if isFullyNull {
975+
resp.Diagnostics = convert.AppendProtoDiag(ctx, resp.Diagnostics, fmt.Errorf(
976+
"Missing Resource Identity After Read: The Terraform provider unexpectedly returned no resource identity after having no errors in the resource read. "+
977+
"This is always a problem with the provider and should be reported to the provider developer",
978+
))
979+
return resp, nil
980+
}
981+
964982
// If we're refreshing the resource state (excluding a recently imported resource), validate that the new identity isn't changing
965983
if !res.ResourceBehavior.MutableIdentity && !readFollowingImport && !currentIdentityVal.IsNull() && !currentIdentityVal.RawEquals(newIdentityVal) {
966984
resp.Diagnostics = convert.AppendProtoDiag(ctx, resp.Diagnostics, fmt.Errorf("Unexpected Identity Change: %s", "During the read operation, the Terraform Provider unexpectedly returned a different identity then the previously stored one.\n\n"+
@@ -1544,6 +1562,28 @@ func (s *GRPCProviderServer) ApplyResourceChange(ctx context.Context, req *tfpro
15441562
return resp, nil
15451563
}
15461564

1565+
isFullyNull := true
1566+
for _, v := range newIdentityVal.AsValueMap() {
1567+
if !v.IsNull() {
1568+
isFullyNull = false
1569+
break
1570+
}
1571+
}
1572+
1573+
if isFullyNull {
1574+
op := "Create"
1575+
if !create {
1576+
op = "Update"
1577+
}
1578+
1579+
resp.Diagnostics = convert.AppendProtoDiag(ctx, resp.Diagnostics, fmt.Errorf(
1580+
"Missing Resource Identity After %s: The Terraform provider unexpectedly returned no resource identity after having no errors in the resource %s. "+
1581+
"This is always a problem with the provider and should be reported to the provider developer", op, strings.ToLower(op),
1582+
))
1583+
1584+
return resp, nil
1585+
}
1586+
15471587
if !res.ResourceBehavior.MutableIdentity && !create && !plannedIdentityVal.IsNull() && !plannedIdentityVal.RawEquals(newIdentityVal) {
15481588
resp.Diagnostics = convert.AppendProtoDiag(ctx, resp.Diagnostics, fmt.Errorf(
15491589
"Unexpected Identity Change: During the update operation, the Terraform Provider unexpectedly returned a different identity than the previously stored one.\n\n"+

‎helper/schema/grpc_provider_test.go‎

Lines changed: 198 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5707,6 +5707,103 @@ func TestReadResource(t *testing.T) {
57075707
},
57085708
},
57095709
},
5710+
"prevent-null-identity": {
5711+
server: NewGRPCProviderServer(&Provider{
5712+
ResourcesMap: map[string]*Resource{
5713+
"test": {
5714+
SchemaVersion: 1,
5715+
Schema: map[string]*Schema{
5716+
"id": {
5717+
Type: TypeString,
5718+
Required: true,
5719+
},
5720+
"test": {
5721+
Type: TypeString,
5722+
},
5723+
},
5724+
Identity: &ResourceIdentity{
5725+
Version: 1,
5726+
SchemaFunc: func() map[string]*Schema {
5727+
return map[string]*Schema{
5728+
"subscription_id": {
5729+
Type: TypeString,
5730+
RequiredForImport: true,
5731+
},
5732+
"resource_group_name": {
5733+
Type: TypeString,
5734+
RequiredForImport: true,
5735+
},
5736+
"name": {
5737+
Type: TypeString,
5738+
RequiredForImport: true,
5739+
},
5740+
}
5741+
},
5742+
},
5743+
ReadContext: func(ctx context.Context, d *ResourceData, meta interface{}) diag.Diagnostics {
5744+
err := d.Set("test", "hello")
5745+
if err != nil {
5746+
return diag.FromErr(err)
5747+
}
5748+
5749+
return nil
5750+
},
5751+
},
5752+
},
5753+
}),
5754+
req: &tfprotov5.ReadResourceRequest{
5755+
TypeName: "test",
5756+
CurrentIdentity: &tfprotov5.ResourceIdentityData{
5757+
IdentityData: &tfprotov5.DynamicValue{
5758+
MsgPack: mustMsgpackMarshal(
5759+
cty.Object(map[string]cty.Type{
5760+
"subscription_id": cty.String,
5761+
"resource_group_name": cty.String,
5762+
"name": cty.String,
5763+
}),
5764+
cty.ObjectVal(map[string]cty.Value{
5765+
"subscription_id": cty.NullVal(cty.String),
5766+
"resource_group_name": cty.NullVal(cty.String),
5767+
"name": cty.NullVal(cty.String),
5768+
}),
5769+
),
5770+
},
5771+
},
5772+
CurrentState: &tfprotov5.DynamicValue{
5773+
MsgPack: mustMsgpackMarshal(
5774+
cty.Object(map[string]cty.Type{
5775+
"id": cty.String,
5776+
"test": cty.String,
5777+
}),
5778+
cty.ObjectVal(map[string]cty.Value{
5779+
"id": cty.StringVal("initial"),
5780+
"test": cty.UnknownVal(cty.String),
5781+
}),
5782+
),
5783+
},
5784+
},
5785+
expected: &tfprotov5.ReadResourceResponse{
5786+
NewState: &tfprotov5.DynamicValue{
5787+
MsgPack: mustMsgpackMarshal(
5788+
cty.Object(map[string]cty.Type{
5789+
"id": cty.String,
5790+
"test": cty.String,
5791+
}),
5792+
cty.ObjectVal(map[string]cty.Value{
5793+
"id": cty.StringVal("initial"),
5794+
"test": cty.StringVal("hello"),
5795+
}),
5796+
),
5797+
},
5798+
Diagnostics: []*tfprotov5.Diagnostic{
5799+
{
5800+
Severity: tfprotov5.DiagnosticSeverityError,
5801+
Summary: "Missing Resource Identity After Read: The Terraform provider unexpectedly returned no resource identity after having no errors in the resource read. " +
5802+
"This is always a problem with the provider and should be reported to the provider developer",
5803+
},
5804+
},
5805+
},
5806+
},
57105807
"update-resource-identity-may-not-change": {
57115808
server: NewGRPCProviderServer(&Provider{
57125809
ResourcesMap: map[string]*Resource{
@@ -8233,6 +8330,107 @@ func TestApplyResourceChange(t *testing.T) {
82338330
},
82348331
},
82358332
},
8333+
"create: null identity not allowed in ApplyResourceChangeResponse": {
8334+
server: NewGRPCProviderServer(&Provider{
8335+
ResourcesMap: map[string]*Resource{
8336+
"test": {
8337+
SchemaVersion: 4,
8338+
CreateContext: func(_ context.Context, rd *ResourceData, _ interface{}) diag.Diagnostics {
8339+
rd.SetId("baz")
8340+
8341+
return nil
8342+
},
8343+
Schema: map[string]*Schema{},
8344+
Identity: &ResourceIdentity{
8345+
Version: 1,
8346+
SchemaFunc: func() map[string]*Schema {
8347+
return map[string]*Schema{
8348+
"subscription_id": {
8349+
Type: TypeString,
8350+
RequiredForImport: true,
8351+
},
8352+
"resource_group_name": {
8353+
Type: TypeString,
8354+
RequiredForImport: true,
8355+
},
8356+
"name": {
8357+
Type: TypeString,
8358+
RequiredForImport: true,
8359+
},
8360+
}
8361+
},
8362+
},
8363+
},
8364+
},
8365+
}),
8366+
req: &tfprotov5.ApplyResourceChangeRequest{
8367+
TypeName: "test",
8368+
PriorState: &tfprotov5.DynamicValue{
8369+
MsgPack: mustMsgpackMarshal(
8370+
cty.Object(map[string]cty.Type{}),
8371+
cty.NullVal(
8372+
cty.Object(map[string]cty.Type{}),
8373+
),
8374+
),
8375+
},
8376+
PlannedState: &tfprotov5.DynamicValue{
8377+
MsgPack: mustMsgpackMarshal(
8378+
cty.Object(map[string]cty.Type{
8379+
"id": cty.String,
8380+
}),
8381+
cty.ObjectVal(map[string]cty.Value{
8382+
"id": cty.UnknownVal(cty.String),
8383+
}),
8384+
),
8385+
},
8386+
PlannedIdentity: &tfprotov5.ResourceIdentityData{
8387+
IdentityData: &tfprotov5.DynamicValue{
8388+
MsgPack: mustMsgpackMarshal(
8389+
cty.Object(map[string]cty.Type{
8390+
"subscription_id": cty.String,
8391+
"resource_group_name": cty.String,
8392+
"name": cty.String,
8393+
}),
8394+
cty.ObjectVal(map[string]cty.Value{
8395+
"subscription_id": cty.NullVal(cty.String),
8396+
"resource_group_name": cty.NullVal(cty.String),
8397+
"name": cty.NullVal(cty.String),
8398+
}),
8399+
),
8400+
},
8401+
},
8402+
Config: &tfprotov5.DynamicValue{
8403+
MsgPack: mustMsgpackMarshal(
8404+
cty.Object(map[string]cty.Type{
8405+
"id": cty.String,
8406+
}),
8407+
cty.ObjectVal(map[string]cty.Value{
8408+
"id": cty.NullVal(cty.String),
8409+
}),
8410+
),
8411+
},
8412+
},
8413+
expected: &tfprotov5.ApplyResourceChangeResponse{
8414+
NewState: &tfprotov5.DynamicValue{
8415+
MsgPack: mustMsgpackMarshal(
8416+
cty.Object(map[string]cty.Type{
8417+
"id": cty.String,
8418+
}),
8419+
cty.ObjectVal(map[string]cty.Value{
8420+
"id": cty.StringVal("baz"),
8421+
}),
8422+
),
8423+
},
8424+
Private: []uint8(`{"schema_version":"4"}`),
8425+
Diagnostics: []*tfprotov5.Diagnostic{
8426+
{
8427+
Severity: tfprotov5.DiagnosticSeverityError,
8428+
Summary: "Missing Resource Identity After Create: The Terraform provider unexpectedly returned no resource identity after having no errors in the resource create. " +
8429+
"This is always a problem with the provider and should be reported to the provider developer",
8430+
},
8431+
},
8432+
},
8433+
},
82368434
"create-resource-identity-may-change": {
82378435
server: NewGRPCProviderServer(&Provider{
82388436
ResourcesMap: map[string]*Resource{

0 commit comments

Comments
 (0)