Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate SQL Warehouse to Go SDK #3044

Merged
merged 39 commits into from
Jan 10, 2024
Merged
Show file tree
Hide file tree
Changes from 34 commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
8ca7989
Migrate SQL Warehouse to Go SDK
mgyucht Dec 19, 2023
3ab53a2
fix compile
mgyucht Dec 20, 2023
97f0f43
check in diff-schema scrip
mgyucht Dec 20, 2023
a69d5f7
Merge branch 'master' into migrate-sql-warehouse-to-go-sdk
mgyucht Dec 20, 2023
69b892b
fix
mgyucht Dec 20, 2023
426598b
fix
mgyucht Dec 20, 2023
97b0643
test fix
mgyucht Dec 20, 2023
a6e5fb5
fix
mgyucht Dec 20, 2023
05184aa
fix
mgyucht Dec 20, 2023
4f62c47
Merge branch 'master' into migrate-sql-warehouse-to-go-sdk
mgyucht Dec 20, 2023
62e5c44
undo extra change
mgyucht Dec 20, 2023
dcc0d59
work
mgyucht Dec 20, 2023
ff1234d
work
mgyucht Dec 20, 2023
faeb466
work
mgyucht Dec 20, 2023
d80936c
fix
mgyucht Dec 20, 2023
6f1218e
fixes
mgyucht Dec 20, 2023
8cb7951
fix
mgyucht Dec 20, 2023
045a26f
remove
mgyucht Dec 21, 2023
5079eca
handle slices as well
mgyucht Dec 21, 2023
e8ecc58
Merge branch 'master' into migrate-sql-warehouse-to-go-sdk
mgyucht Dec 21, 2023
8c113a5
fix stubs in exporter test
mgyucht Dec 21, 2023
3f3aca5
test
mgyucht Dec 21, 2023
0c7d9c6
Fix tags diff suppression
mgyucht Dec 21, 2023
a45c93b
fix
mgyucht Dec 21, 2023
1687e2f
Address comments
mgyucht Dec 22, 2023
fc902b3
comments
mgyucht Dec 22, 2023
387ed42
fix workflow
mgyucht Dec 22, 2023
8ecc720
fix
mgyucht Dec 22, 2023
f1dae30
Merge branch 'master' into migrate-sql-warehouse-to-go-sdk
mgyucht Dec 22, 2023
adb8ed1
only diff against merge-base of master
mgyucht Dec 22, 2023
63ae324
update docs
mgyucht Dec 22, 2023
e08fbe8
remove deprecated resources
mgyucht Dec 22, 2023
20c3d14
undo extra docs changes
mgyucht Dec 22, 2023
61ed26e
remove extra space
mgyucht Dec 22, 2023
a33b53d
Merge branch 'master' into migrate-sql-warehouse-to-go-sdk
mgyucht Jan 5, 2024
a3f415d
small tweak
mgyucht Jan 8, 2024
0a069b7
small tweaks
mgyucht Jan 8, 2024
56e3b46
Merge branch 'master' into migrate-sql-warehouse-to-go-sdk
mgyucht Jan 9, 2024
ce5cec7
tests and address comments
mgyucht Jan 9, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 10 additions & 7 deletions .github/workflows/schema.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,12 @@ on:
workflow_dispatch:
inputs:
base:
description: 'Base ref'
default: 'master'
description: Base ref
default: master
required: true
head:
description: 'Head ref'
default: 'master'
description: Head ref
default: master
required: true

jobs:
Expand All @@ -30,17 +30,20 @@ jobs:
with:
ref: ${{ github.event.inputs.base }}

- name: 'Setup Go'
- name: Fetch master
run: git fetch origin master --depth 1

- name: Setup Go
uses: actions/setup-go@v4
with:
go-version: 1.21.x

- name: 'Setup Terraform'
- name: Setup Terraform
uses: hashicorp/setup-terraform@v2
with:
terraform_wrapper: false

- name: 'Install jd'
- name: Install jd
run: go install github.com/josephburnett/jd@latest

- run: make diff-schema
112 changes: 93 additions & 19 deletions common/reflect_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,13 +77,44 @@ func MustSchemaPath(s map[string]*schema.Schema, path ...string) *schema.Schema
// StructToSchema makes schema from a struct type & applies customizations from callback given
func StructToSchema(v any, customize func(map[string]*schema.Schema) map[string]*schema.Schema) map[string]*schema.Schema {
rv := reflect.ValueOf(v)
scm := typeToSchema(rv, rv.Type(), []string{})
scm := typeToSchema(rv, []string{})
if customize != nil {
scm = customize(scm)
}
return scm
}

// SetSuppressDiff adds diff suppression to a schema. This is necessary for non-computed
// fields for which the platform returns a value, but the user has not configured any value.
// For example: the REST API returns `{"tags": {}}` for a resource with no tags.
func SetSuppressDiff(v *schema.Schema) {
v.DiffSuppressFunc = diffSuppressor(fmt.Sprintf("%v", v.Type.Zero()))
}

// SetDefault sets the default value for a schema.
func SetDefault(v *schema.Schema, value any) {
v.Default = value
v.Optional = true
v.Required = false
}

// SetReadOnly sets the schema to be read-only (i.e. computed, non-optional).
// This should be used for fields that are not user-configurable but are returned
// by the platform.
func SetReadOnly(v *schema.Schema) {
v.Optional = false
v.Required = false
v.MaxItems = 0
v.Computed = true
}

// SetRequired sets the schema to be required.
func SetRequired(v *schema.Schema) {
v.Optional = false
v.Required = true
v.Computed = false
}

func isOptional(typeField reflect.StructField) bool {
if strings.Contains(typeField.Tag.Get("json"), "omitempty") {
return true
Expand Down Expand Up @@ -175,26 +206,52 @@ func diffSuppressor(zero string) func(k, old, new string, d *schema.ResourceData
log.Printf("[DEBUG] Suppressing diff for %v: platform=%#v config=%#v", k, old, new)
return true
}
if strings.HasSuffix(k, ".#") && new == "0" && old != "0" {
field := strings.TrimSuffix(k, ".#")
log.Printf("[DEBUG] Suppressing diff for list or set %v: no value configured but platform returned some value (likely {})", field)
return true
}
return false
}
}

type field struct {
sf reflect.StructField
v reflect.Value
}

func listAllFields(v reflect.Value) []field {
t := v.Type()
fields := make([]field, 0)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about doing

fields := make([]field, 0, v.NumField())

to avoid reallocations?

for i := 0; i < v.NumField(); i++ {
f := t.Field(i)
if f.Anonymous {
fields = append(fields, listAllFields(v.Field(i))...)
} else {
fields = append(fields, field{
sf: f,
v: v.Field(i),
})
}
}
return fields
}

// typeToSchema converts struct into terraform schema. `path` is used for block suppressions
// special path element `"0"` is used to denote either arrays or sets of elements
func typeToSchema(v reflect.Value, t reflect.Type, path []string) map[string]*schema.Schema {
func typeToSchema(v reflect.Value, path []string) map[string]*schema.Schema {
scm := map[string]*schema.Schema{}
rk := v.Kind()
if rk == reflect.Ptr {
v = v.Elem()
t = v.Type()
rk = v.Kind()
}
if rk != reflect.Struct {
panic(fmt.Errorf("Schema value of Struct is expected, but got %s: %#v", reflectKind(rk), v))
}
for i := 0; i < v.NumField(); i++ {
typeField := t.Field(i)

fields := listAllFields(v)
for _, field := range fields {
typeField := field.sf
tfTag := typeField.Tag.Get("tf")

fieldName := chooseFieldName(typeField)
Expand Down Expand Up @@ -260,7 +317,7 @@ func typeToSchema(v reflect.Value, t reflect.Type, path []string) map[string]*sc
scm[fieldName].Type = schema.TypeList
elem := typeField.Type.Elem()
sv := reflect.New(elem).Elem()
nestedSchema := typeToSchema(sv, elem, append(path, fieldName, "0"))
nestedSchema := typeToSchema(sv, append(path, fieldName, "0"))
if strings.Contains(tfTag, "suppress_diff") {
blockCount := strings.Join(append(path, fieldName, "#"), ".")
scm[fieldName].DiffSuppressFunc = makeEmptyBlockSuppressFunc(blockCount)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the new suffix checking in diffSuppressor, can we replace makeEmptyBlockSuppressFunc calls?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can do this as a separate PR?

Expand All @@ -279,7 +336,7 @@ func typeToSchema(v reflect.Value, t reflect.Type, path []string) map[string]*sc
elem := typeField.Type // changed from ptr
sv := reflect.New(elem) // changed from ptr

nestedSchema := typeToSchema(sv, elem, append(path, fieldName, "0"))
nestedSchema := typeToSchema(sv, append(path, fieldName, "0"))
if strings.Contains(tfTag, "suppress_diff") {
blockCount := strings.Join(append(path, fieldName, "#"), ".")
scm[fieldName].DiffSuppressFunc = makeEmptyBlockSuppressFunc(blockCount)
Expand Down Expand Up @@ -310,7 +367,7 @@ func typeToSchema(v reflect.Value, t reflect.Type, path []string) map[string]*sc
case reflect.Struct:
sv := reflect.New(elem).Elem()
scm[fieldName].Elem = &schema.Resource{
Schema: typeToSchema(sv, elem, append(path, fieldName, "0")),
Schema: typeToSchema(sv, append(path, fieldName, "0")),
}
}
default:
Expand Down Expand Up @@ -341,6 +398,20 @@ func IsRequestEmpty(v any) (bool, error) {
return !isNotEmpty, err
}

// isGoSdk returns true if the struct is from databricks-sdk-go or embeds a struct from databricks-sdk-go.
func isGoSdk(v reflect.Value) bool {
if strings.Contains(v.Type().PkgPath(), "databricks-sdk-go") {
return true
}
for i := 0; i < v.NumField(); i++ {
f := v.Type().Field(i)
if f.Anonymous && isGoSdk(v.Field(i)) {
return true
}
}
return false
}

func iterFields(rv reflect.Value, path []string, s map[string]*schema.Schema,
cb func(fieldSchema *schema.Schema, path []string, valueField *reflect.Value) error) error {
rk := rv.Kind()
Expand All @@ -350,9 +421,10 @@ func iterFields(rv reflect.Value, path []string, s map[string]*schema.Schema,
if !rv.IsValid() {
return fmt.Errorf("%s: got invalid reflect value %#v", path, rv)
}
isGoSDK := strings.Contains(rv.Type().PkgPath(), "databricks-sdk-go")
for i := 0; i < rv.NumField(); i++ {
typeField := rv.Type().Field(i)
isGoSDK := isGoSdk(rv)
fields := listAllFields(rv)
for _, field := range fields {
typeField := field.sf
fieldName := chooseFieldName(typeField)
if fieldName == "-" {
continue
Expand All @@ -370,7 +442,7 @@ func iterFields(rv reflect.Value, path []string, s map[string]*schema.Schema,
if fieldSchema.Optional && defaultEmpty && !omitEmpty {
return fmt.Errorf("inconsistency: %s is optional, default is empty, but has no omitempty", fieldName)
}
valueField := rv.Field(i)
valueField := field.v
err := cb(fieldSchema, append(path, fieldName), &valueField)
if err != nil {
return fmt.Errorf("%s: %s", fieldName, err)
Expand All @@ -393,13 +465,15 @@ func collectionToMaps(v any, s *schema.Schema) ([]any, error) {
return nil, fmt.Errorf("not resource")
}
var allItems []reflect.Value
if s.MaxItems == 1 {
allItems = append(allItems, reflect.ValueOf(v))
} else {
vs := reflect.ValueOf(v)
for i := 0; i < vs.Len(); i++ {
allItems = append(allItems, vs.Index(i))
rv := reflect.ValueOf(v)
rvType := rv.Type().Kind()
isList := rvType == reflect.Array || rvType == reflect.Slice
if isList {
for i := 0; i < rv.Len(); i++ {
allItems = append(allItems, rv.Index(i))
}
} else {
allItems = append(allItems, rv)
}
for _, v := range allItems {
data := map[string]any{}
Expand Down
4 changes: 2 additions & 2 deletions common/reflect_resource_test.go
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mgyucht I think that we need to add at least two new test cases:

  1. For inherited structure - so we can see that it's flattened correctly into schema
  2. For a structure with a field from Go SDK

Original file line number Diff line number Diff line change
Expand Up @@ -444,7 +444,7 @@ func TestTypeToSchemaNoStruct(t *testing.T) {
fmt.Sprintf("%s", p))
}()
v := reflect.ValueOf(1)
typeToSchema(v, v.Type(), []string{})
typeToSchema(v, []string{})
}

func TestTypeToSchemaUnsupported(t *testing.T) {
Expand All @@ -457,7 +457,7 @@ func TestTypeToSchemaUnsupported(t *testing.T) {
New chan int `json:"new"`
}
v := reflect.ValueOf(nonsense{})
typeToSchema(v, v.Type(), []string{})
typeToSchema(v, []string{})
}

type data map[string]any
Expand Down
Loading
Loading