-
Notifications
You must be signed in to change notification settings - Fork 62
RFD-322 Networking #2057
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.
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
RFD-322 Networking #2057
Conversation
| /// Create-time parameters for a [`RouterRoute`] | ||
| #[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] | ||
| pub struct RouterRouteCreateParams { | ||
| #[serde(flatten)] | ||
| pub identity: IdentityMetadataCreateParams, | ||
| pub target: RouteTarget, | ||
| pub destination: RouteDestination, | ||
| } | ||
|
|
||
| /// Updateable properties of a [`RouterRoute`] | ||
| #[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] | ||
| pub struct RouterRouteUpdateParams { | ||
| #[serde(flatten)] | ||
| pub identity: IdentityMetadataUpdateParams, | ||
| pub target: RouteTarget, | ||
| pub destination: RouteDestination, | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I moved this over to the params file to be co-located with the other resource params that were already there.
david-crespo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whew, this is a BIG one
| pub struct RoutePath { | ||
| pub route: NameOrId, | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not something to be addressed in this PR, but it's a little silly that these are all the same. If we called it resource in all cases maybe they could share the struct. Something like:
#[derive(Deserialize, JsonSchema)]
pub struct NameOrIdPath {
pub resource: NameOrId,
}There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, the ultimate intent here I think is to have it be generated by a macro. Unfortunately the name matters as it shows up in documentation, the API spec, etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We did that at "previous job" (have a common "resource" struct), and it was such a pain when we needed to add additional fields for a new version, it ended up being a spaghetti nightmare 😅
| Ok(disk) | ||
| } | ||
|
|
||
| /// Create a network interface attached to the provided instance. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I relocated the network interface logic in its own file. We break this resource at the data store layer, might as well be consistent with it.
|
I'm going to push forward here. Please, if folks have any feedback don't hesitate to add it after the merge. I'm more than happy to follow up with any fixes or re-work that may be needed. |
|
Sorry for the delay! I'm nearly done with my comments! |
bnaecker
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey Justin! This is an enormous amount of work. Thank you so much for taking it on, this looks like a big improvement. I have a few comments, all related to documentation and comments. There's nothing in the code itself that appears out of order. Thanks again, great work!
Followup to #2057. Noticed these while upgrading the docs site — deprecated endpoints are filtered out to give priority to the v1 ones, but these were not being filtered out.
See #2028
This PR adds V1 versions of the developer facing networking APIs. Firewall rules aren't in this PR. I'll do that in a follow up.
New Endpoints
All endpoints here are scoped via query parameters by their parent selectors. So for
vpc-subnetsif you provide the vpc by name, you'll also need to provide the project. If the projected is specified by name you'll need the org.So
/v1/vpc-subnet?vpc={vpc_id},/v1/vpc-subnet?vpc={vpc_name}&project={project_id}, etcFor a quick overview of the entire API (and the changes introduced in this PR) check out the nexus_tags.txt file
Notable changes