oxidecomputer / oxidecomputer/maghemite

`mgd` static routing API: restructure to reject nonsense path combinations?

Open
#739 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Rust
Stars
94
Forks
6
Avg merge
1d 8h
Merged PRs (30d)
12

Description

Property-based tests hitting the mgd static routing API produced some API calls that tried to set up multiple overlapping routes; e.g.,

        v4: {
            "0.0.0.0/2": [
                Path {
                    bgp: None,
                    nexthop: 1.2.3.4,
                    nexthop_interface: None,
                    rib_priority: 14,
                    shutdown: false,
                    vlan_id: None,
                },
                Path {
                    bgp: None,
                    nexthop: 1.2.3.4,
                    nexthop_interface: None,
                    rib_priority: 153,
                    shutdown: false,
                    vlan_id: None,
                },
            ],
        },

mgd returned an OK for adding these routes, but internally only kept the priority 14 one. The current structure of this API is a HashMap<String, Vec<Path>> where the key is a stringified prefix. @taspelund noted that the more accurate structure here would be a Map<(Prefix, Nexthop, VlanId), Properties> - we can have at most one entry for that triple. JSON (and therefore OpenAPI) requires keys be strings, but we can still use stronger types on the Rust side.

A technique we've used in other places in Omicron is to make the OpenAPI / JSON type a flat Vec, so in this case a Vec<(Prefix, Nexthop, VlanId, Properties)>, but serialize/deserialize from the stronger Map type (be that a HashMap/BTreeMap or any other map/set-like collection), and have the Deserialize implementation raise an error if there are any duplicate keys.

Contributor guide

No contributing guide indexed for this repository

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

Locate the mgd static routing API and its current HashMap<String, Vec> representation; the payload does not name files or tests. Start by tracing its Rust serialization and deserialization, then exercise the property-based duplicate-route case. Done means duplicate (Prefix, Nexthop, VlanId) entries are rejected instead of silently retaining one.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
api, networking
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.