Refactor CustomizeSchema for ResourceProvider to skip unfeasible customizations (#3481)

* update

* update

* update

* update

Author: edwardfeng-db

clusters/resource_cluster.go

@@ -123,62 +123,62 @@ type ClusterSpec struct {
 	Libraries []compute.Library `json:"libraries,omitempty" tf:"slice_set,alias:library"`
 }
 
-func (ClusterSpec) CustomizeSchema(s map[string]*schema.Schema) map[string]*schema.Schema {
-	common.CustomizeSchemaPath(s, "cluster_source").SetReadOnly()
-	common.CustomizeSchemaPath(s, "enable_elastic_disk").SetComputed()
-	common.CustomizeSchemaPath(s, "enable_local_disk_encryption").SetComputed()
-	common.CustomizeSchemaPath(s, "node_type_id").SetComputed().SetConflictsWith([]string{"driver_instance_pool_id", "instance_pool_id"})
-	common.CustomizeSchemaPath(s, "driver_node_type_id").SetComputed().SetConflictsWith([]string{"driver_instance_pool_id", "instance_pool_id"})
-	common.CustomizeSchemaPath(s, "driver_instance_pool_id").SetComputed().SetConflictsWith([]string{"driver_node_type_id", "node_type_id"})
-	common.CustomizeSchemaPath(s, "ssh_public_keys").SetMaxItems(10)
-	common.CustomizeSchemaPath(s, "init_scripts").SetMaxItems(10)
-	common.CustomizeSchemaPath(s, "init_scripts", "dbfs").SetDeprecated(DbfsDeprecationWarning)
-	common.CustomizeSchemaPath(s, "init_scripts", "dbfs", "destination").SetRequired()
-	common.CustomizeSchemaPath(s, "init_scripts", "s3", "destination").SetRequired()
-	common.CustomizeSchemaPath(s, "init_scripts", "volumes", "destination").SetRequired()
-	common.CustomizeSchemaPath(s, "init_scripts", "workspace", "destination").SetRequired()
-	common.CustomizeSchemaPath(s, "workload_type", "clients").SetRequired()
-	common.CustomizeSchemaPath(s, "workload_type", "clients", "notebooks").SetDefault(true)
-	common.CustomizeSchemaPath(s, "workload_type", "clients", "jobs").SetDefault(true)
-	s["library"].Set = func(i any) int {
+func (ClusterSpec) CustomizeSchema(s *common.CustomizableSchema) *common.CustomizableSchema {
+	s.SchemaPath("cluster_source").SetReadOnly()
+	s.SchemaPath("enable_elastic_disk").SetComputed()
+	s.SchemaPath("enable_local_disk_encryption").SetComputed()
+	s.SchemaPath("node_type_id").SetComputed().SetConflictsWith([]string{"driver_instance_pool_id", "instance_pool_id"})
+	s.SchemaPath("driver_node_type_id").SetComputed().SetConflictsWith([]string{"driver_instance_pool_id", "instance_pool_id"})
+	s.SchemaPath("driver_instance_pool_id").SetComputed().SetConflictsWith([]string{"driver_node_type_id", "node_type_id"})
+	s.SchemaPath("ssh_public_keys").SetMaxItems(10)
+	s.SchemaPath("init_scripts").SetMaxItems(10)
+	s.SchemaPath("init_scripts", "dbfs").SetDeprecated(DbfsDeprecationWarning)
+	s.SchemaPath("init_scripts", "dbfs", "destination").SetRequired()
+	s.SchemaPath("init_scripts", "s3", "destination").SetRequired()
+	s.SchemaPath("init_scripts", "volumes", "destination").SetRequired()
+	s.SchemaPath("init_scripts", "workspace", "destination").SetRequired()
+	s.SchemaPath("workload_type", "clients").SetRequired()
+	s.SchemaPath("workload_type", "clients", "notebooks").SetDefault(true)
+	s.SchemaPath("workload_type", "clients", "jobs").SetDefault(true)
+	s.SchemaPath("library").Schema.Set = func(i any) int {
 		lib := libraries.NewLibraryFromInstanceState(i)
 		return schema.HashString(lib.String())
 	}
-	common.CustomizeSchemaPath(s).AddNewField("idempotency_token", &schema.Schema{
+	s.AddNewField("idempotency_token", &schema.Schema{
 		Type:     schema.TypeString,
 		Optional: true,
 		ForceNew: true,
 	})
-	common.CustomizeSchemaPath(s, "data_security_mode").SetSuppressDiff()
-	common.CustomizeSchemaPath(s, "docker_image", "url").SetRequired()
-	common.CustomizeSchemaPath(s, "docker_image", "basic_auth", "password").SetRequired().SetSensitive()
-	common.CustomizeSchemaPath(s, "docker_image", "basic_auth", "username").SetRequired()
-	common.CustomizeSchemaPath(s, "spark_conf").SetCustomSuppressDiff(SparkConfDiffSuppressFunc)
-	common.CustomizeSchemaPath(s, "aws_attributes").SetSuppressDiff().SetConflictsWith([]string{"azure_attributes", "gcp_attributes"})
-	common.CustomizeSchemaPath(s, "aws_attributes", "zone_id").SetCustomSuppressDiff(ZoneDiffSuppress)
-	common.CustomizeSchemaPath(s, "azure_attributes").SetSuppressDiff().SetConflictsWith([]string{"aws_attributes", "gcp_attributes"})
-	common.CustomizeSchemaPath(s, "gcp_attributes").SetSuppressDiff().SetConflictsWith([]string{"aws_attributes", "azure_attributes"})
-	common.CustomizeSchemaPath(s, "autotermination_minutes").SetDefault(60)
-	common.CustomizeSchemaPath(s, "autoscale", "max_workers").SetOptional()
-	common.CustomizeSchemaPath(s, "autoscale", "min_workers").SetOptional()
-	common.CustomizeSchemaPath(s, "cluster_log_conf", "dbfs", "destination").SetRequired()
-	common.CustomizeSchemaPath(s, "cluster_log_conf", "s3", "destination").SetRequired()
-	common.CustomizeSchemaPath(s, "spark_version").SetRequired()
-	common.CustomizeSchemaPath(s).AddNewField("cluster_id", &schema.Schema{
+	s.SchemaPath("data_security_mode").SetSuppressDiff()
+	s.SchemaPath("docker_image", "url").SetRequired()
+	s.SchemaPath("docker_image", "basic_auth", "password").SetRequired().SetSensitive()
+	s.SchemaPath("docker_image", "basic_auth", "username").SetRequired()
+	s.SchemaPath("spark_conf").SetCustomSuppressDiff(SparkConfDiffSuppressFunc)
+	s.SchemaPath("aws_attributes").SetSuppressDiff().SetConflictsWith([]string{"azure_attributes", "gcp_attributes"})
+	s.SchemaPath("aws_attributes", "zone_id").SetCustomSuppressDiff(ZoneDiffSuppress)
+	s.SchemaPath("azure_attributes").SetSuppressDiff().SetConflictsWith([]string{"aws_attributes", "gcp_attributes"})
+	s.SchemaPath("gcp_attributes").SetSuppressDiff().SetConflictsWith([]string{"aws_attributes", "azure_attributes"})
+	s.SchemaPath("autotermination_minutes").SetDefault(60)
+	s.SchemaPath("autoscale", "max_workers").SetOptional()
+	s.SchemaPath("autoscale", "min_workers").SetOptional()
+	s.SchemaPath("cluster_log_conf", "dbfs", "destination").SetRequired()
+	s.SchemaPath("cluster_log_conf", "s3", "destination").SetRequired()
+	s.SchemaPath("spark_version").SetRequired()
+	s.AddNewField("cluster_id", &schema.Schema{
 		Type:     schema.TypeString,
 		Computed: true,
 	})
-	common.CustomizeSchemaPath(s).AddNewField("default_tags", &schema.Schema{
+	s.AddNewField("default_tags", &schema.Schema{
 		Type:     schema.TypeMap,
 		Computed: true,
 	})
-	common.CustomizeSchemaPath(s).AddNewField("state", &schema.Schema{
+	s.AddNewField("state", &schema.Schema{
 		Type:     schema.TypeString,
 		Computed: true,
 	})
-	common.CustomizeSchemaPath(s, "instance_pool_id").SetConflictsWith([]string{"driver_node_type_id", "node_type_id"})
-	common.CustomizeSchemaPath(s, "runtime_engine").SetValidateFunc(validation.StringInSlice([]string{"PHOTON", "STANDARD"}, false))
-	common.CustomizeSchemaPath(s).AddNewField("is_pinned", &schema.Schema{
+	s.SchemaPath("instance_pool_id").SetConflictsWith([]string{"driver_node_type_id", "node_type_id"})
+	s.SchemaPath("runtime_engine").SetValidateFunc(validation.StringInSlice([]string{"PHOTON", "STANDARD"}, false))
+	s.AddNewField("is_pinned", &schema.Schema{
 		Type:     schema.TypeBool,
 		Optional: true,
 		Default:  false,
@@ -189,12 +189,12 @@ func (ClusterSpec) CustomizeSchema(s map[string]*schema.Schema) map[string]*sche
 			return old == new
 		},
 	})
-	common.CustomizeSchemaPath(s).AddNewField("url", &schema.Schema{
+	s.AddNewField("url", &schema.Schema{
 		Type:     schema.TypeString,
 		Computed: true,
 	})
-	common.CustomizeSchemaPath(s, "num_workers").SetDefault(0).SetValidateDiagFunc(validation.ToDiagFunc(validation.IntAtLeast(0)))
-	common.CustomizeSchemaPath(s).AddNewField("cluster_mount_info", &schema.Schema{
+	s.SchemaPath("num_workers").SetDefault(0).SetValidateDiagFunc(validation.ToDiagFunc(validation.IntAtLeast(0)))
+	s.AddNewField("cluster_mount_info", &schema.Schema{
 		Type:     schema.TypeList,
 		Optional: true,
 		Elem: &schema.Resource{

clusters/resource_library.go

@@ -16,8 +16,8 @@ type LibraryResource struct {
 	compute.Library
 }
 
-func (LibraryResource) CustomizeSchema(s map[string]*schema.Schema) map[string]*schema.Schema {
-	common.CustomizeSchemaPath(s).AddNewField("cluster_id", &schema.Schema{
+func (LibraryResource) CustomizeSchema(s *common.CustomizableSchema) *common.CustomizableSchema {
+	s.AddNewField("cluster_id", &schema.Schema{
 		Type:     schema.TypeString,
 		Required: true,
 	})

common/customizable_schema.go

@@ -2,7 +2,9 @@ package common
 
 import (
 	"fmt"
+	"log"
 	"slices"
+	"strings"
 
 	"github.com/hashicorp/go-cty/cty"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
@@ -13,6 +15,32 @@ type CustomizableSchema struct {
 	Schema         *schema.Schema
 	path           []string
 	isSuppressDiff bool
+	context        schemaPathContext
+}
+
+func (s *CustomizableSchema) pathContainsMultipleItemsList() bool {
+	schemaPath := s.context.schemaPath
+	for _, scm := range schemaPath {
+		if scm.Type == schema.TypeList && scm.MaxItems != 1 {
+			return true
+		}
+	}
+	return false
+}
+
+// Used to get the prefix path for functions like ConflictsWith, by joining `path` in SchemaPathContext.
+func getPrefixedValue(path []string, value []string) []string {
+	var prefix string
+	if len(path) != 0 {
+		prefix = strings.Join(path, ".") + "."
+	} else {
+		prefix = ""
+	}
+	prefixedPaths := make([]string, len(value))
+	for i, item := range value {
+		prefixedPaths[i] = prefix + item
+	}
+	return prefixedPaths
 }
 
 func CustomizeSchemaPath(s map[string]*schema.Schema, path ...string) *CustomizableSchema {
@@ -30,6 +58,22 @@ func CustomizeSchemaPath(s map[string]*schema.Schema, path ...string) *Customiza
 	return &CustomizableSchema{Schema: sch, path: path}
 }
 
+func (s *CustomizableSchema) SchemaPath(path ...string) *CustomizableSchema {
+	sch := MustSchemaPath(s.GetSchemaMap(), path...)
+	return &CustomizableSchema{Schema: sch, path: path, context: s.context}
+}
+
+func (s *CustomizableSchema) GetSchemaMap() map[string]*schema.Schema {
+	if s.Schema.Elem == nil {
+		panic("Elem of Schema field for CustomizableSchema is nil.")
+	}
+	schemaResource, ok := s.Schema.Elem.(*schema.Resource)
+	if !ok {
+		panic("Elem of Schema field for CustomizableSchema is not a *schema.Resource.")
+	}
+	return schemaResource.Schema
+}
+
 func (s *CustomizableSchema) SetOptional() *CustomizableSchema {
 	s.Schema.Optional = true
 	s.Schema.Required = false
@@ -149,31 +193,47 @@ func (s *CustomizableSchema) SetConflictsWith(value []string) *CustomizableSchem
 	if len(value) == 0 {
 		panic("SetConflictsWith cannot take in an empty list")
 	}
-	s.Schema.ConflictsWith = value
+	if s.pathContainsMultipleItemsList() {
+		log.Printf("[DEBUG] ConflictsWith skipped for %v, path contains TypeList block with MaxItems not equal to 1", strings.Join(s.path, "."))
+		return s
+	}
+	s.Schema.ConflictsWith = getPrefixedValue(s.context.path, value)
 	return s
 }
 
 func (s *CustomizableSchema) SetExactlyOneOf(value []string) *CustomizableSchema {
 	if len(value) == 0 {
 		panic("SetExactlyOneOf cannot take in an empty list")
 	}
-	s.Schema.ExactlyOneOf = value
+	if s.pathContainsMultipleItemsList() {
+		log.Printf("[DEBUG] ExactlyOneOf skipped for %v, path contains TypeList block with MaxItems not equal to 1", strings.Join(s.path, "."))
+		return s
+	}
+	s.Schema.ExactlyOneOf = getPrefixedValue(s.context.path, value)
 	return s
 }
 
 func (s *CustomizableSchema) SetAtLeastOneOf(value []string) *CustomizableSchema {
 	if len(value) == 0 {
 		panic("SetAtLeastOneOf cannot take in an empty list")
 	}
-	s.Schema.AtLeastOneOf = value
+	if s.pathContainsMultipleItemsList() {
+		log.Printf("[DEBUG] AtLeastOneOf skipped for %v, path contains TypeList block with MaxItems not equal to 1", strings.Join(s.path, "."))
+		return s
+	}
+	s.Schema.AtLeastOneOf = getPrefixedValue(s.context.path, value)
 	return s
 }
 
 func (s *CustomizableSchema) SetRequiredWith(value []string) *CustomizableSchema {
 	if len(value) == 0 {
 		panic("SetRequiredWith cannot take in an empty list")
 	}
-	s.Schema.RequiredWith = value
+	if s.pathContainsMultipleItemsList() {
+		log.Printf("[DEBUG] SetRequiredWith skipped for %v, path contains TypeList block with MaxItems not equal to 1", strings.Join(s.path, "."))
+		return s
+	}
+	s.Schema.RequiredWith = getPrefixedValue(s.context.path, value)
 	return s
 }
 

common/customizable_schema_test.go

@@ -166,6 +166,34 @@ func TestCustomizableSchemaSetConflictsWith(t *testing.T) {
 	assert.Truef(t, len(testCustomizableSchemaScm["non_optional"].ConflictsWith) == 1, "conflictsWith should be set in field: non_optional")
 }
 
+func TestCustomizableSchemaSetConflictsWith_PathInContext(t *testing.T) {
+	fakeContextWithPath := schemaPathContext{
+		path:       []string{"a", "0", "b"},
+		schemaPath: []*schema.Schema{},
+	}
+	cs := CustomizeSchemaPath(testCustomizableSchemaScm, "float")
+	cs.context = fakeContextWithPath
+	cs.SetConflictsWith([]string{"abc"})
+	assert.Truef(t, len(testCustomizableSchemaScm["float"].ConflictsWith) == 1, "conflictsWith should be set in field: float")
+	assert.Truef(t, cs.Schema.ConflictsWith[0] == "a.0.b.abc", "conflictsWith should be set with the correct prefix")
+}
+
+func TestCustomizableSchemaSetConflictsWith_MultiItemList(t *testing.T) {
+	fakeContextWithPath := schemaPathContext{
+		path: []string{"a", "0", "b"},
+		schemaPath: []*schema.Schema{
+			{
+				Type:     schema.TypeList,
+				MaxItems: 10,
+			},
+		},
+	}
+	cs := CustomizeSchemaPath(testCustomizableSchemaScm, "bool")
+	cs.context = fakeContextWithPath
+	cs.SetConflictsWith([]string{"abc"})
+	assert.Truef(t, len(testCustomizableSchemaScm["bool"].ConflictsWith) == 0, "conflictsWith should not be set when there's multi item list in the path")
+}
+
 func TestCustomizableSchemaSetExactlyOneOf(t *testing.T) {
 	CustomizeSchemaPath(testCustomizableSchemaScm, "non_optional").SetExactlyOneOf([]string{"abc"})
 	assert.Truef(t, len(testCustomizableSchemaScm["non_optional"].ExactlyOneOf) == 1, "ExactlyOneOf should be set in field: non_optional")