Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add vnet param to routes #912

Merged
merged 3 commits into from Jun 2, 2022
Merged

Add vnet param to routes #912

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3119632
Add vnet param to routes
ivan-sabelnikov May 27, 2022
0657062
Merge remote-tracking branch 'origin/master' into add-vnet-param
vavsab May 31, 2022
f16d34c
Fix vnet get param to get route for ip
vavsab Jun 1, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
58 changes: 32 additions & 26 deletions tunnel_routes.go
View file
Open in desktop
Expand Up @@ -19,47 +19,53 @@ var (

// TunnelRoute is the full record for a route.
type TunnelRoute struct {
Network string `json:"network"`
TunnelID string `json:"tunnel_id"`
TunnelName string `json:"tunnel_name"`
Comment string `json:"comment"`
CreatedAt *time.Time `json:"created_at"`
DeletedAt *time.Time `json:"deleted_at"`
Network string `json:"network"`
TunnelID string `json:"tunnel_id"`
TunnelName string `json:"tunnel_name"`
Comment string `json:"comment"`
CreatedAt *time.Time `json:"created_at"`
DeletedAt *time.Time `json:"deleted_at"`
VirtualNetworkID string `json:"virtual_network_id"`
}

type TunnelRoutesListParams struct {
AccountID string `url:"-"`
TunnelID string `url:"tunnel_id,omitempty"`
Comment string `url:"comment,omitempty"`
IsDeleted *bool `url:"is_deleted,omitempty"`
NetworkSubset string `url:"network_subset,omitempty"`
NetworkSuperset string `url:"network_superset,omitempty"`
ExistedAt *time.Time `url:"existed_at,omitempty"`
AccountID string `url:"-"`
TunnelID string `url:"tunnel_id,omitempty"`
Comment string `url:"comment,omitempty"`
IsDeleted *bool `url:"is_deleted,omitempty"`
NetworkSubset string `url:"network_subset,omitempty"`
NetworkSuperset string `url:"network_superset,omitempty"`
ExistedAt *time.Time `url:"existed_at,omitempty"`
VirtualNetworkID string `url:"virtual_network_id,omitempty"`
PaginationOptions
}

type TunnelRoutesCreateParams struct {
AccountID string `json:"-"`
Network string `json:"-"`
TunnelID string `json:"tunnel_id"`
Comment string `json:"comment,omitempty"`
AccountID string `json:"-"`
Network string `json:"-"`
TunnelID string `json:"tunnel_id"`
Comment string `json:"comment,omitempty"`
VirtualNetworkID string `json:"virtual_network_id,omitempty"`
}

type TunnelRoutesUpdateParams struct {
AccountID string `json:"-"`
Network string `json:"network"`
TunnelID string `json:"tunnel_id"`
Comment string `json:"comment,omitempty"`
AccountID string `json:"-"`
Network string `json:"network"`
TunnelID string `json:"tunnel_id"`
Comment string `json:"comment,omitempty"`
VirtualNetworkID string `json:"virtual_network_id,omitempty"`
}

type TunnelRoutesForIPParams struct {
AccountID string `json:"-"`
Network string `json:"-"`
AccountID string `url:"-"`
Network string `url:"-"`
VirtualNetworkID string `url:"virtual_network_id,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
VirtualNetworkID string `url:"virtual_network_id,omitempty"`

}

type TunnelRoutesDeleteParams struct {
AccountID string `json:"-"`
Network string `json:"-"`
AccountID string `json:"-"`
Network string `json:"-"`
VirtualNetworkID string `url:"virtual_network_id,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
VirtualNetworkID string `url:"virtual_network_id,omitempty"`

}

// tunnelRouteListResponse is the API response for listing tunnel routes.
Expand Down Expand Up @@ -112,7 +118,7 @@ func (api *API) GetTunnelRouteForIP(ctx context.Context, params TunnelRoutesForI
return TunnelRoute{}, ErrInvalidNetworkValue
}

uri := fmt.Sprintf("/%s/%s/teamnet/routes/ip/%s", AccountRouteRoot, params.AccountID, params.Network)
uri := buildURI(fmt.Sprintf("/%s/%s/teamnet/routes/ip/%s", AccountRouteRoot, params.AccountID, params.Network), params)

responseBody, err := api.makeRequestContext(ctx, http.MethodGet, uri, nil)
if err != nil {
Expand Down
61 changes: 35 additions & 26 deletions tunnel_routes_test.go
View file
Open in desktop
Expand Up @@ -29,7 +29,8 @@ func TestListTunnelRoutes(t *testing.T) {
"tunnel_name": "blog",
"comment": "Example comment for this route",
"created_at": "2021-01-25T18:22:34.317854Z",
"deleted_at": "2021-01-25T18:22:34.317854Z"
"deleted_at": "2021-01-25T18:22:34.317854Z",
"virtual_network_id": "9f322de4-5988-4945-b770-f1d6ac200f86"
}
]
}`)
Expand All @@ -46,6 +47,7 @@ func TestListTunnelRoutes(t *testing.T) {
"Example comment for this route",
&ts,
&ts,
"9f322de4-5988-4945-b770-f1d6ac200f86",
},
}

Expand Down Expand Up @@ -74,7 +76,8 @@ func TestTunnelRouteForIP(t *testing.T) {
"tunnel_name": "blog",
"comment": "Example comment for this route",
"created_at": "2021-01-25T18:22:34.317854Z",
"deleted_at": "2021-01-25T18:22:34.317854Z"
"deleted_at": "2021-01-25T18:22:34.317854Z",
"virtual_network_id": "9f322de4-5988-4945-b770-f1d6ac200f86"
}
}`)
}
Expand All @@ -83,12 +86,13 @@ func TestTunnelRouteForIP(t *testing.T) {

ts, _ := time.Parse(time.RFC3339Nano, "2021-01-25T18:22:34.317854Z")
want := TunnelRoute{
Network: "ff01::/32",
TunnelID: "f70ff985-a4ef-4643-bbbc-4a0ed4fc8415",
TunnelName: "blog",
Comment: "Example comment for this route",
CreatedAt: &ts,
DeletedAt: &ts,
Network: "ff01::/32",
TunnelID: "f70ff985-a4ef-4643-bbbc-4a0ed4fc8415",
TunnelName: "blog",
Comment: "Example comment for this route",
CreatedAt: &ts,
DeletedAt: &ts,
VirtualNetworkID: "9f322de4-5988-4945-b770-f1d6ac200f86",
}

got, err := client.GetTunnelRouteForIP(context.Background(), TunnelRoutesForIPParams{AccountID: testAccountID, Network: "10.1.0.137"})
Expand All @@ -115,7 +119,8 @@ func TestCreateTunnelRoute(t *testing.T) {
"tunnel_name": "blog",
"comment": "Example comment for this route",
"created_at": "2021-01-25T18:22:34.317854Z",
"deleted_at": "2021-01-25T18:22:34.317854Z"
"deleted_at": "2021-01-25T18:22:34.317854Z",
"virtual_network_id": "9f322de4-5988-4945-b770-f1d6ac200f86"
}
}`)
}
Expand All @@ -124,15 +129,16 @@ func TestCreateTunnelRoute(t *testing.T) {

ts, _ := time.Parse(time.RFC3339Nano, "2021-01-25T18:22:34.317854Z")
want := TunnelRoute{
Network: "10.0.0.0/16",
TunnelID: "f70ff985-a4ef-4643-bbbc-4a0ed4fc8415",
TunnelName: "blog",
Comment: "Example comment for this route",
CreatedAt: &ts,
DeletedAt: &ts,
Network: "10.0.0.0/16",
TunnelID: "f70ff985-a4ef-4643-bbbc-4a0ed4fc8415",
TunnelName: "blog",
Comment: "Example comment for this route",
CreatedAt: &ts,
DeletedAt: &ts,
VirtualNetworkID: "9f322de4-5988-4945-b770-f1d6ac200f86",
}

tunnel, err := client.CreateTunnelRoute(context.Background(), TunnelRoutesCreateParams{AccountID: testAccountID, TunnelID: testTunnelID, Network: "10.0.0.0/16", Comment: "foo"})
tunnel, err := client.CreateTunnelRoute(context.Background(), TunnelRoutesCreateParams{AccountID: testAccountID, TunnelID: testTunnelID, Network: "10.0.0.0/16", Comment: "foo", VirtualNetworkID: "9f322de4-5988-4945-b770-f1d6ac200f86"})
if assert.NoError(t, err) {
assert.Equal(t, want, tunnel)
}
Expand Down Expand Up @@ -161,23 +167,25 @@ func TestUpdateTunnelRoute(t *testing.T) {
"tunnel_name": "blog",
"comment": "Example comment for this route",
"created_at": "2021-01-25T18:22:34.317854Z",
"deleted_at": "2021-01-25T18:22:34.317854Z"
"deleted_at": "2021-01-25T18:22:34.317854Z",
"virtual_network_id": "9f322de4-5988-4945-b770-f1d6ac200f86"
}
}`)
}

ts, _ := time.Parse(time.RFC3339Nano, "2021-01-25T18:22:34.317854Z")
want := TunnelRoute{
Network: "10.0.0.0/16",
TunnelID: "f70ff985-a4ef-4643-bbbc-4a0ed4fc8415",
TunnelName: "blog",
Comment: "Example comment for this route",
CreatedAt: &ts,
DeletedAt: &ts,
Network: "10.0.0.0/16",
TunnelID: "f70ff985-a4ef-4643-bbbc-4a0ed4fc8415",
TunnelName: "blog",
Comment: "Example comment for this route",
CreatedAt: &ts,
DeletedAt: &ts,
VirtualNetworkID: "9f322de4-5988-4945-b770-f1d6ac200f86",
}

mux.HandleFunc("/accounts/"+testAccountID+"/teamnet/routes/network/10.0.0.0/16", handler)
tunnel, err := client.UpdateTunnelRoute(context.Background(), TunnelRoutesUpdateParams{AccountID: testAccountID, TunnelID: testTunnelID, Network: "10.0.0.0/16", Comment: "foo"})
tunnel, err := client.UpdateTunnelRoute(context.Background(), TunnelRoutesUpdateParams{AccountID: testAccountID, TunnelID: testTunnelID, Network: "10.0.0.0/16", Comment: "foo", VirtualNetworkID: "9f322de4-5988-4945-b770-f1d6ac200f86"})

if assert.NoError(t, err) {
assert.Equal(t, want, tunnel)
Expand All @@ -202,12 +210,13 @@ func TestDeleteTunnelRoute(t *testing.T) {
"tunnel_name": "blog",
"comment": "Example comment for this route",
"created_at": "2021-01-25T18:22:34.317854Z",
"deleted_at": "2021-01-25T18:22:34.317854Z"
"deleted_at": "2021-01-25T18:22:34.317854Z",
"virtual_network_id": "9f322de4-5988-4945-b770-f1d6ac200f86"
}
}`)
}

mux.HandleFunc("/accounts/"+testAccountID+"/teamnet/routes/network/10.0.0.0/16", handler)
err := client.DeleteTunnelRoute(context.Background(), TunnelRoutesDeleteParams{AccountID: testAccountID, Network: "10.0.0.0/16"})
err := client.DeleteTunnelRoute(context.Background(), TunnelRoutesDeleteParams{AccountID: testAccountID, Network: "10.0.0.0/16", VirtualNetworkID: "9f322de4-5988-4945-b770-f1d6ac200f86"})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't need to send VirtualNetworkID for the Delete, it is only addressable by the Network parameter.

Copy link
Contributor Author

@vavsab vavsab Jun 1, 2022 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a question here. For example,
I have vnetDev and vnetProd. Both of them have "10.0.0.0/16" routes that lead to the corresponding dev & prod tunnels.

How do I remove only vnetProd route without specifying VirtualNetworkID?

It's not some specific non-real case. I will have exactly this situation on my environment after we merge this PR.

Delete without VirtualNetworkID only removes it for default network.

Copy link
Contributor Author

@vavsab vavsab Jun 1, 2022 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah.. I see that it's not reflected in docs. You may test it in Postman/insomnia as I did. I couldn't remove the route from non-default vnet until I explicitly specified vnetid as Get param.

I guess we need to update api docs s bit. It's super important for those folks who use vnets.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cloudflared is already using it here

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gotcha, one to raise with a support ticket if you can in order to get the docs reflecting the correct state of affairs here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Purchased paid cloudflare plan and created a support ticket to change the docs.

assert.NoError(t, err)
}