Merge lp:~dooferlad/offspring/passing-ssh-key-to-slave2 into lp:offspring

[James Tunnicliffe]

Merged on top of trunk, conflicts resolved.

---

Replaces the merge lp:~dooferlad/offspring/passing-ssh-key-to-slave2 now it contains changes from lp:~bigkevmcd/offspring/passing-ssh-key-to-slave and fixes as a result of code review.

[Kevin McDermott (bigkevmcd)]

Some minor fixes needed and it's good to go...

#1

+ def test_currentBuildIsActive(self):

This is actually four tests hidden in one...can you split them out, your comments are really the docstrings for the other tests...

#2
+ def test_addUserToBzrSshUrl(self):

This is cool, but I'd prefer to see the comparison made more explicit, you know what the result should be, so it might be better to be explicit in the test...

expected_url = "bzr+ssh://<email address hidden>/~dooferlad/offspring/config"

Using the same technique as the implementation is no guarantee that the result is actually correct :-)

#3

+ def test_currentBuildIsActive(self):

Doesn't fit with the test naming conventions (pep8)

#4

There are a couple of minor conflicts with trunk, can merge trunk and resolve?

Other than these minor issues, looks good, nice branch :-)

review: Needs Fixing

[James Tunnicliffe (dooferlad)]

On 9 December 2011 07:42, Kevin McDermott <email address hidden> wrote:

> #3
>
> +    def test_currentBuildIsActive(self):
>
> Doesn't fit with the test naming conventions (pep8)

Please clarify. It fits with the naming convention of all the other
tests doesn't it?

Thanks,

--
James Tunnicliffe

[Kevin McDermott (bigkevmcd)]

Hi James,

    def test_currentBuildIsActive(self):

compared with...

    def test_slave_run_starts_server_and_logs(self):
    def test_slave_stop(self):
    def test_slave_stop_build_no_current_build(self):
    def test_slave_stop_build_with_current_build(self):
    def test_slave_start_build(self):
    def test_currentBuildIsActive(self):
    def test_addUserToBzrSshUrl(self):
    def test_start_build_with_custom_ssh_key(self):
    def test_check_ssh_key_deleted_when_build_finished(self):
    def test__check_ssh_agent(self):

Also (and I think this was my fault), test__check_ssh_agent, can you rename that to test_check_ssh_agent ?

thanks,

Kevin :-)

[James Tunnicliffe (dooferlad)]

I was mostly thinking about test_addUserToBzrSshUrl(self): the other
example :-) Would you suggest test_add_user-_to_bzr_ssh_url then?

I have already fixed up the __ problem.

James

On 9 December 2011 12:03, Kevin McDermott <email address hidden> wrote:
> Hi James,
>
>    def test_currentBuildIsActive(self):
>
> compared with...
>
>    def test_slave_run_starts_server_and_logs(self):
>    def test_slave_stop(self):
>    def test_slave_stop_build_no_current_build(self):
>    def test_slave_stop_build_with_current_build(self):
>    def test_slave_start_build(self):
>    def test_currentBuildIsActive(self):
>    def test_addUserToBzrSshUrl(self):
>    def test_start_build_with_custom_ssh_key(self):
>    def test_check_ssh_key_deleted_when_build_finished(self):
>    def test__check_ssh_agent(self):
>
> Also (and I think this was my fault), test__check_ssh_agent, can you rename that to test_check_ssh_agent ?
>
> thanks,
>
> Kevin :-)
> --
> https://code.launchpad.net/~dooferlad/offspring/passing-ssh-key-to-slave2/+merge/84987
> You are the owner of lp:~dooferlad/offspring/passing-ssh-key-to-slave2.

--
James Tunnicliffe

[Kevin McDermott (bigkevmcd)]

review: Approve

=== modified file 'lib/offspring/build/bin/offspring-build'
--- lib/offspring/build/bin/offspring-build	2011-11-30 14:15:52 +0000
+++ lib/offspring/build/bin/offspring-build	2011-12-09 17:18:46 +0000
@@ -32,6 +32,7 @@
     source "${IBS_FUNC_DIR}/reports.sh"
     source "${IBS_FUNC_DIR}/logging.sh"
     source "${IBS_FUNC_DIR}/files.sh"
+    source "${IBS_FUNC_DIR}/ssh.sh"
 
     [ -n "${IBS_PROXY}" ] && export http_proxy="${IBS_PROXY}"
 
@@ -192,6 +193,9 @@
 echo "Performing initialization..."
 _Initialize
 
+# If any SSH keys have been passed to us, load them into ssh-agent
+AddKeys ${3}
+
 # Setup and start logging
 _Setup
 

=== added file 'lib/offspring/build/functions/ssh.sh'
--- lib/offspring/build/functions/ssh.sh	1970-01-01 00:00:00 +0000
+++ lib/offspring/build/functions/ssh.sh	2011-12-09 17:18:46 +0000
@@ -0,0 +1,23 @@
+#!/bin/bash
+
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+AddKeys() {
+    RUN_SSH_AGENT=1
+
+    if [ -z "${1}" ]; then
+        let RUN_SSH_AGENT=-1
+    fi
+
+    if [ $RUN_SSH_AGENT = 1 ]; then
+        eval `ssh-agent`
+        ssh-add $1
+    fi
+}
+
+StopSSHAgent() {
+    if [ $RUN_SSH_AGENT = 1 ]; then
+        kill $SSH_AGENT_PID
+    fi
+}
\ No newline at end of file

=== added directory 'lib/offspring/build/tests'
=== added file 'lib/offspring/build/tests/offspring_test_key'
--- lib/offspring/build/tests/offspring_test_key	1970-01-01 00:00:00 +0000
+++ lib/offspring/build/tests/offspring_test_key	2011-12-09 17:18:46 +0000
@@ -0,0 +1,27 @@
+-----BEGIN RSA PRIVATE KEY-----
+MIIEowIBAAKCAQEAuzjspIVMkDrI+1MkDCFVh9XBbkc55hCpI/RqMz594jDc86iM
+4vRmq5R+HJszi/mbbklgsD8x0bGMz65u+yvp9AxScn0/d5GXfhZKHz3fROjT4WEf
+1NB2gqAink3e9YJZ+04sAJ1Xz81w8wkoTh7roU35GBXSjbH1rMl17qLny8pUsuv6
+gehemzYAcvM9mAASfAzNvPR87NjrbizYMOwWsQ6stEm4Aemdcio42ZZBHXVm/QV0
+hOTXLfopOH622iIuEFU5j1blUY/NXyDBU+hNUmMeO9DpL06oDu0XIdwb3nPaEQ3n
+U+I1VvTR8gz1sQgXYVGa3qGbAm01mcXWQ+Is8QIDAQABAoIBADFRnYT5WGHmGmua
+SzSm01ElDf9u4+GnIedGy3MUUzTyikHldLeUijdItq/ycnG9HyS+T6od+5Gxo9ZR
+rQqdVtPjKxTdyYpF4BJm7L+uHNKaQrZsT2ZQQ+fFJ1lsSf+ChxGcVhsTV7517/sV
+vnhVzNyBHc0qcnzBFGaf62EhqM4V2ogsj3bLd6nwRAyvpbUMrWWv057teKooxBsk
+j0QXP3WYEzAmdbphkG1qPCGJQNhYBbyjU7Cf4bJg/CbHENynM8zlbiFVUWgfwtNJ
+ZcddrFs3Y1P6nDa/Jd2ifqx6V7M7LAqpKjqwiuCNPiXVWyxmELaKu8Yvmqsv5qXF
+ixKcDAECgYEA9Ozj7r0PHkrrCcoGdIQTs1sZS0eV+yDKdOOlUeFpPMHi6PqROOwY
+cKr4RtkJC/AgjfA3tUEBemVKYoCOVWYeHmlTd3wQpl7lqqGN431A18/Z0psKpFh+
+wLfMZXBslYybY3b+VguICuyICN/hksCxKJNurKalPow/EYqo0dspOkkCgYEAw7AZ
+QKCfnuYfGdGto8JFizkxS6uvTInXNTzRZPdkbgd/JhCmnXENg93QSo/GWe2dehQG
+NZkCpVnZFKkubbxCc/OBAF+es7dVRc8vy8uwa2nOW9SG6yLNlNKdTwJxmRlV+53H
+mzC0PZ4GyMnBFJ46+rem3siLLVMlkolbZzlnHWkCgYA8+e0VNsRYylYRrdZFk8xD
+zt5RO5U/XD6LM1GpPPEySyLu1dLp1P2Qrz/4g3gZHMM+ExwLaA+yJR2LwG2vHSlK
+cPZyvNR4Vw/elzH3/Orzz69vG2Je4BlOaXPdnUurP8I/1RQk3+IStih37ST/oDF6
+5JmdKi/hjpD1EQxOkr2E4QKBgC9Y/3MsqhJ3WZUUr6/MxKjgCLZnbv3U6DZgZcXJ
+OgqJU9Fw++9iOEPsuoYf7X06yfyMtcfoIsTBTY37NVml0Gpfw5nEiRCwzjga3lSw
+DxqeOijr7k0cWaOlphxE2hmSEMTVs0MwcJvsDXYtosMLWffp0b1bxpkL4i5nf68l
+K3bpAoGBAJR5uzQGl5pXg7QKYPv1Sdi6pBp1wGSpFzzPatlpnrO8zZi9c0O29h7R
+3lMfBazjjNQmJF25uh+svgPCboNDh9dhqDBKf/ep0ju4wpXF2I0JM5xhYP2s242K
+mqsBXi4JWa9GAjVqVA48M3Vsu4r1YTOxIqDjnAFpnTU2tzzp2OFZ
+-----END RSA PRIVATE KEY-----

=== added file 'lib/offspring/build/tests/offspring_test_key.pub'
--- lib/offspring/build/tests/offspring_test_key.pub	1970-01-01 00:00:00 +0000
+++ lib/offspring/build/tests/offspring_test_key.pub	2011-12-09 17:18:46 +0000
@@ -0,0 +1,1 @@
+ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQC7OOykhUyQOsj7UyQMIVWH1cFuRznmEKkj9GozPn3iMNzzqIzi9GarlH4cmzOL+ZtuSWCwPzHRsYzPrm77K+n0DFJyfT93kZd+FkofPd9E6NPhYR/U0HaCoCKeTd71gln7TiwAnVfPzXDzCShOHuuhTfkYFdKNsfWsyXXuoufLylSy6/qB6F6bNgBy8z2YABJ8DM289Hzs2OtuLNgw7BaxDqy0SbgB6Z1yKjjZlkEddWb9BXSE5Nct+ik4frbaIi4QVTmPVuVRj81fIMFT6E1SYx470OkvTqgO7Rch3Bvec9oRDedT4jVW9NHyDPWxCBdhUZreoZsCbTWZxdZD4izx dooferlad@JT-Linaro-ThinkPad-T410

=== added file 'lib/offspring/build/tests/test.sh'
--- lib/offspring/build/tests/test.sh	1970-01-01 00:00:00 +0000
+++ lib/offspring/build/tests/test.sh	2011-12-09 17:18:46 +0000
@@ -0,0 +1,27 @@
+#!/bin/bash
+
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+# Get the full directory path of the test script
+DIR="$( cd "$( dirname "$0" )" && pwd )"
+
+# Load functions under test
+source "${DIR}/../functions/ssh.sh"
+
+# Force key permissions to 600 to keep SSH happy
+chmod 600 "${DIR}/offspring_test_key"
+
+# Load test key
+AddKeys "${DIR}/offspring_test_key"
+
+# See what keys are loaded (expect lib/offspring/build/tests/offspring_test_key)
+echo After loading test key, keys are:
+ssh-add -l
+
+# Stop the loaded ssh-agent process
+StopSSHAgent
+
+# Check that no keys are listed: Expect "Could not open a connection to your authentication agent."
+echo "After killing ssh agent, we shouldn't be able to find any keys..."
+ssh-add -l

=== added file 'lib/offspring/build/wrappers/test.sh'
--- lib/offspring/build/wrappers/test.sh	1970-01-01 00:00:00 +0000
+++ lib/offspring/build/wrappers/test.sh	2011-12-09 17:18:46 +0000
@@ -0,0 +1,52 @@
+#!/bin/bash
+
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+Buildtool_setup() {
+    Catch_faults permit_errors
+
+    TARGETFS="${IBS_WORKDIR}/${PROJECT}/chroot"
+
+    if [ -d "${IBS_WORKDIR}/${PROJECT}/config" ]; then
+        rm -rf "${IBS_WORKDIR}/${PROJECT}/config" > /dev/null 2>&1 || true
+    fi
+
+    Action "Copying project configuration directory into work directory for build"
+    Export_project_config "${PROJECT}" "${IBS_WORKDIR}/${PROJECT}/config"
+    Action_done
+
+    if [ "$?" != "0" ]; then
+        Error "Unable to copy project build configuration to %s" "${IBS_WORKDIR}/${PROJECT}/config"
+        Die
+    fi
+    Catch_faults disallow_errors
+}
+
+Buildtool_build() {
+    Action "Test Build - Buildtool_build"
+    Action_done
+}
+
+Buildtool_cleanup() {
+    Action "Test Build - Buildtool_cleanup"
+    Action_done
+}
+
+_Check_directory() {
+    if [ "${PWD}" != "${IBS_WORKDIR}/${PROJECT}" ]; then
+        Error "Working directory is '%s'; expected '%s'" "{$PWD}" "${IBS_WORKDIR}/${PROJECT}"
+        Die
+    fi
+    Debug "Directory check performed and was successful; PWD: %s" "${PWD}"
+}
+
+_SaveConfig() {
+    Action "Test Build - _SaveConfig"
+    Action_done
+}
+
+_SaveBuildWorkDirectory() {
+    Action "Test Build - _SaveBuildWorkDirectory"
+    Action_done
+}

=== modified file 'lib/offspring/master/models.py'
--- lib/offspring/master/models.py	2011-12-01 16:02:08 +0000
+++ lib/offspring/master/models.py	2011-12-09 17:18:46 +0000
@@ -104,9 +104,12 @@
         """
         Pass BuildRequest to a slave server.
         """
-        result = self.server_proxy.startBuild(request.project.name,
-                                              request.project.config_url)
-        if result == LexbuilderSlaveServer.REQUEST_OKAY:
+        status = self.server_proxy.startBuild(request.project.name,
+                                              request.project.config_url,
+                                              request.project.lp_user,
+                                              request.project.lp_ssh_key)
+
+        if status == LexbuilderSlaveServer.REQUEST_OKAY:
             self.previous_state = self.current_state
             self.current_state = LexbuilderSlave.STATE_BUILDING
             self.current_job = BuildResult(request, self)
@@ -174,6 +177,12 @@
     config_url = Unicode()
     notes = Unicode()
 
+    # The Launchpad User and SSH key are stored per project. If we stored them
+    # per LaunchpadProject, anyone who could create a Project referencing that
+    # LaunchpadProject could get access to the private data in it.
+    lp_user = Unicode()
+    lp_ssh_key = Unicode()
+
     def __init__(self, name, priority=40):
         self.title = unicode(name.capitalize())
         self.name = unicode(name)

=== modified file 'lib/offspring/master/tests/helpers.py'
--- lib/offspring/master/tests/helpers.py	2011-11-30 12:31:01 +0000
+++ lib/offspring/master/tests/helpers.py	2011-12-09 17:18:46 +0000
@@ -128,6 +128,8 @@
             "suite VARCHAR(200) NOT NULL, "
             "status VARCHAR(200), "
             "notes TEXT, "
+            "lp_user TEXT, "
+            "lp_ssh_key text, "
             "is_active BOOLEAN NOT NULL DEFAULT TRUE)")
 
         self.connection.execute(

=== modified file 'lib/offspring/slave/slave.py'
--- lib/offspring/slave/slave.py	2011-12-06 15:10:45 +0000
+++ lib/offspring/slave/slave.py	2011-12-09 17:18:46 +0000
@@ -2,6 +2,8 @@
 # GNU Affero General Public License version 3 (see the file LICENSE).
 import logging
 import platform
+import re
+import tempfile
 import time
 import xmlrpclib
 
@@ -55,13 +57,36 @@
         self.server.stop()
         self.server.join()
 
-    def startBuild(self, projectName, configBranchURL):
+    def addUserToBzrSshUrl(self, configBranchURL, lpUser):
+        """Adds lpUser to configBranchURL, returns updated configBranchURL."""
+
+        # check for lp: format URLs. Convert these to bzr+ssh ones
+        configBranchURL = re.sub("lp:", "bzr+ssh://bazaar.launchpad.net/",
+                                 configBranchURL)
+
+        # The assumption is that this function will only be called with a
+        # bzr+ssh URL (hence the name!)
+        assert(re.search("bzr\+ssh://", configBranchURL))
+
+        # Add user to URL
+        configBranchURL = re.sub("bzr\+ssh://",
+                                 "bzr+ssh://" + lpUser + "@", configBranchURL)
+
+        return configBranchURL
+
+    def startBuild(self, projectName, configBranchURL, lpUser=None,
+                   lpSSHKey=None):
         """
         Instigate a new build for projectName, using the configuration at
         configBranchURL.
         """
+        if lpUser:
+            # We don't want to pass the user separately, we want it to be
+            # integrated into the configBranchURL.
+            configBranchURL = self.addUserToBzrSshUrl(configBranchURL, lpUser)
+
         self.currentBuild = ProjectBuildProcess(
-            self, projectName, configBranchURL)
+            self, projectName, configBranchURL, lpSSHKey)
         logging.info("Starting build: %s %s %s" % (
             self.currentBuild.getCommandPath(), projectName, configBranchURL))
         self.currentBuild.start()
@@ -75,9 +100,19 @@
             if self.currentBuild.process is not None:
                 if self.currentBuild.process.poll() is None:
                     self.currentBuild.process.terminate()
-            # Sleep before sending kill.
-            time.sleep(self.config.getint("slave", "child_wait_timeout"))
-            self.currentBuild.process.kill()
+                    if self.currentBuild.process.poll() is None:
+                        # Sleep before sending kill.
+                        time.sleep(int(self.config.get("slave",
+                                                       "child_wait_timeout")))
+                        if self.currentBuild.process.poll() is None:
+                            self.currentBuild.process.kill()
+            self.currentBuild.tidyUp()
+
+    def currentBuildIsActive(self):
+        """Return True if current build is active, else False."""
+        if self.currentBuild is not None:
+            return self.currentBuild.state == ProjectBuildProcess.STATE_ACTIVE
+        return False
 
 
 class LexbuilderSlaveServer(LexbuilderXMLRPCServer):
@@ -101,10 +136,8 @@
         """
         Start building the project on the LexbuilderSlave.
         """
-        current_build = self.slave.currentBuild
-        if current_build is not None:
-            if current_build.state == ProjectBuildProcess.STATE_ACTIVE:
-                return LexbuilderSlaveServer.REQUEST_ERROR
+        if self.slave.currentBuildIsActive():
+            return LexbuilderSlaveServer.REQUEST_ERROR
         self.slave.startBuild(projectName, configBranchURL)
         return LexbuilderSlaveServer.REQUEST_OKAY
 
@@ -229,11 +262,32 @@
 
 class ProjectBuildProcess(MonitoredProcess):
 
-    def __init__(self, supervisor, projectName, configBranchURL):
-        MonitoredProcess.__init__(
-            self, supervisor, [projectName, configBranchURL])
+    def __init__(self, supervisor, projectName, configBranchURL,
+                 lpSSHKey=None):
+        args = [projectName, configBranchURL]
+        self.ssh_key_file = None
+
+        if lpSSHKey:
+            # Write SSH key to disk. tempfile will delete it after the build
+            # when this class goes out of scope.
+            # Need to have this here so we can pass the temp file name
+            # containing the key to the builder process on its command line.
+            self.ssh_key_file = tempfile.NamedTemporaryFile()
+            args.append(self.ssh_key_file.name)
+            self.ssh_key_file.write(lpSSHKey)
+            self.ssh_key_file.flush()
+
+        MonitoredProcess.__init__(self, supervisor, args)
         self.projectName = projectName
 
+    def __del__(self):
+        self.tidyUp()
+
+    def tidyUp(self):
+        if self.ssh_key_file:
+            self.ssh_key_file.close()
+            self.ssh_key_file = None
+
     def setup(self):
         self.buildName = None
 
@@ -256,6 +310,7 @@
         logging.debug(
             "Setting state of slave to %s", LexbuilderSlave.STATE_IDLE)
         self.master.state = LexbuilderSlave.STATE_IDLE
+        self.tidyUp()
 
     def getCommandPath(self):
         return self.master.config.get("slave", "build_command")

=== modified file 'lib/offspring/slave/tests/test_slave.py'
--- lib/offspring/slave/tests/test_slave.py	2011-12-08 17:29:41 +0000
+++ lib/offspring/slave/tests/test_slave.py	2011-12-09 17:18:46 +0000
@@ -1,8 +1,12 @@
 # Copyright 2011 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
+
+from ConfigParser import ConfigParser
+from datetime import datetime
+import os
 import platform
-from datetime import datetime
-from ConfigParser import ConfigParser
+import re
+import subprocess
 import xmlrpclib
 
 from offspring.tests.helpers import CaptureLoggingTestCase
@@ -56,6 +60,9 @@
     def getBuildName(self):
         return self.name
 
+    def tidyUp(self):
+        self.tidied = True
+
 
 class LexbuilderSlaveTestMixin(object):
     """
@@ -78,6 +85,10 @@
         self.config.set("slave", "port", 8765)
 
 
+class FakeProjectBuildProcess(object):
+    state = ProjectBuildProcess.STATE_ACTIVE
+
+
 class TestLexbuilderSlave(LexbuilderSlaveTestMixin, CaptureLoggingTestCase):
     """
     Tests for the LexbuilderSlave class.
@@ -135,6 +146,7 @@
 
         self.assertTrue(fake_build.process.terminated)
         self.assertTrue(fake_build.process.killed)
+        self.assertTrue(fake_build.tidied)
         self.assertEqual("Killing active build.\n", log_file.getvalue())
 
     def test_slave_start_build(self):
@@ -148,10 +160,187 @@
 
         fake_url = u"http://example.com/project"
         self.slave.startBuild(u"testing", fake_url)
+        self.assertEqual(["testing", fake_url],
+                         self.slave.currentBuild.arguments)
         self.assertEqual(
             "Starting build: /usr/bin/offspring-build testing %s\n" % fake_url,
             log_file.getvalue())
 
+    def test_current_build_is_active_no_current_build(self):
+        """Test currentBuildIsActive returns False when currentBuild = False"""
+
+        # To start with, currentBuild should be None, so active == False
+        self.assertEqual(self.slave.currentBuild, None)
+        self.assertEqual(self.slave.currentBuildIsActive(), False)
+
+    def test_current_build_is_active_build_in_active_state(self):
+        """Test currentBuildIsActive returns True when build is active"""
+        # Set up a fake build process (starts off in STATE_ACTIVE)
+        self.slave.currentBuild = FakeProjectBuildProcess()
+
+        # Now we have started a build, currentBuild has been assigned.
+        self.assertNotEqual(self.slave.currentBuild, None)
+        self.assertEqual(self.slave.currentBuild.state,
+                         ProjectBuildProcess.STATE_ACTIVE)
+        self.assertEqual(self.slave.currentBuildIsActive(), True)
+
+    def test_current_build_is_active_build_state_not_active(self):
+        """Test currentBuildIsActive when currentBuild state != STATE_ACTIVE
+
+        currentBuildIsActive should return False when
+        slave.currentBuild.state != STATE_ACTIVE. Clearly, currentBuild needs
+        to be valid for this test.
+
+        """
+
+        # Set up a fake build process (starts off in STATE_ACTIVE)
+        self.slave.currentBuild = FakeProjectBuildProcess()
+
+        self.slave.currentBuild.state = ProjectBuildProcess.STATE_SUCCESS
+        self.assertEqual(self.slave.currentBuildIsActive(), False)
+
+        self.slave.currentBuild.state = ProjectBuildProcess.STATE_FAILED
+        self.assertEqual(self.slave.currentBuildIsActive(), False)
+
+    def test_add_user_to_bzr_ssh_url(self):
+        """Test lp:project URL form converted into bzr+ssh form adding user."""
+
+        config_url = u"lp:~dooferlad/offspring/config"
+        user = "user"
+
+        updated_url = self.slave.addUserToBzrSshUrl(config_url, user)
+        expected_url = ("bzr+ssh://[e-mail]/~dooferlad/" +
+                        "offspring/config")
+        self.assertEqual(updated_url, expected_url)
+
+    def test_start_build_with_custom_ssh_key(self):
+        """Test slave converts lp:project URL into bzr+ssh form adding user.
+
+        When sending a URL to the slave in the form lp:project, and
+        a user and SSH key are sent as well, that the slave adds the user
+        into the URL so a private config branch can be checked out (URL needs
+        to be modified to send the user name through BZR to SSH).
+        """
+        log_file = self.capture_logging()
+        build_process_mock = self.mocker.patch(ProjectBuildProcess)
+        build_process_mock.start()
+
+        sshkey_filename = self.makeFile()
+        tempfile_mock = self.mocker.replace("tempfile.NamedTemporaryFile")
+        tempfile_mock()
+        self.mocker.result(open(sshkey_filename, "w"))
+
+        self.mocker.replay()
+
+        config_url = u"lp:~dooferlad/offspring/config"
+        args = ["project_name", config_url, "user", "key"]
+
+        self.slave.startBuild(*args)
+
+        new_url = re.sub("lp:",
+                          "bzr+ssh://" + args[2] + "@bazaar.launchpad.net/",
+                          args[1])
+        self.assertEqual(["project_name", new_url, sshkey_filename],
+                         self.slave.currentBuild.arguments)
+        self.assertEqual(
+            "Starting build: /usr/bin/offspring-build project_name %s\n"
+            % new_url, log_file.getvalue())
+
+    def test_check_ssh_key_deleted_when_build_finished(self):
+        """Test SSH key file lifetime management.
+
+        The SSH key of a project is written to a temporary file when the
+        build process starts, this is tested by the first assert. When the
+        build has finished or is stopped, the key file is deleted. This is
+        tested by the second assert.
+        """
+        log_file = self.capture_logging()
+        build_process_mock = self.mocker.patch(ProjectBuildProcess)
+        build_process_mock.start()
+        self.mocker.replay()
+
+        config_url = u"lp:~dooferlad/offspring/config"
+        args = ["project_name", config_url, "user", "key"]
+
+        self.slave.startBuild(*args)
+
+        file_path = self.slave.currentBuild.ssh_key_file.name
+        read_key = open(file_path).read()
+
+        # We should have a key written to a file for the slave to read...
+        self.assertEqual(read_key, args[3])
+
+        # Can't use XMLRPC interface here because it checks to see if a build
+        # has actually started. We just want to test the clean up logic.
+        self.slave.stopBuild()
+        # Having stopped the slave, the key should have been erased
+        self.assertFalse(os.path.exists(file_path))
+
+    def test_check_ssh_agent(self):
+        """Check that SSH agent is started and killed by build.
+
+        In order to pass private keys to SSH, the build script starts
+        ssh-agent if a private key file is passed to it. When the build has
+        completed, the build script kills SSH agent.
+
+        This test uses the called shell script to test the functions used
+        by the build script to start ssh-agent, load the specified key and
+        kill the started ssh-agent process.
+
+        """
+        # This is a wrapper around a shell script that runs the commands and
+        # we just check the output.
+
+        dir_of_this_file = os.path.dirname(os.path.abspath(__file__))
+        test_script = os.path.join(dir_of_this_file,
+                                   "../../build/tests/test.sh")
+
+        test_script_process = subprocess.Popen(test_script,
+                                               stdout=subprocess.PIPE,
+                                               stderr=subprocess.STDOUT)
+
+        stdout = test_script_process.communicate()[0]
+
+        lines = stdout.split("\n")
+
+        # Sample output:
+        # Agent pid 9839
+        # Identity added: /bla/offspring_test_key (/bla/offspring_test_key)
+        # After loading test key, keys are:
+        # 2048 <fingerprint> /bla/offspring_test_key (RSA)
+        # After killing ssh agent, we shouldn't be able to find any keys...
+        # Could not open a connection to your authentication agent.
+
+        # Expect the first line to be of the form "Agent pid [PID]"
+        self.assertTrue(re.search("Agent pid \d+", lines[0]),
+                        "Test build ssh-agent check failed: Expected to find" +
+                        "\nAgent pid <PID>\nfound:\n" + lines[0])
+
+        # Expect the second line to be of the form:
+        # Identity added: <path to...>/offspring_test_key
+        self.assertTrue(re.search("Identity added:.*?offspring_test_key",
+                                  lines[1]),
+                        "Test build ssh-agent check failed: Expected to find" +
+                        "\nIdentity added: <path to...>/offspring_test_key" +
+                        "\nfound:\n" + lines[1])
+
+        # Ignore the third line, it is just a fixed message
+        # Expect the fourth line to be:
+        # <key fingerprint> /path/to/offspring_test_key (RSA)
+        self.assertTrue(re.search("offspring_test_key \(RSA\)", lines[3]),
+                        "Test build ssh-agent check failed: Expected to find" +
+                        "\noffspring_test_key (RSA)\nfound:\n" + lines[3])
+
+        # Ignore the fifth line, it is just a fixed message
+        # Expect the sixth line to be:
+        # "Could not open a connection to your authentication agent."
+        search_string = (
+            "^Could not open a connection to your authentication agent\.$")
+        message = ("Test build ssh-agent check failed: Expected to find" +
+                   "\nCould not open a connection to your authentication" +
+                   "agent\n, found:\n" + lines[5])
+        self.assertTrue(re.search(search_string, lines[5]), message)
+
 
 class FakeLexbuilderSlave(object):
     """Fake slave for use in tests."""
@@ -169,6 +358,12 @@
     def stopBuild(self):
         self.stopped = True
 
+    def currentBuildIsActive(self):
+        """Return True if current build is active, else False."""
+        if self.currentBuild is not None:
+            return self.currentBuild.state == ProjectBuildProcess.STATE_ACTIVE
+        return False
+
 
 class TestLexbuilderSlaveServer(LexbuilderSlaveTestMixin,
                                 CaptureLoggingTestCase):

=== modified file 'lib/offspring/web/queuemanager/models.py'
--- lib/offspring/web/queuemanager/models.py	2011-11-30 14:15:52 +0000
+++ lib/offspring/web/queuemanager/models.py	2011-12-09 17:18:46 +0000
@@ -218,6 +218,15 @@
     launchpad_project = models.ForeignKey("LaunchpadProject", blank=True, null=True)
     status = models.CharField('status', max_length=200, null=True, choices=STATUS_CHOICES)
     config_url = models.CharField('config URL', max_length=200) # Add some kind of validator here.
+
+    # The Launchpad User and SSH key are stored per project. If we stored them
+    # per LaunchpadProject, anyone who could create a Project referencing that
+    # LaunchpadProject could get access to the private data in it.
+    lp_user = models.TextField("Launchpad User", null=True, editable=False,
+                               blank=True)
+    lp_ssh_key = models.TextField("Launchpad User's SSH Key", blank=True,
+                                  null=True, editable=False)
+
     notes = models.TextField(blank=True, null=True)
 
     class Meta:

=== added file 'migration/002-bzr-ssh-user-key.sql'
--- migration/002-bzr-ssh-user-key.sql	1970-01-01 00:00:00 +0000
+++ migration/002-bzr-ssh-user-key.sql	2011-12-09 17:18:46 +0000
@@ -0,0 +1,2 @@
+ALTER TABLE projects ADD COLUMN "lp_user" text;
+ALTER TABLE projects ADD COLUMN "lp_ssh_key" text;