flutter/shorebird-updater
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat: add rollback support (#194)

Browse Source 
* feat: add rollback support

* Cleanup

* add test

* docs and cleanup

* log full PatchCheckResponse

* remove unnecessary file permission spec from test helper
This commit is contained in:
Bryan Oltman
2024-07-23 10:41:01 -04:00
committed by GitHub
parent 6ba524716c
commit a9fa67c95a
7 changed files with 285 additions and 58 deletions
Download Patch File Download Diff File Expand all files Collapse all files
library/src/c_api/mod.rs
+7 -14
View File
@@ -281,7 +281,10 @@ pub extern "C" fn shorebird_report_launch_success() {
#[cfg(test)]
mod test {
use super::*;
use crate::network::{testing_set_network_hooks, PatchCheckResponse};
use crate::{
network::{testing_set_network_hooks, PatchCheckResponse},
test_utils::write_fake_apk,
};
use anyhow::Ok;
use serial_test::serial;
use tempdir::TempDir;
@@ -438,18 +441,6 @@ mod test {
shorebird_report_launch_failure();
}
fn write_fake_zip(zip_path: &str, libapp_contents: &[u8]) {
use std::io::Write;
let mut zip = zip::ZipWriter::new(std::fs::File::create(zip_path).unwrap());
let options = zip::write::FileOptions::default()
.compression_method(zip::CompressionMethod::Stored)
.unix_permissions(0o755);
let app_path = crate::android::get_relative_lib_path("libapp.so");
zip.start_file(app_path.to_str().unwrap(), options).unwrap();
zip.write_all(libapp_contents).unwrap();
zip.finish().unwrap();
}
#[serial]
#[test]
fn patch_success() {
@@ -460,7 +451,7 @@ mod test {
let base = "hello world";
let expected_new: &str = "hello tests";
let apk_path = tmp_dir.path().join("base.apk");
write_fake_zip(apk_path.to_str().unwrap(), base.as_bytes());
write_fake_apk(apk_path.to_str().unwrap(), base.as_bytes());
let fake_libapp_path = tmp_dir.path().join("lib/arch/ignored.so");
let c_params = parameters(&tmp_dir, fake_libapp_path.to_str().unwrap());
// app_id is required or shorebird_init will fail.
@@ -482,6 +473,7 @@ mod test {
download_url: "ignored".to_owned(),
hash_signature: None,
}),
rolled_back_patch_numbers: None,
})
},
|_url| {
@@ -584,6 +576,7 @@ mod test {
download_url: "ignored".to_owned(),
hash_signature: None,
}),
rolled_back_patch_numbers: None,
})
},
|_url| {
library/src/cache/patch_manager.rs
Vendored
+29 -15
View File
@@ -110,6 +110,10 @@ pub trait ManagePatches {
/// Whether we have failed to boot from the patch with `patch_number`.
fn is_known_bad_patch(&self, patch_number: usize) -> bool;
/// Deletes artifacts for the provided patch_number if they exist.
/// If the patch is the next_boot_patch, it is cleared.
fn remove_patch(&mut self, patch_number: usize) -> Result<()>;
/// Resets the patch manager to its initial state, removing all patches. This is
/// intended to be used when a new release version is installed.
fn reset(&mut self) -> Result<()>;
@@ -243,9 +247,13 @@ impl PatchManager {
}
fn delete_patch_artifacts(&mut self, patch_number: usize) -> Result<()> {
info!("Deleting patch artifacts for patch {}", patch_number);
let patch_dir = self.patch_dir(patch_number);
if !patch_dir.exists() {
debug!("Patch {} not installed, nothing to delete", patch_number);
return Ok(());
}
info!("Deleting patch artifacts for patch {}", patch_number);
std::fs::remove_dir_all(&patch_dir)
.map_err(|e| {
@@ -258,7 +266,7 @@ impl PatchManager {
/// Deletes artifacts for the provided bad_patch_number and attempts to set the next_boot_patch to the last
/// successfully booted patch. If the last successfully booted patch is not bootable or has the same number
/// as the patch we're falling back from, we clear it as well.
fn try_fall_back_from_patch(&mut self, bad_patch_number: usize) {
fn try_fall_back_from_patch(&mut self, bad_patch_number: usize) -> Result<()> {
// Continue even if we fail to delete the patch artifacts. It's more important to not try to
// boot from a bad patch than to delete its artifacts.
// No need to log failure – delete_patch_artifacts logs for us.
@@ -284,6 +292,8 @@ impl PatchManager {
let _ = self.delete_patch_artifacts(last_boot_patch.number);
}
}
self.save_patches_state()
}
/// Deletes all patch artifacts with numbers less than patch_number.
@@ -387,10 +397,11 @@ impl ManagePatches for PatchManager {
if let Err(e) = self.validate_patch_is_bootable(&next_boot_patch) {
error!("Patch {} is not bootable: {}", next_boot_patch.number, e);
self.try_fall_back_from_patch(next_boot_patch.number);
if let Err(e) = self.save_patches_state() {
error!("Failed to save patches state: {}", e);
if let Err(e) = self.try_fall_back_from_patch(next_boot_patch.number) {
error!(
"Failed to fall back from next_boot_patch {}: {}",
next_boot_patch.number, e
);
}
}
@@ -440,14 +451,17 @@ impl ManagePatches for PatchManager {
fn record_boot_failure_for_patch(&mut self, patch_number: usize) -> Result<()> {
self.patches_state.currently_booting_patch = None;
self.patches_state.known_bad_patches.insert(patch_number);
self.try_fall_back_from_patch(patch_number);
self.save_patches_state()
self.try_fall_back_from_patch(patch_number)
}
fn is_known_bad_patch(&self, patch_number: usize) -> bool {
self.patches_state.known_bad_patches.contains(&patch_number)
}
fn remove_patch(&mut self, patch_number: usize) -> Result<()> {
self.try_fall_back_from_patch(patch_number)
}
fn reset(&mut self) -> Result<()> {
self.patches_state = PatchesState::default();
self.save_patches_state()?;
@@ -871,7 +885,7 @@ mod fall_back_tests {
assert!(manager.patches_state.last_booted_patch.is_none());
assert!(manager.patches_state.next_boot_patch.is_none());
manager.try_fall_back_from_patch(1);
manager.try_fall_back_from_patch(1)?;
assert!(manager.patches_state.last_booted_patch.is_none());
assert!(manager.patches_state.next_boot_patch.is_none());
@@ -892,7 +906,7 @@ mod fall_back_tests {
hash: "hash".to_string(),
signature: Some("signature".to_owned()),
});
manager.try_fall_back_from_patch(1);
manager.try_fall_back_from_patch(1)?;
assert_eq!(
manager.patches_state.next_boot_patch,
@@ -915,7 +929,7 @@ mod fall_back_tests {
// Download and fall back from patch 2
manager.add_patch_for_test(&temp_dir, 2)?;
manager.try_fall_back_from_patch(2);
manager.try_fall_back_from_patch(2)?;
assert_eq!(manager.patches_state.last_booted_patch.unwrap().number, 1);
assert_eq!(manager.patches_state.next_boot_patch.unwrap().number, 1);
@@ -938,7 +952,7 @@ mod fall_back_tests {
// Download and fall back from patch 2
manager.add_patch_for_test(&temp_dir, 2)?;
manager.try_fall_back_from_patch(2);
manager.try_fall_back_from_patch(2)?;
// Neither patch should exist.
assert!(manager.patches_state.last_booted_patch.is_none());
@@ -963,7 +977,7 @@ mod fall_back_tests {
manager.record_boot_failure_for_patch(1)?;
manager.try_fall_back_from_patch(1);
manager.try_fall_back_from_patch(1)?;
assert!(manager.patches_state.last_booted_patch.is_none());
assert_eq!(
@@ -999,7 +1013,7 @@ mod fall_back_tests {
let patch_dir = manager.patch_dir(2);
std::fs::remove_dir_all(patch_dir)?;
manager.try_fall_back_from_patch(2);
manager.try_fall_back_from_patch(2)?;
assert!(manager.patches_state.last_booted_patch.is_none());
assert!(manager.patches_state.next_boot_patch.is_none());
library/src/cache/updater_state.rs
Vendored
+6
View File
@@ -199,6 +199,12 @@ impl UpdaterState {
.add_patch(patch.number, &patch.path, hash, signature)
}
/// Removes the artifacts for patch `patch_number` from disk and updates state to ensure the
/// uninstalled patch is not booted in the future.
pub fn uninstall_patch(&mut self, patch_number: usize) -> Result<()> {
self.patch_manager.remove_patch(patch_number)
}
/// Returns true if we have previously failed to boot from patch `patch_number`.
pub fn is_known_bad_patch(&self, patch_number: usize) -> bool {
self.patch_manager.is_known_bad_patch(patch_number)
library/src/lib.rs
+3
View File
@@ -19,6 +19,9 @@ mod yaml;
#[cfg(any(target_os = "android", test))]
mod android;
#[cfg(test)]
mod test_utils;
// Take all public items from the updater namespace and make them public.
pub use self::updater::*;
library/src/network.rs
+8
View File
@@ -190,13 +190,20 @@ pub struct CreatePatchEventRequest {
event: PatchEvent,
}
/// A response from the server telling us the latest state of patches for this release.
#[derive(Debug, Deserialize, Serialize)]
pub struct PatchCheckResponse {
pub patch_available: bool,
#[serde(default)]
pub patch: Option<Patch>,
/// A list of patch numbers that have been rolled back by app developers. These should be
/// uninstalled from the device and not booted from.
#[serde(default)]
pub rolled_back_patch_numbers: Option<Vec<usize>>,
}
/// Reports a patch event (e.g., install success/failure) to the server.
pub fn send_patch_event(event: PatchEvent, config: &UpdateConfig) -> anyhow::Result<()> {
let request = CreatePatchEventRequest { event };
@@ -205,6 +212,7 @@ pub fn send_patch_event(event: PatchEvent, config: &UpdateConfig) -> anyhow::Res
report_event_fn(url, request)
}
/// Downloads the file at `url` to `path`.
pub fn download_to_path(
network_hooks: &NetworkHooks,
url: &str,
library/src/test_utils.rs
+44
View File
@@ -0,0 +1,44 @@
/// Helper methods for tests.
use std::fs;
use crate::{
cache::{PatchInfo, UpdaterState},
config::with_config,
};
/// Writes a fake patch to the patches directory and sets it as the next boot patch.
pub fn install_fake_patch(patch_number: usize) -> anyhow::Result<()> {
with_config(|config| {
let download_dir = std::path::PathBuf::from(&config.download_dir);
let artifact_path = download_dir.join(patch_number.to_string());
fs::create_dir_all(&download_dir)?;
fs::write(&artifact_path, "hello")?;
let mut state = UpdaterState::load_or_new_on_error(
&config.storage_dir,
&config.release_version,
config.patch_public_key.as_deref(),
);
state.install_patch(
&PatchInfo {
path: artifact_path,
number: patch_number,
},
"hash",
None,
)?;
state.save()
})
}
/// Creates a fake APK at `apk_path` and writes `libapp_contents` to its relative `libapp.so` path.
pub fn write_fake_apk(apk_path: &str, libapp_contents: &[u8]) {
use std::io::Write;
let mut zip = zip::ZipWriter::new(std::fs::File::create(apk_path).unwrap());
let options =
zip::write::FileOptions::default().compression_method(zip::CompressionMethod::Stored);
let app_path = crate::android::get_relative_lib_path("libapp.so");
zip.start_file(app_path.to_str().unwrap(), options).unwrap();
zip.write_all(libapp_contents).unwrap();
zip.finish().unwrap();
}
library/src/updater.rs
+188 -29
View File
@@ -103,7 +103,7 @@ impl Display for UpdateError {
}
}
// `AppConfig` is the rust API. `ResolvedConfig` is the internal storage.
// `AppConfig` is the rust API.
// However rusty api would probably used `&str` instead of `String`,
// but making `&str` from `CStr*` is a bit of a pain.
pub struct AppConfig {
@@ -360,6 +360,21 @@ fn update_internal(_: &UpdaterLockState) -> anyhow::Result<UpdateStatus> {
// Check for update.
let patch_check_request_fn = &(config.network_hooks.patch_check_request_fn);
let response = patch_check_request_fn(&patches_check_url(&config.base_url), request)?;
info!("Patch check response: {:?}", response);
with_mut_state(|state| {
if let Some(rolled_back_patches) = response.rolled_back_patch_numbers {
if !rolled_back_patches.is_empty() {
for patch_number in rolled_back_patches {
info!("Attempting uninstall of patch {}...", patch_number);
state.uninstall_patch(patch_number)?;
}
}
}
Ok(())
})?;
if !response.patch_available {
return Ok(UpdateStatus::NoUpdate);
}
@@ -610,22 +625,23 @@ mod tests {
use tempdir::TempDir;
use crate::{
cache::{PatchInfo, UpdaterState},
cache::UpdaterState,
config::{testing_reset_config, with_config},
events::EventType,
network::{testing_set_network_hooks, NetworkHooks, PatchCheckResponse},
test_utils::install_fake_patch,
time, with_state, ExternalFileProvider, Patch,
};
#[derive(Debug, Clone)]
struct FakeExternalFileProvider {}
pub struct FakeExternalFileProvider {}
impl ExternalFileProvider for FakeExternalFileProvider {
fn open(&self) -> anyhow::Result<Box<dyn crate::ReadSeek>> {
Ok(Box::new(std::io::Cursor::new(vec![])))
}
}
fn init_for_testing(tmp_dir: &TempDir, base_url: Option<&str>) {
pub fn init_for_testing(tmp_dir: &TempDir, base_url: Option<&str>) {
testing_reset_config();
let cache_dir = tmp_dir.path().to_str().unwrap().to_string();
let mut yaml = "app_id: 1234".to_string();
@@ -633,12 +649,19 @@ mod tests {
yaml += &format!("\nbase_url: {}", url);
}
let libapp_path = tmp_dir
.path()
.join("lib/arch/libapp.so")
.to_str()
.unwrap()
.to_string();
crate::init(
crate::AppConfig {
app_storage_dir: cache_dir.clone(),
code_cache_dir: cache_dir.clone(),
release_version: "1.0.0+1".to_string(),
original_libapp_paths: vec!["/dir/lib/arch/libapp.so".to_string()],
original_libapp_paths: vec![libapp_path],
},
Box::new(FakeExternalFileProvider {}),
&yaml,
@@ -646,30 +669,6 @@ mod tests {
.unwrap();
}
fn install_fake_patch(patch_number: usize) -> anyhow::Result<()> {
with_config(|config| {
let download_dir = std::path::PathBuf::from(&config.download_dir);
let artifact_path = download_dir.join(patch_number.to_string());
fs::create_dir_all(&download_dir)?;
fs::write(&artifact_path, "hello")?;
let mut state = UpdaterState::load_or_new_on_error(
&config.storage_dir,
&config.release_version,
config.patch_public_key.as_deref(),
);
state.install_patch(
&PatchInfo {
path: artifact_path,
number: patch_number,
},
"hash",
None,
)?;
state.save()
})
}
#[serial]
#[test]
fn subsequent_init_calls_do_not_update_config() {
@@ -947,6 +946,7 @@ mod tests {
hash: "hash".to_string(),
hash_signature: None,
}),
rolled_back_patch_numbers: None,
};
let check_response_body = serde_json::to_string(&check_response).unwrap();
let _ = server
@@ -994,6 +994,7 @@ mod tests {
download_url: "download_url".to_string(),
hash_signature: None,
}),
rolled_back_patch_numbers: None,
};
let check_response_body = serde_json::to_string(&check_response).unwrap();
let _ = server
@@ -1025,6 +1026,7 @@ mod tests {
let check_response = PatchCheckResponse {
patch_available: false,
patch: None,
rolled_back_patch_numbers: None,
};
let check_response_body = serde_json::to_string(&check_response).unwrap();
let _ = server
@@ -1100,6 +1102,7 @@ mod tests {
return Ok(PatchCheckResponse {
patch_available: false,
patch: None,
rolled_back_patch_numbers: None,
});
}
@@ -1132,3 +1135,159 @@ mod tests {
// the patch check callback.
}
}
#[cfg(test)]
mod rollback_tests {
use anyhow::Result;
use serial_test::serial;
use tempdir::TempDir;
use crate::{
network::PatchCheckResponse,
test_utils::{install_fake_patch, write_fake_apk},
};
use super::{
report_launch_start, report_launch_success, tests::init_for_testing, with_mut_state, Patch,
};
#[serial]
#[test]
fn does_not_roll_back_when_rolled_back_patches_is_empty() -> Result<()> {
let mut server = mockito::Server::new();
let check_response = PatchCheckResponse {
patch_available: false,
patch: None,
rolled_back_patch_numbers: Some(vec![]),
};
let check_response_body = serde_json::to_string(&check_response).unwrap();
let _ = server
.mock("POST", "/api/v1/patches/check")
.with_status(200)
.with_body(check_response_body)
.create();
let tmp_dir = TempDir::new("example").unwrap();
init_for_testing(&tmp_dir, Some(&server.url()));
install_fake_patch(1)?;
report_launch_start()?;
report_launch_success()?;
with_mut_state(|state| {
assert_eq!(state.current_boot_patch().map(|p| p.number), Some(1));
assert_eq!(state.next_boot_patch().map(|p| p.number), Some(1));
Ok(())
})?;
let update_result = crate::update();
assert_eq!(update_result.unwrap(), crate::UpdateStatus::NoUpdate);
Ok(())
}
/// If the next_boot_patch is rolled back, the updater should roll back to the release version
/// if no other patches are available on disk.
#[serial]
#[test]
fn rolls_back_from_current_patch_to_release() -> Result<()> {
let mut server = mockito::Server::new();
let check_response = PatchCheckResponse {
patch_available: false,
patch: None,
rolled_back_patch_numbers: Some(vec![1]),
};
let check_response_body = serde_json::to_string(&check_response).unwrap();
let _ = server
.mock("POST", "/api/v1/patches/check")
.with_status(200)
.with_body(check_response_body)
.create();
let tmp_dir = TempDir::new("example").unwrap();
init_for_testing(&tmp_dir, Some(&server.url()));
install_fake_patch(1)?;
report_launch_start()?;
report_launch_success()?;
with_mut_state(|state| {
assert_eq!(state.next_boot_patch().map(|p| p.number), Some(1));
Ok(())
})?;
crate::update()?;
with_mut_state(|state| {
assert!(state.next_boot_patch().is_none());
Ok(())
})?;
Ok(())
}
/// If an older patch is provided by the patch check response, verify that we uninstall the
/// rolled back patch and install the older patch specified by the patch check response.
#[serial]
#[test]
fn rolls_back_to_previous_patch() -> Result<()> {
let mut server = mockito::Server::new();
let download_url = format!("{}/patch/1", server.url());
let check_response = PatchCheckResponse {
patch_available: true,
patch: Some(Patch {
number: 1,
download_url: download_url.to_string(),
// Generated by `string_patch "hello world" "hello tests"`
hash: "bb8f1d041a5cdc259055afe9617136799543e0a7a86f86db82f8c1fadbd8cc45"
.to_string(),
hash_signature: None,
}),
rolled_back_patch_numbers: Some(vec![2]),
};
let check_response_body = serde_json::to_string(&check_response).unwrap();
let _ = server
.mock("POST", "/api/v1/patches/check")
.with_status(200)
.with_body(check_response_body)
.create();
let _ = server
.mock("GET", "/patch/1")
.with_status(200)
.with_body(
// Generated by `string_patch "hello world" "hello tests"`
[
40, 181, 47, 253, 0, 128, 177, 0, 0, 223, 177, 0, 0, 0, 16, 0, 0, 6, 0, 0, 0,
0, 0, 0, 5, 116, 101, 115, 116, 115, 0,
],
)
.create();
let tmp_dir = TempDir::new("example").unwrap();
init_for_testing(&tmp_dir, Some(&server.url()));
// Install the base apk to allow the "downloaded" patch 1 to successfully inflate and install.
let base = "hello world";
let apk_path = tmp_dir.path().join("base.apk");
write_fake_apk(apk_path.to_str().unwrap(), base.as_bytes());
// Install patch 2, pretend we're starting to boot from it, but don't report success or failure
// to ensure we still have patch 1 on disk.
install_fake_patch(2)?;
report_launch_start()?;
report_launch_success()?;
with_mut_state(|state| {
assert_eq!(state.current_boot_patch().map(|p| p.number), Some(2));
assert_eq!(state.next_boot_patch().map(|p| p.number), Some(2));
Ok(())
})?;
let update_result = crate::update();
assert_eq!(update_result.unwrap(), crate::UpdateStatus::UpdateInstalled);
with_mut_state(|state| {
assert_eq!(state.next_boot_patch().map(|p| p.number), Some(1));
Ok(())
})?;
Ok(())
}
}