feat(cli): refactor Delete to new resource manager client (#2836)

cli/cmd/config.go

@@ -55,61 +55,62 @@ 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"},
 				},
-			},
+			}),
+			resourcemanager.WithDeleteEnabled("Demo successfully deleted"),
 		),
 	).
 	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 +125,29 @@ var resources = resourcemanager.NewRegistry().
 					}
 					return nil
 				},
-			},
+			}),
+			resourcemanager.WithDeleteEnabled("DataStore removed. Defaulting back to no-tracing mode"),
 		),
 	).
 	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"},
 				},
-			},
+			}),
+			resourcemanager.WithDeleteEnabled("Environment successfully deleted"),
 		),
 	).
 	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 +177,8 @@ var resources = resourcemanager.NewRegistry().
 					}
 					return nil
 				},
-			},
+			}),
+			resourcemanager.WithDeleteEnabled("Transaction successfully deleted"),
 		),
 	)
 

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 = &parameters.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 = &parameters.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 := resourceParams.ResourceName
+			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)
 }

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 {

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
+}

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 {

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)

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") 
 
 	// When I try to set up a new demo
 	// Then it should be applied with success

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") 
 
 	// When I try to set up a new environment
 	// Then it should be applied with success

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") 
 
 	// When I try to set up a new transaction
 	// Then it should be applied with success
@@ -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")
 }