diff --git a/cmd/notation/cert/generateTest.go b/cmd/notation/cert/generateTest.go index 3c9aa5a24..3eeebf488 100644 --- a/cmd/notation/cert/generateTest.go +++ b/cmd/notation/cert/generateTest.go @@ -14,8 +14,6 @@ import ( "github.com/notaryproject/notation-go/dir" "github.com/notaryproject/notation/cmd/notation/internal/truststore" "github.com/notaryproject/notation/internal/osutil" - "github.com/notaryproject/notation/internal/slices" - "github.com/notaryproject/notation/pkg/configutil" "github.com/spf13/cobra" "github.com/spf13/pflag" ) @@ -111,21 +109,11 @@ func generateTestCert(opts *certGenerateTestOpts) error { } fmt.Println("wrote certificate:", certPath) - // update config - signingKeys, err := configutil.LoadSigningkeysOnce() - if err != nil { - return err - } - isDefault := opts.isDefault - keySuite := config.KeySuite{ - Name: name, - X509KeyPair: &config.X509KeyPair{ - KeyPath: keyPath, - CertificatePath: certPath, - }, + // update signingkeys.json config + exec := func(s *config.SigningKeys) error { + return s.Add(opts.name, keyPath,certPath, opts.isDefault) } - err = addKeyToSigningKeys(signingKeys, keySuite, isDefault) - if err != nil { + if err := config.LoadExecSaveSigningKeys(exec); err != nil { return err } @@ -134,14 +122,9 @@ func generateTestCert(opts *certGenerateTestOpts) error { return err } - // Save to the SigningKeys.json - if err := signingKeys.Save(); err != nil { - return err - } - // write out fmt.Printf("%s: added to the key list\n", name) - if isDefault { + if opts.isDefault { fmt.Printf("%s: mark as default signing key\n", name) } return nil @@ -169,14 +152,3 @@ func generateSelfSignedCert(privateKey *rsa.PrivateKey, name string) (testhelper rsaCertTuple := testhelper.GetRSASelfSignedCertTupleWithPK(privateKey, name) return rsaCertTuple, generateCertPEM(&rsaCertTuple), nil } - -func addKeyToSigningKeys(signingKeys *config.SigningKeys, key config.KeySuite, markDefault bool) error { - if slices.Contains(signingKeys.Keys, key.Name) { - return fmt.Errorf("signing key with name %q already exists", key.Name) - } - signingKeys.Keys = append(signingKeys.Keys, key) - if markDefault { - signingKeys.Default = key.Name - } - return nil -} diff --git a/cmd/notation/key.go b/cmd/notation/key.go index fecc06432..7c73390bc 100644 --- a/cmd/notation/key.go +++ b/cmd/notation/key.go @@ -4,16 +4,12 @@ import ( "context" "errors" "fmt" + "github.com/notaryproject/notation-go/config" "os" - "github.com/notaryproject/notation-go/config" - "github.com/notaryproject/notation-go/dir" "github.com/notaryproject/notation-go/log" - "github.com/notaryproject/notation-go/plugin" "github.com/notaryproject/notation/internal/cmd" "github.com/notaryproject/notation/internal/ioutil" - "github.com/notaryproject/notation/internal/slices" - "github.com/notaryproject/notation/pkg/configutil" "github.com/spf13/cobra" "github.com/spf13/pflag" ) @@ -166,83 +162,26 @@ func keyDeleteCommand(opts *keyDeleteOpts) *cobra.Command { func addKey(ctx context.Context, opts *keyAddOpts) error { // set log level ctx = opts.LoggingFlagOpts.SetLoggerLevel(ctx) - logger := log.GetLogger(ctx) - signingKeys, err := configutil.LoadSigningkeysOnce() + pluginConfig, err := cmd.ParseFlagPluginConfig(opts.pluginConfig) if err != nil { return err } - var key config.KeySuite - name := opts.name - if name == "" { - return errors.New("key name cannot be empty") - } - pluginName := opts.plugin - if pluginName != "" { - logger.Debugf("Adding key with name %v and plugin name %v", name, pluginName) - key, err = addExternalKey(ctx, opts, pluginName, name) - if err != nil { - return err - } - } else { - return errors.New("plugin name cannot be empty") - } - isDefault := opts.isDefault - err = addKeyCore(signingKeys, key, isDefault) - if err != nil { - return err + // core process + exec := func(s *config.SigningKeys) error { + return s.AddPlugin(ctx, opts.name, opts.id, opts.plugin, pluginConfig, opts.isDefault) } - - if err := signingKeys.Save(); err != nil { + if err := config.LoadExecSaveSigningKeys(exec); err != nil { return err } - // write out - logger.Debugf("Added key with name %s - {%+v}", key.Name, key.ExternalKey) - if isDefault { - fmt.Printf("%s: marked as default\n", key.Name) + if opts.isDefault { + fmt.Printf("%s: marked as default\n", opts.name) } else { - fmt.Println(key.Name) - } - - return nil -} - -func addExternalKey(ctx context.Context, opts *keyAddOpts, pluginName, keyName string) (config.KeySuite, error) { - id := opts.id - if id == "" { - return config.KeySuite{}, errors.New("missing key id") - } - mgr := plugin.NewCLIManager(dir.PluginFS()) - // Check existence of plugin with name pluginName - _, err := mgr.Get(ctx, pluginName) - if err != nil { - return config.KeySuite{}, err - } - pluginConfig, err := cmd.ParseFlagPluginConfig(opts.pluginConfig) - if err != nil { - return config.KeySuite{}, err + fmt.Println(opts.name) } - return config.KeySuite{ - Name: keyName, - ExternalKey: &config.ExternalKey{ - ID: id, - PluginName: pluginName, - PluginConfig: pluginConfig, - }, - }, nil -} - -func addKeyCore(signingKeys *config.SigningKeys, key config.KeySuite, markDefault bool) error { - if slices.Contains(signingKeys.Keys, key.Name) { - return fmt.Errorf("signing key with name %q already exists", key.Name) - } - signingKeys.Keys = append(signingKeys.Keys, key) - if markDefault { - signingKeys.Default = key.Name - } return nil } @@ -251,35 +190,27 @@ func updateKey(ctx context.Context, opts *keyUpdateOpts) error { ctx = opts.LoggingFlagOpts.SetLoggerLevel(ctx) logger := log.GetLogger(ctx) - // initialize - name := opts.name - // core process - signingKeys, err := configutil.LoadSigningkeysOnce() - if err != nil { - return err - } - if !slices.Contains(signingKeys.Keys, name) { - return errors.New(name + ": not found") - } if !opts.isDefault { logger.Warn("--default flag is not set, command did not take effect") return nil } - if signingKeys.Default != name { - signingKeys.Default = name - if err := signingKeys.Save(); err != nil { - return err - } + + // core process + exec := func(s *config.SigningKeys) error { + return s.UpdateDefault(opts.name) + } + if err := config.LoadExecSaveSigningKeys(exec); err != nil { + return err } // write out - fmt.Printf("%s: marked as default\n", name) + fmt.Printf("%s: marked as default\n", opts.name) return nil } func listKeys() error { // core process - signingKeys, err := configutil.LoadSigningkeysOnce() + signingKeys, err := config.LoadSigningKeys() if err != nil { return err } @@ -294,26 +225,19 @@ func deleteKeys(ctx context.Context, opts *keyDeleteOpts) error { logger := log.GetLogger(ctx) // core process - signingKeys, err := configutil.LoadSigningkeysOnce() - if err != nil { - return err - } - - prevDefault := signingKeys.Default - var deletedNames []string - for _, name := range opts.names { - idx := slices.Index(signingKeys.Keys, name) - if idx < 0 { - logger.Warnf("Key %s not found, command did not take effect", name) - return errors.New(name + ": not found") - } - signingKeys.Keys = slices.Delete(signingKeys.Keys, idx) - deletedNames = append(deletedNames, name) - if prevDefault == name { - signingKeys.Default = "" + var deletedNames []string + var prevDefault string + exec := func(s *config.SigningKeys) error { + prevDefault = *s.Default + var err error + deletedNames, err = s.Remove(opts.names) + if err != nil { + logger.Warnf("%v", err) } + return err + } - if err := signingKeys.Save(); err != nil { + if err := config.LoadExecSaveSigningKeys(exec); err != nil { return err } @@ -327,3 +251,4 @@ func deleteKeys(ctx context.Context, opts *keyDeleteOpts) error { } return nil } + diff --git a/internal/ioutil/print.go b/internal/ioutil/print.go index c7661d76e..e57e17c05 100644 --- a/internal/ioutil/print.go +++ b/internal/ioutil/print.go @@ -12,12 +12,12 @@ func newTabWriter(w io.Writer) *tabwriter.Writer { return tabwriter.NewWriter(w, 0, 0, 3, ' ', 0) } -func PrintKeyMap(w io.Writer, target string, v []config.KeySuite) error { +func PrintKeyMap(w io.Writer, target *string, v []config.KeySuite) error { tw := newTabWriter(w) fmt.Fprintln(tw, "NAME\tKEY PATH\tCERTIFICATE PATH\tID\tPLUGIN NAME\t") for _, key := range v { name := key.Name - if key.Name == target { + if target != nil && key.Name == *target { name = "* " + name } kp := key.X509KeyPair diff --git a/internal/slices/slices.go b/internal/slices/slices.go deleted file mode 100644 index 1fffa0883..000000000 --- a/internal/slices/slices.go +++ /dev/null @@ -1,27 +0,0 @@ -package slices - -type isser interface { - Is(string) bool -} - -// Index returns the index of the first occurrence of name in s, -// or -1 if not present. -func Index[E isser](s []E, name string) int { - for i, v := range s { - if v.Is(name) { - return i - } - } - return -1 -} - -// Contains reports whether name is present in s. -func Contains[E isser](s []E, name string) bool { - return Index(s, name) >= 0 -} - -// Delete removes the elements s[i:i+1] from s, -// returning the modified slice. -func Delete[S ~[]E, E isser](s S, i int) S { - return append(s[:i], s[i+1:]...) -} diff --git a/internal/slices/slices_test.go b/internal/slices/slices_test.go deleted file mode 100644 index ae24e6313..000000000 --- a/internal/slices/slices_test.go +++ /dev/null @@ -1,65 +0,0 @@ -package slices - -import ( - "reflect" - "testing" -) - -type iss string - -func (i iss) Is(v string) bool { return string(i) == v } - -func TestIndex(t *testing.T) { - tests := []struct { - s []iss - v string - want int - }{ - {nil, "", -1}, - {[]iss{}, "", -1}, - {[]iss{"1", "2", "3"}, "2", 1}, - {[]iss{"1", "2", "2", "3"}, "2", 1}, - {[]iss{"1", "2", "3", "2"}, "2", 1}, - } - for _, tt := range tests { - if got := Index(tt.s, tt.v); got != tt.want { - t.Errorf("Index() = %v, want %v", got, tt.want) - } - } -} - -func TestContains(t *testing.T) { - tests := []struct { - s []iss - v string - want bool - }{ - {nil, "", false}, - {[]iss{}, "", false}, - {[]iss{"1", "2", "3"}, "2", true}, - {[]iss{"1", "2", "2", "3"}, "2", true}, - {[]iss{"1", "2", "3", "2"}, "2", true}, - } - for _, tt := range tests { - if got := Contains(tt.s, tt.v); got != tt.want { - t.Errorf("Index() = %v, want %v", got, tt.want) - } - } -} - -func TestDelete(t *testing.T) { - tests := []struct { - s []iss - i int - want []iss - }{ - {[]iss{"1", "2", "3"}, 1, []iss{"1", "3"}}, - {[]iss{"1", "2", "2", "3"}, 2, []iss{"1", "2", "3"}}, - {[]iss{"1", "2", "3", "2"}, 2, []iss{"1", "2", "2"}}, - } - for _, tt := range tests { - if got := Delete(tt.s, tt.i); !reflect.DeepEqual(got, tt.want) { - t.Errorf("Delete() = %v, want %v", got, tt.want) - } - } -} diff --git a/pkg/configutil/once.go b/pkg/configutil/once.go index 1b2504709..bae0f2e1d 100644 --- a/pkg/configutil/once.go +++ b/pkg/configutil/once.go @@ -34,15 +34,3 @@ func LoadConfigOnce() (*config.Config, error) { }) return configInfo, err } - -// LoadSigningKeysOnce returns the previously read config file. -// If previous config file does not exist, it reads the config from file -// or return a default config if not found. -// The returned config is only suitable for read only scenarios for short-lived processes. -func LoadSigningkeysOnce() (*config.SigningKeys, error) { - var err error - signingKeysOnce.Do(func() { - signingKeysInfo, err = config.LoadSigningKeys() - }) - return signingKeysInfo, err -} diff --git a/pkg/configutil/once_test.go b/pkg/configutil/once_test.go index b1ec58260..d2636454a 100644 --- a/pkg/configutil/once_test.go +++ b/pkg/configutil/once_test.go @@ -17,17 +17,3 @@ func TestLoadConfigOnce(t *testing.T) { t.Fatal("LoadConfigOnce is invalid.") } } - -func TestLoadSigningKeysOnce(t *testing.T) { - signingKeys1, err := LoadSigningkeysOnce() - if err != nil { - t.Fatal("LoadSigningkeysOnce failed.") - } - signingKeys2, err := LoadSigningkeysOnce() - if err != nil { - t.Fatal("LoadSigningkeysOnce failed.") - } - if signingKeys1 != signingKeys2 { - t.Fatal("LoadSigningkeysOnce is invalid.") - } -} diff --git a/pkg/configutil/util.go b/pkg/configutil/util.go index 145f1d781..312cb62ca 100644 --- a/pkg/configutil/util.go +++ b/pkg/configutil/util.go @@ -5,7 +5,6 @@ import ( "strings" "github.com/notaryproject/notation-go/config" - "github.com/notaryproject/notation/internal/slices" ) var ( @@ -30,25 +29,15 @@ func IsRegistryInsecure(target string) bool { // ResolveKey resolves the key by name. // The default key is attempted if name is empty. func ResolveKey(name string) (config.KeySuite, error) { - signingKeys, err := LoadSigningkeysOnce() + signingKeys, err := config.LoadSigningKeys() if err != nil { return config.KeySuite{}, err } // if name is empty, look for default signing key if name == "" { - name = signingKeys.Default - - // if name is still empty, return error - if name == "" { - return config.KeySuite{}, errors.New("default signing key not set." + - " Please set default singing key or specify a key name") - } + return signingKeys.GetDefault() } - idx := slices.Index(signingKeys.Keys, name) - if idx < 0 { - return config.KeySuite{}, ErrKeyNotFound - } - return signingKeys.Keys[idx], nil + return signingKeys.Get(name) } diff --git a/specs/commandline/sign.md b/specs/commandline/sign.md index 6a756e37e..2f513cf0b 100644 --- a/specs/commandline/sign.md +++ b/specs/commandline/sign.md @@ -114,7 +114,7 @@ notation sign /@ ### Sign an OCI Artifact with user metadata ```shell -# Prerequisites: +# Prerequisites: # A default signing key is configured using CLI "notation key" # sign an artifact stored in a registry and add user-metadata io.wabbit-networks.buildId=123 to the payload @@ -165,7 +165,7 @@ notation sign --image-spec v1.1-image /@ ``` [oci-artifact-manifest]: https://github.com/opencontainers/image-spec/blob/v1.1.0-rc2/artifact.md -[oci-image-spec]: https://github.com/opencontainers/image-spec/blob/v1.1.0-rc2/spec.md +[oci-image-spec]: https://github.com/opencontainers/image-spec/blob/v1.1.0-rc2/spec.md [oci-backward-compatibility]: https://github.com/opencontainers/distribution-spec/blob/v1.1.0-rc1/spec.md#backwards-compatibility [registry-support]: https://notaryproject.dev/docs/registrysupport/ [oras-land]: https://oras.land/