Skip to content

Commit f73d27e

Browse files
SBGoodsSBGoods
committed
Add write-only value nullification to ReadResource, ImportResourceState, UpgradeResourceState, and MoveResourceState RPCs
1 parent 5c9c15c commit f73d27e

11 files changed

+408
-9
lines changed

‎internal/fwserver/server_createresource.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ func (s *Server) CreateResource(ctx context.Context, req *CreateResourceRequest,
163163
}
164164

165165
// Set any write-only attributes in the state to null
166-
modifiedState, err := tftypes.Transform(resp.NewState.Raw, NullifyWriteOnlyAttributes(ctx, req.ResourceSchema))
166+
modifiedState, err := tftypes.Transform(resp.NewState.Raw, NullifyWriteOnlyAttributes(ctx, resp.NewState.Schema))
167167
if err != nil {
168168
resp.Diagnostics.AddError(
169169
"Error Modifying State",

‎internal/fwserver/server_importresourcestate.go‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,18 @@ func (s *Server) ImportResourceState(ctx context.Context, req *ImportResourceSta
142142
return
143143
}
144144

145+
// Set any write-only attributes in the import state to null
146+
modifiedState, err := tftypes.Transform(importResp.State.Raw, NullifyWriteOnlyAttributes(ctx, importResp.State.Schema))
147+
if err != nil {
148+
resp.Diagnostics.AddError(
149+
"Error Modifying Import State",
150+
"There was an unexpected error modifying the Import State. This is always a problem with the provider. Please report the following to the provider developer:\n\n"+err.Error(),
151+
)
152+
return
153+
}
154+
155+
importResp.State.Raw = modifiedState
156+
145157
if importResp.State.Raw.Equal(req.EmptyState.Raw) {
146158
resp.Diagnostics.AddError(
147159
"Missing Resource Import State",

‎internal/fwserver/server_importresourcestate_test.go‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,12 +33,26 @@ func TestServerImportResourceState(t *testing.T) {
3333
},
3434
}
3535

36+
testTypeWriteOnly := tftypes.Object{
37+
AttributeTypes: map[string]tftypes.Type{
38+
"id": tftypes.String,
39+
"write-only": tftypes.String,
40+
"required": tftypes.String,
41+
},
42+
}
43+
3644
testEmptyStateValue := tftypes.NewValue(testType, map[string]tftypes.Value{
3745
"id": tftypes.NewValue(tftypes.String, nil),
3846
"optional": tftypes.NewValue(tftypes.String, nil),
3947
"required": tftypes.NewValue(tftypes.String, nil),
4048
})
4149

50+
testEmptyStateValueWriteOnly := tftypes.NewValue(testTypeWriteOnly, map[string]tftypes.Value{
51+
"id": tftypes.NewValue(tftypes.String, nil),
52+
"write-only": tftypes.NewValue(tftypes.String, nil),
53+
"required": tftypes.NewValue(tftypes.String, nil),
54+
})
55+
4256
testUnknownStateValue := tftypes.NewValue(testType, tftypes.UnknownValue)
4357

4458
testStateValue := tftypes.NewValue(testType, map[string]tftypes.Value{
@@ -61,11 +75,31 @@ func TestServerImportResourceState(t *testing.T) {
6175
},
6276
}
6377

78+
testSchemaWriteOnly := schema.Schema{
79+
Attributes: map[string]schema.Attribute{
80+
"id": schema.StringAttribute{
81+
Computed: true,
82+
},
83+
"write-only": schema.StringAttribute{
84+
Optional: true,
85+
WriteOnly: true,
86+
},
87+
"required": schema.StringAttribute{
88+
Required: true,
89+
},
90+
},
91+
}
92+
6493
testEmptyState := &tfsdk.State{
6594
Raw: testEmptyStateValue,
6695
Schema: testSchema,
6796
}
6897

98+
testEmptyStateWriteOnly := &tfsdk.State{
99+
Raw: testEmptyStateValueWriteOnly,
100+
Schema: testSchemaWriteOnly,
101+
}
102+
69103
testUnknownState := &tfsdk.State{
70104
Raw: testUnknownStateValue,
71105
Schema: testSchema,
@@ -416,6 +450,39 @@ func TestServerImportResourceState(t *testing.T) {
416450
},
417451
},
418452
},
453+
"response-importedresources-write-only-nullification": {
454+
server: &fwserver.Server{
455+
Provider: &testprovider.Provider{},
456+
},
457+
request: &fwserver.ImportResourceStateRequest{
458+
EmptyState: *testEmptyStateWriteOnly,
459+
ID: "test-id",
460+
Resource: &testprovider.ResourceWithImportState{
461+
Resource: &testprovider.Resource{},
462+
ImportStateMethod: func(ctx context.Context, req resource.ImportStateRequest, resp *resource.ImportStateResponse) {
463+
resp.Diagnostics.Append(resp.State.SetAttribute(ctx, path.Root("write-only"), "write-only-val")...)
464+
resource.ImportStatePassthroughID(ctx, path.Root("id"), req, resp)
465+
},
466+
},
467+
TypeName: "test_resource",
468+
},
469+
expectedResponse: &fwserver.ImportResourceStateResponse{
470+
ImportedResources: []fwserver.ImportedResource{
471+
{
472+
State: tfsdk.State{
473+
Raw: tftypes.NewValue(testTypeWriteOnly, map[string]tftypes.Value{
474+
"id": tftypes.NewValue(tftypes.String, "test-id"),
475+
"write-only": tftypes.NewValue(tftypes.String, nil),
476+
"required": tftypes.NewValue(tftypes.String, nil),
477+
}),
478+
Schema: testSchemaWriteOnly,
479+
},
480+
TypeName: "test_resource",
481+
Private: testEmptyPrivate,
482+
},
483+
},
484+
},
485+
},
419486
}
420487

421488
for name, testCase := range testCases {

‎internal/fwserver/server_moveresourcestate.go‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,18 @@ func (s *Server) MoveResourceState(ctx context.Context, req *MoveResourceStateRe
205205
return
206206
}
207207

208+
// Set any write-only attributes in the move resource state to null
209+
modifiedState, err := tftypes.Transform(moveStateResp.TargetState.Raw, NullifyWriteOnlyAttributes(ctx, moveStateResp.TargetState.Schema))
210+
if err != nil {
211+
resp.Diagnostics.AddError(
212+
"Error Modifying Move Resource State",
213+
"There was an unexpected error modifying the Move Resource State. This is always a problem with the provider. Please report the following to the provider developer:\n\n"+err.Error(),
214+
)
215+
return
216+
}
217+
218+
moveStateResp.TargetState.Raw = modifiedState
219+
208220
// If the implement has set the state in any way, return the response.
209221
if !moveStateResp.TargetState.Raw.Equal(tftypes.NewValue(req.TargetResourceSchema.Type().TerraformType(ctx), nil)) {
210222
resp.Diagnostics = moveStateResp.Diagnostics

‎internal/fwserver/server_moveresourcestate_test.go‎

Lines changed: 56 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ import (
99
"testing"
1010

1111
"github.com/google/go-cmp/cmp"
12+
"github.com/hashicorp/terraform-plugin-go/tftypes"
13+
1214
"github.com/hashicorp/terraform-plugin-framework/diag"
1315
"github.com/hashicorp/terraform-plugin-framework/internal/fwserver"
1416
"github.com/hashicorp/terraform-plugin-framework/internal/privatestate"
@@ -18,7 +20,6 @@ import (
1820
"github.com/hashicorp/terraform-plugin-framework/resource/schema"
1921
"github.com/hashicorp/terraform-plugin-framework/tfsdk"
2022
"github.com/hashicorp/terraform-plugin-framework/types"
21-
"github.com/hashicorp/terraform-plugin-go/tftypes"
2223
)
2324

2425
func TestServerMoveResourceState(t *testing.T) {
@@ -40,6 +41,22 @@ func TestServerMoveResourceState(t *testing.T) {
4041
}
4142
schemaType := testSchema.Type().TerraformType(ctx)
4243

44+
testSchemaWriteOnly := schema.Schema{
45+
Attributes: map[string]schema.Attribute{
46+
"id": schema.StringAttribute{
47+
Computed: true,
48+
},
49+
"write_only_attribute": schema.StringAttribute{
50+
Optional: true,
51+
WriteOnly: true,
52+
},
53+
"required_attribute": schema.StringAttribute{
54+
Required: true,
55+
},
56+
},
57+
}
58+
schemaTypeWriteOnly := testSchemaWriteOnly.Type().TerraformType(ctx)
59+
4360
testCases := map[string]struct {
4461
server *fwserver.Server
4562
request *fwserver.MoveResourceStateRequest
@@ -757,6 +774,44 @@ func TestServerMoveResourceState(t *testing.T) {
757774
},
758775
},
759776
},
777+
"response-TargetState-write-only-nullification": {
778+
server: &fwserver.Server{
779+
Provider: &testprovider.Provider{},
780+
},
781+
request: &fwserver.MoveResourceStateRequest{
782+
SourceRawState: testNewRawState(t, map[string]interface{}{
783+
"id": "test-id-value",
784+
"write_only_attribute": nil,
785+
"required_attribute": true,
786+
}),
787+
TargetResource: &testprovider.ResourceWithMoveState{
788+
MoveStateMethod: func(ctx context.Context) []resource.StateMover {
789+
return []resource.StateMover{
790+
{
791+
StateMover: func(_ context.Context, req resource.MoveStateRequest, resp *resource.MoveStateResponse) {
792+
resp.Diagnostics.Append(resp.TargetState.SetAttribute(ctx, path.Root("id"), "test-id-value")...)
793+
resp.Diagnostics.Append(resp.TargetState.SetAttribute(ctx, path.Root("write_only_attribute"), "movestate-val")...)
794+
resp.Diagnostics.Append(resp.TargetState.SetAttribute(ctx, path.Root("required_attribute"), "true")...)
795+
},
796+
},
797+
}
798+
},
799+
},
800+
TargetResourceSchema: testSchemaWriteOnly,
801+
TargetTypeName: "test_resource",
802+
},
803+
expectedResponse: &fwserver.MoveResourceStateResponse{
804+
TargetPrivate: privatestate.EmptyData(ctx),
805+
TargetState: &tfsdk.State{
806+
Raw: tftypes.NewValue(schemaTypeWriteOnly, map[string]tftypes.Value{
807+
"id": tftypes.NewValue(tftypes.String, "test-id-value"),
808+
"write_only_attribute": tftypes.NewValue(tftypes.String, nil),
809+
"required_attribute": tftypes.NewValue(tftypes.String, "true"),
810+
}),
811+
Schema: testSchemaWriteOnly,
812+
},
813+
},
814+
},
760815
}
761816

762817
for name, testCase := range testCases {

‎internal/fwserver/server_planresourcechange.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -341,7 +341,7 @@ func (s *Server) PlanResourceChange(ctx context.Context, req *PlanResourceChange
341341
}
342342

343343
// Set any write-only attributes in the plan to null
344-
modifiedPlan, err := tftypes.Transform(resp.PlannedState.Raw, NullifyWriteOnlyAttributes(ctx, req.ResourceSchema))
344+
modifiedPlan, err := tftypes.Transform(resp.PlannedState.Raw, NullifyWriteOnlyAttributes(ctx, resp.PlannedState.Schema))
345345
if err != nil {
346346
resp.Diagnostics.AddError(
347347
"Error Modifying Planned State",

‎internal/fwserver/server_readresource.go‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ package fwserver
66
import (
77
"context"
88

9+
"github.com/hashicorp/terraform-plugin-go/tftypes"
10+
911
"github.com/hashicorp/terraform-plugin-framework/diag"
1012
"github.com/hashicorp/terraform-plugin-framework/internal/fwschemadata"
1113
"github.com/hashicorp/terraform-plugin-framework/internal/logging"
@@ -157,11 +159,21 @@ func (s *Server) ReadResource(ctx context.Context, req *ReadResourceRequest, res
157159
return
158160
}
159161

160-
if semanticEqualityResp.NewData.TerraformValue.Equal(resp.NewState.Raw) {
161-
return
162+
if !semanticEqualityResp.NewData.TerraformValue.Equal(resp.NewState.Raw) {
163+
logging.FrameworkDebug(ctx, "State updated due to semantic equality")
164+
165+
resp.NewState.Raw = semanticEqualityResp.NewData.TerraformValue
162166
}
163167

164-
logging.FrameworkDebug(ctx, "State updated due to semantic equality")
168+
// Set any write-only attributes in the state to null
169+
modifiedState, err := tftypes.Transform(resp.NewState.Raw, NullifyWriteOnlyAttributes(ctx, resp.NewState.Schema))
170+
if err != nil {
171+
resp.Diagnostics.AddError(
172+
"Error Modifying State",
173+
"There was an unexpected error modifying the NewState. This is always a problem with the provider. Please report the following to the provider developer:\n\n"+err.Error(),
174+
)
175+
return
176+
}
165177

166-
resp.NewState.Raw = semanticEqualityResp.NewData.TerraformValue
178+
resp.NewState.Raw = modifiedState
167179
}

‎internal/fwserver/server_readresource_test.go‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,11 +34,23 @@ func TestServerReadResource(t *testing.T) {
3434
},
3535
}
3636

37+
testTypeWriteOnly := tftypes.Object{
38+
AttributeTypes: map[string]tftypes.Type{
39+
"test_write_only": tftypes.String,
40+
"test_required": tftypes.String,
41+
},
42+
}
43+
3744
testCurrentStateValue := tftypes.NewValue(testType, map[string]tftypes.Value{
3845
"test_computed": tftypes.NewValue(tftypes.String, nil),
3946
"test_required": tftypes.NewValue(tftypes.String, "test-currentstate-value"),
4047
})
4148

49+
testCurrentStateValueWriteOnly := tftypes.NewValue(testTypeWriteOnly, map[string]tftypes.Value{
50+
"test_write_only": tftypes.NewValue(tftypes.String, nil),
51+
"test_required": tftypes.NewValue(tftypes.String, "test-currentstate-value"),
52+
})
53+
4254
testNewStateValue := tftypes.NewValue(testType, map[string]tftypes.Value{
4355
"test_computed": tftypes.NewValue(tftypes.String, "test-newstate-value"),
4456
"test_required": tftypes.NewValue(tftypes.String, "test-currentstate-value"),
@@ -55,6 +67,18 @@ func TestServerReadResource(t *testing.T) {
5567
},
5668
}
5769

70+
testSchemaWriteOnly := schema.Schema{
71+
Attributes: map[string]schema.Attribute{
72+
"test_write_only": schema.StringAttribute{
73+
Optional: true,
74+
WriteOnly: true,
75+
},
76+
"test_required": schema.StringAttribute{
77+
Required: true,
78+
},
79+
},
80+
}
81+
5882
testSchemaWithSemanticEquals := schema.Schema{
5983
Attributes: map[string]schema.Attribute{
6084
"test_computed": schema.StringAttribute{
@@ -97,6 +121,11 @@ func TestServerReadResource(t *testing.T) {
97121
Schema: testSchema,
98122
}
99123

124+
testCurrentStateWriteOnly := &tfsdk.State{
125+
Raw: testCurrentStateValueWriteOnly,
126+
Schema: testSchemaWriteOnly,
127+
}
128+
100129
testNewState := &tfsdk.State{
101130
Raw: testNewStateValue,
102131
Schema: testSchema,
@@ -562,6 +591,38 @@ func TestServerReadResource(t *testing.T) {
562591
Private: testEmptyPrivate,
563592
},
564593
},
594+
"response-state-write-only-nullification": {
595+
server: &fwserver.Server{
596+
Provider: &testprovider.Provider{},
597+
},
598+
request: &fwserver.ReadResourceRequest{
599+
CurrentState: testCurrentStateWriteOnly,
600+
Resource: &testprovider.Resource{
601+
ReadMethod: func(ctx context.Context, req resource.ReadRequest, resp *resource.ReadResponse) {
602+
var data struct {
603+
TestWriteOnly types.String `tfsdk:"test_write_only"`
604+
TestRequired types.String `tfsdk:"test_required"`
605+
}
606+
607+
resp.Diagnostics.Append(req.State.Get(ctx, &data)...)
608+
609+
data.TestWriteOnly = types.StringValue("test-write-only-value")
610+
611+
resp.Diagnostics.Append(resp.State.Set(ctx, &data)...)
612+
},
613+
},
614+
},
615+
expectedResponse: &fwserver.ReadResourceResponse{
616+
NewState: &tfsdk.State{
617+
Raw: tftypes.NewValue(testTypeWriteOnly, map[string]tftypes.Value{
618+
"test_write_only": tftypes.NewValue(tftypes.String, nil),
619+
"test_required": tftypes.NewValue(tftypes.String, "test-currentstate-value"),
620+
}),
621+
Schema: testSchemaWriteOnly,
622+
},
623+
Private: testEmptyPrivate,
624+
},
625+
},
565626
"response-private": {
566627
server: &fwserver.Server{
567628
Provider: &testprovider.Provider{},

‎internal/fwserver/server_updateresource.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ func (s *Server) UpdateResource(ctx context.Context, req *UpdateResourceRequest,
176176
}
177177

178178
// Set any write-only attributes in the state to null
179-
modifiedState, err := tftypes.Transform(resp.NewState.Raw, NullifyWriteOnlyAttributes(ctx, req.ResourceSchema))
179+
modifiedState, err := tftypes.Transform(resp.NewState.Raw, NullifyWriteOnlyAttributes(ctx, resp.NewState.Schema))
180180
if err != nil {
181181
resp.Diagnostics.AddError(
182182
"Error Modifying State",

0 commit comments

Comments
 (0)