Fix systemd unloaded unit

src/service/dbus.rs

@@ -18,7 +18,7 @@ use zbus::{Connection, MatchRule, MessageStream, Proxy};
 
 use super::{
     JobResult, RestartDeadline, RestartExecution, RestartFailure, RestartJobTracker,
-    RestartOutcome, ServiceEvent,
+    RestartOutcome, RestartPhase, ServiceEvent,
 };
 
 const SYSTEMD_NAME: &str = "org.freedesktop.systemd1";
@@ -70,25 +70,39 @@ fn restart_with_transport_report(
     deadline: RestartDeadline,
 ) -> RestartExecution {
     if configured_unit.is_empty() || configured_unit.as_bytes().contains(&0) {
-        return execution(RestartOutcome::Failed(RestartFailure::RequestFailed), false);
+        return execution(
+            RestartOutcome::Failed(RestartFailure::RequestFailed),
+            false,
+            RestartPhase::RequestRestart,
+        );
     }
 
     if let Err(failure) = transport.connect(deadline) {
-        return execution(RestartOutcome::Failed(failure), false);
+        return execution(
+            RestartOutcome::Failed(failure),
+            false,
+            RestartPhase::Connect,
+        );
     }
     if deadline.is_expired_at(Instant::now()) {
         return execution(
             RestartOutcome::Failed(RestartFailure::WaitTimedOutMayStillComplete),
             false,
+            RestartPhase::Connect,
         );
     }
     if let Err(failure) = transport.subscribe(deadline) {
-        return execution(RestartOutcome::Failed(failure), false);
+        return execution(
+            RestartOutcome::Failed(failure),
+            false,
+            RestartPhase::Subscribe,
+        );
     }
     if deadline.is_expired_at(Instant::now()) {
         return execution(
             RestartOutcome::Failed(RestartFailure::WaitTimedOutMayStillComplete),
             false,
+            RestartPhase::Subscribe,
         );
     }
     let primary_unit = match transport.resolve_primary_unit(configured_unit, deadline) {
@@ -97,14 +111,22 @@ fn restart_with_transport_report(
             return execution(
                 RestartOutcome::Failed(RestartFailure::UnitResolutionFailed),
                 false,
+                RestartPhase::ResolveUnit,
+            );
+        }
+        Err(failure) => {
+            return execution(
+                RestartOutcome::Failed(failure),
+                false,
+                RestartPhase::ResolveUnit,
             );
         }
-        Err(failure) => return execution(RestartOutcome::Failed(failure), false),
     };
     if deadline.is_expired_at(Instant::now()) {
         return execution(
             RestartOutcome::Failed(RestartFailure::WaitTimedOutMayStillComplete),
             false,
+            RestartPhase::ResolveUnit,
         );
     }
 
@@ -113,9 +135,15 @@ fn restart_with_transport_report(
         Err(failure) => {
             let submitted = matches!(
                 failure,
-                RestartFailure::WaitTimedOutMayStillComplete | RestartFailure::BusDisconnected
+                RestartFailure::WaitTimedOutMayStillComplete
+                    | RestartFailure::BusDisconnected
+                    | RestartFailure::ManagerOwnerChanged
+            );
+            return execution(
+                RestartOutcome::Failed(failure),
+                submitted,
+                RestartPhase::RequestRestart,
             );
-            return execution(RestartOutcome::Failed(failure), submitted);
         }
     };
     let mut tracker = RestartJobTracker::new(primary_unit, deadline);
@@ -123,15 +151,25 @@ fn restart_with_transport_report(
     while outcome == RestartOutcome::Waiting {
         let event = match transport.next_event(deadline) {
             Ok(event) => event,
-            Err(failure) => return execution(RestartOutcome::Failed(failure), true),
+            Err(failure) => {
+                return execution(
+                    RestartOutcome::Failed(failure),
+                    true,
+                    RestartPhase::WaitForJob,
+                );
+            }
         };
         outcome = tracker.observe(event, Instant::now());
     }
-    execution(outcome, true)
+    execution(outcome, true, RestartPhase::WaitForJob)
 }
 
-fn execution(outcome: RestartOutcome, submitted: bool) -> RestartExecution {
-    RestartExecution { outcome, submitted }
+fn execution(outcome: RestartOutcome, submitted: bool, phase: RestartPhase) -> RestartExecution {
+    RestartExecution {
+        outcome,
+        submitted,
+        phase,
+    }
 }
 
 #[derive(Default)]
@@ -142,6 +180,100 @@ struct SystemdBus {
     owner_events: Option<zbus::MessageStream>,
 }
 
+/// The small systemd unit API used by the shared resolver. Keeping method
+/// selection and canonical-ID lookup here lets synthetic tests exercise the
+/// same resolution flow as the production adapter.
+trait UnitLoadApi {
+    type UnitPath;
+
+    fn load_unit(
+        &mut self,
+        configured_unit: &str,
+        deadline: RestartDeadline,
+    ) -> Result<Self::UnitPath, RestartFailure>;
+    fn load_state(
+        &mut self,
+        unit_path: &Self::UnitPath,
+        deadline: RestartDeadline,
+    ) -> Result<String, RestartFailure>;
+    fn canonical_id(
+        &mut self,
+        unit_path: &Self::UnitPath,
+        deadline: RestartDeadline,
+    ) -> Result<String, RestartFailure>;
+}
+
+fn resolve_loaded_unit(
+    api: &mut impl UnitLoadApi,
+    configured_unit: &str,
+    deadline: RestartDeadline,
+) -> Result<String, RestartFailure> {
+    let unit_path = api.load_unit(configured_unit, deadline)?;
+    if !unit_is_loaded(&api.load_state(&unit_path, deadline)?) {
+        return Err(RestartFailure::UnitResolutionFailed);
+    }
+    api.canonical_id(&unit_path, deadline)
+}
+
+impl UnitLoadApi for SystemdBus {
+    type UnitPath = zbus::zvariant::OwnedObjectPath;
+
+    fn load_unit(
+        &mut self,
+        configured_unit: &str,
+        deadline: RestartDeadline,
+    ) -> Result<Self::UnitPath, RestartFailure> {
+        let manager = self
+            .manager
+            .as_ref()
+            .ok_or(RestartFailure::UnitResolutionFailed)?;
+        drive(manager.call("LoadUnit", &(configured_unit,)), deadline)?
+            .map_err(map_unit_resolution_error)
+    }
+
+    fn load_state(
+        &mut self,
+        unit_path: &Self::UnitPath,
+        deadline: RestartDeadline,
+    ) -> Result<String, RestartFailure> {
+        let unit = self.unit_proxy(unit_path, deadline)?;
+        drive(unit.get_property::<String>("LoadState"), deadline)?
+            .map_err(map_unit_resolution_error)
+    }
+
+    fn canonical_id(
+        &mut self,
+        unit_path: &Self::UnitPath,
+        deadline: RestartDeadline,
+    ) -> Result<String, RestartFailure> {
+        let unit = self.unit_proxy(unit_path, deadline)?;
+        drive(unit.get_property::<String>("Id"), deadline)?.map_err(map_unit_resolution_error)
+    }
+}
+
+impl SystemdBus {
+    fn unit_proxy(
+        &self,
+        unit_path: &zbus::zvariant::OwnedObjectPath,
+        deadline: RestartDeadline,
+    ) -> Result<Proxy<'static>, RestartFailure> {
+        let connection = self
+            .connection
+            .as_ref()
+            .ok_or(RestartFailure::BusDisconnected)?;
+        drive(
+            Proxy::new_owned(
+                connection.clone(),
+                SYSTEMD_NAME,
+                unit_path.clone(),
+                "org.freedesktop.systemd1.Unit",
+            ),
+            deadline,
+        )?
+        .map_err(map_unit_resolution_error)
+    }
+}
+
 impl RestartTransport for SystemdBus {
     fn connect(&mut self, deadline: RestartDeadline) -> Result<(), RestartFailure> {
         let connection = drive(Connection::system(), deadline)?.map_err(map_bus_error)?;
@@ -202,28 +334,7 @@ impl RestartTransport for SystemdBus {
         configured_unit: &str,
         deadline: RestartDeadline,
     ) -> Result<String, RestartFailure> {
-        let manager = self
-            .manager
-            .as_ref()
-            .ok_or(RestartFailure::UnitResolutionFailed)?;
-        let connection = self
-            .connection
-            .as_ref()
-            .ok_or(RestartFailure::BusDisconnected)?;
-        let unit_path: zbus::zvariant::OwnedObjectPath =
-            drive(manager.call("GetUnit", &(configured_unit,)), deadline)?
-                .map_err(map_unit_resolution_error)?;
-        let unit = drive(
-            Proxy::new_owned(
-                connection.clone(),
-                SYSTEMD_NAME,
-                unit_path,
-                "org.freedesktop.systemd1.Unit",
-            ),
-            deadline,
-        )?
-        .map_err(map_unit_resolution_error)?;
-        drive(unit.get_property::<String>("Id"), deadline)?.map_err(map_unit_resolution_error)
+        resolve_loaded_unit(self, configured_unit, deadline)
     }
 
     fn restart_unit(
@@ -348,6 +459,10 @@ impl RestartTransport for SystemdBus {
     }
 }
 
+fn unit_is_loaded(load_state: &str) -> bool {
+    load_state == "loaded"
+}
+
 fn map_job_result(result: &str) -> JobResult {
     match result {
         "done" => JobResult::Done,
@@ -624,6 +739,172 @@ mod tests {
         assert!(fake.deadlines.iter().all(|(_, passed)| *passed == deadline));
     }
 
+    #[test]
+    fn systemd_load_states_allow_only_loaded_units_to_reach_restart() {
+        assert!(unit_is_loaded("loaded"));
+        for state in ["not-found", "bad-setting", "error", "masked", "stub"] {
+            assert!(!unit_is_loaded(state), "unexpected load state: {state}");
+        }
+    }
+
+    struct FakeUnitLoadApi {
+        calls: Vec<String>,
+        configured_unit_seen: Option<String>,
+        load_state: Result<String, RestartFailure>,
+        canonical_id: Result<String, RestartFailure>,
+        load_unit_result: Result<String, RestartFailure>,
+    }
+
+    impl UnitLoadApi for FakeUnitLoadApi {
+        type UnitPath = String;
+
+        fn load_unit(
+            &mut self,
+            configured_unit: &str,
+            _deadline: RestartDeadline,
+        ) -> Result<Self::UnitPath, RestartFailure> {
+            self.calls.push("LoadUnit".to_owned());
+            self.configured_unit_seen = Some(configured_unit.to_owned());
+            self.load_unit_result.clone()
+        }
+
+        fn load_state(
+            &mut self,
+            unit_path: &Self::UnitPath,
+            _deadline: RestartDeadline,
+        ) -> Result<String, RestartFailure> {
+            self.calls.push(format!("LoadState:{unit_path}"));
+            self.load_state.clone()
+        }
+
+        fn canonical_id(
+            &mut self,
+            unit_path: &Self::UnitPath,
+            _deadline: RestartDeadline,
+        ) -> Result<String, RestartFailure> {
+            self.calls.push(format!("Id:{unit_path}"));
+            self.canonical_id.clone()
+        }
+    }
+
+    impl FakeUnitLoadApi {
+        fn loaded(unit_path: &str, canonical_id: &str) -> Self {
+            Self {
+                calls: Vec::new(),
+                configured_unit_seen: None,
+                load_state: Ok("loaded".to_owned()),
+                canonical_id: Ok(canonical_id.to_owned()),
+                load_unit_result: Ok(unit_path.to_owned()),
+            }
+        }
+    }
+
+    #[test]
+    fn production_unit_resolver_loads_unloaded_alias_and_returns_canonical_id() {
+        let mut resolver = FakeUnitLoadApi::loaded(
+            "/org/freedesktop/systemd1/unit/wg_2dquick_40wg0_2eservice",
+            "wg-quick@wg0.service",
+        );
+
+        let canonical = resolve_loaded_unit(&mut resolver, "wireguard-vpn.service", deadline())
+            .expect("LoadUnit loads existing unloaded alias");
+
+        assert_eq!(
+            resolver.configured_unit_seen.as_deref(),
+            Some("wireguard-vpn.service")
+        );
+        assert_eq!(canonical, "wg-quick@wg0.service");
+        assert_eq!(
+            resolver.calls,
+            [
+                "LoadUnit",
+                "LoadState:/org/freedesktop/systemd1/unit/wg_2dquick_40wg0_2eservice",
+                "Id:/org/freedesktop/systemd1/unit/wg_2dquick_40wg0_2eservice",
+            ]
+        );
+    }
+
+    #[test]
+    fn production_unit_resolver_stops_for_missing_units_and_access_denial() {
+        let mut missing = FakeUnitLoadApi::loaded("/unit/missing", "unused");
+        missing.load_state = Ok("not-found".to_owned());
+        assert_eq!(
+            resolve_loaded_unit(&mut missing, "missing.service", deadline()),
+            Err(RestartFailure::UnitResolutionFailed)
+        );
+        assert_eq!(missing.calls, ["LoadUnit", "LoadState:/unit/missing"]);
+
+        let mut load_error = FakeUnitLoadApi::loaded("/unit/unused", "unused");
+        load_error.load_unit_result = Err(RestartFailure::UnitResolutionFailed);
+        assert_eq!(
+            resolve_loaded_unit(&mut load_error, "absent.service", deadline()),
+            Err(RestartFailure::UnitResolutionFailed)
+        );
+        assert_eq!(load_error.calls, ["LoadUnit"]);
+
+        let mut denied = FakeUnitLoadApi::loaded("/unit/denied", "unused");
+        denied.load_unit_result = Err(RestartFailure::PermissionDenied);
+        assert_eq!(
+            resolve_loaded_unit(&mut denied, "restricted.service", deadline()),
+            Err(RestartFailure::PermissionDenied)
+        );
+        assert_eq!(denied.calls, ["LoadUnit"]);
+    }
+
+    #[test]
+    fn alias_resolution_and_access_denial_stop_before_restart() {
+        let mut alias = FakeBus {
+            configured_unit: "wireguard-vpn.service",
+            primary_unit: "wg-quick@wg0.service",
+            job_path: "/job/alias",
+            events: VecDeque::from([removed(
+                "/job/alias",
+                "wg-quick@wg0.service",
+                JobResult::Done,
+            )]),
+            ..FakeBus::default()
+        };
+        let execution =
+            restart_with_transport_report(&mut alias, "wireguard-vpn.service", deadline());
+        assert_eq!(execution.outcome, RestartOutcome::Succeeded);
+        assert_eq!(execution.phase, RestartPhase::WaitForJob);
+        assert_eq!(
+            alias.calls[0..4],
+            ["connect", "subscribe", "resolve", "restart"]
+        );
+
+        let mut denied = FakeBus {
+            failure: Some(("resolve", RestartFailure::PermissionDenied)),
+            ..FakeBus::default()
+        };
+        let denied_unit = denied.configured_unit;
+        let execution = restart_with_transport_report(&mut denied, denied_unit, deadline());
+        assert_eq!(
+            execution.outcome,
+            RestartOutcome::Failed(RestartFailure::PermissionDenied)
+        );
+        assert_eq!(execution.phase, RestartPhase::ResolveUnit);
+        assert!(!denied.calls.contains(&"restart"));
+    }
+
+    #[test]
+    fn owner_change_while_restart_reply_is_pending_is_reported_as_submitted() {
+        let mut fake = FakeBus {
+            failure: Some(("restart", RestartFailure::ManagerOwnerChanged)),
+            ..FakeBus::default()
+        };
+
+        let execution =
+            restart_with_transport_report(&mut fake, "wg-quick@wg0.service", deadline());
+
+        assert_eq!(
+            execution.outcome,
+            RestartOutcome::Failed(RestartFailure::ManagerOwnerChanged)
+        );
+        assert_eq!(execution.phase, RestartPhase::RequestRestart);
+        assert!(execution.submitted);
+    }
+
     #[test]
     fn expired_shared_deadline_stops_between_setup_and_submission() {
         let mut fake = FakeBus {