Skip to content

Commit

Permalink
TUN-7813: Improve tunnel delete command to use cascade delete
Browse files Browse the repository at this point in the history
## 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.
  • Loading branch information
jcsf committed Sep 20, 2023
1 parent 6d1d91d commit 958b6f1
Show file tree
Hide file tree
Showing 5 changed files with 12 additions and 12 deletions.
2 changes: 1 addition & 1 deletion cfapi/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 6 additions & 1 deletion cfapi/tunnel.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
9 changes: 2 additions & 7 deletions cmd/cloudflared/tunnel/subcommand_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
}

Expand Down
2 changes: 1 addition & 1 deletion cmd/cloudflared/tunnel/subcommand_context_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions cmd/cloudflared/tunnel/subcommands.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down

0 comments on commit 958b6f1

Please sign in to comment.