diff --git a/.changes/unreleased/FEATURES-20250121-165644.yaml b/.changes/unreleased/FEATURES-20250121-165644.yaml new file mode 100644 index 0000000000..83ff178c9d --- /dev/null +++ b/.changes/unreleased/FEATURES-20250121-165644.yaml @@ -0,0 +1,6 @@ +kind: FEATURES +body: 'helper/schema: Added `WriteOnly` schema behavior for managed resource schemas to indicate a write-only attribute. +Write-only attribute values are not saved to the Terraform plan or state artifacts.' +time: 2025-01-21T16:56:44.038893-05:00 +custom: + Issue: "1375" diff --git a/.changes/unreleased/FEATURES-20250121-170105.yaml b/.changes/unreleased/FEATURES-20250121-170105.yaml new file mode 100644 index 0000000000..900b07887e --- /dev/null +++ b/.changes/unreleased/FEATURES-20250121-170105.yaml @@ -0,0 +1,6 @@ +kind: FEATURES +body: 'helper/validation: Added `PreferWriteOnlyAttribute()` validator that warns practitioners when a write-only version of +a configured attribute is available.' +time: 2025-01-21T17:01:05.40229-05:00 +custom: + Issue: "1375" diff --git a/.changes/unreleased/FEATURES-20250203-151933.yaml b/.changes/unreleased/FEATURES-20250203-151933.yaml new file mode 100644 index 0000000000..488d65d4f1 --- /dev/null +++ b/.changes/unreleased/FEATURES-20250203-151933.yaml @@ -0,0 +1,5 @@ +kind: FEATURES +body: 'schema/resource: Added `ValidateRawResourceConfigFuncs` field which allows resources to define validation logic during the `ValidateResourceTypeConfig` RPC.' +time: 2025-02-03T15:19:33.669857-05:00 +custom: + Issue: "1375" diff --git a/.changes/unreleased/NOTES-20250121-170545.yaml b/.changes/unreleased/NOTES-20250121-170545.yaml new file mode 100644 index 0000000000..fb75dba49a --- /dev/null +++ b/.changes/unreleased/NOTES-20250121-170545.yaml @@ -0,0 +1,5 @@ +kind: NOTES +body: Write-only attribute support is in technical preview and offered without compatibility promises until Terraform 1.11 is generally available. +time: 2025-01-21T17:05:45.398836-05:00 +custom: + Issue: "1375" diff --git a/helper/schema/core_schema.go b/helper/schema/core_schema.go index bb91e0013d..9247adde7e 100644 --- a/helper/schema/core_schema.go +++ b/helper/schema/core_schema.go @@ -167,6 +167,7 @@ func (s *Schema) coreConfigSchemaAttribute() *configschema.Attribute { Description: desc, DescriptionKind: descKind, Deprecated: s.Deprecated != "", + WriteOnly: s.WriteOnly, } } diff --git a/helper/schema/core_schema_test.go b/helper/schema/core_schema_test.go index b8362f15ec..76fcfa8b94 100644 --- a/helper/schema/core_schema_test.go +++ b/helper/schema/core_schema_test.go @@ -458,6 +458,25 @@ func TestSchemaMapCoreConfigSchema(t *testing.T) { BlockTypes: map[string]*configschema.NestedBlock{}, }), }, + "write-only": { + map[string]*Schema{ + "string": { + Type: TypeString, + Optional: true, + WriteOnly: true, + }, + }, + testResource(&configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "string": { + Type: cty.String, + Optional: true, + WriteOnly: true, + }, + }, + BlockTypes: map[string]*configschema.NestedBlock{}, + }), + }, } for name, test := range tests { diff --git a/helper/schema/grpc_provider.go b/helper/schema/grpc_provider.go index d9870cbc84..f942814806 100644 --- a/helper/schema/grpc_provider.go +++ b/helper/schema/grpc_provider.go @@ -283,6 +283,32 @@ func (s *GRPCProviderServer) ValidateResourceTypeConfig(ctx context.Context, req resp.Diagnostics = convert.AppendProtoDiag(ctx, resp.Diagnostics, err) return resp, nil } + if req.ClientCapabilities == nil || !req.ClientCapabilities.WriteOnlyAttributesAllowed { + resp.Diagnostics = convert.AppendProtoDiag(ctx, resp.Diagnostics, validateWriteOnlyNullValues(configVal, schemaBlock, cty.Path{})) + } + + r := s.provider.ResourcesMap[req.TypeName] + + // Calling all ValidateRawResourceConfigFunc here since they validate on the raw go-cty config value + // and were introduced after the public provider.ValidateResource method. + if r.ValidateRawResourceConfigFuncs != nil { + writeOnlyAllowed := false + + if req.ClientCapabilities != nil { + writeOnlyAllowed = req.ClientCapabilities.WriteOnlyAttributesAllowed + } + + validateReq := ValidateResourceConfigFuncRequest{ + WriteOnlyAttributesAllowed: writeOnlyAllowed, + RawConfig: configVal, + } + + for _, validateFunc := range r.ValidateRawResourceConfigFuncs { + validateResp := &ValidateResourceConfigFuncResponse{} + validateFunc(ctx, validateReq, validateResp) + resp.Diagnostics = convert.AppendProtoDiag(ctx, resp.Diagnostics, validateResp.Diagnostics) + } + } config := terraform.NewResourceConfigShimmed(configVal, schemaBlock) @@ -394,6 +420,9 @@ func (s *GRPCProviderServer) UpgradeResourceState(ctx context.Context, req *tfpr // Normalize the value and fill in any missing blocks. val = objchange.NormalizeObjectFromLegacySDK(val, schemaBlock) + // Set any write-only attribute values to null + val = setWriteOnlyNullValues(val, schemaBlock) + // encode the final state to the expected msgpack format newStateMP, err := msgpack.Marshal(val, schemaBlock.ImpliedType()) if err != nil { @@ -738,6 +767,7 @@ func (s *GRPCProviderServer) ReadResource(ctx context.Context, req *tfprotov5.Re newStateVal = normalizeNullValues(newStateVal, stateVal, false) newStateVal = copyTimeoutValues(newStateVal, stateVal) + newStateVal = setWriteOnlyNullValues(newStateVal, schemaBlock) newStateMP, err := msgpack.Marshal(newStateVal, schemaBlock.ImpliedType()) if err != nil { @@ -937,6 +967,9 @@ func (s *GRPCProviderServer) PlanResourceChange(ctx context.Context, req *tfprot plannedStateVal = SetUnknowns(plannedStateVal, schemaBlock) } + // Set any write-only attribute values to null + plannedStateVal = setWriteOnlyNullValues(plannedStateVal, schemaBlock) + plannedMP, err := msgpack.Marshal(plannedStateVal, schemaBlock.ImpliedType()) if err != nil { resp.Diagnostics = convert.AppendProtoDiag(ctx, resp.Diagnostics, err) @@ -1184,6 +1217,8 @@ func (s *GRPCProviderServer) ApplyResourceChange(ctx context.Context, req *tfpro newStateVal = copyTimeoutValues(newStateVal, plannedStateVal) + newStateVal = setWriteOnlyNullValues(newStateVal, schemaBlock) + newStateMP, err := msgpack.Marshal(newStateVal, schemaBlock.ImpliedType()) if err != nil { resp.Diagnostics = convert.AppendProtoDiag(ctx, resp.Diagnostics, err) @@ -1305,6 +1340,9 @@ func (s *GRPCProviderServer) ImportResourceState(ctx context.Context, req *tfpro newStateVal = cty.ObjectVal(newStateValueMap) } + // Set any write-only attribute values to null + newStateVal = setWriteOnlyNullValues(newStateVal, schemaBlock) + newStateMP, err := msgpack.Marshal(newStateVal, schemaBlock.ImpliedType()) if err != nil { resp.Diagnostics = convert.AppendProtoDiag(ctx, resp.Diagnostics, err) diff --git a/helper/schema/grpc_provider_test.go b/helper/schema/grpc_provider_test.go index e025e13cbe..a343daf809 100644 --- a/helper/schema/grpc_provider_test.go +++ b/helper/schema/grpc_provider_test.go @@ -17,9 +17,9 @@ import ( "github.com/google/go-cmp/cmp" "github.com/hashicorp/go-cty/cty" "github.com/hashicorp/go-cty/cty/msgpack" - "github.com/hashicorp/terraform-plugin-go/tfprotov5" "github.com/hashicorp/terraform-plugin-go/tftypes" + "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/internal/plugin/convert" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" @@ -3488,199 +3488,682 @@ func TestGRPCProviderServerMoveResourceState(t *testing.T) { } } -func TestUpgradeState_jsonState(t *testing.T) { - r := &Resource{ - SchemaVersion: 2, - Schema: map[string]*Schema{ - "two": { - Type: TypeInt, - Optional: true, - }, - }, - } +func TestGRPCProviderServerValidateResourceTypeConfig(t *testing.T) { + t.Parallel() - r.StateUpgraders = []StateUpgrader{ - { - Version: 0, - Type: cty.Object(map[string]cty.Type{ - "id": cty.String, - "zero": cty.Number, + testCases := map[string]struct { + server *GRPCProviderServer + request *tfprotov5.ValidateResourceTypeConfigRequest + expected *tfprotov5.ValidateResourceTypeConfigResponse + }{ + "Provider with empty resource returns no errors": { + server: NewGRPCProviderServer(&Provider{ + ResourcesMap: map[string]*Resource{ + "test_resource": {}, + }, }), - Upgrade: func(ctx context.Context, m map[string]interface{}, meta interface{}) (map[string]interface{}, error) { - _, ok := m["zero"].(float64) - if !ok { - return nil, fmt.Errorf("zero not found in %#v", m) - } - m["one"] = float64(1) - delete(m, "zero") - return m, nil + request: &tfprotov5.ValidateResourceTypeConfigRequest{ + TypeName: "test_resource", + Config: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.NullVal(cty.String), + }), + ), + }, }, + expected: &tfprotov5.ValidateResourceTypeConfigResponse{}, }, - { - Version: 1, - Type: cty.Object(map[string]cty.Type{ - "id": cty.String, - "one": cty.Number, + "Client without WriteOnlyAttributesAllowed capabilities: null WriteOnly attribute returns no errors": { + server: NewGRPCProviderServer(&Provider{ + ResourcesMap: map[string]*Resource{ + "test_resource": { + Schema: map[string]*Schema{ + "foo": { + Type: TypeInt, + Optional: true, + WriteOnly: true, + }, + }, + }, + }, }), - Upgrade: func(ctx context.Context, m map[string]interface{}, meta interface{}) (map[string]interface{}, error) { - _, ok := m["one"].(float64) - if !ok { - return nil, fmt.Errorf("one not found in %#v", m) - } - m["two"] = float64(2) - delete(m, "one") - return m, nil - }, - }, - } - - server := NewGRPCProviderServer(&Provider{ - ResourcesMap: map[string]*Resource{ - "test": r, - }, - }) - - req := &tfprotov5.UpgradeResourceStateRequest{ - TypeName: "test", - Version: 0, - RawState: &tfprotov5.RawState{ - JSON: []byte(`{"id":"bar","zero":0}`), - }, - } - - resp, err := server.UpgradeResourceState(context.Background(), req) - if err != nil { - t.Fatal(err) - } - - if len(resp.Diagnostics) > 0 { - for _, d := range resp.Diagnostics { - t.Errorf("%#v", d) - } - t.Fatal("error") - } - - val, err := msgpack.Unmarshal(resp.UpgradedState.MsgPack, r.CoreConfigSchema().ImpliedType()) - if err != nil { - t.Fatal(err) - } - - expected := cty.ObjectVal(map[string]cty.Value{ - "id": cty.StringVal("bar"), - "two": cty.NumberIntVal(2), - }) - - if !cmp.Equal(expected, val, valueComparer, equateEmpty) { - t.Fatal(cmp.Diff(expected, val, valueComparer, equateEmpty)) - } -} - -func TestUpgradeState_jsonStateBigInt(t *testing.T) { - r := &Resource{ - UseJSONNumber: true, - SchemaVersion: 2, - Schema: map[string]*Schema{ - "int": { - Type: TypeInt, - Required: true, - }, - }, - } - - server := NewGRPCProviderServer(&Provider{ - ResourcesMap: map[string]*Resource{ - "test": r, - }, - }) - - req := &tfprotov5.UpgradeResourceStateRequest{ - TypeName: "test", - Version: 0, - RawState: &tfprotov5.RawState{ - JSON: []byte(`{"id":"bar","int":7227701560655103598}`), - }, - } - - resp, err := server.UpgradeResourceState(context.Background(), req) - if err != nil { - t.Fatal(err) - } - - if len(resp.Diagnostics) > 0 { - for _, d := range resp.Diagnostics { - t.Errorf("%#v", d) - } - t.Fatal("error") - } - - val, err := msgpack.Unmarshal(resp.UpgradedState.MsgPack, r.CoreConfigSchema().ImpliedType()) - if err != nil { - t.Fatal(err) - } - - expected := cty.ObjectVal(map[string]cty.Value{ - "id": cty.StringVal("bar"), - "int": cty.NumberIntVal(7227701560655103598), - }) - - if !cmp.Equal(expected, val, valueComparer, equateEmpty) { - t.Fatal(cmp.Diff(expected, val, valueComparer, equateEmpty)) - } -} - -func TestUpgradeState_removedAttr(t *testing.T) { - r1 := &Resource{ - Schema: map[string]*Schema{ - "two": { - Type: TypeString, - Optional: true, + request: &tfprotov5.ValidateResourceTypeConfigRequest{ + TypeName: "test_resource", + Config: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "foo": cty.Number, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.NullVal(cty.String), + "foo": cty.NullVal(cty.Number), + }), + ), + }, }, + expected: &tfprotov5.ValidateResourceTypeConfigResponse{}, }, - } - - r2 := &Resource{ - Schema: map[string]*Schema{ - "multi": { - Type: TypeSet, - Optional: true, - Elem: &Resource{ - Schema: map[string]*Schema{ - "set": { - Type: TypeSet, - Optional: true, - Elem: &Resource{ - Schema: map[string]*Schema{ - "required": { - Type: TypeString, - Required: true, - }, - }, + "Server without WriteOnlyAttributesAllowed capabilities: WriteOnly Attribute with Value returns an error": { + server: NewGRPCProviderServer(&Provider{ + ResourcesMap: map[string]*Resource{ + "test_resource": { + Schema: map[string]*Schema{ + "foo": { + Type: TypeInt, + Optional: true, + WriteOnly: true, }, }, }, }, + }), + request: &tfprotov5.ValidateResourceTypeConfigRequest{ + TypeName: "test_resource", + Config: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "foo": cty.Number, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.NullVal(cty.String), + "foo": cty.NumberIntVal(2), + }), + ), + }, + }, + expected: &tfprotov5.ValidateResourceTypeConfigResponse{ + Diagnostics: []*tfprotov5.Diagnostic{ + { + Severity: tfprotov5.DiagnosticSeverityError, + Summary: "Write-only Attribute Not Allowed", + Detail: "The resource contains a non-null value for write-only attribute \"foo\" " + + "Write-only attributes are only supported in Terraform 1.11 and later.", + Attribute: tftypes.NewAttributePath().WithAttributeName("foo"), + }, + }, }, }, - } - - r3 := &Resource{ - Schema: map[string]*Schema{ - "config_mode_attr": { - Type: TypeList, - ConfigMode: SchemaConfigModeAttr, - Optional: true, - Elem: &Resource{ - Schema: map[string]*Schema{ - "foo": { - Type: TypeString, - Optional: true, + "Server without WriteOnlyAttributesAllowed capabilities: multiple WriteOnly Attributes with Value returns multiple errors": { + server: NewGRPCProviderServer(&Provider{ + ResourcesMap: map[string]*Resource{ + "test_resource": { + Schema: map[string]*Schema{ + "foo": { + Type: TypeInt, + Optional: true, + WriteOnly: true, + }, + "bar": { + Type: TypeInt, + Optional: true, + WriteOnly: true, + }, }, }, }, + }), + request: &tfprotov5.ValidateResourceTypeConfigRequest{ + TypeName: "test_resource", + Config: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "foo": cty.Number, + "bar": cty.Number, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.NullVal(cty.String), + "foo": cty.NumberIntVal(2), + "bar": cty.NumberIntVal(2), + }), + ), + }, }, - }, - } - + expected: &tfprotov5.ValidateResourceTypeConfigResponse{ + Diagnostics: []*tfprotov5.Diagnostic{ + { + Severity: tfprotov5.DiagnosticSeverityError, + Summary: "Write-only Attribute Not Allowed", + Detail: "The resource contains a non-null value for write-only attribute \"bar\" " + + "Write-only attributes are only supported in Terraform 1.11 and later.", + Attribute: tftypes.NewAttributePath().WithAttributeName("bar"), + }, + { + Severity: tfprotov5.DiagnosticSeverityError, + Summary: "Write-only Attribute Not Allowed", + Detail: "The resource contains a non-null value for write-only attribute \"foo\" " + + "Write-only attributes are only supported in Terraform 1.11 and later.", + Attribute: tftypes.NewAttributePath().WithAttributeName("foo"), + }, + }, + }, + }, + "Server without WriteOnlyAttributesAllowed capabilities: multiple nested WriteOnly Attributes with Value returns multiple errors": { + server: NewGRPCProviderServer(&Provider{ + ResourcesMap: map[string]*Resource{ + "test_resource": { + Schema: map[string]*Schema{ + "foo": { + Type: TypeInt, + Optional: true, + WriteOnly: true, + }, + "bar": { + Type: TypeInt, + Optional: true, + }, + "config_block_attr": { + Type: TypeList, + Optional: true, + WriteOnly: true, + Elem: &Resource{ + Schema: map[string]*Schema{ + "nested_attr": { + Type: TypeString, + Optional: true, + }, + "writeonly_nested_attr": { + Type: TypeString, + WriteOnly: true, + Optional: true, + }, + }, + }, + }, + }, + }, + }, + }), + request: &tfprotov5.ValidateResourceTypeConfigRequest{ + TypeName: "test_resource", + Config: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "foo": cty.Number, + "bar": cty.Number, + "config_block_attr": cty.List(cty.Object(map[string]cty.Type{ + "nested_attr": cty.String, + "writeonly_nested_attr": cty.String, + })), + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.NullVal(cty.String), + "foo": cty.NumberIntVal(2), + "bar": cty.NumberIntVal(2), + "config_block_attr": cty.ListVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "nested_attr": cty.StringVal("value"), + "writeonly_nested_attr": cty.StringVal("value"), + }), + }), + }), + ), + }, + }, + expected: &tfprotov5.ValidateResourceTypeConfigResponse{ + Diagnostics: []*tfprotov5.Diagnostic{ + { + Severity: tfprotov5.DiagnosticSeverityError, + Summary: "Write-only Attribute Not Allowed", + Detail: "The resource contains a non-null value for write-only attribute \"foo\" " + + "Write-only attributes are only supported in Terraform 1.11 and later.", + Attribute: tftypes.NewAttributePath().WithAttributeName("foo"), + }, + { + Severity: tfprotov5.DiagnosticSeverityError, + Summary: "Write-only Attribute Not Allowed", + Detail: "The resource contains a non-null value for write-only attribute \"writeonly_nested_attr\" " + + "Write-only attributes are only supported in Terraform 1.11 and later.", + Attribute: tftypes.NewAttributePath(). + WithAttributeName("config_block_attr"). + WithElementKeyInt(0). + WithAttributeName("writeonly_nested_attr"), + }, + }, + }, + }, + "Server with ValidateRawResourceConfigFunc: WriteOnlyAttributesAllowed true returns diags": { + server: NewGRPCProviderServer(&Provider{ + ResourcesMap: map[string]*Resource{ + "test_resource": { + ValidateRawResourceConfigFuncs: []ValidateRawResourceConfigFunc{ + func(ctx context.Context, req ValidateResourceConfigFuncRequest, resp *ValidateResourceConfigFuncResponse) { + if req.WriteOnlyAttributesAllowed { + resp.Diagnostics = diag.Diagnostics{ + { + Severity: diag.Error, + Summary: "ValidateRawResourceConfigFunc Error", + }, + } + } + }, + func(ctx context.Context, req ValidateResourceConfigFuncRequest, resp *ValidateResourceConfigFuncResponse) { + if req.WriteOnlyAttributesAllowed { + resp.Diagnostics = diag.Diagnostics{ + { + Severity: diag.Error, + Summary: "ValidateRawResourceConfigFunc Error", + }, + } + } + }, + }, + Schema: map[string]*Schema{ + "foo": { + Type: TypeInt, + Optional: true, + WriteOnly: true, + }, + "bar": { + Type: TypeInt, + Optional: true, + }, + }, + }, + }, + }), + request: &tfprotov5.ValidateResourceTypeConfigRequest{ + TypeName: "test_resource", + ClientCapabilities: &tfprotov5.ValidateResourceTypeConfigClientCapabilities{ + WriteOnlyAttributesAllowed: true, + }, + Config: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "foo": cty.Number, + "bar": cty.Number, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.NullVal(cty.String), + "foo": cty.NumberIntVal(2), + "bar": cty.NumberIntVal(2), + }), + ), + }, + }, + expected: &tfprotov5.ValidateResourceTypeConfigResponse{ + Diagnostics: []*tfprotov5.Diagnostic{ + { + Severity: tfprotov5.DiagnosticSeverityError, + Summary: "ValidateRawResourceConfigFunc Error", + }, + { + Severity: tfprotov5.DiagnosticSeverityError, + Summary: "ValidateRawResourceConfigFunc Error", + }, + }, + }, + }, + "Server with ValidateRawResourceConfigFunc: WriteOnlyAttributesAllowed false returns diags": { + server: NewGRPCProviderServer(&Provider{ + ResourcesMap: map[string]*Resource{ + "test_resource": { + ValidateRawResourceConfigFuncs: []ValidateRawResourceConfigFunc{ + func(ctx context.Context, req ValidateResourceConfigFuncRequest, resp *ValidateResourceConfigFuncResponse) { + if !req.WriteOnlyAttributesAllowed { + resp.Diagnostics = diag.Diagnostics{ + { + Severity: diag.Error, + Summary: "ValidateRawResourceConfigFunc Error", + }, + } + } + }, + func(ctx context.Context, req ValidateResourceConfigFuncRequest, resp *ValidateResourceConfigFuncResponse) { + if !req.WriteOnlyAttributesAllowed { + resp.Diagnostics = diag.Diagnostics{ + { + Severity: diag.Error, + Summary: "ValidateRawResourceConfigFunc Error", + }, + } + } + }, + }, + Schema: map[string]*Schema{ + "foo": { + Type: TypeInt, + Optional: true, + }, + "bar": { + Type: TypeInt, + Optional: true, + }, + }, + }, + }, + }), + request: &tfprotov5.ValidateResourceTypeConfigRequest{ + TypeName: "test_resource", + Config: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "foo": cty.Number, + "bar": cty.Number, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.NullVal(cty.String), + "foo": cty.NumberIntVal(2), + "bar": cty.NumberIntVal(2), + }), + ), + }, + }, + expected: &tfprotov5.ValidateResourceTypeConfigResponse{ + Diagnostics: []*tfprotov5.Diagnostic{ + { + Severity: tfprotov5.DiagnosticSeverityError, + Summary: "ValidateRawResourceConfigFunc Error", + }, + { + Severity: tfprotov5.DiagnosticSeverityError, + Summary: "ValidateRawResourceConfigFunc Error", + }, + }, + }, + }, + "Server with ValidateRawResourceConfigFunc: equal config value returns diags": { + server: NewGRPCProviderServer(&Provider{ + ResourcesMap: map[string]*Resource{ + "test_resource": { + ValidateRawResourceConfigFuncs: []ValidateRawResourceConfigFunc{ + func(ctx context.Context, req ValidateResourceConfigFuncRequest, resp *ValidateResourceConfigFuncResponse) { + equals := req.RawConfig.Equals(cty.ObjectVal(map[string]cty.Value{ + "id": cty.NullVal(cty.String), + "foo": cty.NumberIntVal(2), + "bar": cty.NumberIntVal(2), + })) + if equals.True() { + resp.Diagnostics = diag.Diagnostics{ + { + Severity: diag.Error, + Summary: "ValidateRawResourceConfigFunc Error", + }, + } + } + }, + func(ctx context.Context, req ValidateResourceConfigFuncRequest, resp *ValidateResourceConfigFuncResponse) { + equals := req.RawConfig.Equals(cty.ObjectVal(map[string]cty.Value{ + "id": cty.NullVal(cty.String), + "foo": cty.NumberIntVal(2), + "bar": cty.NumberIntVal(2), + })) + if equals.True() { + resp.Diagnostics = diag.Diagnostics{ + { + Severity: diag.Error, + Summary: "ValidateRawResourceConfigFunc Error", + }, + } + } + }, + }, + Schema: map[string]*Schema{ + "foo": { + Type: TypeInt, + Optional: true, + }, + "bar": { + Type: TypeInt, + Optional: true, + }, + }, + }, + }, + }), + request: &tfprotov5.ValidateResourceTypeConfigRequest{ + TypeName: "test_resource", + Config: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "foo": cty.Number, + "bar": cty.Number, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.NullVal(cty.String), + "foo": cty.NumberIntVal(2), + "bar": cty.NumberIntVal(2), + }), + ), + }, + }, + expected: &tfprotov5.ValidateResourceTypeConfigResponse{ + Diagnostics: []*tfprotov5.Diagnostic{ + { + Severity: tfprotov5.DiagnosticSeverityError, + Summary: "ValidateRawResourceConfigFunc Error", + }, + { + Severity: tfprotov5.DiagnosticSeverityError, + Summary: "ValidateRawResourceConfigFunc Error", + }, + }, + }, + }, + } + + for name, testCase := range testCases { + name, testCase := name, testCase + + t.Run(name, func(t *testing.T) { + t.Parallel() + + resp, err := testCase.server.ValidateResourceTypeConfig(context.Background(), testCase.request) + + if testCase.request != nil && err != nil { + t.Fatalf("unexpected error: %s", err) + } + + if diff := cmp.Diff(resp, testCase.expected); diff != "" { + t.Errorf("unexpected difference: %s", diff) + } + }) + } +} + +func TestUpgradeState_jsonState(t *testing.T) { + r := &Resource{ + SchemaVersion: 2, + Schema: map[string]*Schema{ + "two": { + Type: TypeInt, + Optional: true, + }, + }, + } + + r.StateUpgraders = []StateUpgrader{ + { + Version: 0, + Type: cty.Object(map[string]cty.Type{ + "id": cty.String, + "zero": cty.Number, + }), + Upgrade: func(ctx context.Context, m map[string]interface{}, meta interface{}) (map[string]interface{}, error) { + _, ok := m["zero"].(float64) + if !ok { + return nil, fmt.Errorf("zero not found in %#v", m) + } + m["one"] = float64(1) + delete(m, "zero") + return m, nil + }, + }, + { + Version: 1, + Type: cty.Object(map[string]cty.Type{ + "id": cty.String, + "one": cty.Number, + }), + Upgrade: func(ctx context.Context, m map[string]interface{}, meta interface{}) (map[string]interface{}, error) { + _, ok := m["one"].(float64) + if !ok { + return nil, fmt.Errorf("one not found in %#v", m) + } + m["two"] = float64(2) + delete(m, "one") + return m, nil + }, + }, + } + + server := NewGRPCProviderServer(&Provider{ + ResourcesMap: map[string]*Resource{ + "test": r, + }, + }) + + req := &tfprotov5.UpgradeResourceStateRequest{ + TypeName: "test", + Version: 0, + RawState: &tfprotov5.RawState{ + JSON: []byte(`{"id":"bar","zero":0}`), + }, + } + + resp, err := server.UpgradeResourceState(context.Background(), req) + if err != nil { + t.Fatal(err) + } + + if len(resp.Diagnostics) > 0 { + for _, d := range resp.Diagnostics { + t.Errorf("%#v", d) + } + t.Fatal("error") + } + + val, err := msgpack.Unmarshal(resp.UpgradedState.MsgPack, r.CoreConfigSchema().ImpliedType()) + if err != nil { + t.Fatal(err) + } + + expected := cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("bar"), + "two": cty.NumberIntVal(2), + }) + + if !cmp.Equal(expected, val, valueComparer, equateEmpty) { + t.Fatal(cmp.Diff(expected, val, valueComparer, equateEmpty)) + } +} + +func TestUpgradeState_jsonStateBigInt(t *testing.T) { + r := &Resource{ + UseJSONNumber: true, + SchemaVersion: 2, + Schema: map[string]*Schema{ + "int": { + Type: TypeInt, + Required: true, + }, + }, + } + + server := NewGRPCProviderServer(&Provider{ + ResourcesMap: map[string]*Resource{ + "test": r, + }, + }) + + req := &tfprotov5.UpgradeResourceStateRequest{ + TypeName: "test", + Version: 0, + RawState: &tfprotov5.RawState{ + JSON: []byte(`{"id":"bar","int":7227701560655103598}`), + }, + } + + resp, err := server.UpgradeResourceState(context.Background(), req) + if err != nil { + t.Fatal(err) + } + + if len(resp.Diagnostics) > 0 { + for _, d := range resp.Diagnostics { + t.Errorf("%#v", d) + } + t.Fatal("error") + } + + val, err := msgpack.Unmarshal(resp.UpgradedState.MsgPack, r.CoreConfigSchema().ImpliedType()) + if err != nil { + t.Fatal(err) + } + + expected := cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("bar"), + "int": cty.NumberIntVal(7227701560655103598), + }) + + if !cmp.Equal(expected, val, valueComparer, equateEmpty) { + t.Fatal(cmp.Diff(expected, val, valueComparer, equateEmpty)) + } +} + +func TestUpgradeState_removedAttr(t *testing.T) { + r1 := &Resource{ + Schema: map[string]*Schema{ + "two": { + Type: TypeString, + Optional: true, + }, + }, + } + + r2 := &Resource{ + Schema: map[string]*Schema{ + "multi": { + Type: TypeSet, + Optional: true, + Elem: &Resource{ + Schema: map[string]*Schema{ + "set": { + Type: TypeSet, + Optional: true, + Elem: &Resource{ + Schema: map[string]*Schema{ + "required": { + Type: TypeString, + Required: true, + }, + }, + }, + }, + }, + }, + }, + }, + } + + r3 := &Resource{ + Schema: map[string]*Schema{ + "config_mode_attr": { + Type: TypeList, + ConfigMode: SchemaConfigModeAttr, + Optional: true, + Elem: &Resource{ + Schema: map[string]*Schema{ + "foo": { + Type: TypeString, + Optional: true, + }, + }, + }, + }, + }, + } + p := &Provider{ ResourcesMap: map[string]*Resource{ "r1": r1, @@ -3919,9 +4402,88 @@ func TestUpgradeState_flatmapState(t *testing.T) { }, { TypeName: "test", - Version: 4, + Version: 4, + RawState: &tfprotov5.RawState{ + JSON: []byte(`{"id":"bar","four":4}`), + }, + }, + } + + for i, req := range testReqs { + t.Run(fmt.Sprintf("%d-%d", i, req.Version), func(t *testing.T) { + resp, err := server.UpgradeResourceState(context.Background(), req) + if err != nil { + t.Fatal(err) + } + + if len(resp.Diagnostics) > 0 { + for _, d := range resp.Diagnostics { + t.Errorf("%#v", d) + } + t.Fatal("error") + } + + val, err := msgpack.Unmarshal(resp.UpgradedState.MsgPack, r.CoreConfigSchema().ImpliedType()) + if err != nil { + t.Fatal(err) + } + + expected := cty.ObjectVal(map[string]cty.Value{ + "block": cty.ListValEmpty(cty.Object(map[string]cty.Type{"attr": cty.String})), + "id": cty.StringVal("bar"), + "four": cty.NumberIntVal(4), + }) + + if !cmp.Equal(expected, val, valueComparer, equateEmpty) { + t.Fatal(cmp.Diff(expected, val, valueComparer, equateEmpty)) + } + }) + } +} + +func TestUpgradeState_flatmapStateMissingMigrateState(t *testing.T) { + r := &Resource{ + SchemaVersion: 1, + Schema: map[string]*Schema{ + "one": { + Type: TypeInt, + Required: true, + }, + }, + } + + server := NewGRPCProviderServer(&Provider{ + ResourcesMap: map[string]*Resource{ + "test": r, + }, + }) + + testReqs := []*tfprotov5.UpgradeResourceStateRequest{ + { + TypeName: "test", + Version: 0, + RawState: &tfprotov5.RawState{ + Flatmap: map[string]string{ + "id": "bar", + "one": "1", + }, + }, + }, + { + TypeName: "test", + Version: 1, + RawState: &tfprotov5.RawState{ + Flatmap: map[string]string{ + "id": "bar", + "one": "1", + }, + }, + }, + { + TypeName: "test", + Version: 1, RawState: &tfprotov5.RawState{ - JSON: []byte(`{"id":"bar","four":4}`), + JSON: []byte(`{"id":"bar","one":1}`), }, }, } @@ -3946,9 +4508,8 @@ func TestUpgradeState_flatmapState(t *testing.T) { } expected := cty.ObjectVal(map[string]cty.Value{ - "block": cty.ListValEmpty(cty.Object(map[string]cty.Type{"attr": cty.String})), - "id": cty.StringVal("bar"), - "four": cty.NumberIntVal(4), + "id": cty.StringVal("bar"), + "one": cty.NumberIntVal(1), }) if !cmp.Equal(expected, val, valueComparer, equateEmpty) { @@ -3958,277 +4519,700 @@ func TestUpgradeState_flatmapState(t *testing.T) { } } -func TestUpgradeState_flatmapStateMissingMigrateState(t *testing.T) { +func TestUpgradeState_writeOnlyNullification(t *testing.T) { r := &Resource{ - SchemaVersion: 1, + SchemaVersion: 2, Schema: map[string]*Schema{ - "one": { - Type: TypeInt, - Required: true, + "two": { + Type: TypeInt, + Optional: true, + WriteOnly: true, + }, + }, + } + + r.StateUpgraders = []StateUpgrader{ + { + Version: 0, + Type: cty.Object(map[string]cty.Type{ + "id": cty.String, + "zero": cty.Number, + }), + Upgrade: func(ctx context.Context, m map[string]interface{}, meta interface{}) (map[string]interface{}, error) { + _, ok := m["zero"].(float64) + if !ok { + return nil, fmt.Errorf("zero not found in %#v", m) + } + m["one"] = float64(1) + delete(m, "zero") + return m, nil + }, + }, + { + Version: 1, + Type: cty.Object(map[string]cty.Type{ + "id": cty.String, + "one": cty.Number, + }), + Upgrade: func(ctx context.Context, m map[string]interface{}, meta interface{}) (map[string]interface{}, error) { + _, ok := m["one"].(float64) + if !ok { + return nil, fmt.Errorf("one not found in %#v", m) + } + m["two"] = float64(2) + delete(m, "one") + return m, nil }, }, } - server := NewGRPCProviderServer(&Provider{ - ResourcesMap: map[string]*Resource{ - "test": r, - }, - }) + server := NewGRPCProviderServer(&Provider{ + ResourcesMap: map[string]*Resource{ + "test": r, + }, + }) + + req := &tfprotov5.UpgradeResourceStateRequest{ + TypeName: "test", + Version: 0, + RawState: &tfprotov5.RawState{ + JSON: []byte(`{"id":"bar","zero":0}`), + }, + } + + resp, err := server.UpgradeResourceState(context.Background(), req) + if err != nil { + t.Fatal(err) + } + + if len(resp.Diagnostics) > 0 { + for _, d := range resp.Diagnostics { + t.Errorf("%#v", d) + } + t.Fatal("error") + } + + val, err := msgpack.Unmarshal(resp.UpgradedState.MsgPack, r.CoreConfigSchema().ImpliedType()) + if err != nil { + t.Fatal(err) + } + + expected := cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("bar"), + "two": cty.NullVal(cty.Number), + }) + + if !cmp.Equal(expected, val, valueComparer, equateEmpty) { + t.Fatal(cmp.Diff(expected, val, valueComparer, equateEmpty)) + } +} + +func TestReadResource(t *testing.T) { + t.Parallel() + + testCases := map[string]struct { + server *GRPCProviderServer + req *tfprotov5.ReadResourceRequest + expected *tfprotov5.ReadResourceResponse + }{ + "read-resource": { + server: NewGRPCProviderServer(&Provider{ + ResourcesMap: map[string]*Resource{ + "test": { + SchemaVersion: 1, + Schema: map[string]*Schema{ + "id": { + Type: TypeString, + Required: true, + }, + "test_bool": { + Type: TypeBool, + Computed: true, + }, + "test_string": { + Type: TypeString, + Computed: true, + }, + }, + ReadContext: func(ctx context.Context, d *ResourceData, meta interface{}) diag.Diagnostics { + err := d.Set("test_bool", true) + if err != nil { + return diag.FromErr(err) + } + + err = d.Set("test_string", "new-state-val") + if err != nil { + return diag.FromErr(err) + } + + return nil + }, + }, + }, + }), + req: &tfprotov5.ReadResourceRequest{ + TypeName: "test", + CurrentState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "test_bool": cty.Bool, + "test_string": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("test-id"), + "test_bool": cty.BoolVal(false), + "test_string": cty.StringVal("prior-state-val"), + }), + ), + }, + }, + expected: &tfprotov5.ReadResourceResponse{ + NewState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "test_bool": cty.Bool, + "test_string": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("test-id"), + "test_bool": cty.BoolVal(true), + "test_string": cty.StringVal("new-state-val"), + }), + ), + }, + }, + }, + "deferred-response-unknown-val": { + server: NewGRPCProviderServer(&Provider{ + // Deferred response will skip read function and return current state + providerDeferred: &Deferred{ + Reason: DeferredReasonProviderConfigUnknown, + }, + ResourcesMap: map[string]*Resource{ + "test": { + SchemaVersion: 1, + Schema: map[string]*Schema{ + "id": { + Type: TypeString, + Required: true, + }, + "test_bool": { + Type: TypeBool, + Computed: true, + }, + "test_string": { + Type: TypeString, + Computed: true, + }, + }, + ReadContext: func(ctx context.Context, d *ResourceData, meta interface{}) diag.Diagnostics { + return diag.Errorf("Test assertion failed: read shouldn't be called when provider deferred response is present") + }, + }, + }, + }), + req: &tfprotov5.ReadResourceRequest{ + ClientCapabilities: &tfprotov5.ReadResourceClientCapabilities{ + DeferralAllowed: true, + }, + TypeName: "test", + CurrentState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "test_bool": cty.Bool, + "test_string": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("test-id"), + "test_bool": cty.BoolVal(false), + "test_string": cty.StringVal("prior-state-val"), + }), + ), + }, + }, + expected: &tfprotov5.ReadResourceResponse{ + NewState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "test_bool": cty.Bool, + "test_string": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("test-id"), + "test_bool": cty.BoolVal(false), + "test_string": cty.StringVal("prior-state-val"), + }), + ), + }, + Deferred: &tfprotov5.Deferred{ + Reason: tfprotov5.DeferredReasonProviderConfigUnknown, + }, + }, + }, + "write-only values are nullified in ReadResourceResponse": { + server: NewGRPCProviderServer(&Provider{ + ResourcesMap: map[string]*Resource{ + "test": { + SchemaVersion: 1, + Schema: map[string]*Schema{ + "id": { + Type: TypeString, + Required: true, + }, + "test_bool": { + Type: TypeBool, + Computed: true, + }, + "test_string": { + Type: TypeString, + Computed: true, + }, + "test_write_only": { + Type: TypeString, + WriteOnly: true, + }, + }, + ReadContext: func(ctx context.Context, d *ResourceData, meta interface{}) diag.Diagnostics { + err := d.Set("test_bool", true) + if err != nil { + return diag.FromErr(err) + } + + err = d.Set("test_string", "new-state-val") + if err != nil { + return diag.FromErr(err) + } - testReqs := []*tfprotov5.UpgradeResourceStateRequest{ - { - TypeName: "test", - Version: 0, - RawState: &tfprotov5.RawState{ - Flatmap: map[string]string{ - "id": "bar", - "one": "1", + err = d.Set("test_write_only", "write-only-val") + if err != nil { + return diag.FromErr(err) + } + + return nil + }, + }, }, - }, - }, - { - TypeName: "test", - Version: 1, - RawState: &tfprotov5.RawState{ - Flatmap: map[string]string{ - "id": "bar", - "one": "1", + }), + req: &tfprotov5.ReadResourceRequest{ + TypeName: "test", + CurrentState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "test_bool": cty.Bool, + "test_string": cty.String, + "test_write_only": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("test-id"), + "test_bool": cty.BoolVal(false), + "test_string": cty.StringVal("prior-state-val"), + "test_write_only": cty.NullVal(cty.String), + }), + ), }, }, - }, - { - TypeName: "test", - Version: 1, - RawState: &tfprotov5.RawState{ - JSON: []byte(`{"id":"bar","one":1}`), + expected: &tfprotov5.ReadResourceResponse{ + NewState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "test_bool": cty.Bool, + "test_string": cty.String, + "test_write_only": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("test-id"), + "test_bool": cty.BoolVal(true), + "test_string": cty.StringVal("new-state-val"), + "test_write_only": cty.NullVal(cty.String), + }), + ), + }, }, }, } - for i, req := range testReqs { - t.Run(fmt.Sprintf("%d-%d", i, req.Version), func(t *testing.T) { - resp, err := server.UpgradeResourceState(context.Background(), req) - if err != nil { - t.Fatal(err) - } + for name, testCase := range testCases { + name, testCase := name, testCase - if len(resp.Diagnostics) > 0 { - for _, d := range resp.Diagnostics { - t.Errorf("%#v", d) - } - t.Fatal("error") - } + t.Run(name, func(t *testing.T) { + t.Parallel() + resp, err := testCase.server.ReadResource(context.Background(), testCase.req) - val, err := msgpack.Unmarshal(resp.UpgradedState.MsgPack, r.CoreConfigSchema().ImpliedType()) if err != nil { t.Fatal(err) } - expected := cty.ObjectVal(map[string]cty.Value{ - "id": cty.StringVal("bar"), - "one": cty.NumberIntVal(1), - }) + if diff := cmp.Diff(resp, testCase.expected, valueComparer); diff != "" { + ty := testCase.server.getResourceSchemaBlock("test").ImpliedType() - if !cmp.Equal(expected, val, valueComparer, equateEmpty) { - t.Fatal(cmp.Diff(expected, val, valueComparer, equateEmpty)) + if resp != nil && resp.NewState != nil { + t.Logf("resp.NewState.MsgPack: %s", mustMsgpackUnmarshal(ty, resp.NewState.MsgPack)) + } + + if testCase.expected != nil && testCase.expected.NewState != nil { + t.Logf("expected: %s", mustMsgpackUnmarshal(ty, testCase.expected.NewState.MsgPack)) + } + + t.Error(diff) } }) } } -func TestReadResource(t *testing.T) { +func TestPlanResourceChange(t *testing.T) { t.Parallel() testCases := map[string]struct { server *GRPCProviderServer - req *tfprotov5.ReadResourceRequest - expected *tfprotov5.ReadResourceResponse + req *tfprotov5.PlanResourceChangeRequest + expected *tfprotov5.PlanResourceChangeResponse }{ - "read-resource": { + "basic-plan": { server: NewGRPCProviderServer(&Provider{ ResourcesMap: map[string]*Resource{ "test": { - SchemaVersion: 1, + SchemaVersion: 4, Schema: map[string]*Schema{ - "id": { - Type: TypeString, - Required: true, + "foo": { + Type: TypeInt, + Optional: true, }, - "test_bool": { - Type: TypeBool, - Computed: true, + }, + }, + }, + }), + req: &tfprotov5.PlanResourceChangeRequest{ + TypeName: "test", + PriorState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "foo": cty.Number, + }), + cty.NullVal( + cty.Object(map[string]cty.Type{ + "foo": cty.Number, + }), + ), + ), + }, + ProposedNewState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "foo": cty.Number, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.UnknownVal(cty.String), + "foo": cty.NullVal(cty.Number), + }), + ), + }, + Config: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "foo": cty.Number, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.NullVal(cty.String), + "foo": cty.NullVal(cty.Number), + }), + ), + }, + }, + expected: &tfprotov5.PlanResourceChangeResponse{ + PlannedState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "foo": cty.Number, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.UnknownVal(cty.String), + "foo": cty.NullVal(cty.Number), + }), + ), + }, + RequiresReplace: []*tftypes.AttributePath{ + tftypes.NewAttributePath().WithAttributeName("id"), + }, + PlannedPrivate: []byte(`{"_new_extra_shim":{}}`), + UnsafeToUseLegacyTypeSystem: true, + }, + }, + "basic-plan-EnableLegacyTypeSystemPlanErrors": { + server: NewGRPCProviderServer(&Provider{ + ResourcesMap: map[string]*Resource{ + "test": { + // Will set UnsafeToUseLegacyTypeSystem to false + EnableLegacyTypeSystemPlanErrors: true, + Schema: map[string]*Schema{ + "foo": { + Type: TypeInt, + Optional: true, + }, + }, + }, + }, + }), + req: &tfprotov5.PlanResourceChangeRequest{ + TypeName: "test", + PriorState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "foo": cty.Number, + }), + cty.NullVal( + cty.Object(map[string]cty.Type{ + "foo": cty.Number, + }), + ), + ), + }, + ProposedNewState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "foo": cty.Number, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.UnknownVal(cty.String), + "foo": cty.NullVal(cty.Number), + }), + ), + }, + Config: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "foo": cty.Number, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.NullVal(cty.String), + "foo": cty.NullVal(cty.Number), + }), + ), + }, + }, + expected: &tfprotov5.PlanResourceChangeResponse{ + PlannedState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "foo": cty.Number, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.UnknownVal(cty.String), + "foo": cty.NullVal(cty.Number), + }), + ), + }, + RequiresReplace: []*tftypes.AttributePath{ + tftypes.NewAttributePath().WithAttributeName("id"), + }, + PlannedPrivate: []byte(`{"_new_extra_shim":{}}`), + UnsafeToUseLegacyTypeSystem: false, + }, + }, + "deferred-with-provider-plan-modification": { + server: NewGRPCProviderServer(&Provider{ + providerDeferred: &Deferred{ + Reason: DeferredReasonProviderConfigUnknown, + }, + ResourcesMap: map[string]*Resource{ + "test": { + ResourceBehavior: ResourceBehavior{ + ProviderDeferred: ProviderDeferredBehavior{ + // Will ensure that CustomizeDiff is called + EnablePlanModification: true, }, - "test_string": { + }, + SchemaVersion: 4, + CustomizeDiff: func(ctx context.Context, d *ResourceDiff, i interface{}) error { + return d.SetNew("foo", "new-foo-value") + }, + Schema: map[string]*Schema{ + "foo": { Type: TypeString, + Optional: true, Computed: true, }, }, - ReadContext: func(ctx context.Context, d *ResourceData, meta interface{}) diag.Diagnostics { - err := d.Set("test_bool", true) - if err != nil { - return diag.FromErr(err) - } - - err = d.Set("test_string", "new-state-val") - if err != nil { - return diag.FromErr(err) - } - - return nil - }, }, }, }), - req: &tfprotov5.ReadResourceRequest{ + req: &tfprotov5.PlanResourceChangeRequest{ TypeName: "test", - CurrentState: &tfprotov5.DynamicValue{ + ClientCapabilities: &tfprotov5.PlanResourceChangeClientCapabilities{ + DeferralAllowed: true, + }, + PriorState: &tfprotov5.DynamicValue{ MsgPack: mustMsgpackMarshal( cty.Object(map[string]cty.Type{ - "id": cty.String, - "test_bool": cty.Bool, - "test_string": cty.String, + "foo": cty.String, + }), + cty.NullVal( + cty.Object(map[string]cty.Type{ + "foo": cty.String, + }), + ), + ), + }, + ProposedNewState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "foo": cty.String, }), cty.ObjectVal(map[string]cty.Value{ - "id": cty.StringVal("test-id"), - "test_bool": cty.BoolVal(false), - "test_string": cty.StringVal("prior-state-val"), + "id": cty.UnknownVal(cty.String), + "foo": cty.UnknownVal(cty.String), + }), + ), + }, + Config: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "foo": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.NullVal(cty.String), + "foo": cty.NullVal(cty.String), }), ), }, }, - expected: &tfprotov5.ReadResourceResponse{ - NewState: &tfprotov5.DynamicValue{ + expected: &tfprotov5.PlanResourceChangeResponse{ + Deferred: &tfprotov5.Deferred{ + Reason: tfprotov5.DeferredReasonProviderConfigUnknown, + }, + PlannedState: &tfprotov5.DynamicValue{ MsgPack: mustMsgpackMarshal( cty.Object(map[string]cty.Type{ - "id": cty.String, - "test_bool": cty.Bool, - "test_string": cty.String, + "id": cty.String, + "foo": cty.String, }), cty.ObjectVal(map[string]cty.Value{ - "id": cty.StringVal("test-id"), - "test_bool": cty.BoolVal(true), - "test_string": cty.StringVal("new-state-val"), + "id": cty.UnknownVal(cty.String), + "foo": cty.StringVal("new-foo-value"), }), ), }, + RequiresReplace: []*tftypes.AttributePath{ + tftypes.NewAttributePath().WithAttributeName("id"), + }, + PlannedPrivate: []byte(`{"_new_extra_shim":{}}`), + UnsafeToUseLegacyTypeSystem: true, }, }, - "deferred-response-unknown-val": { + "deferred-skip-plan-modification": { server: NewGRPCProviderServer(&Provider{ - // Deferred response will skip read function and return current state providerDeferred: &Deferred{ Reason: DeferredReasonProviderConfigUnknown, }, ResourcesMap: map[string]*Resource{ "test": { - SchemaVersion: 1, + SchemaVersion: 4, + CustomizeDiff: func(ctx context.Context, d *ResourceDiff, i interface{}) error { + return errors.New("Test assertion failed: CustomizeDiff shouldn't be called") + }, Schema: map[string]*Schema{ - "id": { - Type: TypeString, - Required: true, - }, - "test_bool": { - Type: TypeBool, - Computed: true, - }, - "test_string": { + "foo": { Type: TypeString, + Optional: true, Computed: true, }, }, - ReadContext: func(ctx context.Context, d *ResourceData, meta interface{}) diag.Diagnostics { - return diag.Errorf("Test assertion failed: read shouldn't be called when provider deferred response is present") - }, }, }, }), - req: &tfprotov5.ReadResourceRequest{ - ClientCapabilities: &tfprotov5.ReadResourceClientCapabilities{ + req: &tfprotov5.PlanResourceChangeRequest{ + TypeName: "test", + ClientCapabilities: &tfprotov5.PlanResourceChangeClientCapabilities{ DeferralAllowed: true, }, - TypeName: "test", - CurrentState: &tfprotov5.DynamicValue{ + PriorState: &tfprotov5.DynamicValue{ MsgPack: mustMsgpackMarshal( cty.Object(map[string]cty.Type{ - "id": cty.String, - "test_bool": cty.Bool, - "test_string": cty.String, + "foo": cty.String, + }), + cty.NullVal( + cty.Object(map[string]cty.Type{ + "foo": cty.String, + }), + ), + ), + }, + ProposedNewState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "foo": cty.String, }), cty.ObjectVal(map[string]cty.Value{ - "id": cty.StringVal("test-id"), - "test_bool": cty.BoolVal(false), - "test_string": cty.StringVal("prior-state-val"), + "id": cty.UnknownVal(cty.String), + "foo": cty.StringVal("from-config!"), }), ), }, - }, - expected: &tfprotov5.ReadResourceResponse{ - NewState: &tfprotov5.DynamicValue{ + Config: &tfprotov5.DynamicValue{ MsgPack: mustMsgpackMarshal( cty.Object(map[string]cty.Type{ - "id": cty.String, - "test_bool": cty.Bool, - "test_string": cty.String, + "id": cty.String, + "foo": cty.String, }), cty.ObjectVal(map[string]cty.Value{ - "id": cty.StringVal("test-id"), - "test_bool": cty.BoolVal(false), - "test_string": cty.StringVal("prior-state-val"), + "id": cty.NullVal(cty.String), + "foo": cty.StringVal("from-config!"), }), ), }, + }, + expected: &tfprotov5.PlanResourceChangeResponse{ Deferred: &tfprotov5.Deferred{ Reason: tfprotov5.DeferredReasonProviderConfigUnknown, }, + // Returns proposed new state with deferred response + PlannedState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "foo": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.UnknownVal(cty.String), + "foo": cty.StringVal("from-config!"), + }), + ), + }, + UnsafeToUseLegacyTypeSystem: true, }, }, - } - - for name, testCase := range testCases { - name, testCase := name, testCase - - t.Run(name, func(t *testing.T) { - t.Parallel() - resp, err := testCase.server.ReadResource(context.Background(), testCase.req) - - if err != nil { - t.Fatal(err) - } - - if diff := cmp.Diff(resp, testCase.expected, valueComparer); diff != "" { - ty := testCase.server.getResourceSchemaBlock("test").ImpliedType() - - if resp != nil && resp.NewState != nil { - t.Logf("resp.NewState.MsgPack: %s", mustMsgpackUnmarshal(ty, resp.NewState.MsgPack)) - } - - if testCase.expected != nil && testCase.expected.NewState != nil { - t.Logf("expected: %s", mustMsgpackUnmarshal(ty, testCase.expected.NewState.MsgPack)) - } - - t.Error(diff) - } - }) - } -} - -func TestPlanResourceChange(t *testing.T) { - t.Parallel() - - testCases := map[string]struct { - server *GRPCProviderServer - req *tfprotov5.PlanResourceChangeRequest - expected *tfprotov5.PlanResourceChangeResponse - }{ - "basic-plan": { + "create: write-only value can be retrieved in CustomizeDiff": { server: NewGRPCProviderServer(&Provider{ ResourcesMap: map[string]*Resource{ "test": { SchemaVersion: 4, + CustomizeDiff: func(ctx context.Context, d *ResourceDiff, i interface{}) error { + val := d.Get("foo") + if val != "bar" { + t.Fatalf("Incorrect write-only value") + } + + return nil + }, Schema: map[string]*Schema{ "foo": { - Type: TypeInt, - Optional: true, + Type: TypeString, + Optional: true, + WriteOnly: true, }, }, }, @@ -4239,11 +5223,11 @@ func TestPlanResourceChange(t *testing.T) { PriorState: &tfprotov5.DynamicValue{ MsgPack: mustMsgpackMarshal( cty.Object(map[string]cty.Type{ - "foo": cty.Number, + "foo": cty.String, }), cty.NullVal( cty.Object(map[string]cty.Type{ - "foo": cty.Number, + "foo": cty.String, }), ), ), @@ -4252,11 +5236,11 @@ func TestPlanResourceChange(t *testing.T) { MsgPack: mustMsgpackMarshal( cty.Object(map[string]cty.Type{ "id": cty.String, - "foo": cty.Number, + "foo": cty.String, }), cty.ObjectVal(map[string]cty.Value{ "id": cty.UnknownVal(cty.String), - "foo": cty.NullVal(cty.Number), + "foo": cty.StringVal("bar"), }), ), }, @@ -4264,11 +5248,11 @@ func TestPlanResourceChange(t *testing.T) { MsgPack: mustMsgpackMarshal( cty.Object(map[string]cty.Type{ "id": cty.String, - "foo": cty.Number, + "foo": cty.String, }), cty.ObjectVal(map[string]cty.Value{ "id": cty.NullVal(cty.String), - "foo": cty.NullVal(cty.Number), + "foo": cty.StringVal("bar"), }), ), }, @@ -4278,31 +5262,36 @@ func TestPlanResourceChange(t *testing.T) { MsgPack: mustMsgpackMarshal( cty.Object(map[string]cty.Type{ "id": cty.String, - "foo": cty.Number, + "foo": cty.String, }), cty.ObjectVal(map[string]cty.Value{ "id": cty.UnknownVal(cty.String), - "foo": cty.NullVal(cty.Number), + "foo": cty.NullVal(cty.String), }), ), }, + PlannedPrivate: []byte(`{"_new_extra_shim":{}}`), RequiresReplace: []*tftypes.AttributePath{ tftypes.NewAttributePath().WithAttributeName("id"), }, - PlannedPrivate: []byte(`{"_new_extra_shim":{}}`), UnsafeToUseLegacyTypeSystem: true, }, }, - "basic-plan-EnableLegacyTypeSystemPlanErrors": { + "create: write-only values are nullified in PlanResourceChangeResponse": { server: NewGRPCProviderServer(&Provider{ ResourcesMap: map[string]*Resource{ "test": { - // Will set UnsafeToUseLegacyTypeSystem to false - EnableLegacyTypeSystemPlanErrors: true, + SchemaVersion: 4, Schema: map[string]*Schema{ "foo": { - Type: TypeInt, - Optional: true, + Type: TypeString, + Optional: true, + WriteOnly: true, + }, + "bar": { + Type: TypeString, + Optional: true, + WriteOnly: true, }, }, }, @@ -4313,11 +5302,13 @@ func TestPlanResourceChange(t *testing.T) { PriorState: &tfprotov5.DynamicValue{ MsgPack: mustMsgpackMarshal( cty.Object(map[string]cty.Type{ - "foo": cty.Number, + "foo": cty.String, + "bar": cty.String, }), cty.NullVal( cty.Object(map[string]cty.Type{ - "foo": cty.Number, + "foo": cty.String, + "bar": cty.String, }), ), ), @@ -4326,11 +5317,13 @@ func TestPlanResourceChange(t *testing.T) { MsgPack: mustMsgpackMarshal( cty.Object(map[string]cty.Type{ "id": cty.String, - "foo": cty.Number, + "foo": cty.String, + "bar": cty.String, }), cty.ObjectVal(map[string]cty.Value{ "id": cty.UnknownVal(cty.String), - "foo": cty.NullVal(cty.Number), + "foo": cty.StringVal("baz"), + "bar": cty.StringVal("boop"), }), ), }, @@ -4338,11 +5331,13 @@ func TestPlanResourceChange(t *testing.T) { MsgPack: mustMsgpackMarshal( cty.Object(map[string]cty.Type{ "id": cty.String, - "foo": cty.Number, + "foo": cty.String, + "bar": cty.String, }), cty.ObjectVal(map[string]cty.Value{ "id": cty.NullVal(cty.String), - "foo": cty.NullVal(cty.Number), + "foo": cty.StringVal("baz"), + "bar": cty.StringVal("boop"), }), ), }, @@ -4352,43 +5347,45 @@ func TestPlanResourceChange(t *testing.T) { MsgPack: mustMsgpackMarshal( cty.Object(map[string]cty.Type{ "id": cty.String, - "foo": cty.Number, + "foo": cty.String, + "bar": cty.String, }), cty.ObjectVal(map[string]cty.Value{ "id": cty.UnknownVal(cty.String), - "foo": cty.NullVal(cty.Number), + "foo": cty.NullVal(cty.String), + "bar": cty.NullVal(cty.String), }), ), }, + PlannedPrivate: []byte(`{"_new_extra_shim":{}}`), RequiresReplace: []*tftypes.AttributePath{ tftypes.NewAttributePath().WithAttributeName("id"), }, - PlannedPrivate: []byte(`{"_new_extra_shim":{}}`), - UnsafeToUseLegacyTypeSystem: false, + UnsafeToUseLegacyTypeSystem: true, }, }, - "deferred-with-provider-plan-modification": { + "update: write-only value can be retrieved in CustomizeDiff": { server: NewGRPCProviderServer(&Provider{ - providerDeferred: &Deferred{ - Reason: DeferredReasonProviderConfigUnknown, - }, ResourcesMap: map[string]*Resource{ "test": { - ResourceBehavior: ResourceBehavior{ - ProviderDeferred: ProviderDeferredBehavior{ - // Will ensure that CustomizeDiff is called - EnablePlanModification: true, - }, - }, SchemaVersion: 4, CustomizeDiff: func(ctx context.Context, d *ResourceDiff, i interface{}) error { - return d.SetNew("foo", "new-foo-value") + val := d.Get("write_only") + if val != "bar" { + t.Fatalf("Incorrect write-only value") + } + + return nil }, Schema: map[string]*Schema{ - "foo": { + "configured": { Type: TypeString, Optional: true, - Computed: true, + }, + "write_only": { + Type: TypeString, + Optional: true, + WriteOnly: true, }, }, }, @@ -4396,85 +5393,90 @@ func TestPlanResourceChange(t *testing.T) { }), req: &tfprotov5.PlanResourceChangeRequest{ TypeName: "test", - ClientCapabilities: &tfprotov5.PlanResourceChangeClientCapabilities{ - DeferralAllowed: true, - }, PriorState: &tfprotov5.DynamicValue{ MsgPack: mustMsgpackMarshal( cty.Object(map[string]cty.Type{ - "foo": cty.String, + "id": cty.String, + "configured": cty.String, + "write_only": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.NullVal(cty.String), + "configured": cty.StringVal("prior_val"), + "write_only": cty.NullVal(cty.String), }), - cty.NullVal( - cty.Object(map[string]cty.Type{ - "foo": cty.String, - }), - ), ), }, ProposedNewState: &tfprotov5.DynamicValue{ MsgPack: mustMsgpackMarshal( cty.Object(map[string]cty.Type{ - "id": cty.String, - "foo": cty.String, + "id": cty.String, + "configured": cty.String, + "write_only": cty.String, }), cty.ObjectVal(map[string]cty.Value{ - "id": cty.UnknownVal(cty.String), - "foo": cty.UnknownVal(cty.String), + "id": cty.UnknownVal(cty.String), + "configured": cty.StringVal("updated_val"), + "write_only": cty.StringVal("bar"), }), ), }, Config: &tfprotov5.DynamicValue{ MsgPack: mustMsgpackMarshal( cty.Object(map[string]cty.Type{ - "id": cty.String, - "foo": cty.String, + "id": cty.String, + "configured": cty.String, + "write_only": cty.String, }), cty.ObjectVal(map[string]cty.Value{ - "id": cty.NullVal(cty.String), - "foo": cty.NullVal(cty.String), + "id": cty.NullVal(cty.String), + "configured": cty.StringVal("updated_val"), + "write_only": cty.StringVal("bar"), }), ), }, }, expected: &tfprotov5.PlanResourceChangeResponse{ - Deferred: &tfprotov5.Deferred{ - Reason: tfprotov5.DeferredReasonProviderConfigUnknown, - }, PlannedState: &tfprotov5.DynamicValue{ MsgPack: mustMsgpackMarshal( cty.Object(map[string]cty.Type{ - "id": cty.String, - "foo": cty.String, + "id": cty.String, + "configured": cty.String, + "write_only": cty.String, }), cty.ObjectVal(map[string]cty.Value{ - "id": cty.UnknownVal(cty.String), - "foo": cty.StringVal("new-foo-value"), + "id": cty.UnknownVal(cty.String), + "configured": cty.StringVal("updated_val"), + "write_only": cty.NullVal(cty.String), }), ), }, + PlannedPrivate: []byte(`{"_new_extra_shim":{}}`), RequiresReplace: []*tftypes.AttributePath{ tftypes.NewAttributePath().WithAttributeName("id"), }, - PlannedPrivate: []byte(`{"_new_extra_shim":{}}`), UnsafeToUseLegacyTypeSystem: true, }, }, - "deferred-skip-plan-modification": { + "update: write-only values are nullified in PlanResourceChangeResponse": { server: NewGRPCProviderServer(&Provider{ - providerDeferred: &Deferred{ - Reason: DeferredReasonProviderConfigUnknown, - }, ResourcesMap: map[string]*Resource{ "test": { SchemaVersion: 4, - CustomizeDiff: func(ctx context.Context, d *ResourceDiff, i interface{}) error { - return errors.New("Test assertion failed: CustomizeDiff shouldn't be called") - }, Schema: map[string]*Schema{ - "foo": { + "configured": { Type: TypeString, Optional: true, - Computed: true, + }, + "write_onlyA": { + Type: TypeString, + Optional: true, + WriteOnly: true, + }, + "write_onlyB": { + Type: TypeString, + Optional: true, + WriteOnly: true, }, }, }, @@ -4482,63 +5484,76 @@ func TestPlanResourceChange(t *testing.T) { }), req: &tfprotov5.PlanResourceChangeRequest{ TypeName: "test", - ClientCapabilities: &tfprotov5.PlanResourceChangeClientCapabilities{ - DeferralAllowed: true, - }, PriorState: &tfprotov5.DynamicValue{ MsgPack: mustMsgpackMarshal( cty.Object(map[string]cty.Type{ - "foo": cty.String, + "id": cty.String, + "configured": cty.String, + "write_onlyA": cty.String, + "write_onlyB": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.NullVal(cty.String), + "configured": cty.StringVal("prior_val"), + "write_onlyA": cty.NullVal(cty.String), + "write_onlyB": cty.NullVal(cty.String), }), - cty.NullVal( - cty.Object(map[string]cty.Type{ - "foo": cty.String, - }), - ), ), }, ProposedNewState: &tfprotov5.DynamicValue{ MsgPack: mustMsgpackMarshal( cty.Object(map[string]cty.Type{ - "id": cty.String, - "foo": cty.String, + "id": cty.String, + "configured": cty.String, + "write_onlyA": cty.String, + "write_onlyB": cty.String, }), cty.ObjectVal(map[string]cty.Value{ - "id": cty.UnknownVal(cty.String), - "foo": cty.StringVal("from-config!"), + "id": cty.UnknownVal(cty.String), + "configured": cty.StringVal("updated_val"), + "write_onlyA": cty.StringVal("foo"), + "write_onlyB": cty.StringVal("bar"), }), ), }, Config: &tfprotov5.DynamicValue{ MsgPack: mustMsgpackMarshal( cty.Object(map[string]cty.Type{ - "id": cty.String, - "foo": cty.String, + "id": cty.String, + "configured": cty.String, + "write_onlyA": cty.String, + "write_onlyB": cty.String, }), cty.ObjectVal(map[string]cty.Value{ - "id": cty.NullVal(cty.String), - "foo": cty.StringVal("from-config!"), + "id": cty.NullVal(cty.String), + "configured": cty.StringVal("updated_val"), + "write_onlyA": cty.StringVal("foo"), + "write_onlyB": cty.StringVal("bar"), }), ), }, }, expected: &tfprotov5.PlanResourceChangeResponse{ - Deferred: &tfprotov5.Deferred{ - Reason: tfprotov5.DeferredReasonProviderConfigUnknown, - }, - // Returns proposed new state with deferred response PlannedState: &tfprotov5.DynamicValue{ MsgPack: mustMsgpackMarshal( cty.Object(map[string]cty.Type{ - "id": cty.String, - "foo": cty.String, + "id": cty.String, + "configured": cty.String, + "write_onlyA": cty.String, + "write_onlyB": cty.String, }), cty.ObjectVal(map[string]cty.Value{ - "id": cty.UnknownVal(cty.String), - "foo": cty.StringVal("from-config!"), + "id": cty.UnknownVal(cty.String), + "configured": cty.StringVal("updated_val"), + "write_onlyA": cty.NullVal(cty.String), + "write_onlyB": cty.NullVal(cty.String), }), ), }, + PlannedPrivate: []byte(`{"_new_extra_shim":{}}`), + RequiresReplace: []*tftypes.AttributePath{ + tftypes.NewAttributePath().WithAttributeName("id"), + }, UnsafeToUseLegacyTypeSystem: true, }, }, @@ -4629,30 +5644,261 @@ func TestPlanResourceChange_bigint(t *testing.T) { }, } - resp, err := server.PlanResourceChange(context.Background(), testReq) - if err != nil { - t.Fatal(err) - } + resp, err := server.PlanResourceChange(context.Background(), testReq) + if err != nil { + t.Fatal(err) + } + + plannedStateVal, err := msgpack.Unmarshal(resp.PlannedState.MsgPack, schema.ImpliedType()) + if err != nil { + t.Fatal(err) + } + + if !cmp.Equal(proposedVal, plannedStateVal, valueComparer) { + t.Fatal(cmp.Diff(proposedVal, plannedStateVal, valueComparer)) + } + + plannedStateFoo, acc := plannedStateVal.GetAttr("foo").AsBigFloat().Int64() + if acc != big.Exact { + t.Fatalf("Expected exact accuracy, got %s", acc) + } + if plannedStateFoo != 7227701560655103598 { + t.Fatalf("Expected %d, got %d, this represents a loss of precision in planning large numbers", 7227701560655103598, plannedStateFoo) + } +} + +func TestApplyResourceChange(t *testing.T) { + t.Parallel() + + testCases := map[string]struct { + server *GRPCProviderServer + req *tfprotov5.ApplyResourceChangeRequest + expected *tfprotov5.ApplyResourceChangeResponse + }{ + "create: write-only values are nullified in ApplyResourceChangeResponse": { + server: NewGRPCProviderServer(&Provider{ + ResourcesMap: map[string]*Resource{ + "test": { + SchemaVersion: 4, + CreateContext: func(_ context.Context, rd *ResourceData, _ interface{}) diag.Diagnostics { + rd.SetId("baz") + return nil + }, + Schema: map[string]*Schema{ + "foo": { + Type: TypeString, + Optional: true, + WriteOnly: true, + }, + "bar": { + Type: TypeString, + Optional: true, + WriteOnly: true, + }, + }, + }, + }, + }), + req: &tfprotov5.ApplyResourceChangeRequest{ + TypeName: "test", + PriorState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "foo": cty.String, + "bar": cty.String, + }), + cty.NullVal( + cty.Object(map[string]cty.Type{ + "foo": cty.String, + "bar": cty.String, + }), + ), + ), + }, + PlannedState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "foo": cty.String, + "bar": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.UnknownVal(cty.String), + "foo": cty.StringVal("baz"), + "bar": cty.StringVal("boop"), + }), + ), + }, + Config: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "foo": cty.String, + "bar": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.NullVal(cty.String), + "foo": cty.StringVal("baz"), + "bar": cty.StringVal("boop"), + }), + ), + }, + }, + expected: &tfprotov5.ApplyResourceChangeResponse{ + NewState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "foo": cty.String, + "bar": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("baz"), + "foo": cty.NullVal(cty.String), + "bar": cty.NullVal(cty.String), + }), + ), + }, + Private: []uint8(`{"schema_version":"4"}`), + UnsafeToUseLegacyTypeSystem: true, + }, + }, + "update: write-only values are nullified in ApplyResourceChangeResponse": { + server: NewGRPCProviderServer(&Provider{ + ResourcesMap: map[string]*Resource{ + "test": { + SchemaVersion: 4, + CreateContext: func(_ context.Context, rd *ResourceData, _ interface{}) diag.Diagnostics { + rd.SetId("baz") + s := rd.Get("configured").(string) + err := rd.Set("configured", s) + if err != nil { + return nil + } + return nil + }, + Schema: map[string]*Schema{ + "configured": { + Type: TypeString, + Optional: true, + }, + "write_onlyA": { + Type: TypeString, + Optional: true, + WriteOnly: true, + }, + "write_onlyB": { + Type: TypeString, + Optional: true, + WriteOnly: true, + }, + }, + }, + }, + }), + req: &tfprotov5.ApplyResourceChangeRequest{ + TypeName: "test", + PriorState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "configured": cty.String, + "write_onlyA": cty.String, + "write_onlyB": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.NullVal(cty.String), + "configured": cty.StringVal("prior_val"), + "write_onlyA": cty.NullVal(cty.String), + "write_onlyB": cty.NullVal(cty.String), + }), + ), + }, + PlannedState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "configured": cty.String, + "write_onlyA": cty.String, + "write_onlyB": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.UnknownVal(cty.String), + "configured": cty.StringVal("updated_val"), + "write_onlyA": cty.StringVal("foo"), + "write_onlyB": cty.StringVal("bar"), + }), + ), + }, + Config: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "configured": cty.String, + "write_onlyA": cty.String, + "write_onlyB": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.NullVal(cty.String), + "configured": cty.StringVal("updated_val"), + "write_onlyA": cty.StringVal("foo"), + "write_onlyB": cty.StringVal("bar"), + }), + ), + }, + }, + expected: &tfprotov5.ApplyResourceChangeResponse{ + NewState: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "configured": cty.String, + "write_onlyA": cty.String, + "write_onlyB": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("baz"), + "configured": cty.StringVal("updated_val"), + "write_onlyA": cty.NullVal(cty.String), + "write_onlyB": cty.NullVal(cty.String), + }), + ), + }, + Private: []uint8(`{"schema_version":"4"}`), + UnsafeToUseLegacyTypeSystem: true, + }, + }, + } + + for name, testCase := range testCases { + name, testCase := name, testCase + + t.Run(name, func(t *testing.T) { + t.Parallel() + + resp, err := testCase.server.ApplyResourceChange(context.Background(), testCase.req) + if err != nil { + t.Fatal(err) + } - plannedStateVal, err := msgpack.Unmarshal(resp.PlannedState.MsgPack, schema.ImpliedType()) - if err != nil { - t.Fatal(err) - } + if diff := cmp.Diff(resp, testCase.expected, valueComparer); diff != "" { + ty := testCase.server.getResourceSchemaBlock("test").ImpliedType() - if !cmp.Equal(proposedVal, plannedStateVal, valueComparer) { - t.Fatal(cmp.Diff(proposedVal, plannedStateVal, valueComparer)) - } + if resp != nil && resp.NewState != nil { + t.Logf("resp.NewState.MsgPack: %s", mustMsgpackUnmarshal(ty, resp.NewState.MsgPack)) + } - plannedStateFoo, acc := plannedStateVal.GetAttr("foo").AsBigFloat().Int64() - if acc != big.Exact { - t.Fatalf("Expected exact accuracy, got %s", acc) - } - if plannedStateFoo != 7227701560655103598 { - t.Fatalf("Expected %d, got %d, this represents a loss of precision in planning large numbers", 7227701560655103598, plannedStateFoo) + if testCase.expected != nil && testCase.expected.NewState != nil { + t.Logf("expected: %s", mustMsgpackUnmarshal(ty, testCase.expected.NewState.MsgPack)) + } + + t.Error(diff) + } + }) } } -func TestApplyResourceChange(t *testing.T) { +func TestApplyResourceChange_ResourceFuncs(t *testing.T) { t.Parallel() testCases := map[string]struct { @@ -4976,6 +6222,239 @@ func TestApplyResourceChange_bigint(t *testing.T) { } } +func TestApplyResourceChange_ResourceFuncs_writeOnly(t *testing.T) { + t.Parallel() + + testCases := map[string]struct { + TestResource *Resource + ExpectedUnsafeLegacyTypeSystem bool + }{ + "Create: retrieve write-only value using GetRawConfigAt": { + TestResource: &Resource{ + SchemaVersion: 4, + Schema: map[string]*Schema{ + "foo": { + Type: TypeInt, + Optional: true, + }, + "write_only_bar": { + Type: TypeString, + Optional: true, + WriteOnly: true, + }, + }, + Create: func(rd *ResourceData, _ interface{}) error { + rd.SetId("baz") + writeOnlyVal, err := rd.GetRawConfigAt(cty.GetAttrPath("write_only_bar")) + if err != nil { + t.Errorf("Unable to retrieve write only attribute, err: %v", err) + } + if writeOnlyVal.AsString() != "bar" { + t.Errorf("Incorrect write-only value: expected bar but got %s", writeOnlyVal) + } + return nil + }, + }, + ExpectedUnsafeLegacyTypeSystem: true, + }, + "CreateContext: retrieve write-only value using GetRawConfigAt": { + TestResource: &Resource{ + SchemaVersion: 4, + Schema: map[string]*Schema{ + "foo": { + Type: TypeInt, + Optional: true, + }, + "write_only_bar": { + Type: TypeString, + Optional: true, + WriteOnly: true, + }, + }, + CreateContext: func(_ context.Context, rd *ResourceData, _ interface{}) diag.Diagnostics { + rd.SetId("baz") + writeOnlyVal, err := rd.GetRawConfigAt(cty.GetAttrPath("write_only_bar")) + if err != nil { + t.Errorf("Unable to retrieve write only attribute, err: %v", err) + } + if writeOnlyVal.AsString() != "bar" { + t.Errorf("Incorrect write-only value: expected bar but got %s", writeOnlyVal) + } + return nil + }, + }, + ExpectedUnsafeLegacyTypeSystem: true, + }, + "CreateWithoutTimeout: retrieve write-only value using GetRawConfigAt": { + TestResource: &Resource{ + SchemaVersion: 4, + Schema: map[string]*Schema{ + "foo": { + Type: TypeInt, + Optional: true, + }, + "write_only_bar": { + Type: TypeString, + Optional: true, + WriteOnly: true, + }, + }, + CreateWithoutTimeout: func(_ context.Context, rd *ResourceData, _ interface{}) diag.Diagnostics { + rd.SetId("baz") + writeOnlyVal, err := rd.GetRawConfigAt(cty.GetAttrPath("write_only_bar")) + if err != nil { + t.Errorf("Unable to retrieve write only attribute, err: %v", err) + } + if writeOnlyVal.AsString() != "bar" { + t.Errorf("Incorrect write-only value: expected bar but got %s", writeOnlyVal) + } + return nil + }, + }, + ExpectedUnsafeLegacyTypeSystem: true, + }, + "CreateContext with SchemaFunc: retrieve write-only value using GetRawConfigAt": { + TestResource: &Resource{ + SchemaFunc: func() map[string]*Schema { + return map[string]*Schema{ + "id": { + Type: TypeString, + Computed: true, + }, + "write_only_bar": { + Type: TypeString, + Optional: true, + WriteOnly: true, + }, + } + }, + CreateContext: func(_ context.Context, rd *ResourceData, _ interface{}) diag.Diagnostics { + rd.SetId("baz") + writeOnlyVal, err := rd.GetRawConfigAt(cty.GetAttrPath("write_only_bar")) + if err != nil { + t.Errorf("Unable to retrieve write only attribute, err: %v", err) + } + if writeOnlyVal.AsString() != "bar" { + t.Errorf("Incorrect write-only value: expected bar but got %s", writeOnlyVal) + } + return nil + }, + }, + ExpectedUnsafeLegacyTypeSystem: true, + }, + "CreateContext: retrieve write-only value using GetRawConfig": { + TestResource: &Resource{ + SchemaVersion: 4, + Schema: map[string]*Schema{ + "foo": { + Type: TypeInt, + Optional: true, + }, + "write_only_bar": { + Type: TypeString, + Optional: true, + WriteOnly: true, + }, + }, + CreateContext: func(_ context.Context, rd *ResourceData, _ interface{}) diag.Diagnostics { + rd.SetId("baz") + if rd.GetRawConfig().IsNull() { + return diag.FromErr(errors.New("null raw writeOnly val")) + } + if rd.GetRawConfig().GetAttr("write_only_bar").Type() != cty.String { + return diag.FromErr(errors.New("write_only_bar is not of the expected type string")) + } + writeOnlyVal := rd.GetRawConfig().GetAttr("write_only_bar").AsString() + if writeOnlyVal != "bar" { + t.Errorf("Incorrect write-only value: expected bar but got %s", writeOnlyVal) + } + return nil + }, + }, + ExpectedUnsafeLegacyTypeSystem: true, + }, + } + + for name, testCase := range testCases { + name, testCase := name, testCase + + t.Run(name, func(t *testing.T) { + t.Parallel() + + server := NewGRPCProviderServer(&Provider{ + ResourcesMap: map[string]*Resource{ + "test": testCase.TestResource, + }, + }) + + schema := testCase.TestResource.CoreConfigSchema() + priorState, err := msgpack.Marshal(cty.NullVal(schema.ImpliedType()), schema.ImpliedType()) + if err != nil { + t.Fatal(err) + } + + // A proposed state with only the ID unknown will produce a nil diff, and + // should return the proposed state value. + plannedVal, err := schema.CoerceValue(cty.ObjectVal(map[string]cty.Value{ + "id": cty.UnknownVal(cty.String), + })) + if err != nil { + t.Fatal(err) + } + plannedState, err := msgpack.Marshal(plannedVal, schema.ImpliedType()) + if err != nil { + t.Fatal(err) + } + + config, err := schema.CoerceValue(cty.ObjectVal(map[string]cty.Value{ + "id": cty.NullVal(cty.String), + "write_only_bar": cty.StringVal("bar"), + })) + if err != nil { + t.Fatal(err) + } + configBytes, err := msgpack.Marshal(config, schema.ImpliedType()) + if err != nil { + t.Fatal(err) + } + + testReq := &tfprotov5.ApplyResourceChangeRequest{ + TypeName: "test", + PriorState: &tfprotov5.DynamicValue{ + MsgPack: priorState, + }, + PlannedState: &tfprotov5.DynamicValue{ + MsgPack: plannedState, + }, + Config: &tfprotov5.DynamicValue{ + MsgPack: configBytes, + }, + } + + resp, err := server.ApplyResourceChange(context.Background(), testReq) + if err != nil { + t.Fatal(err) + } + + newStateVal, err := msgpack.Unmarshal(resp.NewState.MsgPack, schema.ImpliedType()) + if err != nil { + t.Fatal(err) + } + + id := newStateVal.GetAttr("id").AsString() + if id != "baz" { + t.Fatalf("incorrect final state: %#v\n", newStateVal) + } + + //nolint:staticcheck // explicitly for this SDK + if testCase.ExpectedUnsafeLegacyTypeSystem != resp.UnsafeToUseLegacyTypeSystem { + //nolint:staticcheck // explicitly for this SDK + t.Fatalf("expected UnsafeLegacyTypeSystem %t, got: %t", testCase.ExpectedUnsafeLegacyTypeSystem, resp.UnsafeToUseLegacyTypeSystem) + } + }) + } +} + func TestImportResourceState(t *testing.T) { t.Parallel() @@ -5174,6 +6653,60 @@ func TestImportResourceState(t *testing.T) { }, }, }, + "write-only-nullification": { + server: NewGRPCProviderServer(&Provider{ + ResourcesMap: map[string]*Resource{ + "test": { + SchemaVersion: 1, + Schema: map[string]*Schema{ + "id": { + Type: TypeString, + Required: true, + }, + "test_string": { + Type: TypeString, + Optional: true, + WriteOnly: true, + }, + }, + Importer: &ResourceImporter{ + StateContext: func(ctx context.Context, d *ResourceData, meta interface{}) ([]*ResourceData, error) { + err := d.Set("test_string", "new-imported-val") + if err != nil { + return nil, err + } + + return []*ResourceData{d}, nil + }, + }, + }, + }, + }), + req: &tfprotov5.ImportResourceStateRequest{ + TypeName: "test", + ID: "imported-id", + }, + expected: &tfprotov5.ImportResourceStateResponse{ + ImportedResources: []*tfprotov5.ImportedResource{ + { + TypeName: "test", + State: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + "test_string": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("imported-id"), + "test_string": cty.NullVal(cty.String), + }), + ), + }, + Private: []byte(`{"schema_version":"1"}`), + }, + }, + }, + }, } for name, testCase := range testCases { diff --git a/helper/schema/provider.go b/helper/schema/provider.go index a75ae2fc28..45f1e0d466 100644 --- a/helper/schema/provider.go +++ b/helper/schema/provider.go @@ -13,6 +13,7 @@ import ( "strings" "github.com/hashicorp/terraform-plugin-go/tfprotov5" + "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/internal/configs/configschema" "github.com/hashicorp/terraform-plugin-sdk/v2/internal/logging" @@ -192,11 +193,23 @@ func (p *Provider) InternalValidate() error { } var validationErrors []error + + // Provider schema validation sm := schemaMap(p.Schema) if err := sm.InternalValidate(sm); err != nil { validationErrors = append(validationErrors, err) } + if sm.hasWriteOnly() { + validationErrors = append(validationErrors, fmt.Errorf("provider schema cannot contain write-only attributes")) + } + + // Provider meta schema validation + providerMeta := schemaMap(p.ProviderMetaSchema) + if providerMeta.hasWriteOnly() { + validationErrors = append(validationErrors, fmt.Errorf("provider meta schema cannot contain write-only attributes")) + } + // Provider-specific checks for k := range sm { if isReservedProviderFieldName(k) { @@ -214,6 +227,15 @@ func (p *Provider) InternalValidate() error { if err := r.InternalValidate(nil, false); err != nil { validationErrors = append(validationErrors, fmt.Errorf("data source %s: %s", k, err)) } + + if len(r.ValidateRawResourceConfigFuncs) > 0 { + validationErrors = append(validationErrors, fmt.Errorf("data source %s cannot contain ValidateRawResourceConfigFuncs", k)) + } + + dataSourceSchema := schemaMap(r.SchemaMap()) + if dataSourceSchema.hasWriteOnly() { + validationErrors = append(validationErrors, fmt.Errorf("data source %s cannot contain write-only attributes", k)) + } } return errors.Join(validationErrors...) diff --git a/helper/schema/provider_test.go b/helper/schema/provider_test.go index dcab8acd71..8da40f5c65 100644 --- a/helper/schema/provider_test.go +++ b/helper/schema/provider_test.go @@ -2288,11 +2288,11 @@ func TestProviderMeta(t *testing.T) { } func TestProvider_InternalValidate(t *testing.T) { - cases := []struct { + cases := map[string]struct { P *Provider ExpectedErr error }{ - { + "Provider with schema returns no errors": { P: &Provider{ Schema: map[string]*Schema{ "foo": { @@ -2303,7 +2303,7 @@ func TestProvider_InternalValidate(t *testing.T) { }, ExpectedErr: nil, }, - { // Reserved resource fields should be allowed in provider block + "Reserved resource fields in provider block returns no errors": { P: &Provider{ Schema: map[string]*Schema{ "provisioner": { @@ -2318,7 +2318,7 @@ func TestProvider_InternalValidate(t *testing.T) { }, ExpectedErr: nil, }, - { // Reserved provider fields should not be allowed + "Reserved provider fields returns an error": { // P: &Provider{ Schema: map[string]*Schema{ "alias": { @@ -2329,7 +2329,7 @@ func TestProvider_InternalValidate(t *testing.T) { }, ExpectedErr: fmt.Errorf("%s is a reserved field name for a provider", "alias"), }, - { // ConfigureFunc and ConfigureContext cannot both be set + "Provider with ConfigureFunc and ConfigureContext both set returns an error": { P: &Provider{ Schema: map[string]*Schema{ "foo": { @@ -2346,22 +2346,156 @@ func TestProvider_InternalValidate(t *testing.T) { }, ExpectedErr: fmt.Errorf("ConfigureFunc and ConfigureContextFunc must not both be set"), }, + "Provider schema with WriteOnly attribute set returns an error": { + P: &Provider{ + Schema: map[string]*Schema{ + "foo": { + Type: TypeString, + Optional: true, + WriteOnly: true, + }, + }, + }, + ExpectedErr: fmt.Errorf("provider schema cannot contain write-only attributes"), + }, + "Provider meta schema with WriteOnly attribute set returns an error": { + P: &Provider{ + Schema: map[string]*Schema{ + "foo": { + Type: TypeString, + Optional: true, + }, + }, + ProviderMetaSchema: map[string]*Schema{ + "meta-foo": { + Type: TypeString, + Optional: true, + WriteOnly: true, + }, + }, + }, + ExpectedErr: fmt.Errorf("provider meta schema cannot contain write-only attributes"), + }, + "Data source schema with WriteOnly attribute set returns an error": { + P: &Provider{ + Schema: map[string]*Schema{ + "foo": { + Type: TypeString, + Optional: true, + }, + }, + DataSourcesMap: map[string]*Resource{ + "data-foo": { + Schema: map[string]*Schema{ + "foo": { + Type: TypeString, + Optional: true, + WriteOnly: true, + }, + }, + }, + }, + }, + ExpectedErr: fmt.Errorf("data source data-foo cannot contain write-only attributes"), + }, + "Resource schema with WriteOnly attribute set returns no errors": { + P: &Provider{ + Schema: map[string]*Schema{ + "foo": { + Type: TypeString, + Optional: true, + }, + }, + ResourcesMap: map[string]*Resource{ + "resource-foo": { + Schema: map[string]*Schema{ + "foo": { + Type: TypeString, + Optional: true, + WriteOnly: true, + }, + }, + }, + }, + }, + ExpectedErr: nil, + }, + "Data source with ValidateRawResourceConfigFuncs returns an error": { + P: &Provider{ + Schema: map[string]*Schema{ + "foo": { + Type: TypeString, + Optional: true, + }, + }, + DataSourcesMap: map[string]*Resource{ + "data-foo": { + ValidateRawResourceConfigFuncs: []ValidateRawResourceConfigFunc{ + func(ctx context.Context, req ValidateResourceConfigFuncRequest, resp *ValidateResourceConfigFuncResponse) { + + }, + func(ctx context.Context, req ValidateResourceConfigFuncRequest, resp *ValidateResourceConfigFuncResponse) { + + }, + }, + Schema: map[string]*Schema{ + "foo": { + Type: TypeString, + Optional: true, + }, + }, + }, + }, + }, + ExpectedErr: fmt.Errorf("data source data-foo cannot contain ValidateRawResourceConfigFuncs"), + }, + "Resource with ValidateRawResourceConfigFuncs returns no errors": { + P: &Provider{ + Schema: map[string]*Schema{ + "foo": { + Type: TypeString, + Optional: true, + }, + }, + ResourcesMap: map[string]*Resource{ + "resource-foo": { + ValidateRawResourceConfigFuncs: []ValidateRawResourceConfigFunc{ + func(ctx context.Context, req ValidateResourceConfigFuncRequest, resp *ValidateResourceConfigFuncResponse) { + + }, + func(ctx context.Context, req ValidateResourceConfigFuncRequest, resp *ValidateResourceConfigFuncResponse) { + + }, + }, + Schema: map[string]*Schema{ + "foo": { + Type: TypeString, + Optional: true, + WriteOnly: true, + }, + }, + }, + }, + }, + ExpectedErr: nil, + }, } - for i, tc := range cases { - err := tc.P.InternalValidate() - if tc.ExpectedErr == nil { - if err != nil { - t.Fatalf("%d: Error returned (expected no error): %s", i, err) + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + err := tc.P.InternalValidate() + if tc.ExpectedErr == nil { + if err != nil { + t.Fatalf("Error returned (expected no error): %s", err) + } } - continue - } - if tc.ExpectedErr != nil && err == nil { - t.Fatalf("%d: Expected error (%s), but no error returned", i, tc.ExpectedErr) - } - if err.Error() != tc.ExpectedErr.Error() { - t.Fatalf("%d: Errors don't match. Expected: %#v Given: %#v", i, tc.ExpectedErr, err) - } + if tc.ExpectedErr != nil && err == nil { + t.Fatalf("Expected error (%s), but no error returned", tc.ExpectedErr) + } + if tc.ExpectedErr != nil && err.Error() != tc.ExpectedErr.Error() { + t.Fatalf("Errors don't match. Expected: %#v Given: %#v", tc.ExpectedErr.Error(), err.Error()) + } + }) } } diff --git a/helper/schema/resource.go b/helper/schema/resource.go index ae7bd59752..32a21d2edf 100644 --- a/helper/schema/resource.go +++ b/helper/schema/resource.go @@ -644,6 +644,19 @@ type Resource struct { // ResourceBehavior is used to control SDK-specific logic when // interacting with this resource. ResourceBehavior ResourceBehavior + + // ValidateRawResourceConfigFuncs allows functions to define arbitrary validation + // logic during the ValidateResourceTypeConfig RPC. ValidateRawResourceConfigFunc receives + // the client capabilities from the ValidateResourceTypeConfig RPC and the raw cty + // config value for the entire resource before it is shimmed, and it can return error + // diagnostics based on the inspection of those values. + // + // ValidateRawResourceConfigFuncs is only valid for Managed Resource types and will not be + // called for Data Resource or Provider types. + // + // Developers should prefer other validation methods first as this validation function + // deals with raw cty values. + ValidateRawResourceConfigFuncs []ValidateRawResourceConfigFunc } // ResourceBehavior controls SDK-specific logic when interacting @@ -670,6 +683,25 @@ type ProviderDeferredBehavior struct { EnablePlanModification bool } +// ValidateRawResourceConfigFunc is a function used to validate the raw resource config +// and has Diagnostic support. it is only valid for Managed Resource types and will not be +// called for Data Resource or Block types. +type ValidateRawResourceConfigFunc func(context.Context, ValidateResourceConfigFuncRequest, *ValidateResourceConfigFuncResponse) + +type ValidateResourceConfigFuncRequest struct { + // WriteOnlyAttributesAllowed indicates that the Terraform client + // initiating the request supports write-only attributes for managed + // resources. + WriteOnlyAttributesAllowed bool + + // The raw config value provided by Terraform core + RawConfig cty.Value +} + +type ValidateResourceConfigFuncResponse struct { + Diagnostics diag.Diagnostics +} + // SchemaMap returns the schema information for this Resource whether it is // defined via the SchemaFunc field or Schema field. The SchemaFunc field, if // defined, takes precedence over the Schema field. diff --git a/helper/schema/resource_data.go b/helper/schema/resource_data.go index 56306a190f..5129c925c4 100644 --- a/helper/schema/resource_data.go +++ b/helper/schema/resource_data.go @@ -4,6 +4,7 @@ package schema import ( + "fmt" "log" "reflect" "strings" @@ -13,6 +14,8 @@ import ( "github.com/google/go-cmp/cmp" "github.com/hashicorp/go-cty/cty" "github.com/hashicorp/go-cty/cty/gocty" + + "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" ) @@ -604,6 +607,67 @@ func (d *ResourceData) GetRawConfig() cty.Value { return cty.NullVal(schemaMap(d.schema).CoreConfigSchema().ImpliedType()) } +// GetRawConfigAt is a helper method for retrieving specific values +// from the RawConfig returned from GetRawConfig. It returns the cty.Value +// for a given cty.Path or an error diagnostic if the value at the given path does not exist. +// +// GetRawConfigAt is considered advanced functionality, and +// familiarity with the Terraform protocol is suggested when using it. +func (d *ResourceData) GetRawConfigAt(valPath cty.Path) (cty.Value, diag.Diagnostics) { + rawConfig := d.GetRawConfig() + configVal := cty.DynamicVal + + if rawConfig.IsNull() { + return configVal, diag.Diagnostics{ + { + Severity: diag.Error, + Summary: "Empty Raw Config", + Detail: "The Terraform Provider unexpectedly received an empty configuration. " + + "This is almost always an issue with the Terraform Plugin SDK used to create providers. " + + "Please report this to the provider developers. \n\n" + + "The RawConfig is empty.", + AttributePath: valPath, + }, + } + } + err := cty.Walk(rawConfig, func(path cty.Path, value cty.Value) (bool, error) { + if path.Equals(valPath) { + configVal = value + return false, nil + } + return true, nil + }) + if err != nil { + return configVal, diag.Diagnostics{ + { + Severity: diag.Error, + Summary: "Invalid config path", + Detail: "The Terraform Provider unexpectedly provided a path that does not match the current schema. " + + "This can happen if the path does not correctly follow the schema in structure or types. " + + "Please report this to the provider developers. \n\n" + + fmt.Sprintf("Encountered error while retrieving config value %s", err.Error()), + AttributePath: valPath, + }, + } + } + + if configVal.RawEquals(cty.DynamicVal) { + return configVal, diag.Diagnostics{ + { + Severity: diag.Error, + Summary: "Invalid config path", + Detail: "The Terraform Provider unexpectedly provided a path that does not match the current schema. " + + "This can happen if the path does not correctly follow the schema in structure or types. " + + "Please report this to the provider developers. \n\n" + + "Cannot find config value for given path.", + AttributePath: valPath, + }, + } + } + + return configVal, nil +} + // GetRawState returns the cty.Value that Terraform sent the SDK for the state. // If no value was sent, or if a null value was sent, the value will be a null // value of the resource's type. diff --git a/helper/schema/resource_data_test.go b/helper/schema/resource_data_test.go index c9f71081d3..5e1d53e8a0 100644 --- a/helper/schema/resource_data_test.go +++ b/helper/schema/resource_data_test.go @@ -10,6 +10,10 @@ import ( "testing" "time" + "github.com/google/go-cmp/cmp" + "github.com/hashicorp/go-cty/cty" + + "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" ) @@ -3915,6 +3919,169 @@ func TestResourceData_nonStringValuesInMap(t *testing.T) { } } +func TestResourceDataGetRawConfigAt(t *testing.T) { + cases := map[string]struct { + RawConfig cty.Value + Path cty.Path + Value cty.Value + ExpectedDiags diag.Diagnostics + }{ + "null RawConfig returns error": { + RawConfig: cty.NullVal(cty.EmptyObject), + Path: cty.GetAttrPath("invalid_root_path"), + Value: cty.DynamicVal, + ExpectedDiags: diag.Diagnostics{ + { + Severity: diag.Error, + Summary: "Empty Raw Config", + Detail: "The Terraform Provider unexpectedly received an empty configuration. " + + "This is almost always an issue with the Terraform Plugin SDK used to create providers. " + + "Please report this to the provider developers. \n\n" + + "The RawConfig is empty.", + AttributePath: cty.Path{ + cty.GetAttrStep{Name: "invalid_root_path"}, + }, + }, + }, + }, + "null value in config": { + RawConfig: cty.ObjectVal(map[string]cty.Value{ + "ConfigAttribute": cty.NullVal(cty.Number), + }), + Path: cty.GetAttrPath("ConfigAttribute"), + Value: cty.NullVal(cty.Number), + }, + "invalid path returns error": { + RawConfig: cty.ObjectVal(map[string]cty.Value{ + "ConfigAttribute": cty.NumberIntVal(42), + }), + Path: cty.GetAttrPath("invalid_root_path"), + Value: cty.DynamicVal, + ExpectedDiags: diag.Diagnostics{ + { + Severity: diag.Error, + Summary: "Invalid config path", + Detail: "The Terraform Provider unexpectedly provided a path that does not match the current schema. " + + "This can happen if the path does not correctly follow the schema in structure or types. " + + "Please report this to the provider developers. \n\n" + + "Cannot find config value for given path.", + AttributePath: cty.Path{ + cty.GetAttrStep{Name: "invalid_root_path"}, + }, + }, + }, + }, + "root level attribute": { + RawConfig: cty.ObjectVal(map[string]cty.Value{ + "ConfigAttribute": cty.NumberIntVal(42), + }), + Path: cty.GetAttrPath("ConfigAttribute"), + Value: cty.NumberIntVal(42), + }, + "root level set attribute": { + RawConfig: cty.ObjectVal(map[string]cty.Value{ + "ConfigAttribute": cty.SetVal([]cty.Value{ + cty.StringVal("valueA"), + cty.StringVal("valueB"), + }), + }), + Path: cty.GetAttrPath("ConfigAttribute").Index(cty.StringVal("valueA")), + Value: cty.StringVal("valueA"), + }, + "root level list attribute": { + RawConfig: cty.ObjectVal(map[string]cty.Value{ + "ConfigAttribute": cty.ListVal([]cty.Value{ + cty.StringVal("valueA"), + cty.StringVal("valueB"), + }), + }), + Path: cty.GetAttrPath("ConfigAttribute").IndexInt(0), + Value: cty.StringVal("valueA"), + }, + "root level map attribute": { + RawConfig: cty.ObjectVal(map[string]cty.Value{ + "ConfigAttribute": cty.MapVal(map[string]cty.Value{ + "mapA": cty.StringVal("valueA"), + "mapB": cty.StringVal("valueB"), + }), + }), + Path: cty.GetAttrPath("ConfigAttribute").IndexString("mapB"), + Value: cty.StringVal("valueB"), + }, + "list nested block attribute - get attribute value": { + RawConfig: cty.ObjectVal(map[string]cty.Value{ + "list_nested_block": cty.ListVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "ConfigAttribute": cty.StringVal("valueA"), + }), + cty.ObjectVal(map[string]cty.Value{ + "ConfigAttribute": cty.StringVal("valueB"), + }), + }), + }), + Path: cty.GetAttrPath("list_nested_block").IndexInt(1).GetAttr("ConfigAttribute"), + Value: cty.StringVal("valueB"), + }, + "set nested block attribute - get attribute value": { + RawConfig: cty.ObjectVal(map[string]cty.Value{ + "set_nested_block": cty.SetVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "ConfigAttribute": cty.StringVal("valueA"), + }), + cty.ObjectVal(map[string]cty.Value{ + "ConfigAttribute": cty.StringVal("valueB"), + }), + }), + }), + Path: cty.GetAttrPath("set_nested_block").Index(cty.ObjectVal(map[string]cty.Value{ + "ConfigAttribute": cty.StringVal("valueB"), + })).GetAttr("ConfigAttribute"), + Value: cty.StringVal("valueB"), + }, + "map nested block attribute - get attribute value": { + RawConfig: cty.ObjectVal(map[string]cty.Value{ + "map_nested_block": cty.MapVal(map[string]cty.Value{ + "mapA": cty.ObjectVal(map[string]cty.Value{ + "ConfigAttribute": cty.StringVal("valueA"), + }), + "mapB": cty.ObjectVal(map[string]cty.Value{ + "ConfigAttribute": cty.StringVal("valueB"), + }), + }), + }), + Path: cty.GetAttrPath("map_nested_block").IndexString("mapB").GetAttr("ConfigAttribute"), + Value: cty.StringVal("valueB"), + }, + } + + for tn, tc := range cases { + t.Run(tn, func(t *testing.T) { + diff := &terraform.InstanceDiff{ + RawConfig: tc.RawConfig, + } + d := &ResourceData{ + diff: diff, + } + + v, diags := d.GetRawConfigAt(tc.Path) + if len(diags) != 0 && tc.ExpectedDiags == nil { + t.Fatalf("expected no diagnostics but got %v", diags) + } + + if diff := cmp.Diff(tc.ExpectedDiags, diags, + cmp.AllowUnexported(cty.GetAttrStep{}, cty.IndexStep{}), + cmp.Comparer(indexStepComparer), + ); diff != "" { + t.Errorf("Unexpected diagnostics (-wanted +got): %s", diff) + } + + if !reflect.DeepEqual(v, tc.Value) { + t.Errorf("Bad: %s\n\n%#v\n\nExpected: %#v", tn, v, tc.Value) + } + }) + } +} + func TestResourceDataSetConnInfo(t *testing.T) { d := &ResourceData{} d.SetId("foo") diff --git a/helper/schema/resource_diff.go b/helper/schema/resource_diff.go index 6af9490b9e..9f7dab683b 100644 --- a/helper/schema/resource_diff.go +++ b/helper/schema/resource_diff.go @@ -11,6 +11,8 @@ import ( "sync" "github.com/hashicorp/go-cty/cty" + + "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" ) @@ -480,6 +482,67 @@ func (d *ResourceDiff) GetRawConfig() cty.Value { return cty.NullVal(schemaMap(d.schema).CoreConfigSchema().ImpliedType()) } +// GetRawConfigAt is a helper method for retrieving specific values +// from the RawConfig returned from GetRawConfig. It returns the cty.Value +// for a given cty.Path or an error diagnostic if the value at the given path does not exist. +// +// GetRawConfigAt is considered advanced functionality, and +// familiarity with the Terraform protocol is suggested when using it. +func (d *ResourceDiff) GetRawConfigAt(valPath cty.Path) (cty.Value, diag.Diagnostics) { + rawConfig := d.GetRawConfig() + configVal := cty.DynamicVal + + if rawConfig.IsNull() { + return configVal, diag.Diagnostics{ + { + Severity: diag.Error, + Summary: "Empty Raw Config", + Detail: "The Terraform Provider unexpectedly received an empty configuration. " + + "This is almost always an issue with the Terraform Plugin SDK used to create providers. " + + "Please report this to the provider developers. \n\n" + + "The RawConfig is empty.", + AttributePath: valPath, + }, + } + } + err := cty.Walk(rawConfig, func(path cty.Path, value cty.Value) (bool, error) { + if path.Equals(valPath) { + configVal = value + return false, nil + } + return true, nil + }) + if err != nil { + return configVal, diag.Diagnostics{ + { + Severity: diag.Error, + Summary: "Invalid config path", + Detail: "The Terraform Provider unexpectedly provided a path that does not match the current schema. " + + "This can happen if the path does not correctly follow the schema in structure or types. " + + "Please report this to the provider developers. \n\n" + + fmt.Sprintf("Encountered error while retrieving config value %s", err.Error()), + AttributePath: valPath, + }, + } + } + + if configVal.RawEquals(cty.DynamicVal) { + return configVal, diag.Diagnostics{ + { + Severity: diag.Error, + Summary: "Invalid config path", + Detail: "The Terraform Provider unexpectedly provided a path that does not match the current schema. " + + "This can happen if the path does not correctly follow the schema in structure or types. " + + "Please report this to the provider developers. \n\n" + + "Cannot find config value for given path.", + AttributePath: valPath, + }, + } + } + + return configVal, nil +} + // GetRawState returns the cty.Value that Terraform sent the SDK for the state. // If no value was sent, or if a null value was sent, the value will be a null // value of the resource's type. diff --git a/helper/schema/resource_diff_test.go b/helper/schema/resource_diff_test.go index d9a84676f4..ef5198214b 100644 --- a/helper/schema/resource_diff_test.go +++ b/helper/schema/resource_diff_test.go @@ -12,6 +12,9 @@ import ( "github.com/google/go-cmp/cmp" + "github.com/hashicorp/go-cty/cty" + + "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/internal/configs/hcl2shim" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" ) @@ -2306,3 +2309,103 @@ func TestResourceDiffHasChanges(t *testing.T) { } } } + +func TestResourceDiffGetRawConfigAt(t *testing.T) { + cases := map[string]struct { + RawConfig cty.Value + Path cty.Path + Value cty.Value + ExpectedDiags diag.Diagnostics + }{ + "null RawConfig returns error": { + RawConfig: cty.NullVal(cty.EmptyObject), + Path: cty.GetAttrPath("invalid_root_path"), + Value: cty.DynamicVal, + ExpectedDiags: diag.Diagnostics{ + { + Severity: diag.Error, + Summary: "Empty Raw Config", + Detail: "The Terraform Provider unexpectedly received an empty configuration. " + + "This is almost always an issue with the Terraform Plugin SDK used to create providers. " + + "Please report this to the provider developers. \n\n" + + "The RawConfig is empty.", + AttributePath: cty.Path{ + cty.GetAttrStep{Name: "invalid_root_path"}, + }, + }, + }, + }, + "invalid path returns error": { + RawConfig: cty.ObjectVal(map[string]cty.Value{ + "ConfigAttribute": cty.NumberIntVal(42), + }), + Path: cty.GetAttrPath("invalid_root_path"), + Value: cty.DynamicVal, + ExpectedDiags: diag.Diagnostics{ + { + Severity: diag.Error, + Summary: "Invalid config path", + Detail: "The Terraform Provider unexpectedly provided a path that does not match the current schema. " + + "This can happen if the path does not correctly follow the schema in structure or types. " + + "Please report this to the provider developers. \n\n" + + "Cannot find config value for given path.", + AttributePath: cty.Path{ + cty.GetAttrStep{Name: "invalid_root_path"}, + }, + }, + }, + }, + "root level attribute": { + RawConfig: cty.ObjectVal(map[string]cty.Value{ + "ConfigAttribute": cty.NumberIntVal(42), + }), + Path: cty.GetAttrPath("ConfigAttribute"), + Value: cty.NumberIntVal(42), + }, + "list nested block attribute - get attribute value": { + RawConfig: cty.ObjectVal(map[string]cty.Value{ + "list_nested_block": cty.ListVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "ConfigAttribute": cty.StringVal("valueA"), + }), + cty.ObjectVal(map[string]cty.Value{ + "ConfigAttribute": cty.StringVal("valueB"), + }), + }), + }), + Path: cty.GetAttrPath("list_nested_block").IndexInt(1).GetAttr("ConfigAttribute"), + Value: cty.StringVal("valueB"), + }, + } + + for tn, tc := range cases { + t.Run(tn, func(t *testing.T) { + diff := &terraform.InstanceDiff{ + RawConfig: tc.RawConfig, + } + d := &ResourceDiff{ + diff: diff, + } + + v, diags := d.GetRawConfigAt(tc.Path) + if len(diags) == 0 && tc.ExpectedDiags == nil { + return + } + + if len(diags) != 0 && tc.ExpectedDiags == nil { + t.Fatalf("expected no diagnostics but got %v", diags) + } + + if diff := cmp.Diff(tc.ExpectedDiags, diags, + cmp.AllowUnexported(cty.GetAttrStep{}, cty.IndexStep{}), + cmp.Comparer(indexStepComparer), + ); diff != "" { + t.Errorf("Unexpected diagnostics (-wanted +got): %s", diff) + } + + if !reflect.DeepEqual(v, tc.Value) { + t.Errorf("Bad: %s\n\n%#v\n\nExpected: %#v", tn, v, tc.Value) + } + }) + } +} diff --git a/helper/schema/resource_test.go b/helper/schema/resource_test.go index 5c9fd629ba..447338e6d5 100644 --- a/helper/schema/resource_test.go +++ b/helper/schema/resource_test.go @@ -635,19 +635,18 @@ func TestResourceApply_isNewResource(t *testing.T) { } func TestResourceInternalValidate(t *testing.T) { - cases := []struct { + cases := map[string]struct { In *Resource Writable bool Err bool }{ - 0: { + "nil": { nil, true, true, }, - // No optional and no required - 1: { + "No optional and no required": { &Resource{ Schema: map[string]*Schema{ "foo": { @@ -661,8 +660,7 @@ func TestResourceInternalValidate(t *testing.T) { true, }, - // Update undefined for non-ForceNew field - 2: { + "Update undefined for non-ForceNew field": { &Resource{ Create: Noop, Schema: map[string]*Schema{ @@ -676,8 +674,7 @@ func TestResourceInternalValidate(t *testing.T) { true, }, - // Update defined for ForceNew field - 3: { + "Update defined for ForceNew field": { &Resource{ Create: Noop, Update: Noop, @@ -693,8 +690,7 @@ func TestResourceInternalValidate(t *testing.T) { true, }, - // non-writable doesn't need Update, Create or Delete - 4: { + "non-writable doesn't need Update, Create or Delete": { &Resource{ Schema: map[string]*Schema{ "goo": { @@ -707,8 +703,7 @@ func TestResourceInternalValidate(t *testing.T) { false, }, - // non-writable *must not* have Create - 5: { + "non-writable *must not* have Create": { &Resource{ Create: Noop, Schema: map[string]*Schema{ @@ -722,8 +717,7 @@ func TestResourceInternalValidate(t *testing.T) { true, }, - // writable must have Read - 6: { + "writable must have Read": { &Resource{ Create: Noop, Update: Noop, @@ -739,8 +733,7 @@ func TestResourceInternalValidate(t *testing.T) { true, }, - // writable must have Delete - 7: { + "writable must have Delete": { &Resource{ Create: Noop, Read: Noop, @@ -756,7 +749,7 @@ func TestResourceInternalValidate(t *testing.T) { true, }, - 8: { // Reserved name at root should be disallowed + "Reserved name at root should be disallowed": { &Resource{ Create: Noop, Read: Noop, @@ -773,7 +766,7 @@ func TestResourceInternalValidate(t *testing.T) { true, }, - 9: { // Reserved name at nested levels should be allowed + "Reserved name at nested levels should be allowed": { &Resource{ Create: Noop, Read: Noop, @@ -798,7 +791,7 @@ func TestResourceInternalValidate(t *testing.T) { false, }, - 10: { // Provider reserved name should be allowed in resource + "Provider reserved name should be allowed in resource": { &Resource{ Create: Noop, Read: Noop, @@ -815,7 +808,7 @@ func TestResourceInternalValidate(t *testing.T) { false, }, - 11: { // ID should be allowed in data source + "ID should be allowed in data source": { &Resource{ Read: Noop, Schema: map[string]*Schema{ @@ -829,7 +822,7 @@ func TestResourceInternalValidate(t *testing.T) { false, }, - 12: { // Deprecated ID should be allowed in resource + "Deprecated ID should be allowed in resource": { &Resource{ Create: Noop, Read: Noop, @@ -848,7 +841,7 @@ func TestResourceInternalValidate(t *testing.T) { false, }, - 13: { // non-writable must not define CustomizeDiff + "non-writable must not define CustomizeDiff": { &Resource{ Read: Noop, Schema: map[string]*Schema{ @@ -862,7 +855,7 @@ func TestResourceInternalValidate(t *testing.T) { false, true, }, - 14: { // Deprecated resource + "Deprecated resource": { &Resource{ Read: Noop, Schema: map[string]*Schema{ @@ -876,7 +869,7 @@ func TestResourceInternalValidate(t *testing.T) { true, true, }, - 15: { // Create and CreateContext should not both be set + "Create and CreateContext should not both be set": { &Resource{ Create: Noop, CreateContext: NoopContext, @@ -893,7 +886,7 @@ func TestResourceInternalValidate(t *testing.T) { true, true, }, - 16: { // Read and ReadContext should not both be set + "Read and ReadContext should not both be set": { &Resource{ Create: Noop, Read: Noop, @@ -910,7 +903,7 @@ func TestResourceInternalValidate(t *testing.T) { true, true, }, - 17: { // Update and UpdateContext should not both be set + "Update and UpdateContext should not both be set": { &Resource{ Create: Noop, Read: Noop, @@ -927,7 +920,7 @@ func TestResourceInternalValidate(t *testing.T) { true, true, }, - 18: { // Delete and DeleteContext should not both be set + "Delete and DeleteContext should not both be set": { &Resource{ Create: Noop, Read: Noop, @@ -944,7 +937,7 @@ func TestResourceInternalValidate(t *testing.T) { true, true, }, - 19: { // Create and CreateWithoutTimeout should not both be set + "Create and CreateWithoutTimeout should not both be set": { &Resource{ Create: Noop, CreateWithoutTimeout: NoopContext, @@ -961,7 +954,7 @@ func TestResourceInternalValidate(t *testing.T) { true, true, }, - 20: { // Read and ReadWithoutTimeout should not both be set + "Read and ReadWithoutTimeout should not both be set": { &Resource{ Create: Noop, Read: Noop, @@ -978,7 +971,7 @@ func TestResourceInternalValidate(t *testing.T) { true, true, }, - 21: { // Update and UpdateWithoutTimeout should not both be set + "Update and UpdateWithoutTimeout should not both be set": { &Resource{ Create: Noop, Read: Noop, @@ -995,7 +988,7 @@ func TestResourceInternalValidate(t *testing.T) { true, true, }, - 22: { // Delete and DeleteWithoutTimeout should not both be set + "Delete and DeleteWithoutTimeout should not both be set": { &Resource{ Create: Noop, Read: Noop, @@ -1012,7 +1005,7 @@ func TestResourceInternalValidate(t *testing.T) { true, true, }, - 23: { // CreateContext and CreateWithoutTimeout should not both be set + "CreateContext and CreateWithoutTimeout should not both be set": { &Resource{ CreateContext: NoopContext, CreateWithoutTimeout: NoopContext, @@ -1029,7 +1022,7 @@ func TestResourceInternalValidate(t *testing.T) { true, true, }, - 24: { // ReadContext and ReadWithoutTimeout should not both be set + "ReadContext and ReadWithoutTimeout should not both be set": { &Resource{ Create: Noop, ReadContext: NoopContext, @@ -1046,7 +1039,7 @@ func TestResourceInternalValidate(t *testing.T) { true, true, }, - 25: { // UpdateContext and UpdateWithoutTimeout should not both be set + "UpdateContext and UpdateWithoutTimeout should not both be set": { &Resource{ Create: Noop, Read: Noop, @@ -1063,7 +1056,7 @@ func TestResourceInternalValidate(t *testing.T) { true, true, }, - 26: { // DeleteContext and DeleteWithoutTimeout should not both be set + "DeleteContext and DeleteWithoutTimeout should not both be set": { &Resource{ Create: Noop, Read: Noop, @@ -1080,7 +1073,7 @@ func TestResourceInternalValidate(t *testing.T) { true, true, }, - 27: { // Non-Writable SchemaFunc and Schema should not both be set + "Non-Writable SchemaFunc and Schema should not both be set": { In: &Resource{ Schema: map[string]*Schema{ "test": { @@ -1101,7 +1094,7 @@ func TestResourceInternalValidate(t *testing.T) { Writable: false, Err: true, }, - 28: { // Writable SchemaFunc and Schema should not both be set + "Writable SchemaFunc and Schema should not both be set": { In: &Resource{ Schema: map[string]*Schema{ "test": { @@ -1127,18 +1120,18 @@ func TestResourceInternalValidate(t *testing.T) { }, } - for i, tc := range cases { - t.Run(fmt.Sprintf("#%d", i), func(t *testing.T) { + for name, tc := range cases { + t.Run(name, func(t *testing.T) { sm := schemaMap{} if tc.In != nil { sm = schemaMap(tc.In.Schema) } err := tc.In.InternalValidate(sm, tc.Writable) if err != nil && !tc.Err { - t.Fatalf("%d: expected validation to pass: %s", i, err) + t.Fatalf("%s: expected validation to pass: %s", name, err) } if err == nil && tc.Err { - t.Fatalf("%d: expected validation to fail", i) + t.Fatalf("%s: expected validation to fail", name) } }) } diff --git a/helper/schema/schema.go b/helper/schema/schema.go index 317459f65b..0c779b31de 100644 --- a/helper/schema/schema.go +++ b/helper/schema/schema.go @@ -395,6 +395,18 @@ type Schema struct { // as sensitive. Any outputs containing a sensitive value must enable the // output sensitive argument. Sensitive bool + + // WriteOnly indicates that the practitioner can choose a value for this + // attribute, but Terraform will not store this attribute in plan or state. + // WriteOnly can only be set for managed resource schemas. If WriteOnly is true, + // either Optional or Required must also be true. WriteOnly cannot be set with ForceNew. + // + // WriteOnly cannot be set to true for TypeList, TypeMap, or TypeSet. + // + // This functionality is only supported in Terraform 1.11 and later. + // Practitioners that choose a value for this attribute with older + // versions of Terraform will receive an error. + WriteOnly bool } // SchemaConfigMode is used to influence how a schema item is mapped into a @@ -838,6 +850,18 @@ func (m schemaMap) internalValidate(topSchemaMap schemaMap, attrsOnly bool) erro return fmt.Errorf("%s: One of optional, required, or computed must be set", k) } + if v.WriteOnly && v.Required && v.Optional { + return fmt.Errorf("%s: WriteOnly must be set with either Required or Optional", k) + } + + if v.WriteOnly && v.Computed { + return fmt.Errorf("%s: WriteOnly cannot be set with Computed", k) + } + + if v.WriteOnly && v.ForceNew { + return fmt.Errorf("%s: WriteOnly cannot be set with ForceNew", k) + } + computedOnly := v.Computed && !v.Optional switch v.ConfigMode { @@ -874,6 +898,14 @@ func (m schemaMap) internalValidate(topSchemaMap schemaMap, attrsOnly bool) erro return fmt.Errorf("%s: Default cannot be set with Required", k) } + if v.WriteOnly && v.Default != nil { + return fmt.Errorf("%s: Default cannot be set with WriteOnly", k) + } + + if v.WriteOnly && v.DefaultFunc != nil { + return fmt.Errorf("%s: DefaultFunc cannot be set with WriteOnly", k) + } + if len(v.ComputedWhen) > 0 && !v.Computed { return fmt.Errorf("%s: ComputedWhen can only be set with Computed", k) } @@ -923,6 +955,10 @@ func (m schemaMap) internalValidate(topSchemaMap schemaMap, attrsOnly bool) erro } if v.Type == TypeList || v.Type == TypeSet { + if v.WriteOnly { + return fmt.Errorf("%s: WriteOnly is not valid for lists or sets", k) + } + if v.Elem == nil { return fmt.Errorf("%s: Elem must be set for lists", k) } @@ -939,6 +975,10 @@ func (m schemaMap) internalValidate(topSchemaMap schemaMap, attrsOnly bool) erro case *Resource: attrsOnly := attrsOnly || v.ConfigMode == SchemaConfigModeAttr + if v.Computed && schemaMap(t.SchemaMap()).hasWriteOnly() { + return fmt.Errorf("%s: Block types with Computed set to true cannot contain WriteOnly attributes", k) + } + if err := schemaMap(t.SchemaMap()).internalValidate(topSchemaMap, attrsOnly); err != nil { return err } @@ -956,6 +996,10 @@ func (m schemaMap) internalValidate(topSchemaMap schemaMap, attrsOnly bool) erro } if v.Type == TypeMap && v.Elem != nil { + if v.WriteOnly { + return fmt.Errorf("%s: WriteOnly is not valid for maps", k) + } + switch v.Elem.(type) { case *Resource: return fmt.Errorf("%s: TypeMap with Elem *Resource not supported,"+ @@ -2353,6 +2397,36 @@ func (m schemaMap) validateType( return diags } +// hasWriteOnly returns true if the schemaMap contains any WriteOnly attributes. +func (m schemaMap) hasWriteOnly() bool { + for _, v := range m { + if v.WriteOnly { + return true + } + + if v.Elem != nil { + switch t := v.Elem.(type) { + case *Resource: + return schemaMap(t.SchemaMap()).hasWriteOnly() + case *Schema: + if t.WriteOnly { + return true + } + + // Test the edge case where elements in a collection are set to writeOnly. + // Technically, this is an invalid schema as collections cannot have write-only + // attributes. However, this method is not concerned with the validity of the schema. + isNestedWriteOnly := schemaMap(map[string]*Schema{"nested": t}).hasWriteOnly() + if isNestedWriteOnly { + return true + } + } + } + } + + return false +} + // Zero returns the zero value for a type. func (t ValueType) Zero() interface{} { switch t { diff --git a/helper/schema/schema_test.go b/helper/schema/schema_test.go index 1c9b1f2198..d0fa1a9fdf 100644 --- a/helper/schema/schema_test.go +++ b/helper/schema/schema_test.go @@ -17,6 +17,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/hashicorp/go-cty/cty" + "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/internal/configs/hcl2shim" "github.com/hashicorp/terraform-plugin-sdk/v2/internal/diagutils" @@ -3125,7 +3126,7 @@ func TestSchemaMap_InternalValidate(t *testing.T) { In map[string]*Schema Err bool }{ - "nothing": { + "nothing returns no error": { nil, false, }, @@ -5051,6 +5052,368 @@ func TestSchemaMap_InternalValidate(t *testing.T) { }, true, }, + + "Attribute with WriteOnly and Required set returns no errors": { + map[string]*Schema{ + "foo": { + Type: TypeString, + Required: true, + WriteOnly: true, + }, + }, + false, + }, + + "Attribute with WriteOnly and Optional set returns no errors": { + map[string]*Schema{ + "foo": { + Type: TypeString, + Optional: true, + WriteOnly: true, + }, + }, + false, + }, + + "Attribute with WriteOnly and Computed set returns error": { + map[string]*Schema{ + "foo": { + Type: TypeString, + Computed: true, + WriteOnly: true, + }, + }, + true, + }, + + "Attribute with WriteOnly and ForceNew set returns error": { + map[string]*Schema{ + "foo": { + Type: TypeString, + ForceNew: true, + Optional: true, + WriteOnly: true, + }, + }, + true, + }, + + "Attribute with WriteOnly, Optional, and Computed set returns error": { + map[string]*Schema{ + "foo": { + Type: TypeString, + Optional: true, + Computed: true, + WriteOnly: true, + }, + }, + true, + }, + + "Attribute with WriteOnly, Optional, Required, and Computed set returns error": { + map[string]*Schema{ + "foo": { + Type: TypeString, + Optional: true, + Required: true, + Computed: true, + WriteOnly: true, + }, + }, + true, + }, + + "Attribute with WriteOnly, Optional, and Required set returns error": { + map[string]*Schema{ + "foo": { + Type: TypeString, + Optional: true, + Required: true, + WriteOnly: true, + }, + }, + true, + }, + + "Attribute with WriteOnly, Optional, and Default set returns error": { + map[string]*Schema{ + "foo": { + Type: TypeString, + Optional: true, + Default: true, + WriteOnly: true, + }, + }, + true, + }, + + "Attribute with WriteOnly, Optional, and DefaultFunc set returns error": { + map[string]*Schema{ + "foo": { + Type: TypeString, + Optional: true, + DefaultFunc: func() (interface{}, error) { + return "foo", nil + }, + WriteOnly: true, + }, + }, + true, + }, + + "Attribute with WriteOnly, Required, and DefaultFunc set returns error": { + map[string]*Schema{ + "foo": { + Type: TypeString, + Required: true, + DefaultFunc: func() (interface{}, error) { + return "foo", nil + }, + WriteOnly: true, + }, + }, + true, + }, + + "Attribute with only WriteOnly set returns error": { + map[string]*Schema{ + "foo": { + Type: TypeString, + WriteOnly: true, + }, + }, + true, + }, + + "List attribute with WriteOnly set returns error": { + map[string]*Schema{ + "list_attr": { + Type: TypeList, + Required: true, + WriteOnly: true, + Elem: &Schema{Type: TypeString}, + }, + }, + true, + }, + "Map attribute with WriteOnly set returns error": { + map[string]*Schema{ + "map_attr": { + Type: TypeMap, + Required: true, + WriteOnly: true, + Elem: &Schema{Type: TypeString}, + }, + }, + true, + }, + "Set attribute with WriteOnly set returns error": { + map[string]*Schema{ + "set_attr": { + Type: TypeSet, + Required: true, + WriteOnly: true, + Elem: &Schema{Type: TypeString}, + }, + }, + true, + }, + + "List configuration block with WriteOnly set returns error": { + map[string]*Schema{ + "config_block_attr": { + Type: TypeList, + Optional: true, + WriteOnly: true, + Elem: &Resource{ + Schema: map[string]*Schema{ + "nested_attr": { + Type: TypeString, + Optional: true, + }, + }, + }, + }, + }, + true, + }, + "List configuration block nested attribute with WriteOnly set returns no errors": { + map[string]*Schema{ + "config_block_attr": { + Type: TypeList, + Optional: true, + Elem: &Resource{ + Schema: map[string]*Schema{ + "nested_attr": { + Type: TypeString, + Optional: true, + WriteOnly: true, + }, + }, + }, + }, + }, + false, + }, + + "Map configuration attribute with WriteOnly set returns error": { + map[string]*Schema{ + "config_block_attr": { + Type: TypeMap, + Optional: true, + WriteOnly: true, + Elem: &Schema{ + Type: TypeString, + Optional: true, + }, + }, + }, + true, + }, + "Map configuration attribute nested attribute with WriteOnly set returns no errors": { + map[string]*Schema{ + "config_block_attr": { + Type: TypeMap, + Optional: true, + Elem: &Schema{ + Type: TypeString, + Optional: true, + WriteOnly: true, + }, + }, + }, + false, + }, + + "Set configuration block with WriteOnly set returns error": { + map[string]*Schema{ + "config_block_attr": { + Type: TypeSet, + Optional: true, + WriteOnly: true, + Elem: &Resource{ + Schema: map[string]*Schema{ + "nested_attr": { + Type: TypeString, + Optional: true, + }, + }, + }, + }, + }, + true, + }, + "Set configuration block nested attribute with WriteOnly set returns no errors": { + map[string]*Schema{ + "config_block_attr": { + Type: TypeSet, + Optional: true, + Elem: &Resource{ + Schema: map[string]*Schema{ + "nested_attr": { + Type: TypeString, + Optional: true, + WriteOnly: true, + }, + }, + }, + }, + }, + false, + }, + "List configuration block with ConfigModeAttr set, sub block nested attribute with WriteOnly set returns no errors": { + map[string]*Schema{ + "block": { + Type: TypeList, + ConfigMode: SchemaConfigModeAttr, + Optional: true, + Elem: &Resource{ + Schema: map[string]*Schema{ + "sub": { + Type: TypeList, + ConfigMode: SchemaConfigModeAttr, + Optional: true, + Elem: &Resource{ + Schema: map[string]*Schema{ + "nested_attr": { + Type: TypeString, + Optional: true, + WriteOnly: true, + }, + }, + }, + }, + }, + }, + }, + }, + false, + }, + + "Set configuration block with ConfigModeAttr set, sub block nested attribute with WriteOnly set returns no errors": { + map[string]*Schema{ + "block": { + Type: TypeSet, + ConfigMode: SchemaConfigModeAttr, + Optional: true, + Elem: &Resource{ + Schema: map[string]*Schema{ + "sub": { + Type: TypeSet, + ConfigMode: SchemaConfigModeAttr, + Optional: true, + Elem: &Resource{ + Schema: map[string]*Schema{ + "nested_attr": { + Type: TypeString, + Optional: true, + WriteOnly: true, + }, + }, + }, + }, + }, + }, + }, + }, + false, + }, + "List computed block nested attribute with WriteOnly set returns error": { + map[string]*Schema{ + "config_block_attr": { + Type: TypeList, + Computed: true, + Elem: &Resource{ + Schema: map[string]*Schema{ + "nested_attr": { + Type: TypeString, + Optional: true, + WriteOnly: true, + }, + }, + }, + }, + }, + true, + }, + "Set computed block nested attribute with WriteOnly set returns error": { + map[string]*Schema{ + "config_block_attr": { + Type: TypeSet, + Computed: true, + Elem: &Resource{ + Schema: map[string]*Schema{ + "nested_attr": { + Type: TypeString, + Required: true, + WriteOnly: true, + }, + }, + }, + }, + }, + true, + }, } for tn, tc := range cases { @@ -8822,3 +9185,275 @@ func TestValidateRequiredWithAttributes(t *testing.T) { }) } } + +func TestHasWriteOnly(t *testing.T) { + cases := map[string]struct { + Schema map[string]*Schema + expectWriteOnly bool + }{ + "Empty returns false": { + Schema: map[string]*Schema{}, + expectWriteOnly: false, + }, + "Top-level WriteOnly set returns true": { + Schema: map[string]*Schema{ + "top-level": { + Type: TypeSet, + Optional: true, + WriteOnly: true, + MinItems: 2, + Elem: &Schema{Type: TypeString}, + }, + }, + expectWriteOnly: true, + }, + "Top-level WriteOnly not set returns false": { + Schema: map[string]*Schema{ + "top-level": { + Type: TypeSet, + Optional: true, + MinItems: 2, + Elem: &Schema{Type: TypeString}, + }, + }, + expectWriteOnly: false, + }, + "Multiple top-level WriteOnly set returns true": { + Schema: map[string]*Schema{ + "top-level1": { + Type: TypeSet, + Optional: true, + MinItems: 2, + Elem: &Schema{Type: TypeString}, + }, + "top-level2": { + Type: TypeString, + Optional: true, + WriteOnly: true, + }, + "top-level3": { + Type: TypeInt, + Optional: true, + WriteOnly: true, + MinItems: 2, + Elem: &Schema{Type: TypeString}, + }, + }, + expectWriteOnly: true, + }, + "Elem set with Resource: no WriteOnly returns true": { + Schema: map[string]*Schema{ + "config_block_attr": { + Type: TypeSet, + Computed: true, + Elem: &Resource{ + Schema: map[string]*Schema{ + "nested_attr": { + Type: TypeString, + Computed: true, + }, + }, + }, + }, + }, + expectWriteOnly: false, + }, + "Elem set with Resource: WriteOnly returns true": { + Schema: map[string]*Schema{ + "config_block_attr": { + Type: TypeSet, + Computed: true, + Elem: &Resource{ + Schema: map[string]*Schema{ + "nested_attr": { + Type: TypeString, + Computed: true, + WriteOnly: true, + }, + }, + }, + }, + }, + expectWriteOnly: true, + }, + "Double nested Elem set with Resource: no WriteOnly returns false": { + Schema: map[string]*Schema{ + "config_block_attr": { + Type: TypeSet, + Computed: true, + Elem: &Resource{ + Schema: map[string]*Schema{ + "nested_attr": { + Type: TypeString, + Computed: true, + Elem: &Resource{ + Schema: map[string]*Schema{ + "nested_nested_attr": { + Type: TypeString, + Computed: true, + }, + }, + }, + }, + }, + }, + }, + }, + expectWriteOnly: false, + }, + "Double nested Elem set with Resource: WriteOnly returns true": { + Schema: map[string]*Schema{ + "config_block_attr": { + Type: TypeSet, + Computed: true, + Elem: &Resource{ + Schema: map[string]*Schema{ + "nested_attr": { + Type: TypeString, + Computed: true, + Elem: &Resource{ + Schema: map[string]*Schema{ + "nested_nested_attr": { + Type: TypeString, + Computed: true, + WriteOnly: true, + }, + }, + }, + }, + }, + }, + }, + }, + expectWriteOnly: true, + }, + "Elem set with Schema: no WriteOnly returns false": { + Schema: map[string]*Schema{ + "config_block_attr": { + Type: TypeSet, + Computed: true, + Elem: &Schema{ + Type: TypeString, + Computed: true, + }, + }, + }, + expectWriteOnly: false, + }, + "Elem set with Schema: WriteOnly returns true": { + Schema: map[string]*Schema{ + "config_block_attr": { + Type: TypeSet, + Computed: true, + Elem: &Schema{ + Type: TypeString, + Computed: true, + WriteOnly: true, + }, + }, + }, + expectWriteOnly: true, + }, + "Double nested Elem set with Schema: no WriteOnly returns false": { + Schema: map[string]*Schema{ + "config_block_attr": { + Type: TypeSet, + Computed: true, + Elem: &Schema{ + Type: TypeString, + Elem: &Schema{ + Computed: true, + }, + }, + }, + }, + expectWriteOnly: false, + }, + "Double nested Elem set with Schema: WriteOnly returns true": { + Schema: map[string]*Schema{ + "config_block_attr": { + Type: TypeSet, + Computed: true, + Elem: &Schema{ + Type: TypeString, + Elem: &Schema{ + Computed: true, + WriteOnly: true, + }, + }, + }, + }, + expectWriteOnly: true, + }, + "Multiple nested elements: no WriteOnly returns false": { + Schema: map[string]*Schema{ + "config_block_attr": { + Type: TypeSet, + Computed: true, + Elem: &Schema{ + Type: TypeString, + Elem: &Schema{ + Computed: true, + Elem: &Resource{ + Schema: map[string]*Schema{ + "nested_nested_nested_attr": { + Type: TypeString, + Computed: true, + }, + "nested_nested_nested_attr2": { + Type: TypeString, + Computed: true, + Elem: &Schema{ + Computed: true, + }, + }, + }, + }, + }, + }, + }, + }, + expectWriteOnly: false, + }, + "Multiple nested elements: WriteOnly returns true": { + Schema: map[string]*Schema{ + "config_block_attr": { + Type: TypeSet, + Computed: true, + Elem: &Schema{ + Type: TypeString, + Elem: &Schema{ + Computed: true, + Elem: &Resource{ + Schema: map[string]*Schema{ + "nested_nested_nested_attr": { + Type: TypeString, + Computed: true, + }, + "nested_nested_nested_attr2": { + Type: TypeString, + Computed: true, + Elem: &Schema{ + Computed: true, + WriteOnly: true, + }, + }, + }, + }, + }, + }, + }, + }, + expectWriteOnly: true, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + actualWriteOnly := schemaMap(tc.Schema).hasWriteOnly() + if tc.expectWriteOnly != actualWriteOnly { + t.Fatalf("Expected: %t, got: %t", tc.expectWriteOnly, actualWriteOnly) + } + }) + } +} diff --git a/helper/schema/write_only.go b/helper/schema/write_only.go new file mode 100644 index 0000000000..287c8bd8fd --- /dev/null +++ b/helper/schema/write_only.go @@ -0,0 +1,214 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package schema + +import ( + "fmt" + "sort" + + "github.com/hashicorp/go-cty/cty" + + "github.com/hashicorp/terraform-plugin-sdk/v2/diag" + "github.com/hashicorp/terraform-plugin-sdk/v2/internal/configs/configschema" +) + +// setWriteOnlyNullValues takes a cty.Value, and compares it to the schema setting any non-null +// values that are writeOnly to null. +func setWriteOnlyNullValues(val cty.Value, schema *configschema.Block) cty.Value { + if !val.IsKnown() || val.IsNull() { + return val + } + + valMap := val.AsValueMap() + newVals := make(map[string]cty.Value) + + for name, attr := range schema.Attributes { + v := valMap[name] + + if attr.WriteOnly && !v.IsNull() { + newVals[name] = cty.NullVal(attr.Type) + continue + } + + newVals[name] = v + } + + for name, blockS := range schema.BlockTypes { + blockVal := valMap[name] + if blockVal.IsNull() || !blockVal.IsKnown() { + newVals[name] = blockVal + continue + } + + blockValType := blockVal.Type() + blockElementType := blockS.Block.ImpliedType() + + // This switches on the value type here, so we can correctly switch + // between Tuples/Lists and Maps/Objects. + switch { + case blockS.Nesting == configschema.NestingSingle || blockS.Nesting == configschema.NestingGroup: + // NestingSingle is the only exception here, where we treat the + // block directly as an object + newVals[name] = setWriteOnlyNullValues(blockVal, &blockS.Block) + + case blockValType.IsSetType(), blockValType.IsListType(), blockValType.IsTupleType(): + listVals := blockVal.AsValueSlice() + newListVals := make([]cty.Value, 0, len(listVals)) + + for _, v := range listVals { + newListVals = append(newListVals, setWriteOnlyNullValues(v, &blockS.Block)) + } + + switch { + case blockValType.IsSetType(): + switch len(newListVals) { + case 0: + newVals[name] = cty.SetValEmpty(blockElementType) + default: + newVals[name] = cty.SetVal(newListVals) + } + case blockValType.IsListType(): + switch len(newListVals) { + case 0: + newVals[name] = cty.ListValEmpty(blockElementType) + default: + newVals[name] = cty.ListVal(newListVals) + } + case blockValType.IsTupleType(): + newVals[name] = cty.TupleVal(newListVals) + } + + case blockValType.IsMapType(), blockValType.IsObjectType(): + mapVals := blockVal.AsValueMap() + newMapVals := make(map[string]cty.Value) + + for k, v := range mapVals { + newMapVals[k] = setWriteOnlyNullValues(v, &blockS.Block) + } + + switch { + case blockValType.IsMapType(): + switch len(newMapVals) { + case 0: + newVals[name] = cty.MapValEmpty(blockElementType) + default: + newVals[name] = cty.MapVal(newMapVals) + } + case blockValType.IsObjectType(): + if len(newMapVals) == 0 { + // We need to populate empty values to make a valid object. + for attr, ty := range blockElementType.AttributeTypes() { + newMapVals[attr] = cty.NullVal(ty) + } + } + newVals[name] = cty.ObjectVal(newMapVals) + } + + default: + panic(fmt.Sprintf("failed to set null values for nested block %q:%#v", name, blockValType)) + } + } + + return cty.ObjectVal(newVals) +} + +// validateWriteOnlyNullValues validates that write-only attribute values +// are null to ensure that write-only values are not sent to unsupported +// Terraform client versions. +// +// it takes a cty.Value, and compares it to the schema and throws an +// error diagnostic for each non-null writeOnly attribute value. +func validateWriteOnlyNullValues(val cty.Value, schema *configschema.Block, path cty.Path) diag.Diagnostics { + if !val.IsKnown() || val.IsNull() { + return diag.Diagnostics{} + } + + valMap := val.AsValueMap() + diags := make([]diag.Diagnostic, 0) + + var attrNames []string + for k := range schema.Attributes { + attrNames = append(attrNames, k) + } + + // Sort the attribute names to produce diags in a consistent order. + sort.Strings(attrNames) + + for _, name := range attrNames { + attr := schema.Attributes[name] + v := valMap[name] + + if attr.WriteOnly && !v.IsNull() { + diags = append(diags, diag.Diagnostic{ + Severity: diag.Error, + Summary: "Write-only Attribute Not Allowed", + Detail: fmt.Sprintf("The resource contains a non-null value for write-only attribute %q ", name) + + "Write-only attributes are only supported in Terraform 1.11 and later.", + AttributePath: append(path, cty.GetAttrStep{Name: name}), + }) + } + } + + var blockNames []string + for k := range schema.BlockTypes { + blockNames = append(blockNames, k) + } + + // Sort the block names to produce diags in a consistent order. + sort.Strings(blockNames) + + for _, name := range blockNames { + blockS := schema.BlockTypes[name] + blockVal := valMap[name] + if blockVal.IsNull() || !blockVal.IsKnown() { + continue + } + + blockValType := blockVal.Type() + blockPath := append(path, cty.GetAttrStep{Name: name}) + + // This switches on the value type here, so we can correctly switch + // between Tuples/Lists and Maps/Objects. + switch { + case blockS.Nesting == configschema.NestingSingle || blockS.Nesting == configschema.NestingGroup: + // NestingSingle is the only exception here, where we treat the + // block directly as an object + diags = append(diags, validateWriteOnlyNullValues(blockVal, &blockS.Block, blockPath)...) + case blockValType.IsSetType(): + setVals := blockVal.AsValueSlice() + + for _, v := range setVals { + setBlockPath := append(blockPath, cty.IndexStep{ + Key: v, + }) + diags = append(diags, validateWriteOnlyNullValues(v, &blockS.Block, setBlockPath)...) + } + + case blockValType.IsListType(), blockValType.IsTupleType(): + listVals := blockVal.AsValueSlice() + + for i, v := range listVals { + listBlockPath := append(blockPath, cty.IndexStep{ + Key: cty.NumberIntVal(int64(i)), + }) + diags = append(diags, validateWriteOnlyNullValues(v, &blockS.Block, listBlockPath)...) + } + + case blockValType.IsMapType(), blockValType.IsObjectType(): + mapVals := blockVal.AsValueMap() + + for k, v := range mapVals { + mapBlockPath := append(blockPath, cty.IndexStep{ + Key: cty.StringVal(k), + }) + diags = append(diags, validateWriteOnlyNullValues(v, &blockS.Block, mapBlockPath)...) + } + + default: + panic(fmt.Sprintf("failed to validate WriteOnly values for nested block %q:%#v", name, blockValType)) + } + } + + return diags +} diff --git a/helper/schema/write_only_test.go b/helper/schema/write_only_test.go new file mode 100644 index 0000000000..1f956d4869 --- /dev/null +++ b/helper/schema/write_only_test.go @@ -0,0 +1,928 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package schema + +import ( + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/hashicorp/go-cty/cty" + + "github.com/hashicorp/terraform-plugin-sdk/v2/diag" + "github.com/hashicorp/terraform-plugin-sdk/v2/internal/configs/configschema" +) + +func Test_setWriteOnlyNullValues(t *testing.T) { + for n, tc := range map[string]struct { + Schema *configschema.Block + Val cty.Value + Expected cty.Value + }{ + "Empty returns no empty object": { + &configschema.Block{}, + cty.EmptyObjectVal, + cty.EmptyObjectVal, + }, + "Top level attributes and block: write only attributes with values": { + &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "required_attribute": { + Type: cty.String, + Required: true, + }, + "write_only_attribute": { + Type: cty.String, + Required: true, + WriteOnly: true, + }, + }, + BlockTypes: map[string]*configschema.NestedBlock{ + "nested_block": { + Nesting: configschema.NestingSingle, + Block: configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "write_only_block_attribute": { + Type: cty.String, + Required: true, + WriteOnly: true, + }, + "required_block_attribute": { + Type: cty.String, + Required: true, + }, + }, + }, + }, + }, + }, + cty.ObjectVal(map[string]cty.Value{ + "required_attribute": cty.StringVal("boop"), + "write_only_attribute": cty.StringVal("blep"), + "nested_block": cty.ObjectVal(map[string]cty.Value{ + "write_only_block_attribute": cty.StringVal("blep"), + "required_block_attribute": cty.StringVal("boop"), + }), + }), + cty.ObjectVal(map[string]cty.Value{ + "required_attribute": cty.StringVal("boop"), + "write_only_attribute": cty.NullVal(cty.String), + "nested_block": cty.ObjectVal(map[string]cty.Value{ + "write_only_block_attribute": cty.NullVal(cty.String), + "required_block_attribute": cty.StringVal("boop"), + }), + }), + }, + "Top level attributes and block: all null values": { + &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "write_only_attribute1": { + Type: cty.String, + Optional: true, + WriteOnly: true, + }, + "write_only_attribute2": { + Type: cty.String, + Optional: true, + WriteOnly: true, + }, + }, + BlockTypes: map[string]*configschema.NestedBlock{ + "nested_block": { + Nesting: configschema.NestingSingle, + Block: configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "write_only_block_attribute1": { + Type: cty.String, + Optional: true, + WriteOnly: true, + }, + "write_only_block_attribute2": { + Type: cty.String, + Optional: true, + WriteOnly: true, + }, + }, + }, + }, + }, + }, + cty.NullVal(cty.Object(map[string]cty.Type{ + "write_only_attribute1": cty.String, + "write_only_attribute2": cty.String, + "nested_block": cty.Object(map[string]cty.Type{ + "write_only_block_attribute1": cty.String, + "write_only_block_attribute2": cty.String, + }), + })), + cty.NullVal(cty.Object(map[string]cty.Type{ + "write_only_attribute1": cty.String, + "write_only_attribute2": cty.String, + "nested_block": cty.Object(map[string]cty.Type{ + "write_only_block_attribute1": cty.String, + "write_only_block_attribute2": cty.String, + }), + })), + }, + "Set nested block: write only Nested Attribute": { + &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "required_attribute": { + Type: cty.String, + Required: true, + }, + }, + BlockTypes: map[string]*configschema.NestedBlock{ + "set_block": { + Nesting: configschema.NestingSet, + Block: configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "write_only_block_attribute": { + Type: cty.String, + Required: true, + WriteOnly: true, + }, + }, + }, + }, + }, + }, + cty.ObjectVal(map[string]cty.Value{ + "required_attribute": cty.StringVal("boop"), + "set_block": cty.SetVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "write_only_block_attribute": cty.StringVal("beep"), + }), + }), + }), + cty.ObjectVal(map[string]cty.Value{ + "required_attribute": cty.StringVal("boop"), + "set_block": cty.SetVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "write_only_block_attribute": cty.NullVal(cty.String), + }), + }), + }), + }, + "Nested single block: write only nested attribute": { + &configschema.Block{ + BlockTypes: map[string]*configschema.NestedBlock{ + "nested_block": { + Nesting: configschema.NestingSingle, + Block: configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "write_only_block_attribute": { + Type: cty.String, + Required: true, + WriteOnly: true, + }, + "optional_attribute": { + Type: cty.String, + Optional: true, + Computed: true, + }, + }, + }, + }, + }, + }, + cty.ObjectVal(map[string]cty.Value{ + "nested_block": cty.ObjectVal(map[string]cty.Value{ + "write_only_block_attribute": cty.StringVal("boop"), + "optional_attribute": cty.StringVal("boop"), + }), + }), + cty.ObjectVal(map[string]cty.Value{ + "nested_block": cty.ObjectVal(map[string]cty.Value{ + "write_only_block_attribute": cty.NullVal(cty.String), + "optional_attribute": cty.StringVal("boop"), + }), + }), + }, + "Map nested block: multiple write only nested attributes": { + &configschema.Block{ + BlockTypes: map[string]*configschema.NestedBlock{ + "map_block": { + Nesting: configschema.NestingMap, + Block: configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "optional_block_attribute": { + Type: cty.String, + Optional: true, + Computed: true, + }, + "write_only_block_attribute": { + Type: cty.String, + Required: true, + WriteOnly: true, + }, + }, + }, + }, + }, + }, + cty.ObjectVal(map[string]cty.Value{ + "map_block": cty.MapVal(map[string]cty.Value{ + "a": cty.ObjectVal(map[string]cty.Value{ + "optional_block_attribute": cty.NullVal(cty.String), + "write_only_block_attribute": cty.StringVal("boop"), + }), + "b": cty.ObjectVal(map[string]cty.Value{ + "optional_block_attribute": cty.StringVal("blep"), + "write_only_block_attribute": cty.StringVal("boop2"), + }), + }), + }), + cty.ObjectVal(map[string]cty.Value{ + "map_block": cty.MapVal(map[string]cty.Value{ + "a": cty.ObjectVal(map[string]cty.Value{ + "optional_block_attribute": cty.NullVal(cty.String), + "write_only_block_attribute": cty.NullVal(cty.String), + }), + "b": cty.ObjectVal(map[string]cty.Value{ + "optional_block_attribute": cty.StringVal("blep"), + "write_only_block_attribute": cty.NullVal(cty.String), + }), + }), + }), + }, + "List nested block: multiple write only nested attributes": { + &configschema.Block{ + BlockTypes: map[string]*configschema.NestedBlock{ + "list_block": { + Nesting: configschema.NestingList, + Block: configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "write_only_block_attribute": { + Type: cty.String, + Required: true, + WriteOnly: true, + }, + "optional_block_attribute": { + Type: cty.String, + Optional: true, + Computed: true, + }, + }, + }, + }, + }, + }, + cty.ObjectVal(map[string]cty.Value{ + "list_block": cty.ListVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "write_only_block_attribute": cty.StringVal("beep"), + "optional_block_attribute": cty.StringVal("bap"), + }), + cty.ObjectVal(map[string]cty.Value{ + "write_only_block_attribute": cty.StringVal("boop"), + "optional_block_attribute": cty.StringVal("blep"), + }), + }), + }), + cty.ObjectVal(map[string]cty.Value{ + "list_block": cty.ListVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "write_only_block_attribute": cty.NullVal(cty.String), + "optional_block_attribute": cty.StringVal("bap"), + }), + cty.ObjectVal(map[string]cty.Value{ + "write_only_block_attribute": cty.NullVal(cty.String), + "optional_block_attribute": cty.StringVal("blep"), + }), + }), + }), + }, + "Set nested block: multiple write only nested attributes": { + &configschema.Block{ + BlockTypes: map[string]*configschema.NestedBlock{ + "set_block": { + Nesting: configschema.NestingSet, + Block: configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "write_only_block_attribute": { + Type: cty.String, + Optional: true, + WriteOnly: true, + }, + "optional_block_attribute": { + Type: cty.String, + Optional: true, + Computed: true, + }, + }, + }, + }, + }, + }, + cty.ObjectVal(map[string]cty.Value{ + "set_block": cty.SetVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "write_only_block_attribute": cty.StringVal("blep"), + "optional_block_attribute": cty.NullVal(cty.String), + }), + cty.ObjectVal(map[string]cty.Value{ + "write_only_block_attribute": cty.StringVal("boop"), + "optional_block_attribute": cty.StringVal("boop"), + }), + }), + }), + cty.ObjectVal(map[string]cty.Value{ + "set_block": cty.SetVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "write_only_block_attribute": cty.NullVal(cty.String), + "optional_block_attribute": cty.NullVal(cty.String), + }), + cty.ObjectVal(map[string]cty.Value{ + "write_only_block_attribute": cty.NullVal(cty.String), + "optional_block_attribute": cty.StringVal("boop"), + }), + }), + }), + }, + } { + t.Run(n, func(t *testing.T) { + got := setWriteOnlyNullValues(tc.Val, tc.Schema) + + if !got.RawEquals(tc.Expected) { + t.Errorf("\nexpected: %#v\ngot: %#v\n", tc.Expected, got) + } + }) + } +} + +func Test_validateWriteOnlyNullValues(t *testing.T) { + for n, tc := range map[string]struct { + Schema *configschema.Block + Val cty.Value + Expected diag.Diagnostics + }{ + "Empty returns no diags": { + &configschema.Block{}, + cty.EmptyObjectVal, + diag.Diagnostics{}, + }, + "All null values returns no diags": { + &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "write_only_attribute1": { + Type: cty.String, + Optional: true, + WriteOnly: true, + }, + "write_only_attribute2": { + Type: cty.String, + Optional: true, + WriteOnly: true, + }, + }, + BlockTypes: map[string]*configschema.NestedBlock{ + "single_block": { + Nesting: configschema.NestingSingle, + Block: configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "write_only_block_attribute1": { + Type: cty.String, + Optional: true, + WriteOnly: true, + }, + "write_only_block_attribute2": { + Type: cty.String, + Optional: true, + WriteOnly: true, + }, + }, + }, + }, + }, + }, + cty.NullVal(cty.Object(map[string]cty.Type{ + "write_only_attribute1": cty.String, + "write_only_attribute2": cty.String, + "single_block": cty.Object(map[string]cty.Type{ + "write_only_block_attribute1": cty.String, + "write_only_block_attribute2": cty.String, + }), + })), + diag.Diagnostics{}, + }, + "Set nested block WriteOnly attribute with value returns diag": { + &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "write_only_attribute": { + Type: cty.String, + Optional: true, + WriteOnly: true, + }, + }, + BlockTypes: map[string]*configschema.NestedBlock{ + "set_block": { + Nesting: configschema.NestingSet, + Block: configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "write_only_block_attribute": { + Type: cty.String, + Optional: true, + WriteOnly: true, + }, + }, + }, + }, + }, + }, + cty.ObjectVal(map[string]cty.Value{ + "write_only_attribute": cty.StringVal("val"), + "set_block": cty.SetVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "write_only_block_attribute": cty.StringVal("block_val"), + }), + }), + }), + diag.Diagnostics{ + { + Severity: diag.Error, + Summary: "Write-only Attribute Not Allowed", + Detail: "The resource contains a non-null value for write-only attribute \"write_only_attribute\" " + + "Write-only attributes are only supported in Terraform 1.11 and later.", + AttributePath: cty.Path{ + cty.GetAttrStep{Name: "write_only_attribute"}, + }, + }, + { + Severity: diag.Error, + Summary: "Write-only Attribute Not Allowed", + Detail: "The resource contains a non-null value for write-only attribute \"write_only_block_attribute\" " + + "Write-only attributes are only supported in Terraform 1.11 and later.", + AttributePath: cty.Path{ + cty.GetAttrStep{Name: "set_block"}, + cty.IndexStep{Key: cty.ObjectVal(map[string]cty.Value{ + "write_only_block_attribute": cty.StringVal("block_val"), + })}, + cty.GetAttrStep{Name: "write_only_block_attribute"}, + }, + }, + }, + }, + "List nested block, WriteOnly attribute with value returns diag": { + &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "write_only_attribute": { + Type: cty.String, + Optional: true, + WriteOnly: true, + }, + }, + BlockTypes: map[string]*configschema.NestedBlock{ + "list_block": { + Nesting: configschema.NestingList, + Block: configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "write_only_block_attribute": { + Type: cty.String, + Optional: true, + WriteOnly: true, + }, + "optional_block_attribute": { + Type: cty.String, + Optional: true, + Computed: true, + }, + }, + }, + }, + }, + }, + cty.ObjectVal(map[string]cty.Value{ + "write_only_attribute": cty.StringVal("val"), + "list_block": cty.ListVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "write_only_block_attribute": cty.StringVal("bap"), + }), + }), + }), + diag.Diagnostics{ + { + Severity: diag.Error, + Summary: "Write-only Attribute Not Allowed", + Detail: "The resource contains a non-null value for write-only attribute \"write_only_attribute\" " + + "Write-only attributes are only supported in Terraform 1.11 and later.", + AttributePath: cty.Path{ + cty.GetAttrStep{Name: "write_only_attribute"}, + }, + }, + { + Severity: diag.Error, + Summary: "Write-only Attribute Not Allowed", + Detail: "The resource contains a non-null value for write-only attribute \"write_only_block_attribute\" " + + "Write-only attributes are only supported in Terraform 1.11 and later.", + AttributePath: cty.Path{ + cty.GetAttrStep{Name: "list_block"}, + cty.IndexStep{Key: cty.NumberIntVal(0)}, + cty.GetAttrStep{Name: "write_only_block_attribute"}, + }, + }, + }, + }, + "Map nested block, WriteOnly attribute with value returns diag": { + &configschema.Block{ + BlockTypes: map[string]*configschema.NestedBlock{ + "map_block": { + Nesting: configschema.NestingMap, + Block: configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "optional_attribute": { + Type: cty.String, + Optional: true, + Computed: true, + }, + "write_only_block_attribute": { + Type: cty.String, + Optional: true, + WriteOnly: true, + }, + }, + }, + }, + }, + }, + cty.ObjectVal(map[string]cty.Value{ + "map_block": cty.MapVal(map[string]cty.Value{ + "a": cty.ObjectVal(map[string]cty.Value{ + "optional_attribute": cty.NullVal(cty.String), + "write_only_block_attribute": cty.StringVal("boop"), + }), + "b": cty.ObjectVal(map[string]cty.Value{ + "optional_attribute": cty.StringVal("blep"), + "write_only_block_attribute": cty.NullVal(cty.String), + }), + }), + }), + diag.Diagnostics{ + { + Severity: diag.Error, + Summary: "Write-only Attribute Not Allowed", + Detail: "The resource contains a non-null value for write-only attribute \"write_only_block_attribute\" " + + "Write-only attributes are only supported in Terraform 1.11 and later.", + AttributePath: cty.Path{ + cty.GetAttrStep{Name: "map_block"}, + cty.IndexStep{Key: cty.StringVal("a")}, + cty.GetAttrStep{Name: "write_only_block_attribute"}, + }, + }, + }, + }, + "Nested single block, WriteOnly attribute with value returns diag": { + &configschema.Block{ + BlockTypes: map[string]*configschema.NestedBlock{ + "nested_block": { + Nesting: configschema.NestingSingle, + Block: configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "write_only_block_attribute": { + Type: cty.String, + Optional: true, + WriteOnly: true, + }, + "optional_block_attribute": { + Type: cty.String, + Optional: true, + Computed: true, + }, + }, + }, + }, + }, + }, + cty.ObjectVal(map[string]cty.Value{ + "nested_block": cty.ObjectVal(map[string]cty.Value{ + "write_only_block_attribute": cty.StringVal("beep"), + "optional_block_attribute1": cty.StringVal("boop"), + }), + }), + diag.Diagnostics{ + { + Severity: diag.Error, + Summary: "Write-only Attribute Not Allowed", + Detail: "The resource contains a non-null value for write-only attribute \"write_only_block_attribute\" " + + "Write-only attributes are only supported in Terraform 1.11 and later.", + AttributePath: cty.Path{ + cty.GetAttrStep{Name: "nested_block"}, + cty.GetAttrStep{Name: "write_only_block_attribute"}, + }, + }, + }, + }, + "Set nested block, WriteOnly attribute with multiple values returns multiple diags": { + &configschema.Block{ + BlockTypes: map[string]*configschema.NestedBlock{ + "set_block": { + Nesting: configschema.NestingSet, + Block: configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "write_only_block_attribute1": { + Type: cty.String, + Optional: true, + WriteOnly: true, + }, + "write_only_block_attribute2": { + Type: cty.String, + Optional: true, + WriteOnly: true, + }, + }, + }, + }, + }, + }, + cty.ObjectVal(map[string]cty.Value{ + "set_block": cty.SetVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "write_only_block_attribute1": cty.StringVal("blep"), + "write_only_block_attribute2": cty.NullVal(cty.String), + }), + cty.ObjectVal(map[string]cty.Value{ + "write_only_block_attribute1": cty.StringVal("boop"), + "write_only_block_attribute2": cty.NullVal(cty.String), + }), + }), + }), + diag.Diagnostics{ + { + Severity: diag.Error, + Summary: "Write-only Attribute Not Allowed", + Detail: "The resource contains a non-null value for write-only attribute \"write_only_block_attribute1\" " + + "Write-only attributes are only supported in Terraform 1.11 and later.", + AttributePath: cty.Path{ + cty.GetAttrStep{Name: "set_block"}, + cty.IndexStep{Key: cty.ObjectVal(map[string]cty.Value{ + "write_only_block_attribute1": cty.StringVal("blep"), + "write_only_block_attribute2": cty.NullVal(cty.String), + })}, + cty.GetAttrStep{Name: "write_only_block_attribute1"}, + }, + }, + { + Severity: diag.Error, + Summary: "Write-only Attribute Not Allowed", + Detail: "The resource contains a non-null value for write-only attribute \"write_only_block_attribute1\" " + + "Write-only attributes are only supported in Terraform 1.11 and later.", + AttributePath: cty.Path{ + cty.GetAttrStep{Name: "set_block"}, + cty.IndexStep{Key: cty.ObjectVal(map[string]cty.Value{ + "write_only_block_attribute1": cty.StringVal("boop"), + "write_only_block_attribute2": cty.NullVal(cty.String), + })}, + cty.GetAttrStep{Name: "write_only_block_attribute1"}, + }, + }, + }, + }, + "List nested block, WriteOnly attribute with multiple values returns multiple diags": { + &configschema.Block{ + BlockTypes: map[string]*configschema.NestedBlock{ + "list_block": { + Nesting: configschema.NestingList, + Block: configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "write_only_block_attribute": { + Type: cty.String, + Optional: true, + WriteOnly: true, + }, + "optional_block_attribute": { + Type: cty.String, + Optional: true, + Computed: true, + }, + }, + }, + }, + }, + }, + cty.ObjectVal(map[string]cty.Value{ + "list_block": cty.ListVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "write_only_block_attribute": cty.StringVal("bap"), + }), + cty.ObjectVal(map[string]cty.Value{ + "write_only_block_attribute": cty.StringVal("blep"), + }), + }), + }), + diag.Diagnostics{ + { + Severity: diag.Error, + Summary: "Write-only Attribute Not Allowed", + Detail: "The resource contains a non-null value for write-only attribute \"write_only_block_attribute\" " + + "Write-only attributes are only supported in Terraform 1.11 and later.", + AttributePath: cty.Path{ + cty.GetAttrStep{Name: "list_block"}, + cty.IndexStep{Key: cty.NumberIntVal(0)}, + cty.GetAttrStep{Name: "write_only_block_attribute"}, + }, + }, + { + Severity: diag.Error, + Summary: "Write-only Attribute Not Allowed", + Detail: "The resource contains a non-null value for write-only attribute \"write_only_block_attribute\" " + + "Write-only attributes are only supported in Terraform 1.11 and later.", + AttributePath: cty.Path{ + cty.GetAttrStep{Name: "list_block"}, + cty.IndexStep{Key: cty.NumberIntVal(1)}, + cty.GetAttrStep{Name: "write_only_block_attribute"}, + }, + }, + }, + }, + "Map nested block, WriteOnly attribute with multiple values returns multiple diags": { + &configschema.Block{ + BlockTypes: map[string]*configschema.NestedBlock{ + "map_block": { + Nesting: configschema.NestingMap, + Block: configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "optional_block_attribute": { + Type: cty.String, + Optional: true, + Computed: true, + }, + "write_only_block_attribute": { + Type: cty.String, + Optional: true, + WriteOnly: true, + }, + }, + }, + }, + }, + }, + cty.ObjectVal(map[string]cty.Value{ + "map_block": cty.MapVal(map[string]cty.Value{ + "a": cty.ObjectVal(map[string]cty.Value{ + "optional_block_attribute": cty.NullVal(cty.String), + "write_only_block_attribute": cty.StringVal("boop"), + }), + "b": cty.ObjectVal(map[string]cty.Value{ + "optional_block_attribute": cty.StringVal("blep"), + "write_only_block_attribute": cty.StringVal("boop2"), + }), + }), + }), + diag.Diagnostics{ + { + Severity: diag.Error, + Summary: "Write-only Attribute Not Allowed", + Detail: "The resource contains a non-null value for write-only attribute \"write_only_block_attribute\" " + + "Write-only attributes are only supported in Terraform 1.11 and later.", + AttributePath: cty.Path{ + cty.GetAttrStep{Name: "map_block"}, + cty.IndexStep{Key: cty.StringVal("a")}, + cty.GetAttrStep{Name: "write_only_block_attribute"}, + }, + }, + { + Severity: diag.Error, + Summary: "Write-only Attribute Not Allowed", + Detail: "The resource contains a non-null value for write-only attribute \"write_only_block_attribute\" " + + "Write-only attributes are only supported in Terraform 1.11 and later.", + AttributePath: cty.Path{ + cty.GetAttrStep{Name: "map_block"}, + cty.IndexStep{Key: cty.StringVal("b")}, + cty.GetAttrStep{Name: "write_only_block_attribute"}, + }, + }, + }, + }, + "Nested single block, WriteOnly attribute with multiple values returns multiple diags": { + &configschema.Block{ + BlockTypes: map[string]*configschema.NestedBlock{ + "nested_block1": { + Nesting: configschema.NestingSingle, + Block: configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "write_only_block_attribute": { + Type: cty.String, + Optional: true, + WriteOnly: true, + }, + "optional_block_attribute": { + Type: cty.String, + Optional: true, + Computed: true, + }, + }, + }, + }, + "nested_block2": { + Nesting: configschema.NestingSingle, + Block: configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "write_only_block_attribute": { + Type: cty.String, + Optional: true, + WriteOnly: true, + }, + "optional_block_attribute": { + Type: cty.String, + Optional: true, + Computed: true, + }, + }, + }, + }, + }, + }, + cty.ObjectVal(map[string]cty.Value{ + "nested_block1": cty.ObjectVal(map[string]cty.Value{ + "write_only_block_attribute": cty.StringVal("beep"), + "optional_block_attribute1": cty.StringVal("boop"), + }), + "nested_block2": cty.ObjectVal(map[string]cty.Value{ + "write_only_block_attribute": cty.StringVal("beep"), + "optional_block_attribute1": cty.StringVal("boop"), + }), + }), + diag.Diagnostics{ + { + Severity: diag.Error, + Summary: "Write-only Attribute Not Allowed", + Detail: "The resource contains a non-null value for write-only attribute \"write_only_block_attribute\" " + + "Write-only attributes are only supported in Terraform 1.11 and later.", + AttributePath: cty.Path{ + cty.GetAttrStep{Name: "nested_block1"}, + cty.GetAttrStep{Name: "write_only_block_attribute"}, + }, + }, + { + Severity: diag.Error, + Summary: "Write-only Attribute Not Allowed", + Detail: "The resource contains a non-null value for write-only attribute \"write_only_block_attribute\" " + + "Write-only attributes are only supported in Terraform 1.11 and later.", + + AttributePath: cty.Path{ + cty.GetAttrStep{Name: "nested_block2"}, + cty.GetAttrStep{Name: "write_only_block_attribute"}, + }, + }, + }, + }, + "List nested block, WriteOnly attribute with dynamic value returns diag": { + &configschema.Block{ + BlockTypes: map[string]*configschema.NestedBlock{ + "list_block": { + Nesting: configschema.NestingList, + Block: configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "optional_block_attribute": { + Type: cty.String, + Optional: true, + Computed: true, + }, + "write_only_block_attribute": { + Type: cty.DynamicPseudoType, + Optional: true, + WriteOnly: true, + }, + }, + }, + }, + }, + }, + cty.ObjectVal(map[string]cty.Value{ + "list_block": cty.TupleVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "optional_block_attribute": cty.NullVal(cty.String), + "write_only_block_attribute": cty.NumberIntVal(8), + }), + }), + }), + diag.Diagnostics{ + { + Severity: diag.Error, + Summary: "Write-only Attribute Not Allowed", + Detail: "The resource contains a non-null value for write-only attribute \"write_only_block_attribute\" " + + "Write-only attributes are only supported in Terraform 1.11 and later.", + AttributePath: cty.Path{ + cty.GetAttrStep{Name: "list_block"}, + cty.IndexStep{Key: cty.NumberIntVal(0)}, + cty.GetAttrStep{Name: "write_only_block_attribute"}, + }, + }, + }, + }, + } { + t.Run(n, func(t *testing.T) { + got := validateWriteOnlyNullValues(tc.Val, tc.Schema, cty.Path{}) + + if diff := cmp.Diff(got, tc.Expected, + cmp.AllowUnexported(cty.GetAttrStep{}, cty.IndexStep{}), + cmp.Comparer(indexStepComparer)); diff != "" { + t.Errorf("unexpected difference: %s", diff) + } + }) + } +} + +func indexStepComparer(step cty.IndexStep, other cty.IndexStep) bool { + return true +} diff --git a/helper/validation/path.go b/helper/validation/path.go new file mode 100644 index 0000000000..b8707330d0 --- /dev/null +++ b/helper/validation/path.go @@ -0,0 +1,55 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package validation + +import ( + "github.com/hashicorp/go-cty/cty" +) + +// PathMatches compares two Paths for equality. For cty.IndexStep, +// unknown key values are treated as an Any qualifier and will +// match any index step of the same type. +func PathMatches(p cty.Path, other cty.Path) bool { + if len(p) != len(other) { + return false + } + + for i := range p { + pv := p[i] + switch pv := pv.(type) { + case cty.GetAttrStep: + ov, ok := other[i].(cty.GetAttrStep) + if !ok || pv != ov { + return false + } + case cty.IndexStep: + ov, ok := other[i].(cty.IndexStep) + if !ok { + return false + } + + // Sets need special handling since their Type is the entire object + // with attributes. + if pv.Key.Type().IsObjectType() && ov.Key.Type().IsObjectType() { + if !pv.Key.IsKnown() || !ov.Key.IsKnown() { + break + } + } + if !pv.Key.Type().Equals(ov.Key.Type()) { + return false + } + + if pv.Key.IsKnown() && ov.Key.IsKnown() { + if !pv.Key.RawEquals(ov.Key) { + return false + } + } + default: + // Any invalid steps default to evaluating false. + return false + } + } + + return true +} diff --git a/helper/validation/path_test.go b/helper/validation/path_test.go new file mode 100644 index 0000000000..b85b837ed6 --- /dev/null +++ b/helper/validation/path_test.go @@ -0,0 +1,116 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package validation + +import ( + "testing" + + "github.com/hashicorp/go-cty/cty" +) + +func TestPathMatches(t *testing.T) { + tests := map[string]struct { + p cty.Path + other cty.Path + want bool + }{ + "null paths returns true": { + p: nil, + other: nil, + want: true, + }, + "empty paths returns true": { + p: cty.Path{}, + other: cty.Path{}, + want: true, + }, + "exact same path returns true": { + p: cty.GetAttrPath("attribute").Index(cty.NumberIntVal(1)).GetAttr("nestedAttribute"), + other: cty.GetAttrPath("attribute").Index(cty.NumberIntVal(1)).GetAttr("nestedAttribute"), + want: true, + }, + "path with unknown number index returns true": { + p: cty.GetAttrPath("attribute").Index(cty.UnknownVal(cty.Number)).GetAttr("nestedAttribute"), + other: cty.GetAttrPath("attribute").Index(cty.NumberIntVal(1)).GetAttr("nestedAttribute"), + want: true, + }, + "other path with unknown number index returns true": { + p: cty.GetAttrPath("attribute").Index(cty.NumberIntVal(1)).GetAttr("nestedAttribute"), + other: cty.GetAttrPath("attribute").Index(cty.UnknownVal(cty.Number)).GetAttr("nestedAttribute"), + want: true, + }, + "both paths with unknown number index returns true": { + p: cty.GetAttrPath("attribute").Index(cty.UnknownVal(cty.Number)).GetAttr("nestedAttribute"), + other: cty.GetAttrPath("attribute").Index(cty.UnknownVal(cty.Number)).GetAttr("nestedAttribute"), + want: true, + }, + "path with unknown string index returns true": { + p: cty.GetAttrPath("attribute").Index(cty.UnknownVal(cty.String)).GetAttr("nestedAttribute"), + other: cty.GetAttrPath("attribute").Index(cty.StringVal("key")).GetAttr("nestedAttribute"), + want: true, + }, + "other path with unknown string index returns true": { + p: cty.GetAttrPath("attribute").Index(cty.StringVal("key")).GetAttr("nestedAttribute"), + other: cty.GetAttrPath("attribute").Index(cty.UnknownVal(cty.String)).GetAttr("nestedAttribute"), + want: true, + }, + "both paths with unknown string index returns true": { + p: cty.GetAttrPath("attribute").Index(cty.UnknownVal(cty.String)).GetAttr("nestedAttribute"), + other: cty.GetAttrPath("attribute").Index(cty.UnknownVal(cty.String)).GetAttr("nestedAttribute"), + want: true, + }, + "path with unknown object index returns true": { + p: cty.GetAttrPath("attribute").Index(cty.UnknownVal(cty.Object(nil))).GetAttr("nestedAttribute"), + other: cty.GetAttrPath("attribute").Index(cty.ObjectVal( + map[string]cty.Value{ + "oldAttribute": cty.StringVal("old"), + "writeOnlyAttribute": cty.StringVal("writeOnly"), + }, + )).GetAttr("nestedAttribute"), + want: true, + }, + "other path with unknown object index returns true": { + p: cty.GetAttrPath("attribute").Index(cty.ObjectVal( + map[string]cty.Value{ + "oldAttribute": cty.StringVal("old"), + "writeOnlyAttribute": cty.StringVal("writeOnly"), + }, + )).GetAttr("nestedAttribute"), + other: cty.GetAttrPath("attribute").Index(cty.UnknownVal(cty.Object(nil))).GetAttr("nestedAttribute"), + want: true, + }, + "both paths with unknown object index returns true": { + p: cty.GetAttrPath("attribute").Index(cty.UnknownVal(cty.Object(nil))).GetAttr("nestedAttribute"), + other: cty.GetAttrPath("attribute").Index(cty.UnknownVal(cty.Object(nil))).GetAttr("nestedAttribute"), + want: true, + }, + "paths with unequal steps return false": { + p: cty.GetAttrPath("attribute").Index(cty.UnknownVal(cty.Number)), + other: cty.GetAttrPath("attribute").Index(cty.NumberIntVal(1)).GetAttr("nestedAttribute"), + want: false, + }, + "paths with mismatched attribute names return false": { + p: cty.GetAttrPath("attribute").Index(cty.NumberIntVal(1)).GetAttr("nestedAttribute"), + other: cty.GetAttrPath("incorrect").Index(cty.NumberIntVal(1)).GetAttr("nestedAttribute"), + want: false, + }, + "paths with mismatched unknown index types return false": { + p: cty.GetAttrPath("attribute").Index(cty.UnknownVal(cty.Number)).GetAttr("nestedAttribute"), + other: cty.GetAttrPath("attribute").Index(cty.UnknownVal(cty.String)).GetAttr("nestedAttribute"), + want: false, + }, + "other path with unknown index, different type return false": { + p: cty.GetAttrPath("attribute").Index(cty.NumberIntVal(1)).GetAttr("nestedAttribute"), + other: cty.GetAttrPath("attribute").Index(cty.UnknownVal(cty.String)).GetAttr("nestedAttribute"), + want: false, + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + if got := PathMatches(tc.p, tc.other); got != tc.want { + t.Errorf("PathMatches() = %v, want %v", got, tc.want) + } + }) + } +} diff --git a/helper/validation/write_only.go b/helper/validation/write_only.go new file mode 100644 index 0000000000..303e72dd2d --- /dev/null +++ b/helper/validation/write_only.go @@ -0,0 +1,116 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package validation + +import ( + "context" + "fmt" + + "github.com/hashicorp/go-cty/cty" + + "github.com/hashicorp/terraform-plugin-sdk/v2/diag" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" +) + +// PreferWriteOnlyAttribute is a ValidateRawResourceConfigFunc that returns a warning +// if the Terraform client supports write-only attributes and the old attribute is +// not null. +// The last step in the path must be a cty.GetAttrStep{}. +// When creating a cty.IndexStep{} to into a nested attribute, use an unknown value +// of the index type to indicate any key value. +// For lists: cty.Index(cty.UnknownVal(cty.Number)), +// For maps: cty.Index(cty.UnknownVal(cty.String)), +// For sets: cty.Index(cty.UnknownVal(cty.Object(nil))), +func PreferWriteOnlyAttribute(oldAttribute cty.Path, writeOnlyAttribute cty.Path) schema.ValidateRawResourceConfigFunc { + return func(ctx context.Context, req schema.ValidateResourceConfigFuncRequest, resp *schema.ValidateResourceConfigFuncResponse) { + if !req.WriteOnlyAttributesAllowed { + return + } + + pathLen := len(writeOnlyAttribute) + + if pathLen == 0 { + return + } + + lastStep := writeOnlyAttribute[pathLen-1] + + // Only attribute steps have a Name field + writeOnlyAttrStep, ok := lastStep.(cty.GetAttrStep) + if !ok { + resp.Diagnostics = diag.Diagnostics{ + { + Severity: diag.Error, + Summary: "Invalid writeOnlyAttribute path", + Detail: "The Terraform Provider unexpectedly provided a path that does not match the current schema. " + + "This can happen if the path does not correctly follow the schema in structure or types. " + + "Please report this to the provider developers. \n\n" + + "The writeOnlyAttribute path provided is invalid. The last step in the path must be a cty.GetAttrStep{}", + AttributePath: writeOnlyAttribute, + }, + } + return + } + + var oldAttrs []attribute + + err := cty.Walk(req.RawConfig, func(path cty.Path, value cty.Value) (bool, error) { + if PathMatches(path, oldAttribute) { + oldAttrs = append(oldAttrs, attribute{ + value: value, + path: path, + }) + } + + return true, nil + }) + if err != nil { + return + } + + for _, attr := range oldAttrs { + attrPath := attr.path.Copy() + + pathLen = len(attrPath) + + if pathLen == 0 { + return + } + + lastStep = attrPath[pathLen-1] + + // Only attribute steps have a Name field + attrStep, ok := lastStep.(cty.GetAttrStep) + if !ok { + resp.Diagnostics = diag.Diagnostics{ + { + Severity: diag.Error, + Summary: "Invalid oldAttribute path", + Detail: "The Terraform Provider unexpectedly provided a path that does not match the current schema. " + + "This can happen if the path does not correctly follow the schema in structure or types. " + + "Please report this to the provider developers. \n\n" + + "The oldAttribute path provided is invalid. The last step in the path must be a cty.GetAttrStep{}", + AttributePath: attrPath, + }, + } + return + } + + if !attr.value.IsNull() { + resp.Diagnostics = append(resp.Diagnostics, diag.Diagnostic{ + Severity: diag.Warning, + Summary: "Available Write-only Attribute Alternative", + Detail: fmt.Sprintf("The attribute %s has a write-only alternative %s available. "+ + "Use the write-only alternative of the attribute when possible.", attrStep.Name, writeOnlyAttrStep.Name), + AttributePath: attr.path, + }) + } + } + } +} + +type attribute struct { + value cty.Value + path cty.Path +} diff --git a/helper/validation/write_only_test.go b/helper/validation/write_only_test.go new file mode 100644 index 0000000000..97388826df --- /dev/null +++ b/helper/validation/write_only_test.go @@ -0,0 +1,783 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package validation + +import ( + "context" + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/hashicorp/go-cty/cty" + + "github.com/hashicorp/terraform-plugin-sdk/v2/diag" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" +) + +func TestPreferWriteOnlyAttribute(t *testing.T) { + cases := map[string]struct { + oldAttributePath cty.Path + validateConfigReq schema.ValidateResourceConfigFuncRequest + expectedDiags diag.Diagnostics + }{ + "writeOnlyAttributeAllowed set to false with oldAttribute set returns no diags": { + oldAttributePath: cty.GetAttrPath("oldAttribute"), + validateConfigReq: schema.ValidateResourceConfigFuncRequest{ + WriteOnlyAttributesAllowed: false, + RawConfig: cty.ObjectVal(map[string]cty.Value{ + "oldAttribute": cty.NumberIntVal(42), + "writeOnlyAttribute": cty.NullVal(cty.Number), + }), + }, + }, + "invalid oldAttributePath returns error diag": { + oldAttributePath: cty.GetAttrPath("oldAttribute").Index(cty.UnknownVal(cty.Number)), + validateConfigReq: schema.ValidateResourceConfigFuncRequest{ + WriteOnlyAttributesAllowed: true, + RawConfig: cty.ObjectVal(map[string]cty.Value{ + "oldAttribute": cty.ListVal([]cty.Value{ + cty.StringVal("val1"), + cty.StringVal("val2"), + }), + "writeOnlyAttribute": cty.NullVal(cty.Number), + }), + }, + expectedDiags: diag.Diagnostics{ + { + Severity: diag.Error, + Summary: "Invalid oldAttribute path", + Detail: "The Terraform Provider unexpectedly provided a path that does not match the current schema. " + + "This can happen if the path does not correctly follow the schema in structure or types. " + + "Please report this to the provider developers. \n\n" + + "The oldAttribute path provided is invalid. The last step in the path must be a cty.GetAttrStep{}", + AttributePath: cty.Path{ + cty.GetAttrStep{Name: "oldAttribute"}, + cty.IndexStep{ + Key: cty.NumberIntVal(0), + }, + }, + }, + }, + }, + "oldAttribute and writeOnlyAttribute set returns warning diags": { + oldAttributePath: cty.GetAttrPath("oldAttribute"), + validateConfigReq: schema.ValidateResourceConfigFuncRequest{ + WriteOnlyAttributesAllowed: true, + RawConfig: cty.ObjectVal(map[string]cty.Value{ + "oldAttribute": cty.NumberIntVal(42), + "writeOnlyAttribute": cty.NumberIntVal(42), + }), + }, + expectedDiags: diag.Diagnostics{ + { + Severity: diag.Warning, + Summary: "Available Write-only Attribute Alternative", + Detail: "The attribute oldAttribute has a write-only alternative writeOnlyAttribute available. " + + "Use the write-only alternative of the attribute when possible.", + AttributePath: cty.Path{cty.GetAttrStep{Name: "oldAttribute"}}, + }, + }, + }, + "writeOnlyAttribute set returns no diags": { + oldAttributePath: cty.GetAttrPath("oldAttribute"), + validateConfigReq: schema.ValidateResourceConfigFuncRequest{ + WriteOnlyAttributesAllowed: true, + RawConfig: cty.ObjectVal(map[string]cty.Value{ + "oldAttribute": cty.NullVal(cty.Number), + "writeOnlyAttribute": cty.NumberIntVal(42), + }), + }, + }, + "oldAttributePath pointing to missing attribute returns no diags": { + oldAttributePath: cty.GetAttrPath("oldAttribute"), + validateConfigReq: schema.ValidateResourceConfigFuncRequest{ + WriteOnlyAttributesAllowed: true, + RawConfig: cty.ObjectVal(map[string]cty.Value{ + "writeOnlyAttribute": cty.NumberIntVal(42), + }), + }, + expectedDiags: nil, + }, + "oldAttributePath with empty path returns no diags": { + oldAttributePath: cty.Path{}, + validateConfigReq: schema.ValidateResourceConfigFuncRequest{ + WriteOnlyAttributesAllowed: true, + RawConfig: cty.ObjectVal(map[string]cty.Value{ + "oldAttribute": cty.NumberIntVal(42), + "writeOnlyAttribute": cty.NumberIntVal(42), + }), + }, + expectedDiags: nil, + }, + "only oldAttribute set returns warning diag": { + oldAttributePath: cty.GetAttrPath("oldAttribute"), + validateConfigReq: schema.ValidateResourceConfigFuncRequest{ + WriteOnlyAttributesAllowed: true, + RawConfig: cty.ObjectVal(map[string]cty.Value{ + "oldAttribute": cty.NumberIntVal(42), + "writeOnlyAttribute": cty.NullVal(cty.Number), + }), + }, + expectedDiags: diag.Diagnostics{ + { + Severity: diag.Warning, + Summary: "Available Write-only Attribute Alternative", + Detail: "The attribute oldAttribute has a write-only alternative writeOnlyAttribute available. " + + "Use the write-only alternative of the attribute when possible.", + AttributePath: cty.Path{cty.GetAttrStep{Name: "oldAttribute"}}, + }, + }, + }, + "block: oldAttribute and writeOnlyAttribute set returns warning diag": { + oldAttributePath: cty.Path{ + cty.GetAttrStep{Name: "config_block_attr"}, + cty.GetAttrStep{Name: "oldAttribute"}, + }, + validateConfigReq: schema.ValidateResourceConfigFuncRequest{ + WriteOnlyAttributesAllowed: true, + RawConfig: cty.ObjectVal(map[string]cty.Value{ + "id": cty.NullVal(cty.String), + "config_block_attr": cty.ObjectVal(map[string]cty.Value{ + "oldAttribute": cty.StringVal("value"), + "writeOnlyAttribute": cty.StringVal("value"), + }), + }), + }, + expectedDiags: diag.Diagnostics{ + { + Severity: diag.Warning, + Summary: "Available Write-only Attribute Alternative", + Detail: "The attribute oldAttribute has a write-only alternative writeOnlyAttribute available. " + + "Use the write-only alternative of the attribute when possible.", + AttributePath: cty.Path{ + cty.GetAttrStep{Name: "config_block_attr"}, + cty.GetAttrStep{Name: "oldAttribute"}, + }, + }, + }, + }, + "block: writeOnlyAttribute set returns no diags": { + oldAttributePath: cty.Path{ + cty.GetAttrStep{Name: "config_block_attr"}, + cty.GetAttrStep{Name: "oldAttribute"}, + }, + validateConfigReq: schema.ValidateResourceConfigFuncRequest{ + WriteOnlyAttributesAllowed: true, + RawConfig: cty.ObjectVal(map[string]cty.Value{ + "id": cty.NullVal(cty.String), + "config_block_attr": cty.ObjectVal(map[string]cty.Value{ + "oldAttribute": cty.NullVal(cty.String), + "writeOnlyAttribute": cty.StringVal("value"), + }), + }), + }, + }, + "block: only oldAttribute set returns warning diag": { + oldAttributePath: cty.Path{ + cty.GetAttrStep{Name: "config_block_attr"}, + cty.GetAttrStep{Name: "oldAttribute"}, + }, + validateConfigReq: schema.ValidateResourceConfigFuncRequest{ + WriteOnlyAttributesAllowed: true, + RawConfig: cty.ObjectVal(map[string]cty.Value{ + "id": cty.NullVal(cty.String), + "config_block_attr": cty.ObjectVal(map[string]cty.Value{ + "oldAttribute": cty.StringVal("value"), + "writeOnlyAttribute": cty.NullVal(cty.String), + }), + }), + }, + expectedDiags: diag.Diagnostics{ + { + Severity: diag.Warning, + Summary: "Available Write-only Attribute Alternative", + Detail: "The attribute oldAttribute has a write-only alternative writeOnlyAttribute available. " + + "Use the write-only alternative of the attribute when possible.", + AttributePath: cty.Path{ + cty.GetAttrStep{Name: "config_block_attr"}, + cty.GetAttrStep{Name: "oldAttribute"}, + }, + }, + }, + }, + "list nested block: oldAttribute and writeOnlyAttribute set returns warning diag": { + oldAttributePath: cty.Path{ + cty.GetAttrStep{Name: "list_nested_block"}, + cty.IndexStep{Key: cty.UnknownVal(cty.Number)}, + cty.GetAttrStep{Name: "oldAttribute"}, + }, + validateConfigReq: schema.ValidateResourceConfigFuncRequest{ + WriteOnlyAttributesAllowed: true, + RawConfig: cty.ObjectVal(map[string]cty.Value{ + "id": cty.NullVal(cty.String), + "list_nested_block": cty.ListVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "oldAttribute": cty.StringVal("value"), + "writeOnlyAttribute": cty.StringVal("value"), + }), + }), + }), + }, + expectedDiags: diag.Diagnostics{ + { + Severity: diag.Warning, + Summary: "Available Write-only Attribute Alternative", + Detail: "The attribute oldAttribute has a write-only alternative writeOnlyAttribute available. " + + "Use the write-only alternative of the attribute when possible.", + AttributePath: cty.Path{ + cty.GetAttrStep{Name: "list_nested_block"}, + cty.IndexStep{Key: cty.NumberIntVal(0)}, + cty.GetAttrStep{Name: "oldAttribute"}, + }, + }, + }, + }, + "list nested block: writeOnlyAttribute set returns no diags": { + oldAttributePath: cty.Path{ + cty.GetAttrStep{Name: "list_nested_block"}, + cty.IndexStep{Key: cty.UnknownVal(cty.Number)}, + cty.GetAttrStep{Name: "oldAttribute"}, + }, + validateConfigReq: schema.ValidateResourceConfigFuncRequest{ + WriteOnlyAttributesAllowed: true, + RawConfig: cty.ObjectVal(map[string]cty.Value{ + "id": cty.NullVal(cty.String), + "list_nested_block": cty.ListVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "writeOnlyAttribute": cty.StringVal("value"), + }), + }), + }), + }, + expectedDiags: nil, + }, + "list nested block: only oldAttribute set returns warning diag": { + oldAttributePath: cty.Path{ + cty.GetAttrStep{Name: "list_nested_block"}, + cty.IndexStep{Key: cty.UnknownVal(cty.Number)}, + cty.GetAttrStep{Name: "oldAttribute"}, + }, + validateConfigReq: schema.ValidateResourceConfigFuncRequest{ + WriteOnlyAttributesAllowed: true, + RawConfig: cty.ObjectVal(map[string]cty.Value{ + "id": cty.NullVal(cty.String), + "list_nested_block": cty.ListVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "oldAttribute": cty.StringVal("value"), + }), + }), + }), + }, + expectedDiags: diag.Diagnostics{ + { + Severity: diag.Warning, + Summary: "Available Write-only Attribute Alternative", + Detail: "The attribute oldAttribute has a write-only alternative writeOnlyAttribute available. " + + "Use the write-only alternative of the attribute when possible.", + AttributePath: cty.Path{ + cty.GetAttrStep{Name: "list_nested_block"}, + cty.IndexStep{Key: cty.NumberIntVal(0)}, + cty.GetAttrStep{Name: "oldAttribute"}, + }, + }, + }, + }, + "list nested block: multiple oldAttribute set returns multiple warning diags": { + oldAttributePath: cty.Path{ + cty.GetAttrStep{Name: "list_nested_block"}, + cty.IndexStep{Key: cty.UnknownVal(cty.Number)}, + cty.GetAttrStep{Name: "oldAttribute"}, + }, + validateConfigReq: schema.ValidateResourceConfigFuncRequest{ + WriteOnlyAttributesAllowed: true, + RawConfig: cty.ObjectVal(map[string]cty.Value{ + "id": cty.NullVal(cty.String), + "list_nested_block": cty.ListVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "oldAttribute": cty.StringVal("value1"), + "writeOnlyAttribute": cty.NullVal(cty.String), + }), + cty.ObjectVal(map[string]cty.Value{ + "oldAttribute": cty.NullVal(cty.String), + "writeOnlyAttribute": cty.StringVal("value2"), + }), + cty.ObjectVal(map[string]cty.Value{ + "oldAttribute": cty.StringVal("value3"), + "writeOnlyAttribute": cty.NullVal(cty.String), + }), + }), + }), + }, + expectedDiags: diag.Diagnostics{ + { + Severity: diag.Warning, + Summary: "Available Write-only Attribute Alternative", + Detail: "The attribute oldAttribute has a write-only alternative writeOnlyAttribute available. " + + "Use the write-only alternative of the attribute when possible.", + AttributePath: cty.Path{ + cty.GetAttrStep{Name: "list_nested_block"}, + cty.IndexStep{Key: cty.NumberIntVal(0)}, + cty.GetAttrStep{Name: "oldAttribute"}, + }, + }, + { + Severity: diag.Warning, + Summary: "Available Write-only Attribute Alternative", + Detail: "The attribute oldAttribute has a write-only alternative writeOnlyAttribute available. " + + "Use the write-only alternative of the attribute when possible.", + AttributePath: cty.Path{ + cty.GetAttrStep{Name: "list_nested_block"}, + cty.IndexStep{Key: cty.NumberIntVal(2)}, + cty.GetAttrStep{Name: "oldAttribute"}, + }, + }, + }, + }, + "set nested block: oldAttribute and writeOnlyAttribute set returns warning diags": { + oldAttributePath: cty.Path{ + cty.GetAttrStep{Name: "set_nested_block"}, + cty.IndexStep{Key: cty.UnknownVal(cty.Object( + map[string]cty.Type{ + "oldAttribute": cty.String, + "writeOnlyAttribute": cty.String, + }, + ))}, + cty.GetAttrStep{Name: "oldAttribute"}, + }, + validateConfigReq: schema.ValidateResourceConfigFuncRequest{ + WriteOnlyAttributesAllowed: true, + RawConfig: cty.ObjectVal(map[string]cty.Value{ + "id": cty.NullVal(cty.String), + "set_nested_block": cty.SetVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "oldAttribute": cty.StringVal("value"), + "writeOnlyAttribute": cty.StringVal("value"), + }), + }), + }), + }, + expectedDiags: diag.Diagnostics{ + { + Severity: diag.Warning, + Summary: "Available Write-only Attribute Alternative", + Detail: "The attribute oldAttribute has a write-only alternative writeOnlyAttribute available. " + + "Use the write-only alternative of the attribute when possible.", + AttributePath: cty.Path{ + cty.GetAttrStep{Name: "set_nested_block"}, + cty.IndexStep{Key: cty.ObjectVal(map[string]cty.Value{ + "oldAttribute": cty.StringVal("value"), + "writeOnlyAttribute": cty.StringVal("value"), + })}, + cty.GetAttrStep{Name: "oldAttribute"}, + }, + }, + }, + }, + "set nested block: writeOnlyAttribute set returns no diags": { + oldAttributePath: cty.Path{ + cty.GetAttrStep{Name: "set_nested_block"}, + cty.IndexStep{Key: cty.UnknownVal(cty.Object( + map[string]cty.Type{ + "oldAttribute": cty.String, + }, + ))}, + cty.GetAttrStep{Name: "oldAttribute"}, + }, + validateConfigReq: schema.ValidateResourceConfigFuncRequest{ + WriteOnlyAttributesAllowed: true, + RawConfig: cty.ObjectVal(map[string]cty.Value{ + "id": cty.NullVal(cty.String), + "set_nested_block": cty.SetVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "writeOnlyAttribute": cty.StringVal("value"), + }), + }), + }), + }, + expectedDiags: nil, + }, + "set nested block: only oldAttribute set returns warning diag": { + oldAttributePath: cty.Path{ + cty.GetAttrStep{Name: "set_nested_block"}, + cty.IndexStep{Key: cty.UnknownVal(cty.Object( + map[string]cty.Type{ + "oldAttribute": cty.String, + }, + ))}, + cty.GetAttrStep{Name: "oldAttribute"}, + }, + validateConfigReq: schema.ValidateResourceConfigFuncRequest{ + WriteOnlyAttributesAllowed: true, + RawConfig: cty.ObjectVal(map[string]cty.Value{ + "id": cty.NullVal(cty.String), + "set_nested_block": cty.SetVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "oldAttribute": cty.StringVal("value"), + }), + }), + }), + }, + expectedDiags: diag.Diagnostics{ + { + Severity: diag.Warning, + Summary: "Available Write-only Attribute Alternative", + Detail: "The attribute oldAttribute has a write-only alternative writeOnlyAttribute available. " + + "Use the write-only alternative of the attribute when possible.", + AttributePath: cty.Path{ + cty.GetAttrStep{Name: "set_nested_block"}, + cty.IndexStep{Key: cty.ObjectVal(map[string]cty.Value{ + "oldAttribute": cty.StringVal("value"), + })}, + cty.GetAttrStep{Name: "oldAttribute"}, + }, + }, + }, + }, + "set nested block: multiple oldAttribute set returns multiple warning diags": { + oldAttributePath: cty.Path{ + cty.GetAttrStep{Name: "set_nested_block"}, + cty.IndexStep{Key: cty.UnknownVal(cty.Object(nil))}, + cty.GetAttrStep{Name: "oldAttribute"}, + }, + validateConfigReq: schema.ValidateResourceConfigFuncRequest{ + WriteOnlyAttributesAllowed: true, + RawConfig: cty.ObjectVal(map[string]cty.Value{ + "id": cty.NullVal(cty.String), + "set_nested_block": cty.SetVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "oldAttribute": cty.StringVal("value1"), + "writeOnlyAttribute": cty.NullVal(cty.String), + }), + cty.ObjectVal(map[string]cty.Value{ + "oldAttribute": cty.NullVal(cty.String), + "writeOnlyAttribute": cty.StringVal("value2"), + }), + cty.ObjectVal(map[string]cty.Value{ + "oldAttribute": cty.StringVal("value3"), + "writeOnlyAttribute": cty.NullVal(cty.String), + }), + }), + }), + }, + expectedDiags: diag.Diagnostics{ + { + Severity: diag.Warning, + Summary: "Available Write-only Attribute Alternative", + Detail: "The attribute oldAttribute has a write-only alternative writeOnlyAttribute available. " + + "Use the write-only alternative of the attribute when possible.", + AttributePath: cty.Path{ + cty.GetAttrStep{Name: "set_nested_block"}, + cty.IndexStep{Key: cty.ObjectVal(map[string]cty.Value{ + "oldAttribute": cty.StringVal("value1"), + "writeOnlyAttribute": cty.NullVal(cty.String), + })}, + cty.GetAttrStep{Name: "oldAttribute"}, + }, + }, + { + Severity: diag.Warning, + Summary: "Available Write-only Attribute Alternative", + Detail: "The attribute oldAttribute has a write-only alternative writeOnlyAttribute available. " + + "Use the write-only alternative of the attribute when possible.", + AttributePath: cty.Path{ + cty.GetAttrStep{Name: "set_nested_block"}, + cty.IndexStep{Key: cty.ObjectVal(map[string]cty.Value{ + "oldAttribute": cty.StringVal("value3"), + "writeOnlyAttribute": cty.NullVal(cty.String), + })}, + cty.GetAttrStep{Name: "oldAttribute"}, + }, + }, + }, + }, + "map nested block: oldAttribute and writeOnlyAttribute map returns warning diags": { + oldAttributePath: cty.Path{ + cty.GetAttrStep{Name: "map_nested_block"}, + cty.IndexStep{Key: cty.UnknownVal(cty.String)}, + cty.GetAttrStep{Name: "oldAttribute"}, + }, + validateConfigReq: schema.ValidateResourceConfigFuncRequest{ + WriteOnlyAttributesAllowed: true, + RawConfig: cty.ObjectVal(map[string]cty.Value{ + "id": cty.NullVal(cty.String), + "map_nested_block": cty.MapVal(map[string]cty.Value{ + "key1": cty.ObjectVal(map[string]cty.Value{ + "oldAttribute": cty.StringVal("value"), + "writeOnlyAttribute": cty.StringVal("value"), + }), + }), + }), + }, + expectedDiags: diag.Diagnostics{ + { + Severity: diag.Warning, + Summary: "Available Write-only Attribute Alternative", + Detail: "The attribute oldAttribute has a write-only alternative writeOnlyAttribute available. " + + "Use the write-only alternative of the attribute when possible.", + AttributePath: cty.Path{ + cty.GetAttrStep{Name: "map_nested_block"}, + cty.IndexStep{Key: cty.StringVal("key1")}, + cty.GetAttrStep{Name: "oldAttribute"}, + }, + }, + }, + }, + "map nested block: writeOnlyAttribute map returns no diags": { + oldAttributePath: cty.Path{ + cty.GetAttrStep{Name: "map_nested_block"}, + cty.IndexStep{Key: cty.UnknownVal(cty.String)}, + cty.GetAttrStep{Name: "oldAttribute"}, + }, + validateConfigReq: schema.ValidateResourceConfigFuncRequest{ + WriteOnlyAttributesAllowed: true, + RawConfig: cty.ObjectVal(map[string]cty.Value{ + "id": cty.NullVal(cty.String), + "map_nested_block": cty.MapVal(map[string]cty.Value{ + "key1": cty.ObjectVal(map[string]cty.Value{ + "writeOnlyAttribute": cty.StringVal("value"), + }), + }), + }), + }, + expectedDiags: nil, + }, + "map nested block: only oldAttribute map returns warning diag": { + oldAttributePath: cty.Path{ + cty.GetAttrStep{Name: "map_nested_block"}, + cty.IndexStep{Key: cty.UnknownVal(cty.String)}, + cty.GetAttrStep{Name: "oldAttribute"}, + }, + validateConfigReq: schema.ValidateResourceConfigFuncRequest{ + WriteOnlyAttributesAllowed: true, + RawConfig: cty.ObjectVal(map[string]cty.Value{ + "id": cty.NullVal(cty.String), + "map_nested_block": cty.MapVal(map[string]cty.Value{ + "key1": cty.ObjectVal(map[string]cty.Value{ + "oldAttribute": cty.StringVal("value"), + }), + }), + }), + }, + expectedDiags: diag.Diagnostics{ + { + Severity: diag.Warning, + Summary: "Available Write-only Attribute Alternative", + Detail: "The attribute oldAttribute has a write-only alternative writeOnlyAttribute available. " + + "Use the write-only alternative of the attribute when possible.", + AttributePath: cty.Path{ + cty.GetAttrStep{Name: "map_nested_block"}, + cty.IndexStep{Key: cty.StringVal("key1")}, + cty.GetAttrStep{Name: "oldAttribute"}, + }, + }, + }, + }, + "map nested block: multiple oldAttribute map returns multiple warning diags": { + oldAttributePath: cty.Path{ + cty.GetAttrStep{Name: "map_nested_block"}, + cty.IndexStep{Key: cty.UnknownVal(cty.String)}, + cty.GetAttrStep{Name: "oldAttribute"}, + }, + validateConfigReq: schema.ValidateResourceConfigFuncRequest{ + WriteOnlyAttributesAllowed: true, + RawConfig: cty.ObjectVal(map[string]cty.Value{ + "id": cty.NullVal(cty.String), + "map_nested_block": cty.MapVal(map[string]cty.Value{ + "key1": cty.ObjectVal(map[string]cty.Value{ + "oldAttribute": cty.StringVal("value1"), + "writeOnlyAttribute": cty.NullVal(cty.String), + }), + "key2": cty.ObjectVal(map[string]cty.Value{ + "oldAttribute": cty.NullVal(cty.String), + "writeOnlyAttribute": cty.StringVal("value2"), + }), + "key3": cty.ObjectVal(map[string]cty.Value{ + "oldAttribute": cty.StringVal("value3"), + "writeOnlyAttribute": cty.NullVal(cty.String), + }), + }), + }), + }, + expectedDiags: diag.Diagnostics{ + { + Severity: diag.Warning, + Summary: "Available Write-only Attribute Alternative", + Detail: "The attribute oldAttribute has a write-only alternative writeOnlyAttribute available. " + + "Use the write-only alternative of the attribute when possible.", + AttributePath: cty.Path{ + cty.GetAttrStep{Name: "map_nested_block"}, + cty.IndexStep{Key: cty.StringVal("key1")}, + cty.GetAttrStep{Name: "oldAttribute"}, + }, + }, + { + Severity: diag.Warning, + Summary: "Available Write-only Attribute Alternative", + Detail: "The attribute oldAttribute has a write-only alternative writeOnlyAttribute available. " + + "Use the write-only alternative of the attribute when possible.", + AttributePath: cty.Path{ + cty.GetAttrStep{Name: "map_nested_block"}, + cty.IndexStep{Key: cty.StringVal("key3")}, + cty.GetAttrStep{Name: "oldAttribute"}, + }, + }, + }, + }, + "map nested set nested block: multiple oldAttribute map returns multiple warning diags": { + oldAttributePath: cty.Path{ + cty.GetAttrStep{Name: "map_nested_block"}, + cty.IndexStep{Key: cty.UnknownVal(cty.String)}, + cty.GetAttrStep{Name: "set_nested_block"}, + cty.IndexStep{Key: cty.UnknownVal(cty.Object(nil))}, + cty.GetAttrStep{Name: "oldAttribute"}, + }, + validateConfigReq: schema.ValidateResourceConfigFuncRequest{ + WriteOnlyAttributesAllowed: true, + RawConfig: cty.ObjectVal(map[string]cty.Value{ + "id": cty.NullVal(cty.String), + "map_nested_block": cty.MapVal(map[string]cty.Value{ + "key1": cty.ObjectVal(map[string]cty.Value{ + "set_nested_block": cty.SetVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "oldAttribute": cty.StringVal("value1"), + "writeOnlyAttribute": cty.NullVal(cty.String), + }), + cty.ObjectVal(map[string]cty.Value{ + "oldAttribute": cty.NullVal(cty.String), + "writeOnlyAttribute": cty.StringVal("value2"), + }), + cty.ObjectVal(map[string]cty.Value{ + "oldAttribute": cty.StringVal("value3"), + "writeOnlyAttribute": cty.NullVal(cty.String), + }), + }), + "string_nested_attribute": cty.NullVal(cty.String), + }), + "key2": cty.ObjectVal(map[string]cty.Value{ + "set_nested_block": cty.SetVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "oldAttribute": cty.NullVal(cty.String), + "writeOnlyAttribute": cty.StringVal("value2"), + }), + }), + "string_nested_attribute": cty.StringVal("value1"), + }), + "key3": cty.ObjectVal(map[string]cty.Value{ + "set_nested_block": cty.SetVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "oldAttribute": cty.StringVal("value1"), + "writeOnlyAttribute": cty.NullVal(cty.String), + }), + cty.ObjectVal(map[string]cty.Value{ + "oldAttribute": cty.NullVal(cty.String), + "writeOnlyAttribute": cty.StringVal("value2"), + }), + cty.ObjectVal(map[string]cty.Value{ + "oldAttribute": cty.StringVal("value3"), + "writeOnlyAttribute": cty.NullVal(cty.String), + }), + }), + "string_nested_attribute": cty.StringVal("value1"), + }), + }), + }), + }, + expectedDiags: diag.Diagnostics{ + { + Severity: diag.Warning, + Summary: "Available Write-only Attribute Alternative", + Detail: "The attribute oldAttribute has a write-only alternative writeOnlyAttribute available. " + + "Use the write-only alternative of the attribute when possible.", + AttributePath: cty.Path{ + cty.GetAttrStep{Name: "map_nested_block"}, + cty.IndexStep{Key: cty.StringVal("key1")}, + cty.GetAttrStep{Name: "set_nested_block"}, + cty.IndexStep{Key: cty.ObjectVal(map[string]cty.Value{ + "oldAttribute": cty.StringVal("value1"), + "writeOnlyAttribute": cty.NullVal(cty.String), + }), + }, + cty.GetAttrStep{Name: "oldAttribute"}, + }, + }, + { + Severity: diag.Warning, + Summary: "Available Write-only Attribute Alternative", + Detail: "The attribute oldAttribute has a write-only alternative writeOnlyAttribute available. " + + "Use the write-only alternative of the attribute when possible.", + AttributePath: cty.Path{ + cty.GetAttrStep{Name: "map_nested_block"}, + cty.IndexStep{Key: cty.StringVal("key1")}, + cty.GetAttrStep{Name: "set_nested_block"}, + cty.IndexStep{Key: cty.ObjectVal(map[string]cty.Value{ + "oldAttribute": cty.StringVal("value3"), + "writeOnlyAttribute": cty.NullVal(cty.String), + })}, + cty.GetAttrStep{Name: "oldAttribute"}, + }, + }, + { + Severity: diag.Warning, + Summary: "Available Write-only Attribute Alternative", + Detail: "The attribute oldAttribute has a write-only alternative writeOnlyAttribute available. " + + "Use the write-only alternative of the attribute when possible.", + AttributePath: cty.Path{ + cty.GetAttrStep{Name: "map_nested_block"}, + cty.IndexStep{Key: cty.StringVal("key3")}, + cty.GetAttrStep{Name: "set_nested_block"}, + cty.IndexStep{Key: cty.ObjectVal(map[string]cty.Value{ + "oldAttribute": cty.StringVal("value1"), + "writeOnlyAttribute": cty.NullVal(cty.String), + })}, + cty.GetAttrStep{Name: "oldAttribute"}, + }, + }, + { + Severity: diag.Warning, + Summary: "Available Write-only Attribute Alternative", + Detail: "The attribute oldAttribute has a write-only alternative writeOnlyAttribute available. " + + "Use the write-only alternative of the attribute when possible.", + AttributePath: cty.Path{ + cty.GetAttrStep{Name: "map_nested_block"}, + cty.IndexStep{Key: cty.StringVal("key3")}, + cty.GetAttrStep{Name: "set_nested_block"}, + cty.IndexStep{Key: cty.ObjectVal(map[string]cty.Value{ + "oldAttribute": cty.StringVal("value3"), + "writeOnlyAttribute": cty.NullVal(cty.String), + }), + }, + cty.GetAttrStep{Name: "oldAttribute"}, + }, + }, + }, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + f := PreferWriteOnlyAttribute(tc.oldAttributePath, cty.GetAttrPath("writeOnlyAttribute")) + + actual := &schema.ValidateResourceConfigFuncResponse{} + f(context.Background(), tc.validateConfigReq, actual) + + if len(actual.Diagnostics) == 0 && tc.expectedDiags == nil { + return + } + + if len(actual.Diagnostics) != 0 && tc.expectedDiags == nil { + t.Fatalf("expected no diagnostics but got %v", actual.Diagnostics) + } + + if diff := cmp.Diff(tc.expectedDiags, actual.Diagnostics, + cmp.AllowUnexported(cty.GetAttrStep{}, cty.IndexStep{}), + cmp.Comparer(indexStepComparer), + ); diff != "" { + t.Errorf("Unexpected diagnostics (-wanted +got): %s", diff) + } + }) + } +} + +func indexStepComparer(step cty.IndexStep, other cty.IndexStep) bool { + return step.Key.RawEquals(other.Key) +} diff --git a/internal/configs/configschema/schema.go b/internal/configs/configschema/schema.go index fafe3fa91c..983d20bdf0 100644 --- a/internal/configs/configschema/schema.go +++ b/internal/configs/configschema/schema.go @@ -83,6 +83,18 @@ type Attribute struct { // Deprecated indicates whether the attribute has been marked as deprecated in the // provider and usage should be discouraged. Deprecated bool + + // WriteOnly indicates that the practitioner can choose a value for this + // attribute, but Terraform will not store this attribute in plan or state. + // WriteOnly can only be set for managed resource schemas. If WriteOnly is true, + // either Optional or Required must also be true. WriteOnly cannot be set with ForceNew. + // + // WriteOnly cannot be set to true for TypeList, TypeMap, or TypeSet. + // + // This functionality is only supported in Terraform 1.11 and later. + // Practitioners that choose a value for this attribute with older + // versions of Terraform will receive an error. + WriteOnly bool } // NestedBlock represents the embedding of one block within another. diff --git a/internal/plugin/convert/schema.go b/internal/plugin/convert/schema.go index e2b4e431ce..a02aaec007 100644 --- a/internal/plugin/convert/schema.go +++ b/internal/plugin/convert/schema.go @@ -12,6 +12,7 @@ import ( "github.com/hashicorp/go-cty/cty" "github.com/hashicorp/terraform-plugin-go/tfprotov5" "github.com/hashicorp/terraform-plugin-go/tftypes" + "github.com/hashicorp/terraform-plugin-sdk/v2/internal/configs/configschema" "github.com/hashicorp/terraform-plugin-sdk/v2/internal/logging" ) @@ -151,6 +152,7 @@ func ConfigSchemaToProto(ctx context.Context, b *configschema.Block) *tfprotov5. Required: a.Required, Sensitive: a.Sensitive, Deprecated: a.Deprecated, + WriteOnly: a.WriteOnly, } var err error diff --git a/internal/plugin/convert/schema_test.go b/internal/plugin/convert/schema_test.go index cf8b17aded..993fb47694 100644 --- a/internal/plugin/convert/schema_test.go +++ b/internal/plugin/convert/schema_test.go @@ -13,6 +13,7 @@ import ( "github.com/hashicorp/terraform-plugin-go/tfprotov5" "github.com/hashicorp/terraform-plugin-go/tftypes" + "github.com/hashicorp/terraform-plugin-sdk/v2/internal/configs/configschema" ) @@ -232,6 +233,12 @@ func TestConvertProtoSchemaBlocks(t *testing.T) { Type: tftypes.Number, Required: true, }, + { + Name: "write-only", + Type: tftypes.String, + WriteOnly: true, + Optional: true, + }, }, }, &configschema.Block{ @@ -253,6 +260,11 @@ func TestConvertProtoSchemaBlocks(t *testing.T) { Type: cty.Number, Required: true, }, + "write-only": { + Type: cty.String, + WriteOnly: true, + Optional: true, + }, }, }, }, diff --git a/website/data/plugin-sdkv2-nav-data.json b/website/data/plugin-sdkv2-nav-data.json index 86afe647a0..74574dc34e 100644 --- a/website/data/plugin-sdkv2-nav-data.json +++ b/website/data/plugin-sdkv2-nav-data.json @@ -38,6 +38,10 @@ { "title": "State Migration", "path": "resources/state-migration" + }, + { + "title": "Write-only Arguments", + "path": "resources/write-only-arguments" } ] }, diff --git a/website/docs/plugin/sdkv2/resources/index.mdx b/website/docs/plugin/sdkv2/resources/index.mdx index 4202a983f8..127148e968 100644 --- a/website/docs/plugin/sdkv2/resources/index.mdx +++ b/website/docs/plugin/sdkv2/resources/index.mdx @@ -30,3 +30,12 @@ Terraform has data consistency rules for resources, which may not be easily disc Resources define the data types and API interactions required to create, update, and destroy infrastructure with a cloud vendor, while the [Terraform state](/terraform/language/state) stores mapping and metadata information for those remote objects. When resource implementations change (due to bug fixes, improvements, or changes to the backend APIs Terraform interacts with), they can sometimes become incompatible with existing state. When this happens, a migration is needed for resources provisioned in the wild with old schema configurations. Terraform resources support migrating state values in these scenarios via [State Migration](/terraform/plugin/sdkv2/resources/state-migration). + +## Write-only Arguments + +~> **NOTE:** Write-only arguments are only supported in Terraform `v1.11` or higher + +Write-only arguments are a special type of managed resource attribute +that are configured by practitioners but are not persisted in the Terraform plan or state artifacts. +Write-only arguments can accept [ephemeral values](/terraform/language/resources/ephemeral) and are not required to be consistent between plan and apply operations. +The [Write-only arguments](/terraform/plugin/sdkv2/resources/write-only-arguments) page discusses how to create these arguments. \ No newline at end of file diff --git a/website/docs/plugin/sdkv2/resources/write-only-arguments.mdx b/website/docs/plugin/sdkv2/resources/write-only-arguments.mdx new file mode 100644 index 0000000000..1ed7faa6cf --- /dev/null +++ b/website/docs/plugin/sdkv2/resources/write-only-arguments.mdx @@ -0,0 +1,248 @@ +--- +page_title: Resources - Write-only Arguments +description: Implementing write-only arguments within resources. +--- + +# Resources - Write-only Arguments + +~> **NOTE:** Write-only arguments are only supported in Terraform `v1.11` or higher + +Write-only arguments are managed resource attributes that are configured by practitioners but are not persisted to the Terraform plan or state artifacts. Write-only arguments +should be used to handle secret values that do not need to be persisted in Terraform state, such as passwords, API keys, etc. +The provider is expected to be the terminal point for an ephemeral value, +which should either use the value by making the appropriate change to the API or ignore the value. Write-only arguments can accept [ephemeral values](/terraform/language/resources/ephemeral) and are not required to be consistent between plan and apply operations. + +## General Concepts + +The following are high level differences between `Required`/`Optional` arguments and write-only arguments: + +- Write-only arguments can accept ephemeral and non-ephemeral values + +- Write-only argument values are only available in the configuration. The prior state, planned state, and final state values for +write-only arguments should always be `null`. + - Provider developers do not need to explicitly set write-only argument values to `null` after using them as the SDKv2 will handle the nullification of write-only arguments for all RPCs. + +- Any value that is set for a write-only argument using `(*ResourceData).Set()` by the provider will be reverted to `null` by SDKv2 before the RPC response is sent to Terraform + +- Write-only argument values cannot produce a Terraform plan difference. + - This is because the prior state value for a write-only argument will always be `null` and the planned/final state value will also be `null`, therefore, it cannot produce a diff on its own. + - The one exception to this case is if the write-only argument is added to `requires_replace` via [CustomizeDiff](/terraform/plugin/sdkv2/resources/customizing-differences), in that case, the write-only argument will always cause a diff/trigger a resource recreation + +- Since write-only arguments can accept ephemeral values, write-only argument configuration values are not expected to be consistent between plan and apply. + +## Schema Behavior + +**Schema example:** + +```go +"password_wo": { + Type: schema.TypeString, + Required: true, + WriteOnly: true, +}, +``` + +**Restrictions:** + +- Cannot be used in data source or provider schemas +- Must be set with either `Required` is `true` or `Optional` is `true` +- Cannot be used when `Computed` is `true` +- Cannot be used when `ForceNew` is `true` +- Cannot be used when `Default` is `specified` +- Cannot be used with `DefaultFunc` +- Cannot be used with aggregate schema types (e.g. `typeMap`, `typeList`, `typeSet`), but non-computed nested block types can contain write-only arguments. + +## Retrieving Write-only Values + +Write-only argument values are only available in the raw resource configuration, you cannot retrieve it using `(*resourceData).Get()` like other attribute values. + +Use the `(*schema.ResourceData).GetRawConfigAt()` method to retrieve the raw config value. + +This method is an advanced method that uses the `hashicorp/go-cty` library for its type system. + +```go +woVal, diags := d.GetRawConfigAt(cty.GetAttrPath("password_wo")) +``` + +### cty.Path + +`(*schema.ResourceData).GetRawConfigAt()` uses `cty.Path` to specify locations in the raw configuration. + +This is very similar to the `terraform-plugin-framework` [paths](/terraform/plugin/framework/handling-data/paths) or `terraform-plugin-testing` [json paths](/terraform/plugin/testing/acceptance-tests/tfjson-paths). + +All top level attributes or blocks can be referred to using `cty.GetAttrPath()` + +**Configuration example:** + +```hcl +resource "example_resource" "example" { + "top_level_schema_attribute" = 1 +} +``` + +**Path example:** + +```go +cty.GetAttrPath("top_level_schema_attribute") // returns cty.NumberIntVal(1) +``` + +Maps can be traversed using `IndexString()` + +**Configuration example:** + +```hcl +resource "example_resource" "example" { + map_attribute { + key1 = "value1" + } +} +``` + +**Path example:** + +```go +// Map traversal +cty.GetAttrPath("map_attribute").IndexString("key1") // returns cty.StringVal("value1") +``` + +Lists or list nested blocks can be traversed using `IndexInt()` + +**Configuration example:** + +```hcl +resource "example_resource" "example" { + list_attribute = ["value1", "value2"] + list_nested_block { + list_nested_block_attribute = "value3" + } + + list_nested_block { + list_nested_block_attribute = "value4" + } +} +``` + +**Path example:** + +```go +// List traversal +cty.GetAttrPath("list_attribute").IndexInt(0) // returns cty.StringVal("value1") + + +// List nested block traversal +cty.GetAttrPath("list_nested_block").IndexInt(1).getAttr("list_nested_block_attribute") // returns cty.StringVal("value4") +``` + +Sets or set nested blocks can be traversed using `Index()`. `Index()` takes in a `cty.Value` of the set element that you want to traverse into. + +However, if you do not know the specific value of the desired set element, +you can also retrieve the entire set using `cty.GetAttrPath()`. + +**Configuration example:** + +```hcl +resource "example_resource" "example" { + set_attribute = ["value1", "value2"] + set_nested_block { + set_nested_block_attribute = "value3" + } + + set_nested_block { + set_nested_block_attribute = "value4" + } +} +``` + +**Path example:** + +```go +// Set attribute - root traversal +cty.GetAttrPath("set_attribute") // returns cty.SetVal([]cty.Value{cty.StringVal("value1"), cty.StringVal("value2")}) + +// Set attribute - index traversal +cty.GetAttrPath("set_attribute").Index(cty.StringVal("value2")) // returns cty.StringVal("value2") + + +// Set nested block - root traversal +cty.GetAttrPath("set_nested_block") +// returns: +// cty.SetVal([]cty.Value{ +// cty.ObjectVal(map[string]cty.Value{ +// "set_nested_block_attribute": cty.StringVal("value3"), +// }), +// cty.ObjectVal(map[string]cty.Value{ +// "set_nested_block_attribute": cty.StringVal("value4"), +// }), +// }), + +// Set nested block - index traversal +cty.GetAttrPath("set_nested_block") +.Index(cty.ObjectVal(map[string]cty.Value{"set_nested_block_attribute": cty.StringVal("value4")})) +.GetAttr("set_nested_block_attribute") // returns cty.String("value4") +``` + +### cty.Value + +When working with `cty.Value`, you must always check the type of the value before converting it to a Go value or else the conversion could cause a panic. + +```go +// Check that the type is a cty.String before conversion +if !woVal.Type().Equals(cty.String) { + return errors.New("error retrieving write-only argument: password_wo - retrieved config value is not a string") +} + +// Check if the value is not null +if !woVal.IsNull() { + // Now we can safely convert to a Go string + encryptedValue = woVal.AsString() +} +``` + +## PreferWriteOnlyAttribute Validator + +`PreferWriteOnlyAttribute()` is a validator that takes a `cty.Path` to an existing configuration attribute (required/optional) and a `cty.Path` to a write-only argument. + +Use this validator when you have a write-only version of an existing attribute, and you want to encourage practitioners to use the write-only version whenever possible. + +The validator returns a warning if the Terraform client is 1.11 or above and the value to the regular attribute is non-null. + +Usage: + +```go +func resourceDbInstance() *schema.Resource { + return &schema.Resource{ + Create: resourceCreate, + Read: resourceRead, + Delete: resourceDelete, + Importer: &schema.ResourceImporter{ + State: resourceImport, + }, + Schema: //omitted for brevity + ValidateRawResourceConfigFuncs: []schema.ValidateRawResourceConfigFunc{ + validation.PreferWriteOnlyAttribute(cty.GetAttrPath("password"), cty.GetAttrPath("password_wo")), + }, + } +} +``` + +```hcl +resource "example_db_instance" "ex" { + username = "foo" + password = "bar" # returns a warning encouraging practitioners to use `password_wo` instead. +} +``` + +When using `cty.Path` to traverse into a nested block, use an unknown value to indicate any key value: + +- For lists: `cty.Index(cty.UnknownVal(cty.Number))`, +- For maps: `cty.Index(cty.UnknownVal(cty.String))`, +- For sets: `cty.Index(cty.UnknownVal(cty.Object(nil)))`, + + +## Best Practices + +Since write-only arguments have no prior values, user intent cannot be determined with a write-only argument alone. To determine when to use/not use a write-only argument value in your provider, we recommend using other non-write-only arguments in the provider. For example: + +- Pair write-only arguments with a configuration attribute (required or optional) to “trigger” the use of the write-only argument + - For example, a `password_wo` write-only argument can be paired with a configured `password_wo_version` attribute. When the `password_wo_version` is modified, the provider will send the `password_wo` value to the API. +- Use a keepers attribute (which is used in the [Random Provider](https://registry.terraform.io/providers/hashicorp/random/latest/docs#resource-keepers)) that will take in arbitrary key-pair values. Whenever there is a change to the `keepers` attribute, the provider will use the write-only argument value. \ No newline at end of file diff --git a/website/docs/plugin/sdkv2/schemas/schema-behaviors.mdx b/website/docs/plugin/sdkv2/schemas/schema-behaviors.mdx index 7b473577ff..9e98a4abbb 100644 --- a/website/docs/plugin/sdkv2/schemas/schema-behaviors.mdx +++ b/website/docs/plugin/sdkv2/schemas/schema-behaviors.mdx @@ -197,6 +197,53 @@ resource "example_instance" "ex" { } ``` +### WriteOnly + +~> **NOTE:** Write-only arguments are only supported in Terraform `v1.11` or higher + +**Data structure:** [bool](https://pkg.go.dev/builtin#bool) + +**Value:** `true` + +**Restrictions:** + +- Cannot be used in data source or provider schemas +- Must be set with either `Required` is `true` or `Optional` is `true` +- Cannot be used when `Computed` is `true` +- Cannot be used when `ForceNew` is `true` +- Cannot be used when `Default` is `specified` +- Cannot be used with `DefaultFunc` +- Cannot be used with aggregate schema types (e.g. `typeMap`, `typeList`, `typeSet`), but non-computed nested block types can contain write-only arguments. + +`WriteOnly` should be used for arguments that handle secret values that do not need to be persisted in Terraform plan or state, +such as passwords, API keys, etc. Write-only argument values are not sent to Terraform and do not +persist in the Terraform plan or state artifacts. +Arguments marked +as `WriteOnly` can accept [ephemeral values](/terraform/language/resources/ephemeral). + +**Schema example:** + +```go +"password_wo": { + Type: schema.TypeString, + Required: true, + WriteOnly: true, +}, +``` + +**Configuration example:** + +```hcl +ephemeral "random_password" "ex_pass" { + length = 15 +} + +resource "example_db_instance" "ex" { + username = "foo" + password_wo = random_password.ex_pass.password +} +``` + ## Function Behaviors ### DiffSuppressFunc