feat: skip default vpc #9750
feat: skip default vpc #9750
Conversation
sudomateo
left a comment
sudomateo
left a comment
There was a problem hiding this comment.
Gave things a one pass after getting the API versioning stuff to work locally.
| method = POST, | ||
| path = "/v1/projects", | ||
| tags = ["projects"], | ||
| versions = VERSION_SKIP_DEFAULT_VPC.., |
There was a problem hiding this comment.
I'm unsure if the operation_id is needed here.
| method = POST, | ||
| path = "/v1/projects", | ||
| tags = ["projects"], | ||
| operation_id = "project_create", |
There was a problem hiding this comment.
I'm unsure if the operation_id is needed here in the versioned function.
| "vpc_create_params", | ||
| )); | ||
|
|
||
| if !params.project_create.skip_default_vpc { |
There was a problem hiding this comment.
I'm unsure if skipping parts of sagas like this is the preferred way to handle such conditional logic but it seemed to be the cleanest method here.
| .await; | ||
| } | ||
|
|
||
| #[nexus_test(server = crate::Server)] |
There was a problem hiding this comment.
I'm still figuring out tests for this. It's not pretty currently.
|
|
||
| /// Whether to skip creating the "default" VPC when the project is created. | ||
| #[serde(default)] | ||
| pub skip_default_vpc: bool, |
There was a problem hiding this comment.
I opted to use a field that defaults to false to represent the current behavior since serialization (e.g., Go) often treats omitted booleans as false. I know this has the consequence of reading like a double negative so I'm open to suggestions here. I really want the user to understand the act of skipping the creation of the default VPC must be intentional.
sudomateo
force-pushed
the
sudomateo/lsswxqnvlnty
branch
from
6df8c22 to
15e4839
Compare
sudomateo
force-pushed
the
sudomateo/lsswxqnvlnty
branch
2 times, most recently
from
b4b9843 to
fd22224
Compare
|
This is great to see! It goes in line with an initiative from a while ago https://rfd.shared.oxide.computer/rfd/0390 . I have to wonder though, how many users want/use the default VPC? If the majority don't use it (e.g. they manage their resources via terraform or similar), would it make more sense to have an optional field that triggers the creation of a default VPC instead of a field that skips the creation of said VPC? |
Added the ability to skip creating the `default` VPC when calling `project_create`. This allows users to control every aspect of their project using automation, rather than needing to delete `default` VPC first. Closes oxidecomputer/customer-support#567.
sudomateo
force-pushed
the
sudomateo/lsswxqnvlnty
branch
from
fd22224 to
299174d
Compare
Heck yeah! I haven't seen this previously but I have chatted with enough customers that want this feature and I feel their pain trying to use automation to manage Oxide completely without needing to add some out of band API call to delete the default VPC.
I don't have numbers on this personally, but with the upcoming dual-stack networking changes I have to wonder how useful it is for us to create any default networking resources in a project. It's not like we'd know whether the user will want a V4, V6, or dual-stack. I went with the approach I did, which I'm not married to, just because it was backwards compatible in the sense that creating a project after this change will still create the default VPC as it did before. I'm open to different designs though especially if we decide to step further away from any default resources. |
Ugh, yeah. That's not really great at all 😖
It could be useful to check in with the networking team and have a discussion on whether we want to have any default resources at all. Back when https://rfd.shared.oxide.computer/rfd/0390 was written, we didn't have customer feedback, but now we do! A simpler API is always easier to use, if users don't have to decide on default resource creation, that would be really nice. |
Added the ability to skip creating the
defaultVPC when callingproject_create. This allows users to control every aspect of their project using automation, rather than needing to deletedefaultVPC first.Closes https://github.com/oxidecomputer/customer-support/issues/567.