Skip to content

Commit 0a7bd1a

Browse files
authored
Explicit copy of structs in tnresources (#3352)
## Changes - In terranova/tnresources, copy structs field-by-field instead of with copyViaJSON helper. - Enable exhaustruct linter on tnresources to ensure that all fields are listed. ## Why - With this approach we need to name every field, highlighting unused ones. - In case SDK adds new fields to request types, we would be alerted and would need to add them manually. - We don't rely on json annotation here, might be useful in the future if SDK removes that. Note, we still don't have a check that we use every field _from_ config. Probably need a custom linter for that. ## Tests Existing tests. Manually commented out fields to ensure that linter catches that.
1 parent 9ba2186 commit 0a7bd1a

File tree

8 files changed

+124
-40
lines changed

8 files changed

+124
-40
lines changed

.golangci.yaml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ linters:
1414
- testifylint
1515
- unconvert
1616
- unused
17+
- exhaustruct
1718
settings:
1819
copyloopvar:
1920
check-alias: true
@@ -73,6 +74,10 @@ linters:
7374
- common-false-positives
7475
- legacy
7576
- std-error-handling
77+
rules:
78+
- path-except: bundle/terranova/tnresources
79+
linters:
80+
- exhaustruct
7681
issues:
7782
max-issues-per-linter: 1000
7883
max-same-issues: 1000

bundle/terranova/tnresources/app.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,9 @@ func (r *ResourceApp) Config() any {
2727

2828
func (r *ResourceApp) DoCreate(ctx context.Context) (string, error) {
2929
request := apps.CreateAppRequest{
30-
App: r.config,
31-
NoCompute: true,
30+
App: r.config,
31+
NoCompute: true,
32+
ForceSendFields: nil,
3233
}
3334
waiter, err := r.client.Apps.Create(ctx, request)
3435
if err != nil {

bundle/terranova/tnresources/job.go

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -73,11 +73,39 @@ func (r *ResourceJob) WaitAfterUpdate(ctx context.Context) error {
7373
}
7474

7575
func makeCreateJob(config jobs.JobSettings) (jobs.CreateJob, error) {
76-
result := jobs.CreateJob{}
76+
result := jobs.CreateJob{
77+
AccessControlList: nil, // Not supported by DABs
78+
BudgetPolicyId: config.BudgetPolicyId,
79+
Continuous: config.Continuous,
80+
Deployment: config.Deployment,
81+
Description: config.Description,
82+
EditMode: config.EditMode,
83+
EmailNotifications: config.EmailNotifications,
84+
Environments: config.Environments,
85+
Format: config.Format,
86+
GitSource: config.GitSource,
87+
Health: config.Health,
88+
JobClusters: config.JobClusters,
89+
MaxConcurrentRuns: config.MaxConcurrentRuns,
90+
Name: config.Name,
91+
NotificationSettings: config.NotificationSettings,
92+
Parameters: config.Parameters,
93+
PerformanceTarget: config.PerformanceTarget,
94+
Queue: config.Queue,
95+
RunAs: config.RunAs,
96+
Schedule: config.Schedule,
97+
Tags: config.Tags,
98+
Tasks: config.Tasks,
99+
TimeoutSeconds: config.TimeoutSeconds,
100+
Trigger: config.Trigger,
101+
WebhookNotifications: config.WebhookNotifications,
102+
103+
ForceSendFields: filterFields[jobs.CreateJob](config.ForceSendFields),
104+
}
105+
77106
// TODO: Validate copy - all fields must be initialized or explicitly allowed to be empty
78107
// Unset AccessControlList
79-
err := copyViaJSON(&result, config)
80-
return result, err
108+
return result, nil
81109
}
82110

83111
func makeResetJob(config jobs.JobSettings, id string) (jobs.ResetJob, error) {

bundle/terranova/tnresources/pipeline.go

Lines changed: 34 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -33,14 +33,42 @@ func (r *ResourcePipeline) DoCreate(ctx context.Context) (string, error) {
3333
}
3434

3535
func (r *ResourcePipeline) DoUpdate(ctx context.Context, id string) error {
36-
request := pipelines.EditPipeline{}
37-
err := copyViaJSON(&request, r.config)
38-
if err != nil {
39-
return err
36+
request := pipelines.EditPipeline{
37+
AllowDuplicateNames: r.config.AllowDuplicateNames,
38+
BudgetPolicyId: r.config.BudgetPolicyId,
39+
Catalog: r.config.Catalog,
40+
Channel: r.config.Channel,
41+
Clusters: r.config.Clusters,
42+
Configuration: r.config.Configuration,
43+
Continuous: r.config.Continuous,
44+
Deployment: r.config.Deployment,
45+
Development: r.config.Development,
46+
Edition: r.config.Edition,
47+
Environment: r.config.Environment,
48+
EventLog: r.config.EventLog,
49+
ExpectedLastModified: 0,
50+
Filters: r.config.Filters,
51+
GatewayDefinition: r.config.GatewayDefinition,
52+
Id: r.config.Id,
53+
IngestionDefinition: r.config.IngestionDefinition,
54+
Libraries: r.config.Libraries,
55+
Name: r.config.Name,
56+
Notifications: r.config.Notifications,
57+
Photon: r.config.Photon,
58+
RestartWindow: r.config.RestartWindow,
59+
RootPath: r.config.RootPath,
60+
RunAs: r.config.RunAs,
61+
Schema: r.config.Schema,
62+
Serverless: r.config.Serverless,
63+
Storage: r.config.Storage,
64+
Tags: r.config.Tags,
65+
Target: r.config.Target,
66+
Trigger: r.config.Trigger,
67+
PipelineId: id,
68+
ForceSendFields: filterFields[pipelines.EditPipeline](r.config.ForceSendFields),
4069
}
41-
request.PipelineId = id
4270

43-
err = r.client.Pipelines.Update(ctx, request)
71+
err := r.client.Pipelines.Update(ctx, request)
4472
if err != nil {
4573
return SDKError{Method: "Pipelines.Update", Err: err}
4674
}

bundle/terranova/tnresources/schema.go

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -34,14 +34,16 @@ func (r *ResourceSchema) DoCreate(ctx context.Context) (string, error) {
3434
}
3535

3636
func (r *ResourceSchema) DoUpdate(ctx context.Context, id string) error {
37-
updateRequest := catalog.UpdateSchema{}
38-
err := copyViaJSON(&updateRequest, r.config)
39-
if err != nil {
40-
return err
37+
updateRequest := catalog.UpdateSchema{
38+
Comment: r.config.Comment,
39+
EnablePredictiveOptimization: "", // Not supported by DABs
40+
FullName: id,
41+
NewName: "", // We recreate schemas on name change intentionally.
42+
Owner: "", // Not supported by DABs
43+
Properties: r.config.Properties,
44+
ForceSendFields: filterFields[catalog.UpdateSchema](r.config.ForceSendFields),
4145
}
4246

43-
updateRequest.FullName = id
44-
4547
response, err := r.client.Schemas.Update(ctx, updateRequest)
4648
if err != nil {
4749
return SDKError{Method: "Schemas.Update", Err: err}
@@ -55,7 +57,11 @@ func (r *ResourceSchema) DoUpdate(ctx context.Context, id string) error {
5557
}
5658

5759
func DeleteSchema(ctx context.Context, client *databricks.WorkspaceClient, id string) error {
58-
err := client.Schemas.Delete(ctx, catalog.DeleteSchemaRequest{FullName: id, Force: true})
60+
err := client.Schemas.Delete(ctx, catalog.DeleteSchemaRequest{
61+
FullName: id,
62+
Force: true,
63+
ForceSendFields: nil,
64+
})
5965
if err != nil {
6066
return SDKError{Method: "Schemas.Delete", Err: err}
6167
}

bundle/terranova/tnresources/sql_warehouse.go

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,12 +35,23 @@ func (r *ResourceSqlWarehouse) DoCreate(ctx context.Context) (string, error) {
3535
}
3636

3737
func (r *ResourceSqlWarehouse) DoUpdate(ctx context.Context, id string) error {
38-
request := sql.EditWarehouseRequest{}
39-
err := copyViaJSON(&request, r.config)
40-
if err != nil {
41-
return err
38+
request := sql.EditWarehouseRequest{
39+
AutoStopMins: r.config.AutoStopMins,
40+
Channel: r.config.Channel,
41+
ClusterSize: r.config.ClusterSize,
42+
CreatorName: r.config.CreatorName,
43+
EnablePhoton: r.config.EnablePhoton,
44+
EnableServerlessCompute: r.config.EnableServerlessCompute,
45+
Id: id,
46+
InstanceProfileArn: r.config.InstanceProfileArn,
47+
MaxNumClusters: r.config.MaxNumClusters,
48+
MinNumClusters: r.config.MinNumClusters,
49+
Name: r.config.Name,
50+
SpotInstancePolicy: r.config.SpotInstancePolicy,
51+
Tags: r.config.Tags,
52+
WarehouseType: sql.EditWarehouseRequestWarehouseType(r.config.WarehouseType),
53+
ForceSendFields: filterFields[sql.EditWarehouseRequest](r.config.ForceSendFields),
4254
}
43-
request.Id = id
4455

4556
waiter, err := r.client.Warehouses.Edit(ctx, request)
4657
if err != nil {
Lines changed: 13 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,20 @@
11
package tnresources
22

33
import (
4-
"encoding/json"
5-
"errors"
6-
"fmt"
4+
"reflect"
75
)
86

9-
// Utility to copy from one type to another based on intermediate JSON transformation
10-
// (e.g. to copy from JobSettings to CreateJob)
11-
func copyViaJSON[T1, T2 any](dest *T1, src T2) error {
12-
if dest == nil {
13-
return errors.New("internal error: unexpected nil")
14-
}
15-
data, err := json.Marshal(src)
16-
if err != nil {
17-
return fmt.Errorf("Failed to serialize %T: %w", src, err)
18-
}
19-
err = json.Unmarshal(data, dest)
20-
if err != nil {
21-
return fmt.Errorf("Failed JSON roundtrip from %T to %T: %w", src, dest, err)
7+
// filterFields creates a new slice with fields present only in the provided type.
8+
// We must use that when copying structs because JSON marshaller in SDK crashes if it sees unknown field.
9+
func filterFields[T any](fields []string) []string {
10+
var result []string
11+
typeOfT := reflect.TypeOf((*T)(nil)).Elem()
12+
13+
for _, field := range fields {
14+
if _, ok := typeOfT.FieldByName(field); ok {
15+
result = append(result, field)
16+
}
2217
}
23-
return nil
18+
19+
return result
2420
}

bundle/terranova/tnresources/volume.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,10 @@ func (r *ResourceVolume) DoUpdate(ctx context.Context, id string) error {
4141
updateRequest := catalog.UpdateVolumeRequestContent{
4242
Comment: r.config.Comment,
4343
Name: id,
44+
NewName: "", // Not supported by Update(). Needs DoUpdateWithID()
45+
Owner: "", // Not supported by DABs
46+
47+
ForceSendFields: nil,
4448
}
4549

4650
nameFromID, err := getNameFromID(id)
@@ -68,6 +72,11 @@ func (r *ResourceVolume) DoUpdateWithID(ctx context.Context, id string) (string,
6872
updateRequest := catalog.UpdateVolumeRequestContent{
6973
Comment: r.config.Comment,
7074
Name: id,
75+
76+
NewName: "", // Initialized below if needed
77+
Owner: "", // Not supported by DABs
78+
79+
ForceSendFields: nil,
7180
}
7281

7382
items := strings.Split(id, ".")

0 commit comments

Comments
 (0)