Merge lp:~bigkevmcd/offspring/remove-ssh-url into lp:offspring

[Kevin McDermott]

This strips the SSH url logic and tests.

[lp:~bigkevmcd/offspring/remove-ssh-url]

126. By Kevin McDermott on 2012-03-08

And remove from master.

127. By Kevin McDermott on 2012-03-08

No longer need the migration directory.

[Cody A.W. Somerville (cody-somerville)]

You haven't reverted the changes to offspring-build or offspring-web but I'll take care of that when I merge your branch.

review: Approve

=== modified file 'lib/offspring/master/models.py'
--- lib/offspring/master/models.py	2012-02-16 06:16:58 +0000
+++ lib/offspring/master/models.py	2012-03-08 18:59:19 +0000
@@ -105,9 +105,7 @@
         Pass BuildRequest to a slave server.
         """
         status = self.server_proxy.startBuild(request.project.name,
-                                              request.project.config_url,
-                                              request.project.lp_user,
-                                              request.project.lp_ssh_key)
+                                              request.project.config_url)
 
         if status == LexbuilderSlaveServer.REQUEST_OKAY:
             self.previous_state = self.current_state
@@ -177,12 +175,6 @@
     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	2012-02-16 06:16:58 +0000
+++ lib/offspring/master/tests/helpers.py	2012-03-08 18:59:19 +0000
@@ -128,8 +128,6 @@
             "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	2012-01-24 17:44:17 +0000
+++ lib/offspring/slave/slave.py	2012-03-08 18:59:19 +0000
@@ -57,36 +57,13 @@
         self.server.stop()
         self.server.join()
 
-    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):
+    def startBuild(self, projectName, configBranchURL):
         """
         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, lpSSHKey)
+            self, projectName, configBranchURL)
         logging.info("Starting build: %s %s %s" % (
             self.currentBuild.getCommandPath(), projectName, configBranchURL))
         self.currentBuild.start()
@@ -263,32 +240,11 @@
 
 class ProjectBuildProcess(MonitoredProcess):
 
-    def __init__(self, supervisor, projectName, configBranchURL,
-                 lpSSHKey=None):
+    def __init__(self, supervisor, projectName, configBranchURL):
         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
 
@@ -311,7 +267,6 @@
         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")

=== added file 'lib/offspring/slave/tests/helpers.py'
--- lib/offspring/slave/tests/helpers.py	1970-01-01 00:00:00 +0000
+++ lib/offspring/slave/tests/helpers.py	2012-03-08 18:59:19 +0000
@@ -0,0 +1,24 @@
+from ConfigParser import ConfigParser
+
+SLAVE_PORT = 8765
+
+
+class LexbuilderSlaveTestMixin(object):
+    """
+    Sets up a configuration object usable by Slave tests.
+    """
+
+    def setUp(self):
+        super(LexbuilderSlaveTestMixin, self).setUp()
+        self.log_dir = self.makeDir()
+        self.config = ConfigParser()
+        self.config.add_section("slave")
+        self.config.set("slave", "bin_dir", "/usr/bin")
+        self.config.set("slave", "log_dir", self.log_dir)
+        self.config.set("slave", "build_command",
+                        "%(bin_dir)s/offspring-build")
+        self.config.set("slave", "child_wait_timeout", "0")
+        self.config.set("slave", "xmlrpc_server_auth", "admin:password")
+        self.config.set("slave", "request_timeout", "60")
+        self.config.set("slave", "logfile", "%(log_dir)s/offspring-slave-log")
+        self.config.set("slave", "port", str(SLAVE_PORT))

=== modified file 'lib/offspring/slave/tests/test_slave.py'
--- lib/offspring/slave/tests/test_slave.py	2012-01-24 17:44:17 +0000
+++ lib/offspring/slave/tests/test_slave.py	2012-03-08 18:59:19 +0000
@@ -8,11 +8,11 @@
 import subprocess
 import xmlrpclib
 from datetime import datetime
-from ConfigParser import ConfigParser
 
 from mocker import MockerTestCase
 
 from offspring.tests.helpers import CaptureLoggingTestCase
+from offspring.slave.tests.helpers import LexbuilderSlaveTestMixin
 
 from offspring.enums import ProjectBuildStates
 from offspring.slave import (
@@ -68,27 +68,6 @@
         self.tidied = True
 
 
-class LexbuilderSlaveTestMixin(object):
-    """
-    Sets up a configuration object usable by Slave tests.
-    """
-
-    def setUp(self):
-        super(LexbuilderSlaveTestMixin, self).setUp()
-        self.log_dir = self.makeDir()
-        self.config = ConfigParser()
-        self.config.add_section("slave")
-        self.config.set("slave", "bin_dir", "/usr/bin")
-        self.config.set("slave", "log_dir", self.log_dir)
-        self.config.set("slave", "build_command",
-                        "%(bin_dir)s/offspring-build")
-        self.config.set("slave", "child_wait_timeout", "0")
-        self.config.set("slave", "xmlrpc_server_auth", "admin:password")
-        self.config.set("slave", "request_timeout", "60")
-        self.config.set("slave", "logfile", "%(log_dir)s/offspring-slave-log")
-        self.config.set("slave", "port", 8765)
-
-
 class FakeProjectBuildProcess(object):
     state = ProjectBuildProcess.STATE_ACTIVE
 
@@ -202,145 +181,6 @@
         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."""

=== removed directory 'migration'
=== removed file 'migration/002-bzr-ssh-user-key.sql'
--- migration/002-bzr-ssh-user-key.sql	2011-11-30 14:20:32 +0000
+++ migration/002-bzr-ssh-user-key.sql	1970-01-01 00:00:00 +0000
@@ -1,2 +0,0 @@
-ALTER TABLE projects ADD COLUMN "lp_user" text;
-ALTER TABLE projects ADD COLUMN "lp_ssh_key" text;