facebook/sapling
1
1
Fork 0
mirror of https://github.com/facebook/sapling.git synced 2024-10-10 16:57:49 +03:00
Code Issues Projects Releases Wiki Activity

Integrate Aclchecker with hooks

Browse Source 
Summary:
Integrate AclChecker into hooks.

Note, TODO: Create a new hipster group "reviewers"

Reviewed By: StanislavGlebik

Differential Revision: D10507735

fbshipit-source-id: 62c02bdd2e79b8b77ea118ed97bec4a8f48f01db
This commit is contained in:
Tim Fox 2018-11-13 05:40:04 -08:00 committed by Facebook Github Bot
parent aae4562523
commit dafbef1dc3
2 changed files with 76 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
hooks/src/lib.rs
View File

@ -14,6 +14,7 @@
#![deny(warnings)]
#![feature(try_from)]
extern crate aclchecker;
#[cfg(test)]
#[macro_use]
extern crate assert_matches;
@ -54,7 +55,9 @@ pub mod lua_hook;
pub mod rust_hook;
pub mod hook_loader;
pub mod errors;
mod facebook;
use aclchecker::{AclChecker, Identity};
use asyncmemo::{Asyncmemo, Filler, Weight};
use blobrepo::{BlobRepo, HgBlobChangeset};
use bookmarks::Bookmark;
@ -91,6 +94,7 @@ pub struct HookManager {
changeset_store: Box<ChangesetStore>,
content_store: Arc<FileContentStore>,
logger: Logger,
reviewers_acl_checker: Arc<Option<AclChecker>>,
}
impl HookManager {
@ -110,7 +114,15 @@ impl HookManager {
repo_name: repo_name.clone(),
};
let cache = Asyncmemo::with_limits("hooks", filler, entrylimit, weightlimit);
let reviewers_acl_checker =
AclChecker::new(&Identity::from_groupname(facebook::REVIEWERS_ACL_GROUP_NAME));
// This can block, but not too big a deal as we create hook manager in server startup
let updated = reviewers_acl_checker.do_wait_updated(10000);
let reviewers_acl_checker = if updated {
Some(reviewers_acl_checker)
} else {
None
};
HookManager {
cache,
changeset_hooks,
@ -120,6 +132,7 @@ impl HookManager {
changeset_store,
content_store,
logger,
reviewers_acl_checker: Arc::new(reviewers_acl_checker),
}
}
@ -414,6 +427,7 @@ impl HookManager {
let hg_changeset = self.changeset_store
.get_changeset_by_changesetid(&changeset_id);
let changed_files = self.changeset_store.get_changed_files(&changeset_id);
let reviewers_acl_checker = self.reviewers_acl_checker.clone();
Box::new((hg_changeset, changed_files).into_future().and_then(
move |(changeset, changed_files)| {
let author = str::from_utf8(changeset.user())?.into();
@ -432,6 +446,7 @@ impl HookManager {
parents,
changeset_id,
content_store,
reviewers_acl_checker,
))
},
))
@ -499,6 +514,7 @@ pub struct HookChangeset {
pub parents: HookChangesetParents,
content_store: Arc<FileContentStore>,
changeset_id: HgChangesetId,
reviewers_acl_checker: Arc<Option<AclChecker>>,
}
impl fmt::Debug for HookChangeset {
@ -637,6 +653,7 @@ impl HookChangeset {
parents: HookChangesetParents,
changeset_id: HgChangesetId,
content_store: Arc<FileContentStore>,
reviewers_acl_checker: Arc<Option<AclChecker>>,
) -> HookChangeset {
HookChangeset {
author,
@ -645,6 +662,7 @@ impl HookChangeset {
parents,
content_store,
changeset_id,
reviewers_acl_checker,
}
}
@ -1419,6 +1437,7 @@ mod test {
.collect();
let parents =
HookChangesetParents::One("2f866e7e549760934e31bf0420a873f65100ad63".into());
let reviewers_acl_checker = Arc::new(None);
let data = HookChangeset::new(
"Stanislau Hlebik <stash@fb.com>".into(),
hook_files,
@ -1426,6 +1445,7 @@ mod test {
parents,
cs_id,
content_store,
reviewers_acl_checker,
);
let expected_context = HookContext {
hook_name: "hook1".into(),

62
hooks/src/lua_hook.rs
View File

@ -11,6 +11,7 @@
use super::{ChangedFileType, Hook, HookChangeset, HookChangesetParents, HookContext,
HookExecution, HookFile, HookRejectionInfo};
use super::errors::*;
use aclchecker::Identity;
use failure::Error;
use futures::{failed, Future};
use futures::future::ok;
@ -199,14 +200,15 @@ impl Hook<HookChangeset> for LuaHook {
};
let parse_commit_msg = function0(parse_commit_msg);
let reviewers_acl_checker = context.data.reviewers_acl_checker.clone();
let is_valid_reviewer = {
move |user: String| -> Result<AnyFuture, Error> {
// TODO(stash): actual implementation that checks the permissions
if user == "invalidreviewer" {
Ok(AnyFuture::new(ok(AnyLuaValue::LuaBoolean(false))))
} else {
Ok(AnyFuture::new(ok(AnyLuaValue::LuaBoolean(true))))
}
let user = Identity::with_user(&user);
let valid = match *reviewers_acl_checker {
Some(ref reviewers_acl_checker) => reviewers_acl_checker.is_member(&[&user]),
None => false,
};
Ok(AnyFuture::new(ok(AnyLuaValue::LuaBoolean(valid))))
}
};
let is_valid_reviewer = function1(is_valid_reviewer);
@ -521,6 +523,7 @@ mod test {
use super::*;
use super::super::{ChangedFileType, HookChangeset, HookChangesetParents,
InMemoryFileContentStore};
use aclchecker::AclChecker;
use async_unit;
use bytes::Bytes;
use futures::Future;
@ -721,6 +724,7 @@ Signature: 111111111:1111111111:bbbbbbbbbbbbbbbb",
let cs_id =
HgChangesetId::from_str("473b2e715e0df6b2316010908879a3c78e275dd9").unwrap();
let content_store = InMemoryFileContentStore::new();
let reviewers_acl_checker = acl_checker();
let hcs = HookChangeset::new(
"some-author".into(),
vec![],
@ -728,6 +732,7 @@ Signature: 111111111:1111111111:bbbbbbbbbbbbbbbb",
HookChangesetParents::One("p1-hash".into()),
cs_id,
Arc::new(content_store),
reviewers_acl_checker,
);
let code = String::from(
"hook = function (ctx)\n\
@ -757,6 +762,7 @@ Signature: 111111111:1111111111:bbbbbbbbbbbbbbbb",
let cs_id =
HgChangesetId::from_str("473b2e715e0df6b2316010908879a3c78e275dd9").unwrap();
let content_store = InMemoryFileContentStore::new();
let reviewers_acl_checker = acl_checker();
let hcs = HookChangeset::new(
"some-author".into(),
vec![],
@ -764,6 +770,7 @@ Signature: 111111111:1111111111:bbbbbbbbbbbbbbbb",
HookChangesetParents::One("p1-hash".into()),
cs_id,
Arc::new(content_store),
reviewers_acl_checker,
);
let code = String::from(
"hook = function (ctx)\n\
@ -792,6 +799,7 @@ Signature: 111111111:1111111111:bbbbbbbbbbbbbbbb",
let cs_id =
HgChangesetId::from_str("473b2e715e0df6b2316010908879a3c78e275dd9").unwrap();
let content_store = InMemoryFileContentStore::new();
let reviewers_acl_checker = acl_checker();
let hcs = HookChangeset::new(
"Stanislau Hlebik <stash@fb.com>".into(),
vec![],
@ -799,6 +807,7 @@ Signature: 111111111:1111111111:bbbbbbbbbbbbbbbb",
HookChangesetParents::One("p1-hash".into()),
cs_id,
Arc::new(content_store),
reviewers_acl_checker,
);
let code = String::from(
"hook = function (ctx)\n\
@ -809,6 +818,38 @@ Signature: 111111111:1111111111:bbbbbbbbbbbbbbbb",
});
}
#[test]
fn test_cs_hook_valid_reviewer() {
async_unit::tokio_unit_test(|| {
let changeset = default_changeset();
let code = String::from(
"hook = function (ctx)\n\
return ctx.is_valid_reviewer('tfox')\n\
end",
);
assert_matches!(
run_changeset_hook(code, changeset),
Ok(HookExecution::Accepted)
);
});
}
#[test]
fn test_cs_hook_not_valid_reviewer() {
async_unit::tokio_unit_test(|| {
let changeset = default_changeset();
let code = String::from(
"hook = function (ctx)\n\
return not ctx.is_valid_reviewer('uyqdyqduygqwduygqwuydgqdfgbducbe2ubjweuhqwudh37')\n\
end",
);
assert_matches!(
run_changeset_hook(code, changeset),
Ok(HookExecution::Accepted)
);
});
}
#[test]
fn test_cs_hook_rejected_short_and_long_desc() {
async_unit::tokio_unit_test(|| {
@ -1552,7 +1593,7 @@ Signature: 111111111:1111111111:bbbbbbbbbbbbbbbb",
hook_files.extend(create_hook_files(added, ChangedFileType::Added));
hook_files.extend(create_hook_files(deleted, ChangedFileType::Deleted));
hook_files.extend(create_hook_files(modified, ChangedFileType::Modified));
let reviewers_acl_checker = acl_checker();
HookChangeset::new(
"some-author".into(),
hook_files,
@ -1560,6 +1601,7 @@ Signature: 111111111:1111111111:bbbbbbbbbbbbbbbb",
HookChangesetParents::One("p1-hash".into()),
cs_id,
content_store2,
reviewers_acl_checker,
)
}
@ -1604,4 +1646,10 @@ Signature: 111111111:1111111111:bbbbbbbbbbbbbbbb",
)
}
fn acl_checker() -> Arc<Option<AclChecker>> {
let checker = AclChecker::new(&Identity::from_groupname("engineers"));
assert!(checker.do_wait_updated(10000));
Arc::new(Some(checker))
}
}