From 5a3eda4cba4ecf78869ab1f3d069c49cf3c8fd00 Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Fri, 3 May 2024 17:30:07 +0200 Subject: [PATCH] Use : as a config --set list separator This allows settings such as: nvidia-ctk config --set nvidia-container-runtime.runtimes=crun:runc to be applied correctly. Signed-off-by: Evan Lezar --- cmd/nvidia-ctk/config/config.go | 34 +++++++++++++++++++---- cmd/nvidia-ctk/config/config_test.go | 41 ++++++++++++++++++---------- 2 files changed, 55 insertions(+), 20 deletions(-) diff --git a/cmd/nvidia-ctk/config/config.go b/cmd/nvidia-ctk/config/config.go index 3c4555f4..e1e163ca 100644 --- a/cmd/nvidia-ctk/config/config.go +++ b/cmd/nvidia-ctk/config/config.go @@ -38,7 +38,8 @@ type command struct { // options stores the subcommand options type options struct { flags.Options - sets cli.StringSlice + setListSeparator string + sets cli.StringSlice } // NewCommand constructs an config command with the specified logger @@ -57,6 +58,9 @@ func (m command) build() *cli.Command { c := cli.Command{ Name: "config", Usage: "Interact with the NVIDIA Container Toolkit configuration", + Before: func(ctx *cli.Context) error { + return validateFlags(ctx, &opts) + }, Action: func(ctx *cli.Context) error { return run(ctx, &opts) }, @@ -71,10 +75,21 @@ func (m command) build() *cli.Command { Destination: &opts.Config, }, &cli.StringSliceFlag{ - Name: "set", - Usage: "Set a config value using the pattern key=value. If value is empty, this is equivalent to specifying the same key in unset. This flag can be specified multiple times", + Name: "set", + Usage: "Set a config value using the pattern 'key[=value]'. " + + "Specifying only 'key' is equivalent to 'key=true' for boolean settings. " + + "This flag can be specified multiple times, but only the last value for a specific " + + "config option is applied. " + + "If the setting represents a list, the elements are colon-separated.", Destination: &opts.sets, }, + &cli.StringFlag{ + Name: "set-list-separator", + Usage: "Specify a separator for lists applied using the set command.", + Hidden: true, + Value: ":", + Destination: &opts.setListSeparator, + }, &cli.BoolFlag{ Name: "in-place", Aliases: []string{"i"}, @@ -96,6 +111,13 @@ func (m command) build() *cli.Command { return &c } +func validateFlags(c *cli.Context, opts *options) error { + if opts.setListSeparator == "" { + return fmt.Errorf("set-list-separator must be set") + } + return nil +} + func run(c *cli.Context, opts *options) error { cfgToml, err := config.New( config.WithConfigFile(opts.Config), @@ -105,7 +127,7 @@ func run(c *cli.Context, opts *options) error { } for _, set := range opts.sets.Value() { - key, value, err := setFlagToKeyValue(set) + key, value, err := setFlagToKeyValue(set, opts.setListSeparator) if err != nil { return fmt.Errorf("invalid --set option %v: %w", set, err) } @@ -139,7 +161,7 @@ var errInvalidFormat = errors.New("invalid format") // setFlagToKeyValue converts a --set flag to a key-value pair. // The set flag is of the form key[=value], with the value being optional if key refers to a // boolean config option. -func setFlagToKeyValue(setFlag string) (string, interface{}, error) { +func setFlagToKeyValue(setFlag string, setListSeparator string) (string, interface{}, error) { setParts := strings.SplitN(setFlag, "=", 2) key := setParts[0] @@ -172,7 +194,7 @@ func setFlagToKeyValue(setFlag string) (string, interface{}, error) { case reflect.String: return key, value, nil case reflect.Slice: - valueParts := strings.Split(value, ",") + valueParts := strings.Split(value, setListSeparator) switch field.Elem().Kind() { case reflect.String: return key, valueParts, nil diff --git a/cmd/nvidia-ctk/config/config_test.go b/cmd/nvidia-ctk/config/config_test.go index eca474e9..32d328af 100644 --- a/cmd/nvidia-ctk/config/config_test.go +++ b/cmd/nvidia-ctk/config/config_test.go @@ -25,11 +25,12 @@ import ( func TestSetFlagToKeyValue(t *testing.T) { // TODO: We need to enable this test again since switching to reflect. testCases := []struct { - description string - setFlag string - expectedKey string - expectedValue interface{} - expectedError error + description string + setFlag string + setListSeparator string + expectedKey string + expectedValue interface{} + expectedError error }{ { description: "option not present returns an error", @@ -106,22 +107,34 @@ func TestSetFlagToKeyValue(t *testing.T) { expectedValue: []string{"string-value"}, }, { - description: "[]string option returns multiple values", - setFlag: "nvidia-container-cli.environment=first,second", - expectedKey: "nvidia-container-cli.environment", - expectedValue: []string{"first", "second"}, + description: "[]string option returns multiple values", + setFlag: "nvidia-container-cli.environment=first,second", + setListSeparator: ",", + expectedKey: "nvidia-container-cli.environment", + expectedValue: []string{"first", "second"}, }, { - description: "[]string option returns values with equals", - setFlag: "nvidia-container-cli.environment=first=1,second=2", - expectedKey: "nvidia-container-cli.environment", - expectedValue: []string{"first=1", "second=2"}, + description: "[]string option returns values with equals", + setFlag: "nvidia-container-cli.environment=first=1,second=2", + setListSeparator: ",", + expectedKey: "nvidia-container-cli.environment", + expectedValue: []string{"first=1", "second=2"}, + }, + { + description: "[]string option returns multiple values semi-colon", + setFlag: "nvidia-container-cli.environment=first;second", + setListSeparator: ";", + expectedKey: "nvidia-container-cli.environment", + expectedValue: []string{"first", "second"}, }, } for _, tc := range testCases { t.Run(tc.description, func(t *testing.T) { - k, v, err := setFlagToKeyValue(tc.setFlag) + if tc.setListSeparator == "" { + tc.setListSeparator = "," + } + k, v, err := setFlagToKeyValue(tc.setFlag, tc.setListSeparator) require.ErrorIs(t, err, tc.expectedError) require.EqualValues(t, tc.expectedKey, k) require.EqualValues(t, tc.expectedValue, v)