oxidecomputer / oxidecomputer/omicron

Subnet update endpoint clears custom router ID when field is left out of body

Open
#6,406 8 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

known issue
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.

image

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>>.

https://github.com/oxidecomputer/omicron/blob/d24003b74d232d2ff643a6fc9b5ee10cb5f2b056/nexus/types/src/external_api/params.rs#L1191-L1200

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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.