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.

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

Move network interface authz to the data store #778

Merged
merged 2 commits into from
Mar 16, 2022
Merged

Conversation

bnaecker
Copy link
Collaborator

  • Adds a module-private function for actually inserting the database
    record, after performing authz checks. This is used in the
    publicly-available method and in tests.
  • Adds authz objects to the
    DataStore::instance_create_network_interface method and does authz
    checks inside them.
  • Reorders the instance-creation saga. This moves the instance DB record
    creation before the NIC creation, since the latter can only be
    attached to an existing instance record. This also allows uniform
    authz checks inside the DataStore method, which wouldn't be possible
    if the instance record were not yet in the database. Note that this
    also requires a small change to the data the instance-record-creation
    saga node serializes. It previously contained the NICs, but these are
    no longer available at that time. Instead, the NICs are deserialized
    from the saga node that creates them and used to instantiate the
    instance runtime object only inside the sic_instance_ensure saga
    node.
  • Moves authz check for listing NICs for an instance into DataStore
    method
  • Moves authz check for fetching a single NIC for an instance into the
    DataStore method
  • Adds the network_interface_fetch method, for returning an authz
    interface and the database record, after checking read access. This
    uses a *_noauthz method as well, both of which are analogous to the
    other similarly-named methods. Note there's no lookup by ID or path at
    this point, since they're not really needed yet.
  • Moves the check for deleting an interface into the DataStore method.
  • Changes how deletion of a previously-deleted NIC works. We used to
    return a success, but we now return a not-found error, in line with
    the rest of the API.

- Adds a module-private function for actually inserting the database
  record, after performing authz checks. This is used in the
  publicly-available method and in tests.
- Adds authz objects to the
  `DataStore::instance_create_network_interface` method and does authz
  checks inside them.
- Reorders the instance-creation saga. This moves the instance DB record
  creation before the NIC creation, since the latter can only be
  attached to an existing instance record. This also allows uniform
  authz checks inside the `DataStore` method, which wouldn't be possible
  if the instance record were not yet in the database. Note that this
  also requires a small change to the data the instance-record-creation
  saga node serializes. It previously contained the NICs, but these are
  no longer available at that time. Instead, the NICs are deserialized
  from the saga node that creates them and used to instantiate the
  instance runtime object only inside the `sic_instance_ensure` saga
  node.
- Moves authz check for listing NICs for an instance into `DataStore`
  method
- Moves authz check for fetching a single NIC for an instance into the
  `DataStore` method
- Adds the `network_interface_fetch` method, for returning an authz
  interface and the database record, after checking read access. This
  uses a `*_noauthz` method as well, both of which are analogous to the
  other similarly-named methods. Note there's no lookup by ID or path at
  this point, since they're not really needed yet.
- Moves the check for deleting an interface into the `DataStore` method.
- Changes how deletion of a previously-deleted NIC works. We used to
  return a success, but we now return a not-found error, in line with
  the rest of the API.
nexus/src/db/datastore.rs Show resolved Hide resolved
nexus/src/db/datastore.rs Outdated Show resolved Hide resolved
@davepacheco davepacheco mentioned this pull request Mar 16, 2022
71 tasks
@bnaecker bnaecker enabled auto-merge (squash) March 16, 2022 22:11
@bnaecker bnaecker merged commit 281a070 into main Mar 16, 2022
@bnaecker bnaecker deleted the move-nic-authz branch March 16, 2022 22:45
leftwo pushed a commit that referenced this pull request Oct 25, 2024
Crucible changes
Add test and fix for replay race condition (#1519)
Fix clippy warnings (#1517)
Add edition to `crucible-workspace-hack` (#1516)
Split out Downstairs-specific stats (#1511)
Move remaining `GuestWork` functionality into `Downstairs` (#1510)
Track both jobs and bytes in each IO state (#1507)
Fix new `rustc` and `clippy` warnings (#1509)
Remove IOP/BW limits (for now) (#1506)
Move `GuestBlockRes` into `DownstairsIO` (#1502)
Update actions/checkout digest to eef6144 (#1499)
Update Rust crate hyper-staticfile to 0.10.1 (#1411)
Turn off test-up-2region-encrypted.sh (#1504)
Add `IOop::Barrier` (#1494)
Fix IPv6 addresses in `crutest` (#1503)
Add region set options to more tests. (#1496)
Simplify `CompleteJobs` (#1493)
Removed ignored CI jobs (#1497)
Minor cleanups to `print_last_completed` (#1501)
Remove remaining `Arc<Volume>` instances (#1500)
Add `VolumeBuilder` type (#1492)
remove old unused scripts (#1495)
More multiple region support. (#1484)
Simplify matches (#1490)
Move complete job tracker to a helper object (#1489)
Expand summary and add documentation references to the README. (#1486)
Remove `GuestWorkId` (2/2) (#1482)
Remove `JobId` from `DownstairsIO` (1/2) (#1481)
Remove unused `#[derive(..)]` (#1483)
Update more tests to use dsc (#1480)
Crutest now Volume only (#1479)

Propolis changes
manually impl Deserialize for PciPath for validation purposes (#801)
phd: gate OS-specific tests, make others more OS-agnostic (#799)
lib: log vCPU diagnostics on triple fault and for some unhandled exit types (#795)
add marker trait to help check safety of guest memory reads (#794)
clippy fixes for 1.82 (#796)
lib: move cpuid::Set to cpuid_utils; prevent semantic subleaf conflicts (#782)
PHD: write efivars in one go (#786)
PHD: support guest-initiated reboot (#785)
server: accept CPUID values in instance specs and plumb them to bhyve (#780)
PHD: allow patched Crucible dependencies (#778)
server: add a first-class error type to machine init (#777)
PciPath to Bdf conversion is infallible; prove it and refactor (#774)
instance spec rework: flatten InstanceSpecV0 (#767)
Make PUT /instance/state 503 when waiting to init
Less anxiety-inducing `Vm::{get, state_watcher}`
leftwo added a commit that referenced this pull request Oct 30, 2024
Crucible changes
Add test and fix for replay race condition (#1519) Fix clippy warnings
(#1517)
Add edition to `crucible-workspace-hack` (#1516)
Split out Downstairs-specific stats (#1511)
Move remaining `GuestWork` functionality into `Downstairs` (#1510) Track
both jobs and bytes in each IO state (#1507) Fix new `rustc` and
`clippy` warnings (#1509)
Remove IOP/BW limits (for now) (#1506)
Move `GuestBlockRes` into `DownstairsIO` (#1502)
Update actions/checkout digest to eef6144 (#1499)
Update Rust crate hyper-staticfile to 0.10.1 (#1411) Turn off
test-up-2region-encrypted.sh (#1504)
Add `IOop::Barrier` (#1494)
Fix IPv6 addresses in `crutest` (#1503)
Add region set options to more tests. (#1496)
Simplify `CompleteJobs` (#1493)
Removed ignored CI jobs (#1497)
Minor cleanups to `print_last_completed` (#1501)
Remove remaining `Arc<Volume>` instances (#1500)
Add `VolumeBuilder` type (#1492)
remove old unused scripts (#1495)
More multiple region support. (#1484)
Simplify matches (#1490)
Move complete job tracker to a helper object (#1489) Expand summary and
add documentation references to the README. (#1486) Remove `GuestWorkId`
(2/2) (#1482)
Remove `JobId` from `DownstairsIO` (1/2) (#1481)
Remove unused `#[derive(..)]` (#1483)
Update more tests to use dsc (#1480)
Crutest now Volume only (#1479)

Propolis changes
manually impl Deserialize for PciPath for validation purposes (#801)
phd: gate OS-specific tests, make others more OS-agnostic (#799) lib:
log vCPU diagnostics on triple fault and for some unhandled exit types
(#795) add marker trait to help check safety of guest memory reads
(#794) clippy fixes for 1.82 (#796)
lib: move cpuid::Set to cpuid_utils; prevent semantic subleaf conflicts
(#782) PHD: write efivars in one go (#786)
PHD: support guest-initiated reboot (#785)
server: accept CPUID values in instance specs and plumb them to bhyve
(#780) PHD: allow patched Crucible dependencies (#778)
server: add a first-class error type to machine init (#777) PciPath to
Bdf conversion is infallible; prove it and refactor (#774) instance spec
rework: flatten InstanceSpecV0 (#767) Make PUT /instance/state 503 when
waiting to init
Less anxiety-inducing `Vm::{get, state_watcher}`

---------

Co-authored-by: Alan Hanson <alan@oxide.computer>
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.

2 participants