Radish alpha
r
radicle-ci-broker
Radicle CI broker
Radicle
Git (anonymous pull)
Log in to clone via SSH
Commits ›
feat: more flexible filtering
Lars Wirzenius committed 2 years ago
commit a4047dc96833ae628262f95401ecbdbdac78bb60
parent 176235883e225baca2c65dacd8aa328e4b38d533
2 files changed +103 -7
modified doc/requirements.md
@@ -115,3 +115,34 @@ CI and a selection of external CI engines.
* Broker updates the Radicle network about CI runs.

* As much as is reasonably possible of the broker code is shared

  between different CI engines.
+
+ 
# Requirements for event filtering in the CI broker
+
+ 
The purpose of event filtering in the CI broker is to allow the node
+ 
admin, who runs the broker, useful control of what changes in the
+ 
repository trigger a CI run.
+
+ 
For all of the following use cases, "I" is the node admin.
+
+ 
* I want to only run CI on a specific repository so that I spend my CI
+ 
  resources on things that are important to me.
+ 
  - filter on repository id
+
+ 
* I want to only run CI on changes to a specific branch so that I can
+ 
  build and deliver artifacts from the branch. For example, I might
+ 
  build and publish binaries for a program, or build and update a web
+ 
  site.
+ 
  - filter on branch name
+
+ 
* I want to run CI on any proposed patches that are created or
+ 
  modified for a project so that I can provide feedback on whether the
+ 
  changes in the patch pass the project's test suite.
+ 
  - filter on any branches that refer to Radicle patches
+
+ 
* I want to run CI only on a specific patch, when it's updated, so
+ 
  that I can check that that patch is OK.
+ 
  - filter on the branch for a specific Radicle patch
+
+ 
* I want to filter on a complex condition based on a combination of
+ 
  several simpler conditions.
+ 
  - allow boolean expressions on filter conditions with AND, OR, NOT
modified src/filter.rs
@@ -47,9 +47,23 @@ pub enum EventFilter {
    /// Event concerns a git ref that ends with a given string.

    RefSuffix(String),
- 
    /// Combine two filters that both must allow the events. FIXME: Is
- 
    /// two enough? Should this be a vector that all have to allow?
- 
    And(Box<Self>, Box<Self>),
+ 
    /// Event concerns a specific git branch.
+ 
    Branch(String),
+
+ 
    /// Event concerns any Radicle patch.
+ 
    AnyPatch,
+
+ 
    /// Event concerns a specific Radicle patch.
+ 
    Patch(String),
+
+ 
    /// Combine any number of filters that both must allow the events.
+ 
    And(Vec<Box<Self>>),
+
+ 
    /// Combine any number of filters such that at least one allows the events.
+ 
    Or(Vec<Box<Self>>),
+
+ 
    /// Combine any number of filters such that none allows the events.
+ 
    Not(Vec<Box<Self>>),

}



impl EventFilter {
@@ -63,9 +77,19 @@ impl EventFilter {
        Ok(Self::RefSuffix(pattern.into()))

    }
- 
    /// Create a filter combining two other filters.
- 
    pub fn and(a: Self, b: Self) -> Self {
- 
        Self::And(Box::new(a), Box::new(b))
+ 
    /// Create a filter combining other filters with AND.
+ 
    pub fn and(conds: &[Self]) -> Self {
+ 
        Self::And(conds.iter().map(|c| Box::new(c.clone())).collect())
+ 
    }
+
+ 
    /// Create a filter combining other filters with OR.
+ 
    pub fn or(conds: &[Self]) -> Self {
+ 
        Self::Or(conds.iter().map(|c| Box::new(c.clone())).collect())
+ 
    }
+
+ 
    /// Create a filter combining other filters with NOT.
+ 
    pub fn not(conds: &[Self]) -> Self {
+ 
        Self::Not(conds.iter().map(|c| Box::new(c.clone())).collect())

    }



    /// Read filters from a JSON file.
@@ -167,7 +191,48 @@ impl BrokerEvent {
        match filter {

            EventFilter::Repository(wanted) => rid == wanted,

            EventFilter::RefSuffix(wanted) => name.ends_with(wanted),
- 
            EventFilter::And(a, b) => self.is_allowed(a) && self.is_allowed(b),
+ 
            EventFilter::Branch(wanted) => name.ends_with(&format!("/refs/heads/{}", wanted)),
+ 
            EventFilter::AnyPatch => is_patch_ref(name).is_some(),
+ 
            EventFilter::Patch(wanted) => is_patch_ref(name) == Some(wanted),
+ 
            EventFilter::And(conds) => conds.iter().all(|cond| self.is_allowed(cond)),
+ 
            EventFilter::Or(conds) => conds.iter().any(|cond| self.is_allowed(cond)),
+ 
            EventFilter::Not(conds) => !conds.iter().any(|cond| self.is_allowed(cond)),

        }

    }

}
+
+ 
fn is_patch_ref(name: &str) -> Option<&str> {
+ 
    let mut parts = name.split("/refs/heads/patches/");
+ 
    if let Some(suffix) = parts.nth(1) {
+ 
        if parts.next().is_none() {
+ 
            return Some(suffix);
+ 
        }
+ 
    }
+
+ 
    None
+ 
}
+
+ 
#[cfg(test)]
+ 
mod test {
+ 
    use super::is_patch_ref;
+
+ 
    #[test]
+ 
    fn branch_is_not_patch() {
+ 
        assert_eq!(
+ 
            is_patch_ref(
+ 
                "refs/namespaces/z6MkuhvCnrcow7vzkyQzkuFixzpTa42iC2Cfa4DA8HRLCmys/refs/heads/main"
+ 
            ),
+ 
            None
+ 
        );
+ 
    }
+
+ 
    #[test]
+ 
    fn is_patch() {
+ 
        assert_eq!(
+ 
            is_patch_ref(
+ 
                "refs/namespaces/z6MkuhvCnrcow7vzkyQzkuFixzpTa42iC2Cfa4DA8HRLCmys/refs/heads/patches/bbb54a2c9314a528a4fff9d6c2aae874ed098433"
+ 
            ),
+ 
            Some("bbb54a2c9314a528a4fff9d6c2aae874ed098433")
+ 
        );
+ 
    }
+ 
}