hgserver: pash command for remote backouts (bug 1288845); r?glob draft
authorGregory Szorc <gps@mozilla.com>
Thu, 25 Aug 2016 16:50:23 -0700
changeset 11052 2f8abd7e7c612215413a5df5303a215bea68f577
parent 11051 6bd62db11a38926b61ddca43003f85be9b8f2988
push id1676
push userbmo:gps@mozilla.com
push dateThu, 18 May 2017 00:17:53 +0000
reviewersglob
bugs1288845
hgserver: pash command for remote backouts (bug 1288845); r?glob We recently introduced the mozbackout extension and command with the intent of unifying how backouts are performed and transitioning to a "backouts as a service" workflow. This commit adds the server-side plumbing to enable `hg mozbackout` to be invoked via SSH. Pash is taught a "remote-backout <repo> <revs>" command. It invokes `hg mozbackout`. The feature is disabled by default in pash. But it is enabled on hg.mozilla.org. It is possible to invoke the command on any repo. But if the user doesn't have permissions to obtain a lock on that repo, they'll get a permissions error. Open issues: * Restrict to specific repos? * Interaction with hooks. e.g. should we ignore the treeclosure hook? * Restrict to specific users? (I'm leaning no because we shouldn't have to wait for a sheriff to perform a backout.) * Invert --dry-run argument so we require explicit consent to perform the backout (this will prevent surprises) MozReview-Commit-ID: JDC7xerhvCQ
ansible/roles/hg-ssh/templates/pash.json.j2
hgserver/pash/hg_helper.py
hgserver/pash/pash.py
hgserver/tests/test-remote-backout.t
--- a/ansible/roles/hg-ssh/templates/pash.json.j2
+++ b/ansible/roles/hg-ssh/templates/pash.json.j2
@@ -1,6 +1,7 @@
 {
   "hostname": "{{ pash_hostname | mandatory }}",
   "repo_config": true,
   "repo_group": true,
-  "user_repos": true
+  "user_repos": true,
+  "remote_backout": true
 }
--- a/hgserver/pash/hg_helper.py
+++ b/hgserver/pash/hg_helper.py
@@ -21,17 +21,19 @@ from sh_helper import (
     run_command,
 )
 
 
 Popen = subprocess.Popen
 PIPE = subprocess.PIPE
 
 HG = '/var/hg/venv_pash/bin/hg'
-MR_ASSOCIATE_LDAP = '/var/hg/version-control-tools/scripts/mozreview-associate-ldap'
+VCT = '/var/hg/version-control-tools'
+MR_ASSOCIATE_LDAP = '%s/scripts/mozreview-associate-ldap' % VCT
+MOZBACKOUT_EXTENSION = '%s/hgext/mozbackout' % VCT
 
 SUCCESSFUL_AUTH = '''
 A SSH connection has been successfully established.
 
 Your account (%s) has privileges to access Mercurial over
 SSH.
 
 '''.lstrip()
@@ -145,16 +147,24 @@ def assert_valid_repo_name(repo_name):
     if not is_valid_repo_name(repo_name):
         sys.stderr.write('Only alpha-numeric characters, ".", "_", and "-" are '
                          'allowed in repository\n')
         sys.stderr.write('names.  Additionally the first character of '
                          'repository names must be alpha-numeric.\n')
         sys.exit(1)
 
 
+def assert_valid_repo(repo_name):
+    assert_valid_repo_name(repo_name)
+
+    if not os.path.isdir(os.path.join(DOC_ROOT, repo_name, '.hg')):
+        sys.stderr.write('%s is not a valid Mercurial repository' % repo_name)
+        sys.exit(1)
+
+
 def run_hg_clone(user_repo_dir, repo_name, source_repo_path):
     userdir = "%s/users/%s" % (DOC_ROOT, user_repo_dir)
     dest_dir = "%s/%s" % (userdir, repo_name)
     dest_url = "/users/%s/%s" % (user_repo_dir, repo_name)
 
     if os.path.exists(dest_dir):
         print(USER_REPO_EXISTS % repo_name)
         sys.exit(1)
@@ -576,17 +586,18 @@ def mozreview_ldap_associate(args):
 
     print('LDAP account successfully associated!')
     print('exiting')
     return 0
 
 
 def serve(cname, enable_repo_config=False, enable_repo_group=False,
           enable_user_repos=False,
-          enable_mozreview_ldap_associate=False):
+          enable_mozreview_ldap_associate=False,
+          enable_remote_backout=False):
     ssh_command = os.getenv('SSH_ORIGINAL_COMMAND')
     if not ssh_command:
         sys.stderr.write(SUCCESSFUL_AUTH % os.environ['USER'])
         sys.stderr.write(NO_SSH_COMMAND)
         sys.exit(1)
 
     args = shlex.split(ssh_command)
 
@@ -667,12 +678,70 @@ def serve(cname, enable_repo_config=Fals
             with open(hgrc, 'rb') as fh:
                 sys.stdout.write(fh.read())
     elif args[0] == 'mozreview-ldap-associate':
         if not enable_mozreview_ldap_associate:
             print('mozreview-ldap-associate command not available')
             sys.exit(1)
 
         sys.exit(mozreview_ldap_associate(args[1:]))
+
+    # Instruct commits to be dropped from the autoland repo.
+    elif args[0] == 'remote-backout':
+        if not enable_remote_backout:
+            print('remote-backout command not available')
+            sys.exit(1)
+
+        # Args: remote-backout <repo> <revs>
+        if len(args) == 3:
+            flag = None
+            repo, revs = args[1:]
+        elif len(args) == 4:
+            flag, repo, revs = args[1:]
+        else:
+            print('usage: remote-backout [--dry-run] <repo> <revs>')
+            sys.exit(1)
+
+        if flag not in (None, '--dry-run'):
+            print('usage: invalid flag: %s; can only specify --dry-run' % flag)
+            sys.exit(1)
+
+        assert_valid_repo(repo)
+
+        # We need to be *very* careful about the user-specified argument
+        # passed here, as unsanitized values could e.g. pass unwanted flags
+        # to Mercurial. While the argument can be any revset, we limit it to
+        # well-defined values.
+        if not re.match('^[a-f0-9:+]+$', revs, re.IGNORECASE):
+            print('abort: specified revision is not allowed')
+            print('(hint: revisions must consist of hexidecimal characters, '
+                  'integers, and the special characters ":" and "+")')
+            sys.exit(1)
+
+        # Since the command may create a new changeset, we need to fill in the
+        # author field appropriately. We resolve the authenticated user's
+        # name and email from LDAP.
+        email = os.environ['USER']
+        cn = get_ldap_attribute(email, 'cn', get_ldap_settings()['url'])
+
+        # We don't need to check permissions to invoke this command because
+        # we're running as the authenticated user and filesystem permissions
+        # will prevent unprivileged changes.
+
+        hg_args = [
+            HG,
+            '-R', os.path.join(DOC_ROOT, repo),
+            '--config', 'ui.interactive=true',
+            '--config', 'extensions.mozbackout=%s' % MOZBACKOUT_EXTENSION,
+            'mozbackout',
+            '-u', '%s <%s>' % (cn, email),
+            revs,
+        ]
+
+        if flag:
+            hg_args.append('--dry-run')
+
+        os.execv(HG, hg_args)
+
     else:
         sys.stderr.write(SUCCESSFUL_AUTH % os.environ['USER'])
         sys.stderr.write(INVALID_SSH_COMMAND)
         sys.exit(1)
--- a/hgserver/pash/pash.py
+++ b/hgserver/pash/pash.py
@@ -86,15 +86,16 @@ def process_non_root_login(user):
 
     with open('/etc/mercurial/pash.json', 'rb') as fh:
         pash_settings = json.load(fh)
 
     hg_helper.serve(cname=pash_settings['hostname'],
                     enable_repo_config=pash_settings.get('repo_config', False),
                     enable_repo_group=pash_settings.get('repo_group', False),
                     enable_user_repos=pash_settings.get('user_repos', False),
-                    enable_mozreview_ldap_associate=pash_settings.get('mr_ldap_associate', False))
+                    enable_mozreview_ldap_associate=pash_settings.get('mr_ldap_associate', False),
+                    enable_remote_backout=pash_settings.get('remote_backout', False))
     sys.exit(0)
 
 
 if __name__ == '__main__':
     user = os.environ.get('USER')
     process_non_root_login(user)
new file mode 100644
--- /dev/null
+++ b/hgserver/tests/test-remote-backout.t
@@ -0,0 +1,123 @@
+#require hgmodocker
+
+  $ . $TESTDIR/hgserver/tests/helpers.sh
+  $ hgmoenv
+
+  $ pulse create-queue exchange/hgpushes/v2 v2
+
+  $ hgmo create-repo mozilla-central scm_level_3
+  (recorded repository creation in replication log)
+
+  $ hgmo create-ldap-user normal@mozilla.com normal 1500 'Normal User' --scm-level 1 --key-file normaluser
+
+  $ hgmo create-ldap-user sheriff@mozilla.com sheriff 1501 'Mozilla Sheriff' --key-file sheriff
+  $ hgmo add-user-to-group sheriff@mozilla.com scm_level_3
+
+  $ cat >> $HGRCPATH << EOF
+  > [ui]
+  > ssh = ssh -F `pwd`/ssh_config -i `pwd`/sheriff -l sheriff@mozilla.com
+  > EOF
+
+  $ alias sheriffssh="ssh -F `pwd`/ssh_config -i `pwd`/sheriff -l sheriff@mozilla.com -p ${SSH_PORT} ${SSH_SERVER}"
+
+No arguments prints usage
+
+  $ sheriffssh remote-backout
+  usage: remote-backout [--dry-run] <repo> <revs>
+  [1]
+
+1 argument not accepted
+
+  $ sheriffssh remote-backout mozilla-central
+  usage: remote-backout [--dry-run] <repo> <revs>
+  [1]
+
+Valid argument count, invalid repo name
+
+  $ sheriffssh remote-backout ../mozilla-central 1
+  Only alpha-numeric characters, ".", "_", and "-" are allowed in repository
+  names.  Additionally the first character of repository names must be alpha-numeric.
+  [1]
+
+  $ sheriffssh remote-backout --debugger 1
+  Only alpha-numeric characters, ".", "_", and "-" are allowed in repository
+  names.  Additionally the first character of repository names must be alpha-numeric.
+  [1]
+
+Invalid revs argument
+
+  $ sheriffssh remote-backout mozilla-central foo
+  abort: specified revision is not allowed
+  (hint: revisions must consist of hexidecimal characters, integers, and the special characters ":" and "+")
+  [1]
+
+  $ sheriffssh remote-backout mozilla-central 'author(gps)'
+  abort: specified revision is not allowed
+  (hint: revisions must consist of hexidecimal characters, integers, and the special characters ":" and "+")
+  [1]
+
+  $ sheriffssh remote-backout mozilla-central \"1 + 2\"
+  abort: specified revision is not allowed
+  (hint: revisions must consist of hexidecimal characters, integers, and the special characters ":" and "+")
+  [1]
+
+  $ sheriffssh remote-backout mozilla-central 1-2
+  abort: specified revision is not allowed
+  (hint: revisions must consist of hexidecimal characters, integers, and the special characters ":" and "+")
+  [1]
+
+Seed repo with data
+
+  $ hg -q clone ssh://${SSH_SERVER}:${SSH_PORT}/mozilla-central mozilla-central
+  $ cd mozilla-central
+  $ touch foo
+  $ hg -q commit -A -m initial
+  $ hg -q push -r .
+
+  $ touch file0
+  $ hg -q commit -A -m p1c1
+  $ touch file1
+  $ hg -q commit -A -m p1c2
+  $ hg -q push -r .
+
+  $ touch file2
+  $ hg -q commit -A -m p2c1
+  $ touch file3
+  $ hg -q commit -A -m p2c2
+  $ hg -q push -r .
+  $ hg -q up null
+
+A user without permissions to the repo can't perform operation
+
+  $ ssh -F $TESTTMP/ssh_config -i $TESTTMP/normaluser -l normal@mozilla.com -p ${SSH_PORT} ${SSH_SERVER} remote-backout mozilla-central 63a61534cf94 << EOF
+  > testing remote backout
+  > EOF
+  reason for backout:  abort: could not lock repository /repo/hg/mozilla/mozilla-central: Permission denied
+  [255]
+
+A user with permissions can perform operation
+
+  $ sheriffssh remote-backout --dry-run mozilla-central ebbf0c1f3aaa << EOF
+  > testing remote backout
+  > EOF
+  reason for backout:  will discard ebbf0c1f3aaa (requested explicitly): p1c1
+  will discard 6d7db80a435e (shares push with discarded changeset): p1c2
+  will retain 63a61534cf94: p2c1
+  will retain ea75f4c834c9: p2c2
+  created backout changeset 15d8e27d12d1
+  (aborting because dry run requested)
+  transaction abort!
+  rollback completed
+
+  $ sheriffssh remote-backout mozilla-central ebbf0c1f3aaa << EOF
+  > testing remote backout
+  > EOF
+  reason for backout:  will discard ebbf0c1f3aaa (requested explicitly): p1c1
+  will discard 6d7db80a435e (shares push with discarded changeset): p1c2
+  will retain 63a61534cf94: p2c1
+  will retain ea75f4c834c9: p2c2
+  created backout changeset * (glob)
+  
+  View your change here:
+    https://hg.mozilla.org/mozilla-central/rev/* (glob)
+  recorded changegroup in replication log in \d+\.\d+s (re)