oxidecomputer / oxidecomputer/omicron
Subnet update endpoint clears custom router ID when field is left out of body
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 572
- Forks
- 97
- Avg merge
- 2d 12h
- Merged PRs (30d)
- 96
Description
I think the pattern this is supposed to follow is that you pass null to clear the field out, but if it's not in the body at all, it should of course be left alone.
The problem is that because the custom router field is optional in the DB, we need to distinguish between a) left out of the PUT body entirely, which means don't change it, and b) put in the PUT body as custom_router: null, which means clear out the field if it was already non-null. I believe we've talked about this before, I'll have to find the discussion. You might need something like Option<Option<NameOrId>>.
Considering we don't seem to have any option of options in the API code, I think we must be solving this another way, with an explicit enum representing these options.
$ rg 'Option<Option'
wicket-common/src/inventory.rs
61: pub caboose_stage0: Option<Option<SpComponentCaboose>>,
62: pub caboose_stage0next: Option<Option<SpComponentCaboose>>,
wicket/src/state/inventory.rs
176: // caboose_stage0 is an Option<Option<SpComponentCaboose>>, so we
186: // caboose_stage0next is an Option<Option<SpComponentCaboose>>, so we
installinator/src/reporter.rs
26: on_tick_task: Option<JoinHandle<Option<Option<usize>>>>,
93: fn spawn_on_tick_task(&self) -> JoinHandle<Option<Option<usize>>> {
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with nexus/types/src/external_api/params.rs at lines 1191-1200 and inspect how the subnet update endpoint represents the custom router field. Review the existing Option examples found by the issue's rg search, then verify that an omitted field preserves the value while custom_router: null clears it.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- api
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 42/100