[mg-admin-api] fix v2/v3 MessageHistory and related types#730
Merged
sunshowers merged 1 commit intoMay 7, 2026
Merged
Conversation
Created using spr 1.3.6-beta.1
Contributor
Author
|
With drift 0.1.4 but without this fix, we would see this error: https://gist.github.com/sunshowers/cc4b0ce3702acef9cad27ac7d0e36136 |
Contributor
|
@sunshowers I recently opened #724 that I think tries to do the same thing as this PR, but applies it to the whole repo. If you have time to give it a look, I'd love to get your feedback on it |
Contributor
Author
Oh neat. I actually started on this myself, can definitely have a look at that later. Would it be okay if we got this in? This is a correctness fix. I'm happy to help merge main into your branch afterwards. |
Contributor
|
Yeah, totally. This looks great, thank you for the fix :-) |
taspelund
approved these changes
May 7, 2026
sunshowers
deleted the
sunshowers/spr/mg-admin-api-fix-v2v3-messagehistory-and-related-types
branch
taspelund
added a commit
that referenced
this pull request
May 9, 2026
Brings in 15 commits from main, the most substantive being: - #730 [mg-admin-api] fix v2/v3 MessageHistory and related types - #715 Update Rust crate rust to v1.95.0 (with clippy::useless_conversion fix) - #710 dropshot 0.7.1 - #707 opte update - assorted dep bumps (tokio, clap, oxnet, uuid, oxide-tokio-rt, openssl, reqwest, libc) and lock-file maintenance Notes on conflict resolution: - mg-types/versions/src/ipv6_basic/bgp.rs: incorporated #730's v2 MessageHistory/MessageHistoryEntry/Message/UpdateMessage types and their From-from-live conversions, but retargeted imports through bgp-types-versions and rdb-types-versions to preserve the leaf-crate rule cleanup from this branch (the commit message on #730 calls out that this is a known short-term shortcut). Same approach for mg-types/versions/src/mp_bgp/bgp.rs (where #730 added a v4 MessageHistoryResponse). - mg-types/versions/Cargo.toml: kept the leaf-rule-clean dep set (bgp-types-versions, rdb-types-versions) and added chrono per #730's need for chrono::DateTime in MessageHistoryEntry. - bgp-types/versions/src/ipv6_basic/session.rs: promoted MessageHistoryEntry's timestamp/message/connection_id fields back from pub(crate) to pub. The migration in this branch had inadvertently tightened the visibility (they were pub on main); #730 needs pub access for its From impl. - bgp-types/versions/src/impls/messages.rs: applied #715's clippy::useless_conversion fix at the migrated location (impl OpenMessage::get_capabilities), which is where the original bgp::messages code now lives after our migration. - bgp/src/messages.rs and bgp/src/session.rs: took our side; #715's clippy fix applies to the migrated location, not the bgp-local free-fn replacements. - mg-api/src/lib.rs: kept our use bfd_types::{BfdPeerConfig, BfdPeerInfo} import; took main's mg_types_versions::{latest, v1, v2, v4, v5} (gaining v4 for #730's MessageHistoryResponse v4 endpoint). - Cargo.lock: regenerated. Verified: cargo check --workspace --all-features clean, cargo clippy --workspace --all-features --no-deps clean (only pre-existing lab/ warnings), cargo run -p xtask -- openapi check shows all 10 documents fresh (byte-identical schemas), 152/152 tests pass on bgp + bfd + *-types-versions crates. Signed-off-by: Trey Aspelund <trey@oxidecomputer.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
This type was changed in v4, but the change was accidentally also applied to v2 and v3. The nontrivial change was not detected by drift due to oxidecomputer/drift#22, which is now fixed in drift 0.1.4.
Fix this up by introducing types and conversions for v2. This is unfortunately complicated by the fact that a lot of the BGP types are not in mg-admin-types-versions, partly because untangling this is quite difficult. I've opted to just do a relatively minimal fix for now where the v2 types are defined in mg-admin-types-versions, but the newest types continue to live in bgp. I'll try untangling this further in the future.