[Internal] Use Attributes by default for List Objects (#4315)

## Changes
Resources implemented on the SDKv2 used blocks to represent two kinds of
fields: traditional lists, and nested complex types (which were treated
as a list of length at most 1). The Plugin Framework now has support for
list attributes, and it is recommended to use this in general, except
for places where compatibility with SDKv2 is needed.

To achieve this, this change ensures that *StructToSchema methods in the
plugin framework generate attributes for lists by default. Then, for
resources migrated from SDKv2, the implementer must call
`ConfigureForSdkV2Migration()` in the customize callback. This will
convert any list attributes into list blocks, preserving compatibility
with SDKv2-implemented resources.

Additionally, this PR improves the handling of computed, non-optional
fields. For lists annotated as `tf:"computed"`, the resulting schema is
invalid: it is marked as computed and as required because of the absence
of the `"optional"` tag. When a field is labeled as computed, it can be
marked as optional but should never be marked as required; that is
addressed here as well.

Finally, this PR also includes some autogenerated changes modifying Apps
to use `types.Object` in place of `types.List` for nested complex types.

## Tests
<!-- 
How is this tested? Please see the checklist below and also describe any
other relevant tests
-->

- [ ] `make test` run locally
- [ ] relevant change in `docs/` folder
- [ ] covered with integration tests in `internal/acceptance`
- [ ] relevant acceptance tests are passing
- [ ] using Go SDK

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.ConfigureAsSdkV2Compatible()` in the `ResourceStructToSchema` callback:
+```go
+resp.Schema = tfschema.ResourceStructToSchema(ctx, Resource{}, func(c tfschema.CustomizableSchema) tfschema.CustomizableSchema {
+    cs.ConfigureAsSdkV2Compatible()
+    // 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.ConfigureAsSdkV2Compatible()
 		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.ConfigureAsSdkV2Compatible()
 		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.ConfigureAsSdkV2Compatible()
 		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]...)
-	}
-
+// ConfigureAsSdkV2Compatible 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) ConfigureAsSdkV2Compatible() *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.ConfigureAsSdkV2Compatible()
 			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.ConfigureAsSdkV2Compatible()
 			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.ConfigureAsSdkV2Compatible()
 			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.ConfigureAsSdkV2Compatible()
 			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.ConfigureAsSdkV2Compatible()
 			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_ConfigureAsSdkV2Compatible(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).ConfigureAsSdkV2Compatible()
 				})
 			} else {
-				got := ConstructCustomizableSchema(c.baseSchema).ConvertToAttribute(c.path...)
+				got := ConstructCustomizableSchema(c.baseSchema).ConfigureAsSdkV2Compatible()
 				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))
+}