Skip to content

Commit

Permalink
Don't delete provisioning backup when configuration changes (#2989)
Browse files Browse the repository at this point in the history
At startup, if iotedged detects that `config.yaml` has changed it will remove the `{homedir}/cache/` folder, rebuild the `settings_state` file in that folder (the hash used to detect `config.yaml` changes), and remove any running containers so edge agent can recreate them. When this logic was originally introduced into iotedged, the _other_ file in that folder, `provisioning_backup.json`, would have also been recreated a little later in the startup cycle after provisioning took place.

The problem is that, in 1.0.9, the startup sequence was adjusted and provisioning now takes place _before_ we detect changes in `config.yaml`. As a result, `provisioning_backup.json` is created but then subsequently deleted if `config.yaml` changes are detected.

The impact is that users who specify some form of DPS provisioning in their `config.yaml`, and who upgrade their devices to 1.0.9 will not be able to reprovision their offline devices without doing an additional reboot.

This change removes the code that deleted the entire `cache/` folder. The existing code to recreate `settings_state` already creates the file if it didn't exist, or overwrites it otherwise. The provisioning backup file will already have been refreshed earlier in the startup sequence and doesn't need to be recreated.
  • Loading branch information
damonbarry authored May 26, 2020
1 parent 9161dc8 commit 377e51b
Showing 1 changed file with 72 additions and 12 deletions.
84 changes: 72 additions & 12 deletions edgelet/iotedged/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1077,7 +1077,6 @@ where
M::Settings: Serialize,
C: CreateCertificate + GetIssuerAlias + MasterEncryptionKey,
{
// Remove all edge containers and destroy the cache (settings and dps backup)
info!("Removing all modules...");
tokio_runtime
.block_on(runtime.remove_all())
Expand All @@ -1086,22 +1085,13 @@ where
))?;
info!("Finished removing modules.");

// Ignore errors from this operation because we could be recovering from a previous bad
// configuration and shouldn't stall the current configuration because of that
let _u = fs::remove_dir_all(subdir);

let path = subdir.join(filename);

DirBuilder::new()
.recursive(true)
.create(subdir)
.context(ErrorKind::Initialize(
InitializeErrorReason::CreateSettingsDirectory,
))?;

// regenerate the workload CA certificate
destroy_workload_ca(crypto)?;
prepare_workload_ca(crypto)?;

// regenerate settings_state
let mut file =
File::create(path).context(ErrorKind::Initialize(InitializeErrorReason::SaveSettings))?;
let digest = compute_settings_digest(settings, id_cert_thumbprint)
Expand Down Expand Up @@ -2281,6 +2271,76 @@ mod tests {
assert_ne!(written1, written);
}

#[test]
fn check_settings_state_does_not_delete_other_files() {
let tmp_dir = TempDir::new("blah").unwrap();
let settings = Settings::new(Some(Path::new(GOOD_SETTINGS))).unwrap();
let config = DockerConfig::new(
"microsoft/test-image".to_string(),
ContainerCreateBody::new(),
None,
)
.unwrap();

// create baseline settings_state
let state = ModuleRuntimeState::default();
let module: TestModule<Error, _> =
TestModule::new_with_config("test-module".to_string(), config, Ok(state));
let runtime = TestRuntime::make_runtime(
settings.clone(),
TestProvisioningResult::new(),
TestHsm::default(),
)
.wait()
.unwrap()
.with_module(Ok(module));
let crypto = TestCrypto {
use_expired_ca: false,
fail_device_ca_alias: false,
fail_decrypt: false,
fail_encrypt: true,
};
let mut tokio_runtime = tokio::runtime::Runtime::new().unwrap();
check_settings_state::<TestRuntime<_, Settings>, _>(
tmp_dir.path(),
"settings_state",
&settings,
&runtime,
&crypto,
&mut tokio_runtime,
None,
)
.unwrap();

// create a couple extra files in the same folder as settings_state
let provisioning_backup_json = tmp_dir.path().join("provisioning_backup.json");
File::create(&provisioning_backup_json)
.unwrap()
.write_all(b"{}")
.unwrap();

let file1 = tmp_dir.path().join("file1.txt");
File::create(&file1).unwrap().write_all(b"hello").unwrap();

// change settings; will force update of settings_state
let settings1 = Settings::new(Some(Path::new(GOOD_SETTINGS1))).unwrap();
let mut tokio_runtime = tokio::runtime::Runtime::new().unwrap();
check_settings_state::<TestRuntime<_, Settings>, _>(
tmp_dir.path(),
"settings_state",
&settings1,
&runtime,
&crypto,
&mut tokio_runtime,
None,
)
.unwrap();

// verify extra files weren't deleted
assert!(provisioning_backup_json.exists());
assert!(file1.exists());
}

#[test]
fn get_proxy_uri_recognizes_https_proxy() {
// Use existing "https_proxy" env var if it's set, otherwise invent one
Expand Down

0 comments on commit 377e51b

Please sign in to comment.