-
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
Merged
Merged
RFD-322 Networking #2057
Changes from all commits
Commits
Show all changes
62 commits
Select commit
Hold shift + click to select a range
8d51dbb
Revamp selectors (again), convert organizations
just-be-dev 129011e
Add project APIs, update routes
just-be-dev c29c67d
Fix clippy failures
just-be-dev 599e6eb
Require less conversions when going from NameOrId to name
just-be-dev 114d39a
Revamp selectors (again x2), convert projects
just-be-dev 6ae7da3
Update nexus.json
just-be-dev 63eddd7
Skip deprecated endpoints in auth check; update test to assert contents
just-be-dev 8db8fa2
Fix authz test
just-be-dev bd1fafe
Merge branch 'main' into rfd-322-orgs-n-projs
just-be-dev f949c6c
Update nexus.json
just-be-dev bd033a0
format -_-
just-be-dev ae4ddcd
Merge branch 'main' into rfd-322-orgs-n-projs
just-be-dev 69ac008
Add expanded error checking
just-be-dev 83768b1
Start networking v1 apis
just-be-dev 52db40c
Start on subnets
just-be-dev 1176211
Update nexus.json
just-be-dev 5580d22
Merge branch 'main' into rfd-322-orgs-n-projs
just-be-dev 5ceccc4
Merge branch 'rfd-322-orgs-n-projs' into rfd-322-networking
just-be-dev e18b5b8
Add v1 subnet create
just-be-dev 96786b3
Added deleted; Added deprecations; Updated nexus.json
just-be-dev 341dbd1
Add subnet update
just-be-dev f34aaf6
Merge branch 'main' into rfd-322-networking
just-be-dev 5518c43
Merge branch 'main' into rfd-322-networking
just-be-dev bbf0f36
Update pagination strategy
just-be-dev f3c1d85
Update authz tests
just-be-dev 12396b5
Update api spec
just-be-dev 653df3d
Merge branch 'main' into rfd-322-networking
just-be-dev a336a5b
Update vpc tests
just-be-dev 99c0466
Merge branch 'main' into rfd-322-networking
just-be-dev 6cff4f9
Add comments, housekeeping
just-be-dev 88cff99
Update vpc router and routes
just-be-dev 7a2ddbc
Update API spec with new routes
just-be-dev 507ffb0
Merge branch 'main' into rfd-322-networking
just-be-dev c0d325f
Update authz tests
just-be-dev ba1b8b8
WIP subnet tests
just-be-dev 76c15dc
Merge branch 'main' into rfd-322-networking
just-be-dev 92f9fda
WIP: progress on network interfaces
just-be-dev a226674
Merge branch 'main' into rfd-322-networking
just-be-dev 7d19f04
Merge branch 'main' into rfd-322-networking
just-be-dev 45427c1
Add nic update
just-be-dev 35c9881
Move network interfaces into their own file
just-be-dev 5f4d1b3
Fix type failures
just-be-dev 6ef30f0
Remove unncessary into
just-be-dev 2be9970
Remove unused imports
just-be-dev 706df91
Merge branch 'main' into rfd-322-networking
just-be-dev 7aeab07
Ensure selectors can be flattened
just-be-dev 0b2f882
Fix remaining path issues; update nexus spec
just-be-dev 9985ba2
Remove unused URLs
just-be-dev d6acd22
Fix doctest failure
just-be-dev b14dba6
Fix vpc subnets test
just-be-dev 267ecc4
Fix authz test
just-be-dev b51834b
Merge branch 'main' into rfd-322-networking
just-be-dev dc7de20
Add authz coverage for network interfaces
just-be-dev e9818c2
Merge branch 'main' into rfd-322-networking
just-be-dev e30a398
Fix conflicts after merge
just-be-dev 8876986
Merge branch 'main' into rfd-322-networking
just-be-dev 1ea7279
Merge branch 'main' into rfd-322-networking
just-be-dev c832346
PR feedback
just-be-dev 388dcd8
Add missed docstring
just-be-dev fd06033
Add missing deprecation
just-be-dev 735255d
Add other missing deprecations
just-be-dev 0ec1eeb
Update spec
just-be-dev File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,14 +10,12 @@ use super::MAX_NICS_PER_INSTANCE; | |
| use crate::app::sagas; | ||
| use crate::authn; | ||
| use crate::authz; | ||
| use crate::authz::ApiResource; | ||
| use crate::cidata::InstanceCiData; | ||
| use crate::context::OpContext; | ||
| use crate::db; | ||
| use crate::db::identity::Resource; | ||
| use crate::db::lookup; | ||
| use crate::db::lookup::LookupPath; | ||
| use crate::db::queries::network_interface; | ||
| use crate::external_api::params; | ||
| use futures::future::Fuse; | ||
| use futures::{FutureExt, SinkExt, StreamExt}; | ||
|
|
@@ -799,220 +797,6 @@ impl super::Nexus { | |
| Ok(disk) | ||
| } | ||
|
|
||
| /// Create a network interface attached to the provided instance. | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
| // TODO-performance: Add a version of this that accepts the instance ID | ||
| // directly. This will avoid all the internal database lookups in the event | ||
| // that we create many NICs for the same instance, such as in a saga. | ||
| pub async fn instance_create_network_interface( | ||
| &self, | ||
| opctx: &OpContext, | ||
| organization_name: &Name, | ||
| project_name: &Name, | ||
| instance_name: &Name, | ||
| params: ¶ms::NetworkInterfaceCreate, | ||
| ) -> CreateResult<db::model::NetworkInterface> { | ||
| let (.., authz_project, authz_instance) = | ||
| LookupPath::new(opctx, &self.db_datastore) | ||
| .organization_name(organization_name) | ||
| .project_name(project_name) | ||
| .instance_name(instance_name) | ||
| .lookup_for(authz::Action::Modify) | ||
| .await?; | ||
|
|
||
| // NOTE: We need to lookup the VPC and VPC Subnet, since we need both | ||
| // IDs for creating the network interface. | ||
| // | ||
| // TODO-correctness: There are additional races here. The VPC and VPC | ||
| // Subnet could both be deleted between the time we fetch them and | ||
| // actually insert the record for the interface. The solution is likely | ||
| // to make both objects implement `DatastoreCollection` for their | ||
| // children, and then use `VpcSubnet::insert_resource` inside the | ||
| // `instance_create_network_interface` method. See | ||
| // https://github.com/oxidecomputer/omicron/issues/738. | ||
| let vpc_name = db::model::Name(params.vpc_name.clone()); | ||
| let subnet_name = db::model::Name(params.subnet_name.clone()); | ||
| let (.., authz_vpc, authz_subnet, db_subnet) = | ||
| LookupPath::new(opctx, &self.db_datastore) | ||
| .project_id(authz_project.id()) | ||
| .vpc_name(&vpc_name) | ||
| .vpc_subnet_name(&subnet_name) | ||
| .fetch() | ||
| .await?; | ||
| let interface_id = Uuid::new_v4(); | ||
| let interface = db::model::IncompleteNetworkInterface::new( | ||
| interface_id, | ||
| authz_instance.id(), | ||
| authz_vpc.id(), | ||
| db_subnet, | ||
| params.identity.clone(), | ||
| params.ip, | ||
| )?; | ||
| self.db_datastore | ||
| .instance_create_network_interface( | ||
| opctx, | ||
| &authz_subnet, | ||
| &authz_instance, | ||
| interface, | ||
| ) | ||
| .await | ||
| .map_err(|e| { | ||
| debug!( | ||
| self.log, | ||
| "failed to create network interface"; | ||
| "instance_id" => ?authz_instance.id(), | ||
| "interface_id" => ?interface_id, | ||
| "error" => ?e, | ||
| ); | ||
| if matches!( | ||
| e, | ||
| network_interface::InsertError::InstanceNotFound(_) | ||
| ) { | ||
| // Return the not-found message of the authz interface | ||
| // object, so that the message reflects how the caller | ||
| // originally looked it up | ||
| authz_instance.not_found() | ||
| } else { | ||
| // Convert other errors into an appropriate client error | ||
| network_interface::InsertError::into_external(e) | ||
| } | ||
| }) | ||
| } | ||
|
|
||
| /// Lists network interfaces attached to the instance. | ||
| pub async fn instance_list_network_interfaces( | ||
| &self, | ||
| opctx: &OpContext, | ||
| organization_name: &Name, | ||
| project_name: &Name, | ||
| instance_name: &Name, | ||
| pagparams: &DataPageParams<'_, Name>, | ||
| ) -> ListResultVec<db::model::NetworkInterface> { | ||
| let (.., authz_instance) = LookupPath::new(opctx, &self.db_datastore) | ||
| .organization_name(organization_name) | ||
| .project_name(project_name) | ||
| .instance_name(instance_name) | ||
| .lookup_for(authz::Action::ListChildren) | ||
| .await?; | ||
| self.db_datastore | ||
| .instance_list_network_interfaces(opctx, &authz_instance, pagparams) | ||
| .await | ||
| } | ||
|
|
||
| /// Fetch a network interface attached to the given instance. | ||
| pub async fn network_interface_fetch( | ||
| &self, | ||
| opctx: &OpContext, | ||
| organization_name: &Name, | ||
| project_name: &Name, | ||
| instance_name: &Name, | ||
| interface_name: &Name, | ||
| ) -> LookupResult<db::model::NetworkInterface> { | ||
| let (.., db_interface) = LookupPath::new(opctx, &self.db_datastore) | ||
| .organization_name(organization_name) | ||
| .project_name(project_name) | ||
| .instance_name(instance_name) | ||
| .network_interface_name(interface_name) | ||
| .fetch() | ||
| .await?; | ||
| Ok(db_interface) | ||
| } | ||
|
|
||
| pub async fn network_interface_fetch_by_id( | ||
| &self, | ||
| opctx: &OpContext, | ||
| interface_id: &Uuid, | ||
| ) -> LookupResult<db::model::NetworkInterface> { | ||
| let (.., db_interface) = LookupPath::new(opctx, &self.db_datastore) | ||
| .network_interface_id(*interface_id) | ||
| .fetch() | ||
| .await?; | ||
| Ok(db_interface) | ||
| } | ||
|
|
||
| /// Update a network interface for the given instance. | ||
| pub async fn network_interface_update( | ||
| &self, | ||
| opctx: &OpContext, | ||
| organization_name: &Name, | ||
| project_name: &Name, | ||
| instance_name: &Name, | ||
| interface_name: &Name, | ||
| updates: params::NetworkInterfaceUpdate, | ||
| ) -> UpdateResult<db::model::NetworkInterface> { | ||
| let (.., authz_instance) = LookupPath::new(opctx, &self.db_datastore) | ||
| .organization_name(organization_name) | ||
| .project_name(project_name) | ||
| .instance_name(instance_name) | ||
| .lookup_for(authz::Action::Modify) | ||
| .await?; | ||
| let (.., authz_interface) = LookupPath::new(opctx, &self.db_datastore) | ||
| .instance_id(authz_instance.id()) | ||
| .network_interface_name(interface_name) | ||
| .lookup_for(authz::Action::Modify) | ||
| .await?; | ||
| self.db_datastore | ||
| .instance_update_network_interface( | ||
| opctx, | ||
| &authz_instance, | ||
| &authz_interface, | ||
| db::model::NetworkInterfaceUpdate::from(updates), | ||
| ) | ||
| .await | ||
| } | ||
|
|
||
| /// Delete a network interface from the provided instance. | ||
| /// | ||
| /// Note that the primary interface for an instance cannot be deleted if | ||
| /// there are any secondary interfaces. | ||
| pub async fn instance_delete_network_interface( | ||
| &self, | ||
| opctx: &OpContext, | ||
| organization_name: &Name, | ||
| project_name: &Name, | ||
| instance_name: &Name, | ||
| interface_name: &Name, | ||
| ) -> DeleteResult { | ||
| let (.., authz_instance) = LookupPath::new(opctx, &self.db_datastore) | ||
| .organization_name(organization_name) | ||
| .project_name(project_name) | ||
| .instance_name(instance_name) | ||
| .lookup_for(authz::Action::Modify) | ||
| .await?; | ||
| let (.., authz_interface) = LookupPath::new(opctx, &self.db_datastore) | ||
| .instance_id(authz_instance.id()) | ||
| .network_interface_name(interface_name) | ||
| .lookup_for(authz::Action::Delete) | ||
| .await?; | ||
| self.db_datastore | ||
| .instance_delete_network_interface( | ||
| opctx, | ||
| &authz_instance, | ||
| &authz_interface, | ||
| ) | ||
| .await | ||
| .map_err(|e| { | ||
| debug!( | ||
| self.log, | ||
| "failed to delete network interface"; | ||
| "instance_id" => ?authz_instance.id(), | ||
| "interface_id" => ?authz_interface.id(), | ||
| "error" => ?e, | ||
| ); | ||
| if matches!( | ||
| e, | ||
| network_interface::DeleteError::InstanceNotFound(_) | ||
| ) { | ||
| // Return the not-found message of the authz interface | ||
| // object, so that the message reflects how the caller | ||
| // originally looked it up | ||
| authz_instance.not_found() | ||
| } else { | ||
| // Convert other errors into an appropriate client error | ||
| network_interface::DeleteError::into_external(e) | ||
| } | ||
| }) | ||
| } | ||
|
|
||
| /// Invoked by a sled agent to publish an updated runtime state for an | ||
| /// Instance. | ||
| pub async fn notify_instance_updated( | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
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.