From 9754205373d476303fe16df1d7c13117750672a5 Mon Sep 17 00:00:00 2001 From: Sebastian Choren Date: Wed, 28 Jun 2023 17:33:50 -0300 Subject: [PATCH 1/7] add delete verb --- cli/cmd/config.go | 28 +++++++------- cli/cmd/delete_cmd.go | 62 +++++++++++++++++-------------- cli/pkg/resourcemanager/client.go | 38 +++++++++++++++++-- cli/pkg/resourcemanager/delete.go | 44 ++++++++++++++++++++++ cli/pkg/resourcemanager/get.go | 2 +- cli/pkg/resourcemanager/list.go | 2 +- 6 files changed, 130 insertions(+), 46 deletions(-) create mode 100644 cli/pkg/resourcemanager/delete.go diff --git a/cli/cmd/config.go b/cli/cmd/config.go index 8a0907d3d8..5b2e6cd85d 100644 --- a/cli/cmd/config.go +++ b/cli/cmd/config.go @@ -55,61 +55,61 @@ var resources = resourcemanager.NewRegistry(). resourcemanager.NewClient( httpClient, "config", "configs", - resourcemanager.TableConfig{ + resourcemanager.WithTableConfig(resourcemanager.TableConfig{ Cells: []resourcemanager.TableCellConfig{ {Header: "ID", Path: "spec.id"}, {Header: "NAME", Path: "spec.name"}, {Header: "ANALYTICS ENABLED", Path: "spec.analyticsEnabled"}, }, - }, + }), ), ). Register( resourcemanager.NewClient( httpClient, "analyzer", "analyzers", - resourcemanager.TableConfig{ + resourcemanager.WithTableConfig(resourcemanager.TableConfig{ Cells: []resourcemanager.TableCellConfig{ {Header: "ID", Path: "spec.id"}, {Header: "NAME", Path: "spec.name"}, {Header: "ENABLED", Path: "spec.enabled"}, {Header: "MINIMUM SCORE", Path: "spec.minimumScore"}, }, - }, + }), ), ). Register( resourcemanager.NewClient( httpClient, "pollingprofile", "pollingprofiles", - resourcemanager.TableConfig{ + resourcemanager.WithTableConfig(resourcemanager.TableConfig{ Cells: []resourcemanager.TableCellConfig{ {Header: "ID", Path: "spec.id"}, {Header: "NAME", Path: "spec.name"}, {Header: "STRATEGY", Path: "spec.strategy"}, }, - }, + }), ), ). Register( resourcemanager.NewClient( httpClient, "demo", "demos", - resourcemanager.TableConfig{ + resourcemanager.WithTableConfig(resourcemanager.TableConfig{ Cells: []resourcemanager.TableCellConfig{ {Header: "ID", Path: "spec.id"}, {Header: "NAME", Path: "spec.name"}, {Header: "TYPE", Path: "spec.type"}, {Header: "ENABLED", Path: "spec.enabled"}, }, - }, + }), ), ). Register( resourcemanager.NewClient( httpClient, "datastore", "datastores", - resourcemanager.TableConfig{ + resourcemanager.WithTableConfig(resourcemanager.TableConfig{ Cells: []resourcemanager.TableCellConfig{ {Header: "ID", Path: "spec.id"}, {Header: "NAME", Path: "spec.name"}, @@ -124,27 +124,27 @@ var resources = resourcemanager.NewRegistry(). } return nil }, - }, + }), ), ). Register( resourcemanager.NewClient( httpClient, "environment", "environments", - resourcemanager.TableConfig{ + resourcemanager.WithTableConfig(resourcemanager.TableConfig{ Cells: []resourcemanager.TableCellConfig{ {Header: "ID", Path: "spec.id"}, {Header: "NAME", Path: "spec.name"}, {Header: "DESCRIPTION", Path: "spec.description"}, }, - }, + }), ), ). Register( resourcemanager.NewClient( httpClient, "transaction", "transactions", - resourcemanager.TableConfig{ + resourcemanager.WithTableConfig(resourcemanager.TableConfig{ Cells: []resourcemanager.TableCellConfig{ {Header: "ID", Path: "spec.id"}, {Header: "NAME", Path: "spec.name"}, @@ -174,7 +174,7 @@ var resources = resourcemanager.NewRegistry(). } return nil }, - }, + }), ), ) diff --git a/cli/cmd/delete_cmd.go b/cli/cmd/delete_cmd.go index c58c84b707..e588d84c4c 100644 --- a/cli/cmd/delete_cmd.go +++ b/cli/cmd/delete_cmd.go @@ -3,40 +3,48 @@ package cmd import ( "context" "fmt" - "strings" "github.com/kubeshop/tracetest/cli/parameters" + "github.com/kubeshop/tracetest/cli/pkg/resourcemanager" "github.com/spf13/cobra" ) -var deleteParams = ¶meters.ResourceIdParams{} - -var deleteCmd = &cobra.Command{ - GroupID: cmdGroupResources.ID, - Use: fmt.Sprintf("delete %s", strings.Join(parameters.ValidResources, "|")), - Short: "Delete resources", - Long: "Delete resources from your Tracetest server", - PreRun: setupCommand(), - Run: WithResourceMiddleware(func(_ *cobra.Command, args []string) (string, error) { - resourceType := resourceParams.ResourceName - ctx := context.Background() - - resourceActions, err := resourceRegistry.Get(resourceType) - if err != nil { - return "", err - } - - message, err := resourceActions.Delete(ctx, deleteParams.ResourceID) - if err != nil { - return "", err - } - - return fmt.Sprintf("✔ %s", message), nil - }, deleteParams), - PostRun: teardownCommand, -} +var ( + deleteParams = ¶meters.ResourceIdParams{} + deleteCmd *cobra.Command +) func init() { + deleteCmd = &cobra.Command{ + GroupID: cmdGroupResources.ID, + Use: "delete " + resourceList(), + Short: "Delete resources", + Long: "Delete resources from your Tracetest server", + PreRun: setupCommand(), + Run: WithResourceMiddleware(func(_ *cobra.Command, args []string) (string, error) { + resourceType := args[0] + ctx := context.Background() + + resourceClient, err := resources.Get(resourceType) + if err != nil { + return "", err + } + + resultFormat, err := resourcemanager.Formats.Get(output, "yaml") + if err != nil { + return "", err + } + + result, err := resourceClient.Delete(ctx, deleteParams.ResourceID, resultFormat) + if err != nil { + return "", err + } + + return fmt.Sprintf("✔ %s", result), nil + }, deleteParams), + PostRun: teardownCommand, + } + deleteCmd.Flags().StringVar(&deleteParams.ResourceID, "id", "", "id of the resource to delete") rootCmd.AddCommand(deleteCmd) } diff --git a/cli/pkg/resourcemanager/client.go b/cli/pkg/resourcemanager/client.go index 4da0e0da9e..49c7fac5c8 100644 --- a/cli/pkg/resourcemanager/client.go +++ b/cli/pkg/resourcemanager/client.go @@ -1,6 +1,7 @@ package resourcemanager import ( + "errors" "fmt" "io" "net/http" @@ -14,6 +15,7 @@ type client struct { client *HTTPClient resourceName string resourceNamePlural string + deleteSuccessMsg string tableConfig TableConfig } @@ -45,16 +47,41 @@ func (c HTTPClient) do(req *http.Request) (*http.Response, error) { return c.client.Do(req) } +type options func(c *client) + +func WithDeleteEnabled(deleteSuccessMssg string) options { + return func(c *client) { + c.deleteSuccessMsg = deleteSuccessMssg + } +} + +func WithTableConfig(tableConfig TableConfig) options { + return func(c *client) { + c.tableConfig = tableConfig + } +} + +var ErrNotSupportedResourceAction = errors.New("the specified resource type doesn't support the action") + // NewClient creates a new client for a resource managed by the resourceamanger. // The tableConfig parameter configures how the table view should be rendered. // This configuration work both for a single resource from a Get, or a ResourceList from a List -func NewClient(httpClient *HTTPClient, resourceName, resourceNamePlural string, tableConfig TableConfig) client { - return client{ +func NewClient( + httpClient *HTTPClient, + resourceName, resourceNamePlural string, + opts ...options) client { + c := client{ client: httpClient, resourceName: resourceName, resourceNamePlural: resourceNamePlural, - tableConfig: tableConfig, } + + for _, opt := range opts { + opt(&c) + } + + return c + } type requestError struct { @@ -66,6 +93,11 @@ func (e requestError) Error() string { return e.Message } +func isSuccessResponse(resp *http.Response) bool { + // successfull http status codes are 2xx + return resp.StatusCode >= 200 && resp.StatusCode < 300 +} + func parseRequestError(resp *http.Response, format Format) error { body, err := io.ReadAll(resp.Body) if err != nil { diff --git a/cli/pkg/resourcemanager/delete.go b/cli/pkg/resourcemanager/delete.go new file mode 100644 index 0000000000..e2ab8b63d6 --- /dev/null +++ b/cli/pkg/resourcemanager/delete.go @@ -0,0 +1,44 @@ +package resourcemanager + +import ( + "context" + "fmt" + "net/http" +) + +const VerbDelete Verb = "delete" + +func (c client) Delete(ctx context.Context, id string, format Format) (string, error) { + if c.deleteSuccessMsg == "" { + return "", ErrNotSupportedResourceAction + } + + url := c.client.url(c.resourceNamePlural, id) + req, err := http.NewRequestWithContext(ctx, http.MethodDelete, url.String(), nil) + if err != nil { + return "", fmt.Errorf("cannot build Delete request: %w", err) + } + + err = format.BuildRequest(req, VerbDelete) + if err != nil { + return "", fmt.Errorf("cannot build Delete request: %w", err) + } + + resp, err := c.client.do(req) + if err != nil { + return "", fmt.Errorf("cannot execute Delete request: %w", err) + } + defer resp.Body.Close() + + if !isSuccessResponse(resp) { + err := parseRequestError(resp, format) + reqErr, ok := err.(requestError) + if ok && reqErr.Code == http.StatusNotFound { + return "", fmt.Errorf("Resource %s with ID %s not found", c.resourceName, id) + } + + return "", fmt.Errorf("could not Delete resource: %w", err) + } + + return c.deleteSuccessMsg, nil +} diff --git a/cli/pkg/resourcemanager/get.go b/cli/pkg/resourcemanager/get.go index aefd3f3f08..d651ca30ad 100644 --- a/cli/pkg/resourcemanager/get.go +++ b/cli/pkg/resourcemanager/get.go @@ -27,7 +27,7 @@ func (c client) Get(ctx context.Context, id string, format Format) (string, erro } defer resp.Body.Close() - if resp.StatusCode != http.StatusOK { + if !isSuccessResponse(resp) { err := parseRequestError(resp, format) reqErr, ok := err.(requestError) if ok && reqErr.Code == http.StatusNotFound { diff --git a/cli/pkg/resourcemanager/list.go b/cli/pkg/resourcemanager/list.go index 59e8147b10..287f6e5b3a 100644 --- a/cli/pkg/resourcemanager/list.go +++ b/cli/pkg/resourcemanager/list.go @@ -44,7 +44,7 @@ func (c client) List(ctx context.Context, opt ListOption, format Format) (string } defer resp.Body.Close() - if resp.StatusCode != http.StatusOK { + if !isSuccessResponse(resp) { err := parseRequestError(resp, format) return "", fmt.Errorf("could not list resource: %w", err) From 780f792907cb931c461167d4043d6795a369b0d5 Mon Sep 17 00:00:00 2001 From: Sebastian Choren Date: Wed, 28 Jun 2023 17:53:06 -0300 Subject: [PATCH 2/7] fixes --- cli/cmd/config.go | 4 ++++ cli/cmd/delete_cmd.go | 2 +- cli/pkg/resourcemanager/delete.go | 5 +++++ testing/cli-e2etest/testscenarios/demo/delete_demo_test.go | 2 +- .../testscenarios/environment/delete_environment_test.go | 2 +- .../testscenarios/transaction/delete_transaction_test.go | 2 +- 6 files changed, 13 insertions(+), 4 deletions(-) diff --git a/cli/cmd/config.go b/cli/cmd/config.go index 5b2e6cd85d..4905a76740 100644 --- a/cli/cmd/config.go +++ b/cli/cmd/config.go @@ -103,6 +103,7 @@ var resources = resourcemanager.NewRegistry(). {Header: "ENABLED", Path: "spec.enabled"}, }, }), + resourcemanager.WithDeleteEnabled("Demo successfully deleted"), ), ). Register( @@ -125,6 +126,7 @@ var resources = resourcemanager.NewRegistry(). return nil }, }), + resourcemanager.WithDeleteEnabled("DataStore removed. Defaulting back to no-tracing mode"), ), ). Register( @@ -138,6 +140,7 @@ var resources = resourcemanager.NewRegistry(). {Header: "DESCRIPTION", Path: "spec.description"}, }, }), + resourcemanager.WithDeleteEnabled("Environment successfully deleted"), ), ). Register( @@ -175,6 +178,7 @@ var resources = resourcemanager.NewRegistry(). return nil }, }), + resourcemanager.WithDeleteEnabled("Transaction successfully deleted"), ), ) diff --git a/cli/cmd/delete_cmd.go b/cli/cmd/delete_cmd.go index e588d84c4c..e325767a11 100644 --- a/cli/cmd/delete_cmd.go +++ b/cli/cmd/delete_cmd.go @@ -22,7 +22,7 @@ func init() { Long: "Delete resources from your Tracetest server", PreRun: setupCommand(), Run: WithResourceMiddleware(func(_ *cobra.Command, args []string) (string, error) { - resourceType := args[0] + resourceType := resourceParams.ResourceName ctx := context.Background() resourceClient, err := resources.Get(resourceType) diff --git a/cli/pkg/resourcemanager/delete.go b/cli/pkg/resourcemanager/delete.go index e2ab8b63d6..e6c1527d6a 100644 --- a/cli/pkg/resourcemanager/delete.go +++ b/cli/pkg/resourcemanager/delete.go @@ -4,11 +4,13 @@ import ( "context" "fmt" "net/http" + "net/http/httputil" ) const VerbDelete Verb = "delete" func (c client) Delete(ctx context.Context, id string, format Format) (string, error) { + fmt.Println(c.deleteSuccessMsg) if c.deleteSuccessMsg == "" { return "", ErrNotSupportedResourceAction } @@ -19,6 +21,9 @@ func (c client) Delete(ctx context.Context, id string, format Format) (string, e return "", fmt.Errorf("cannot build Delete request: %w", err) } + d, _ := httputil.DumpRequestOut(req, true) + fmt.Println(string(d)) + err = format.BuildRequest(req, VerbDelete) if err != nil { return "", fmt.Errorf("cannot build Delete request: %w", err) diff --git a/testing/cli-e2etest/testscenarios/demo/delete_demo_test.go b/testing/cli-e2etest/testscenarios/demo/delete_demo_test.go index 89e2739af6..3f78d91002 100644 --- a/testing/cli-e2etest/testscenarios/demo/delete_demo_test.go +++ b/testing/cli-e2etest/testscenarios/demo/delete_demo_test.go @@ -28,7 +28,7 @@ func TestDeleteDemo(t *testing.T) { // Then it should return an error and say that this resource does not exist result := tracetestcli.Exec(t, "delete demo --id some-demo", tracetestcli.WithCLIConfig(cliConfig)) helpers.RequireExitCodeEqual(t, result, 1) - require.Contains(result.StdErr, "Resource demos with ID some-demo not found") // TODO: update this message to singular + require.Contains(result.StdErr, "Resource demo with ID some-demo not found") // TODO: update this message to singular // When I try to set up a new demo // Then it should be applied with success diff --git a/testing/cli-e2etest/testscenarios/environment/delete_environment_test.go b/testing/cli-e2etest/testscenarios/environment/delete_environment_test.go index d717798d72..82258eb8e2 100644 --- a/testing/cli-e2etest/testscenarios/environment/delete_environment_test.go +++ b/testing/cli-e2etest/testscenarios/environment/delete_environment_test.go @@ -28,7 +28,7 @@ func TestDeleteEnvironment(t *testing.T) { // Then it should return an error and say that this resource does not exist result := tracetestcli.Exec(t, "delete environment --id .env", tracetestcli.WithCLIConfig(cliConfig)) helpers.RequireExitCodeEqual(t, result, 1) - require.Contains(result.StdErr, "Resource environments with ID .env not found") // TODO: update this message to singular + require.Contains(result.StdErr, "Resource environment with ID .env not found") // TODO: update this message to singular // When I try to set up a new environment // Then it should be applied with success diff --git a/testing/cli-e2etest/testscenarios/transaction/delete_transaction_test.go b/testing/cli-e2etest/testscenarios/transaction/delete_transaction_test.go index a185a7a9d6..3244306c81 100644 --- a/testing/cli-e2etest/testscenarios/transaction/delete_transaction_test.go +++ b/testing/cli-e2etest/testscenarios/transaction/delete_transaction_test.go @@ -27,7 +27,7 @@ func TestDeleteTransaction(t *testing.T) { // Then it should return an error and say that this resource does not exist result := tracetestcli.Exec(t, "delete transaction --id dont-exist", tracetestcli.WithCLIConfig(cliConfig)) helpers.RequireExitCodeEqual(t, result, 1) - require.Contains(result.StdErr, "Resource transactions with ID dont-exist not found") // TODO: update this message to singular + require.Contains(result.StdErr, "Resource transaction with ID dont-exist not found") // TODO: update this message to singular // When I try to set up a new transaction // Then it should be applied with success From 3cf4503fa3e167dd34afd8ffd759605f0ffd1e0a Mon Sep 17 00:00:00 2001 From: Sebastian Choren Date: Wed, 28 Jun 2023 18:08:34 -0300 Subject: [PATCH 3/7] fix test --- .../testscenarios/transaction/delete_transaction_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testing/cli-e2etest/testscenarios/transaction/delete_transaction_test.go b/testing/cli-e2etest/testscenarios/transaction/delete_transaction_test.go index 3244306c81..be0005d0e2 100644 --- a/testing/cli-e2etest/testscenarios/transaction/delete_transaction_test.go +++ b/testing/cli-e2etest/testscenarios/transaction/delete_transaction_test.go @@ -46,5 +46,5 @@ func TestDeleteTransaction(t *testing.T) { // Then it should return a message saying that the transaction was not found result = tracetestcli.Exec(t, "delete transaction --id Qti5R3_VR", tracetestcli.WithCLIConfig(cliConfig)) helpers.RequireExitCodeEqual(t, result, 1) - require.Contains(result.StdErr, "Resource transactions with ID Qti5R3_VR not found") + require.Contains(result.StdErr, "Resource transaction with ID Qti5R3_VR not found") } From 7d363b60d88b536d12a6168ba700b8ae86a485c6 Mon Sep 17 00:00:00 2001 From: Sebastian Choren Date: Thu, 29 Jun 2023 11:33:45 -0300 Subject: [PATCH 4/7] Update testing/cli-e2etest/testscenarios/demo/delete_demo_test.go Co-authored-by: Daniel Baptista Dias --- testing/cli-e2etest/testscenarios/demo/delete_demo_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testing/cli-e2etest/testscenarios/demo/delete_demo_test.go b/testing/cli-e2etest/testscenarios/demo/delete_demo_test.go index 3f78d91002..e61b615dbf 100644 --- a/testing/cli-e2etest/testscenarios/demo/delete_demo_test.go +++ b/testing/cli-e2etest/testscenarios/demo/delete_demo_test.go @@ -28,7 +28,7 @@ func TestDeleteDemo(t *testing.T) { // Then it should return an error and say that this resource does not exist result := tracetestcli.Exec(t, "delete demo --id some-demo", tracetestcli.WithCLIConfig(cliConfig)) helpers.RequireExitCodeEqual(t, result, 1) - require.Contains(result.StdErr, "Resource demo with ID some-demo not found") // TODO: update this message to singular + require.Contains(result.StdErr, "Resource demo with ID some-demo not found") // When I try to set up a new demo // Then it should be applied with success From fb5308b07939eeeae45d60b98a00829bd0fa3f1c Mon Sep 17 00:00:00 2001 From: Sebastian Choren Date: Thu, 29 Jun 2023 11:33:52 -0300 Subject: [PATCH 5/7] Update testing/cli-e2etest/testscenarios/environment/delete_environment_test.go Co-authored-by: Daniel Baptista Dias --- .../testscenarios/environment/delete_environment_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testing/cli-e2etest/testscenarios/environment/delete_environment_test.go b/testing/cli-e2etest/testscenarios/environment/delete_environment_test.go index 82258eb8e2..343d56353d 100644 --- a/testing/cli-e2etest/testscenarios/environment/delete_environment_test.go +++ b/testing/cli-e2etest/testscenarios/environment/delete_environment_test.go @@ -28,7 +28,7 @@ func TestDeleteEnvironment(t *testing.T) { // Then it should return an error and say that this resource does not exist result := tracetestcli.Exec(t, "delete environment --id .env", tracetestcli.WithCLIConfig(cliConfig)) helpers.RequireExitCodeEqual(t, result, 1) - require.Contains(result.StdErr, "Resource environment with ID .env not found") // TODO: update this message to singular + require.Contains(result.StdErr, "Resource environment with ID .env not found") // When I try to set up a new environment // Then it should be applied with success From 2e816e7db79e46135d36a701046b2d944484b739 Mon Sep 17 00:00:00 2001 From: Sebastian Choren Date: Thu, 29 Jun 2023 11:33:59 -0300 Subject: [PATCH 6/7] Update testing/cli-e2etest/testscenarios/transaction/delete_transaction_test.go Co-authored-by: Daniel Baptista Dias --- .../testscenarios/transaction/delete_transaction_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testing/cli-e2etest/testscenarios/transaction/delete_transaction_test.go b/testing/cli-e2etest/testscenarios/transaction/delete_transaction_test.go index be0005d0e2..611c2a9260 100644 --- a/testing/cli-e2etest/testscenarios/transaction/delete_transaction_test.go +++ b/testing/cli-e2etest/testscenarios/transaction/delete_transaction_test.go @@ -27,7 +27,7 @@ func TestDeleteTransaction(t *testing.T) { // Then it should return an error and say that this resource does not exist result := tracetestcli.Exec(t, "delete transaction --id dont-exist", tracetestcli.WithCLIConfig(cliConfig)) helpers.RequireExitCodeEqual(t, result, 1) - require.Contains(result.StdErr, "Resource transaction with ID dont-exist not found") // TODO: update this message to singular + require.Contains(result.StdErr, "Resource transaction with ID dont-exist not found") // When I try to set up a new transaction // Then it should be applied with success From 5578c98adb53a3e107b5ac46345ebe2b5014463c Mon Sep 17 00:00:00 2001 From: Sebastian Choren Date: Thu, 29 Jun 2023 11:34:59 -0300 Subject: [PATCH 7/7] remove debug code --- cli/pkg/resourcemanager/delete.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/cli/pkg/resourcemanager/delete.go b/cli/pkg/resourcemanager/delete.go index e6c1527d6a..e2ab8b63d6 100644 --- a/cli/pkg/resourcemanager/delete.go +++ b/cli/pkg/resourcemanager/delete.go @@ -4,13 +4,11 @@ import ( "context" "fmt" "net/http" - "net/http/httputil" ) const VerbDelete Verb = "delete" func (c client) Delete(ctx context.Context, id string, format Format) (string, error) { - fmt.Println(c.deleteSuccessMsg) if c.deleteSuccessMsg == "" { return "", ErrNotSupportedResourceAction } @@ -21,9 +19,6 @@ func (c client) Delete(ctx context.Context, id string, format Format) (string, e return "", fmt.Errorf("cannot build Delete request: %w", err) } - d, _ := httputil.DumpRequestOut(req, true) - fmt.Println(string(d)) - err = format.BuildRequest(req, VerbDelete) if err != nil { return "", fmt.Errorf("cannot build Delete request: %w", err)