Merge pull request #564 from cole-h/disable-trusted-users

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

config.public.json

@@ -12,6 +12,7 @@
             "grahamc/ofborg",
             "grahamc/nixpkgs"
         ],
+        "disable_trusted_users": true,
         "trusted_users": [
             "1000101",
             "7c6f434c",

ofborg/src/acl.rs

@@ -1,18 +1,17 @@
 use crate::systems::System;
 
-pub struct ACL {
+pub struct Acl {
-    trusted_users: Vec<String>,
+    trusted_users: Option<Vec<String>>,
     repos: Vec<String>,
 }
 
-impl ACL {
+impl Acl {
-    pub fn new(repos: Vec<String>, mut trusted_users: Vec<String>) -> ACL {
+    pub fn new(repos: Vec<String>, mut trusted_users: Option<Vec<String>>) -> Acl {
-        trusted_users
+        if let Some(ref mut users) = trusted_users {
-            .iter_mut()
+            users.iter_mut().map(|x| *x = x.to_lowercase()).last();
-            .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" {
+        if let Some(ref users) = self.trusted_users {
-            self.trusted_users.contains(&user.to_lowercase())
+            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
         }
     }
 }

ofborg/src/bin/mass-rebuilder.rs

@@ -38,7 +38,7 @@ fn main() -> Result<(), Box<dyn Error>> {
     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())?,
     );

ofborg/src/bin/stats.rs

@@ -18,7 +18,7 @@ fn main() -> Result<(), Box<dyn Error>> {
     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())?,
     );

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,

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<hubcaps::Error> 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),
         }

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<GithubConfig>,
     pub github_app: Option<GithubAppConfig>,
     pub log_storage: Option<LogStorage>,
@@ -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<String>,
@@ -67,6 +67,7 @@ pub struct LogStorage {
 pub struct RunnerConfig {
     pub identity: String,
     pub repos: Option<Vec<String>>,
+    pub disable_trusted_users: bool,
     pub trusted_users: Option<Vec<String>>,
 
     /// 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 {
+    pub fn acl(&self) -> acl::Acl {
-        acl::ACL::new(
+        let repos = self
-            self.runner
+            .runner
-                .repos
+            .repos
-                .clone()
+            .clone()
-                .expect("fetching config's runner.repos"),
+            .expect("fetching config's runner.repos");
-            self.runner
+
-                .trusted_users
+        let trusted_users = if self.runner.disable_trusted_users {
-                .clone()
+            None
-                .expect("fetching config's runner.trusted_users"),
+        } 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!(
             "{}://{}:{}@{}/{}",

ofborg/src/easyamqp.rs

@@ -89,9 +89,9 @@ pub enum ExchangeType {
     Custom(String),
 }
 
-impl Into<String> for ExchangeType {
+impl From<ExchangeType> for String {
-    fn into(self) -> String {
+    fn from(exchange_type: ExchangeType) -> String {
-        match self {
+        match exchange_type {
             ExchangeType::Topic => "topic".to_owned(),
             ExchangeType::Headers => "headers".to_owned(),
             ExchangeType::Fanout => "fanout".to_owned(),

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<Connection, lapin::Error> {
+pub fn from_config(cfg: &RabbitMqConfig) -> Result<Connection, lapin::Error> {
     let mut props = FieldTable::default();
     props.insert(
         "ofborg_version".into(),

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<Operation> },
     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<fs::File, fs::File> = nix.run(

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,
         }
     }

ofborg/src/stats.rs

@@ -19,21 +19,21 @@ pub struct EventMessage {
     pub events: Vec<Event>,
 }
 
-pub struct RabbitMQ<C> {
+pub struct RabbitMq<C> {
     identity: String,
     channel: C,
 }
 
-impl RabbitMQ<lapin::Channel> {
+impl RabbitMq<lapin::Channel> {
     pub fn from_lapin(identity: &str, channel: lapin::Channel) -> Self {
-        RabbitMQ {
+        RabbitMq {
             identity: identity.to_owned(),
             channel,
         }
     }
 }
 
-impl SysEvents for RabbitMQ<lapin::Channel> {
+impl SysEvents for RabbitMq<lapin::Channel> {
     fn notify(&mut self, event: Event) {
         let props = lapin::BasicProperties::default().with_content_type("application/json".into());
         task::block_on(async {

ofborg/src/tagger.rs

@@ -235,14 +235,14 @@ impl RebuildTagger {
     }
 }
 
-pub struct MaintainerPRTagger {
+pub struct MaintainerPrTagger {
     possible: Vec<String>,
     selected: Vec<String>,
 }
 
-impl Default for MaintainerPRTagger {
+impl Default for MaintainerPrTagger {
-    fn default() -> MaintainerPRTagger {
+    fn default() -> MaintainerPrTagger {
-        let mut t = 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 {
+impl MaintainerPrTagger {
-    pub fn new() -> MaintainerPRTagger {
+    pub fn new() -> MaintainerPrTagger {
         Default::default()
     }
 

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("."),

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<E> {
     nix: nix::Nix,
     github: hubcaps::Github,
     github_vend: RwLock<GithubAppVendingMachine>,
-    acl: ACL,
+    acl: Acl,
     identity: String,
     events: E,
 }
@@ -55,7 +55,7 @@ impl<E: stats::SysEvents> EvaluationWorker<E> {
         nix: &nix::Nix,
         github: hubcaps::Github,
         github_vend: GithubAppVendingMachine,
-        acl: ACL,
+        acl: Acl,
         identity: String,
         events: E,
     ) -> EvaluationWorker<E> {
@@ -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

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 =
+        let mut worker = EvaluationFilterWorker::new(acl::Acl::new(
-            EvaluationFilterWorker::new(acl::ACL::new(vec!["nixos/nixpkgs".to_owned()], vec![]));
+            vec!["nixos/nixpkgs".to_owned()],
+            Some(vec![]),
+        ));
 
         assert_eq!(
             worker.consumer(&job),

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 }
     }
 }

ofborg/src/tasks/githubcommentposter.rs

@@ -157,6 +157,8 @@ fn result_to_check(result: &LegacyBuildResult, timestamp: DateTime<Utc>) -> 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<String> = vec![];
 

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<File, String> {
+    fn open_file(&self, path: &Path) -> Result<File, String> {
         let dir = path.parent().unwrap();
         fs::create_dir_all(dir).unwrap();