diff --git a/master/buildbot/changes/p4poller.py b/master/buildbot/changes/p4poller.py index 65a94af67b9..1db6d2319b8 100644 --- a/master/buildbot/changes/p4poller.py +++ b/master/buildbot/changes/p4poller.py @@ -21,22 +21,64 @@ import dateutil import exceptions import os +import os.path import re from twisted.internet import defer +from twisted.internet import protocol +from twisted.internet import reactor from twisted.internet import utils from twisted.python import log +from buildbot import config from buildbot import util from buildbot.changes import base +debug_logging = False + + class P4PollerError(Exception): """Something went wrong with the poll. This is used as a distinctive exception type so that unit tests can detect and ignore it.""" +class TicketLoginProtocol(protocol.ProcessProtocol): + + """ Twisted process protocol to run `p4 login` and enter our password + in the stdin.""" + + def __init__(self, stdin, p4base): + self.deferred = defer.Deferred() + self.stdin = stdin + self.stdout = '' + self.stderr = '' + self.p4base = p4base + + def connectionMade(self): + if self.stdin: + if debug_logging: + log.msg("P4Poller: entering password for %s: %s" % (self.p4base, self.stdin)) + self.transport.write(self.stdin) + self.transport.closeStdin() + + def processEnded(self, reason): + if debug_logging: + log.msg("P4Poller: login process finished for %s: %s" % (self.p4base, reason.value.exitCode)) + self.deferred.callback(reason.value.exitCode) + + def outReceived(self, data): + if debug_logging: + log.msg("P4Poller: login stdout for %s: %s" % (self.p4base, data)) + self.stdout += data + + def errReceived(self, data): + if debug_logging: + log.msg("P4Poller: login stderr for %s: %s" % (self.p4base, data)) + self.stderr += data + + def get_simple_split(branchfile): """Splits the branchfile argument and assuming branch is the first path component in branchfile, will return @@ -76,6 +118,7 @@ def __init__(self, p4port=None, p4user=None, p4passwd=None, split_file=lambda branchfile: (None, branchfile), pollInterval=60 * 10, histmax=None, pollinterval=-2, encoding='utf8', project=None, name=None, + use_tickets=False, ticket_login_interval=60 * 60 * 24, server_tz=None, pollAtLaunch=False): # for backward compatibility; the parameter used to be spelled with 'i' @@ -92,6 +135,9 @@ def __init__(self, p4port=None, p4user=None, p4passwd=None, if project is None: project = '' + if use_tickets and not p4passwd: + config.error("You need to provide a P4 password to use ticket authentication") + self.p4port = p4port self.p4user = p4user self.p4passwd = p4passwd @@ -100,14 +146,19 @@ def __init__(self, p4port=None, p4user=None, p4passwd=None, self.split_file = split_file self.encoding = encoding self.project = util.ascii2unicode(project) + self.use_tickets = use_tickets + self.ticket_login_interval = ticket_login_interval self.server_tz = server_tz + self._ticket_passwd = None + self._ticket_login_counter = 0 + def describe(self): return "p4source %s %s" % (self.p4port, self.p4base) def poll(self): d = self._poll() - d.addErrback(log.err, 'P4 poll failed') + d.addErrback(log.err, 'P4 poll failed on %s, %s' % (self.p4port, self.p4base)) return d def _get_process_output(self, args): @@ -115,15 +166,52 @@ def _get_process_output(self, args): d = utils.getProcessOutput(self.p4bin, args, env) return d + def _acquireTicket(self, protocol): + command = [self.p4bin, ] + if self.p4port: + command.extend(['-p', self.p4port]) + if self.p4user: + command.extend(['-u', self.p4user]) + command.extend(['login', '-p']) + command = [c.encode('utf-8') for c in command] + + reactor.spawnProcess(protocol, self.p4bin, command, env=os.environ) + + def _parseTicketPassword(self, text): + lines = text.split("\n") + if len(lines) < 2: + return None + return lines[1].strip() + + def _getPasswd(self): + if self.use_tickets: + return self._ticket_passwd + return self.p4passwd + @defer.inlineCallbacks def _poll(self): + if self.use_tickets: + self._ticket_login_counter -= 1 + if self._ticket_login_counter <= 0: + # Re-acquire the ticket and reset the counter. + log.msg("P4Poller: (re)acquiring P4 ticket for %s..." % self.p4base) + protocol = TicketLoginProtocol(self.p4passwd + "\n", self.p4base) + self._acquireTicket(protocol) + yield protocol.deferred + + self._ticket_passwd = self._parseTicketPassword(protocol.stdout) + self._ticket_login_counter = max(self.ticket_login_interval / self.pollInterval, 1) + if debug_logging: + log.msg("P4Poller: got ticket password: %s" % self._ticket_passwd) + log.msg("P4Poller: next ticket acquisition in %d polls" % self._ticket_login_counter) + args = [] if self.p4port: args.extend(['-p', self.p4port]) if self.p4user: args.extend(['-u', self.p4user]) if self.p4passwd: - args.extend(['-P', self.p4passwd]) + args.extend(['-P', self._getPasswd()]) args.extend(['changes']) if self.last_change is not None: args.extend(['%s...@%d,now' % (self.p4base, self.last_change + 1)]) @@ -159,7 +247,7 @@ def _poll(self): if self.p4user: args.extend(['-u', self.p4user]) if self.p4passwd: - args.extend(['-P', self.p4passwd]) + args.extend(['-P', self._getPasswd()]) args.extend(['describe', '-s', str(num)]) result = yield self._get_process_output(args) @@ -169,7 +257,7 @@ def _poll(self): except exceptions.UnicodeError, ex: log.msg("P4Poller: couldn't decode changelist description: %s" % ex.encoding) log.msg("P4Poller: in object: %s" % ex.object) - log.err("P4Poller: poll failed") + log.err("P4Poller: poll failed on %s, %s" % (self.p4port, self.p4base)) raise lines = result.split('\n') diff --git a/master/buildbot/steps/source/p4.py b/master/buildbot/steps/source/p4.py index 13e48ef7b3e..690e9fd941b 100644 --- a/master/buildbot/steps/source/p4.py +++ b/master/buildbot/steps/source/p4.py @@ -46,7 +46,7 @@ class P4(Source): name = 'p4' - renderables = ['p4base', 'p4client', 'p4viewspec', 'p4branch'] + renderables = ['mode', 'p4base', 'p4client', 'p4viewspec', 'p4branch'] possible_modes = ('incremental', 'full') def __init__(self, mode='incremental', @@ -58,6 +58,7 @@ def __init__(self, mode='incremental', p4client_spec_options='allwrite rmdir', p4extra_args=None, p4bin='p4', + use_tickets=False, **kwargs): self.method = method self.mode = mode @@ -74,11 +75,12 @@ def __init__(self, mode='incremental', self.p4client = p4client self.p4client_spec_options = p4client_spec_options self.p4extra_args = p4extra_args + self.use_tickets = use_tickets Source.__init__(self, **kwargs) - if self.mode not in self.possible_modes: - config.error("mode %s is not one of %s" % (self.mode, self.possible_modes)) + if self.mode not in self.possible_modes and not interfaces.IRenderable.providedBy(self.mode): + config.error("mode %s is not an IRenderable, or one of %s" % (self.mode, self.possible_modes)) if not p4viewspec and p4base is None: config.error("You must provide p4base or p4viewspec") @@ -117,6 +119,9 @@ def checkInstall(p4Installed): return 0 d.addCallback(checkInstall) + if self.use_tickets and self.p4passwd: + d.addCallback(self._acquireTicket) + if self.mode == 'full': d.addCallback(self.full) elif self.mode == 'incremental': @@ -197,11 +202,12 @@ def _buildVCCommand(self, doCommand): command.extend(['-p', self.p4port]) if self.p4user: command.extend(['-u', self.p4user]) - if self.p4passwd: + if not self.use_tickets and self.p4passwd: # Need to find out if there's a way to obfuscate this command.extend(['-P', self.p4passwd]) if self.p4client: command.extend(['-c', self.p4client]) + # Only add the extra arguments for the `sync` command. if doCommand[0] == 'sync' and self.p4extra_args: command.extend(self.p4extra_args) @@ -216,6 +222,7 @@ def _dovccmd(self, command, collectStdout=False, initialStdin=None): if debug_logging: log.msg("P4:_dovccmd():workdir->%s" % self.workdir) + cmd = buildstep.RemoteShellCommand(self.workdir, command, env=self.env, logEnviron=self.logEnviron, @@ -313,6 +320,15 @@ def _createClientSpec(self): mo = re.search(r'Client (\S+) (.+)$', stdout, re.M) defer.returnValue(mo and (mo.group(2) == 'saved.' or mo.group(2) == 'not changed.')) + @defer.inlineCallbacks + def _acquireTicket(self, _): + if debug_logging: + log.msg("P4:acquireTicket()") + + # TODO: check first if the ticket is still valid? + initialStdin = self.p4passwd + "\n" + yield self._dovccmd(['login'], initialStdin=initialStdin) + def parseGotRevision(self, _): command = self._buildVCCommand(['changes', '-m1', '#have']) diff --git a/master/buildbot/test/unit/test_changes_p4poller.py b/master/buildbot/test/unit/test_changes_p4poller.py index 13a300766cf..0f2b90e55d5 100644 --- a/master/buildbot/test/unit/test_changes_p4poller.py +++ b/master/buildbot/test/unit/test_changes_p4poller.py @@ -21,56 +21,59 @@ from buildbot.test.util import changesource from buildbot.test.util import gpo from buildbot.util import datetime2epoch +from twisted.internet import error +from twisted.internet import reactor +from twisted.python import failure from twisted.trial import unittest first_p4changes = \ -"""Change 1 on 2006/04/13 by slamb@testclient 'first rev' + """Change 1 on 2006/04/13 by slamb@testclient 'first rev' """ second_p4changes = \ -"""Change 3 on 2006/04/13 by bob@testclient 'short desc truncated' + """Change 3 on 2006/04/13 by bob@testclient 'short desc truncated' Change 2 on 2006/04/13 by slamb@testclient 'bar' """ third_p4changes = \ -"""Change 5 on 2006/04/13 by mpatel@testclient 'first rev' + """Change 5 on 2006/04/13 by mpatel@testclient 'first rev' """ change_4_log = \ -"""Change 4 by mpatel@testclient on 2006/04/13 21:55:39 + """Change 4 by mpatel@testclient on 2006/04/13 21:55:39 \tshort desc truncated because this is a long description. """ change_3_log = \ -u"""Change 3 by bob@testclient on 2006/04/13 21:51:39 + u"""Change 3 by bob@testclient on 2006/04/13 21:51:39 \tshort desc truncated because this is a long description. ASDF-GUI-P3-\u2018Upgrade Icon\u2019 disappears sometimes. """ change_2_log = \ -"""Change 2 by slamb@testclient on 2006/04/13 21:46:23 + """Change 2 by slamb@testclient on 2006/04/13 21:46:23 \tcreation """ p4change = { 3: change_3_log + -"""Affected files ... + """Affected files ... ... //depot/myproject/branch_b/branch_b_file#1 add ... //depot/myproject/branch_b/whatbranch#1 branch ... //depot/myproject/branch_c/whatbranch#1 branch """, 2: change_2_log + -"""Affected files ... + """Affected files ... ... //depot/myproject/trunk/whatbranch#1 add ... //depot/otherproject/trunk/something#1 add """, 5: change_4_log + -"""Affected files ... + """Affected files ... ... //depot/myproject/branch_b/branch_b_file#1 add ... //depot/myproject/branch_b#75 edit @@ -250,6 +253,47 @@ def check(_): self.assertAllCommandsRan() return d + def test_acquire_ticket_auth(self): + self.attachChangeSource( + P4Source(p4port=None, p4user=None, p4passwd='pass', + p4base='//depot/myproject/', + split_file=lambda x: x.split('/', 1), + use_tickets=True)) + self.expectCommands( + gpo.Expect('p4', '-P', 'TICKET_ID_GOES_HERE', + 'changes', '-m', '1', '//depot/myproject/...').stdout(first_p4changes) + ) + + class FakeTransport: + + def __init__(self): + self.msg = None + + def write(self, msg): + self.msg = msg + + def closeStdin(self): + pass + + transport = FakeTransport() + + def spawnProcess(pp, cmd, argv, env): # p4poller uses only those arguments at the moment + self.assertEqual([cmd, argv], + ['p4', ['p4', 'login', '-p']]) + pp.makeConnection(transport) + self.assertEqual('pass\n', transport.msg) + pp.outReceived('Enter password:\nTICKET_ID_GOES_HERE\n') + so = error.ProcessDone(None) + pp.processEnded(failure.Failure(so)) + self.patch(reactor, 'spawnProcess', spawnProcess) + + d = self.changesource.poll() + + def check_ticket_passwd(_): + self.assertEquals(self.changesource._ticket_passwd, 'TICKET_ID_GOES_HERE') + d.addCallback(check_ticket_passwd) + return d + def test_poll_split_file(self): """Make sure split file works on branch only changes""" self.attachChangeSource( diff --git a/master/buildbot/test/unit/test_steps_source_p4.py b/master/buildbot/test/unit/test_steps_source_p4.py index 00349610cd8..71e1aee0eda 100644 --- a/master/buildbot/test/unit/test_steps_source_p4.py +++ b/master/buildbot/test/unit/test_steps_source_p4.py @@ -742,3 +742,64 @@ def test_slave_connection_lost(self): self.expectOutcome(result=RETRY, status_text=["update", "exception", "slave", "lost"]) return self.runStep() + + def test_ticket_auth(self): + self.setupStep(P4(p4port='localhost:12000', + p4base='//depot', p4branch='trunk', + p4user='user', p4client='p4_client1', + p4passwd='pass', use_tickets=True)) + + root_dir = '/home/user/workspace/wkdir' + if _is_windows: + root_dir = r'C:\Users\username\Workspace\wkdir' + client_spec = textwrap.dedent('''\ + Client: p4_client1 + + Owner: user + + Description: + \tCreated by user + + Root:\t%s + + Options:\tallwrite rmdir + + LineEnd:\tlocal + + View: + \t//depot/trunk/... //p4_client1/... + ''' % root_dir) + + self.expectCommands( + ExpectShell(workdir='wkdir', command=['p4', '-V']) + + 0, + + # This is the extra step that gets run when using tickets, + # and the password is not passed anymore after that. + ExpectShell(workdir='wkdir', + command=['p4', '-p', 'localhost:12000', '-u', 'user', + '-c', 'p4_client1', + 'login'], + initialStdin='pass\n') + + 0, + + ExpectShell(workdir='wkdir', + command=['p4', '-p', 'localhost:12000', '-u', 'user', + '-c', 'p4_client1', + 'client', '-i'], + initialStdin=client_spec) + + 0, + ExpectShell(workdir='wkdir', + command=(['p4', '-p', 'localhost:12000', '-u', 'user', + '-c', 'p4_client1', 'sync'])) + + 0, + ExpectShell(workdir='wkdir', + command=['p4', '-p', 'localhost:12000', '-u', 'user', + '-c', 'p4_client1', + 'changes', '-m1', '#have']) + + ExpectShell.log('stdio', + stdout="Change 100 on 2013/03/21 by user@machine \'duh\'") + + 0, + ) + self.expectOutcome(result=SUCCESS, status_text=["update"]) + return self.runStep() diff --git a/master/docs/manual/cfg-buildsteps.rst b/master/docs/manual/cfg-buildsteps.rst index f5b124e63a7..c3295063a15 100644 --- a/master/docs/manual/cfg-buildsteps.rst +++ b/master/docs/manual/cfg-buildsteps.rst @@ -869,6 +869,9 @@ you will receive a configuration error exception. P4(p4extra_args=['-Zproxyload'], ...) +``use_tickets`` + Set to ``True`` to use ticket-based authentication, instead of passwords (but + you still need to specify ``p4passwd``). .. index:: double: Gerrit integration; Repo Build Step diff --git a/master/docs/manual/cfg-changesources.rst b/master/docs/manual/cfg-changesources.rst index d3a25f4bfda..cc38fd43543 100644 --- a/master/docs/manual/cfg-changesources.rst +++ b/master/docs/manual/cfg-changesources.rst @@ -621,6 +621,14 @@ depot for changes. It accepts the following arguments: (e.g: ``Europe/Stockholm``) in case it's in a different timezone than the buildbot master. +``use_tickets`` + Set to ``True`` to use ticket-based authentication, instead of passwords (but + you still need to specify ``p4passwd``). + +``ticket_login_interval`` + How often to get a new ticket, in seconds, when ``use_tickets`` is enabled. + Defaults to 86400 (24 hours). + Example +++++++ diff --git a/master/docs/relnotes/index.rst b/master/docs/relnotes/index.rst index 9eeb4d9aacd..c5bcebe043d 100644 --- a/master/docs/relnotes/index.rst +++ b/master/docs/relnotes/index.rst @@ -106,6 +106,8 @@ Master Features ~~~~~~~~ +* Both the P4 source step and P4 change source support ticket-based authentication. + Fixes ~~~~~