Skip to content

Commit bb796b6

Browse files
authored
set_target_release: Fail if setting target release to a pruned TUF repo (#9117)
Fixes #9114.
1 parent 887bc42 commit bb796b6

File tree

3 files changed

+228
-41
lines changed

3 files changed

+228
-41
lines changed

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

nexus/db-queries/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,7 @@ pretty_assertions.workspace = true
107107
rcgen.workspace = true
108108
regex.workspace = true
109109
rustls.workspace = true
110+
sha2.workspace = true
110111
subprocess.workspace = true
111112
term.workspace = true
112113

nexus/db-queries/src/db/datastore/target_release.rs

Lines changed: 226 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,9 @@ use crate::db::model::{
1313
use async_bb8_diesel::AsyncRunQueryDsl as _;
1414
use diesel::insert_into;
1515
use diesel::prelude::*;
16+
use diesel::sql_types;
1617
use nexus_db_errors::{ErrorHandler, public_error_from_diesel};
18+
use nexus_db_schema::enums::TargetReleaseSourceEnum;
1719
use nexus_db_schema::schema::target_release::dsl;
1820
use nexus_types::external_api::views;
1921
use omicron_common::api::external::{CreateResult, Error, LookupResult};
@@ -77,12 +79,104 @@ impl DataStore {
7779
.authorize(authz::Action::Modify, &authz::TARGET_RELEASE_CONFIG)
7880
.await?;
7981
let conn = self.pool_connection_authorized(opctx).await?;
80-
insert_into(dsl::target_release)
81-
.values(target_release)
82-
.returning(TargetRelease::as_returning())
83-
.get_result_async(&*conn)
84-
.await
85-
.map_err(|e| public_error_from_diesel(e, ErrorHandler::Server))
82+
83+
// If we have a TUF repo ID, we need a more complex query to confirm
84+
// (transactionally) that it isn't the pruned as we make it the target
85+
// release.
86+
if let Some(tuf_repo_id) = target_release.tuf_repo_id {
87+
let selection = {
88+
use nexus_db_schema::schema::tuf_repo::dsl as repo_dsl;
89+
90+
// This statement is just here to force a compilation error if
91+
// the set of columns in `target_release` changes, because that
92+
// will affect the correctness of the query below.
93+
//
94+
// If you're here because of a compile error, you might be
95+
// changing the `target_release` table. Update the statement
96+
// below to match!
97+
let _: (
98+
dsl::generation,
99+
dsl::time_requested,
100+
dsl::release_source,
101+
dsl::tuf_repo_id,
102+
) = dsl::target_release::all_columns();
103+
104+
// What we want to write here is a query that confirms
105+
// `tuf_repo_id` is not pruned and avoids performing an insert
106+
// otherwise. We'll do that via an `INSERT SELECT ...` where the
107+
// `SELECT` is:
108+
//
109+
// ```
110+
// SELECT $target_release WHERE EXISTS (
111+
// SELECT 1 FROM tuf_repo WHERE
112+
// id = $tuf_repo_id
113+
// AND time_pruned IS NULL
114+
// )
115+
// ```
116+
//
117+
// but with a couple of diesel quirks:
118+
//
119+
// 1. We can't splat the `$target_release` value directly into a
120+
// SELECT, so we select each of its columns individually. See
121+
// the above check that the columns of this table haven't
122+
// changed.
123+
// 2. We don't bother getting it to `SELECT 1 ...` in the
124+
// subquery. diesel defaults to `SELECT * ...` there instead,
125+
// but that should be fine since it's inside a `WHERE
126+
// EXISTS`.
127+
diesel::select((
128+
target_release.generation.into_sql::<sql_types::Int8>(),
129+
target_release
130+
.time_requested
131+
.into_sql::<sql_types::Timestamptz>(),
132+
target_release
133+
.release_source
134+
.into_sql::<TargetReleaseSourceEnum>(),
135+
tuf_repo_id
136+
.into_sql::<sql_types::Nullable<sql_types::Uuid>>(),
137+
))
138+
.filter(diesel::dsl::exists(
139+
repo_dsl::tuf_repo
140+
.filter(repo_dsl::id.eq(tuf_repo_id))
141+
.filter(repo_dsl::time_pruned.is_null()),
142+
))
143+
};
144+
145+
// Attempt the insert; use `.optional()` so we can attach a custom
146+
// error message if we get back no rows.
147+
let result = insert_into(dsl::target_release)
148+
.values(selection)
149+
.returning(TargetRelease::as_returning())
150+
.get_result_async(&*conn)
151+
.await
152+
.optional()
153+
.map_err(|e| {
154+
public_error_from_diesel(e, ErrorHandler::Server)
155+
})?;
156+
157+
match result {
158+
Some(target_release) => {
159+
// Insertion succeeded and returned the newly-inserted
160+
// target release.
161+
Ok(target_release)
162+
}
163+
None => {
164+
// Insertion succeeded but didn't return any rows: we tried
165+
// to insert a target release for a pruned repo.
166+
Err(Error::invalid_request(format!(
167+
"cannot make TUF repo {tuf_repo_id} the \
168+
target release: it has been pruned from the system"
169+
)))
170+
}
171+
}
172+
} else {
173+
insert_into(dsl::target_release)
174+
.values(target_release)
175+
.returning(TargetRelease::as_returning())
176+
.get_result_async(&*conn)
177+
.await
178+
.map_err(|e| public_error_from_diesel(e, ErrorHandler::Server))
179+
}
86180
}
87181

88182
/// Convert a model-level target release to an external view.
@@ -135,13 +229,56 @@ mod test {
135229
use crate::db::model::{Generation, TargetReleaseSource};
136230
use crate::db::pub_test_utils::TestDatabase;
137231
use chrono::{TimeDelta, Utc};
232+
use nexus_db_model::TufRepo;
138233
use omicron_common::api::external::{
139234
TufArtifactMeta, TufRepoDescription, TufRepoMeta,
140235
};
141236
use omicron_common::update::ArtifactId;
142237
use omicron_test_utils::dev;
143238
use semver::Version;
144-
use tufaceous_artifact::{ArtifactKind, ArtifactVersion};
239+
use sha2::Digest;
240+
use sha2::Sha256;
241+
use slog_error_chain::InlineErrorChain;
242+
use tufaceous_artifact::{ArtifactHash, ArtifactKind, ArtifactVersion};
243+
244+
async fn insert_tuf_repo(
245+
opctx: &OpContext,
246+
datastore: &DataStore,
247+
version: &Version,
248+
) -> TufRepo {
249+
let artifact_version = ArtifactVersion::new(version.to_string())
250+
.expect("version is valid for artifacts too");
251+
let hash = ArtifactHash(Sha256::digest(version.to_string()).into());
252+
253+
datastore
254+
.tuf_repo_insert(
255+
opctx,
256+
&TufRepoDescription {
257+
repo: TufRepoMeta {
258+
hash,
259+
targets_role_version: 0,
260+
valid_until: Utc::now(),
261+
system_version: version.clone(),
262+
file_name: String::new(),
263+
},
264+
artifacts: vec![TufArtifactMeta {
265+
id: ArtifactId {
266+
name: String::new(),
267+
version: artifact_version,
268+
kind: ArtifactKind::from_static("empty"),
269+
},
270+
hash,
271+
size: 0,
272+
board: None,
273+
sign: None,
274+
}],
275+
},
276+
)
277+
.await
278+
.expect("inserted TUF repo description")
279+
.recorded
280+
.repo
281+
}
145282

146283
#[tokio::test]
147284
async fn target_release_datastore() {
@@ -203,40 +340,7 @@ mod test {
203340

204341
// Now add a new TUF repo and use it as the source.
205342
let version = Version::new(0, 0, 1);
206-
const ARTIFACT_VERSION: ArtifactVersion =
207-
ArtifactVersion::new_const("0.0.1");
208-
let hash =
209-
"e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"
210-
.parse()
211-
.expect("SHA256('')");
212-
let repo = datastore
213-
.tuf_repo_insert(
214-
opctx,
215-
&TufRepoDescription {
216-
repo: TufRepoMeta {
217-
hash,
218-
targets_role_version: 0,
219-
valid_until: Utc::now(),
220-
system_version: version.clone(),
221-
file_name: String::new(),
222-
},
223-
artifacts: vec![TufArtifactMeta {
224-
id: ArtifactId {
225-
name: String::new(),
226-
version: ARTIFACT_VERSION,
227-
kind: ArtifactKind::from_static("empty"),
228-
},
229-
hash,
230-
size: 0,
231-
board: None,
232-
sign: None,
233-
}],
234-
},
235-
)
236-
.await
237-
.unwrap()
238-
.recorded
239-
.repo;
343+
let repo = insert_tuf_repo(opctx, datastore, &version).await;
240344
assert_eq!(repo.system_version, version.into());
241345
let tuf_repo_id = repo.id;
242346

@@ -262,4 +366,85 @@ mod test {
262366
db.terminate().await;
263367
logctx.cleanup_successful();
264368
}
369+
370+
#[tokio::test]
371+
async fn reject_target_release_if_repo_pruned() {
372+
let logctx =
373+
dev::test_setup_log("reject_target_release_if_repo_pruned");
374+
let db = TestDatabase::new_with_datastore(&logctx.log).await;
375+
let (opctx, datastore) = (db.opctx(), db.datastore());
376+
377+
// Insert two TUF repos.
378+
let repo1 =
379+
insert_tuf_repo(opctx, datastore, &Version::new(0, 0, 1)).await;
380+
let repo2 =
381+
insert_tuf_repo(opctx, datastore, &Version::new(0, 0, 2)).await;
382+
383+
// Manually prune the second one.
384+
{
385+
use nexus_db_schema::schema::tuf_repo::dsl;
386+
387+
let conn = datastore
388+
.pool_connection_for_tests()
389+
.await
390+
.expect("got connection");
391+
let n = diesel::update(dsl::tuf_repo)
392+
.filter(dsl::id.eq(repo2.id))
393+
.set(dsl::time_pruned.eq(Some(Utc::now())))
394+
.execute_async(&*conn)
395+
.await
396+
.expect("pruned repo2");
397+
assert_eq!(n, 1, "should have only pruned 1 repo");
398+
}
399+
400+
// There should always be an initial target release.
401+
let target_release = datastore
402+
.target_release_get_current(opctx)
403+
.await
404+
.expect("should be a target release");
405+
406+
// Make repo1 the target release. This should succeed.
407+
let target_release = datastore
408+
.target_release_insert(
409+
opctx,
410+
TargetRelease::new_system_version(&target_release, repo1.id),
411+
)
412+
.await
413+
.expect("made repo1 the target release");
414+
assert_eq!(target_release.generation, Generation(2.into()));
415+
416+
// Attempting to make repo1 the target release again should fail with a
417+
// reasonable error message (we need a higher generation).
418+
let err = datastore
419+
.target_release_insert(opctx, target_release.clone())
420+
.await
421+
.expect_err("making repo1 the target release again should fail");
422+
let err = InlineErrorChain::new(&err).to_string();
423+
assert!(
424+
err.contains("violates unique constraint"),
425+
"unexpected error: {err}"
426+
);
427+
428+
// Attempt to make repo2 the target release. This should fail with a
429+
// reasonable error message (it's been pruned).
430+
let err = datastore
431+
.target_release_insert(
432+
opctx,
433+
TargetRelease::new_system_version(&target_release, repo2.id),
434+
)
435+
.await
436+
.expect_err("making repo2 the target release should fail");
437+
let err = InlineErrorChain::new(&err).to_string();
438+
assert!(
439+
err.contains("cannot make TUF repo")
440+
&& err.contains(&repo2.id.to_string())
441+
&& err.contains("target release")
442+
&& err.contains("pruned"),
443+
"unexpected error: {err}"
444+
);
445+
446+
// Clean up.
447+
db.terminate().await;
448+
logctx.cleanup_successful();
449+
}
265450
}

0 commit comments

Comments
 (0)