diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index e28c2c3e58..483af422ab 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -6442,14 +6442,44 @@ impl NexusExternalApi for NexusExternalApiImpl { crate::context::op_context_for_external_api(&rqctx).await?; let params = body.into_inner(); let system_version = params.system_version; + + // We don't need a transaction for the following queries because + // (1) the generation numbers provide optimistic concurrency control: + // if another request were to successfully update the target release + // between when we fetch it here and when we try to update it below, + // our update would fail because the next generation number would + // would already be taken; and + // (2) we assume that TUF repo depot records are immutable, i.e., + // system version X.Y.Z won't designate different repos over time. + let current_target_release = + nexus.datastore().target_release_get_current(&opctx).await?; + + // Disallow downgrades. + if let views::TargetReleaseSource::SystemVersion { version } = nexus + .datastore() + .target_release_view(&opctx, ¤t_target_release) + .await? + .release_source + { + if version > system_version { + return Err(HttpError::for_bad_request( + None, + format!( + "The requested target system release ({system_version}) \ + is older than the current target system release ({version}). \ + This is not supported." + ), + )); + } + } + + // Fetch the TUF repo metadata and update the target release. let tuf_repo_id = nexus .datastore() .update_tuf_repo_get(&opctx, system_version.into()) .await? .repo .id; - let current_target_release = - nexus.datastore().target_release_get_current(&opctx).await?; let next_target_release = nexus_db_model::TargetRelease::new_system_version( ¤t_target_release, @@ -6459,6 +6489,7 @@ impl NexusExternalApi for NexusExternalApiImpl { .datastore() .target_release_insert(&opctx, next_target_release) .await?; + Ok(HttpResponseCreated( nexus .datastore() diff --git a/nexus/tests/integration_tests/target_release.rs b/nexus/tests/integration_tests/target_release.rs index 298db8720e..f7f052232a 100644 --- a/nexus/tests/integration_tests/target_release.rs +++ b/nexus/tests/integration_tests/target_release.rs @@ -4,6 +4,7 @@ //! Get/set the target release via the external API. +use anyhow::Result; use camino::Utf8Path; use camino_tempfile::Utf8TempDir; use chrono::Utc; @@ -23,7 +24,7 @@ use omicron_sled_agent::sim; use semver::Version; #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn get_set_target_release() -> anyhow::Result<()> { +async fn get_set_target_release() -> Result<()> { let mut config = load_test_config(); config.pkg.updates = Some(UpdatesConfig { // XXX: This is currently not used by the update system, but @@ -87,11 +88,11 @@ async fn get_set_target_release() -> anyhow::Result<()> { assert_eq!( system_version, - upload_tuf_repo(client, &path).await.recorded.repo.system_version + upload_tuf_repo(client, &path).await?.recorded.repo.system_version ); let target_release = - set_target_release(client, system_version.clone()).await; + set_target_release(client, system_version.clone()).await?; let after = Utc::now(); assert_eq!(target_release.generation, 2); assert!(target_release.time_requested >= before); @@ -102,7 +103,7 @@ async fn get_set_target_release() -> anyhow::Result<()> { ); } - // Add a repo with non-semver versions. + // Adding a repo with non-semver artifact versions should be ok, too. { let before = Utc::now(); let system_version = Version::new(2, 0, 0); @@ -121,11 +122,11 @@ async fn get_set_target_release() -> anyhow::Result<()> { assert_eq!( system_version, - upload_tuf_repo(client, &path).await.recorded.repo.system_version + upload_tuf_repo(client, &path).await?.recorded.repo.system_version ); let target_release = - set_target_release(client, system_version.clone()).await; + set_target_release(client, system_version.clone()).await?; let after = Utc::now(); assert_eq!(target_release.generation, 3); assert!(target_release.time_requested >= before); @@ -136,6 +137,12 @@ async fn get_set_target_release() -> anyhow::Result<()> { ); } + // Attempting to downgrade to an earlier system version (2.0.0 → 1.0.0) + // should not be allowed. + set_target_release(client, Version::new(1, 0, 0)) + .await + .expect_err("shouldn't be able to downgrade system"); + ctx.teardown().await; logctx.cleanup_successful(); Ok(()) @@ -144,7 +151,7 @@ async fn get_set_target_release() -> anyhow::Result<()> { async fn upload_tuf_repo( client: &ClientTestContext, path: &Utf8Path, -) -> TufRepoInsertResponse { +) -> Result { NexusRequest::new( RequestBuilder::new( client, @@ -157,15 +164,13 @@ async fn upload_tuf_repo( .authn_as(AuthnMode::PrivilegedUser) .execute() .await - .unwrap() - .parsed_body::() - .unwrap() + .map(|response| response.parsed_body().unwrap()) } async fn set_target_release( client: &ClientTestContext, system_version: Version, -) -> TargetRelease { +) -> Result { NexusRequest::new( RequestBuilder::new( client, @@ -178,7 +183,5 @@ async fn set_target_release( .authn_as(AuthnMode::PrivilegedUser) .execute() .await - .unwrap() - .parsed_body() - .unwrap() + .map(|response| response.parsed_body().unwrap()) }