Skip to content

tfsdk: Update Config, Plan, and State type GetAttribute methods to use reflection #167

New issue

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

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

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Oct 28, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changelog/167.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
```release-note:breaking-change
tfsdk: The `Config`, `Plan`, and `State` type `GetAttribute` methods now return diagnostics only and require the target as the last parameter, similar to the `Get` method.
```

```release-note:enhancement
tfsdk: The `Config`, `Plan`, and `State` type `GetAttribute` methods can now be used to fetch values directly into `attr.Value` implementations and Go types.
```
9 changes: 4 additions & 5 deletions tfsdk/attribute.go
Original file line number Diff line number Diff line change
Expand Up @@ -262,8 +262,7 @@ func (a Attribute) validate(ctx context.Context, req ValidateAttributeRequest, r
return
}

attributeConfig, diags := req.Config.GetAttribute(ctx, req.AttributePath)

attributeConfig, diags := req.Config.getAttributeValue(ctx, req.AttributePath)
resp.Diagnostics.Append(diags...)

if diags.HasError() {
Expand Down Expand Up @@ -447,23 +446,23 @@ func (a Attribute) validate(ctx context.Context, req ValidateAttributeRequest, r

// modifyPlan runs all AttributePlanModifiers
func (a Attribute) modifyPlan(ctx context.Context, req ModifyAttributePlanRequest, resp *ModifyAttributePlanResponse) {
attrConfig, diags := req.Config.GetAttribute(ctx, req.AttributePath)
attrConfig, diags := req.Config.getAttributeValue(ctx, req.AttributePath)
resp.Diagnostics.Append(diags...)
// Only on new errors.
if diags.HasError() {
return
}
req.AttributeConfig = attrConfig

attrState, diags := req.State.GetAttribute(ctx, req.AttributePath)
attrState, diags := req.State.getAttributeValue(ctx, req.AttributePath)
resp.Diagnostics.Append(diags...)
// Only on new errors.
if diags.HasError() {
return
}
req.AttributeState = attrState

attrPlan, diags := req.Plan.GetAttribute(ctx, req.AttributePath)
attrPlan, diags := req.Plan.getAttributeValue(ctx, req.AttributePath)
resp.Diagnostics.Append(diags...)
// Only on new errors.
if diags.HasError() {
Expand Down
49 changes: 47 additions & 2 deletions tfsdk/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,55 @@ func (c Config) Get(ctx context.Context, target interface{}) diag.Diagnostics {
return reflect.Into(ctx, c.Schema.AttributeType(), c.Raw, target, reflect.Options{})
}

// GetAttribute retrieves the attribute found at `path` and returns it as an
// GetAttribute retrieves the attribute found at `path` and populates the
// `target` with the value.
func (c Config) GetAttribute(ctx context.Context, path *tftypes.AttributePath, target interface{}) diag.Diagnostics {
attrValue, diags := c.getAttributeValue(ctx, path)

if diags.HasError() {
return diags
}

if attrValue == nil {
diags.AddAttributeError(
path,
"Config Read Error",
"An unexpected error was encountered trying to read an attribute from the configuration. This is always an error in the provider. Please report the following to the provider developer:\n\n"+
"Missing attribute value, however no error was returned. Preventing the panic from this situation.",
)
return diags
}

valueAsDiags := ValueAs(ctx, attrValue, target)

// ValueAs does not have path information for its Diagnostics.
// TODO: Update to use diagnostic SetPath method.
// Reference: https://github.com/hashicorp/terraform-plugin-framework/issues/169
for idx, valueAsDiag := range valueAsDiags {
if valueAsDiag.Severity() == diag.SeverityError {
valueAsDiags[idx] = diag.NewAttributeErrorDiagnostic(
path,
valueAsDiag.Summary(),
valueAsDiag.Detail(),
)
} else if valueAsDiag.Severity() == diag.SeverityWarning {
valueAsDiags[idx] = diag.NewAttributeWarningDiagnostic(
path,
valueAsDiag.Summary(),
valueAsDiag.Detail(),
)
}
Comment on lines +49 to +62
Copy link
Contributor

Choose a reason for hiding this comment

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

Eek. Can we add an issue at least to have diag.Diagnostic have a SetPath method to avoid this kind of stuff?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I created #169, but it might require some refactoring to the diag package.

}

diags.Append(valueAsDiags...)

return diags
}

// getAttributeValue retrieves the attribute found at `path` and returns it as an
// attr.Value. Consumers should assert the type of the returned value with the
// desired attr.Type.
func (c Config) GetAttribute(ctx context.Context, path *tftypes.AttributePath) (attr.Value, diag.Diagnostics) {
func (c Config) getAttributeValue(ctx context.Context, path *tftypes.AttributePath) (attr.Value, diag.Diagnostics) {
var diags diag.Diagnostics

attrType, err := c.Schema.AttributeTypeAtPath(path)
Expand Down
210 changes: 209 additions & 1 deletion tfsdk/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,14 @@ package tfsdk

import (
"context"
"fmt"
"reflect"
"testing"

"github.com/google/go-cmp/cmp"
"github.com/hashicorp/terraform-plugin-framework/attr"
"github.com/hashicorp/terraform-plugin-framework/diag"
intreflect "github.com/hashicorp/terraform-plugin-framework/internal/reflect"
testtypes "github.com/hashicorp/terraform-plugin-framework/internal/testing/types"
"github.com/hashicorp/terraform-plugin-framework/types"
"github.com/hashicorp/terraform-plugin-go/tftypes"
Expand Down Expand Up @@ -155,6 +158,210 @@ func TestConfigGet_testTypes(t *testing.T) {
func TestConfigGetAttribute(t *testing.T) {
t.Parallel()

type testCase struct {
config Config
target interface{}
expected interface{}
expectedDiags diag.Diagnostics
}

testCases := map[string]testCase{
"string": {
config: Config{
Raw: tftypes.NewValue(tftypes.Object{
AttributeTypes: map[string]tftypes.Type{
"name": tftypes.String,
},
}, map[string]tftypes.Value{
"name": tftypes.NewValue(tftypes.String, "namevalue"),
}),
Schema: Schema{
Attributes: map[string]Attribute{
"name": {
Type: types.StringType,
Required: true,
},
},
},
},
target: new(string),
expected: newStringPointer("namevalue"),
},
"*string": {
config: Config{
Raw: tftypes.NewValue(tftypes.Object{
AttributeTypes: map[string]tftypes.Type{
"name": tftypes.String,
},
}, map[string]tftypes.Value{
"name": tftypes.NewValue(tftypes.String, "namevalue"),
}),
Schema: Schema{
Attributes: map[string]Attribute{
"name": {
Type: types.StringType,
Required: true,
},
},
},
},
target: new(*string),
expected: newStringPointerPointer("namevalue"),
},
"types.String": {
config: Config{
Raw: tftypes.NewValue(tftypes.Object{
AttributeTypes: map[string]tftypes.Type{
"name": tftypes.String,
},
}, map[string]tftypes.Value{
"name": tftypes.NewValue(tftypes.String, "namevalue"),
}),
Schema: Schema{
Attributes: map[string]Attribute{
"name": {
Type: types.StringType,
Required: true,
},
},
},
},
target: new(types.String),
expected: &types.String{Value: "namevalue"},
},
"incompatible-target": {
config: Config{
Raw: tftypes.NewValue(tftypes.Object{
AttributeTypes: map[string]tftypes.Type{
"name": tftypes.String,
},
}, map[string]tftypes.Value{
"name": tftypes.NewValue(tftypes.String, "namevalue"),
}),
Schema: Schema{
Attributes: map[string]Attribute{
"name": {
Type: types.StringType,
Required: true,
},
},
},
},
target: new(testtypes.String),
expected: new(testtypes.String),
expectedDiags: diag.Diagnostics{
diag.NewAttributeErrorDiagnostic(
tftypes.NewAttributePath().WithAttributeName("name"),
"Value Conversion Error",
intreflect.DiagNewAttributeValueIntoWrongType{
ValType: reflect.TypeOf(types.String{Value: "namevalue"}),
TargetType: reflect.TypeOf(testtypes.String{}),
AttrPath: tftypes.NewAttributePath().WithAttributeName("name"),
SchemaType: types.StringType,
}.Detail(),
),
},
},
"incompatible-type": {
config: Config{
Raw: tftypes.NewValue(tftypes.Object{
AttributeTypes: map[string]tftypes.Type{
"name": tftypes.String,
},
}, map[string]tftypes.Value{
"name": tftypes.NewValue(tftypes.String, "namevalue"),
}),
Schema: Schema{
Attributes: map[string]Attribute{
"name": {
Type: types.StringType,
Required: true,
},
},
},
},
target: new(bool),
expected: new(bool),
expectedDiags: diag.Diagnostics{
diag.NewAttributeErrorDiagnostic(
tftypes.NewAttributePath().WithAttributeName("name"),
"Value Conversion Error",
intreflect.DiagIntoIncompatibleType{
Val: tftypes.NewValue(tftypes.String, "namevalue"),
TargetType: reflect.TypeOf(false),
Err: fmt.Errorf("can't unmarshal %s into *%T, expected boolean", tftypes.String, false),
AttrPath: tftypes.NewAttributePath().WithAttributeName("name"),
}.Detail(),
),
},
},
"AttrTypeWithValidateError": {
config: Config{
Raw: tftypes.NewValue(tftypes.Object{
AttributeTypes: map[string]tftypes.Type{
"name": tftypes.String,
},
}, map[string]tftypes.Value{
"name": tftypes.NewValue(tftypes.String, "namevalue"),
}),
Schema: Schema{
Attributes: map[string]Attribute{
"name": {
Type: testtypes.StringTypeWithValidateError{},
Required: true,
},
},
},
},
target: new(testtypes.String),
expected: new(testtypes.String),
expectedDiags: diag.Diagnostics{testtypes.TestErrorDiagnostic(tftypes.NewAttributePath().WithAttributeName("name"))},
},
"AttrTypeWithValidateWarning": {
config: Config{
Raw: tftypes.NewValue(tftypes.Object{
AttributeTypes: map[string]tftypes.Type{
"name": tftypes.String,
},
}, map[string]tftypes.Value{
"name": tftypes.NewValue(tftypes.String, "namevalue"),
}),
Schema: Schema{
Attributes: map[string]Attribute{
"name": {
Type: testtypes.StringTypeWithValidateWarning{},
Required: true,
},
},
},
},
target: new(testtypes.String),
expected: &testtypes.String{String: types.String{Value: "namevalue"}, CreatedBy: testtypes.StringTypeWithValidateWarning{}},
expectedDiags: diag.Diagnostics{testtypes.TestWarningDiagnostic(tftypes.NewAttributePath().WithAttributeName("name"))},
},
}

for name, tc := range testCases {
name, tc := name, tc
t.Run(name, func(t *testing.T) {
t.Parallel()

diags := tc.config.GetAttribute(context.Background(), tftypes.NewAttributePath().WithAttributeName("name"), tc.target)

if diff := cmp.Diff(diags, tc.expectedDiags); diff != "" {
t.Errorf("unexpected diagnostics (+wanted, -got): %s", diff)
}

if diff := cmp.Diff(tc.target, tc.expected, cmp.Transformer("testtypes", func(in *testtypes.String) testtypes.String { return *in }), cmp.Transformer("types", func(in *types.String) types.String { return *in })); diff != "" {
t.Errorf("unexpected value (+wanted, -got): %s", diff)
}
})
}
}

func TestConfigGetAttributeValue(t *testing.T) {
t.Parallel()

type testCase struct {
config Config
path *tftypes.AttributePath
Expand Down Expand Up @@ -1025,7 +1232,8 @@ func TestConfigGetAttribute(t *testing.T) {
t.Run(name, func(t *testing.T) {
t.Parallel()

val, diags := tc.config.GetAttribute(context.Background(), tc.path)
val, diags := tc.config.getAttributeValue(context.Background(), tc.path)

if diff := cmp.Diff(diags, tc.expectedDiags); diff != "" {
t.Errorf("unexpected diagnostics (+wanted, -got): %s", diff)
}
Expand Down
49 changes: 47 additions & 2 deletions tfsdk/plan.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,55 @@ func (p Plan) Get(ctx context.Context, target interface{}) diag.Diagnostics {
return reflect.Into(ctx, p.Schema.AttributeType(), p.Raw, target, reflect.Options{})
}

// GetAttribute retrieves the attribute found at `path` and returns it as an
// GetAttribute retrieves the attribute found at `path` and populates the
// `target` with the value.
func (p Plan) GetAttribute(ctx context.Context, path *tftypes.AttributePath, target interface{}) diag.Diagnostics {
attrValue, diags := p.getAttributeValue(ctx, path)

if diags.HasError() {
return diags
}

if attrValue == nil {
diags.AddAttributeError(
path,
"Plan Read Error",
"An unexpected error was encountered trying to read an attribute from the plan. This is always an error in the provider. Please report the following to the provider developer:\n\n"+
"Missing attribute value, however no error was returned. Preventing the panic from this situation.",
)
return diags
}

valueAsDiags := ValueAs(ctx, attrValue, target)

// ValueAs does not have path information for its Diagnostics.
// TODO: Update to use diagnostic SetPath method.
// Reference: https://github.com/hashicorp/terraform-plugin-framework/issues/169
for idx, valueAsDiag := range valueAsDiags {
if valueAsDiag.Severity() == diag.SeverityError {
valueAsDiags[idx] = diag.NewAttributeErrorDiagnostic(
path,
valueAsDiag.Summary(),
valueAsDiag.Detail(),
)
} else if valueAsDiag.Severity() == diag.SeverityWarning {
valueAsDiags[idx] = diag.NewAttributeWarningDiagnostic(
path,
valueAsDiag.Summary(),
valueAsDiag.Detail(),
)
}
}

diags.Append(valueAsDiags...)

return diags
}

// getAttributeValue retrieves the attribute found at `path` and returns it as an
// attr.Value. Consumers should assert the type of the returned value with the
// desired attr.Type.
func (p Plan) GetAttribute(ctx context.Context, path *tftypes.AttributePath) (attr.Value, diag.Diagnostics) {
func (p Plan) getAttributeValue(ctx context.Context, path *tftypes.AttributePath) (attr.Value, diag.Diagnostics) {
var diags diag.Diagnostics

attrType, err := p.Schema.AttributeTypeAtPath(path)
Expand Down
Loading