diff --git a/cmd/notation/common.go b/cmd/notation/common.go index 92265bc92..46f0871d1 100644 --- a/cmd/notation/common.go +++ b/cmd/notation/common.go @@ -16,7 +16,7 @@ var ( flagUsername = &pflag.Flag{ Name: "username", Shorthand: "u", - Usage: "username for registry operations (if not specified, defaults to $NOTATION_USERNAME)", + Usage: "username for registry operations (default to $NOTATION_USERNAME if not specified)", } setflagUsername = func(fs *pflag.FlagSet, p *string) { fs.StringVarP(p, flagUsername.Name, flagUsername.Shorthand, "", flagUsername.Usage) @@ -25,7 +25,7 @@ var ( flagPassword = &pflag.Flag{ Name: "password", Shorthand: "p", - Usage: "password for registry operations (if not specified, defaults to $NOTATION_PASSWORD)", + Usage: "password for registry operations (default to $NOTATION_PASSWORD if not specified)", } setFlagPassword = func(fs *pflag.FlagSet, p *string) { fs.StringVarP(p, flagPassword.Name, flagPassword.Shorthand, "", flagPassword.Usage) @@ -33,7 +33,7 @@ var ( flagPlainHTTP = &pflag.Flag{ Name: "plain-http", - Usage: "Registry access via plain HTTP", + Usage: "registry access via plain HTTP", DefValue: "false", } setFlagPlainHTTP = func(fs *pflag.FlagSet, p *bool) { diff --git a/cmd/notation/key.go b/cmd/notation/key.go index 117bcb6f3..653f0acaa 100644 --- a/cmd/notation/key.go +++ b/cmd/notation/key.go @@ -33,7 +33,7 @@ type keyAddOpts struct { name string plugin string id string - pluginConfig string + pluginConfig []string isDefault bool keyPath string certPath string diff --git a/cmd/notation/key_test.go b/cmd/notation/key_test.go index 8fee4d0d9..f06159f1e 100644 --- a/cmd/notation/key_test.go +++ b/cmd/notation/key_test.go @@ -14,20 +14,20 @@ func TestKeyAddCommand_BasicArgs(t *testing.T) { id: "pluginid", keyPath: "keypath", certPath: "certpath", - pluginConfig: "pluginconfig", + pluginConfig: []string{"pluginconfig"}, } if err := cmd.ParseFlags([]string{ "-n", expected.name, "--plugin", expected.plugin, "--id", expected.id, - "-c", expected.pluginConfig, + "-c", "pluginconfig", expected.keyPath, expected.certPath}); err != nil { t.Fatalf("Parse Flag failed: %v", err) } if err := cmd.Args(cmd, cmd.Flags().Args()); err != nil { t.Fatalf("Parse Args failed: %v", err) } - if *expected != *opts { + if !reflect.DeepEqual(*expected, *opts) { t.Fatalf("Expect key add opts: %v, got: %v", expected, opts) } } diff --git a/cmd/notation/main.go b/cmd/notation/main.go index d8aadbf14..0c9e4c98e 100644 --- a/cmd/notation/main.go +++ b/cmd/notation/main.go @@ -20,10 +20,8 @@ func main() { keyCommand(), pluginCommand(), loginCommand(nil), - logoutCommand(nil), - versionCommand(), - ) - cmd.PersistentFlags().Bool(flagPlainHTTP.Name, false, flagPlainHTTP.Usage) + logoutCommand(nil)) + if err := cmd.Execute(); err != nil { log.Fatal(err) } diff --git a/cmd/notation/sign.go b/cmd/notation/sign.go index e955d7d3f..75030be2a 100644 --- a/cmd/notation/sign.go +++ b/cmd/notation/sign.go @@ -19,7 +19,7 @@ type signOpts struct { timestamp string expiry time.Duration originReference string - pluginConfig string + pluginConfig []string reference string } diff --git a/cmd/notation/sign_test.go b/cmd/notation/sign_test.go index afe5ba345..5170b6708 100644 --- a/cmd/notation/sign_test.go +++ b/cmd/notation/sign_test.go @@ -2,6 +2,7 @@ package main import ( "fmt" + "reflect" "testing" "time" @@ -29,6 +30,7 @@ func TestSignCommand_BasicArgs(t *testing.T) { CertFile: "certfile", EnvelopeType: envelope.JWS, }, + pluginConfig: []string{"key0=val0"}, } if err := command.ParseFlags([]string{ expected.reference, @@ -36,13 +38,14 @@ func TestSignCommand_BasicArgs(t *testing.T) { "--password", expected.Password, "--key", expected.Key, "--key-file", expected.KeyFile, - "--cert-file", expected.CertFile}); err != nil { + "--cert-file", expected.CertFile, + "--plugin-config", "key0=val0"}); err != nil { t.Fatalf("Parse Flag failed: %v", err) } if err := command.Args(command, command.Flags().Args()); err != nil { t.Fatalf("Parse args failed: %v", err) } - if *expected != *opts { + if !reflect.DeepEqual(*expected, *opts) { t.Fatalf("Expect sign opts: %v, got: %v", expected, opts) } } @@ -69,7 +72,8 @@ func TestSignCommand_MoreArgs(t *testing.T) { CertFile: "certfile", EnvelopeType: envelope.COSE, }, - expiry: 24 * time.Hour, + expiry: 24 * time.Hour, + pluginConfig: []string{"key0=val0"}, } if err := command.ParseFlags([]string{ expected.reference, @@ -82,13 +86,14 @@ func TestSignCommand_MoreArgs(t *testing.T) { "--media-type", expected.MediaType, "-l", "--envelope-type", expected.SignerFlagOpts.EnvelopeType, - "--expiry", expected.expiry.String()}); err != nil { + "--expiry", expected.expiry.String(), + "--plugin-config", "key0=val0"}); err != nil { t.Fatalf("Parse Flag failed: %v", err) } if err := command.Args(command, command.Flags().Args()); err != nil { t.Fatalf("Parse args failed: %v", err) } - if *expected != *opts { + if !reflect.DeepEqual(*expected, *opts) { t.Fatalf("Expect sign opts: %v, got: %v", expected, opts) } } @@ -111,7 +116,7 @@ func TestSignCommand_CorrectConfig(t *testing.T) { EnvelopeType: envelope.JWS, }, expiry: 365 * 24 * time.Hour, - pluginConfig: "key0=val0,key1=val1,key2=val2", + pluginConfig: []string{"key0=val0", "key1=val1"}, originReference: "originref", } if err := command.ParseFlags([]string{ @@ -124,23 +129,24 @@ func TestSignCommand_CorrectConfig(t *testing.T) { "--local", "--envelope-type", expected.SignerFlagOpts.EnvelopeType, "--expiry", expected.expiry.String(), - "--pluginConfig", expected.pluginConfig}); err != nil { + "--plugin-config", "key0=val0", + "--plugin-config", "key1=val1"}); err != nil { t.Fatalf("Parse Flag failed: %v", err) } if err := command.Args(command, command.Flags().Args()); err != nil { t.Fatalf("Parse args failed: %v", err) } - if *expected != *opts { + if !reflect.DeepEqual(*expected, *opts) { t.Fatalf("Expect sign opts: %v, got: %v", expected, opts) } config, err := cmd.ParseFlagPluginConfig(opts.pluginConfig) if err != nil { t.Fatalf("Parse plugin Config flag failed: %v", err) } - if len(config) != 3 { - t.Fatalf("Expect plugin config number: %v, got: %v ", 3, len(config)) + if len(config) != 2 { + t.Fatalf("Expect plugin config number: %v, got: %v ", 2, len(config)) } - for i := 0; i < 3; i++ { + for i := 0; i < 2; i++ { key, val := fmt.Sprintf("key%v", i), fmt.Sprintf("val%v", i) configVal, ok := config[key] if !ok { diff --git a/cmd/notation/verify.go b/cmd/notation/verify.go index 6199db2f9..4d4f66086 100644 --- a/cmd/notation/verify.go +++ b/cmd/notation/verify.go @@ -17,7 +17,7 @@ import ( type verifyOpts struct { SecureFlagOpts reference string - pluginConfig string + pluginConfig []string } func verifyCommand(opts *verifyOpts) *cobra.Command { @@ -26,12 +26,12 @@ func verifyCommand(opts *verifyOpts) *cobra.Command { } command := &cobra.Command{ Use: "verify [flags] ", - Short: "Verifies OCI Artifacts", - Long: `Verifies OCI Artifacts + Short: "Verify Artifacts", + Long: `Verify signatures associated with the artifact. Prerequisite: a trusted certificate needs to be generated or added using the command "notation cert". -notation verify [--plugin-config =,...] [--username ] [--password ] `, +notation verify [--plugin-config =...] [--username ] [--password ] `, Args: func(cmd *cobra.Command, args []string) error { if len(args) == 0 { return errors.New("missing reference") @@ -44,7 +44,7 @@ notation verify [--plugin-config =,...] [--username ] [--p }, } opts.ApplyFlags(command.Flags()) - command.Flags().StringVarP(&opts.pluginConfig, "plugin-config", "c", "", "list of comma-separated {key}={value} pairs that are passed as is to the plugin") + command.Flags().StringArrayVarP(&opts.pluginConfig, "plugin-config", "c", nil, "{key}={value} pairs that are passed as it is to a plugin, if the verification is associated with a verification plugin, refer plugin documentation to set appropriate values") return command } diff --git a/cmd/notation/verify_test.go b/cmd/notation/verify_test.go index c3f03ee6b..69d879f25 100644 --- a/cmd/notation/verify_test.go +++ b/cmd/notation/verify_test.go @@ -14,11 +14,13 @@ func TestVerifyCommand_BasicArgs(t *testing.T) { Username: "user", Password: "password", }, + pluginConfig: []string{"key1=val1"}, } if err := command.ParseFlags([]string{ expected.reference, "--username", expected.Username, - "--password", expected.Password}); err != nil { + "--password", expected.Password, + "--plugin-config", "key1=val1"}); err != nil { t.Fatalf("Parse Flag failed: %v", err) } if err := command.Args(command, command.Flags().Args()); err != nil { @@ -37,12 +39,13 @@ func TestVerifyCommand_MoreArgs(t *testing.T) { SecureFlagOpts: SecureFlagOpts{ PlainHTTP: true, }, - pluginConfig: "key1=val1,key2=val2", + pluginConfig: []string{"key1=val1", "key2=val2"}, } if err := command.ParseFlags([]string{ expected.reference, "--plain-http", - "--plugin-config", expected.pluginConfig}); err != nil { + "--plugin-config", "key1=val1", + "--plugin-config", "key2=val2"}); err != nil { t.Fatalf("Parse Flag failed: %v", err) } if err := command.Args(command, command.Flags().Args()); err != nil { diff --git a/internal/cmd/flags.go b/internal/cmd/flags.go index a8f632f58..5afbcaec8 100644 --- a/internal/cmd/flags.go +++ b/internal/cmd/flags.go @@ -2,7 +2,6 @@ package cmd import ( - "errors" "fmt" "strings" "time" @@ -73,12 +72,12 @@ var ( } PflagPluginConfig = &pflag.Flag{ - Name: "pluginConfig", + Name: "plugin-config", Shorthand: "c", - Usage: "list of comma-separated {key}={value} pairs that are passed as is to the plugin, refer plugin documentation to set appropriate values", + Usage: "{key}={value} pairs that are passed as it is to a plugin, refer plugin's documentation to set appropriate values", } - SetPflagPluginConfig = func(fs *pflag.FlagSet, p *string) { - fs.StringVarP(p, PflagPluginConfig.Name, PflagPluginConfig.Shorthand, "", PflagPluginConfig.Usage) + SetPflagPluginConfig = func(fs *pflag.FlagSet, p *[]string) { + fs.StringArrayVarP(p, PflagPluginConfig.Name, PflagPluginConfig.Shorthand, nil, PflagPluginConfig.Usage) } ) @@ -88,94 +87,14 @@ type KeyValueSlice interface { String() string } -func ParseFlagPluginConfig(config string) (map[string]string, error) { - pluginConfig, err := ParseKeyValueListFlag(config) - if err != nil { - return nil, fmt.Errorf("could not parse %q as value for flag %s: %s", pluginConfig, PflagPluginConfig.Name, err) - } - return pluginConfig, nil -} - -func ParseKeyValueListFlag(val string) (map[string]string, error) { - if val == "" { - return nil, nil - } - flags, err := splitQuoted(val) - if err != nil { - return nil, err - } - m := make(map[string]string, len(flags)) - for _, c := range flags { - c := strings.TrimSpace(c) - if c == "" { - return nil, fmt.Errorf("empty entry: %q", c) - } - if k, v, ok := strings.Cut(c, "="); ok { - k := strings.TrimSpace(k) - v := strings.TrimSpace(v) - if k == "" || v == "" { - return nil, errors.New("empty key value") - } - if _, exist := m[k]; exist { - return nil, fmt.Errorf("duplicated key: %q", k) - } - m[k] = v - } else { - return nil, fmt.Errorf("malformed entry: %q", c) +func ParseFlagPluginConfig(config []string) (map[string]string, error) { + pluginConfig := make(map[string]string, len(config)) + for _, pair := range config { + key, val, found := strings.Cut(pair, "=") + if !found || key == "" || val == "" { + return nil, fmt.Errorf("could not parse flag %s: key-value pair requires \"=\" as separator", PflagPluginConfig.Name) } + pluginConfig[key] = val } - return m, nil -} - -// splitQuoted splits the string s around each instance of one or more consecutive -// comma characters while taking into account quotes and escaping, and -// returns an array of substrings of s or an empty list if s is empty. -// Single quotes and double quotes are recognized to prevent splitting within the -// quoted region, and are removed from the resulting substrings. If a quote in s -// isn't closed err will be set and r will have the unclosed argument as the -// last element. The backslash is used for escaping. -// -// For example, the following string: -// -// `a,b:"c,d",'e''f',,"g\""` -// -// Would be parsed as: -// -// []string{"a", "b:c,d", "ef", "", `g"`} -func splitQuoted(s string) (r []string, err error) { - var args []string - arg := make([]rune, len(s)) - escaped := false - quote := '\x00' - i := 0 - for _, r := range s { - switch { - case escaped: - escaped = false - case r == '\\': - escaped = true - continue - case quote != 0: - if r == quote { - quote = 0 - continue - } - case r == '"' || r == '\'': - quote = r - continue - case r == ',': - args = append(args, string(arg[:i])) - i = 0 - continue - } - arg[i] = r - i++ - } - args = append(args, string(arg[:i])) - if quote != 0 { - err = errors.New("unclosed quote") - } else if escaped { - err = errors.New("unfinished escaping") - } - return args, err + return pluginConfig, nil } diff --git a/internal/cmd/flags_test.go b/internal/cmd/flags_test.go deleted file mode 100644 index 2655e960e..000000000 --- a/internal/cmd/flags_test.go +++ /dev/null @@ -1,44 +0,0 @@ -// Package cmd contains common flags and routines for all CLIs. -package cmd - -import ( - "reflect" - "testing" -) - -func TestParseKeyValueListFlag(t *testing.T) { - type args struct { - s string - } - tests := []struct { - name string - args args - want map[string]string - wantErr bool - }{ - {"empty", args{""}, nil, false}, - {"single", args{"a=b"}, map[string]string{"a": "b"}, false}, - {"multiple", args{"a=b,c=d"}, map[string]string{"a": "b", "c": "d"}, false}, - {"spaces", args{"a=b , c=d"}, map[string]string{"a": "b", "c": "d"}, false}, - {"quoted", args{"a=b,\"c\"=d"}, map[string]string{"a": "b", "c": "d"}, false}, - {"quoted comma", args{"a=b,\"c,h\"=d"}, map[string]string{"a": "b", "c,h": "d"}, false}, - {"empty value", args{"a=b,,c=d"}, nil, true}, - {"duplicated", args{"a=b,a=d"}, nil, true}, - {"malformed", args{"a=b,c:d"}, nil, true}, - {"only equal", args{"="}, nil, true}, - {"entry only equal", args{"a=b,="}, nil, true}, - {"entry only equal and space", args{"a=b, = "}, nil, true}, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got, err := ParseKeyValueListFlag(tt.args.s) - if (err != nil) != tt.wantErr { - t.Errorf("ParseKeyValueListFlag() error = %v, wantErr %v", err, tt.wantErr) - return - } - if !reflect.DeepEqual(got, tt.want) { - t.Errorf("ParseKeyValueListFlag() = %v, want %v", got, tt.want) - } - }) - } -} diff --git a/specs/commandline/verify.md b/specs/commandline/verify.md index 1a779e62d..f5e9b5c3b 100644 --- a/specs/commandline/verify.md +++ b/specs/commandline/verify.md @@ -14,9 +14,10 @@ Usage: Flags: -h, --help help for verify - -p, --password string Password for registry operations (default to $NOTATION_PASSWORD if not specified) - --plugin-config strings {key}={value} pairs that are passed as is to a plugin, if the verification is associated with a verification plugin, refer plugin documentation to set appropriate values - -u, --username string Username for registry operations (default to $NOTATION_USERNAME if not specified) + -p, --password string password for registry operations (default to $NOTATION_PASSWORD if not specified) + --plain-http registry access via plain HTTP + --plugin-config strings {key}={value} pairs that are passed as it is to a plugin, if the verification is associated with a verification plugin, refer plugin documentation to set appropriate values + -u, --username string username for registry operations (default to $NOTATION_USERNAME if not specified) ``` ## Usage