Skip to content

Commit b6cde87

Browse files
authored
(Chore): Support overriding codeowners type (#1053)
* more logs * impl * env for rspec * address comments
1 parent ce2568e commit b6cde87

11 files changed

Lines changed: 169 additions & 21 deletions

File tree

Cargo.lock

Lines changed: 14 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

bundle/src/files.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use std::{
66
};
77

88
use chrono::{DateTime, Utc, serde::ts_milliseconds};
9-
use codeowners::{CodeOwners, Owners, OwnersOfPath};
9+
use codeowners::{CodeOwners, Owners, OwnersOfPath, OwnersSource};
1010
use constants::ALLOW_LIST;
1111
use context::junit::junit_path::{
1212
JunitReportFileWithTestRunnerReport, TestRunnerReport, TestRunnerReportStatus,
@@ -35,11 +35,12 @@ impl FileSetBuilder {
3535
repo_root: T,
3636
junit_paths: &[JunitReportFileWithTestRunnerReport],
3737
codeowners_path: &Option<U>,
38+
codeowners_type: Option<OwnersSource>,
3839
exec_start: Option<SystemTime>,
3940
) -> anyhow::Result<Self> {
4041
let repo_root = repo_root.as_ref();
4142

42-
let codeowners = CodeOwners::find_file(repo_root, codeowners_path);
43+
let codeowners = CodeOwners::find_file(repo_root, codeowners_path, codeowners_type);
4344

4445
let file_set_builder =
4546
Self::file_sets_from_glob(repo_root, junit_paths, codeowners, exec_start)?;

cli/src/context.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ use bundle::{
1515
FileSet, FileSetBuilder, FileSetType, INTERNAL_BIN_FILENAME, META_VERSION,
1616
QuarantineBulkTestStatus, bin_parse,
1717
};
18-
use codeowners::CodeOwners;
18+
use codeowners::{CodeOwners, OwnersSource};
1919
use constants::{ENVS_TO_GET, TRUNK_PR_NUMBER_ENV};
2020
use context::{
2121
bazel_bep::{
@@ -212,13 +212,15 @@ pub fn gather_post_test_context<U: AsRef<Path>>(
212212
meta: &mut BundleMeta,
213213
junit_path_wrappers: Vec<JunitReportFileWithTestRunnerReport>,
214214
codeowners_path: &Option<U>,
215+
codeowners_type: Option<OwnersSource>,
215216
allow_empty_test_results: bool,
216217
test_run_result: &Option<TestRunResult>,
217218
) -> anyhow::Result<FileSetBuilder> {
218219
let mut file_set_builder = FileSetBuilder::build_file_sets(
219220
&meta.base_props.repo.repo_root,
220221
&junit_path_wrappers,
221222
codeowners_path,
223+
codeowners_type,
222224
test_run_result.as_ref().and_then(|r| r.exec_start),
223225
)?;
224226

cli/src/upload_command.rs

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ use api::client::{ApiClient, ApiErrorEndpoint};
77
use api::{client::get_api_host, urls::url_for_test_case};
88
use bundle::{BundleMeta, BundlerUtil, Test, unzip_tarball};
99
use clap::{ArgAction, Args};
10+
use codeowners::OwnersSource;
1011
use constants::EXIT_SUCCESS;
1112
use context::bazel_bep::common::BepParseResult;
1213
use display::{end_output::EndOutput, message::DisplayMessage};
@@ -123,6 +124,13 @@ pub struct UploadArgs {
123124
help = "Override the path to a CODEOWNERS file. Used to associate test failures with code owners."
124125
)]
125126
pub codeowners_path: Option<String>,
127+
#[arg(
128+
long,
129+
env = constants::TRUNK_CODEOWNERS_TYPE_ENV,
130+
value_enum,
131+
help = "Specify CODEOWNERS parser type ('github' or 'gitlab'). Optional; when unset, codeowners type will be inferred."
132+
)]
133+
pub codeowners_type: Option<CodeownersType>,
126134
#[arg(
127135
long,
128136
help = "Run commands with the quarantining step. Deprecated, prefer --disable-quarantining, which takes priority over this flag, to control quarantining.",
@@ -259,6 +267,23 @@ pub struct UploadArgs {
259267
pub show_failure_messages: bool,
260268
}
261269

270+
#[derive(clap::ValueEnum, Clone, Copy, Debug, Default, PartialEq, Eq)]
271+
#[value(rename_all = "lower")]
272+
pub enum CodeownersType {
273+
#[default]
274+
GitHub,
275+
GitLab,
276+
}
277+
278+
impl From<CodeownersType> for OwnersSource {
279+
fn from(value: CodeownersType) -> Self {
280+
match value {
281+
CodeownersType::GitHub => OwnersSource::GitHub,
282+
CodeownersType::GitLab => OwnersSource::GitLab,
283+
}
284+
}
285+
}
286+
262287
fn parse_sha(s: &str) -> anyhow::Result<String> {
263288
if s.len() > 40 {
264289
anyhow::bail!(anyhow::Error::msg(format!(
@@ -384,6 +409,7 @@ pub async fn run_upload(
384409
&mut meta,
385410
junit_path_wrappers,
386411
&upload_args.codeowners_path,
412+
upload_args.codeowners_type.map(Into::into),
387413
upload_args.allow_empty_test_results,
388414
&test_run_result,
389415
)?;

cli/src/validate_command.rs

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use std::{
66

77
use bundle::{FileSet, FileSetBuilder, FileSetTestRunnerReport};
88
use clap::{ArgAction, Args, arg};
9-
use codeowners::CodeOwners;
9+
use codeowners::{CodeOwners, OwnersSource};
1010
use constants::{EXIT_FAILURE, EXIT_SUCCESS};
1111
use context::{
1212
bazel_bep::{common::BepParseResult, parser::BazelBepParser},
@@ -31,7 +31,7 @@ use superconsole::{
3131

3232
use crate::{
3333
context::fall_back_to_binary_parse, error_report::InterruptingError,
34-
report_limiting::ValidationReport,
34+
report_limiting::ValidationReport, upload_command::CodeownersType,
3535
};
3636

3737
#[derive(Args, Clone, Debug)]
@@ -66,6 +66,13 @@ pub struct ValidateArgs {
6666
help = "Override the path to a CODEOWNERS file. Used to validate code ownership associations."
6767
)]
6868
pub codeowners_path: Option<String>,
69+
#[arg(
70+
long,
71+
env = constants::TRUNK_CODEOWNERS_TYPE_ENV,
72+
value_enum,
73+
help = "Specify CODEOWNERS parser type ('github' or 'gitlab'). Optional; when unset, codeowners type will be inferred."
74+
)]
75+
pub codeowners_type: Option<CodeownersType>,
6976
#[arg(
7077
long,
7178
help = "Deprecated (does nothing, left in to avoid breaking existing flows)",
@@ -456,6 +463,7 @@ pub async fn run_validate(validate_args: ValidateArgs) -> anyhow::Result<Validat
456463
test_reports,
457464
show_warnings: _,
458465
codeowners_path,
466+
codeowners_type,
459467
..
460468
} = validate_args;
461469

@@ -496,7 +504,13 @@ pub async fn run_validate(validate_args: ValidateArgs) -> anyhow::Result<Validat
496504
),
497505
}
498506
};
499-
validate(junit_file_paths, codeowners_path, bep_validate_result).await
507+
validate(
508+
junit_file_paths,
509+
codeowners_path,
510+
codeowners_type.map(OwnersSource::from),
511+
bep_validate_result,
512+
)
513+
.await
500514
}
501515

502516
type JunitFileToReportAndParseIssues = BTreeMap<
@@ -514,14 +528,20 @@ type JunitFileToValidation = BTreeMap<String, JunitReportValidation>;
514528
async fn validate(
515529
junit_paths: Vec<JunitReportFileWithTestRunnerReport>,
516530
codeowners_path: Option<String>,
531+
codeowners_type: Option<OwnersSource>,
517532
bep_result: Option<BepValidateResult>,
518533
) -> anyhow::Result<ValidateRunResult> {
519534
let current_dir = std::env::current_dir()
520535
.ok()
521536
.and_then(|p| p.to_str().map(String::from))
522537
.unwrap_or_default();
523-
let file_set_builder =
524-
FileSetBuilder::build_file_sets(&current_dir, &junit_paths, &Option::<&str>::None, None)?;
538+
let file_set_builder = FileSetBuilder::build_file_sets(
539+
&current_dir,
540+
&junit_paths,
541+
&Option::<&str>::None,
542+
None,
543+
None,
544+
)?;
525545
if file_set_builder.no_files_found() {
526546
let msg = "No test output files found to validate";
527547
tracing::warn!(msg);
@@ -569,7 +589,7 @@ async fn validate(
569589
.collect();
570590
let test_issues = gen_test_issues(&report_validations);
571591

572-
let codeowners = CodeOwners::find_file(&current_dir, &codeowners_path);
592+
let codeowners = CodeOwners::find_file(&current_dir, &codeowners_path, codeowners_type);
573593
let codeowners_issues = gen_codeowners_issues(codeowners, &report_validations);
574594

575595
Ok(ValidateRunResult {

cli/tests/common/command_builder.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ pub struct UploadArgs {
1515
no_upload: Option<bool>,
1616
team: Option<String>,
1717
codeowners_path: Option<String>,
18+
codeowners_type: Option<String>,
1819
disable_quarantining: Option<bool>,
1920
allow_empty_test_results: Option<bool>,
2021
variant: Option<String>,
@@ -38,6 +39,7 @@ impl UploadArgs {
3839
no_upload: None,
3940
team: None,
4041
codeowners_path: None,
42+
codeowners_type: None,
4143
disable_quarantining: None,
4244
allow_empty_test_results: None,
4345
variant: None,
@@ -124,6 +126,14 @@ impl UploadArgs {
124126
vec![String::from("--codeowners-path"), codeowners_path]
125127
}),
126128
)
129+
.chain(
130+
self.codeowners_type
131+
.clone()
132+
.into_iter()
133+
.flat_map(|codeowners_type: String| {
134+
vec![String::from("--codeowners-type"), codeowners_type]
135+
}),
136+
)
127137
.chain(
128138
self.disable_quarantining
129139
.into_iter()

codeowners/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ js-sys = { version = "0.3", optional = true }
3131
pyo3-stub-gen = { version = "0.6.1", optional = true }
3232
tokio = { version = "*", default-features = false, features = ["rt", "macros"] }
3333
tracing = "0.1.41"
34+
tracing-subscriber = { version = "0.3", features = ["env-filter"] }
3435
once_cell = "1.21.3"
3536

3637
[target.'cfg(target_os = "linux")'.dependencies]

codeowners/src/codeowners.rs

Lines changed: 56 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use std::{
22
fs::File,
3+
io::{Read, Seek},
34
path::{Path, PathBuf},
45
sync::Arc,
56
};
@@ -83,6 +84,7 @@ impl CodeOwners {
8384
pub fn find_file<T: AsRef<Path>, U: AsRef<Path>>(
8485
repo_root: T,
8586
codeowners_path_cli_option: &Option<U>,
87+
owners_source: Option<OwnersSource>,
8688
) -> Option<Self> {
8789
let cli_option_location = codeowners_path_cli_option
8890
.as_slice()
@@ -97,13 +99,23 @@ impl CodeOwners {
9799
all_locations.find_map(|location| locate_codeowners(&repo_root, location));
98100

99101
codeowners_path.map(|path| {
100-
let owners_result = File::open(&path)
102+
let owners_result: anyhow::Result<Owners> = File::open(&path)
101103
.map_err(anyhow::Error::from)
102-
.and_then(|file| GitLabOwners::from_reader(&file).map(Owners::GitLabOwners))
103-
.or_else(|_| {
104-
File::open(&path)
105-
.map_err(anyhow::Error::from)
106-
.and_then(|file| GitHubOwners::from_reader(&file).map(Owners::GitHubOwners))
104+
.and_then(|mut file| {
105+
let parse_github = |r: &mut File| {
106+
GitHubOwners::from_reader(r.by_ref()).map(Owners::GitHubOwners)
107+
};
108+
let parse_gitlab = |r: &mut File| {
109+
GitLabOwners::from_reader(r.by_ref()).map(Owners::GitLabOwners)
110+
};
111+
match owners_source {
112+
Some(OwnersSource::GitHub) => parse_github(&mut file),
113+
Some(OwnersSource::GitLab) => parse_gitlab(&mut file),
114+
_ => parse_gitlab(&mut file).or_else(|_| {
115+
file.seek(std::io::SeekFrom::Start(0))?;
116+
parse_github(&mut file)
117+
}),
118+
}
107119
});
108120

109121
if let Err(ref err) = owners_result {
@@ -322,6 +334,10 @@ where
322334

323335
#[cfg(test)]
324336
mod tests {
337+
use std::fs;
338+
use std::path::Path;
339+
use std::time::{SystemTime, UNIX_EPOCH};
340+
325341
use super::*;
326342

327343
fn make_codeowners_bytes(i: usize, owners_source: Option<OwnersSource>) -> CodeOwnersFile {
@@ -432,4 +448,38 @@ mod tests {
432448
assert_eq!(owners[0], format!("@user{user_id}"));
433449
}
434450
}
451+
452+
#[test]
453+
fn test_find_file_with_explicit_owners_source() {
454+
let unique = SystemTime::now()
455+
.duration_since(UNIX_EPOCH)
456+
.unwrap()
457+
.as_nanos();
458+
let temp_dir = std::env::temp_dir().join(format!("codeowners-source-test-{unique}"));
459+
fs::create_dir_all(&temp_dir).unwrap();
460+
let codeowners_file = temp_dir.join("CODEOWNERS");
461+
fs::write(
462+
&codeowners_file,
463+
"[DefaultOwners] @gitlab-owner\nREADME.md\n",
464+
)
465+
.unwrap();
466+
467+
let parsed_gitlab =
468+
CodeOwners::find_file(&temp_dir, &Some(Path::new(".")), Some(OwnersSource::GitLab))
469+
.unwrap();
470+
let gitlab_owners = flatten_code_owners(&parsed_gitlab, &"README.md".to_string());
471+
assert_eq!(gitlab_owners, vec!["@gitlab-owner".to_string()]);
472+
473+
let parsed_github =
474+
CodeOwners::find_file(&temp_dir, &Some(Path::new(".")), Some(OwnersSource::GitHub))
475+
.unwrap();
476+
let github_owners = flatten_code_owners(&parsed_github, &"README.md".to_string());
477+
assert!(github_owners.is_empty());
478+
479+
let parsed_default = CodeOwners::find_file(&temp_dir, &Some(Path::new(".")), None).unwrap();
480+
let default_owners = flatten_code_owners(&parsed_default, &"README.md".to_string());
481+
assert_eq!(default_owners, vec!["@gitlab-owner".to_string()]);
482+
483+
fs::remove_dir_all(temp_dir).ok();
484+
}
435485
}

0 commit comments

Comments
 (0)