Skip to content

Commit 09a7167

Browse files
stephybunstephybun
committed
add validation to check if a resource identity is null after an apply or a read
1 parent 66313d0 commit 09a7167

File tree

3 files changed

+354
-0
lines changed

3 files changed

+354
-0
lines changed

‎helper/schema/grpc_provider.go‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -880,6 +880,7 @@ func (s *GRPCProviderServer) ReadResource(ctx context.Context, req *tfprotov5.Re
880880
resp.Diagnostics = convert.AppendProtoDiag(ctx, resp.Diagnostics, err)
881881
return resp, nil
882882
}
883+
883884
// Step 2: Turn cty.Value into flatmap representation
884885
identityAttrs := hcl2shim.FlatmapValueFromHCL2(currentIdentityVal)
885886
// Step 3: Well, set it in the instanceState
@@ -970,6 +971,24 @@ func (s *GRPCProviderServer) ReadResource(ctx context.Context, req *tfprotov5.Re
970971
return resp, nil
971972
}
972973

974+
if !res.ResourceBehavior.AllowNullIdentity {
975+
isFullyNull := true
976+
for _, v := range newIdentityVal.AsValueMap() {
977+
if !v.IsNull() {
978+
isFullyNull = false
979+
break
980+
}
981+
}
982+
983+
if isFullyNull {
984+
resp.Diagnostics = convert.AppendProtoDiag(ctx, resp.Diagnostics, fmt.Errorf(
985+
"Missing Resource Identity After Read: The Terraform provider unexpectedly returned no resource identity after having no errors in the resource read. "+
986+
"This is always a problem with the provider and should be reported to the provider developer",
987+
))
988+
return resp, nil
989+
}
990+
}
991+
973992
newIdentityMP, err := msgpack.Marshal(newIdentityVal, identityBlock.ImpliedType())
974993
if err != nil {
975994
resp.Diagnostics = convert.AppendProtoDiag(ctx, resp.Diagnostics, err)
@@ -1544,6 +1563,24 @@ func (s *GRPCProviderServer) ApplyResourceChange(ctx context.Context, req *tfpro
15441563
return resp, nil
15451564
}
15461565

1566+
if !res.ResourceBehavior.AllowNullIdentity {
1567+
isFullyNull := true
1568+
for _, v := range newIdentityVal.AsValueMap() {
1569+
if !v.IsNull() {
1570+
isFullyNull = false
1571+
break
1572+
}
1573+
}
1574+
1575+
if isFullyNull {
1576+
resp.Diagnostics = convert.AppendProtoDiag(ctx, resp.Diagnostics, fmt.Errorf(
1577+
"Missing Resource Identity After Create: The Terraform provider unexpectedly returned no resource identity after having no errors in the resource create. "+
1578+
"This is always a problem with the provider and should be reported to the provider developer",
1579+
))
1580+
return resp, nil
1581+
}
1582+
}
1583+
15471584
if !res.ResourceBehavior.MutableIdentity && !create && !plannedIdentityVal.IsNull() && !plannedIdentityVal.RawEquals(newIdentityVal) {
15481585
resp.Diagnostics = convert.AppendProtoDiag(ctx, resp.Diagnostics, fmt.Errorf(
15491586
"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: 312 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,221 @@ 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+
},
8434+
"create: null identity allowed in ApplyResourceChangeResponse": {
8435+
server: NewGRPCProviderServer(&Provider{
8436+
ResourcesMap: map[string]*Resource{
8437+
"test": {
8438+
ResourceBehavior: ResourceBehavior{
8439+
AllowNullIdentity: true,
8440+
},
8441+
SchemaVersion: 4,
8442+
CreateContext: func(_ context.Context, rd *ResourceData, _ interface{}) diag.Diagnostics {
8443+
rd.SetId("baz")
8444+
8445+
return nil
8446+
},
8447+
Schema: map[string]*Schema{},
8448+
Identity: &ResourceIdentity{
8449+
Version: 1,
8450+
SchemaFunc: func() map[string]*Schema {
8451+
return map[string]*Schema{
8452+
"subscription_id": {
8453+
Type: TypeString,
8454+
RequiredForImport: true,
8455+
},
8456+
"resource_group_name": {
8457+
Type: TypeString,
8458+
RequiredForImport: true,
8459+
},
8460+
"name": {
8461+
Type: TypeString,
8462+
RequiredForImport: true,
8463+
},
8464+
}
8465+
},
8466+
},
8467+
},
8468+
},
8469+
}),
8470+
req: &tfprotov5.ApplyResourceChangeRequest{
8471+
TypeName: "test",
8472+
PriorState: &tfprotov5.DynamicValue{
8473+
MsgPack: mustMsgpackMarshal(
8474+
cty.Object(map[string]cty.Type{}),
8475+
cty.NullVal(
8476+
cty.Object(map[string]cty.Type{}),
8477+
),
8478+
),
8479+
},
8480+
PlannedState: &tfprotov5.DynamicValue{
8481+
MsgPack: mustMsgpackMarshal(
8482+
cty.Object(map[string]cty.Type{
8483+
"id": cty.String,
8484+
}),
8485+
cty.ObjectVal(map[string]cty.Value{
8486+
"id": cty.UnknownVal(cty.String),
8487+
}),
8488+
),
8489+
},
8490+
PlannedIdentity: &tfprotov5.ResourceIdentityData{
8491+
IdentityData: &tfprotov5.DynamicValue{
8492+
MsgPack: mustMsgpackMarshal(
8493+
cty.Object(map[string]cty.Type{
8494+
"subscription_id": cty.String,
8495+
"resource_group_name": cty.String,
8496+
"name": cty.String,
8497+
}),
8498+
cty.ObjectVal(map[string]cty.Value{
8499+
"subscription_id": cty.NullVal(cty.String),
8500+
"resource_group_name": cty.NullVal(cty.String),
8501+
"name": cty.NullVal(cty.String),
8502+
}),
8503+
),
8504+
},
8505+
},
8506+
Config: &tfprotov5.DynamicValue{
8507+
MsgPack: mustMsgpackMarshal(
8508+
cty.Object(map[string]cty.Type{
8509+
"id": cty.String,
8510+
}),
8511+
cty.ObjectVal(map[string]cty.Value{
8512+
"id": cty.NullVal(cty.String),
8513+
}),
8514+
),
8515+
},
8516+
},
8517+
expected: &tfprotov5.ApplyResourceChangeResponse{
8518+
NewState: &tfprotov5.DynamicValue{
8519+
MsgPack: mustMsgpackMarshal(
8520+
cty.Object(map[string]cty.Type{
8521+
"id": cty.String,
8522+
}),
8523+
cty.ObjectVal(map[string]cty.Value{
8524+
"id": cty.StringVal("baz"),
8525+
}),
8526+
),
8527+
},
8528+
Private: []uint8(`{"schema_version":"4"}`),
8529+
UnsafeToUseLegacyTypeSystem: true,
8530+
NewIdentity: &tfprotov5.ResourceIdentityData{
8531+
IdentityData: &tfprotov5.DynamicValue{
8532+
MsgPack: mustMsgpackMarshal(
8533+
cty.Object(map[string]cty.Type{
8534+
"subscription_id": cty.String,
8535+
"resource_group_name": cty.String,
8536+
"name": cty.String,
8537+
}),
8538+
cty.ObjectVal(map[string]cty.Value{
8539+
"subscription_id": cty.NullVal(cty.String),
8540+
"resource_group_name": cty.NullVal(cty.String),
8541+
"name": cty.NullVal(cty.String),
8542+
}),
8543+
),
8544+
},
8545+
},
8546+
},
8547+
},
82368548
"create-resource-identity-may-change": {
82378549
server: NewGRPCProviderServer(&Provider{
82388550
ResourcesMap: map[string]*Resource{

‎helper/schema/resource.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -679,6 +679,11 @@ type ResourceBehavior struct {
679679
// resource's lifecycle. Setting this flag to true will disable the SDK validation that ensures identity
680680
// data doesn't change during RPC calls.
681681
MutableIdentity bool
682+
683+
// AllowNullIdentity toggles whether the managed resource allows identities to be null. Setting this flag to true
684+
// disables the SDK validation that ensures the identity cannot be null at the end of ApplyResourceChange and ReadResource
685+
// RPC calls in certain situations.
686+
AllowNullIdentity bool
682687
}
683688

684689
// ProviderDeferredBehavior enables provider-defined logic to be executed

0 commit comments

Comments
 (0)