Don't allow downgrading the system target release #7807
Conversation
Attempting to set the target system release to an earlier version should fail. That constraint is now enforced and tested.
| @@ -6442,14 +6442,30 @@ impl NexusExternalApi for NexusExternalApiImpl { | |||
| crate::context::op_context_for_external_api(&rqctx).await?; | |||
| let params = body.into_inner(); | |||
| let system_version = params.system_version; | |||
|
|
|||
| let current_target_release = | |||
There was a problem hiding this comment.
Cool. I like the test, too.
I wonder if this condition needs to be checked transactionally with the update, though? I'm thinking of the time-of-check-to-time-of-use race where say the system is at version 14, the user is trying to update to 15, but someone else comes in and sets it to 16 in the meantime, then we overwrite that with 15. But maybe this is prevented because at L6469 below, we construct the next target release with a generation number of 1 + whatever we just saw (that was version 14), so we'll necessarily fail the database update if someone else comes in in between? If that's the case, could we add a comment here explaining why this race isn't a problem?
There was a problem hiding this comment.
That is indeed the case; documented in c2233b9.
| if version > system_version { | ||
| return Err(HttpError::for_bad_request( | ||
| None, | ||
| "can't downgrade system".into(), |
There was a problem hiding this comment.
| "can't downgrade system".into(), | |
| "The requested target system release ({system_version}) is older than the current target system release ({version}). This is not supported.".into() |
I'm imagining an end user might see this message and want to make sure it's clear what's going on and what they've done wrong.
There was a problem hiding this comment.
Indeed, thanks! Also added in c2233b9.
update-common/manifests/fake0.toml
Outdated
| @@ -0,0 +1,87 @@ | |||
| # This is an artifact manifest that generates fake entries for all components. | |||
There was a problem hiding this comment.
How about calling this fake-old.toml and adding a comment here that the only difference from fake.toml is the version numbers of everything?
plotnick
force-pushed
the
no-downgrades
branch
2 times, most recently
from
0a76d62 to
e019fda
Compare
Attempting to set the target system release to an earlier version should fail. That constraint is now enforced and tested.