Skip to content

Commit

Permalink
[Internal] Add ConvertToAttribute() to convert blocks in a resource/d…
Browse files Browse the repository at this point in the history
…ata source schema to attributes (#4284)

## Changes
Blocks in the Plugin Framework cannot be computed. Terraform checks that
the number of blocks in the plan matches the number in the config
(https://github.com/hashicorp/terraform/blob/81441564dd29b4aa5fb9576323ef90b9787d1967/internal/plans/objchange/plan_valid.go#L115).
Today, for compatibility with existing resources in the TF provider
implemented with the SDKv2, we treat nested structures as lists with a
single element. As a result, this precludes any nested structures from
being read-only (since those would be converted into ListNestedBlocks,
which cannot be computed). This blocks databricks_app from being
implemented on the plugin framework, as its app_status and
compute_status fields
are objects and not primitives. Note that TF recommends computing
computed-only blocks to attributes
[here](https://developer.hashicorp.com/terraform/plugin/framework/migrating/attributes-blocks/blocks-computed).
Other useful discussions can be found
[here](https://discuss.hashicorp.com/t/set-list-attribute-migration-from-sdkv2-to-framework/56472)
and
[here](https://discuss.hashicorp.com/t/optional-computed-block-handling-in-plugin-framework/56337).
The migration guide for blocks can be found
[here](https://developer.hashicorp.com/terraform/plugin/framework/migrating/attributes-blocks/blocks).

As an intermediate step, we add `ConvertToAttribute()` in
`CustomizableSchema`. This allows resource maintainers to convert
specific, known read-only blocks to attributes in accordance with [the
migration
guide](https://developer.hashicorp.com/terraform/plugin/framework/migrating/attributes-blocks/blocks-computed),
which then can be marked as read-only.

As blocks can contain other blocks, this method also recursively
converts nested blocks into nested attributes.

This is based on
#4283,
so we should wait for that PR to merge & rebase this before merging
this.

## Tests
Unit tests validate that ListNestedBlocks are converted to
ListNestedAttributes and that SingleNestedBlocks are converted to
SingleNestedAttributes.
  • Loading branch information
mgyucht authored Dec 5, 2024
1 parent 76d2659 commit cc49ffd
Show file tree
Hide file tree
Showing 8 changed files with 329 additions and 5 deletions.
6 changes: 6 additions & 0 deletions internal/providers/pluginfw/tfschema/attribute_converter.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package tfschema

type BlockToAttributeConverter interface {
// ConvertBlockToAttribute converts a contained block to its corresponding attribute type.
ConvertBlockToAttribute(string) BaseSchemaBuilder
}
6 changes: 6 additions & 0 deletions internal/providers/pluginfw/tfschema/block_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,12 @@ import (
// This common interface prevents us from keeping two copies of StructToSchema and CustomizableSchema.
type BlockBuilder interface {
BaseSchemaBuilder

// ToAttribute converts a block to its corresponding attribute type. Currently, ResourceStructToSchema converts all
// nested struct fields and slices to blocks. This method is used to convert those blocks to their corresponding
// attribute type. The resulting attribute will not have any of the Computed/Optional/Required/Sensitive flags set.
ToAttribute() AttributeBuilder

BuildDataSourceBlock() dataschema.Block
BuildResourceBlock() schema.Block
}
Expand Down
30 changes: 30 additions & 0 deletions internal/providers/pluginfw/tfschema/customizable_schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
)

// CustomizableSchema is a wrapper struct on top of BaseSchemaBuilder that can be used to navigate through nested schema add customizations.
// The methods of CustomizableSchema that modify the underlying schema should return the same CustomizableSchema object to allow chaining.
type CustomizableSchema struct {
attr BaseSchemaBuilder
}
Expand Down Expand Up @@ -199,6 +200,35 @@ func (s *CustomizableSchema) SetReadOnly(path ...string) *CustomizableSchema {
return s
}

// ConvertToAttribute converts the last element of the path from a block to an attribute.
// It panics if the path is empty, if the path does not exist in the schema, or if the path
// points to an attribute, not a block.
func (s *CustomizableSchema) ConvertToAttribute(path ...string) *CustomizableSchema {
if len(path) == 0 {
panic(fmt.Errorf("ConvertToAttribute called on root schema. %s", common.TerraformBugErrorMessage))
}
field := path[len(path)-1]

cb := func(attr BaseSchemaBuilder) BaseSchemaBuilder {
switch a := attr.(type) {
case BlockToAttributeConverter:
return a.ConvertBlockToAttribute(field)
default:
panic(fmt.Errorf("ConvertToAttribute called on invalid attribute type: %s. %s", reflect.TypeOf(attr).String(), common.TerraformBugErrorMessage))
}
}

// We have to go only as far as the second-to-last entry, since we need to change the parent schema
// by moving the last entry from a block to an attribute.
if len(path) == 1 {
s.attr = cb(s.attr)
} else {
navigateSchemaWithCallback(&s.attr, cb, path[0:len(path)-1]...)
}

return s
}

// navigateSchemaWithCallback navigates through schema attributes and executes callback on the target, panics if path does not exist or invalid.
func navigateSchemaWithCallback(s *BaseSchemaBuilder, cb func(BaseSchemaBuilder) BaseSchemaBuilder, path ...string) (BaseSchemaBuilder, error) {
currentScm := s
Expand Down
217 changes: 217 additions & 0 deletions internal/providers/pluginfw/tfschema/customizable_schema_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"testing"

"github.com/hashicorp/terraform-plugin-framework/resource/schema"
"github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier"
"github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier"
"github.com/hashicorp/terraform-plugin-framework/schema/validator"
"github.com/hashicorp/terraform-plugin-framework/types"
Expand Down Expand Up @@ -175,3 +176,219 @@ func TestCustomizeSchema_SetComputed_PanicOnBlock(t *testing.T) {
})
})
}

type mockPlanModifier struct{}

// Description implements planmodifier.List.
func (m mockPlanModifier) Description(context.Context) string {
panic("unimplemented")
}

// MarkdownDescription implements planmodifier.List.
func (m mockPlanModifier) MarkdownDescription(context.Context) string {
panic("unimplemented")
}

// PlanModifyList implements planmodifier.List.
func (m mockPlanModifier) PlanModifyList(context.Context, planmodifier.ListRequest, *planmodifier.ListResponse) {
panic("unimplemented")
}

// PlanModifyList implements planmodifier.List.
func (m mockPlanModifier) PlanModifyObject(context.Context, planmodifier.ObjectRequest, *planmodifier.ObjectResponse) {
panic("unimplemented")
}

var _ planmodifier.List = mockPlanModifier{}
var _ planmodifier.Object = mockPlanModifier{}

type mockValidator struct{}

// Description implements validator.List.
func (m mockValidator) Description(context.Context) string {
panic("unimplemented")
}

// MarkdownDescription implements validator.List.
func (m mockValidator) MarkdownDescription(context.Context) string {
panic("unimplemented")
}

// ValidateList implements validator.List.
func (m mockValidator) ValidateList(context.Context, validator.ListRequest, *validator.ListResponse) {
panic("unimplemented")
}

// ValidateList implements validator.Object.
func (m mockValidator) ValidateObject(context.Context, validator.ObjectRequest, *validator.ObjectResponse) {
panic("unimplemented")
}

var _ validator.List = mockValidator{}
var _ validator.Object = mockValidator{}

func TestCustomizeSchema_ConvertToAttribute(t *testing.T) {
v := mockValidator{}
pm := mockPlanModifier{}
testCases := []struct {
name string
baseSchema NestedBlockObject
path []string
want NestedBlockObject
expectPanic bool
}{
{
name: "ListNestedBlock",
baseSchema: NestedBlockObject{
Blocks: map[string]BlockBuilder{
"list": ListNestedBlockBuilder{
NestedObject: NestedBlockObject{
Attributes: map[string]AttributeBuilder{
"attr": StringAttributeBuilder{},
},
},
DeprecationMessage: "deprecated",
Validators: []validator.List{v},
PlanModifiers: []planmodifier.List{pm},
},
},
},
path: []string{"list"},
want: NestedBlockObject{
Attributes: map[string]AttributeBuilder{
"list": ListNestedAttributeBuilder{
NestedObject: NestedAttributeObject{
Attributes: map[string]AttributeBuilder{
"attr": StringAttributeBuilder{},
},
},
DeprecationMessage: "deprecated",
Validators: []validator.List{v},
PlanModifiers: []planmodifier.List{pm},
},
},
},
},
{
name: "ListNestedBlock/CalledOnInnerBlock",
baseSchema: NestedBlockObject{
Blocks: map[string]BlockBuilder{
"list": ListNestedBlockBuilder{
NestedObject: NestedBlockObject{
Blocks: map[string]BlockBuilder{
"nested_block": ListNestedBlockBuilder{
NestedObject: NestedBlockObject{
Attributes: map[string]AttributeBuilder{
"attr": StringAttributeBuilder{},
},
},
},
},
},
},
},
},
path: []string{"list", "nested_block"},
want: NestedBlockObject{
Blocks: map[string]BlockBuilder{
"list": ListNestedBlockBuilder{
NestedObject: NestedBlockObject{
Attributes: map[string]AttributeBuilder{
"nested_block": ListNestedAttributeBuilder{
NestedObject: NestedAttributeObject{
Attributes: map[string]AttributeBuilder{
"attr": StringAttributeBuilder{},
},
},
},
},
},
},
},
},
},
{
name: "SingleNestedBlock",
baseSchema: NestedBlockObject{
Blocks: map[string]BlockBuilder{
"single": SingleNestedBlockBuilder{
NestedObject: NestedBlockObject{
Attributes: map[string]AttributeBuilder{
"attr": StringAttributeBuilder{},
},
},
DeprecationMessage: "deprecated",
Validators: []validator.Object{v},
PlanModifiers: []planmodifier.Object{pm},
},
},
},
path: []string{"single"},
want: NestedBlockObject{
Attributes: map[string]AttributeBuilder{
"single": SingleNestedAttributeBuilder{
Attributes: map[string]AttributeBuilder{
"attr": StringAttributeBuilder{},
},
DeprecationMessage: "deprecated",
Validators: []validator.Object{v},
PlanModifiers: []planmodifier.Object{pm},
},
},
},
},
{
name: "SingleNestedBlock/RecursiveBlocks",
baseSchema: NestedBlockObject{
Blocks: map[string]BlockBuilder{
"single": SingleNestedBlockBuilder{
NestedObject: NestedBlockObject{
Blocks: map[string]BlockBuilder{
"nested_block": ListNestedBlockBuilder{
NestedObject: NestedBlockObject{
Attributes: map[string]AttributeBuilder{
"attr": StringAttributeBuilder{},
},
},
},
},
},
},
},
},
path: []string{"single"},
want: NestedBlockObject{
Attributes: map[string]AttributeBuilder{
"single": SingleNestedAttributeBuilder{
Attributes: map[string]AttributeBuilder{
"nested_block": ListNestedAttributeBuilder{
NestedObject: NestedAttributeObject{
Attributes: map[string]AttributeBuilder{
"attr": StringAttributeBuilder{},
},
},
},
},
},
},
},
},
{
name: "PanicOnEmptyPath",
path: nil,
expectPanic: true,
},
}
for _, c := range testCases {
t.Run(c.name, func(t *testing.T) {
if c.expectPanic {
assert.Panics(t, func() {
ConstructCustomizableSchema(c.baseSchema).ConvertToAttribute(c.path...)
})
} else {
got := ConstructCustomizableSchema(c.baseSchema).ConvertToAttribute(c.path...)
assert.Equal(t, c.want, got.attr.(SingleNestedBlockBuilder).NestedObject)
}
})
}
}
10 changes: 5 additions & 5 deletions internal/providers/pluginfw/tfschema/list_nested_attribute.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func (a ListNestedAttributeBuilder) BuildResourceAttribute() schema.Attribute {
}
}

func (a ListNestedAttributeBuilder) SetOptional() BaseSchemaBuilder {
func (a ListNestedAttributeBuilder) SetOptional() AttributeBuilder {
if a.Optional && !a.Required {
panic("attribute is already optional")
}
Expand All @@ -53,7 +53,7 @@ func (a ListNestedAttributeBuilder) SetOptional() BaseSchemaBuilder {
return a
}

func (a ListNestedAttributeBuilder) SetRequired() BaseSchemaBuilder {
func (a ListNestedAttributeBuilder) SetRequired() AttributeBuilder {
if !a.Optional && a.Required {
panic("attribute is already required")
}
Expand All @@ -62,23 +62,23 @@ func (a ListNestedAttributeBuilder) SetRequired() BaseSchemaBuilder {
return a
}

func (a ListNestedAttributeBuilder) SetSensitive() BaseSchemaBuilder {
func (a ListNestedAttributeBuilder) SetSensitive() AttributeBuilder {
if a.Sensitive {
panic("attribute is already sensitive")
}
a.Sensitive = true
return a
}

func (a ListNestedAttributeBuilder) SetComputed() BaseSchemaBuilder {
func (a ListNestedAttributeBuilder) SetComputed() AttributeBuilder {
if a.Computed {
panic("attribute is already computed")
}
a.Computed = true
return a
}

func (a ListNestedAttributeBuilder) SetReadOnly() BaseSchemaBuilder {
func (a ListNestedAttributeBuilder) SetReadOnly() AttributeBuilder {
if a.Computed && !a.Optional && !a.Required {
panic("attribute is already read only")
}
Expand Down
27 changes: 27 additions & 0 deletions internal/providers/pluginfw/tfschema/list_nested_block.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package tfschema

import (
"fmt"

dataschema "github.com/hashicorp/terraform-plugin-framework/datasource/schema"
"github.com/hashicorp/terraform-plugin-framework/resource/schema"
"github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier"
Expand All @@ -16,6 +18,15 @@ type ListNestedBlockBuilder struct {
PlanModifiers []planmodifier.List
}

func (a ListNestedBlockBuilder) ToAttribute() AttributeBuilder {
return ListNestedAttributeBuilder{
NestedObject: a.NestedObject.ToNestedAttributeObject(),
DeprecationMessage: a.DeprecationMessage,
Validators: a.Validators,
PlanModifiers: a.PlanModifiers,
}
}

func (a ListNestedBlockBuilder) BuildDataSourceBlock() dataschema.Block {
return dataschema.ListNestedBlock{
NestedObject: a.NestedObject.BuildDataSourceAttribute(),
Expand Down Expand Up @@ -47,3 +58,19 @@ func (a ListNestedBlockBuilder) AddPlanModifier(v planmodifier.List) BaseSchemaB
a.PlanModifiers = append(a.PlanModifiers, v)
return a
}

func (a ListNestedBlockBuilder) ConvertBlockToAttribute(field string) BaseSchemaBuilder {
elem, ok := a.NestedObject.Blocks[field]
if !ok {
panic(fmt.Errorf("field %s does not exist in nested block", field))
}
if a.NestedObject.Attributes == nil {
a.NestedObject.Attributes = make(map[string]AttributeBuilder)
}
a.NestedObject.Attributes[field] = elem.ToAttribute()
delete(a.NestedObject.Blocks, field)
if len(a.NestedObject.Blocks) == 0 {
a.NestedObject.Blocks = nil
}
return a
}
Loading

0 comments on commit cc49ffd

Please sign in to comment.