Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Initial setup for adding GitHub on-prem support. #65

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ ARG BITBUCKET_CLIENT_SECRET
ARG BITBUCKET_BASE_URL
ARG INSTALL_ID
ARG SERVER_URL
ARG GITHUB_APP_ID
ARG GITHUB_APP_CLIENT_ID
ARG GITHUB_APP_CLIENT_SECRET
ARG GITHUB_BASE_URL


ENV GCP_CREDENTIALS=$GCP_CREDENTIALS
Expand All @@ -29,9 +33,14 @@ ENV BITBUCKET_CLIENT_SECRET=$BITBUCKET_CLIENT_SECRET
ENV BITBUCKET_BASE_URL=$BITBUCKET_BASE_URL
ENV INSTALL_ID=$INSTALL_ID
ENV SERVER_URL=$SERVER_URL
ENV GITHUB_APP_ID=$GITHUB_APP_ID
ENV GITHUB_APP_CLIENT_ID=$GITHUB_APP_CLIENT_ID
ENV GITHUB_APP_CLIENT_SECRET=$GITHUB_APP_CLIENT_SECRET
ENV GITHUB_BASE_URL=$GITHUB_BASE_URL

COPY ./vibi-dpu/target/debug/vibi-dpu /app/vibi-dpu
COPY ./pubsub-sa.json /app/pubsub-sa.json
COPY ./repo-profiler.pem /app/repo-profiler.pem

# Start the Rust application
CMD ["/app/vibi-dpu"]
1 change: 1 addition & 0 deletions vibi-dpu/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -34,5 +34,6 @@ rand = "0.8.5"
chrono = "0.4.26"
tonic = "0.9.2"
once_cell = "1.18.0" # MIT
jsonwebtoken = "8.3.0" # MIT

# todo - check all lib licences
3 changes: 1 addition & 2 deletions vibi-dpu/src/bitbucket/auth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,8 @@ use std::env;
use std::str;
use std::process::Command;
use std::time::{SystemTime, UNIX_EPOCH};
use reqwest::Client;
use super::config::get_client;
use crate::db::auth::{save_auth_info_to_db, auth_info};
use crate::utils::reqwest_client::get_client;
use crate::utils::auth::AuthInfo;
use crate::utils::review::Review;

Expand Down
11 changes: 2 additions & 9 deletions vibi-dpu/src/bitbucket/comment.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,8 @@
use std::{env, collections::HashMap};

use reqwest::{Response, header::{HeaderMap, HeaderValue}};
use serde::Serialize;
use serde_json::Value;

use crate::db::auth::auth_info;
use crate::db::user::get_workspace_user_from_db;
use crate::utils::review::Review;
use crate::utils::user::BitbucketUser;

use super::{config::{bitbucket_base_url, get_client, prepare_headers}};
use crate::utils::reqwest_client::get_client;
use super::config::{bitbucket_base_url, prepare_headers};

#[derive(Serialize)]
struct Comment {
Expand Down
13 changes: 1 addition & 12 deletions vibi-dpu/src/bitbucket/config.rs
Original file line number Diff line number Diff line change
@@ -1,19 +1,8 @@
use std::{env, collections::HashMap};
use std::error::Error;

use reqwest::{Response, header::{HeaderMap, HeaderValue}};
use serde_json::Value;
use std::sync::Arc;
use once_cell::sync::Lazy;
use reqwest::Client;

static CLIENT: Lazy<Arc<Client>> = Lazy::new(|| {
Arc::new(Client::new())
});

pub fn get_client() -> Arc<Client> {
Arc::clone(&CLIENT)
}
use crate::utils::reqwest_client::get_client;

pub fn bitbucket_base_url() -> String {
env::var("BITBUCKET_BASE_URL").expect("BITBUCKET_BASE_URL must be set")
Expand Down
5 changes: 3 additions & 2 deletions vibi-dpu/src/bitbucket/prs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@ use reqwest::header::HeaderMap;
use serde_json::Value;
use std::collections::HashMap;
use std::str;
use crate::{utils::pr_info::PrInfo, db::prs::update_pr_info_in_db};
use crate::utils::{pr_info::PrInfo, reqwest_client::get_client};
use crate::db::prs::update_pr_info_in_db;

use super::config::{get_client, prepare_auth_headers, bitbucket_base_url};
use super::config::{prepare_auth_headers, bitbucket_base_url};

pub async fn list_prs_bitbucket(repo_owner: &str, repo_name: &str, access_token: &str, state: &str) -> Option<Vec<String>> {
let headers_opt = prepare_auth_headers(access_token);
Expand Down
3 changes: 2 additions & 1 deletion vibi-dpu/src/bitbucket/reviewer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@ use serde_json::Value;

use crate::utils::review::Review;
use crate::utils::user::BitbucketUser;
use crate::utils::reqwest_client::get_client;

use super::config::{get_client, prepare_headers};
use super::config::prepare_headers;

pub async fn add_reviewers(user: &BitbucketUser, review: &Review, access_token: &str) {
let url = prepare_get_prinfo_url(review.repo_owner(), review.repo_name(), review.id());
Expand Down
4 changes: 2 additions & 2 deletions vibi-dpu/src/bitbucket/webhook.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ use reqwest::{header::HeaderValue, Response, Error};
use serde_json::json;

use crate::{db::webhook::save_webhook_to_db, utils::webhook::{Webhook, WebhookResponse}, bitbucket::config::{bitbucket_base_url, get_api_values}};

use super::config::{get_client, prepare_auth_headers};
use crate::utils::reqwest_client::get_client;
use super::config::prepare_auth_headers;


pub async fn get_webhooks_in_repo(workspace_slug: &str, repo_slug: &str, access_token: &str) -> Vec<Webhook> {
Expand Down
1 change: 1 addition & 0 deletions vibi-dpu/src/core/bitbucket/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
pub mod setup;
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use serde::{Deserialize, Serialize};
use tokio::{task, fs};

use crate::bitbucket::auth::get_access_token_from_bitbucket;
use crate::bitbucket::config::get_client;
use crate::utils::reqwest_client::get_client;
use crate::bitbucket::repo::get_workspace_repos;
use crate::bitbucket::workspace::get_bitbucket_workspaces;
use crate::bitbucket::webhook::{get_webhooks_in_repo, add_webhook};
Expand Down Expand Up @@ -94,7 +94,6 @@ pub async fn handle_install_bitbucket(installation_code: &str) {
send_setup_info(&pubreqs).await;
}


async fn send_setup_info(setup_info: &Vec<SetupInfo>) {
let installation_id = env::var("INSTALL_ID")
.expect("INSTALL_ID must be set");
Comment on lines 94 to 99
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function send_setup_info is missing error handling for the case when the environment variables "INSTALL_ID" and "SERVER_URL" are not set. This could lead to a panic at runtime. Consider using env::var().ok() instead of env::var().expect(), and handle the None case appropriately.

let installation_id = env::var("INSTALL_ID").ok();
if installation_id.is_none() {
    eprintln!("INSTALL_ID must be set");
    return;
}
let base_url = env::var("SERVER_URL").ok();
if base_url.is_none() {
    eprintln!("SERVER_URL must be set");
    return;
}

The rest of the code looks fine, but it's hard to review without the full context. It would be helpful to see how these functions are used in the larger application.

Expand Down
1 change: 1 addition & 0 deletions vibi-dpu/src/core/github/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
pub mod setup;
46 changes: 46 additions & 0 deletions vibi-dpu/src/core/github/setup.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
// setup_gh.rs

use crate::github::auth::fetch_access_token; // Import shared utilities
use reqwest::Client;
use serde::{Deserialize, Serialize};
use tokio::task;

#[derive(Debug, Deserialize, Serialize, Clone)]
struct SetupInfoGithub {
provider: String,
owner: String,
repos: Vec<String>,
}

#[derive(Debug, Deserialize, Serialize, Clone)]
struct PublishRequestGithub {
installationId: String,
info: Vec<SetupInfoGithub>,
}

pub async fn handle_install_github(installation_code: &str) {
// TODO: Implement the logic to handle GitHub installation

// For example:
// 1. Get access token from GitHub using the installation code
let auth_info = fetch_access_token(installation_code).await;
println!("[handle_install_github] auth_info = {:?}", &auth_info);
// 2. Fetch user repositories and other necessary data
// 3. Process webhooks or other setup tasks
// 4. Send setup info or any other post-processing
}

async fn get_github_repositories(access_token: &str) -> Vec<String> {
// TODO: Implement the logic to fetch user repositories from GitHub
Vec::new()
}

async fn process_webhooks_github(repo_name: String, access_token: String) {
// TODO: Implement the logic to process GitHub webhooks
}

async fn send_setup_info_github(setup_info: &Vec<SetupInfoGithub>) {
// TODO: Implement the logic to send setup info for GitHub
}

// Add other necessary functions and utilities specific to GitHub setup
6 changes: 4 additions & 2 deletions vibi-dpu/src/core/mod.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
pub mod setup;
pub mod review;
mod coverage;
mod coverage;

pub mod bitbucket;
pub mod github;
4 changes: 2 additions & 2 deletions vibi-dpu/src/core/review.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,12 @@ use crate::{
get_excluded_files,
generate_diff,
process_diffmap,
generate_blame}},
generate_blame},
reqwest_client::get_client},
db::{hunk::{get_hunk_from_db, store_hunkmap_to_db},
repo::get_clone_url_clone_dir,
review::save_review_to_db,
repo_config::save_repo_config_to_db},
bitbucket::config::get_client,
core::coverage::process_coverage};

pub async fn process_review(message_data: &Vec<u8>) {
Expand Down
89 changes: 89 additions & 0 deletions vibi-dpu/src/github/auth.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
use jsonwebtoken::{encode, Header, EncodingKey, Algorithm};
use serde::{Deserialize, Serialize};
use serde_json::Value;
use std::env;
use chrono::{Utc, Duration};
use std::fs;
use crate::{utils::reqwest_client::get_client, utils::auth::AuthInfo};

#[derive(Debug, Serialize, Deserialize)]
struct AccessTokenResponse {
token: String,
// Add other fields if necessary
}

#[derive(Debug, Serialize)]
struct Claims {
iat: i64,
exp: i64,
iss: String,
}

fn generate_jwt(github_app_id: &str) -> Option<String> {
let pem_file_path = "/app/repoprofiler_private.pem";
let pem_data_res = fs::read(pem_file_path);

if pem_data_res.is_err() {
let pem_data_err = pem_data_res.expect_err("No error in reading pem file");
println!("Error reading pem file: {:?}", pem_data_err);
return None;
}
let pem_data = pem_data_res.expect("Error reading pem file");

let my_claims = Claims {
iat: Utc::now().timestamp(),
exp: (Utc::now() + Duration::minutes(5)).timestamp(),
iss: github_app_id.to_string(),
};

let encoding_key = EncodingKey::from_rsa_pem(&pem_data);
if encoding_key.is_err() {
println!("Error creating encoding key");
return None;
}

let token_res = encode(&Header::new(Algorithm::RS256), &my_claims, &encoding_key.unwrap());
if token_res.is_err() {
let token_err = token_res.expect_err("No error in fetching token");
println!("Error encoding JWT: {:?}", token_err);
return None;
};
let token = token_res.expect("Error encoding JWT");
Some(token)
}
Comment on lines +22 to +53
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The generate_jwt function reads a private key from a file, creates a JWT token, and returns it. The function handles errors correctly and returns None if any step fails. However, it would be better to return a Result instead of an Option to propagate the error message to the caller. This would help in debugging if something goes wrong. Also, consider using the ? operator to simplify error handling.

fn generate_jwt(github_app_id: &str) -> Result<String, Box<dyn std::error::Error>> {
    let pem_file_path = "/app/repoprofiler_private.pem";
    let pem_data = fs::read(pem_file_path)?;

    let my_claims = Claims {
        iat: Utc::now().timestamp(),
        exp: (Utc::now() + Duration::minutes(5)).timestamp(),
        iss: github_app_id.to_string(),
    };

    let encoding_key = EncodingKey::from_rsa_pem(&pem_data)?;
    let token = encode(&Header::new(Algorithm::RS256), &my_claims, &encoding_key)?;

    Ok(token)
}


pub async fn fetch_access_token(installation_id: &str) -> Option<Value> {
let github_app_id = env::var("GITHUB_APP_ID");
let github_app_id_str = github_app_id.expect("GITHUB_APP_ID must be set");
let jwt_token = generate_jwt(&github_app_id_str).expect("Error generating JWT");

let client = get_client();
let response = client.post(&format!("https://api.github.com/app/installations/{}/access_tokens", installation_id))
.header("Accept", "application/vnd.github+json")
.header("Authorization", format!("Bearer {}", jwt_token))
.header("User-Agent", "Vibinex code review Test App")
.send()
.await;
if response.is_err() {
let e = response.expect_err("No error in response");
eprintln!("error in calling github api : {:?}", e);
return None;
}
let response_access_token = response.expect("Uncaught error in reponse");
if !response_access_token.status().is_success() {
println!(
"Failed to exchange code for access token. Status code: {}, Response content: {:?}",
response_access_token.status(),
response_access_token.text().await
);
return None;
}
let parse_res = response_access_token.json().await ;
if parse_res.is_err() {
let e = parse_res.expect_err("No error in parse_res for AuthInfo");
eprintln!("error deserializing AuthInfo: {:?}", e);
return None;
}
let response_json: Value = parse_res.expect("Uncaught error in parse_res for AuthInfo");
return Some(response_json);
}
Comment on lines +55 to +89
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fetch_access_token function fetches an access token from the GitHub API. It handles errors correctly and returns None if any step fails. However, similar to the generate_jwt function, it would be better to return a Result instead of an Option to propagate the error message to the caller. Also, consider using the ? operator to simplify error handling.

pub async fn fetch_access_token(installation_id: &str) -> Result<Value, Box<dyn std::error::Error>> {
    let github_app_id = env::var("GITHUB_APP_ID")?;
    let jwt_token = generate_jwt(&github_app_id)?;

    let client = get_client();
    let response = client.post(&format!("https://api.github.com/app/installations/{}/access_tokens", installation_id))
        .header("Accept", "application/vnd.github+json")
        .header("Authorization", format!("Bearer {}", jwt_token))
        .header("User-Agent", "Vibinex code review Test App")
        .send()
        .await?;

    if !response.status().is_success() {
        return Err(format!(
            "Failed to exchange code for access token. Status code: {}, Response content: {:?}",
            response.status(),
            response.text().await
        ).into());
    }

    let response_json: Value = response.json().await?;
    return Ok(response_json);
}

1 change: 1 addition & 0 deletions vibi-dpu/src/github/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
pub mod auth;
1 change: 1 addition & 0 deletions vibi-dpu/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ mod pubsub;
mod db;
mod core;
mod bitbucket;
mod github;
mod utils;

#[tokio::main]
Expand Down
3 changes: 2 additions & 1 deletion vibi-dpu/src/pubsub/listener.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ use std::collections::VecDeque;
use sha256::digest;
use tonic::Code;
use crate::db::prs::process_and_update_pr_if_different;
use crate::core::{setup::handle_install_bitbucket, review::process_review};
use crate::core::review::process_review;
use crate::core::bitbucket::setup::handle_install_bitbucket;

#[derive(Debug, Deserialize)]
struct InstallCallback {
Expand Down
3 changes: 2 additions & 1 deletion vibi-dpu/src/utils/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,5 @@ pub mod gitops;
pub mod user;
pub mod lineitem;
pub mod repo_config;
pub mod pr_info;
pub mod pr_info;
pub mod reqwest_client;
11 changes: 11 additions & 0 deletions vibi-dpu/src/utils/reqwest_client.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
use std::sync::Arc;
use once_cell::sync::Lazy;
use reqwest::Client;

static CLIENT: Lazy<Arc<Client>> = Lazy::new(|| {
Arc::new(Client::new())
});

pub fn get_client() -> Arc<Client> {
Arc::clone(&CLIENT)
}