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]>

Author: lann

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::*;

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(())
     }
 }

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![],

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(())
 }