Skip to content

Commit

Permalink
Refactor ToTerraformValue to return a tftypes.Value. (#231)
Browse files Browse the repository at this point in the history 
Refactor the `attr.Value.ToTerraformValue` to return a `tftypes.Value`,
which simplifies a bunch of code and makes the interface safer by
allowing it to be type checked by the compiler. It did require quite a
bit of refactoring of internal code, however.

This also simplifies checking whether an arbitrary `attr.Value` is null
or unknown considerably.

Fixes #171.

Co-authored-by: Brian Flad <bflad417@gmail.com>
  • Loading branch information
@paddycarver @bflad
paddycarver and bflad committed Dec 20, 2021
1 parent 574e52c commit 88f0847
Show file tree
Hide file tree
Showing 33 changed files with 303 additions and 290 deletions.
3 changes: 3 additions & 0 deletions .changelog/231.txt
View file
@@ -0,0 +1,3 @@
```release-note:breaking-change
The `ToTerraformValue` method of the `attr.Value` interface now returns a `tftypes.Value`, instead of an `interface{}`. Existing types need to be updated to call `tftypes.ValidateValue` and `tftypes.NewValue`, passing the value they were returning before, instead of returning the value directly.
```
6 changes: 4 additions & 2 deletions attr/value.go
View file
Expand Up @@ -2,6 +2,8 @@ package attr

import (
"context"

"github.com/hashicorp/terraform-plugin-go/tftypes"
)

// Value defines an interface for describing data associated with an attribute.
Expand All @@ -12,8 +14,8 @@ type Value interface {
Type(context.Context) Type

// ToTerraformValue returns the data contained in the Value as
// a Go type that tftypes.NewValue will accept.
ToTerraformValue(context.Context) (interface{}, error)
// a tftypes.Value.
ToTerraformValue(context.Context) (tftypes.Value, error)

// Equal must return true if the Value is considered semantically equal
// to the Value passed as an argument.
Expand Down
4 changes: 1 addition & 3 deletions internal/reflect/interfaces.go
View file
Expand Up @@ -311,14 +311,12 @@ func FromAttributeValue(ctx context.Context, typ attr.Type, val attr.Value, path
var diags diag.Diagnostics

if typeWithValidate, ok := typ.(attr.TypeWithValidate); ok {
tfType := typ.TerraformType(ctx)
tfVal, err := val.ToTerraformValue(ctx)

if err != nil {
return val, append(diags, toTerraformValueErrorDiag(err, path))
}

diags.Append(typeWithValidate.Validate(ctx, tftypes.NewValue(tfType, tfVal), path)...)
diags.Append(typeWithValidate.Validate(ctx, tfVal, path)...)

if diags.HasError() {
return val, diags
Expand Down
13 changes: 2 additions & 11 deletions internal/reflect/map.go
View file
Expand Up @@ -143,24 +143,15 @@ func FromMap(ctx context.Context, typ attr.TypeWithElementType, val reflect.Valu
return nil, append(diags, toTerraformValueErrorDiag(err, path))
}

tfElemType := elemType.TerraformType(ctx)
err = tftypes.ValidateValue(tfElemType, tfVal)

if err != nil {
return nil, append(diags, validateValueErrorDiag(err, path))
}

tfElemVal := tftypes.NewValue(tfElemType, tfVal)

if typeWithValidate, ok := typ.(attr.TypeWithValidate); ok {
diags.Append(typeWithValidate.Validate(ctx, tfElemVal, path.WithElementKeyString(key.String()))...)
diags.Append(typeWithValidate.Validate(ctx, tfVal, path.WithElementKeyString(key.String()))...)

if diags.HasError() {
return nil, diags
}
}

tfElems[key.String()] = tfElemVal
tfElems[key.String()] = tfVal
}

err := tftypes.ValidateValue(tfType, tfElems)
Expand Down
16 changes: 3 additions & 13 deletions internal/reflect/slice.go
View file
Expand Up @@ -151,32 +151,22 @@ func FromSlice(ctx context.Context, typ attr.Type, val reflect.Value, path *tfty
}

tfVal, err := val.ToTerraformValue(ctx)

if err != nil {
return nil, append(diags, toTerraformValueErrorDiag(err, path))
}

err = tftypes.ValidateValue(elemType.TerraformType(ctx), tfVal)

if err != nil {
return nil, append(diags, validateValueErrorDiag(err, path))
}

tfElemVal := tftypes.NewValue(elemType.TerraformType(ctx), tfVal)

if tfType.Is(tftypes.Set{}) {
valPath = path.WithElementKeyValue(tfElemVal)
valPath = path.WithElementKeyValue(tfVal)
}

if typeWithValidate, ok := elemType.(attr.TypeWithValidate); ok {
diags.Append(typeWithValidate.Validate(ctx, tfElemVal, valPath)...)

diags.Append(typeWithValidate.Validate(ctx, tfVal, valPath)...)
if diags.HasError() {
return nil, diags
}
}

tfElems = append(tfElems, tfElemVal)
tfElems = append(tfElems, tfVal)
}

err := tftypes.ValidateValue(tfType, tfElems)
Expand Down
8 changes: 1 addition & 7 deletions internal/reflect/struct.go
View file
Expand Up @@ -186,16 +186,10 @@ func FromStruct(ctx context.Context, typ attr.TypeWithAttributeTypes, val reflec

objTypes[name] = attrType.TerraformType(ctx)

tfVal, err := attrVal.ToTerraformValue(ctx)
tfObjVal, err := attrVal.ToTerraformValue(ctx)
if err != nil {
return nil, append(diags, toTerraformValueErrorDiag(err, path))
}
err = tftypes.ValidateValue(objTypes[name], tfVal)
if err != nil {
return nil, append(diags, validateValueErrorDiag(err, path))
}

tfObjVal := tftypes.NewValue(objTypes[name], tfVal)

if typeWithValidate, ok := typ.(attr.TypeWithValidate); ok {
diags.Append(typeWithValidate.Validate(ctx, tfObjVal, path)...)
Expand Down
13 changes: 3 additions & 10 deletions tfsdk/attribute.go
View file
Expand Up @@ -305,7 +305,6 @@ func (a Attribute) validate(ctx context.Context, req ValidateAttributeRequest, r

if a.DeprecationMessage != "" && attributeConfig != nil {
tfValue, err := attributeConfig.ToTerraformValue(ctx)

if err != nil {
resp.Diagnostics.AddAttributeError(
req.AttributePath,
Expand All @@ -316,7 +315,7 @@ func (a Attribute) validate(ctx context.Context, req ValidateAttributeRequest, r
return
}

if tfValue != nil {
if !tfValue.IsNull() {
resp.Diagnostics.AddAttributeWarning(
req.AttributePath,
"Attribute Deprecated",
Expand Down Expand Up @@ -378,8 +377,7 @@ func (a Attribute) validateAttributes(ctx context.Context, req ValidateAttribute
}

for _, value := range s.Elems {
tfValueRaw, err := value.ToTerraformValue(ctx)

tfValue, err := value.ToTerraformValue(ctx)
if err != nil {
err := fmt.Errorf("error running ToTerraformValue on element value: %v", value)
resp.Diagnostics.AddAttributeError(
Expand All @@ -391,8 +389,6 @@ func (a Attribute) validateAttributes(ctx context.Context, req ValidateAttribute
return
}

tfValue := tftypes.NewValue(s.ElemType.TerraformType(ctx), tfValueRaw)

for nestedName, nestedAttr := range a.Attributes.GetAttributes() {
nestedAttrReq := ValidateAttributeRequest{
AttributePath: req.AttributePath.WithElementKeyValue(tfValue).WithAttributeName(nestedName),
Expand Down Expand Up @@ -584,8 +580,7 @@ func (a Attribute) modifyPlan(ctx context.Context, req ModifyAttributePlanReques
}

for _, value := range s.Elems {
tfValueRaw, err := value.ToTerraformValue(ctx)

tfValue, err := value.ToTerraformValue(ctx)
if err != nil {
err := fmt.Errorf("error running ToTerraformValue on element value: %v", value)
resp.Diagnostics.AddAttributeError(
Expand All @@ -597,8 +592,6 @@ func (a Attribute) modifyPlan(ctx context.Context, req ModifyAttributePlanReques
return
}

tfValue := tftypes.NewValue(s.ElemType.TerraformType(ctx), tfValueRaw)

for name, attr := range a.Attributes.GetAttributes() {
attrReq := ModifyAttributePlanRequest{
AttributePath: req.AttributePath.WithElementKeyValue(tfValue).WithAttributeName(name),
Expand Down
10 changes: 5 additions & 5 deletions tfsdk/attribute_plan_modification.go
View file
Expand Up @@ -147,7 +147,7 @@ func (r RequiresReplaceModifier) Modify(ctx context.Context, req ModifyAttribute
)
return
}
if configRaw == nil && attrSchema.Computed {
if configRaw.IsNull() && attrSchema.Computed {
// if the config is null and the attribute is computed, this
// could be an out of band change, don't require replace
return
Expand Down Expand Up @@ -287,7 +287,7 @@ func (r RequiresReplaceIfModifier) Modify(ctx context.Context, req ModifyAttribu
)
return
}
if configRaw == nil && attrSchema.Computed {
if configRaw.IsNull() && attrSchema.Computed {
// if the config is null and the attribute is computed, this
// could be an out of band change, don't require replace
return
Expand Down Expand Up @@ -354,7 +354,7 @@ func (r UseStateForUnknownModifier) Modify(ctx context.Context, req ModifyAttrib
}

// if we have no state value, there's nothing to preserve
if val == nil {
if val.IsNull() {
return
}

Expand All @@ -369,7 +369,7 @@ func (r UseStateForUnknownModifier) Modify(ctx context.Context, req ModifyAttrib

// if it's not planned to be the unknown value, stick with
// the concrete plan
if val != tftypes.UnknownValue {
if val.IsKnown() {
return
}

Expand All @@ -384,7 +384,7 @@ func (r UseStateForUnknownModifier) Modify(ctx context.Context, req ModifyAttrib

// if the config is the unknown value, use the unknown value
// otherwise, interpolation gets messed up
if val == tftypes.UnknownValue {
if !val.IsKnown() {
return
}

Expand Down
13 changes: 6 additions & 7 deletions tfsdk/attribute_plan_modification_test.go
View file
Expand Up @@ -95,31 +95,30 @@ func TestUseStateForUnknownModifier(t *testing.T) {
},
}

var configRaw, planRaw, stateRaw interface{}
configVal := tftypes.NewValue(tftypes.String, nil)
stateVal := tftypes.NewValue(tftypes.String, nil)
planVal := tftypes.NewValue(tftypes.String, nil)
if tc.config != nil {
val, err := tc.config.ToTerraformValue(context.Background())
if err != nil {
t.Fatal(err)
}
configRaw = val
configVal = val
}
if tc.state != nil {
val, err := tc.state.ToTerraformValue(context.Background())
if err != nil {
t.Fatal(err)
}
stateRaw = val
stateVal = val
}
if tc.plan != nil {
val, err := tc.plan.ToTerraformValue(context.Background())
if err != nil {
t.Fatal(err)
}
planRaw = val
planVal = val
}
configVal := tftypes.NewValue(tftypes.String, configRaw)
stateVal := tftypes.NewValue(tftypes.String, stateRaw)
planVal := tftypes.NewValue(tftypes.String, planRaw)

req := ModifyAttributePlanRequest{
AttributePath: tftypes.NewAttributePath(),
Expand Down
12 changes: 3 additions & 9 deletions tfsdk/block.go
View file
Expand Up @@ -269,8 +269,7 @@ func (b Block) modifyPlan(ctx context.Context, req ModifyAttributePlanRequest, r
}

for _, value := range s.Elems {
tfValueRaw, err := value.ToTerraformValue(ctx)

tfValue, err := value.ToTerraformValue(ctx)
if err != nil {
err := fmt.Errorf("error running ToTerraformValue on element value: %v", value)
resp.Diagnostics.AddAttributeError(
Expand All @@ -282,8 +281,6 @@ func (b Block) modifyPlan(ctx context.Context, req ModifyAttributePlanRequest, r
return
}

tfValue := tftypes.NewValue(s.ElemType.TerraformType(ctx), tfValueRaw)

for name, attr := range b.Attributes {
attrReq := ModifyAttributePlanRequest{
AttributePath: req.AttributePath.WithElementKeyValue(tfValue).WithAttributeName(name),
Expand Down Expand Up @@ -482,8 +479,7 @@ func (b Block) validate(ctx context.Context, req ValidateAttributeRequest, resp
}

for _, value := range s.Elems {
tfValueRaw, err := value.ToTerraformValue(ctx)

tfValue, err := value.ToTerraformValue(ctx)
if err != nil {
err := fmt.Errorf("error running ToTerraformValue on element value: %v", value)
resp.Diagnostics.AddAttributeError(
Expand All @@ -495,8 +491,6 @@ func (b Block) validate(ctx context.Context, req ValidateAttributeRequest, resp
return
}

tfValue := tftypes.NewValue(s.ElemType.TerraformType(ctx), tfValueRaw)

for name, attr := range b.Attributes {
nestedAttrReq := ValidateAttributeRequest{
AttributePath: req.AttributePath.WithElementKeyValue(tfValue).WithAttributeName(name),
Expand Down Expand Up @@ -549,7 +543,7 @@ func (b Block) validate(ctx context.Context, req ValidateAttributeRequest, resp
return
}

if tfValue != nil {
if !tfValue.IsNull() {
resp.Diagnostics.AddAttributeWarning(
req.AttributePath,
"Block Deprecated",
Expand Down
11 changes: 1 addition & 10 deletions tfsdk/convert.go
View file
Expand Up @@ -6,27 +6,18 @@ import (

"github.com/hashicorp/terraform-plugin-framework/attr"
"github.com/hashicorp/terraform-plugin-framework/diag"
"github.com/hashicorp/terraform-plugin-go/tftypes"
)

// ConvertValue creates a new attr.Value of the attr.Type `typ`, using the data
// in `val`, which can be of any attr.Type so long as its TerraformType method
// returns a tftypes.Type that `typ`'s ValueFromTerraform method can accept.
func ConvertValue(ctx context.Context, val attr.Value, typ attr.Type) (attr.Value, diag.Diagnostics) {
tftype := typ.TerraformType(ctx)
tfval, err := val.ToTerraformValue(ctx)
newVal, err := val.ToTerraformValue(ctx)
if err != nil {
return nil, diag.Diagnostics{diag.NewErrorDiagnostic("Error converting value",
fmt.Sprintf("An unexpected error was encountered converting a %T to a %s. This is always a problem with the provider. Please tell the provider developers that %T ran into the following error during ToTerraformValue: %s", val, typ, val, err),
)}
}
err = tftypes.ValidateValue(tftype, tfval)
if err != nil {
return nil, diag.Diagnostics{diag.NewErrorDiagnostic("Error converting value",
fmt.Sprintf("An unexpected error was encountered converting a %T to a %s. This is always a problem with the provider. Please tell the provider developers that %T is not compatible with %s.", val, typ, val, typ),
)}
}
newVal := tftypes.NewValue(tftype, tfval)
res, err := typ.ValueFromTerraform(ctx, newVal)
if err != nil {
return nil, diag.Diagnostics{diag.NewErrorDiagnostic("Error converting value",
Expand Down
5 changes: 4 additions & 1 deletion tfsdk/convert_test.go
View file
Expand Up @@ -43,7 +43,7 @@ func TestConvert(t *testing.T) {
typ: types.NumberType,
expectedDiags: diag.Diagnostics{diag.NewErrorDiagnostic(
"Error converting value",
"An unexpected error was encountered converting a types.String to a types.NumberType. This is always a problem with the provider. Please tell the provider developers that types.String is not compatible with types.NumberType.",
"An unexpected error was encountered converting a types.String to a types.NumberType. This is always a problem with the provider. Please tell the provider developers that types.NumberType returned the following error when calling ValueFromTerraform: can't unmarshal tftypes.String into *big.Float, expected *big.Float",
)},
},
}
Expand All @@ -56,6 +56,9 @@ func TestConvert(t *testing.T) {
got, diags := ConvertValue(context.Background(), tc.val, tc.typ)

if diff := cmp.Diff(diags, tc.expectedDiags); diff != "" {
for _, diag := range diags {
t.Logf("Diag summary: %q, Diag details: %q", diag.Summary(), diag.Detail())
}
t.Fatalf("Unexpected diff in diags (-wanted, +got): %s", diff)
}

Expand Down
8 changes: 2 additions & 6 deletions tfsdk/plan.go
View file
Expand Up @@ -122,7 +122,7 @@ func (p *Plan) Set(ctx context.Context, val interface{}) diag.Diagnostics {
return diags
}

newPlanVal, err := newPlanAttrValue.ToTerraformValue(ctx)
newPlan, err := newPlanAttrValue.ToTerraformValue(ctx)
if err != nil {
err = fmt.Errorf("error running ToTerraformValue on plan: %w", err)
diags.AddError(
Expand All @@ -132,8 +132,6 @@ func (p *Plan) Set(ctx context.Context, val interface{}) diag.Diagnostics {
return diags
}

newPlan := tftypes.NewValue(p.Schema.AttributeType().TerraformType(ctx), newPlanVal)

p.Raw = newPlan
return diags
}
Expand Down Expand Up @@ -167,7 +165,7 @@ func (p *Plan) SetAttribute(ctx context.Context, path *tftypes.AttributePath, va
return diags
}

newTfVal, err := newVal.ToTerraformValue(ctx)
tfVal, err := newVal.ToTerraformValue(ctx)
if err != nil {
err = fmt.Errorf("error running ToTerraformValue on new plan value: %w", err)
diags.AddAttributeError(
Expand All @@ -178,8 +176,6 @@ func (p *Plan) SetAttribute(ctx context.Context, path *tftypes.AttributePath, va
return diags
}

tfVal := tftypes.NewValue(attrType.TerraformType(ctx), newTfVal)

if attrTypeWithValidate, ok := attrType.(attr.TypeWithValidate); ok {
diags.Append(attrTypeWithValidate.Validate(ctx, tfVal, path)...)

Expand Down

0 comments on commit 88f0847

Please sign in to comment.