From 5f8076c0dabaea5c797088b884cffdf9a335a104 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Wed, 20 Aug 2025 23:05:04 -0500 Subject: [PATCH 01/31] update status endpoint --- nexus/db-model/src/target_release.rs | 1 - nexus/external-api/output/nexus_tags.txt | 1 + nexus/external-api/src/lib.rs | 9 + nexus/src/app/update.rs | 166 +++++++++++++++++- nexus/src/external_api/http_entrypoints.rs | 18 ++ nexus/tests/integration_tests/endpoints.rs | 8 + .../tests/integration_tests/target_release.rs | 17 +- nexus/tests/integration_tests/updates.rs | 91 ++++++++++ nexus/types/src/external_api/views.rs | 18 +- openapi/nexus.json | 64 ++++++- 10 files changed, 373 insertions(+), 20 deletions(-) diff --git a/nexus/db-model/src/target_release.rs b/nexus/db-model/src/target_release.rs index cbc681912f6..d89cf069926 100644 --- a/nexus/db-model/src/target_release.rs +++ b/nexus/db-model/src/target_release.rs @@ -66,7 +66,6 @@ impl TargetRelease { release_source: views::TargetReleaseSource, ) -> views::TargetRelease { views::TargetRelease { - generation: (&self.generation.0).into(), time_requested: self.time_requested, release_source, } diff --git a/nexus/external-api/output/nexus_tags.txt b/nexus/external-api/output/nexus_tags.txt index 4d3daee3807..4d25bbcd1a3 100644 --- a/nexus/external-api/output/nexus_tags.txt +++ b/nexus/external-api/output/nexus_tags.txt @@ -304,6 +304,7 @@ system_update_trust_root_list GET /v1/system/update/trust-roots system_update_trust_root_view GET /v1/system/update/trust-roots/{trust_root_id} target_release_update PUT /v1/system/update/target-release target_release_view GET /v1/system/update/target-release +update_status GET /v1/system/update/status API operations found with tag "tokens" OPERATION ID METHOD URL PATH diff --git a/nexus/external-api/src/lib.rs b/nexus/external-api/src/lib.rs index 75e3ac47685..77bcdf1b3f4 100644 --- a/nexus/external-api/src/lib.rs +++ b/nexus/external-api/src/lib.rs @@ -3072,6 +3072,15 @@ pub trait NexusExternalApi { params: TypedBody, ) -> Result, HttpError>; + #[endpoint { + method = GET, + path = "/v1/system/update/status", + tags = ["system/update"], + }] + async fn update_status( + rqctx: RequestContext, + ) -> Result, HttpError>; + // Silo users /// List users diff --git a/nexus/src/app/update.rs b/nexus/src/app/update.rs index c4fb9428a47..7c13a62fc41 100644 --- a/nexus/src/app/update.rs +++ b/nexus/src/app/update.rs @@ -9,15 +9,22 @@ use dropshot::HttpError; use futures::Stream; use nexus_auth::authz; use nexus_db_lookup::LookupPath; -use nexus_db_model::{TufRepoDescription, TufTrustRoot}; +use nexus_db_model::{Generation, TufRepoDescription, TufTrustRoot}; use nexus_db_queries::context::OpContext; use nexus_db_queries::db::{datastore::SQL_BATCH_SIZE, pagination::Paginator}; +use nexus_types::deployment::TargetReleaseDescription; use nexus_types::external_api::shared::TufSignedRootRole; +use nexus_types::external_api::views; +use nexus_types::internal_api::views as internal_views; +use nexus_types::inventory::RotSlot; +use omicron_common::api::external::InternalContext; use omicron_common::api::external::{ DataPageParams, Error, TufRepoInsertResponse, TufRepoInsertStatus, }; +use omicron_common::disk::M2Slot; use omicron_uuid_kinds::{GenericUuid, TufTrustRootUuid}; use semver::Version; +use std::collections::BTreeMap; use update_common::artifacts::{ ArtifactsWithPlan, ControlPlaneZonesMode, VerificationMode, }; @@ -149,4 +156,161 @@ impl super::Nexus { .await .map_err(HttpError::from) } + + /// Get external update status with aggregated component counts + pub async fn update_status_external( + &self, + opctx: &OpContext, + ) -> Result { + // ? because we expect there to always be a current target release. but + // it can still have an Unspecified release_source + let db_target_release = + self.datastore().target_release_get_current(opctx).await?; + let target_release = self + .datastore() + .target_release_view(opctx, &db_target_release) + .await?; + + let components_by_release_version = + self.component_version_counts(opctx, &db_target_release).await?; + + let time_last_progress = self + .datastore() + .blueprint_target_get_current(opctx) + .await? + .time_made_target; + + Ok(views::UpdateStatus { + target_release, + components_by_release_version, + time_last_progress, + }) + } + + /// Get component status using read-only queries to avoid batch operations + async fn component_version_counts( + &self, + opctx: &OpContext, + target_release: &nexus_db_model::TargetRelease, + ) -> Result, Error> { + // Get the latest inventory collection + let Some(inventory) = + self.datastore().inventory_get_latest_collection(opctx).await? + else { + // No inventory collection available, return empty counts + return Ok(BTreeMap::new()); + }; + + // Build current TargetReleaseDescription, defaulting to Initial if + // there is no tuf repo ID which, based on DB constraints, happens if + // and only if target_release_source is 'unspecified', which should only + // happen in the initial state before any target release has been set + let curr_target_desc = match target_release.tuf_repo_id { + Some(id) => TargetReleaseDescription::TufRepo( + self.datastore() + .tuf_repo_get_by_id(opctx, id.into()) + .await? + .into_external(), + ), + None => TargetReleaseDescription::Initial, + }; + + // Get previous target release (if it exists). Build the "prev" + // TargetReleaseDescription from the previous generation if available, + // otherwise fall back to Initial. + let prev_repo_id = + if let Some(prev_gen) = target_release.generation.prev() { + self.datastore() + .target_release_get_generation(opctx, Generation(prev_gen)) + .await + .internal_context("fetching previous target release")? + .and_then(|r| r.tuf_repo_id) + } else { + None + }; + + // It should never happen that a target release other than the initial + // one with target_release_source unspecified should be missing a + // tuf_repo_id. So if we have a tuf_repo_id for the previous target + // release, we should always have one for the current target. + if prev_repo_id.is_some() && target_release.tuf_repo_id.is_none() { + return Err(Error::internal_error( + "Target release has no tuf repo but previous release has one", + )); + } + + let prev_target_desc = match prev_repo_id { + Some(id) => TargetReleaseDescription::TufRepo( + self.datastore() + .tuf_repo_get_by_id(opctx, id.into()) + .await? + .into_external(), + ), + None => TargetReleaseDescription::Initial, + }; + + // It's weird to use the internal view this way. It would feel more + // correct to extract shared logic and call it in both places. On the + // other hand, that sharing would be boilerplatey and not add much yet. + // So for now, use the internal view, but plan to extract shared logic + // or do our own thing here once things settle. + let status = internal_views::UpdateStatus::new( + &prev_target_desc, + &curr_target_desc, + &inventory, + ); + + let sled_versions = status.sleds.into_iter().flat_map(|sled| { + let zone_versions = sled.zones.into_iter().map(|zone| zone.version); + + // boot_disk tells you which slot is relevant + let host_version = + sled.host_phase_2.boot_disk.ok().map(|slot| match slot { + M2Slot::A => sled.host_phase_2.slot_a_version.clone(), + M2Slot::B => sled.host_phase_2.slot_b_version.clone(), + }); + + zone_versions.chain(host_version) + }); + + let mgs_driven_versions = + status.mgs_driven.into_iter().flat_map(|status| { + // for the SP, slot0_version is the active one + let sp_version = status.sp.slot0_version.clone(); + + // for the bootloader, stage0_version is the active one. + let bootloader_version = + status.rot_bootloader.stage0_version.clone(); + + let rot_version = + status.rot.active_slot.map(|slot| match slot { + RotSlot::A => status.rot.slot_a_version.clone(), + RotSlot::B => status.rot.slot_b_version.clone(), + }); + + let host_version = match &status.host_os_phase_1 { + internal_views::HostPhase1Status::Sled { + slot_a_version, + slot_b_version, + active_slot, + .. + } => active_slot.map(|slot| match slot { + M2Slot::A => slot_a_version.clone(), + M2Slot::B => slot_b_version.clone(), + }), + _ => None, + }; + + std::iter::once(sp_version) + .chain(rot_version) + .chain(std::iter::once(bootloader_version)) + .chain(host_version) + }); + + let mut counts = BTreeMap::new(); + for version in sled_versions.chain(mgs_driven_versions) { + *counts.entry(version.to_string()).or_insert(0) += 1; + } + Ok(counts) + } } diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index 42e094ccc7b..ffd7d69bee0 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -6893,6 +6893,24 @@ impl NexusExternalApi for NexusExternalApiImpl { .await } + async fn update_status( + rqctx: RequestContext, + ) -> Result, HttpError> { + let apictx = rqctx.context(); + let handler = async { + let nexus = &apictx.context.nexus; + let opctx = + crate::context::op_context_for_external_api(&rqctx).await?; + let status = nexus.update_status_external(&opctx).await?; + Ok(HttpResponseOk(status)) + }; + apictx + .context + .external_latencies + .instrument_dropshot_handler(&rqctx, handler) + .await + } + // Silo users async fn user_list( diff --git a/nexus/tests/integration_tests/endpoints.rs b/nexus/tests/integration_tests/endpoints.rs index d8c62a95d94..d3580a5e4fe 100644 --- a/nexus/tests/integration_tests/endpoints.rs +++ b/nexus/tests/integration_tests/endpoints.rs @@ -2560,6 +2560,14 @@ pub static VERIFY_ENDPOINTS: LazyLock> = LazyLock::new( ), ], }, + VerifyEndpoint { + url: "/v1/system/update/status", + visibility: Visibility::Public, + unprivileged_access: UnprivilegedAccess::None, + allowed_methods: vec![ + AllowedMethod::Get, + ], + }, /* Metrics */ VerifyEndpoint { url: &DEMO_SYSTEM_METRICS_URL, diff --git a/nexus/tests/integration_tests/target_release.rs b/nexus/tests/integration_tests/target_release.rs index 68e6008c928..ea2eeb02164 100644 --- a/nexus/tests/integration_tests/target_release.rs +++ b/nexus/tests/integration_tests/target_release.rs @@ -37,7 +37,6 @@ async fn get_set_target_release() -> Result<()> { .unwrap() .parsed_body() .unwrap(); - assert_eq!(target_release.generation, 1); assert!(target_release.time_requested < Utc::now()); assert_eq!(target_release.release_source, TargetReleaseSource::Unspecified); @@ -71,9 +70,8 @@ async fn get_set_target_release() -> Result<()> { assert_eq!(system_version, response.recorded.repo.system_version); let target_release = - set_target_release(client, system_version.clone()).await?; + set_target_release(client, &system_version).await?; let after = Utc::now(); - assert_eq!(target_release.generation, 2); assert!(target_release.time_requested >= before); assert!(target_release.time_requested <= after); assert_eq!( @@ -103,9 +101,8 @@ async fn get_set_target_release() -> Result<()> { assert_eq!(system_version, response.recorded.repo.system_version); let target_release = - set_target_release(client, system_version.clone()).await?; + set_target_release(client, &system_version).await?; let after = Utc::now(); - assert_eq!(target_release.generation, 3); assert!(target_release.time_requested >= before); assert!(target_release.time_requested <= after); assert_eq!( @@ -116,7 +113,7 @@ async fn get_set_target_release() -> Result<()> { // Attempting to downgrade to an earlier system version (2.0.0 → 1.0.0) // should not be allowed. - set_target_release(client, Version::new(1, 0, 0)) + set_target_release(client, &Version::new(1, 0, 0)) .await .expect_err("shouldn't be able to downgrade system"); @@ -124,9 +121,9 @@ async fn get_set_target_release() -> Result<()> { Ok(()) } -async fn set_target_release( +pub async fn set_target_release( client: &ClientTestContext, - system_version: Version, + system_version: &Version, ) -> Result { NexusRequest::new( RequestBuilder::new( @@ -134,7 +131,9 @@ async fn set_target_release( Method::PUT, "/v1/system/update/target-release", ) - .body(Some(&SetTargetReleaseParams { system_version })) + .body(Some(&SetTargetReleaseParams { + system_version: system_version.clone(), + })) .expect_status(Some(StatusCode::CREATED)), ) .authn_as(AuthnMode::PrivilegedUser) diff --git a/nexus/tests/integration_tests/updates.rs b/nexus/tests/integration_tests/updates.rs index ef47345cfae..428b523e9ef 100644 --- a/nexus/tests/integration_tests/updates.rs +++ b/nexus/tests/integration_tests/updates.rs @@ -12,8 +12,10 @@ use nexus_db_queries::context::OpContext; use nexus_test_utils::background::run_tuf_artifact_replication_step; use nexus_test_utils::background::wait_tuf_artifact_replication_step; use nexus_test_utils::http_testing::{AuthnMode, NexusRequest, RequestBuilder}; +use nexus_test_utils::resource_helpers::object_get; use nexus_test_utils::test_setup; use nexus_test_utils_macros::nexus_test; +use nexus_types::external_api::views; use nexus_types::external_api::views::UpdatesTrustRoot; use omicron_common::api::external::{ TufRepoGetResponse, TufRepoInsertResponse, TufRepoInsertStatus, @@ -25,11 +27,14 @@ use std::collections::HashSet; use std::fmt::Debug; use tough::editor::signed::SignedRole; use tough::schema::Root; +use tufaceous_artifact::ArtifactVersion; use tufaceous_artifact::KnownArtifactKind; use tufaceous_lib::Key; use tufaceous_lib::assemble::{ArtifactManifest, OmicronRepoAssembler}; use tufaceous_lib::assemble::{DeserializedManifest, ManifestTweak}; +use crate::integration_tests::target_release::set_target_release; + const TRUST_ROOTS_URL: &str = "/v1/system/update/trust-roots"; type ControlPlaneTestContext = @@ -625,3 +630,89 @@ async fn test_trust_root_operations(cptestctx: &ControlPlaneTestContext) { .expect("failed to parse list after delete response"); assert!(response.items.is_empty()); } + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_update_status() -> Result<()> { + let cptestctx = + test_setup::("test_update_uninitialized", 0) + .await; + let client = &cptestctx.external_client; + let logctx = &cptestctx.logctx; + + // initial status + let status: views::UpdateStatus = + object_get(client, "/v1/system/update/status").await; + assert_eq!( + status.target_release.release_source, + views::TargetReleaseSource::Unspecified + ); + let counts = status.components_by_release_version; + assert_eq!(counts.get("install dataset").unwrap(), &7); + assert_eq!(counts.get("unknown").unwrap(), &15); + + // hold onto this to compare it to later values + let time_last_progress = status.time_last_progress; + + // Upload a fake TUF repo and set it as the target release + let trust_root = TestTrustRoot::generate().await?; + trust_root.to_upload_request(client, StatusCode::CREATED).execute().await?; + trust_root + .assemble_repo(&logctx.log, &[]) + .await? + .to_upload_request(client, StatusCode::OK) + .execute() + .await?; + let v1 = Version::new(1, 0, 0); + set_target_release(client, &v1).await?; + + let status: views::UpdateStatus = + object_get(client, "/v1/system/update/status").await; + assert_eq!( + status.target_release.release_source, + views::TargetReleaseSource::SystemVersion { version: v1 } + ); + + // blueprint time doesn't change + assert_eq!(time_last_progress, status.time_last_progress); + + let counts = status.components_by_release_version; + assert_eq!(counts.get("install dataset").unwrap(), &7); + assert_eq!(counts.get("unknown").unwrap(), &15); + + // do it again so there are two, so both versions are associated with tuf repos + let v2 = Version::new(2, 0, 0); + let tweaks = &[ + ManifestTweak::SystemVersion(v2.clone()), + ManifestTweak::ArtifactVersion { + kind: KnownArtifactKind::SwitchRotBootloader, + version: ArtifactVersion::new("non-semver-2").unwrap(), + }, + ]; + let trust_root = TestTrustRoot::generate().await?; + trust_root.to_upload_request(client, StatusCode::CREATED).execute().await?; + trust_root + .assemble_repo(&logctx.log, tweaks) + .await? + .to_upload_request(client, StatusCode::OK) + .execute() + .await?; + set_target_release(client, &v2).await?; + + let status: views::UpdateStatus = + object_get(client, "/v1/system/update/status").await; + + assert_eq!( + status.target_release.release_source, + views::TargetReleaseSource::SystemVersion { version: v2 } + ); + + // blueprint time doesn't change + assert_eq!(time_last_progress, status.time_last_progress); + + let counts = status.components_by_release_version; + assert_eq!(counts.get("install dataset").unwrap(), &7); + assert_eq!(counts.get("unknown").unwrap(), &15); + + cptestctx.teardown().await; + Ok(()) +} diff --git a/nexus/types/src/external_api/views.rs b/nexus/types/src/external_api/views.rs index 321b29bd12d..2fa2548f348 100644 --- a/nexus/types/src/external_api/views.rs +++ b/nexus/types/src/external_api/views.rs @@ -1549,9 +1549,6 @@ pub enum TargetReleaseSource { /// View of a system software target release. #[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize, JsonSchema)] pub struct TargetRelease { - /// The target-release generation number. - pub generation: i64, - /// The time it was set as the target release. pub time_requested: DateTime, @@ -1571,6 +1568,21 @@ pub struct UpdatesTrustRoot { pub root_role: TufSignedRootRole, } +#[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize, JsonSchema)] +pub struct UpdateStatus { + /// Current target release config + pub target_release: TargetRelease, + + /// Count of components running each release version + pub components_by_release_version: BTreeMap, + + /// Time of last meaningful change to update status + /// + /// Internally, this represents the last time a blueprint (proposed system + /// configuration) was made a target by the update system. + pub time_last_progress: DateTime, +} + fn expected_one_of() -> String { use std::fmt::Write; let mut msg = "expected one of:".to_string(); diff --git a/openapi/nexus.json b/openapi/nexus.json index 104f4239dbb..0145178eb8e 100644 --- a/openapi/nexus.json +++ b/openapi/nexus.json @@ -10992,6 +10992,32 @@ } } }, + "/v1/system/update/status": { + "get": { + "tags": [ + "system/update" + ], + "operationId": "update_status", + "responses": { + "200": { + "description": "successful operation", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/UpdateStatus" + } + } + } + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + } + } + }, "/v1/system/update/target-release": { "get": { "tags": [ @@ -25690,11 +25716,6 @@ "description": "View of a system software target release.", "type": "object", "properties": { - "generation": { - "description": "The target-release generation number.", - "type": "integer", - "format": "int64" - }, "release_source": { "description": "The source of the target release.", "allOf": [ @@ -25710,7 +25731,6 @@ } }, "required": [ - "generation", "release_source", "time_requested" ] @@ -26216,6 +26236,38 @@ } ] }, + "UpdateStatus": { + "type": "object", + "properties": { + "components_by_release_version": { + "description": "Count of components running each release version", + "type": "object", + "additionalProperties": { + "type": "integer", + "format": "uint", + "minimum": 0 + } + }, + "target_release": { + "description": "Current target release config", + "allOf": [ + { + "$ref": "#/components/schemas/TargetRelease" + } + ] + }, + "time_last_progress": { + "description": "Time of last meaningful change to update status\n\nInternally, this represents the last time a blueprint (proposed system configuration) was made a target by the update system.", + "type": "string", + "format": "date-time" + } + }, + "required": [ + "components_by_release_version", + "target_release", + "time_last_progress" + ] + }, "UpdatesTrustRoot": { "description": "Trusted root role used by the update system to verify update repositories.", "type": "object", From f3b99a2f5c200e804e29fc475b88ca5e412e8b02 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Tue, 23 Sep 2025 16:05:46 -0500 Subject: [PATCH 02/31] delete target_release_view, rework TargetRelease --- nexus/db-model/src/target_release.rs | 11 -- .../src/db/datastore/target_release.rs | 48 +------- nexus/db-queries/src/db/datastore/update.rs | 26 ++++- nexus/external-api/output/nexus_tags.txt | 3 +- nexus/external-api/src/lib.rs | 27 +---- nexus/src/app/update.rs | 17 ++- nexus/src/external_api/http_entrypoints.rs | 62 +++------- nexus/tests/integration_tests/endpoints.rs | 1 - .../tests/integration_tests/target_release.rs | 50 ++++----- nexus/tests/integration_tests/updates.rs | 15 +-- nexus/types/src/external_api/views.rs | 31 +++-- openapi/nexus.json | 106 +++--------------- 12 files changed, 114 insertions(+), 283 deletions(-) diff --git a/nexus/db-model/src/target_release.rs b/nexus/db-model/src/target_release.rs index d89cf069926..9dd767aaaa2 100644 --- a/nexus/db-model/src/target_release.rs +++ b/nexus/db-model/src/target_release.rs @@ -6,7 +6,6 @@ use super::{Generation, impl_enum_type}; use crate::typed_uuid::DbTypedUuid; use chrono::{DateTime, Utc}; use nexus_db_schema::schema::target_release; -use nexus_types::external_api::views; use omicron_uuid_kinds::TufRepoKind; impl_enum_type!( @@ -60,14 +59,4 @@ impl TargetRelease { tuf_repo_id: Some(tuf_repo_id), } } - - pub fn into_external( - &self, - release_source: views::TargetReleaseSource, - ) -> views::TargetRelease { - views::TargetRelease { - time_requested: self.time_requested, - release_source, - } - } } diff --git a/nexus/db-queries/src/db/datastore/target_release.rs b/nexus/db-queries/src/db/datastore/target_release.rs index 0a02eab1f17..04455b903d5 100644 --- a/nexus/db-queries/src/db/datastore/target_release.rs +++ b/nexus/db-queries/src/db/datastore/target_release.rs @@ -7,15 +7,12 @@ use super::DataStore; use crate::authz; use crate::context::OpContext; -use crate::db::model::{ - Generation, SemverVersion, TargetRelease, TargetReleaseSource, -}; +use crate::db::model::{Generation, TargetRelease}; use async_bb8_diesel::AsyncRunQueryDsl as _; use diesel::insert_into; use diesel::prelude::*; use nexus_db_errors::{ErrorHandler, public_error_from_diesel}; use nexus_db_schema::schema::target_release::dsl; -use nexus_types::external_api::views; use omicron_common::api::external::{CreateResult, Error, LookupResult}; impl DataStore { @@ -84,49 +81,6 @@ impl DataStore { .await .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server)) } - - /// Convert a model-level target release to an external view. - /// This method lives here because we have to look up the version - /// corresponding to the TUF repo. - pub async fn target_release_view( - &self, - opctx: &OpContext, - target_release: &TargetRelease, - ) -> LookupResult { - opctx - .authorize(authz::Action::Read, &authz::TARGET_RELEASE_CONFIG) - .await?; - let conn = self.pool_connection_authorized(opctx).await?; - let release_source = match target_release.release_source { - TargetReleaseSource::Unspecified => { - views::TargetReleaseSource::Unspecified - } - TargetReleaseSource::SystemVersion => { - use nexus_db_schema::schema::tuf_repo; - if let Some(tuf_repo_id) = target_release.tuf_repo_id { - views::TargetReleaseSource::SystemVersion { - version: tuf_repo::table - .select(tuf_repo::system_version) - .filter(tuf_repo::id.eq(tuf_repo_id)) - .first_async::(&*conn) - .await - .map_err(|e| { - public_error_from_diesel( - e, - ErrorHandler::Server, - ) - })? - .into(), - } - } else { - return Err(Error::internal_error( - "missing TUF repo ID for specified system version", - )); - } - } - }; - Ok(target_release.into_external(release_source)) - } } #[cfg(test)] diff --git a/nexus/db-queries/src/db/datastore/update.rs b/nexus/db-queries/src/db/datastore/update.rs index dabe70f5100..5d8d44e2ec3 100644 --- a/nexus/db-queries/src/db/datastore/update.rs +++ b/nexus/db-queries/src/db/datastore/update.rs @@ -18,8 +18,8 @@ use nexus_db_errors::OptionalError; use nexus_db_errors::{ErrorHandler, public_error_from_diesel}; use nexus_db_lookup::DbConnection; use nexus_db_model::{ - ArtifactHash, TufArtifact, TufRepo, TufRepoDescription, TufTrustRoot, - to_db_typed_uuid, + ArtifactHash, DbTypedUuid, TufArtifact, TufRepo, TufRepoDescription, + TufTrustRoot, to_db_typed_uuid, }; use omicron_common::api::external::{ self, CreateResult, DataPageParams, DeleteResult, Generation, @@ -175,6 +175,28 @@ impl DataStore { Ok(TufRepoDescription { repo, artifacts }) } + /// Given a TUF repo ID, get its version. We could use `tuf_repo_get_by_id`, + /// but that makes an additional query for the artifacts that we don't need + /// in the code that uses this method. + pub async fn tuf_repo_get_version( + &self, + opctx: &OpContext, + tuf_repo_id: &DbTypedUuid, + ) -> LookupResult { + opctx + .authorize(authz::Action::Read, &authz::TARGET_RELEASE_CONFIG) + .await?; + let conn = self.pool_connection_authorized(opctx).await?; + use nexus_db_schema::schema::tuf_repo; + tuf_repo::table + .select(tuf_repo::system_version) + .filter(tuf_repo::id.eq(tuf_repo_id.into_untyped_uuid())) + .first_async::(&*conn) + .await + .map(|v| v.0) + .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server)) + } + /// Returns the list of all TUF repo artifacts known to the system. pub async fn tuf_list_repos( &self, diff --git a/nexus/external-api/output/nexus_tags.txt b/nexus/external-api/output/nexus_tags.txt index 4d25bbcd1a3..496067dae17 100644 --- a/nexus/external-api/output/nexus_tags.txt +++ b/nexus/external-api/output/nexus_tags.txt @@ -298,13 +298,12 @@ API operations found with tag "system/update" OPERATION ID METHOD URL PATH system_update_get_repository GET /v1/system/update/repository/{system_version} system_update_put_repository PUT /v1/system/update/repository +system_update_status GET /v1/system/update/status system_update_trust_root_create POST /v1/system/update/trust-roots system_update_trust_root_delete DELETE /v1/system/update/trust-roots/{trust_root_id} system_update_trust_root_list GET /v1/system/update/trust-roots system_update_trust_root_view GET /v1/system/update/trust-roots/{trust_root_id} target_release_update PUT /v1/system/update/target-release -target_release_view GET /v1/system/update/target-release -update_status GET /v1/system/update/status API operations found with tag "tokens" OPERATION ID METHOD URL PATH diff --git a/nexus/external-api/src/lib.rs b/nexus/external-api/src/lib.rs index 77bcdf1b3f4..a5d2a5bfa0e 100644 --- a/nexus/external-api/src/lib.rs +++ b/nexus/external-api/src/lib.rs @@ -3041,27 +3041,12 @@ pub trait NexusExternalApi { path_params: Path, ) -> Result; - /// Get the current target release of the rack's system software - /// - /// This may not correspond to the actual software running on the rack - /// at the time of request; it is instead the release that the rack - /// reconfigurator should be moving towards as a goal state. After some - /// number of planning and execution phases, the software running on the - /// rack should eventually correspond to the release described here. - #[endpoint { - method = GET, - path = "/v1/system/update/target-release", - tags = ["system/update"], - }] - async fn target_release_view( - rqctx: RequestContext, - ) -> Result, HttpError>; - /// Set the current target release of the rack's system software /// - /// The rack reconfigurator will treat the software specified here as - /// a goal state for the rack's software, and attempt to asynchronously - /// update to that release. + /// The rack reconfigurator will treat the software specified here as a goal + /// state for the rack's software, and attempt to asynchronously update to + /// that release. Use the update status endpoint to view the current target + /// release. #[endpoint { method = PUT, path = "/v1/system/update/target-release", @@ -3070,14 +3055,14 @@ pub trait NexusExternalApi { async fn target_release_update( rqctx: RequestContext, params: TypedBody, - ) -> Result, HttpError>; + ) -> Result; #[endpoint { method = GET, path = "/v1/system/update/status", tags = ["system/update"], }] - async fn update_status( + async fn system_update_status( rqctx: RequestContext, ) -> Result, HttpError>; diff --git a/nexus/src/app/update.rs b/nexus/src/app/update.rs index 7c13a62fc41..2a250eabb79 100644 --- a/nexus/src/app/update.rs +++ b/nexus/src/app/update.rs @@ -166,10 +166,19 @@ impl super::Nexus { // it can still have an Unspecified release_source let db_target_release = self.datastore().target_release_get_current(opctx).await?; - let target_release = self - .datastore() - .target_release_view(opctx, &db_target_release) - .await?; + let target_release = match db_target_release.tuf_repo_id { + Some(tuf_repo_id) => { + let version = self + .datastore() + .tuf_repo_get_version(opctx, &tuf_repo_id) + .await?; + Some(views::TargetRelease { + time_requested: db_target_release.time_requested, + version, + }) + } + None => None, + }; let components_by_release_version = self.component_version_counts(opctx, &db_target_release).await?; diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index ffd7d69bee0..c816c55f97e 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -6796,34 +6796,10 @@ impl NexusExternalApi for NexusExternalApiImpl { .await } - async fn target_release_view( - rqctx: RequestContext, - ) -> Result, HttpError> { - let apictx = rqctx.context(); - let handler = async { - let nexus = &apictx.context.nexus; - let opctx = - crate::context::op_context_for_external_api(&rqctx).await?; - let target_release = - nexus.datastore().target_release_get_current(&opctx).await?; - Ok(HttpResponseOk( - nexus - .datastore() - .target_release_view(&opctx, &target_release) - .await?, - )) - }; - apictx - .context - .external_latencies - .instrument_dropshot_handler(&rqctx, handler) - .await - } - async fn target_release_update( rqctx: RequestContext, body: TypedBody, - ) -> Result, HttpError> { + ) -> Result { let apictx = rqctx.context(); let handler = async { let nexus = &apictx.context.nexus; @@ -6844,21 +6820,17 @@ impl NexusExternalApi for NexusExternalApiImpl { nexus.datastore().target_release_get_current(&opctx).await?; // Disallow downgrades. - if let views::TargetReleaseSource::SystemVersion { version } = nexus - .datastore() - .target_release_view(&opctx, ¤t_target_release) - .await? - .release_source - { + if let Some(tuf_repo_id) = current_target_release.tuf_repo_id { + let version = nexus + .datastore() + .tuf_repo_get_version(&opctx, &tuf_repo_id) + .await?; if version > system_version { - return Err(HttpError::for_bad_request( - None, - format!( - "The requested target system release ({system_version}) \ - is older than the current target system release ({version}). \ - This is not supported." - ), - )); + return Err(Error::invalid_request(format!( + "The requested target release ({system_version}) must \ + be newer than the current target release ({version})." + )) + .into()); } } @@ -6874,17 +6846,11 @@ impl NexusExternalApi for NexusExternalApiImpl { ¤t_target_release, tuf_repo_id, ); - let target_release = nexus + nexus .datastore() .target_release_insert(&opctx, next_target_release) .await?; - - Ok(HttpResponseCreated( - nexus - .datastore() - .target_release_view(&opctx, &target_release) - .await?, - )) + Ok(HttpResponseUpdatedNoContent()) }; apictx .context @@ -6893,7 +6859,7 @@ impl NexusExternalApi for NexusExternalApiImpl { .await } - async fn update_status( + async fn system_update_status( rqctx: RequestContext, ) -> Result, HttpError> { let apictx = rqctx.context(); diff --git a/nexus/tests/integration_tests/endpoints.rs b/nexus/tests/integration_tests/endpoints.rs index d3580a5e4fe..649207d3f98 100644 --- a/nexus/tests/integration_tests/endpoints.rs +++ b/nexus/tests/integration_tests/endpoints.rs @@ -2554,7 +2554,6 @@ pub static VERIFY_ENDPOINTS: LazyLock> = LazyLock::new( visibility: Visibility::Public, unprivileged_access: UnprivilegedAccess::None, allowed_methods: vec![ - AllowedMethod::Get, AllowedMethod::Put( serde_json::to_value(&*DEMO_TARGET_RELEASE).unwrap(), ), diff --git a/nexus/tests/integration_tests/target_release.rs b/nexus/tests/integration_tests/target_release.rs index ea2eeb02164..d5cc71ec1e5 100644 --- a/nexus/tests/integration_tests/target_release.rs +++ b/nexus/tests/integration_tests/target_release.rs @@ -11,9 +11,10 @@ use http::StatusCode; use http::method::Method; use nexus_test_utils::http_testing::AuthnMode; use nexus_test_utils::http_testing::{NexusRequest, RequestBuilder}; +use nexus_test_utils::resource_helpers::object_get; use nexus_test_utils::test_setup; use nexus_types::external_api::params::SetTargetReleaseParams; -use nexus_types::external_api::views::{TargetRelease, TargetReleaseSource}; +use nexus_types::external_api::views; use omicron_common::api::external::TufRepoInsertResponse; use semver::Version; use tufaceous_artifact::{ArtifactVersion, KnownArtifactKind}; @@ -28,17 +29,10 @@ async fn get_set_target_release() -> Result<()> { let client = &ctx.external_client; let logctx = &ctx.logctx; - // There should always be a target release. - let target_release: TargetRelease = - NexusRequest::object_get(client, "/v1/system/update/target-release") - .authn_as(AuthnMode::PrivilegedUser) - .execute() - .await - .unwrap() - .parsed_body() - .unwrap(); - assert!(target_release.time_requested < Utc::now()); - assert_eq!(target_release.release_source, TargetReleaseSource::Unspecified); + // There is no target release before one has ever been specified + let status: views::UpdateStatus = + object_get(client, "/v1/system/update/status").await; + assert_eq!(status.target_release, None); // Attempting to set an invalid system version should fail. let system_version = Version::new(0, 0, 0); @@ -69,15 +63,16 @@ async fn get_set_target_release() -> Result<()> { .parsed_body()?; assert_eq!(system_version, response.recorded.repo.system_version); - let target_release = - set_target_release(client, &system_version).await?; + set_target_release(client, &system_version).await?; + + let status: views::UpdateStatus = + object_get(client, "/v1/system/update/status").await; + + let target_release = status.target_release.unwrap(); let after = Utc::now(); assert!(target_release.time_requested >= before); assert!(target_release.time_requested <= after); - assert_eq!( - target_release.release_source, - TargetReleaseSource::SystemVersion { version: system_version }, - ); + assert_eq!(target_release.version, system_version); } // Adding a repo with non-semver artifact versions should be ok, too. @@ -100,15 +95,16 @@ async fn get_set_target_release() -> Result<()> { .parsed_body()?; assert_eq!(system_version, response.recorded.repo.system_version); - let target_release = - set_target_release(client, &system_version).await?; + set_target_release(client, &system_version).await?; + + let status: views::UpdateStatus = + object_get(client, "/v1/system/update/status").await; + + let target_release = status.target_release.unwrap(); let after = Utc::now(); assert!(target_release.time_requested >= before); assert!(target_release.time_requested <= after); - assert_eq!( - target_release.release_source, - TargetReleaseSource::SystemVersion { version: system_version }, - ); + assert_eq!(target_release.version, system_version); } // Attempting to downgrade to an earlier system version (2.0.0 → 1.0.0) @@ -124,7 +120,7 @@ async fn get_set_target_release() -> Result<()> { pub async fn set_target_release( client: &ClientTestContext, system_version: &Version, -) -> Result { +) -> Result<(), anyhow::Error> { NexusRequest::new( RequestBuilder::new( client, @@ -134,10 +130,10 @@ pub async fn set_target_release( .body(Some(&SetTargetReleaseParams { system_version: system_version.clone(), })) - .expect_status(Some(StatusCode::CREATED)), + .expect_status(Some(StatusCode::NO_CONTENT)), ) .authn_as(AuthnMode::PrivilegedUser) .execute() .await - .map(|response| response.parsed_body().unwrap()) + .map(|_| ()) } diff --git a/nexus/tests/integration_tests/updates.rs b/nexus/tests/integration_tests/updates.rs index 428b523e9ef..053a0e36633 100644 --- a/nexus/tests/integration_tests/updates.rs +++ b/nexus/tests/integration_tests/updates.rs @@ -642,10 +642,7 @@ async fn test_update_status() -> Result<()> { // initial status let status: views::UpdateStatus = object_get(client, "/v1/system/update/status").await; - assert_eq!( - status.target_release.release_source, - views::TargetReleaseSource::Unspecified - ); + assert_eq!(status.target_release, None); let counts = status.components_by_release_version; assert_eq!(counts.get("install dataset").unwrap(), &7); assert_eq!(counts.get("unknown").unwrap(), &15); @@ -667,10 +664,7 @@ async fn test_update_status() -> Result<()> { let status: views::UpdateStatus = object_get(client, "/v1/system/update/status").await; - assert_eq!( - status.target_release.release_source, - views::TargetReleaseSource::SystemVersion { version: v1 } - ); + assert_eq!(status.target_release.unwrap().version, v1); // blueprint time doesn't change assert_eq!(time_last_progress, status.time_last_progress); @@ -701,10 +695,7 @@ async fn test_update_status() -> Result<()> { let status: views::UpdateStatus = object_get(client, "/v1/system/update/status").await; - assert_eq!( - status.target_release.release_source, - views::TargetReleaseSource::SystemVersion { version: v2 } - ); + assert_eq!(status.target_release.unwrap().version, v2); // blueprint time doesn't change assert_eq!(time_last_progress, status.time_last_progress); diff --git a/nexus/types/src/external_api/views.rs b/nexus/types/src/external_api/views.rs index 2fa2548f348..000c99487cb 100644 --- a/nexus/types/src/external_api/views.rs +++ b/nexus/types/src/external_api/views.rs @@ -1535,25 +1535,14 @@ pub struct AlertProbeResult { // UPDATE -/// Source of a system software target release. -#[derive(Clone, Debug, Deserialize, Eq, PartialEq, JsonSchema, Serialize)] -#[serde(tag = "type", rename_all = "snake_case")] -pub enum TargetReleaseSource { - /// Unspecified or unknown source (probably MUPdate). - Unspecified, - - /// The specified release of the rack's system software. - SystemVersion { version: Version }, -} - -/// View of a system software target release. +/// View of a system software target release #[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize, JsonSchema)] pub struct TargetRelease { - /// The time it was set as the target release. + /// Time this was set as the target release pub time_requested: DateTime, - /// The source of the target release. - pub release_source: TargetReleaseSource, + /// The specified release of the rack's system software + pub version: Version, } /// Trusted root role used by the update system to verify update repositories. @@ -1570,8 +1559,16 @@ pub struct UpdatesTrustRoot { #[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize, JsonSchema)] pub struct UpdateStatus { - /// Current target release config - pub target_release: TargetRelease, + /// Current target release of the rack's system software + /// + /// This may not correspond to the actual software running on the rack + /// at the time of request; it is instead the release that the rack + /// reconfigurator should be moving towards as a goal state. After some + /// number of planning and execution phases, the software running on the + /// rack should eventually correspond to the release described here. + /// + /// Will only be null if a target release has never been set. + pub target_release: Option, /// Count of components running each release version pub components_by_release_version: BTreeMap, diff --git a/openapi/nexus.json b/openapi/nexus.json index 0145178eb8e..1d09b00a9dc 100644 --- a/openapi/nexus.json +++ b/openapi/nexus.json @@ -10997,7 +10997,7 @@ "tags": [ "system/update" ], - "operationId": "update_status", + "operationId": "system_update_status", "responses": { "200": { "description": "successful operation", @@ -11019,38 +11019,12 @@ } }, "/v1/system/update/target-release": { - "get": { - "tags": [ - "system/update" - ], - "summary": "Get the current target release of the rack's system software", - "description": "This may not correspond to the actual software running on the rack at the time of request; it is instead the release that the rack reconfigurator should be moving towards as a goal state. After some number of planning and execution phases, the software running on the rack should eventually correspond to the release described here.", - "operationId": "target_release_view", - "responses": { - "200": { - "description": "successful operation", - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/TargetRelease" - } - } - } - }, - "4XX": { - "$ref": "#/components/responses/Error" - }, - "5XX": { - "$ref": "#/components/responses/Error" - } - } - }, "put": { "tags": [ "system/update" ], "summary": "Set the current target release of the rack's system software", - "description": "The rack reconfigurator will treat the software specified here as a goal state for the rack's software, and attempt to asynchronously update to that release.", + "description": "The rack reconfigurator will treat the software specified here as a goal state for the rack's software, and attempt to asynchronously update to that release. Use the update status endpoint to view the current target release.", "operationId": "target_release_update", "requestBody": { "content": { @@ -11063,15 +11037,8 @@ "required": true }, "responses": { - "201": { - "description": "successful creation", - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/TargetRelease" - } - } - } + "204": { + "description": "resource updated" }, "4XX": { "$ref": "#/components/responses/Error" @@ -25713,66 +25680,23 @@ ] }, "TargetRelease": { - "description": "View of a system software target release.", + "description": "View of a system software target release", "type": "object", "properties": { - "release_source": { - "description": "The source of the target release.", - "allOf": [ - { - "$ref": "#/components/schemas/TargetReleaseSource" - } - ] - }, "time_requested": { - "description": "The time it was set as the target release.", + "description": "Time this was set as the target release", "type": "string", "format": "date-time" + }, + "version": { + "description": "The specified release of the rack's system software", + "type": "string", + "pattern": "^(0|[1-9]\\d*)\\.(0|[1-9]\\d*)\\.(0|[1-9]\\d*)(?:-((?:0|[1-9]\\d*|\\d*[a-zA-Z-][0-9a-zA-Z-]*)(?:\\.(?:0|[1-9]\\d*|\\d*[a-zA-Z-][0-9a-zA-Z-]*))*))?(?:\\+([0-9a-zA-Z-]+(?:\\.[0-9a-zA-Z-]+)*))?$" } }, "required": [ - "release_source", - "time_requested" - ] - }, - "TargetReleaseSource": { - "description": "Source of a system software target release.", - "oneOf": [ - { - "description": "Unspecified or unknown source (probably MUPdate).", - "type": "object", - "properties": { - "type": { - "type": "string", - "enum": [ - "unspecified" - ] - } - }, - "required": [ - "type" - ] - }, - { - "description": "The specified release of the rack's system software.", - "type": "object", - "properties": { - "type": { - "type": "string", - "enum": [ - "system_version" - ] - }, - "version": { - "type": "string", - "pattern": "^(0|[1-9]\\d*)\\.(0|[1-9]\\d*)\\.(0|[1-9]\\d*)(?:-((?:0|[1-9]\\d*|\\d*[a-zA-Z-][0-9a-zA-Z-]*)(?:\\.(?:0|[1-9]\\d*|\\d*[a-zA-Z-][0-9a-zA-Z-]*))*))?(?:\\+([0-9a-zA-Z-]+(?:\\.[0-9a-zA-Z-]+)*))?$" - } - }, - "required": [ - "type", - "version" - ] - } + "time_requested", + "version" ] }, "Timeseries": { @@ -26249,7 +26173,8 @@ } }, "target_release": { - "description": "Current target release config", + "nullable": true, + "description": "Current target release of the rack's system software\n\nThis may not correspond to the actual software running on the rack at the time of request; it is instead the release that the rack reconfigurator should be moving towards as a goal state. After some number of planning and execution phases, the software running on the rack should eventually correspond to the release described here.\n\nWill only be null if a target release has never been set.", "allOf": [ { "$ref": "#/components/schemas/TargetRelease" @@ -26264,7 +26189,6 @@ }, "required": [ "components_by_release_version", - "target_release", "time_last_progress" ] }, From 37ba90fd422e3497016586c3ce3ac558bbb12836 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Wed, 24 Sep 2025 18:03:03 -0500 Subject: [PATCH 03/31] avoid _ in match, avoid future surprises --- nexus/src/app/update.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nexus/src/app/update.rs b/nexus/src/app/update.rs index 2a250eabb79..0b8b4f4dc27 100644 --- a/nexus/src/app/update.rs +++ b/nexus/src/app/update.rs @@ -307,7 +307,7 @@ impl super::Nexus { M2Slot::A => slot_a_version.clone(), M2Slot::B => slot_b_version.clone(), }), - _ => None, + internal_views::HostPhase1Status::NotASled => None, }; std::iter::once(sp_version) From 1f828893612f74811f1c6cf8cce78678253e1e19 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Wed, 24 Sep 2025 22:00:13 -0500 Subject: [PATCH 04/31] Option -> Nullable, helpful doc comment --- common/src/api/external/mod.rs | 2 +- nexus/src/app/update.rs | 3 ++- nexus/tests/integration_tests/target_release.rs | 6 +++--- nexus/tests/integration_tests/updates.rs | 6 +++--- nexus/types/src/external_api/views.rs | 9 +++++---- openapi/nexus.json | 3 ++- 6 files changed, 16 insertions(+), 13 deletions(-) diff --git a/common/src/api/external/mod.rs b/common/src/api/external/mod.rs index 64b5d310b84..ce6ff9f3746 100644 --- a/common/src/api/external/mod.rs +++ b/common/src/api/external/mod.rs @@ -3566,7 +3566,7 @@ pub enum ImportExportPolicy { /// will fail to parse if the key is not present. The JSON Schema in the /// OpenAPI definition will also reflect that the field is required. See /// . -#[derive(Clone, Debug, Serialize)] +#[derive(Clone, Debug, Serialize, PartialEq, Eq)] pub struct Nullable(pub Option); impl From> for Nullable { diff --git a/nexus/src/app/update.rs b/nexus/src/app/update.rs index 0b8b4f4dc27..c4b8141a1c4 100644 --- a/nexus/src/app/update.rs +++ b/nexus/src/app/update.rs @@ -18,6 +18,7 @@ use nexus_types::external_api::views; use nexus_types::internal_api::views as internal_views; use nexus_types::inventory::RotSlot; use omicron_common::api::external::InternalContext; +use omicron_common::api::external::Nullable; use omicron_common::api::external::{ DataPageParams, Error, TufRepoInsertResponse, TufRepoInsertStatus, }; @@ -190,7 +191,7 @@ impl super::Nexus { .time_made_target; Ok(views::UpdateStatus { - target_release, + target_release: Nullable(target_release), components_by_release_version, time_last_progress, }) diff --git a/nexus/tests/integration_tests/target_release.rs b/nexus/tests/integration_tests/target_release.rs index d5cc71ec1e5..813d779cd2a 100644 --- a/nexus/tests/integration_tests/target_release.rs +++ b/nexus/tests/integration_tests/target_release.rs @@ -32,7 +32,7 @@ async fn get_set_target_release() -> Result<()> { // There is no target release before one has ever been specified let status: views::UpdateStatus = object_get(client, "/v1/system/update/status").await; - assert_eq!(status.target_release, None); + assert_eq!(status.target_release.0, None); // Attempting to set an invalid system version should fail. let system_version = Version::new(0, 0, 0); @@ -68,7 +68,7 @@ async fn get_set_target_release() -> Result<()> { let status: views::UpdateStatus = object_get(client, "/v1/system/update/status").await; - let target_release = status.target_release.unwrap(); + let target_release = status.target_release.0.unwrap(); let after = Utc::now(); assert!(target_release.time_requested >= before); assert!(target_release.time_requested <= after); @@ -100,7 +100,7 @@ async fn get_set_target_release() -> Result<()> { let status: views::UpdateStatus = object_get(client, "/v1/system/update/status").await; - let target_release = status.target_release.unwrap(); + let target_release = status.target_release.0.unwrap(); let after = Utc::now(); assert!(target_release.time_requested >= before); assert!(target_release.time_requested <= after); diff --git a/nexus/tests/integration_tests/updates.rs b/nexus/tests/integration_tests/updates.rs index 053a0e36633..b9f60fe8201 100644 --- a/nexus/tests/integration_tests/updates.rs +++ b/nexus/tests/integration_tests/updates.rs @@ -642,7 +642,7 @@ async fn test_update_status() -> Result<()> { // initial status let status: views::UpdateStatus = object_get(client, "/v1/system/update/status").await; - assert_eq!(status.target_release, None); + assert_eq!(status.target_release.0, None); let counts = status.components_by_release_version; assert_eq!(counts.get("install dataset").unwrap(), &7); assert_eq!(counts.get("unknown").unwrap(), &15); @@ -664,7 +664,7 @@ async fn test_update_status() -> Result<()> { let status: views::UpdateStatus = object_get(client, "/v1/system/update/status").await; - assert_eq!(status.target_release.unwrap().version, v1); + assert_eq!(status.target_release.0.unwrap().version, v1); // blueprint time doesn't change assert_eq!(time_last_progress, status.time_last_progress); @@ -695,7 +695,7 @@ async fn test_update_status() -> Result<()> { let status: views::UpdateStatus = object_get(client, "/v1/system/update/status").await; - assert_eq!(status.target_release.unwrap().version, v2); + assert_eq!(status.target_release.0.unwrap().version, v2); // blueprint time doesn't change assert_eq!(time_last_progress, status.time_last_progress); diff --git a/nexus/types/src/external_api/views.rs b/nexus/types/src/external_api/views.rs index 000c99487cb..78cbeca211e 100644 --- a/nexus/types/src/external_api/views.rs +++ b/nexus/types/src/external_api/views.rs @@ -17,7 +17,7 @@ pub use omicron_common::api::external::IpVersion; use omicron_common::api::external::{ AffinityPolicy, AllowedSourceIps as ExternalAllowedSourceIps, ByteCount, Digest, Error, FailureDomain, IdentityMetadata, InstanceState, Name, - ObjectIdentity, SimpleIdentity, SimpleIdentityOrName, + Nullable, ObjectIdentity, SimpleIdentity, SimpleIdentityOrName, }; use omicron_uuid_kinds::*; use oxnet::{Ipv4Net, Ipv6Net}; @@ -1557,7 +1557,7 @@ pub struct UpdatesTrustRoot { pub root_role: TufSignedRootRole, } -#[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize, JsonSchema)] +#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] pub struct UpdateStatus { /// Current target release of the rack's system software /// @@ -1568,7 +1568,7 @@ pub struct UpdateStatus { /// rack should eventually correspond to the release described here. /// /// Will only be null if a target release has never been set. - pub target_release: Option, + pub target_release: Nullable, /// Count of components running each release version pub components_by_release_version: BTreeMap, @@ -1576,7 +1576,8 @@ pub struct UpdateStatus { /// Time of last meaningful change to update status /// /// Internally, this represents the last time a blueprint (proposed system - /// configuration) was made a target by the update system. + /// configuration) was made a target by the update system. In other words, + /// it's the last time the system decided on a next step to take. pub time_last_progress: DateTime, } diff --git a/openapi/nexus.json b/openapi/nexus.json index 1d09b00a9dc..aac3b155a47 100644 --- a/openapi/nexus.json +++ b/openapi/nexus.json @@ -26182,13 +26182,14 @@ ] }, "time_last_progress": { - "description": "Time of last meaningful change to update status\n\nInternally, this represents the last time a blueprint (proposed system configuration) was made a target by the update system.", + "description": "Time of last meaningful change to update status\n\nInternally, this represents the last time a blueprint (proposed system configuration) was made a target by the update system. In other words, it's the last time the system decided on a next step to take.", "type": "string", "format": "date-time" } }, "required": [ "components_by_release_version", + "target_release", "time_last_progress" ] }, From 3ca18a7f3969db1917e1f07296d6dc03de877c5f Mon Sep 17 00:00:00 2001 From: David Crespo Date: Thu, 2 Oct 2025 14:27:17 -0500 Subject: [PATCH 05/31] address dave's comments: docs, add paused field --- nexus/external-api/src/lib.rs | 13 +++++++--- nexus/src/app/update.rs | 23 +++++++++-------- nexus/tests/integration_tests/updates.rs | 12 ++++++--- nexus/types/src/external_api/views.rs | 33 +++++++++++++++--------- openapi/nexus.json | 19 +++++++++----- 5 files changed, 64 insertions(+), 36 deletions(-) diff --git a/nexus/external-api/src/lib.rs b/nexus/external-api/src/lib.rs index e55a5dc3fad..be9a2b28027 100644 --- a/nexus/external-api/src/lib.rs +++ b/nexus/external-api/src/lib.rs @@ -3041,11 +3041,12 @@ pub trait NexusExternalApi { path_params: Path, ) -> Result; - /// Set the current target release of the rack's system software + /// Set target release /// - /// The rack reconfigurator will treat the software specified here as a goal - /// state for the rack's software, and attempt to asynchronously update to - /// that release. Use the update status endpoint to view the current target + /// Set the current target release of the rack's system software. The rack + /// reconfigurator will treat the software specified here as a goal state + /// for the rack's software, and attempt to asynchronously update to that + /// release. Use the update status endpoint to view the current target /// release. #[endpoint { method = PUT, @@ -3057,6 +3058,10 @@ pub trait NexusExternalApi { params: TypedBody, ) -> Result; + /// Fetch system update status + /// + /// Returns information about the current target release and the + /// progress of system software updates. #[endpoint { method = GET, path = "/v1/system/update/status", diff --git a/nexus/src/app/update.rs b/nexus/src/app/update.rs index c4b8141a1c4..6c21656a424 100644 --- a/nexus/src/app/update.rs +++ b/nexus/src/app/update.rs @@ -163,8 +163,6 @@ impl super::Nexus { &self, opctx: &OpContext, ) -> Result { - // ? because we expect there to always be a current target release. but - // it can still have an Unspecified release_source let db_target_release = self.datastore().target_release_get_current(opctx).await?; let target_release = match db_target_release.tuf_repo_id { @@ -184,16 +182,21 @@ impl super::Nexus { let components_by_release_version = self.component_version_counts(opctx, &db_target_release).await?; - let time_last_progress = self - .datastore() - .blueprint_target_get_current(opctx) - .await? - .time_made_target; + let (blueprint_target, blueprint) = + self.datastore().blueprint_target_get_current_full(opctx).await?; + + let time_last_blueprint = blueprint_target.time_made_target; + + // Update activity is paused if the current target release generation + // is less than or equal to the blueprint's minimum generation + let paused = *db_target_release.generation + <= blueprint.target_release_minimum_generation; Ok(views::UpdateStatus { target_release: Nullable(target_release), components_by_release_version, - time_last_progress, + time_last_blueprint, + paused, }) } @@ -203,12 +206,10 @@ impl super::Nexus { opctx: &OpContext, target_release: &nexus_db_model::TargetRelease, ) -> Result, Error> { - // Get the latest inventory collection let Some(inventory) = self.datastore().inventory_get_latest_collection(opctx).await? else { - // No inventory collection available, return empty counts - return Ok(BTreeMap::new()); + return Err(Error::internal_error("No inventory collection found")); }; // Build current TargetReleaseDescription, defaulting to Initial if diff --git a/nexus/tests/integration_tests/updates.rs b/nexus/tests/integration_tests/updates.rs index 7b13839639d..bc012ae7baf 100644 --- a/nexus/tests/integration_tests/updates.rs +++ b/nexus/tests/integration_tests/updates.rs @@ -716,12 +716,16 @@ async fn test_update_status() -> Result<()> { let status: views::UpdateStatus = object_get(client, "/v1/system/update/status").await; assert_eq!(status.target_release.0, None); + assert!( + status.paused, + "should be paused initially when no target release is set" + ); let counts = status.components_by_release_version; assert_eq!(counts.get("install dataset").unwrap(), &7); assert_eq!(counts.get("unknown").unwrap(), &15); // hold onto this to compare it to later values - let time_last_progress = status.time_last_progress; + let time_last_blueprint = status.time_last_blueprint; // Upload a fake TUF repo and set it as the target release let trust_root = TestTrustRoot::generate().await?; @@ -738,9 +742,10 @@ async fn test_update_status() -> Result<()> { let status: views::UpdateStatus = object_get(client, "/v1/system/update/status").await; assert_eq!(status.target_release.0.unwrap().version, v1); + assert!(!status.paused, "should not be paused after setting v1"); // blueprint time doesn't change - assert_eq!(time_last_progress, status.time_last_progress); + assert_eq!(time_last_blueprint, status.time_last_blueprint); let counts = status.components_by_release_version; assert_eq!(counts.get("install dataset").unwrap(), &7); @@ -769,9 +774,10 @@ async fn test_update_status() -> Result<()> { object_get(client, "/v1/system/update/status").await; assert_eq!(status.target_release.0.unwrap().version, v2); + assert!(!status.paused, "should not be paused after setting v2"); // blueprint time doesn't change - assert_eq!(time_last_progress, status.time_last_progress); + assert_eq!(time_last_blueprint, status.time_last_blueprint); let counts = status.components_by_release_version; assert_eq!(counts.get("install dataset").unwrap(), &7); diff --git a/nexus/types/src/external_api/views.rs b/nexus/types/src/external_api/views.rs index 78cbeca211e..647e0d34ead 100644 --- a/nexus/types/src/external_api/views.rs +++ b/nexus/types/src/external_api/views.rs @@ -1559,26 +1559,35 @@ pub struct UpdatesTrustRoot { #[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] pub struct UpdateStatus { - /// Current target release of the rack's system software + /// Current target release of the system software /// - /// This may not correspond to the actual software running on the rack - /// at the time of request; it is instead the release that the rack - /// reconfigurator should be moving towards as a goal state. After some - /// number of planning and execution phases, the software running on the - /// rack should eventually correspond to the release described here. + /// This may not correspond to the actual system software running + /// at the time of request; it is instead the release that the system + /// should be moving towards as a goal state. The system asynchronously + /// updates software to match this target release. /// - /// Will only be null if a target release has never been set. + /// Will only be null if a target release has never been set. In that case, + /// the system is not automatically attempting to manage software versions. pub target_release: Nullable, /// Count of components running each release version pub components_by_release_version: BTreeMap, - /// Time of last meaningful change to update status + /// Time of most recent update planning activity /// - /// Internally, this represents the last time a blueprint (proposed system - /// configuration) was made a target by the update system. In other words, - /// it's the last time the system decided on a next step to take. - pub time_last_progress: DateTime, + /// This is intended as a rough indicator of the last time something + /// happened in the update planner. A blueprint is the update system's plan + /// for the next state of the system, so this timestamp indicates the last + /// time the update system made a plan. + pub time_last_blueprint: DateTime, + + /// Whether update activity is paused + /// + /// When true, the system has stopped attempting to make progress toward the + /// target release. This happens after a MUPdate because the system wants to + /// make sure of the operator's intent. To resume update, set a new target + /// release. + pub paused: bool, } fn expected_one_of() -> String { diff --git a/openapi/nexus.json b/openapi/nexus.json index 36a3975292b..c421334efda 100644 --- a/openapi/nexus.json +++ b/openapi/nexus.json @@ -10997,6 +10997,8 @@ "tags": [ "system/update" ], + "summary": "Fetch system update status", + "description": "Returns information about the current target release and the progress of system software updates.", "operationId": "system_update_status", "responses": { "200": { @@ -11023,8 +11025,8 @@ "tags": [ "system/update" ], - "summary": "Set the current target release of the rack's system software", - "description": "The rack reconfigurator will treat the software specified here as a goal state for the rack's software, and attempt to asynchronously update to that release. Use the update status endpoint to view the current target release.", + "summary": "Set target release", + "description": "Set the current target release of the rack's system software. The rack reconfigurator will treat the software specified here as a goal state for the rack's software, and attempt to asynchronously update to that release. Use the update status endpoint to view the current target release.", "operationId": "target_release_update", "requestBody": { "content": { @@ -26172,25 +26174,30 @@ "minimum": 0 } }, + "paused": { + "description": "Whether update activity is paused\n\nWhen true, the system has stopped attempting to make progress toward the target release. This happens after a MUPdate because the system wants to make sure of the operator's intent. To resume update, set a new target release.", + "type": "boolean" + }, "target_release": { "nullable": true, - "description": "Current target release of the rack's system software\n\nThis may not correspond to the actual software running on the rack at the time of request; it is instead the release that the rack reconfigurator should be moving towards as a goal state. After some number of planning and execution phases, the software running on the rack should eventually correspond to the release described here.\n\nWill only be null if a target release has never been set.", + "description": "Current target release of the system software\n\nThis may not correspond to the actual system software running at the time of request; it is instead the release that the system should be moving towards as a goal state. The system asynchronously updates software to match this target release.\n\nWill only be null if a target release has never been set. In that case, the system is not automatically attempting to manage software versions.", "allOf": [ { "$ref": "#/components/schemas/TargetRelease" } ] }, - "time_last_progress": { - "description": "Time of last meaningful change to update status\n\nInternally, this represents the last time a blueprint (proposed system configuration) was made a target by the update system. In other words, it's the last time the system decided on a next step to take.", + "time_last_blueprint": { + "description": "Time of most recent update planning activity\n\nThis is intended as a rough indicator of the last time something happened in the update planner. A blueprint is the update system's plan for the next state of the system, so this timestamp indicates the last time the update system made a plan.", "type": "string", "format": "date-time" } }, "required": [ "components_by_release_version", + "paused", "target_release", - "time_last_progress" + "time_last_blueprint" ] }, "UpdatesTrustRoot": { From 321374dcafcf25a87be43573d764d3352bcb4be3 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Thu, 2 Oct 2025 15:30:35 -0500 Subject: [PATCH 06/31] get current target blueprint from watch channel --- nexus/src/app/quiesce.rs | 6 ++++++ nexus/src/app/update.rs | 10 ++++++++-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/nexus/src/app/quiesce.rs b/nexus/src/app/quiesce.rs index adffc9ab7a6..7efca01c1fb 100644 --- a/nexus/src/app/quiesce.rs +++ b/nexus/src/app/quiesce.rs @@ -92,6 +92,12 @@ impl NexusQuiesceHandle { self.state.borrow().clone() } + pub fn latest_blueprint( + &self, + ) -> Option> { + self.latest_blueprint.borrow().clone() + } + pub fn set_quiescing(&self, quiescing: bool) { let new_state = if quiescing { let time_requested = Utc::now(); diff --git a/nexus/src/app/update.rs b/nexus/src/app/update.rs index 6c21656a424..5d81fc88cb2 100644 --- a/nexus/src/app/update.rs +++ b/nexus/src/app/update.rs @@ -182,8 +182,14 @@ impl super::Nexus { let components_by_release_version = self.component_version_counts(opctx, &db_target_release).await?; - let (blueprint_target, blueprint) = - self.datastore().blueprint_target_get_current_full(opctx).await?; + let (blueprint_target, blueprint) = self + .quiesce + .latest_blueprint() + .ok_or_else(|| { + Error::internal_error("Tried to get update status before target blueprint is loaded") + })? + .as_ref() + .clone(); let time_last_blueprint = blueprint_target.time_made_target; From a2d6054717b600c4fa21a7232c29a01a72d1825c Mon Sep 17 00:00:00 2001 From: David Crespo Date: Thu, 2 Oct 2025 16:16:54 -0500 Subject: [PATCH 07/31] avoid redundant tuf repo retrieval --- nexus/src/app/update.rs | 45 +++++++++++++++++++++++------------------ 1 file changed, 25 insertions(+), 20 deletions(-) diff --git a/nexus/src/app/update.rs b/nexus/src/app/update.rs index 5d81fc88cb2..f65a99f5105 100644 --- a/nexus/src/app/update.rs +++ b/nexus/src/app/update.rs @@ -165,22 +165,29 @@ impl super::Nexus { ) -> Result { let db_target_release = self.datastore().target_release_get_current(opctx).await?; - let target_release = match db_target_release.tuf_repo_id { - Some(tuf_repo_id) => { - let version = self - .datastore() - .tuf_repo_get_version(opctx, &tuf_repo_id) - .await?; - Some(views::TargetRelease { - time_requested: db_target_release.time_requested, - version, - }) - } + + let current_tuf_repo = match db_target_release.tuf_repo_id { + Some(tuf_repo_id) => Some( + self.datastore() + .tuf_repo_get_by_id(opctx, tuf_repo_id.into()) + .await?, + ), None => None, }; - let components_by_release_version = - self.component_version_counts(opctx, &db_target_release).await?; + let target_release = + current_tuf_repo.as_ref().map(|repo| views::TargetRelease { + time_requested: db_target_release.time_requested, + version: repo.repo.system_version.0.clone(), + }); + + let components_by_release_version = self + .component_version_counts( + opctx, + &db_target_release, + current_tuf_repo, + ) + .await?; let (blueprint_target, blueprint) = self .quiesce @@ -211,6 +218,7 @@ impl super::Nexus { &self, opctx: &OpContext, target_release: &nexus_db_model::TargetRelease, + current_tuf_repo: Option, ) -> Result, Error> { let Some(inventory) = self.datastore().inventory_get_latest_collection(opctx).await? @@ -222,13 +230,10 @@ impl super::Nexus { // there is no tuf repo ID which, based on DB constraints, happens if // and only if target_release_source is 'unspecified', which should only // happen in the initial state before any target release has been set - let curr_target_desc = match target_release.tuf_repo_id { - Some(id) => TargetReleaseDescription::TufRepo( - self.datastore() - .tuf_repo_get_by_id(opctx, id.into()) - .await? - .into_external(), - ), + let curr_target_desc = match current_tuf_repo { + Some(repo) => { + TargetReleaseDescription::TufRepo(repo.into_external()) + } None => TargetReleaseDescription::Initial, }; From 5ab24ecbcbf280da511e26b0acb76ef99457b2ba Mon Sep 17 00:00:00 2001 From: David Crespo Date: Thu, 25 Sep 2025 10:01:18 -0500 Subject: [PATCH 08/31] tuf repo list endpoint --- nexus/db-queries/src/db/datastore/update.rs | 40 ++++++++++ nexus/external-api/output/nexus_tags.txt | 1 + nexus/external-api/src/lib.rs | 14 ++++ nexus/src/app/update.rs | 12 +++ nexus/src/external_api/http_entrypoints.rs | 59 +++++++++++++++ openapi/nexus.json | 81 +++++++++++++++++++++ 6 files changed, 207 insertions(+) diff --git a/nexus/db-queries/src/db/datastore/update.rs b/nexus/db-queries/src/db/datastore/update.rs index b22d013ccdd..78513dfafad 100644 --- a/nexus/db-queries/src/db/datastore/update.rs +++ b/nexus/db-queries/src/db/datastore/update.rs @@ -6,6 +6,8 @@ use std::collections::HashMap; +use chrono::{DateTime, Utc}; + use super::DataStore; use crate::authz; use crate::context::OpContext; @@ -14,6 +16,7 @@ use crate::db::datastore::target_release::RecentTargetReleases; use crate::db::model::SemverVersion; use crate::db::pagination::Paginator; use crate::db::pagination::paginated; +use crate::db::pagination::paginated_multicolumn; use async_bb8_diesel::AsyncRunQueryDsl; use diesel::prelude::*; use diesel::result::Error as DieselError; @@ -460,6 +463,43 @@ impl DataStore { .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server)) } + /// List all TUF repositories ordered by creation time (descending). + pub async fn tuf_repo_list( + &self, + opctx: &OpContext, + pagparams: &DataPageParams<'_, (DateTime, Uuid)>, + ) -> ListResultVec { + opctx.authorize(authz::Action::Read, &authz::FLEET).await?; + + use nexus_db_schema::schema::tuf_repo::dsl; + + let conn = self.pool_connection_authorized(opctx).await?; + + // Order by time_created descending (newest first), then by id for stable pagination + let repos = paginated_multicolumn( + dsl::tuf_repo, + (dsl::time_created, dsl::id), + pagparams, + ) + .order_by((dsl::time_created.desc(), dsl::id)) + .select(TufRepo::as_select()) + .load_async(&*conn) + .await + .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server))?; + + // For each repo, fetch its artifacts + let mut results = Vec::new(); + for repo in repos { + let artifacts = + artifacts_for_repo(repo.id.into(), &conn).await.map_err( + |e| public_error_from_diesel(e, ErrorHandler::Server), + )?; + results.push(TufRepoDescription { repo, artifacts }); + } + + Ok(results) + } + /// List the trusted TUF root roles in the trust store. pub async fn tuf_trust_root_list( &self, diff --git a/nexus/external-api/output/nexus_tags.txt b/nexus/external-api/output/nexus_tags.txt index 496067dae17..7239db8baf0 100644 --- a/nexus/external-api/output/nexus_tags.txt +++ b/nexus/external-api/output/nexus_tags.txt @@ -298,6 +298,7 @@ API operations found with tag "system/update" OPERATION ID METHOD URL PATH system_update_get_repository GET /v1/system/update/repository/{system_version} system_update_put_repository PUT /v1/system/update/repository +system_update_repository_list GET /v1/system/update/repositories system_update_status GET /v1/system/update/status system_update_trust_root_create POST /v1/system/update/trust-roots system_update_trust_root_delete DELETE /v1/system/update/trust-roots/{trust_root_id} diff --git a/nexus/external-api/src/lib.rs b/nexus/external-api/src/lib.rs index be9a2b28027..f6a90f8e556 100644 --- a/nexus/external-api/src/lib.rs +++ b/nexus/external-api/src/lib.rs @@ -2988,6 +2988,20 @@ pub trait NexusExternalApi { path_params: Path, ) -> Result, HttpError>; + /// List all TUF repositories + /// + /// Returns a paginated list of all TUF repositories ordered by creation time + /// (descending), with the most recently created repositories appearing first. + #[endpoint { + method = GET, + path = "/v1/system/update/repositories", + tags = ["system/update"], + }] + async fn system_update_repository_list( + rqctx: RequestContext, + query_params: Query, + ) -> Result>, HttpError>; + /// List root roles in the updates trust store /// /// A root role is a JSON document describing the cryptographic keys that diff --git a/nexus/src/app/update.rs b/nexus/src/app/update.rs index f65a99f5105..50d6b52144b 100644 --- a/nexus/src/app/update.rs +++ b/nexus/src/app/update.rs @@ -5,6 +5,7 @@ //! Software Updates use bytes::Bytes; +use chrono::{DateTime, Utc}; use dropshot::HttpError; use futures::Stream; use nexus_auth::authz; @@ -109,6 +110,17 @@ impl super::Nexus { .map_err(HttpError::from) } + pub(crate) async fn updates_list_repositories( + &self, + opctx: &OpContext, + pagparams: &DataPageParams<'_, (DateTime, Uuid)>, + ) -> Result, HttpError> { + self.db_datastore + .tuf_repo_list(opctx, pagparams) + .await + .map_err(HttpError::from) + } + pub(crate) async fn updates_add_trust_root( &self, opctx: &OpContext, diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index 4195ed4634f..0a143c94685 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -17,6 +17,7 @@ use crate::app::external_endpoints::authority_for_request; use crate::app::support_bundles::SupportBundleQueryType; use crate::context::ApiContext; use crate::external_api::shared; +use chrono::{DateTime, Utc}; use dropshot::Body; use dropshot::EmptyScanParams; use dropshot::Header; @@ -113,6 +114,7 @@ use propolis_client::support::tungstenite::protocol::{ }; use range_requests::PotentialRange; use ref_cast::RefCast; +use uuid::Uuid; type NexusApiDescription = ApiDescription; @@ -6687,6 +6689,63 @@ impl NexusExternalApi for NexusExternalApiImpl { .await } + async fn system_update_repository_list( + rqctx: RequestContext, + query_params: Query, + ) -> Result>, HttpError> + { + let apictx = rqctx.context(); + let nexus = &apictx.context.nexus; + let handler = async { + let opctx = + crate::context::op_context_for_external_api(&rqctx).await?; + let query = query_params.into_inner(); + let pagparams = data_page_params_for(&rqctx, &query)?; + let repos = + nexus.updates_list_repositories(&opctx, &pagparams).await?; + + // Create a helper struct to maintain the association between response and database info + struct TufRepoWithMeta { + response: TufRepoGetResponse, + time_created: DateTime, + repo_id: Uuid, + } + + let items: Vec = repos + .into_iter() + .map(|description| { + let time_created = description.repo.time_created; + let repo_id = description.repo.id.into_untyped_uuid(); + let response = TufRepoGetResponse { + description: description.into_external(), + }; + TufRepoWithMeta { response, time_created, repo_id } + }) + .collect(); + + let responses: Vec = + items.iter().map(|item| item.response.clone()).collect(); + + Ok(HttpResponseOk(ScanByTimeAndId::results_page( + &query, + responses, + &|_scan_params, item: &TufRepoGetResponse| { + // Find the corresponding metadata for this response + let meta = items + .iter() + .find(|meta| std::ptr::eq(&meta.response, item)) + .expect("Response should have corresponding metadata"); + (meta.time_created, meta.repo_id) + }, + )?)) + }; + apictx + .context + .external_latencies + .instrument_dropshot_handler(&rqctx, handler) + .await + } + async fn system_update_trust_root_list( rqctx: RequestContext, query_params: Query, diff --git a/openapi/nexus.json b/openapi/nexus.json index c421334efda..1f41cfd13f5 100644 --- a/openapi/nexus.json +++ b/openapi/nexus.json @@ -10903,6 +10903,66 @@ } } }, + "/v1/system/update/repositories": { + "get": { + "tags": [ + "system/update" + ], + "summary": "List all TUF repositories", + "description": "Returns a paginated list of all TUF repositories ordered by creation time (descending), with the most recently created repositories appearing first.", + "operationId": "system_update_repository_list", + "parameters": [ + { + "in": "query", + "name": "limit", + "description": "Maximum number of items returned by a single call", + "schema": { + "nullable": true, + "type": "integer", + "format": "uint32", + "minimum": 1 + } + }, + { + "in": "query", + "name": "page_token", + "description": "Token returned by previous call to retrieve the subsequent page", + "schema": { + "nullable": true, + "type": "string" + } + }, + { + "in": "query", + "name": "sort_by", + "schema": { + "$ref": "#/components/schemas/TimeAndIdSortMode" + } + } + ], + "responses": { + "200": { + "description": "successful operation", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/TufRepoGetResponseResultsPage" + } + } + } + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + }, + "x-dropshot-pagination": { + "required": [] + } + } + }, "/v1/system/update/repository": { "put": { "tags": [ @@ -25915,6 +25975,27 @@ "description" ] }, + "TufRepoGetResponseResultsPage": { + "description": "A single page of results", + "type": "object", + "properties": { + "items": { + "description": "list of items on this page of results", + "type": "array", + "items": { + "$ref": "#/components/schemas/TufRepoGetResponse" + } + }, + "next_page": { + "nullable": true, + "description": "token used to fetch the next page of results (if any)", + "type": "string" + } + }, + "required": [ + "items" + ] + }, "TufRepoInsertResponse": { "description": "Data about a successful TUF repo import into Nexus.", "type": "object", From 324aeed017d64f2d1a941c6dfc9f90a5118960bd Mon Sep 17 00:00:00 2001 From: David Crespo Date: Thu, 25 Sep 2025 12:16:29 -0500 Subject: [PATCH 09/31] add integration test for tuf repo list --- nexus/db-queries/src/db/datastore/update.rs | 1 - nexus/tests/integration_tests/updates.rs | 185 +++++++++++++++----- 2 files changed, 142 insertions(+), 44 deletions(-) diff --git a/nexus/db-queries/src/db/datastore/update.rs b/nexus/db-queries/src/db/datastore/update.rs index 78513dfafad..3e4f6abe334 100644 --- a/nexus/db-queries/src/db/datastore/update.rs +++ b/nexus/db-queries/src/db/datastore/update.rs @@ -481,7 +481,6 @@ impl DataStore { (dsl::time_created, dsl::id), pagparams, ) - .order_by((dsl::time_created.desc(), dsl::id)) .select(TufRepo::as_select()) .load_async(&*conn) .await diff --git a/nexus/tests/integration_tests/updates.rs b/nexus/tests/integration_tests/updates.rs index bc012ae7baf..606c4a79076 100644 --- a/nexus/tests/integration_tests/updates.rs +++ b/nexus/tests/integration_tests/updates.rs @@ -15,6 +15,8 @@ use nexus_test_utils::background::run_tuf_artifact_replication_step; use nexus_test_utils::background::wait_tuf_artifact_replication_step; use nexus_test_utils::http_testing::{AuthnMode, NexusRequest, RequestBuilder}; use nexus_test_utils::resource_helpers::object_get; +use nexus_test_utils::resource_helpers::object_get_error; +use nexus_test_utils::resource_helpers::objects_list_page_authz; use nexus_test_utils::test_setup; use nexus_test_utils_macros::nexus_test; use nexus_types::external_api::views; @@ -168,16 +170,12 @@ async fn test_repo_upload_unconfigured() -> Result<()> { // Attempt to fetch a repository description from Nexus. This should fail // with a 404 error. - { - make_get_request( - client, - "1.0.0".parse().unwrap(), - StatusCode::NOT_FOUND, - ) - .execute() - .await - .context("repository fetch should have failed with 404 error")?; - } + object_get_error( + client, + "/v1/system/update/repository/1.0.0", + StatusCode::NOT_FOUND, + ) + .await; cptestctx.teardown().await; Ok(()) @@ -313,18 +311,11 @@ async fn test_repo_upload() -> Result<()> { // Now get the repository that was just uploaded. let mut get_description = { - let response = make_get_request( + let response = object_get::( client, - "1.0.0".parse().unwrap(), // this is the system version of the fake manifest - StatusCode::OK, + "/v1/system/update/repository/1.0.0", ) - .execute() - .await - .context("error fetching repository")?; - - let response = - serde_json::from_slice::(&response.body) - .context("error deserializing response body")?; + .await; response.description }; @@ -501,12 +492,11 @@ async fn test_repo_upload() -> Result<()> { // Now get the repository that was just uploaded and make sure the // artifact list is the same. - let response: TufRepoGetResponse = - make_get_request(client, "2.0.0".parse().unwrap(), StatusCode::OK) - .execute() - .await - .context("error fetching repository")? - .parsed_body()?; + let response = object_get::( + client, + "/v1/system/update/repository/2.0.0", + ) + .await; let mut get_description = response.description; get_description.sort_artifacts(); @@ -606,23 +596,6 @@ async fn test_repo_upload() -> Result<()> { Ok(()) } -fn make_get_request( - client: &dropshot::test_util::ClientTestContext, - system_version: Version, - expected_status: StatusCode, -) -> NexusRequest<'_> { - let request = NexusRequest::new( - RequestBuilder::new( - client, - Method::GET, - &format!("/v1/system/update/repository/{system_version}"), - ) - .expect_status(Some(expected_status)), - ) - .authn_as(AuthnMode::PrivilegedUser); - request -} - #[derive(Debug, Deserialize)] struct ErrorBody { message: String, @@ -786,6 +759,7 @@ async fn test_update_status() -> Result<()> { cptestctx.teardown().await; Ok(()) } + #[nexus_test] async fn test_repo_prune(cptestctx: &ControlPlaneTestContext) { let logctx = &cptestctx.logctx; @@ -826,3 +800,128 @@ async fn test_repo_prune(cptestctx: &ControlPlaneTestContext) { assert!(repos.iter().any(|r| r.id() == repo3id)); assert!(repos.iter().any(|r| r.id() == repo4id)); } + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_repo_list() -> Result<()> { + let cptestctx = test_setup::( + "test_update_repo_list", + 3, // 4 total sled agents + ) + .await; + let client = &cptestctx.external_client; + let logctx = &cptestctx.logctx; + + // Initially, list should be empty + let initial_list: ResultsPage = + objects_list_page_authz(client, "/v1/system/update/repositories").await; + assert_eq!(initial_list.items.len(), 0); + assert!(initial_list.next_page.is_none()); + + // Add a trust root + let trust_root = TestTrustRoot::generate().await?; + trust_root.to_upload_request(client, StatusCode::CREATED).execute().await?; + + // Upload first repository (system version 1.0.0) + let repo1 = trust_root.assemble_repo(&logctx.log, &[]).await?; + let upload_response1 = repo1 + .into_upload_request(client, StatusCode::OK) + .execute() + .await + .context("error uploading first repository")?; + let response1 = + serde_json::from_slice::(&upload_response1.body) + .context("error deserializing first response body")?; + assert_eq!(response1.status, TufRepoInsertStatus::Inserted); + + // Upload second repository (system version 2.0.0) + let tweaks = &[ManifestTweak::SystemVersion("2.0.0".parse().unwrap())]; + let repo2 = trust_root.assemble_repo(&logctx.log, tweaks).await?; + let upload_response2 = repo2 + .into_upload_request(client, StatusCode::OK) + .execute() + .await + .context("error uploading second repository")?; + let response2 = + serde_json::from_slice::(&upload_response2.body) + .context("error deserializing second response body")?; + assert_eq!(response2.status, TufRepoInsertStatus::Inserted); + + // Upload third repository (system version 3.0.0) + let tweaks = &[ManifestTweak::SystemVersion("3.0.0".parse().unwrap())]; + let repo3 = trust_root.assemble_repo(&logctx.log, tweaks).await?; + let upload_response3 = repo3 + .into_upload_request(client, StatusCode::OK) + .execute() + .await + .context("error uploading third repository")?; + let response3 = + serde_json::from_slice::(&upload_response3.body) + .context("error deserializing third response body")?; + assert_eq!(response3.status, TufRepoInsertStatus::Inserted); + + // List repositories - should return all 3, ordered by creation time (newest first) + let list: ResultsPage = + objects_list_page_authz(client, "/v1/system/update/repositories").await; + + assert_eq!(list.items.len(), 3); + + // Repositories should be ordered by creation time descending (newest first) + // Since repo3 was created last, it should be first in the list + let system_versions: Vec = list + .items + .iter() + .map(|item| item.description.repo.system_version.to_string()) + .collect(); + assert_eq!(system_versions, vec!["3.0.0", "2.0.0", "1.0.0"]); + + // Verify that each response contains the correct artifacts + for (i, item) in list.items.iter().enumerate() { + let expected_version = match i { + 0 => "3.0.0", + 1 => "2.0.0", + 2 => "1.0.0", + _ => panic!("unexpected index"), + }; + assert_eq!( + item.description.repo.system_version.to_string(), + expected_version + ); + + // Verify artifacts are present (should have Zone artifacts) + let zone_artifacts: Vec<_> = item + .description + .artifacts + .iter() + .filter(|artifact| { + artifact.id.kind == KnownArtifactKind::Zone.into() + }) + .collect(); + assert_eq!(zone_artifacts.len(), 2, "should have 2 zone artifacts"); + + // Should not have ControlPlane artifacts (they get decomposed into Zones) + assert!(!item.description.artifacts.iter().any(|artifact| { + artifact.id.kind == KnownArtifactKind::ControlPlane.into() + })); + } + + // Test pagination by setting a small limit + let paginated_list = objects_list_page_authz::( + client, + "/v1/system/update/repositories?limit=2", + ) + .await; + + assert_eq!(paginated_list.items.len(), 2); + assert!(paginated_list.next_page.is_some()); + + // First two items should be 3.0.0 and 2.0.0 (newest first) + let paginated_versions: Vec = paginated_list + .items + .iter() + .map(|item| item.description.repo.system_version.to_string()) + .collect(); + assert_eq!(paginated_versions, vec!["3.0.0", "2.0.0"]); + + cptestctx.teardown().await; + Ok(()) +} From 350c44c31b37320bdc2b7e7f4450dc2bc0e6e9e7 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Thu, 25 Sep 2025 16:17:58 -0500 Subject: [PATCH 10/31] switch to paginating by version --- common/src/api/external/http_pagination.rs | 56 +++++++++++++++++++++ nexus/db-queries/src/db/datastore/update.rs | 35 +++++++------ nexus/external-api/src/lib.rs | 8 +-- nexus/src/app/update.rs | 3 +- nexus/src/external_api/http_entrypoints.rs | 36 +++---------- nexus/tests/integration_tests/updates.rs | 35 +++++++++++-- openapi/nexus.json | 23 ++++++++- 7 files changed, 142 insertions(+), 54 deletions(-) diff --git a/common/src/api/external/http_pagination.rs b/common/src/api/external/http_pagination.rs index 710de77f483..2c8d91eb0fd 100644 --- a/common/src/api/external/http_pagination.rs +++ b/common/src/api/external/http_pagination.rs @@ -53,6 +53,7 @@ use dropshot::RequestContext; use dropshot::ResultsPage; use dropshot::WhichPage; use schemars::JsonSchema; +use semver::Version; use serde::Deserialize; use serde::Serialize; use serde::de::DeserializeOwned; @@ -163,6 +164,54 @@ pub fn marker_for_name_or_id( } } +// Pagination by semantic version in ascending or descending order + +/// Scan parameters for resources that support scanning by semantic version +#[derive(Clone, Debug, Deserialize, JsonSchema, PartialEq, Serialize)] +pub struct ScanByVersion { + #[serde(default = "default_version_sort_mode")] + sort_by: VersionSortMode, + #[serde(flatten)] + pub selector: Selector, +} + +/// Supported sort modes when scanning by semantic version +#[derive(Copy, Clone, Debug, Deserialize, JsonSchema, PartialEq, Serialize)] +#[serde(rename_all = "snake_case")] +pub enum VersionSortMode { + /// Sort in increasing semantic version order (oldest first) + VersionAscending, + /// Sort in decreasing semantic version order (newest first) + VersionDescending, +} + +fn default_version_sort_mode() -> VersionSortMode { + VersionSortMode::VersionDescending +} + +impl ScanParams for ScanByVersion +where + T: Clone + Debug + DeserializeOwned + JsonSchema + PartialEq + Serialize, +{ + type MarkerValue = Version; + + fn direction(&self) -> PaginationOrder { + match self.sort_by { + VersionSortMode::VersionAscending => PaginationOrder::Ascending, + VersionSortMode::VersionDescending => PaginationOrder::Descending, + } + } + + fn from_query( + p: &PaginationParams>, + ) -> Result<&Self, HttpError> { + Ok(match p.page { + WhichPage::First(ref scan_params) => scan_params, + WhichPage::Next(PageSelector { ref scan, .. }) => scan, + }) + } +} + /// See `dropshot::ResultsPage::new` fn page_selector_for( item: &T, @@ -313,6 +362,13 @@ pub type PaginatedByNameOrId = PaginationParams< pub type PageSelectorByNameOrId = PageSelector, NameOrId>; +/// Query parameters for pagination by semantic version +pub type PaginatedByVersion = + PaginationParams, PageSelectorByVersion>; +/// Page selector for pagination by semantic version +pub type PageSelectorByVersion = + PageSelector, Version>; + pub fn id_pagination<'a, Selector>( pag_params: &'a DataPageParams, scan_params: &'a ScanById, diff --git a/nexus/db-queries/src/db/datastore/update.rs b/nexus/db-queries/src/db/datastore/update.rs index 3e4f6abe334..e5e698fd222 100644 --- a/nexus/db-queries/src/db/datastore/update.rs +++ b/nexus/db-queries/src/db/datastore/update.rs @@ -6,8 +6,6 @@ use std::collections::HashMap; -use chrono::{DateTime, Utc}; - use super::DataStore; use crate::authz; use crate::context::OpContext; @@ -16,7 +14,6 @@ use crate::db::datastore::target_release::RecentTargetReleases; use crate::db::model::SemverVersion; use crate::db::pagination::Paginator; use crate::db::pagination::paginated; -use crate::db::pagination::paginated_multicolumn; use async_bb8_diesel::AsyncRunQueryDsl; use diesel::prelude::*; use diesel::result::Error as DieselError; @@ -36,6 +33,7 @@ use omicron_common::api::external::{Error, InternalContext}; use omicron_uuid_kinds::TufRepoKind; use omicron_uuid_kinds::TypedUuid; use omicron_uuid_kinds::{GenericUuid, TufRepoUuid}; +use semver::Version; use swrite::{SWrite, swrite}; use tufaceous_artifact::ArtifactVersion; use uuid::Uuid; @@ -463,11 +461,11 @@ impl DataStore { .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server)) } - /// List all TUF repositories ordered by creation time (descending). + /// List all TUF repositories ordered by system version (newest first by default). pub async fn tuf_repo_list( &self, opctx: &OpContext, - pagparams: &DataPageParams<'_, (DateTime, Uuid)>, + pagparams: &DataPageParams<'_, Version>, ) -> ListResultVec { opctx.authorize(authz::Action::Read, &authz::FLEET).await?; @@ -475,16 +473,23 @@ impl DataStore { let conn = self.pool_connection_authorized(opctx).await?; - // Order by time_created descending (newest first), then by id for stable pagination - let repos = paginated_multicolumn( - dsl::tuf_repo, - (dsl::time_created, dsl::id), - pagparams, - ) - .select(TufRepo::as_select()) - .load_async(&*conn) - .await - .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server))?; + let marker_owner = pagparams + .marker + .map(|version| SemverVersion::from(version.clone())); + let db_pagparams = DataPageParams { + marker: marker_owner.as_ref(), + direction: pagparams.direction, + limit: pagparams.limit, + }; + + let repos = + paginated(dsl::tuf_repo, dsl::system_version, &db_pagparams) + .select(TufRepo::as_select()) + .load_async(&*conn) + .await + .map_err(|e| { + public_error_from_diesel(e, ErrorHandler::Server) + })?; // For each repo, fetch its artifacts let mut results = Vec::new(); diff --git a/nexus/external-api/src/lib.rs b/nexus/external-api/src/lib.rs index f6a90f8e556..949cec6aa8a 100644 --- a/nexus/external-api/src/lib.rs +++ b/nexus/external-api/src/lib.rs @@ -24,7 +24,7 @@ use nexus_types::{ use omicron_common::api::external::{ http_pagination::{ PaginatedById, PaginatedByName, PaginatedByNameOrId, - PaginatedByTimeAndId, + PaginatedByTimeAndId, PaginatedByVersion, }, *, }; @@ -2990,8 +2990,8 @@ pub trait NexusExternalApi { /// List all TUF repositories /// - /// Returns a paginated list of all TUF repositories ordered by creation time - /// (descending), with the most recently created repositories appearing first. + /// Returns a paginated list of all TUF repositories ordered by system + /// version (newest first by default). #[endpoint { method = GET, path = "/v1/system/update/repositories", @@ -2999,7 +2999,7 @@ pub trait NexusExternalApi { }] async fn system_update_repository_list( rqctx: RequestContext, - query_params: Query, + query_params: Query, ) -> Result>, HttpError>; /// List root roles in the updates trust store diff --git a/nexus/src/app/update.rs b/nexus/src/app/update.rs index 50d6b52144b..20ad663ba36 100644 --- a/nexus/src/app/update.rs +++ b/nexus/src/app/update.rs @@ -5,7 +5,6 @@ //! Software Updates use bytes::Bytes; -use chrono::{DateTime, Utc}; use dropshot::HttpError; use futures::Stream; use nexus_auth::authz; @@ -113,7 +112,7 @@ impl super::Nexus { pub(crate) async fn updates_list_repositories( &self, opctx: &OpContext, - pagparams: &DataPageParams<'_, (DateTime, Uuid)>, + pagparams: &DataPageParams<'_, Version>, ) -> Result, HttpError> { self.db_datastore .tuf_repo_list(opctx, pagparams) diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index 0a143c94685..a74abc0375a 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -17,7 +17,6 @@ use crate::app::external_endpoints::authority_for_request; use crate::app::support_bundles::SupportBundleQueryType; use crate::context::ApiContext; use crate::external_api::shared; -use chrono::{DateTime, Utc}; use dropshot::Body; use dropshot::EmptyScanParams; use dropshot::Header; @@ -95,10 +94,12 @@ use omicron_common::api::external::http_pagination::PaginatedById; use omicron_common::api::external::http_pagination::PaginatedByName; use omicron_common::api::external::http_pagination::PaginatedByNameOrId; use omicron_common::api::external::http_pagination::PaginatedByTimeAndId; +use omicron_common::api::external::http_pagination::PaginatedByVersion; use omicron_common::api::external::http_pagination::ScanById; use omicron_common::api::external::http_pagination::ScanByName; use omicron_common::api::external::http_pagination::ScanByNameOrId; use omicron_common::api::external::http_pagination::ScanByTimeAndId; +use omicron_common::api::external::http_pagination::ScanByVersion; use omicron_common::api::external::http_pagination::ScanParams; use omicron_common::api::external::http_pagination::data_page_params_for; use omicron_common::api::external::http_pagination::marker_for_id; @@ -114,7 +115,6 @@ use propolis_client::support::tungstenite::protocol::{ }; use range_requests::PotentialRange; use ref_cast::RefCast; -use uuid::Uuid; type NexusApiDescription = ApiDescription; @@ -6691,7 +6691,7 @@ impl NexusExternalApi for NexusExternalApiImpl { async fn system_update_repository_list( rqctx: RequestContext, - query_params: Query, + query_params: Query, ) -> Result>, HttpError> { let apictx = rqctx.context(); @@ -6704,38 +6704,18 @@ impl NexusExternalApi for NexusExternalApiImpl { let repos = nexus.updates_list_repositories(&opctx, &pagparams).await?; - // Create a helper struct to maintain the association between response and database info - struct TufRepoWithMeta { - response: TufRepoGetResponse, - time_created: DateTime, - repo_id: Uuid, - } - - let items: Vec = repos + let responses: Vec = repos .into_iter() - .map(|description| { - let time_created = description.repo.time_created; - let repo_id = description.repo.id.into_untyped_uuid(); - let response = TufRepoGetResponse { - description: description.into_external(), - }; - TufRepoWithMeta { response, time_created, repo_id } + .map(|description| TufRepoGetResponse { + description: description.into_external(), }) .collect(); - let responses: Vec = - items.iter().map(|item| item.response.clone()).collect(); - - Ok(HttpResponseOk(ScanByTimeAndId::results_page( + Ok(HttpResponseOk(ScanByVersion::results_page( &query, responses, &|_scan_params, item: &TufRepoGetResponse| { - // Find the corresponding metadata for this response - let meta = items - .iter() - .find(|meta| std::ptr::eq(&meta.response, item)) - .expect("Response should have corresponding metadata"); - (meta.time_created, meta.repo_id) + item.description.repo.system_version.clone() }, )?)) }; diff --git a/nexus/tests/integration_tests/updates.rs b/nexus/tests/integration_tests/updates.rs index 606c4a79076..7e965cfaf94 100644 --- a/nexus/tests/integration_tests/updates.rs +++ b/nexus/tests/integration_tests/updates.rs @@ -859,14 +859,13 @@ async fn test_repo_list() -> Result<()> { .context("error deserializing third response body")?; assert_eq!(response3.status, TufRepoInsertStatus::Inserted); - // List repositories - should return all 3, ordered by creation time (newest first) + // List repositories - should return all 3, ordered by system version (newest first) let list: ResultsPage = objects_list_page_authz(client, "/v1/system/update/repositories").await; assert_eq!(list.items.len(), 3); - // Repositories should be ordered by creation time descending (newest first) - // Since repo3 was created last, it should be first in the list + // Repositories should be ordered by system version descending (newest first) let system_versions: Vec = list .items .iter() @@ -904,6 +903,23 @@ async fn test_repo_list() -> Result<()> { })); } + // Request ascending order and expect the versions oldest-first + let ascending_list: ResultsPage = + objects_list_page_authz( + client, + "/v1/system/update/repositories?sort_by=ascending", + ) + .await; + + assert_eq!(ascending_list.items.len(), 3); + + let ascending_versions: Vec = ascending_list + .items + .iter() + .map(|item| item.description.repo.system_version.to_string()) + .collect(); + assert_eq!(ascending_versions, vec!["1.0.0", "2.0.0", "3.0.0"]); + // Test pagination by setting a small limit let paginated_list = objects_list_page_authz::( client, @@ -922,6 +938,19 @@ async fn test_repo_list() -> Result<()> { .collect(); assert_eq!(paginated_versions, vec!["3.0.0", "2.0.0"]); + // Fetch the next page via the returned page token and expect the remaining repo + let next_page_url = format!( + "/v1/system/update/repositories?limit=2&page_token={}", + paginated_list.next_page.clone().expect("expected next page token"), + ); + let next_page: ResultsPage = + objects_list_page_authz(client, &next_page_url).await; + assert_eq!(next_page.items.len(), 1); + assert_eq!( + next_page.items[0].description.repo.system_version.to_string(), + "1.0.0" + ); + cptestctx.teardown().await; Ok(()) } diff --git a/openapi/nexus.json b/openapi/nexus.json index 1f41cfd13f5..23c6ee4c6a8 100644 --- a/openapi/nexus.json +++ b/openapi/nexus.json @@ -10909,7 +10909,7 @@ "system/update" ], "summary": "List all TUF repositories", - "description": "Returns a paginated list of all TUF repositories ordered by creation time (descending), with the most recently created repositories appearing first.", + "description": "Returns a paginated list of all TUF repositories ordered by system version (newest first by default).", "operationId": "system_update_repository_list", "parameters": [ { @@ -10936,7 +10936,7 @@ "in": "query", "name": "sort_by", "schema": { - "$ref": "#/components/schemas/TimeAndIdSortMode" + "$ref": "#/components/schemas/VersionSortMode" } } ], @@ -28045,6 +28045,25 @@ "descending" ] }, + "VersionSortMode": { + "description": "Supported sort modes when scanning by semantic version", + "oneOf": [ + { + "description": "Sort in increasing semantic version order (oldest first)", + "type": "string", + "enum": [ + "version_ascending" + ] + }, + { + "description": "Sort in decreasing semantic version order (newest first)", + "type": "string", + "enum": [ + "version_descending" + ] + } + ] + }, "NameSortMode": { "description": "Supported set of sort modes for scanning by name only\n\nCurrently, we only support scanning in ascending order.", "oneOf": [ From 0708c5363baeb9e6f0a985035b64e12c22b7f804 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Thu, 25 Sep 2025 18:16:41 -0500 Subject: [PATCH 11/31] do artifacts fetch in a single query (still bad) --- nexus/db-queries/src/db/datastore/update.rs | 51 ++++++++++++++++++--- 1 file changed, 44 insertions(+), 7 deletions(-) diff --git a/nexus/db-queries/src/db/datastore/update.rs b/nexus/db-queries/src/db/datastore/update.rs index e5e698fd222..e01eaca4cd8 100644 --- a/nexus/db-queries/src/db/datastore/update.rs +++ b/nexus/db-queries/src/db/datastore/update.rs @@ -469,7 +469,9 @@ impl DataStore { ) -> ListResultVec { opctx.authorize(authz::Action::Read, &authz::FLEET).await?; - use nexus_db_schema::schema::tuf_repo::dsl; + use nexus_db_schema::schema::tuf_artifact; + use nexus_db_schema::schema::tuf_repo; + use nexus_db_schema::schema::tuf_repo_artifact; let conn = self.pool_connection_authorized(opctx).await?; @@ -482,8 +484,9 @@ impl DataStore { limit: pagparams.limit, }; + // First get the paginated repos let repos = - paginated(dsl::tuf_repo, dsl::system_version, &db_pagparams) + paginated(tuf_repo::table, tuf_repo::system_version, &db_pagparams) .select(TufRepo::as_select()) .load_async(&*conn) .await @@ -491,13 +494,47 @@ impl DataStore { public_error_from_diesel(e, ErrorHandler::Server) })?; - // For each repo, fetch its artifacts + if repos.is_empty() { + return Ok(Vec::new()); + } + + // Get all repo IDs for the artifacts query + let repo_ids: Vec<_> = repos.iter().map(|repo| repo.id).collect(); + + // Fetch all artifacts for these repos in a single query + let repo_artifacts: Vec<(TufRepo, TufArtifact)> = + tuf_repo::table + .filter(tuf_repo::id.eq_any(repo_ids)) + .inner_join( + tuf_repo_artifact::table + .on(tuf_repo::id.eq(tuf_repo_artifact::tuf_repo_id)), + ) + .inner_join(tuf_artifact::table.on( + tuf_repo_artifact::tuf_artifact_id.eq(tuf_artifact::id), + )) + .select((TufRepo::as_select(), TufArtifact::as_select())) + .load_async(&*conn) + .await + .map_err(|e| { + public_error_from_diesel(e, ErrorHandler::Server) + })?; + + // Group artifacts by repo ID + let mut artifacts_by_repo: HashMap< + TypedUuid, + Vec, + > = HashMap::new(); + for (repo, artifact) in repo_artifacts { + artifacts_by_repo.entry(repo.id.into()).or_default().push(artifact); + } + + // Build the final results, maintaining the original pagination order let mut results = Vec::new(); for repo in repos { - let artifacts = - artifacts_for_repo(repo.id.into(), &conn).await.map_err( - |e| public_error_from_diesel(e, ErrorHandler::Server), - )?; + let artifacts = artifacts_by_repo + .get(&repo.id.into()) + .cloned() + .unwrap_or_default(); results.push(TufRepoDescription { repo, artifacts }); } From 442e9e128774de773333b78e69ad0e5722024a49 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Fri, 26 Sep 2025 11:32:30 -0500 Subject: [PATCH 12/31] try doing views::TufRepo --- Cargo.lock | 1 + common/src/api/external/mod.rs | 7 - nexus/db-queries/src/db/datastore/update.rs | 81 ++---------- nexus/external-api/Cargo.toml | 1 + nexus/external-api/src/lib.rs | 4 +- nexus/src/app/update.rs | 8 +- nexus/src/external_api/http_entrypoints.rs | 22 ++-- nexus/tests/integration_tests/updates.rs | 139 ++++++++------------ nexus/types/src/external_api/views.rs | 46 ++++++- openapi/nexus.json | 101 ++++++++------ 10 files changed, 188 insertions(+), 222 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 88b9029aa93..24c4ab76cf0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6674,6 +6674,7 @@ dependencies = [ "openapiv3", "oximeter-types 0.1.0", "oxql-types", + "tufaceous-artifact", ] [[package]] diff --git a/common/src/api/external/mod.rs b/common/src/api/external/mod.rs index 9802a75a4b3..00d252053c1 100644 --- a/common/src/api/external/mod.rs +++ b/common/src/api/external/mod.rs @@ -3526,13 +3526,6 @@ pub enum TufRepoInsertStatus { Inserted, } -/// Data about a successful TUF repo get from Nexus. -#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema)] -#[serde(rename_all = "snake_case")] -pub struct TufRepoGetResponse { - /// The description of the repository. - pub description: TufRepoDescription, -} #[derive( Clone, Debug, Deserialize, JsonSchema, Serialize, PartialEq, ObjectIdentity, diff --git a/nexus/db-queries/src/db/datastore/update.rs b/nexus/db-queries/src/db/datastore/update.rs index e01eaca4cd8..87c18053975 100644 --- a/nexus/db-queries/src/db/datastore/update.rs +++ b/nexus/db-queries/src/db/datastore/update.rs @@ -148,19 +148,19 @@ impl DataStore { Ok(TufRepoDescription { repo, artifacts }) } - /// Returns the TUF repo description corresponding to this system version. + /// Returns the TUF repo corresponding to this system version. pub async fn tuf_repo_get_by_version( &self, opctx: &OpContext, system_version: SemverVersion, - ) -> LookupResult { + ) -> LookupResult { opctx.authorize(authz::Action::Read, &authz::FLEET).await?; use nexus_db_schema::schema::tuf_repo::dsl; let conn = self.pool_connection_authorized(opctx).await?; - let repo = dsl::tuf_repo + dsl::tuf_repo .filter(dsl::system_version.eq(system_version.clone())) .select(TufRepo::as_select()) .first_async::(&*conn) @@ -173,12 +173,7 @@ impl DataStore { LookupType::ByCompositeId(system_version.to_string()), ), ) - })?; - - let artifacts = artifacts_for_repo(repo.id.into(), &conn) - .await - .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server))?; - Ok(TufRepoDescription { repo, artifacts }) + }) } /// Given a TUF repo ID, get its version. We could use `tuf_repo_get_by_id`, @@ -461,17 +456,15 @@ impl DataStore { .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server)) } - /// List all TUF repositories ordered by system version (newest first by default). - pub async fn tuf_repo_list( + /// List all TUF repositories (without artifacts) ordered by system version (newest first by default). + pub async fn tuf_repo_list_no_artifacts( &self, opctx: &OpContext, pagparams: &DataPageParams<'_, Version>, - ) -> ListResultVec { + ) -> ListResultVec { opctx.authorize(authz::Action::Read, &authz::FLEET).await?; - use nexus_db_schema::schema::tuf_artifact; use nexus_db_schema::schema::tuf_repo; - use nexus_db_schema::schema::tuf_repo_artifact; let conn = self.pool_connection_authorized(opctx).await?; @@ -484,61 +477,11 @@ impl DataStore { limit: pagparams.limit, }; - // First get the paginated repos - let repos = - paginated(tuf_repo::table, tuf_repo::system_version, &db_pagparams) - .select(TufRepo::as_select()) - .load_async(&*conn) - .await - .map_err(|e| { - public_error_from_diesel(e, ErrorHandler::Server) - })?; - - if repos.is_empty() { - return Ok(Vec::new()); - } - - // Get all repo IDs for the artifacts query - let repo_ids: Vec<_> = repos.iter().map(|repo| repo.id).collect(); - - // Fetch all artifacts for these repos in a single query - let repo_artifacts: Vec<(TufRepo, TufArtifact)> = - tuf_repo::table - .filter(tuf_repo::id.eq_any(repo_ids)) - .inner_join( - tuf_repo_artifact::table - .on(tuf_repo::id.eq(tuf_repo_artifact::tuf_repo_id)), - ) - .inner_join(tuf_artifact::table.on( - tuf_repo_artifact::tuf_artifact_id.eq(tuf_artifact::id), - )) - .select((TufRepo::as_select(), TufArtifact::as_select())) - .load_async(&*conn) - .await - .map_err(|e| { - public_error_from_diesel(e, ErrorHandler::Server) - })?; - - // Group artifacts by repo ID - let mut artifacts_by_repo: HashMap< - TypedUuid, - Vec, - > = HashMap::new(); - for (repo, artifact) in repo_artifacts { - artifacts_by_repo.entry(repo.id.into()).or_default().push(artifact); - } - - // Build the final results, maintaining the original pagination order - let mut results = Vec::new(); - for repo in repos { - let artifacts = artifacts_by_repo - .get(&repo.id.into()) - .cloned() - .unwrap_or_default(); - results.push(TufRepoDescription { repo, artifacts }); - } - - Ok(results) + paginated(tuf_repo::table, tuf_repo::system_version, &db_pagparams) + .select(TufRepo::as_select()) + .load_async(&*conn) + .await + .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server)) } /// List the trusted TUF root roles in the trust store. diff --git a/nexus/external-api/Cargo.toml b/nexus/external-api/Cargo.toml index a2c4b481190..1d589be6375 100644 --- a/nexus/external-api/Cargo.toml +++ b/nexus/external-api/Cargo.toml @@ -22,3 +22,4 @@ openapiv3.workspace = true oximeter-types.workspace = true oxql-types.workspace = true omicron-uuid-kinds.workspace = true +tufaceous-artifact.workspace = true diff --git a/nexus/external-api/src/lib.rs b/nexus/external-api/src/lib.rs index 949cec6aa8a..b8ffcaf52ba 100644 --- a/nexus/external-api/src/lib.rs +++ b/nexus/external-api/src/lib.rs @@ -2986,7 +2986,7 @@ pub trait NexusExternalApi { async fn system_update_get_repository( rqctx: RequestContext, path_params: Path, - ) -> Result, HttpError>; + ) -> Result, HttpError>; /// List all TUF repositories /// @@ -3000,7 +3000,7 @@ pub trait NexusExternalApi { async fn system_update_repository_list( rqctx: RequestContext, query_params: Query, - ) -> Result>, HttpError>; + ) -> Result>, HttpError>; /// List root roles in the updates trust store /// diff --git a/nexus/src/app/update.rs b/nexus/src/app/update.rs index 20ad663ba36..c7ba5d2381f 100644 --- a/nexus/src/app/update.rs +++ b/nexus/src/app/update.rs @@ -9,7 +9,7 @@ use dropshot::HttpError; use futures::Stream; use nexus_auth::authz; use nexus_db_lookup::LookupPath; -use nexus_db_model::{Generation, TufRepoDescription, TufTrustRoot}; +use nexus_db_model::{Generation, TufTrustRoot}; use nexus_db_queries::context::OpContext; use nexus_db_queries::db::{datastore::SQL_BATCH_SIZE, pagination::Paginator}; use nexus_types::deployment::TargetReleaseDescription; @@ -102,7 +102,7 @@ impl super::Nexus { &self, opctx: &OpContext, system_version: Version, - ) -> Result { + ) -> Result { self.db_datastore .tuf_repo_get_by_version(opctx, system_version.into()) .await @@ -113,9 +113,9 @@ impl super::Nexus { &self, opctx: &OpContext, pagparams: &DataPageParams<'_, Version>, - ) -> Result, HttpError> { + ) -> Result, HttpError> { self.db_datastore - .tuf_repo_list(opctx, pagparams) + .tuf_repo_list_no_artifacts(opctx, pagparams) .await .map_err(HttpError::from) } diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index a74abc0375a..f6785ef4b55 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -85,7 +85,6 @@ use omicron_common::api::external::ServiceIcmpConfig; use omicron_common::api::external::SwitchPort; use omicron_common::api::external::SwitchPortSettings; use omicron_common::api::external::SwitchPortSettingsIdentity; -use omicron_common::api::external::TufRepoGetResponse; use omicron_common::api::external::TufRepoInsertResponse; use omicron_common::api::external::VpcFirewallRuleUpdateParams; use omicron_common::api::external::VpcFirewallRules; @@ -6668,19 +6667,17 @@ impl NexusExternalApi for NexusExternalApiImpl { async fn system_update_get_repository( rqctx: RequestContext, path_params: Path, - ) -> Result, HttpError> { + ) -> Result, HttpError> { let apictx = rqctx.context(); let nexus = &apictx.context.nexus; let handler = async { let opctx = crate::context::op_context_for_external_api(&rqctx).await?; let params = path_params.into_inner(); - let description = nexus + let repo = nexus .updates_get_repository(&opctx, params.system_version) .await?; - Ok(HttpResponseOk(TufRepoGetResponse { - description: description.into_external(), - })) + Ok(HttpResponseOk(repo.into_external().into())) }; apictx .context @@ -6692,7 +6689,7 @@ impl NexusExternalApi for NexusExternalApiImpl { async fn system_update_repository_list( rqctx: RequestContext, query_params: Query, - ) -> Result>, HttpError> + ) -> Result>, HttpError> { let apictx = rqctx.context(); let nexus = &apictx.context.nexus; @@ -6704,18 +6701,16 @@ impl NexusExternalApi for NexusExternalApiImpl { let repos = nexus.updates_list_repositories(&opctx, &pagparams).await?; - let responses: Vec = repos + let responses: Vec = repos .into_iter() - .map(|description| TufRepoGetResponse { - description: description.into_external(), - }) + .map(|repo| repo.into_external().into()) .collect(); Ok(HttpResponseOk(ScanByVersion::results_page( &query, responses, - &|_scan_params, item: &TufRepoGetResponse| { - item.description.repo.system_version.clone() + &|_scan_params, item: &views::TufRepo| { + item.system_version.clone() }, )?)) }; @@ -6881,7 +6876,6 @@ impl NexusExternalApi for NexusExternalApiImpl { .datastore() .tuf_repo_get_by_version(&opctx, system_version.into()) .await? - .repo .id; let next_target_release = nexus_db_model::TargetRelease::new_system_version( diff --git a/nexus/tests/integration_tests/updates.rs b/nexus/tests/integration_tests/updates.rs index 7e965cfaf94..4d9238487e7 100644 --- a/nexus/tests/integration_tests/updates.rs +++ b/nexus/tests/integration_tests/updates.rs @@ -20,9 +20,8 @@ use nexus_test_utils::resource_helpers::objects_list_page_authz; use nexus_test_utils::test_setup; use nexus_test_utils_macros::nexus_test; use nexus_types::external_api::views; -use nexus_types::external_api::views::UpdatesTrustRoot; use omicron_common::api::external::{ - TufRepoGetResponse, TufRepoInsertResponse, TufRepoInsertStatus, + TufRepoInsertResponse, TufRepoInsertStatus, }; use pretty_assertions::assert_eq; use semver::Version; @@ -310,20 +309,25 @@ async fn test_repo_upload() -> Result<()> { ); // Now get the repository that was just uploaded. - let mut get_description = { - let response = object_get::( - client, - "/v1/system/update/repository/1.0.0", - ) - .await; - response.description - }; - - get_description.sort_artifacts(); + let get_repo = object_get::( + client, + "/v1/system/update/repository/1.0.0", + ) + .await; + // Compare just the repo metadata (not artifacts) + assert_eq!( + initial_description.repo.hash, + get_repo.hash.into(), + "repo hash matches" + ); + assert_eq!( + initial_description.repo.system_version, get_repo.system_version, + "system version matches" + ); assert_eq!( - initial_description, get_description, - "initial description matches fetched description" + initial_description.repo.valid_until, get_repo.valid_until, + "valid_until matches" ); // Upload a new repository with the same system version but a different @@ -490,22 +494,15 @@ async fn test_repo_upload() -> Result<()> { "artifacts for 1.0.0 and 2.0.0 should match" ); - // Now get the repository that was just uploaded and make sure the - // artifact list is the same. - let response = object_get::( + // Now get the repository that was just uploaded. + let get_repo = object_get::( client, "/v1/system/update/repository/2.0.0", ) .await; - let mut get_description = response.description; - get_description.sort_artifacts(); - - assert_eq!( - description, get_description, - "initial description matches fetched description" - ); - installinator_doc_1 + // Validate the repo metadata + assert_eq!(get_repo.system_version.to_string(), "2.0.0"); }; // The installinator document changed, so the generation number is bumped to // 3. @@ -624,7 +621,7 @@ async fn test_trust_root_operations(cptestctx: &ControlPlaneTestContext) { TestTrustRoot::generate().await.expect("trust root generation failed"); // POST /v1/system/update/trust-roots - let trust_root_view: UpdatesTrustRoot = trust_root + let trust_root_view: views::UpdatesTrustRoot = trust_root .to_upload_request(client, StatusCode::CREATED) .execute() .await @@ -635,20 +632,21 @@ async fn test_trust_root_operations(cptestctx: &ControlPlaneTestContext) { // GET /v1/system/update/trust-roots let request = RequestBuilder::new(client, Method::GET, TRUST_ROOTS_URL) .expect_status(Some(StatusCode::OK)); - let response: ResultsPage = NexusRequest::new(request) - .authn_as(AuthnMode::PrivilegedUser) - .execute() - .await - .expect("trust root list failed") - .parsed_body() - .expect("failed to parse list response"); + let response: ResultsPage = + NexusRequest::new(request) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .expect("trust root list failed") + .parsed_body() + .expect("failed to parse list response"); assert_eq!(response.items, std::slice::from_ref(&trust_root_view.clone())); // GET /v1/system/update/trust-roots/{id} let id_url = format!("{TRUST_ROOTS_URL}/{}", trust_root_view.id); let request = RequestBuilder::new(client, Method::GET, &id_url) .expect_status(Some(StatusCode::OK)); - let response: UpdatesTrustRoot = NexusRequest::new(request) + let response: views::UpdatesTrustRoot = NexusRequest::new(request) .authn_as(AuthnMode::PrivilegedUser) .execute() .await @@ -667,13 +665,14 @@ async fn test_trust_root_operations(cptestctx: &ControlPlaneTestContext) { .expect("trust root delete failed"); let request = RequestBuilder::new(client, Method::GET, TRUST_ROOTS_URL) .expect_status(Some(StatusCode::OK)); - let response: ResultsPage = NexusRequest::new(request) - .authn_as(AuthnMode::PrivilegedUser) - .execute() - .await - .expect("trust root list after delete failed") - .parsed_body() - .expect("failed to parse list after delete response"); + let response: ResultsPage = + NexusRequest::new(request) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .expect("trust root list after delete failed") + .parsed_body() + .expect("failed to parse list after delete response"); assert!(response.items.is_empty()); } @@ -812,7 +811,7 @@ async fn test_repo_list() -> Result<()> { let logctx = &cptestctx.logctx; // Initially, list should be empty - let initial_list: ResultsPage = + let initial_list: ResultsPage = objects_list_page_authz(client, "/v1/system/update/repositories").await; assert_eq!(initial_list.items.len(), 0); assert!(initial_list.next_page.is_none()); @@ -860,20 +859,17 @@ async fn test_repo_list() -> Result<()> { assert_eq!(response3.status, TufRepoInsertStatus::Inserted); // List repositories - should return all 3, ordered by system version (newest first) - let list: ResultsPage = + let list: ResultsPage = objects_list_page_authz(client, "/v1/system/update/repositories").await; assert_eq!(list.items.len(), 3); // Repositories should be ordered by system version descending (newest first) - let system_versions: Vec = list - .items - .iter() - .map(|item| item.description.repo.system_version.to_string()) - .collect(); + let system_versions: Vec = + list.items.iter().map(|item| item.system_version.to_string()).collect(); assert_eq!(system_versions, vec!["3.0.0", "2.0.0", "1.0.0"]); - // Verify that each response contains the correct artifacts + // Verify that each response contains the correct system version for (i, item) in list.items.iter().enumerate() { let expected_version = match i { 0 => "3.0.0", @@ -881,47 +877,27 @@ async fn test_repo_list() -> Result<()> { 2 => "1.0.0", _ => panic!("unexpected index"), }; - assert_eq!( - item.description.repo.system_version.to_string(), - expected_version - ); - - // Verify artifacts are present (should have Zone artifacts) - let zone_artifacts: Vec<_> = item - .description - .artifacts - .iter() - .filter(|artifact| { - artifact.id.kind == KnownArtifactKind::Zone.into() - }) - .collect(); - assert_eq!(zone_artifacts.len(), 2, "should have 2 zone artifacts"); - - // Should not have ControlPlane artifacts (they get decomposed into Zones) - assert!(!item.description.artifacts.iter().any(|artifact| { - artifact.id.kind == KnownArtifactKind::ControlPlane.into() - })); + assert_eq!(item.system_version.to_string(), expected_version); } // Request ascending order and expect the versions oldest-first - let ascending_list: ResultsPage = - objects_list_page_authz( - client, - "/v1/system/update/repositories?sort_by=ascending", - ) - .await; + let ascending_list: ResultsPage = objects_list_page_authz( + client, + "/v1/system/update/repositories?sort_by=ascending", + ) + .await; assert_eq!(ascending_list.items.len(), 3); let ascending_versions: Vec = ascending_list .items .iter() - .map(|item| item.description.repo.system_version.to_string()) + .map(|item| item.system_version.to_string()) .collect(); assert_eq!(ascending_versions, vec!["1.0.0", "2.0.0", "3.0.0"]); // Test pagination by setting a small limit - let paginated_list = objects_list_page_authz::( + let paginated_list = objects_list_page_authz::( client, "/v1/system/update/repositories?limit=2", ) @@ -934,7 +910,7 @@ async fn test_repo_list() -> Result<()> { let paginated_versions: Vec = paginated_list .items .iter() - .map(|item| item.description.repo.system_version.to_string()) + .map(|item| item.system_version.to_string()) .collect(); assert_eq!(paginated_versions, vec!["3.0.0", "2.0.0"]); @@ -943,13 +919,10 @@ async fn test_repo_list() -> Result<()> { "/v1/system/update/repositories?limit=2&page_token={}", paginated_list.next_page.clone().expect("expected next page token"), ); - let next_page: ResultsPage = + let next_page: ResultsPage = objects_list_page_authz(client, &next_page_url).await; assert_eq!(next_page.items.len(), 1); - assert_eq!( - next_page.items[0].description.repo.system_version.to_string(), - "1.0.0" - ); + assert_eq!(next_page.items[0].system_version.to_string(), "1.0.0"); cptestctx.teardown().await; Ok(()) diff --git a/nexus/types/src/external_api/views.rs b/nexus/types/src/external_api/views.rs index 647e0d34ead..67827befad6 100644 --- a/nexus/types/src/external_api/views.rs +++ b/nexus/types/src/external_api/views.rs @@ -15,9 +15,9 @@ use chrono::Utc; use daft::Diffable; pub use omicron_common::api::external::IpVersion; use omicron_common::api::external::{ - AffinityPolicy, AllowedSourceIps as ExternalAllowedSourceIps, ByteCount, - Digest, Error, FailureDomain, IdentityMetadata, InstanceState, Name, - Nullable, ObjectIdentity, SimpleIdentity, SimpleIdentityOrName, + self, AffinityPolicy, AllowedSourceIps as ExternalAllowedSourceIps, + ByteCount, Digest, Error, FailureDomain, IdentityMetadata, InstanceState, + Name, Nullable, ObjectIdentity, SimpleIdentity, SimpleIdentityOrName, }; use omicron_uuid_kinds::*; use oxnet::{Ipv4Net, Ipv6Net}; @@ -30,6 +30,7 @@ use std::fmt; use std::net::IpAddr; use std::sync::LazyLock; use strum::{EnumIter, IntoEnumIterator}; +use tufaceous_artifact::ArtifactHash; use url::Url; use uuid::Uuid; @@ -1590,6 +1591,45 @@ pub struct UpdateStatus { pub paused: bool, } +/// Metadata about a TUF repository +#[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize, JsonSchema)] +pub struct TufRepo { + /// The hash of the repository. + /// + /// This is a slight abuse of `ArtifactHash`, since that's the hash of + /// individual artifacts within the repository. However, we use it here for + /// convenience. + pub hash: ArtifactHash, + + /// The version of the targets role + pub targets_role_version: u64, + + /// The time until which the repo is valid + pub valid_until: DateTime, + + /// The system version in artifacts.json + pub system_version: Version, + + /// The file name of the repository + /// + /// This is purely used for debugging and may not always be correct (e.g., + /// with wicket, we read the file contents from stdin so we don't know the + /// correct file name). + pub file_name: String, +} + +impl From for TufRepo { + fn from(meta: external::TufRepoMeta) -> Self { + Self { + hash: meta.hash, + targets_role_version: meta.targets_role_version, + valid_until: meta.valid_until, + system_version: meta.system_version, + file_name: meta.file_name, + } + } +} + fn expected_one_of() -> String { use std::fmt::Write; let mut msg = "expected one of:".to_string(); diff --git a/openapi/nexus.json b/openapi/nexus.json index 23c6ee4c6a8..c2b96bf19b7 100644 --- a/openapi/nexus.json +++ b/openapi/nexus.json @@ -10946,7 +10946,7 @@ "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/TufRepoGetResponseResultsPage" + "$ref": "#/components/schemas/TufRepoResultsPage" } } } @@ -11038,7 +11038,7 @@ "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/TufRepoGetResponse" + "$ref": "#/components/schemas/TufRepo" } } } @@ -25933,6 +25933,44 @@ "size" ] }, + "TufRepo": { + "description": "Metadata about a TUF repository", + "type": "object", + "properties": { + "file_name": { + "description": "The file name of the repository\n\nThis is purely used for debugging and may not always be correct (e.g., with wicket, we read the file contents from stdin so we don't know the correct file name).", + "type": "string" + }, + "hash": { + "description": "The hash of the repository.\n\nThis is a slight abuse of `ArtifactHash`, since that's the hash of individual artifacts within the repository. However, we use it here for convenience.", + "type": "string", + "format": "hex string (32 bytes)" + }, + "system_version": { + "description": "The system version in artifacts.json", + "type": "string", + "pattern": "^(0|[1-9]\\d*)\\.(0|[1-9]\\d*)\\.(0|[1-9]\\d*)(?:-((?:0|[1-9]\\d*|\\d*[a-zA-Z-][0-9a-zA-Z-]*)(?:\\.(?:0|[1-9]\\d*|\\d*[a-zA-Z-][0-9a-zA-Z-]*))*))?(?:\\+([0-9a-zA-Z-]+(?:\\.[0-9a-zA-Z-]+)*))?$" + }, + "targets_role_version": { + "description": "The version of the targets role", + "type": "integer", + "format": "uint64", + "minimum": 0 + }, + "valid_until": { + "description": "The time until which the repo is valid", + "type": "string", + "format": "date-time" + } + }, + "required": [ + "file_name", + "hash", + "system_version", + "targets_role_version", + "valid_until" + ] + }, "TufRepoDescription": { "description": "A description of an uploaded TUF repository.", "type": "object", @@ -25958,44 +25996,6 @@ "repo" ] }, - "TufRepoGetResponse": { - "description": "Data about a successful TUF repo get from Nexus.", - "type": "object", - "properties": { - "description": { - "description": "The description of the repository.", - "allOf": [ - { - "$ref": "#/components/schemas/TufRepoDescription" - } - ] - } - }, - "required": [ - "description" - ] - }, - "TufRepoGetResponseResultsPage": { - "description": "A single page of results", - "type": "object", - "properties": { - "items": { - "description": "list of items on this page of results", - "type": "array", - "items": { - "$ref": "#/components/schemas/TufRepoGetResponse" - } - }, - "next_page": { - "nullable": true, - "description": "token used to fetch the next page of results (if any)", - "type": "string" - } - }, - "required": [ - "items" - ] - }, "TufRepoInsertResponse": { "description": "Data about a successful TUF repo import into Nexus.", "type": "object", @@ -26079,6 +26079,27 @@ "valid_until" ] }, + "TufRepoResultsPage": { + "description": "A single page of results", + "type": "object", + "properties": { + "items": { + "description": "list of items on this page of results", + "type": "array", + "items": { + "$ref": "#/components/schemas/TufRepo" + } + }, + "next_page": { + "nullable": true, + "description": "token used to fetch the next page of results (if any)", + "type": "string" + } + }, + "required": [ + "items" + ] + }, "TxEqConfig": { "description": "Per-port tx-eq overrides. This can be used to fine-tune the transceiver equalization settings to improve signal integrity.", "type": "object", From 8cc58c1aad7396db87f87caef4cde5d6148734df Mon Sep 17 00:00:00 2001 From: David Crespo Date: Fri, 26 Sep 2025 11:32:30 -0500 Subject: [PATCH 13/31] replace TufRepoInsertResponse with views::TufRepoUpload (tests fail) --- common/src/api/external/mod.rs | 18 +- nexus/db-queries/src/db/datastore/mod.rs | 2 +- nexus/db-queries/src/db/datastore/update.rs | 9 +- nexus/external-api/src/lib.rs | 2 +- nexus/src/app/update.rs | 5 +- nexus/src/external_api/http_entrypoints.rs | 9 +- .../tests/integration_tests/target_release.rs | 17 +- nexus/tests/integration_tests/updates.rs | 71 +++---- nexus/types/src/external_api/views.rs | 29 +++ openapi/nexus.json | 182 ++---------------- 10 files changed, 106 insertions(+), 238 deletions(-) diff --git a/common/src/api/external/mod.rs b/common/src/api/external/mod.rs index 00d252053c1..bf382db8f2a 100644 --- a/common/src/api/external/mod.rs +++ b/common/src/api/external/mod.rs @@ -3426,6 +3426,10 @@ pub struct ServiceIcmpConfig { pub enabled: bool, } +// TODO: move these TUF repo structs out of this file. They're not external +// anymore after refactors that use views::TufRepo in the external API. They are +// still used extensively in internal services. + /// A description of an uploaded TUF repository. #[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize, JsonSchema)] pub struct TufRepoDescription { @@ -3500,20 +3504,7 @@ pub struct TufArtifactMeta { pub sign: Option>, } -/// Data about a successful TUF repo import into Nexus. -#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema)] -#[serde(rename_all = "snake_case")] -pub struct TufRepoInsertResponse { - /// The repository as present in the database. - pub recorded: TufRepoDescription, - - /// Whether this repository already existed or is new. - pub status: TufRepoInsertStatus, -} - /// Status of a TUF repo import. -/// -/// Part of `TufRepoInsertResponse`. #[derive( Debug, Clone, Copy, PartialEq, Eq, Deserialize, Serialize, JsonSchema, )] @@ -3526,7 +3517,6 @@ pub enum TufRepoInsertStatus { Inserted, } - #[derive( Clone, Debug, Deserialize, JsonSchema, Serialize, PartialEq, ObjectIdentity, )] diff --git a/nexus/db-queries/src/db/datastore/mod.rs b/nexus/db-queries/src/db/datastore/mod.rs index eaff385e7e1..3de51674720 100644 --- a/nexus/db-queries/src/db/datastore/mod.rs +++ b/nexus/db-queries/src/db/datastore/mod.rs @@ -111,7 +111,7 @@ mod switch_port; mod target_release; #[cfg(test)] pub(crate) mod test_utils; -mod update; +pub mod update; mod user_data_export; mod utilization; mod v2p_mapping; diff --git a/nexus/db-queries/src/db/datastore/update.rs b/nexus/db-queries/src/db/datastore/update.rs index 87c18053975..95ead5b2db2 100644 --- a/nexus/db-queries/src/db/datastore/update.rs +++ b/nexus/db-queries/src/db/datastore/update.rs @@ -24,6 +24,7 @@ use nexus_db_model::{ ArtifactHash, DbTypedUuid, TargetRelease, TufArtifact, TufRepo, TufRepoDescription, TufTrustRoot, to_db_typed_uuid, }; +use nexus_types::external_api::views; use omicron_common::api::external::{ self, CreateResult, DataPageParams, DeleteResult, Generation, ListResultVec, LookupResult, LookupType, ResourceType, TufRepoInsertStatus, @@ -48,10 +49,10 @@ pub struct TufRepoInsertResponse { } impl TufRepoInsertResponse { - pub fn into_external(self) -> external::TufRepoInsertResponse { - external::TufRepoInsertResponse { - recorded: self.recorded.into_external(), - status: self.status, + pub fn into_external(self) -> views::TufRepoUpload { + views::TufRepoUpload { + repo: self.recorded.repo.into_external().into(), + status: self.status.into(), } } } diff --git a/nexus/external-api/src/lib.rs b/nexus/external-api/src/lib.rs index b8ffcaf52ba..548b2995be2 100644 --- a/nexus/external-api/src/lib.rs +++ b/nexus/external-api/src/lib.rs @@ -2975,7 +2975,7 @@ pub trait NexusExternalApi { rqctx: RequestContext, query: Query, body: StreamingBody, - ) -> Result, HttpError>; + ) -> Result, HttpError>; /// Fetch system release repository description by version #[endpoint { diff --git a/nexus/src/app/update.rs b/nexus/src/app/update.rs index c7ba5d2381f..c367baada4b 100644 --- a/nexus/src/app/update.rs +++ b/nexus/src/app/update.rs @@ -11,6 +11,7 @@ use nexus_auth::authz; use nexus_db_lookup::LookupPath; use nexus_db_model::{Generation, TufTrustRoot}; use nexus_db_queries::context::OpContext; +use nexus_db_queries::db::datastore::update::TufRepoInsertResponse; use nexus_db_queries::db::{datastore::SQL_BATCH_SIZE, pagination::Paginator}; use nexus_types::deployment::TargetReleaseDescription; use nexus_types::external_api::shared::TufSignedRootRole; @@ -20,7 +21,7 @@ use nexus_types::inventory::RotSlot; use omicron_common::api::external::InternalContext; use omicron_common::api::external::Nullable; use omicron_common::api::external::{ - DataPageParams, Error, TufRepoInsertResponse, TufRepoInsertStatus, + DataPageParams, Error, TufRepoInsertStatus, }; use omicron_common::disk::M2Slot; use omicron_uuid_kinds::{GenericUuid, TufTrustRootUuid}; @@ -95,7 +96,7 @@ impl super::Nexus { self.background_tasks.task_tuf_artifact_replication.activate(); } - Ok(response.into_external()) + Ok(response) } pub(crate) async fn updates_get_repository( diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index f6785ef4b55..650ad6dc239 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -85,7 +85,6 @@ use omicron_common::api::external::ServiceIcmpConfig; use omicron_common::api::external::SwitchPort; use omicron_common::api::external::SwitchPortSettings; use omicron_common::api::external::SwitchPortSettingsIdentity; -use omicron_common::api::external::TufRepoInsertResponse; use omicron_common::api::external::VpcFirewallRuleUpdateParams; use omicron_common::api::external::VpcFirewallRules; use omicron_common::api::external::http_pagination::PaginatedBy; @@ -6644,7 +6643,7 @@ impl NexusExternalApi for NexusExternalApiImpl { rqctx: RequestContext, query: Query, body: StreamingBody, - ) -> Result, HttpError> { + ) -> Result, HttpError> { let apictx = rqctx.context(); let nexus = &apictx.context.nexus; let handler = async { @@ -6655,7 +6654,8 @@ impl NexusExternalApi for NexusExternalApiImpl { let update = nexus .updates_put_repository(&opctx, body, query.file_name) .await?; - Ok(HttpResponseOk(update)) + let repo_upload = update.into_external(); + Ok(HttpResponseOk(repo_upload)) }; apictx .context @@ -6689,8 +6689,7 @@ impl NexusExternalApi for NexusExternalApiImpl { async fn system_update_repository_list( rqctx: RequestContext, query_params: Query, - ) -> Result>, HttpError> - { + ) -> Result>, HttpError> { let apictx = rqctx.context(); let nexus = &apictx.context.nexus; let handler = async { diff --git a/nexus/tests/integration_tests/target_release.rs b/nexus/tests/integration_tests/target_release.rs index 813d779cd2a..a3755fb5406 100644 --- a/nexus/tests/integration_tests/target_release.rs +++ b/nexus/tests/integration_tests/target_release.rs @@ -14,8 +14,7 @@ use nexus_test_utils::http_testing::{NexusRequest, RequestBuilder}; use nexus_test_utils::resource_helpers::object_get; use nexus_test_utils::test_setup; use nexus_types::external_api::params::SetTargetReleaseParams; -use nexus_types::external_api::views; -use omicron_common::api::external::TufRepoInsertResponse; +use nexus_types::external_api::views::{TufRepoUpload, UpdateStatus}; use semver::Version; use tufaceous_artifact::{ArtifactVersion, KnownArtifactKind}; use tufaceous_lib::assemble::ManifestTweak; @@ -30,7 +29,7 @@ async fn get_set_target_release() -> Result<()> { let logctx = &ctx.logctx; // There is no target release before one has ever been specified - let status: views::UpdateStatus = + let status: UpdateStatus = object_get(client, "/v1/system/update/status").await; assert_eq!(status.target_release.0, None); @@ -54,18 +53,18 @@ async fn get_set_target_release() -> Result<()> { { let before = Utc::now(); let system_version = Version::new(1, 0, 0); - let response: TufRepoInsertResponse = trust_root + let response: TufRepoUpload = trust_root .assemble_repo(&logctx.log, &[]) .await? .into_upload_request(client, StatusCode::OK) .execute() .await? .parsed_body()?; - assert_eq!(system_version, response.recorded.repo.system_version); + assert_eq!(system_version, response.repo.system_version); set_target_release(client, &system_version).await?; - let status: views::UpdateStatus = + let status: UpdateStatus = object_get(client, "/v1/system/update/status").await; let target_release = status.target_release.0.unwrap(); @@ -86,18 +85,18 @@ async fn get_set_target_release() -> Result<()> { version: ArtifactVersion::new("non-semver-2").unwrap(), }, ]; - let response: TufRepoInsertResponse = trust_root + let response: TufRepoUpload = trust_root .assemble_repo(&logctx.log, tweaks) .await? .into_upload_request(client, StatusCode::OK) .execute() .await? .parsed_body()?; - assert_eq!(system_version, response.recorded.repo.system_version); + assert_eq!(system_version, response.repo.system_version); set_target_release(client, &system_version).await?; - let status: views::UpdateStatus = + let status: UpdateStatus = object_get(client, "/v1/system/update/status").await; let target_release = status.target_release.0.unwrap(); diff --git a/nexus/tests/integration_tests/updates.rs b/nexus/tests/integration_tests/updates.rs index 4d9238487e7..aa28cc0b362 100644 --- a/nexus/tests/integration_tests/updates.rs +++ b/nexus/tests/integration_tests/updates.rs @@ -20,9 +20,7 @@ use nexus_test_utils::resource_helpers::objects_list_page_authz; use nexus_test_utils::test_setup; use nexus_test_utils_macros::nexus_test; use nexus_types::external_api::views; -use omicron_common::api::external::{ - TufRepoInsertResponse, TufRepoInsertStatus, -}; +use nexus_types::external_api::views::{TufRepoUpload, TufRepoUploadStatus}; use pretty_assertions::assert_eq; use semver::Version; use serde::Deserialize; @@ -205,20 +203,19 @@ async fn test_repo_upload() -> Result<()> { let repo = trust_root.assemble_repo(&logctx.log, &[]).await?; // Generate a repository and upload it to Nexus. - let mut initial_description = { + let mut initial_repo = { let response = repo .to_upload_request(client, StatusCode::OK) .execute() .await .context("error uploading repository")?; - let response = - serde_json::from_slice::(&response.body) - .context("error deserializing response body")?; - assert_eq!(response.status, TufRepoInsertStatus::Inserted); - response.recorded + let response = serde_json::from_slice::(&response.body) + .context("error deserializing response body")?; + assert_eq!(response.status, TufRepoUploadStatus::Inserted); + response.repo }; - let unique_sha256_count = initial_description + let unique_sha256_count = initial_repo .artifacts .iter() .map(|artifact| artifact.hash) @@ -227,7 +224,7 @@ async fn test_repo_upload() -> Result<()> { // The repository description should have `Zone` artifacts instead of the // composite `ControlPlane` artifact. assert_eq!( - initial_description + initial_repo .artifacts .iter() .filter_map(|artifact| { @@ -240,7 +237,7 @@ async fn test_repo_upload() -> Result<()> { .collect::>(), ["zone-1", "zone-2"] ); - assert!(!initial_description.artifacts.iter().any(|artifact| { + assert!(!initial_repo.artifacts.iter().any(|artifact| { artifact.id.kind == KnownArtifactKind::ControlPlane.into() })); // The generation number should now be 2. @@ -287,18 +284,17 @@ async fn test_repo_upload() -> Result<()> { .await .context("error uploading repository a second time")?; - let response = - serde_json::from_slice::(&response.body) - .context("error deserializing response body")?; - assert_eq!(response.status, TufRepoInsertStatus::AlreadyExists); - response.recorded + let response = serde_json::from_slice::(&response.body) + .context("error deserializing response body")?; + assert_eq!(response.status, TufRepoUploadStatus::AlreadyExists); + response.repo }; - initial_description.sort_artifacts(); + initial_repo.sort_artifacts(); reupload_description.sort_artifacts(); assert_eq!( - initial_description, reupload_description, + initial_repo, reupload_description, "initial description matches reupload" ); @@ -309,24 +305,20 @@ async fn test_repo_upload() -> Result<()> { ); // Now get the repository that was just uploaded. - let get_repo = object_get::( + let repo = object_get::( client, "/v1/system/update/repository/1.0.0", ) .await; // Compare just the repo metadata (not artifacts) + assert_eq!(initial_repo.hash, repo.hash, "repo hash matches"); assert_eq!( - initial_description.repo.hash, - get_repo.hash.into(), - "repo hash matches" - ); - assert_eq!( - initial_description.repo.system_version, get_repo.system_version, + initial_repo.system_version, repo.system_version, "system version matches" ); assert_eq!( - initial_description.repo.valid_until, get_repo.valid_until, + initial_repo.valid_until, repo.valid_until, "valid_until matches" ); @@ -441,18 +433,17 @@ async fn test_repo_upload() -> Result<()> { (should succeed)", )?; - let response = - serde_json::from_slice::(&response.body) - .context("error deserializing response body")?; - assert_eq!(response.status, TufRepoInsertStatus::Inserted); - let mut description = response.recorded; + let response = serde_json::from_slice::(&response.body) + .context("error deserializing response body")?; + assert_eq!(response.status, TufRepoUploadStatus::Inserted); + let mut description = response.repo; description.sort_artifacts(); // The artifacts should be exactly the same as the 1.0.0 repo we // uploaded, other than the installinator document (which will have // system version 2.0.0). let mut installinator_doc_1 = None; - let filtered_artifacts_1 = initial_description + let filtered_artifacts_1 = initial_repo .artifacts .iter() .filter(|artifact| { @@ -828,9 +819,9 @@ async fn test_repo_list() -> Result<()> { .await .context("error uploading first repository")?; let response1 = - serde_json::from_slice::(&upload_response1.body) + serde_json::from_slice::(&upload_response1.body) .context("error deserializing first response body")?; - assert_eq!(response1.status, TufRepoInsertStatus::Inserted); + assert_eq!(response1.status, TufRepoUploadStatus::Inserted); // Upload second repository (system version 2.0.0) let tweaks = &[ManifestTweak::SystemVersion("2.0.0".parse().unwrap())]; @@ -841,9 +832,9 @@ async fn test_repo_list() -> Result<()> { .await .context("error uploading second repository")?; let response2 = - serde_json::from_slice::(&upload_response2.body) + serde_json::from_slice::(&upload_response2.body) .context("error deserializing second response body")?; - assert_eq!(response2.status, TufRepoInsertStatus::Inserted); + assert_eq!(response2.status, TufRepoUploadStatus::Inserted); // Upload third repository (system version 3.0.0) let tweaks = &[ManifestTweak::SystemVersion("3.0.0".parse().unwrap())]; @@ -854,9 +845,9 @@ async fn test_repo_list() -> Result<()> { .await .context("error uploading third repository")?; let response3 = - serde_json::from_slice::(&upload_response3.body) + serde_json::from_slice::(&upload_response3.body) .context("error deserializing third response body")?; - assert_eq!(response3.status, TufRepoInsertStatus::Inserted); + assert_eq!(response3.status, TufRepoUploadStatus::Inserted); // List repositories - should return all 3, ordered by system version (newest first) let list: ResultsPage = @@ -883,7 +874,7 @@ async fn test_repo_list() -> Result<()> { // Request ascending order and expect the versions oldest-first let ascending_list: ResultsPage = objects_list_page_authz( client, - "/v1/system/update/repositories?sort_by=ascending", + "/v1/system/update/repositories?sort_by=version_ascending", ) .await; diff --git a/nexus/types/src/external_api/views.rs b/nexus/types/src/external_api/views.rs index 67827befad6..3c85a07dffe 100644 --- a/nexus/types/src/external_api/views.rs +++ b/nexus/types/src/external_api/views.rs @@ -1630,6 +1630,35 @@ impl From for TufRepo { } } +#[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize, JsonSchema)] +pub struct TufRepoUpload { + pub repo: TufRepo, + pub status: TufRepoUploadStatus, +} + +/// Whether the uploaded TUF repo already existed or was new and had to be +/// inserted. Part of `TufRepoUpload`. +#[derive( + Debug, Clone, Copy, PartialEq, Eq, Deserialize, Serialize, JsonSchema, +)] +#[serde(rename_all = "snake_case")] +pub enum TufRepoUploadStatus { + /// The repository already existed in the database + AlreadyExists, + + /// The repository did not exist, and was inserted into the database + Inserted, +} + +impl From for TufRepoUploadStatus { + fn from(status: external::TufRepoInsertStatus) -> Self { + match status { + external::TufRepoInsertStatus::AlreadyExists => Self::AlreadyExists, + external::TufRepoInsertStatus::Inserted => Self::Inserted, + } + } +} + fn expected_one_of() -> String { use std::fmt::Write; let mut msg = "expected one of:".to_string(); diff --git a/openapi/nexus.json b/openapi/nexus.json index c2b96bf19b7..36be0a3861a 100644 --- a/openapi/nexus.json +++ b/openapi/nexus.json @@ -10999,7 +10999,7 @@ "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/TufRepoInsertResponse" + "$ref": "#/components/schemas/TufRepoUpload" } } } @@ -14648,29 +14648,6 @@ } } }, - "ArtifactId": { - "description": "An identifier for an artifact.", - "type": "object", - "properties": { - "kind": { - "description": "The kind of artifact this is.", - "type": "string" - }, - "name": { - "description": "The artifact's name.", - "type": "string" - }, - "version": { - "description": "The artifact's version.", - "type": "string" - } - }, - "required": [ - "kind", - "name", - "version" - ] - }, "AuditLogEntry": { "description": "Audit log entry", "type": "object", @@ -25888,51 +25865,6 @@ "items" ] }, - "TufArtifactMeta": { - "description": "Metadata about an individual TUF artifact.\n\nFound within a `TufRepoDescription`.", - "type": "object", - "properties": { - "board": { - "nullable": true, - "description": "Contents of the `BORD` field of a Hubris archive caboose. Only applicable to artifacts that are Hubris archives.\n\nThis field should always be `Some(_)` if `sign` is `Some(_)`, but the opposite is not true (SP images will have a `board` but not a `sign`).", - "type": "string" - }, - "hash": { - "description": "The hash of the artifact.", - "type": "string", - "format": "hex string (32 bytes)" - }, - "id": { - "description": "The artifact ID.", - "allOf": [ - { - "$ref": "#/components/schemas/ArtifactId" - } - ] - }, - "sign": { - "nullable": true, - "description": "Contents of the `SIGN` field of a Hubris archive caboose, i.e., an identifier for the set of valid signing keys. Currently only applicable to RoT image and bootloader artifacts, where it will be an LPC55 Root Key Table Hash (RKTH).", - "type": "array", - "items": { - "type": "integer", - "format": "uint8", - "minimum": 0 - } - }, - "size": { - "description": "The size of the artifact in bytes.", - "type": "integer", - "format": "uint64", - "minimum": 0 - } - }, - "required": [ - "hash", - "id", - "size" - ] - }, "TufRepo": { "description": "Metadata about a TUF repository", "type": "object", @@ -25971,69 +25903,54 @@ "valid_until" ] }, - "TufRepoDescription": { - "description": "A description of an uploaded TUF repository.", + "TufRepoResultsPage": { + "description": "A single page of results", "type": "object", "properties": { - "artifacts": { - "description": "Information about the artifacts present in the repository.", + "items": { + "description": "list of items on this page of results", "type": "array", "items": { - "$ref": "#/components/schemas/TufArtifactMeta" + "$ref": "#/components/schemas/TufRepo" } }, - "repo": { - "description": "Information about the repository.", - "allOf": [ - { - "$ref": "#/components/schemas/TufRepoMeta" - } - ] + "next_page": { + "nullable": true, + "description": "token used to fetch the next page of results (if any)", + "type": "string" } }, "required": [ - "artifacts", - "repo" + "items" ] }, - "TufRepoInsertResponse": { - "description": "Data about a successful TUF repo import into Nexus.", + "TufRepoUpload": { "type": "object", "properties": { - "recorded": { - "description": "The repository as present in the database.", - "allOf": [ - { - "$ref": "#/components/schemas/TufRepoDescription" - } - ] + "repo": { + "$ref": "#/components/schemas/TufRepo" }, "status": { - "description": "Whether this repository already existed or is new.", - "allOf": [ - { - "$ref": "#/components/schemas/TufRepoInsertStatus" - } - ] + "$ref": "#/components/schemas/TufRepoUploadStatus" } }, "required": [ - "recorded", + "repo", "status" ] }, - "TufRepoInsertStatus": { - "description": "Status of a TUF repo import.\n\nPart of `TufRepoInsertResponse`.", + "TufRepoUploadStatus": { + "description": "Whether the uploaded TUF repo already existed or was new and had to be inserted. Part of `TufRepoUpload`.", "oneOf": [ { - "description": "The repository already existed in the database.", + "description": "The repository already existed in the database", "type": "string", "enum": [ "already_exists" ] }, { - "description": "The repository did not exist, and was inserted into the database.", + "description": "The repository did not exist, and was inserted into the database", "type": "string", "enum": [ "inserted" @@ -26041,65 +25958,6 @@ } ] }, - "TufRepoMeta": { - "description": "Metadata about a TUF repository.\n\nFound within a `TufRepoDescription`.", - "type": "object", - "properties": { - "file_name": { - "description": "The file name of the repository.\n\nThis is purely used for debugging and may not always be correct (e.g. with wicket, we read the file contents from stdin so we don't know the correct file name).", - "type": "string" - }, - "hash": { - "description": "The hash of the repository.\n\nThis is a slight abuse of `ArtifactHash`, since that's the hash of individual artifacts within the repository. However, we use it here for convenience.", - "type": "string", - "format": "hex string (32 bytes)" - }, - "system_version": { - "description": "The system version in artifacts.json.", - "type": "string", - "pattern": "^(0|[1-9]\\d*)\\.(0|[1-9]\\d*)\\.(0|[1-9]\\d*)(?:-((?:0|[1-9]\\d*|\\d*[a-zA-Z-][0-9a-zA-Z-]*)(?:\\.(?:0|[1-9]\\d*|\\d*[a-zA-Z-][0-9a-zA-Z-]*))*))?(?:\\+([0-9a-zA-Z-]+(?:\\.[0-9a-zA-Z-]+)*))?$" - }, - "targets_role_version": { - "description": "The version of the targets role.", - "type": "integer", - "format": "uint64", - "minimum": 0 - }, - "valid_until": { - "description": "The time until which the repo is valid.", - "type": "string", - "format": "date-time" - } - }, - "required": [ - "file_name", - "hash", - "system_version", - "targets_role_version", - "valid_until" - ] - }, - "TufRepoResultsPage": { - "description": "A single page of results", - "type": "object", - "properties": { - "items": { - "description": "list of items on this page of results", - "type": "array", - "items": { - "$ref": "#/components/schemas/TufRepo" - } - }, - "next_page": { - "nullable": true, - "description": "token used to fetch the next page of results (if any)", - "type": "string" - } - }, - "required": [ - "items" - ] - }, "TxEqConfig": { "description": "Per-port tx-eq overrides. This can be used to fine-tune the transceiver equalization settings to improve signal integrity.", "type": "object", From 0ddef05597815dd5b523e2ccbda19795271f09b7 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Fri, 26 Sep 2025 14:46:04 -0500 Subject: [PATCH 14/31] add lockstep endpoint for listing artifacts for a repo --- nexus/db-queries/src/db/datastore/update.rs | 24 +++ nexus/lockstep-api/src/lib.rs | 18 +++ nexus/src/lockstep_api/http_entrypoints.rs | 48 ++++++ openapi/nexus-lockstep.json | 153 ++++++++++++++++++++ 4 files changed, 243 insertions(+) diff --git a/nexus/db-queries/src/db/datastore/update.rs b/nexus/db-queries/src/db/datastore/update.rs index 95ead5b2db2..d3d596096eb 100644 --- a/nexus/db-queries/src/db/datastore/update.rs +++ b/nexus/db-queries/src/db/datastore/update.rs @@ -485,6 +485,30 @@ impl DataStore { .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server)) } + /// List artifacts for a specific TUF repository by system version. + pub async fn tuf_repo_artifacts_list_by_version( + &self, + opctx: &OpContext, + system_version: SemverVersion, + _pagparams: &DataPageParams<'_, Uuid>, + ) -> ListResultVec { + opctx.authorize(authz::Action::Read, &authz::FLEET).await?; + + let conn = self.pool_connection_authorized(opctx).await?; + + // First get the repo by version + let repo = self.tuf_repo_get_by_version(opctx, system_version).await?; + + // Get all artifacts for this repo and apply simple pagination + let all_artifacts = artifacts_for_repo(repo.id.into(), &conn) + .await + .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server))?; + + // For now, return all artifacts since each repo should only have a few (under 20) + // The existing artifacts_for_repo comment mentions this limitation + Ok(all_artifacts) + } + /// List the trusted TUF root roles in the trust store. pub async fn tuf_trust_root_list( &self, diff --git a/nexus/lockstep-api/src/lib.rs b/nexus/lockstep-api/src/lib.rs index 43b54ac9260..4638c0e43ff 100644 --- a/nexus/lockstep-api/src/lib.rs +++ b/nexus/lockstep-api/src/lib.rs @@ -44,6 +44,7 @@ use nexus_types::internal_api::views::QuiesceStatus; use nexus_types::internal_api::views::Saga; use nexus_types::internal_api::views::UpdateStatus; use omicron_common::api::external::Instance; +use omicron_common::api::external::TufArtifactMeta; use omicron_common::api::external::http_pagination::PaginatedById; use omicron_common::api::external::http_pagination::PaginatedByTimeAndId; use omicron_uuid_kinds::*; @@ -304,6 +305,17 @@ pub trait NexusLockstepApi { rqctx: RequestContext, ) -> Result, HttpError>; + /// List artifacts for a TUF repository by version + #[endpoint { + method = GET, + path = "/deployment/repositories/{version}/artifacts" + }] + async fn tuf_repo_artifacts_list( + rqctx: RequestContext, + path_params: Path, + query_params: Query, + ) -> Result>, HttpError>; + /// List uninitialized sleds #[endpoint { method = GET, @@ -570,3 +582,9 @@ pub struct SledId { pub struct VersionPathParam { pub version: u32, } + +/// Path parameters for TUF repository version requests +#[derive(Deserialize, JsonSchema)] +pub struct TufRepoVersionPathParam { + pub version: String, +} diff --git a/nexus/src/lockstep_api/http_entrypoints.rs b/nexus/src/lockstep_api/http_entrypoints.rs index 463b0883deb..327d12fb3df 100644 --- a/nexus/src/lockstep_api/http_entrypoints.rs +++ b/nexus/src/lockstep_api/http_entrypoints.rs @@ -49,6 +49,7 @@ use nexus_types::internal_api::views::Saga; use nexus_types::internal_api::views::UpdateStatus; use nexus_types::internal_api::views::to_list; use omicron_common::api::external::Instance; +use omicron_common::api::external::TufArtifactMeta; use omicron_common::api::external::http_pagination::PaginatedById; use omicron_common::api::external::http_pagination::PaginatedByTimeAndId; use omicron_common::api::external::http_pagination::ScanById; @@ -57,6 +58,8 @@ use omicron_common::api::external::http_pagination::ScanParams; use omicron_common::api::external::http_pagination::data_page_params_for; use omicron_uuid_kinds::*; use range_requests::PotentialRange; +use semver::Version; +use nexus_db_model::SemverVersion; use crate::app::support_bundles::SupportBundleQueryType; use crate::context::ApiContext; @@ -501,6 +504,51 @@ impl NexusLockstepApi for NexusLockstepApiImpl { .await } + async fn tuf_repo_artifacts_list( + rqctx: RequestContext, + path_params: Path, + query_params: Query, + ) -> Result>, HttpError> { + let apictx = &rqctx.context().context; + let handler = async { + let opctx = + crate::context::op_context_for_internal_api(&rqctx).await; + let nexus = &apictx.nexus; + let datastore = nexus.datastore(); + let path = path_params.into_inner(); + let query = query_params.into_inner(); + let pagparams = data_page_params_for(&rqctx, &query)?; + + // Parse the version string as semver + let system_version = Version::parse(&path.version) + .map_err(|e| HttpError::for_bad_request( + None, + format!("Invalid version '{}': {}", path.version, e), + ))?; + let semver_version = SemverVersion::from(system_version); + + let artifacts = datastore + .tuf_repo_artifacts_list_by_version(&opctx, semver_version, &pagparams) + .await?; + + // Convert TufArtifact to TufArtifactMeta + let artifact_metas: Vec = artifacts + .into_iter() + .map(|artifact| artifact.into_external()) + .collect(); + + // Since TufArtifactMeta.id is not a UUID but a composite ArtifactId, + // and we don't have a direct UUID for artifacts, we'll return all + // artifacts without pagination for now (which is fine since each + // repo should only have a few artifacts under 20) + Ok(HttpResponseOk(ResultsPage { items: artifact_metas, next_page: None })) + }; + apictx + .internal_latencies + .instrument_dropshot_handler(&rqctx, handler) + .await + } + async fn sled_list_uninitialized( rqctx: RequestContext, ) -> Result>, HttpError> { diff --git a/openapi/nexus-lockstep.json b/openapi/nexus-lockstep.json index 9136d7bc60f..236d3b72fd4 100644 --- a/openapi/nexus-lockstep.json +++ b/openapi/nexus-lockstep.json @@ -551,6 +551,70 @@ } } }, + "/deployment/repositories/{version}/artifacts": { + "get": { + "summary": "List artifacts for a TUF repository by version", + "operationId": "tuf_repo_artifacts_list", + "parameters": [ + { + "in": "path", + "name": "version", + "required": true, + "schema": { + "type": "string" + } + }, + { + "in": "query", + "name": "limit", + "description": "Maximum number of items returned by a single call", + "schema": { + "nullable": true, + "type": "integer", + "format": "uint32", + "minimum": 1 + } + }, + { + "in": "query", + "name": "page_token", + "description": "Token returned by previous call to retrieve the subsequent page", + "schema": { + "nullable": true, + "type": "string" + } + }, + { + "in": "query", + "name": "sort_by", + "schema": { + "$ref": "#/components/schemas/IdSortMode" + } + } + ], + "responses": { + "200": { + "description": "successful operation", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/TufArtifactMetaResultsPage" + } + } + } + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + }, + "x-dropshot-pagination": { + "required": [] + } + } + }, "/deployment/update-status": { "get": { "summary": "Show deployed versions of artifacts", @@ -1370,6 +1434,29 @@ "dependency" ] }, + "ArtifactId": { + "description": "An identifier for an artifact.", + "type": "object", + "properties": { + "kind": { + "description": "The kind of artifact this is.", + "type": "string" + }, + "name": { + "description": "The artifact's name.", + "type": "string" + }, + "version": { + "description": "The artifact's version.", + "type": "string" + } + }, + "required": [ + "kind", + "name", + "version" + ] + }, "ArtifactVersion": { "description": "An artifact version.\n\nThis is a freeform identifier with some basic validation. It may be the serialized form of a semver version, or a custom identifier that uses the same character set as a semver, plus `_`.\n\nThe exact pattern accepted is `^[a-zA-Z0-9._+-]{1,63}$`.\n\n# Ord implementation\n\n`ArtifactVersion`s are not intended to be sorted, just compared for equality. `ArtifactVersion` implements `Ord` only for storage within sorted collections.", "type": "string", @@ -7490,6 +7577,72 @@ } } }, + "TufArtifactMeta": { + "description": "Metadata about an individual TUF artifact.\n\nFound within a `TufRepoDescription`.", + "type": "object", + "properties": { + "board": { + "nullable": true, + "description": "Contents of the `BORD` field of a Hubris archive caboose. Only applicable to artifacts that are Hubris archives.\n\nThis field should always be `Some(_)` if `sign` is `Some(_)`, but the opposite is not true (SP images will have a `board` but not a `sign`).", + "type": "string" + }, + "hash": { + "description": "The hash of the artifact.", + "type": "string", + "format": "hex string (32 bytes)" + }, + "id": { + "description": "The artifact ID.", + "allOf": [ + { + "$ref": "#/components/schemas/ArtifactId" + } + ] + }, + "sign": { + "nullable": true, + "description": "Contents of the `SIGN` field of a Hubris archive caboose, i.e., an identifier for the set of valid signing keys. Currently only applicable to RoT image and bootloader artifacts, where it will be an LPC55 Root Key Table Hash (RKTH).", + "type": "array", + "items": { + "type": "integer", + "format": "uint8", + "minimum": 0 + } + }, + "size": { + "description": "The size of the artifact in bytes.", + "type": "integer", + "format": "uint64", + "minimum": 0 + } + }, + "required": [ + "hash", + "id", + "size" + ] + }, + "TufArtifactMetaResultsPage": { + "description": "A single page of results", + "type": "object", + "properties": { + "items": { + "description": "list of items on this page of results", + "type": "array", + "items": { + "$ref": "#/components/schemas/TufArtifactMeta" + } + }, + "next_page": { + "nullable": true, + "description": "token used to fetch the next page of results (if any)", + "type": "string" + } + }, + "required": [ + "items" + ] + }, "TufRepoVersion": { "oneOf": [ { From 78373e274dce41c7fcc1ef46be7692e404887196 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Fri, 26 Sep 2025 15:28:35 -0500 Subject: [PATCH 15/31] make repo upload test pass using artifacts endpoint --- nexus/tests/integration_tests/updates.rs | 103 +++++++++++++++-------- 1 file changed, 69 insertions(+), 34 deletions(-) diff --git a/nexus/tests/integration_tests/updates.rs b/nexus/tests/integration_tests/updates.rs index aa28cc0b362..04c7672f631 100644 --- a/nexus/tests/integration_tests/updates.rs +++ b/nexus/tests/integration_tests/updates.rs @@ -21,6 +21,7 @@ use nexus_test_utils::test_setup; use nexus_test_utils_macros::nexus_test; use nexus_types::external_api::views; use nexus_types::external_api::views::{TufRepoUpload, TufRepoUploadStatus}; +use omicron_common::api::external::TufArtifactMeta; use pretty_assertions::assert_eq; use semver::Version; use serde::Deserialize; @@ -41,6 +42,17 @@ const TRUST_ROOTS_URL: &str = "/v1/system/update/trust-roots"; type ControlPlaneTestContext = nexus_test_utils::ControlPlaneTestContext; +/// Fetch artifacts for a repository using the lockstep API +async fn fetch_repo_artifacts( + lockstep_client: &dropshot::test_util::ClientTestContext, + version: &str, +) -> Vec { + let url = format!("/deployment/repositories/{}/artifacts", version); + objects_list_page_authz::(lockstep_client, &url) + .await + .items +} + pub struct TestTrustRoot { pub key: Key, pub expiry: DateTime, @@ -203,7 +215,7 @@ async fn test_repo_upload() -> Result<()> { let repo = trust_root.assemble_repo(&logctx.log, &[]).await?; // Generate a repository and upload it to Nexus. - let mut initial_repo = { + let initial_repo = { let response = repo .to_upload_request(client, StatusCode::OK) .execute() @@ -215,29 +227,31 @@ async fn test_repo_upload() -> Result<()> { assert_eq!(response.status, TufRepoUploadStatus::Inserted); response.repo }; - let unique_sha256_count = initial_repo - .artifacts + + // Fetch artifacts using the new lockstep endpoint + let initial_artifacts = + fetch_repo_artifacts(&cptestctx.lockstep_client, "1.0.0").await; + let unique_sha256_count = initial_artifacts .iter() .map(|artifact| artifact.hash) .collect::>() .len(); // The repository description should have `Zone` artifacts instead of the // composite `ControlPlane` artifact. - assert_eq!( - initial_repo - .artifacts - .iter() - .filter_map(|artifact| { - if artifact.id.kind == KnownArtifactKind::Zone.into() { - Some(&artifact.id.name) - } else { - None - } - }) - .collect::>(), - ["zone-1", "zone-2"] - ); - assert!(!initial_repo.artifacts.iter().any(|artifact| { + let zone_names: HashSet<&str> = initial_artifacts + .iter() + .filter_map(|artifact| { + if artifact.id.kind == KnownArtifactKind::Zone.into() { + Some(artifact.id.name.as_str()) + } else { + None + } + }) + .collect(); + let expected_zones: HashSet<&str> = + ["zone-1", "zone-2"].into_iter().collect(); + assert_eq!(zone_names, expected_zones); + assert!(!initial_artifacts.iter().any(|artifact| { artifact.id.kind == KnownArtifactKind::ControlPlane.into() })); // The generation number should now be 2. @@ -277,7 +291,7 @@ async fn test_repo_upload() -> Result<()> { // Upload the repository to Nexus again. This should return a 200 with an // `AlreadyExists` status. - let mut reupload_description = { + let reupload_description = { let response = repo .into_upload_request(client, StatusCode::OK) .execute() @@ -290,12 +304,32 @@ async fn test_repo_upload() -> Result<()> { response.repo }; - initial_repo.sort_artifacts(); - reupload_description.sort_artifacts(); + // Fetch artifacts again and compare them + let mut reupload_artifacts = + fetch_repo_artifacts(&cptestctx.lockstep_client, "1.0.0").await; + let mut initial_artifacts_sorted = initial_artifacts.clone(); + + // Sort artifacts by their ID for comparison (same order as ArtifactId::cmp) + initial_artifacts_sorted.sort_by(|a, b| a.id.cmp(&b.id)); + reupload_artifacts.sort_by(|a, b| a.id.cmp(&b.id)); assert_eq!( - initial_repo, reupload_description, - "initial description matches reupload" + initial_artifacts_sorted, reupload_artifacts, + "initial artifacts match reupload artifacts" + ); + + // Also verify that the repo metadata (without artifacts) matches + assert_eq!( + initial_repo.hash, reupload_description.hash, + "repo hash matches" + ); + assert_eq!( + initial_repo.system_version, reupload_description.system_version, + "system version matches" + ); + assert_eq!( + initial_repo.valid_until, reupload_description.valid_until, + "valid_until matches" ); // We didn't insert a new repo, so the generation number should still be 2. @@ -420,7 +454,7 @@ async fn test_repo_upload() -> Result<()> { // Upload a new repository with a different system version but no other // changes. This should be accepted. - let initial_installinator_doc = { + let initial_installinator_doc_hash = { let tweaks = &[ManifestTweak::SystemVersion("2.0.0".parse().unwrap())]; let response = trust_root .assemble_repo(&logctx.log, tweaks) @@ -436,15 +470,16 @@ async fn test_repo_upload() -> Result<()> { let response = serde_json::from_slice::(&response.body) .context("error deserializing response body")?; assert_eq!(response.status, TufRepoUploadStatus::Inserted); - let mut description = response.repo; - description.sort_artifacts(); + + // Fetch artifacts for the 2.0.0 repository + let artifacts_2_0_0 = + fetch_repo_artifacts(&cptestctx.lockstep_client, "2.0.0").await; // The artifacts should be exactly the same as the 1.0.0 repo we // uploaded, other than the installinator document (which will have // system version 2.0.0). let mut installinator_doc_1 = None; - let filtered_artifacts_1 = initial_repo - .artifacts + let filtered_artifacts_1 = initial_artifacts .iter() .filter(|artifact| { if artifact.id.kind @@ -458,8 +493,7 @@ async fn test_repo_upload() -> Result<()> { }) .collect::>(); let mut installinator_doc_2 = None; - let filtered_artifacts_2 = description - .artifacts + let filtered_artifacts_2 = artifacts_2_0_0 .iter() .filter(|artifact| { if artifact.id.kind @@ -494,6 +528,8 @@ async fn test_repo_upload() -> Result<()> { // Validate the repo metadata assert_eq!(get_repo.system_version.to_string(), "2.0.0"); + + installinator_doc_1.hash.to_string() }; // The installinator document changed, so the generation number is bumped to // 3. @@ -521,10 +557,9 @@ async fn test_repo_upload() -> Result<()> { assert_eq!(status.local_repos, 0); // Verify the initial installinator document is present on all sled-agents. - let installinator_doc_hash = initial_installinator_doc.hash.to_string(); for sled_agent in &cptestctx.sled_agents { for dir in sled_agent.sled_agent().artifact_store().storage_paths() { - let path = dir.join(&installinator_doc_hash); + let path = dir.join(&initial_installinator_doc_hash); assert!(path.exists(), "{path} does not exist"); } } @@ -550,7 +585,7 @@ async fn test_repo_upload() -> Result<()> { &opctx, status.generation, &recent_releases, - initial_repo.repo.id(), + initial_repo.id(), ) .await .unwrap(); @@ -575,7 +610,7 @@ async fn test_repo_upload() -> Result<()> { // Verify the installinator document from the initial repo is deleted. for sled_agent in &cptestctx.sled_agents { for dir in sled_agent.sled_agent().artifact_store().storage_paths() { - let path = dir.join(&installinator_doc_hash); + let path = dir.join(&initial_installinator_doc_hash); assert!(!path.exists(), "{path} was not deleted"); } } From 5cfe2b3b6146abe562ff131d97afa5bf64009be8 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Fri, 26 Sep 2025 16:33:55 -0500 Subject: [PATCH 16/31] fetch artifacts directly from datastore, no endpoint required --- nexus/tests/integration_tests/updates.rs | 55 ++++++++++++++---------- 1 file changed, 33 insertions(+), 22 deletions(-) diff --git a/nexus/tests/integration_tests/updates.rs b/nexus/tests/integration_tests/updates.rs index 04c7672f631..00d1e2472f1 100644 --- a/nexus/tests/integration_tests/updates.rs +++ b/nexus/tests/integration_tests/updates.rs @@ -8,6 +8,7 @@ use camino_tempfile::{Builder, Utf8TempPath}; use chrono::{DateTime, Duration, Timelike, Utc}; use dropshot::ResultsPage; 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_utils::background::activate_background_task; @@ -21,7 +22,7 @@ use nexus_test_utils::test_setup; use nexus_test_utils_macros::nexus_test; use nexus_types::external_api::views; use nexus_types::external_api::views::{TufRepoUpload, TufRepoUploadStatus}; -use omicron_common::api::external::TufArtifactMeta; +use omicron_common::api::external::{DataPageParams, TufArtifactMeta}; use pretty_assertions::assert_eq; use semver::Version; use serde::Deserialize; @@ -36,21 +37,39 @@ use tufaceous_lib::assemble::{ArtifactManifest, OmicronRepoAssembler}; use tufaceous_lib::assemble::{DeserializedManifest, ManifestTweak}; use crate::integration_tests::target_release::set_target_release; +use uuid::Uuid; const TRUST_ROOTS_URL: &str = "/v1/system/update/trust-roots"; type ControlPlaneTestContext = nexus_test_utils::ControlPlaneTestContext; -/// Fetch artifacts for a repository using the lockstep API -async fn fetch_repo_artifacts( - lockstep_client: &dropshot::test_util::ClientTestContext, +/// Get artifacts for a repository using the datastore directly, sorted by ID +async fn get_repo_artifacts( + cptestctx: &ControlPlaneTestContext, version: &str, ) -> Vec { - let url = format!("/deployment/repositories/{}/artifacts", version); - objects_list_page_authz::(lockstep_client, &url) + let datastore = cptestctx.server.server_context().nexus.datastore(); + let opctx = + OpContext::for_tests(cptestctx.logctx.log.new(o!()), datastore.clone()); + let system_version = SemverVersion::from( + version.parse::().expect("version should parse"), + ); + let pagparams = DataPageParams::::max_page(); + + let artifacts = datastore + .tuf_repo_artifacts_list_by_version(&opctx, system_version, &pagparams) .await - .items + .expect("should get artifacts"); + + let mut result: Vec = artifacts + .into_iter() + .map(|artifact| artifact.into_external()) + .collect(); + + // Sort artifacts by their ID for consistent comparison + result.sort_by(|a, b| a.id.cmp(&b.id)); + result } pub struct TestTrustRoot { @@ -228,9 +247,8 @@ async fn test_repo_upload() -> Result<()> { response.repo }; - // Fetch artifacts using the new lockstep endpoint - let initial_artifacts = - fetch_repo_artifacts(&cptestctx.lockstep_client, "1.0.0").await; + // Get artifacts using the datastore directly + let initial_artifacts = get_repo_artifacts(&cptestctx, "1.0.0").await; let unique_sha256_count = initial_artifacts .iter() .map(|artifact| artifact.hash) @@ -304,17 +322,11 @@ async fn test_repo_upload() -> Result<()> { response.repo }; - // Fetch artifacts again and compare them - let mut reupload_artifacts = - fetch_repo_artifacts(&cptestctx.lockstep_client, "1.0.0").await; - let mut initial_artifacts_sorted = initial_artifacts.clone(); - - // Sort artifacts by their ID for comparison (same order as ArtifactId::cmp) - initial_artifacts_sorted.sort_by(|a, b| a.id.cmp(&b.id)); - reupload_artifacts.sort_by(|a, b| a.id.cmp(&b.id)); + // Get artifacts again and compare them + let reupload_artifacts = get_repo_artifacts(&cptestctx, "1.0.0").await; assert_eq!( - initial_artifacts_sorted, reupload_artifacts, + initial_artifacts, reupload_artifacts, "initial artifacts match reupload artifacts" ); @@ -471,9 +483,8 @@ async fn test_repo_upload() -> Result<()> { .context("error deserializing response body")?; assert_eq!(response.status, TufRepoUploadStatus::Inserted); - // Fetch artifacts for the 2.0.0 repository - let artifacts_2_0_0 = - fetch_repo_artifacts(&cptestctx.lockstep_client, "2.0.0").await; + // Get artifacts for the 2.0.0 repository + let artifacts_2_0_0 = get_repo_artifacts(&cptestctx, "2.0.0").await; // The artifacts should be exactly the same as the 1.0.0 repo we // uploaded, other than the installinator document (which will have From 23a785b8c27f5d7191ccd8a81082adab0dfdd749 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Fri, 26 Sep 2025 16:41:20 -0500 Subject: [PATCH 17/31] never mind about that lockstep endpoint --- nexus/lockstep-api/src/lib.rs | 18 --- nexus/src/lockstep_api/http_entrypoints.rs | 48 ------- openapi/nexus-lockstep.json | 153 --------------------- 3 files changed, 219 deletions(-) diff --git a/nexus/lockstep-api/src/lib.rs b/nexus/lockstep-api/src/lib.rs index 4638c0e43ff..43b54ac9260 100644 --- a/nexus/lockstep-api/src/lib.rs +++ b/nexus/lockstep-api/src/lib.rs @@ -44,7 +44,6 @@ use nexus_types::internal_api::views::QuiesceStatus; use nexus_types::internal_api::views::Saga; use nexus_types::internal_api::views::UpdateStatus; use omicron_common::api::external::Instance; -use omicron_common::api::external::TufArtifactMeta; use omicron_common::api::external::http_pagination::PaginatedById; use omicron_common::api::external::http_pagination::PaginatedByTimeAndId; use omicron_uuid_kinds::*; @@ -305,17 +304,6 @@ pub trait NexusLockstepApi { rqctx: RequestContext, ) -> Result, HttpError>; - /// List artifacts for a TUF repository by version - #[endpoint { - method = GET, - path = "/deployment/repositories/{version}/artifacts" - }] - async fn tuf_repo_artifacts_list( - rqctx: RequestContext, - path_params: Path, - query_params: Query, - ) -> Result>, HttpError>; - /// List uninitialized sleds #[endpoint { method = GET, @@ -582,9 +570,3 @@ pub struct SledId { pub struct VersionPathParam { pub version: u32, } - -/// Path parameters for TUF repository version requests -#[derive(Deserialize, JsonSchema)] -pub struct TufRepoVersionPathParam { - pub version: String, -} diff --git a/nexus/src/lockstep_api/http_entrypoints.rs b/nexus/src/lockstep_api/http_entrypoints.rs index 327d12fb3df..463b0883deb 100644 --- a/nexus/src/lockstep_api/http_entrypoints.rs +++ b/nexus/src/lockstep_api/http_entrypoints.rs @@ -49,7 +49,6 @@ use nexus_types::internal_api::views::Saga; use nexus_types::internal_api::views::UpdateStatus; use nexus_types::internal_api::views::to_list; use omicron_common::api::external::Instance; -use omicron_common::api::external::TufArtifactMeta; use omicron_common::api::external::http_pagination::PaginatedById; use omicron_common::api::external::http_pagination::PaginatedByTimeAndId; use omicron_common::api::external::http_pagination::ScanById; @@ -58,8 +57,6 @@ use omicron_common::api::external::http_pagination::ScanParams; use omicron_common::api::external::http_pagination::data_page_params_for; use omicron_uuid_kinds::*; use range_requests::PotentialRange; -use semver::Version; -use nexus_db_model::SemverVersion; use crate::app::support_bundles::SupportBundleQueryType; use crate::context::ApiContext; @@ -504,51 +501,6 @@ impl NexusLockstepApi for NexusLockstepApiImpl { .await } - async fn tuf_repo_artifacts_list( - rqctx: RequestContext, - path_params: Path, - query_params: Query, - ) -> Result>, HttpError> { - let apictx = &rqctx.context().context; - let handler = async { - let opctx = - crate::context::op_context_for_internal_api(&rqctx).await; - let nexus = &apictx.nexus; - let datastore = nexus.datastore(); - let path = path_params.into_inner(); - let query = query_params.into_inner(); - let pagparams = data_page_params_for(&rqctx, &query)?; - - // Parse the version string as semver - let system_version = Version::parse(&path.version) - .map_err(|e| HttpError::for_bad_request( - None, - format!("Invalid version '{}': {}", path.version, e), - ))?; - let semver_version = SemverVersion::from(system_version); - - let artifacts = datastore - .tuf_repo_artifacts_list_by_version(&opctx, semver_version, &pagparams) - .await?; - - // Convert TufArtifact to TufArtifactMeta - let artifact_metas: Vec = artifacts - .into_iter() - .map(|artifact| artifact.into_external()) - .collect(); - - // Since TufArtifactMeta.id is not a UUID but a composite ArtifactId, - // and we don't have a direct UUID for artifacts, we'll return all - // artifacts without pagination for now (which is fine since each - // repo should only have a few artifacts under 20) - Ok(HttpResponseOk(ResultsPage { items: artifact_metas, next_page: None })) - }; - apictx - .internal_latencies - .instrument_dropshot_handler(&rqctx, handler) - .await - } - async fn sled_list_uninitialized( rqctx: RequestContext, ) -> Result>, HttpError> { diff --git a/openapi/nexus-lockstep.json b/openapi/nexus-lockstep.json index 236d3b72fd4..9136d7bc60f 100644 --- a/openapi/nexus-lockstep.json +++ b/openapi/nexus-lockstep.json @@ -551,70 +551,6 @@ } } }, - "/deployment/repositories/{version}/artifacts": { - "get": { - "summary": "List artifacts for a TUF repository by version", - "operationId": "tuf_repo_artifacts_list", - "parameters": [ - { - "in": "path", - "name": "version", - "required": true, - "schema": { - "type": "string" - } - }, - { - "in": "query", - "name": "limit", - "description": "Maximum number of items returned by a single call", - "schema": { - "nullable": true, - "type": "integer", - "format": "uint32", - "minimum": 1 - } - }, - { - "in": "query", - "name": "page_token", - "description": "Token returned by previous call to retrieve the subsequent page", - "schema": { - "nullable": true, - "type": "string" - } - }, - { - "in": "query", - "name": "sort_by", - "schema": { - "$ref": "#/components/schemas/IdSortMode" - } - } - ], - "responses": { - "200": { - "description": "successful operation", - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/TufArtifactMetaResultsPage" - } - } - } - }, - "4XX": { - "$ref": "#/components/responses/Error" - }, - "5XX": { - "$ref": "#/components/responses/Error" - } - }, - "x-dropshot-pagination": { - "required": [] - } - } - }, "/deployment/update-status": { "get": { "summary": "Show deployed versions of artifacts", @@ -1434,29 +1370,6 @@ "dependency" ] }, - "ArtifactId": { - "description": "An identifier for an artifact.", - "type": "object", - "properties": { - "kind": { - "description": "The kind of artifact this is.", - "type": "string" - }, - "name": { - "description": "The artifact's name.", - "type": "string" - }, - "version": { - "description": "The artifact's version.", - "type": "string" - } - }, - "required": [ - "kind", - "name", - "version" - ] - }, "ArtifactVersion": { "description": "An artifact version.\n\nThis is a freeform identifier with some basic validation. It may be the serialized form of a semver version, or a custom identifier that uses the same character set as a semver, plus `_`.\n\nThe exact pattern accepted is `^[a-zA-Z0-9._+-]{1,63}$`.\n\n# Ord implementation\n\n`ArtifactVersion`s are not intended to be sorted, just compared for equality. `ArtifactVersion` implements `Ord` only for storage within sorted collections.", "type": "string", @@ -7577,72 +7490,6 @@ } } }, - "TufArtifactMeta": { - "description": "Metadata about an individual TUF artifact.\n\nFound within a `TufRepoDescription`.", - "type": "object", - "properties": { - "board": { - "nullable": true, - "description": "Contents of the `BORD` field of a Hubris archive caboose. Only applicable to artifacts that are Hubris archives.\n\nThis field should always be `Some(_)` if `sign` is `Some(_)`, but the opposite is not true (SP images will have a `board` but not a `sign`).", - "type": "string" - }, - "hash": { - "description": "The hash of the artifact.", - "type": "string", - "format": "hex string (32 bytes)" - }, - "id": { - "description": "The artifact ID.", - "allOf": [ - { - "$ref": "#/components/schemas/ArtifactId" - } - ] - }, - "sign": { - "nullable": true, - "description": "Contents of the `SIGN` field of a Hubris archive caboose, i.e., an identifier for the set of valid signing keys. Currently only applicable to RoT image and bootloader artifacts, where it will be an LPC55 Root Key Table Hash (RKTH).", - "type": "array", - "items": { - "type": "integer", - "format": "uint8", - "minimum": 0 - } - }, - "size": { - "description": "The size of the artifact in bytes.", - "type": "integer", - "format": "uint64", - "minimum": 0 - } - }, - "required": [ - "hash", - "id", - "size" - ] - }, - "TufArtifactMetaResultsPage": { - "description": "A single page of results", - "type": "object", - "properties": { - "items": { - "description": "list of items on this page of results", - "type": "array", - "items": { - "$ref": "#/components/schemas/TufArtifactMeta" - } - }, - "next_page": { - "nullable": true, - "description": "token used to fetch the next page of results (if any)", - "type": "string" - } - }, - "required": [ - "items" - ] - }, "TufRepoVersion": { "oneOf": [ { From 9e33fc96adeda099b0ef0de7c6ffcc60fefec8c9 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Fri, 26 Sep 2025 16:50:07 -0500 Subject: [PATCH 18/31] make repo get and put endpoints more conventional --- nexus/external-api/output/nexus_tags.txt | 4 ++-- nexus/external-api/src/lib.rs | 8 ++++---- nexus/src/external_api/http_entrypoints.rs | 4 ++-- nexus/tests/integration_tests/endpoints.rs | 17 +++++++++++------ nexus/tests/integration_tests/updates.rs | 8 ++++---- nexus/types/src/external_api/params.rs | 4 ++-- openapi/nexus.json | 10 ++++------ 7 files changed, 29 insertions(+), 26 deletions(-) diff --git a/nexus/external-api/output/nexus_tags.txt b/nexus/external-api/output/nexus_tags.txt index 7239db8baf0..bf3f5ceda80 100644 --- a/nexus/external-api/output/nexus_tags.txt +++ b/nexus/external-api/output/nexus_tags.txt @@ -296,9 +296,9 @@ ping GET /v1/ping API operations found with tag "system/update" OPERATION ID METHOD URL PATH -system_update_get_repository GET /v1/system/update/repository/{system_version} -system_update_put_repository PUT /v1/system/update/repository system_update_repository_list GET /v1/system/update/repositories +system_update_repository_upload PUT /v1/system/update/repositories +system_update_repository_view GET /v1/system/update/repositories/{system_version} system_update_status GET /v1/system/update/status system_update_trust_root_create POST /v1/system/update/trust-roots system_update_trust_root_delete DELETE /v1/system/update/trust-roots/{trust_root_id} diff --git a/nexus/external-api/src/lib.rs b/nexus/external-api/src/lib.rs index 548b2995be2..b09f7082069 100644 --- a/nexus/external-api/src/lib.rs +++ b/nexus/external-api/src/lib.rs @@ -2967,11 +2967,11 @@ pub trait NexusExternalApi { /// System release repositories are verified by the updates trust store. #[endpoint { method = PUT, - path = "/v1/system/update/repository", + path = "/v1/system/update/repositories", tags = ["system/update"], request_body_max_bytes = PUT_UPDATE_REPOSITORY_MAX_BYTES, }] - async fn system_update_put_repository( + async fn system_update_repository_upload( rqctx: RequestContext, query: Query, body: StreamingBody, @@ -2980,10 +2980,10 @@ pub trait NexusExternalApi { /// Fetch system release repository description by version #[endpoint { method = GET, - path = "/v1/system/update/repository/{system_version}", + path = "/v1/system/update/repositories/{system_version}", tags = ["system/update"], }] - async fn system_update_get_repository( + async fn system_update_repository_view( rqctx: RequestContext, path_params: Path, ) -> Result, HttpError>; diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index 650ad6dc239..59148de27e3 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -6639,7 +6639,7 @@ impl NexusExternalApi for NexusExternalApiImpl { // Updates - async fn system_update_put_repository( + async fn system_update_repository_upload( rqctx: RequestContext, query: Query, body: StreamingBody, @@ -6664,7 +6664,7 @@ impl NexusExternalApi for NexusExternalApiImpl { .await } - async fn system_update_get_repository( + async fn system_update_repository_view( rqctx: RequestContext, path_params: Path, ) -> Result, HttpError> { diff --git a/nexus/tests/integration_tests/endpoints.rs b/nexus/tests/integration_tests/endpoints.rs index 649207d3f98..b88b41a143e 100644 --- a/nexus/tests/integration_tests/endpoints.rs +++ b/nexus/tests/integration_tests/endpoints.rs @@ -2535,16 +2535,21 @@ pub static VERIFY_ENDPOINTS: LazyLock> = LazyLock::new( ], }, VerifyEndpoint { - url: "/v1/system/update/repository?file_name=demo-repo.zip", + url: "/v1/system/update/repositories?file_name=demo-repo.zip", visibility: Visibility::Public, unprivileged_access: UnprivilegedAccess::None, - allowed_methods: vec![AllowedMethod::Put( - // In reality this is the contents of a zip file. - serde_json::Value::Null, - )], + allowed_methods: vec![ + // the query param is only relevant to the put + AllowedMethod::Put( + // In reality this is the contents of a zip file. + serde_json::Value::Null, + ), + // get doesn't use the query param but it doesn't break if it's there + AllowedMethod::Get + ], }, VerifyEndpoint { - url: "/v1/system/update/repository/1.0.0", + url: "/v1/system/update/repositories/1.0.0", visibility: Visibility::Public, unprivileged_access: UnprivilegedAccess::None, allowed_methods: vec![AllowedMethod::Get], diff --git a/nexus/tests/integration_tests/updates.rs b/nexus/tests/integration_tests/updates.rs index 00d1e2472f1..cbe11433747 100644 --- a/nexus/tests/integration_tests/updates.rs +++ b/nexus/tests/integration_tests/updates.rs @@ -144,7 +144,7 @@ impl TestRepo { expected_status: StatusCode, ) -> NexusRequest<'a> { let url = format!( - "/v1/system/update/repository?file_name={}", + "/v1/system/update/repositories?file_name={}", self.0.file_name().expect("archive path must have a file name") ); let request = RequestBuilder::new(client, Method::PUT, &url) @@ -200,7 +200,7 @@ async fn test_repo_upload_unconfigured() -> Result<()> { // with a 404 error. object_get_error( client, - "/v1/system/update/repository/1.0.0", + "/v1/system/update/repositories/1.0.0", StatusCode::NOT_FOUND, ) .await; @@ -353,7 +353,7 @@ async fn test_repo_upload() -> Result<()> { // Now get the repository that was just uploaded. let repo = object_get::( client, - "/v1/system/update/repository/1.0.0", + "/v1/system/update/repositories/1.0.0", ) .await; @@ -533,7 +533,7 @@ async fn test_repo_upload() -> Result<()> { // Now get the repository that was just uploaded. let get_repo = object_get::( client, - "/v1/system/update/repository/2.0.0", + "/v1/system/update/repositories/2.0.0", ) .await; diff --git a/nexus/types/src/external_api/params.rs b/nexus/types/src/external_api/params.rs index 8a1cd6f6fa7..a3f4fa0e7a6 100644 --- a/nexus/types/src/external_api/params.rs +++ b/nexus/types/src/external_api/params.rs @@ -2398,14 +2398,14 @@ pub struct ResourceMetrics { // SYSTEM UPDATE -/// Parameters for PUT requests for `/v1/system/update/repository`. +/// Parameters for PUT requests for `/v1/system/update/repositories`. #[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] pub struct UpdatesPutRepositoryParams { /// The name of the uploaded file. pub file_name: String, } -/// Parameters for GET requests for `/v1/system/update/repository`. +/// Parameters for GET requests for `/v1/system/update/repositories`. #[derive(Clone, Debug, Deserialize, JsonSchema)] pub struct UpdatesGetRepositoryParams { /// The version to get. diff --git a/openapi/nexus.json b/openapi/nexus.json index 36be0a3861a..c0d6ccb85e0 100644 --- a/openapi/nexus.json +++ b/openapi/nexus.json @@ -10961,16 +10961,14 @@ "x-dropshot-pagination": { "required": [] } - } - }, - "/v1/system/update/repository": { + }, "put": { "tags": [ "system/update" ], "summary": "Upload system release repository", "description": "System release repositories are verified by the updates trust store.", - "operationId": "system_update_put_repository", + "operationId": "system_update_repository_upload", "parameters": [ { "in": "query", @@ -11013,13 +11011,13 @@ } } }, - "/v1/system/update/repository/{system_version}": { + "/v1/system/update/repositories/{system_version}": { "get": { "tags": [ "system/update" ], "summary": "Fetch system release repository description by version", - "operationId": "system_update_get_repository", + "operationId": "system_update_repository_view", "parameters": [ { "in": "path", From de3c39e822c1a7213f27c4c6b0ea8ab905f43e70 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Fri, 26 Sep 2025 17:27:28 -0500 Subject: [PATCH 19/31] self-review fixes --- nexus/db-queries/src/db/datastore/update.rs | 4 ++-- nexus/src/app/update.rs | 10 +++------- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/nexus/db-queries/src/db/datastore/update.rs b/nexus/db-queries/src/db/datastore/update.rs index d3d596096eb..f5b63b999c5 100644 --- a/nexus/db-queries/src/db/datastore/update.rs +++ b/nexus/db-queries/src/db/datastore/update.rs @@ -41,7 +41,7 @@ use uuid::Uuid; /// The return value of [`DataStore::tuf_repo_insert`]. /// -/// This is similar to [`external::TufRepoInsertResponse`], but uses +/// This is similar to [`views::TufRepoUpload`], but uses /// nexus-db-model's types instead of external types. pub struct TufRepoInsertResponse { pub recorded: TufRepoDescription, @@ -458,7 +458,7 @@ impl DataStore { } /// List all TUF repositories (without artifacts) ordered by system version (newest first by default). - pub async fn tuf_repo_list_no_artifacts( + pub async fn tuf_repo_list( &self, opctx: &OpContext, pagparams: &DataPageParams<'_, Version>, diff --git a/nexus/src/app/update.rs b/nexus/src/app/update.rs index c367baada4b..cdc628d7687 100644 --- a/nexus/src/app/update.rs +++ b/nexus/src/app/update.rs @@ -103,22 +103,18 @@ impl super::Nexus { &self, opctx: &OpContext, system_version: Version, - ) -> Result { + ) -> Result { self.db_datastore .tuf_repo_get_by_version(opctx, system_version.into()) .await - .map_err(HttpError::from) } pub(crate) async fn updates_list_repositories( &self, opctx: &OpContext, pagparams: &DataPageParams<'_, Version>, - ) -> Result, HttpError> { - self.db_datastore - .tuf_repo_list_no_artifacts(opctx, pagparams) - .await - .map_err(HttpError::from) + ) -> Result, Error> { + self.db_datastore.tuf_repo_list(opctx, pagparams).await } pub(crate) async fn updates_add_trust_root( From 9365744e7e3acfe793140bf2eb59675bab2de439 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Sat, 27 Sep 2025 17:21:04 -0500 Subject: [PATCH 20/31] no reference to "external" types in external API call tree --- common/src/api/external/mod.rs | 13 ----- nexus/db-model/src/tuf_repo.rs | 59 ++++++++++++++++++--- nexus/db-queries/src/db/datastore/update.rs | 38 +++---------- nexus/src/app/update.rs | 11 ++-- nexus/src/external_api/http_entrypoints.rs | 11 ++-- nexus/types/src/external_api/views.rs | 27 ++-------- 6 files changed, 73 insertions(+), 86 deletions(-) diff --git a/common/src/api/external/mod.rs b/common/src/api/external/mod.rs index bf382db8f2a..deac70363bb 100644 --- a/common/src/api/external/mod.rs +++ b/common/src/api/external/mod.rs @@ -3504,19 +3504,6 @@ pub struct TufArtifactMeta { pub sign: Option>, } -/// Status of a TUF repo import. -#[derive( - Debug, Clone, Copy, PartialEq, Eq, Deserialize, Serialize, JsonSchema, -)] -#[serde(rename_all = "snake_case")] -pub enum TufRepoInsertStatus { - /// The repository already existed in the database. - AlreadyExists, - - /// The repository did not exist, and was inserted into the database. - Inserted, -} - #[derive( Clone, Debug, Deserialize, JsonSchema, Serialize, PartialEq, ObjectIdentity, )] diff --git a/nexus/db-model/src/tuf_repo.rs b/nexus/db-model/src/tuf_repo.rs index fd54f95ccca..f9afe231340 100644 --- a/nexus/db-model/src/tuf_repo.rs +++ b/nexus/db-model/src/tuf_repo.rs @@ -30,8 +30,6 @@ use uuid::Uuid; /// A description of a TUF update: a repo, along with the artifacts it /// contains. -/// -/// This is the internal variant of [`external::TufRepoDescription`]. #[derive(Debug, Clone)] pub struct TufRepoDescription { /// The repository. @@ -64,7 +62,6 @@ impl TufRepoDescription { } } - /// Converts self into [`external::TufRepoDescription`]. pub fn into_external(self) -> external::TufRepoDescription { external::TufRepoDescription { repo: self.repo.into_external(), @@ -78,8 +75,6 @@ impl TufRepoDescription { } /// A record representing an uploaded TUF repository. -/// -/// This is the internal variant of [`external::TufRepoMeta`]. #[derive( Queryable, Identifiable, Insertable, Clone, Debug, Selectable, AsChangeset, )] @@ -134,7 +129,6 @@ impl TufRepo { ) } - /// Converts self into [`external::TufRepoMeta`]. pub fn into_external(self) -> external::TufRepoMeta { external::TufRepoMeta { hash: self.sha256.into(), @@ -156,6 +150,18 @@ impl TufRepo { } } +impl From for views::TufRepo { + fn from(repo: TufRepo) -> views::TufRepo { + views::TufRepo { + hash: repo.sha256.into(), + targets_role_version: repo.targets_role_version as u64, + valid_until: repo.valid_until, + system_version: repo.system_version.into(), + file_name: repo.file_name, + } + } +} + #[derive(Queryable, Insertable, Clone, Debug, Selectable, AsChangeset)] #[diesel(table_name = tuf_artifact)] pub struct TufArtifact { @@ -413,3 +419,44 @@ impl FromSql for DbTufSignedRootRole { .map_err(|e| e.into()) } } + +// The following aren't real models in the sense that they represent DB data, +// but they are the return types of datastore functions + +/// Status of a TUF repo import +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum TufRepoUploadStatus { + /// The repository already existed in the database + AlreadyExists, + + /// The repository did not exist, and was inserted into the database + Inserted, +} + +impl From for views::TufRepoUploadStatus { + fn from(status: TufRepoUploadStatus) -> Self { + match status { + TufRepoUploadStatus::AlreadyExists => { + views::TufRepoUploadStatus::AlreadyExists + } + TufRepoUploadStatus::Inserted => { + views::TufRepoUploadStatus::Inserted + } + } + } +} + +/// The return value of the tuf repo insert function +pub struct TufRepoUpload { + pub recorded: TufRepoDescription, + pub status: TufRepoUploadStatus, +} + +impl From for views::TufRepoUpload { + fn from(upload: TufRepoUpload) -> Self { + views::TufRepoUpload { + repo: upload.recorded.repo.into(), + status: upload.status.into(), + } + } +} diff --git a/nexus/db-queries/src/db/datastore/update.rs b/nexus/db-queries/src/db/datastore/update.rs index f5b63b999c5..0fab430f973 100644 --- a/nexus/db-queries/src/db/datastore/update.rs +++ b/nexus/db-queries/src/db/datastore/update.rs @@ -22,13 +22,12 @@ use nexus_db_errors::{ErrorHandler, public_error_from_diesel}; use nexus_db_lookup::DbConnection; use nexus_db_model::{ ArtifactHash, DbTypedUuid, TargetRelease, TufArtifact, TufRepo, - TufRepoDescription, TufTrustRoot, to_db_typed_uuid, + TufRepoDescription, TufRepoUpload, TufRepoUploadStatus, TufTrustRoot, + to_db_typed_uuid, }; -use nexus_types::external_api::views; use omicron_common::api::external::{ self, CreateResult, DataPageParams, DeleteResult, Generation, - ListResultVec, LookupResult, LookupType, ResourceType, TufRepoInsertStatus, - UpdateResult, + ListResultVec, LookupResult, LookupType, ResourceType, UpdateResult, }; use omicron_common::api::external::{Error, InternalContext}; use omicron_uuid_kinds::TufRepoKind; @@ -39,24 +38,6 @@ use swrite::{SWrite, swrite}; use tufaceous_artifact::ArtifactVersion; use uuid::Uuid; -/// The return value of [`DataStore::tuf_repo_insert`]. -/// -/// This is similar to [`views::TufRepoUpload`], but uses -/// nexus-db-model's types instead of external types. -pub struct TufRepoInsertResponse { - pub recorded: TufRepoDescription, - pub status: TufRepoInsertStatus, -} - -impl TufRepoInsertResponse { - pub fn into_external(self) -> views::TufRepoUpload { - views::TufRepoUpload { - repo: self.recorded.repo.into_external().into(), - status: self.status.into(), - } - } -} - async fn artifacts_for_repo( repo_id: TypedUuid, conn: &async_bb8_diesel::Connection, @@ -90,7 +71,7 @@ impl DataStore { &self, opctx: &OpContext, description: &external::TufRepoDescription, - ) -> CreateResult { + ) -> CreateResult { opctx.authorize(authz::Action::Modify, &authz::FLEET).await?; let log = opctx.log.new( slog::o!( @@ -581,7 +562,7 @@ async fn insert_impl( conn: async_bb8_diesel::Connection, desc: &external::TufRepoDescription, err: OptionalError, -) -> Result { +) -> Result { // Load the current generation from the database and increment it, then // use that when creating the `TufRepoDescription`. If we determine there // are any artifacts to be inserted, we update the generation to this value @@ -618,9 +599,9 @@ async fn insert_impl( let recorded = TufRepoDescription { repo: existing_repo, artifacts }; - return Ok(TufRepoInsertResponse { + return Ok(TufRepoUpload { recorded, - status: TufRepoInsertStatus::AlreadyExists, + status: TufRepoUploadStatus::AlreadyExists, }); } @@ -824,10 +805,7 @@ async fn insert_impl( } let recorded = TufRepoDescription { repo, artifacts: all_artifacts }; - Ok(TufRepoInsertResponse { - recorded, - status: TufRepoInsertStatus::Inserted, - }) + Ok(TufRepoUpload { recorded, status: TufRepoUploadStatus::Inserted }) } async fn get_generation( diff --git a/nexus/src/app/update.rs b/nexus/src/app/update.rs index cdc628d7687..fda49c39c74 100644 --- a/nexus/src/app/update.rs +++ b/nexus/src/app/update.rs @@ -9,9 +9,10 @@ use dropshot::HttpError; use futures::Stream; use nexus_auth::authz; use nexus_db_lookup::LookupPath; +use nexus_db_model::TufRepoUpload; +use nexus_db_model::TufRepoUploadStatus; use nexus_db_model::{Generation, TufTrustRoot}; use nexus_db_queries::context::OpContext; -use nexus_db_queries::db::datastore::update::TufRepoInsertResponse; use nexus_db_queries::db::{datastore::SQL_BATCH_SIZE, pagination::Paginator}; use nexus_types::deployment::TargetReleaseDescription; use nexus_types::external_api::shared::TufSignedRootRole; @@ -20,9 +21,7 @@ use nexus_types::internal_api::views as internal_views; use nexus_types::inventory::RotSlot; use omicron_common::api::external::InternalContext; use omicron_common::api::external::Nullable; -use omicron_common::api::external::{ - DataPageParams, Error, TufRepoInsertStatus, -}; +use omicron_common::api::external::{DataPageParams, Error}; use omicron_common::disk::M2Slot; use omicron_uuid_kinds::{GenericUuid, TufTrustRootUuid}; use semver::Version; @@ -38,7 +37,7 @@ impl super::Nexus { opctx: &OpContext, body: impl Stream> + Send + Sync + 'static, file_name: String, - ) -> Result { + ) -> Result { let mut trusted_roots = Vec::new(); let mut paginator = Paginator::new( SQL_BATCH_SIZE, @@ -76,7 +75,7 @@ impl super::Nexus { // carries with it the `Utf8TempDir`s storing the artifacts) into the // artifact replication background task, then immediately activate the // task. - if response.status == TufRepoInsertStatus::Inserted { + if response.status == TufRepoUploadStatus::Inserted { self.tuf_artifact_replication_tx .send(artifacts_with_plan) .await diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index 59148de27e3..d9c4cc40d31 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -6654,8 +6654,7 @@ impl NexusExternalApi for NexusExternalApiImpl { let update = nexus .updates_put_repository(&opctx, body, query.file_name) .await?; - let repo_upload = update.into_external(); - Ok(HttpResponseOk(repo_upload)) + Ok(HttpResponseOk(update.into())) }; apictx .context @@ -6677,7 +6676,7 @@ impl NexusExternalApi for NexusExternalApiImpl { let repo = nexus .updates_get_repository(&opctx, params.system_version) .await?; - Ok(HttpResponseOk(repo.into_external().into())) + Ok(HttpResponseOk(repo.into())) }; apictx .context @@ -6700,10 +6699,8 @@ impl NexusExternalApi for NexusExternalApiImpl { let repos = nexus.updates_list_repositories(&opctx, &pagparams).await?; - let responses: Vec = repos - .into_iter() - .map(|repo| repo.into_external().into()) - .collect(); + let responses: Vec = + repos.into_iter().map(Into::into).collect(); Ok(HttpResponseOk(ScanByVersion::results_page( &query, diff --git a/nexus/types/src/external_api/views.rs b/nexus/types/src/external_api/views.rs index 3c85a07dffe..f2627f48738 100644 --- a/nexus/types/src/external_api/views.rs +++ b/nexus/types/src/external_api/views.rs @@ -15,9 +15,9 @@ use chrono::Utc; use daft::Diffable; pub use omicron_common::api::external::IpVersion; use omicron_common::api::external::{ - self, AffinityPolicy, AllowedSourceIps as ExternalAllowedSourceIps, - ByteCount, Digest, Error, FailureDomain, IdentityMetadata, InstanceState, - Name, Nullable, ObjectIdentity, SimpleIdentity, SimpleIdentityOrName, + AffinityPolicy, AllowedSourceIps as ExternalAllowedSourceIps, ByteCount, + Digest, Error, FailureDomain, IdentityMetadata, InstanceState, Name, + Nullable, ObjectIdentity, SimpleIdentity, SimpleIdentityOrName, }; use omicron_uuid_kinds::*; use oxnet::{Ipv4Net, Ipv6Net}; @@ -1618,18 +1618,6 @@ pub struct TufRepo { pub file_name: String, } -impl From for TufRepo { - fn from(meta: external::TufRepoMeta) -> Self { - Self { - hash: meta.hash, - targets_role_version: meta.targets_role_version, - valid_until: meta.valid_until, - system_version: meta.system_version, - file_name: meta.file_name, - } - } -} - #[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize, JsonSchema)] pub struct TufRepoUpload { pub repo: TufRepo, @@ -1650,15 +1638,6 @@ pub enum TufRepoUploadStatus { Inserted, } -impl From for TufRepoUploadStatus { - fn from(status: external::TufRepoInsertStatus) -> Self { - match status { - external::TufRepoInsertStatus::AlreadyExists => Self::AlreadyExists, - external::TufRepoInsertStatus::Inserted => Self::Inserted, - } - } -} - fn expected_one_of() -> String { use std::fmt::Write; let mut msg = "expected one of:".to_string(); From 4a9d8694c268bd7ad190c1e4899635b7381d706b Mon Sep 17 00:00:00 2001 From: David Crespo Date: Thu, 2 Oct 2025 16:37:42 -0500 Subject: [PATCH 21/31] remove targets_role_version and valid_until from TufRepo view, add time_created --- nexus/db-model/src/tuf_repo.rs | 3 +-- nexus/tests/integration_tests/updates.rs | 8 -------- nexus/types/src/external_api/views.rs | 9 +++------ openapi/nexus.json | 13 +++---------- 4 files changed, 7 insertions(+), 26 deletions(-) diff --git a/nexus/db-model/src/tuf_repo.rs b/nexus/db-model/src/tuf_repo.rs index f9afe231340..9de9106ba27 100644 --- a/nexus/db-model/src/tuf_repo.rs +++ b/nexus/db-model/src/tuf_repo.rs @@ -154,10 +154,9 @@ impl From for views::TufRepo { fn from(repo: TufRepo) -> views::TufRepo { views::TufRepo { hash: repo.sha256.into(), - targets_role_version: repo.targets_role_version as u64, - valid_until: repo.valid_until, system_version: repo.system_version.into(), file_name: repo.file_name, + time_created: repo.time_created, } } } diff --git a/nexus/tests/integration_tests/updates.rs b/nexus/tests/integration_tests/updates.rs index cbe11433747..da7ee06a9cf 100644 --- a/nexus/tests/integration_tests/updates.rs +++ b/nexus/tests/integration_tests/updates.rs @@ -339,10 +339,6 @@ async fn test_repo_upload() -> Result<()> { initial_repo.system_version, reupload_description.system_version, "system version matches" ); - assert_eq!( - initial_repo.valid_until, reupload_description.valid_until, - "valid_until matches" - ); // We didn't insert a new repo, so the generation number should still be 2. assert_eq!( @@ -363,10 +359,6 @@ async fn test_repo_upload() -> Result<()> { initial_repo.system_version, repo.system_version, "system version matches" ); - assert_eq!( - initial_repo.valid_until, repo.valid_until, - "valid_until matches" - ); // Upload a new repository with the same system version but a different // version for one of the components. This will produce a different hash, diff --git a/nexus/types/src/external_api/views.rs b/nexus/types/src/external_api/views.rs index f2627f48738..40bab4d395a 100644 --- a/nexus/types/src/external_api/views.rs +++ b/nexus/types/src/external_api/views.rs @@ -1601,12 +1601,6 @@ pub struct TufRepo { /// convenience. pub hash: ArtifactHash, - /// The version of the targets role - pub targets_role_version: u64, - - /// The time until which the repo is valid - pub valid_until: DateTime, - /// The system version in artifacts.json pub system_version: Version, @@ -1616,6 +1610,9 @@ pub struct TufRepo { /// with wicket, we read the file contents from stdin so we don't know the /// correct file name). pub file_name: String, + + /// Time the repository was uploaded + pub time_created: DateTime, } #[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize, JsonSchema)] diff --git a/openapi/nexus.json b/openapi/nexus.json index c0d6ccb85e0..a711ca6b68c 100644 --- a/openapi/nexus.json +++ b/openapi/nexus.json @@ -25881,14 +25881,8 @@ "type": "string", "pattern": "^(0|[1-9]\\d*)\\.(0|[1-9]\\d*)\\.(0|[1-9]\\d*)(?:-((?:0|[1-9]\\d*|\\d*[a-zA-Z-][0-9a-zA-Z-]*)(?:\\.(?:0|[1-9]\\d*|\\d*[a-zA-Z-][0-9a-zA-Z-]*))*))?(?:\\+([0-9a-zA-Z-]+(?:\\.[0-9a-zA-Z-]+)*))?$" }, - "targets_role_version": { - "description": "The version of the targets role", - "type": "integer", - "format": "uint64", - "minimum": 0 - }, - "valid_until": { - "description": "The time until which the repo is valid", + "time_created": { + "description": "Time the repository was uploaded", "type": "string", "format": "date-time" } @@ -25897,8 +25891,7 @@ "file_name", "hash", "system_version", - "targets_role_version", - "valid_until" + "time_created" ] }, "TufRepoResultsPage": { From d73c2fee72fc1233b778ca39580450159f805591 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Tue, 7 Oct 2025 13:02:27 -0500 Subject: [PATCH 22/31] make diff slightly smaller --- nexus/tests/integration_tests/target_release.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/nexus/tests/integration_tests/target_release.rs b/nexus/tests/integration_tests/target_release.rs index a3755fb5406..897b197a859 100644 --- a/nexus/tests/integration_tests/target_release.rs +++ b/nexus/tests/integration_tests/target_release.rs @@ -14,7 +14,7 @@ use nexus_test_utils::http_testing::{NexusRequest, RequestBuilder}; use nexus_test_utils::resource_helpers::object_get; use nexus_test_utils::test_setup; use nexus_types::external_api::params::SetTargetReleaseParams; -use nexus_types::external_api::views::{TufRepoUpload, UpdateStatus}; +use nexus_types::external_api::views; use semver::Version; use tufaceous_artifact::{ArtifactVersion, KnownArtifactKind}; use tufaceous_lib::assemble::ManifestTweak; @@ -29,7 +29,7 @@ async fn get_set_target_release() -> Result<()> { let logctx = &ctx.logctx; // There is no target release before one has ever been specified - let status: UpdateStatus = + let status: views::UpdateStatus = object_get(client, "/v1/system/update/status").await; assert_eq!(status.target_release.0, None); @@ -53,7 +53,7 @@ async fn get_set_target_release() -> Result<()> { { let before = Utc::now(); let system_version = Version::new(1, 0, 0); - let response: TufRepoUpload = trust_root + let response: views::TufRepoUpload = trust_root .assemble_repo(&logctx.log, &[]) .await? .into_upload_request(client, StatusCode::OK) @@ -64,7 +64,7 @@ async fn get_set_target_release() -> Result<()> { set_target_release(client, &system_version).await?; - let status: UpdateStatus = + let status: views::UpdateStatus = object_get(client, "/v1/system/update/status").await; let target_release = status.target_release.0.unwrap(); @@ -85,7 +85,7 @@ async fn get_set_target_release() -> Result<()> { version: ArtifactVersion::new("non-semver-2").unwrap(), }, ]; - let response: TufRepoUpload = trust_root + let response: views::TufRepoUpload = trust_root .assemble_repo(&logctx.log, tweaks) .await? .into_upload_request(client, StatusCode::OK) @@ -96,7 +96,7 @@ async fn get_set_target_release() -> Result<()> { set_target_release(client, &system_version).await?; - let status: UpdateStatus = + let status: views::UpdateStatus = object_get(client, "/v1/system/update/status").await; let target_release = status.target_release.0.unwrap(); From 7693a68b268afed8407850d7ef2cd2fcf3fdd760 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Tue, 7 Oct 2025 18:12:59 -0500 Subject: [PATCH 23/31] use our own channel watcher instead of the quiesce handle --- nexus/src/app/mod.rs | 7 ++++++- nexus/src/app/quiesce.rs | 6 ------ nexus/src/app/update.rs | 30 +++++++++++++++++++++++++++--- 3 files changed, 33 insertions(+), 10 deletions(-) diff --git a/nexus/src/app/mod.rs b/nexus/src/app/mod.rs index 0f33f470873..9b202ee2dd6 100644 --- a/nexus/src/app/mod.rs +++ b/nexus/src/app/mod.rs @@ -9,6 +9,7 @@ use self::saga::SagaExecutor; use crate::DropshotServer; use crate::app::background::BackgroundTasksData; use crate::app::background::SagaRecoveryHelpers; +use crate::app::update::UpdateStatusHandle; use crate::populate::PopulateArgs; use crate::populate::PopulateStatus; use crate::populate::populate_start; @@ -285,6 +286,9 @@ pub struct Nexus { #[allow(dead_code)] repo_depot_resolver: Box, + /// handle to pull update status data + update_status: UpdateStatusHandle, + /// state of overall Nexus quiesce activity quiesce: NexusQuiesceHandle, } @@ -351,7 +355,7 @@ impl Nexus { let quiesce = NexusQuiesceHandle::new( db_datastore.clone(), config.deployment.id, - blueprint_load_rx, + blueprint_load_rx.clone(), quiesce_opctx, ); @@ -534,6 +538,7 @@ impl Nexus { mgs_update_status_rx, mgs_resolver, repo_depot_resolver, + update_status: UpdateStatusHandle::new(blueprint_load_rx), quiesce, }; diff --git a/nexus/src/app/quiesce.rs b/nexus/src/app/quiesce.rs index 7efca01c1fb..adffc9ab7a6 100644 --- a/nexus/src/app/quiesce.rs +++ b/nexus/src/app/quiesce.rs @@ -92,12 +92,6 @@ impl NexusQuiesceHandle { self.state.borrow().clone() } - pub fn latest_blueprint( - &self, - ) -> Option> { - self.latest_blueprint.borrow().clone() - } - pub fn set_quiescing(&self, quiescing: bool) { let new_state = if quiescing { let time_requested = Utc::now(); diff --git a/nexus/src/app/update.rs b/nexus/src/app/update.rs index f65a99f5105..dd97e3b8d52 100644 --- a/nexus/src/app/update.rs +++ b/nexus/src/app/update.rs @@ -12,7 +12,9 @@ use nexus_db_lookup::LookupPath; use nexus_db_model::{Generation, TufRepoDescription, TufTrustRoot}; use nexus_db_queries::context::OpContext; use nexus_db_queries::db::{datastore::SQL_BATCH_SIZE, pagination::Paginator}; -use nexus_types::deployment::TargetReleaseDescription; +use nexus_types::deployment::{ + Blueprint, BlueprintTarget, TargetReleaseDescription, +}; use nexus_types::external_api::shared::TufSignedRootRole; use nexus_types::external_api::views; use nexus_types::internal_api::views as internal_views; @@ -26,11 +28,30 @@ use omicron_common::disk::M2Slot; use omicron_uuid_kinds::{GenericUuid, TufTrustRootUuid}; use semver::Version; use std::collections::BTreeMap; +use std::sync::Arc; +use tokio::sync::watch; use update_common::artifacts::{ ArtifactsWithPlan, ControlPlaneZonesMode, VerificationMode, }; use uuid::Uuid; +/// Used to pull data out of the channels +#[derive(Clone)] +pub struct UpdateStatusHandle { + latest_blueprint: + watch::Receiver>>, +} + +impl UpdateStatusHandle { + pub fn new( + latest_blueprint: watch::Receiver< + Option>, + >, + ) -> Self { + Self { latest_blueprint } + } +} + impl super::Nexus { pub(crate) async fn updates_put_repository( &self, @@ -190,8 +211,11 @@ impl super::Nexus { .await?; let (blueprint_target, blueprint) = self - .quiesce - .latest_blueprint() + .update_status + .latest_blueprint + .borrow() + .clone() // drop read lock held by outstanding borrow + .as_ref() .ok_or_else(|| { Error::internal_error("Tried to get update status before target blueprint is loaded") })? From e804964ee9ad716327a1e74243f72a95dc496ae0 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Tue, 7 Oct 2025 17:30:16 -0500 Subject: [PATCH 24/31] address more comments from dave, including <= bug --- nexus/db-queries/src/db/datastore/update.rs | 5 +++++ nexus/src/app/update.rs | 8 ++++---- nexus/tests/integration_tests/updates.rs | 9 +++++---- 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/nexus/db-queries/src/db/datastore/update.rs b/nexus/db-queries/src/db/datastore/update.rs index b22d013ccdd..618b99452d4 100644 --- a/nexus/db-queries/src/db/datastore/update.rs +++ b/nexus/db-queries/src/db/datastore/update.rs @@ -200,6 +200,11 @@ impl DataStore { .await .map(|v| v.0) .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server)) + // looking up a non-existent ID will 500, but it doesn't + // automatically include the bad ID + .with_internal_context(|| { + format!("tuf_repo_get_version {tuf_repo_id}") + }) } /// Returns the list of all TUF repo artifacts known to the system. diff --git a/nexus/src/app/update.rs b/nexus/src/app/update.rs index dd97e3b8d52..29a30fc496c 100644 --- a/nexus/src/app/update.rs +++ b/nexus/src/app/update.rs @@ -224,10 +224,10 @@ impl super::Nexus { let time_last_blueprint = blueprint_target.time_made_target; - // Update activity is paused if the current target release generation - // is less than or equal to the blueprint's minimum generation + // Update activity is paused if the current target release generation is + // less than the blueprint's minimum generation let paused = *db_target_release.generation - <= blueprint.target_release_minimum_generation; + < blueprint.target_release_minimum_generation; Ok(views::UpdateStatus { target_release: Nullable(target_release), @@ -237,7 +237,7 @@ impl super::Nexus { }) } - /// Get component status using read-only queries to avoid batch operations + /// Build a map of version strings to the number of components on that version async fn component_version_counts( &self, opctx: &OpContext, diff --git a/nexus/tests/integration_tests/updates.rs b/nexus/tests/integration_tests/updates.rs index bc012ae7baf..f44078de9f6 100644 --- a/nexus/tests/integration_tests/updates.rs +++ b/nexus/tests/integration_tests/updates.rs @@ -716,10 +716,11 @@ async fn test_update_status() -> Result<()> { let status: views::UpdateStatus = object_get(client, "/v1/system/update/status").await; assert_eq!(status.target_release.0, None); - assert!( - status.paused, - "should be paused initially when no target release is set" - ); + // does not start paused because the DB migration initialized the + // target_release table with a row with gen 1, and the initial target + // blueprint also has gen 1 + assert!(!status.paused); + let counts = status.components_by_release_version; assert_eq!(counts.get("install dataset").unwrap(), &7); assert_eq!(counts.get("unknown").unwrap(), &15); From 273058f82495afc905ded79bf1df222768116f3f Mon Sep 17 00:00:00 2001 From: David Crespo Date: Wed, 8 Oct 2025 14:58:56 -0500 Subject: [PATCH 25/31] time_last_blueprint -> time_last_step_planned --- nexus/src/app/update.rs | 4 ++-- nexus/src/external_api/http_entrypoints.rs | 4 ++-- nexus/tests/integration_tests/updates.rs | 6 +++--- nexus/types/src/external_api/views.rs | 6 ++---- openapi/nexus.json | 6 +++--- 5 files changed, 12 insertions(+), 14 deletions(-) diff --git a/nexus/src/app/update.rs b/nexus/src/app/update.rs index 29a30fc496c..9f2b6a16e48 100644 --- a/nexus/src/app/update.rs +++ b/nexus/src/app/update.rs @@ -222,7 +222,7 @@ impl super::Nexus { .as_ref() .clone(); - let time_last_blueprint = blueprint_target.time_made_target; + let time_last_step_planned = blueprint_target.time_made_target; // Update activity is paused if the current target release generation is // less than the blueprint's minimum generation @@ -232,7 +232,7 @@ impl super::Nexus { Ok(views::UpdateStatus { target_release: Nullable(target_release), components_by_release_version, - time_last_blueprint, + time_last_step_planned, paused, }) } diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index 4195ed4634f..a715fdead6e 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -6830,8 +6830,8 @@ impl NexusExternalApi for NexusExternalApiImpl { &system_version, ) { return Err(Error::invalid_request(format!( - "The requested target release ({system_version}) must \ - be newer than the current target release ({version})." + "Requested target release ({system_version}) must not \ + be older than current target release ({version})." )) .into()); } diff --git a/nexus/tests/integration_tests/updates.rs b/nexus/tests/integration_tests/updates.rs index f44078de9f6..71999333583 100644 --- a/nexus/tests/integration_tests/updates.rs +++ b/nexus/tests/integration_tests/updates.rs @@ -726,7 +726,7 @@ async fn test_update_status() -> Result<()> { assert_eq!(counts.get("unknown").unwrap(), &15); // hold onto this to compare it to later values - let time_last_blueprint = status.time_last_blueprint; + let time_last_step_planned = status.time_last_step_planned; // Upload a fake TUF repo and set it as the target release let trust_root = TestTrustRoot::generate().await?; @@ -746,7 +746,7 @@ async fn test_update_status() -> Result<()> { assert!(!status.paused, "should not be paused after setting v1"); // blueprint time doesn't change - assert_eq!(time_last_blueprint, status.time_last_blueprint); + assert_eq!(time_last_step_planned, status.time_last_step_planned); let counts = status.components_by_release_version; assert_eq!(counts.get("install dataset").unwrap(), &7); @@ -778,7 +778,7 @@ async fn test_update_status() -> Result<()> { assert!(!status.paused, "should not be paused after setting v2"); // blueprint time doesn't change - assert_eq!(time_last_blueprint, status.time_last_blueprint); + assert_eq!(time_last_step_planned, status.time_last_step_planned); let counts = status.components_by_release_version; assert_eq!(counts.get("install dataset").unwrap(), &7); diff --git a/nexus/types/src/external_api/views.rs b/nexus/types/src/external_api/views.rs index 647e0d34ead..67b07dd0230 100644 --- a/nexus/types/src/external_api/views.rs +++ b/nexus/types/src/external_api/views.rs @@ -1576,10 +1576,8 @@ pub struct UpdateStatus { /// Time of most recent update planning activity /// /// This is intended as a rough indicator of the last time something - /// happened in the update planner. A blueprint is the update system's plan - /// for the next state of the system, so this timestamp indicates the last - /// time the update system made a plan. - pub time_last_blueprint: DateTime, + /// happened in the update planner. + pub time_last_step_planned: DateTime, /// Whether update activity is paused /// diff --git a/openapi/nexus.json b/openapi/nexus.json index c421334efda..c0930ff0ecb 100644 --- a/openapi/nexus.json +++ b/openapi/nexus.json @@ -26187,8 +26187,8 @@ } ] }, - "time_last_blueprint": { - "description": "Time of most recent update planning activity\n\nThis is intended as a rough indicator of the last time something happened in the update planner. A blueprint is the update system's plan for the next state of the system, so this timestamp indicates the last time the update system made a plan.", + "time_last_step_planned": { + "description": "Time of most recent update planning activity\n\nThis is intended as a rough indicator of the last time something happened in the update planner.", "type": "string", "format": "date-time" } @@ -26197,7 +26197,7 @@ "components_by_release_version", "paused", "target_release", - "time_last_blueprint" + "time_last_step_planned" ] }, "UpdatesTrustRoot": { From 99f5d98f62a43d70444d190ae508cc8c9cbd6a4f Mon Sep 17 00:00:00 2001 From: David Crespo Date: Wed, 8 Oct 2025 15:24:54 -0500 Subject: [PATCH 26/31] paused -> suspended --- nexus/src/app/update.rs | 8 ++++---- nexus/tests/integration_tests/updates.rs | 8 ++++---- nexus/types/src/external_api/views.rs | 12 ++++++------ openapi/nexus.json | 6 +++--- 4 files changed, 17 insertions(+), 17 deletions(-) diff --git a/nexus/src/app/update.rs b/nexus/src/app/update.rs index 9f2b6a16e48..aaa3b375ead 100644 --- a/nexus/src/app/update.rs +++ b/nexus/src/app/update.rs @@ -224,16 +224,16 @@ impl super::Nexus { let time_last_step_planned = blueprint_target.time_made_target; - // Update activity is paused if the current target release generation is - // less than the blueprint's minimum generation - let paused = *db_target_release.generation + // Update activity is suspended if the current target release generation + // is less than the blueprint's minimum generation + let suspended = *db_target_release.generation < blueprint.target_release_minimum_generation; Ok(views::UpdateStatus { target_release: Nullable(target_release), components_by_release_version, time_last_step_planned, - paused, + suspended, }) } diff --git a/nexus/tests/integration_tests/updates.rs b/nexus/tests/integration_tests/updates.rs index 71999333583..86d5fe3d9c2 100644 --- a/nexus/tests/integration_tests/updates.rs +++ b/nexus/tests/integration_tests/updates.rs @@ -716,10 +716,10 @@ async fn test_update_status() -> Result<()> { let status: views::UpdateStatus = object_get(client, "/v1/system/update/status").await; assert_eq!(status.target_release.0, None); - // does not start paused because the DB migration initialized the + // does not start suspended because the DB migration initialized the // target_release table with a row with gen 1, and the initial target // blueprint also has gen 1 - assert!(!status.paused); + assert!(!status.suspended); let counts = status.components_by_release_version; assert_eq!(counts.get("install dataset").unwrap(), &7); @@ -743,7 +743,7 @@ async fn test_update_status() -> Result<()> { let status: views::UpdateStatus = object_get(client, "/v1/system/update/status").await; assert_eq!(status.target_release.0.unwrap().version, v1); - assert!(!status.paused, "should not be paused after setting v1"); + assert!(!status.suspended, "should not be suspended after setting v1"); // blueprint time doesn't change assert_eq!(time_last_step_planned, status.time_last_step_planned); @@ -775,7 +775,7 @@ async fn test_update_status() -> Result<()> { object_get(client, "/v1/system/update/status").await; assert_eq!(status.target_release.0.unwrap().version, v2); - assert!(!status.paused, "should not be paused after setting v2"); + assert!(!status.suspended, "should not be suspended after setting v2"); // blueprint time doesn't change assert_eq!(time_last_step_planned, status.time_last_step_planned); diff --git a/nexus/types/src/external_api/views.rs b/nexus/types/src/external_api/views.rs index 67b07dd0230..16ec1958dbc 100644 --- a/nexus/types/src/external_api/views.rs +++ b/nexus/types/src/external_api/views.rs @@ -1579,13 +1579,13 @@ pub struct UpdateStatus { /// happened in the update planner. pub time_last_step_planned: DateTime, - /// Whether update activity is paused + /// Whether automatic update is suspended due to manual update activity /// - /// When true, the system has stopped attempting to make progress toward the - /// target release. This happens after a MUPdate because the system wants to - /// make sure of the operator's intent. To resume update, set a new target - /// release. - pub paused: bool, + /// After a manual support procedure that changes the system software, + /// automatic update activity is suspended to avoid undoing the change. To + /// resume automatic update, first upload the TUF repository matching the + /// manually applied update, then set that as the target release. + pub suspended: bool, } fn expected_one_of() -> String { diff --git a/openapi/nexus.json b/openapi/nexus.json index c0930ff0ecb..8d955cf8141 100644 --- a/openapi/nexus.json +++ b/openapi/nexus.json @@ -26174,8 +26174,8 @@ "minimum": 0 } }, - "paused": { - "description": "Whether update activity is paused\n\nWhen true, the system has stopped attempting to make progress toward the target release. This happens after a MUPdate because the system wants to make sure of the operator's intent. To resume update, set a new target release.", + "suspended": { + "description": "Whether automatic update is suspended due to manual update activity\n\nAfter a manual support procedure that changes the system software, automatic update activity is suspended to avoid undoing the change. To resume automatic update, first upload the TUF repository matching the manually applied update, then set that as the target release.", "type": "boolean" }, "target_release": { @@ -26195,7 +26195,7 @@ }, "required": [ "components_by_release_version", - "paused", + "suspended", "target_release", "time_last_step_planned" ] From 576581d357a4c84ba6cff1f070ba84e38ef1b8c0 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Wed, 8 Oct 2025 17:21:22 -0500 Subject: [PATCH 27/31] a couple of helpful suggestions from gpt-5 --- nexus/src/app/update.rs | 9 ++++----- nexus/types/src/external_api/views.rs | 8 ++++++++ openapi/nexus.json | 2 +- 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/nexus/src/app/update.rs b/nexus/src/app/update.rs index aaa3b375ead..9b4e4ff3a9b 100644 --- a/nexus/src/app/update.rs +++ b/nexus/src/app/update.rs @@ -210,17 +210,16 @@ impl super::Nexus { ) .await?; - let (blueprint_target, blueprint) = self + let bp_arc = self .update_status .latest_blueprint .borrow() .clone() // drop read lock held by outstanding borrow - .as_ref() .ok_or_else(|| { Error::internal_error("Tried to get update status before target blueprint is loaded") - })? - .as_ref() - .clone(); + })?; + + let (blueprint_target, blueprint) = &*bp_arc; let time_last_step_planned = blueprint_target.time_made_target; diff --git a/nexus/types/src/external_api/views.rs b/nexus/types/src/external_api/views.rs index 16ec1958dbc..82ff8471f7c 100644 --- a/nexus/types/src/external_api/views.rs +++ b/nexus/types/src/external_api/views.rs @@ -1571,6 +1571,14 @@ pub struct UpdateStatus { pub target_release: Nullable, /// Count of components running each release version + /// + /// Keys will be either: + /// + /// * Semver-like release version strings + /// * "install dataset", representing the initial rack software before + /// any updates + /// * "unknown", which means there is no TUF repo uploaded that matches + /// the software running on the component) pub components_by_release_version: BTreeMap, /// Time of most recent update planning activity diff --git a/openapi/nexus.json b/openapi/nexus.json index 8d955cf8141..be1d26e0b26 100644 --- a/openapi/nexus.json +++ b/openapi/nexus.json @@ -26166,7 +26166,7 @@ "type": "object", "properties": { "components_by_release_version": { - "description": "Count of components running each release version", + "description": "Count of components running each release version\n\nKeys will be either:\n\n* Semver-like release version strings * \"install dataset\", representing the initial rack software before any updates * \"unknown\", which means there is no TUF repo uploaded that matches the software running on the component)", "type": "object", "additionalProperties": { "type": "integer", From 894bef9f6393f158476c42c147b7eed8f5601f36 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Wed, 8 Oct 2025 18:25:07 -0500 Subject: [PATCH 28/31] address dave's comments --- nexus/db-queries/src/db/datastore/update.rs | 28 +++------------------ nexus/tests/integration_tests/updates.rs | 12 ++++++--- nexus/types/src/external_api/views.rs | 15 ++++++----- openapi/nexus.json | 4 +-- 4 files changed, 20 insertions(+), 39 deletions(-) diff --git a/nexus/db-queries/src/db/datastore/update.rs b/nexus/db-queries/src/db/datastore/update.rs index 0002349085c..febb6b8436b 100644 --- a/nexus/db-queries/src/db/datastore/update.rs +++ b/nexus/db-queries/src/db/datastore/update.rs @@ -443,7 +443,8 @@ impl DataStore { .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server)) } - /// List all TUF repositories (without artifacts) ordered by system version (newest first by default). + /// List all TUF repositories (without artifacts) ordered by system version + /// (newest first by default). pub async fn tuf_repo_list( &self, opctx: &OpContext, @@ -465,36 +466,13 @@ impl DataStore { }; paginated(tuf_repo::table, tuf_repo::system_version, &db_pagparams) + .filter(tuf_repo::time_pruned.is_null()) .select(TufRepo::as_select()) .load_async(&*conn) .await .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server)) } - /// List artifacts for a specific TUF repository by system version. - pub async fn tuf_repo_artifacts_list_by_version( - &self, - opctx: &OpContext, - system_version: SemverVersion, - _pagparams: &DataPageParams<'_, Uuid>, - ) -> ListResultVec { - opctx.authorize(authz::Action::Read, &authz::FLEET).await?; - - let conn = self.pool_connection_authorized(opctx).await?; - - // First get the repo by version - let repo = self.tuf_repo_get_by_version(opctx, system_version).await?; - - // Get all artifacts for this repo and apply simple pagination - let all_artifacts = artifacts_for_repo(repo.id.into(), &conn) - .await - .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server))?; - - // For now, return all artifacts since each repo should only have a few (under 20) - // The existing artifacts_for_repo comment mentions this limitation - Ok(all_artifacts) - } - /// List the trusted TUF root roles in the trust store. pub async fn tuf_trust_root_list( &self, diff --git a/nexus/tests/integration_tests/updates.rs b/nexus/tests/integration_tests/updates.rs index fc18f48d5a0..c0080440b54 100644 --- a/nexus/tests/integration_tests/updates.rs +++ b/nexus/tests/integration_tests/updates.rs @@ -22,7 +22,7 @@ use nexus_test_utils::test_setup; use nexus_test_utils_macros::nexus_test; use nexus_types::external_api::views; use nexus_types::external_api::views::{TufRepoUpload, TufRepoUploadStatus}; -use omicron_common::api::external::{DataPageParams, TufArtifactMeta}; +use omicron_common::api::external::TufArtifactMeta; use pretty_assertions::assert_eq; use semver::Version; use serde::Deserialize; @@ -37,7 +37,6 @@ use tufaceous_lib::assemble::{ArtifactManifest, OmicronRepoAssembler}; use tufaceous_lib::assemble::{DeserializedManifest, ManifestTweak}; use crate::integration_tests::target_release::set_target_release; -use uuid::Uuid; const TRUST_ROOTS_URL: &str = "/v1/system/update/trust-roots"; @@ -55,10 +54,14 @@ async fn get_repo_artifacts( let system_version = SemverVersion::from( version.parse::().expect("version should parse"), ); - let pagparams = DataPageParams::::max_page(); + + let repo = datastore + .tuf_repo_get_by_version(&opctx, system_version) + .await + .expect("should get repo by version"); let artifacts = datastore - .tuf_repo_artifacts_list_by_version(&opctx, system_version, &pagparams) + .tuf_list_repo_artifacts(&opctx, repo.id.into()) .await .expect("should get artifacts"); @@ -843,6 +846,7 @@ async fn test_repo_list() -> Result<()> { // Initially, list should be empty let initial_list: ResultsPage = objects_list_page_authz(client, "/v1/system/update/repositories").await; + assert_eq!(initial_list.items.len(), 0); assert!(initial_list.next_page.is_none()); diff --git a/nexus/types/src/external_api/views.rs b/nexus/types/src/external_api/views.rs index 6618cfc2ea1..90d98360ede 100644 --- a/nexus/types/src/external_api/views.rs +++ b/nexus/types/src/external_api/views.rs @@ -1600,11 +1600,10 @@ pub struct UpdateStatus { /// Metadata about a TUF repository #[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize, JsonSchema)] pub struct TufRepo { - /// The hash of the repository. - /// - /// This is a slight abuse of `ArtifactHash`, since that's the hash of - /// individual artifacts within the repository. However, we use it here for - /// convenience. + /// The hash of the repository + // This is a slight abuse of `ArtifactHash`, since that's the hash of + // individual artifacts within the repository. However, we use it here for + // convenience. pub hash: ArtifactHash, /// The system version in artifacts.json @@ -1612,9 +1611,9 @@ pub struct TufRepo { /// The file name of the repository /// - /// This is purely used for debugging and may not always be correct (e.g., - /// with wicket, we read the file contents from stdin so we don't know the - /// correct file name). + /// This is intended for debugging and may not always be correct. + // (e.g., with wicket, we read the file contents from stdin so we don't know + // the correct file name). pub file_name: String, /// Time the repository was uploaded diff --git a/openapi/nexus.json b/openapi/nexus.json index 9cd5a62aabe..1812980c9b2 100644 --- a/openapi/nexus.json +++ b/openapi/nexus.json @@ -25868,11 +25868,11 @@ "type": "object", "properties": { "file_name": { - "description": "The file name of the repository\n\nThis is purely used for debugging and may not always be correct (e.g., with wicket, we read the file contents from stdin so we don't know the correct file name).", + "description": "The file name of the repository\n\nThis is intended for debugging and may not always be correct.", "type": "string" }, "hash": { - "description": "The hash of the repository.\n\nThis is a slight abuse of `ArtifactHash`, since that's the hash of individual artifacts within the repository. However, we use it here for convenience.", + "description": "The hash of the repository", "type": "string", "format": "hex string (32 bytes)" }, From 6293043441a9aa4c825a370bd8178e1a34dc48d8 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Thu, 9 Oct 2025 12:25:33 -0500 Subject: [PATCH 29/31] use view struct directly for datastore upload status --- nexus/db-model/src/tuf_repo.rs | 34 +++++---------------- nexus/db-queries/src/db/datastore/update.rs | 4 +-- nexus/src/app/update.rs | 3 +- 3 files changed, 10 insertions(+), 31 deletions(-) diff --git a/nexus/db-model/src/tuf_repo.rs b/nexus/db-model/src/tuf_repo.rs index 9de9106ba27..d898e4ca006 100644 --- a/nexus/db-model/src/tuf_repo.rs +++ b/nexus/db-model/src/tuf_repo.rs @@ -12,7 +12,7 @@ use nexus_db_schema::schema::{ tuf_artifact, tuf_repo, tuf_repo_artifact, tuf_trust_root, }; use nexus_types::external_api::shared::TufSignedRootRole; -use nexus_types::external_api::views; +use nexus_types::external_api::views::{self, TufRepoUploadStatus}; use omicron_common::{api::external, update::ArtifactId}; use omicron_uuid_kinds::GenericUuid; use omicron_uuid_kinds::TufArtifactKind; @@ -419,31 +419,11 @@ impl FromSql for DbTufSignedRootRole { } } -// The following aren't real models in the sense that they represent DB data, -// but they are the return types of datastore functions - -/// Status of a TUF repo import -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum TufRepoUploadStatus { - /// The repository already existed in the database - AlreadyExists, - - /// The repository did not exist, and was inserted into the database - Inserted, -} - -impl From for views::TufRepoUploadStatus { - fn from(status: TufRepoUploadStatus) -> Self { - match status { - TufRepoUploadStatus::AlreadyExists => { - views::TufRepoUploadStatus::AlreadyExists - } - TufRepoUploadStatus::Inserted => { - views::TufRepoUploadStatus::Inserted - } - } - } -} +// The following isn't a real model in the sense that it represents DB data, +// but it is the return type of a datastore function. The main reason we can't +// just use the view for this like we do with TufRepoUploadStatus is that +// TufRepoDescription has a bit more info in it that we rely on in code outside +// of the external API, like tests and internal APIs /// The return value of the tuf repo insert function pub struct TufRepoUpload { @@ -455,7 +435,7 @@ impl From for views::TufRepoUpload { fn from(upload: TufRepoUpload) -> Self { views::TufRepoUpload { repo: upload.recorded.repo.into(), - status: upload.status.into(), + status: upload.status, } } } diff --git a/nexus/db-queries/src/db/datastore/update.rs b/nexus/db-queries/src/db/datastore/update.rs index febb6b8436b..d64c11731be 100644 --- a/nexus/db-queries/src/db/datastore/update.rs +++ b/nexus/db-queries/src/db/datastore/update.rs @@ -22,9 +22,9 @@ use nexus_db_errors::{ErrorHandler, public_error_from_diesel}; use nexus_db_lookup::DbConnection; use nexus_db_model::{ ArtifactHash, DbTypedUuid, TargetRelease, TufArtifact, TufRepo, - TufRepoDescription, TufRepoUpload, TufRepoUploadStatus, TufTrustRoot, - to_db_typed_uuid, + TufRepoDescription, TufRepoUpload, TufTrustRoot, to_db_typed_uuid, }; +use nexus_types::external_api::views::TufRepoUploadStatus; use omicron_common::api::external::{ self, CreateResult, DataPageParams, DeleteResult, Generation, ListResultVec, LookupResult, LookupType, ResourceType, UpdateResult, diff --git a/nexus/src/app/update.rs b/nexus/src/app/update.rs index 2295adb720b..bb275dd1cc1 100644 --- a/nexus/src/app/update.rs +++ b/nexus/src/app/update.rs @@ -10,7 +10,6 @@ use futures::Stream; use nexus_auth::authz; use nexus_db_lookup::LookupPath; use nexus_db_model::TufRepoUpload; -use nexus_db_model::TufRepoUploadStatus; use nexus_db_model::{Generation, TufTrustRoot}; use nexus_db_queries::context::OpContext; use nexus_db_queries::db::{datastore::SQL_BATCH_SIZE, pagination::Paginator}; @@ -18,7 +17,7 @@ use nexus_types::deployment::{ Blueprint, BlueprintTarget, TargetReleaseDescription, }; use nexus_types::external_api::shared::TufSignedRootRole; -use nexus_types::external_api::views; +use nexus_types::external_api::views::{self, TufRepoUploadStatus}; use nexus_types::internal_api::views as internal_views; use nexus_types::inventory::RotSlot; use omicron_common::api::external::InternalContext; From 9f004f6d50da4241674d5b7ad2f4613a6436a0ac Mon Sep 17 00:00:00 2001 From: David Crespo Date: Thu, 9 Oct 2025 12:55:17 -0500 Subject: [PATCH 30/31] delete unused tuf_list_repos, test pruned filtering --- nexus/db-queries/src/db/datastore/update.rs | 20 ------------ nexus/tests/integration_tests/updates.rs | 36 +++++++++++++++++++++ 2 files changed, 36 insertions(+), 20 deletions(-) diff --git a/nexus/db-queries/src/db/datastore/update.rs b/nexus/db-queries/src/db/datastore/update.rs index d64c11731be..c9586a33075 100644 --- a/nexus/db-queries/src/db/datastore/update.rs +++ b/nexus/db-queries/src/db/datastore/update.rs @@ -185,26 +185,6 @@ impl DataStore { }) } - /// Returns the list of all TUF repo artifacts known to the system. - pub async fn tuf_list_repos( - &self, - opctx: &OpContext, - generation: Generation, - pagparams: &DataPageParams<'_, Uuid>, - ) -> ListResultVec { - opctx.authorize(authz::Action::Read, &authz::FLEET).await?; - - use nexus_db_schema::schema::tuf_artifact::dsl; - - let generation = nexus_db_model::Generation(generation); - paginated(dsl::tuf_artifact, dsl::id, pagparams) - .filter(dsl::generation_added.le(generation)) - .select(TufArtifact::as_select()) - .load_async(&*self.pool_connection_authorized(opctx).await?) - .await - .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server)) - } - /// Pages through the list of all not-yet-pruned TUF repos in the system pub async fn tuf_list_repos_unpruned( &self, diff --git a/nexus/tests/integration_tests/updates.rs b/nexus/tests/integration_tests/updates.rs index c0080440b54..2e37431300c 100644 --- a/nexus/tests/integration_tests/updates.rs +++ b/nexus/tests/integration_tests/updates.rs @@ -958,6 +958,42 @@ async fn test_repo_list() -> Result<()> { assert_eq!(next_page.items.len(), 1); assert_eq!(next_page.items[0].system_version.to_string(), "1.0.0"); + // Test filtering out pruned repos + + // Mark the 1.0.0 repo as pruned (use datastore methods since there's no API + // for it) + let datastore = cptestctx.server.server_context().nexus.datastore(); + let opctx = OpContext::for_tests(logctx.log.new(o!()), datastore.clone()); + + let v1 = "1.0.0".parse::().unwrap(); + let repo_to_prune = + datastore.tuf_repo_get_by_version(&opctx, v1.into()).await?; + + let generation = datastore.tuf_get_generation(&opctx).await?; + let recent_releases = + datastore.target_release_fetch_recent_distinct(&opctx, 3).await?; + + datastore + .tuf_repo_mark_pruned( + &opctx, + generation, + &recent_releases, + repo_to_prune.id(), + ) + .await?; + + // List repositories again - the pruned repo should not appear + let list_after_prune: ResultsPage = + objects_list_page_authz(client, "/v1/system/update/repositories").await; + + assert_eq!(list_after_prune.items.len(), 2); + let versions_after_prune: Vec = list_after_prune + .items + .iter() + .map(|item| item.system_version.to_string()) + .collect(); + assert_eq!(versions_after_prune, vec!["3.0.0", "2.0.0"]); + cptestctx.teardown().await; Ok(()) } From 4d204b019672df8fedf18f81724317806e8ad653 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Thu, 9 Oct 2025 16:07:53 -0500 Subject: [PATCH 31/31] get repo should 404 on pruned repos --- nexus/db-queries/src/db/datastore/update.rs | 2 +- nexus/tests/integration_tests/updates.rs | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/nexus/db-queries/src/db/datastore/update.rs b/nexus/db-queries/src/db/datastore/update.rs index 2357081f920..348babba343 100644 --- a/nexus/db-queries/src/db/datastore/update.rs +++ b/nexus/db-queries/src/db/datastore/update.rs @@ -144,6 +144,7 @@ impl DataStore { dsl::tuf_repo .filter(dsl::system_version.eq(system_version.clone())) + .filter(dsl::time_pruned.is_null()) .select(TufRepo::as_select()) .first_async::(&*conn) .await @@ -174,7 +175,6 @@ impl DataStore { tuf_repo::table .select(tuf_repo::system_version) .filter(tuf_repo::id.eq(tuf_repo_id.into_untyped_uuid())) - // TODO: filter out pruned .first_async::(&*conn) .await .map(|v| v.0) diff --git a/nexus/tests/integration_tests/updates.rs b/nexus/tests/integration_tests/updates.rs index 2e37431300c..b1e319045ed 100644 --- a/nexus/tests/integration_tests/updates.rs +++ b/nexus/tests/integration_tests/updates.rs @@ -960,6 +960,10 @@ async fn test_repo_list() -> Result<()> { // Test filtering out pruned repos + // Confirm that GET works for 1.0.0 before pruning + let _repo_before_prune: views::TufRepo = + object_get(client, "/v1/system/update/repositories/1.0.0").await; + // Mark the 1.0.0 repo as pruned (use datastore methods since there's no API // for it) let datastore = cptestctx.server.server_context().nexus.datastore(); @@ -982,6 +986,14 @@ async fn test_repo_list() -> Result<()> { ) .await?; + // pruned repo now 404s + object_get_error( + client, + "/v1/system/update/repositories/1.0.0", + StatusCode::NOT_FOUND, + ) + .await; + // List repositories again - the pruned repo should not appear let list_after_prune: ResultsPage = objects_list_page_authz(client, "/v1/system/update/repositories").await;