fix: properly handle maps in unmarshaller (#697)

Author: loicbourgois

internal/args/args_test.go

@@ -81,6 +81,13 @@ type Enum struct {
 	Size  Size
 }
 
+type RecursiveWithMapOfRecursive struct {
+	ID       int
+	Name     string
+	Short    string
+	Elements map[string]*RecursiveWithMapOfRecursive
+}
+
 func (c *CustomStruct) UnmarshalArgs(value string) error {
 	c.value = strings.ToUpper(value)
 	return nil

internal/args/unmarshal.go

@@ -213,10 +213,17 @@ func set(dest reflect.Value, argNameWords []string, value string) error {
 		if len(argNameWords) == 0 {
 			return &MissingMapKeyError{}
 		}
-		// Create a new value call set and add result in the map
-		newValue := reflect.New(dest.Type().Elem())
-		err := set(newValue.Elem(), argNameWords[1:], value)
-		dest.SetMapIndex(reflect.ValueOf(argNameWords[0]), newValue.Elem())
+
+		// Create a new value if it does not exist, then call set and add result in the map
+		mapKey := reflect.ValueOf(argNameWords[0])
+		mapValue := dest.MapIndex(mapKey)
+
+		if !mapValue.IsValid() {
+			mapValue = reflect.New(dest.Type().Elem()).Elem()
+		}
+		err := set(mapValue, argNameWords[1:], value)
+		dest.SetMapIndex(mapKey, mapValue)
+
 		return err
 
 	case reflect.Struct:

internal/args/unmarshal_test.go

@@ -331,6 +331,52 @@ func TestUnmarshalStruct(t *testing.T) {
 			All: "all",
 		},
 	}))
+
+	t.Run("recursive-with-map-of-recursive-with-one-field-set", run(TestCase{
+		args: []string{
+			"name=coucou",
+			"elements.0.name=bob",
+			"elements.0.elements.plop.name=world",
+		},
+		expected: &RecursiveWithMapOfRecursive{
+			Name: "coucou",
+			Elements: map[string]*RecursiveWithMapOfRecursive{
+				"0": {
+					Name: "bob",
+					Elements: map[string]*RecursiveWithMapOfRecursive{
+						"plop": {
+							Name: "world",
+						},
+					},
+				},
+			},
+		},
+	}))
+
+	t.Run("recursive-with-map-of-recursive-with-multiple-fields-set", run(TestCase{
+		args: []string{
+			"name=coucou",
+			"elements.0.id=1453",
+			"elements.0.name=bob",
+			"elements.0.elements.plop.name=world",
+			"elements.0.elements.plop.short=long",
+		},
+		expected: &RecursiveWithMapOfRecursive{
+			Name: "coucou",
+			Elements: map[string]*RecursiveWithMapOfRecursive{
+				"0": {
+					ID:   1453,
+					Name: "bob",
+					Elements: map[string]*RecursiveWithMapOfRecursive{
+						"plop": {
+							Name:  "world",
+							Short: "long",
+						},
+					},
+				},
+			},
+		},
+	}))
 }
 
 func TestIsUmarshalableValue(t *testing.T) {

internal/namespaces/instance/v1/custom_server_create.go

@@ -259,8 +259,12 @@ func instanceServerCreateRun(ctx context.Context, argsI interface{}) (i interfac
 		}
 
 		// Validate root volume type and size.
-		if err := validateRootVolume(getImageResponse.Image.RootVolume.Size, volumes["0"]); err != nil {
-			return nil, err
+		if getImageResponse != nil {
+			if err := validateRootVolume(getImageResponse.Image.RootVolume.Size, volumes["0"]); err != nil {
+				return nil, err
+			}
+		} else {
+			logger.Warningf("skipping root volume validation")
 		}
 
 		// Validate total local volume sizes.