policy-engine: add TOML configuration for policy plugins

cincinnati/src/plugins/catalog.rs

@@ -7,7 +7,7 @@ use super::internal::channel_filter::ChannelFilterPlugin;
 use super::internal::edge_add_remove::EdgeAddRemovePlugin;
 use super::internal::metadata_fetch_quay::QuayMetadataFetchPlugin;
 use super::internal::node_remove::NodeRemovePlugin;
-use super::{Plugin, PluginIO};
+use crate::plugins::BoxedPlugin;
 use failure::Fallible;
 use std::fmt::Debug;
 
@@ -17,7 +17,7 @@ static CONFIG_PLUGIN_NAME_KEY: &str = "name";
 /// Settings for a plugin.
 pub trait PluginSettings: Debug + Send {
     /// Build the corresponding plugin for this configuration.
-    fn build_plugin(&self) -> Fallible<Box<Plugin<PluginIO>>>;
+    fn build_plugin(&self) -> Fallible<BoxedPlugin>;
 }
 
 /// Validate configuration for a plugin and fill in defaults.

cincinnati/src/plugins/internal/channel_filter.rs

@@ -3,7 +3,7 @@
 //! and the value must match the regex specified at CHANNEL_VALIDATION_REGEX_STR
 
 use crate::plugins::{
-    InternalIO, InternalPlugin, InternalPluginWrapper, Plugin, PluginIO, PluginSettings,
+    BoxedPlugin, InternalIO, InternalPlugin, InternalPluginWrapper, PluginSettings,
 };
 use failure::Fallible;
 
@@ -21,7 +21,7 @@ pub struct ChannelFilterPlugin {
 }
 
 impl PluginSettings for ChannelFilterPlugin {
-    fn build_plugin(&self) -> Fallible<Box<Plugin<PluginIO>>> {
+    fn build_plugin(&self) -> Fallible<BoxedPlugin> {
         Ok(Box::new(InternalPluginWrapper(self.clone())))
     }
 }

cincinnati/src/plugins/internal/edge_add_remove.rs

@@ -2,7 +2,7 @@
 
 use crate as cincinnati;
 use crate::plugins::{
-    InternalIO, InternalPlugin, InternalPluginWrapper, Plugin, PluginIO, PluginSettings,
+    BoxedPlugin, InternalIO, InternalPlugin, InternalPluginWrapper, PluginSettings,
 };
 use crate::ReleaseId;
 use failure::Fallible;
@@ -33,7 +33,7 @@ impl InternalPlugin for EdgeAddRemovePlugin {
 }
 
 impl PluginSettings for EdgeAddRemovePlugin {
-    fn build_plugin(&self) -> Fallible<Box<Plugin<PluginIO>>> {
+    fn build_plugin(&self) -> Fallible<BoxedPlugin> {
         Ok(Box::new(InternalPluginWrapper(self.clone())))
     }
 }

cincinnati/src/plugins/internal/metadata_fetch_quay.rs

@@ -10,7 +10,7 @@ extern crate tokio;
 
 use self::futures::future::Future;
 use crate::plugins::{
-    InternalIO, InternalPlugin, InternalPluginWrapper, Plugin, PluginIO, PluginSettings,
+    BoxedPlugin, InternalIO, InternalPlugin, InternalPluginWrapper, PluginSettings,
 };
 use crate::ReleaseId;
 use failure::{Fallible, ResultExt};
@@ -50,7 +50,7 @@ pub struct QuayMetadataFetchPlugin {
 }
 
 impl PluginSettings for QuayMetadataSettings {
-    fn build_plugin(&self) -> Fallible<Box<Plugin<PluginIO>>> {
+    fn build_plugin(&self) -> Fallible<BoxedPlugin> {
         let cfg = self.clone();
         let plugin = QuayMetadataFetchPlugin::try_new(
             cfg.repository,

cincinnati/src/plugins/internal/node_remove.rs

@@ -1,7 +1,7 @@
 //! This plugin removes releases according to its metadata
 
 use crate::plugins::{
-    InternalIO, InternalPlugin, InternalPluginWrapper, Plugin, PluginIO, PluginSettings,
+    BoxedPlugin, InternalIO, InternalPlugin, InternalPluginWrapper, PluginSettings,
 };
 use failure::Fallible;
 
@@ -15,7 +15,7 @@ pub struct NodeRemovePlugin {
 }
 
 impl PluginSettings for NodeRemovePlugin {
-    fn build_plugin(&self) -> Fallible<Box<Plugin<PluginIO>>> {
+    fn build_plugin(&self) -> Fallible<BoxedPlugin> {
         Ok(Box::new(InternalPluginWrapper(self.clone())))
     }
 }

cincinnati/src/plugins/mod.rs

@@ -17,6 +17,18 @@ use std::collections::HashMap;
 use std::fmt::Debug;
 use try_from::{TryFrom, TryInto};
 
+/// Type for storing policy plugins in applications.
+pub type BoxedPlugin = Box<Plugin<PluginIO> + Sync + Send>;
+
+// NOTE(lucab): this abuses `Debug`, because `PartialEq` is not object-safe and
+// thus cannot be required on the underlying trait. It is a crude hack, but
+// only meant to be used by test assertions.
+impl PartialEq<BoxedPlugin> for BoxedPlugin {
+    fn eq(&self, other: &Self) -> bool {
+        format!("{:?}", self) == format!("{:?}", other)
+    }
+}
+
 /// Enum for the two IO variants used by InternalPlugin and ExternalPlugin respectively
 #[derive(Debug)]
 pub enum PluginIO {

commons/src/config.rs

@@ -13,12 +13,12 @@ macro_rules! assign_if_some {
     }};
 }
 
-/// Merge configuration options into runtime settings.
+/// Try to merge configuration options into runtime settings.
 ///
-/// This consumes a generic configuration object, merging its options
+/// This consumes a generic configuration object, trying to merge its options
 /// into runtime settings. It only overlays populated values from config,
 /// leaving unset ones preserved as-is from existing settings.
 pub trait MergeOptions<T> {
     /// MergeOptions values from `options` into current settings.
-    fn merge(&mut self, options: T);
+    fn try_merge(&mut self, options: T) -> failure::Fallible<()>;
 }

graph-builder/src/config/cli.rs

@@ -3,6 +3,7 @@
 use super::options;
 use super::AppSettings;
 use commons::MergeOptions;
+use failure::Fallible;
 
 /// CLI configuration flags, top-level.
 #[derive(Debug, StructOpt)]
@@ -34,22 +35,24 @@ pub struct CliOptions {
 }
 
 impl MergeOptions<CliOptions> for AppSettings {
-    fn merge(&mut self, opts: CliOptions) {
+    fn try_merge(&mut self, opts: CliOptions) -> Fallible<()> {
         self.verbosity = match opts.verbosity {
             0 => self.verbosity,
             1 => log::LevelFilter::Info,
             2 => log::LevelFilter::Debug,
             _ => log::LevelFilter::Trace,
         };
-        self.merge(Some(opts.service));
-        self.merge(Some(opts.status));
-        self.merge(Some(opts.upstream_registry));
+        self.try_merge(Some(opts.service))?;
+        self.try_merge(Some(opts.status))?;
+        self.try_merge(Some(opts.upstream_registry))?;
 
         // TODO(lucab): drop this when plugins are configurable.
         assign_if_some!(
             self.disable_quay_api_metadata,
             opts.disable_quay_api_metadata
         );
+
+        Ok(())
     }
 }
 
@@ -87,7 +90,7 @@ mod tests {
         let cli = CliOptions::from_iter_safe(args).unwrap();
         assert_eq!(cli.upstream_registry.repository, Some(repo.to_string()));
 
-        settings.merge(cli);
+        settings.try_merge(cli).unwrap();
         assert_eq!(settings.repository, repo.to_string());
     }
 
@@ -102,14 +105,14 @@ mod tests {
         let file_opts: FileOptions = toml::from_str(toml_verbosity).unwrap();
         assert_eq!(file_opts.verbosity, Some(log::LevelFilter::Trace));
 
-        settings.merge(Some(file_opts));
+        settings.try_merge(Some(file_opts)).unwrap();
         assert_eq!(settings.verbosity, log::LevelFilter::Trace);
 
         let args = vec!["argv0", "-vv"];
         let cli_opts = CliOptions::from_iter_safe(args).unwrap();
         assert_eq!(cli_opts.verbosity, 2);
 
-        settings.merge(cli_opts);
+        settings.try_merge(cli_opts).unwrap();
         assert_eq!(settings.verbosity, log::LevelFilter::Debug);
     }
 }

graph-builder/src/config/file.rs

@@ -48,13 +48,14 @@ impl FileOptions {
 }
 
 impl MergeOptions<Option<FileOptions>> for AppSettings {
-    fn merge(&mut self, opts: Option<FileOptions>) {
+    fn try_merge(&mut self, opts: Option<FileOptions>) -> Fallible<()>{
         if let Some(file) = opts {
             assign_if_some!(self.verbosity, file.verbosity);
-            self.merge(file.service);
-            self.merge(file.status);
-            self.merge(file.upstream);
+            self.try_merge(file.service)?;
+            self.try_merge(file.status)?;
+            self.try_merge(file.upstream)?;
         }
+        Ok(())
     }
 }
 
@@ -69,10 +70,11 @@ pub struct UpstreamOptions {
 }
 
 impl MergeOptions<Option<UpstreamOptions>> for AppSettings {
-    fn merge(&mut self, opts: Option<UpstreamOptions>) {
+    fn try_merge(&mut self, opts: Option<UpstreamOptions>) -> Fallible<()> {
         if let Some(upstream) = opts {
-            self.merge(upstream.registry);
+            self.try_merge(upstream.registry)?;
         }
+        Ok(())
     }
 }
 
@@ -105,7 +107,7 @@ mod tests {
         let toml_input = "status.port = 2222";
         let file_opts: FileOptions = toml::from_str(toml_input).unwrap();
 
-        settings.merge(Some(file_opts));
+        settings.try_merge(Some(file_opts)).unwrap();
         assert_eq!(settings.status_port, 2222);
     }
 

graph-builder/src/config/options.rs

@@ -2,6 +2,7 @@
 
 use super::AppSettings;
 use commons::{parse_params_set, parse_path_prefix, MergeOptions};
+use failure::Fallible;
 use std::collections::HashSet;
 use std::net::IpAddr;
 use std::path::PathBuf;
@@ -77,7 +78,7 @@ pub struct DockerRegistryOptions {
 }
 
 impl MergeOptions<Option<ServiceOptions>> for AppSettings {
-    fn merge(&mut self, opts: Option<ServiceOptions>) {
+    fn try_merge(&mut self, opts: Option<ServiceOptions>) -> Fallible<()> {
         if let Some(service) = opts {
             assign_if_some!(self.address, service.address);
             assign_if_some!(self.port, service.port);
@@ -86,27 +87,30 @@ impl MergeOptions<Option<ServiceOptions>> for AppSettings {
                 self.mandatory_client_parameters.extend(params);
             }
         }
+        Ok(())
     }
 }
 
 impl MergeOptions<Option<StatusOptions>> for AppSettings {
-    fn merge(&mut self, opts: Option<StatusOptions>) {
+    fn try_merge(&mut self, opts: Option<StatusOptions>) -> Fallible<()> {
         if let Some(status) = opts {
             assign_if_some!(self.status_address, status.address);
             assign_if_some!(self.status_port, status.port);
         }
+        Ok(())
     }
 }
 
 impl MergeOptions<Option<DockerRegistryOptions>> for AppSettings {
-    fn merge(&mut self, opts: Option<DockerRegistryOptions>) {
+    fn try_merge(&mut self, opts: Option<DockerRegistryOptions>) -> Fallible<()> {
         if let Some(registry) = opts {
             assign_if_some!(self.pause_secs, registry.pause_secs);
             assign_if_some!(self.registry, registry.url);
             assign_if_some!(self.repository, registry.repository);
             assign_if_some!(self.credentials_path, registry.credentials_path);
             assign_if_some!(self.manifestref_key, registry.manifestref_key);
         }
+        Ok(())
     }
 }
 

graph-builder/src/config/settings.rs

@@ -76,8 +76,8 @@ impl AppSettings {
 
         // Combine options into a single config.
         let mut cfg = defaults;
-        cfg.merge(cli_opts);
-        cfg.merge(file_opts);
+        cfg.try_merge(cli_opts)?;
+        cfg.try_merge(file_opts)?;
 
         // Validate and convert to settings.
         Self::try_validate(cfg)

policy-engine/src/config/cli.rs

@@ -3,6 +3,7 @@
 use super::options;
 use super::AppSettings;
 use commons::MergeOptions;
+use failure::Fallible;
 
 /// CLI configuration flags, top-level.
 #[derive(Debug, StructOpt)]
@@ -33,16 +34,19 @@ pub struct CliOptions {
 }
 
 impl MergeOptions<CliOptions> for AppSettings {
-    fn merge(&mut self, opts: CliOptions) {
+    fn try_merge(&mut self, opts: CliOptions) -> Fallible<()> {
         self.verbosity = match opts.verbosity {
             0 => self.verbosity,
             1 => log::LevelFilter::Info,
             2 => log::LevelFilter::Debug,
             _ => log::LevelFilter::Trace,
         };
-        self.merge(Some(opts.service));
-        self.merge(Some(opts.status));
-        self.merge(Some(opts.upstream_cincinnati));
+
+        self.try_merge(Some(opts.service))?;
+        self.try_merge(Some(opts.status))?;
+        self.try_merge(Some(opts.upstream_cincinnati))?;
+
+        Ok(())
     }
 }
 
@@ -84,13 +88,14 @@ mod tests {
         let cli = CliOptions::from_iter_safe(args).unwrap();
         assert_eq!(cli.upstream_cincinnati.url, Some(up_url.clone()));
 
-        settings.merge(cli);
+        settings.try_merge(cli).unwrap();
         assert_eq!(settings.upstream, up_url);
     }
 
     #[test]
     fn cli_override_toml() {
         use crate::config::file::FileOptions;
+        use commons::MergeOptions;
 
         let mut settings = AppSettings::default();
         assert_eq!(settings.verbosity, log::LevelFilter::Warn);
@@ -99,14 +104,14 @@ mod tests {
         let file_opts: FileOptions = toml::from_str(toml_verbosity).unwrap();
         assert_eq!(file_opts.verbosity, Some(log::LevelFilter::Trace));
 
-        settings.merge(Some(file_opts));
+        settings.try_merge(Some(file_opts)).unwrap();
         assert_eq!(settings.verbosity, log::LevelFilter::Trace);
 
         let args = vec!["argv0", "-vv"];
         let cli_opts = CliOptions::from_iter_safe(args).unwrap();
         assert_eq!(cli_opts.verbosity, 2);
 
-        settings.merge(cli_opts);
+        settings.try_merge(cli_opts).unwrap();
         assert_eq!(settings.verbosity, log::LevelFilter::Debug);
     }
 }