Radish alpha
r
rad:zwTxygwuz5LDGBq255RA2CbNGrz8
Radicle CI broker
Radicle
Git
feat: allow configuring adapters in the `cib` configuration file
Lars Wirzenius committed 10 months ago
commit d1db5e96ac29aae0a8a947fa6b1e7a6299368067
parent e8285d7
4 files changed +195 -18
modified ci-broker.md
@@ -49,8 +49,11 @@ queue_len_interval: 1min
adapters:
  mcadapterface:
    command: ./adapter.sh
+
    config:
+
      foo: bar
+
    config_env: RADICLE_NATIVE_CI
    env:
-
      RADICLE_NATIVE_CI: native-ci.yaml
+
      PATH: /bin
    sensitive_env:
      API_KEY: xyzzy
filters:
@@ -127,7 +130,15 @@ set -eu
cat > /dev/null
echo '{"response":"triggered","run_id":{"id":"xyzzy"}}'
echo '{"response":"finished","result":"success"}'
-
echo "This is an adapter error: Mordor" 1>&2
+
(
+
echo "This is an adapter error: Mordor" 
+
echo "Environment:"
+
env
+
if [ "${RADICLE_NATIVE_CI:-}" != "" ]; then
+
    echo "Adapter config:"
+
    nl "$RADICLE_NATIVE_CI"
+
fi
+
) 1>&2
~~~

# Custom scenario steps
@@ -326,6 +337,25 @@ then command is successful
~~~


+
## Shows adapter specification
+

+
_Want:_ The CI broker can write out the specification for an adapter.
+

+
_Why:_ This is helpful for the node operator to verify that
+
they have specified the adapter correctly.
+

+
_Who:_ `cib-devs`
+

+
~~~scenario
+
given a Radicle node, with CI configured with broker.yaml and adapter dummy.sh
+
when I run cib --config broker.yaml adapters --output adapters.json
+
when I run jq . adapters.json
+
then command is successful
+
then stdout contains ""foo": "bar""
+
then stdout contains ""config_env": "RADICLE_NATIVE_CI""
+
~~~
+

+

## Refuses config with an unknown field

_Want:_ The CI broker refused to load a configuration file that has
@@ -395,6 +425,37 @@ then stdout contains ""id": "xyzzy""
~~~


+
## Runs adapter with configuration
+

+
_Want:_ CI broker can run its adapter and give it the configuration in
+
the CI broker adapter specification.
+

+
_Why:_ Being able to embed the adapter configuration in the `cib`
+
configuration file makes is more convenient for the node operators to
+
specify different adapter configurations for different purposes.
+

+
_Who:_ `node-ops`
+

+
~~~scenario
+
given a Radicle node, with CI configured with broker.yaml and adapter dummy.sh
+
given a Git repository xyzzy in the Radicle node
+
given the Radicle node emits a refsUpdated event for xyzzy
+
when I run ./env.sh synthetic-events synt.sock event.json --log log.txt
+
given a directory reports
+
when I run ./env.sh cib --config broker.yaml process-events
+

+
then stderr contains "RADICLE_NATIVE_CI="
+
then stderr contains "foo: bar"
+
then stderr contains "API_KEY: xyzzy"
+

+
when I run cibtool --db ci-broker.db event list
+
then stdout is empty
+

+
when I run cibtool --db ci-broker.db run list --json
+
then stdout contains ""id": "xyzzy""
+
~~~
+

+

## Runs adapter without a report directory

_Want:_ CI broker can run without a report directory.
modified src/adapter.rs
@@ -16,10 +16,11 @@ use std::{
    time::Duration,
};

-
//use tracing::Instrument;
+
use serde::Serialize;
+
use tempfile::{tempdir, TempDir};

use crate::{
-
    config::AdapterConfig,
+
    config::AdapterSpec,
    db::{Db, DbError},
    logger,
    msg::{MessageError, Request, Response},
@@ -30,7 +31,7 @@ use crate::{
};

/// The set of all configured adapters.
-
#[derive(Clone)]
+
#[derive(Clone, Serialize)]
pub struct Adapters {
    adapters: HashMap<String, Adapter>,
    default_adapter: Option<String>,
@@ -38,7 +39,7 @@ pub struct Adapters {

impl Adapters {
    pub fn new(
-
        adapters: &HashMap<String, AdapterConfig>,
+
        adapters: &HashMap<String, AdapterSpec>,
        default_adapter: Option<&str>,
    ) -> Result<Self, AdapterError> {
        if let Some(default) = default_adapter {
@@ -68,13 +69,19 @@ impl Adapters {
    pub fn get(&self, name: &str) -> Option<&Adapter> {
        self.adapters.get(name)
    }
+

+
    pub fn to_json(&self) -> Result<String, AdapterError> {
+
        serde_json::to_string_pretty(self).map_err(AdapterError::AdaptersToJson)
+
    }
}

/// An external executable that runs CI on request.
-
#[derive(Debug, Default, Clone, Eq, PartialEq)]
+
#[derive(Debug, Default, Clone, Eq, PartialEq, Serialize)]
pub struct Adapter {
    bin: PathBuf,
    env: HashMap<String, String>,
+
    config: HashMap<String, serde_yml::Value>,
+
    config_env: Option<String>,
}

impl Adapter {
@@ -82,6 +89,7 @@ impl Adapter {
        Self {
            bin: bin.into(),
            env: HashMap::new(),
+
            ..Default::default()
        }
    }

@@ -99,6 +107,25 @@ impl Adapter {
        self
    }

+
    pub fn with_config(mut self, config: HashMap<String, serde_yml::Value>) -> Self {
+
        self.config = config;
+
        self
+
    }
+

+
    pub fn with_config_env(mut self, config_env: &str) -> Self {
+
        self.config_env = Some(config_env.to_string());
+
        self
+
    }
+

+
    fn write_adapter_config(&self, tmpdir: &TempDir) -> Result<PathBuf, AdapterError> {
+
        let filename = tmpdir.path().join("adapter.yaml");
+

+
        let json = serde_yml::to_string(self).map_err(AdapterError::AdapterConfigToYaml)?;
+
        std::fs::write(&filename, json.as_bytes()).map_err(AdapterError::AdapterConfigWrite)?;
+

+
        Ok(filename)
+
    }
+

    fn envs(&self) -> impl Iterator<Item = (&OsStr, &OsStr)> {
        self.env.iter().map(|(k, v)| (k.as_ref(), v.as_ref()))
    }
@@ -132,9 +159,19 @@ impl Adapter {
    ) -> Result<(), AdapterError> {
        assert!(matches!(trigger, Request::Trigger { .. }));

-
        // Spawn the adapter sub-process.
+
        let tmp = tempdir().map_err(AdapterError::AdapterConfigDir)?;
+

+
        // Start setting up the command to run the adapter sub-process.
        let mut cmd = Command::new(&self.bin);
        cmd.envs(self.envs());
+

+
        // Write adapter config file, if requested.
+
        if let Some(config_env) = &self.config_env {
+
            let filename = self.write_adapter_config(&tmp)?;
+
            cmd.env(config_env, filename);
+
        }
+

+
        // Spawn the adapter sub-process.
        let mut child = TimeoutCommand::new(max_run_time);
        let stdout = child.stdout();
        child.feed_stdin(trigger.to_string().as_bytes());
@@ -282,6 +319,10 @@ pub enum AdapterError {
    #[error("the default adapter is not defined in the configuration")]
    NoDefaultAdapter,

+
    /// Can't serialize [`Adapters`] to JSON.
+
    #[error("failed to serialize adapters as JSON")]
+
    AdaptersToJson(#[source] serde_json::Error),
+

    /// Error from [`TimeoutCommand`] or [`RunningProcess`].
    #[error(transparent)]
    TimeoutCommand(#[from] crate::timeoutcmd::TimeoutError),
@@ -360,6 +401,18 @@ pub enum AdapterError {
    /// Could not send notification of changes to CI runs.
    #[error(transparent)]
    Notif(#[from] crate::notif::NotificationError),
+

+
    /// Can't create temp directory for adapter config.
+
    #[error("failed to create temporary directory for adapter configuration")]
+
    AdapterConfigDir(#[source] std::io::Error),
+

+
    /// Can't serialize adapter configuration to YAML.
+
    #[error("can't convert adapter configuration to YAML")]
+
    AdapterConfigToYaml(#[source] serde_yml::Error),
+

+
    /// Can't write adapter config.
+
    #[error("failed to write adapter configuration")]
+
    AdapterConfigWrite(#[source] std::io::Error),
}

#[cfg(test)]
modified src/bin/cib.rs
@@ -75,6 +75,7 @@ struct Args {
impl Args {
    fn run(&self, config: &Config) -> Result<(), CibError> {
        match &self.cmd {
+
            Cmd::Adapters(x) => x.run(self, config)?,
            Cmd::Config(x) => x.run(self, config)?,
            Cmd::Insert(x) => x.run(self, config)?,
            Cmd::Queued(x) => x.run(self, config)?,
@@ -101,12 +102,40 @@ enum Cmd {
    /// The configuration file location is specified with the
    /// `--config` option.
    Config(ConfigCmd),
+
    Adapters(AdaptersCmd),
    Insert(InsertCmd),
    Queued(QueuedCmd),
    ProcessEvents(ProcessEventsCmd),
}

#[derive(Debug, Parser)]
+
struct AdaptersCmd {
+
    /// Write output to this file. The default is to write to the
+
    /// standard output. This option is an alternative to redirecting
+
    /// output using shell constructs, for situations when that is not
+
    /// an option.
+
    #[clap(long)]
+
    output: Option<PathBuf>,
+
}
+

+
impl AdaptersCmd {
+
    fn run(&self, args: &Args, config: &Config) -> Result<(), CibError> {
+
        let adapters = config.to_adapters().map_err(CibError::GetAdapters)?;
+
        let json = adapters
+
            .to_json()
+
            .map_err(|e| CibError::adapters_to_json(&args.config, e))?;
+

+
        if let Some(output) = &self.output {
+
            write(output, json.as_bytes()).map_err(|e| CibError::write_config(output, e))?;
+
        } else {
+
            println!("{json}");
+
        }
+

+
        Ok(())
+
    }
+
}
+

+
#[derive(Debug, Parser)]
struct ConfigCmd {
    /// Write output to this file. The default is to write to the
    /// standard output. This option is an alternative to redirecting
@@ -120,7 +149,7 @@ impl ConfigCmd {
    fn run(&self, args: &Args, config: &Config) -> Result<(), CibError> {
        let json = config
            .to_json()
-
            .map_err(|e| CibError::to_json(&args.config, e))?;
+
            .map_err(|e| CibError::config_to_json(&args.config, e))?;

        if let Some(output) = &self.output {
            write(output, json.as_bytes()).map_err(|e| CibError::write_config(output, e))?;
@@ -308,7 +337,10 @@ enum CibError {
    WriteConfig(PathBuf, #[source] std::io::Error),

    #[error("failed to convert config as JSON")]
-
    ToJson(PathBuf, #[source] ConfigError),
+
    ConfigToJson(PathBuf, #[source] ConfigError),
+

+
    #[error("failed to convert adapters as JSON")]
+
    AdaptersToJson(PathBuf, #[source] AdapterError),

    #[error("failed to look up node profile")]
    Profile(#[source] radicle::profile::Error),
@@ -334,6 +366,9 @@ enum CibError {
    #[error("could not construct list of adapters from configuration")]
    Adapters(#[source] AdapterError),

+
    #[error("could not get list of adapters from configuration")]
+
    GetAdapters(#[source] AdapterError),
+

    #[error("programming error: failed to set up inter-thread notification channel")]
    Notification(#[source] NotificationError),
}
@@ -347,8 +382,12 @@ impl CibError {
        Self::WriteConfig(filename.into(), e)
    }

-
    fn to_json(filename: &Path, e: ConfigError) -> Self {
-
        Self::ToJson(filename.into(), e)
+
    fn config_to_json(filename: &Path, e: ConfigError) -> Self {
+
        Self::ConfigToJson(filename.into(), e)
+
    }
+

+
    fn adapters_to_json(filename: &Path, e: AdapterError) -> Self {
+
        Self::AdaptersToJson(filename.into(), e)
    }

    fn profile(e: radicle::profile::Error) -> Self {
modified src/config.rs
@@ -23,7 +23,7 @@ const DEFAULT_STATUS_PAGE_UPDATE_INTERVAL: u64 = 10;
#[serde(deny_unknown_fields)]
pub struct Config {
    default_adapter: Option<String>,
-
    adapters: HashMap<String, AdapterConfig>,
+
    adapters: HashMap<String, AdapterSpec>,
    filters: Option<Vec<EventFilter>>,
    triggers: Option<Vec<TriggerConfig>>,
    report_dir: Option<PathBuf>,
@@ -122,7 +122,8 @@ impl Config {
}

#[derive(Debug, Serialize, Deserialize)]
-
pub struct AdapterConfig {
+
#[serde(deny_unknown_fields)]
+
pub struct AdapterSpec {
    pub command: PathBuf,

    #[serde(default)]
@@ -130,9 +131,26 @@ pub struct AdapterConfig {

    #[serde(default)]
    sensitive_env: HashMap<String, Sensitive>,
+

+
    /// Configuration for the adapter. If the `Self::config_env` field
+
    /// is set, the configuration is serialize into YAML and written
+
    /// to a temporary file. The environment variable named in
+
    /// `config_env` is set to the path of the temporary file.
+
    #[serde(default)]
+
    config: HashMap<String, serde_yml::Value>,
+

+
    config_env: Option<String>,
}

-
impl AdapterConfig {
+
impl AdapterSpec {
+
    pub fn config_env(&self) -> Option<&str> {
+
        self.config_env.as_deref()
+
    }
+

+
    pub fn config(&self) -> &HashMap<String, serde_yml::Value> {
+
        &self.config
+
    }
+

    pub fn envs(&self) -> &HashMap<String, String> {
        &self.env
    }
@@ -142,11 +160,17 @@ impl AdapterConfig {
    }
}

-
impl From<&AdapterConfig> for Adapter {
-
    fn from(config: &AdapterConfig) -> Self {
-
        Self::new(&config.command)
+
impl From<&AdapterSpec> for Adapter {
+
    fn from(config: &AdapterSpec) -> Self {
+
        let adapter = Self::new(&config.command)
            .with_environment(config.envs())
            .with_sensitive_environment(config.sensitive_envs())
+
            .with_config(config.config.clone());
+
        if let Some(x) = &config.config_env {
+
            adapter.with_config_env(x)
+
        } else {
+
            adapter
+
        }
    }
}