scs_server: add pushvar support

Summary: Allow source control service clients to set pushvars.

Reviewed By: krallin

Differential Revision: D24136870

fbshipit-source-id: 34f9176ec66ca319b363c91015dae9b59a55a244

eden/mononoke/mononoke_api/src/repo_write/create_bookmark.rs

@@ -5,9 +5,11 @@
  * GNU General Public License version 2.
  */
 
+use std::collections::HashMap;
 use std::sync::Arc;
 
 use bookmarks::{BookmarkName, BookmarkUpdateReason};
+use bytes::Bytes;
 use metaconfig_types::BookmarkAttrs;
 use mononoke_types::ChangesetId;
 use reachabilityindex::LeastCommonAncestorsHint;
@@ -21,6 +23,7 @@ impl RepoWriteContext {
         &self,
         bookmark: impl AsRef<str>,
         target: ChangesetId,
+        pushvars: Option<&HashMap<String, Bytes>>,
     ) -> Result<(), MononokeError> {
         let bookmark = bookmark.as_ref();
         self.check_method_permitted("create_bookmark")?;
@@ -35,7 +38,8 @@ impl RepoWriteContext {
             &bookmark,
             target,
             BookmarkUpdateReason::ApiRequest,
-        );
+        )
+        .with_pushvars(pushvars);
 
         if let PermissionsModel::ServiceIdentity(service_identity) = &self.permissions_model {
             op = op.for_service(service_identity, &self.config().source_control_service);

eden/mononoke/mononoke_api/src/repo_write/delete_bookmark.rs

@@ -5,8 +5,11 @@
  * GNU General Public License version 2.
  */
 
+use std::collections::HashMap;
+
 use anyhow::Context;
 use bookmarks::{BookmarkName, BookmarkUpdateReason};
+use bytes::Bytes;
 use metaconfig_types::BookmarkAttrs;
 use mononoke_types::ChangesetId;
 
@@ -19,6 +22,7 @@ impl RepoWriteContext {
         &self,
         bookmark: impl AsRef<str>,
         old_target: Option<ChangesetId>,
+        pushvars: Option<&HashMap<String, Bytes>>,
     ) -> Result<(), MononokeError> {
         let bookmark = bookmark.as_ref();
         self.check_method_permitted("delete_bookmark")?;
@@ -46,7 +50,8 @@ impl RepoWriteContext {
             &bookmark,
             old_target,
             BookmarkUpdateReason::ApiRequest,
-        );
+        )
+        .with_pushvars(pushvars);
 
         if let PermissionsModel::ServiceIdentity(service_identity) = &self.permissions_model {
             op = op.for_service(service_identity, &self.config().source_control_service);

eden/mononoke/mononoke_api/src/repo_write/land_stack.rs

@@ -5,11 +5,12 @@
  * GNU General Public License version 2.
  */
 
-use std::collections::HashSet;
+use std::collections::{HashMap, HashSet};
 use std::sync::Arc;
 
 use blobstore::Loadable;
 use bookmarks::BookmarkName;
+use bytes::Bytes;
 use futures::compat::Stream01CompatExt;
 use futures::future::{self, TryFutureExt};
 use futures::stream::TryStreamExt;
@@ -30,6 +31,7 @@ impl RepoWriteContext {
         bookmark: impl AsRef<str>,
         head: ChangesetId,
         base: ChangesetId,
+        pushvars: Option<&HashMap<String, Bytes>>,
     ) -> Result<PushrebaseOutcome, MononokeError> {
         let bookmark = bookmark.as_ref();
         self.check_method_permitted("land_stack")?;
@@ -80,7 +82,8 @@ impl RepoWriteContext {
         .await?;
 
         // Pushrebase these commits onto the bookmark.
-        let mut op = bookmarks_movement::PushrebaseOntoBookmarkOp::new(&bookmark, changesets);
+        let mut op = bookmarks_movement::PushrebaseOntoBookmarkOp::new(&bookmark, changesets)
+            .with_pushvars(pushvars);
 
         if let PermissionsModel::ServiceIdentity(service_identity) = &self.permissions_model {
             op = op.for_service(service_identity, &self.config().source_control_service);

eden/mononoke/mononoke_api/src/repo_write/move_bookmark.rs

@@ -5,11 +5,13 @@
  * GNU General Public License version 2.
  */
 
+use std::collections::HashMap;
 use std::sync::Arc;
 
 use anyhow::Context;
 use bookmarks::{BookmarkName, BookmarkUpdateReason};
 use bookmarks_movement::{BookmarkUpdatePolicy, BookmarkUpdateTargets};
+use bytes::Bytes;
 use metaconfig_types::BookmarkAttrs;
 use mononoke_types::ChangesetId;
 use reachabilityindex::LeastCommonAncestorsHint;
@@ -25,6 +27,7 @@ impl RepoWriteContext {
         target: ChangesetId,
         old_target: Option<ChangesetId>,
         allow_non_fast_forward: bool,
+        pushvars: Option<&HashMap<String, Bytes>>,
     ) -> Result<(), MononokeError> {
         let bookmark = bookmark.as_ref();
         self.check_method_permitted("move_bookmark")?;
@@ -62,7 +65,8 @@ impl RepoWriteContext {
                 BookmarkUpdatePolicy::FastForwardOnly
             },
             BookmarkUpdateReason::ApiRequest,
-        );
+        )
+        .with_pushvars(pushvars);
 
         if let PermissionsModel::ServiceIdentity(service_identity) = &self.permissions_model {
             op = op.for_service(service_identity, &self.config().source_control_service);

eden/mononoke/mononoke_api/src/test/test_repo_land_stack.rs

@@ -54,7 +54,7 @@ async fn land_stack(fb: FacebookInit) -> Result<()> {
 
     // Land G - it should be rewritten even though its parent is C.
     let outcome = repo
-        .land_stack("trunk", changesets["G"], changesets["C"])
+        .land_stack("trunk", changesets["G"], changesets["C"], None)
         .await?;
     let trunk_g = repo
         .resolve_bookmark("trunk", BookmarkFreshness::MostRecent)
@@ -67,7 +67,7 @@ async fn land_stack(fb: FacebookInit) -> Result<()> {
 
     // Land D and E, both commits should get mapped
     let outcome = repo
-        .land_stack("trunk", changesets["E"], changesets["A"])
+        .land_stack("trunk", changesets["E"], changesets["A"], None)
         .await?;
     let trunk_e = repo
         .resolve_bookmark("trunk", BookmarkFreshness::MostRecent)
@@ -87,7 +87,7 @@ async fn land_stack(fb: FacebookInit) -> Result<()> {
 
     // Land F, its parent should be the landed version of E
     let outcome = repo
-        .land_stack("trunk", changesets["F"], changesets["B"])
+        .land_stack("trunk", changesets["F"], changesets["B"], None)
         .await?;
     let trunk_f = repo
         .resolve_bookmark("trunk", BookmarkFreshness::MostRecent)

eden/mononoke/mononoke_api/src/test/test_repo_modify_bookmarks.rs

@@ -51,7 +51,8 @@ async fn create_bookmark(fb: FacebookInit) -> Result<()> {
     let repo = repo.write().await?;
 
     // Can create public bookmarks on existing changesets (ancestors of trunk).
-    repo.create_bookmark("bookmark1", changesets["A"]).await?;
+    repo.create_bookmark("bookmark1", changesets["A"], None)
+        .await?;
     let bookmark1 = repo
         .resolve_bookmark("bookmark1", BookmarkFreshness::MostRecent)
         .await?
@@ -59,7 +60,8 @@ async fn create_bookmark(fb: FacebookInit) -> Result<()> {
     assert_eq!(bookmark1.id(), changesets["A"]);
 
     // Can create public bookmarks on other changesets (not ancestors of trunk).
-    repo.create_bookmark("bookmark2", changesets["F"]).await?;
+    repo.create_bookmark("bookmark2", changesets["F"], None)
+        .await?;
     let bookmark2 = repo
         .resolve_bookmark("bookmark2", BookmarkFreshness::MostRecent)
         .await?
@@ -67,7 +69,7 @@ async fn create_bookmark(fb: FacebookInit) -> Result<()> {
     assert_eq!(bookmark2.id(), changesets["F"]);
 
     // Can create scratch bookmarks.
-    repo.create_bookmark("scratch/bookmark3", changesets["G"])
+    repo.create_bookmark("scratch/bookmark3", changesets["G"], None)
         .await?;
     let bookmark3 = repo
         .resolve_bookmark("scratch/bookmark3", BookmarkFreshness::MostRecent)
@@ -89,7 +91,7 @@ async fn move_bookmark(fb: FacebookInit) -> Result<()> {
     let (repo, changesets) = init_repo(&ctx).await?;
     let repo = repo.write().await?;
 
-    repo.move_bookmark("trunk", changesets["E"], None, false)
+    repo.move_bookmark("trunk", changesets["E"], None, false, None)
         .await?;
     let trunk = repo
         .resolve_bookmark("trunk", BookmarkFreshness::MostRecent)
@@ -100,11 +102,11 @@ async fn move_bookmark(fb: FacebookInit) -> Result<()> {
     // Attempt to move to a non-descendant commit without allowing
     // non-fast-forward moves should fail.
     assert!(
-        repo.move_bookmark("trunk", changesets["G"], None, false)
+        repo.move_bookmark("trunk", changesets["G"], None, false, None)
             .await
             .is_err()
     );
-    repo.move_bookmark("trunk", changesets["G"], None, true)
+    repo.move_bookmark("trunk", changesets["G"], None, true, None)
         .await?;
     let trunk = repo
         .resolve_bookmark("trunk", BookmarkFreshness::MostRecent)
@@ -144,13 +146,15 @@ async fn delete_bookmark(fb: FacebookInit) -> Result<()> {
     let (repo, changesets) = init_repo(&ctx).await?;
     let repo = repo.write().await?;
 
-    repo.create_bookmark("bookmark1", changesets["A"]).await?;
+    repo.create_bookmark("bookmark1", changesets["A"], None)
-    repo.create_bookmark("bookmark2", changesets["F"]).await?;
+        .await?;
-    repo.create_bookmark("scratch/bookmark3", changesets["G"])
+    repo.create_bookmark("bookmark2", changesets["F"], None)
+        .await?;
+    repo.create_bookmark("scratch/bookmark3", changesets["G"], None)
         .await?;
 
     // Can delete public bookmarks.
-    repo.delete_bookmark("bookmark1", None).await?;
+    repo.delete_bookmark("bookmark1", None, None).await?;
     assert!(
         repo.resolve_bookmark("bookmark1", BookmarkFreshness::MostRecent)
             .await?
@@ -159,7 +163,7 @@ async fn delete_bookmark(fb: FacebookInit) -> Result<()> {
 
     // Deleting a bookmark with the wrong old-target fails.
     assert!(
-        repo.delete_bookmark("bookmark2", Some(changesets["E"]))
+        repo.delete_bookmark("bookmark2", Some(changesets["E"]), None)
             .await
             .is_err()
     );
@@ -170,7 +174,7 @@ async fn delete_bookmark(fb: FacebookInit) -> Result<()> {
     assert_eq!(bookmark2.id(), changesets["F"]);
 
     // But with the right old-target succeeds.
-    repo.delete_bookmark("bookmark2", Some(changesets["F"]))
+    repo.delete_bookmark("bookmark2", Some(changesets["F"]), None)
         .await?;
     assert!(
         repo.resolve_bookmark("bookmark1", BookmarkFreshness::MostRecent)
@@ -180,7 +184,7 @@ async fn delete_bookmark(fb: FacebookInit) -> Result<()> {
 
     // Can't delete scratch bookmarks.
     assert!(
-        repo.delete_bookmark("scratch/bookmark3", None)
+        repo.delete_bookmark("scratch/bookmark3", None, None)
             .await
             .is_err()
     );

eden/mononoke/scs_server/src/from_request.rs

@@ -5,11 +5,13 @@
  * GNU General Public License version 2.
  */
 
+use std::collections::{BTreeMap, HashMap};
 use std::convert::{TryFrom, TryInto};
 use std::fmt::{Debug, Display};
 use std::ops::RangeBounds;
 use std::str::FromStr;
 
+use bytes::Bytes;
 use chrono::{DateTime, FixedOffset, TimeZone};
 use faster_hex::hex_string;
 use mononoke_api::specifiers::{GitSha1, Globalrev};
@@ -283,3 +285,16 @@ pub(crate) fn validate_timestamp(
         Some(ts) => Ok(Some(ts)),
     }
 }
+
+/// Convert a pushvars map from thrift's representation to the one used
+/// internally in mononoke.
+pub(crate) fn convert_pushvars(
+    pushvars: Option<BTreeMap<String, Vec<u8>>>,
+) -> Option<HashMap<String, Bytes>> {
+    pushvars.map(|pushvars| {
+        pushvars
+            .into_iter()
+            .map(|(name, value)| (name, Bytes::from(value)))
+            .collect()
+    })
+}

eden/mononoke/scs_server/src/methods/repo.rs

@@ -29,7 +29,7 @@ use source_control as thrift;
 
 use crate::commit_id::{map_commit_identities, map_commit_identity, CommitIdExt};
 use crate::errors;
-use crate::from_request::{check_range_and_convert, FromRequest};
+use crate::from_request::{check_range_and_convert, convert_pushvars, FromRequest};
 use crate::into_response::{AsyncIntoResponseWith, IntoResponse};
 use crate::source_control_impl::SourceControlServiceImpl;
 
@@ -450,8 +450,9 @@ impl SourceControlServiceImpl {
             .changeset(ChangesetSpecifier::from_request(target)?)
             .await?
             .ok_or_else(|| errors::commit_not_found(target.to_string()))?;
+        let pushvars = convert_pushvars(params.pushvars);
 
-        repo.create_bookmark(&params.bookmark, changeset.id())
+        repo.create_bookmark(&params.bookmark, changeset.id(), pushvars.as_ref())
             .await?;
         Ok(thrift::RepoCreateBookmarkResponse {})
     }
@@ -481,12 +482,14 @@ impl SourceControlServiceImpl {
             ),
             None => None,
         };
+        let pushvars = convert_pushvars(params.pushvars);
 
         repo.move_bookmark(
             &params.bookmark,
             changeset.id(),
             old_changeset_id,
             params.allow_non_fast_forward_move,
+            pushvars.as_ref(),
         )
         .await?;
         Ok(thrift::RepoMoveBookmarkResponse {})
@@ -512,8 +515,9 @@ impl SourceControlServiceImpl {
             ),
             None => None,
         };
+        let pushvars = convert_pushvars(params.pushvars);
 
-        repo.delete_bookmark(&params.bookmark, old_changeset_id)
+        repo.delete_bookmark(&params.bookmark, old_changeset_id, pushvars.as_ref())
             .await?;
         Ok(thrift::RepoDeleteBookmarkResponse {})
     }
@@ -538,9 +542,10 @@ impl SourceControlServiceImpl {
             .changeset(ChangesetSpecifier::from_request(base)?)
             .await?
             .ok_or_else(|| errors::commit_not_found(base.to_string()))?;
+        let pushvars = convert_pushvars(params.pushvars);
 
         let pushrebase_outcome = repo
-            .land_stack(&params.bookmark, head.id(), base.id())
+            .land_stack(&params.bookmark, head.id(), base.id(), pushvars.as_ref())
             .await?
             .into_response_with(&(
                 repo.clone(),

eden/mononoke/tests/integration/test-scs-land-stack.t

@@ -144,8 +144,8 @@ attempt to land stack to G, hooks will fail
   error: SourceControlService::repo_land_stack failed with RequestError { kind: RequestErrorKind::INVALID_REQUEST, reason: "hooks failed:\n  limit_filesize for 6b5b61e224d26706b1eb361a3900f168042bd4f7f936c64cd91963e365aec4e9: File size limit is 10 bytes. You tried to push file glarge that is over the limit (14 bytes).  See https://fburl.com/landing_big_diffs for instructions." }
   [1]
 
-we don't have pushvars on the thrift interface yet, but services can bypass hooks
+bypass the hook to land the stack
-  $ scsc land-stack -R repo --name trunk -i $G -i $A --service-id trunk-permitted-service
+  $ scsc land-stack -R repo --name trunk -i $G -i $A --pushvar ALLOW_LARGE_FILES=true
   trunk updated to: f3be859b0ddb06d8c11c2bd43edd71528ff055a7
   1e82c5967442db2a0774744065e7f0f4ce810e9839897f81cc012959a783884b => a194cadd16930608adaa649035ad4c16930cbd0f
   6b5b61e224d26706b1eb361a3900f168042bd4f7f936c64cd91963e365aec4e9 => f3be859b0ddb06d8c11c2bd43edd71528ff055a7

eden/mononoke/tests/integration/test-scs-modify-bookmarks.t

@@ -150,8 +150,8 @@ try to move trunk over a commit that fails the hooks
   error: SourceControlService::repo_move_bookmark failed with RequestError { kind: RequestErrorKind::INVALID_REQUEST, reason: "hooks failed:\n  limit_filesize for 88dbd25ba00277e3dfdfc642d67f2c22c75ea4f8d94f011b7f526f07b6ecc345: File size limit is 10 bytes. You tried to push file dlarge that is over the limit (14 bytes).  See https://fburl.com/landing_big_diffs for instructions." }
   [1]
 
-there's no way to specify pushvars at the moment, however service writes can bypass hooks
+use a pushvar to bypass the hook
-  $ scsc move-bookmark -R repo --name trunk -i $E --service-id trunk-permitted-service
+  $ scsc move-bookmark -R repo --name trunk -i $E --pushvar ALLOW_LARGE_FILES=true
   $ wait_for_bookmark_update repo trunk $E
   $ scsc info -R repo -B trunk
   Commit: 6f95f2e47a180912e108a8dbfe9d45fc417834c3