add ClientSpecifiesVersionInHeader::on_missing (#1475)

Author: sunshowers

CHANGELOG.adoc

@@ -15,6 +15,8 @@
 
 https://github.com/oxidecomputer/dropshot/compare/v0.16.4\...HEAD[Full list of commits]
 
+* https://github.com/oxidecomputer/dropshot/pull/1475[#1475] Added `ClientSpecifiesVersionInHeader::on_missing` to provide a default version when the header is missing, intended for use when you're not in control of all clients
+
 == 0.16.4 (released 2025-09-04)
 
 https://github.com/oxidecomputer/dropshot/compare/v0.16.3\...v0.16.4[Full list of commits]

dropshot/src/versioning.rs

@@ -114,9 +114,9 @@ pub trait DynamicVersionPolicy: std::fmt::Debug + Send + Sync {
     ) -> Result<Version, HttpError>;
 }
 
-/// Implementation of `DynamicVersionPolicy` where the client must specify a
+/// Implementation of `DynamicVersionPolicy` where the client specifies a
 /// specific semver in a specific header and we always use whatever they
-/// requested
+/// requested.
 ///
 /// An incoming request will be rejected with a 400-level error if:
 ///
@@ -125,17 +125,22 @@ pub trait DynamicVersionPolicy: std::fmt::Debug + Send + Sync {
 ///   [`ClientSpecifiesVersionInHeader::new()`], which implies that the client
 ///   is trying to use a newer version of the API than this server supports.
 ///
+/// By default, incoming requests will also be rejected with a 400-level error
+/// if the header is missing. To override this behavior, supply a default
+/// version via [`Self::on_missing`].
+///
 /// If you need anything more flexible (e.g., validating the provided version
 /// against a fixed set of supported versions), you'll want to impl
 /// `DynamicVersionPolicy` yourself.
 #[derive(Debug)]
 pub struct ClientSpecifiesVersionInHeader {
     name: HeaderName,
     max_version: Version,
+    on_missing: Option<Version>,
 }
 
 impl ClientSpecifiesVersionInHeader {
-    /// Make a new `ClientSpecifiesVersionInHeader` policy
+    /// Make a new `ClientSpecifiesVersionInHeader` policy.
     ///
     /// Arguments:
     ///
@@ -148,7 +153,25 @@ impl ClientSpecifiesVersionInHeader {
         name: HeaderName,
         max_version: Version,
     ) -> ClientSpecifiesVersionInHeader {
-        ClientSpecifiesVersionInHeader { name, max_version }
+        ClientSpecifiesVersionInHeader { name, max_version, on_missing: None }
+    }
+
+    /// If the header is missing, use the provided version instead.
+    ///
+    /// By default, the policy will reject requests with a missing header. Call
+    /// this function to use the provided version instead.
+    ///
+    /// Typically, the provided version should either be a fixed supported
+    /// version (for backwards compatibility with older clients), or the newest
+    /// supported version (in case clients are generally kept up-to-date but not
+    /// all clients send the header).
+    ///
+    /// Using this function is not recommended if you control all clients—in
+    /// that case, arrange for clients to send the header instead. In
+    /// particular, **at Oxide, do not use this function for internal APIs.**
+    pub fn on_missing(mut self, version: Version) -> Self {
+        self.on_missing = Some(version);
+        self
     }
 }
 
@@ -159,13 +182,25 @@ impl DynamicVersionPolicy for ClientSpecifiesVersionInHeader {
         _log: &Logger,
     ) -> Result<Version, HttpError> {
         let v = parse_header(request.headers(), &self.name)?;
-        if v <= self.max_version {
-            Ok(v)
-        } else {
-            Err(HttpError::for_bad_request(
+        match (v, &self.on_missing) {
+            (Some(v), _) => {
+                if v <= self.max_version {
+                    Ok(v)
+                } else {
+                    Err(HttpError::for_bad_request(
+                        None,
+                        format!(
+                            "server does not support this API version: {}",
+                            v
+                        ),
+                    ))
+                }
+            }
+            (None, Some(on_missing)) => Ok(on_missing.clone()),
+            (None, None) => Err(HttpError::for_bad_request(
                 None,
-                format!("server does not support this API version: {}", v),
-            ))
+                format!("missing expected header {:?}", self.name),
+            )),
         }
     }
 }
@@ -175,17 +210,12 @@ impl DynamicVersionPolicy for ClientSpecifiesVersionInHeader {
 fn parse_header<T>(
     headers: &http::HeaderMap,
     header_name: &HeaderName,
-) -> Result<T, HttpError>
+) -> Result<Option<T>, HttpError>
 where
     T: FromStr,
     <T as FromStr>::Err: std::fmt::Display,
 {
-    let v_value = headers.get(header_name).ok_or_else(|| {
-        HttpError::for_bad_request(
-            None,
-            format!("missing expected header {:?}", header_name),
-        )
-    })?;
+    let Some(v_value) = headers.get(header_name) else { return Ok(None) };
 
     let v_str = v_value.to_str().map_err(|_| {
         HttpError::for_bad_request(
@@ -197,10 +227,12 @@ where
         )
     })?;
 
-    v_str.parse::<T>().map_err(|e| {
+    let v = v_str.parse::<T>().map_err(|e| {
         HttpError::for_bad_request(
             None,
             format!("bad value for header {:?}: {}: {}", header_name, e, v_str),
         )
-    })
+    })?;
+
+    Ok(Some(v))
 }

dropshot/tests/integration-tests/versions.rs

@@ -325,6 +325,40 @@ async fn test_versions() {
     }
 }
 
+#[tokio::test]
+async fn test_versions_on_missing() {
+    let logctx = common::create_log_context("test_versions_on_missing");
+    let server = ServerBuilder::new(api(), (), logctx.log.clone())
+        .version_policy(VersionPolicy::Dynamic(Box::new(
+            ClientSpecifiesVersionInHeader::new(
+                VERSION_HEADER_NAME.parse().unwrap(),
+                Version::new(1, 4, 0),
+            )
+            .on_missing(Version::new(1, 1, 0)),
+        )))
+        .start()
+        .unwrap();
+
+    let server_addr = server.local_addr();
+    let url = format!("http://{}/demo", server_addr);
+    let client = reqwest::Client::new();
+
+    // Missing header => should resolve to 1.1.0.
+    let request = client.request(Method::GET, &url);
+    let response = request.send().await.unwrap();
+    assert_eq!(response.status(), StatusCode::OK);
+    let body: String = response.json().await.unwrap();
+    assert_eq!(body, HANDLER2_MSG);
+
+    // Now add a version header.
+    let request =
+        client.request(Method::GET, &url).header(VERSION_HEADER_NAME, "1.4.0");
+    let response = request.send().await.unwrap();
+    assert_eq!(response.status(), StatusCode::OK);
+    let body: String = response.json().await.unwrap();
+    assert_eq!(body, HANDLER4_MSG);
+}
+
 /// Test that the generated OpenAPI spec only refers to handlers in that version
 /// and types that are used by those handlers.
 #[test]