1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-10 17:02:41 +01:00
Commit graph

12 commits

Author SHA1 Message Date
epriestley
8e2f054fe4 Remove TERM=dumb, which is causing difficult-to-reproduce hangs
Summary:
Ref T7119. Three users have now reported that this causes their systems to hang, so it's looking like the cure is worse than the disease.

Revert this until we can eventually reproduce and understand the hang.

Test Plan: Users running into issues have reported that this resolves things.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7119

Differential Revision: https://secure.phabricator.com/D11644
2015-02-03 09:54:02 -08:00
epriestley
9b359affe7 Prepare SSH connections for proxying
Summary:
Ref T7034.

In a cluster environment, when a user connects with a VCS request over SSH (like `git pull`), the receiving server may need to proxy it to a server which can actually satisfy the request.

In order to proxy the request, we need to know which repository the user is interested in accessing.

Split the SSH workflow into two steps:

  # First, identify the repository.
  # Then, execute the operation.

In the future, this will allow us to put a possible "proxy the whole thing somewhere else" step in the middle, mirroring the behavior of Conduit.

This is trivially easy in `git` and `hg`. Both identify the repository on the commmand line.

This is fiendishly complex in `svn`, for the same reasons that hosting SVN was hard in the first place. Specifically:

  - The client doesn't tell us what it's after.
  - To get it to tell us, we have to send it a server capabilities string //first//.
  - We can't just start an `svnserve` process and read the repository out after a little while, because we may need to proxy the request once we figure out the repository.
  - We can't consume the client protocol frame that tells us what the client wants, because when we start the real server request it won't know what the client is after if it never receives that frame.
  - On the other hand, we must consume the second copy of the server protocol frame that would be sent to the client, or they'll get two "HELLO" messages and not know what to do.

The approach here is straightforward, but the implementation is not trivial. Roughly:

  - Start `svnserve`, read the "hello" frame from it.
  - Kill `svnserve`.
  - Send the "hello" to the client.
  - Wait for the client to send us "I want repository X".
  - Save the message it sent us in the "peekBuffer".
  - Return "this is a request for repository X", so we can proxy it.

Then, to continue the request:

  - Start the real `svnserve`.
  - Read the "hello" frame from it and throw it away.
  - Write the data in the "peekBuffer" to it, as though we'd just received it from the client.
  - State of the world is normal again, so we can continue.

Also fixed some other issues:

  - SVN could choke if `repository.default-local-path` contained extra slashes.
  - PHP might emit some complaints when executing the commit hook; silence those.

Test Plan: Pushed and pulled repositories in SVN, Mercurial and Git.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7034

Differential Revision: https://secure.phabricator.com/D11541
2015-01-28 10:18:07 -08:00
epriestley
d8550c114d Promote instance identity to the upstream and pass it to commit hooks
Summary:
Fixes T7019. In a cluster environment, pushes currently fail because the commit hook can't identify the instance.

For web processes, the hostname identifies the instance -- but we don't have a hostname in the hook.

For CLI processes, the environment identifies the instance -- but we don't have an environment in the hook under SVN.

Promote the instance identifier into the upstream and pack/unpack it explicitly for hooks. This is probably not useful for anyone but us, but the amount of special-purpose code we're introducing is very small.

I poked at trying to do this in a more general way, but:

  - We MUST know this BEFORE we run code, so the normal subclassing stuff is useless.
  - I couldn't come up with any other parameter which might ever be useful to pass in.

Test Plan: Used `git push` to push code through proxied HTTP, got a clean push.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7019

Differential Revision: https://secure.phabricator.com/D11495
2015-01-27 14:51:48 -08:00
John Mullanaphy
b960c8114b Changed \$callsign to \$argv[1] on commit_hook.php since is undefined causing an error when trying to report an error.
Summary:
Phabricator was going to give me an error message via commit_hook.php, unfortunately said error wasn't being set since
\$callsign was undefined. So, just changed \$callsign to \$argv[1] and now I get the appropriate commit.

Test Plan:
1. Add commit_hook.php to an SVN pre-commit.
2. Set the SVN to be hosted off of Phabricator.
3. Attempt to commit to commit to SVN repository.
Expected: Error message saying that the repository isn't hosted on Phabricator
Results: Error message saying undefined function.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D7920
2014-01-09 10:36:01 -08:00
epriestley
972dfa7bfc Add 'hook.d/' directories to SVN and Git repositories for custom hooks
Summary:
Fixes T4189. Ref T4151. Allows repositories to have additional custom hooks for operations which can't be expressed with Herald (one such operation is lint).

This adds only local hook directories, since they're easier to use with existing hooks than global directories. I might add global directories eventually.

This doesn't support Mercurial since we have no demand for it and it's more complicated (we lose compatibility and power by just dropping a `hooks.d/` somewhere).

Test Plan:
  - Pulled hosted SVN and Git repos to verify the hook directories generate correctly.
  - Added a variety of hooks to the hook directories (echo + pass, fail).
  - Pushed commits and verified the hooks fired (output expected info, or failed).
  - Verified push log reflected the correct error code ("3", external) and detail ("nope.sh") when rejecting.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4151, T4189

Differential Revision: https://secure.phabricator.com/D7884
2014-01-03 12:26:10 -08:00
epriestley
2cfc3acf32 Allow Herald pre-commit rules to act on repository projects
Summary:
Fixes T4264. Adds:

  - New "Repository's projects" field to Herald pre-commit rules, so you can write global rules which act based on projects.
  - Allows pre-ref/pre-content rules to bind to projects, and fire for all repositories in that project, so users with limited power can write rules which apply to many repositories.
  - The pre-ref and pre-content classes were starting to share a fair amount of code, so I made them both extend an abstract base class.

Test Plan: Wrote new pre-ref and pre-content rules bound to projects, then pushed commits into repositories in those projects and not in those projects. The "repository projects" field populated, and the rules fired for repositories in the relevant projects.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4264

Differential Revision: https://secure.phabricator.com/D7883
2014-01-03 12:24:28 -08:00
epriestley
74251b3636 Support bookmark hook operations in Mercurial
Summary: Ref T4195. Turns bookmark mutations in Mercurial into log objects.

Test Plan:
Pushed a pile of bookmarks and got logs:

{F89313}

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4195

Differential Revision: https://secure.phabricator.com/D7764
2013-12-17 08:34:30 -08:00
epriestley
e28b848ab2 Store pusher remote address and push protocol in PushLog
Summary: Ref T4195. Stores remote address and protocol in the logs, where possible.

Test Plan: Pushed some stuff, looked at the log, saw data.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4195

Differential Revision: https://secure.phabricator.com/D7711
2013-12-05 11:59:22 -08:00
epriestley
d2e9aee16d Reject dangerous changes in Git repositories by default
Summary: Ref T4189. This adds a per-repository "dangerous changes" flag, which defaults to off. This flag must be enabled to do non-appending branch mutation (delete branches / rewrite history).

Test Plan:
With flag on and off, performed various safe and dangerous pushes.

  >>> orbital ~/repos/POEMS $ git push origin :blarp
  remote: +---------------------------------------------------------------+
  remote: |      * * * PUSH REJECTED BY EVIL DRAGON BUREAUCRATS * * *     |
  remote: +---------------------------------------------------------------+
  remote:             \
  remote:              \                    ^    /^
  remote:               \                  / \  // \
  remote:                \   |\___/|      /   \//  .\
  remote:                 \  /V  V  \__  /    //  | \ \           *----*
  remote:                   /     /  \/_/    //   |  \  \          \   |
  remote:                   @___@`    \/_   //    |   \   \         \/\ \
  remote:                  0/0/|       \/_ //     |    \    \         \  \
  remote:              0/0/0/0/|        \///      |     \     \       |  |
  remote:           0/0/0/0/0/_|_ /   (  //       |      \     _\     |  /
  remote:        0/0/0/0/0/0/`/,_ _ _/  ) ; -.    |    _ _\.-~       /   /
  remote:                    ,-}        _      *-.|.-~-.           .~    ~
  remote:   \     \__/        `/\      /                 ~-. _ .-~      /
  remote:    \____(Oo)           *.   }            {                   /
  remote:    (    (--)          .----~-.\        \-`                 .~
  remote:    //__\\  \ DENIED!  ///.----..<        \             _ -~
  remote:   //    \\               ///-._ _ _ _ _ _ _{^ - - - - ~
  remote:
  remote:
  remote: DANGEROUS CHANGE: The change you're attempting to push deletes the branch 'blarp'.
  remote: Dangerous change protection is enabled for this repository.
  remote: Edit the repository configuration before making dangerous changes.
  remote:
  To ssh://dweller@localhost/diffusion/POEMS/
   ! [remote rejected] blarp (pre-receive hook declined)
  error: failed to push some refs to 'ssh://dweller@localhost/diffusion/POEMS/'

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, chad, richardvanvelzen

Maniphest Tasks: T4189

Differential Revision: https://secure.phabricator.com/D7689
2013-12-03 10:28:39 -08:00
epriestley
f93c6985ad Support Mercurial pretxnchangegroup hooks
Summary: Ref T4189. Fixes T2066. Mercurial has a //lot// of hooks so I'm not 100% sure this is all we need to install (we may need separate hooks for tags/bookmarks) but it should cover most of what we're after at least.

Test Plan:
  - `bin/repository pull`'d a Mercurial repo and got a hook install.
  - Pushed to a Mercurial repository over SSH and HTTP, with good/bad hooks. Saw hooks fire.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2066, T4189

Differential Revision: https://secure.phabricator.com/D7685
2013-12-02 15:46:03 -08:00
epriestley
017d6ccd07 Support SVN pre-commit hoooks
Summary:
Ref T4189. This adds SVN support, which was a little more messy than I though. Principally, we can not use `PHABRICATOR_USER` for Subversion, because it strips away the entire environment for "security reasons".

Instead, use `--tunnel-user` plus `svnlook author` to figure out the author.

Also fix "ssh://" clone URIs, which needs to be "svn+ssh://".

Test Plan:
  - Made SVN commits through the hook.
  - Made Git commits, too, to make sure I didn't break anything.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4189

Differential Revision: https://secure.phabricator.com/D7683
2013-12-02 15:45:55 -08:00
epriestley
618b5cbbc4 Install pre-commit hooks in Git repositories
Summary:
Ref T4189. T4189 describes most of the intent here:

  - When updating hosted repositories, sync a pre-commit hook into them instead of doing a `git fetch`.
  - The hook calls into Phabricator. The acting Phabricator user is sent via PHABRICATOR_USER in the environment. The active repository is sent via CLI.
  - The hook doesn't do anything useful yet; it just veifies basic parameters, does a little parsing, and exits 0 to allow the commit.

Test Plan:
  - Performed Git pushes and pulls over SSH and HTTP.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4189

Differential Revision: https://secure.phabricator.com/D7682
2013-12-02 15:45:36 -08:00