Skip to content

Commit 7057933

Browse files
stephybunstephybun
committed
review comments
1 parent 09a7167 commit 7057933

File tree

3 files changed

+34
-150
lines changed

3 files changed

+34
-150
lines changed

‎helper/schema/grpc_provider.go‎

Lines changed: 34 additions & 31 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"
@@ -962,6 +963,22 @@ func (s *GRPCProviderServer) ReadResource(ctx context.Context, req *tfprotov5.Re
962963
return resp, nil
963964
}
964965

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+
965982
// If we're refreshing the resource state (excluding a recently imported resource), validate that the new identity isn't changing
966983
if !res.ResourceBehavior.MutableIdentity && !readFollowingImport && !currentIdentityVal.IsNull() && !currentIdentityVal.RawEquals(newIdentityVal) {
967984
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"+
@@ -971,24 +988,6 @@ func (s *GRPCProviderServer) ReadResource(ctx context.Context, req *tfprotov5.Re
971988
return resp, nil
972989
}
973990

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-
992991
newIdentityMP, err := msgpack.Marshal(newIdentityVal, identityBlock.ImpliedType())
993992
if err != nil {
994993
resp.Diagnostics = convert.AppendProtoDiag(ctx, resp.Diagnostics, err)
@@ -1563,22 +1562,26 @@ func (s *GRPCProviderServer) ApplyResourceChange(ctx context.Context, req *tfpro
15631562
return resp, nil
15641563
}
15651564

1566-
if !res.ResourceBehavior.AllowNullIdentity {
1567-
isFullyNull := true
1568-
for _, v := range newIdentityVal.AsValueMap() {
1569-
if !v.IsNull() {
1570-
isFullyNull = false
1571-
break
1572-
}
1565+
isFullyNull := true
1566+
for _, v := range newIdentityVal.AsValueMap() {
1567+
if !v.IsNull() {
1568+
isFullyNull = false
1569+
break
15731570
}
1571+
}
15741572

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
1573+
if isFullyNull {
1574+
op := "Create"
1575+
if !create {
1576+
op = "Update"
15811577
}
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
15821585
}
15831586

15841587
if !res.ResourceBehavior.MutableIdentity && !create && !plannedIdentityVal.IsNull() && !plannedIdentityVal.RawEquals(newIdentityVal) {

‎helper/schema/grpc_provider_test.go‎

Lines changed: 0 additions & 114 deletions
Original file line numberDiff line numberDiff line change
@@ -8431,120 +8431,6 @@ func TestApplyResourceChange(t *testing.T) {
84318431
},
84328432
},
84338433
},
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-
},
85488434
"create-resource-identity-may-change": {
85498435
server: NewGRPCProviderServer(&Provider{
85508436
ResourcesMap: map[string]*Resource{

‎helper/schema/resource.go‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -679,11 +679,6 @@ 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
687682
}
688683

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

0 commit comments

Comments
 (0)