stop service if critical task activated (#2966)

filter out union lists instead of erroring

container-runtime/src/Adapters/Systems/SystemForEmbassy/__fixtures__/rtlConfig.ts

@@ -0,0 +1,153 @@
+export default {
+  nodes: {
+    type: "list",
+    subtype: "union",
+    name: "Lightning Nodes",
+    description: "List of Lightning Network node instances to manage",
+    range: "[1,*)",
+    default: ["lnd"],
+    spec: {
+      type: "string",
+      "display-as": "{{name}}",
+      "unique-by": "name",
+      name: "Node Implementation",
+      tag: {
+        id: "type",
+        name: "Type",
+        description:
+          "- LND: Lightning Network Daemon from Lightning Labs\n- CLN: Core Lightning from Blockstream\n",
+        "variant-names": {
+          lnd: "Lightning Network Daemon (LND)",
+          "c-lightning": "Core Lightning (CLN)",
+        },
+      },
+      default: "lnd",
+      variants: {
+        lnd: {
+          name: {
+            type: "string",
+            name: "Node Name",
+            description: "Name of this node in the list",
+            default: "StartOS LND",
+            nullable: false,
+          },
+          "connection-settings": {
+            type: "union",
+            name: "Connection Settings",
+            description: "The Lightning Network Daemon node to connect to.",
+            tag: {
+              id: "type",
+              name: "Type",
+              description:
+                "- Internal: The Lightning Network Daemon service installed to your StartOS server.\n- External: A Lightning Network Daemon instance running on a remote device (advanced).\n",
+              "variant-names": {
+                internal: "Internal",
+                external: "External",
+              },
+            },
+            default: "internal",
+            variants: {
+              internal: {},
+              external: {
+                address: {
+                  type: "string",
+                  name: "Public Address",
+                  description:
+                    "The public address of your LND REST server\nNOTE: RTL does not support a .onion URL here\n",
+                  nullable: false,
+                },
+                "rest-port": {
+                  type: "number",
+                  name: "REST Port",
+                  description:
+                    "The port that your Lightning Network Daemon REST server is bound to",
+                  nullable: false,
+                  range: "[0,65535]",
+                  integral: true,
+                  default: 8080,
+                },
+                macaroon: {
+                  type: "string",
+                  name: "Macaroon",
+                  description:
+                    'Your admin.macaroon file, Base64URL encoded. This is the same as the value after "macaroon=" in your lndconnect URL.',
+                  nullable: false,
+                  masked: true,
+                  pattern: "[=A-Za-z0-9_-]+",
+                  "pattern-description":
+                    "Macaroon must be encoded in Base64URL format (only A-Z, a-z, 0-9, _, - and = allowed)",
+                },
+              },
+            },
+          },
+        },
+        "c-lightning": {
+          name: {
+            type: "string",
+            name: "Node Name",
+            description: "Name of this node in the list",
+            default: "StartOS CLN",
+            nullable: false,
+          },
+          "connection-settings": {
+            type: "union",
+            name: "Connection Settings",
+            description: "The Core Lightning (CLN) node to connect to.",
+            tag: {
+              id: "type",
+              name: "Type",
+              description:
+                "- Internal: The Core Lightning (CLN) service installed to your StartOS server.\n- External: A Core Lightning (CLN) instance running on a remote device (advanced).\n",
+              "variant-names": {
+                internal: "Internal",
+                external: "External",
+              },
+            },
+            default: "internal",
+            variants: {
+              internal: {},
+              external: {
+                address: {
+                  type: "string",
+                  name: "Public Address",
+                  description:
+                    "The public address of your CLNRest server\nNOTE: RTL does not support a .onion URL here\n",
+                  nullable: false,
+                },
+                "rest-port": {
+                  type: "number",
+                  name: "CLNRest Port",
+                  description: "The port that your CLNRest server is bound to",
+                  nullable: false,
+                  range: "[0,65535]",
+                  integral: true,
+                  default: 3010,
+                },
+                macaroon: {
+                  type: "string",
+                  name: "Rune",
+                  description:
+                    "Your CLNRest unrestricted Rune, Base64URL encoded.",
+                  nullable: false,
+                  masked: true,
+                },
+              },
+            },
+          },
+        },
+      },
+    },
+  },
+  password: {
+    type: "string",
+    name: "Password",
+    description: "The password for your Ride the Lightning dashboard",
+    nullable: false,
+    copyable: true,
+    masked: true,
+    default: {
+      charset: "a-z,A-Z,0-9",
+      len: 22,
+    },
+  },
+}

container-runtime/src/Adapters/Systems/SystemForEmbassy/__snapshots__/transformConfigSpec.test.ts.snap

@@ -1,5 +1,30 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
+exports[`transformConfigSpec transformConfigSpec(RTL) 1`] = `
+{
+  "password": {
+    "default": {
+      "charset": "a-z,A-Z,0-9",
+      "len": 22,
+    },
+    "description": "The password for your Ride the Lightning dashboard",
+    "disabled": false,
+    "generate": null,
+    "immutable": false,
+    "inputmode": "text",
+    "masked": true,
+    "maxLength": null,
+    "minLength": null,
+    "name": "Password",
+    "patterns": [],
+    "placeholder": null,
+    "required": true,
+    "type": "text",
+    "warning": null,
+  },
+}
+`;
+
 exports[`transformConfigSpec transformConfigSpec(bitcoind) 1`] = `
 {
   "advanced": {

container-runtime/src/Adapters/Systems/SystemForEmbassy/transformConfigSpec.test.ts

@@ -1,5 +1,10 @@
-import { matchOldConfigSpec, transformConfigSpec } from "./transformConfigSpec"
-import fixtureEmbasyPagesConfig from "./__fixtures__/embasyPagesConfig"
+import {
+  matchOldConfigSpec,
+  matchOldValueSpecList,
+  transformConfigSpec,
+} from "./transformConfigSpec"
+import fixtureEmbassyPagesConfig from "./__fixtures__/embassyPagesConfig"
+import fixtureRTLConfig from "./__fixtures__/rtlConfig"
 import searNXG from "./__fixtures__/searNXG"
 import bitcoind from "./__fixtures__/bitcoind"
 import nostr from "./__fixtures__/nostr"
@@ -8,14 +13,25 @@ import nostrConfig2 from "./__fixtures__/nostrConfig2"
 describe("transformConfigSpec", () => {
   test("matchOldConfigSpec(embassyPages.homepage.variants[web-page])", () => {
     matchOldConfigSpec.unsafeCast(
-      fixtureEmbasyPagesConfig.homepage.variants["web-page"],
+      fixtureEmbassyPagesConfig.homepage.variants["web-page"],
     )
   })
   test("matchOldConfigSpec(embassyPages)", () => {
-    matchOldConfigSpec.unsafeCast(fixtureEmbasyPagesConfig)
+    matchOldConfigSpec.unsafeCast(fixtureEmbassyPagesConfig)
   })
   test("transformConfigSpec(embassyPages)", () => {
-    const spec = matchOldConfigSpec.unsafeCast(fixtureEmbasyPagesConfig)
+    const spec = matchOldConfigSpec.unsafeCast(fixtureEmbassyPagesConfig)
+    expect(transformConfigSpec(spec)).toMatchSnapshot()
+  })
+
+  test("matchOldConfigSpec(RTL.nodes)", () => {
+    matchOldValueSpecList.unsafeCast(fixtureRTLConfig.nodes)
+  })
+  test("matchOldConfigSpec(RTL)", () => {
+    matchOldConfigSpec.unsafeCast(fixtureRTLConfig)
+  })
+  test("transformConfigSpec(RTL)", () => {
+    const spec = matchOldConfigSpec.unsafeCast(fixtureRTLConfig)
     expect(transformConfigSpec(spec)).toMatchSnapshot()
   })
 

container-runtime/src/Adapters/Systems/SystemForEmbassy/transformConfigSpec.ts

@@ -47,6 +47,7 @@ export function transformConfigSpec(oldSpec: OldConfigSpec): IST.InputSpec {
         immutable: false,
       }
     } else if (oldVal.type === "list") {
+      if (isUnionList(oldVal)) return inputSpec
       newVal = getListSpec(oldVal)
     } else if (oldVal.type === "number") {
       const range = Range.from(oldVal.range)
@@ -177,15 +178,17 @@ export function transformOldConfigToNew(
       }
     }
 
-    if (isList(val) && isObjectList(val)) {
+    if (isList(val)) {
       if (!config[key]) return obj
 
-      newVal = (config[key] as object[]).map((obj) =>
-        transformOldConfigToNew(
-          matchOldConfigSpec.unsafeCast(val.spec.spec),
-          obj,
-        ),
-      )
+      if (isObjectList(val)) {
+        newVal = (config[key] as object[]).map((obj) =>
+          transformOldConfigToNew(
+            matchOldConfigSpec.unsafeCast(val.spec.spec),
+            obj,
+          ),
+        )
+      } else if (isUnionList(val)) return obj
     }
 
     if (isPointer(val)) {
@@ -224,13 +227,15 @@ export function transformNewConfigToOld(
       }
     }
 
-    if (isList(val) && isObjectList(val)) {
-      newVal = (config[key] as object[]).map((obj) =>
-        transformNewConfigToOld(
-          matchOldConfigSpec.unsafeCast(val.spec.spec),
-          obj,
-        ),
-      )
+    if (isList(val)) {
+      if (isObjectList(val)) {
+        newVal = (config[key] as object[]).map((obj) =>
+          transformNewConfigToOld(
+            matchOldConfigSpec.unsafeCast(val.spec.spec),
+            obj,
+          ),
+        )
+      } else if (isUnionList(val)) return obj
     }
 
     return {
@@ -376,15 +381,17 @@ function isNumberList(
 ): val is OldValueSpecList & { subtype: "number" } {
   return val.subtype === "number"
 }
-
 function isObjectList(
   val: OldValueSpecList,
 ): val is OldValueSpecList & { subtype: "object" } {
-  if (["union"].includes(val.subtype)) {
-    throw new Error("Invalid list subtype. enum, string, and object permitted.")
-  }
   return val.subtype === "object"
 }
+function isUnionList(
+  val: OldValueSpecList,
+): val is OldValueSpecList & { subtype: "union" } {
+  return val.subtype === "union"
+}
+
 export type OldConfigSpec = Record<string, OldValueSpec>
 const [_matchOldConfigSpec, setMatchOldConfigSpec] = deferred<unknown>()
 export const matchOldConfigSpec = _matchOldConfigSpec as Parser<
@@ -491,6 +498,12 @@ const matchOldListValueSpecObject = object({
   "unique-by": matchOldUniqueBy.nullable().optional(), // indicates whether duplicates can be permitted in the list
   "display-as": string.nullable().optional(), // this should be a handlebars template which can make use of the entire config which corresponds to 'spec'
 })
+const matchOldListValueSpecUnion = object({
+  "unique-by": matchOldUniqueBy.nullable().optional(),
+  "display-as": string.nullable().optional(),
+  tag: matchOldUnionTagSpec,
+  variants: dictionary([string, _matchOldConfigSpec]),
+})
 const matchOldListValueSpecString = object({
   masked: boolean.nullable().optional(),
   copyable: boolean.nullable().optional(),
@@ -511,7 +524,7 @@ const matchOldListValueSpecNumber = object({
 })
 
 // represents a spec for a list
-const matchOldValueSpecList = every(
+export const matchOldValueSpecList = every(
   object({
     type: literals("list"),
     range: string, // '[0,1]' (inclusive) OR '[0,*)' (right unbounded), normal math rules
@@ -542,6 +555,10 @@ const matchOldValueSpecList = every(
       subtype: literals("number"),
       spec: matchOldListValueSpecNumber,
     }),
+    object({
+      subtype: literals("union"),
+      spec: matchOldListValueSpecUnion,
+    }),
   ),
 )
 type OldValueSpecList = typeof matchOldValueSpecList._TYPE

core/startos/src/context/rpc.rs

@@ -24,6 +24,7 @@ use super::setup::CURRENT_SECRET;
 use crate::account::AccountInfo;
 use crate::auth::Sessions;
 use crate::context::config::ServerConfig;
+use crate::db::model::package::TaskSeverity;
 use crate::db::model::Database;
 use crate::disk::OsPartitionInfo;
 use crate::init::{check_time_is_synchronized, InitResult};
@@ -403,21 +404,46 @@ impl RpcContext {
                 }
             }
         }
-        self.db
-            .mutate(|db| {
-                for (package_id, action_input) in &action_input {
-                    for (action_id, input) in action_input {
-                        for (_, pde) in db.as_public_mut().as_package_data_mut().as_entries_mut()? {
-                            pde.as_tasks_mut().mutate(|tasks| {
-                                Ok(update_tasks(tasks, package_id, action_id, input, false))
-                            })?;
+        for id in
+            self.db
+                .mutate::<Vec<PackageId>>(|db| {
+                    for (package_id, action_input) in &action_input {
+                        for (action_id, input) in action_input {
+                            for (_, pde) in
+                                db.as_public_mut().as_package_data_mut().as_entries_mut()?
+                            {
+                                pde.as_tasks_mut().mutate(|tasks| {
+                                    Ok(update_tasks(tasks, package_id, action_id, input, false))
+                                })?;
+                            }
                         }
                     }
-                }
-                Ok(())
-            })
-            .await
-            .result?;
+                    db.as_public()
+                        .as_package_data()
+                        .as_entries()?
+                        .into_iter()
+                        .filter_map(|(id, pkg)| {
+                            (|| {
+                                if pkg.as_tasks().de()?.into_iter().any(|(_, t)| {
+                                    t.active && t.task.severity == TaskSeverity::Critical
+                                }) {
+                                    Ok(Some(id))
+                                } else {
+                                    Ok(None)
+                                }
+                            })()
+                            .transpose()
+                        })
+                        .collect()
+                })
+                .await
+                .result?
+        {
+            let svc = self.services.get(&id).await;
+            if let Some(svc) = &*svc {
+                svc.stop(procedure_id.clone(), false).await?;
+            }
+        }
         check_tasks.complete();
 
         Ok(())

core/startos/src/service/action.rs

@@ -6,7 +6,7 @@ use models::{ActionId, PackageId, ProcedureName, ReplayId};
 
 use crate::action::{ActionInput, ActionResult};
 use crate::db::model::package::{
-    ActionVisibility, AllowedStatuses, TaskCondition, TaskEntry, TaskInput,
+    ActionVisibility, AllowedStatuses, TaskCondition, TaskEntry, TaskInput, TaskSeverity,
 };
 use crate::prelude::*;
 use crate::rpc_continuations::Guid;
@@ -78,7 +78,8 @@ pub fn update_tasks(
     action_id: &ActionId,
     input: &Value,
     was_run: bool,
-) {
+) -> bool {
+    let mut critical_activated = false;
     tasks.retain(|_, v| {
         if &v.task.package_id != package_id || &v.task.action_id != action_id {
             return true;
@@ -95,6 +96,9 @@ pub fn update_tasks(
                             }
                         } else {
                             v.active = true;
+                            if v.task.severity == TaskSeverity::Critical {
+                                critical_activated = true;
+                            }
                         }
                     }
                     None => {
@@ -106,7 +110,8 @@ pub fn update_tasks(
         } else {
             !was_run
         }
-    })
+    });
+    critical_activated
 }
 
 pub(super) struct RunAction {
@@ -125,7 +130,7 @@ impl Handler<RunAction> for ServiceActor {
             id: ref action_id,
             input,
         }: RunAction,
-        _: &BackgroundJobQueue,
+        jobs: &BackgroundJobQueue,
     ) -> Self::Response {
         let container = &self.0.persistent_container;
         let package_id = &self.0.id;
@@ -162,7 +167,7 @@ impl Handler<RunAction> for ServiceActor {
         }
         let result = container
             .execute::<Option<ActionResult>>(
-                id,
+                id.clone(),
                 ProcedureName::RunAction(action_id.clone()),
                 json!({
                     "input": input,
@@ -171,19 +176,30 @@ impl Handler<RunAction> for ServiceActor {
             )
             .await
             .with_kind(ErrorKind::Action)?;
-        self.0
+        if self
+            .0
             .ctx
             .db
             .mutate(|db| {
+                let mut critical_activated = false;
                 for (_, pde) in db.as_public_mut().as_package_data_mut().as_entries_mut()? {
-                    pde.as_tasks_mut().mutate(|tasks| {
+                    critical_activated |= pde.as_tasks_mut().mutate(|tasks| {
                         Ok(update_tasks(tasks, package_id, action_id, &input, true))
                     })?;
                 }
-                Ok(())
+                Ok(critical_activated)
             })
             .await
-            .result?;
+            .result?
+        {
+            <Self as Handler<super::control::Stop>>::handle(
+                self,
+                id,
+                super::control::Stop { wait: false },
+                jobs,
+            )
+            .await;
+        }
         Ok(result)
     }
 }

core/startos/src/service/control.rs

@@ -28,8 +28,8 @@ impl Service {
     }
 }
 
-struct Stop {
-    wait: bool,
+pub(super) struct Stop {
+    pub wait: bool,
 }
 impl Handler<Stop> for ServiceActor {
     type Response = ();

core/startos/src/service/mod.rs

@@ -35,7 +35,7 @@ use url::Url;
 use crate::context::{CliContext, RpcContext};
 use crate::db::model::package::{
     InstalledState, ManifestPreference, PackageDataEntry, PackageState, PackageStateMatchModelRef,
-    UpdatingState,
+    TaskSeverity, UpdatingState,
 };
 use crate::disk::mount::filesystem::ReadOnly;
 use crate::disk::mount::guard::{GenericMountGuard, MountGuard};
@@ -526,7 +526,8 @@ impl Service {
                 }
             }
         }
-        ctx.db
+        let has_critical = ctx
+            .db
             .mutate(|db| {
                 for (action_id, input) in &action_input {
                     for (_, pde) in db.as_public_mut().as_package_data_mut().as_entries_mut()? {
@@ -541,10 +542,12 @@ impl Service {
                     .as_idx_mut(&manifest.id)
                     .or_not_found(&manifest.id)?;
                 let actions = entry.as_actions().keys()?;
-                entry.as_tasks_mut().mutate(|t| {
-                    Ok(t.retain(|_, v| {
+                let has_critical = entry.as_tasks_mut().mutate(|t| {
+                    t.retain(|_, v| {
                         v.task.package_id != manifest.id || actions.contains(&v.task.action_id)
-                    }))
+                    });
+                    Ok(t.iter()
+                        .any(|(_, t)| t.active && t.task.severity == TaskSeverity::Critical))
                 })?;
                 entry
                     .as_state_info_mut()
@@ -553,11 +556,15 @@ impl Service {
                 entry.as_icon_mut().ser(&icon)?;
                 entry.as_registry_mut().ser(registry)?;
 
-                Ok(())
+                Ok(has_critical)
             })
             .await
             .result?;
 
+        if prev_state == Some(StartStop::Start) && !has_critical {
+            service.start(procedure_id).await?;
+        }
+
         Ok(service)
     }
 

core/startos/src/service/service_map.rs

@@ -25,7 +25,6 @@ use crate::prelude::*;
 use crate::progress::{
     FullProgressTracker, PhaseProgressTrackerHandle, ProgressTrackerWriter, ProgressUnits,
 };
-use crate::rpc_continuations::Guid;
 use crate::s9pk::manifest::PackageId;
 use crate::s9pk::merkle_archive::source::FileSource;
 use crate::s9pk::S9pk;
@@ -344,9 +343,6 @@ impl ServiceMap {
                         }),
                     )
                     .await?;
-                    if prev == Some(StartStop::Start) {
-                        new_service.start(Guid::new()).await?;
-                    }
                     *service = Some(new_service.into());
                     drop(service);