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

github-on-prem - auth_info retrieval from github app #14

Merged
merged 8 commits into from
Oct 21, 2023

Conversation

MuskanPaliwal
Copy link
Member

@MuskanPaliwal MuskanPaliwal commented Oct 14, 2023

  • Changed Docker file
  • Tested the whole code and fixed a few errors

Summary by CodeRabbit

  • New Feature: Added GitHub authentication support, enhancing the software's compatibility with different platforms.
  • New Feature: Introduced Bitbucket comment and reviewer modules, enabling users to add comments and reviewers to Bitbucket pull requests directly from the application.
  • New Feature: Implemented a shared instance of reqwest::Client across the application, improving performance and resource usage.
  • New Feature: Added code coverage calculation and review processing functionalities, providing users with more insights into their code quality.
  • Refactor: Improved error handling and logging across various functions, enhancing the robustness and maintainability of the software.
  • Refactor: Renamed and restructured several functions for better clarity and consistency.
  • Chore: Updated Dockerfile and added new environment variables for GitHub authentication, simplifying the setup process.

These updates aim to improve the software's usability, extend its functionality, and enhance its performance and reliability.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 14, 2023

Walkthrough

The changes primarily focus on enhancing the functionality of the vibi-dpu application by adding new modules and refactoring existing ones. The updates include improved error handling, logging, and code modularity. The codebase now supports both Bitbucket and GitHub, with the addition of new functions for authentication, commenting, reviewing, and webhook handling. The changes also introduce shared HTTP client instances, improved database operations, and new utilities for handling Git operations and code hunks.

Changes

File(s) Summary
Dockerfile Environment variables for GitHub authentication added. Updated the name of the JSON file being copied. Added a new file being copied.
vibi-dpu/src/bitbucket/*.rs Refactored Bitbucket related modules to improve error handling, modularity, and added new functionalities like commenting, reviewing, and webhook handling.
vibi-dpu/src/github/*.rs Added a new module for GitHub authentication.
vibi-dpu/src/client/*.rs Introduced a shared HTTP client instance across the application.
vibi-dpu/src/db/*.rs Improved database operations with better error handling and logging. Added new functionalities for handling hunks, reviews, repo configurations, and users.
vibi-dpu/src/core/*.rs Added new modules for handling reviews and code coverage.
vibi-dpu/src/setup/*.rs Added new modules for setting up Bitbucket and GitHub.
vibi-dpu/src/pubsub/listener.rs Refactored the message processing function and improved handling of install callbacks.
vibi-dpu/src/utils/*.rs Added new utilities for handling Git operations, code hunks, reviews, and repository configurations.
vibi-dpu/src/main.rs Added new modules for setup, GitHub, client, and database.

Tips

Chat with CodeRabbit Bot (@coderabbitai)

  • Mention @coderabbitai in any review comment for bot assistance.
  • Note: Review comments are made on code diffs or files, not on the PR overview.

Pause Incremental Reviews

  • Insert @coderabbitai: ignore in the PR description to halt the bot's ongoing reviews. Remove the line to resume.
  • Additionally, enter @coderabbitai pause to halt the bot's ongoing reviews as any issue or review comment. To
    resume reviews, enter @coderabbitai resume as any issue or review comment.

@MuskanPaliwal MuskanPaliwal changed the base branch from main to tr/github October 14, 2023 06:57
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 14, 2023

CodeRabbit review skipped

By default, CodeRabbit only reviews PRs on the default branch. If you wish to have PRs reviewed on additional branches, you can configure that under the repository settings in the UI.

@MuskanPaliwal MuskanPaliwal requested a review from tapishr October 14, 2023 06:57
Copy link
Member

@tapishr tapishr left a comment

Choose a reason for hiding this comment

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

error handling requires fixing for the given functions

@MuskanPaliwal MuskanPaliwal requested a review from tapishr October 16, 2023 06:12
Comment on lines 23 to 29
let pem_data = match fs::read(pem_file_path) {
Ok(data) => data,
Err(e) => {
println!("Error reading pem file: {}", e);
return None;
},
};
Copy link
Member

Choose a reason for hiding this comment

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

Don't use match for anything except debugging. You should read the result of fs::read(pem_file) in a variable, check if that variable is_err(), print the error and return . if not, the rest of the code follows. This helps in keeping the code clean (separation of logic and error handling). For reference, you can see any function in gitops.rs, like - get_excluded_files

Copy link
Member Author

Choose a reason for hiding this comment

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

done!

Comment on lines 36 to 49
let encoding_key = match EncodingKey::from_rsa_pem(&pem_data) {
Ok(key) => key,
Err(e) => {
println!("Error creating encoding key: {}", e);
return None;
},
};
match encode(&Header::new(Algorithm::RS256), &my_claims, &encoding_key) {
Ok(token) => Some(token),
Err(e) => {
println!("Error encoding JWT: {}", e);
None
},
}
Copy link
Member

Choose a reason for hiding this comment

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

remove match, same as above

Copy link
Member Author

Choose a reason for hiding this comment

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

yess yess!

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

Choose a reason for hiding this comment

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

This will be pem_data_res

println!("Error reading pem file: {:?}", pem_data_err);
return None;
}
let pem_data_res = pem_data.expect("Error reading pem file");
Copy link
Member

Choose a reason for hiding this comment

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

This will be named pem_data, and then used further along

println!("Error encoding JWT: {:?}", token_err);
return None;
};
let token_result = token.expect("Error encoding JWT");
Copy link
Member

Choose a reason for hiding this comment

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

Same as above

Copy link
Member Author

Choose a reason for hiding this comment

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

I just did

@sonarcloud
Copy link

sonarcloud bot commented Oct 21, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot E 2 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

idea Catch issues before they fail your Quality Gate with our IDE extension sonarlint SonarLint

@MuskanPaliwal MuskanPaliwal requested a review from tapishr October 21, 2023 05:25
@tapishr tapishr merged commit 0b549f0 into tr/github Oct 21, 2023
1 of 2 checks passed
@tapishr tapishr deleted the mkp/github-on-prem branch October 21, 2023 05:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants