Remove 'allow_transient_write' from spin-loader

This functionality is now handled in spin-core by mounting files with write capabilites disabled. Various tidying in the vicinity of removals. Signed-off-by: Lann Martin <[email protected]>
fermyon · Sep 13, 2022 · 2bc3d99 · 2bc3d99
1 parent 98ddf08
commit 2bc3d99
Show file tree

Hide file tree

Showing 7 changed files with 41 additions and 142 deletions.
diff --git a/crates/loader/src/assets.rs b/crates/loader/src/assets.rs
@@ -90,19 +90,6 @@ pub fn file_sha256_digest_string(path: impl AsRef<Path>) -> std::io::Result<Stri
  Ok(digest_string)
 }
 
-/// Change file to writable or readonly
-pub(crate) async fn change_file_permission(
- path: impl AsRef<Path>,
- allow_transient_write: bool,
-) -> Result<()> {
- let mut perms = tokio::fs::metadata(&path).await?.permissions();
- perms.set_readonly(!allow_transient_write);
- tokio::fs::set_permissions(path, perms.clone())
- .await
- .with_context(|| anyhow!("Cannot set permission {:?}", perms.clone()))?;
- Ok(())
-}
-
 #[cfg(test)]
 mod test {
  use super::*;

diff --git a/crates/loader/src/bindle/assets.rs b/crates/loader/src/bindle/assets.rs
@@ -12,7 +12,7 @@ use tracing::log;
 
 use crate::file_sha256_digest_string;
 use crate::{
- assets::{change_file_permission, create_dir, ensure_under},
+ assets::{create_dir, ensure_under},
  bindle::utils::BindleReader,
 };
 
@@ -22,15 +22,12 @@ pub(crate) async fn prepare_component(
  parcels: &[Label],
  base_dst: impl AsRef<Path>,
  component: &str,
- allow_transient_write: bool,
 ) -> Result<DirectoryMount> {
  let copier = Copier {
  reader: reader.clone(),
  id: bindle_id.clone(),
  };
- copier
- .prepare(parcels, base_dst, component, allow_transient_write)
- .await
+ copier.prepare(parcels, base_dst, component).await
 }
 
 pub(crate) struct Copier {
@@ -44,7 +41,6 @@ impl Copier {
  parcels: &[Label],
  base_dst: impl AsRef<Path>,
  component: &str,
- allow_transient_write: bool,
  ) -> Result<DirectoryMount> {
  log::info!(
  "Mounting files from '{}' to '{}'",
@@ -54,56 +50,34 @@ impl Copier {
 
  let host = create_dir(&base_dst, component).await?;
  let guest = "/".to_string();
- self.copy_all(parcels, &host, allow_transient_write).await?;
+ self.copy_all(parcels, &host).await?;
 
  Ok(DirectoryMount { host, guest })
  }
 
- async fn copy_all(
- &self,
- parcels: &[Label],
- dir: impl AsRef<Path>,
- allow_transient_write: bool,
- ) -> Result<()> {
- match stream::iter(
- parcels
- .iter()
- .map(|p| self.copy(p, &dir, allow_transient_write)),
- )
- .buffer_unordered(crate::MAX_PARALLEL_ASSET_PROCESSING)
- .filter_map(|r| future::ready(r.err()))
- .map(|e| log::error!("{:?}", e))
- .count()
- .await
+ async fn copy_all(&self, parcels: &[Label], dir: impl AsRef<Path>) -> Result<()> {
+ match stream::iter(parcels.iter().map(|p| self.copy(p, &dir)))
+ .buffer_unordered(crate::MAX_PARALLEL_ASSET_PROCESSING)
+ .filter_map(|r| future::ready(r.err()))
+ .map(|e| log::error!("{:?}", e))
+ .count()
+ .await
  {
  0 => Ok(()),
  n => bail!("Error copying assets: {} file(s) not copied", n),
  }
  }
 
- async fn copy(
- &self,
- p: &Label,
- dir: impl AsRef<Path>,
- allow_transient_write: bool,
- ) -> Result<()> {
+ async fn copy(&self, p: &Label, dir: impl AsRef<Path>) -> Result<()> {
  let to = dir.as_ref().join(&p.name);
 
  ensure_under(&dir, &to)?;
 
  if to.exists() {
  match check_existing_file(to.clone(), p).await {
- Ok(true) => {
- change_file_permission(&to, allow_transient_write).await?;
- return Ok(());
- }
- Ok(false) => {
- // file exists but digest doesn't match, set it to writable first for further writing
- let perms = tokio::fs::metadata(&to).await?.permissions();
- if perms.readonly() {
- change_file_permission(&to, true).await?;
- }
- }
+ // Copy already exists
+ Ok(true) => return Ok(()),
+ Ok(false) => (),
  Err(err) => tracing::error!("Error verifying existing parcel: {}", err),
  }
  }
@@ -144,8 +118,6 @@ impl Copier {
  })?;
  }
 
- change_file_permission(&to, allow_transient_write).await?;
-
  Ok(())
  }
 }

diff --git a/crates/loader/src/bindle/mod.rs b/crates/loader/src/bindle/mod.rs
@@ -15,13 +15,11 @@ use std::path::Path;
 use anyhow::{anyhow, Context, Result};
 use bindle::Invoice;
 use futures::future;
-
 use outbound_http::allowed_http_hosts::validate_allowed_http_hosts;
 use spin_manifest::{
  Application, ApplicationInformation, ApplicationOrigin, CoreComponent, ModuleSource,
  SpinVersion, WasmConfig,
 };
-use tracing::log;
 
 use crate::bindle::{
  config::{RawAppManifest, RawComponentManifest},
@@ -35,19 +33,14 @@ pub use utils::SPIN_MANIFEST_MEDIA_TYPE;
 /// prepared application configuration consumable by a Spin execution context.
 /// If a directory is provided, use it as the base directory to expand the assets,
 /// otherwise create a new temporary directory.
-pub async fn from_bindle(
- id: &str,
- url: &str,
- base_dst: impl AsRef<Path>,
- allow_transient_write: bool,
-) -> Result<Application> {
+pub async fn from_bindle(id: &str, url: &str, base_dst: impl AsRef<Path>) -> Result<Application> {
  // TODO
  // Handle Bindle authentication.
  let connection_info = BindleConnectionInfo::new(url, false, None, None);
  let client = connection_info.client()?;
  let reader = BindleReader::remote(&client, &id.parse()?);
 
- prepare(id, url, &reader, base_dst, allow_transient_write).await
+ prepare(id, url, &reader, base_dst).await
 }
 
 /// Converts a Bindle invoice into Spin configuration.
@@ -56,7 +49,6 @@ async fn prepare(
  url: &str,
  reader: &BindleReader,
  base_dst: impl AsRef<Path>,
- allow_transient_write: bool,
 ) -> Result<Application> {
  // First, get the invoice from the Bindle server.
  let invoice = reader
@@ -67,12 +59,12 @@ async fn prepare(
  // Then, reconstruct the application manifest from the parcels.
  let raw: RawAppManifest =
  toml::from_slice(&reader.get_parcel(&find_manifest(&invoice)?).await?)?;
- log::trace!("Recreated manifest from bindle: {:?}", raw);
+ tracing::trace!("Recreated manifest from bindle: {:?}", raw);
 
  validate_raw_app_manifest(&raw)?;
 
  let info = info(&raw, &invoice, url);
- log::trace!("Application information from bindle: {:?}", info);
+ tracing::trace!("Application information from bindle: {:?}", info);
  let component_triggers = raw
  .components
  .iter()
@@ -81,7 +73,7 @@ async fn prepare(
  let components = future::join_all(
  raw.components
  .into_iter()
- .map(|c| async { core(c, &invoice, reader, &base_dst, allow_transient_write).await })
+ .map(|c| async { core(c, &invoice, reader, &base_dst).await })
  .collect::<Vec<_>>(),
  )
  .await
@@ -115,7 +107,6 @@ async fn core(
  invoice: &Invoice,
  reader: &BindleReader,
  base_dst: impl AsRef<Path>,
- allow_transient_write: bool,
 ) -> Result<CoreComponent> {
  let bytes = reader
  .get_parcel(&raw.source)
@@ -129,15 +120,8 @@ async fn core(
  Some(group) => {
  let parcels = parcels_in_group(invoice, &group);
  vec![
- assets::prepare_component(
- reader,
- &invoice.bindle.id,
- &parcels,
- base_dst,
- &id,
- allow_transient_write,
- )
- .await?,
+ assets::prepare_component(reader, &invoice.bindle.id, &parcels, base_dst, &id)
+ .await?,
  ]
  }
  None => vec![],

diff --git a/crates/loader/src/local/assets.rs b/crates/loader/src/local/assets.rs
@@ -1,16 +1,13 @@
 #![deny(missing_docs)]
 
-use crate::assets::{
- change_file_permission, create_dir, ensure_all_under, ensure_under, to_relative,
-};
+use crate::assets::{create_dir, ensure_all_under, ensure_under, to_relative};
 use anyhow::{anyhow, bail, ensure, Context, Result};
 use futures::{future, stream, StreamExt};
 use spin_manifest::DirectoryMount;
 use std::{
  path::{Path, PathBuf},
  vec,
 };
-use tracing::log;
 use walkdir::WalkDir;
 
 use super::config::{RawDirectoryPlacement, RawFileMount};
@@ -22,10 +19,9 @@ pub(crate) async fn prepare_component(
  src: impl AsRef<Path>,
  base_dst: impl AsRef<Path>,
  id: &str,
- allow_transient_write: bool,
  exclude_files: &[String],
 ) -> Result<Vec<DirectoryMount>> {
- log::info!(
+ tracing::info!(
  "Mounting files from '{}' to '{}'",
  src.as_ref().display(),
  base_dst.as_ref().display()
@@ -34,7 +30,7 @@ pub(crate) async fn prepare_component(
  let files = collect(raw_mounts, exclude_files, src)?;
  let host = create_dir(&base_dst, id).await?;
  let guest = "/".to_string();
- copy_all(&files, &host, allow_transient_write).await?;
+ copy_all(&files, &host).await?;
 
  Ok(vec![DirectoryMount { guest, host }])
 }
@@ -173,7 +169,7 @@ fn collect_placement(
 /// Generate a vector of file mounts given a file pattern.
 fn collect_pattern(pattern: &str, rel: impl AsRef<Path>) -> Result<Vec<FileMount>> {
  let abs = rel.as_ref().join(pattern);
- log::trace!("Resolving asset file pattern '{:?}'", abs);
+ tracing::trace!("Resolving asset file pattern '{:?}'", abs);
 
  let matches = glob::glob(&abs.to_string_lossy())?;
  let specifiers = matches
@@ -186,53 +182,36 @@ fn collect_pattern(pattern: &str, rel: impl AsRef<Path>) -> Result<Vec<FileMount
 }
 
 /// Copy all files to the mount directory.
-async fn copy_all(
- files: &[FileMount],
- dir: impl AsRef<Path>,
- allow_transient_write: bool,
-) -> Result<()> {
- let copy_futures = files.iter().map(|f| copy(f, &dir, allow_transient_write));
+async fn copy_all(files: &[FileMount], dir: impl AsRef<Path>) -> Result<()> {
+ let copy_futures = files.iter().map(|f| copy(f, &dir));
  let errors = stream::iter(copy_futures)
  .buffer_unordered(crate::MAX_PARALLEL_ASSET_PROCESSING)
  .filter_map(|r| future::ready(r.err()))
- .map(|e| log::error!("{:?}", e))
+ .map(|e| tracing::error!("{e:?}"))
  .count()
  .await;
  ensure!(
  errors == 0,
- "Error copying assets: {} file(s) not copied",
- errors
+ "Error copying assets: {errors} file(s) not copied",
  );
  Ok(())
 }
 
 /// Copy a single file to the mount directory, setting it as read-only.
-async fn copy(file: &FileMount, dir: impl AsRef<Path>, allow_transient_write: bool) -> Result<()> {
+async fn copy(file: &FileMount, dir: impl AsRef<Path>) -> Result<()> {
  let from = &file.src;
  let to = dir.as_ref().join(&file.relative_dst);
 
  ensure_under(&dir.as_ref(), &to.as_path())?;
 
- log::trace!(
- "Copying asset file '{}' -> '{}'",
- from.display(),
- to.display()
- );
+ tracing::trace!("Copying asset file '{from:?}' -> '{to:?}'");
 
- tokio::fs::create_dir_all(to.parent().expect("Cannot copy to file '/'")).await?;
-
- // if destination file is read-only, set it to writable first
- let metadata = tokio::fs::metadata(&to).await;
- if metadata.is_ok() && metadata.unwrap().permissions().readonly() {
- change_file_permission(&to, true).await?;
- }
+ tokio::fs::create_dir_all(to.parent().context("Cannot copy to file '/'")?).await?;
 
  let _ = tokio::fs::copy(&from, &to)
  .await
  .with_context(|| anyhow!("Error copying asset file '{}'", from.display()))?;
 
- change_file_permission(&to, allow_transient_write).await?;
-
  Ok(())
 }