diff --git a/README.md b/README.md index 30871f9..789dd89 100644 --- a/README.md +++ b/README.md @@ -122,7 +122,11 @@ combinations: @ofborg build list of attrs looks good to me! ``` -## Trusted Users +## Trusted Users (Currently Disabled) + +> **NOTE:** The Trusted Users functionality is currently disabled, as the +> current darwin builder is reset very frequently. This means that _all_ users +> will have their PRs build on the darwin machine. Trusted users have their builds and tests executed on _all_ available platforms, including those without good sandboxing. Because this exposes the host to a diff --git a/config.public.json b/config.public.json index a06680e..4316785 100644 --- a/config.public.json +++ b/config.public.json @@ -12,6 +12,7 @@ "grahamc/ofborg", "grahamc/nixpkgs" ], + "disable_trusted_users": true, "trusted_users": [ "1000101", "7c6f434c", diff --git a/ofborg/src/acl.rs b/ofborg/src/acl.rs index aeef24e..6fa9886 100644 --- a/ofborg/src/acl.rs +++ b/ofborg/src/acl.rs @@ -1,18 +1,17 @@ use crate::systems::System; -pub struct ACL { - trusted_users: Vec, +pub struct Acl { + trusted_users: Option>, repos: Vec, } -impl ACL { - pub fn new(repos: Vec, mut trusted_users: Vec) -> ACL { - trusted_users - .iter_mut() - .map(|x| *x = x.to_lowercase()) - .last(); +impl Acl { + pub fn new(repos: Vec, mut trusted_users: Option>) -> Acl { + if let Some(ref mut users) = trusted_users { + users.iter_mut().map(|x| *x = x.to_lowercase()).last(); + } - ACL { + Acl { trusted_users, repos, } @@ -47,10 +46,16 @@ impl ACL { } pub fn can_build_unrestricted(&self, user: &str, repo: &str) -> bool { - if repo.to_lowercase() == "nixos/nixpkgs" { - self.trusted_users.contains(&user.to_lowercase()) + if let Some(ref users) = self.trusted_users { + if repo.to_lowercase() == "nixos/nixpkgs" { + users.contains(&user.to_lowercase()) + } else { + user == "grahamc" + } } else { - user == "grahamc" + // If trusted_users is disabled (and thus None), everybody can build + // unrestricted + true } } } diff --git a/ofborg/src/bin/mass-rebuilder.rs b/ofborg/src/bin/mass-rebuilder.rs index bae6c7b..ebd5339 100644 --- a/ofborg/src/bin/mass-rebuilder.rs +++ b/ofborg/src/bin/mass-rebuilder.rs @@ -38,7 +38,7 @@ fn main() -> Result<(), Box> { let cloner = checkout::cached_cloner(Path::new(&cfg.checkout.root)); let nix = cfg.nix(); - let events = stats::RabbitMQ::from_lapin( + let events = stats::RabbitMq::from_lapin( &format!("{}-{}", cfg.runner.identity, cfg.nix.system), task::block_on(conn.create_channel())?, ); diff --git a/ofborg/src/bin/stats.rs b/ofborg/src/bin/stats.rs index 45d8158..7717927 100644 --- a/ofborg/src/bin/stats.rs +++ b/ofborg/src/bin/stats.rs @@ -18,7 +18,7 @@ fn main() -> Result<(), Box> { let conn = easylapin::from_config(&cfg.rabbitmq)?; let mut chan = task::block_on(conn.create_channel())?; - let events = stats::RabbitMQ::from_lapin( + let events = stats::RabbitMq::from_lapin( &format!("{}-{}", cfg.runner.identity, cfg.nix.system), task::block_on(conn.create_channel())?, ); diff --git a/ofborg/src/commentparser.rs b/ofborg/src/commentparser.rs index 8488894..31e7989 100644 --- a/ofborg/src/commentparser.rs +++ b/ofborg/src/commentparser.rs @@ -48,7 +48,7 @@ named!( value!(None, many_till!(take!(1), tag_no_case!("@grahamcofborg"))) ) )))) >> eof!() - >> (Some(res.into_iter().filter_map(|x| x).collect())) + >> (Some(res.into_iter().flatten().collect())) ) | value!(None) ) ); @@ -70,6 +70,7 @@ pub enum Instruction { Eval, } +#[allow(clippy::upper_case_acronyms)] #[derive(Serialize, Deserialize, Debug, PartialEq)] pub enum Subset { Nixpkgs, diff --git a/ofborg/src/commitstatus.rs b/ofborg/src/commitstatus.rs index 8ee6dd6..74d606b 100644 --- a/ofborg/src/commitstatus.rs +++ b/ofborg/src/commitstatus.rs @@ -74,7 +74,7 @@ impl<'a> CommitStatus<'a> { #[derive(Debug)] pub enum CommitStatusError { ExpiredCreds(hubcaps::Error), - MissingSHA(hubcaps::Error), + MissingSha(hubcaps::Error), Error(hubcaps::Error), } @@ -91,7 +91,7 @@ impl From for CommitStatusError { if code == &StatusCode::UnprocessableEntity && error.message.starts_with("No commit found for SHA:") => { - CommitStatusError::MissingSHA(e) + CommitStatusError::MissingSha(e) } _otherwise => CommitStatusError::Error(e), } diff --git a/ofborg/src/config.rs b/ofborg/src/config.rs index f9f8fbc..be7bfac 100644 --- a/ofborg/src/config.rs +++ b/ofborg/src/config.rs @@ -18,7 +18,7 @@ pub struct Config { pub feedback: FeedbackConfig, pub checkout: CheckoutConfig, pub nix: NixConfig, - pub rabbitmq: RabbitMQConfig, + pub rabbitmq: RabbitMqConfig, pub github: Option, pub github_app: Option, pub log_storage: Option, @@ -30,7 +30,7 @@ pub struct FeedbackConfig { } #[derive(Serialize, Deserialize, Debug, Clone)] -pub struct RabbitMQConfig { +pub struct RabbitMqConfig { pub ssl: bool, pub host: String, pub virtualhost: Option, @@ -67,6 +67,7 @@ pub struct LogStorage { pub struct RunnerConfig { pub identity: String, pub repos: Option>, + pub disable_trusted_users: bool, pub trusted_users: Option>, /// If true, will create its own queue attached to the build job @@ -88,17 +89,25 @@ impl Config { format!("{}-{}", self.runner.identity, self.nix.system) } - pub fn acl(&self) -> acl::ACL { - acl::ACL::new( - self.runner - .repos - .clone() - .expect("fetching config's runner.repos"), - self.runner - .trusted_users - .clone() - .expect("fetching config's runner.trusted_users"), - ) + pub fn acl(&self) -> acl::Acl { + let repos = self + .runner + .repos + .clone() + .expect("fetching config's runner.repos"); + + let trusted_users = if self.runner.disable_trusted_users { + None + } else { + Some( + self.runner + .trusted_users + .clone() + .expect("fetching config's runner.trusted_users"), + ) + }; + + acl::Acl::new(repos, trusted_users) } pub fn github(&self) -> Github { @@ -133,7 +142,7 @@ impl Config { } } -impl RabbitMQConfig { +impl RabbitMqConfig { pub fn as_uri(&self) -> String { format!( "{}://{}:{}@{}/{}", diff --git a/ofborg/src/easyamqp.rs b/ofborg/src/easyamqp.rs index 0df4da6..ccdba9f 100644 --- a/ofborg/src/easyamqp.rs +++ b/ofborg/src/easyamqp.rs @@ -89,9 +89,9 @@ pub enum ExchangeType { Custom(String), } -impl Into for ExchangeType { - fn into(self) -> String { - match self { +impl From for String { + fn from(exchange_type: ExchangeType) -> String { + match exchange_type { ExchangeType::Topic => "topic".to_owned(), ExchangeType::Headers => "headers".to_owned(), ExchangeType::Fanout => "fanout".to_owned(), diff --git a/ofborg/src/easylapin.rs b/ofborg/src/easylapin.rs index 2b07e7a..38ef5fb 100644 --- a/ofborg/src/easylapin.rs +++ b/ofborg/src/easylapin.rs @@ -1,6 +1,6 @@ use std::pin::Pin; -use crate::config::RabbitMQConfig; +use crate::config::RabbitMqConfig; use crate::easyamqp::{ BindQueueConfig, ChannelExt, ConsumeConfig, ConsumerExt, ExchangeConfig, ExchangeType, QueueConfig, @@ -21,7 +21,7 @@ use lapin::types::{AMQPValue, FieldTable}; use lapin::{BasicProperties, Channel, Connection, ConnectionProperties, ExchangeKind}; use tracing::{debug, trace}; -pub fn from_config(cfg: &RabbitMQConfig) -> Result { +pub fn from_config(cfg: &RabbitMqConfig) -> Result { let mut props = FieldTable::default(); props.insert( "ofborg_version".into(), diff --git a/ofborg/src/nix.rs b/ofborg/src/nix.rs index ee1760f..54cda60 100644 --- a/ofborg/src/nix.rs +++ b/ofborg/src/nix.rs @@ -13,6 +13,7 @@ use std::process::{Command, Stdio}; use tempfile::tempfile; +#[allow(clippy::upper_case_acronyms)] #[derive(Clone, Copy, Debug, PartialEq)] pub enum File { DefaultNixpkgs, @@ -33,7 +34,7 @@ pub enum Operation { Evaluate, Instantiate, Build, - QueryPackagesJSON, + QueryPackagesJson, QueryPackagesOutputs, NoOp { operation: Box }, Unknown { program: String }, @@ -45,7 +46,7 @@ impl Operation { Operation::Evaluate => Command::new("nix-instantiate"), Operation::Instantiate => Command::new("nix-instantiate"), Operation::Build => Command::new("nix-build"), - Operation::QueryPackagesJSON => Command::new("nix-env"), + Operation::QueryPackagesJson => Command::new("nix-env"), Operation::QueryPackagesOutputs => Command::new("nix-env"), Operation::NoOp { .. } => Command::new("echo"), Operation::Unknown { ref program } => Command::new(program), @@ -57,7 +58,7 @@ impl Operation { Operation::Build => { command.args(&["--no-out-link", "--keep-going"]); } - Operation::QueryPackagesJSON => { + Operation::QueryPackagesJson => { command.args(&["--query", "--available", "--json"]); } Operation::QueryPackagesOutputs => { @@ -85,7 +86,7 @@ impl fmt::Display for Operation { match *self { Operation::Build => write!(f, "nix-build"), Operation::Instantiate => write!(f, "nix-instantiate"), - Operation::QueryPackagesJSON => write!(f, "nix-env -qa --json"), + Operation::QueryPackagesJson => write!(f, "nix-env -qa --json"), Operation::QueryPackagesOutputs => write!(f, "nix-env -qaP --no-name --out-path"), Operation::NoOp { ref operation } => operation.fmt(f), Operation::Unknown { ref program } => write!(f, "{}", program), @@ -517,7 +518,7 @@ mod tests { if expectation_held && missed_requirements == 0 { } else { - panic!(output); + panic!("{}", output); } } @@ -556,7 +557,7 @@ mod tests { #[test] fn test_query_packages_json() { let nix = nix(); - let op = noop(Operation::QueryPackagesJSON); + let op = noop(Operation::QueryPackagesJson); assert_eq!(op.to_string(), "nix-env -qa --json"); let ret: Result = nix.run( diff --git a/ofborg/src/nixenv.rs b/ofborg/src/nixenv.rs index 000661a..625025a 100644 --- a/ofborg/src/nixenv.rs +++ b/ofborg/src/nixenv.rs @@ -19,8 +19,8 @@ pub struct HydraNixEnv { impl HydraNixEnv { pub fn new(nix: nix::Nix, path: PathBuf, check_meta: bool) -> HydraNixEnv { HydraNixEnv { - nix, path, + nix, check_meta, } } diff --git a/ofborg/src/stats.rs b/ofborg/src/stats.rs index e5363ab..e907ec4 100644 --- a/ofborg/src/stats.rs +++ b/ofborg/src/stats.rs @@ -19,21 +19,21 @@ pub struct EventMessage { pub events: Vec, } -pub struct RabbitMQ { +pub struct RabbitMq { identity: String, channel: C, } -impl RabbitMQ { +impl RabbitMq { pub fn from_lapin(identity: &str, channel: lapin::Channel) -> Self { - RabbitMQ { + RabbitMq { identity: identity.to_owned(), channel, } } } -impl SysEvents for RabbitMQ { +impl SysEvents for RabbitMq { fn notify(&mut self, event: Event) { let props = lapin::BasicProperties::default().with_content_type("application/json".into()); task::block_on(async { diff --git a/ofborg/src/tagger.rs b/ofborg/src/tagger.rs index 67011a0..9673726 100644 --- a/ofborg/src/tagger.rs +++ b/ofborg/src/tagger.rs @@ -235,14 +235,14 @@ impl RebuildTagger { } } -pub struct MaintainerPRTagger { +pub struct MaintainerPrTagger { possible: Vec, selected: Vec, } -impl Default for MaintainerPRTagger { - fn default() -> MaintainerPRTagger { - let mut t = MaintainerPRTagger { +impl Default for MaintainerPrTagger { + fn default() -> MaintainerPrTagger { + let mut t = MaintainerPrTagger { possible: vec![String::from("11.by: package-maintainer")], selected: vec![], }; @@ -252,8 +252,8 @@ impl Default for MaintainerPRTagger { } } -impl MaintainerPRTagger { - pub fn new() -> MaintainerPRTagger { +impl MaintainerPrTagger { + pub fn new() -> MaintainerPrTagger { Default::default() } diff --git a/ofborg/src/tasks/eval/nixpkgs.rs b/ofborg/src/tasks/eval/nixpkgs.rs index 2714abe..6027163 100644 --- a/ofborg/src/tasks/eval/nixpkgs.rs +++ b/ofborg/src/tasks/eval/nixpkgs.rs @@ -8,7 +8,7 @@ use crate::message::evaluationjob::EvaluationJob; use crate::nix::{self, Nix}; use crate::nixenv::HydraNixEnv; use crate::outpathdiff::{OutPathDiff, PackageArch}; -use crate::tagger::{MaintainerPRTagger, PkgsAddedRemovedTagger, RebuildTagger, StdenvTagger}; +use crate::tagger::{MaintainerPrTagger, PkgsAddedRemovedTagger, RebuildTagger, StdenvTagger}; use crate::tasks::eval::{ stdenvs::Stdenvs, Error, EvaluationComplete, EvaluationStrategy, StepResult, }; @@ -279,7 +279,7 @@ impl<'a> NixpkgsStrategy<'a> { if let Ok(ref maint) = m { request_reviews(&maint, &self.pull); - let mut maint_tagger = MaintainerPRTagger::new(); + let mut maint_tagger = MaintainerPrTagger::new(); maint_tagger .record_maintainer(&self.issue.user.login, &maint.maintainers_by_package()); update_labels( @@ -423,13 +423,13 @@ impl<'a> EvaluationStrategy for NixpkgsStrategy<'a> { vec![ EvalChecker::new( "package-list", - nix::Operation::QueryPackagesJSON, + nix::Operation::QueryPackagesJson, vec![String::from("--file"), String::from(".")], self.nix.clone(), ), EvalChecker::new( "package-list-no-aliases", - nix::Operation::QueryPackagesJSON, + nix::Operation::QueryPackagesJson, vec![ String::from("--file"), String::from("."), diff --git a/ofborg/src/tasks/evaluate.rs b/ofborg/src/tasks/evaluate.rs index 5bdcbf4..7891a1f 100644 --- a/ofborg/src/tasks/evaluate.rs +++ b/ofborg/src/tasks/evaluate.rs @@ -1,5 +1,5 @@ /// This is what evaluates every pull-request -use crate::acl::ACL; +use crate::acl::Acl; use crate::checkout; use crate::commitstatus::{CommitStatus, CommitStatusError}; use crate::config::GithubAppVendingMachine; @@ -26,7 +26,7 @@ pub struct EvaluationWorker { nix: nix::Nix, github: hubcaps::Github, github_vend: RwLock, - acl: ACL, + acl: Acl, identity: String, events: E, } @@ -55,7 +55,7 @@ impl EvaluationWorker { nix: &nix::Nix, github: hubcaps::Github, github_vend: GithubAppVendingMachine, - acl: ACL, + acl: Acl, identity: String, events: E, ) -> EvaluationWorker { @@ -125,7 +125,7 @@ struct OneEval<'a, E> { repo: hubcaps::repositories::Repository<'a>, gists: Gists<'a>, nix: &'a nix::Nix, - acl: &'a ACL, + acl: &'a Acl, events: &'a mut E, identity: &'a str, cloner: &'a checkout::CachedCloner, @@ -138,7 +138,7 @@ impl<'a, E: stats::SysEvents + 'static> OneEval<'a, E> { client_app: &'a hubcaps::Github, client_legacy: &'a hubcaps::Github, nix: &'a nix::Nix, - acl: &'a ACL, + acl: &'a Acl, events: &'a mut E, identity: &'a str, cloner: &'a checkout::CachedCloner, @@ -242,7 +242,7 @@ impl<'a, E: stats::SysEvents + 'static> OneEval<'a, E> { error!("Failed writing commit status: creds expired: {:?}", e); self.actions().retry_later(&self.job) } - Err(Err(CommitStatusError::MissingSHA(e))) => { + Err(Err(CommitStatusError::MissingSha(e))) => { error!( "Failed writing commit status: commit sha was force-pushed away: {:?}", e diff --git a/ofborg/src/tasks/evaluationfilter.rs b/ofborg/src/tasks/evaluationfilter.rs index 6735e89..2995101 100644 --- a/ofborg/src/tasks/evaluationfilter.rs +++ b/ofborg/src/tasks/evaluationfilter.rs @@ -6,11 +6,11 @@ use crate::worker; use tracing::{debug_span, info}; pub struct EvaluationFilterWorker { - acl: acl::ACL, + acl: acl::Acl, } impl EvaluationFilterWorker { - pub fn new(acl: acl::ACL) -> EvaluationFilterWorker { + pub fn new(acl: acl::Acl) -> EvaluationFilterWorker { EvaluationFilterWorker { acl } } } @@ -110,8 +110,10 @@ mod tests { let job: ghevent::PullRequestEvent = serde_json::from_str(&data.to_string()).expect("Should properly deserialize"); - let mut worker = - EvaluationFilterWorker::new(acl::ACL::new(vec!["nixos/nixpkgs".to_owned()], vec![])); + let mut worker = EvaluationFilterWorker::new(acl::Acl::new( + vec!["nixos/nixpkgs".to_owned()], + Some(vec![]), + )); assert_eq!( worker.consumer(&job), diff --git a/ofborg/src/tasks/githubcommentfilter.rs b/ofborg/src/tasks/githubcommentfilter.rs index 9314f67..1c34c98 100644 --- a/ofborg/src/tasks/githubcommentfilter.rs +++ b/ofborg/src/tasks/githubcommentfilter.rs @@ -8,12 +8,12 @@ use tracing::{debug_span, error, info}; use uuid::Uuid; pub struct GitHubCommentWorker { - acl: acl::ACL, + acl: acl::Acl, github: hubcaps::Github, } impl GitHubCommentWorker { - pub fn new(acl: acl::ACL, github: hubcaps::Github) -> GitHubCommentWorker { + pub fn new(acl: acl::Acl, github: hubcaps::Github) -> GitHubCommentWorker { GitHubCommentWorker { acl, github } } } diff --git a/ofborg/src/tasks/githubcommentposter.rs b/ofborg/src/tasks/githubcommentposter.rs index 16dd07b..de4bc81 100644 --- a/ofborg/src/tasks/githubcommentposter.rs +++ b/ofborg/src/tasks/githubcommentposter.rs @@ -157,6 +157,8 @@ fn result_to_check(result: &LegacyBuildResult, timestamp: DateTime) -> Chec )); } + // Allow the clippy violation for improved readability + #[allow(clippy::vec_init_then_push)] let text: String = if !result.output.is_empty() { let mut reply: Vec = vec![]; diff --git a/ofborg/src/tasks/log_message_collector.rs b/ofborg/src/tasks/log_message_collector.rs index 3240531..c49963d 100644 --- a/ofborg/src/tasks/log_message_collector.rs +++ b/ofborg/src/tasks/log_message_collector.rs @@ -5,7 +5,7 @@ use crate::writetoline::LineWriter; use std::fs::{self, File, OpenOptions}; use std::io::Write; -use std::path::{Component, PathBuf}; +use std::path::{Component, Path, PathBuf}; use lru_cache::LruCache; use tracing::warn; @@ -34,7 +34,7 @@ pub struct LogMessage { message: MsgType, } -fn validate_path_segment(segment: &PathBuf) -> Result<(), String> { +fn validate_path_segment(segment: &Path) -> Result<(), String> { let components = segment.components(); if components.count() == 0 { @@ -148,7 +148,7 @@ impl LogMessageCollector { } } - fn open_file(&self, path: &PathBuf) -> Result { + fn open_file(&self, path: &Path) -> Result { let dir = path.parent().unwrap(); fs::create_dir_all(dir).unwrap();