From 122333b676d1e1750e0d1260a216ddd0f4d9cd36 Mon Sep 17 00:00:00 2001 From: Mark McDonnell Date: Wed, 26 Jul 2023 16:48:13 +0100 Subject: [PATCH] feat(kv_store): support deleting all keys (#981) * feat(kv_store): support deleting all keys * fix(kvstoreentry_test): correct test assertion order * tests(kv_store): add tests for kv store entries delete all feature * fix: use thread safe Buffer type to avoid data races in test runs * feat(kvstore/entry): support control of concurrency level when deleting all keys --- go.mod | 2 +- go.sum | 4 +- pkg/api/interface.go | 1 + pkg/commands/kvstoreentry/delete.go | 114 ++++++++++++++++-- .../kvstoreentry/kvstoreentry_test.go | 85 ++++++++++++- pkg/commands/kvstoreentry/list.go | 39 ++++++ pkg/errors/errors.go | 16 ++- pkg/mock/api.go | 6 + 8 files changed, 251 insertions(+), 16 deletions(-) diff --git a/go.mod b/go.mod index d753db59b..36a869609 100644 --- a/go.mod +++ b/go.mod @@ -28,7 +28,7 @@ require ( ) require ( - github.com/fastly/go-fastly/v8 v8.5.5 + github.com/fastly/go-fastly/v8 v8.5.7 github.com/kennygrant/sanitize v1.2.4 github.com/mholt/archiver v3.1.1+incompatible github.com/otiai10/copy v1.12.0 diff --git a/go.sum b/go.sum index 480ad28b3..96371c123 100644 --- a/go.sum +++ b/go.sum @@ -19,8 +19,8 @@ github.com/dsnet/compress v0.0.2-0.20210315054119-f66993602bf5/go.mod h1:qssHWj6 github.com/dsnet/golib v0.0.0-20171103203638-1ea166775780/go.mod h1:Lj+Z9rebOhdfkVLjJ8T6VcRQv3SXugXy999NBtR9aFY= github.com/dustinkirkland/golang-petname v0.0.0-20191129215211-8e5a1ed0cff0 h1:90Ly+6UfUypEF6vvvW5rQIv9opIL8CbmW9FT20LDQoY= github.com/dustinkirkland/golang-petname v0.0.0-20191129215211-8e5a1ed0cff0/go.mod h1:V+Qd57rJe8gd4eiGzZyg4h54VLHmYVVw54iMnlAMrF8= -github.com/fastly/go-fastly/v8 v8.5.5 h1:okmeQDxjyK9FEh5mj2K+AwN9SiZDz3epQHm4dAh3+2s= -github.com/fastly/go-fastly/v8 v8.5.5/go.mod h1:jmjaUGq1RUdP05XOuD1ICvuuzo0EdCexShviy2sFfHU= +github.com/fastly/go-fastly/v8 v8.5.7 h1:trLguwCGvpH1leT+R4nEFqhKNKFR45k85lPjCcggQuE= +github.com/fastly/go-fastly/v8 v8.5.7/go.mod h1:jmjaUGq1RUdP05XOuD1ICvuuzo0EdCexShviy2sFfHU= github.com/fastly/kingpin v2.1.12-0.20191105091915-95d230a53780+incompatible h1:FhrXlfhgGCS+uc6YwyiFUt04alnjpoX7vgDKJxS6Qbk= github.com/fastly/kingpin v2.1.12-0.20191105091915-95d230a53780+incompatible/go.mod h1:U8UynVoU1SQaqD2I4ZqgYd5lx3A1ipQYn4aSt2Y5h6c= github.com/fatih/color v1.15.0 h1:kOqh6YHBtK8aywxGerMG2Eq3H6Qgoqeo13Bk2Mv/nBs= diff --git a/pkg/api/interface.go b/pkg/api/interface.go index b2cb5679b..a3a52b618 100644 --- a/pkg/api/interface.go +++ b/pkg/api/interface.go @@ -288,6 +288,7 @@ type Interface interface { NewListACLEntriesPaginator(i *fastly.ListACLEntriesInput) fastly.PaginatorACLEntries NewListDictionaryItemsPaginator(i *fastly.ListDictionaryItemsInput) fastly.PaginatorDictionaryItems NewListServicesPaginator(i *fastly.ListServicesInput) fastly.PaginatorServices + NewListKVStoreKeysPaginator(i *fastly.ListKVStoreKeysInput) fastly.PaginatorKVStoreEntries GetCustomTLSConfiguration(i *fastly.GetCustomTLSConfigurationInput) (*fastly.CustomTLSConfiguration, error) ListCustomTLSConfigurations(i *fastly.ListCustomTLSConfigurationsInput) ([]*fastly.CustomTLSConfiguration, error) diff --git a/pkg/commands/kvstoreentry/delete.go b/pkg/commands/kvstoreentry/delete.go index de4ea1a84..00fd9e3cf 100644 --- a/pkg/commands/kvstoreentry/delete.go +++ b/pkg/commands/kvstoreentry/delete.go @@ -1,7 +1,11 @@ package kvstoreentry import ( + "fmt" "io" + "sort" + "strings" + "sync" "github.com/fastly/go-fastly/v8/fastly" @@ -12,13 +16,19 @@ import ( "github.com/fastly/cli/pkg/text" ) +// deleteKeysConcurrencyLimit is used to limit the concurrency when deleting ALL keys. +const deleteKeysConcurrencyLimit int = 1000 + // DeleteCommand calls the Fastly API to delete an kv store. type DeleteCommand struct { cmd.Base cmd.JSONOutput - manifest manifest.Data - Input fastly.DeleteKVStoreKeyInput + concurrency cmd.OptionalInt + deleteAll bool + key cmd.OptionalString + manifest manifest.Data + storeID string } // NewDeleteCommand returns a usable command registered under the parent. @@ -32,22 +42,51 @@ func NewDeleteCommand(parent cmd.Registerer, g *global.Data, m manifest.Data) *D c.CmdClause = parent.Command("delete", "Delete a key") // Required. - c.CmdClause.Flag("key", "Key name").Short('k').Required().StringVar(&c.Input.Key) - c.CmdClause.Flag("store-id", "Store ID").Short('s').Required().StringVar(&c.Input.ID) + c.CmdClause.Flag("store-id", "Store ID").Short('s').Required().StringVar(&c.storeID) // Optional. + c.CmdClause.Flag("all", "Delete all entries within the store").Short('a').BoolVar(&c.deleteAll) + c.CmdClause.Flag("concurrency", "Control thread pool size (ignored when set without the --all flag)").Short('c').Action(c.concurrency.Set).IntVar(&c.concurrency.Value) c.RegisterFlagBool(c.JSONFlag()) // --json + c.CmdClause.Flag("key", "Key name").Short('k').Action(c.key.Set).StringVar(&c.key.Value) return &c } // Exec invokes the application logic for the command. -func (c *DeleteCommand) Exec(_ io.Reader, out io.Writer) error { +func (c *DeleteCommand) Exec(in io.Reader, out io.Writer) error { if c.Globals.Verbose() && c.JSONOutput.Enabled { return fsterr.ErrInvalidVerboseJSONCombo } + if c.deleteAll && c.key.WasSet { + return fsterr.ErrInvalidDeleteAllKeyCombo + } + if !c.deleteAll && !c.key.WasSet { + return fsterr.ErrMissingDeleteAllKeyCombo + } + + if c.deleteAll { + if !c.Globals.Flags.AutoYes && !c.Globals.Flags.NonInteractive { + text.Warning(out, "This will delete ALL entries from your store!") + text.Break(out) + cont, err := text.AskYesNo(out, "Are you sure you want to continue? [yes/no]: ", in) + if err != nil { + return err + } + if !cont { + return nil + } + text.Break(out) + } + return c.deleteAllKeys(out) + } - err := c.Globals.APIClient.DeleteKVStoreKey(&c.Input) + input := fastly.DeleteKVStoreKeyInput{ + ID: c.storeID, + Key: c.key.Value, + } + + err := c.Globals.APIClient.DeleteKVStoreKey(&input) if err != nil { c.Globals.ErrLog.Add(err) return err @@ -59,14 +98,71 @@ func (c *DeleteCommand) Exec(_ io.Reader, out io.Writer) error { ID string `json:"store_id"` Deleted bool `json:"deleted"` }{ - c.Input.Key, - c.Input.ID, + c.key.Value, + c.storeID, true, } _, err := c.WriteJSON(out, o) return err } - text.Success(out, "Deleted key '%s' from KV Store '%s'", c.Input.Key, c.Input.ID) + text.Success(out, "Deleted key '%s' from KV Store '%s'", c.key.Value, c.storeID) + return nil +} + +func (c *DeleteCommand) deleteAllKeys(out io.Writer) error { + p := c.Globals.APIClient.NewListKVStoreKeysPaginator(&fastly.ListKVStoreKeysInput{ + ID: c.storeID, + }) + + var ( + mu sync.Mutex + wg sync.WaitGroup + ) + poolSize := deleteKeysConcurrencyLimit + if c.concurrency.WasSet { + poolSize = c.concurrency.Value + } + semaphore := make(chan struct{}, poolSize) + + failedKeys := []string{} + + for p.Next() { + // IMPORTANT: Use copies of the keys when processing data concurrently. + keys := p.Keys() + copiedKeys := make([]string, len(keys)) + copy(copiedKeys, keys) + + wg.Add(1) + go func(keys []string) { + semaphore <- struct{}{} + defer func() { <-semaphore }() + defer wg.Done() + + sort.Strings(keys) + for _, key := range keys { + text.Output(out, "Deleting key: %s", key) + err := c.Globals.APIClient.DeleteKVStoreKey(&fastly.DeleteKVStoreKeyInput{ID: c.storeID, Key: key}) + if err != nil { + c.Globals.ErrLog.Add(fmt.Errorf("failed to delete key '%s': %s", key, err)) + mu.Lock() + failedKeys = append(failedKeys, key) + mu.Unlock() + } + } + }(keys) + } + + wg.Wait() + close(semaphore) + + if err := p.Err(); err != nil { + return fmt.Errorf("failed to delete keys: %s", err) + } + if len(failedKeys) > 0 { + return fmt.Errorf("failed to delete keys: %s", strings.Join(failedKeys, ", ")) + } + + text.Success(out, "Deleted all keys from KV Store '%s'", c.storeID) return nil } diff --git a/pkg/commands/kvstoreentry/kvstoreentry_test.go b/pkg/commands/kvstoreentry/kvstoreentry_test.go index 509e01c0b..37570b82b 100644 --- a/pkg/commands/kvstoreentry/kvstoreentry_test.go +++ b/pkg/commands/kvstoreentry/kvstoreentry_test.go @@ -16,6 +16,7 @@ import ( fstfmt "github.com/fastly/cli/pkg/fmt" "github.com/fastly/cli/pkg/mock" "github.com/fastly/cli/pkg/testutil" + "github.com/fastly/cli/pkg/threadsafe" ) func TestCreateCommand(t *testing.T) { @@ -158,6 +159,14 @@ func TestDeleteCommand(t *testing.T) { Args: testutil.Args(kvstoreentry.RootName + " delete --key a-key"), WantError: "error parsing arguments: required flag --store-id not provided", }, + { + Args: testutil.Args(kvstoreentry.RootName + " delete --store-id " + storeID), + WantError: "invalid command, neither --all or --key provided", + }, + { + Args: testutil.Args(kvstoreentry.RootName + " delete --key a-key --all --store-id " + storeID), + WantError: "invalid flag combination, --all and --key", + }, { Args: testutil.Args(fmt.Sprintf("%s delete --store-id %s --key %s", kvstoreentry.RootName, storeID, itemKey)), API: mock.API{ @@ -185,20 +194,68 @@ func TestDeleteCommand(t *testing.T) { }, WantOutput: fstfmt.JSON(`{"key": "%s", "store_id": "%s", "deleted": true}`, itemKey, storeID), }, + { + Args: testutil.Args(fmt.Sprintf("%s delete --store-id %s --all --auto-yes", kvstoreentry.RootName, storeID)), + API: mock.API{ + NewListKVStoreKeysPaginatorFn: func(i *fastly.ListKVStoreKeysInput) fastly.PaginatorKVStoreEntries { + return &mockKVStoresEntriesPaginator{ + next: true, + keys: []string{"foo", "bar", "baz"}, + } + }, + DeleteKVStoreKeyFn: func(i *fastly.DeleteKVStoreKeyInput) error { + return nil + }, + }, + WantOutput: fmt.Sprintf(`Deleting key: bar +Deleting key: baz +Deleting key: foo + +SUCCESS: Deleted all keys from KV Store '%s' +`, storeID), + }, + { + Args: testutil.Args(fmt.Sprintf("%s delete --store-id %s --all --auto-yes", kvstoreentry.RootName, storeID)), + API: mock.API{ + NewListKVStoreKeysPaginatorFn: func(i *fastly.ListKVStoreKeysInput) fastly.PaginatorKVStoreEntries { + return &mockKVStoresEntriesPaginator{ + next: true, + keys: []string{"foo", "bar", "baz"}, + } + }, + DeleteKVStoreKeyFn: func(i *fastly.DeleteKVStoreKeyInput) error { + return errors.New("whoops") + }, + }, + WantError: "failed to delete keys: bar, baz, foo", + }, + { + Args: testutil.Args(fmt.Sprintf("%s delete --store-id %s --all --auto-yes", kvstoreentry.RootName, storeID)), + API: mock.API{ + NewListKVStoreKeysPaginatorFn: func(i *fastly.ListKVStoreKeysInput) fastly.PaginatorKVStoreEntries { + return &mockKVStoresEntriesPaginator{ + err: errors.New("whoops"), + } + }, + }, + WantError: "failed to delete keys: whoops", + }, } for _, testcase := range scenarios { testcase := testcase t.Run(testcase.Name, func(t *testing.T) { - var stdout bytes.Buffer + var stdout threadsafe.Buffer opts := testutil.NewRunOpts(testcase.Args, &stdout) opts.APIClient = mock.APIClient(testcase.API) err := app.Run(opts) + t.Log(stdout.String()) + testutil.AssertErrorContains(t, err, testcase.WantError) - testutil.AssertString(t, testcase.WantOutput, stdout.String()) + testutil.AssertStringContains(t, stdout.String(), testcase.WantOutput) }) } } @@ -313,7 +370,29 @@ func TestListCommand(t *testing.T) { err := app.Run(opts) testutil.AssertErrorContains(t, err, testcase.WantError) - testutil.AssertString(t, testcase.WantOutput, stdout.String()) + testutil.AssertStringContains(t, stdout.String(), testcase.WantOutput) }) } } + +type mockKVStoresEntriesPaginator struct { + next bool + keys []string + err error +} + +func (m *mockKVStoresEntriesPaginator) Next() bool { + ret := m.next + if m.next { + m.next = false // allow one instance of true before stopping + } + return ret +} + +func (m *mockKVStoresEntriesPaginator) Keys() []string { + return m.keys +} + +func (m *mockKVStoresEntriesPaginator) Err() error { + return m.err +} diff --git a/pkg/commands/kvstoreentry/list.go b/pkg/commands/kvstoreentry/list.go index 4b445e531..275af07fd 100644 --- a/pkg/commands/kvstoreentry/list.go +++ b/pkg/commands/kvstoreentry/list.go @@ -54,10 +54,32 @@ func (c *ListCommand) Exec(_ io.Reader, out io.Writer) error { c.Input.Cursor = cursor + spinner, err := text.NewSpinner(out) + if err != nil { + return err + } + msg := "Getting data" + + // A spinner produces output and is incompatible with JSON expected output. + if !c.JSONOutput.Enabled { + err := spinner.Start() + if err != nil { + return err + } + spinner.Message(msg + "... (this can take a few minutes depending on the number of entries)") + } + for { o, err := c.Globals.APIClient.ListKVStoreKeys(&c.Input) if err != nil { c.Globals.ErrLog.Add(err) + if !c.JSONOutput.Enabled { + spinner.StopFailMessage(msg) + spinErr := spinner.StopFail() + if spinErr != nil { + return spinErr + } + } return err } @@ -69,6 +91,23 @@ func (c *ListCommand) Exec(_ io.Reader, out io.Writer) error { } } + if !c.JSONOutput.Enabled { + spinner.StopMessage(msg) + err := spinner.Stop() + if err != nil { + return err + } + } + + if keys == nil { + if ok, err := c.WriteJSON(out, []string{}); ok { + return err + } + text.Break(out) + text.Output(out, "no keys") + return nil + } + if ok, err := c.WriteJSON(out, keys); ok { return err } diff --git a/pkg/errors/errors.go b/pkg/errors/errors.go index a569c73e0..e464bc9ab 100644 --- a/pkg/errors/errors.go +++ b/pkg/errors/errors.go @@ -101,12 +101,26 @@ var ErrBuildStopped = RemediationError{ } // ErrInvalidVerboseJSONCombo means the user provided both a --verbose and -// --json flag which are mutally exclusive behaviours. +// --json flag which are mutually exclusive behaviours. var ErrInvalidVerboseJSONCombo = RemediationError{ Inner: fmt.Errorf("invalid flag combination, --verbose and --json"), Remediation: "Use either --verbose or --json, not both.", } +// ErrInvalidDeleteAllKeyCombo means the user provided both a --all and --key +// flag which are mutually exclusive behaviours. +var ErrInvalidDeleteAllKeyCombo = RemediationError{ + Inner: fmt.Errorf("invalid flag combination, --all and --key"), + Remediation: "Use either --all or --key, not both.", +} + +// ErrMissingDeleteAllKeyCombo means the user omitted both the --all and --key +// flags and we need at least one of them. +var ErrMissingDeleteAllKeyCombo = RemediationError{ + Inner: fmt.Errorf("invalid command, neither --all or --key provided"), + Remediation: "Provide at least one of: --all or --key, not both.", +} + // ErrNoSTDINData indicates the --stdin flag was specified but no data was piped // into stdin. var ErrNoSTDINData = RemediationError{ diff --git a/pkg/mock/api.go b/pkg/mock/api.go index d60b7e411..110a319cb 100644 --- a/pkg/mock/api.go +++ b/pkg/mock/api.go @@ -280,6 +280,7 @@ type API struct { NewListACLEntriesPaginatorFn func(i *fastly.ListACLEntriesInput) fastly.PaginatorACLEntries NewListDictionaryItemsPaginatorFn func(i *fastly.ListDictionaryItemsInput) fastly.PaginatorDictionaryItems NewListServicesPaginatorFn func(i *fastly.ListServicesInput) fastly.PaginatorServices + NewListKVStoreKeysPaginatorFn func(i *fastly.ListKVStoreKeysInput) fastly.PaginatorKVStoreEntries GetCustomTLSConfigurationFn func(i *fastly.GetCustomTLSConfigurationInput) (*fastly.CustomTLSConfiguration, error) ListCustomTLSConfigurationsFn func(i *fastly.ListCustomTLSConfigurationsInput) ([]*fastly.CustomTLSConfiguration, error) @@ -1494,6 +1495,11 @@ func (m API) NewListServicesPaginator(i *fastly.ListServicesInput) fastly.Pagina return m.NewListServicesPaginatorFn(i) } +// NewListKVStoreKeysPaginator implements Interface. +func (m API) NewListKVStoreKeysPaginator(i *fastly.ListKVStoreKeysInput) fastly.PaginatorKVStoreEntries { + return m.NewListKVStoreKeysPaginatorFn(i) +} + // GetCustomTLSConfiguration implements Interface. func (m API) GetCustomTLSConfiguration(i *fastly.GetCustomTLSConfigurationInput) (*fastly.CustomTLSConfiguration, error) { return m.GetCustomTLSConfigurationFn(i)