Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
386 changes: 201 additions & 185 deletions Cargo.lock

Large diffs are not rendered by default.

6 changes: 3 additions & 3 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -441,9 +441,9 @@ dns-server = { path = "dns-server" }
dns-server-api = { path = "dns-server-api" }
dns-service-client = { path = "clients/dns-service-client" }
dpd-client = { git = "https://github.com/oxidecomputer/dendrite", rev = "cc8e02a0800034c431c8cf96b889ea638da3d194" }
dropshot = { version = "0.16.3", features = [ "usdt-probes" ] }
dropshot-api-manager = "0.2.3"
dropshot-api-manager-types = "0.2.3"
dropshot = { version = "0.16.5", features = [ "usdt-probes" ] }
dropshot-api-manager = "0.2.4"
dropshot-api-manager-types = "0.2.4"
dyn-clone = "1.0.20"
either = "1.15.0"
ereport-types = { path = "ereport/types" }
Expand Down
2 changes: 1 addition & 1 deletion clients/oxide-client/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use std::sync::Arc;
use thiserror::Error;

progenitor::generate_api!(
spec = "../../openapi/nexus.json",
spec = "../../openapi/nexus/nexus-latest.json",
interface = Builder,
tags = Separate,
);
Expand Down
6 changes: 3 additions & 3 deletions dev-tools/dropshot-apis/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,9 +192,9 @@ fn all_apis() -> anyhow::Result<ManagedApis> {
},
ManagedApiConfig {
title: "Oxide Region API",
versions: Versions::new_lockstep(semver::Version::new(
20251208, 0, 0,
)),
versions: Versions::new_versioned(
nexus_external_api::supported_versions(),
),
metadata: ManagedApiMetadata {
description: Some(
"API for interacting with the Oxide control plane",
Expand Down
153 changes: 108 additions & 45 deletions nexus/external-api/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use dropshot::{
Query, RequestContext, ResultsPage, StreamingBody, TypedBody,
WebsocketChannelResult, WebsocketConnection,
};
use dropshot_api_manager_types::ValidationContext;
use dropshot_api_manager_types::{ValidationContext, api_versions};
use http::Response;
use ipnetwork::IpNetwork;
use nexus_types::{
Expand All @@ -33,7 +33,48 @@ use omicron_common::api::external::{
};
use openapiv3::OpenAPI;

pub const API_VERSION: &str = "20251208.0.0";
api_versions!([
// API versions are in the format YYYYMMDDNN.0.0, defined below as
// YYYYMMDDNN. Here, NN is a two-digit number starting at 00 for a
// particular date.
//
// WHEN CHANGING THE API (part 1 of 2):
//
// +- First, determine the next API version number to use.
// |
// | * On the main branch: Take today's date in YYYYMMDD format, e.g. 20251112.
// | Find the smallest NN that isn't already defined in the list below. In
// | most cases, that is 00, but if 00 is already taken, use 01, 02, etc.
// |
// | * On a release branch, don't alter the date. Instead, always bump the NN.
// |
// | Duplicate this line, uncomment the *second* copy, update that copy for
// | your new API version, and leave the first copy commented out as an
// | example for the next person.
// |
// | If there's a merge conflict, update the version number to the current
// | date. Otherwise, it is okay to leave the version number unchanged even
// | if you land your change on a different day from the one you make it on.
// |
// | Ensure that version numbers are sorted in descending order. (This macro
// | will panic at runtime if they're not in descending order.) The newest
// | date-based version should be at the top of the list.
// v
// (next_yyyymmddnn, IDENT),
(2025112000, INITIAL),
]);

// WHEN CHANGING THE API (part 2 of 2):
//
// The call to `api_versions!` above defines constants of type
// `semver::Version` that you can use in your Dropshot API definition to specify
// the version when a particular endpoint was added or removed. For example, if
// you used:
//
// (2025120100, ADD_FOOBAR)
//
// Then you could use `VERSION_ADD_FOOBAR` as the version in which endpoints
// were added or removed.

const MIB: usize = 1024 * 1024;
const GIB: usize = 1024 * MIB;
Expand Down Expand Up @@ -4364,8 +4405,37 @@ pub trait NexusExternalApi {
) -> Result<HttpResponseDeleted, HttpError>;
}

/// Perform extra validations on the OpenAPI spec.
/// Perform extra validations on the OpenAPI document, and generate the
/// nexus_tags.txt file.
pub fn validate_api(spec: &OpenAPI, mut cx: ValidationContext<'_>) {
let blessed = cx
.is_blessed()
.expect("this is a versioned API so is_blessed should always be Some");

// There are two parts to this function:
//
// 1. Perform validation on the OpenAPI document.
// 2. Generate the nexus_tags.txt file.
//
// Step 1 should only be performed on non-blessed versions. That's because
// blessed versions are immutable, and if new checks are added in the
// future, we don't want old API versions to be affected.
//
// nexus_tags.txt is unversioned, so step 2 should only be performed on the
// latest version, whether or not it's blessed.

if !blessed {
validate_api_doc(spec, &mut cx);
}

// nexus_tags.txt is unversioned, so only write it out for the latest
// version (whether it's blessed or not).
if cx.is_latest() {
generate_tags_file(spec, &mut cx);
}
}

fn validate_api_doc(spec: &OpenAPI, cx: &mut ValidationContext<'_>) {
if spec.openapi != "3.0.3" {
cx.report_error(anyhow!(
"Expected OpenAPI version to be 3.0.3, found {}",
Expand All @@ -4378,13 +4448,6 @@ pub fn validate_api(spec: &OpenAPI, mut cx: ValidationContext<'_>) {
spec.info.title,
));
}
if spec.info.version != API_VERSION {
cx.report_error(anyhow!(
"Expected OpenAPI version to be '{}', found '{}'",
API_VERSION,
spec.info.version,
));
}

// Spot check a couple of items.
if spec.paths.paths.is_empty() {
Expand All @@ -4394,13 +4457,7 @@ pub fn validate_api(spec: &OpenAPI, mut cx: ValidationContext<'_>) {
cx.report_error(anyhow!("Expected a path for /v1/projects"));
}

// Construct a string that helps us identify the organization of tags and
// operations.
let mut ops_by_tag =
BTreeMap::<String, Vec<(String, String, String)>>::new();

let mut ops_by_tag_valid = true;
for (path, method, op) in spec.operations() {
for (_path, _method, op) in spec.operations() {
// Make sure each operation has exactly one tag. Note, we intentionally
// do this before validating the OpenAPI output as fixing an error here
// would necessitate refreshing the spec file again.
Expand All @@ -4410,8 +4467,6 @@ pub fn validate_api(spec: &OpenAPI, mut cx: ValidationContext<'_>) {
op.operation_id.as_ref().unwrap(),
op.tags.len()
));
ops_by_tag_valid = false;
continue;
}

// Every non-hidden endpoint must have a summary
Expand All @@ -4421,8 +4476,21 @@ pub fn validate_api(spec: &OpenAPI, mut cx: ValidationContext<'_>) {
"operation '{}' is missing a summary doc comment",
op.operation_id.as_ref().unwrap()
));
// This error does not prevent `ops_by_tag` from being populated
// correctly, so we can continue.
}
}
}

fn generate_tags_file(spec: &OpenAPI, cx: &mut ValidationContext<'_>) {
// Construct a string that helps us identify the organization of tags and
// operations.
let mut ops_by_tag =
BTreeMap::<String, Vec<(String, String, String)>>::new();

for (path, method, op) in spec.operations() {
// If an operation doesn't have exactly one tag, skip generating the
// tags file entirely. (Validation above catches this case).
if op.tags.len() != 1 {
return;
}

ops_by_tag
Expand All @@ -4435,34 +4503,29 @@ pub fn validate_api(spec: &OpenAPI, mut cx: ValidationContext<'_>) {
));
}

if ops_by_tag_valid {
let mut tags = String::new();
for (tag, mut ops) in ops_by_tag {
ops.sort();
tags.push_str(&format!(
r#"API operations found with tag "{}""#,
tag
));
let mut tags = String::new();
for (tag, mut ops) in ops_by_tag {
ops.sort();
tags.push_str(&format!(r#"API operations found with tag "{}""#, tag));
tags.push_str(&format!(
"\n{:40} {:8} {}\n",
"OPERATION ID", "METHOD", "URL PATH"
));
for (operation_id, method, path) in ops {
tags.push_str(&format!(
"\n{:40} {:8} {}\n",
"OPERATION ID", "METHOD", "URL PATH"
"{:40} {:8} {}\n",
operation_id, method, path
));
for (operation_id, method, path) in ops {
tags.push_str(&format!(
"{:40} {:8} {}\n",
operation_id, method, path
));
}
tags.push('\n');
}

// When this fails, verify that operations on which you're adding,
// renaming, or changing the tags are what you intend.
cx.record_file_contents(
"nexus/external-api/output/nexus_tags.txt",
tags.into_bytes(),
);
tags.push('\n');
}

// When this fails, verify that operations on which you're adding,
// renaming, or changing the tags are what you intend.
cx.record_file_contents(
"nexus/external-api/output/nexus_tags.txt",
tags.into_bytes(),
);
}

pub type IpPoolRangePaginationParams =
Expand Down
12 changes: 12 additions & 0 deletions nexus/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,18 @@ impl Server {
log.new(o!("component" => "dropshot_external")),
)
.config(config.deployment.dropshot_external.dropshot.clone())
.version_policy(dropshot::VersionPolicy::Dynamic(Box::new(
dropshot::ClientSpecifiesVersionInHeader::new(
omicron_common::api::VERSION_HEADER,
nexus_external_api::latest_version(),
)
// Since we don't have control over all clients to the external
// API, we allow the api-version header to not be specified
// (picking the latest version in that case). However, all
// clients that *are* under our control should specify the
// api-version header.
.on_missing(nexus_external_api::latest_version()),
)))
.tls(tls_config.clone().map(dropshot::ConfigTls::Dynamic))
.start()
.map_err(|error| {
Expand Down
5 changes: 5 additions & 0 deletions nexus/test-utils/src/http_testing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,11 @@ impl<'a> RequestBuilder<'a> {
self
}

/// Return the current header map.
pub fn headers(&self) -> &http::HeaderMap {
&self.headers
}

/// Set the outgoing request body
///
/// If `body` is `None`, the request body will be empty.
Expand Down
2 changes: 1 addition & 1 deletion nexus/tests/integration_tests/unauthorized_coverage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use std::collections::BTreeMap;
#[test]
fn test_unauthorized_coverage() {
// Load the OpenAPI schema for Nexus's public API.
let schema_path = "../openapi/nexus.json";
let schema_path = "../openapi/nexus/nexus-latest.json";
let schema_contents = std::fs::read_to_string(&schema_path)
.expect("failed to read Nexus OpenAPI spec");
let spec: OpenAPI = serde_json::from_str(&schema_contents)
Expand Down
30 changes: 30 additions & 0 deletions nexus/tests/integration_tests/updates.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,12 @@ use camino::Utf8Path;
use camino_tempfile::{Builder, Utf8TempPath};
use chrono::{DateTime, Duration, Timelike, Utc};
use dropshot::ResultsPage;
use dropshot::test_util::ClientTestContext;
use http::{Method, StatusCode};
use nexus_db_model::SemverVersion;
use nexus_db_queries::context::OpContext;
use nexus_db_queries::db::pub_test_utils::helpers::insert_test_tuf_repo;
use nexus_test_interface::NexusServer;
use nexus_test_utils::background::activate_background_task;
use nexus_test_utils::background::run_tuf_artifact_replication_step;
use nexus_test_utils::background::wait_tuf_artifact_replication_step;
Expand Down Expand Up @@ -1039,3 +1041,31 @@ async fn test_repo_list() -> Result<()> {
cptestctx.teardown().await;
Ok(())
}

/// Test that a request without an API version header still succeeds.
///
/// Unlike internal APIs, we don't control all clients for our external API, so
/// we make the api-version header optional. This test ensures that a request
/// without the header succeeds.
#[nexus_test]
async fn test_request_without_api_version(cptestctx: &ControlPlaneTestContext) {
// We can't use cptestctx.external_client directly since it always sets the
// header. Instead, construct a NexusRequest by hand.
let server_addr = cptestctx.server.get_http_server_external_address();
let test_cx =
ClientTestContext::new(server_addr, cptestctx.logctx.log.clone());
let req_builder = RequestBuilder::new(
&test_cx,
http::Method::GET,
"/v1/system/update/status",
);
assert_eq!(
req_builder.headers().get(&omicron_common::api::VERSION_HEADER),
None,
"api-version header is not set"
);
let req =
NexusRequest::new(req_builder).authn_as(AuthnMode::PrivilegedUser);
let status: views::UpdateStatus = req.execute_and_parse_unwrap().await;
assert_eq!(status.target_release.0, None);
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
"url": "https://oxide.computer",
"email": "[email protected]"
},
"version": "20251208.0.0"
"version": "2025112000.0.0"
},
"paths": {
"/device/auth": {
Expand Down
1 change: 1 addition & 0 deletions openapi/nexus/nexus-latest.json
Loading
Loading