[Internal] Use Attributes by default for List Objects

CONTRIBUTING.md

@@ -135,10 +135,18 @@ We are migrating the resource from SDKv2 to Plugin Framework provider and hence
 6. Create a PR and send it for review. 
 
 ### Migrating resource to plugin framework
-Ideally there shouldn't be any behaviour change when migrating a resource or data source to either Go SDk or Plugin Framework.
+There must not be any behaviour change or schema change when migrating a resource or data source to either Go SDK or Plugin Framework.
 - Please make sure there are no breaking differences due to changes in schema by running: `make diff-schema`. 
 - Integration tests shouldn't require any major changes.   
 
+By default, `ResourceStructToSchema` will convert a `types.List` field to a `ListAttribute` or `ListNestedAttribute`. For resources or data sources migrated from the SDKv2, `ListNestedBlock` must be used for such fields. To do this, call `cs.ConfigureForSdkV2Migration()` in the `ResourceStructToSchema` callback:
+```go
+resp.Schema = tfschema.ResourceStructToSchema(ctx, Resource{}, func(c tfschema.CustomizableSchema) tfschema.CustomizableSchema {
+    cs.ConfigureForSdkV2Migration()
+    // Add any additional configuration here
+    return cs
+})
+```
 
 ### Code Organization
 Each resource and data source should be defined in package `internal/providers/plugnifw/products/<resource>`, e.g.: `internal/providers/plugnifw/products/volume` package will contain both resource, data sources and other utils specific to volumes. Tests (both unit and integration tests) will also remain in this package. 

internal/providers/pluginfw/products/library/resource_library.go

@@ -88,6 +88,7 @@ func (r *LibraryResource) Metadata(ctx context.Context, req resource.MetadataReq
 
 func (r *LibraryResource) Schema(ctx context.Context, req resource.SchemaRequest, resp *resource.SchemaResponse) {
 	attrs, blocks := tfschema.ResourceStructToSchemaMap(ctx, LibraryExtended{}, func(c tfschema.CustomizableSchema) tfschema.CustomizableSchema {
+		c.ConfigureForSdkV2Migration()
 		for field, attribute := range c.ToNestedBlockObject().Attributes {
 			switch attribute.(type) {
 			case tfschema.StringAttributeBuilder:

internal/providers/pluginfw/products/qualitymonitor/resource_quality_monitor.go

@@ -78,6 +78,7 @@ func (r *QualityMonitorResource) Metadata(ctx context.Context, req resource.Meta
 
 func (r *QualityMonitorResource) Schema(ctx context.Context, req resource.SchemaRequest, resp *resource.SchemaResponse) {
 	attrs, blocks := tfschema.ResourceStructToSchemaMap(ctx, MonitorInfoExtended{}, func(c tfschema.CustomizableSchema) tfschema.CustomizableSchema {
+		c.ConfigureForSdkV2Migration()
 		c.SetRequired("assets_dir")
 		c.SetRequired("output_schema_name")
 		c.SetReadOnly("monitor_version")

internal/providers/pluginfw/products/sharing/resource_share.go

@@ -145,6 +145,7 @@ func (r *ShareResource) Metadata(ctx context.Context, req resource.MetadataReque
 
 func (r *ShareResource) Schema(ctx context.Context, req resource.SchemaRequest, resp *resource.SchemaResponse) {
 	attrs, blocks := tfschema.ResourceStructToSchemaMap(ctx, ShareInfoExtended{}, func(c tfschema.CustomizableSchema) tfschema.CustomizableSchema {
+		c.ConfigureForSdkV2Migration()
 		c.SetRequired("name")
 
 		c.AddPlanModifier(stringplanmodifier.RequiresReplace(), "name") // ForceNew

internal/providers/pluginfw/tfschema/attribute_converter.go

@@ -1,6 +1,34 @@
 package tfschema
 
-type BlockToAttributeConverter interface {
-	// ConvertBlockToAttribute converts a contained block to its corresponding attribute type.
-	ConvertBlockToAttribute(string) BaseSchemaBuilder
+// Blockable is an interface that can be implemented by an AttributeBuilder to convert it to a BlockBuilder.
+type Blockable interface {
+	// ToBlock converts the AttributeBuilder to a BlockBuilder.
+	ToBlock() BlockBuilder
+}
+
+// convertAttributesToBlocks converts all attributes implementing the Blockable interface to blocks, returning
+// a new NestedBlockObject with the converted attributes and the original blocks.
+func convertAttributesToBlocks(attributes map[string]AttributeBuilder, blocks map[string]BlockBuilder) NestedBlockObject {
+	newAttributes := make(map[string]AttributeBuilder)
+	newBlocks := make(map[string]BlockBuilder)
+	for name, attr := range attributes {
+		if lnab, ok := attr.(Blockable); ok {
+			newBlocks[name] = lnab.ToBlock()
+		} else {
+			newAttributes[name] = attr
+		}
+	}
+	for name, block := range blocks {
+		newBlocks[name] = block
+	}
+	if len(newAttributes) == 0 {
+		newAttributes = nil
+	}
+	if len(newBlocks) == 0 {
+		newBlocks = nil
+	}
+	return NestedBlockObject{
+		Attributes: newAttributes,
+		Blocks:     newBlocks,
+	}
 }

internal/providers/pluginfw/tfschema/customizable_schema.go

@@ -202,32 +202,12 @@ 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]...)
-	}
-
+// ConfigureForSdkV2Migration modifies the underlying schema to be compatible with SDKv2. This method must
+// be called on all resources that were originally implemented using the SDKv2 and are migrated to the plugin
+// framework.
+func (s *CustomizableSchema) ConfigureForSdkV2Migration() *CustomizableSchema {
+	nbo := s.attr.(SingleNestedBlockBuilder).NestedObject
+	s.attr = SingleNestedBlockBuilder{NestedObject: convertAttributesToBlocks(nbo.Attributes, nbo.Blocks)}
 	return s
 }
 

internal/providers/pluginfw/tfschema/customizable_schema_test.go

@@ -75,7 +75,7 @@ func TestCustomizeSchemaSetRequired(t *testing.T) {
 		return c
 	})
 
-	assert.True(t, scm.Blocks["nested"].(schema.ListNestedBlock).NestedObject.Attributes["enabled"].IsRequired())
+	assert.True(t, scm.Attributes["nested"].(schema.ListNestedAttribute).NestedObject.Attributes["enabled"].IsRequired())
 }
 
 func TestCustomizeSchemaSetOptional(t *testing.T) {
@@ -93,7 +93,7 @@ func TestCustomizeSchemaSetSensitive(t *testing.T) {
 		return c
 	})
 
-	assert.True(t, scm.Blocks["nested"].(schema.ListNestedBlock).NestedObject.Attributes["name"].IsSensitive())
+	assert.True(t, scm.Attributes["nested"].(schema.ListNestedAttribute).NestedObject.Attributes["name"].IsSensitive())
 }
 
 func TestCustomizeSchemaSetDeprecated(t *testing.T) {
@@ -127,7 +127,7 @@ func (testTfSdkListNestedAttribute) GetComplexFieldTypes(context.Context) map[st
 
 func TestCustomizeSchemaSetReadOnly_RecursivelySetsFieldsOfListNestedAttributes(t *testing.T) {
 	scm := ResourceStructToSchema(context.Background(), testTfSdkListNestedAttribute{}, func(c CustomizableSchema) CustomizableSchema {
-		c.ConvertToAttribute("list").SetReadOnly("list")
+		c.SetReadOnly("list")
 		return c
 	})
 	for _, field := range []string{"name", "enabled"} {
@@ -160,12 +160,13 @@ func TestCustomizeSchemaObjectTypeValidatorAdded(t *testing.T) {
 		return c
 	})
 
-	assert.True(t, len(scm.Blocks["nested_slice_object"].(schema.ListNestedBlock).Validators) == 1)
+	assert.True(t, len(scm.Attributes["nested_slice_object"].(schema.ListNestedAttribute).Validators) == 1)
 }
 
 func TestCustomizeSchema_SetRequired_PanicOnBlock(t *testing.T) {
 	assert.Panics(t, func() {
 		_ = ResourceStructToSchema(context.Background(), TestTfSdk{}, func(c CustomizableSchema) CustomizableSchema {
+			c.ConfigureForSdkV2Migration()
 			c.SetRequired("nested")
 			return c
 		})
@@ -175,6 +176,7 @@ func TestCustomizeSchema_SetRequired_PanicOnBlock(t *testing.T) {
 func TestCustomizeSchema_SetOptional_PanicOnBlock(t *testing.T) {
 	assert.Panics(t, func() {
 		_ = ResourceStructToSchema(context.Background(), TestTfSdk{}, func(c CustomizableSchema) CustomizableSchema {
+			c.ConfigureForSdkV2Migration()
 			c.SetOptional("nested")
 			return c
 		})
@@ -184,6 +186,7 @@ func TestCustomizeSchema_SetOptional_PanicOnBlock(t *testing.T) {
 func TestCustomizeSchema_SetSensitive_PanicOnBlock(t *testing.T) {
 	assert.Panics(t, func() {
 		_ = ResourceStructToSchema(context.Background(), TestTfSdk{}, func(c CustomizableSchema) CustomizableSchema {
+			c.ConfigureForSdkV2Migration()
 			c.SetSensitive("nested")
 			return c
 		})
@@ -193,6 +196,7 @@ func TestCustomizeSchema_SetSensitive_PanicOnBlock(t *testing.T) {
 func TestCustomizeSchema_SetReadOnly_PanicOnBlock(t *testing.T) {
 	assert.Panics(t, func() {
 		_ = ResourceStructToSchema(context.Background(), TestTfSdk{}, func(c CustomizableSchema) CustomizableSchema {
+			c.ConfigureForSdkV2Migration()
 			c.SetReadOnly("nested")
 			return c
 		})
@@ -202,6 +206,7 @@ func TestCustomizeSchema_SetReadOnly_PanicOnBlock(t *testing.T) {
 func TestCustomizeSchema_SetComputed_PanicOnBlock(t *testing.T) {
 	assert.Panics(t, func() {
 		_ = ResourceStructToSchema(context.Background(), TestTfSdk{}, func(c CustomizableSchema) CustomizableSchema {
+			c.ConfigureForSdkV2Migration()
 			c.SetComputed("nested")
 			return c
 		})
@@ -258,22 +263,21 @@ func (m mockValidator) ValidateObject(context.Context, validator.ObjectRequest,
 var _ validator.List = mockValidator{}
 var _ validator.Object = mockValidator{}
 
-func TestCustomizeSchema_ConvertToAttribute(t *testing.T) {
+func TestCustomizeSchema_ConfigureForSdkV2Migration(t *testing.T) {
 	v := mockValidator{}
 	pm := mockPlanModifier{}
 	testCases := []struct {
 		name        string
 		baseSchema  NestedBlockObject
-		path        []string
 		want        NestedBlockObject
 		expectPanic bool
 	}{
 		{
-			name: "ListNestedBlock",
+			name: "ListNestedAttribute",
 			baseSchema: NestedBlockObject{
-				Blocks: map[string]BlockBuilder{
-					"list": ListNestedBlockBuilder{
-						NestedObject: NestedBlockObject{
+				Attributes: map[string]AttributeBuilder{
+					"list": ListNestedAttributeBuilder{
+						NestedObject: NestedAttributeObject{
 							Attributes: map[string]AttributeBuilder{
 								"attr": StringAttributeBuilder{},
 							},
@@ -284,11 +288,10 @@ func TestCustomizeSchema_ConvertToAttribute(t *testing.T) {
 					},
 				},
 			},
-			path: []string{"list"},
 			want: NestedBlockObject{
-				Attributes: map[string]AttributeBuilder{
-					"list": ListNestedAttributeBuilder{
-						NestedObject: NestedAttributeObject{
+				Blocks: map[string]BlockBuilder{
+					"list": ListNestedBlockBuilder{
+						NestedObject: NestedBlockObject{
 							Attributes: map[string]AttributeBuilder{
 								"attr": StringAttributeBuilder{},
 							},
@@ -301,14 +304,14 @@ func TestCustomizeSchema_ConvertToAttribute(t *testing.T) {
 			},
 		},
 		{
-			name: "ListNestedBlock/CalledOnInnerBlock",
+			name: "ListNestedAttribute/RecursiveBlocks",
 			baseSchema: NestedBlockObject{
-				Blocks: map[string]BlockBuilder{
-					"list": ListNestedBlockBuilder{
-						NestedObject: NestedBlockObject{
-							Blocks: map[string]BlockBuilder{
-								"nested_block": ListNestedBlockBuilder{
-									NestedObject: NestedBlockObject{
+				Attributes: map[string]AttributeBuilder{
+					"list": ListNestedAttributeBuilder{
+						NestedObject: NestedAttributeObject{
+							Attributes: map[string]AttributeBuilder{
+								"nested_block": ListNestedAttributeBuilder{
+									NestedObject: NestedAttributeObject{
 										Attributes: map[string]AttributeBuilder{
 											"attr": StringAttributeBuilder{},
 										},
@@ -319,14 +322,13 @@ func TestCustomizeSchema_ConvertToAttribute(t *testing.T) {
 					},
 				},
 			},
-			path: []string{"list", "nested_block"},
 			want: NestedBlockObject{
 				Blocks: map[string]BlockBuilder{
 					"list": ListNestedBlockBuilder{
 						NestedObject: NestedBlockObject{
-							Attributes: map[string]AttributeBuilder{
-								"nested_block": ListNestedAttributeBuilder{
-									NestedObject: NestedAttributeObject{
+							Blocks: map[string]BlockBuilder{
+								"nested_block": ListNestedBlockBuilder{
+									NestedObject: NestedBlockObject{
 										Attributes: map[string]AttributeBuilder{
 											"attr": StringAttributeBuilder{},
 										},
@@ -339,23 +341,8 @@ func TestCustomizeSchema_ConvertToAttribute(t *testing.T) {
 			},
 		},
 		{
-			name: "SingleNestedBlock",
+			name: "SingleNestedBlock/Panics",
 			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{
@@ -367,57 +354,17 @@ func TestCustomizeSchema_ConvertToAttribute(t *testing.T) {
 					},
 				},
 			},
-		},
-		{
-			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...)
+					ConstructCustomizableSchema(c.baseSchema).ConfigureForSdkV2Migration()
 				})
 			} else {
-				got := ConstructCustomizableSchema(c.baseSchema).ConvertToAttribute(c.path...)
+				got := ConstructCustomizableSchema(c.baseSchema).ConfigureForSdkV2Migration()
 				assert.Equal(t, c.want, got.attr.(SingleNestedBlockBuilder).NestedObject)
 			}
 		})

internal/providers/pluginfw/tfschema/list_nested_attribute.go

@@ -103,3 +103,12 @@ func (a ListNestedAttributeBuilder) AddPlanModifier(v planmodifier.List) BaseSch
 	a.PlanModifiers = append(a.PlanModifiers, v)
 	return a
 }
+
+func (a ListNestedAttributeBuilder) ToBlock() BlockBuilder {
+	return ListNestedBlockBuilder{
+		NestedObject:       convertAttributesToBlocks(a.NestedObject.Attributes, nil),
+		DeprecationMessage: a.DeprecationMessage,
+		Validators:         a.Validators,
+		PlanModifiers:      a.PlanModifiers,
+	}
+}

internal/providers/pluginfw/tfschema/single_nested_attribute.go

@@ -1,6 +1,9 @@
 package tfschema
 
 import (
+	"fmt"
+
+	"github.com/databricks/terraform-provider-databricks/common"
 	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"
@@ -102,3 +105,7 @@ func (a SingleNestedAttributeBuilder) AddPlanModifier(v planmodifier.Object) Att
 	a.PlanModifiers = append(a.PlanModifiers, v)
 	return a
 }
+
+func (a SingleNestedAttributeBuilder) ToBlock() BlockBuilder {
+	panic(fmt.Errorf("ToBlock() called on SingleNestedAttributeBuilder. This means that the corresponding field is a types.Object, which should never happen for legacy resources. %s", common.TerraformBugErrorMessage))
+}