From cffdca25d67f20784f9c5e0f74ed5df2dbdd2599 Mon Sep 17 00:00:00 2001 From: Shawn Wang Date: Wed, 29 May 2024 18:20:20 -0700 Subject: [PATCH] Improve error reporting and error handling in reflect lib This change improves error checking and reporting in the reflect lib. Now an error will be returned on both convert directions if either: - The name of a field is not found in the passed in struct - reflect called panic for whatever operation failed Signed-off-by: Shawn Wang --- nsxt/metadata/metadata.go | 54 ++++++++++++++++--- nsxt/metadata/metadata_test.go | 9 ++-- ...ource_nsxt_policy_mac_discovery_profile.go | 12 +++-- ...ce_nsxt_policy_segment_security_profile.go | 8 ++- 4 files changed, 67 insertions(+), 16 deletions(-) diff --git a/nsxt/metadata/metadata.go b/nsxt/metadata/metadata.go index 4f97ceb9d..50ab7b9fd 100644 --- a/nsxt/metadata/metadata.go +++ b/nsxt/metadata/metadata.go @@ -83,8 +83,15 @@ func GetSchemaFromExtendedSchema(ext map[string]*ExtendedSchema) map[string]*sch // StructToSchema converts NSX model struct to terraform schema // currently supports nested subtype and trivial types -func StructToSchema(elem reflect.Value, d *schema.ResourceData, metadata map[string]*ExtendedSchema, parent string, parentMap map[string]interface{}) { +func StructToSchema(elem reflect.Value, d *schema.ResourceData, metadata map[string]*ExtendedSchema, parent string, parentMap map[string]interface{}) (err error) { ctx := fmt.Sprintf("[from %s]", elem.Type()) + defer func() { + if r := recover(); r != nil { + err = fmt.Errorf("%s recovered from panic: %v", ctx, r) + logger.Printf("[ERROR] %v", err) + } + }() + for key, item := range metadata { if item.Metadata.Skip { continue @@ -94,6 +101,14 @@ func StructToSchema(elem reflect.Value, d *schema.ResourceData, metadata map[str if len(parent) > 0 { logger.Printf("[TRACE] %s parent %s key %s", ctx, parent, key) } + if !elem.FieldByName(item.Metadata.SdkFieldName).IsValid() { + // FieldByName can't find the field by name + logger.Printf("[ERROR] %s skip key %s as %s not found in struct", + ctx, key, item.Metadata.SdkFieldName) + err = fmt.Errorf("%s key %s not found in %s", + ctx, key, item.Metadata.SdkFieldName) + return + } if elem.FieldByName(item.Metadata.SdkFieldName).IsNil() { logger.Printf("[TRACE] %s skip key %s with nil value", ctx, key) continue @@ -102,7 +117,9 @@ func StructToSchema(elem reflect.Value, d *schema.ResourceData, metadata map[str nestedObj := elem.FieldByName(item.Metadata.SdkFieldName) nestedSchema := make(map[string]interface{}) childElem := item.Schema.Elem.(*ExtendedResource) - StructToSchema(nestedObj.Elem(), d, childElem.Schema, key, nestedSchema) + if err = StructToSchema(nestedObj.Elem(), d, childElem.Schema, key, nestedSchema); err != nil { + return + } logger.Printf("[TRACE] %s assigning struct %+v to %s", ctx, nestedObj, key) var nestedSlice []map[string]interface{} nestedSlice = append(nestedSlice, nestedSchema) @@ -127,7 +144,9 @@ func StructToSchema(elem reflect.Value, d *schema.ResourceData, metadata map[str var nestedSlice []map[string]interface{} for i := 0; i < sliceElem.Len(); i++ { nestedSchema := make(map[string]interface{}) - StructToSchema(sliceElem.Index(i), d, childElem.Schema, key, nestedSchema) + if err = StructToSchema(sliceElem.Index(i), d, childElem.Schema, key, nestedSchema); err != nil { + return + } nestedSlice = append(nestedSlice, nestedSchema) logger.Printf("[TRACE] %s appending slice item %+v to %s", ctx, nestedSchema, key) } @@ -149,12 +168,21 @@ func StructToSchema(elem reflect.Value, d *schema.ResourceData, metadata map[str } } } + + return } // SchemaToStruct converts terraform schema to NSX model struct // currently supports nested subtype and trivial types -func SchemaToStruct(elem reflect.Value, d *schema.ResourceData, metadata map[string]*ExtendedSchema, parent string, parentMap map[string]interface{}) { +func SchemaToStruct(elem reflect.Value, d *schema.ResourceData, metadata map[string]*ExtendedSchema, parent string, parentMap map[string]interface{}) (err error) { ctx := fmt.Sprintf("[to %s]", elem.Type()) + defer func() { + if r := recover(); r != nil { + err = fmt.Errorf("%s recovered from panic: %v", ctx, r) + logger.Printf("[ERROR] %v", err) + } + }() + for key, item := range metadata { if item.Metadata.ReadOnly { logger.Printf("[TRACE] %s skip key %s as read only", ctx, key) @@ -168,6 +196,14 @@ func SchemaToStruct(elem reflect.Value, d *schema.ResourceData, metadata map[str logger.Printf("[TRACE] %s skip key %s as NSX does not have support", ctx, key) continue } + if !elem.FieldByName(item.Metadata.SdkFieldName).IsValid() { + // FieldByName can't find the field by name + logger.Printf("[WARN] %s skip key %s as %s not found in struct", + ctx, key, item.Metadata.SdkFieldName) + err = fmt.Errorf("%s key %s not found in %s", + ctx, key, item.Metadata.SdkFieldName) + return + } logger.Printf("[TRACE] %s inspecting key %s with type %s", ctx, key, item.Metadata.SchemaType) if len(parent) > 0 { @@ -217,7 +253,9 @@ func SchemaToStruct(elem reflect.Value, d *schema.ResourceData, metadata map[str nestedSchema := itemList[0].(map[string]interface{}) childElem := item.Schema.Elem.(*ExtendedResource) - SchemaToStruct(nestedObj.Elem(), d, childElem.Schema, key, nestedSchema) + if err = SchemaToStruct(nestedObj.Elem(), d, childElem.Schema, key, nestedSchema); err != nil { + return + } logger.Printf("[TRACE] %s assigning struct %v to %s", ctx, nestedObj, key) elem.FieldByName(item.Metadata.SdkFieldName).Set(nestedObj) } @@ -270,11 +308,15 @@ func SchemaToStruct(elem reflect.Value, d *schema.ResourceData, metadata map[str for i, childItem := range itemList { nestedObj := reflect.New(item.Metadata.ReflectType) nestedSchema := childItem.(map[string]interface{}) - SchemaToStruct(nestedObj.Elem(), d, childElem.Schema, key, nestedSchema) + if err = SchemaToStruct(nestedObj.Elem(), d, childElem.Schema, key, nestedSchema); err != nil { + return + } sliceElem.Index(i).Set(nestedObj.Elem()) logger.Printf("[TRACE] %s appending %+v to %s", ctx, nestedObj.Elem(), key) } } } } + + return } diff --git a/nsxt/metadata/metadata_test.go b/nsxt/metadata/metadata_test.go index 5397daddd..268072cd3 100644 --- a/nsxt/metadata/metadata_test.go +++ b/nsxt/metadata/metadata_test.go @@ -475,7 +475,8 @@ func TestStructToSchema(t *testing.T) { t, testSchema, map[string]interface{}{}) elem := reflect.ValueOf(&obj).Elem() - StructToSchema(elem, d, testExtendedSchema, "", nil) + err := StructToSchema(elem, d, testExtendedSchema, "", nil) + assert.NoError(t, err, "unexpected error calling StructToSchema") t.Run("Base types", func(t *testing.T) { assert.Equal(t, "test_string", d.Get("string_field").(string)) @@ -613,7 +614,8 @@ func TestSchemaToStruct(t *testing.T) { obj := testStruct{} elem := reflect.ValueOf(&obj).Elem() - SchemaToStruct(elem, d, testExtendedSchema, "", nil) + err := SchemaToStruct(elem, d, testExtendedSchema, "", nil) + assert.NoError(t, err, "unexpected error calling SchemaToStruct") nestStr1, nestStr2, nestStr3 := "nested_string_1", "nested_string_2", "nested_string_3" trueVal, falseVal := true, false @@ -697,7 +699,8 @@ func TestSchemaToStructEmptySlice(t *testing.T) { obj := testDeepNestedStruct{} elem := reflect.ValueOf(&obj).Elem() testSch := mixedStructSchema().Elem.(*ExtendedResource).Schema - SchemaToStruct(elem, d, testSch, "", nil) + err := SchemaToStruct(elem, d, testSch, "", nil) + assert.NoError(t, err, "unexpected error calling SchemaToStruct") t.Run("Empty bool list", func(t *testing.T) { assert.NotNil(t, obj.BoolList) diff --git a/nsxt/resource_nsxt_policy_mac_discovery_profile.go b/nsxt/resource_nsxt_policy_mac_discovery_profile.go index 50642b3a0..68ab3216f 100644 --- a/nsxt/resource_nsxt_policy_mac_discovery_profile.go +++ b/nsxt/resource_nsxt_policy_mac_discovery_profile.go @@ -174,7 +174,9 @@ func resourceNsxtPolicyMacDiscoveryProfileCreate(d *schema.ResourceData, m inter } elem := reflect.ValueOf(&obj).Elem() - metadata.SchemaToStruct(elem, d, macDiscoveryProfileSchema, "", nil) + if err := metadata.SchemaToStruct(elem, d, macDiscoveryProfileSchema, "", nil); err != nil { + return err + } // Create the resource using PATCH log.Printf("[INFO] Creating MacDiscoveryProfile with ID %s", id) @@ -213,9 +215,7 @@ func resourceNsxtPolicyMacDiscoveryProfileRead(d *schema.ResourceData, m interfa d.Set("path", obj.Path) elem := reflect.ValueOf(&obj).Elem() - metadata.StructToSchema(elem, d, macDiscoveryProfileSchema, "", nil) - - return nil + return metadata.StructToSchema(elem, d, macDiscoveryProfileSchema, "", nil) } func resourceNsxtPolicyMacDiscoveryProfileUpdate(d *schema.ResourceData, m interface{}) error { @@ -241,7 +241,9 @@ func resourceNsxtPolicyMacDiscoveryProfileUpdate(d *schema.ResourceData, m inter } elem := reflect.ValueOf(&obj).Elem() - metadata.SchemaToStruct(elem, d, macDiscoveryProfileSchema, "", nil) + if err := metadata.SchemaToStruct(elem, d, macDiscoveryProfileSchema, "", nil); err != nil { + return err + } // Update the resource using PATCH boolFalse := false diff --git a/nsxt/resource_nsxt_policy_segment_security_profile.go b/nsxt/resource_nsxt_policy_segment_security_profile.go index 897542cbb..1cf0d5a0c 100644 --- a/nsxt/resource_nsxt_policy_segment_security_profile.go +++ b/nsxt/resource_nsxt_policy_segment_security_profile.go @@ -282,7 +282,9 @@ func resourceNsxtPolicySegmentSecurityProfilePatch(d *schema.ResourceData, m int BpduFilterAllow: bpduFilterAllow, } elem := reflect.ValueOf(&obj).Elem() - metadata.SchemaToStruct(elem, d, segmentSecurityProfileSchema, "", nil) + if err := metadata.SchemaToStruct(elem, d, segmentSecurityProfileSchema, "", nil); err != nil { + return err + } log.Printf("[INFO] Sending SegmentSecurityProfile with ID %s", id) client := infra.NewSegmentSecurityProfilesClient(getSessionContext(d, m), connector) @@ -323,7 +325,9 @@ func resourceNsxtPolicySegmentSecurityProfileRead(d *schema.ResourceData, m inte } elem := reflect.ValueOf(&obj).Elem() - metadata.StructToSchema(elem, d, segmentSecurityProfileSchema, "", nil) + if err := metadata.StructToSchema(elem, d, segmentSecurityProfileSchema, "", nil); err != nil { + return err + } d.Set("display_name", obj.DisplayName) d.Set("description", obj.Description)