Handle nested parameters in granted settings set command (#690)

* add more robust way to update settings including nested attributes

* comments

* Add failing test coverage

* fix config setting for keychain

* fix test

---------

Co-authored-by: JoshuaWilkes <[email protected]>

pkg/granted/settings/set.go

@@ -7,6 +7,7 @@ import (
 
 	"github.com/AlecAivazis/survey/v2"
 	"github.com/common-fate/clio"
+	"github.com/common-fate/grab"
 	"github.com/common-fate/granted/pkg/config"
 	"github.com/urfave/cli/v2"
 )
@@ -24,27 +25,21 @@ var SetConfigCommand = cli.Command{
 			return err
 		}
 
-		// Get the type and value of the Config struct
-		configType := reflect.TypeOf(*cfg)
-		configValue := reflect.ValueOf(cfg).Elem()
-		type field struct {
-			ftype  reflect.StructField
-			fvalue reflect.Value
+		fieldMap := FieldOptions(cfg)
+
+		if cfg.Keyring == nil {
+			cfg.Keyring = &config.KeyringConfig{}
 		}
-		var fields []string
-		var fieldMap = make(map[string]field)
-		// Iterate over the fields of the Config struct
-		for i := 0; i < configType.NumField(); i++ {
-			fieldType := configType.Field(i)
-			kind := fieldType.Type.Kind()
-			if kind == reflect.Bool || kind == reflect.String || kind == reflect.Int {
-				fieldValue := configValue.Field(i)
-				fields = append(fields, fieldType.Name)
-				fieldMap[fieldType.Name] = field{
-					fvalue: fieldValue,
-					ftype:  fieldType,
-				}
-			}
+
+		// custom mapping for the keychain fields because the field options generator doesn;t work for nillable fields
+		fieldMap["Keyring.Backend"] = keyringFields{&cfg.Keyring.Backend}
+		fieldMap["Keyring.KeychainName"] = keyringFields{&cfg.Keyring.KeychainName}
+		fieldMap["Keyring.FileDir"] = keyringFields{&cfg.Keyring.FileDir}
+		fieldMap["Keyring.LibSecretCollectionName"] = keyringFields{&cfg.Keyring.LibSecretCollectionName}
+
+		fields := make([]string, 0, len(fieldMap))
+		for k := range fieldMap {
+			fields = append(fields, k)
 		}
 
 		var selectedFieldName = c.String("setting")
@@ -59,21 +54,21 @@ var SetConfigCommand = cli.Command{
 			}
 		}
 
-		var selectedField field
 		var ok bool
-		selectedField, ok = fieldMap[selectedFieldName]
+		selectedField, ok := fieldMap[selectedFieldName]
 		if !ok {
 			return fmt.Errorf("the selected field %s is not a valid config parameter", selectedFieldName)
 		}
 		// Prompt the user to update the field
 		var value interface{}
 		var prompt survey.Prompt
-		switch selectedField.ftype.Type.Kind() {
+
+		switch selectedField.Kind() {
 		case reflect.Bool:
 			if !c.IsSet("value") {
 				prompt = &survey.Confirm{
 					Message: fmt.Sprintf("Enter new value for %s:", selectedFieldName),
-					Default: selectedField.fvalue.Bool(),
+					Default: selectedField.Value().(bool),
 				}
 				err = survey.AskOne(prompt, &value)
 				if err != nil {
@@ -92,7 +87,7 @@ var SetConfigCommand = cli.Command{
 				var str string
 				prompt = &survey.Input{
 					Message: fmt.Sprintf("Enter new value for %s:", selectedFieldName),
-					Default: fmt.Sprintf("%v", selectedField.fvalue.Interface()),
+					Default: selectedField.Value().(string),
 				}
 				err = survey.AskOne(prompt, &str)
 				if err != nil {
@@ -106,7 +101,7 @@ var SetConfigCommand = cli.Command{
 			if !c.IsSet("value") {
 				prompt = &survey.Input{
 					Message: fmt.Sprintf("Enter new value for %s:", selectedFieldName),
-					Default: fmt.Sprintf("%v", selectedField.fvalue.Interface()),
+					Default: fmt.Sprintf("%v", selectedField.Value()),
 				}
 				err = survey.AskOne(prompt, &value)
 				if err != nil {
@@ -121,12 +116,9 @@ var SetConfigCommand = cli.Command{
 			}
 		}
 
-		// Set the new value for the field
-		newValue := reflect.ValueOf(value)
-		if newValue.Type().ConvertibleTo(selectedField.ftype.Type) {
-			selectedField.fvalue.Set(newValue.Convert(selectedField.ftype.Type))
-		} else {
-			return fmt.Errorf("invalid type for %s", selectedField.ftype.Name)
+		err = selectedField.Set(value)
+		if err != nil {
+			return err
 		}
 
 		clio.Infof("Updating the value of %s to %v", selectedFieldName, value)
@@ -138,3 +130,106 @@ var SetConfigCommand = cli.Command{
 		return nil
 	},
 }
+
+type Field interface {
+	Set(value any) error
+	Value() any
+	Kind() reflect.Kind
+}
+
+type keyringFields struct {
+	// double pointer here is a pointer to a pointer value in the config
+	// so that we can initialise it if it is unset
+	field **string
+}
+
+func (f keyringFields) Set(value any) error {
+	if *f.field == nil {
+		*f.field = new(string)
+	}
+	**f.field = value.(string)
+	return nil
+}
+func (f keyringFields) Value() any {
+	return grab.Value(grab.Value(f.field))
+}
+func (f keyringFields) Kind() reflect.Kind {
+	return reflect.String
+}
+
+type field struct {
+	ftype  reflect.StructField
+	fvalue reflect.Value
+}
+
+func (f field) Set(value any) error {
+	// Set the new value for the field
+	newValue := reflect.ValueOf(value)
+	if newValue.Type().ConvertibleTo(f.ftype.Type) {
+		f.fvalue.Set(newValue.Convert(f.ftype.Type))
+	} else {
+		return fmt.Errorf("invalid type for %s", f.ftype.Name)
+	}
+	return nil
+}
+func (f field) Value() any {
+	return f.fvalue.Interface()
+}
+func (f field) Kind() reflect.Kind {
+	return f.ftype.Type.Kind()
+}
+
+// FieldOptions doesn't handle setting nillable fields with no existing value
+// for the keychain, we have a customer mapping to set those
+func FieldOptions(cfg any) map[string]Field {
+	// Get the type and value of the Config struct
+	configType := reflect.TypeOf(cfg)
+	configValue := reflect.ValueOf(cfg)
+
+	// Check if cfg is a pointer to a struct
+	if configType.Kind() == reflect.Ptr && configType.Elem().Kind() == reflect.Struct {
+		configType = configType.Elem()
+		configValue = configValue.Elem()
+	} else if configType.Kind() != reflect.Struct {
+		// cfg is neither a struct nor a pointer to a struct
+		return nil
+	}
+
+	var fieldMap = make(map[string]Field)
+
+	//traverseConfigFields goes through all config variables taking note of each of the types and saves them to the fieldmap
+	//In the case where there are sub fields in the toml config, it is recursively called to traverse the sub config
+	var traverseConfigFields func(reflect.Type, reflect.Value, string)
+	traverseConfigFields = func(t reflect.Type, v reflect.Value, parent string) {
+		for i := 0; i < t.NumField(); i++ {
+			fieldType := t.Field(i)
+			fieldValue := v.Field(i)
+			kind := fieldType.Type.Kind()
+			fieldName := fieldType.Name
+			if parent != "" {
+				fieldName = parent + "." + fieldType.Name
+			}
+
+			//subfield structs reflect as a pointer
+			if kind == reflect.Ptr {
+				// Dereference the pointer to get the underlying value
+				if !fieldValue.IsNil() {
+					fieldValue = fieldValue.Elem()
+					kind = fieldValue.Kind()
+				}
+			}
+
+			if kind == reflect.Bool || kind == reflect.String || kind == reflect.Int {
+				fieldMap[fieldName] = field{
+					ftype:  fieldType,
+					fvalue: fieldValue,
+				}
+			} else if kind == reflect.Struct {
+				traverseConfigFields(fieldValue.Type(), fieldValue, fieldName)
+			}
+		}
+	}
+	traverseConfigFields(configType, configValue, "")
+
+	return fieldMap
+}

pkg/granted/settings/set_test.go

@@ -0,0 +1,64 @@
+package settings
+
+import (
+	"slices"
+	"testing"
+
+	"github.com/common-fate/grab"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestFieldOptions(t *testing.T) {
+	type input struct {
+		A string
+		B struct {
+			C string
+			D *string
+		}
+	}
+	tests := []struct {
+		name  string
+		input any
+		want  []string
+		want1 map[string]field
+	}{
+		{
+			name:  "ok",
+			input: input{},
+			want:  []string{"A", "B.C"},
+		},
+		{
+			name:  "ok",
+			input: &input{},
+			want:  []string{"A", "B.C"},
+		},
+		{
+			name: "ok",
+			input: &input{
+				A: "A",
+				B: struct {
+					C string
+					D *string
+				}{
+					C: "C",
+					D: grab.Ptr("D"),
+				},
+			},
+			want: []string{"A", "B.C", "B.D"},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := FieldOptions(tt.input)
+			keys := make([]string, 0, len(got))
+			for k := range got {
+				keys = append(keys, k)
+			}
+
+			//sort to make sure the keys are in the correct order for the test
+			slices.Sort(keys)
+
+			assert.Equal(t, tt.want, keys)
+		})
+	}
+}