diff --git a/docs/EdgeApps.md b/docs/EdgeApps.md index 96e20b3..2be4c90 100644 --- a/docs/EdgeApps.md +++ b/docs/EdgeApps.md @@ -294,7 +294,11 @@ 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. +> **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` refuse to run while `EDGE_APP_ID` is set, since both commands always create a brand new Edge App and would otherwise leave the configured id pointing at an app you didn't mean to create. Unset `EDGE_APP_ID` first if you want to create a new app, or use `screenly edge-app deploy` if you meant to deploy to the existing one. +> +> `screenly edge-app delete` clears the `id` field from `screenly.yml`, but it can't clear `EDGE_APP_ID` from your environment. If it's still set, subsequent commands keep resolving to the deleted app's id until you unset or update it yourself; the CLI prints a warning after deletion as a reminder. #### Entrypoint diff --git a/src/cli.rs b/src/cli.rs index 1509aa2..ee4c1ec 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::{edge_app_id_from_env, EDGE_APP_ID_ENV}; use crate::commands::edge_app::instance_manifest::InstanceManifest; use crate::commands::edge_app::manifest::EdgeAppManifest; use crate::commands::edge_app::server::MOCK_DATA_FILENAME; @@ -996,6 +997,11 @@ pub fn handle_cli_edge_app_command(command: &EdgeAppCommands, output: OutputForm std::process::exit(1); } } + + if let Some(id) = edge_app_id_from_env() { + eprintln!("Warning: the {EDGE_APP_ID_ENV} environment variable is still set to \"{id}\". Commands will keep using that id until you unset or update it."); + } + std::process::exit(0); } Err(e) => { diff --git a/src/commands/edge_app/app.rs b/src/commands/edge_app/app.rs index ceada28..306189b 100644 --- a/src/commands/edge_app/app.rs +++ b/src/commands/edge_app/app.rs @@ -29,10 +29,21 @@ 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(crate) const EDGE_APP_ID_ENV: &str = "EDGE_APP_ID"; pub const INJECT_JS_FILE_NAME: &str = "screenly_inject.js"; +pub(crate) fn edge_app_id_from_env() -> Option { + std::env::var(EDGE_APP_ID_ENV).ok().and_then(|id| { + let trimmed = id.trim(); + if trimmed.is_empty() { + None + } else { + Some(trimmed.to_string()) + } + }) +} + // Edge apps commands impl EdgeAppCommand { pub fn create( @@ -46,6 +57,12 @@ impl EdgeAppCommand { ))?; let index_html_path = parent_dir_path.join("index.html"); + if let Some(id) = edge_app_id_from_env() { + return Err(CommandError::InitializationError(format!( + "An Edge App id is already configured via the {EDGE_APP_ID_ENV} environment variable ({id}). 'screenly edge-app create' always creates a new Edge App; unset {EDGE_APP_ID_ENV} first if you want to create a new one, or use 'screenly edge-app deploy' if you meant to deploy to the existing app" + ))); + } + if Path::new(&path).exists() || Path::new(&index_html_path).exists() { return Err(CommandError::FileSystemError(format!( "The directory {} already contains a screenly.yml or index.html file. Use --in-place if you want to create an Edge App in this directory", @@ -184,6 +201,12 @@ impl EdgeAppCommand { ))); } + if let Some(id) = edge_app_id_from_env() { + return Err(CommandError::InitializationError(format!( + "An Edge App id is already configured via the {EDGE_APP_ID_ENV} environment variable ({id}). The operation can only proceed when no Edge App id is already configured; unset {EDGE_APP_ID_ENV} first if you want to create a new Edge App, or use 'screenly edge-app deploy' if you meant to deploy to the existing app" + ))); + } + let data = fs::read_to_string(path)?; let mut manifest: EdgeAppManifest = serde_yaml::from_str(&data)?; @@ -701,11 +724,8 @@ 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()); - } + if let Some(id) = edge_app_id_from_env() { + return Ok(id); } let edge_app_manifest = EdgeAppManifest::new(&transform_edge_app_path_to_manifest(&path)?)?; @@ -1748,6 +1768,93 @@ mod tests { assert_eq!(app_id.unwrap(), manifest.unwrap().id.unwrap()); } + #[test] + fn test_edge_app_create_when_env_var_is_set_should_return_error() { + let (tmp_dir, command, _mock_server, _manifest, _instance_manifest) = + prepare_edge_apps_test(false, false); + + let _lock = lock_test(); + let _env = set_env( + OsString::from(EDGE_APP_ID_ENV), + "01ENVOVERRIDEXXXXXXXXXXXXX", + ); + let result = command.create( + "Best app ever", + tmp_dir.path().join("screenly.yml").as_path(), + None, + ); + + assert!(result.is_err()); + assert!(result + .unwrap_err() + .to_string() + .contains("EDGE_APP_ID environment variable (01ENVOVERRIDEXXXXXXXXXXXXX)")); + assert!(!tmp_dir.path().join("screenly.yml").exists()); + } + + #[test] + fn test_create_in_place_edge_app_when_env_var_is_set_should_return_error() { + let (tmp_dir, command, _mock_server, _manifest, _instance_manifest) = + prepare_edge_apps_test(false, false); + + File::create(tmp_dir.path().join("index.html")).unwrap(); + + let manifest = EdgeAppManifest { + id: None, + syntax: MANIFEST_VERSION.to_owned(), + ..Default::default() + }; + + EdgeAppManifest::save_to_file(&manifest, tmp_dir.path().join("screenly.yml").as_path()) + .unwrap(); + + let _lock = lock_test(); + let _env = set_env( + OsString::from(EDGE_APP_ID_ENV), + "01ENVOVERRIDEXXXXXXXXXXXXX", + ); + let result = command.create_in_place( + "Best app ever", + tmp_dir.path().join("screenly.yml").as_path(), + ); + + assert!(result.is_err()); + assert!(result + .unwrap_err() + .to_string() + .contains("EDGE_APP_ID environment variable (01ENVOVERRIDEXXXXXXXXXXXXX)")); + } + + #[test] + fn test_create_in_place_edge_app_when_env_var_is_set_and_manifest_is_malformed_should_still_return_env_var_error( + ) { + let (tmp_dir, command, _mock_server, _manifest, _instance_manifest) = + prepare_edge_apps_test(false, false); + + File::create(tmp_dir.path().join("index.html")).unwrap(); + fs::write( + tmp_dir.path().join("screenly.yml"), + "not: [valid, yaml: manifest", + ) + .unwrap(); + + let _lock = lock_test(); + let _env = set_env( + OsString::from(EDGE_APP_ID_ENV), + "01ENVOVERRIDEXXXXXXXXXXXXX", + ); + let result = command.create_in_place( + "Best app ever", + tmp_dir.path().join("screenly.yml").as_path(), + ); + + assert!(result.is_err()); + assert!(result + .unwrap_err() + .to_string() + .contains("EDGE_APP_ID environment variable (01ENVOVERRIDEXXXXXXXXXXXXX)")); + } + #[test] fn test_clear_app_id_should_remove_app_id_from_manifest() { let (temp_dir, command, _mock_server, _manifest, _instance_manifest) =