Skip to content

NETPROD: add api documentations for vpc subnets and routes - #1230

Open
jvasilevsky wants to merge 3 commits into
digitalocean:mainfrom
jvasilevsky:jvasilevsky/NETPROD-add-subnet-routes-documentation
Open

jvasilevsky wants to merge 3 commits into
digitalocean:mainfrom
jvasilevsky:jvasilevsky/NETPROD-add-subnet-routes-documentation

Conversation

@jvasilevsky

Copy link
Copy Markdown
Contributor

add api documentations for vpc subnets and routes

@jvasilevsky
jvasilevsky force-pushed the jvasilevsky/NETPROD-add-subnet-routes-documentation branch from 9e6d5ca to 72a49b2 Compare September 18, 2026 00:29

@lfundaro-do lfundaro-do left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

  • Aggregator already has GET /v2/vpcs/{vpc_uuid}/subnets/{subnet_uuid}/members (resource_type, pagination). VPC-level members are already in the public spec (vpcs_list_members). If members ships with subnets, this PR is incomplete.

curl -X POST \
-H "Content-Type: application/json" \
-H "Authorization: Bearer $DIGITALOCEAN_TOKEN" \
-d '{"destination_cidr":"0.0.0.0/0","target_urns":["do:droplet:14aa1d1b-e6ab-4ccb-bb10-dade56fcb8ec"]}' \

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

we will accept only numerical droplets, no uuids allowed

@lfundaro-do lfundaro-do left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

  • Aggregator already has GET /v2/vpcs/{vpc_uuid}/subnets/{subnet_uuid}/members (resource_type, pagination). VPC-level members are already in the public spec (vpcs_list_members). If members ships with subnets, this PR is incomplete.

@jvasilevsky
jvasilevsky force-pushed the jvasilevsky/NETPROD-add-subnet-routes-documentation branch from 72a49b2 to dce264c Compare September 21, 2026 13:51
@jvasilevsky
jvasilevsky force-pushed the jvasilevsky/NETPROD-add-subnet-routes-documentation branch from dce264c to a98d63c Compare September 21, 2026 15:14
@@ -0,0 +1,41 @@
list_vpc:
lang: cURL

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

tested this against a live account. Please address the following:

  1. Pagination bug on list_members
    The spec says per_page and page are optional (defaulting to 20 and 1, like the rest of our API). But if you actually hit the GET endpoint without those query params, it throws a 422 unprocessable_entity: "Pagination is invalid". If you explicitly add ?page=1&per_page=20, it returns a 200.
    We either need to fix the backend to respect the defaults, or mark these as required: true in the spec. (Though making them required breaks our usual API pagination convention, so I'd vote for fixing the backend).

  2. Missing resource_type enum value
    The resource_type enum in parameters.yml only lists droplet, loadbalancer, dbaas, and kubernetes. However, I saw a real subnet member return "urn":"do:nfs_share:...". We should add nfs_share (and double-check if there are any others) so the filter actually works for everything.

  3. name shouldn't be readOnly
    In vpc_subnet.yml, name is marked as readOnly: true, but this PR also adds a PATCH endpoint to rename subnets. We should drop the readOnly flag here since the client can clearly change it, similar to how it's handled in the base vpc.yml model.

  4. 404 on empty VPC routes
    I hit GET /v2/vpcs/{vpc_uuid}/routes on a VPC that didn't have any static routes configured and got a 404. Is that expected behavior? Normally, I'd expect a 200 with an empty list []. Might just be a feature flag/rollout quirk on my end, but wanted to flag it just in case.

  5. Dual security scopes on create/delete
    The specs for creating and deleting subnets accept either vpc:create/vpc:delete OR vpc:update. That means a token scoped strictly to vpc:update can completely create or wipe out subnets.
    If the idea is that subnets just inherit permissions from their parent VPC, that's a totally fair product call. I just wanted to point it out to confirm it's intentional, since the rest of the PR sticks pretty strictly to one scope per verb.

@lfundaro-do lfundaro-do Sep 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

404 on empty VPC routes
I hit GET /v2/vpcs/{vpc_uuid}/routes on a VPC that didn't have any static routes configured and got a 404. Is that expected behavior? Normally, I'd expect a 200 with an empty list []. Might just be a feature flag/rollout quirk on my end, but wanted to flag it just in case.

you would need to be added to https://flipperui.internal.digitalocean.com/features/vpc_route_enabled
in stage2: https://flipperui.s2r1.internal.digitalocean.com/features/vpc_route_enabled

@lfundaro-do lfundaro-do Sep 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Dual security scopes on create/delete
The specs for creating and deleting subnets accept either vpc:create/vpc:delete OR vpc:update. That means a token scoped strictly to vpc:update can completely create or wipe out subnets.
If the idea is that subnets just inherit permissions from their parent VPC, that's a totally fair product call. I just wanted to point it out to confirm it's intentional, since the rest of the PR sticks pretty strictly to one scope per verb.

that's intended and analog to what we do on dns api with records permissions.

@jvasilevsky
jvasilevsky force-pushed the jvasilevsky/NETPROD-add-subnet-routes-documentation branch 2 times, most recently from 0282946 to 3011993 Compare September 22, 2026 15:14
@jvasilevsky
jvasilevsky force-pushed the jvasilevsky/NETPROD-add-subnet-routes-documentation branch from 3011993 to 3b27506 Compare September 22, 2026 15:30

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants