Added tf:optional tag to simplify the code (#1395)

databricks · Jul 17, 2022 · 96e8825 · 96e8825
1 parent a39cf20
commit 96e8825
Show file tree

Hide file tree

Showing 9 changed files with 84 additions and 47 deletions.
diff --git a/clusters/clusters_api.go b/clusters/clusters_api.go
@@ -188,12 +188,12 @@ type S3StorageInfo struct {
 
 // GcsStorageInfo contains the struct for when storing files in GCS
 type GcsStorageInfo struct {
-	Destination string `json:"destination,omitempty" tf:"optional"`
+	Destination string `json:"destination,omitempty"`
 }
 
 // LocalFileInfo represents a local file on disk, e.g. in a customer's container.
 type LocalFileInfo struct {
-	Destination string `json:"destination,omitempty" tf:"optional"`
+	Destination string `json:"destination,omitempty"`
 }
 
 // StorageInfo contains the struct for either DBFS or S3 storage depending on which one is relevant.
@@ -207,7 +207,7 @@ type InitScriptStorageInfo struct {
 	Dbfs *DbfsStorageInfo `json:"dbfs,omitempty" tf:"group:storage"`
 	Gcs  *GcsStorageInfo  `json:"gcs,omitempty" tf:"group:storage"`
 	S3   *S3StorageInfo   `json:"s3,omitempty" tf:"group:storage"`
-	File *LocalFileInfo   `json:"file,omitempty" tf:"optional"`
+	File *LocalFileInfo   `json:"file,omitempty"`
 }
 
 // SparkNodeAwsAttributes is the struct that determines if the node is a spot instance or not

diff --git a/clusters/data_spark_version.go b/clusters/data_spark_version.go
@@ -27,15 +27,15 @@ type SparkVersionsList struct {
 
 // SparkVersionRequest - filtering request
 type SparkVersionRequest struct {
-	LongTermSupport bool   `json:"long_term_support,omitempty" tf:"optional,default:false"`
-	Beta            bool   `json:"beta,omitempty" tf:"optional,default:false,conflicts:long_term_support"`
-	Latest          bool   `json:"latest,omitempty" tf:"optional,default:true"`
-	ML              bool   `json:"ml,omitempty" tf:"optional,default:false"`
-	Genomics        bool   `json:"genomics,omitempty" tf:"optional,default:false"`
-	GPU             bool   `json:"gpu,omitempty" tf:"optional,default:false"`
-	Scala           string `json:"scala,omitempty" tf:"optional,default:2.12"`
-	SparkVersion    string `json:"spark_version,omitempty" tf:"optional,default:"`
-	Photon          bool   `json:"photon,omitempty" tf:"optional,default:false"`
+	LongTermSupport bool   `json:"long_term_support,omitempty"`
+	Beta            bool   `json:"beta,omitempty" tf:"conflicts:long_term_support"`
+	Latest          bool   `json:"latest,omitempty" tf:"default:true"`
+	ML              bool   `json:"ml,omitempty"`
+	Genomics        bool   `json:"genomics,omitempty"`
+	GPU             bool   `json:"gpu,omitempty"`
+	Scala           string `json:"scala,omitempty" tf:"default:2.12"`
+	SparkVersion    string `json:"spark_version,omitempty"`
+	Photon          bool   `json:"photon,omitempty"`
 	Graviton        bool   `json:"graviton,omitempty"`
 }
 

diff --git a/common/reflect_resource.go b/common/reflect_resource.go
@@ -84,8 +84,21 @@ func StructToSchema(v any, customize func(map[string]*schema.Schema) map[string]
 	return scm
 }
 
-func handleOptional(typeField reflect.StructField, schema *schema.Schema) {
+func isOptional(typeField reflect.StructField) bool {
 	if strings.Contains(typeField.Tag.Get("json"), "omitempty") {
+		return true
+	}
+	tfTags := strings.Split(typeField.Tag.Get("tf"), ",")
+	for _, tag := range tfTags {
+		if tag == "optional" {
+			return true
+		}
+	}
+	return false
+}
+
+func handleOptional(typeField reflect.StructField, schema *schema.Schema) {
+	if isOptional(typeField) {
 		schema.Optional = true
 	} else {
 		schema.Required = true
@@ -284,7 +297,6 @@ func iterFields(rv reflect.Value, path []string, s map[string]*schema.Schema,
 	}
 	for i := 0; i < rv.NumField(); i++ {
 		typeField := rv.Type().Field(i)
-		jsonTag := typeField.Tag.Get("json")
 		fieldName := chooseFieldName(typeField)
 		if fieldName == "-" {
 			continue
@@ -293,7 +305,7 @@ func iterFields(rv reflect.Value, path []string, s map[string]*schema.Schema,
 		if !ok {
 			continue
 		}
-		omitEmpty := strings.Contains(jsonTag, "omitempty")
+		omitEmpty := isOptional(typeField)
 		if omitEmpty && !fieldSchema.Optional {
 			return fmt.Errorf("inconsistency: %s has omitempty, but is not optional", fieldName)
 		}

diff --git a/common/reflect_resource_test.go b/common/reflect_resource_test.go
@@ -66,6 +66,7 @@ type testStruct struct {
 	IntSlice       []int             `json:"int_slice,omitempty"`
 	FloatSlice     []float64         `json:"float_slice,omitempty"`
 	BoolSlice      []bool            `json:"bool_slice,omitempty"`
+	TfOptional     string            `json:"tf_optional" tf:"optional"`
 	Hidden         string            `json:"-"`
 	Hidden2        string
 }
@@ -74,10 +75,10 @@ var scm = StructToSchema(testStruct{}, nil)
 
 var testStructFields = []string{"integer", "float", "non_optional", "string", "computed_field", "force_new_field", "map_field",
 	"slice_set_struct", "slice_set_string", "ptr_item", "string_slice", "bool", "int_slice", "float_slice",
-	"bool_slice"}
+	"bool_slice", "tf_optional"}
 
 var testStructOptionalFields = []string{"integer", "float", "string", "computed_field", "force_new_field", "map_field", "slice_set_struct",
-	"ptr_item", "slice_set_string", "bool", "int_slice", "float_slice", "bool_slice"}
+	"ptr_item", "slice_set_string", "bool", "int_slice", "float_slice", "bool_slice", "tf_optional"}
 
 var testStructRequiredFields = []string{"non_optional"}
 

diff --git a/mlflow/resource_mlflow_webhook.go b/mlflow/resource_mlflow_webhook.go
@@ -11,7 +11,7 @@ import (
 
 type HttpUrlSpec struct {
 	URL                   string `json:"url"`
-	EnableSslVerification bool   `json:"enable_ssl_verification" tf:"default:true"`
+	EnableSslVerification bool   `json:"enable_ssl_verification" tf:"optional,default:true"`
 	Secret                string `json:"string,omitempty"`
 	Authorization         string `json:"authorization,omitempty" tf:"sensitive"`
 }
@@ -91,10 +91,6 @@ func ResourceMlflowWebhook() *schema.Resource {
 			m["status"].ValidateFunc = validation.StringInSlice([]string{"ACTIVE", "TEST_MODE", "DISABLED"}, true)
 			// TODO: do we need a validation for Events?
 			delete(m, "id")
-			if p, err := common.SchemaPath(m, "http_url_spec", "enable_ssl_verification"); err == nil {
-				p.Required = false
-				p.Optional = true
-			}
 			if p, err := common.SchemaPath(m, "http_url_spec", "url"); err == nil {
 				p.ValidateFunc = validation.IsURLWithHTTPS
 			}

diff --git a/mws/resource_mws_workspaces.go b/mws/resource_mws_workspaces.go
@@ -91,7 +91,7 @@ type Workspace struct {
 	PricingTier                         string                `json:"pricing_tier,omitempty" tf:"computed"`
 	PrivateAccessSettingsID             string                `json:"private_access_settings_id,omitempty"`
 	NetworkID                           string                `json:"network_id,omitempty"`
-	IsNoPublicIPEnabled                 bool                  `json:"is_no_public_ip_enabled"`
+	IsNoPublicIPEnabled                 bool                  `json:"is_no_public_ip_enabled" tf:"optional,default:true"`
 	WorkspaceID                         int64                 `json:"workspace_id,omitempty" tf:"computed"`
 	WorkspaceURL                        string                `json:"workspace_url,omitempty" tf:"computed"`
 	WorkspaceStatus                     string                `json:"workspace_status,omitempty" tf:"computed"`
@@ -444,12 +444,6 @@ func ResourceMwsWorkspaces() *schema.Resource {
 				// https://github.com/databricks/terraform-provider-databricks/issues/382
 				return !strings.HasSuffix(new, old)
 			}
-			// It cannot be marked as `omitempty` in the struct annotation because Go's JON marshaller
-			// skips booleans set to `false` if set. Thus, we mark it optional here.
-			s["is_no_public_ip_enabled"].Optional = true
-			s["is_no_public_ip_enabled"].Required = false
-			// The API defaults this field to `true`. Apply the same behavior here.
-			s["is_no_public_ip_enabled"].Default = true
 			// The value of `is_no_public_ip_enabled` isn't part of the GET payload.
 			// Keep diff when creating (i.e. `old` == ""), suppress diff otherwise.
 			s["is_no_public_ip_enabled"].DiffSuppressFunc = func(k, old, new string, d *schema.ResourceData) bool {

diff --git a/sql/resource_sql_endpoint.go b/sql/resource_sql_endpoint.go
@@ -24,11 +24,11 @@ type SQLEndpoint struct {
 	ID                      string          `json:"id,omitempty" tf:"computed"`
 	Name                    string          `json:"name"`
 	ClusterSize             string          `json:"cluster_size"`
-	AutoStopMinutes         int             `json:"auto_stop_mins" tf:"default:120"`
+	AutoStopMinutes         int             `json:"auto_stop_mins" tf:"optional,default:120"`
 	MinNumClusters          int             `json:"min_num_clusters,omitempty" tf:"default:1,suppress_diff"`
 	MaxNumClusters          int             `json:"max_num_clusters,omitempty" tf:"default:1"`
 	NumClusters             int             `json:"num_clusters,omitempty" tf:"default:1,suppress_diff"`
-	EnablePhoton            bool            `json:"enable_photon" tf:"default:true"`
+	EnablePhoton            bool            `json:"enable_photon" tf:"optional,default:true"`
 	EnableServerlessCompute bool            `json:"enable_serverless_compute,omitempty" tf:"suppress_diff"`
 	InstanceProfileARN      string          `json:"instance_profile_arn,omitempty"`
 	State                   string          `json:"state,omitempty" tf:"computed"`
@@ -194,14 +194,10 @@ func (a SQLEndpointsAPI) Delete(endpointID string) error {
 func ResourceSqlEndpoint() *schema.Resource {
 	s := common.StructToSchema(SQLEndpoint{}, func(
 		m map[string]*schema.Schema) map[string]*schema.Schema {
-		m["auto_stop_mins"].Required = false
-		m["auto_stop_mins"].Optional = true
 		m["cluster_size"].ValidateDiagFunc = validation.ToDiagFunc(
 			validation.StringInSlice(ClusterSizes, false))
 		m["max_num_clusters"].ValidateDiagFunc = validation.ToDiagFunc(
 			validation.IntBetween(1, MaxNumClusters))
-		m["enable_photon"].Optional = true
-		m["enable_photon"].Required = false
 		return m
 	})
 	return common.Resource{

diff --git a/sql/resource_sql_endpoint_test.go b/sql/resource_sql_endpoint_test.go
@@ -113,6 +113,54 @@ func TestResourceSQLEndpointCreate(t *testing.T) {
 	assert.Equal(t, "d7c9d05c-7496-4c69-b089-48823edad40c", d.Get("data_source_id"))
 }
 
+func TestResourceSQLEndpointCreateNoAutoTermination(t *testing.T) {
+	d, err := qa.ResourceFixture{
+		Fixtures: []qa.HTTPFixture{
+			{
+				Method:   "POST",
+				Resource: "/api/2.0/sql/warehouses",
+				ExpectedRequest: SQLEndpoint{
+					Name:               "foo",
+					ClusterSize:        "Small",
+					MaxNumClusters:     1,
+					AutoStopMinutes:    0,
+					MinNumClusters:     1,
+					NumClusters:        1,
+					EnablePhoton:       true,
+					SpotInstancePolicy: "COST_OPTIMIZED",
+				},
+				Response: SQLEndpoint{
+					ID: "abc",
+				},
+			},
+			{
+				Method:       "GET",
+				Resource:     "/api/2.0/sql/warehouses/abc",
+				ReuseRequest: true,
+				Response: SQLEndpoint{
+					Name:           "foo",
+					ClusterSize:    "Small",
+					ID:             "abc",
+					State:          "RUNNING",
+					Tags:           &Tags{},
+					MaxNumClusters: 1,
+				},
+			},
+			dataSourceListHTTPFixture,
+		},
+		Resource: ResourceSqlEndpoint(),
+		Create:   true,
+		HCL: `
+		name = "foo"
+  		cluster_size = "Small"
+		auto_stop_mins = 0
+		`,
+	}.Apply(t)
+	require.NoError(t, err, err)
+	assert.Equal(t, "abc", d.Id(), "Id should not be empty")
+	assert.Equal(t, "d7c9d05c-7496-4c69-b089-48823edad40c", d.Get("data_source_id"))
+}
+
 func TestResourceSQLEndpointCreate_ErrorDisabled(t *testing.T) {
 	qa.ResourceFixture{
 		Fixtures: []qa.HTTPFixture{

diff --git a/sql/resource_sql_widget.go b/sql/resource_sql_widget.go
@@ -32,8 +32,8 @@ type WidgetEntity struct {
 type WidgetPosition struct {
 	SizeX      int  `json:"size_x"`
 	SizeY      int  `json:"size_y"`
-	PosX       int  `json:"pos_x" tf:"default:0"`
-	PosY       int  `json:"pos_y" tf:"default:0"`
+	PosX       int  `json:"pos_x" tf:"optional,default:0"`
+	PosY       int  `json:"pos_y" tf:"optional,default:0"`
 	AutoHeight bool `json:"auto_height,omitempty"`
 }
 
@@ -267,16 +267,6 @@ func ResourceSqlWidget() *schema.Resource {
 			m["visualization_id"].DiffSuppressFunc = func(_, old, new string, d *schema.ResourceData) bool {
 				return extractVisualizationID(old) == extractVisualizationID(new)
 			}
-			p, err := common.SchemaPath(m, "position", "pos_x")
-			if err == nil {
-				p.Optional = true
-				p.Required = false
-			}
-			p, err = common.SchemaPath(m, "position", "pos_y")
-			if err == nil {
-				p.Optional = true
-				p.Required = false
-			}
 
 			return m
 		})