From dfcdbf432b98e096678cc7a9258a583bb719f104 Mon Sep 17 00:00:00 2001 From: nicomiguelino Date: Mon, 31 Aug 2026 09:43:50 -0700 Subject: [PATCH 1/5] Deprecate reading Edge App id from manifest in favor of EDGE_APP_ID env var get_app_id now checks the EDGE_APP_ID environment variable first and uses it if present (trimmed, ignoring whitespace-only values). If unset, it falls back to screenly.yml's id field like before, but now prints a deprecation warning pointing at the env var. --- docs/EdgeApps.md | 2 ++ src/commands/edge_app/app.rs | 63 +++++++++++++++++++++++++++++++++++- 2 files changed, 64 insertions(+), 1 deletion(-) diff --git a/docs/EdgeApps.md b/docs/EdgeApps.md index 0202143..96e20b3 100644 --- a/docs/EdgeApps.md +++ b/docs/EdgeApps.md @@ -294,6 +294,8 @@ The `syntax` field specifies the version of the manifest file. The current versi The `id` field is a unique identifier for the Edge App. This ID is generated by the system and is used to identify the Edge App across the platform. +> **Deprecated:** Reading the `id` from `screenly.yml` is deprecated. Set the `EDGE_APP_ID` environment variable instead — for example, via a repo secret in CI or a local `.env` file loaded into your shell. When `EDGE_APP_ID` is set, it takes priority over the manifest's `id` field. The CLI still falls back to reading `id` from `screenly.yml` for now, but prints a deprecation warning each time it does. + #### Entrypoint The `entrypoint` field specifies the entry point for the Edge App. It is optional and defaults to the file type. The `entrypoint` field contains the following subfields: diff --git a/src/commands/edge_app/app.rs b/src/commands/edge_app/app.rs index d6f7354..2c9ae67 100644 --- a/src/commands/edge_app/app.rs +++ b/src/commands/edge_app/app.rs @@ -29,6 +29,8 @@ use crate::commands::edge_app::utils::{ use crate::commands::edge_app::EdgeAppCommand; use crate::commands::{CommandError, EdgeApps}; +const EDGE_APP_ID_ENV: &str = "EDGE_APP_ID"; + pub const INJECT_JS_FILE_NAME: &str = "screenly_inject.js"; // Edge apps commands @@ -699,9 +701,21 @@ impl EdgeAppCommand { } pub fn get_app_id(&self, path: Option) -> Result { + if let Ok(id) = std::env::var(EDGE_APP_ID_ENV) { + let id = id.trim(); + if !id.is_empty() { + return Ok(id.to_string()); + } + } + let edge_app_manifest = EdgeAppManifest::new(&transform_edge_app_path_to_manifest(&path)?)?; match edge_app_manifest.id { - Some(id) if !id.is_empty() => Ok(id), + Some(id) if !id.is_empty() => { + eprintln!( + "Warning: reading the Edge App id from the manifest file is deprecated, set the {EDGE_APP_ID_ENV} environment variable instead." + ); + Ok(id) + } _ => Err(CommandError::MissingAppId), } } @@ -1678,6 +1692,53 @@ mod tests { assert!(command.delete_app("test-id").is_ok()); } + #[test] + fn test_get_app_id_should_prefer_env_var_over_manifest() { + let (_temp_dir, command, _mock_server, _manifest, _instance_manifest) = + prepare_edge_apps_test(true, false); + + env::set_var(EDGE_APP_ID_ENV, "01ENVOVERRIDEXXXXXXXXXXXXX"); + let app_id = command.get_app_id(Some(_temp_dir.path().to_str().unwrap().to_string())); + env::remove_var(EDGE_APP_ID_ENV); + + assert_eq!(app_id.unwrap(), "01ENVOVERRIDEXXXXXXXXXXXXX"); + } + + #[test] + fn test_get_app_id_should_fall_back_to_manifest_when_env_var_is_not_set() { + let (temp_dir, command, _mock_server, manifest, _instance_manifest) = + prepare_edge_apps_test(true, false); + + env::remove_var(EDGE_APP_ID_ENV); + let app_id = command.get_app_id(Some(temp_dir.path().to_str().unwrap().to_string())); + + assert_eq!(app_id.unwrap(), manifest.unwrap().id.unwrap()); + } + + #[test] + fn test_get_app_id_should_trim_whitespace_from_env_var() { + let (temp_dir, command, _mock_server, _manifest, _instance_manifest) = + prepare_edge_apps_test(true, false); + + env::set_var(EDGE_APP_ID_ENV, " 01ENVOVERRIDEXXXXXXXXXXXXX \n"); + let app_id = command.get_app_id(Some(temp_dir.path().to_str().unwrap().to_string())); + env::remove_var(EDGE_APP_ID_ENV); + + assert_eq!(app_id.unwrap(), "01ENVOVERRIDEXXXXXXXXXXXXX"); + } + + #[test] + fn test_get_app_id_should_fall_back_to_manifest_when_env_var_is_whitespace_only() { + let (temp_dir, command, _mock_server, manifest, _instance_manifest) = + prepare_edge_apps_test(true, false); + + env::set_var(EDGE_APP_ID_ENV, " "); + let app_id = command.get_app_id(Some(temp_dir.path().to_str().unwrap().to_string())); + env::remove_var(EDGE_APP_ID_ENV); + + assert_eq!(app_id.unwrap(), manifest.unwrap().id.unwrap()); + } + #[test] fn test_clear_app_id_should_remove_app_id_from_manifest() { let (temp_dir, command, _mock_server, _manifest, _instance_manifest) = From 0d2407421aa1c4852c6afeac736c3c42229dec70 Mon Sep 17 00:00:00 2001 From: nicomiguelino Date: Mon, 31 Aug 2026 09:51:45 -0700 Subject: [PATCH 2/5] Use envtestkit for EDGE_APP_ID tests instead of raw std::env env::set_var/remove_var mutated a process-wide env var without synchronization or automatic restoration, risking flakiness when tests run in parallel. Switch to the lock_test() + set_env() RAII pattern already used in authentication.rs, which serializes env-mutating tests and restores the prior value on drop, even on panic. --- src/commands/edge_app/app.rs | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/src/commands/edge_app/app.rs b/src/commands/edge_app/app.rs index 2c9ae67..c5000dd 100644 --- a/src/commands/edge_app/app.rs +++ b/src/commands/edge_app/app.rs @@ -723,8 +723,10 @@ impl EdgeAppCommand { #[cfg(test)] mod tests { - use std::env; + use std::ffi::OsString; + use envtestkit::lock::lock_test; + use envtestkit::set_env; use httpmock::Method::{DELETE, GET, PATCH, POST}; use tempfile::tempdir; @@ -1694,12 +1696,15 @@ mod tests { #[test] fn test_get_app_id_should_prefer_env_var_over_manifest() { - let (_temp_dir, command, _mock_server, _manifest, _instance_manifest) = + let (temp_dir, command, _mock_server, _manifest, _instance_manifest) = prepare_edge_apps_test(true, false); - env::set_var(EDGE_APP_ID_ENV, "01ENVOVERRIDEXXXXXXXXXXXXX"); - let app_id = command.get_app_id(Some(_temp_dir.path().to_str().unwrap().to_string())); - env::remove_var(EDGE_APP_ID_ENV); + let _lock = lock_test(); + let _env = set_env( + OsString::from(EDGE_APP_ID_ENV), + "01ENVOVERRIDEXXXXXXXXXXXXX", + ); + let app_id = command.get_app_id(Some(temp_dir.path().to_str().unwrap().to_string())); assert_eq!(app_id.unwrap(), "01ENVOVERRIDEXXXXXXXXXXXXX"); } @@ -1709,7 +1714,7 @@ mod tests { let (temp_dir, command, _mock_server, manifest, _instance_manifest) = prepare_edge_apps_test(true, false); - env::remove_var(EDGE_APP_ID_ENV); + let _lock = lock_test(); let app_id = command.get_app_id(Some(temp_dir.path().to_str().unwrap().to_string())); assert_eq!(app_id.unwrap(), manifest.unwrap().id.unwrap()); @@ -1720,9 +1725,12 @@ mod tests { let (temp_dir, command, _mock_server, _manifest, _instance_manifest) = prepare_edge_apps_test(true, false); - env::set_var(EDGE_APP_ID_ENV, " 01ENVOVERRIDEXXXXXXXXXXXXX \n"); + let _lock = lock_test(); + let _env = set_env( + OsString::from(EDGE_APP_ID_ENV), + " 01ENVOVERRIDEXXXXXXXXXXXXX \n", + ); let app_id = command.get_app_id(Some(temp_dir.path().to_str().unwrap().to_string())); - env::remove_var(EDGE_APP_ID_ENV); assert_eq!(app_id.unwrap(), "01ENVOVERRIDEXXXXXXXXXXXXX"); } @@ -1732,9 +1740,9 @@ mod tests { let (temp_dir, command, _mock_server, manifest, _instance_manifest) = prepare_edge_apps_test(true, false); - env::set_var(EDGE_APP_ID_ENV, " "); + let _lock = lock_test(); + let _env = set_env(OsString::from(EDGE_APP_ID_ENV), " "); let app_id = command.get_app_id(Some(temp_dir.path().to_str().unwrap().to_string())); - env::remove_var(EDGE_APP_ID_ENV); assert_eq!(app_id.unwrap(), manifest.unwrap().id.unwrap()); } From 2fd3efe377e2bdd3cb078545dfb54136e3dfc783 Mon Sep 17 00:00:00 2001 From: nicomiguelino Date: Mon, 31 Aug 2026 10:09:27 -0700 Subject: [PATCH 3/5] Isolate get_app_id fallback test from ambient EDGE_APP_ID env var The fallback test assumed EDGE_APP_ID wasn't already set in the process environment, so it would fail on any machine or CI runner with it exported. Explicitly set it to an empty string via envtestkit (treated as unset by edge_app_id_from_env, restored on drop) so the test is isolated from ambient state. --- src/commands/edge_app/app.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/commands/edge_app/app.rs b/src/commands/edge_app/app.rs index c5000dd..ceada28 100644 --- a/src/commands/edge_app/app.rs +++ b/src/commands/edge_app/app.rs @@ -1715,6 +1715,7 @@ mod tests { prepare_edge_apps_test(true, false); let _lock = lock_test(); + let _env = set_env(OsString::from(EDGE_APP_ID_ENV), ""); let app_id = command.get_app_id(Some(temp_dir.path().to_str().unwrap().to_string())); assert_eq!(app_id.unwrap(), manifest.unwrap().id.unwrap()); From 4fb0538824f40a384647b631393d49a5bf4e618b Mon Sep 17 00:00:00 2001 From: nicomiguelino Date: Tue, 1 Sep 2026 17:33:52 -0700 Subject: [PATCH 4/5] Address review feedback on EDGE_APP_ID precedence Fixes deploy/delete targeting the wrong app when EDGE_APP_ID is set, extends the same precedence to create/create_in_place, restores --path validation, deduplicates the deprecation warning, and removes env-var mutation from tests to stop suite flakiness. --- docs/EdgeApps.md | 2 + src/cli.rs | 22 +++--- src/commands/edge_app/app.rs | 125 ++++++++++++++++++----------------- src/mcp/tools/edge_app.rs | 4 +- 4 files changed, 82 insertions(+), 71 deletions(-) diff --git a/docs/EdgeApps.md b/docs/EdgeApps.md index 96e20b3..8b2e68e 100644 --- a/docs/EdgeApps.md +++ b/docs/EdgeApps.md @@ -295,6 +295,8 @@ The `syntax` field specifies the version of the manifest file. The current versi The `id` field is a unique identifier for the Edge App. This ID is generated by the system and is used to identify the Edge App across the platform. > **Deprecated:** Reading the `id` from `screenly.yml` is deprecated. Set the `EDGE_APP_ID` environment variable instead — for example, via a repo secret in CI or a local `.env` file loaded into your shell. When `EDGE_APP_ID` is set, it takes priority over the manifest's `id` field. The CLI still falls back to reading `id` from `screenly.yml` for now, but prints a deprecation warning each time it does. +> +> `screenly edge-app create` and `screenly edge-app create --in-place` also respect `EDGE_APP_ID`: if it's set, the CLI verifies that Edge App exists and uses it instead of creating a new one, without writing anything into `screenly.yml`. #### Entrypoint diff --git a/src/cli.rs b/src/cli.rs index 1509aa2..2aa3667 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -11,6 +11,7 @@ use thiserror::Error; use crate::authentication::{verify_and_store_token, Authentication, AuthenticationError, Config}; use crate::commands; +use crate::commands::edge_app::app::app_id_override; use crate::commands::edge_app::instance_manifest::InstanceManifest; use crate::commands::edge_app::manifest::EdgeAppManifest; use crate::commands::edge_app::server::MOCK_DATA_FILENAME; @@ -923,7 +924,7 @@ pub fn handle_cli_edge_app_command(command: &EdgeAppCommands, output: OutputForm EdgeAppCommands::Deploy { path, delete_missing_settings, - } => match edge_app_command.deploy(path.clone(), *delete_missing_settings) { + } => match edge_app_command.deploy(None, path.clone(), *delete_missing_settings) { Ok(revision) => { println!("Edge App successfully deployed. Revision: {revision}."); } @@ -986,15 +987,18 @@ pub fn handle_cli_edge_app_command(command: &EdgeAppCommands, output: OutputForm } }; - // If the user didn't specify an app id, we need to clear it from the manifest - match edge_app_command.clear_app_id(manifest_path.as_path()) { - Ok(()) => { - println!("App id cleared from manifest."); - } - Err(e) => { - error!("Error occurred while clearing manifest: {e}"); - std::process::exit(1); + if app_id_override().is_none() { + match edge_app_command.clear_app_id(manifest_path.as_path()) { + Ok(()) => { + println!("App id cleared from manifest."); + } + Err(e) => { + error!("Error occurred while clearing manifest: {e}"); + std::process::exit(1); + } } + } else { + println!("Skipping manifest cleanup: the deleted app's id came from the EDGE_APP_ID environment variable, not the manifest."); } std::process::exit(0); } diff --git a/src/commands/edge_app/app.rs b/src/commands/edge_app/app.rs index ceada28..7e15493 100644 --- a/src/commands/edge_app/app.rs +++ b/src/commands/edge_app/app.rs @@ -2,7 +2,7 @@ use std::collections::HashMap; use std::fs::File; use std::io::Write; use std::path::{Path, PathBuf}; -use std::sync::{Arc, Mutex}; +use std::sync::{Arc, Mutex, Once}; use std::time::{Duration, Instant}; use std::{fs, io, str, thread}; @@ -29,10 +29,27 @@ use crate::commands::edge_app::utils::{ use crate::commands::edge_app::EdgeAppCommand; use crate::commands::{CommandError, EdgeApps}; -const EDGE_APP_ID_ENV: &str = "EDGE_APP_ID"; +pub const EDGE_APP_ID_ENV: &str = "EDGE_APP_ID"; pub const INJECT_JS_FILE_NAME: &str = "screenly_inject.js"; +static DEPRECATED_MANIFEST_ID_WARNING: Once = Once::new(); + +pub fn app_id_override() -> Option { + normalize_app_id_override(std::env::var(EDGE_APP_ID_ENV).ok()) +} + +fn normalize_app_id_override(raw: Option) -> Option { + let id = raw?; + let trimmed = id.trim(); + + if trimmed.is_empty() { + return None; + } + + Some(trimmed.to_string()) +} + // Edge apps commands impl EdgeAppCommand { pub fn create( @@ -74,11 +91,17 @@ impl EdgeAppCommand { }), }; - let app_id = self.api.create_app(name.to_string())?; + let app_id = match app_id_override() { + Some(id) => { + self.api.get_app(&id)?; + None + } + None => Some(self.api.create_app(name.to_string())?), + }; let manifest = EdgeAppManifest { syntax: MANIFEST_VERSION.to_owned(), - id: Some(app_id), + id: app_id, entrypoint: entrypoint_value, settings: vec![ Setting { @@ -184,6 +207,11 @@ impl EdgeAppCommand { ))); } + if let Some(id) = app_id_override() { + self.api.get_app(&id)?; + return Ok(()); + } + let data = fs::read_to_string(path)?; let mut manifest: EdgeAppManifest = serde_yaml::from_str(&data)?; @@ -206,6 +234,7 @@ impl EdgeAppCommand { pub fn deploy( self, + app_id: Option, path: Option, delete_missing_settings: Option, ) -> Result { @@ -214,9 +243,12 @@ impl EdgeAppCommand { EdgeAppManifest::ensure_manifest_is_valid(&manifest_path)?; let manifest = EdgeAppManifest::new(&manifest_path)?; - let actual_app_id = match self.get_app_id(path.clone()) { - Ok(id) => id, - Err(_) => return Err(CommandError::MissingAppId), + let actual_app_id = match app_id { + Some(id) => id, + None => match self.get_app_id(path.clone()) { + Ok(id) => id, + Err(_) => return Err(CommandError::MissingAppId), + }, }; let version_metadata_changed = @@ -701,19 +733,20 @@ impl EdgeAppCommand { } pub fn get_app_id(&self, path: Option) -> Result { - if let Ok(id) = std::env::var(EDGE_APP_ID_ENV) { - let id = id.trim(); - if !id.is_empty() { - return Ok(id.to_string()); - } + let manifest_path = transform_edge_app_path_to_manifest(&path)?; + + if let Some(id) = app_id_override() { + return Ok(id); } - let edge_app_manifest = EdgeAppManifest::new(&transform_edge_app_path_to_manifest(&path)?)?; + let edge_app_manifest = EdgeAppManifest::new(&manifest_path)?; match edge_app_manifest.id { Some(id) if !id.is_empty() => { - eprintln!( - "Warning: reading the Edge App id from the manifest file is deprecated, set the {EDGE_APP_ID_ENV} environment variable instead." - ); + DEPRECATED_MANIFEST_ID_WARNING.call_once(|| { + eprintln!( + "Warning: reading the Edge App id from the manifest file is deprecated, set the {EDGE_APP_ID_ENV} environment variable instead." + ); + }); Ok(id) } _ => Err(CommandError::MissingAppId), @@ -723,10 +756,6 @@ impl EdgeAppCommand { #[cfg(test)] mod tests { - use std::ffi::OsString; - - use envtestkit::lock::lock_test; - use envtestkit::set_env; use httpmock::Method::{DELETE, GET, PATCH, POST}; use tempfile::tempdir; @@ -1366,6 +1395,7 @@ mod tests { write!(file, "test").unwrap(); let result = command.deploy( + None, Some(temp_dir.path().to_str().unwrap().to_string()), Some(true), ); @@ -1695,57 +1725,29 @@ mod tests { } #[test] - fn test_get_app_id_should_prefer_env_var_over_manifest() { - let (temp_dir, command, _mock_server, _manifest, _instance_manifest) = - prepare_edge_apps_test(true, false); - - let _lock = lock_test(); - let _env = set_env( - OsString::from(EDGE_APP_ID_ENV), - "01ENVOVERRIDEXXXXXXXXXXXXX", + fn test_normalize_app_id_override_should_return_the_trimmed_value_when_present() { + assert_eq!( + normalize_app_id_override(Some("01ENVOVERRIDEXXXXXXXXXXXXX".to_string())), + Some("01ENVOVERRIDEXXXXXXXXXXXXX".to_string()) ); - let app_id = command.get_app_id(Some(temp_dir.path().to_str().unwrap().to_string())); - - assert_eq!(app_id.unwrap(), "01ENVOVERRIDEXXXXXXXXXXXXX"); } #[test] - fn test_get_app_id_should_fall_back_to_manifest_when_env_var_is_not_set() { - let (temp_dir, command, _mock_server, manifest, _instance_manifest) = - prepare_edge_apps_test(true, false); - - let _lock = lock_test(); - let _env = set_env(OsString::from(EDGE_APP_ID_ENV), ""); - let app_id = command.get_app_id(Some(temp_dir.path().to_str().unwrap().to_string())); - - assert_eq!(app_id.unwrap(), manifest.unwrap().id.unwrap()); + fn test_normalize_app_id_override_should_trim_whitespace() { + assert_eq!( + normalize_app_id_override(Some(" 01ENVOVERRIDEXXXXXXXXXXXXX \n".to_string())), + Some("01ENVOVERRIDEXXXXXXXXXXXXX".to_string()) + ); } #[test] - fn test_get_app_id_should_trim_whitespace_from_env_var() { - let (temp_dir, command, _mock_server, _manifest, _instance_manifest) = - prepare_edge_apps_test(true, false); - - let _lock = lock_test(); - let _env = set_env( - OsString::from(EDGE_APP_ID_ENV), - " 01ENVOVERRIDEXXXXXXXXXXXXX \n", - ); - let app_id = command.get_app_id(Some(temp_dir.path().to_str().unwrap().to_string())); - - assert_eq!(app_id.unwrap(), "01ENVOVERRIDEXXXXXXXXXXXXX"); + fn test_normalize_app_id_override_should_treat_whitespace_only_as_none() { + assert_eq!(normalize_app_id_override(Some(" ".to_string())), None); } #[test] - fn test_get_app_id_should_fall_back_to_manifest_when_env_var_is_whitespace_only() { - let (temp_dir, command, _mock_server, manifest, _instance_manifest) = - prepare_edge_apps_test(true, false); - - let _lock = lock_test(); - let _env = set_env(OsString::from(EDGE_APP_ID_ENV), " "); - let app_id = command.get_app_id(Some(temp_dir.path().to_str().unwrap().to_string())); - - assert_eq!(app_id.unwrap(), manifest.unwrap().id.unwrap()); + fn test_normalize_app_id_override_should_treat_absent_value_as_none() { + assert_eq!(normalize_app_id_override(None), None); } #[test] @@ -1815,6 +1817,7 @@ mod tests { write!(file, "test").unwrap(); let result = command.deploy( + None, Some(temp_dir.path().to_str().unwrap().to_string()), Some(true), ); diff --git a/src/mcp/tools/edge_app.rs b/src/mcp/tools/edge_app.rs index 7858d32..ed83373 100644 --- a/src/mcp/tools/edge_app.rs +++ b/src/mcp/tools/edge_app.rs @@ -9,6 +9,7 @@ use sha2::{Digest, Sha256}; use crate::authentication::Authentication; use crate::commands; +use crate::commands::edge_app::app::app_id_override; use crate::commands::edge_app::manifest::{ EdgeAppManifest, Entrypoint, EntrypointType, MANIFEST_VERSION, }; @@ -279,10 +280,11 @@ impl EdgeAppTools { let app_id = EdgeAppManifest::new(&manifest_path) .map_err(|e| format!("Failed to read Edge App id: {}", e))? .id + .or_else(app_id_override) .ok_or_else(|| "Edge App id missing after create".to_string())?; let revision = command - .deploy(Some(path), Some(false)) + .deploy(Some(app_id.clone()), Some(path), Some(false)) .map_err(|e| format!("Failed to deploy Edge App: {}", e))?; // Create/deploy already happened. Instance + local memory must not hide app_id. From 4486fa4f56cb92a26fdaabfe35ed357e2ec72153 Mon Sep 17 00:00:00 2001 From: nicomiguelino Date: Wed, 2 Sep 2026 16:16:17 -0700 Subject: [PATCH 5/5] Fix version payload losing app_id when EDGE_APP_ID is used create_version now takes the resolved app id explicitly instead of reading it from the manifest, which was empty once EDGE_APP_ID replaces the manifest id. Threads the same explicit id through update_entrypoint_value/set_setting. Also clarifies docs on create/create_in_place's verify-and-reuse behavior. --- docs/EdgeApps.md | 2 +- src/cli.rs | 7 +- src/commands/edge_app/app.rs | 371 +++++++++++++++++++++++++++++- src/commands/edge_app/instance.rs | 2 +- src/commands/edge_app/setting.rs | 18 +- 5 files changed, 382 insertions(+), 18 deletions(-) diff --git a/docs/EdgeApps.md b/docs/EdgeApps.md index 8b2e68e..d296f38 100644 --- a/docs/EdgeApps.md +++ b/docs/EdgeApps.md @@ -296,7 +296,7 @@ The `id` field is a unique identifier for the Edge App. This ID is generated by > **Deprecated:** Reading the `id` from `screenly.yml` is deprecated. Set the `EDGE_APP_ID` environment variable instead — for example, via a repo secret in CI or a local `.env` file loaded into your shell. When `EDGE_APP_ID` is set, it takes priority over the manifest's `id` field. The CLI still falls back to reading `id` from `screenly.yml` for now, but prints a deprecation warning each time it does. > -> `screenly edge-app create` and `screenly edge-app create --in-place` also respect `EDGE_APP_ID`: if it's set, the CLI verifies that Edge App exists and uses it instead of creating a new one, without writing anything into `screenly.yml`. +> `screenly edge-app create` and `screenly edge-app create --in-place` also respect `EDGE_APP_ID`: if it's set, the CLI verifies that Edge App exists and uses it instead of creating a new one, without writing anything into `screenly.yml`. This means any name you pass to `create` is ignored for the purpose of choosing which app to target: the deploy that follows always republishes over the app named by `EDGE_APP_ID`, never a new one. If you're publishing through a tool that manages `EDGE_APP_ID` on your behalf (for example, an integration that remembers an id per name), a "new" name no longer produces a new Edge App while the environment variable is set; it republishes over the existing one and updates its own bookkeeping to match. #### Entrypoint diff --git a/src/cli.rs b/src/cli.rs index 2aa3667..a341413 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -941,7 +941,12 @@ pub fn handle_cli_edge_app_command(command: &EdgeAppCommands, output: OutputForm ); } EdgeAppSettingsCommands::Set { setting_pair, path } => { - match edge_app_command.set_setting(path.clone(), &setting_pair.0, &setting_pair.1) { + match edge_app_command.set_setting( + None, + path.clone(), + &setting_pair.0, + &setting_pair.1, + ) { Ok(()) => { println!("Edge App setting successfully set."); } diff --git a/src/commands/edge_app/app.rs b/src/commands/edge_app/app.rs index 7e15493..2f1d6e7 100644 --- a/src/commands/edge_app/app.rs +++ b/src/commands/edge_app/app.rs @@ -299,7 +299,7 @@ impl EdgeAppCommand { changed_settings, )?; - self.update_entrypoint_value(path.clone())?; + self.update_entrypoint_value(Some(actual_app_id.clone()), path.clone())?; let file_tree = generate_file_tree(&local_files, edge_app_dir); @@ -317,8 +317,11 @@ impl EdgeAppCommand { || version_metadata_changed; let final_revision = if needs_new_version { - let revision = - self.create_version(&manifest, generate_file_tree(&local_files, edge_app_dir))?; + let revision = self.create_version( + &actual_app_id, + &manifest, + generate_file_tree(&local_files, edge_app_dir), + )?; self.upload_changed_files(edge_app_dir, &actual_app_id, revision, &changed_files)?; debug!("Files uploaded"); @@ -397,7 +400,11 @@ impl EdgeAppCommand { Ok(()) } - pub fn update_entrypoint_value(&self, path: Option) -> Result<(), CommandError> { + pub fn update_entrypoint_value( + &self, + app_id: Option, + path: Option, + ) -> Result<(), CommandError> { let manifest = EdgeAppManifest::new(&transform_edge_app_path_to_manifest(&path)?)?; let setting_key = "screenly_entrypoint"; @@ -408,7 +415,7 @@ impl EdgeAppCommand { Some(ref uri) => uri.clone(), None => "".to_owned(), }; - self.set_setting(path, setting_key, &setting_value)?; + self.set_setting(app_id.clone(), path, setting_key, &setting_value)?; } EntrypointType::RemoteLocal => { let instance_manifest = InstanceManifest::new( @@ -418,7 +425,7 @@ impl EdgeAppCommand { Some(ref uri) => uri.clone(), None => "".to_owned(), }; - self.set_setting(path, setting_key, &setting_value)?; + self.set_setting(app_id.clone(), path, setting_key, &setting_value)?; } _ => {} } @@ -510,10 +517,12 @@ impl EdgeAppCommand { fn create_version( &self, + app_id: &str, manifest: &EdgeAppManifest, file_tree: HashMap, ) -> Result { let mut json = EdgeAppManifest::prepare_payload(manifest); + json.insert("app_id", json!(app_id)); json.insert("file_tree", json!(file_tree)); self.api.create_version(json) @@ -1419,6 +1428,344 @@ mod tests { assert!(result.is_ok()); } + #[test] + fn test_deploy_with_explicit_app_id_and_no_manifest_id_should_send_correct_requests() { + let (temp_dir, command, mock_server, _manifest, _instance_manifest) = + prepare_edge_apps_test(false, false); + + let mut manifest = create_edge_app_manifest_for_test(vec![ + Setting { + name: "asetting".to_string(), + type_: SettingType::String, + title: Some("atitle".to_string()), + optional: false, + default_value: Some("".to_string()), + is_global: false, + help_text: "help text".to_string(), + }, + Setting { + name: "nsetting".to_string(), + type_: SettingType::String, + title: Some("ntitle".to_string()), + optional: false, + default_value: Some("".to_string()), + is_global: false, + help_text: "help text".to_string(), + }, + ]); + + manifest.id = None; + manifest.user_version = None; + manifest.author = None; + manifest.entrypoint = None; + + let last_versions_mock = mock_server.mock(|when, then| { + when.method(GET) + .path("/v4.1/edge-apps/versions") + .header("Authorization", "Token token") + .header( + "user-agent", + format!("screenly-cli {}", env!("CARGO_PKG_VERSION")), + ) + .query_param( + "select", + "user_version,description,icon,author,homepage_url,categories,revision,ready_signal", + ) + .query_param("app_id", "eq.01H2QZ6Z8WXWNDC0KQ198XCZEW") + .query_param("order", "revision.desc") + .query_param("limit", "1"); + then.status(200).json_body(json!([ + { + "user_version": "1", + "description": "desc", + "icon": "icon", + "author": "author", + "homepage_url": "homepage_url", + "categories": [], + "ready_signal": false, + "revision": 7, + } + ])); + }); + + let assets_mock = mock_server.mock(|when, then| { + when.method(GET) + .path("/v4/assets") + .header("Authorization", "Token token") + .header( + "user-agent", + format!("screenly-cli {}", env!("CARGO_PKG_VERSION")), + ) + .query_param("select", "signature") + .query_param("app_id", "eq.01H2QZ6Z8WXWNDC0KQ198XCZEW") + .query_param("app_revision", "eq.7") + .query_param("type", "eq.edge-app-file"); + then.status(200).json_body(json!([{"signature": "sig"}])); + }); + + let file_tree_from_version_mock = mock_server.mock(|when, then| { + when.method(GET) + .path("/v4/edge-apps/versions") + .header("Authorization", "Token token") + .header( + "user-agent", + format!("screenly-cli {}", env!("CARGO_PKG_VERSION")), + ) + .query_param("app_id", "eq.01H2QZ6Z8WXWNDC0KQ198XCZEW") + .query_param("revision", "eq.7") + .query_param("select", "file_tree"); + then.status(200).json_body(json!([{"index.html": "sig"}])); + }); + + let settings_mock = mock_server.mock(|when, then| { + when.method(GET) + .path("/v4.1/edge-apps/settings") + .header("Authorization", "Token token") + .header( + "user-agent", + format!("screenly-cli {}", env!("CARGO_PKG_VERSION")), + ) + .query_param("app_id", "eq.01H2QZ6Z8WXWNDC0KQ198XCZEW") + .query_param("select", "name,type,default_value,optional,title,help_text") + .query_param("order", "name.asc"); + then.status(200).json_body(json!([{ + "name": "nsetting".to_string(), + "type": SettingType::String, + "default_value": "5".to_string(), + "title": "ntitle".to_string(), + "optional": true, + "help_text": "For how long to display the map overlay every time the rover has moved to a new position.".to_string(), + "is_global": false, + }, { + "name": "isetting".to_string(), + "type": SettingType::String, + "default_value": "5".to_string(), + "title": null, + "optional": true, + "help_text": "Some text".to_string(), + "is_global": false, + }])); + }); + + let create_version_mock = mock_server.mock(|when, then| { + when.method(POST) + .path("/v4/edge-apps/versions") + .header("Authorization", "Token token") + .header( + "user-agent", + format!("screenly-cli {}", env!("CARGO_PKG_VERSION")), + ) + .json_body(json!({ + "app_id": "01H2QZ6Z8WXWNDC0KQ198XCZEW", + "description": "asdf", + "icon": "asdf", + "homepage_url": "asdfasdf", + "categories": ["Utilities", "Dashboards"], + "file_tree": { + "index.html": "0a209f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08122086cebd0c365d241e32d5b0972c07aae3a8d6499c2a9471aa85943a35577200021a180a14a94a8fe5ccb19ba61c4c0873d391e987982fbbd31000" + }, + "ready_signal": false, + })); + then.status(201).json_body(json!([{"revision": 8}])); + }); + + let settings_mock_create = mock_server.mock(|when, then| { + when.method(POST) + .path("/v4.1/edge-apps/settings") + .header("Authorization", "Token token") + .header( + "user-agent", + format!("screenly-cli {}", env!("CARGO_PKG_VERSION")), + ) + .json_body(json!({ + "name": "asetting", + "app_id": "01H2QZ6Z8WXWNDC0KQ198XCZEW", + "type": "string", + "default_value": "", + "title": "atitle", + "optional": false, + "help_text": { + "schema_version": 1, + "properties": { + "help_text": "help text", + "display_order": 0, + }, + }, + })); + then.status(201).json_body(json!( + [{ + "name": "asetting", + "app_id": "01H2QZ6Z8WXWNDC0KQ198XCZEW", + "type": "string", + "default_value": "", + "title": "atitle", + "optional": false, + "help_text": "help text", + }])); + }); + + let settings_mock_patch = mock_server.mock(|when, then| { + when.method(PATCH) + .path("/v4.1/edge-apps/settings") + .header("Authorization", "Token token") + .header( + "user-agent", + format!("screenly-cli {}", env!("CARGO_PKG_VERSION")), + ) + .query_param("app_id", "eq.01H2QZ6Z8WXWNDC0KQ198XCZEW") + .query_param("name", "eq.nsetting") + .json_body(json!({ + "name": "nsetting", + "type": "string", + "default_value": "", + "title": "ntitle", + "optional": false, + "help_text": { + "schema_version": 1, + "properties": { + "help_text": "help text", + "display_order": 1, + }, + }, + })); + then.status(200).json_body(json!( + [{ + "name": "nsetting", + "app_id": "01H2QZ6Z8WXWNDC0KQ198XCZEW", + "type": "string", + "default_value": "", + "title": "ntitle", + "optional": false, + "help_text": "help text", + }])); + }); + + let settings_mock_delete = mock_server.mock(|when, then| { + when.method(DELETE) + .path("/v4.1/edge-apps/settings") + .header("Authorization", "Token token") + .header( + "user-agent", + format!("screenly-cli {}", env!("CARGO_PKG_VERSION")), + ) + .query_param("app_id", "eq.01H2QZ6Z8WXWNDC0KQ198XCZEW") + .query_param("name", "eq.isetting"); + then.status(204).json_body(json!({})); + }); + + let copy_assets_mock = mock_server.mock(|when, then| { + when.method(POST) + .path("/v4/edge-apps/copy-assets") + .header("Authorization", "Token token") + .header( + "user-agent", + format!("screenly-cli {}", env!("CARGO_PKG_VERSION")), + ).json_body(json!({ + "app_id": "01H2QZ6Z8WXWNDC0KQ198XCZEW", + "revision": 8, + "signatures": ["0a209f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08122086cebd0c365d241e32d5b0972c07aae3a8d6499c2a9471aa85943a35577200021a180a14a94a8fe5ccb19ba61c4c0873d391e987982fbbd31000"] + })); + then.status(201).json_body(json!([])); + }); + + let upload_assets_mock = mock_server.mock(|when, then| { + when.method(POST).path("/v4/assets"); + then.status(201).body(""); + }); + let finished_processing_mock = mock_server.mock(|when, then| { + when.method(GET) + .path("/v4/assets") + .query_param("select", "status,processing_error,title") + .query_param("app_id", "eq.01H2QZ6Z8WXWNDC0KQ198XCZEW") + .query_param("app_revision", "eq.8") + .query_param("status", "neq.finished"); + then.status(200).json_body(json!([])); + }); + + let publish_mock = mock_server.mock(|when, then| { + when.method(PATCH) + .path("/v4/edge-apps/versions") + .header("Authorization", "Token token") + .header( + "user-agent", + format!("screenly-cli {}", env!("CARGO_PKG_VERSION")), + ) + .query_param("app_id", "eq.01H2QZ6Z8WXWNDC0KQ198XCZEW") + .query_param("revision", "eq.8") + .json_body(json!({"published": true })); + then.status(200); + }); + + let get_version_mock = mock_server.mock(|when, then| { + when.method(GET) + .path("/v4/edge-apps/versions") + .header("Authorization", "Token token") + .header( + "user-agent", + format!("screenly-cli {}", env!("CARGO_PKG_VERSION")), + ) + .query_param("select", "revision") + .query_param("app_id", "eq.01H2QZ6Z8WXWNDC0KQ198XCZEW") + .query_param("revision", "eq.8"); + + then.status(200).json_body(json!([ + { + "revision": 8, + } + ])); + }); + + let promote_mock = mock_server.mock(|when, then| { + when.method(PATCH) + .path("/v4/edge-apps/channels") + .header("Authorization", "Token token") + .header( + "user-agent", + format!("screenly-cli {}", env!("CARGO_PKG_VERSION")), + ) + .query_param("app_id", "eq.01H2QZ6Z8WXWNDC0KQ198XCZEW") + .query_param("channel", "eq.stable") + .query_param("select", "channel,app_revision") + .json_body(json!({ + "app_revision": 8, + })); + then.status(200).json_body(json!([ + { + "channel": "stable", + "app_revision": 8 + } + ])); + }); + + EdgeAppManifest::save_to_file(&manifest, temp_dir.path().join("screenly.yml").as_path()) + .unwrap(); + let mut file = File::create(temp_dir.path().join("index.html")).unwrap(); + write!(file, "test").unwrap(); + + let result = command.deploy( + Some("01H2QZ6Z8WXWNDC0KQ198XCZEW".to_string()), + Some(temp_dir.path().to_str().unwrap().to_string()), + Some(true), + ); + + last_versions_mock.assert_calls(2); + assets_mock.assert(); + file_tree_from_version_mock.assert(); + settings_mock.assert(); + create_version_mock.assert(); + settings_mock_create.assert(); + settings_mock_patch.assert(); + settings_mock_delete.assert(); + upload_assets_mock.assert(); + finished_processing_mock.assert(); + publish_mock.assert(); + copy_assets_mock.assert(); + get_version_mock.assert(); + promote_mock.assert(); + + assert!(result.is_ok()); + } + #[test] fn test_detect_version_metadata_changes_when_no_changes_should_return_false() { let (temp_dir, command, mock_server, _manifest, _instance_manifest) = @@ -2174,8 +2521,8 @@ mod tests { ) .unwrap(); - let result = - command.update_entrypoint_value(Some(temp_dir.path().to_str().unwrap().to_string())); + let result = command + .update_entrypoint_value(None, Some(temp_dir.path().to_str().unwrap().to_string())); setting_is_global_get_mock.assert(); setting_mock_get.assert(); @@ -2265,8 +2612,8 @@ mod tests { ) .unwrap(); - let result = - command.update_entrypoint_value(Some(temp_dir.path().to_str().unwrap().to_string())); + let result = command + .update_entrypoint_value(None, Some(temp_dir.path().to_str().unwrap().to_string())); setting_is_global_get_mock.assert(); setting_mock_get.assert(); @@ -2361,8 +2708,8 @@ mod tests { ) .unwrap(); - let result = - command.update_entrypoint_value(Some(temp_dir.path().to_str().unwrap().to_string())); + let result = command + .update_entrypoint_value(None, Some(temp_dir.path().to_str().unwrap().to_string())); setting_is_global_get_mock.assert(); setting_mock_get.assert(); diff --git a/src/commands/edge_app/instance.rs b/src/commands/edge_app/instance.rs index 8831331..8c29d43 100644 --- a/src/commands/edge_app/instance.rs +++ b/src/commands/edge_app/instance.rs @@ -70,7 +70,7 @@ impl EdgeAppCommand { .update_installation_name(&installation_id, &instance_manifest.name)?; } - self.update_entrypoint_value(path)?; + self.update_entrypoint_value(None, path)?; Ok(()) } diff --git a/src/commands/edge_app/setting.rs b/src/commands/edge_app/setting.rs index 2dd4880..a98fd69 100644 --- a/src/commands/edge_app/setting.rs +++ b/src/commands/edge_app/setting.rs @@ -14,6 +14,7 @@ impl EdgeAppCommand { pub fn set_setting( &self, + app_id: Option, path: Option, setting_key: &str, setting_value: &str, @@ -23,9 +24,12 @@ impl EdgeAppCommand { Some(id) => id.clone(), None => "".to_string(), }; - let app_id: String = match self.get_app_id(path.clone()) { - Ok(id) => id, - Err(_) => return Err(CommandError::MissingAppId), + let app_id: String = match app_id { + Some(id) => id, + None => match self.get_app_id(path.clone()) { + Ok(id) => id, + Err(_) => return Err(CommandError::MissingAppId), + }, }; let _is_setting_global = self.api.is_setting_global(&app_id, setting_key)?; @@ -345,6 +349,7 @@ mod tests { }); let result = command.set_setting( + None, Some(tmp_dir.path().to_str().unwrap().to_string()), "best_setting", "best_value", @@ -429,6 +434,7 @@ mod tests { }); let result = command.set_setting( + None, Some(tmp_dir.path().to_str().unwrap().to_string()), "best_setting", "best_value1", @@ -513,6 +519,7 @@ mod tests { }); let result = command.set_setting( + None, Some(temp_dir.path().to_str().unwrap().to_string()), "best_setting", "best_value1", @@ -591,6 +598,7 @@ mod tests { }); let result = command.set_setting( + None, Some(temp_dir.path().to_str().unwrap().to_string()), "best_setting", "best_value1", @@ -623,6 +631,7 @@ mod tests { }); let result = command.set_setting( + None, Some(temp_dir.path().to_str().unwrap().to_string()), "best_setting", "best_value1", @@ -705,6 +714,7 @@ mod tests { }); let result = command.set_setting( + None, Some(temp_dir.path().to_str().unwrap().to_string()), "best_secret_setting", "best_secret_value", @@ -786,6 +796,7 @@ mod tests { }); let result = command.set_setting( + None, Some(temp_dir.path().to_str().unwrap().to_string()), "best_secret_setting", "best_secret_value", @@ -853,6 +864,7 @@ mod tests { }); let result = command.set_setting( + None, Some(temp_dir.path().to_str().unwrap().to_string()), "best_setting", "best_value",