From 7e1dc8b54fe9864d688b79375e25d293847edac6 Mon Sep 17 00:00:00 2001
From: James Prestwich <10149425+prestwich@users.noreply.github.com>
Date: Thu, 14 Jul 2022 09:43:20 -0700
Subject: [PATCH] fix: prevent processor from attempting to run an empty
 select_all (#211)

* feature: check that replicas is non-empty

* bug: filter subsidized remotes

* fix: prevent select_all from being called with empty specified subsidized

* chore: update changelog with bugfixes
---
 agents/processor/CHANGELOG.md     |  1 +
 agents/processor/src/processor.rs | 23 +++++++++++++++--------
 nomad-base/CHANGELOG.md           |  2 ++
 nomad-base/src/agent.rs           | 11 +++++++++++
 4 files changed, 29 insertions(+), 8 deletions(-)

diff --git a/agents/processor/CHANGELOG.md b/agents/processor/CHANGELOG.md
index 9c7c546a..0f0d593f 100644
--- a/agents/processor/CHANGELOG.md
+++ b/agents/processor/CHANGELOG.md
@@ -2,6 +2,7 @@
 
 ### Unreleased
 
+- bug: add check for empty intersection of specified and subsidized
 - refactor: processor now uses global AWS client when proof pushing is enabled
 - prevent processor from retrying messages it has previously attempted to
   process
diff --git a/agents/processor/src/processor.rs b/agents/processor/src/processor.rs
index 9eb28fb6..c970b404 100644
--- a/agents/processor/src/processor.rs
+++ b/agents/processor/src/processor.rs
@@ -337,12 +337,21 @@ impl NomadAgent for Processor {
     where
         Self: Sized,
     {
+        // we filter this so that the agent doesn't think it should subsidize
+        // remotes it is unaware of
+        let subsidized_remotes = settings
+            .agent
+            .subsidized_remotes
+            .iter()
+            .filter(|r| settings.base.replicas.contains_key(*r))
+            .cloned()
+            .collect();
         Ok(Self::new(
             settings.agent.interval,
             settings.as_ref().try_into_core(AGENT_NAME).await?,
             settings.agent.allowed,
             settings.agent.denied,
-            settings.agent.subsidized_remotes,
+            subsidized_remotes,
             settings.agent.s3,
         ))
     }
@@ -404,20 +413,18 @@ impl NomadAgent for Processor {
             let mut tasks = vec![home_sync_task, prover_sync_task, home_fail_watch_task];
 
             if !self.subsidized_remotes.is_empty() {
-                let specified_remotes: HashSet<String> =
-                    self.replicas().keys().map(String::to_owned).collect();
-
                 // Get intersection of specified remotes (replicas in settings)
                 // and subsidized remotes
                 let specified_subsidized: Vec<&str> = self
                     .subsidized_remotes
-                    .intersection(&specified_remotes)
-                    .collect::<Vec<_>>()
                     .iter()
-                    .map(|x| x.as_str())
+                    .filter(|r| self.replicas().contains_key(*r))
+                    .map(AsRef::as_ref)
                     .collect();
 
-                tasks.push(self.run_many(&specified_subsidized));
+                if !specified_subsidized.is_empty() {
+                    tasks.push(self.run_many(&specified_subsidized));
+                }
             }
 
             // if we have a bucket, add a task to push to it
diff --git a/nomad-base/CHANGELOG.md b/nomad-base/CHANGELOG.md
index 488bb7e1..aec72fc0 100644
--- a/nomad-base/CHANGELOG.md
+++ b/nomad-base/CHANGELOG.md
@@ -2,6 +2,8 @@
 
 ### Unreleased
 
+- bug: add checks for empty replica name arrays in `NomadAgent::run_many` and
+  `NomadAgent::run_all`
 - add `previously_attempted` to the DB schema
 - remove `enabled` flag from agents project-wide
 - adds a changelog
diff --git a/nomad-base/src/agent.rs b/nomad-base/src/agent.rs
index a4ce166d..d1ac531b 100644
--- a/nomad-base/src/agent.rs
+++ b/nomad-base/src/agent.rs
@@ -167,6 +167,12 @@ pub trait NomadAgent: Send + Sync + Sized + std::fmt::Debug + AsRef<AgentCore> {
     #[allow(clippy::unit_arg)]
     fn run_many(&self, replicas: &[&str]) -> Instrumented<JoinHandle<Result<()>>> {
         let span = info_span!("run_many");
+
+        // easy check that the slice is non-empty
+        replicas
+            .first()
+            .expect("Attempted to run without any replicas");
+
         let handles: Vec<_> = replicas
             .iter()
             .map(|replica| self.run_report_error(replica.to_string()))
@@ -196,6 +202,11 @@ pub trait NomadAgent: Send + Sync + Sized + std::fmt::Debug + AsRef<AgentCore> {
             // this is the unused must use
             let names: Vec<&str> = self.replicas().keys().map(|k| k.as_str()).collect();
 
+            // quick check that at least 1 replica is configured
+            names
+                .first()
+                .expect("Attempted to run without any replicas");
+
             let run_task = self.run_many(&names);
             let mut tasks = vec![run_task];