Skip to content

Commit

Permalink
internal/fwserver: Prevent two sources of errors/panics (#475)
Browse files Browse the repository at this point in the history 
Previously, it was possible to get a `Attribute Plan Modification Walk Error` diagnostic when a resource was being created and the `req.AttributeState` was set to `nil` instead of a null value for the given `attr.Value`. This changes the logic to always return a proper `attr.Value`.

Previously, it was possible that while iterating through the plan elements for a list or set collection, that configuration or state elements at the same index may not exist. The logic was off-by-one due to Go's 0-based slice indexing to return early with a null value in those cases.
  • Loading branch information
@bflad
bflad committed Sep 12, 2022
1 parent 8ce2dcd commit ff9c66b
Show file tree
Hide file tree
Showing 4 changed files with 328 additions and 6 deletions.
18 changes: 15 additions & 3 deletions internal/fwschemadata/data_value.go
View file
Expand Up @@ -39,10 +39,22 @@ func (d Data) ValueAtPath(ctx context.Context, schemaPath path.Path) (attr.Value
return nil, diags
}

// if the whole config is nil, the value of a valid attribute is also
// nil
// if the data is null, return a null value of the type
if d.TerraformValue.IsNull() {
return nil, nil
attrValue, err := attrType.ValueFromTerraform(ctx, tftypes.NewValue(attrType.TerraformType(ctx), nil))

if err != nil {
diags.AddAttributeError(
schemaPath,
d.Description.Title()+" Read Error",
"An unexpected error was encountered trying to create a null attribute value from the given path. "+
"Please report the following to the provider developer:\n\n"+
"Type: "+attrType.String()+"\n"+
"Error:"+err.Error(),
)
}

return attrValue, diags
}

tfValue, err := d.TerraformValueAtTerraformPath(ctx, tftypesPath)
Expand Down
2 changes: 1 addition & 1 deletion internal/fwschemadata/data_value_test.go
View file
Expand Up @@ -48,7 +48,7 @@ func TestDataValueAtPath(t *testing.T) {
},
},
path: path.Root("test"),
expected: nil,
expected: types.String{Null: true},
},
"WithAttributeName-nonexistent": {
data: fwschemadata.Data{
Expand Down
4 changes: 2 additions & 2 deletions internal/fwserver/attr_value.go
View file
Expand Up @@ -68,7 +68,7 @@ func listElemObject(ctx context.Context, schemaPath path.Path, list types.List,
return listElemObjectFromTerraformValue(ctx, schemaPath, list, description, tftypes.UnknownValue)
}

if index > len(list.Elems) {
if index >= len(list.Elems) {
return listElemObjectFromTerraformValue(ctx, schemaPath, list, description, nil)
}

Expand Down Expand Up @@ -156,7 +156,7 @@ func setElemObject(ctx context.Context, schemaPath path.Path, set types.Set, ind
return setElemObjectFromTerraformValue(ctx, schemaPath, set, description, tftypes.UnknownValue)
}

if index > len(set.Elems) {
if index >= len(set.Elems) {
return setElemObjectFromTerraformValue(ctx, schemaPath, set, description, nil)
}

Expand Down
310 changes: 310 additions & 0 deletions internal/fwserver/block_plan_modification_test.go
View file
Expand Up @@ -344,6 +344,161 @@ func TestBlockModifyPlan(t *testing.T) {
Private: testProviderData,
},
},
"block-list-null-plan": {
block: tfsdk.Block{
Attributes: map[string]tfsdk.Attribute{
"nested_attr": {
Type: types.StringType,
Required: true,
PlanModifiers: tfsdk.AttributePlanModifiers{
planmodifiers.TestAttrPlanPrivateModifierGet{},
},
},
},
NestingMode: tfsdk.BlockNestingModeList,
PlanModifiers: tfsdk.AttributePlanModifiers{
planmodifiers.TestAttrPlanPrivateModifierSet{},
},
},
req: tfsdk.ModifyAttributePlanRequest{
AttributeConfig: types.List{
ElemType: types.ObjectType{
AttrTypes: map[string]attr.Type{
"nested_attr": types.StringType,
},
},
Elems: []attr.Value{
types.Object{
AttrTypes: map[string]attr.Type{
"nested_attr": types.StringType,
},
Attrs: map[string]attr.Value{
"nested_attr": types.String{Value: "testvalue"},
},
},
},
},
AttributePath: path.Root("test"),
AttributePlan: types.List{
ElemType: types.ObjectType{
AttrTypes: map[string]attr.Type{
"nested_attr": types.StringType,
},
},
Null: true,
},
AttributeState: types.List{
ElemType: types.ObjectType{
AttrTypes: map[string]attr.Type{
"nested_attr": types.StringType,
},
},
Elems: []attr.Value{
types.Object{
AttrTypes: map[string]attr.Type{
"nested_attr": types.StringType,
},
Attrs: map[string]attr.Value{
"nested_attr": types.String{Value: "testvalue"},
},
},
},
},
},
expectedResp: ModifyAttributePlanResponse{
AttributePlan: types.List{
ElemType: types.ObjectType{
AttrTypes: map[string]attr.Type{
"nested_attr": types.StringType,
},
},
Null: true,
},
Private: testProviderData,
},
},
"block-list-null-state": {
block: tfsdk.Block{
Attributes: map[string]tfsdk.Attribute{
"nested_attr": {
Type: types.StringType,
Required: true,
PlanModifiers: tfsdk.AttributePlanModifiers{
planmodifiers.TestAttrPlanPrivateModifierGet{},
},
},
},
NestingMode: tfsdk.BlockNestingModeList,
PlanModifiers: tfsdk.AttributePlanModifiers{
planmodifiers.TestAttrPlanPrivateModifierSet{},
},
},
req: tfsdk.ModifyAttributePlanRequest{
AttributeConfig: types.List{
ElemType: types.ObjectType{
AttrTypes: map[string]attr.Type{
"nested_attr": types.StringType,
},
},
Elems: []attr.Value{
types.Object{
AttrTypes: map[string]attr.Type{
"nested_attr": types.StringType,
},
Attrs: map[string]attr.Value{
"nested_attr": types.String{Value: "testvalue"},
},
},
},
},
AttributePath: path.Root("test"),
AttributePlan: types.List{
ElemType: types.ObjectType{
AttrTypes: map[string]attr.Type{
"nested_attr": types.StringType,
},
},
Elems: []attr.Value{
types.Object{
AttrTypes: map[string]attr.Type{
"nested_attr": types.StringType,
},
Attrs: map[string]attr.Value{
"nested_attr": types.String{Value: "testvalue"},
},
},
},
},
AttributeState: types.List{
ElemType: types.ObjectType{
AttrTypes: map[string]attr.Type{
"nested_attr": types.StringType,
},
},
Null: true,
},
},
expectedResp: ModifyAttributePlanResponse{
AttributePlan: types.List{
ElemType: types.ObjectType{
AttrTypes: map[string]attr.Type{
"nested_attr": types.StringType,
},
},
Elems: []attr.Value{
types.Object{
AttrTypes: map[string]attr.Type{
"nested_attr": types.StringType,
},
Attrs: map[string]attr.Value{
"nested_attr": types.String{Value: "testvalue"},
},
},
},
},
Private: testProviderData,
},
},
"block-list-nested-private": {
block: tfsdk.Block{
Attributes: map[string]tfsdk.Attribute{
Expand Down Expand Up @@ -435,6 +590,161 @@ func TestBlockModifyPlan(t *testing.T) {
Private: testProviderData,
},
},
"block-set-null-plan": {
block: tfsdk.Block{
Attributes: map[string]tfsdk.Attribute{
"nested_attr": {
Type: types.StringType,
Required: true,
PlanModifiers: tfsdk.AttributePlanModifiers{
planmodifiers.TestAttrPlanPrivateModifierGet{},
},
},
},
NestingMode: tfsdk.BlockNestingModeSet,
PlanModifiers: tfsdk.AttributePlanModifiers{
planmodifiers.TestAttrPlanPrivateModifierSet{},
},
},
req: tfsdk.ModifyAttributePlanRequest{
AttributeConfig: types.Set{
ElemType: types.ObjectType{
AttrTypes: map[string]attr.Type{
"nested_attr": types.StringType,
},
},
Elems: []attr.Value{
types.Object{
AttrTypes: map[string]attr.Type{
"nested_attr": types.StringType,
},
Attrs: map[string]attr.Value{
"nested_attr": types.String{Value: "testvalue"},
},
},
},
},
AttributePath: path.Root("test"),
AttributePlan: types.Set{
ElemType: types.ObjectType{
AttrTypes: map[string]attr.Type{
"nested_attr": types.StringType,
},
},
Null: true,
},
AttributeState: types.Set{
ElemType: types.ObjectType{
AttrTypes: map[string]attr.Type{
"nested_attr": types.StringType,
},
},
Elems: []attr.Value{
types.Object{
AttrTypes: map[string]attr.Type{
"nested_attr": types.StringType,
},
Attrs: map[string]attr.Value{
"nested_attr": types.String{Value: "testvalue"},
},
},
},
},
},
expectedResp: ModifyAttributePlanResponse{
AttributePlan: types.Set{
ElemType: types.ObjectType{
AttrTypes: map[string]attr.Type{
"nested_attr": types.StringType,
},
},
Null: true,
},
Private: testProviderData,
},
},
"block-set-null-state": {
block: tfsdk.Block{
Attributes: map[string]tfsdk.Attribute{
"nested_attr": {
Type: types.StringType,
Required: true,
PlanModifiers: tfsdk.AttributePlanModifiers{
planmodifiers.TestAttrPlanPrivateModifierGet{},
},
},
},
NestingMode: tfsdk.BlockNestingModeSet,
PlanModifiers: tfsdk.AttributePlanModifiers{
planmodifiers.TestAttrPlanPrivateModifierSet{},
},
},
req: tfsdk.ModifyAttributePlanRequest{
AttributeConfig: types.Set{
ElemType: types.ObjectType{
AttrTypes: map[string]attr.Type{
"nested_attr": types.StringType,
},
},
Elems: []attr.Value{
types.Object{
AttrTypes: map[string]attr.Type{
"nested_attr": types.StringType,
},
Attrs: map[string]attr.Value{
"nested_attr": types.String{Value: "testvalue"},
},
},
},
},
AttributePath: path.Root("test"),
AttributePlan: types.Set{
ElemType: types.ObjectType{
AttrTypes: map[string]attr.Type{
"nested_attr": types.StringType,
},
},
Elems: []attr.Value{
types.Object{
AttrTypes: map[string]attr.Type{
"nested_attr": types.StringType,
},
Attrs: map[string]attr.Value{
"nested_attr": types.String{Value: "testvalue"},
},
},
},
},
AttributeState: types.Set{
ElemType: types.ObjectType{
AttrTypes: map[string]attr.Type{
"nested_attr": types.StringType,
},
},
Null: true,
},
},
expectedResp: ModifyAttributePlanResponse{
AttributePlan: types.Set{
ElemType: types.ObjectType{
AttrTypes: map[string]attr.Type{
"nested_attr": types.StringType,
},
},
Elems: []attr.Value{
types.Object{
AttrTypes: map[string]attr.Type{
"nested_attr": types.StringType,
},
Attrs: map[string]attr.Value{
"nested_attr": types.String{Value: "testvalue"},
},
},
},
},
Private: testProviderData,
},
},
"block-set-nested-private": {
block: tfsdk.Block{
Attributes: map[string]tfsdk.Attribute{
Expand Down

0 comments on commit ff9c66b

Please sign in to comment.