Normalize away websocket response change in dropshot 0.17#87
Merged
sunshowers merged 7 commits intomainfrom Apr 15, 2026
Merged
Normalize away websocket response change in dropshot 0.17#87sunshowers merged 7 commits intomainfrom
sunshowers merged 7 commits intomainfrom
Conversation
Dropshot 0.17 changed websocket endpoint representation in OpenAPI (oxidecomputer/dropshot#1554): from a default response with */* content to explicit 101/4XX/5XX responses. This is a spec-generation change, not a wire-format change, but drift reports it as incompatible. Uses the new drift::compare_with_normalizer to rewrite old-format websocket responses in blessed specs before comparison. The normalizer detects operations with x-dropshot-websocket that have a default response and replaces their responses with those from the generated spec. Also adds a versioned_ws test fixture with a #[channel] endpoint and integration tests proving the normalization works (including a test that verifies drift fails without it).
sunshowers
reviewed
Apr 14, 2026
| } | ||
|
|
||
| #[test] | ||
| fn test_normalize_missing_generated_path_leaves_blessed_unchanged() { |
sunshowers
reviewed
Apr 14, 2026
|
|
||
| /// Rewrite a versioned API document to old websocket format and rename the | ||
| /// file to match the new content hash. Returns the new relative path. | ||
| fn rewrite_versioned_doc_ws_format( |
Collaborator
There was a problem hiding this comment.
Actually I have a suggestion for a better way to do this -- can you write a custom extractor that produces the old response type?
Contributor
Author
There was a problem hiding this comment.
Great idea, done in e19835c. Let me know if that's what you had in mind.
sunshowers
reviewed
Apr 15, 2026
sunshowers
reviewed
Apr 15, 2026
| blessed: &mut serde_json::Value, | ||
| generated: &serde_json::Value, | ||
| ) { | ||
| // Exact JSON for the old and new websocket response formats. We only |
Collaborator
There was a problem hiding this comment.
This will re-initialize these objects for each blessed API, but I think that's fine (this is parallelized + also there are many things that are much more time-consuming).
The alternative would be to put these in a OnceLock.
sunshowers
approved these changes
Apr 15, 2026
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.
Problem
While trying to bump Dropshot to 0.17 in oxidecomputer/omicron#9995 in order to turn on gzipped responses, I ran into a problem where dropshot-api-manager doesn't know how to deal with the schema change introduced by oxidecomputer/dropshot#1554.
Bumping the API version on the affected crates does not help because the problem is when it tries to verify the existing blessed version, not the new version.
Solution
The fix is to convert any existing schema using the old pattern (on websocket endpoints specifically) to the new pattern before the comparison, so the change is effectively ignored.
@sunshowers and I talked about putting this in drift, but looking at drift, it felt wrong to me to put something so specific in there. Drift is quite abstract. Initially I thought this would involve changing drift to take a normalizer function to apply to the schema before doing the comparison, but the function was like two lines — we just do it here before calling
compare. In order for putting something like that in drift to buy us something, it would have to be more constrained than a function that just takes the whole schema and returns a new one.