Radish alpha
r
radicle-ci-broker
rad:zwTxygwuz5LDGBq255RA2CbNGrz8
Radicle CI broker
Radicle
Git
Patches › 25997977
feat: "event filter --explain" shows why an event was allowed or not
Merged liw opened 1 year ago

Also refactor the code that makes the decision for simplicity and clarity.

Signed-off-by: Lars Wirzenius liw@liw.fi

Activity Changes +250 -91
4 files changed +250 -91 515cf386 → 746445cb
modified src/bin/cibtoolcmd/event.rs
@@ -499,6 +499,10 @@ pub struct FilterEvents {


    /// Read broker events from this file.

    input: PathBuf,
+
+ 
    /// Show why the decision was made.
+ 
    #[clap(long)]
+ 
    explain: bool,

}



impl Leaf for FilterEvents {
@@ -520,7 +524,14 @@ impl Leaf for FilterEvents {


        for event in ci_events.iter() {

            for filter in filters.iter() {
- 
                if filter.allows(event) {
+ 
                let decision = filter.decide(event);
+ 
                if self.explain {
+ 
                    println!("event={event:#?}");
+ 
                    println!("filter={filter:#?}");
+ 
                    decision.print(0);
+ 
                    println!();
+ 
                }
+ 
                if decision.allowed() {

                    let json = serde_json::to_string_pretty(event)

                        .map_err(|e| CibToolError::EventToJson(event.clone(), e))?;

                    println!("{json}");
modified src/ci_event.rs
@@ -59,6 +59,61 @@ pub enum CiEventV1 {
}



impl CiEvent {
+ 
    pub fn from_node(&self) -> Option<&NodeId> {
+ 
        match self {
+ 
            Self::V1(CiEventV1::Shutdown) => None,
+ 
            Self::V1(CiEventV1::BranchCreated { from_node, .. }) => Some(from_node),
+ 
            Self::V1(CiEventV1::BranchUpdated { from_node, .. }) => Some(from_node),
+ 
            Self::V1(CiEventV1::BranchDeleted { from_node, .. }) => Some(from_node),
+ 
            Self::V1(CiEventV1::PatchCreated { from_node, .. }) => Some(from_node),
+ 
            Self::V1(CiEventV1::PatchUpdated { from_node, .. }) => Some(from_node),
+ 
        }
+ 
    }
+
+ 
    pub fn repository(&self) -> Option<&RepoId> {
+ 
        match self {
+ 
            Self::V1(CiEventV1::Shutdown) => None,
+ 
            Self::V1(CiEventV1::BranchCreated { repo, .. }) => Some(repo),
+ 
            Self::V1(CiEventV1::BranchUpdated { repo, .. }) => Some(repo),
+ 
            Self::V1(CiEventV1::BranchDeleted { repo, .. }) => Some(repo),
+ 
            Self::V1(CiEventV1::PatchCreated { repo, .. }) => Some(repo),
+ 
            Self::V1(CiEventV1::PatchUpdated { repo, .. }) => Some(repo),
+ 
        }
+ 
    }
+
+ 
    pub fn branch(&self) -> Option<&RefString> {
+ 
        match self {
+ 
            Self::V1(CiEventV1::Shutdown) => None,
+ 
            Self::V1(CiEventV1::BranchCreated { branch, .. }) => Some(branch),
+ 
            Self::V1(CiEventV1::BranchUpdated { branch, .. }) => Some(branch),
+ 
            Self::V1(CiEventV1::BranchDeleted { branch, .. }) => Some(branch),
+ 
            Self::V1(CiEventV1::PatchCreated { .. }) => None,
+ 
            Self::V1(CiEventV1::PatchUpdated { .. }) => None,
+ 
        }
+ 
    }
+
+ 
    pub fn patch_id(&self) -> Option<&PatchId> {
+ 
        match self {
+ 
            Self::V1(CiEventV1::Shutdown) => None,
+ 
            Self::V1(CiEventV1::BranchCreated { .. }) => None,
+ 
            Self::V1(CiEventV1::BranchUpdated { .. }) => None,
+ 
            Self::V1(CiEventV1::BranchDeleted { .. }) => None,
+ 
            Self::V1(CiEventV1::PatchCreated { patch, .. }) => Some(patch),
+ 
            Self::V1(CiEventV1::PatchUpdated { patch, .. }) => Some(patch),
+ 
        }
+ 
    }
+
+ 
    pub fn tip(&self) -> Option<&Oid> {
+ 
        match self {
+ 
            Self::V1(CiEventV1::Shutdown) => None,
+ 
            Self::V1(CiEventV1::BranchCreated { tip, .. }) => Some(tip),
+ 
            Self::V1(CiEventV1::BranchUpdated { tip, .. }) => Some(tip),
+ 
            Self::V1(CiEventV1::BranchDeleted { tip, .. }) => Some(tip),
+ 
            Self::V1(CiEventV1::PatchCreated { new_tip, .. }) => Some(new_tip),
+ 
            Self::V1(CiEventV1::PatchUpdated { new_tip, .. }) => Some(new_tip),
+ 
        }
+ 
    }
+ 


    pub fn branch_created(

        from_node: NodeId,

        repo: RepoId,
modified src/filter.rs
@@ -13,7 +13,6 @@ use radicle::{
use crate::{

    ci_event::{namespaced_branch, CiEvent, CiEventV1},

    config::TriggerConfig,
- 
    logger,

};



#[cfg(test)]
@@ -101,103 +100,181 @@ pub enum EventFilter {
}



impl EventFilter {
+ 
    pub fn decide(&self, event: &CiEvent) -> Decision {
+ 
        if matches!(event, CiEvent::V1(CiEventV1::Shutdown)) {
+ 
            return Decision::new("Shutdown", true, "shutdown event is always allowed");
+ 
        }
+
+ 
        match self {
+ 
            Self::Not(expr) => {
+ 
                let conds: Vec<Decision> = expr.iter().map(|op| op.decide(event)).collect();
+ 
                let allowed = !conds.iter().any(|op| op.allowed);
+ 
                Decision::parent("Not", allowed, "(combo)", conds)
+ 
            }
+ 
            Self::And(expr) => {
+ 
                let conds: Vec<Decision> = expr.iter().map(|op| op.decide(event)).collect();
+ 
                let allowed = conds.iter().all(|op| op.allowed);
+ 
                Decision::parent("And", allowed, "(combo)", conds)
+ 
            }
+ 
            Self::Or(expr) => {
+ 
                let conds: Vec<Decision> = expr.iter().map(|op| op.decide(event)).collect();
+ 
                let allowed = conds.iter().any(|op| op.allowed);
+ 
                Decision::parent("Or", allowed, "(combo)", conds)
+ 
            }
+ 
            Self::Allow => Decision::new("Allow", true, "always allowed"),
+ 
            Self::Deny => Decision::new("Deny", false, "never allowed"),
+ 
            Self::Node(wanted) => {
+ 
                let actual = event.from_node();
+ 
                let allowed = Some(wanted) == actual;
+ 
                Decision::string(
+ 
                    "Node",
+ 
                    allowed,
+ 
                    format!("wanted={wanted} actual={actual:?}"),
+ 
                )
+ 
            }
+ 
            Self::Repository(wanted) => {
+ 
                let actual = event.repository();
+ 
                let allowed = Some(wanted) == actual;
+ 
                Decision::string(
+ 
                    "Repository",
+ 
                    allowed,
+ 
                    format!("wanted={wanted} actual={actual:?}"),
+ 
                )
+ 
            }
+ 
            Self::Branch(wanted) => {
+ 
                let actual = event.branch();
+ 
                let allowed = Some(wanted) == actual;
+ 
                Decision::string(
+ 
                    "Branch",
+ 
                    allowed,
+ 
                    format!("wanted={wanted} actual={actual:?}"),
+ 
                )
+ 
            }
+ 
            Self::DefaultBranch => {
+ 
                let repo = event.repository();
+ 
                let actual = event.branch();
+ 
                let allowed = match (repo, actual) {
+ 
                    (Some(repo), Some(actual)) => is_default_branch(repo, actual),
+ 
                    _ => false,
+ 
                };
+ 
                Decision::string(
+ 
                    "DefaultBranch",
+ 
                    allowed,
+ 
                    format!("repo={repo:?} actual={actual:?}"),
+ 
                )
+ 
            }
+ 
            Self::BranchCreated => {
+ 
                let allowed = matches!(event, CiEvent::V1(CiEventV1::BranchCreated { .. }));
+ 
                Decision::new("BranchCreated", allowed, "")
+ 
            }
+ 
            Self::BranchUpdated => {
+ 
                let allowed = matches!(event, CiEvent::V1(CiEventV1::BranchUpdated { .. }));
+ 
                Decision::new("BranchUpdated", allowed, "")
+ 
            }
+ 
            Self::BranchDeleted => {
+ 
                let allowed = matches!(event, CiEvent::V1(CiEventV1::BranchDeleted { .. }));
+ 
                Decision::new("BranchDeleted", allowed, "")
+ 
            }
+ 
            Self::Patch(wanted) => {
+ 
                let actual = event.patch_id();
+ 
                let allowed = Some(&PatchId::from(wanted)) == actual;
+ 
                Decision::string(
+ 
                    "Patch",
+ 
                    allowed,
+ 
                    format!("wanted={wanted} actual={actual:?}"),
+ 
                )
+ 
            }
+ 
            Self::PatchCreated => {
+ 
                let allowed = matches!(event, CiEvent::V1(CiEventV1::PatchCreated { .. }));
+ 
                Decision::new("PatchCreated", allowed, "")
+ 
            }
+ 
            Self::PatchUpdated => {
+ 
                let allowed = matches!(event, CiEvent::V1(CiEventV1::PatchUpdated { .. }));
+ 
                Decision::new("PatchUpdated", allowed, "")
+ 
            }
+ 
            Self::HasFile(wanted) => {
+ 
                let repo = event.repository();
+ 
                let tip = event.tip();
+ 
                let allowed = match (repo, tip) {
+ 
                    (Some(repo), Some(tip)) => has_file(repo, tip, wanted),
+ 
                    _ => false,
+ 
                };
+ 
                Decision::string(
+ 
                    "HasFile",
+ 
                    allowed,
+ 
                    format!("repo={repo:?} tip={tip:?} wanted={wanted:?}"),
+ 
                )
+ 
            }
+ 
        }
+ 
    }
+ 


    pub fn allows(&self, event: &CiEvent) -> bool {
- 
        let decision = match self {
- 
            Self::Allow => Some(true),
- 
            Self::Deny => Some(false),
- 
            Self::Not(expr) => Some(!expr.iter().any(|e| e.allows(event))),
- 
            Self::And(exprs) => Some(exprs.iter().all(|e| e.allows(event))),
- 
            Self::Or(exprs) => Some(exprs.iter().any(|e| e.allows(event))),
- 
            _ => None,
- 
        };
+ 
        self.decide(event).allowed
+ 
    }
- 
        if let Some(allowed) = decision {
- 
            logger::queueproc_predicate_decision(event, self, allowed);
- 
            return allowed;
+ 
    pub fn from_file(filename: &Path) -> Result<Vec<Self>, FilterError> {
+ 
        Filters::from_file(filename)
+ 
    }
+ 
}
+
+ 
pub struct Decision {
+ 
    filter: &'static str,
+ 
    allowed: bool,
+ 
    reason: String,
+ 
    children: Vec<Self>,
+ 
}
+
+ 
impl Decision {
+ 
    fn new(filter: &'static str, allowed: bool, reason: &'static str) -> Self {
+ 
        Self {
+ 
            filter,
+ 
            allowed,
+ 
            reason: reason.into(),
+ 
            children: vec![],

        }
+ 
    }
- 
        let decision = match event {
- 
            CiEvent::V1(CiEventV1::Shutdown) => true,
- 
            CiEvent::V1(CiEventV1::BranchCreated {
- 
                from_node,
- 
                repo,
- 
                branch,
- 
                tip,
- 
            }) => match self {
- 
                Self::Node(wanted) => from_node == wanted,
- 
                Self::Repository(wanted) => repo == wanted,
- 
                Self::Branch(wanted) => branch == wanted,
- 
                Self::DefaultBranch => is_default_branch(repo, branch),
- 
                Self::BranchCreated => true,
- 
                Self::HasFile(wanted) => has_file(repo, tip, wanted),
- 
                _ => false,
- 
            },
- 
            CiEvent::V1(CiEventV1::BranchUpdated {
- 
                from_node,
- 
                repo,
- 
                branch,
- 
                tip,
- 
                ..
- 
            }) => match self {
- 
                Self::Node(wanted) => from_node == wanted,
- 
                Self::Repository(wanted) => repo == wanted,
- 
                Self::Branch(wanted) => branch == wanted,
- 
                Self::DefaultBranch => is_default_branch(repo, branch),
- 
                Self::BranchUpdated => true,
- 
                Self::HasFile(wanted) => has_file(repo, tip, wanted),
- 
                _ => false,
- 
            },
- 
            CiEvent::V1(CiEventV1::BranchDeleted {
- 
                from_node,
- 
                repo,
- 
                branch,
- 
                tip,
- 
            }) => match self {
- 
                Self::Node(wanted) => from_node == wanted,
- 
                Self::Repository(wanted) => repo == wanted,
- 
                Self::Branch(wanted) => branch == wanted,
- 
                Self::DefaultBranch => is_default_branch(repo, branch),
- 
                Self::BranchDeleted => true,
- 
                Self::HasFile(wanted) => has_file(repo, tip, wanted),
- 
                _ => false,
- 
            },
- 
            CiEvent::V1(CiEventV1::PatchCreated {
- 
                from_node,
- 
                repo,
- 
                patch,
- 
                new_tip,
- 
                ..
- 
            }) => match self {
- 
                Self::Node(wanted) => from_node == wanted,
- 
                Self::Repository(wanted) => repo == wanted,
- 
                Self::Patch(wanted) => *patch == PatchId::from(wanted),
- 
                Self::PatchCreated => true,
- 
                Self::HasFile(wanted) => has_file(repo, new_tip, wanted),
- 
                _ => false,
- 
            },
- 
            CiEvent::V1(CiEventV1::PatchUpdated {
- 
                from_node,
- 
                repo,
- 
                patch,
- 
                new_tip,
- 
                ..
- 
            }) => match self {
- 
                Self::Node(wanted) => from_node == wanted,
- 
                Self::Repository(wanted) => repo == wanted,
- 
                Self::Patch(wanted) => *patch == PatchId::from(wanted),
- 
                Self::PatchUpdated => true,
- 
                Self::HasFile(wanted) => has_file(repo, new_tip, wanted),
- 
                _ => false,
- 
            },
- 
        };
+ 
    fn parent(
+ 
        filter: &'static str,
+ 
        allowed: bool,
+ 
        reason: &'static str,
+ 
        children: Vec<Self>,
+ 
    ) -> Self {
+ 
        Self {
+ 
            filter,
+ 
            allowed,
+ 
            reason: reason.into(),
+ 
            children,
+ 
        }
+ 
    }
- 
        logger::queueproc_filter_decision(event, self, decision);
+ 
    fn string(filter: &'static str, allowed: bool, reason: String) -> Self {
+ 
        Self {
+ 
            filter,
+ 
            allowed,
+ 
            reason,
+ 
            children: vec![],
+ 
        }
+ 
    }
- 
        decision
+ 
    pub fn allowed(&self) -> bool {
+ 
        self.allowed

    }
- 
    pub fn from_file(filename: &Path) -> Result<Vec<Self>, FilterError> {
- 
        Filters::from_file(filename)
+ 
    pub fn print(&self, level: usize) {
+ 
        let mut x = String::new();
+ 
        for _ in 0..level {
+ 
            x.push_str("  ");
+ 
        }
+
+ 
        println!(
+ 
            "{x}{} allowed={} {}",
+ 
            self.filter, self.allowed, self.reason
+ 
        );
+
+ 
        for kid in self.children.iter() {
+ 
            kid.print(level + 1);
+ 
        }

    }

}
modified src/logger.rs
@@ -40,6 +40,7 @@ pub enum Kind {
    StartRun,

    AdapterMessage,

    FinishRun,
+ 
    FilterDecision,

}



impl std::fmt::Display for Kind {
@@ -52,6 +53,7 @@ impl std::fmt::Display for Kind {
            Self::StartRun => "start_run",

            Self::FinishRun => "finish_run",

            Self::AdapterMessage => "adapter_message",
+ 
            Self::FilterDecision => "filter_decision",

        };



        write!(f, "{s}")
@@ -70,6 +72,7 @@ impl TryFrom<&str> for Kind {
            "start_run" => Ok(Self::StartRun),

            "finish_run" => Ok(Self::FinishRun),

            "adapter_message" => Ok(Self::AdapterMessage),
+ 
            "filter_decision" => Ok(Self::FilterDecision),

            _ => Err(KindError::Unknown(value.into())),

        }

    }
@@ -88,6 +91,7 @@ impl TryFrom<Value> for Kind {
                "start_run" => Ok(Self::StartRun),

                "finish_run" => Ok(Self::FinishRun),

                "adapter_message" => Ok(Self::AdapterMessage),
+ 
                "filter_decision" => Ok(Self::FilterDecision),

                _ => Err(KindError::Unknown(s)),

            },

            _ => Err(KindError::UnknownValue(value)),
@@ -183,6 +187,8 @@ enum Id {
    TimeoutWaitStdoutReaderEnd,



    TriggerCreate,
+
+ 
    FilterDecision,

}



#[derive(Debug, thiserror::Error)]
@@ -967,6 +973,16 @@ pub fn timeoutcmd_nonblocking_ends(thread: &'static str) {
    );

}
+ 
pub fn event_filter_decision(filter: &'static str, allowed: bool, reason: &str) {
+ 
    info!(
+ 
        msg_id = ?Id::FilterDecision,
+ 
        kind = %Kind::FilterDecision,
+ 
        allowed,
+ 
        reason,
+ 
        "{filter}"
+ 
    );
+ 
}
+ 


pub fn debug(msg: &str) {

    debug!(

        msg_id = ?Id::AdHoc,