From 958b6f1d24b756b783fe8a9576ec444d0b835158 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20=22Pisco=22=20Fernandes?= Date: Wed, 20 Sep 2023 11:10:50 +0100 Subject: [PATCH] TUN-7813: Improve tunnel delete command to use cascade delete ## Summary Previously the force flag in the tunnel delete command was only explicitly deleting the connections of a tunnel. Therefore, we are changing it to use the cascade query parameter supported by the API. That parameter will delegate to the server the deletion of the tunnel dependencies implicitly instead of the client doing it explicitly. This means that not only the connections will get deleted, but also the tunnel routes, ensuring that no dependencies are left without a non-deleted tunnel. --- cfapi/client.go | 2 +- cfapi/tunnel.go | 7 ++++++- cmd/cloudflared/tunnel/subcommand_context.go | 9 ++------- cmd/cloudflared/tunnel/subcommand_context_test.go | 2 +- cmd/cloudflared/tunnel/subcommands.go | 4 ++-- 5 files changed, 12 insertions(+), 12 deletions(-) diff --git a/cfapi/client.go b/cfapi/client.go index 0431ad53..4b7e3508 100644 --- a/cfapi/client.go +++ b/cfapi/client.go @@ -9,7 +9,7 @@ type TunnelClient interface { GetTunnel(tunnelID uuid.UUID) (*Tunnel, error) GetTunnelToken(tunnelID uuid.UUID) (string, error) GetManagementToken(tunnelID uuid.UUID) (string, error) - DeleteTunnel(tunnelID uuid.UUID) error + DeleteTunnel(tunnelID uuid.UUID, cascade bool) error ListTunnels(filter *TunnelFilter) ([]*Tunnel, error) ListActiveClients(tunnelID uuid.UUID) ([]*ActiveClient, error) CleanupConnections(tunnelID uuid.UUID, params *CleanupParams) error diff --git a/cfapi/tunnel.go b/cfapi/tunnel.go index fa6f8f33..d1b5f3a2 100644 --- a/cfapi/tunnel.go +++ b/cfapi/tunnel.go @@ -159,9 +159,14 @@ func (r *RESTClient) GetManagementToken(tunnelID uuid.UUID) (token string, err e return "", r.statusCodeToError("get tunnel token", resp) } -func (r *RESTClient) DeleteTunnel(tunnelID uuid.UUID) error { +func (r *RESTClient) DeleteTunnel(tunnelID uuid.UUID, cascade bool) error { endpoint := r.baseEndpoints.accountLevel endpoint.Path = path.Join(endpoint.Path, fmt.Sprintf("%v", tunnelID)) + // Cascade will delete all tunnel dependencies (connections, routes, etc.) that + // are linked to the deleted tunnel. + if cascade { + endpoint.RawQuery = "cascade=true" + } resp, err := r.sendRequest("DELETE", endpoint, nil) if err != nil { return errors.Wrap(err, "REST request failed") diff --git a/cmd/cloudflared/tunnel/subcommand_context.go b/cmd/cloudflared/tunnel/subcommand_context.go index f49c15eb..c0bddd9d 100644 --- a/cmd/cloudflared/tunnel/subcommand_context.go +++ b/cmd/cloudflared/tunnel/subcommand_context.go @@ -156,7 +156,7 @@ func (sc *subcommandContext) create(name string, credentialsFilePath string, sec var errorLines []string errorLines = append(errorLines, fmt.Sprintf("Your tunnel '%v' was created with ID %v. However, cloudflared couldn't write tunnel credentials to %s.", tunnel.Name, tunnel.ID, credentialsFilePath)) errorLines = append(errorLines, fmt.Sprintf("The file-writing error is: %v", writeFileErr)) - if deleteErr := client.DeleteTunnel(tunnel.ID); deleteErr != nil { + if deleteErr := client.DeleteTunnel(tunnel.ID, true); deleteErr != nil { errorLines = append(errorLines, fmt.Sprintf("Cloudflared tried to delete the tunnel for you, but encountered an error. You should use `cloudflared tunnel delete %v` to delete the tunnel yourself, because the tunnel can't be run without the tunnelfile.", tunnel.ID)) errorLines = append(errorLines, fmt.Sprintf("The delete tunnel error is: %v", deleteErr)) } else { @@ -206,13 +206,8 @@ func (sc *subcommandContext) delete(tunnelIDs []uuid.UUID) error { if !tunnel.DeletedAt.IsZero() { return fmt.Errorf("Tunnel %s has already been deleted", tunnel.ID) } - if forceFlagSet { - if err := client.CleanupConnections(tunnel.ID, cfapi.NewCleanupParams()); err != nil { - return errors.Wrapf(err, "Error cleaning up connections for tunnel %s", tunnel.ID) - } - } - if err := client.DeleteTunnel(tunnel.ID); err != nil { + if err := client.DeleteTunnel(tunnel.ID, forceFlagSet); err != nil { return errors.Wrapf(err, "Error deleting tunnel %s", tunnel.ID) } diff --git a/cmd/cloudflared/tunnel/subcommand_context_test.go b/cmd/cloudflared/tunnel/subcommand_context_test.go index c2293463..ca2a42b2 100644 --- a/cmd/cloudflared/tunnel/subcommand_context_test.go +++ b/cmd/cloudflared/tunnel/subcommand_context_test.go @@ -219,7 +219,7 @@ func (d *deleteMockTunnelStore) GetTunnelToken(tunnelID uuid.UUID) (string, erro return "token", nil } -func (d *deleteMockTunnelStore) DeleteTunnel(tunnelID uuid.UUID) error { +func (d *deleteMockTunnelStore) DeleteTunnel(tunnelID uuid.UUID, cascade bool) error { tunnel, ok := d.mockTunnels[tunnelID] if !ok { return fmt.Errorf("Couldn't find tunnel: %v", tunnelID) diff --git a/cmd/cloudflared/tunnel/subcommands.go b/cmd/cloudflared/tunnel/subcommands.go index 8da12c19..bef86887 100644 --- a/cmd/cloudflared/tunnel/subcommands.go +++ b/cmd/cloudflared/tunnel/subcommands.go @@ -119,8 +119,8 @@ var ( forceDeleteFlag = &cli.BoolFlag{ Name: "force", Aliases: []string{"f"}, - Usage: "Cleans up any stale connections before the tunnel is deleted. cloudflared will not " + - "delete a tunnel with connections without this flag.", + Usage: "Deletes a tunnel even if tunnel is connected and it has dependencies associated to it. (eg. IP routes)." + + " It is not possible to delete tunnels that have connections or non-deleted dependencies, without this flag.", EnvVars: []string{"TUNNEL_RUN_FORCE_OVERWRITE"}, } selectProtocolFlag = altsrc.NewStringFlag(&cli.StringFlag{