Merge pull request #45 from hughdanliu/improper-parameter-logging

Improve error logging when users provide improperly formatted parameters

Author: k8s-ci-robot

pkg/util/util.go

@@ -138,8 +138,10 @@ func ConvertJsonStringToObject(input string, object any) error {
 	return nil
 }
 
-// ConvertObjectType converts a given object into another object.
-// The to object must be passed in by reference
+// ConvertObjectType converts a given object into another object type.
+// This is done by converting the object into a JSON string, then using the
+// JSON string to populate the parameters of a new object.
+// The "to" object must be passed in by reference
 func ConvertObjectType(from any, to any) error {
 	fromString, err := ConvertObjectToJsonString(from)
 	if err != nil {
@@ -158,6 +160,9 @@ func ConvertObjectType(from any, to any) error {
 func RemoveParametersAndPopulateObject(parameters map[string]string, config any) error {
 	from := ConvertStringMapToAny(parameters)
 	err := ConvertObjectType(from, config)
+	if err != nil && strings.Contains(err.Error(), "error calling MarshalJSON") {
+		return identifyImproperlyFormattedParameter(from)
+	}
 
 	for i := 0; i < reflect.TypeOf(config).Elem().NumField(); i++ {
 		name := reflect.TypeOf(config).Elem().Field(i).Name
@@ -210,3 +215,20 @@ func MapCopy(oldMap map[string]string) map[string]string {
 	}
 	return newMap
 }
+
+// identifyImproperlyFormattedParameter is called if the JSON marshal fails to parse the parameter map.
+// It attempts to convert each individual parameter into a JSON string to identify the specific
+// parameter that is improperly formatted on the storage class. This allows us to provide a more
+// granular error message to the user.
+func identifyImproperlyFormattedParameter(parameters map[string]any) error {
+	for key, value := range parameters {
+		parameter := map[string]any{
+			key: value,
+		}
+		_, err := ConvertObjectToJsonString(parameter)
+		if err != nil {
+			return fmt.Errorf("failed to parse parameter key=%s, value=%s with error=%s", key, value, err)
+		}
+	}
+	return fmt.Errorf("failed to parse parameter JSON, but could not determine which parameter could not be parsed")
+}

pkg/util/util_test.go

@@ -19,6 +19,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"reflect"
+	"strings"
 	"testing"
 )
 
@@ -247,3 +248,45 @@ func TestConvertJsonStringToObject(t *testing.T) {
 		})
 	}
 }
+
+func TestIdentifyImproperlyFormattedParameter(t *testing.T) {
+	testCases := []struct {
+		name  string
+		input map[string]string
+		error string
+	}{
+		{
+			name: "Success: Improperly formatted SubnetIds",
+			input: map[string]string{
+				"DeploymentType":            "\"SINGLE_AZ_1\"",
+				"ThroughputCapacity":        "64",
+				"SubnetIds":                 "[\"subnet-016affca9638a1e61\"",
+				"SkipFinalBackupOnDeletion": "true",
+			},
+			error: "failed to parse parameter key=SubnetIds, value=[\"subnet-016affca9638a1e61\"",
+		},
+		{
+			name: "Success: Unable to identify improperly formatted parameter",
+			input: map[string]string{
+				"DeploymentType":            "\"SINGLE_AZ_1\"",
+				"ThroughputCapacity":        "64",
+				"SubnetIds":                 "[\"subnet-016affca9638a1e61\"]",
+				"SkipFinalBackupOnDeletion": "true",
+			},
+			error: "failed to parse parameter JSON, but could not determine which parameter could not be parsed",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			err := identifyImproperlyFormattedParameter(ConvertStringMapToAny(tc.input))
+			if err == nil {
+				t.Fatalf(FailureExpectedError, "identifyImproperlyFormattedParameter")
+			}
+
+			if !strings.Contains(err.Error(), tc.error) {
+				t.Fatalf(FailureWrongResult, "identifyImproperlyFormattedParameter", err.Error(), tc.error)
+			}
+		})
+	}
+}