From 40fa4f9d50879bfd8a50a58b0a2eabf21216e67b Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Sun, 24 Sep 2023 21:50:29 -0400 Subject: [PATCH 1/3] Support for private repos - Use GitHub API for downloading files. - Use installation token when cloning and fetching repositories. - Remove unused function. --- Cargo.lock | 2 + crates/diffbot_lib/Cargo.toml | 1 + crates/diffbot_lib/src/github/github_api.rs | 83 +++++++++++++-------- crates/mapdiffbot2/Cargo.toml | 1 + crates/mapdiffbot2/src/git_operations.rs | 44 +++++++++-- crates/mapdiffbot2/src/job_processor.rs | 14 +++- crates/mapdiffbot2/src/runner.rs | 15 +--- 7 files changed, 103 insertions(+), 57 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index fd532bef..0006daa9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1195,6 +1195,7 @@ version = "0.1.0" dependencies = [ "actix-web", "async-fs", + "base64 0.21.4", "chrono", "derive_builder", "eyre", @@ -2345,6 +2346,7 @@ dependencies = [ "once_cell", "path-absolutize", "rayon", + "secrecy", "serde", "serde_json", "simple-eyre", diff --git a/crates/diffbot_lib/Cargo.toml b/crates/diffbot_lib/Cargo.toml index a810a8d4..1da41d08 100644 --- a/crates/diffbot_lib/Cargo.toml +++ b/crates/diffbot_lib/Cargo.toml @@ -26,3 +26,4 @@ flume = "0.11.0" actix-web = "4.4.0" async-fs = "1.6.0" +base64 = "0.21.4" diff --git a/crates/diffbot_lib/src/github/github_api.rs b/crates/diffbot_lib/src/github/github_api.rs index 8b570551..83dc51ed 100644 --- a/crates/diffbot_lib/src/github/github_api.rs +++ b/crates/diffbot_lib/src/github/github_api.rs @@ -1,14 +1,12 @@ use crate::github::github_types::{ CreateCheckRun, Output, RawCheckRun, Repository, UpdateCheckRunBuilder, }; -use async_fs::File; use eyre::{format_err, Context, Result}; -use futures_lite::io::AsyncWriteExt; use octocrab::models::repos::Content; use octocrab::models::InstallationId; use serde::{Deserialize, Serialize}; -use std::path::PathBuf; use std::{future::Future, pin::Pin}; +use base64::{Engine as _, engine::general_purpose}; pub struct GithubEvent(pub String, pub Option>); @@ -207,8 +205,6 @@ impl CheckRun { } } -static DOWNLOAD_DIR: &str = "download"; - async fn find_content>( installation: &InstallationId, repo: &Repository, @@ -244,34 +240,57 @@ pub async fn download_url>( ) -> Result> { let target = find_content(installation, repo, filename, commit).await?; - let download_url = target - .download_url - .as_ref() - .ok_or_else(|| format_err!("No download URL given by GitHub"))?; - - let response = reqwest::get(download_url).await?; + let content = target.content + .ok_or_else(|| format_err!("File had no content!"))? + .replace("\n", ""); - Ok(response.bytes().await?.to_vec()) + general_purpose::STANDARD + .decode(content) + .map_err(|decode_error| format_err!("DecodeError: {}", decode_error)) } -pub async fn download_file>( - installation: &InstallationId, - repo: &Repository, - filename: S, - commit: S, -) -> Result { - let target = find_content(installation, repo, &filename, &commit).await?; - - let mut path = PathBuf::new(); - path.push("."); - path.push(DOWNLOAD_DIR); - path.push(&target.sha); - path.set_extension("dmi"); // Method should have an IDB qualifier due to being a shared crate - - async_fs::create_dir_all(path.parent().unwrap()).await?; - let mut file = File::create(&path).await?; - - let data = download_url(installation, repo, &filename, &commit).await?; - file.write_all(&data).await?; - Ok(path) +/* local test requires commenting out the .installation(...) call in find_content(), a valid github token with access, and the following dep: actix-rt = "2.9.0" + +#[actix_web::rt::test] +async fn test_private_repo_file_download() { + octocrab::initialise(octocrab::OctocrabBuilder::new() + .personal_token("lol".to_owned()) + .build() + .unwrap()); + + let bytes = download_url( + &InstallationId(0), + &Repository{ + url: "https://api.github.com/repos/Cyberboss/tgstation-private-test".to_owned(), + id:0, + }, ".tgs.yml", "140c79189849ea616f09b3484f8930211d3705cd").await.unwrap(); + + let text = std::str::from_utf8(bytes.as_slice()).unwrap(); + assert_eq!(r#"# This file is used by TGS (https://github.com/tgstation/tgstation-server) clients to quickly initialize a server instance for the codebase +# The format isn't documented anywhere but hopefully we never have to change it. If there are questions, contact the TGS maintainer Cyberboss/@Dominion#0444 +version: 1 +# The BYOND version to use (kept in sync with dependencies.sh by the "TGS Test Suite" CI job) +# Must be interpreted as a string, keep quoted +byond: "514.1588" +# Folders to create in "/Configuration/GameStaticFiles/" +static_files: + # Config directory should be static + - name: config + # This implies the folder should be pre-populated with contents from the repo + populate: true + # Data directory must be static + - name: data +# String dictionary. The value is the location of the file in the repo to upload to TGS. The key is the name of the file to upload to "/Configuration/EventScripts/" +# This one is for Linux hosted servers +linux_scripts: + PreCompile.sh: tools/tgs_scripts/PreCompile.sh + WatchdogLaunch.sh: tools/tgs_scripts/WatchdogLaunch.sh + InstallDeps.sh: tools/tgs_scripts/InstallDeps.sh +# Same as above for Windows hosted servers +windows_scripts: + PreCompile.bat: tools/tgs_scripts/PreCompile.bat +# The security level the game should be run at +security: Trusted +"#, text) } +*/ diff --git a/crates/mapdiffbot2/Cargo.toml b/crates/mapdiffbot2/Cargo.toml index c48e7c35..006b63dd 100644 --- a/crates/mapdiffbot2/Cargo.toml +++ b/crates/mapdiffbot2/Cargo.toml @@ -39,6 +39,7 @@ tokio = { version = "1.32.0", features = ["io-util", "rt"] } mysql_async = "0.32.2" time = "0.3.28" +secrecy = "0.8.0" [target.'cfg(not(target_env = "msvc"))'.dependencies] tikv-jemallocator = "0.5.4" diff --git a/crates/mapdiffbot2/src/git_operations.rs b/crates/mapdiffbot2/src/git_operations.rs index ac83ede6..0414111e 100644 --- a/crates/mapdiffbot2/src/git_operations.rs +++ b/crates/mapdiffbot2/src/git_operations.rs @@ -1,7 +1,8 @@ use eyre::{Context, Result}; +use secrecy::{Secret, ExposeSecret}; use std::path::Path; -use git2::{build::CheckoutBuilder, FetchOptions, Repository}; +use git2::{build::{CheckoutBuilder, RepoBuilder}, FetchOptions, Repository, RemoteCallbacks, Cred}; pub fn fetch_and_get_branches<'a>( base_sha: &str, @@ -9,20 +10,20 @@ pub fn fetch_and_get_branches<'a>( repo: &'a git2::Repository, head_branch_name: &str, base_branch_name: &str, + repo_token: Secret, ) -> Result<(git2::Reference<'a>, git2::Reference<'a>)> { let base_id = git2::Oid::from_str(base_sha).wrap_err("Parsing base sha")?; let head_id = git2::Oid::from_str(head_sha).wrap_err("Parsing head sha")?; let mut remote = repo.find_remote("origin")?; - remote - .connect(git2::Direction::Fetch) - .wrap_err("Connecting to remote")?; + let mut fetch_options = create_fetch_options_for_token(repo_token); + fetch_options.prune(git2::FetchPrune::On); remote .fetch( &[base_branch_name], - Some(FetchOptions::new().prune(git2::FetchPrune::On)), + Some(&mut fetch_options), None, ) .wrap_err("Fetching base")?; @@ -70,7 +71,7 @@ pub fn fetch_and_get_branches<'a>( remote .fetch( &[head_branch_name], - Some(FetchOptions::new().prune(git2::FetchPrune::On)), + Some(&mut fetch_options), None, ) .wrap_err("Fetching head")?; @@ -176,7 +177,34 @@ pub fn with_checkout( f() } -pub fn clone_repo(url: &str, dir: &Path) -> Result<()> { - git2::Repository::clone(url, dir.as_os_str()).wrap_err("Cloning repo")?; +fn create_fetch_options_for_token(repo_token: Secret) -> FetchOptions<'static>{ + let mut callbacks = RemoteCallbacks::new(); + callbacks.credentials(move |_url, _username_from_url, _allowed_types| { + Cred::userpass_plaintext(repo_token.expose_secret(), "") + }); + + let mut fetch_options = git2::FetchOptions::new(); + fetch_options.remote_callbacks(callbacks); + fetch_options +} + +pub fn clone_repo(url: &str, dir: &Path, repo_token: Secret) -> Result<()> { + let mut builder = RepoBuilder::new(); + builder.fetch_options(create_fetch_options_for_token(repo_token)); + + builder.clone(url, dir).wrap_err("Cloning repo")?; Ok(()) } + +/* local testing +#[test] +fn test_private_clone(){ + clone_repo("https://github.com/Cyberboss/tgstation-private-test", &Path::new("S:/garbage/tgtest"), Secret::new("lol".to_string())).unwrap(); +} + +#[test] +fn test_private_fetch(){ + let repo = git2::Repository::open("S:/garbage/tgtest").unwrap(); + fetch_and_get_branches("140c79189849ea616f09b3484f8930211d3705cd", "a34219208f6526d01d88c9fe02cc08554fe29dda", &repo, "TestPRForMDB", "master", Secret::new("lol".to_string())).unwrap(); +} + */ diff --git a/crates/mapdiffbot2/src/job_processor.rs b/crates/mapdiffbot2/src/job_processor.rs index be5db1f1..c7f36131 100644 --- a/crates/mapdiffbot2/src/job_processor.rs +++ b/crates/mapdiffbot2/src/job_processor.rs @@ -1,5 +1,6 @@ use eyre::{Context, Result}; use path_absolutize::Absolutize; +use secrecy::Secret; use std::path::Path; use std::path::PathBuf; @@ -39,6 +40,7 @@ fn render( (repo, base_branch_name): (&git2::Repository, &str), (repo_dir, out_dir, blob_client): (&Path, &Path, Azure), pull_request_number: u64, + repo_token: Secret, // feel like this is a bit of a hack but it works for now ) -> Result { tracing::debug!( @@ -51,7 +53,7 @@ fn render( let head_branch = format!("pull/{pull_request_number}/head:{pull_branch}"); let (base_branch, head_branch) = - fetch_and_get_branches(&base.sha, &head.sha, repo, &head_branch, base_branch_name) + fetch_and_get_branches(&base.sha, &head.sha, repo, &head_branch, base_branch_name, repo_token) .wrap_err("Fetching and constructing diffs")?; let path = repo_dir @@ -331,7 +333,7 @@ fn generate_finished_output>( Ok(builder.build()) } -pub fn do_job(job: Job, blob_client: Azure) -> Result { +pub async fn do_job(job: Job, blob_client: Azure) -> Result { tracing::debug!( "Starting Job on repo: {}, pr number: {}, base commit: {}, head commit: {}", job.repo.full_name(), @@ -342,6 +344,11 @@ pub fn do_job(job: Job, blob_client: Azure) -> Result { let base = &job.base; let head = &job.head; + + let (_, secret_token) = octocrab::instance() + .installation_and_token(job.installation) + .await?; + let repo = format!("https://github.com/{}", job.repo.full_name()); let repo_dir: PathBuf = ["./repos/", &job.repo.full_name()].iter().collect(); @@ -358,7 +365,7 @@ pub fn do_job(job: Job, blob_client: Azure) -> Result { }; let _ = job.check_run.set_output(output).await; // we don't really care if updating the job fails, just continue }); - clone_repo(&repo, &repo_dir).wrap_err("Cloning repo")?; + clone_repo(&repo, &repo_dir, secret_token.clone()).wrap_err("Cloning repo")?; } let non_abs_directory: PathBuf = [ @@ -413,6 +420,7 @@ pub fn do_job(job: Job, blob_client: Azure) -> Result { (&repository, &job.base.r#ref), (&repo_dir, output_directory, blob_client), job.pull_request, + secret_token, ) .wrap_err("") { diff --git a/crates/mapdiffbot2/src/runner.rs b/crates/mapdiffbot2/src/runner.rs index d3ac576d..c8fd7fd0 100644 --- a/crates/mapdiffbot2/src/runner.rs +++ b/crates/mapdiffbot2/src/runner.rs @@ -116,7 +116,7 @@ async fn job_handler(name: &str, job: Job, blob_client: Azure) { let output = actix_web::rt::time::timeout( Duration::from_secs(7200), - actix_web::rt::task::spawn_blocking(move || do_job(job, blob_client)), + do_job(job, blob_client), ) .await; @@ -135,19 +135,6 @@ async fn job_handler(name: &str, job: Job, blob_client: Azure) { output.unwrap() }; - if let Err(e) = output { - let fuckup = match e.try_into_panic() { - Ok(panic) => { - format!("{panic:#?}") - } - Err(e) => e.to_string(), - }; - tracing::error!("Join Handle error: {fuckup}"); - let _ = check_run.mark_failed(&fuckup).await; - return; - } - - let output = output.unwrap(); if let Err(e) = output { let fuckup = format!("{e:?}"); tracing::error!("Other rendering error: {fuckup}"); From 0403e85c843ec391aa04224103e1f89d4529b1cc Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Sat, 18 Nov 2023 01:16:44 -0500 Subject: [PATCH 2/3] Follow async guidelines --- crates/mapdiffbot2/src/job_processor.rs | 10 ++++------ crates/mapdiffbot2/src/runner.rs | 15 ++++++++++++++- 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/crates/mapdiffbot2/src/job_processor.rs b/crates/mapdiffbot2/src/job_processor.rs index c7f36131..70d74fbe 100644 --- a/crates/mapdiffbot2/src/job_processor.rs +++ b/crates/mapdiffbot2/src/job_processor.rs @@ -333,7 +333,7 @@ fn generate_finished_output>( Ok(builder.build()) } -pub async fn do_job(job: Job, blob_client: Azure) -> Result { +pub fn do_job(job: Job, blob_client: Azure) -> Result { tracing::debug!( "Starting Job on repo: {}, pr number: {}, base commit: {}, head commit: {}", job.repo.full_name(), @@ -345,15 +345,13 @@ pub async fn do_job(job: Job, blob_client: Azure) -> Result { let base = &job.base; let head = &job.head; - let (_, secret_token) = octocrab::instance() - .installation_and_token(job.installation) - .await?; + let handle = actix_web::rt::Runtime::new()?; + let (_, secret_token) = handle.block_on(octocrab::instance() + .installation_and_token(job.installation))?; let repo = format!("https://github.com/{}", job.repo.full_name()); let repo_dir: PathBuf = ["./repos/", &job.repo.full_name()].iter().collect(); - let handle = actix_web::rt::Runtime::new()?; - if !repo_dir.exists() { tracing::debug!("Directory {:?} doesn't exist, creating dir", repo_dir); std::fs::create_dir_all(&repo_dir)?; diff --git a/crates/mapdiffbot2/src/runner.rs b/crates/mapdiffbot2/src/runner.rs index c8fd7fd0..d3ac576d 100644 --- a/crates/mapdiffbot2/src/runner.rs +++ b/crates/mapdiffbot2/src/runner.rs @@ -116,7 +116,7 @@ async fn job_handler(name: &str, job: Job, blob_client: Azure) { let output = actix_web::rt::time::timeout( Duration::from_secs(7200), - do_job(job, blob_client), + actix_web::rt::task::spawn_blocking(move || do_job(job, blob_client)), ) .await; @@ -135,6 +135,19 @@ async fn job_handler(name: &str, job: Job, blob_client: Azure) { output.unwrap() }; + if let Err(e) = output { + let fuckup = match e.try_into_panic() { + Ok(panic) => { + format!("{panic:#?}") + } + Err(e) => e.to_string(), + }; + tracing::error!("Join Handle error: {fuckup}"); + let _ = check_run.mark_failed(&fuckup).await; + return; + } + + let output = output.unwrap(); if let Err(e) = output { let fuckup = format!("{e:?}"); tracing::error!("Other rendering error: {fuckup}"); From 7b927fc85408939a42a16de8b4d5d0610888bb85 Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Sat, 18 Nov 2023 03:06:47 -0500 Subject: [PATCH 3/3] Fix MDB cloning/fetching --- crates/mapdiffbot2/src/git_operations.rs | 54 ++++-------------------- crates/mapdiffbot2/src/job_processor.rs | 19 +++++---- 2 files changed, 20 insertions(+), 53 deletions(-) diff --git a/crates/mapdiffbot2/src/git_operations.rs b/crates/mapdiffbot2/src/git_operations.rs index 0414111e..85098b85 100644 --- a/crates/mapdiffbot2/src/git_operations.rs +++ b/crates/mapdiffbot2/src/git_operations.rs @@ -1,8 +1,7 @@ use eyre::{Context, Result}; -use secrecy::{Secret, ExposeSecret}; use std::path::Path; -use git2::{build::{CheckoutBuilder, RepoBuilder}, FetchOptions, Repository, RemoteCallbacks, Cred}; +use git2::{build::CheckoutBuilder, FetchOptions, Repository}; pub fn fetch_and_get_branches<'a>( base_sha: &str, @@ -10,23 +9,23 @@ pub fn fetch_and_get_branches<'a>( repo: &'a git2::Repository, head_branch_name: &str, base_branch_name: &str, - repo_token: Secret, ) -> Result<(git2::Reference<'a>, git2::Reference<'a>)> { let base_id = git2::Oid::from_str(base_sha).wrap_err("Parsing base sha")?; let head_id = git2::Oid::from_str(head_sha).wrap_err("Parsing head sha")?; let mut remote = repo.find_remote("origin")?; - let mut fetch_options = create_fetch_options_for_token(repo_token); - fetch_options.prune(git2::FetchPrune::On); + remote + .connect(git2::Direction::Fetch) + .wrap_err("Connecting to remote")?; remote .fetch( - &[base_branch_name], - Some(&mut fetch_options), + &[base_branch_name, head_branch_name], + Some(FetchOptions::new().prune(git2::FetchPrune::On)), None, ) - .wrap_err("Fetching base")?; + .wrap_err("Fetching base and head")?; let fetch_head = repo .find_reference("FETCH_HEAD") .wrap_err("Getting FETCH_HEAD")?; @@ -68,14 +67,6 @@ pub fn fetch_and_get_branches<'a>( .resolve_reference_from_short_name(base_branch_name) .wrap_err("Getting the base reference")?; - remote - .fetch( - &[head_branch_name], - Some(&mut fetch_options), - None, - ) - .wrap_err("Fetching head")?; - let fetch_head = repo .find_reference("FETCH_HEAD") .wrap_err("Getting FETCH_HEAD")?; @@ -177,34 +168,7 @@ pub fn with_checkout( f() } -fn create_fetch_options_for_token(repo_token: Secret) -> FetchOptions<'static>{ - let mut callbacks = RemoteCallbacks::new(); - callbacks.credentials(move |_url, _username_from_url, _allowed_types| { - Cred::userpass_plaintext(repo_token.expose_secret(), "") - }); - - let mut fetch_options = git2::FetchOptions::new(); - fetch_options.remote_callbacks(callbacks); - fetch_options -} - -pub fn clone_repo(url: &str, dir: &Path, repo_token: Secret) -> Result<()> { - let mut builder = RepoBuilder::new(); - builder.fetch_options(create_fetch_options_for_token(repo_token)); - - builder.clone(url, dir).wrap_err("Cloning repo")?; +pub fn clone_repo(url: &str, dir: &Path) -> Result<()> { + git2::Repository::clone(url, dir.as_os_str()).wrap_err("Cloning repo")?; Ok(()) } - -/* local testing -#[test] -fn test_private_clone(){ - clone_repo("https://github.com/Cyberboss/tgstation-private-test", &Path::new("S:/garbage/tgtest"), Secret::new("lol".to_string())).unwrap(); -} - -#[test] -fn test_private_fetch(){ - let repo = git2::Repository::open("S:/garbage/tgtest").unwrap(); - fetch_and_get_branches("140c79189849ea616f09b3484f8930211d3705cd", "a34219208f6526d01d88c9fe02cc08554fe29dda", &repo, "TestPRForMDB", "master", Secret::new("lol".to_string())).unwrap(); -} - */ diff --git a/crates/mapdiffbot2/src/job_processor.rs b/crates/mapdiffbot2/src/job_processor.rs index 70d74fbe..2f61b266 100644 --- a/crates/mapdiffbot2/src/job_processor.rs +++ b/crates/mapdiffbot2/src/job_processor.rs @@ -1,6 +1,6 @@ use eyre::{Context, Result}; use path_absolutize::Absolutize; -use secrecy::Secret; +use secrecy::ExposeSecret; use std::path::Path; use std::path::PathBuf; @@ -40,7 +40,6 @@ fn render( (repo, base_branch_name): (&git2::Repository, &str), (repo_dir, out_dir, blob_client): (&Path, &Path, Azure), pull_request_number: u64, - repo_token: Secret, // feel like this is a bit of a hack but it works for now ) -> Result { tracing::debug!( @@ -53,7 +52,7 @@ fn render( let head_branch = format!("pull/{pull_request_number}/head:{pull_branch}"); let (base_branch, head_branch) = - fetch_and_get_branches(&base.sha, &head.sha, repo, &head_branch, base_branch_name, repo_token) + fetch_and_get_branches(&base.sha, &head.sha, repo, &head_branch, base_branch_name) .wrap_err("Fetching and constructing diffs")?; let path = repo_dir @@ -349,10 +348,11 @@ pub fn do_job(job: Job, blob_client: Azure) -> Result { let (_, secret_token) = handle.block_on(octocrab::instance() .installation_and_token(job.installation))?; - let repo = format!("https://github.com/{}", job.repo.full_name()); let repo_dir: PathBuf = ["./repos/", &job.repo.full_name()].iter().collect(); - if !repo_dir.exists() { + let url = format!("https://x-access-token:{}@github.com/{}", secret_token.expose_secret(), job.repo.full_name()); + let clone_required = !repo_dir.exists(); + if clone_required { tracing::debug!("Directory {:?} doesn't exist, creating dir", repo_dir); std::fs::create_dir_all(&repo_dir)?; handle.block_on(async { @@ -363,7 +363,7 @@ pub fn do_job(job: Job, blob_client: Azure) -> Result { }; let _ = job.check_run.set_output(output).await; // we don't really care if updating the job fails, just continue }); - clone_repo(&repo, &repo_dir, secret_token.clone()).wrap_err("Cloning repo")?; + clone_repo(&url, &repo_dir).wrap_err("Cloning repo")?; } let non_abs_directory: PathBuf = [ @@ -397,6 +397,10 @@ pub fn do_job(job: Job, blob_client: Azure) -> Result { let repository = git2::Repository::open(&repo_dir).wrap_err("Opening repository")?; + if !clone_required { + repository.remote_set_url("origin", &url)?; + } + let mut remote = repository.find_remote("origin")?; remote @@ -417,8 +421,7 @@ pub fn do_job(job: Job, blob_client: Azure) -> Result { (&added_files, &modified_files, &removed_files), (&repository, &job.base.r#ref), (&repo_dir, output_directory, blob_client), - job.pull_request, - secret_token, + job.pull_request ) .wrap_err("") {