From 8fe97a9c39ae83c85d25519a2fee58127d17a54b Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Wed, 15 Jan 2025 11:48:36 -0700 Subject: [PATCH 01/19] target_release schema --- nexus/db-model/src/schema.rs | 9 ++++++ nexus/db-model/src/schema_versions.rs | 3 +- schema/crdb/create-target-release/up1.sql | 5 ++++ schema/crdb/create-target-release/up2.sql | 11 +++++++ schema/crdb/create-target-release/up3.sql | 12 ++++++++ schema/crdb/dbinit.sql | 35 ++++++++++++++++++++++- 6 files changed, 73 insertions(+), 2 deletions(-) create mode 100644 schema/crdb/create-target-release/up1.sql create mode 100644 schema/crdb/create-target-release/up2.sql create mode 100644 schema/crdb/create-target-release/up3.sql diff --git a/nexus/db-model/src/schema.rs b/nexus/db-model/src/schema.rs index 8343d2ff2f..138c501c98 100644 --- a/nexus/db-model/src/schema.rs +++ b/nexus/db-model/src/schema.rs @@ -1369,6 +1369,15 @@ allow_tables_to_appear_in_same_query!( joinable!(tuf_repo_artifact -> tuf_repo (tuf_repo_id)); joinable!(tuf_repo_artifact -> tuf_artifact (tuf_artifact_id)); +table! { + target_release (generation) { + generation -> Int8, + time_requested -> Timestamptz, + release_source -> crate::TargetReleaseSourceEnum, + system_version -> Nullable, + } +} + table! { support_bundle { id -> Uuid, diff --git a/nexus/db-model/src/schema_versions.rs b/nexus/db-model/src/schema_versions.rs index 79831241d9..849fc23a95 100644 --- a/nexus/db-model/src/schema_versions.rs +++ b/nexus/db-model/src/schema_versions.rs @@ -17,7 +17,7 @@ use std::collections::BTreeMap; /// /// This must be updated when you change the database schema. Refer to /// schema/crdb/README.adoc in the root of this repository for details. -pub const SCHEMA_VERSION: SemverVersion = SemverVersion::new(123, 0, 0); +pub const SCHEMA_VERSION: SemverVersion = SemverVersion::new(124, 0, 0); /// List of all past database schema versions, in *reverse* order /// @@ -29,6 +29,7 @@ static KNOWN_VERSIONS: Lazy> = Lazy::new(|| { // | leaving the first copy as an example for the next person. // v // KnownVersion::new(next_int, "unique-dirname-with-the-sql-files"), + KnownVersion::new(124, "create-target-release"), KnownVersion::new(123, "vpc-subnet-contention"), KnownVersion::new(122, "tuf-artifact-replication"), KnownVersion::new(121, "dataset-to-crucible-dataset"), diff --git a/schema/crdb/create-target-release/up1.sql b/schema/crdb/create-target-release/up1.sql new file mode 100644 index 0000000000..796fdd5f64 --- /dev/null +++ b/schema/crdb/create-target-release/up1.sql @@ -0,0 +1,5 @@ +-- The source of the software release that should be deployed to the rack. +CREATE TYPE IF NOT EXISTS omicron.public.target_release_source AS ENUM ( + 'install_dataset', + 'system_version' +); diff --git a/schema/crdb/create-target-release/up2.sql b/schema/crdb/create-target-release/up2.sql new file mode 100644 index 0000000000..16972972e0 --- /dev/null +++ b/schema/crdb/create-target-release/up2.sql @@ -0,0 +1,11 @@ +-- The software release that should be deployed to the rack. +CREATE TABLE IF NOT EXISTS omicron.public.target_release ( + generation INT8 NOT NULL PRIMARY KEY, + time_requested TIMESTAMPTZ NOT NULL, + release_source omicron.public.target_release_source NOT NULL, + system_version STRING(64), -- "foreign key" into the `tuf_repo` table + CONSTRAINT system_version_for_release CHECK ( + (release_source != 'system_version') OR + (release_source = 'system_version' AND system_version IS NOT NULL) + ) +); diff --git a/schema/crdb/create-target-release/up3.sql b/schema/crdb/create-target-release/up3.sql new file mode 100644 index 0000000000..35a1ef9eaf --- /dev/null +++ b/schema/crdb/create-target-release/up3.sql @@ -0,0 +1,12 @@ +-- System software is by default from the `install` dataset. +INSERT INTO omicron.public.target_release ( + generation, + time_requested, + release_source, + system_version +) VALUES ( + 0, + NOW(), + 'install_dataset', + NULL +) ON CONFLICT DO NOTHING; diff --git a/schema/crdb/dbinit.sql b/schema/crdb/dbinit.sql index 08fe84241a..878679aeaa 100644 --- a/schema/crdb/dbinit.sql +++ b/schema/crdb/dbinit.sql @@ -2399,6 +2399,39 @@ CREATE TABLE IF NOT EXISTS omicron.public.tuf_repo_artifact ( /*******************************************************************/ +-- The source of the software release that should be deployed to the rack. +CREATE TYPE IF NOT EXISTS omicron.public.target_release_source AS ENUM ( + 'install_dataset', + 'system_version' +); + +-- The software release that should be deployed to the rack. +CREATE TABLE IF NOT EXISTS omicron.public.target_release ( + generation INT8 NOT NULL PRIMARY KEY, + time_requested TIMESTAMPTZ NOT NULL, + release_source omicron.public.target_release_source NOT NULL, + system_version STRING(64), -- "foreign key" into the `tuf_repo` table + CONSTRAINT system_version_for_release CHECK ( + (release_source != 'system_version') OR + (release_source = 'system_version' AND system_version IS NOT NULL) + ) +); + +-- System software is by default from the `install` dataset. +INSERT INTO omicron.public.target_release ( + generation, + time_requested, + release_source, + system_version +) VALUES ( + 0, + NOW(), + 'install_dataset', + NULL +) ON CONFLICT DO NOTHING; + +/*******************************************************************/ + /* * Support Bundles */ @@ -4831,7 +4864,7 @@ INSERT INTO omicron.public.db_metadata ( version, target_version ) VALUES - (TRUE, NOW(), NOW(), '123.0.0', NULL) + (TRUE, NOW(), NOW(), '124.0.0', NULL) ON CONFLICT DO NOTHING; COMMIT; From 7722d7c3e23a787a2011cea8358a4309838c7da7 Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Mon, 20 Jan 2025 14:30:10 -0700 Subject: [PATCH 02/19] target_release model --- nexus/db-model/src/lib.rs | 2 + nexus/db-model/src/target_release.rs | 68 ++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+) create mode 100644 nexus/db-model/src/target_release.rs diff --git a/nexus/db-model/src/lib.rs b/nexus/db-model/src/lib.rs index bec35c233c..80cf4c1a18 100644 --- a/nexus/db-model/src/lib.rs +++ b/nexus/db-model/src/lib.rs @@ -62,6 +62,7 @@ mod rendezvous_debug_dataset; mod semver_version; mod switch_interface; mod switch_port; +mod target_release; mod v2p_mapping; mod vmm_state; // These actually represent subqueries, not real table. @@ -211,6 +212,7 @@ pub use support_bundle::*; pub use switch::*; pub use switch_interface::*; pub use switch_port::*; +pub use target_release::*; pub use tuf_repo::*; pub use typed_uuid::to_db_typed_uuid; pub use upstairs_repair::*; diff --git a/nexus/db-model/src/target_release.rs b/nexus/db-model/src/target_release.rs new file mode 100644 index 0000000000..2c3a2d24bf --- /dev/null +++ b/nexus/db-model/src/target_release.rs @@ -0,0 +1,68 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +use super::{impl_enum_type, Generation}; +use crate::schema::target_release; +use crate::SemverVersion; +use chrono::{DateTime, Utc}; + +impl_enum_type!( + #[derive(SqlType, Debug, QueryId)] + #[diesel(postgres_type(name = "target_release_source", schema = "public"))] + pub struct TargetReleaseSourceEnum; + + /// The source of the software release that should be deployed to the rack. + #[derive(Copy, Clone, Debug, AsExpression, FromSqlRow, PartialEq, Eq, Hash)] + #[diesel(sql_type = TargetReleaseSourceEnum)] + pub enum TargetReleaseSource; + + InstallDataset => b"install_dataset" + SystemVersion => b"system_version" +); + +/// Specify the target system software release. +#[derive(Clone, Debug, Insertable, Queryable, Selectable)] +#[diesel(table_name = target_release)] +pub struct TargetRelease { + /// Each change to the target release is recorded as a row with + /// a monotonically increasing generation number. The row with + /// the largest generation is the current target release. + pub generation: Generation, + + /// The time at which this target release was requested. + pub time_requested: DateTime, + + /// The source of the target release. + pub release_source: TargetReleaseSource, + + /// The semantic version of the target release. + pub system_version: Option, +} + +impl TargetRelease { + pub fn new( + generation: Generation, + release_source: TargetReleaseSource, + system_version: Option, + ) -> Self { + Self { + generation, + time_requested: Utc::now(), + release_source, + system_version, + } + } + + pub fn new_from_prev( + prev: TargetRelease, + release_source: TargetReleaseSource, + system_version: Option, + ) -> Self { + Self::new( + Generation(prev.generation.next()), + release_source, + system_version, + ) + } +} From 2010dca6f92630668b999b4b310bde8498c5edcf Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Wed, 22 Jan 2025 11:49:03 -0700 Subject: [PATCH 03/19] target_release datastore methods --- nexus/db-queries/src/db/datastore/mod.rs | 1 + .../src/db/datastore/target_release.rs | 268 ++++++++++++++++++ 2 files changed, 269 insertions(+) create mode 100644 nexus/db-queries/src/db/datastore/target_release.rs diff --git a/nexus/db-queries/src/db/datastore/mod.rs b/nexus/db-queries/src/db/datastore/mod.rs index b2d9f8f247..d9b403ac47 100644 --- a/nexus/db-queries/src/db/datastore/mod.rs +++ b/nexus/db-queries/src/db/datastore/mod.rs @@ -99,6 +99,7 @@ mod support_bundle; mod switch; mod switch_interface; mod switch_port; +mod target_release; #[cfg(test)] pub(crate) mod test_utils; mod update; diff --git a/nexus/db-queries/src/db/datastore/target_release.rs b/nexus/db-queries/src/db/datastore/target_release.rs new file mode 100644 index 0000000000..e3238a6f9e --- /dev/null +++ b/nexus/db-queries/src/db/datastore/target_release.rs @@ -0,0 +1,268 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +//! [`DataStore`] methods to get/set the current [`TargetRelease`]. + +use super::DataStore; +use crate::authz; +use crate::context::OpContext; +use crate::db::error::{public_error_from_diesel, ErrorHandler}; +use crate::db::model::{TargetRelease, TargetReleaseSource}; +use crate::db::schema::target_release::dsl; +use crate::transaction_retry::OptionalError; +use async_bb8_diesel::AsyncRunQueryDsl as _; +use diesel::insert_into; +use diesel::prelude::*; +use omicron_common::api::external::{CreateResult, Error, LookupResult}; + +impl DataStore { + /// Fetch the current target release, i.e., the row with the largest + /// generation number. + pub async fn get_target_release( + &self, + opctx: &OpContext, + ) -> LookupResult { + opctx.authorize(authz::Action::Read, &authz::FLEET).await?; + let conn = self.pool_connection_authorized(opctx).await?; + + // We should be able to fetch the row with the largest generation + // with one nested query, but diesel makes it unreasonably difficult + // to express: + // SELECT * FROM target_release + // WHERE generation IN (SELECT MAX(generation) FROM target_release); + // So we use two queries. This is okay because rows are immutable. + let latest: i64 = if let Some(generation) = dsl::target_release + .select(diesel::dsl::max(dsl::generation)) + .first_async::>(&*conn) + .await + .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server))? + { + generation + } else { + return Err(Error::internal_error( + "there should always be at least one target_release row", + )); + }; + dsl::target_release + .filter(dsl::generation.eq(latest)) + .select(TargetRelease::as_select()) + .first_async(&*conn) + .await + .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server)) + } + + /// Insert a new target release row and return it. It will only become + /// the current target release if its generation is larger than any + /// existing row. + pub async fn set_target_release( + &self, + opctx: &OpContext, + target_release: TargetRelease, + ) -> CreateResult { + opctx.authorize(authz::Action::Modify, &authz::FLEET).await?; + let conn = self.pool_connection_authorized(opctx).await?; + let err = OptionalError::new(); + self.transaction_retry_wrapper("set_target_release") + .transaction(&conn, |conn| { + let target_release = target_release.clone(); + let err = err.clone(); + async move { + // Ensure that we have a TUF repo representing a system version. + if let TargetReleaseSource::SystemVersion = + target_release.release_source + { + if let Some(system_version) = + target_release.system_version.clone() + { + assert_eq!( + system_version.clone(), + self.update_tuf_repo_get(opctx, system_version) + .await + .map_err(|e| err.bail(e))? + .repo + .system_version, + "inconsistent system version" + ); + } + } + + // Insert the target_release row. + insert_into(dsl::target_release) + .values(target_release) + .returning(TargetRelease::as_returning()) + .get_result_async(&conn) + .await + } + }) + .await + .map_err(|e| { + if let Some(err) = err.take() { + err + } else { + public_error_from_diesel(e, ErrorHandler::Server) + } + }) + } +} + +#[cfg(test)] +mod test { + use super::*; + use crate::db::model::{ + ArtifactHash, Generation, SemverVersion, TargetReleaseSource, + TufArtifact, TufRepo, TufRepoDescription, + }; + use crate::db::pub_test_utils::TestDatabase; + use chrono::{TimeDelta, Utc}; + use omicron_common::update::{ArtifactId, ArtifactKind}; + use omicron_test_utils::dev; + + #[tokio::test] + async fn target_release_datastore() { + let logctx = dev::test_setup_log("target_release_datastore"); + let db = TestDatabase::new_with_datastore(&logctx.log).await; + let (opctx, datastore) = (db.opctx(), db.datastore()); + + // There should always be a target release. + // This is ensured by the schema migration. + let target_release = datastore + .get_target_release(opctx) + .await + .expect("should be a target release"); + assert_eq!(target_release.generation, Generation(0.into())); + assert!(target_release.time_requested < Utc::now()); + assert_eq!( + target_release.release_source, + TargetReleaseSource::InstallDataset + ); + assert!(target_release.system_version.is_none()); + + // We should be able to set a new generation just like the first, + // with some (very small) fuzz allowed in the timestamp reported + // by the database. + let initial_target_release = TargetRelease::new_from_prev( + target_release, + TargetReleaseSource::InstallDataset, + None, + ); + let target_release = datastore + .set_target_release(opctx, initial_target_release.clone()) + .await + .unwrap(); + assert_eq!( + target_release.generation, + initial_target_release.generation + ); + assert!( + (target_release.time_requested + - initial_target_release.time_requested) + .abs() + < TimeDelta::new(0, 1_000).expect("1 μsec") + ); + assert!(target_release.system_version.is_none()); + + // Trying to reuse a generation should fail. + assert!(datastore + .set_target_release( + opctx, + TargetRelease::new( + target_release.generation, + TargetReleaseSource::InstallDataset, + None, + ) + ) + .await + .is_err()); + + // But the next generation should be fine, even with the same source. + let target_release = datastore + .set_target_release( + opctx, + TargetRelease::new_from_prev( + target_release, + TargetReleaseSource::InstallDataset, + None, + ), + ) + .await + .unwrap(); + + // If we specify a system version, it had better exist! + let _ = datastore + .set_target_release( + opctx, + TargetRelease::new_from_prev( + target_release.clone(), + TargetReleaseSource::SystemVersion, + Some(SemverVersion::new(0, 0, 0)), + ), + ) + .await + .expect_err("unknown system version"); + + // Finally, use a new generation number and a valid system version. + // We assume the queries above will have taken some non-trivial + // amount of time. + let version = SemverVersion::new(0, 0, 1); + let hash = ArtifactHash( + "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855" + .parse() + .expect("SHA256('')"), + ); + let system_version = datastore + .update_tuf_repo_insert( + opctx, + TufRepoDescription { + repo: TufRepo::new( + hash, + 0, + Utc::now(), + version.clone(), + String::from(""), + ), + artifacts: vec![TufArtifact::new( + ArtifactId { + name: String::from(""), + version: version.clone().into(), + kind: ArtifactKind::from_static("empty"), + }, + hash, + 0, + )], + }, + ) + .await + .unwrap() + .recorded + .repo + .system_version; + assert_eq!(version, system_version); + let target_release = datastore + .set_target_release( + opctx, + TargetRelease::new_from_prev( + target_release, + TargetReleaseSource::SystemVersion, + Some(version.clone()), + ), + ) + .await + .unwrap(); + assert_eq!(target_release.generation, Generation(3.into())); + assert!( + (target_release.time_requested + - initial_target_release.time_requested) + > TimeDelta::new(0, 1_000_000).expect("1 msec") + ); + assert_eq!( + target_release.release_source, + TargetReleaseSource::SystemVersion + ); + assert_eq!(target_release.system_version, Some(version)); + + // Clean up. + db.terminate().await; + logctx.cleanup_successful(); + } +} From 816970be51878ca6b8bf8bf8cb1a6f3c950e844f Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Mon, 10 Feb 2025 15:37:05 -0700 Subject: [PATCH 04/19] target_release API --- common/src/api/external/mod.rs | 1 + nexus/auth/src/authz/api_resources.rs | 8 + nexus/db-model/src/target_release.rs | 20 ++ .../src/policy_test/resource_builder.rs | 1 + nexus/external-api/src/lib.rs | 33 ++++ nexus/src/external_api/http_entrypoints.rs | 65 +++++++ nexus/tests/integration_tests/mod.rs | 1 + .../tests/integration_tests/target_release.rs | 171 ++++++++++++++++++ nexus/types/src/external_api/params.rs | 8 +- nexus/types/src/external_api/views.rs | 28 ++- 10 files changed, 334 insertions(+), 2 deletions(-) create mode 100644 nexus/tests/integration_tests/target_release.rs diff --git a/common/src/api/external/mod.rs b/common/src/api/external/mod.rs index 8adb45beb5..a8416a1fdc 100644 --- a/common/src/api/external/mod.rs +++ b/common/src/api/external/mod.rs @@ -1046,6 +1046,7 @@ pub enum ResourceType { Oximeter, MetricProducer, RoleBuiltin, + TargetRelease, TufRepo, TufArtifact, SwitchPort, diff --git a/nexus/auth/src/authz/api_resources.rs b/nexus/auth/src/authz/api_resources.rs index 745a699cf2..e65b605526 100644 --- a/nexus/auth/src/authz/api_resources.rs +++ b/nexus/auth/src/authz/api_resources.rs @@ -885,6 +885,14 @@ authz_resource! { polar_snippet = FleetChild, } +authz_resource! { + name = "TargetRelease", + parent = "Rack", + primary_key = u64, // generation + roles_allowed = false, + polar_snippet = FleetChild, +} + authz_resource! { name = "Silo", parent = "Fleet", diff --git a/nexus/db-model/src/target_release.rs b/nexus/db-model/src/target_release.rs index 2c3a2d24bf..b18771747d 100644 --- a/nexus/db-model/src/target_release.rs +++ b/nexus/db-model/src/target_release.rs @@ -6,6 +6,7 @@ use super::{impl_enum_type, Generation}; use crate::schema::target_release; use crate::SemverVersion; use chrono::{DateTime, Utc}; +use nexus_types::external_api::views; impl_enum_type!( #[derive(SqlType, Debug, QueryId)] @@ -65,4 +66,23 @@ impl TargetRelease { system_version, ) } + + pub fn into_external(self) -> views::TargetRelease { + views::TargetRelease { + generation: (&self.generation.0).into(), + time_requested: self.time_requested, + release_source: match self.release_source { + TargetReleaseSource::InstallDataset => { + views::TargetReleaseSource::InstallDataset + } + TargetReleaseSource::SystemVersion => { + views::TargetReleaseSource::SystemVersion( + self.system_version + .expect("CONSTRAINT system_version_for_release") + .into(), + ) + } + }, + } + } } diff --git a/nexus/db-queries/src/policy_test/resource_builder.rs b/nexus/db-queries/src/policy_test/resource_builder.rs index b6d7d97553..8bb54086ba 100644 --- a/nexus/db-queries/src/policy_test/resource_builder.rs +++ b/nexus/db-queries/src/policy_test/resource_builder.rs @@ -272,6 +272,7 @@ impl_dyn_authorized_resource_for_resource!(authz::Sled); impl_dyn_authorized_resource_for_resource!(authz::Snapshot); impl_dyn_authorized_resource_for_resource!(authz::SshKey); impl_dyn_authorized_resource_for_resource!(authz::SupportBundle); +impl_dyn_authorized_resource_for_resource!(authz::TargetRelease); impl_dyn_authorized_resource_for_resource!(authz::TufArtifact); impl_dyn_authorized_resource_for_resource!(authz::TufRepo); impl_dyn_authorized_resource_for_resource!(authz::Vpc); diff --git a/nexus/external-api/src/lib.rs b/nexus/external-api/src/lib.rs index d76192072a..528ae57447 100644 --- a/nexus/external-api/src/lib.rs +++ b/nexus/external-api/src/lib.rs @@ -2662,6 +2662,39 @@ pub trait NexusExternalApi { path_params: Path, ) -> Result, HttpError>; + /// Get the current target release of the rack's system software + /// + /// This may not correpond 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"], + unpublished = true, + }] + async fn system_update_get_target_release( + 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. + #[endpoint { + method = POST, + path = "/v1/system/update/target-release", + tags = ["system/update"], + unpublished = true, + }] + async fn system_update_set_target_release( + rqctx: RequestContext, + params: TypedBody, + ) -> Result, HttpError>; + // Silo users /// List users diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index 191c304216..f45b4937f9 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -5732,6 +5732,71 @@ impl NexusExternalApi for NexusExternalApiImpl { .await } + async fn system_update_get_target_release( + rqctx: RequestContext, + ) -> 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?; + Ok(HttpResponseOk( + nexus + .datastore() + .get_target_release(&opctx) + .await? + .into_external(), + )) + }; + apictx + .context + .external_latencies + .instrument_dropshot_handler(&rqctx, handler) + .await + } + + async fn system_update_set_target_release( + rqctx: RequestContext, + body: TypedBody, + ) -> 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 = body.into_inner(); + let (release_source, version) = match params.release_source { + views::TargetReleaseSource::InstallDataset => { + (nexus_db_model::TargetReleaseSource::InstallDataset, None) + } + views::TargetReleaseSource::SystemVersion(version) => ( + nexus_db_model::TargetReleaseSource::SystemVersion, + Some(version), + ), + }; + let current_target_release = + nexus.datastore().get_target_release(&opctx).await?; + let next_target_release = + nexus_db_model::TargetRelease::new_from_prev( + current_target_release, + release_source, + version.map(nexus_db_model::SemverVersion), + ); + Ok(HttpResponseCreated( + nexus + .datastore() + .set_target_release(&opctx, next_target_release) + .await? + .into_external(), + )) + }; + apictx + .context + .external_latencies + .instrument_dropshot_handler(&rqctx, handler) + .await + } + // Silo users async fn user_list( diff --git a/nexus/tests/integration_tests/mod.rs b/nexus/tests/integration_tests/mod.rs index dc404736cd..ee927739b3 100644 --- a/nexus/tests/integration_tests/mod.rs +++ b/nexus/tests/integration_tests/mod.rs @@ -45,6 +45,7 @@ mod ssh_keys; mod subnet_allocation; mod support_bundles; mod switch_port; +mod target_release; mod unauthorized; mod unauthorized_coverage; mod updates; diff --git a/nexus/tests/integration_tests/target_release.rs b/nexus/tests/integration_tests/target_release.rs new file mode 100644 index 0000000000..e9d225153d --- /dev/null +++ b/nexus/tests/integration_tests/target_release.rs @@ -0,0 +1,171 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +//! Get/set the target release via the external API. + +use camino_tempfile::Utf8TempDir; +use chrono::{TimeDelta, Utc}; +use clap::Parser as _; +use dropshot::test_util::LogContext; +use nexus_config::UpdatesConfig; +use nexus_test_utils::http_testing::AuthnMode; +use nexus_test_utils::http_testing::{NexusRequest, RequestBuilder}; +use nexus_test_utils::load_test_config; +use nexus_test_utils::test_setup_with_config; +use nexus_types::external_api::params::SetTargetReleaseParams; +use nexus_types::external_api::views::{TargetRelease, TargetReleaseSource}; +use omicron_common::api::external::{SemverVersion, TufRepoInsertResponse}; +use omicron_sled_agent::sim; + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn get_set_target_release() -> anyhow::Result<()> { + let mut config = load_test_config(); + config.pkg.updates = Some(UpdatesConfig { + // XXX: This is currently not used by the update system, but + // trusted_root will become meaningful in the future. + trusted_root: "does-not-exist.json".into(), + }); + let ctx = test_setup_with_config::( + "test_update_uninitialized", + &mut config, + sim::SimMode::Explicit, + None, + 0, + ) + .await; + let client = &ctx.external_client; + + // 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_eq!(target_release.generation, 0); + assert!(target_release.time_requested < Utc::now()); + assert_eq!( + target_release.release_source, + TargetReleaseSource::InstallDataset + ); + + // We should be able to set a new generation of target release + // still set to whatever is on the `install` dataset. + let target_release: TargetRelease = NexusRequest::objects_post( + client, + "/v1/system/update/target-release", + &SetTargetReleaseParams { + release_source: TargetReleaseSource::InstallDataset, + }, + ) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .unwrap() + .parsed_body() + .unwrap(); + assert_eq!(target_release.generation, 1); + assert!( + (target_release.time_requested - Utc::now()).abs() + < TimeDelta::new(1, 0).expect("1 sec") + ); + assert_eq!( + target_release.release_source, + TargetReleaseSource::InstallDataset, + ); + + // Fetching the current target release should now return the same. + assert_eq!( + NexusRequest::object_get(client, "/v1/system/update/target-release") + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .unwrap() + .parsed_body::() + .unwrap(), + target_release, + ); + + // Attempting to set an invalid system version should fail. + let version = SemverVersion::new(0, 0, 0); + NexusRequest::objects_post( + client, + "/v1/system/update/target-release", + &SetTargetReleaseParams { + release_source: TargetReleaseSource::SystemVersion(version), + }, + ) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .expect_err("invalid TUF repo"); + + // Finally, adding a fake (tufaceous) repo and then setting it as the + // target release should succeed. + let version = SemverVersion::new(1, 0, 0); + let logctx = LogContext::new("get_set_target_release", &config.pkg.log); + let temp = Utf8TempDir::new().unwrap(); + let path = temp.path().join("repo.zip"); + tufaceous::Args::try_parse_from([ + "tufaceous", + "assemble", + "../tufaceous/manifests/fake.toml", + path.as_str(), + ]) + .expect("can't parse tufaceous args") + .exec(&logctx.log) + .await + .expect("can't assemble TUF repo"); + + assert_eq!( + version, + NexusRequest::new( + RequestBuilder::new( + client, + http::Method::PUT, + "/v1/system/update/repository?file_name=/tmp/foo.zip", + ) + .body_file(Some(&path)) + .expect_status(Some(http::StatusCode::OK)), + ) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .unwrap() + .parsed_body::() + .unwrap() + .recorded + .repo + .system_version + ); + + let target_release: TargetRelease = NexusRequest::objects_post( + client, + "/v1/system/update/target-release", + &SetTargetReleaseParams { + release_source: TargetReleaseSource::SystemVersion(version.clone()), + }, + ) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .unwrap() + .parsed_body() + .unwrap(); + assert_eq!(target_release.generation, 2); + assert!( + (target_release.time_requested - Utc::now()).abs() + < TimeDelta::new(1, 0).expect("1 sec") + ); + assert_eq!( + target_release.release_source, + TargetReleaseSource::SystemVersion(version), + ); + + ctx.teardown().await; + logctx.cleanup_successful(); + Ok(()) +} diff --git a/nexus/types/src/external_api/params.rs b/nexus/types/src/external_api/params.rs index 8294e23e2f..f44ede0a2f 100644 --- a/nexus/types/src/external_api/params.rs +++ b/nexus/types/src/external_api/params.rs @@ -2165,13 +2165,19 @@ pub struct UpdatesPutRepositoryParams { } /// Parameters for GET requests for `/v1/system/update/repository`. - #[derive(Clone, Debug, Deserialize, JsonSchema)] pub struct UpdatesGetRepositoryParams { /// The version to get. pub system_version: SemverVersion, } +/// Parameters for POST requests to `/v1/system/update/target-release`. +#[derive(Clone, Debug, Deserialize, JsonSchema, Serialize)] +pub struct SetTargetReleaseParams { + /// Source of the requested target release. + pub release_source: super::views::TargetReleaseSource, +} + // Probes /// Create time parameters for probes. diff --git a/nexus/types/src/external_api/views.rs b/nexus/types/src/external_api/views.rs index 3bfda3cf0b..37624f602c 100644 --- a/nexus/types/src/external_api/views.rs +++ b/nexus/types/src/external_api/views.rs @@ -15,7 +15,7 @@ use daft::Diffable; use omicron_common::api::external::{ AllowedSourceIps as ExternalAllowedSourceIps, ByteCount, Digest, Error, IdentityMetadata, InstanceState, Name, ObjectIdentity, RoleName, - SimpleIdentityOrName, + SemverVersion, SimpleIdentityOrName, }; use oxnet::{Ipv4Net, Ipv6Net}; use schemars::JsonSchema; @@ -873,6 +873,32 @@ impl fmt::Display for PhysicalDiskState { } } +// UPDATES + +/// The source of the target release. +#[derive(Clone, Debug, Deserialize, Eq, PartialEq, JsonSchema, Serialize)] +#[serde(rename_all = "snake_case")] +pub enum TargetReleaseSource { + /// Obtain the release artifact from the `install` dataset. + InstallDataset, + + /// Use the specified release of the rack's system software. + SystemVersion(SemverVersion), +} + +/// View of a system software target release +#[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize, JsonSchema)] +pub struct TargetRelease { + /// The target-release generation. + pub generation: i64, + + /// The time it was or is to be set as the target release. + pub time_requested: DateTime, + + /// The source of the target release. + pub release_source: TargetReleaseSource, +} + // SILO USERS /// View of a User From 1a5b4559763adc49a1c36fc2ac47d6085e59131e Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Tue, 11 Feb 2025 12:48:44 -0700 Subject: [PATCH 05/19] One query to fetch the current target_release --- .../src/db/datastore/target_release.rs | 30 +++++++------------ 1 file changed, 10 insertions(+), 20 deletions(-) diff --git a/nexus/db-queries/src/db/datastore/target_release.rs b/nexus/db-queries/src/db/datastore/target_release.rs index e3238a6f9e..c68c6c867e 100644 --- a/nexus/db-queries/src/db/datastore/target_release.rs +++ b/nexus/db-queries/src/db/datastore/target_release.rs @@ -14,7 +14,7 @@ use crate::transaction_retry::OptionalError; use async_bb8_diesel::AsyncRunQueryDsl as _; use diesel::insert_into; use diesel::prelude::*; -use omicron_common::api::external::{CreateResult, Error, LookupResult}; +use omicron_common::api::external::{CreateResult, LookupResult}; impl DataStore { /// Fetch the current target release, i.e., the row with the largest @@ -26,26 +26,16 @@ impl DataStore { opctx.authorize(authz::Action::Read, &authz::FLEET).await?; let conn = self.pool_connection_authorized(opctx).await?; - // We should be able to fetch the row with the largest generation - // with one nested query, but diesel makes it unreasonably difficult - // to express: - // SELECT * FROM target_release - // WHERE generation IN (SELECT MAX(generation) FROM target_release); - // So we use two queries. This is okay because rows are immutable. - let latest: i64 = if let Some(generation) = dsl::target_release - .select(diesel::dsl::max(dsl::generation)) - .first_async::>(&*conn) - .await - .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server))? - { - generation - } else { - return Err(Error::internal_error( - "there should always be at least one target_release row", - )); - }; + // Fetch the row in the `target_release` table with the largest + // generation number. The subquery accesses the same table, so we + // have to make an alias to not confuse diesel. + let target_release2 = diesel::alias!( + crate::db::schema::target_release as target_release_2 + ); dsl::target_release - .filter(dsl::generation.eq(latest)) + .filter(dsl::generation.nullable().eq_any(target_release2.select( + diesel::dsl::max(target_release2.field(dsl::generation)), + ))) .select(TargetRelease::as_select()) .first_async(&*conn) .await From fe23073e9469892d20aed86429c7048c99e8edc3 Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Tue, 11 Feb 2025 13:54:43 -0700 Subject: [PATCH 06/19] Move views into shared module --- nexus/db-model/src/target_release.rs | 10 +++---- nexus/external-api/src/lib.rs | 4 +-- nexus/src/external_api/http_entrypoints.rs | 8 +++--- .../tests/integration_tests/target_release.rs | 2 +- nexus/types/src/external_api/params.rs | 2 +- nexus/types/src/external_api/shared.rs | 25 +++++++++++++++++ nexus/types/src/external_api/views.rs | 28 +------------------ 7 files changed, 39 insertions(+), 40 deletions(-) diff --git a/nexus/db-model/src/target_release.rs b/nexus/db-model/src/target_release.rs index b18771747d..db79421594 100644 --- a/nexus/db-model/src/target_release.rs +++ b/nexus/db-model/src/target_release.rs @@ -6,7 +6,7 @@ use super::{impl_enum_type, Generation}; use crate::schema::target_release; use crate::SemverVersion; use chrono::{DateTime, Utc}; -use nexus_types::external_api::views; +use nexus_types::external_api::shared; impl_enum_type!( #[derive(SqlType, Debug, QueryId)] @@ -67,16 +67,16 @@ impl TargetRelease { ) } - pub fn into_external(self) -> views::TargetRelease { - views::TargetRelease { + pub fn into_external(self) -> shared::TargetRelease { + shared::TargetRelease { generation: (&self.generation.0).into(), time_requested: self.time_requested, release_source: match self.release_source { TargetReleaseSource::InstallDataset => { - views::TargetReleaseSource::InstallDataset + shared::TargetReleaseSource::InstallDataset } TargetReleaseSource::SystemVersion => { - views::TargetReleaseSource::SystemVersion( + shared::TargetReleaseSource::SystemVersion( self.system_version .expect("CONSTRAINT system_version_for_release") .into(), diff --git a/nexus/external-api/src/lib.rs b/nexus/external-api/src/lib.rs index 528ae57447..1a6f91f32c 100644 --- a/nexus/external-api/src/lib.rs +++ b/nexus/external-api/src/lib.rs @@ -2677,7 +2677,7 @@ pub trait NexusExternalApi { }] async fn system_update_get_target_release( rqctx: RequestContext, - ) -> Result, HttpError>; + ) -> Result, HttpError>; /// Set the current target release of the rack's system software /// @@ -2693,7 +2693,7 @@ pub trait NexusExternalApi { async fn system_update_set_target_release( rqctx: RequestContext, params: TypedBody, - ) -> Result, HttpError>; + ) -> Result, HttpError>; // Silo users diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index f45b4937f9..b6a5980e78 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -5734,7 +5734,7 @@ impl NexusExternalApi for NexusExternalApiImpl { async fn system_update_get_target_release( rqctx: RequestContext, - ) -> Result, HttpError> { + ) -> Result, HttpError> { let apictx = rqctx.context(); let nexus = &apictx.context.nexus; let handler = async { @@ -5758,7 +5758,7 @@ impl NexusExternalApi for NexusExternalApiImpl { async fn system_update_set_target_release( rqctx: RequestContext, body: TypedBody, - ) -> Result, HttpError> { + ) -> Result, HttpError> { let apictx = rqctx.context(); let nexus = &apictx.context.nexus; let handler = async { @@ -5766,10 +5766,10 @@ impl NexusExternalApi for NexusExternalApiImpl { crate::context::op_context_for_external_api(&rqctx).await?; let params = body.into_inner(); let (release_source, version) = match params.release_source { - views::TargetReleaseSource::InstallDataset => { + shared::TargetReleaseSource::InstallDataset => { (nexus_db_model::TargetReleaseSource::InstallDataset, None) } - views::TargetReleaseSource::SystemVersion(version) => ( + shared::TargetReleaseSource::SystemVersion(version) => ( nexus_db_model::TargetReleaseSource::SystemVersion, Some(version), ), diff --git a/nexus/tests/integration_tests/target_release.rs b/nexus/tests/integration_tests/target_release.rs index e9d225153d..75dfe865c3 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::load_test_config; use nexus_test_utils::test_setup_with_config; use nexus_types::external_api::params::SetTargetReleaseParams; -use nexus_types::external_api::views::{TargetRelease, TargetReleaseSource}; +use nexus_types::external_api::shared::{TargetRelease, TargetReleaseSource}; use omicron_common::api::external::{SemverVersion, TufRepoInsertResponse}; use omicron_sled_agent::sim; diff --git a/nexus/types/src/external_api/params.rs b/nexus/types/src/external_api/params.rs index f44ede0a2f..a1ee009a4e 100644 --- a/nexus/types/src/external_api/params.rs +++ b/nexus/types/src/external_api/params.rs @@ -2175,7 +2175,7 @@ pub struct UpdatesGetRepositoryParams { #[derive(Clone, Debug, Deserialize, JsonSchema, Serialize)] pub struct SetTargetReleaseParams { /// Source of the requested target release. - pub release_source: super::views::TargetReleaseSource, + pub release_source: shared::TargetReleaseSource, } // Probes diff --git a/nexus/types/src/external_api/shared.rs b/nexus/types/src/external_api/shared.rs index ae8214a637..831a1754bf 100644 --- a/nexus/types/src/external_api/shared.rs +++ b/nexus/types/src/external_api/shared.rs @@ -11,6 +11,7 @@ use anyhow::Context; use chrono::DateTime; use chrono::Utc; use omicron_common::api::external::Name; +use omicron_common::api::external::SemverVersion; use omicron_common::api::internal::shared::NetworkInterface; use omicron_uuid_kinds::SupportBundleUuid; use parse_display::FromStr; @@ -510,3 +511,27 @@ impl RelayState { .context("json from relay state string") } } + +/// The source of the target release. +#[derive(Clone, Debug, Deserialize, Eq, PartialEq, JsonSchema, Serialize)] +#[serde(rename_all = "snake_case")] +pub enum TargetReleaseSource { + /// Obtain the release artifact from the `install` dataset. + InstallDataset, + + /// Use the specified release of the rack's system software. + SystemVersion(SemverVersion), +} + +/// View of a system software target release +#[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize, JsonSchema)] +pub struct TargetRelease { + /// The target-release generation. + pub generation: i64, + + /// The time it was or is to be set as the target release. + pub time_requested: DateTime, + + /// The source of the target release. + pub release_source: TargetReleaseSource, +} diff --git a/nexus/types/src/external_api/views.rs b/nexus/types/src/external_api/views.rs index 37624f602c..3bfda3cf0b 100644 --- a/nexus/types/src/external_api/views.rs +++ b/nexus/types/src/external_api/views.rs @@ -15,7 +15,7 @@ use daft::Diffable; use omicron_common::api::external::{ AllowedSourceIps as ExternalAllowedSourceIps, ByteCount, Digest, Error, IdentityMetadata, InstanceState, Name, ObjectIdentity, RoleName, - SemverVersion, SimpleIdentityOrName, + SimpleIdentityOrName, }; use oxnet::{Ipv4Net, Ipv6Net}; use schemars::JsonSchema; @@ -873,32 +873,6 @@ impl fmt::Display for PhysicalDiskState { } } -// UPDATES - -/// The source of the target release. -#[derive(Clone, Debug, Deserialize, Eq, PartialEq, JsonSchema, Serialize)] -#[serde(rename_all = "snake_case")] -pub enum TargetReleaseSource { - /// Obtain the release artifact from the `install` dataset. - InstallDataset, - - /// Use the specified release of the rack's system software. - SystemVersion(SemverVersion), -} - -/// View of a system software target release -#[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize, JsonSchema)] -pub struct TargetRelease { - /// The target-release generation. - pub generation: i64, - - /// The time it was or is to be set as the target release. - pub time_requested: DateTime, - - /// The source of the target release. - pub release_source: TargetReleaseSource, -} - // SILO USERS /// View of a User From 80f69029c800b42836dfd4e6ec4dfe1b0191a693 Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Sun, 23 Feb 2025 13:33:34 -0700 Subject: [PATCH 07/19] =?UTF-8?q?InstallDataset=20=E2=86=92=20Unspecified,?= =?UTF-8?q?=20resolve=20tuf=5Frepo=5Fid?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- nexus/db-model/src/schema.rs | 2 +- nexus/db-model/src/target_release.rs | 37 ++--- .../src/db/datastore/target_release.rs | 135 +++++++++--------- nexus/src/external_api/http_entrypoints.rs | 34 +++-- .../tests/integration_tests/target_release.rs | 56 ++------ nexus/types/src/external_api/shared.rs | 8 +- schema/crdb/create-target-release/up1.sql | 2 +- schema/crdb/create-target-release/up2.sql | 9 +- schema/crdb/create-target-release/up3.sql | 6 +- schema/crdb/dbinit.sql | 17 +-- 10 files changed, 133 insertions(+), 173 deletions(-) diff --git a/nexus/db-model/src/schema.rs b/nexus/db-model/src/schema.rs index 2a270aaa28..db11a5555a 100644 --- a/nexus/db-model/src/schema.rs +++ b/nexus/db-model/src/schema.rs @@ -1417,7 +1417,7 @@ table! { generation -> Int8, time_requested -> Timestamptz, release_source -> crate::TargetReleaseSourceEnum, - system_version -> Nullable, + tuf_repo_id -> Nullable, } } diff --git a/nexus/db-model/src/target_release.rs b/nexus/db-model/src/target_release.rs index db79421594..2173bd7c7a 100644 --- a/nexus/db-model/src/target_release.rs +++ b/nexus/db-model/src/target_release.rs @@ -4,9 +4,9 @@ use super::{impl_enum_type, Generation}; use crate::schema::target_release; -use crate::SemverVersion; +use crate::typed_uuid::DbTypedUuid; use chrono::{DateTime, Utc}; -use nexus_types::external_api::shared; +use omicron_uuid_kinds::TufRepoKind; impl_enum_type!( #[derive(SqlType, Debug, QueryId)] @@ -18,7 +18,7 @@ impl_enum_type!( #[diesel(sql_type = TargetReleaseSourceEnum)] pub enum TargetReleaseSource; - InstallDataset => b"install_dataset" + Unspecified => b"unspecified" SystemVersion => b"system_version" ); @@ -37,52 +37,33 @@ pub struct TargetRelease { /// The source of the target release. pub release_source: TargetReleaseSource, - /// The semantic version of the target release. - pub system_version: Option, + /// The TUF repo containing the target release. + pub tuf_repo_id: Option>, } impl TargetRelease { pub fn new( generation: Generation, release_source: TargetReleaseSource, - system_version: Option, + tuf_repo_id: Option>, ) -> Self { Self { generation, time_requested: Utc::now(), release_source, - system_version, + tuf_repo_id, } } pub fn new_from_prev( prev: TargetRelease, release_source: TargetReleaseSource, - system_version: Option, + tuf_repo_id: Option>, ) -> Self { Self::new( Generation(prev.generation.next()), release_source, - system_version, + tuf_repo_id, ) } - - pub fn into_external(self) -> shared::TargetRelease { - shared::TargetRelease { - generation: (&self.generation.0).into(), - time_requested: self.time_requested, - release_source: match self.release_source { - TargetReleaseSource::InstallDataset => { - shared::TargetReleaseSource::InstallDataset - } - TargetReleaseSource::SystemVersion => { - shared::TargetReleaseSource::SystemVersion( - self.system_version - .expect("CONSTRAINT system_version_for_release") - .into(), - ) - } - }, - } - } } diff --git a/nexus/db-queries/src/db/datastore/target_release.rs b/nexus/db-queries/src/db/datastore/target_release.rs index c68c6c867e..b542bf7db5 100644 --- a/nexus/db-queries/src/db/datastore/target_release.rs +++ b/nexus/db-queries/src/db/datastore/target_release.rs @@ -8,12 +8,12 @@ use super::DataStore; use crate::authz; use crate::context::OpContext; use crate::db::error::{public_error_from_diesel, ErrorHandler}; -use crate::db::model::{TargetRelease, TargetReleaseSource}; +use crate::db::model::{SemverVersion, TargetRelease, TargetReleaseSource}; use crate::db::schema::target_release::dsl; -use crate::transaction_retry::OptionalError; use async_bb8_diesel::AsyncRunQueryDsl as _; use diesel::insert_into; use diesel::prelude::*; +use nexus_types::external_api::shared; use omicron_common::api::external::{CreateResult, LookupResult}; impl DataStore { @@ -29,12 +29,12 @@ impl DataStore { // Fetch the row in the `target_release` table with the largest // generation number. The subquery accesses the same table, so we // have to make an alias to not confuse diesel. - let target_release2 = diesel::alias!( + let target_release_2 = diesel::alias!( crate::db::schema::target_release as target_release_2 ); dsl::target_release - .filter(dsl::generation.nullable().eq_any(target_release2.select( - diesel::dsl::max(target_release2.field(dsl::generation)), + .filter(dsl::generation.nullable().eq_any(target_release_2.select( + diesel::dsl::max(target_release_2.field(dsl::generation)), ))) .select(TargetRelease::as_select()) .first_async(&*conn) @@ -52,32 +52,10 @@ impl DataStore { ) -> CreateResult { opctx.authorize(authz::Action::Modify, &authz::FLEET).await?; let conn = self.pool_connection_authorized(opctx).await?; - let err = OptionalError::new(); self.transaction_retry_wrapper("set_target_release") .transaction(&conn, |conn| { let target_release = target_release.clone(); - let err = err.clone(); async move { - // Ensure that we have a TUF repo representing a system version. - if let TargetReleaseSource::SystemVersion = - target_release.release_source - { - if let Some(system_version) = - target_release.system_version.clone() - { - assert_eq!( - system_version.clone(), - self.update_tuf_repo_get(opctx, system_version) - .await - .map_err(|e| err.bail(e))? - .repo - .system_version, - "inconsistent system version" - ); - } - } - - // Insert the target_release row. insert_into(dsl::target_release) .values(target_release) .returning(TargetRelease::as_returning()) @@ -86,13 +64,49 @@ impl DataStore { } }) .await - .map_err(|e| { - if let Some(err) = err.take() { - err - } else { - public_error_from_diesel(e, ErrorHandler::Server) + .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 export_target_release( + &self, + opctx: &OpContext, + target_release: &TargetRelease, + ) -> LookupResult { + opctx.authorize(authz::Action::Read, &authz::FLEET).await?; + let conn = self.pool_connection_authorized(opctx).await?; + Ok(shared::TargetRelease { + generation: (&target_release.generation.0).into(), + time_requested: target_release.time_requested, + release_source: match target_release.release_source { + TargetReleaseSource::Unspecified => { + shared::TargetReleaseSource::Unspecified } - }) + TargetReleaseSource::SystemVersion => { + use crate::db::schema::tuf_repo; + shared::TargetReleaseSource::SystemVersion( + tuf_repo::table + .select(tuf_repo::system_version) + .filter(tuf_repo::id.eq( + target_release.tuf_repo_id.expect( + "CONSTRAINT tuf_repo_for_system_version", + ), + )) + .first_async::(&*conn) + .await + .map_err(|e| { + public_error_from_diesel( + e, + ErrorHandler::Server, + ) + })? + .into(), + ) + } + }, + }) } } @@ -120,20 +134,20 @@ mod test { .get_target_release(opctx) .await .expect("should be a target release"); - assert_eq!(target_release.generation, Generation(0.into())); + assert_eq!(target_release.generation, Generation(1.into())); assert!(target_release.time_requested < Utc::now()); assert_eq!( target_release.release_source, - TargetReleaseSource::InstallDataset + TargetReleaseSource::Unspecified ); - assert!(target_release.system_version.is_none()); + assert!(target_release.tuf_repo_id.is_none()); // We should be able to set a new generation just like the first, // with some (very small) fuzz allowed in the timestamp reported // by the database. let initial_target_release = TargetRelease::new_from_prev( target_release, - TargetReleaseSource::InstallDataset, + TargetReleaseSource::Unspecified, None, ); let target_release = datastore @@ -150,7 +164,7 @@ mod test { .abs() < TimeDelta::new(0, 1_000).expect("1 μsec") ); - assert!(target_release.system_version.is_none()); + assert!(target_release.tuf_repo_id.is_none()); // Trying to reuse a generation should fail. assert!(datastore @@ -158,7 +172,7 @@ mod test { opctx, TargetRelease::new( target_release.generation, - TargetReleaseSource::InstallDataset, + TargetReleaseSource::Unspecified, None, ) ) @@ -171,36 +185,21 @@ mod test { opctx, TargetRelease::new_from_prev( target_release, - TargetReleaseSource::InstallDataset, + TargetReleaseSource::Unspecified, None, ), ) .await .unwrap(); - // If we specify a system version, it had better exist! - let _ = datastore - .set_target_release( - opctx, - TargetRelease::new_from_prev( - target_release.clone(), - TargetReleaseSource::SystemVersion, - Some(SemverVersion::new(0, 0, 0)), - ), - ) - .await - .expect_err("unknown system version"); - - // Finally, use a new generation number and a valid system version. - // We assume the queries above will have taken some non-trivial - // amount of time. + // Finally, use a new generation number and a TUF repo source. let version = SemverVersion::new(0, 0, 1); let hash = ArtifactHash( "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855" .parse() .expect("SHA256('')"), ); - let system_version = datastore + let repo = datastore .update_tuf_repo_insert( opctx, TufRepoDescription { @@ -225,31 +224,31 @@ mod test { .await .unwrap() .recorded - .repo - .system_version; - assert_eq!(version, system_version); + .repo; + assert_eq!(repo.system_version, version); + let tuf_repo_id = repo.id; + + let before = Utc::now(); let target_release = datastore .set_target_release( opctx, TargetRelease::new_from_prev( target_release, TargetReleaseSource::SystemVersion, - Some(version.clone()), + Some(tuf_repo_id), ), ) .await .unwrap(); - assert_eq!(target_release.generation, Generation(3.into())); - assert!( - (target_release.time_requested - - initial_target_release.time_requested) - > TimeDelta::new(0, 1_000_000).expect("1 msec") - ); + let after = Utc::now(); + assert_eq!(target_release.generation, Generation(4.into())); + assert!(target_release.time_requested >= before); + assert!(target_release.time_requested <= after); assert_eq!( target_release.release_source, TargetReleaseSource::SystemVersion ); - assert_eq!(target_release.system_version, Some(version)); + assert_eq!(target_release.tuf_repo_id, Some(tuf_repo_id)); // Clean up. db.terminate().await; diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index 710ea392bf..4d12600a24 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -6095,12 +6095,13 @@ impl NexusExternalApi for NexusExternalApiImpl { let handler = async { let opctx = crate::context::op_context_for_external_api(&rqctx).await?; + let target_release = + nexus.datastore().get_target_release(&opctx).await?; Ok(HttpResponseOk( nexus .datastore() - .get_target_release(&opctx) - .await? - .into_external(), + .export_target_release(&opctx, &target_release) + .await?, )) }; apictx @@ -6121,28 +6122,43 @@ impl NexusExternalApi for NexusExternalApiImpl { crate::context::op_context_for_external_api(&rqctx).await?; let params = body.into_inner(); let (release_source, version) = match params.release_source { - shared::TargetReleaseSource::InstallDataset => { - (nexus_db_model::TargetReleaseSource::InstallDataset, None) + shared::TargetReleaseSource::Unspecified => { + (nexus_db_model::TargetReleaseSource::Unspecified, None) } shared::TargetReleaseSource::SystemVersion(version) => ( nexus_db_model::TargetReleaseSource::SystemVersion, Some(version), ), }; + let tuf_repo_id = if let Some(version) = version { + Some( + nexus + .datastore() + .update_tuf_repo_get(&opctx, version.into()) + .await? + .repo + .id, + ) + } else { + None + }; let current_target_release = nexus.datastore().get_target_release(&opctx).await?; let next_target_release = nexus_db_model::TargetRelease::new_from_prev( current_target_release, release_source, - version.map(nexus_db_model::SemverVersion), + tuf_repo_id, ); + let target_release = nexus + .datastore() + .set_target_release(&opctx, next_target_release) + .await?; Ok(HttpResponseCreated( nexus .datastore() - .set_target_release(&opctx, next_target_release) - .await? - .into_external(), + .export_target_release(&opctx, &target_release) + .await?, )) }; apictx diff --git a/nexus/tests/integration_tests/target_release.rs b/nexus/tests/integration_tests/target_release.rs index 044e46cb07..b6c198a168 100644 --- a/nexus/tests/integration_tests/target_release.rs +++ b/nexus/tests/integration_tests/target_release.rs @@ -5,7 +5,7 @@ //! Get/set the target release via the external API. use camino_tempfile::Utf8TempDir; -use chrono::{TimeDelta, Utc}; +use chrono::Utc; use clap::Parser as _; use dropshot::test_util::LogContext; use nexus_config::UpdatesConfig; @@ -46,49 +46,9 @@ async fn get_set_target_release() -> anyhow::Result<()> { .unwrap() .parsed_body() .unwrap(); - assert_eq!(target_release.generation, 0); - assert!(target_release.time_requested < Utc::now()); - assert_eq!( - target_release.release_source, - TargetReleaseSource::InstallDataset - ); - - // We should be able to set a new generation of target release - // still set to whatever is on the `install` dataset. - let target_release: TargetRelease = NexusRequest::objects_post( - client, - "/v1/system/update/target-release", - &SetTargetReleaseParams { - release_source: TargetReleaseSource::InstallDataset, - }, - ) - .authn_as(AuthnMode::PrivilegedUser) - .execute() - .await - .unwrap() - .parsed_body() - .unwrap(); assert_eq!(target_release.generation, 1); - assert!( - (target_release.time_requested - Utc::now()).abs() - < TimeDelta::new(1, 0).expect("1 sec") - ); - assert_eq!( - target_release.release_source, - TargetReleaseSource::InstallDataset, - ); - - // Fetching the current target release should now return the same. - assert_eq!( - NexusRequest::object_get(client, "/v1/system/update/target-release") - .authn_as(AuthnMode::PrivilegedUser) - .execute() - .await - .unwrap() - .parsed_body::() - .unwrap(), - target_release, - ); + assert!(target_release.time_requested < Utc::now()); + assert_eq!(target_release.release_source, TargetReleaseSource::Unspecified); // Attempting to set an invalid system version should fail. let version = Version::new(0, 0, 0); @@ -104,8 +64,9 @@ async fn get_set_target_release() -> anyhow::Result<()> { .await .expect_err("invalid TUF repo"); - // Finally, adding a fake (tufaceous) repo and then setting it as the + // Adding a fake (tufaceous) repo and then setting it as the // target release should succeed. + let before = Utc::now(); let version = Version::new(1, 0, 0); let logctx = LogContext::new("get_set_target_release", &config.pkg.log); let temp = Utf8TempDir::new().unwrap(); @@ -156,11 +117,10 @@ async fn get_set_target_release() -> anyhow::Result<()> { .unwrap() .parsed_body() .unwrap(); + let after = Utc::now(); assert_eq!(target_release.generation, 2); - assert!( - (target_release.time_requested - Utc::now()).abs() - < TimeDelta::new(1, 0).expect("1 sec") - ); + assert!(target_release.time_requested >= before); + assert!(target_release.time_requested <= after); assert_eq!( target_release.release_source, TargetReleaseSource::SystemVersion(version), diff --git a/nexus/types/src/external_api/shared.rs b/nexus/types/src/external_api/shared.rs index eee593ca39..18f5083670 100644 --- a/nexus/types/src/external_api/shared.rs +++ b/nexus/types/src/external_api/shared.rs @@ -516,17 +516,19 @@ impl RelayState { #[derive(Clone, Debug, Deserialize, Eq, PartialEq, JsonSchema, Serialize)] #[serde(rename_all = "snake_case")] pub enum TargetReleaseSource { - /// Obtain the release artifact from the `install` dataset. - InstallDataset, + /// Unspecified or unknown source (probably MUPdate). + Unspecified, /// Use the specified release of the rack's system software. + /// A TUF repo containing that release must be available via + /// the repo depot. SystemVersion(Version), } /// View of a system software target release #[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize, JsonSchema)] pub struct TargetRelease { - /// The target-release generation. + /// The target-release generation number. pub generation: i64, /// The time it was or is to be set as the target release. diff --git a/schema/crdb/create-target-release/up1.sql b/schema/crdb/create-target-release/up1.sql index 796fdd5f64..12a1c3f2b6 100644 --- a/schema/crdb/create-target-release/up1.sql +++ b/schema/crdb/create-target-release/up1.sql @@ -1,5 +1,5 @@ -- The source of the software release that should be deployed to the rack. CREATE TYPE IF NOT EXISTS omicron.public.target_release_source AS ENUM ( - 'install_dataset', + 'unspecified', 'system_version' ); diff --git a/schema/crdb/create-target-release/up2.sql b/schema/crdb/create-target-release/up2.sql index 16972972e0..13142323f3 100644 --- a/schema/crdb/create-target-release/up2.sql +++ b/schema/crdb/create-target-release/up2.sql @@ -1,11 +1,12 @@ --- The software release that should be deployed to the rack. +-- Software releases that should be/have been deployed to the rack. The +-- current target release is the one with the largest generation number. CREATE TABLE IF NOT EXISTS omicron.public.target_release ( generation INT8 NOT NULL PRIMARY KEY, time_requested TIMESTAMPTZ NOT NULL, release_source omicron.public.target_release_source NOT NULL, - system_version STRING(64), -- "foreign key" into the `tuf_repo` table - CONSTRAINT system_version_for_release CHECK ( + tuf_repo_id UUID, -- "foreign key" into the `tuf_repo` table + CONSTRAINT tuf_repo_for_system_version CHECK ( (release_source != 'system_version') OR - (release_source = 'system_version' AND system_version IS NOT NULL) + (release_source = 'system_version' AND tuf_repo_id IS NOT NULL) ) ); diff --git a/schema/crdb/create-target-release/up3.sql b/schema/crdb/create-target-release/up3.sql index 35a1ef9eaf..98aefd8f88 100644 --- a/schema/crdb/create-target-release/up3.sql +++ b/schema/crdb/create-target-release/up3.sql @@ -3,10 +3,10 @@ INSERT INTO omicron.public.target_release ( generation, time_requested, release_source, - system_version + tuf_repo_id ) VALUES ( - 0, + 1, NOW(), - 'install_dataset', + 'unspecified', NULL ) ON CONFLICT DO NOTHING; diff --git a/schema/crdb/dbinit.sql b/schema/crdb/dbinit.sql index 5697ea06a8..d004797dfc 100644 --- a/schema/crdb/dbinit.sql +++ b/schema/crdb/dbinit.sql @@ -2423,19 +2423,20 @@ CREATE TABLE IF NOT EXISTS omicron.public.tuf_repo_artifact ( -- The source of the software release that should be deployed to the rack. CREATE TYPE IF NOT EXISTS omicron.public.target_release_source AS ENUM ( - 'install_dataset', + 'unspecified', 'system_version' ); --- The software release that should be deployed to the rack. +-- Software releases that should be/have been deployed to the rack. The +-- current target release is the one with the largest generation number. CREATE TABLE IF NOT EXISTS omicron.public.target_release ( generation INT8 NOT NULL PRIMARY KEY, time_requested TIMESTAMPTZ NOT NULL, release_source omicron.public.target_release_source NOT NULL, - system_version STRING(64), -- "foreign key" into the `tuf_repo` table - CONSTRAINT system_version_for_release CHECK ( + tuf_repo_id UUID, -- "foreign key" into the `tuf_repo` table + CONSTRAINT tuf_repo_for_system_version CHECK ( (release_source != 'system_version') OR - (release_source = 'system_version' AND system_version IS NOT NULL) + (release_source = 'system_version' AND tuf_repo_id IS NOT NULL) ) ); @@ -2444,11 +2445,11 @@ INSERT INTO omicron.public.target_release ( generation, time_requested, release_source, - system_version + tuf_repo_id ) VALUES ( - 0, + 1, NOW(), - 'install_dataset', + 'unspecified', NULL ) ON CONFLICT DO NOTHING; From 87b20001ed7791a334da8e7c6bda3ddfcc416df4 Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Mon, 24 Feb 2025 14:20:39 -0700 Subject: [PATCH 08/19] Make version mandatory for setting target_release --- .../src/db/datastore/target_release.rs | 12 +++--- nexus/external-api/src/lib.rs | 4 +- nexus/src/external_api/http_entrypoints.rs | 40 ++++++------------- .../tests/integration_tests/target_release.rs | 18 ++++----- nexus/types/src/external_api/params.rs | 4 +- nexus/types/src/external_api/shared.rs | 27 ------------- nexus/types/src/external_api/views.rs | 27 +++++++++++++ 7 files changed, 57 insertions(+), 75 deletions(-) diff --git a/nexus/db-queries/src/db/datastore/target_release.rs b/nexus/db-queries/src/db/datastore/target_release.rs index b542bf7db5..29d4a63f16 100644 --- a/nexus/db-queries/src/db/datastore/target_release.rs +++ b/nexus/db-queries/src/db/datastore/target_release.rs @@ -13,7 +13,7 @@ use crate::db::schema::target_release::dsl; use async_bb8_diesel::AsyncRunQueryDsl as _; use diesel::insert_into; use diesel::prelude::*; -use nexus_types::external_api::shared; +use nexus_types::external_api::views; use omicron_common::api::external::{CreateResult, LookupResult}; impl DataStore { @@ -70,23 +70,23 @@ impl DataStore { /// 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 export_target_release( + pub async fn target_release_view( &self, opctx: &OpContext, target_release: &TargetRelease, - ) -> LookupResult { + ) -> LookupResult { opctx.authorize(authz::Action::Read, &authz::FLEET).await?; let conn = self.pool_connection_authorized(opctx).await?; - Ok(shared::TargetRelease { + Ok(views::TargetRelease { generation: (&target_release.generation.0).into(), time_requested: target_release.time_requested, release_source: match target_release.release_source { TargetReleaseSource::Unspecified => { - shared::TargetReleaseSource::Unspecified + views::TargetReleaseSource::Unspecified } TargetReleaseSource::SystemVersion => { use crate::db::schema::tuf_repo; - shared::TargetReleaseSource::SystemVersion( + views::TargetReleaseSource::SystemVersion( tuf_repo::table .select(tuf_repo::system_version) .filter(tuf_repo::id.eq( diff --git a/nexus/external-api/src/lib.rs b/nexus/external-api/src/lib.rs index 844173e7f0..c6132ae2b9 100644 --- a/nexus/external-api/src/lib.rs +++ b/nexus/external-api/src/lib.rs @@ -2902,7 +2902,7 @@ pub trait NexusExternalApi { }] async fn system_update_get_target_release( rqctx: RequestContext, - ) -> Result, HttpError>; + ) -> Result, HttpError>; /// Set the current target release of the rack's system software /// @@ -2918,7 +2918,7 @@ pub trait NexusExternalApi { async fn system_update_set_target_release( rqctx: RequestContext, params: TypedBody, - ) -> Result, HttpError>; + ) -> Result, HttpError>; // Silo users diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index 4d12600a24..d646be878e 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -6089,7 +6089,7 @@ impl NexusExternalApi for NexusExternalApiImpl { async fn system_update_get_target_release( rqctx: RequestContext, - ) -> Result, HttpError> { + ) -> Result, HttpError> { let apictx = rqctx.context(); let nexus = &apictx.context.nexus; let handler = async { @@ -6100,7 +6100,7 @@ impl NexusExternalApi for NexusExternalApiImpl { Ok(HttpResponseOk( nexus .datastore() - .export_target_release(&opctx, &target_release) + .target_release_view(&opctx, &target_release) .await?, )) }; @@ -6114,41 +6114,27 @@ impl NexusExternalApi for NexusExternalApiImpl { async fn system_update_set_target_release( rqctx: RequestContext, body: TypedBody, - ) -> 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 = body.into_inner(); - let (release_source, version) = match params.release_source { - shared::TargetReleaseSource::Unspecified => { - (nexus_db_model::TargetReleaseSource::Unspecified, None) - } - shared::TargetReleaseSource::SystemVersion(version) => ( - nexus_db_model::TargetReleaseSource::SystemVersion, - Some(version), - ), - }; - let tuf_repo_id = if let Some(version) = version { - Some( - nexus - .datastore() - .update_tuf_repo_get(&opctx, version.into()) - .await? - .repo - .id, - ) - } else { - None - }; + let system_version = params.system_version; + let tuf_repo_id = nexus + .datastore() + .update_tuf_repo_get(&opctx, system_version.into()) + .await? + .repo + .id; let current_target_release = nexus.datastore().get_target_release(&opctx).await?; let next_target_release = nexus_db_model::TargetRelease::new_from_prev( current_target_release, - release_source, - tuf_repo_id, + nexus_db_model::TargetReleaseSource::SystemVersion, + Some(tuf_repo_id), ); let target_release = nexus .datastore() @@ -6157,7 +6143,7 @@ impl NexusExternalApi for NexusExternalApiImpl { Ok(HttpResponseCreated( nexus .datastore() - .export_target_release(&opctx, &target_release) + .target_release_view(&opctx, &target_release) .await?, )) }; diff --git a/nexus/tests/integration_tests/target_release.rs b/nexus/tests/integration_tests/target_release.rs index b6c198a168..e31cd5087a 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::load_test_config; use nexus_test_utils::test_setup_with_config; use nexus_types::external_api::params::SetTargetReleaseParams; -use nexus_types::external_api::shared::{TargetRelease, TargetReleaseSource}; +use nexus_types::external_api::views::{TargetRelease, TargetReleaseSource}; use omicron_common::api::external::TufRepoInsertResponse; use omicron_sled_agent::sim; use semver::Version; @@ -51,13 +51,11 @@ async fn get_set_target_release() -> anyhow::Result<()> { assert_eq!(target_release.release_source, TargetReleaseSource::Unspecified); // Attempting to set an invalid system version should fail. - let version = Version::new(0, 0, 0); + let system_version = Version::new(0, 0, 0); NexusRequest::objects_post( client, "/v1/system/update/target-release", - &SetTargetReleaseParams { - release_source: TargetReleaseSource::SystemVersion(version), - }, + &SetTargetReleaseParams { system_version }, ) .authn_as(AuthnMode::PrivilegedUser) .execute() @@ -67,7 +65,7 @@ async fn get_set_target_release() -> anyhow::Result<()> { // Adding a fake (tufaceous) repo and then setting it as the // target release should succeed. let before = Utc::now(); - let version = Version::new(1, 0, 0); + let system_version = Version::new(1, 0, 0); let logctx = LogContext::new("get_set_target_release", &config.pkg.log); let temp = Utf8TempDir::new().unwrap(); let path = temp.path().join("repo.zip"); @@ -83,7 +81,7 @@ async fn get_set_target_release() -> anyhow::Result<()> { .expect("can't assemble TUF repo"); assert_eq!( - version, + system_version, NexusRequest::new( RequestBuilder::new( client, @@ -107,9 +105,7 @@ async fn get_set_target_release() -> anyhow::Result<()> { let target_release: TargetRelease = NexusRequest::objects_post( client, "/v1/system/update/target-release", - &SetTargetReleaseParams { - release_source: TargetReleaseSource::SystemVersion(version.clone()), - }, + &SetTargetReleaseParams { system_version: system_version.clone() }, ) .authn_as(AuthnMode::PrivilegedUser) .execute() @@ -123,7 +119,7 @@ async fn get_set_target_release() -> anyhow::Result<()> { assert!(target_release.time_requested <= after); assert_eq!( target_release.release_source, - TargetReleaseSource::SystemVersion(version), + TargetReleaseSource::SystemVersion(system_version), ); ctx.teardown().await; diff --git a/nexus/types/src/external_api/params.rs b/nexus/types/src/external_api/params.rs index 9204a11f25..a1d24e6a42 100644 --- a/nexus/types/src/external_api/params.rs +++ b/nexus/types/src/external_api/params.rs @@ -2241,8 +2241,8 @@ pub struct UpdatesGetRepositoryParams { /// Parameters for POST requests to `/v1/system/update/target-release`. #[derive(Clone, Debug, Deserialize, JsonSchema, Serialize)] pub struct SetTargetReleaseParams { - /// Source of the requested target release. - pub release_source: shared::TargetReleaseSource, + /// Version of the system software to make the target release. + pub system_version: Version, } // Probes diff --git a/nexus/types/src/external_api/shared.rs b/nexus/types/src/external_api/shared.rs index 18f5083670..ae8214a637 100644 --- a/nexus/types/src/external_api/shared.rs +++ b/nexus/types/src/external_api/shared.rs @@ -15,7 +15,6 @@ use omicron_common::api::internal::shared::NetworkInterface; use omicron_uuid_kinds::SupportBundleUuid; use parse_display::FromStr; use schemars::JsonSchema; -use semver::Version; use serde::de::Error as _; use serde::Deserialize; use serde::Deserializer; @@ -511,29 +510,3 @@ impl RelayState { .context("json from relay state string") } } - -/// The source of the target release. -#[derive(Clone, Debug, Deserialize, Eq, PartialEq, JsonSchema, Serialize)] -#[serde(rename_all = "snake_case")] -pub enum TargetReleaseSource { - /// Unspecified or unknown source (probably MUPdate). - Unspecified, - - /// Use the specified release of the rack's system software. - /// A TUF repo containing that release must be available via - /// the repo depot. - SystemVersion(Version), -} - -/// 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 or is to be set as the target release. - pub time_requested: DateTime, - - /// The source of the target release. - pub release_source: TargetReleaseSource, -} diff --git a/nexus/types/src/external_api/views.rs b/nexus/types/src/external_api/views.rs index 02aaffd3cc..7308a280a1 100644 --- a/nexus/types/src/external_api/views.rs +++ b/nexus/types/src/external_api/views.rs @@ -19,6 +19,7 @@ use omicron_common::api::external::{ }; use oxnet::{Ipv4Net, Ipv6Net}; use schemars::JsonSchema; +use semver::Version; use serde::{Deserialize, Serialize}; use std::collections::BTreeMap; use std::collections::BTreeSet; @@ -1046,3 +1047,29 @@ pub struct OxqlQueryResult { /// Tables resulting from the query, each containing timeseries. pub tables: Vec, } + +// UPDATE + +/// Source of a system software target release. +#[derive(Clone, Debug, Deserialize, Eq, PartialEq, JsonSchema, Serialize)] +#[serde(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), +} + +/// 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, + + /// The source of the target release. + pub release_source: TargetReleaseSource, +} From 5fe4bb31bc9f2768563130a42e88f19a868babc6 Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Mon, 24 Feb 2025 16:38:24 -0700 Subject: [PATCH 09/19] Synthetic authz resource TargetReleaseConfig --- common/src/api/external/mod.rs | 1 - nexus/auth/src/authz/api_resources.rs | 51 ++++++++++++++++--- nexus/auth/src/authz/omicron.polar | 14 +++++ nexus/auth/src/authz/oso_generic.rs | 1 + .../src/db/datastore/target_release.rs | 32 +++++++----- .../src/policy_test/resource_builder.rs | 2 +- nexus/db-queries/src/policy_test/resources.rs | 1 + nexus/db-queries/tests/output/authz-roles.out | 14 +++++ nexus/external-api/src/lib.rs | 6 +-- nexus/src/external_api/http_entrypoints.rs | 14 ++--- 10 files changed, 103 insertions(+), 33 deletions(-) diff --git a/common/src/api/external/mod.rs b/common/src/api/external/mod.rs index a9f228a204..30fbd7a21e 100644 --- a/common/src/api/external/mod.rs +++ b/common/src/api/external/mod.rs @@ -1008,7 +1008,6 @@ pub enum ResourceType { Oximeter, MetricProducer, RoleBuiltin, - TargetRelease, TufRepo, TufArtifact, SwitchPort, diff --git a/nexus/auth/src/authz/api_resources.rs b/nexus/auth/src/authz/api_resources.rs index 5def6696a3..14b5d485a6 100644 --- a/nexus/auth/src/authz/api_resources.rs +++ b/nexus/auth/src/authz/api_resources.rs @@ -668,6 +668,49 @@ impl AuthorizedResource for SiloUserList { } } +/// System software target version configuration +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub struct TargetReleaseConfig; + +pub const TARGET_RELEASE_CONFIG: TargetReleaseConfig = TargetReleaseConfig; + +impl oso::PolarClass for TargetReleaseConfig { + fn get_polar_class_builder() -> oso::ClassBuilder { + oso::Class::builder() + .with_equality_check() + .add_attribute_getter("fleet", |_: &TargetReleaseConfig| FLEET) + } +} + +impl AuthorizedResource for TargetReleaseConfig { + fn load_roles<'fut>( + &'fut self, + opctx: &'fut OpContext, + authn: &'fut authn::Context, + roleset: &'fut mut RoleSet, + ) -> futures::future::BoxFuture<'fut, Result<(), Error>> { + // There are no roles on the TargetReleaseConfig, only permissions. But we + // still need to load the Fleet-related roles to verify that the actor + // has the "admin" role on the Fleet (possibly conferred from a Silo + // role). + load_roles_for_resource_tree(&FLEET, opctx, authn, roleset).boxed() + } + + fn on_unauthorized( + &self, + _: &Authz, + error: Error, + _: AnyActor, + _: Action, + ) -> Error { + error + } + + fn polar_class(&self) -> oso::Class { + Self::get_polar_class() + } +} + // Main resource hierarchy: Projects and their resources authz_resource! { @@ -902,14 +945,6 @@ authz_resource! { polar_snippet = FleetChild, } -authz_resource! { - name = "TargetRelease", - parent = "Rack", - primary_key = u64, // generation - roles_allowed = false, - polar_snippet = FleetChild, -} - authz_resource! { name = "Silo", parent = "Fleet", diff --git a/nexus/auth/src/authz/omicron.polar b/nexus/auth/src/authz/omicron.polar index f9382401fd..c1e463e9bf 100644 --- a/nexus/auth/src/authz/omicron.polar +++ b/nexus/auth/src/authz/omicron.polar @@ -383,6 +383,20 @@ resource BlueprintConfig { has_relation(fleet: Fleet, "parent_fleet", list: BlueprintConfig) if list.fleet = fleet; +# Describes the policy for accessing blueprints +resource TargetReleaseConfig { + permissions = [ + "read", # read the current target release + "modify", # change the current target release + ]; + + relations = { parent_fleet: Fleet }; + "read" if "viewer" on "parent_fleet"; + "modify" if "admin" on "parent_fleet"; +} +has_relation(fleet: Fleet, "parent_fleet", resource: TargetReleaseConfig) + if resource.fleet = fleet; + # Describes the policy for reading and modifying low-level inventory resource Inventory { permissions = [ "read", "modify" ]; diff --git a/nexus/auth/src/authz/oso_generic.rs b/nexus/auth/src/authz/oso_generic.rs index 32b3dbd1f8..9d08e242c2 100644 --- a/nexus/auth/src/authz/oso_generic.rs +++ b/nexus/auth/src/authz/oso_generic.rs @@ -114,6 +114,7 @@ pub fn make_omicron_oso(log: &slog::Logger) -> Result { SiloCertificateList::get_polar_class(), SiloIdentityProviderList::get_polar_class(), SiloUserList::get_polar_class(), + TargetReleaseConfig::get_polar_class(), ]; for c in classes { oso_builder = oso_builder.register_class(c)?; diff --git a/nexus/db-queries/src/db/datastore/target_release.rs b/nexus/db-queries/src/db/datastore/target_release.rs index 29d4a63f16..5432edb522 100644 --- a/nexus/db-queries/src/db/datastore/target_release.rs +++ b/nexus/db-queries/src/db/datastore/target_release.rs @@ -9,6 +9,7 @@ use crate::authz; use crate::context::OpContext; use crate::db::error::{public_error_from_diesel, ErrorHandler}; use crate::db::model::{SemverVersion, TargetRelease, TargetReleaseSource}; +use crate::db::schema::target_release; use crate::db::schema::target_release::dsl; use async_bb8_diesel::AsyncRunQueryDsl as _; use diesel::insert_into; @@ -19,19 +20,20 @@ use omicron_common::api::external::{CreateResult, LookupResult}; impl DataStore { /// Fetch the current target release, i.e., the row with the largest /// generation number. - pub async fn get_target_release( + pub async fn target_release_get_current( &self, opctx: &OpContext, ) -> LookupResult { - opctx.authorize(authz::Action::Read, &authz::FLEET).await?; + opctx + .authorize(authz::Action::Read, &authz::TARGET_RELEASE_CONFIG) + .await?; let conn = self.pool_connection_authorized(opctx).await?; // Fetch the row in the `target_release` table with the largest // generation number. The subquery accesses the same table, so we // have to make an alias to not confuse diesel. - let target_release_2 = diesel::alias!( - crate::db::schema::target_release as target_release_2 - ); + let target_release_2 = + diesel::alias!(target_release as target_release_2); dsl::target_release .filter(dsl::generation.nullable().eq_any(target_release_2.select( diesel::dsl::max(target_release_2.field(dsl::generation)), @@ -45,12 +47,14 @@ impl DataStore { /// Insert a new target release row and return it. It will only become /// the current target release if its generation is larger than any /// existing row. - pub async fn set_target_release( + pub async fn target_release_insert( &self, opctx: &OpContext, target_release: TargetRelease, ) -> CreateResult { - opctx.authorize(authz::Action::Modify, &authz::FLEET).await?; + opctx + .authorize(authz::Action::Modify, &authz::TARGET_RELEASE_CONFIG) + .await?; let conn = self.pool_connection_authorized(opctx).await?; self.transaction_retry_wrapper("set_target_release") .transaction(&conn, |conn| { @@ -75,7 +79,9 @@ impl DataStore { opctx: &OpContext, target_release: &TargetRelease, ) -> LookupResult { - opctx.authorize(authz::Action::Read, &authz::FLEET).await?; + opctx + .authorize(authz::Action::Read, &authz::TARGET_RELEASE_CONFIG) + .await?; let conn = self.pool_connection_authorized(opctx).await?; Ok(views::TargetRelease { generation: (&target_release.generation.0).into(), @@ -131,7 +137,7 @@ mod test { // There should always be a target release. // This is ensured by the schema migration. let target_release = datastore - .get_target_release(opctx) + .target_release_get_current(opctx) .await .expect("should be a target release"); assert_eq!(target_release.generation, Generation(1.into())); @@ -151,7 +157,7 @@ mod test { None, ); let target_release = datastore - .set_target_release(opctx, initial_target_release.clone()) + .target_release_insert(opctx, initial_target_release.clone()) .await .unwrap(); assert_eq!( @@ -168,7 +174,7 @@ mod test { // Trying to reuse a generation should fail. assert!(datastore - .set_target_release( + .target_release_insert( opctx, TargetRelease::new( target_release.generation, @@ -181,7 +187,7 @@ mod test { // But the next generation should be fine, even with the same source. let target_release = datastore - .set_target_release( + .target_release_insert( opctx, TargetRelease::new_from_prev( target_release, @@ -230,7 +236,7 @@ mod test { let before = Utc::now(); let target_release = datastore - .set_target_release( + .target_release_insert( opctx, TargetRelease::new_from_prev( target_release, diff --git a/nexus/db-queries/src/policy_test/resource_builder.rs b/nexus/db-queries/src/policy_test/resource_builder.rs index ce91af1a3e..28c3adafed 100644 --- a/nexus/db-queries/src/policy_test/resource_builder.rs +++ b/nexus/db-queries/src/policy_test/resource_builder.rs @@ -274,7 +274,6 @@ impl_dyn_authorized_resource_for_resource!(authz::Sled); impl_dyn_authorized_resource_for_resource!(authz::Snapshot); impl_dyn_authorized_resource_for_resource!(authz::SshKey); impl_dyn_authorized_resource_for_resource!(authz::SupportBundle); -impl_dyn_authorized_resource_for_resource!(authz::TargetRelease); impl_dyn_authorized_resource_for_resource!(authz::TufArtifact); impl_dyn_authorized_resource_for_resource!(authz::TufRepo); impl_dyn_authorized_resource_for_resource!(authz::Vpc); @@ -288,6 +287,7 @@ impl_dyn_authorized_resource_for_global!(authz::DeviceAuthRequestList); impl_dyn_authorized_resource_for_global!(authz::DnsConfig); impl_dyn_authorized_resource_for_global!(authz::IpPoolList); impl_dyn_authorized_resource_for_global!(authz::Inventory); +impl_dyn_authorized_resource_for_global!(authz::TargetReleaseConfig); impl DynAuthorizedResource for authz::SiloCertificateList { fn do_authorize<'a, 'b>( diff --git a/nexus/db-queries/src/policy_test/resources.rs b/nexus/db-queries/src/policy_test/resources.rs index 0465541053..b069d1df2a 100644 --- a/nexus/db-queries/src/policy_test/resources.rs +++ b/nexus/db-queries/src/policy_test/resources.rs @@ -73,6 +73,7 @@ pub async fn make_resources( builder.new_resource(authz::DEVICE_AUTH_REQUEST_LIST); builder.new_resource(authz::INVENTORY); builder.new_resource(authz::IP_POOL_LIST); + builder.new_resource(authz::TARGET_RELEASE_CONFIG); // Silo/organization/project hierarchy make_silo(&mut builder, "silo1", main_silo_id, true).await; diff --git a/nexus/db-queries/tests/output/authz-roles.out b/nexus/db-queries/tests/output/authz-roles.out index e0d43250d1..6ff2685369 100644 --- a/nexus/db-queries/tests/output/authz-roles.out +++ b/nexus/db-queries/tests/output/authz-roles.out @@ -110,6 +110,20 @@ resource: authz::IpPoolList silo1-proj1-viewer ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ unauthenticated ! ! ! ! ! ! ! ! +resource: authz::TargetReleaseConfig + + USER Q R LC RP M MP CC D + fleet-admin ✘ ✔ ✘ ✔ ✔ ✔ ✘ ✔ + fleet-collaborator ✘ ✔ ✘ ✔ ✘ ✘ ✘ ✘ + fleet-viewer ✘ ✔ ✘ ✔ ✘ ✘ ✘ ✘ + silo1-admin ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + silo1-collaborator ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + silo1-viewer ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + silo1-proj1-admin ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + silo1-proj1-collaborator ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + silo1-proj1-viewer ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + unauthenticated ! ! ! ! ! ! ! ! + resource: Silo "silo1" USER Q R LC RP M MP CC D diff --git a/nexus/external-api/src/lib.rs b/nexus/external-api/src/lib.rs index c6132ae2b9..d216306bfd 100644 --- a/nexus/external-api/src/lib.rs +++ b/nexus/external-api/src/lib.rs @@ -2889,7 +2889,7 @@ pub trait NexusExternalApi { /// Get the current target release of the rack's system software /// - /// This may not correpond to the actual software running on the rack + /// 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 @@ -2900,7 +2900,7 @@ pub trait NexusExternalApi { tags = ["system/update"], unpublished = true, }] - async fn system_update_get_target_release( + async fn target_release_get( rqctx: RequestContext, ) -> Result, HttpError>; @@ -2915,7 +2915,7 @@ pub trait NexusExternalApi { tags = ["system/update"], unpublished = true, }] - async fn system_update_set_target_release( + async fn target_release_set( rqctx: RequestContext, params: TypedBody, ) -> Result, HttpError>; diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index d646be878e..f9267ee75f 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -6087,16 +6087,16 @@ impl NexusExternalApi for NexusExternalApiImpl { .await } - async fn system_update_get_target_release( + async fn target_release_get( rqctx: RequestContext, ) -> Result, HttpError> { let apictx = rqctx.context(); - let nexus = &apictx.context.nexus; let handler = async { + let nexus = &apictx.context.nexus; let opctx = crate::context::op_context_for_external_api(&rqctx).await?; let target_release = - nexus.datastore().get_target_release(&opctx).await?; + nexus.datastore().target_release_get_current(&opctx).await?; Ok(HttpResponseOk( nexus .datastore() @@ -6111,13 +6111,13 @@ impl NexusExternalApi for NexusExternalApiImpl { .await } - async fn system_update_set_target_release( + async fn target_release_set( rqctx: RequestContext, body: TypedBody, ) -> Result, HttpError> { let apictx = rqctx.context(); - let nexus = &apictx.context.nexus; let handler = async { + let nexus = &apictx.context.nexus; let opctx = crate::context::op_context_for_external_api(&rqctx).await?; let params = body.into_inner(); @@ -6129,7 +6129,7 @@ impl NexusExternalApi for NexusExternalApiImpl { .repo .id; let current_target_release = - nexus.datastore().get_target_release(&opctx).await?; + nexus.datastore().target_release_get_current(&opctx).await?; let next_target_release = nexus_db_model::TargetRelease::new_from_prev( current_target_release, @@ -6138,7 +6138,7 @@ impl NexusExternalApi for NexusExternalApiImpl { ); let target_release = nexus .datastore() - .set_target_release(&opctx, next_target_release) + .target_release_insert(&opctx, next_target_release) .await?; Ok(HttpResponseCreated( nexus From f141d8e7f239b25b66cbd89724022e3005c90c58 Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Wed, 26 Feb 2025 09:10:46 -0700 Subject: [PATCH 10/19] Don't panic! --- .../src/db/datastore/target_release.rs | 40 ++++++++++--------- 1 file changed, 21 insertions(+), 19 deletions(-) diff --git a/nexus/db-queries/src/db/datastore/target_release.rs b/nexus/db-queries/src/db/datastore/target_release.rs index 5432edb522..625228c75d 100644 --- a/nexus/db-queries/src/db/datastore/target_release.rs +++ b/nexus/db-queries/src/db/datastore/target_release.rs @@ -15,7 +15,7 @@ use async_bb8_diesel::AsyncRunQueryDsl as _; use diesel::insert_into; use diesel::prelude::*; use nexus_types::external_api::views; -use omicron_common::api::external::{CreateResult, LookupResult}; +use omicron_common::api::external::{CreateResult, Error, LookupResult}; impl DataStore { /// Fetch the current target release, i.e., the row with the largest @@ -92,24 +92,26 @@ impl DataStore { } TargetReleaseSource::SystemVersion => { use crate::db::schema::tuf_repo; - views::TargetReleaseSource::SystemVersion( - tuf_repo::table - .select(tuf_repo::system_version) - .filter(tuf_repo::id.eq( - target_release.tuf_repo_id.expect( - "CONSTRAINT tuf_repo_for_system_version", - ), - )) - .first_async::(&*conn) - .await - .map_err(|e| { - public_error_from_diesel( - e, - ErrorHandler::Server, - ) - })? - .into(), - ) + if let Some(tuf_repo_id) = target_release.tuf_repo_id { + views::TargetReleaseSource::SystemVersion( + 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::invalid_request( + "missing TUF repo ID for specified system version", + )); + } } }, }) From 5898757a73546bf5c3c19f0e003b51191620c6bd Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Tue, 4 Mar 2025 13:45:47 -0700 Subject: [PATCH 11/19] Safer TargetRelease constructors --- nexus/db-model/src/target_release.rs | 30 +++++++--------- .../src/db/datastore/target_release.rs | 36 ++++--------------- nexus/src/external_api/http_entrypoints.rs | 7 ++-- 3 files changed, 22 insertions(+), 51 deletions(-) diff --git a/nexus/db-model/src/target_release.rs b/nexus/db-model/src/target_release.rs index 2173bd7c7a..bb2512ed61 100644 --- a/nexus/db-model/src/target_release.rs +++ b/nexus/db-model/src/target_release.rs @@ -42,28 +42,24 @@ pub struct TargetRelease { } impl TargetRelease { - pub fn new( - generation: Generation, - release_source: TargetReleaseSource, - tuf_repo_id: Option>, - ) -> Self { + pub fn new_unspecified(prev: &TargetRelease) -> Self { Self { - generation, + generation: Generation(prev.generation.next()), time_requested: Utc::now(), - release_source, - tuf_repo_id, + release_source: TargetReleaseSource::Unspecified, + tuf_repo_id: None, } } - pub fn new_from_prev( - prev: TargetRelease, - release_source: TargetReleaseSource, - tuf_repo_id: Option>, + pub fn new_system_version( + prev: &TargetRelease, + tuf_repo_id: DbTypedUuid, ) -> Self { - Self::new( - Generation(prev.generation.next()), - release_source, - tuf_repo_id, - ) + Self { + generation: Generation(prev.generation.next()), + time_requested: Utc::now(), + release_source: TargetReleaseSource::SystemVersion, + tuf_repo_id: Some(tuf_repo_id), + } } } diff --git a/nexus/db-queries/src/db/datastore/target_release.rs b/nexus/db-queries/src/db/datastore/target_release.rs index 625228c75d..51dfb5ac40 100644 --- a/nexus/db-queries/src/db/datastore/target_release.rs +++ b/nexus/db-queries/src/db/datastore/target_release.rs @@ -153,11 +153,8 @@ mod test { // We should be able to set a new generation just like the first, // with some (very small) fuzz allowed in the timestamp reported // by the database. - let initial_target_release = TargetRelease::new_from_prev( - target_release, - TargetReleaseSource::Unspecified, - None, - ); + let initial_target_release = + TargetRelease::new_unspecified(&target_release); let target_release = datastore .target_release_insert(opctx, initial_target_release.clone()) .await @@ -174,33 +171,16 @@ mod test { ); assert!(target_release.tuf_repo_id.is_none()); - // Trying to reuse a generation should fail. - assert!(datastore - .target_release_insert( - opctx, - TargetRelease::new( - target_release.generation, - TargetReleaseSource::Unspecified, - None, - ) - ) - .await - .is_err()); - - // But the next generation should be fine, even with the same source. + // Inserting a new "unspecified" target should be fine. let target_release = datastore .target_release_insert( opctx, - TargetRelease::new_from_prev( - target_release, - TargetReleaseSource::Unspecified, - None, - ), + TargetRelease::new_unspecified(&target_release), ) .await .unwrap(); - // Finally, use a new generation number and a TUF repo source. + // Now add a new TUF repo and use it as the source. let version = SemverVersion::new(0, 0, 1); let hash = ArtifactHash( "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855" @@ -240,11 +220,7 @@ mod test { let target_release = datastore .target_release_insert( opctx, - TargetRelease::new_from_prev( - target_release, - TargetReleaseSource::SystemVersion, - Some(tuf_repo_id), - ), + TargetRelease::new_system_version(&target_release, tuf_repo_id), ) .await .unwrap(); diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index eabc6caee0..83dfc3e3dc 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -6451,10 +6451,9 @@ impl NexusExternalApi for NexusExternalApiImpl { let current_target_release = nexus.datastore().target_release_get_current(&opctx).await?; let next_target_release = - nexus_db_model::TargetRelease::new_from_prev( - current_target_release, - nexus_db_model::TargetReleaseSource::SystemVersion, - Some(tuf_repo_id), + nexus_db_model::TargetRelease::new_system_version( + ¤t_target_release, + tuf_repo_id, ); let target_release = nexus .datastore() From e7181dbbe5ef921386d2cdce2fc47fc0e3af8588 Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Tue, 4 Mar 2025 16:05:22 -0700 Subject: [PATCH 12/19] Rework target release datastore methods --- .../src/db/datastore/target_release.rs | 40 ++++++++----------- 1 file changed, 17 insertions(+), 23 deletions(-) diff --git a/nexus/db-queries/src/db/datastore/target_release.rs b/nexus/db-queries/src/db/datastore/target_release.rs index 51dfb5ac40..8f896834fc 100644 --- a/nexus/db-queries/src/db/datastore/target_release.rs +++ b/nexus/db-queries/src/db/datastore/target_release.rs @@ -9,7 +9,6 @@ use crate::authz; use crate::context::OpContext; use crate::db::error::{public_error_from_diesel, ErrorHandler}; use crate::db::model::{SemverVersion, TargetRelease, TargetReleaseSource}; -use crate::db::schema::target_release; use crate::db::schema::target_release::dsl; use async_bb8_diesel::AsyncRunQueryDsl as _; use diesel::insert_into; @@ -28,20 +27,22 @@ impl DataStore { .authorize(authz::Action::Read, &authz::TARGET_RELEASE_CONFIG) .await?; let conn = self.pool_connection_authorized(opctx).await?; - - // Fetch the row in the `target_release` table with the largest - // generation number. The subquery accesses the same table, so we - // have to make an alias to not confuse diesel. - let target_release_2 = - diesel::alias!(target_release as target_release_2); - dsl::target_release - .filter(dsl::generation.nullable().eq_any(target_release_2.select( - diesel::dsl::max(target_release_2.field(dsl::generation)), - ))) + let current = dsl::target_release .select(TargetRelease::as_select()) + .order_by(dsl::generation.desc()) .first_async(&*conn) .await - .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server)) + .optional() + .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server))?; + + // We expect there to always be a current target release, + // since the database migration `create-target-release/up3.sql` + // adds an initial row. + let current = current.ok_or_else(|| Error::InternalError { + internal_message: "no target release".to_string(), + })?; + + Ok(current) } /// Insert a new target release row and return it. It will only become @@ -56,17 +57,10 @@ impl DataStore { .authorize(authz::Action::Modify, &authz::TARGET_RELEASE_CONFIG) .await?; let conn = self.pool_connection_authorized(opctx).await?; - self.transaction_retry_wrapper("set_target_release") - .transaction(&conn, |conn| { - let target_release = target_release.clone(); - async move { - insert_into(dsl::target_release) - .values(target_release) - .returning(TargetRelease::as_returning()) - .get_result_async(&conn) - .await - } - }) + insert_into(dsl::target_release) + .values(target_release) + .returning(TargetRelease::as_returning()) + .get_result_async(&*conn) .await .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server)) } From 27b7e8da8bb745fb6e464f4bf32b0e98dfffdd55 Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Tue, 4 Mar 2025 16:37:08 -0700 Subject: [PATCH 13/19] Refactor target_release_view --- nexus/db-model/src/target_release.rs | 12 ++++ .../src/db/datastore/target_release.rs | 59 +++++++++---------- 2 files changed, 40 insertions(+), 31 deletions(-) diff --git a/nexus/db-model/src/target_release.rs b/nexus/db-model/src/target_release.rs index bb2512ed61..3ba1881bc5 100644 --- a/nexus/db-model/src/target_release.rs +++ b/nexus/db-model/src/target_release.rs @@ -6,6 +6,7 @@ use super::{impl_enum_type, Generation}; use crate::schema::target_release; use crate::typed_uuid::DbTypedUuid; use chrono::{DateTime, Utc}; +use nexus_types::external_api::views; use omicron_uuid_kinds::TufRepoKind; impl_enum_type!( @@ -62,4 +63,15 @@ impl TargetRelease { tuf_repo_id: Some(tuf_repo_id), } } + + pub fn into_external( + &self, + release_source: views::TargetReleaseSource, + ) -> views::TargetRelease { + views::TargetRelease { + generation: (&self.generation.0).into(), + 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 8f896834fc..02e0271ddb 100644 --- a/nexus/db-queries/src/db/datastore/target_release.rs +++ b/nexus/db-queries/src/db/datastore/target_release.rs @@ -77,38 +77,35 @@ impl DataStore { .authorize(authz::Action::Read, &authz::TARGET_RELEASE_CONFIG) .await?; let conn = self.pool_connection_authorized(opctx).await?; - Ok(views::TargetRelease { - generation: (&target_release.generation.0).into(), - time_requested: target_release.time_requested, - release_source: match target_release.release_source { - TargetReleaseSource::Unspecified => { - views::TargetReleaseSource::Unspecified + let release_source = match target_release.release_source { + TargetReleaseSource::Unspecified => { + views::TargetReleaseSource::Unspecified + } + TargetReleaseSource::SystemVersion => { + use crate::db::schema::tuf_repo; + if let Some(tuf_repo_id) = target_release.tuf_repo_id { + views::TargetReleaseSource::SystemVersion( + 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::invalid_request( + "missing TUF repo ID for specified system version", + )); } - TargetReleaseSource::SystemVersion => { - use crate::db::schema::tuf_repo; - if let Some(tuf_repo_id) = target_release.tuf_repo_id { - views::TargetReleaseSource::SystemVersion( - 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::invalid_request( - "missing TUF repo ID for specified system version", - )); - } - } - }, - }) + } + }; + Ok(target_release.into_external(release_source)) } } From 8e3301482361a2be05b04978190fd0843afac5a1 Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Tue, 4 Mar 2025 16:39:58 -0700 Subject: [PATCH 14/19] Internal error for missing TUF repo --- nexus/db-queries/src/db/datastore/target_release.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/nexus/db-queries/src/db/datastore/target_release.rs b/nexus/db-queries/src/db/datastore/target_release.rs index 02e0271ddb..f6913d086f 100644 --- a/nexus/db-queries/src/db/datastore/target_release.rs +++ b/nexus/db-queries/src/db/datastore/target_release.rs @@ -38,9 +38,8 @@ impl DataStore { // We expect there to always be a current target release, // since the database migration `create-target-release/up3.sql` // adds an initial row. - let current = current.ok_or_else(|| Error::InternalError { - internal_message: "no target release".to_string(), - })?; + let current = current + .ok_or_else(|| Error::internal_error("no target release"))?; Ok(current) } @@ -99,7 +98,7 @@ impl DataStore { .into(), ) } else { - return Err(Error::invalid_request( + return Err(Error::internal_error( "missing TUF repo ID for specified system version", )); } From a709786aca7cf62c83b5d1c82e9595ea100ac08a Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Tue, 4 Mar 2025 16:43:17 -0700 Subject: [PATCH 15/19] PUT target-release --- nexus/external-api/src/lib.rs | 2 +- .../tests/integration_tests/target_release.rs | 20 +++++++++++++------ 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/nexus/external-api/src/lib.rs b/nexus/external-api/src/lib.rs index d216306bfd..1811ee96a6 100644 --- a/nexus/external-api/src/lib.rs +++ b/nexus/external-api/src/lib.rs @@ -2910,7 +2910,7 @@ pub trait NexusExternalApi { /// a goal state for the rack's software, and attempt to asynchronously /// update to that release. #[endpoint { - method = POST, + method = PUT, path = "/v1/system/update/target-release", tags = ["system/update"], unpublished = true, diff --git a/nexus/tests/integration_tests/target_release.rs b/nexus/tests/integration_tests/target_release.rs index e31cd5087a..b79c73ec41 100644 --- a/nexus/tests/integration_tests/target_release.rs +++ b/nexus/tests/integration_tests/target_release.rs @@ -8,6 +8,8 @@ use camino_tempfile::Utf8TempDir; use chrono::Utc; use clap::Parser as _; use dropshot::test_util::LogContext; +use http::method::Method; +use http::StatusCode; use nexus_config::UpdatesConfig; use nexus_test_utils::http_testing::AuthnMode; use nexus_test_utils::http_testing::{NexusRequest, RequestBuilder}; @@ -52,10 +54,10 @@ async fn get_set_target_release() -> anyhow::Result<()> { // Attempting to set an invalid system version should fail. let system_version = Version::new(0, 0, 0); - NexusRequest::objects_post( + NexusRequest::object_put( client, "/v1/system/update/target-release", - &SetTargetReleaseParams { system_version }, + Some(&SetTargetReleaseParams { system_version }), ) .authn_as(AuthnMode::PrivilegedUser) .execute() @@ -102,10 +104,16 @@ async fn get_set_target_release() -> anyhow::Result<()> { .system_version ); - let target_release: TargetRelease = NexusRequest::objects_post( - client, - "/v1/system/update/target-release", - &SetTargetReleaseParams { system_version: system_version.clone() }, + let target_release: TargetRelease = NexusRequest::new( + RequestBuilder::new( + client, + Method::PUT, + "/v1/system/update/target-release", + ) + .body(Some(&SetTargetReleaseParams { + system_version: system_version.clone(), + })) + .expect_status(Some(StatusCode::CREATED)), ) .authn_as(AuthnMode::PrivilegedUser) .execute() From 1a83b85221575db38e3ea9f947bb3cb8cc57f427 Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Tue, 4 Mar 2025 18:23:00 -0700 Subject: [PATCH 16/19] Further constrain tuf_repo_for_system_version --- schema/crdb/create-target-release/up2.sql | 2 +- schema/crdb/dbinit.sql | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/schema/crdb/create-target-release/up2.sql b/schema/crdb/create-target-release/up2.sql index 13142323f3..5e22aec5ef 100644 --- a/schema/crdb/create-target-release/up2.sql +++ b/schema/crdb/create-target-release/up2.sql @@ -6,7 +6,7 @@ CREATE TABLE IF NOT EXISTS omicron.public.target_release ( release_source omicron.public.target_release_source NOT NULL, tuf_repo_id UUID, -- "foreign key" into the `tuf_repo` table CONSTRAINT tuf_repo_for_system_version CHECK ( - (release_source != 'system_version') OR + (release_source != 'system_version' AND tuf_repo_id IS NULL) OR (release_source = 'system_version' AND tuf_repo_id IS NOT NULL) ) ); diff --git a/schema/crdb/dbinit.sql b/schema/crdb/dbinit.sql index 5d11184143..217f21b4f0 100644 --- a/schema/crdb/dbinit.sql +++ b/schema/crdb/dbinit.sql @@ -2424,7 +2424,7 @@ CREATE TABLE IF NOT EXISTS omicron.public.target_release ( release_source omicron.public.target_release_source NOT NULL, tuf_repo_id UUID, -- "foreign key" into the `tuf_repo` table CONSTRAINT tuf_repo_for_system_version CHECK ( - (release_source != 'system_version') OR + (release_source != 'system_version' AND tuf_repo_id IS NULL) OR (release_source = 'system_version' AND tuf_repo_id IS NOT NULL) ) ); From 25a88b094596ac4f22a7feb736ef7414285e5f3f Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Tue, 4 Mar 2025 19:27:20 -0700 Subject: [PATCH 17/19] Publish hidden system/update endpoints --- common/src/api/external/mod.rs | 10 +- .../src/db/datastore/target_release.rs | 6 +- nexus/external-api/output/nexus_tags.txt | 2 + nexus/external-api/src/lib.rs | 6 +- .../tests/integration_tests/target_release.rs | 2 +- .../output/uncovered-authz-endpoints.txt | 2 + nexus/types/src/external_api/params.rs | 2 +- nexus/types/src/external_api/views.rs | 4 +- openapi/nexus.json | 147 ++++++++++++++++++ 9 files changed, 165 insertions(+), 16 deletions(-) diff --git a/common/src/api/external/mod.rs b/common/src/api/external/mod.rs index 30fbd7a21e..e51407046d 100644 --- a/common/src/api/external/mod.rs +++ b/common/src/api/external/mod.rs @@ -3089,10 +3089,10 @@ pub enum BfdMode { /// A description of an uploaded TUF repository. #[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize, JsonSchema)] pub struct TufRepoDescription { - // Information about the repository. + /// Information about the repository. pub repo: TufRepoMeta, - // Information about the artifacts present in the repository. + /// Information about the artifacts present in the repository. pub artifacts: Vec, } @@ -3105,7 +3105,7 @@ impl TufRepoDescription { /// Metadata about a TUF repository. /// -/// Found within a [`TufRepoDescription`]. +/// Found within a `TufRepoDescription`. #[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize, JsonSchema)] pub struct TufRepoMeta { /// The hash of the repository. @@ -3134,7 +3134,7 @@ pub struct TufRepoMeta { /// Metadata about an individual TUF artifact. /// -/// Found within a [`TufRepoDescription`]. +/// Found within a `TufRepoDescription`. #[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize, JsonSchema)] pub struct TufArtifactMeta { /// The artifact ID. @@ -3160,7 +3160,7 @@ pub struct TufRepoInsertResponse { /// Status of a TUF repo import. /// -/// Part of [`TufRepoInsertResponse`]. +/// Part of `TufRepoInsertResponse`. #[derive( Debug, Clone, Copy, PartialEq, Eq, Deserialize, Serialize, JsonSchema, )] diff --git a/nexus/db-queries/src/db/datastore/target_release.rs b/nexus/db-queries/src/db/datastore/target_release.rs index f6913d086f..e78bfe59ca 100644 --- a/nexus/db-queries/src/db/datastore/target_release.rs +++ b/nexus/db-queries/src/db/datastore/target_release.rs @@ -83,8 +83,8 @@ impl DataStore { TargetReleaseSource::SystemVersion => { use crate::db::schema::tuf_repo; if let Some(tuf_repo_id) = target_release.tuf_repo_id { - views::TargetReleaseSource::SystemVersion( - tuf_repo::table + views::TargetReleaseSource::SystemVersion { + version: tuf_repo::table .select(tuf_repo::system_version) .filter(tuf_repo::id.eq(tuf_repo_id)) .first_async::(&*conn) @@ -96,7 +96,7 @@ impl DataStore { ) })? .into(), - ) + } } else { return Err(Error::internal_error( "missing TUF repo ID for specified system version", diff --git a/nexus/external-api/output/nexus_tags.txt b/nexus/external-api/output/nexus_tags.txt index 59ba14e5fb..834675fc48 100644 --- a/nexus/external-api/output/nexus_tags.txt +++ b/nexus/external-api/output/nexus_tags.txt @@ -60,6 +60,8 @@ support_bundle_head_file HEAD /experimental/v1/system/suppor support_bundle_index GET /experimental/v1/system/support-bundles/{support_bundle}/index support_bundle_list GET /experimental/v1/system/support-bundles support_bundle_view GET /experimental/v1/system/support-bundles/{support_bundle} +target_release_get GET /v1/system/update/target-release +target_release_set PUT /v1/system/update/target-release timeseries_query POST /v1/timeseries/query API operations found with tag "images" diff --git a/nexus/external-api/src/lib.rs b/nexus/external-api/src/lib.rs index 1811ee96a6..42573309a2 100644 --- a/nexus/external-api/src/lib.rs +++ b/nexus/external-api/src/lib.rs @@ -2897,8 +2897,7 @@ pub trait NexusExternalApi { #[endpoint { method = GET, path = "/v1/system/update/target-release", - tags = ["system/update"], - unpublished = true, + tags = ["hidden"], // "system/update" }] async fn target_release_get( rqctx: RequestContext, @@ -2912,8 +2911,7 @@ pub trait NexusExternalApi { #[endpoint { method = PUT, path = "/v1/system/update/target-release", - tags = ["system/update"], - unpublished = true, + tags = ["hidden"], // "system/update" }] async fn target_release_set( rqctx: RequestContext, diff --git a/nexus/tests/integration_tests/target_release.rs b/nexus/tests/integration_tests/target_release.rs index b79c73ec41..1184e4c723 100644 --- a/nexus/tests/integration_tests/target_release.rs +++ b/nexus/tests/integration_tests/target_release.rs @@ -127,7 +127,7 @@ async fn get_set_target_release() -> anyhow::Result<()> { assert!(target_release.time_requested <= after); assert_eq!( target_release.release_source, - TargetReleaseSource::SystemVersion(system_version), + TargetReleaseSource::SystemVersion { version: system_version }, ); ctx.teardown().await; diff --git a/nexus/tests/output/uncovered-authz-endpoints.txt b/nexus/tests/output/uncovered-authz-endpoints.txt index 8a639f1224..a637112ad2 100644 --- a/nexus/tests/output/uncovered-authz-endpoints.txt +++ b/nexus/tests/output/uncovered-authz-endpoints.txt @@ -9,6 +9,7 @@ ping (get "/v1/ping") networking_switch_port_lldp_neighbors (get "/v1/system/hardware/rack-switch-port/{rack_id}/{switch_location}/{port}/lldp/neighbors") networking_switch_port_lldp_config_view (get "/v1/system/hardware/switch-port/{port}/lldp/config") networking_switch_port_status (get "/v1/system/hardware/switch-port/{port}/status") +target_release_get (get "/v1/system/update/target-release") support_bundle_head (head "/experimental/v1/system/support-bundles/{support_bundle}/download") support_bundle_head_file (head "/experimental/v1/system/support-bundles/{support_bundle}/download/{file}") device_auth_request (post "/device/auth") @@ -19,3 +20,4 @@ login_saml (post "/login/{silo_name}/saml/{provi login_local (post "/v1/login/{silo_name}/local") logout (post "/v1/logout") networking_switch_port_lldp_config_update (post "/v1/system/hardware/switch-port/{port}/lldp/config") +target_release_set (put "/v1/system/update/target-release") diff --git a/nexus/types/src/external_api/params.rs b/nexus/types/src/external_api/params.rs index a1d24e6a42..bd4fbdd844 100644 --- a/nexus/types/src/external_api/params.rs +++ b/nexus/types/src/external_api/params.rs @@ -2238,7 +2238,7 @@ pub struct UpdatesGetRepositoryParams { pub system_version: Version, } -/// Parameters for POST requests to `/v1/system/update/target-release`. +/// Parameters for PUT requests to `/v1/system/update/target-release`. #[derive(Clone, Debug, Deserialize, JsonSchema, Serialize)] pub struct SetTargetReleaseParams { /// Version of the system software to make the target release. diff --git a/nexus/types/src/external_api/views.rs b/nexus/types/src/external_api/views.rs index 7308a280a1..6d8e232f12 100644 --- a/nexus/types/src/external_api/views.rs +++ b/nexus/types/src/external_api/views.rs @@ -1052,13 +1052,13 @@ pub struct OxqlQueryResult { /// Source of a system software target release. #[derive(Clone, Debug, Deserialize, Eq, PartialEq, JsonSchema, Serialize)] -#[serde(rename_all = "snake_case")] +#[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), + SystemVersion { version: Version }, } /// View of a system software target release. diff --git a/openapi/nexus.json b/openapi/nexus.json index 17088ed5c9..409ab5f8c3 100644 --- a/openapi/nexus.json +++ b/openapi/nexus.json @@ -10057,6 +10057,70 @@ } } }, + "/v1/system/update/target-release": { + "get": { + "tags": [ + "hidden" + ], + "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_get", + "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": [ + "hidden" + ], + "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.", + "operationId": "target_release_set", + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/SetTargetReleaseParams" + } + } + }, + "required": true + }, + "responses": { + "201": { + "description": "successful creation", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/TargetRelease" + } + } + } + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + } + } + }, "/v1/system/users": { "get": { "tags": [ @@ -20989,6 +21053,20 @@ } ] }, + "SetTargetReleaseParams": { + "description": "Parameters for PUT requests to `/v1/system/update/target-release`.", + "type": "object", + "properties": { + "system_version": { + "description": "Version of the system software to make the target release.", + "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": [ + "system_version" + ] + }, "Silo": { "description": "View of a Silo\n\nA Silo is the highest level unit of isolation.", "type": "object", @@ -22895,6 +22973,75 @@ "timeseries" ] }, + "TargetRelease": { + "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": [ + { + "$ref": "#/components/schemas/TargetReleaseSource" + } + ] + }, + "time_requested": { + "description": "The time it was set as the target release.", + "type": "string", + "format": "date-time" + } + }, + "required": [ + "generation", + "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" + ] + } + ] + }, "Timeseries": { "description": "A timeseries contains a timestamped set of values from one source.\n\nThis includes the typed key-value pairs that uniquely identify it, and the set of timestamps and data values from it.", "type": "object", From 09fd2f3b29196a6186a18a47aa12d401320ba76f Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Thu, 6 Mar 2025 12:11:31 -0700 Subject: [PATCH 18/19] Improve target_release datastore test --- .../src/db/datastore/target_release.rs | 44 +++++++++++-------- 1 file changed, 26 insertions(+), 18 deletions(-) diff --git a/nexus/db-queries/src/db/datastore/target_release.rs b/nexus/db-queries/src/db/datastore/target_release.rs index 11d075798b..3f156df3f3 100644 --- a/nexus/db-queries/src/db/datastore/target_release.rs +++ b/nexus/db-queries/src/db/datastore/target_release.rs @@ -129,40 +129,47 @@ mod test { // There should always be a target release. // This is ensured by the schema migration. - let target_release = datastore + let initial_target_release = datastore .target_release_get_current(opctx) .await .expect("should be a target release"); - assert_eq!(target_release.generation, Generation(1.into())); - assert!(target_release.time_requested < Utc::now()); + assert_eq!(initial_target_release.generation, Generation(1.into())); + assert!(initial_target_release.time_requested < Utc::now()); assert_eq!( - target_release.release_source, + initial_target_release.release_source, TargetReleaseSource::Unspecified ); - assert!(target_release.tuf_repo_id.is_none()); + assert!(initial_target_release.tuf_repo_id.is_none()); - // We should be able to set a new generation just like the first, - // with some (very small) fuzz allowed in the timestamp reported - // by the database. - let initial_target_release = - TargetRelease::new_unspecified(&target_release); + // We should be able to set a new generation just like the first. + // We allow some slack in the timestamp comparison because the + // database only stores timestamps with μsec precision. + let target_release = + TargetRelease::new_unspecified(&initial_target_release); let target_release = datastore - .target_release_insert(opctx, initial_target_release.clone()) + .target_release_insert(opctx, target_release) .await .unwrap(); - assert_eq!( - target_release.generation, - initial_target_release.generation - ); + assert_eq!(target_release.generation, Generation(2.into())); assert!( - (target_release.time_requested - - initial_target_release.time_requested) + (target_release.time_requested - target_release.time_requested) .abs() < TimeDelta::new(0, 1_000).expect("1 μsec") ); assert!(target_release.tuf_repo_id.is_none()); - // Inserting a new "unspecified" target should be fine. + // Trying to reuse a generation should fail. + assert!( + datastore + .target_release_insert( + opctx, + TargetRelease::new_unspecified(&initial_target_release), + ) + .await + .is_err() + ); + + // But inserting a new unspecified target should be fine. let target_release = datastore .target_release_insert( opctx, @@ -170,6 +177,7 @@ mod test { ) .await .unwrap(); + assert_eq!(target_release.generation, Generation(3.into())); // Now add a new TUF repo and use it as the source. let version = SemverVersion::new(0, 0, 1); From ee5b3896ca019b9cba719665a40d82bde4f1bb83 Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Thu, 6 Mar 2025 12:44:32 -0700 Subject: [PATCH 19/19] Authz coverage for target_release endpoints --- nexus/tests/integration_tests/endpoints.rs | 18 ++++++++++++++++++ .../tests/output/uncovered-authz-endpoints.txt | 2 -- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/nexus/tests/integration_tests/endpoints.rs b/nexus/tests/integration_tests/endpoints.rs index ee685589cc..a606a0c94c 100644 --- a/nexus/tests/integration_tests/endpoints.rs +++ b/nexus/tests/integration_tests/endpoints.rs @@ -38,6 +38,7 @@ use omicron_common::api::external::RouteTarget; use omicron_common::api::external::UserId; use omicron_common::api::external::VpcFirewallRuleUpdateParams; use omicron_test_utils::certificates::CertificateChain; +use semver::Version; use std::net::IpAddr; use std::net::Ipv4Addr; use std::str::FromStr; @@ -1137,6 +1138,12 @@ pub static ALLOW_LIST_UPDATE: LazyLock = allowed_ips: AllowedSourceIps::Any, }); +// Updates +pub static DEMO_TARGET_RELEASE: LazyLock = + LazyLock::new(|| params::SetTargetReleaseParams { + system_version: Version::new(0, 0, 0), + }); + /// Describes an API endpoint to be verified by the "unauthorized" test /// /// These structs are also used to check whether we're covering all endpoints in @@ -2350,6 +2357,17 @@ pub static VERIFY_ENDPOINTS: LazyLock> = // privileged users. That is captured by GetUnimplemented. allowed_methods: vec![AllowedMethod::GetUnimplemented], }, + VerifyEndpoint { + url: "/v1/system/update/target-release", + visibility: Visibility::Public, + unprivileged_access: UnprivilegedAccess::None, + allowed_methods: vec![ + AllowedMethod::Get, + AllowedMethod::Put( + serde_json::to_value(&*DEMO_TARGET_RELEASE).unwrap(), + ), + ], + }, /* Metrics */ VerifyEndpoint { url: &DEMO_SYSTEM_METRICS_URL, diff --git a/nexus/tests/output/uncovered-authz-endpoints.txt b/nexus/tests/output/uncovered-authz-endpoints.txt index a637112ad2..8a639f1224 100644 --- a/nexus/tests/output/uncovered-authz-endpoints.txt +++ b/nexus/tests/output/uncovered-authz-endpoints.txt @@ -9,7 +9,6 @@ ping (get "/v1/ping") networking_switch_port_lldp_neighbors (get "/v1/system/hardware/rack-switch-port/{rack_id}/{switch_location}/{port}/lldp/neighbors") networking_switch_port_lldp_config_view (get "/v1/system/hardware/switch-port/{port}/lldp/config") networking_switch_port_status (get "/v1/system/hardware/switch-port/{port}/status") -target_release_get (get "/v1/system/update/target-release") support_bundle_head (head "/experimental/v1/system/support-bundles/{support_bundle}/download") support_bundle_head_file (head "/experimental/v1/system/support-bundles/{support_bundle}/download/{file}") device_auth_request (post "/device/auth") @@ -20,4 +19,3 @@ login_saml (post "/login/{silo_name}/saml/{provi login_local (post "/v1/login/{silo_name}/local") logout (post "/v1/logout") networking_switch_port_lldp_config_update (post "/v1/system/hardware/switch-port/{port}/lldp/config") -target_release_set (put "/v1/system/update/target-release")