[Internal] Remove unused configuration from blocks (#4283)

## Changes Currently, our schema builder elements in the plugin framework components allow you to set Computed/Optional/Required/Sensitive fields on Block schema elements. However, these are simply dropped when converting to actual schema elements: only attributes have these fields. This PR moves the SetComputed/SetOptional/SetRequired/SetSensitive/SetReadOnly methods to the AttributeBuilder interface and panics if you attempt to set them on a block. ## Tests Added a unit test to verify that these methods panic when called on an element that is converted into a block.
databricks · Dec 2, 2024 · 8e3117a · 8e3117a
1 parent 00eac36
commit 8e3117a
Show file tree

Hide file tree

Showing 15 changed files with 151 additions and 170 deletions.
diff --git a/internal/providers/pluginfw/tfschema/attribute_builder.go b/internal/providers/pluginfw/tfschema/attribute_builder.go
@@ -10,6 +10,25 @@ import (
 // This common interface prevents us from keeping two copies of StructToSchema and CustomizableSchema.
 type AttributeBuilder interface {
 	BaseSchemaBuilder
+
+	// SetOptional sets the attribute as optional in the schema. This does not affect whether the attribute is computed.
+	// It fails if the attribute is already optional.
+	SetOptional() AttributeBuilder
+
+	// SetRequired sets the attribute as required in the schema. This does not affect whether the attribute is computed.
+	// It fails if the attribute is already required.
+	SetRequired() AttributeBuilder
+
+	// SetSensitive sets the attribute as sensitive in the schema. It fails if the attribute is already sensitive.
+	SetSensitive() AttributeBuilder
+
+	// SetComputed sets the attribute as computed in the schema. It fails if the attribute is already computed.
+	SetComputed() AttributeBuilder
+
+	// Sets the attribute as read-only in the schema, i.e. computed and neither optional or required. It fails if the
+	// attribute is already read-only.
+	SetReadOnly() AttributeBuilder
+
 	BuildDataSourceAttribute() dataschema.Attribute
 	BuildResourceAttribute() schema.Attribute
 }

diff --git a/internal/providers/pluginfw/tfschema/base_schema_builder.go b/internal/providers/pluginfw/tfschema/base_schema_builder.go
@@ -3,10 +3,5 @@ package tfschema
 // BaseSchemaBuilder is the common interface for all blocks and attributes, it can be used to build data source and resource.
 // Both AttributeBuilder and BlockBuilder extend this interface.
 type BaseSchemaBuilder interface {
-	SetOptional() BaseSchemaBuilder
-	SetRequired() BaseSchemaBuilder
-	SetSensitive() BaseSchemaBuilder
-	SetComputed() BaseSchemaBuilder
-	SetReadOnly() BaseSchemaBuilder
 	SetDeprecated(string) BaseSchemaBuilder
 }
diff --git a/internal/providers/pluginfw/tfschema/bool_attribute.go b/internal/providers/pluginfw/tfschema/bool_attribute.go
@@ -40,7 +40,7 @@ func (a BoolAttributeBuilder) BuildResourceAttribute() schema.Attribute {
 	}
 }
 
-func (a BoolAttributeBuilder) SetOptional() BaseSchemaBuilder {
+func (a BoolAttributeBuilder) SetOptional() AttributeBuilder {
 	if a.Optional && !a.Required {
 		panic("attribute is already optional")
 	}
@@ -49,7 +49,7 @@ func (a BoolAttributeBuilder) SetOptional() BaseSchemaBuilder {
 	return a
 }
 
-func (a BoolAttributeBuilder) SetRequired() BaseSchemaBuilder {
+func (a BoolAttributeBuilder) SetRequired() AttributeBuilder {
 	if !a.Optional && a.Required {
 		panic("attribute is already required")
 	}
@@ -58,23 +58,23 @@ func (a BoolAttributeBuilder) SetRequired() BaseSchemaBuilder {
 	return a
 }
 
-func (a BoolAttributeBuilder) SetSensitive() BaseSchemaBuilder {
+func (a BoolAttributeBuilder) SetSensitive() AttributeBuilder {
 	if a.Sensitive {
 		panic("attribute is already sensitive")
 	}
 	a.Sensitive = true
 	return a
 }
 
-func (a BoolAttributeBuilder) SetComputed() BaseSchemaBuilder {
+func (a BoolAttributeBuilder) SetComputed() AttributeBuilder {
 	if a.Computed {
 		panic("attribute is already computed")
 	}
 	a.Computed = true
 	return a
 }
 
-func (a BoolAttributeBuilder) SetReadOnly() BaseSchemaBuilder {
+func (a BoolAttributeBuilder) SetReadOnly() AttributeBuilder {
 	if a.Computed && !a.Optional && !a.Required {
 		panic("attribute is already read only")
 	}
@@ -89,12 +89,12 @@ func (a BoolAttributeBuilder) SetDeprecated(msg string) BaseSchemaBuilder {
 	return a
 }
 
-func (a BoolAttributeBuilder) AddValidator(v validator.Bool) BaseSchemaBuilder {
+func (a BoolAttributeBuilder) AddValidator(v validator.Bool) AttributeBuilder {
 	a.Validators = append(a.Validators, v)
 	return a
 }
 
-func (a BoolAttributeBuilder) AddPlanModifier(v planmodifier.Bool) BaseSchemaBuilder {
+func (a BoolAttributeBuilder) AddPlanModifier(v planmodifier.Bool) AttributeBuilder {
 	a.PlanModifiers = append(a.PlanModifiers, v)
 	return a
 }
diff --git a/internal/providers/pluginfw/tfschema/customizable_schema.go b/internal/providers/pluginfw/tfschema/customizable_schema.go
@@ -16,8 +16,7 @@ type CustomizableSchema struct {
 
 // ConstructCustomizableSchema constructs a CustomizableSchema given a NestedBlockObject.
 func ConstructCustomizableSchema(nestedObject NestedBlockObject) *CustomizableSchema {
-	attr := AttributeBuilder(SingleNestedBlockBuilder{NestedObject: nestedObject})
-	return &CustomizableSchema{attr: attr}
+	return &CustomizableSchema{attr: SingleNestedBlockBuilder{NestedObject: nestedObject}}
 }
 
 // ToAttributeMap converts CustomizableSchema into BaseSchemaBuilder.
@@ -116,7 +115,12 @@ func (s *CustomizableSchema) AddPlanModifier(v any, path ...string) *Customizabl
 
 func (s *CustomizableSchema) SetOptional(path ...string) *CustomizableSchema {
 	cb := func(attr BaseSchemaBuilder) BaseSchemaBuilder {
-		return attr.SetOptional()
+		switch a := attr.(type) {
+		case AttributeBuilder:
+			return a.SetOptional()
+		default:
+			panic(fmt.Errorf("SetOptional called on invalid attribute type: %s. %s", reflect.TypeOf(attr).String(), common.TerraformBugErrorMessage))
+		}
 	}
 
 	navigateSchemaWithCallback(&s.attr, cb, path...)
@@ -126,7 +130,12 @@ func (s *CustomizableSchema) SetOptional(path ...string) *CustomizableSchema {
 
 func (s *CustomizableSchema) SetRequired(path ...string) *CustomizableSchema {
 	cb := func(attr BaseSchemaBuilder) BaseSchemaBuilder {
-		return attr.SetRequired()
+		switch a := attr.(type) {
+		case AttributeBuilder:
+			return a.SetRequired()
+		default:
+			panic(fmt.Errorf("SetRequired called on invalid attribute type: %s. %s", reflect.TypeOf(attr).String(), common.TerraformBugErrorMessage))
+		}
 	}
 
 	navigateSchemaWithCallback(&s.attr, cb, path...)
@@ -136,7 +145,12 @@ func (s *CustomizableSchema) SetRequired(path ...string) *CustomizableSchema {
 
 func (s *CustomizableSchema) SetSensitive(path ...string) *CustomizableSchema {
 	cb := func(attr BaseSchemaBuilder) BaseSchemaBuilder {
-		return attr.SetSensitive()
+		switch a := attr.(type) {
+		case AttributeBuilder:
+			return a.SetSensitive()
+		default:
+			panic(fmt.Errorf("SetSensitive called on invalid attribute type: %s. %s", reflect.TypeOf(attr).String(), common.TerraformBugErrorMessage))
+		}
 	}
 
 	navigateSchemaWithCallback(&s.attr, cb, path...)
@@ -155,7 +169,12 @@ func (s *CustomizableSchema) SetDeprecated(msg string, path ...string) *Customiz
 
 func (s *CustomizableSchema) SetComputed(path ...string) *CustomizableSchema {
 	cb := func(attr BaseSchemaBuilder) BaseSchemaBuilder {
-		return attr.SetComputed()
+		switch a := attr.(type) {
+		case AttributeBuilder:
+			return a.SetComputed()
+		default:
+			panic(fmt.Errorf("SetComputed called on invalid attribute type: %s. %s", reflect.TypeOf(attr).String(), common.TerraformBugErrorMessage))
+		}
 	}
 
 	navigateSchemaWithCallback(&s.attr, cb, path...)
@@ -167,7 +186,12 @@ func (s *CustomizableSchema) SetComputed(path ...string) *CustomizableSchema {
 // by the platform.
 func (s *CustomizableSchema) SetReadOnly(path ...string) *CustomizableSchema {
 	cb := func(attr BaseSchemaBuilder) BaseSchemaBuilder {
-		return attr.SetReadOnly()
+		switch a := attr.(type) {
+		case AttributeBuilder:
+			return a.SetReadOnly()
+		default:
+			panic(fmt.Errorf("SetReadOnly called on invalid attribute type: %s. %s", reflect.TypeOf(attr).String(), common.TerraformBugErrorMessage))
+		}
 	}
 
 	navigateSchemaWithCallback(&s.attr, cb, path...)

diff --git a/internal/providers/pluginfw/tfschema/customizable_schema_test.go b/internal/providers/pluginfw/tfschema/customizable_schema_test.go
@@ -130,3 +130,48 @@ func TestCustomizeSchemaObjectTypeValidatorAdded(t *testing.T) {
 
 	assert.True(t, len(scm.Blocks["nested_slice_object"].(schema.ListNestedBlock).Validators) == 1)
 }
+
+func TestCustomizeSchema_SetRequired_PanicOnBlock(t *testing.T) {
+	assert.Panics(t, func() {
+		_ = ResourceStructToSchema(TestTfSdk{}, func(c CustomizableSchema) CustomizableSchema {
+			c.SetRequired("nested")
+			return c
+		})
+	})
+}
+
+func TestCustomizeSchema_SetOptional_PanicOnBlock(t *testing.T) {
+	assert.Panics(t, func() {
+		_ = ResourceStructToSchema(TestTfSdk{}, func(c CustomizableSchema) CustomizableSchema {
+			c.SetOptional("nested")
+			return c
+		})
+	})
+}
+
+func TestCustomizeSchema_SetSensitive_PanicOnBlock(t *testing.T) {
+	assert.Panics(t, func() {
+		_ = ResourceStructToSchema(TestTfSdk{}, func(c CustomizableSchema) CustomizableSchema {
+			c.SetSensitive("nested")
+			return c
+		})
+	})
+}
+
+func TestCustomizeSchema_SetReadOnly_PanicOnBlock(t *testing.T) {
+	assert.Panics(t, func() {
+		_ = ResourceStructToSchema(TestTfSdk{}, func(c CustomizableSchema) CustomizableSchema {
+			c.SetReadOnly("nested")
+			return c
+		})
+	})
+}
+
+func TestCustomizeSchema_SetComputed_PanicOnBlock(t *testing.T) {
+	assert.Panics(t, func() {
+		_ = ResourceStructToSchema(TestTfSdk{}, func(c CustomizableSchema) CustomizableSchema {
+			c.SetComputed("nested")
+			return c
+		})
+	})
+}
diff --git a/internal/providers/pluginfw/tfschema/float64_attribute.go b/internal/providers/pluginfw/tfschema/float64_attribute.go
@@ -40,7 +40,7 @@ func (a Float64AttributeBuilder) BuildResourceAttribute() schema.Attribute {
 	}
 }
 
-func (a Float64AttributeBuilder) SetOptional() BaseSchemaBuilder {
+func (a Float64AttributeBuilder) SetOptional() AttributeBuilder {
 	if a.Optional && !a.Required {
 		panic("attribute is already optional")
 	}
@@ -49,7 +49,7 @@ func (a Float64AttributeBuilder) SetOptional() BaseSchemaBuilder {
 	return a
 }
 
-func (a Float64AttributeBuilder) SetRequired() BaseSchemaBuilder {
+func (a Float64AttributeBuilder) SetRequired() AttributeBuilder {
 	if !a.Optional && a.Required {
 		panic("attribute is already required")
 	}
@@ -58,23 +58,23 @@ func (a Float64AttributeBuilder) SetRequired() BaseSchemaBuilder {
 	return a
 }
 
-func (a Float64AttributeBuilder) SetSensitive() BaseSchemaBuilder {
+func (a Float64AttributeBuilder) SetSensitive() AttributeBuilder {
 	if a.Sensitive {
 		panic("attribute is already sensitive")
 	}
 	a.Sensitive = true
 	return a
 }
 
-func (a Float64AttributeBuilder) SetComputed() BaseSchemaBuilder {
+func (a Float64AttributeBuilder) SetComputed() AttributeBuilder {
 	if a.Computed {
 		panic("attribute is already computed")
 	}
 	a.Computed = true
 	return a
 }
 
-func (a Float64AttributeBuilder) SetReadOnly() BaseSchemaBuilder {
+func (a Float64AttributeBuilder) SetReadOnly() AttributeBuilder {
 	if a.Computed && !a.Optional && !a.Required {
 		panic("attribute is already read only")
 	}
@@ -89,12 +89,12 @@ func (a Float64AttributeBuilder) SetDeprecated(msg string) BaseSchemaBuilder {
 	return a
 }
 
-func (a Float64AttributeBuilder) AddValidator(v validator.Float64) BaseSchemaBuilder {
+func (a Float64AttributeBuilder) AddValidator(v validator.Float64) AttributeBuilder {
 	a.Validators = append(a.Validators, v)
 	return a
 }
 
-func (a Float64AttributeBuilder) AddPlanModifier(v planmodifier.Float64) BaseSchemaBuilder {
+func (a Float64AttributeBuilder) AddPlanModifier(v planmodifier.Float64) AttributeBuilder {
 	a.PlanModifiers = append(a.PlanModifiers, v)
 	return a
 }
diff --git a/internal/providers/pluginfw/tfschema/int64_attribute.go b/internal/providers/pluginfw/tfschema/int64_attribute.go
@@ -40,7 +40,7 @@ func (a Int64AttributeBuilder) BuildResourceAttribute() schema.Attribute {
 	}
 }
 
-func (a Int64AttributeBuilder) SetOptional() BaseSchemaBuilder {
+func (a Int64AttributeBuilder) SetOptional() AttributeBuilder {
 	if a.Optional && !a.Required {
 		panic("attribute is already optional")
 	}
@@ -49,7 +49,7 @@ func (a Int64AttributeBuilder) SetOptional() BaseSchemaBuilder {
 	return a
 }
 
-func (a Int64AttributeBuilder) SetRequired() BaseSchemaBuilder {
+func (a Int64AttributeBuilder) SetRequired() AttributeBuilder {
 	if !a.Optional && a.Required {
 		panic("attribute is already required")
 	}
@@ -58,23 +58,23 @@ func (a Int64AttributeBuilder) SetRequired() BaseSchemaBuilder {
 	return a
 }
 
-func (a Int64AttributeBuilder) SetSensitive() BaseSchemaBuilder {
+func (a Int64AttributeBuilder) SetSensitive() AttributeBuilder {
 	if a.Sensitive {
 		panic("attribute is already sensitive")
 	}
 	a.Sensitive = true
 	return a
 }
 
-func (a Int64AttributeBuilder) SetComputed() BaseSchemaBuilder {
+func (a Int64AttributeBuilder) SetComputed() AttributeBuilder {
 	if a.Computed {
 		panic("attribute is already computed")
 	}
 	a.Computed = true
 	return a
 }
 
-func (a Int64AttributeBuilder) SetReadOnly() BaseSchemaBuilder {
+func (a Int64AttributeBuilder) SetReadOnly() AttributeBuilder {
 	if a.Computed && !a.Optional && !a.Required {
 		panic("attribute is already read only")
 	}
@@ -89,12 +89,12 @@ func (a Int64AttributeBuilder) SetDeprecated(msg string) BaseSchemaBuilder {
 	return a
 }
 
-func (a Int64AttributeBuilder) AddValidator(v validator.Int64) BaseSchemaBuilder {
+func (a Int64AttributeBuilder) AddValidator(v validator.Int64) AttributeBuilder {
 	a.Validators = append(a.Validators, v)
 	return a
 }
 
-func (a Int64AttributeBuilder) AddPlanModifier(v planmodifier.Int64) BaseSchemaBuilder {
+func (a Int64AttributeBuilder) AddPlanModifier(v planmodifier.Int64) AttributeBuilder {
 	a.PlanModifiers = append(a.PlanModifiers, v)
 	return a
 }