Skip to content

Commit

Permalink
Split the permissions errors apart from ownership
Browse files Browse the repository at this point in the history 
This adds a new error category, "permissions", which helps to pinpoint
the actual error cause. Turns out, one test actually had a bug, hidden
by the combining of the two error types.
  • Loading branch information
@iustin
iustin committed Mar 9, 2024
1 parent 8d26626 commit 6292663
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 21 deletions.
52 changes: 32 additions & 20 deletions src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,13 +65,15 @@ pub fn relative_age(reference: SystemTime, m: &Metadata) -> Duration {
pub enum ErrorType {
Scan,
Ownership,
Permissions,
}

impl EncodeLabelValue for ErrorType {
fn encode(&self, encoder: &mut LabelValueEncoder) -> Result<(), std::fmt::Error> {
let s = match self {
ErrorType::Scan => "scan",
ErrorType::Ownership => "ownership",
ErrorType::Permissions => "permissions",
};
EncodeLabelValue::encode(&s, encoder)
}
Expand Down Expand Up @@ -155,7 +157,11 @@ pub struct Backlog {
impl Backlog {
pub fn new(buckets: impl Iterator<Item = f64>) -> Self {
Self {
total_errors: HashMap::from([(ErrorType::Scan, 0), (ErrorType::Ownership, 0)]),
total_errors: HashMap::from([
(ErrorType::Scan, 0),
(ErrorType::Ownership, 0),
(ErrorType::Permissions, 0),
]),
total_files: 0,
folders: HashMap::new(),
ages_histogram: Histogram::new(buckets),
Expand Down Expand Up @@ -196,7 +202,7 @@ impl Backlog {
self.record_error(ErrorType::Ownership);
}
if !check_mode(config, path, &metadata) {
self.record_error(ErrorType::Ownership);
self.record_error(ErrorType::Permissions);
}
// We don't track directories by themselves,
// only via file contents.
Expand All @@ -221,7 +227,7 @@ impl Backlog {
self.record_error(ErrorType::Ownership);
}
if !check_mode(config, path, &metadata) {
self.record_error(ErrorType::Ownership);
self.record_error(ErrorType::Permissions);
}

// Find owner top-level dir.
Expand Down Expand Up @@ -311,11 +317,14 @@ mod tests {
expect_files: i64,
scan_errors: i64,
ownership_errors: i64,
permissions_errors: i64,
) {
assert_that!(backlog.folders).has_length(expect_folders);
assert_that!(backlog.total_files).is_equal_to(expect_files);
assert_that!(backlog.total_errors).contains_entry(ErrorType::Scan, scan_errors);
assert_that!(backlog.total_errors).contains_entry(ErrorType::Ownership, ownership_errors)
assert_that!(backlog.total_errors).contains_entry(ErrorType::Ownership, ownership_errors);
assert_that!(backlog.total_errors)
.contains_entry(ErrorType::Permissions, permissions_errors);
}

fn check_has_dir_with(backlog: &Backlog, folder: &str, file_count: i64) {
Expand All @@ -336,7 +345,7 @@ mod tests {
let mut backlog = Backlog::new([].into_iter());
let now = SystemTime::now();
backlog.scan(&config, now);
check_backlog(&backlog, 0, 0, 0, 0);
check_backlog(&backlog, 0, 0, 0, 0, 0);
}
#[test]
fn empty_dir_is_empty() {
Expand All @@ -345,7 +354,7 @@ mod tests {
let mut backlog = Backlog::new([].into_iter());
let now = SystemTime::now();
backlog.scan(&config, now);
check_backlog(&backlog, 0, 0, 0, 0);
check_backlog(&backlog, 0, 0, 0, 0, 0);
}
#[test]
fn no_extension_is_ignored() {
Expand All @@ -355,7 +364,7 @@ mod tests {
let mut backlog = Backlog::new([].into_iter());
let now = SystemTime::now();
backlog.scan(&config, now);
check_backlog(&backlog, 0, 0, 0, 0);
check_backlog(&backlog, 0, 0, 0, 0, 0);
}
#[test]
fn ignored_extension_is_ignored() {
Expand All @@ -368,7 +377,7 @@ mod tests {
let mut backlog = Backlog::new([].into_iter());
let now = SystemTime::now();
backlog.scan(&config, now);
check_backlog(&backlog, 1, 1, 0, 0);
check_backlog(&backlog, 1, 1, 0, 0, 0);
check_has_dir_with(&backlog, SUBDIR, 1);
}
#[test]
Expand All @@ -379,7 +388,7 @@ mod tests {
let mut backlog = Backlog::new([].into_iter());
let now = SystemTime::now();
backlog.scan(&config, now);
check_backlog(&backlog, 1, 1, 0, 0);
check_backlog(&backlog, 1, 1, 0, 0, 0);
check_has_dir_with(&backlog, SUBDIR, 1);
}
#[test]
Expand All @@ -391,7 +400,7 @@ mod tests {
let mut backlog = Backlog::new([].into_iter());
let now = SystemTime::now();
backlog.scan(&config, now);
check_backlog(&backlog, 1, 2, 0, 0);
check_backlog(&backlog, 1, 2, 0, 0, 0);
check_has_dir_with(&backlog, SUBDIR, 2);
}
#[test]
Expand All @@ -402,7 +411,7 @@ mod tests {
let mut backlog = Backlog::new([].into_iter());
let now = SystemTime::now();
backlog.scan(&config, now);
check_backlog(&backlog, 1, 1, 0, 0);
check_backlog(&backlog, 1, 1, 0, 0, 0);
check_has_dir_with(&backlog, ROOT_FILE_DIR, 1);
}

Expand All @@ -415,7 +424,7 @@ mod tests {
let config = build_config(&missing_dir, None, None, None, None);
let now = SystemTime::now();
backlog.scan(&config, now);
check_backlog(&backlog, 0, 0, 1, 0);
check_backlog(&backlog, 0, 0, 1, 0, 0);
}

enum FailMode {
Expand Down Expand Up @@ -454,7 +463,7 @@ mod tests {
(FailMode::Bad, _) | (_, FailMode::Bad) => 2,
_ => 0,
};
check_backlog(&backlog, 1, 1, 0, expected_errors);
check_backlog(&backlog, 1, 1, 0, expected_errors, 0);
check_has_dir_with(&backlog, ROOT_FILE_DIR, 1);
}

Expand Down Expand Up @@ -509,21 +518,24 @@ mod tests {
_ => 0,
};
let expected_errors = file_errors + dir_errors;
check_backlog(&backlog, 1, 1, 0, expected_errors);
check_backlog(&backlog, 1, 1, 0, 0, expected_errors);
check_has_dir_with(&backlog, subdir.file_name().unwrap().to_str().unwrap(), 1);
}

#[test]
fn ignored_files_are_ignored() {
let _ = env_logger::builder().is_test(true).try_init();

let (temp_dir, subdir) = get_subdir();
// File with good extension.
let _nef = add_file(&subdir, "file.nef");
let nef = add_file(&subdir, "file.nef");
// File with ignored extension.
let readme = add_file(&subdir, "readme.md");
let _readme = add_file(&subdir, "readme.md");
// File with no extension.
let _checksums = add_file(&subdir, "SHA1SUMS");
let m = std::fs::metadata(readme).expect("Can't stat just created file!");
let wrong_mode = if m.mode() == 0o644 { 0o600 } else { 0o644 };
std::fs::set_permissions(&nef, std::fs::Permissions::from_mode(0o600)).unwrap();
let m = std::fs::metadata(&nef).expect("Can't stat just created file!");
let wrong_mode = 0o644;
let wrong_uid = m.uid() + 1;
let wrong_gid = m.gid() + 1;

Expand All @@ -544,7 +556,7 @@ mod tests {
// same ownership, which is generally correct), and the real file as
// well, but the two extra files are ignored.
let expected_errors = 3;
check_backlog(&backlog, 1, 1, 0, expected_errors);
check_backlog(&backlog, 1, 1, 0, expected_errors, 1);
check_has_dir_with(&backlog, subdir.file_name().unwrap().to_str().unwrap(), 1);
}

Expand Down Expand Up @@ -579,6 +591,6 @@ mod tests {
let now = SystemTime::now();
backlog.scan(&config, now);
std::fs::set_permissions(&temp_dir, std::fs::Permissions::from_mode(0o755)).unwrap();
check_backlog(&backlog, 0, 0, 3, 0);
check_backlog(&backlog, 0, 0, 3, 0, 0);
}
}
3 changes: 2 additions & 1 deletion tests/log-tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ fn test_ownership_logs() {
assert_that!(backlog.folders).has_length(1);
assert_that!(backlog.total_files).is_equal_to(1);
assert_that!(backlog.total_errors).contains_entry(ErrorType::Scan, 0);
assert_that!(backlog.total_errors).contains_entry(ErrorType::Ownership, 3);
assert_that!(backlog.total_errors).contains_entry(ErrorType::Ownership, 2);
assert_that!(backlog.total_errors).contains_entry(ErrorType::Permissions, 1);
testing_logger::validate(|captured_logs| {
let v: Vec<String> = captured_logs.iter().map(|e| e.body.clone()).collect();
assert_that!(v).matching_contains(|val| val.contains("has wrong owner:group"));
Expand Down

0 comments on commit 6292663

Please sign in to comment.