Skip to content

Commit 4753535

Browse files
authored
Ignore build metadata when comparing system versions (#9072)
See #9071 for context; this is the short/medium-term fix proposed in that issue.
1 parent 65c8aed commit 4753535

File tree

1 file changed

+62
-1
lines changed

1 file changed

+62
-1
lines changed

nexus/src/external_api/http_entrypoints.rs

Lines changed: 62 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6850,7 +6850,10 @@ impl NexusExternalApi for NexusExternalApiImpl {
68506850
.await?
68516851
.release_source
68526852
{
6853-
if version > system_version {
6853+
if !is_new_target_release_version_allowed(
6854+
&version,
6855+
&system_version,
6856+
) {
68546857
return Err(HttpError::for_bad_request(
68556858
None,
68566859
format!(
@@ -8814,3 +8817,61 @@ impl NexusExternalApi for NexusExternalApiImpl {
88148817
.await
88158818
}
88168819
}
8820+
8821+
fn is_new_target_release_version_allowed(
8822+
current_version: &semver::Version,
8823+
proposed_new_version: &semver::Version,
8824+
) -> bool {
8825+
let mut current_version = current_version.clone();
8826+
let mut proposed_new_version = proposed_new_version.clone();
8827+
8828+
// Strip out the build metadata; this allows upgrading from one commit on
8829+
// the same major/minor/release/patch to another. This isn't always right -
8830+
// we shouldn't allow downgrading to an earlier commit - but we don't have
8831+
// enough information in the version strings today to determine that. See
8832+
// <https://github.com/oxidecomputer/omicron/issues/9071>.
8833+
current_version.build = semver::BuildMetadata::EMPTY;
8834+
proposed_new_version.build = semver::BuildMetadata::EMPTY;
8835+
8836+
proposed_new_version >= current_version
8837+
}
8838+
8839+
#[cfg(test)]
8840+
mod tests {
8841+
use super::*;
8842+
8843+
#[test]
8844+
fn test_is_new_target_release_version_allowed() {
8845+
// Updating between versions that differ only in build metadata should
8846+
// be allowed in both directions.
8847+
let v1: semver::Version = "16.0.0-0.ci+git544f608e05a".parse().unwrap();
8848+
let v2: semver::Version = "16.0.0-0.ci+git8571be38c0b".parse().unwrap();
8849+
assert!(is_new_target_release_version_allowed(&v1, &v2));
8850+
assert!(is_new_target_release_version_allowed(&v2, &v1));
8851+
8852+
// Updating from a version to itself is always allowed. (This is
8853+
// important for clearing mupdate overrides.)
8854+
assert!(is_new_target_release_version_allowed(&v1, &v1));
8855+
assert!(is_new_target_release_version_allowed(&v2, &v2));
8856+
8857+
// We should be able to upgrade but not downgrade if the versions differ
8858+
// in major/minor/patch/prerelease.
8859+
for (v1, v2) in [
8860+
("15.0.0-0.ci+git12345", "16.0.0-0.ci+git12345"),
8861+
("16.0.0-0.ci+git12345", "16.1.0-0.ci+git12345"),
8862+
("16.1.0-0.ci+git12345", "16.1.1-0.ci+git12345"),
8863+
("16.1.1-0.ci+git12345", "16.1.1-1.ci+git12345"),
8864+
] {
8865+
let v1: semver::Version = v1.parse().unwrap();
8866+
let v2: semver::Version = v2.parse().unwrap();
8867+
assert!(
8868+
is_new_target_release_version_allowed(&v1, &v2),
8869+
"should be allowed to upgrade from {v1} to {v2}"
8870+
);
8871+
assert!(
8872+
!is_new_target_release_version_allowed(&v2, &v1),
8873+
"should not be allowed to upgrade from {v1} to {v2}"
8874+
);
8875+
}
8876+
}
8877+
}

0 commit comments

Comments
 (0)