launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #01865
[Merge] lp:~jelmer/launchpad/672476 into lp:launchpad/devel
Jelmer Vernooij has proposed merging lp:~jelmer/launchpad/672476 into lp:launchpad/devel.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
At the moment a private PPA may be published before .htaccess/.htpasswd file is created for it since the publisher and the generate_ppa_htaccess script are two different processes.
This branch fixes that race condition by always making the publisher create a .htaccess file first when it publishes a private PPA.
--
https://code.launchpad.net/~jelmer/launchpad/672476/+merge/40349
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jelmer/launchpad/672476 into lp:launchpad/devel.
=== added file 'lib/lp/archivepublisher/htaccess.py'
--- lib/lp/archivepublisher/htaccess.py 1970-01-01 00:00:00 +0000
+++ lib/lp/archivepublisher/htaccess.py 2010-11-08 16:59:48 +0000
@@ -0,0 +1,90 @@
+#!/usr/bin/python
+#
+# Copyright 2010 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+# pylint: disable-msg=C0103,W0403
+
+"""Writing of htaccess and htpasswd files."""
+
+__metaclass__ = type
+
+__all__ = [
+ 'htpasswd_credentials_for_archive',
+ 'write_htaccess',
+ 'write_htpasswd',
+ ]
+
+
+import crypt
+import os
+
+from operator import attrgetter
+from zope.component import getUtility
+
+from lp.soyuz.interfaces.archiveauthtoken import IArchiveAuthTokenSet
+
+HTACCESS_TEMPLATE = """
+AuthType Basic
+AuthName "Token Required"
+AuthUserFile %(path)s/.htpasswd
+Require valid-user
+"""
+
+BUILDD_USER_NAME = "buildd"
+
+
+def write_htaccess(htaccess_filename, distroot):
+ """Write a htaccess file for a private archive.
+
+ :param htaccess_filename: Filename of the htaccess file.
+ :param distroot: Archive root path
+ """
+ interpolations = {"path": distroot}
+ file = open(htaccess_filename, "w")
+ try:
+ file.write(HTACCESS_TEMPLATE % interpolations)
+ finally:
+ file.close()
+
+
+def write_htpasswd(filename, users):
+ """Write out a new htpasswd file.
+
+ :param filename: The file to create.
+ :param users: Iterabel over (user, password, salt) tuples.
+ """
+ if os.path.isfile(filename):
+ os.remove(filename)
+
+ file = open(filename, "a")
+ try:
+ for entry in users:
+ user, password, salt = entry
+ encrypted = crypt.crypt(password, salt)
+ file.write("%s:%s\n" % (user, encrypted))
+ finally:
+ file.close()
+
+
+def htpasswd_credentials_for_archive(archive, tokens=None):
+ """Return credentials for an archive for use with write_htpasswd.
+
+ :param archive: An `IArchive` (must be private)
+ :param tokens: Optional iterable of `IArchiveAuthToken`s.
+ :return: Iterable of tuples with (user, password, salt) for use with
+ write_htpasswd.
+ """
+ assert archive.private, "Archive %r must be private" % archive
+
+ if tokens is None:
+ tokens = getUtility(IArchiveAuthTokenSet).getByArchive(archive)
+
+ # The first .htpasswd entry is the buildd_secret.
+ yield (BUILDD_USER_NAME, archive.buildd_secret, BUILDD_USER_NAME[:2])
+
+ # Iterate over tokens and write the appropriate htpasswd
+ # entries for them. Use a consistent sort order so that the
+ # generated file can be compared to an existing one later.
+ for token in sorted(tokens, key=attrgetter("id")):
+ yield (token.person.name, token.token, token.person.name[:2])
=== modified file 'lib/lp/archivepublisher/publishing.py'
--- lib/lp/archivepublisher/publishing.py 2010-10-17 09:04:48 +0000
+++ lib/lp/archivepublisher/publishing.py 2010-11-08 16:59:48 +0000
@@ -27,6 +27,11 @@
from lp.archivepublisher.diskpool import DiskPool
from lp.archivepublisher.domination import Dominator
from lp.archivepublisher.ftparchive import FTPArchiveHandler
+from lp.archivepublisher.htaccess import (
+ htpasswd_credentials_for_archive,
+ write_htaccess,
+ write_htpasswd,
+ )
from lp.archivepublisher.interfaces.archivesigningkey import (
IArchiveSigningKey,
)
@@ -84,6 +89,27 @@
return dp
+def _setupHtaccess(archive, pubconf, log):
+ """Setup .htaccess/.htpasswd files for an archive.
+ """
+ if not archive.private:
+ # FIXME: JRV 20101108 leftover .htaccess and .htpasswd files
+ # should be removed when support for making existing 3PA's public
+ # is added; bug=376072
+ return
+
+ htaccess_path = os.path.join(pubconf.htaccessroot, ".htaccess")
+ htpasswd_path = os.path.join(pubconf.htaccessroot, ".htpasswd")
+ # After the initial htaccess/htpasswd files
+ # are created generate_ppa_htaccess is responsible for
+ # updating the tokens.
+ if not os.path.exists(htaccess_path):
+ log.debug("Writing htaccess file.")
+ write_htaccess(htaccess_path, pubconf.htaccessroot)
+ passwords = htpasswd_credentials_for_archive(archive)
+ write_htpasswd(htpasswd_path, passwords)
+
+
def getPublisher(archive, allowed_suites, log, distsroot=None):
"""Return an initialised Publisher instance for the given context.
@@ -104,6 +130,8 @@
disk_pool = _getDiskPool(pubconf, log)
+ _setupHtaccess(archive, pubconf, log)
+
if distsroot is not None:
log.debug("Overriding dists root with %s." % distsroot)
pubconf.distsroot = distsroot
=== modified file 'lib/lp/archivepublisher/scripts/generate_ppa_htaccess.py'
--- lib/lp/archivepublisher/scripts/generate_ppa_htaccess.py 2010-11-03 12:07:17 +0000
+++ lib/lp/archivepublisher/scripts/generate_ppa_htaccess.py 2010-11-08 16:59:48 +0000
@@ -5,14 +5,12 @@
# pylint: disable-msg=C0103,W0403
-import crypt
import filecmp
import os
import pytz
import tempfile
from datetime import datetime, timedelta
-from operator import attrgetter
from zope.component import getUtility
from canonical.config import config
@@ -28,6 +26,11 @@
from lp.services.mail.mailwrapper import MailWrapper
from lp.services.scripts.base import LaunchpadCronScript
from lp.services.scripts.interfaces.scriptactivity import IScriptActivitySet
+from lp.archivepublisher.htaccess import (
+ write_htaccess,
+ write_htpasswd,
+ htpasswd_credentials_for_archive,
+ )
from lp.soyuz.enums import ArchiveStatus
from lp.soyuz.enums import ArchiveSubscriberStatus
from lp.soyuz.model.archiveauthtoken import ArchiveAuthToken
@@ -39,15 +42,6 @@
'ubuntuone': ['ppa'],
}
-HTACCESS_TEMPLATE = """
-AuthType Basic
-AuthName "Token Required"
-AuthUserFile %(path)s/.htpasswd
-Require valid-user
-"""
-
-BUILDD_USER_NAME = "buildd"
-
class HtaccessTokenGenerator(LaunchpadCronScript):
"""Helper class for generating .htaccess files for private PPAs."""
@@ -65,23 +59,6 @@
dest="no_deactivation", default=False,
help="If set, tokens are not deactivated.")
- def writeHtpasswd(self, filename, list_of_users):
- """Write out a new htpasswd file.
-
- :param filename: The file to create.
- :param list_of_users: A list of (user, password, salt) tuples.
- """
- if os.path.isfile(filename):
- os.remove(filename)
-
- file = open(filename, "a")
- for entry in list_of_users:
- user, password, salt = entry
- encrypted = crypt.crypt(password, salt)
- file.write("%s:%s\n" % (user, encrypted))
-
- file.close()
-
def ensureHtaccess(self, ppa):
"""Generate a .htaccess for `ppa`."""
if self.options.dryrun:
@@ -95,10 +72,7 @@
# It's not there, so create it.
if not os.path.exists(pub_config.htaccessroot):
os.makedirs(pub_config.htaccessroot)
- interpolations = {"path": pub_config.htaccessroot}
- file = open(htaccess_filename, "w")
- file.write(HTACCESS_TEMPLATE % interpolations)
- file.close()
+ write_htaccess(htaccess_filename, pub_config.htaccessroot)
self.logger.debug("Created .htaccess for %s" % ppa.displayname)
def generateHtpasswd(self, ppa, tokens):
@@ -113,18 +87,8 @@
fd, temp_filename = tempfile.mkstemp(dir=pub_config.htaccessroot)
os.close(fd)
- # The first .htpasswd entry is the buildd_secret.
- list_of_users = [
- (BUILDD_USER_NAME, ppa.buildd_secret, BUILDD_USER_NAME[:2])]
-
- # Iterate over tokens and write the appropriate htpasswd
- # entries for them. Use a consistent sort order so that the
- # generated file can be compared to an existing one later.
- for token in sorted(tokens, key=attrgetter("id")):
- entry = (token.person.name, token.token, token.person.name[:2])
- list_of_users.append(entry)
-
- self.writeHtpasswd(temp_filename, list_of_users)
+ write_htpasswd(temp_filename,
+ htpasswd_credentials_for_archive(ppa, tokens))
return temp_filename
=== added file 'lib/lp/archivepublisher/tests/test_htaccess.py'
--- lib/lp/archivepublisher/tests/test_htaccess.py 1970-01-01 00:00:00 +0000
+++ lib/lp/archivepublisher/tests/test_htaccess.py 2010-11-08 16:59:48 +0000
@@ -0,0 +1,122 @@
+# Copyright 2009-2010 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Test htaccess/htpasswd file generation. """
+
+import os
+import tempfile
+
+from zope.component import getUtility
+
+from canonical.testing.layers import LaunchpadZopelessLayer
+from lp.archivepublisher.htaccess import (
+ htpasswd_credentials_for_archive,
+ write_htaccess,
+ write_htpasswd,
+ )
+from lp.registry.interfaces.distribution import IDistributionSet
+from lp.registry.interfaces.person import IPersonSet
+from lp.testing import TestCaseWithFactory
+
+
+class TestHtpasswdGeneration(TestCaseWithFactory):
+ """Test htpasswd generation."""
+
+ layer = LaunchpadZopelessLayer
+
+ def setUp(self):
+ super(TestHtpasswdGeneration, self).setUp()
+ self.owner = self.factory.makePerson(
+ name="joe", displayname="Joe Smith")
+ self.ppa = self.factory.makeArchive(
+ owner=self.owner, name="myppa", private=True)
+
+ # "Ubuntu" doesn't have a proper publisher config but Ubuntutest
+ # does, so override the PPA's distro here.
+ ubuntutest = getUtility(IDistributionSet)['ubuntutest']
+ self.ppa.distribution = ubuntutest
+
+ def test_write_htpasswd(self):
+ """Test that writing the .htpasswd file works properly."""
+ fd, filename = tempfile.mkstemp()
+ os.close(fd)
+
+ TEST_PASSWORD = "password"
+ TEST_PASSWORD2 = "passwor2"
+
+ # We provide a constant salt to the crypt function so that we
+ # can test the encrypted result.
+ SALT = "XX"
+
+ user1 = ("user", TEST_PASSWORD, SALT)
+ user2 = ("user2", TEST_PASSWORD2, SALT)
+ list_of_users = [user1]
+ list_of_users.append(user2)
+
+ write_htpasswd(filename, list_of_users)
+
+ expected_contents = [
+ "user:XXq2wKiyI43A2",
+ "user2:XXaQB8b5Gtwi.",
+ ]
+
+ file = open(filename, "r")
+ file_contents = file.read().splitlines()
+ file.close()
+ os.remove(filename)
+
+ self.assertEqual(expected_contents, file_contents)
+
+ def test_write_htaccess(self):
+ # write_access can write a correct htaccess file.
+ fd, filename = tempfile.mkstemp()
+ os.close(fd)
+
+ write_htaccess(filename, "/some/distroot")
+ self.assertTrue(
+ os.path.isfile(filename),
+ "%s is not present when it should be" % filename)
+ self.addCleanup(os.remove, filename)
+
+ contents = [
+ "",
+ "AuthType Basic",
+ "AuthName \"Token Required\"",
+ "AuthUserFile /some/distroot/.htpasswd",
+ "Require valid-user",
+ ]
+
+ file = open(filename, "r")
+ file_contents = file.read().splitlines()
+ file.close()
+
+ self.assertEqual(contents, file_contents)
+
+ def test_credentials_for_archive_empty(self):
+ # If there are no ArchiveAuthTokens for an archive just
+ # the buildd secret is returned.
+ self.ppa.buildd_secret = "sekr1t"
+ self.assertEquals([("buildd", "sekr1t", "bu")],
+ list(htpasswd_credentials_for_archive(self.ppa)))
+
+ def test_credentials_for_archive(self):
+ # ArchiveAuthTokens for an archive are returned by
+ # credentials_for_archive.
+ # Make some subscriptions and tokens.
+ self.ppa.buildd_secret = "geheim"
+ name12 = getUtility(IPersonSet).getByName("name12")
+ name16 = getUtility(IPersonSet).getByName("name16")
+ self.ppa.newSubscription(name12, self.ppa.owner)
+ self.ppa.newSubscription(name16, self.ppa.owner)
+ tokens = []
+ tokens.append(self.ppa.newAuthToken(name12))
+ tokens.append(self.ppa.newAuthToken(name16))
+
+ credentials = list(htpasswd_credentials_for_archive(self.ppa, tokens))
+
+ self.assertContentEqual(
+ credentials, [
+ ("buildd", "geheim", "bu"),
+ ("name12", tokens[0].token, "na"),
+ ("name16", tokens[1].token, "na")
+ ])
=== modified file 'lib/lp/archivepublisher/tests/test_publisher.py'
--- lib/lp/archivepublisher/tests/test_publisher.py 2010-10-17 04:13:24 +0000
+++ lib/lp/archivepublisher/tests/test_publisher.py 2010-11-08 16:59:48 +0000
@@ -7,6 +7,7 @@
import bz2
+import crypt
import gzip
import os
import shutil
@@ -395,11 +396,9 @@
# Stub parameters.
allowed_suites = [
('breezy-autotest', PackagePublishingPocket.RELEASE)]
- distsroot = None
distro_publisher = getPublisher(
- self.ubuntutest.main_archive, allowed_suites, self.logger,
- distsroot)
+ self.ubuntutest.main_archive, allowed_suites, self.logger)
# check the publisher context, pointing to the 'main_archive'
self.assertEqual(
@@ -416,7 +415,7 @@
partner_archive = getUtility(IArchiveSet).getByDistroPurpose(
self.ubuntutest, ArchivePurpose.PARTNER)
distro_publisher = getPublisher(
- partner_archive, allowed_suites, self.logger, distsroot)
+ partner_archive, allowed_suites, self.logger)
self.assertEqual(partner_archive, distro_publisher.archive)
self.assertEqual('/var/tmp/archive/ubuntutest-partner/dists',
distro_publisher._config.distsroot)
@@ -660,10 +659,8 @@
work on the deleted publications.
"""
allowed_suites = []
- distsroot = None
publisher = getPublisher(
- self.ubuntutest.main_archive, allowed_suites, self.logger,
- distsroot)
+ self.ubuntutest.main_archive, allowed_suites, self.logger)
publisher.A2_markPocketsWithDeletionsDirty()
self.checkDirtyPockets(publisher, expected=[])
@@ -735,10 +732,8 @@
('breezy-autotest', PackagePublishingPocket.SECURITY),
('breezy-autotest', PackagePublishingPocket.UPDATES),
]
- distsroot = None
publisher = getPublisher(
- self.ubuntutest.main_archive, allowed_suites, self.logger,
- distsroot)
+ self.ubuntutest.main_archive, allowed_suites, self.logger)
publisher.A2_markPocketsWithDeletionsDirty()
self.checkDirtyPockets(publisher, expected=[])
@@ -1040,6 +1035,48 @@
# The Label: field should be set to the archive displayname
self.assertEqual(release_contents[1], 'Label: Partner archive')
+ def testHtaccessForPrivatePPA(self):
+ # A htaccess file is created for new private PPA's.
+
+ ppa = self.factory.makeArchive(
+ distribution=self.ubuntutest, private=True)
+ ppa.buildd_secret = "geheim"
+
+ # Setup the publisher for it and publish its repository.
+ archive_publisher = getPublisher(ppa, [], self.logger)
+ pubconf = getPubConfig(ppa)
+ htaccess_path = os.path.join(pubconf.htaccessroot, ".htaccess")
+ self.assertTrue(os.path.exists(htaccess_path))
+ htaccess_f = open(htaccess_path, 'r')
+ try:
+ self.assertEquals(htaccess_f.read(), """
+AuthType Basic
+AuthName "Token Required"
+AuthUserFile %s/.htpasswd
+Require valid-user
+""" % pubconf.htaccessroot)
+ finally:
+ htaccess_f.close()
+
+ htpasswd_path = os.path.join(pubconf.htaccessroot, ".htpasswd")
+
+ # Read it back in.
+ htpasswd_f = open(htpasswd_path, "r")
+ try:
+ file_contents = htpasswd_f.readlines()
+ finally:
+ htpasswd_f.close()
+
+ self.assertEquals(1, len(file_contents))
+
+ # The first line should be the buildd_secret.
+ [user, password] = file_contents[0].strip().split(":", 1)
+ self.assertEqual(user, "buildd")
+ # We can re-encrypt the buildd_secret and it should match the
+ # one in the .htpasswd file.
+ encrypted_secret = crypt.crypt(ppa.buildd_secret, password)
+ self.assertEqual(encrypted_secret, password)
+
class TestArchiveIndices(TestPublisherBase):
"""Tests for the native publisher's index generation.
