Skip to content

Disallow null resource identities #1193

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Disallow null resource identities #1193

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ddafa01
wip: add a test with null identity
bbasata Jul 24, 2025
3193352
update: disallow null resource identities
bbasata Jul 24, 2025
c59ea5b
create: disallow null resource identities
bbasata Jul 24, 2025
8aef9b0
read: disallow null resource identities
bbasata Jul 28, 2025
94cc297
use https://github.com/hashicorp/terraform-plugin-go/pull/541
bbasata Jul 28, 2025
7a6de00
use terraform-plugin-go@main
bbasata Jul 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ toolchain go1.23.7

require (
github.com/google/go-cmp v0.7.0
github.com/hashicorp/terraform-plugin-go v0.29.0-alpha.1.0.20250717133739-e33a5336fb19
github.com/hashicorp/terraform-plugin-go v0.29.0-alpha.1.0.20250730161949-dfefeb84dbb0
github.com/hashicorp/terraform-plugin-log v0.9.0
)

Expand All @@ -28,7 +28,7 @@ require (
golang.org/x/net v0.41.0 // indirect
golang.org/x/sys v0.33.0 // indirect
golang.org/x/text v0.26.0 // indirect
google.golang.org/genproto/googleapis/rpc v0.0.0-20250324211829-b45e905df463 // indirect
google.golang.org/grpc v1.73.0 // indirect
google.golang.org/genproto/googleapis/rpc v0.0.0-20250528174236-200df99c418a // indirect
google.golang.org/grpc v1.74.2 // indirect
google.golang.org/protobuf v1.36.6 // indirect
)
36 changes: 18 additions & 18 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/fatih/color v1.13.0 h1:8LOYc1KYPPmyKMuN8QV2DNRWNbLo6LZ0iLs8+mlH53w=
github.com/fatih/color v1.13.0/go.mod h1:kLAiJbzzSOZDVNGyDpeOxJ47H46qBXwg5ILebYFFOfk=
github.com/go-logr/logr v1.4.2 h1:6pFjapn8bFcIbiKo3XT4j/BhANplGihG6tvd+8rYgrY=
github.com/go-logr/logr v1.4.2/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY=
github.com/go-logr/logr v1.4.3 h1:CjnDlHq8ikf6E492q6eKboGOC0T8CDaOvkHCIg8idEI=
github.com/go-logr/logr v1.4.3/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY=
github.com/go-logr/stdr v1.2.2 h1:hSWxHoqTgW2S2qGc0LTAI563KZ5YKYRhT3MFKZMbjag=
github.com/go-logr/stdr v1.2.2/go.mod h1:mMo/vtBO5dYbehREoey6XUKy/eSumjCCveDpRre4VKE=
github.com/golang/protobuf v1.5.4 h1:i7eJL8qZTpSEXOPTxNKhASYpMn+8e5Q6AdndVa1dWek=
Expand All @@ -21,8 +21,8 @@ github.com/hashicorp/go-plugin v1.6.3 h1:xgHB+ZUSYeuJi96WtxEjzi23uh7YQpznjGh0U0U
github.com/hashicorp/go-plugin v1.6.3/go.mod h1:MRobyh+Wc/nYy1V4KAXUiYfzxoYhs7V1mlH1Z7iY2h0=
github.com/hashicorp/go-uuid v1.0.3 h1:2gKiV6YVmrJ1i2CKKa9obLvRieoRGviZFL26PcT/Co8=
github.com/hashicorp/go-uuid v1.0.3/go.mod h1:6SBZvOh/SIDV7/2o3Jml5SYk/TvGqwFJ/bN7x4byOro=
github.com/hashicorp/terraform-plugin-go v0.29.0-alpha.1.0.20250717133739-e33a5336fb19 h1:P/ZVGEGXt9xSiLz+CrP/JzV2V8rtlE7994AX4jzcGB8=
github.com/hashicorp/terraform-plugin-go v0.29.0-alpha.1.0.20250717133739-e33a5336fb19/go.mod h1:hL//wLEfYo0YVt0TC/VLzia/ADQQto3HEm4/jX2gkdY=
github.com/hashicorp/terraform-plugin-go v0.29.0-alpha.1.0.20250730161949-dfefeb84dbb0 h1:fJenjDp71EUMUkerig4LCYQhjXuGA9VZ8ZIF5y6YRew=
github.com/hashicorp/terraform-plugin-go v0.29.0-alpha.1.0.20250730161949-dfefeb84dbb0/go.mod h1:5pww/UULn9C2tItq6o5sbScEkJxBUt9X9kI4DkeRsIw=
github.com/hashicorp/terraform-plugin-log v0.9.0 h1:i7hOA+vdAItN1/7UrfBqBwvYPQ9TFvymaRGZED3FCV0=
github.com/hashicorp/terraform-plugin-log v0.9.0/go.mod h1:rKL8egZQ/eXSyDqzLUuwUYLVdlYeamldAHSxjUFADow=
github.com/hashicorp/terraform-registry-address v0.3.0 h1:HMpK3nqaGFPS9VmgRXrJL/dzHNdheGVKk5k7VlFxzCo=
Expand Down Expand Up @@ -56,16 +56,16 @@ github.com/vmihailenco/tagparser/v2 v2.0.0 h1:y09buUbR+b5aycVFQs/g70pqKVZNBmxwAh
github.com/vmihailenco/tagparser/v2 v2.0.0/go.mod h1:Wri+At7QHww0WTrCBeu4J6bNtoV6mEfg5OIWRZA9qds=
go.opentelemetry.io/auto/sdk v1.1.0 h1:cH53jehLUN6UFLY71z+NDOiNJqDdPRaXzTel0sJySYA=
go.opentelemetry.io/auto/sdk v1.1.0/go.mod h1:3wSPjt5PWp2RhlCcmmOial7AvC4DQqZb7a7wCow3W8A=
go.opentelemetry.io/otel v1.35.0 h1:xKWKPxrxB6OtMCbmMY021CqC45J+3Onta9MqjhnusiQ=
go.opentelemetry.io/otel v1.35.0/go.mod h1:UEqy8Zp11hpkUrL73gSlELM0DupHoiq72dR+Zqel/+Y=
go.opentelemetry.io/otel/metric v1.35.0 h1:0znxYu2SNyuMSQT4Y9WDWej0VpcsxkuklLa4/siN90M=
go.opentelemetry.io/otel/metric v1.35.0/go.mod h1:nKVFgxBZ2fReX6IlyW28MgZojkoAkJGaE8CpgeAU3oE=
go.opentelemetry.io/otel/sdk v1.35.0 h1:iPctf8iprVySXSKJffSS79eOjl9pvxV9ZqOWT0QejKY=
go.opentelemetry.io/otel/sdk v1.35.0/go.mod h1:+ga1bZliga3DxJ3CQGg3updiaAJoNECOgJREo9KHGQg=
go.opentelemetry.io/otel/sdk/metric v1.35.0 h1:1RriWBmCKgkeHEhM7a2uMjMUfP7MsOF5JpUCaEqEI9o=
go.opentelemetry.io/otel/sdk/metric v1.35.0/go.mod h1:is6XYCUMpcKi+ZsOvfluY5YstFnhW0BidkR+gL+qN+w=
go.opentelemetry.io/otel/trace v1.35.0 h1:dPpEfJu1sDIqruz7BHFG3c7528f6ddfSWfFDVt/xgMs=
go.opentelemetry.io/otel/trace v1.35.0/go.mod h1:WUk7DtFp1Aw2MkvqGdwiXYDZZNvA/1J8o6xRXLrIkyc=
go.opentelemetry.io/otel v1.36.0 h1:UumtzIklRBY6cI/lllNZlALOF5nNIzJVb16APdvgTXg=
go.opentelemetry.io/otel v1.36.0/go.mod h1:/TcFMXYjyRNh8khOAO9ybYkqaDBb/70aVwkNML4pP8E=
go.opentelemetry.io/otel/metric v1.36.0 h1:MoWPKVhQvJ+eeXWHFBOPoBOi20jh6Iq2CcCREuTYufE=
go.opentelemetry.io/otel/metric v1.36.0/go.mod h1:zC7Ks+yeyJt4xig9DEw9kuUFe5C3zLbVjV2PzT6qzbs=
go.opentelemetry.io/otel/sdk v1.36.0 h1:b6SYIuLRs88ztox4EyrvRti80uXIFy+Sqzoh9kFULbs=
go.opentelemetry.io/otel/sdk v1.36.0/go.mod h1:+lC+mTgD+MUWfjJubi2vvXWcVxyr9rmlshZni72pXeY=
go.opentelemetry.io/otel/sdk/metric v1.36.0 h1:r0ntwwGosWGaa0CrSt8cuNuTcccMXERFwHX4dThiPis=
go.opentelemetry.io/otel/sdk/metric v1.36.0/go.mod h1:qTNOhFDfKRwX0yXOqJYegL5WRaW376QbB7P4Pb0qva4=
go.opentelemetry.io/otel/trace v1.36.0 h1:ahxWNuqZjpdiFAyrIoQ4GIiAIhxAunQR6MUoKrsNd4w=
go.opentelemetry.io/otel/trace v1.36.0/go.mod h1:gQ+OnDZzrybY4k4seLzPAWNwVBBVlF2szhehOBB/tGA=
golang.org/x/net v0.41.0 h1:vBTly1HeNPEn3wtREYfy4GZ/NECgw2Cnl+nK6Nz3uvw=
golang.org/x/net v0.41.0/go.mod h1:B/K4NNqkfmg07DQYrbwvSluqCJOOXwUjeb/5lOisjbA=
golang.org/x/sys v0.0.0-20200116001909-b77594299b42/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
Expand All @@ -78,10 +78,10 @@ golang.org/x/sys v0.33.0 h1:q3i8TbbEz+JRD9ywIRlyRAQbM0qF7hu24q3teo2hbuw=
golang.org/x/sys v0.33.0/go.mod h1:BJP2sWEmIv4KK5OTEluFJCKSidICx8ciO85XgH3Ak8k=
golang.org/x/text v0.26.0 h1:P42AVeLghgTYr4+xUnTRKDMqpar+PtX7KWuNQL21L8M=
golang.org/x/text v0.26.0/go.mod h1:QK15LZJUUQVJxhz7wXgxSy/CJaTFjd0G+YLonydOVQA=
google.golang.org/genproto/googleapis/rpc v0.0.0-20250324211829-b45e905df463 h1:e0AIkUUhxyBKh6ssZNrAMeqhA7RKUj42346d1y02i2g=
google.golang.org/genproto/googleapis/rpc v0.0.0-20250324211829-b45e905df463/go.mod h1:qQ0YXyHHx3XkvlzUtpXDkS29lDSafHMZBAZDc03LQ3A=
google.golang.org/grpc v1.73.0 h1:VIWSmpI2MegBtTuFt5/JWy2oXxtjJ/e89Z70ImfD2ok=
google.golang.org/grpc v1.73.0/go.mod h1:50sbHOUqWoCQGI8V2HQLJM0B+LMlIUjNSZmow7EVBQc=
google.golang.org/genproto/googleapis/rpc v0.0.0-20250528174236-200df99c418a h1:v2PbRU4K3llS09c7zodFpNePeamkAwG3mPrAery9VeE=
google.golang.org/genproto/googleapis/rpc v0.0.0-20250528174236-200df99c418a/go.mod h1:qQ0YXyHHx3XkvlzUtpXDkS29lDSafHMZBAZDc03LQ3A=
google.golang.org/grpc v1.74.2 h1:WoosgB65DlWVC9FqI82dGsZhWFNBSLjQ84bjROOpMu4=
google.golang.org/grpc v1.74.2/go.mod h1:CtQ+BGjaAIXHs/5YS3i473GqwBBa1zGQNevxdeBEXrM=
google.golang.org/protobuf v1.36.6 h1:z1NpPI8ku2WgiWnf+t9wTPsn6eP1L7ksHUlkfLvd9xY=
google.golang.org/protobuf v1.36.6/go.mod h1:jduwjTPXsFjZGTmRluh+L6NjiWu7pchiJ2/5YcXBHnY=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
Expand Down
11 changes: 11 additions & 0 deletions internal/fwserver/server_createresource.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,17 @@ func (s *Server) CreateResource(ctx context.Context, req *CreateResourceRequest,
return
}

if req.IdentitySchema != nil {
if resp.NewIdentity.Raw.IsFullyNull() {
resp.Diagnostics.AddError(
"Missing Resource Identity After Create",
"The Terraform Provider unexpectedly returned no resource identity data after having no errors in the resource create. "+
"This is always an issue in the Terraform Provider and should be reported to the provider developers.",
)
return
}
}

semanticEqualityReq := SchemaSemanticEqualityRequest{
PriorData: fwschemadata.Data{
Description: fwschemadata.DataDescriptionPlan,
Expand Down
100 changes: 100 additions & 0 deletions internal/fwserver/server_createresource_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -580,6 +580,106 @@ func TestServerCreateResource(t *testing.T) {
Private: testEmptyPrivate,
},
},
"response-invalid-nil-identity": {
server: &fwserver.Server{
Provider: &testprovider.Provider{},
},
request: &fwserver.CreateResourceRequest{
PlannedState: &tfsdk.Plan{
Raw: tftypes.NewValue(testSchemaType, map[string]tftypes.Value{
"test_computed": tftypes.NewValue(tftypes.String, nil),
"test_required": tftypes.NewValue(tftypes.String, "test-plannedstate-value"),
}),
Schema: testSchema,
},
IdentitySchema: testIdentitySchema,
ResourceSchema: testSchema,
Resource: &testprovider.ResourceWithIdentity{
Resource: &testprovider.Resource{
CreateMethod: func(ctx context.Context, req resource.CreateRequest, resp *resource.CreateResponse) {
resp.Identity.Raw = tftypes.NewValue(testIdentitySchema.Type().TerraformType(ctx), nil)
// Prevent missing resource state error diagnostic
var data testSchemaData

resp.Diagnostics.Append(req.Plan.Get(ctx, &data)...)
resp.Diagnostics.Append(resp.State.Set(ctx, &data)...)
},
},
},
},
expectedResponse: &fwserver.CreateResourceResponse{
Diagnostics: []diag.Diagnostic{
diag.NewErrorDiagnostic(
"Missing Resource Identity After Create",
"The Terraform Provider unexpectedly returned no resource identity data after having no errors in the resource create. "+
"This is always an issue in the Terraform Provider and should be reported to the provider developers.",
),
},
NewIdentity: &tfsdk.ResourceIdentity{
Raw: tftypes.NewValue(testIdentityType, nil),
Schema: testIdentitySchema,
},
NewState: &tfsdk.State{
Raw: tftypes.NewValue(testSchemaType, map[string]tftypes.Value{
"test_computed": tftypes.NewValue(tftypes.String, nil),
"test_required": tftypes.NewValue(tftypes.String, "test-plannedstate-value"),
}),
Schema: testSchema,
},
Private: testEmptyPrivate,
},
},
"response-invalid-null-identity": {
server: &fwserver.Server{
Provider: &testprovider.Provider{},
},
request: &fwserver.CreateResourceRequest{
PlannedState: &tfsdk.Plan{
Raw: tftypes.NewValue(testSchemaType, map[string]tftypes.Value{
"test_computed": tftypes.NewValue(tftypes.String, nil),
"test_required": tftypes.NewValue(tftypes.String, "test-plannedstate-value"),
}),
Schema: testSchema,
},
IdentitySchema: testIdentitySchema,
ResourceSchema: testSchema,
Resource: &testprovider.ResourceWithIdentity{
Resource: &testprovider.Resource{
CreateMethod: func(ctx context.Context, req resource.CreateRequest, resp *resource.CreateResponse) {
resp.Diagnostics.Append(resp.Identity.Set(ctx, testIdentitySchemaData{})...)
// Prevent missing resource state error diagnostic
var data testSchemaData

resp.Diagnostics.Append(req.Plan.Get(ctx, &data)...)
resp.Diagnostics.Append(resp.State.Set(ctx, &data)...)
},
},
},
},
expectedResponse: &fwserver.CreateResourceResponse{
Diagnostics: []diag.Diagnostic{
diag.NewErrorDiagnostic(
"Missing Resource Identity After Create",
"The Terraform Provider unexpectedly returned no resource identity data after having no errors in the resource create. "+
"This is always an issue in the Terraform Provider and should be reported to the provider developers.",
),
},
NewIdentity: &tfsdk.ResourceIdentity{
Raw: tftypes.NewValue(testIdentityType, map[string]tftypes.Value{
"test_id": tftypes.NewValue(tftypes.String, nil),
}),
Schema: testIdentitySchema,
},
NewState: &tfsdk.State{
Raw: tftypes.NewValue(testSchemaType, map[string]tftypes.Value{
"test_computed": tftypes.NewValue(tftypes.String, nil),
"test_required": tftypes.NewValue(tftypes.String, "test-plannedstate-value"),
}),
Schema: testSchema,
},
Private: testEmptyPrivate,
},
},
"response-invalid-newidentity": {
server: &fwserver.Server{
Provider: &testprovider.Provider{},
Expand Down
11 changes: 11 additions & 0 deletions internal/fwserver/server_readresource.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,17 @@ func (s *Server) ReadResource(ctx context.Context, req *ReadResourceRequest, res
}
}

if req.IdentitySchema != nil {
if resp.NewIdentity.Raw.IsFullyNull() {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💭 Thinking about whether or not we need to make an exception for readFollowingImport here. I think we still expect the provider to return a proper resource identity in order for the resource to be valid 🧐.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you're right. To repeat it back, I'd say what we're enforcing here is that if the provider indicates it supports identity (by providing a schema), it must be able to return said identity from a read, even if it was recently imported. Which sounds correct to me

readFollowingImport was introduced specifically to avoid validating the mutation of an identity due to the Terraform import methods using ReadResource with a stub state/identity. After the read is complete, the entire identity should be present or a diagnostic should be returned 👍🏻 . The same reason why we don't need an "updateFollowingImport", since the identity should be fully populated prior to that

resp.Diagnostics.AddError(
"Missing Resource Identity After Read",
"The Terraform Provider unexpectedly returned no resource identity data after having no errors in the resource read. "+
"This is always an issue in the Terraform Provider and should be reported to the provider developers.",
)
return
}
}

semanticEqualityReq := SchemaSemanticEqualityRequest{
PriorData: fwschemadata.Data{
Description: fwschemadata.DataDescriptionState,
Expand Down
34 changes: 34 additions & 0 deletions internal/fwserver/server_readresource_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,11 @@ func TestServerReadResource(t *testing.T) {
Schema: testIdentitySchema,
}

testEmptyIdentity := &tfsdk.ResourceIdentity{
Schema: testIdentitySchema,
Raw: tftypes.NewValue(testIdentitySchema.Type().TerraformType(context.Background()), nil),
}

testNewStateRemoved := &tfsdk.State{
Raw: tftypes.NewValue(testType, nil),
Schema: testSchema,
Expand Down Expand Up @@ -636,6 +641,35 @@ func TestServerReadResource(t *testing.T) {
Private: testEmptyPrivate,
},
},
"response-invalid-nil-identity": {
server: &fwserver.Server{
Provider: &testprovider.Provider{},
},
request: &fwserver.ReadResourceRequest{
CurrentState: testCurrentState,
CurrentIdentity: nil,
IdentitySchema: testIdentitySchema,
Resource: &testprovider.ResourceWithIdentity{
Resource: &testprovider.Resource{
ReadMethod: func(ctx context.Context, req resource.ReadRequest, resp *resource.ReadResponse) {
resp.Identity = req.Identity
},
},
},
},
expectedResponse: &fwserver.ReadResourceResponse{
Diagnostics: diag.Diagnostics{
diag.NewErrorDiagnostic(
"Missing Resource Identity After Read",
"The Terraform Provider unexpectedly returned no resource identity data after having no errors in the resource read. "+
"This is always an issue in the Terraform Provider and should be reported to the provider developers.",
),
},
NewState: testCurrentState,
NewIdentity: testEmptyIdentity,
Private: testEmptyPrivate,
},
},
"response-identity-valid-update-null-currentidentity": {
server: &fwserver.Server{
Provider: &testprovider.Provider{},
Expand Down
11 changes: 11 additions & 0 deletions internal/fwserver/server_updateresource.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,17 @@ func (s *Server) UpdateResource(ctx context.Context, req *UpdateResourceRequest,
}
}

if req.IdentitySchema != nil {
if resp.NewIdentity.Raw.IsFullyNull() {
resp.Diagnostics.AddError(
"Missing Resource Identity After Update",
"The Terraform Provider unexpectedly returned no resource identity data after having no errors in the resource update. "+
"This is always an issue in the Terraform Provider and should be reported to the provider developers.",
)
return
}
}

semanticEqualityReq := SchemaSemanticEqualityRequest{
PriorData: fwschemadata.Data{
Description: fwschemadata.DataDescriptionPlan,
Expand Down
Loading
Loading