diff --git a/buildit-utils/src/github.rs b/buildit-utils/src/github.rs index 24a3dc8..845b057 100644 --- a/buildit-utils/src/github.rs +++ b/buildit-utils/src/github.rs @@ -89,7 +89,7 @@ pub async fn open_pr( let _lock = ABBS_REPO_LOCK.lock().await; - update_abbs(&git_ref, &abbs_path, &git_ref, false).await?; + update_abbs(&git_ref, &abbs_path, false).await?; let abbs_path_clone = abbs_path.clone(); let commits = task::spawn_blocking(move || get_commits(&abbs_path_clone)) @@ -332,7 +332,6 @@ fn get_commits(path: &Path) -> anyhow::Result> { pub async fn update_abbs>( git_ref: &str, abbs_path: P, - local_branch: &str, skip_git_fetch: bool, ) -> anyhow::Result<()> { info!("Running git checkout -b stable ..."); @@ -362,6 +361,27 @@ pub async fn update_abbs>( print_stdout_and_stderr(&output); + if skip_git_fetch { + info!("Skippping git fetch ...") + } else { + info!("Running git fetch origin {git_ref} ..."); + + let output = process::Command::new("git") + .arg("fetch") + .arg("origin") + .arg(git_ref) + .current_dir(abbs_path) + .output() + .instrument(info_span!("git_fetch_origin")) + .await?; + + print_stdout_and_stderr(&output); + + if !output.status.success() { + bail!("Failed to fetch origin git-ref: {git_ref}"); + } + } + info!("Running git reset origin/stable --hard ..."); let output = process::Command::new("git") @@ -375,12 +395,12 @@ pub async fn update_abbs>( print_stdout_and_stderr(&output); - info!("Running git checkout -b {local_branch} ..."); + info!("Running git checkout -b {git_ref} ..."); let output = process::Command::new("git") .arg("checkout") .arg("-b") - .arg(local_branch) + .arg(git_ref) .current_dir(abbs_path) .output() .instrument(info_span!("git_checkout_branch")) @@ -388,11 +408,11 @@ pub async fn update_abbs>( print_stdout_and_stderr(&output); - info!("Running git checkout {local_branch} ..."); + info!("Running git checkout {git_ref} ..."); let output = process::Command::new("git") .arg("checkout") - .arg(local_branch) + .arg(git_ref) .current_dir(abbs_path) .output() .instrument(info_span!("git_checkout_branch")) @@ -401,28 +421,7 @@ pub async fn update_abbs>( print_stdout_and_stderr(&output); if !output.status.success() { - bail!("Failed to checkout {local_branch}"); - } - - if skip_git_fetch { - info!("Skippping git fetch ...") - } else { - info!("Running git fetch origin {git_ref} ..."); - - let output = process::Command::new("git") - .arg("fetch") - .arg("origin") - .arg(git_ref) - .current_dir(abbs_path) - .output() - .instrument(info_span!("git_fetch_origin")) - .await?; - - print_stdout_and_stderr(&output); - - if !output.status.success() { - bail!("Failed to fetch origin git-ref: {git_ref}"); - } + bail!("Failed to checkout {git_ref}"); } info!("Running git reset FETCH_HEAD --hard ..."); diff --git a/buildit-utils/src/lib.rs b/buildit-utils/src/lib.rs index 9ff8774..8d6ed2e 100644 --- a/buildit-utils/src/lib.rs +++ b/buildit-utils/src/lib.rs @@ -86,7 +86,7 @@ pub async fn find_update_and_update_checksum( let _lock = ABBS_REPO_LOCK.lock().await; // switch to stable branch - update_abbs("stable", &abbs_path, "stable", false).await?; + update_abbs("stable", &abbs_path, false).await?; match manual_update { Some(version) => { diff --git a/common/src/lib.rs b/common/src/lib.rs index 08c172a..0dcf94c 100644 --- a/common/src/lib.rs +++ b/common/src/lib.rs @@ -13,13 +13,7 @@ pub struct WorkerPollRequest { #[derive(Serialize, Deserialize, Debug)] pub struct WorkerPollResponse { pub job_id: i32, - /// PR HEAD branch (for non-fork PRs only) or 'stable'. - /// - /// Either github_pr or git_branch must be present. - #[serde(default)] - pub git_branch: Option, - #[serde(default)] - pub github_pr: Option, + pub git_branch: String, pub git_sha: String, pub packages: String, } diff --git a/server/src/api.rs b/server/src/api.rs index be204a1..356016b 100644 --- a/server/src/api.rs +++ b/server/src/api.rs @@ -18,7 +18,7 @@ use diesel::{ connection::{AnsiTransactionManager, TransactionManager}, }; use serde::{Deserialize, Serialize}; -use std::{borrow::Cow, collections::BTreeMap}; +use std::collections::BTreeMap; use tracing::warn; #[derive(Debug, Copy, Clone, Serialize, Deserialize)] @@ -55,7 +55,7 @@ async fn create_check_run(crab: octocrab::Octocrab, arch: String, git_sha: Strin #[allow(clippy::too_many_arguments)] pub async fn pipeline_new( pool: DbPool, - git_branch: Option<&str>, + git_branch: &str, git_sha: Option<&str>, github_pr: Option, packages: &str, @@ -97,27 +97,15 @@ pub async fn pipeline_new( } // sanitize git_branch arg - if let Some(git_branch) = git_branch - && !git_branch.chars().all(|ch| { - ch.is_ascii_alphanumeric() || ch == '.' || ch == '-' || ch == '+' || ch == '_' - }) + if !git_branch + .chars() + .all(|ch| ch.is_ascii_alphanumeric() || ch == '.' || ch == '-' || ch == '+' || ch == '_') { return Err(anyhow!("Invalid branch: {git_branch}")); } - let (git_ref, local_branch) = if let Some(git_branch) = git_branch { - (Cow::Borrowed(git_branch), Cow::Borrowed(git_branch)) - } else if let Some(pr_number) = github_pr { - ( - Cow::Owned(format!("refs/pull/{}/head", pr_number)), - Cow::Owned(format!("pr{}", pr_number)), - ) - } else { - bail!("Either git_branch or github_pr must be set when creating new pipeline") - }; - let lock = ABBS_REPO_LOCK.lock().await; - update_abbs(&local_branch, &ARGS.abbs_path, &git_ref, skip_git_fetch) + update_abbs(git_branch, &ARGS.abbs_path, skip_git_fetch) .await .context("Failed to update ABBS tree")?; @@ -183,7 +171,7 @@ pub async fn pipeline_new( let new_pipeline = NewPipeline { packages: packages.to_string(), archs: archs.join(","), - git_branch: local_branch.into_owned(), + git_branch: git_branch.to_string(), git_sha: git_sha.clone(), creation_time: chrono::Utc::now(), source: source.to_string(), @@ -286,7 +274,9 @@ pub async fn pipeline_new_pr( (pr.head.ref_field.as_str(), &pr.head.sha) }; - let fork = pr.head.repo.as_ref().and_then(|x| x.fork).unwrap_or(false); + if pr.head.repo.as_ref().and_then(|x| x.fork).unwrap_or(false) { + return Err(anyhow!("Failed to create job: Pull request is a fork")); + } // find lines starting with #buildit let packages = get_packages_from_pr(&pr); @@ -296,17 +286,9 @@ pub async fn pipeline_new_pr( archs.to_string() } else { let path = &ARGS.abbs_path; - let (git_ref, local_branch) = if fork { - ( - Cow::Owned(format!("refs/pull/{}/head", pr.number)), - Cow::Owned(format!("pr{}", pr.number)), - ) - } else { - (Cow::Borrowed(git_branch), Cow::Borrowed(git_branch)) - }; let _lock = ABBS_REPO_LOCK.lock().await; - update_abbs(&git_ref, &ARGS.abbs_path, &local_branch, false) + update_abbs(git_branch, &ARGS.abbs_path, false) .await .context("Failed to update ABBS tree")?; // skip next git fetch in pipeline_new @@ -320,7 +302,7 @@ pub async fn pipeline_new_pr( pipeline_new( pool, - if fork { None } else { Some(git_branch) }, + git_branch, Some(git_sha), Some(pr.number), &packages.join(","), diff --git a/server/src/bot.rs b/server/src/bot.rs index 42bb8bb..e35773b 100644 --- a/server/src/bot.rs +++ b/server/src/bot.rs @@ -3,7 +3,6 @@ use crate::{ api::{JobSource, job_restart, pipeline_new, pipeline_new_pr, pipeline_status, worker_status}, formatter::to_html_new_pipeline_summary, github::{get_github_token, login_github}, - is_maintainer, models::{NewUser, User}, paste_to_aosc_io, }; @@ -168,7 +167,7 @@ async fn pipeline_new_and_report( match wait_with_send_typing( pipeline_new( pool, - Some(git_branch), + git_branch, None, None, packages, @@ -310,42 +309,6 @@ async fn get_user(pool: DbPool, chat_id: ChatId, access_token: String) -> anyhow bail!("Failed to get user info") } -async fn check_maintainership(bot: &Bot, pool: &DbPool, chat_id: ChatId) -> ResponseResult { - match check_maintainership_inner(pool, chat_id).await { - Ok(_) => Ok(true), - Err(error) => { - bot.send_message(chat_id, format!("Failed maintainership check: {error:?}")) - .await?; - Ok(false) - } - } -} - -async fn check_maintainership_inner(pool: &DbPool, chat_id: ChatId) -> anyhow::Result<()> { - let mut conn = pool - .get() - .context("Failed to get db connection from pool")?; - - use crate::schema::users::dsl::*; - if let Some(user) = users - .filter(telegram_chat_id.eq(&chat_id.0)) - .first::(&mut conn) - .optional()? - { - if let Some(gh_login) = user.github_login { - if is_maintainer(&gh_login).await? { - Ok(()) - } else { - bail!("You are not a member of AOSC-Dev yet") - } - } else { - bail!("GitHub account is not connected") - } - } else { - bail!("user not found") - } -} - async fn create_pipeline_from_pr( pool: DbPool, pr_number: u64, @@ -401,9 +364,6 @@ pub async fn answer(bot: Bot, msg: Message, cmd: Command, pool: DbPool) -> Respo .await?; } Command::PR(arguments) => { - if !check_maintainership(&bot, &pool, msg.chat.id).await? { - return Ok(()); - } let parts = arguments.split_ascii_whitespace().collect::>(); if !(1..=2).contains(&parts.len()) { bot.send_message( @@ -446,9 +406,6 @@ pub async fn answer(bot: Bot, msg: Message, cmd: Command, pool: DbPool) -> Respo } } Command::Build(arguments) => { - if !check_maintainership(&bot, &pool, msg.chat.id).await? { - return Ok(()); - } let parts: Vec<&str> = arguments.split(' ').collect(); if parts.len() == 3 { let git_branch = parts[0]; @@ -639,9 +596,6 @@ pub async fn answer(bot: Bot, msg: Message, cmd: Command, pool: DbPool) -> Respo } } Command::Dickens(arguments) => { - if !check_maintainership(&bot, &pool, msg.chat.id).await? { - return Ok(()); - } let mut pr_numbers = vec![]; for part in arguments.split(',') { match str::parse::(part) { @@ -681,11 +635,7 @@ pub async fn answer(bot: Bot, msg: Message, cmd: Command, pool: DbPool) -> Respo .await { Ok(pr) => match dickens::topic::report( - &if pr.head.repo.as_ref().and_then(|x| x.fork).unwrap_or(false) { - Cow::Owned(format!("pr{pr_number}")) - } else { - Cow::Borrowed(pr.head.ref_field.as_str()) - }, + pr.head.ref_field.as_str(), ARGS.local_repo.clone(), ) .await @@ -767,9 +717,6 @@ pub async fn answer(bot: Bot, msg: Message, cmd: Command, pool: DbPool) -> Respo } } Command::QA(arguments) => { - if !check_maintainership(&bot, &pool, msg.chat.id).await? { - return Ok(()); - } let parts: Vec<&str> = arguments.split(' ').collect(); if parts.len() == 2 && ALL_ARCH.contains(&parts[0]) @@ -835,9 +782,6 @@ pub async fn answer(bot: Bot, msg: Message, cmd: Command, pool: DbPool) -> Respo } Command::Restart(arguments) => match str::parse::(&arguments) { Ok(job_id) => { - if !check_maintainership(&bot, &pool, msg.chat.id).await? { - return Ok(()); - } match wait_with_send_typing(job_restart(pool, job_id), &bot, msg.chat.id.0).await { Ok(new_job) => { bot.send_message( @@ -865,10 +809,6 @@ pub async fn answer(bot: Bot, msg: Message, cmd: Command, pool: DbPool) -> Respo } }, Command::Bump(package_and_version) => { - if !check_maintainership(&bot, &pool, msg.chat.id).await? { - return Ok(()); - } - let app_private_key = match ARGS.github_app_key.as_ref() { Some(p) => p, None => { @@ -1014,29 +954,24 @@ pub async fn answer(bot: Bot, msg: Message, cmd: Command, pool: DbPool) -> Respo } }; } - Command::Roll => { - if !check_maintainership(&bot, &pool, msg.chat.id).await? { - return Ok(()); - } - match wait_with_send_typing(roll(), &bot, msg.chat.id.0).await { - Ok(pkgs) => { - let mut s = String::new(); - for i in pkgs { - s.push_str(&i.to_string()); - s.push('\n'); - } - - bot.send_message(msg.chat.id, truncate(&s)).await?; - } - Err(e) => { - bot.send_message( - msg.chat.id, - truncate(&format!("Failed to roll packages: {}", e)), - ) - .await?; + Command::Roll => match wait_with_send_typing(roll(), &bot, msg.chat.id.0).await { + Ok(pkgs) => { + let mut s = String::new(); + for i in pkgs { + s.push_str(&i.to_string()); + s.push('\n'); } + + bot.send_message(msg.chat.id, truncate(&s)).await?; } - } + Err(e) => { + bot.send_message( + msg.chat.id, + truncate(&format!("Failed to roll packages: {}", e)), + ) + .await?; + } + }, }; Ok(()) @@ -1048,9 +983,6 @@ pub async fn answer_callback(bot: Bot, pool: DbPool, query: CallbackQuery) -> Re if let Some(msg) = query.message { if let Some(ref data) = query.data { if let Some(strip) = data.strip_prefix("restart_") { - if !check_maintainership(&bot, &pool, msg.chat().id).await? { - return Ok(()); - } match str::parse::(strip) { Ok(job_id) => { match wait_with_send_typing( @@ -1086,9 +1018,6 @@ pub async fn answer_callback(bot: Bot, pool: DbPool, query: CallbackQuery) -> Re } } } else if let Some(strip) = data.strip_prefix("buildpr_") { - if !check_maintainership(&bot, &pool, msg.chat().id).await? { - return Ok(()); - } match str::parse::(strip) { Ok(pr_num) => { let pipeline = create_pipeline_from_pr( diff --git a/server/src/lib.rs b/server/src/lib.rs index bd7e8cb..4c1dbf8 100644 --- a/server/src/lib.rs +++ b/server/src/lib.rs @@ -165,10 +165,3 @@ async fn test_paste_to_aosc_io() { .unwrap(); dbg!(id); } - -async fn is_maintainer(user: &str) -> anyhow::Result { - let crab = octocrab::Octocrab::builder() - .user_access_token(ARGS.github_access_token.clone()) - .build()?; - Ok(crab.orgs("AOSC-Dev").check_membership(user).await?) -} diff --git a/server/src/routes/pipeline.rs b/server/src/routes/pipeline.rs index e7b5072..b3266fc 100644 --- a/server/src/routes/pipeline.rs +++ b/server/src/routes/pipeline.rs @@ -31,7 +31,7 @@ pub async fn pipeline_new( ) -> Result, AnyhowError> { let (pipeline, _) = api::pipeline_new( pool, - Some(&payload.git_branch), + &payload.git_branch, None, None, &payload.packages, diff --git a/server/src/routes/webhook.rs b/server/src/routes/webhook.rs index aa5137d..2416ea7 100644 --- a/server/src/routes/webhook.rs +++ b/server/src/routes/webhook.rs @@ -5,7 +5,7 @@ use serde_json::Value; use tracing::{info, warn}; use crate::{ - api, bot::GitHubUser, formatter::to_html_new_pipeline_summary, is_maintainer, paste_to_aosc_io, DbPool, ARGS + ARGS, DbPool, api, bot::GitHubUser, formatter::to_html_new_pipeline_summary, paste_to_aosc_io, }; use super::{AnyhowError, AppState}; @@ -76,7 +76,7 @@ async fn handle_webhook_comment( return Ok(()); } - let is_org_user = is_maintainer(&comment.user.login).await?; + let is_org_user = is_org_user(&comment.user.login).await?; if !is_org_user { return Ok(()); @@ -160,3 +160,10 @@ async fn pipeline_new_pr_impl( Ok(()) } + +async fn is_org_user(user: &str) -> anyhow::Result { + let crab = octocrab::Octocrab::builder() + .user_access_token(ARGS.github_access_token.clone()) + .build()?; + Ok(crab.orgs("AOSC-Dev").check_membership(user).await?) +} diff --git a/server/src/routes/worker.rs b/server/src/routes/worker.rs index 0e05fdf..1458776 100644 --- a/server/src/routes/worker.rs +++ b/server/src/routes/worker.rs @@ -301,21 +301,10 @@ pub async fn worker_poll( }); } - // detect pipelines from fork repositories - let gh_pr = pipeline.github_pr.map(|pr| pr as u64); - let git_branch = if let Some(gh_pr) = gh_pr - && pipeline.git_branch == format!("pr{gh_pr}") - { - None - } else { - Some(pipeline.git_branch) - }; - // job allocated Ok(Json(Some(WorkerPollResponse { job_id: job.id, - git_branch, - github_pr: gh_pr, + git_branch: pipeline.git_branch, git_sha: pipeline.git_sha, packages: job.packages, }))) diff --git a/worker/src/build.rs b/worker/src/build.rs index 12bb2b9..c93a561 100644 --- a/worker/src/build.rs +++ b/worker/src/build.rs @@ -1,12 +1,10 @@ use crate::{Args, get_memory_bytes}; -use anyhow::bail; use chrono::Local; use common::{JobOk, WorkerJobUpdateRequest, WorkerPollRequest, WorkerPollResponse}; use flume::Sender; use futures_util::future::try_join3; use log::{error, info, warn}; use std::{ - borrow::Cow, path::Path, process::{Output, Stdio}, time::{Duration, Instant}, @@ -165,22 +163,8 @@ async fn build( let mut build_success = false; let mut logs = vec![]; - let (git_ref, local_branch) = if let Some(branch) = &job.git_branch { - ( - Cow::Borrowed(branch.as_str()), - Cow::Borrowed(branch.as_str()), - ) - } else if let Some(pr_num) = job.github_pr { - ( - Cow::Owned(format!("refs/pull/{}/head", pr_num)), - Cow::Owned(format!("pr{}", pr_num)), - ) - } else { - bail!("Job without either git_branch and github_pr") - }; - let mut output_path = args.ciel_path.clone(); - output_path.push(format!("OUTPUT-{}", local_branch)); + output_path.push(format!("OUTPUT-{}", job.git_branch)); // clear output directory if output_path.exists() { @@ -193,7 +177,7 @@ async fn build( &[ "fetch", "https://github.com/AOSC-Dev/aosc-os-abbs.git", - &git_ref, + &job.git_branch, ], tree_path, &mut logs, @@ -208,7 +192,7 @@ async fn build( // ensure branch exists get_output_logged( "git", - &["checkout", "-b", &local_branch], + &["checkout", "-b", &job.git_branch], tree_path, &mut logs, tx.clone(), @@ -217,7 +201,7 @@ async fn build( // checkout to branch get_output_logged( "git", - &["checkout", &local_branch], + &["checkout", &job.git_branch], tree_path, &mut logs, tx.clone(), @@ -310,12 +294,12 @@ async fn build( "-i", upload_ssh_key, "maintainers", - &local_branch, + &job.git_branch, ]; if !args.pushpkg_options.is_empty() { pushpkg_args.insert(0, &args.pushpkg_options); } - if local_branch != "stable" { + if &job.git_branch != "stable" { // allow force push if noarch and non stable pushpkg_args.insert(0, "--force-push-noarch-package"); } @@ -335,7 +319,7 @@ async fn build( let file_name = format!( "{}-{}-{}-{}-{}.txt", job.job_id, - local_branch, + job.git_branch, args.arch, gethostname::gethostname().to_string_lossy(), Local::now().format("%Y-%m-%d-%H:%M:%S")