Improve code style and documentation

.gitignore

@@ -9,3 +9,4 @@
 /.nsjail
 /pkgs
 /pkgs-*-link
+/dump.rdb

README.md

@@ -160,8 +160,17 @@ package, you can use the following command:
 nix profile install --profile pkgs .#packages.<package-name>
 ```
 
-If you want to install all packages, use `all` for `<package-name>`. You can also add or remove
-packages later, but you need to restart Sandkasten after doing so.
+If you want to install all packages, use `all` for `<package-name>`. You can also add, upgrade or
+remove packages later, but you need to restart Sandkasten after doing so. See
+[`nix profile --help`](https://nixos.org/manual/nix/stable/command-ref/new-cli/nix3-profile.html)
+for details.
+
+#### Setup Redis
+Sandkasten uses [Redis](https://redis.io/) for caching. To start a redis server for development,
+simply run `redis-server` in the development shell. Otherwise, if you already have a redis server
+running somewhere, that you would like to use instead, set the environment variable `REDIS_URL`
+to the url of your redis server (see https://docs.rs/redis/latest/redis/#connection-parameters for
+details).
 
 #### Start the application
 In the development shell you can just use `cargo run` to start Sandkasten.

client/src/schemas/environments.rs

@@ -44,12 +44,30 @@ pub struct ListEnvironmentsResponse(pub HashMap<String, Environment>);
 pub struct BaseResourceUsage {
     /// The base resource usage of the build step.
     pub build: Option<ResourceUsage>,
-    /// The minimum base resource usage of the run step.
-    pub run_min: ResourceUsage,
-    /// The average base resource usage of the run step.
-    pub run_avg: ResourceUsage,
-    /// The maximum base resource usage of the run step.
-    pub run_max: ResourceUsage,
+    /// The base resource usage of the run step.
+    pub run: RunResourceUsage,
+}
+
+/// The base resource usage of the run step.
+#[derive(Debug, Clone, Serialize, Deserialize)]
+#[cfg_attr(feature = "poem-openapi", derive(Object))]
+pub struct RunResourceUsage {
+    /// The number of **milliseconds** the process ran.
+    pub time: BenchmarkResult,
+    /// The amount of memory the process used (in **KB**)
+    pub memory: BenchmarkResult,
+}
+
+/// Accumulated benchmark results.
+#[derive(Debug, Clone, Serialize, Deserialize)]
+#[cfg_attr(feature = "poem-openapi", derive(Object))]
+pub struct BenchmarkResult {
+    /// The minimum of the measured values.
+    pub min: u64,
+    /// The average of the measured values.
+    pub avg: u64,
+    /// The maximum of the measured values.
+    pub max: u64,
 }
 
 /// The error responses that may be returned when calculating the base resource
@@ -90,3 +108,25 @@ impl Example for ListEnvironmentsResponse {
         ]))
     }
 }
+
+impl FromIterator<u64> for BenchmarkResult {
+    fn from_iter<T: IntoIterator<Item = u64>>(iter: T) -> Self {
+        let mut iter = iter.into_iter();
+        let first = iter.next().unwrap();
+        let mut min = first;
+        let mut max = first;
+        let mut sum = first;
+        let mut cnt = 1;
+        for x in iter {
+            min = min.min(x);
+            max = max.max(x);
+            sum += x;
+            cnt += 1;
+        }
+        BenchmarkResult {
+            min,
+            max,
+            avg: sum / cnt,
+        }
+    }
+}

nix/dev/shell.nix

@@ -92,7 +92,7 @@
     if [[ "$UID" != 0 ]]; then
       exec sudo "$0" "$@"
     fi
-    cp ${pkgs.nsjail}/bin/nsjail .nsjail
+    cp -a ${pkgs.nsjail}/bin/nsjail .nsjail
     chmod +s .nsjail
   '';
   scripts = pkgs.stdenv.mkDerivation {

src/api/environments.rs

@@ -1,12 +1,12 @@
-use std::{iter::Map, slice::Iter, sync::Arc};
+use std::sync::Arc;
 
 use fnct::key;
 use key_rwlock::KeyRwLock;
 use poem_ext::{response, responses::ErrorResponse, shield_mw::shield};
 use poem_openapi::{param::Path, OpenApi};
 use sandkasten_client::schemas::{
-    environments::{BaseResourceUsage, Environment, ListEnvironmentsResponse},
-    programs::{BuildRequest, ResourceUsage},
+    environments::{BaseResourceUsage, Environment, ListEnvironmentsResponse, RunResourceUsage},
+    programs::BuildRequest,
 };
 use tokio::sync::Semaphore;
 use uuid::Uuid;
@@ -15,7 +15,7 @@ use super::Tags;
 use crate::{
     config::Config,
     environments::{self, Environments},
-    program::{build_program, run_program},
+    program::{build::build_program, run::run_program},
     Cache,
 };
 
@@ -26,12 +26,15 @@ pub struct EnvironmentsApi {
     pub program_lock: Arc<KeyRwLock<Uuid>>,
     pub job_lock: Arc<KeyRwLock<Uuid>>,
     pub cache: Arc<Cache>,
-    pub bru_lock: Arc<KeyRwLock<String>>,
+    pub base_resource_usage_lock: Arc<KeyRwLock<String>>,
 }
 
 #[OpenApi(tag = "Tags::Environments")]
 impl EnvironmentsApi {
-    /// Return a list of all environments.
+    /// Return a map of all environments.
+    ///
+    /// The keys represent the environment ids and the values contain additional
+    /// information about the environments.
     #[oai(path = "/environments", method = "get")]
     async fn list_environments(&self) -> ListEnvironments::Response {
         ListEnvironments::ok(ListEnvironmentsResponse(
@@ -52,8 +55,12 @@ impl EnvironmentsApi {
         ))
     }
 
-    /// Return the base resource usage of an environment when running just a
-    /// very basic program.
+    /// Return the base resource usage of an environment.
+    ///
+    /// The base resource usage of an environment is measured by benchmarking a
+    /// very simple program in this environment that barely does anything. Note
+    /// that the compile step is run only once as recompiling the same program
+    /// again and again would take too much time in most cases.
     #[oai(
         path = "/environments/:name/resource_usage",
         method = "get",
@@ -64,7 +71,7 @@ impl EnvironmentsApi {
             return GetBaseResourceUsage::environment_not_found();
         };
 
-        let _guard = self.bru_lock.write(name.0.clone()).await;
+        let _guard = self.base_resource_usage_lock.write(name.0.clone()).await;
         let result = self
             .cache
             .cached_result(key!(&name.0), &[], None, || async {
@@ -100,6 +107,7 @@ response!(GetBaseResourceUsage = {
     EnvironmentNotFound(404, error),
 });
 
+/// Measure the base resource usage of a given environment.
 async fn get_base_resource_usage(
     config: Arc<Config>,
     environments: Arc<Environments>,
@@ -108,6 +116,7 @@ async fn get_base_resource_usage(
     environment_id: &str,
     environment: &environments::Environment,
 ) -> Result<BaseResourceUsage, ErrorResponse> {
+    // compile the program once
     let (build, _guard) = build_program(
         Arc::clone(&config),
         Arc::clone(&environments),
@@ -122,9 +131,10 @@ async fn get_base_resource_usage(
     )
     .await?;
 
-    let mut res = Vec::with_capacity(config.base_resource_usage_runs);
+    // run the program multiple times and collect the resource_usage measurements
+    let mut results = Vec::with_capacity(config.base_resource_usage_runs);
     for _ in 0..config.base_resource_usage_runs {
-        res.push(
+        results.push(
             run_program(
                 Arc::clone(&config),
                 build.program_id,
@@ -139,19 +149,9 @@ async fn get_base_resource_usage(
 
     Ok(BaseResourceUsage {
         build: build.compile_result.map(|x| x.resource_usage),
-        run_min: acc(&res, |x| x.min().unwrap()),
-        run_max: acc(&res, |x| x.max().unwrap()),
-        run_avg: acc(&res, |x| {
-            let n = x.len();
-            x.sum::<u64>() / n as u64
-        }),
+        run: RunResourceUsage {
+            time: results.iter().map(|x| x.time).collect(),
+            memory: results.iter().map(|x| x.memory).collect(),
+        },
     })
 }
-
-type Acc = fn(Map<Iter<ResourceUsage>, fn(&ResourceUsage) -> u64>) -> u64;
-fn acc(res: &[ResourceUsage], f: Acc) -> ResourceUsage {
-    ResourceUsage {
-        time: f(res.iter().map(|r| r.time)),
-        memory: f(res.iter().map(|r| r.memory)),
-    }
-}

src/api/mod.rs

@@ -33,7 +33,7 @@ pub fn get_api(
             job_lock: Arc::clone(&job_lock),
             config: Arc::clone(&config),
             cache,
-            bru_lock: Arc::new(KeyRwLock::new()),
+            base_resource_usage_lock: Arc::new(KeyRwLock::new()),
         },
         ProgramsApi {
             request_semaphore,

src/api/programs.rs

@@ -16,7 +16,10 @@ use super::Tags;
 use crate::{
     config::Config,
     environments::Environments,
-    program::{build_program, run_program, BuildProgramError, RunProgramError},
+    program::{
+        build::{build_program, BuildProgramError},
+        run::{run_program, RunProgramError},
+    },
 };
 
 pub struct ProgramsApi {
@@ -41,7 +44,9 @@ impl ProgramsApi {
         if !check_env_vars(&data.0.build.env_vars) || !check_env_vars(&data.0.run.env_vars) {
             return BuildRun::invalid_env_vars();
         }
+
         let _guard = self.request_semaphore.acquire().await?;
+
         let (
             BuildResult {
                 program_id,
@@ -101,7 +106,9 @@ impl ProgramsApi {
         if !check_env_vars(&data.0.env_vars) {
             return Build::invalid_env_vars();
         }
+
         let _guard = self.request_semaphore.acquire().await?;
+
         match build_program(
             Arc::clone(&self.config),
             Arc::clone(&self.environments),
@@ -133,7 +140,9 @@ impl ProgramsApi {
         if !check_env_vars(&data.0.env_vars) {
             return Run::invalid_env_vars();
         }
+
         let _guard = self.request_semaphore.acquire().await?;
+
         match run_program(
             Arc::clone(&self.config),
             program_id.0,

src/config.rs

@@ -1,52 +1,86 @@
 use std::{env, path::PathBuf};
 
+use anyhow::Context;
 use config::{Environment, File};
 use sandkasten_client::schemas::programs::Limits;
 use serde::{Deserialize, Deserializer};
+use tracing::info;
 use url::Url;
 
 pub fn load() -> Result<Config, anyhow::Error> {
+    let path = env::var("CONFIG_PATH").unwrap_or("config.toml".to_owned());
+    info!("Loading config from {path}");
     let conf: Config = config::Config::builder()
-        .add_source(File::with_name(
-            &env::var("CONFIG_PATH").unwrap_or("config.toml".to_owned()),
-        ))
+        .add_source(File::with_name(&path))
         .add_source(Environment::default().separator("__"))
-        .build()?
-        .try_deserialize()?;
+        .build()
+        .context("Failed to load config")?
+        .try_deserialize()
+        .context("Failed to parse config")?;
 
     Ok(Config {
-        nsjail_path: conf.nsjail_path.canonicalize()?,
-        time_path: conf.time_path.canonicalize()?,
+        nsjail_path: conf.nsjail_path.canonicalize().with_context(|| {
+            format!(
+                "Failed to resolve `nsjail_path` {}",
+                conf.nsjail_path.display()
+            )
+        })?,
+        time_path: conf.time_path.canonicalize().with_context(|| {
+            format!("Failed to resolve `time_path` {}", conf.time_path.display())
+        })?,
         ..conf
     })
 }
 
 #[derive(Debug, Deserialize)]
 pub struct Config {
+    /// The host to listen on.
     pub host: String,
+    /// The port to listen on.
     pub port: u16,
+    /// The path prefix added by a reverse proxy. Set to `"/"` if you don't have
+    /// a reverse proxy.
     pub server: String,
 
+    /// The url of the redis server (see https://docs.rs/redis/latest/redis/#connection-parameters).
     pub redis_url: Url,
-    pub cache_ttl: u64, // in seconds
+    /// The default time to live for cache entries in seconds.
+    pub cache_ttl: u64,
 
+    /// The directory where programs are stored.
     pub programs_dir: PathBuf,
+    /// The directory where files for jobs are stored.
     pub jobs_dir: PathBuf,
 
-    pub program_ttl: u64,             // in seconds
-    pub prune_programs_interval: u64, // in seconds
+    /// The time to live for programs in seconds.
+    pub program_ttl: u64,
+    /// The number of seconds to wait between deleting old programs.
+    pub prune_programs_interval: u64,
 
+    /// The maximum number of jobs that can be run at the same time.
     pub max_concurrent_jobs: usize,
 
+    /// The maximum allowed limits for compile steps.
     pub compile_limits: Limits,
+    /// The maximum allowed limits for run steps.
     pub run_limits: Limits,
 
+    /// The number of times the program is run when measuring the base resource
+    /// usage of an environment.
     pub base_resource_usage_runs: usize,
 
+    /// Whether to use cgroup to set resource limits where possible. It is
+    /// strongly recommended to set this to true in production environments!
     pub use_cgroup: bool,
+    /// The path to the nsjail binary. This binary must have the setuid bit set
+    /// and it must be owned by root OR sandkasten itself must be run as root.
     pub nsjail_path: PathBuf,
+    /// The path to the time binary.
     pub time_path: PathBuf,
 
+    /// A list of paths to load environments from. If specified as an
+    /// environment variable, separate the paths using a `:`
+    /// (e.g. `"/foo/path1:/bar/path2:/baz/path3"`).
     #[serde(deserialize_with = "path")]
     pub environments_path: Vec<PathBuf>,
 }