TUN-7787: Refactor cloudflared to use new route endpoints based on route IDs

This commits makes sure that cloudflared starts using the new API
endpoints for managing routes.

Additionally, the delete route operation still allows deleting by CIDR
and VNet but it is being marked as deprecated in favor of specifying the
route ID.

The goal of this change is to make it simpler for the user to delete
routes without specifying Vnet.
This commit is contained in:
João Oliveirinha 2023-09-15 15:17:23 +01:00
parent fc0ecf4185
commit 6d1d91d9f9
6 changed files with 84 additions and 54 deletions

View File

@ -22,7 +22,7 @@ type HostnameClient interface {
type IPRouteClient interface { type IPRouteClient interface {
ListRoutes(filter *IpRouteFilter) ([]*DetailedRoute, error) ListRoutes(filter *IpRouteFilter) ([]*DetailedRoute, error)
AddRoute(newRoute NewRoute) (Route, error) AddRoute(newRoute NewRoute) (Route, error)
DeleteRoute(params DeleteRouteParams) error DeleteRoute(id uuid.UUID) error
GetByIP(params GetRouteByIpParams) (DetailedRoute, error) GetByIP(params GetRouteByIpParams) (DetailedRoute, error)
} }

View File

@ -75,10 +75,12 @@ type NewRoute struct {
// MarshalJSON handles fields with non-JSON types (e.g. net.IPNet). // MarshalJSON handles fields with non-JSON types (e.g. net.IPNet).
func (r NewRoute) MarshalJSON() ([]byte, error) { func (r NewRoute) MarshalJSON() ([]byte, error) {
return json.Marshal(&struct { return json.Marshal(&struct {
Network string `json:"network"`
TunnelID uuid.UUID `json:"tunnel_id"` TunnelID uuid.UUID `json:"tunnel_id"`
Comment string `json:"comment"` Comment string `json:"comment"`
VNetID *uuid.UUID `json:"virtual_network_id,omitempty"` VNetID *uuid.UUID `json:"virtual_network_id,omitempty"`
}{ }{
Network: r.Network.String(),
TunnelID: r.TunnelID, TunnelID: r.TunnelID,
Comment: r.Comment, Comment: r.Comment,
VNetID: r.VNetID, VNetID: r.VNetID,
@ -87,6 +89,7 @@ func (r NewRoute) MarshalJSON() ([]byte, error) {
// DetailedRoute is just a Route with some extra fields, e.g. TunnelName. // DetailedRoute is just a Route with some extra fields, e.g. TunnelName.
type DetailedRoute struct { type DetailedRoute struct {
ID uuid.UUID `json:"id"`
Network CIDR `json:"network"` Network CIDR `json:"network"`
TunnelID uuid.UUID `json:"tunnel_id"` TunnelID uuid.UUID `json:"tunnel_id"`
// Optional field. When unset, it means the DetailedRoute belongs to the default virtual network. // Optional field. When unset, it means the DetailedRoute belongs to the default virtual network.
@ -115,7 +118,8 @@ func (r DetailedRoute) TableString() string {
} }
return fmt.Sprintf( return fmt.Sprintf(
"%s\t%s\t%s\t%s\t%s\t%s\t%s\t", "%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\t",
r.ID,
r.Network.String(), r.Network.String(),
vnetColumn, vnetColumn,
r.Comment, r.Comment,
@ -126,12 +130,6 @@ func (r DetailedRoute) TableString() string {
) )
} }
type DeleteRouteParams struct {
Network net.IPNet
// Optional field. If unset, backend will assume the default vnet for the account.
VNetID *uuid.UUID
}
type GetRouteByIpParams struct { type GetRouteByIpParams struct {
Ip net.IP Ip net.IP
// Optional field. If unset, backend will assume the default vnet for the account. // Optional field. If unset, backend will assume the default vnet for the account.
@ -158,7 +156,7 @@ func (r *RESTClient) ListRoutes(filter *IpRouteFilter) ([]*DetailedRoute, error)
// AddRoute calls the Tunnelstore POST endpoint for a given route. // AddRoute calls the Tunnelstore POST endpoint for a given route.
func (r *RESTClient) AddRoute(newRoute NewRoute) (Route, error) { func (r *RESTClient) AddRoute(newRoute NewRoute) (Route, error) {
endpoint := r.baseEndpoints.accountRoutes endpoint := r.baseEndpoints.accountRoutes
endpoint.Path = path.Join(endpoint.Path, "network", url.PathEscape(newRoute.Network.String())) endpoint.Path = path.Join(endpoint.Path)
resp, err := r.sendRequest("POST", endpoint, newRoute) resp, err := r.sendRequest("POST", endpoint, newRoute)
if err != nil { if err != nil {
return Route{}, errors.Wrap(err, "REST request failed") return Route{}, errors.Wrap(err, "REST request failed")
@ -173,10 +171,9 @@ func (r *RESTClient) AddRoute(newRoute NewRoute) (Route, error) {
} }
// DeleteRoute calls the Tunnelstore DELETE endpoint for a given route. // DeleteRoute calls the Tunnelstore DELETE endpoint for a given route.
func (r *RESTClient) DeleteRoute(params DeleteRouteParams) error { func (r *RESTClient) DeleteRoute(id uuid.UUID) error {
endpoint := r.baseEndpoints.accountRoutes endpoint := r.baseEndpoints.accountRoutes
endpoint.Path = path.Join(endpoint.Path, "network", url.PathEscape(params.Network.String())) endpoint.Path = path.Join(endpoint.Path, url.PathEscape(id.String()))
setVnetParam(&endpoint, params.VNetID)
resp, err := r.sendRequest("DELETE", endpoint, nil) resp, err := r.sendRequest("DELETE", endpoint, nil)
if err != nil { if err != nil {

View File

@ -58,31 +58,29 @@ type IpRouteFilter struct {
// NewIpRouteFilterFromCLI parses CLI flags to discover which filters should get applied. // NewIpRouteFilterFromCLI parses CLI flags to discover which filters should get applied.
func NewIpRouteFilterFromCLI(c *cli.Context) (*IpRouteFilter, error) { func NewIpRouteFilterFromCLI(c *cli.Context) (*IpRouteFilter, error) {
f := &IpRouteFilter{ f := NewIPRouteFilter()
queryParams: url.Values{},
}
// Set deletion filter // Set deletion filter
if flag := filterIpRouteDeleted.Name; c.IsSet(flag) && c.Bool(flag) { if flag := filterIpRouteDeleted.Name; c.IsSet(flag) && c.Bool(flag) {
f.deleted() f.Deleted()
} else { } else {
f.notDeleted() f.NotDeleted()
} }
if subset, err := cidrFromFlag(c, filterSubsetIpRoute); err != nil { if subset, err := cidrFromFlag(c, filterSubsetIpRoute); err != nil {
return nil, err return nil, err
} else if subset != nil { } else if subset != nil {
f.networkIsSupersetOf(*subset) f.NetworkIsSupersetOf(*subset)
} }
if superset, err := cidrFromFlag(c, filterSupersetIpRoute); err != nil { if superset, err := cidrFromFlag(c, filterSupersetIpRoute); err != nil {
return nil, err return nil, err
} else if superset != nil { } else if superset != nil {
f.networkIsSupersetOf(*superset) f.NetworkIsSupersetOf(*superset)
} }
if comment := c.String(filterIpRouteComment.Name); comment != "" { if comment := c.String(filterIpRouteComment.Name); comment != "" {
f.commentIs(comment) f.CommentIs(comment)
} }
if tunnelID := c.String(filterIpRouteTunnelID.Name); tunnelID != "" { if tunnelID := c.String(filterIpRouteTunnelID.Name); tunnelID != "" {
@ -90,7 +88,7 @@ func NewIpRouteFilterFromCLI(c *cli.Context) (*IpRouteFilter, error) {
if err != nil { if err != nil {
return nil, errors.Wrapf(err, "Couldn't parse UUID from %s", filterIpRouteTunnelID.Name) return nil, errors.Wrapf(err, "Couldn't parse UUID from %s", filterIpRouteTunnelID.Name)
} }
f.tunnelID(u) f.TunnelID(u)
} }
if vnetId := c.String(filterIpRouteByVnet.Name); vnetId != "" { if vnetId := c.String(filterIpRouteByVnet.Name); vnetId != "" {
@ -98,7 +96,7 @@ func NewIpRouteFilterFromCLI(c *cli.Context) (*IpRouteFilter, error) {
if err != nil { if err != nil {
return nil, errors.Wrapf(err, "Couldn't parse UUID from %s", filterIpRouteByVnet.Name) return nil, errors.Wrapf(err, "Couldn't parse UUID from %s", filterIpRouteByVnet.Name)
} }
f.vnetID(u) f.VNetID(u)
} }
if maxFetch := c.Int("max-fetch-size"); maxFetch > 0 { if maxFetch := c.Int("max-fetch-size"); maxFetch > 0 {
@ -124,35 +122,39 @@ func cidrFromFlag(c *cli.Context, flag cli.StringFlag) (*net.IPNet, error) {
return subset, nil return subset, nil
} }
func (f *IpRouteFilter) commentIs(comment string) { func NewIPRouteFilter() *IpRouteFilter {
return &IpRouteFilter{queryParams: url.Values{}}
}
func (f *IpRouteFilter) CommentIs(comment string) {
f.queryParams.Set("comment", comment) f.queryParams.Set("comment", comment)
} }
func (f *IpRouteFilter) notDeleted() { func (f *IpRouteFilter) NotDeleted() {
f.queryParams.Set("is_deleted", "false") f.queryParams.Set("is_deleted", "false")
} }
func (f *IpRouteFilter) deleted() { func (f *IpRouteFilter) Deleted() {
f.queryParams.Set("is_deleted", "true") f.queryParams.Set("is_deleted", "true")
} }
func (f *IpRouteFilter) networkIsSubsetOf(superset net.IPNet) { func (f *IpRouteFilter) NetworkIsSubsetOf(superset net.IPNet) {
f.queryParams.Set("network_subset", superset.String()) f.queryParams.Set("network_subset", superset.String())
} }
func (f *IpRouteFilter) networkIsSupersetOf(subset net.IPNet) { func (f *IpRouteFilter) NetworkIsSupersetOf(subset net.IPNet) {
f.queryParams.Set("network_superset", subset.String()) f.queryParams.Set("network_superset", subset.String())
} }
func (f *IpRouteFilter) existedAt(existedAt time.Time) { func (f *IpRouteFilter) ExistedAt(existedAt time.Time) {
f.queryParams.Set("existed_at", existedAt.Format(time.RFC3339)) f.queryParams.Set("existed_at", existedAt.Format(time.RFC3339))
} }
func (f *IpRouteFilter) tunnelID(id uuid.UUID) { func (f *IpRouteFilter) TunnelID(id uuid.UUID) {
f.queryParams.Set("tunnel_id", id.String()) f.queryParams.Set("tunnel_id", id.String())
} }
func (f *IpRouteFilter) vnetID(id uuid.UUID) { func (f *IpRouteFilter) VNetID(id uuid.UUID) {
f.queryParams.Set("virtual_network_id", id.String()) f.queryParams.Set("virtual_network_id", id.String())
} }

View File

@ -69,6 +69,7 @@ func TestDetailedRouteJsonRoundtrip(t *testing.T) {
}{ }{
{ {
`{ `{
"id":"91ebc578-cc99-4641-9937-0fb630505fa0",
"network":"10.1.2.40/29", "network":"10.1.2.40/29",
"tunnel_id":"fba6ffea-807f-4e7a-a740-4184ee1b82c8", "tunnel_id":"fba6ffea-807f-4e7a-a740-4184ee1b82c8",
"comment":"test", "comment":"test",
@ -80,6 +81,7 @@ func TestDetailedRouteJsonRoundtrip(t *testing.T) {
}, },
{ {
`{ `{
"id":"91ebc578-cc99-4641-9937-0fb630505fa0",
"network":"10.1.2.40/29", "network":"10.1.2.40/29",
"tunnel_id":"fba6ffea-807f-4e7a-a740-4184ee1b82c8", "tunnel_id":"fba6ffea-807f-4e7a-a740-4184ee1b82c8",
"virtual_network_id":"38c95083-8191-4110-8339-3f438d44fdb9", "virtual_network_id":"38c95083-8191-4110-8339-3f438d44fdb9",
@ -167,9 +169,10 @@ func TestRouteTableString(t *testing.T) {
require.NoError(t, err) require.NoError(t, err)
require.NotNil(t, network) require.NotNil(t, network)
r := DetailedRoute{ r := DetailedRoute{
ID: uuid.Nil,
Network: CIDR(*network), Network: CIDR(*network),
} }
row := r.TableString() row := r.TableString()
fmt.Println(row) fmt.Println(row)
require.True(t, strings.HasPrefix(row, "1.2.3.4/32")) require.True(t, strings.HasPrefix(row, "00000000-0000-0000-0000-000000000000\t1.2.3.4/32"))
} }

View File

@ -1,6 +1,9 @@
package tunnel package tunnel
import ( import (
"net"
"github.com/google/uuid"
"github.com/pkg/errors" "github.com/pkg/errors"
"github.com/cloudflare/cloudflared/cfapi" "github.com/cloudflare/cloudflared/cfapi"
@ -24,12 +27,12 @@ func (sc *subcommandContext) addRoute(newRoute cfapi.NewRoute) (cfapi.Route, err
return client.AddRoute(newRoute) return client.AddRoute(newRoute)
} }
func (sc *subcommandContext) deleteRoute(params cfapi.DeleteRouteParams) error { func (sc *subcommandContext) deleteRoute(id uuid.UUID) error {
client, err := sc.client() client, err := sc.client()
if err != nil { if err != nil {
return errors.Wrap(err, noClientMsg) return errors.Wrap(err, noClientMsg)
} }
return client.DeleteRoute(params) return client.DeleteRoute(id)
} }
func (sc *subcommandContext) getRouteByIP(params cfapi.GetRouteByIpParams) (cfapi.DetailedRoute, error) { func (sc *subcommandContext) getRouteByIP(params cfapi.GetRouteByIpParams) (cfapi.DetailedRoute, error) {
@ -39,3 +42,25 @@ func (sc *subcommandContext) getRouteByIP(params cfapi.GetRouteByIpParams) (cfap
} }
return client.GetByIP(params) return client.GetByIP(params)
} }
func (sc *subcommandContext) getRouteId(network net.IPNet, vnetId *uuid.UUID) (uuid.UUID, error) {
filters := cfapi.NewIPRouteFilter()
filters.NotDeleted()
filters.NetworkIsSubsetOf(network)
filters.NetworkIsSupersetOf(network)
if vnetId != nil {
filters.VNetID(*vnetId)
}
result, err := sc.listRoutes(filters)
if err != nil {
return uuid.Nil, err
}
if len(result) != 1 {
return uuid.Nil, errors.New("unable to find route for provided network and vnet")
}
return result[0].ID, nil
}

View File

@ -21,6 +21,8 @@ var (
Aliases: []string{"vn"}, Aliases: []string{"vn"},
Usage: "The ID or name of the virtual network to which the route is associated to.", Usage: "The ID or name of the virtual network to which the route is associated to.",
} }
routeAddError = errors.New("You must supply exactly one argument, the ID or CIDR of the route you want to delete")
) )
func buildRouteIPSubcommand() *cli.Command { func buildRouteIPSubcommand() *cli.Command {
@ -68,11 +70,9 @@ which virtual network's routing table you want to add the route to with:
Name: "delete", Name: "delete",
Action: cliutil.ConfiguredAction(deleteRouteCommand), Action: cliutil.ConfiguredAction(deleteRouteCommand),
Usage: "Delete a row from your organization's private routing table", Usage: "Delete a row from your organization's private routing table",
UsageText: "cloudflared tunnel [--config FILEPATH] route ip delete [flags] [CIDR]", UsageText: "cloudflared tunnel [--config FILEPATH] route ip delete [flags] [Route ID or CIDR]",
Description: `Deletes the row for a given CIDR from your routing table. That portion of your network Description: `Deletes the row for the given route ID from your routing table. That portion of your network
will no longer be reachable by the WARP clients. Note that if you use virtual will no longer be reachable.`,
networks, then you have to tell which virtual network whose routing table you
have a row deleted from.`,
Flags: []cli.Flag{vnetFlag}, Flags: []cli.Flag{vnetFlag},
}, },
{ {
@ -187,33 +187,36 @@ func deleteRouteCommand(c *cli.Context) error {
} }
if c.NArg() != 1 { if c.NArg() != 1 {
return errors.New("You must supply exactly one argument, the network whose route you want to delete (in CIDR form e.g. 1.2.3.4/32)") return routeAddError
} }
_, network, err := net.ParseCIDR(c.Args().First()) var routeId uuid.UUID
routeId, err = uuid.Parse(c.Args().First())
if err != nil { if err != nil {
return errors.Wrap(err, "Invalid network CIDR") _, network, err := net.ParseCIDR(c.Args().First())
} if err != nil || network == nil {
if network == nil { return routeAddError
return errors.New("Invalid network CIDR")
}
params := cfapi.DeleteRouteParams{
Network: *network,
} }
var vnetId *uuid.UUID
if c.IsSet(vnetFlag.Name) { if c.IsSet(vnetFlag.Name) {
vnetId, err := getVnetId(sc, c.String(vnetFlag.Name)) id, err := getVnetId(sc, c.String(vnetFlag.Name))
if err != nil { if err != nil {
return err return err
} }
params.VNetID = &vnetId vnetId = &id
} }
if err := sc.deleteRoute(params); err != nil { routeId, err = sc.getRouteId(*network, vnetId)
if err != nil {
return err
}
}
if err := sc.deleteRoute(routeId); err != nil {
return errors.Wrap(err, "API error") return errors.Wrap(err, "API error")
} }
fmt.Printf("Successfully deleted route for %s\n", network) fmt.Printf("Successfully deleted route with ID %s\n", routeId)
return nil return nil
} }
@ -269,7 +272,7 @@ func formatAndPrintRouteList(routes []*cfapi.DetailedRoute) {
defer writer.Flush() defer writer.Flush()
// Print column headers with tabbed columns // Print column headers with tabbed columns
_, _ = fmt.Fprintln(writer, "NETWORK\tVIRTUAL NET ID\tCOMMENT\tTUNNEL ID\tTUNNEL NAME\tCREATED\tDELETED\t") _, _ = fmt.Fprintln(writer, "ID\tNETWORK\tVIRTUAL NET ID\tCOMMENT\tTUNNEL ID\tTUNNEL NAME\tCREATED\tDELETED\t")
// Loop through routes, create formatted string for each, and print using tabwriter // Loop through routes, create formatted string for each, and print using tabwriter
for _, route := range routes { for _, route := range routes {