7 files changed, 223 insertions, 212 deletions

diff --git a/src/mailman/commands/cli_aliases.py b/src/mailman/commands/cli_aliases.py
index d692ba356..d26e5a754 100644
--- a/src/mailman/commands/cli_aliases.py
+++ b/src/mailman/commands/cli_aliases.py
@@ -25,18 +25,11 @@ __all__ = [
     ]
 
 
-import sys
-
-from operator import attrgetter
-from zope.component import getUtility
 from zope.interface import implementer
 
 from mailman.config import config
 from mailman.core.i18n import _
 from mailman.interfaces.command import ICLISubCommand
-from mailman.interfaces.listmanager import IListManager
-from mailman.interfaces.mta import (
-    IMailTransportAgentAliases, IMailTransportAgentLifecycle)
 from mailman.utilities.modules import call_name
 
 
@@ -51,84 +44,11 @@ class Aliases:
         """See `ICLISubCommand`."""
         self.parser = parser
         command_parser.add_argument(
-            '-o', '--output',
-            action='store', help=_("""\
-            File to send the output to.  If not given, a file in $VAR/data is
-            used.  The argument can be '-' to use standard output.."""))
-        command_parser.add_argument(
-            '-f', '--format',
+            '-d', '--directory',
             action='store', help=_("""\
-            Alternative output format to use.  This is the Python object path
-            to an implementation of the `IMailTransportAgentLifecycle`
-            interface."""))
-        command_parser.add_argument(
-            '-s', '--simple',
-            action='store_true', default=False, help=_("""\
-            Simply output the list of aliases.
-            """))
+            An alternative directory to output the various MTA files to."""))
 
     def process(self, args):
         """See `ICLISubCommand`."""
-        if args.format is not None and args.simple:
-            self.parser.error(_('Cannot use both -s and -f'))
-            # Does not return.
-        output = None
-        if args.output == '-':
-            output = sys.stdout
-        elif args.output is None:
-            output = None
-        else:
-            output = args.output
-        if args.simple:
-            Dummy().regenerate(output)
-        else:
-            format_arg = (config.mta.incoming
-                          if args.format is None
-                          else args.format)
-            # Call the MTA-specific regeneration method.
-            call_name(format_arg).regenerate(output)
-
-
-
-@implementer(IMailTransportAgentLifecycle)
-class Dummy:
-    """Dummy aliases implementation for simpler output format."""
-
-    def create(self, mlist):
-        """See `IMailTransportAgentLifecycle`."""
-        raise NotImplementedError
-
-    def delete(self, mlist):
-        """See `IMailTransportAgentLifecycle`."""
-        raise NotImplementedError
-
-    def regenerate(self, output=None):
-        """See `IMailTransportAgentLifecycle`."""
-        fp = None
-        close = False
-        try:
-            if output is None:
-                # There's really no place to print the output.
-                return
-            elif isinstance(output, basestring):
-                fp = open(output, 'w')
-                close = True
-            else:
-                fp = output
-            self._do_write_file(fp)
-        finally:
-            if fp is not None and close:
-                fp.close()
-
-    def _do_write_file(self, fp):
-        # First, sort mailing lists by domain.
-        by_domain = {}
-        for mlist in getUtility(IListManager).mailing_lists:
-            by_domain.setdefault(mlist.mail_host, []).append(mlist)
-        sort_key = attrgetter('list_name')
-        for domain in sorted(by_domain):
-            for mlist in sorted(by_domain[domain], key=sort_key):
-                utility = getUtility(IMailTransportAgentAliases)
-                for alias in utility.aliases(mlist):
-                    print(alias, file=fp)
-                print(file=fp)
+        # Call the MTA-specific regeneration method.
+        call_name(config.mta.incoming).regenerate(args.directory)
diff --git a/src/mailman/commands/docs/aliases.rst b/src/mailman/commands/docs/aliases.rst
index c0d4c10c9..89843c8ad 100644
--- a/src/mailman/commands/docs/aliases.rst
+++ b/src/mailman/commands/docs/aliases.rst
@@ -2,16 +2,14 @@
 Generating aliases
 ==================
 
-For some mail servers, Mailman must generate a data file that is used to hook
+For some mail servers, Mailman must generate data files that are used to hook
 Mailman up to the mail server.  The details of this differ for each mail
 server.  Generally these files are automatically kept up-to-date when mailing
 lists are created or removed, but you might occasionally need to manually
 regenerate the file.  The ``bin/mailman aliases`` command does this.
 
     >>> class FakeArgs:
-    ...     output = None
-    ...     format = None
-    ...     simple = None
+    ...     directory = None
     >>> from mailman.commands.cli_aliases import Aliases
     >>> command = Aliases()
 
@@ -32,12 +30,29 @@ generation.
     ... """)
 
 Let's create a mailing list and then display the transport map for it.  We'll
-send the output to stdout.
+write the appropriate files to a temporary directory.
 ::
 
-    >>> FakeArgs.output = '-'
+    >>> import os, shutil, tempfile
+    >>> output_directory = tempfile.mkdtemp()
+    >>> cleanups.append((shutil.rmtree, output_directory))
+
+    >>> FakeArgs.directory = output_directory
     >>> mlist = create_list('test@example.com')
     >>> command.process(FakeArgs)
+
+For Postfix, there are two files in the output directory.
+
+    >>> files = sorted(os.listdir(output_directory))
+    >>> for file in files:
+    ...     print file
+    postfix_domains
+    postfix_lmtp
+
+The transport map file contains all the aliases for the mailing list.
+
+    >>> with open(os.path.join(output_directory, 'postfix_lmtp')) as fp:
+    ...     print fp.read()
     # AUTOMATICALLY GENERATED BY MAILMAN ON ...
     ...
     test@example.com               lmtp:[lmtp.example.com]:24
@@ -51,61 +66,14 @@ send the output to stdout.
     test-unsubscribe@example.com   lmtp:[lmtp.example.com]:24
     <BLANKLINE>
 
-    >>> config.pop('postfix')
-
+The relay domains file contains a list of all the domains.
 
-Alternative output
-==================
-
-By using a command line switch, we can select a different output format.  The
-option must point to an alternative implementation of the
-``IMailTransportAgentAliases`` interface.
-
-Mailman comes with an alternative implementation that just prints the aliases,
-with no adornment.
-
-    >>> FakeArgs.format = 'mailman.commands.cli_aliases.Dummy'
-    >>> command.process(FakeArgs)
-    test@example.com
-    test-bounces@example.com
-    test-confirm@example.com
-    test-join@example.com
-    test-leave@example.com
-    test-owner@example.com
-    test-request@example.com
-    test-subscribe@example.com
-    test-unsubscribe@example.com
-    <BLANKLINE>
-
-A simpler way of getting the same output is with the ``--simple`` flag.
-
-    >>> FakeArgs.format = None
-    >>> FakeArgs.simple = True
-    >>> command.process(FakeArgs)
-    test@example.com
-    test-bounces@example.com
-    test-confirm@example.com
-    test-join@example.com
-    test-leave@example.com
-    test-owner@example.com
-    test-request@example.com
-    test-subscribe@example.com
-    test-unsubscribe@example.com
-    <BLANKLINE>
-
-
-Mutually exclusive arguments
-============================
-
-You cannot use both ``--simple`` and ``--format``.
-
-    >>> FakeArgs.format = 'mailman.commands.cli_aliases.Dummy'
-    >>> FakeArgs.simple = True
-    >>> class Parser:
-    ...     def error(self, message):
-    ...         raise RuntimeError(message)
-    >>> command.parser = Parser()
-    >>> command.process(FakeArgs)
-    Traceback (most recent call last):
+    >>> with open(os.path.join(output_directory, 'postfix_domains')) as fp:
+    ...     print fp.read()
+    # AUTOMATICALLY GENERATED BY MAILMAN ON ...
     ...
-    RuntimeError: Cannot use both -s and -f
+    example.com example.com
+
+..
+    Clean up.
+    >>> config.pop('postfix')
diff --git a/src/mailman/docs/MTA.rst b/src/mailman/docs/MTA.rst
index c6d2230c4..ccfd18c0e 100644
--- a/src/mailman/docs/MTA.rst
+++ b/src/mailman/docs/MTA.rst
@@ -93,7 +93,7 @@ Transport maps
 
 By default, Mailman works well with Postfix transport maps as a way to deliver
 incoming messages to Mailman's LMTP server.  Mailman will automatically write
-the correct transport map when its `bin/mailman genaliases` command is run, or
+the correct transport map when its `bin/mailman aliases` command is run, or
 whenever a mailing list is created or removed via other commands.  To connect
 Postfix to Mailman's LMTP server, add the following to Postfix's `main.cf`
 file::
@@ -102,17 +102,33 @@ file::
         hash:/path-to-mailman/var/data/postfix_lmtp
     local_recipient_maps =
         hash:/path-to-mailman/var/data/postfix_lmtp
+    relay_domains =
+        hash:/path-to-mailman/var/data/postfix_domains
 
 where `path-to-mailman` is replaced with the actual path that you're running
 Mailman from.  Setting `local_recipient_maps` as well as `transport_maps`
 allows Postfix to properly reject all messages destined for non-existent local
-users.
+users.  Setting `relay_domains`_ means Postfix will start to accept mail for
+newly added domains even if they are not part of `mydestination`_.
 
+Note that if you are not using virtual domains, then `relay_domains`_ isn't
+strictly needed (but it is harmless).  All you need to do in this scenario is
+to make sure that Postfix accepts mail for your one domain, normally by
+including it in `mydestination`.
 
-Virtual domains
----------------
 
-TBD: figure out how virtual domains interact with the transport maps.
+Postfix documentation
+---------------------
+
+For more information regarding how to configure Postfix, please see
+the Postfix documentation at:
+
+.. _`The official Postfix documentation`:
+   http://www.postfix.org/documentation.html
+.. _`The reference page for all Postfix configuration parameters`:
+   http://www.postfix.org/postconf.5.html
+.. _`relay_domains`: http://www.postfix.org/postconf.5.html#relay_domains
+.. _`mydestination`: http://www.postfix.org/postconf.5.html#mydestination
 
 
 Sendmail
diff --git a/src/mailman/docs/NEWS.rst b/src/mailman/docs/NEWS.rst
index 8fec45957..367c6c53a 100644
--- a/src/mailman/docs/NEWS.rst
+++ b/src/mailman/docs/NEWS.rst
@@ -29,6 +29,17 @@ REST
    unverify an address more than once, but verifying an already verified
    address does not change its `.verified_on` date.  (LP: #1054730)
 
+Integration
+-----------
+ * Added support for Postfix `relay_domains` setting for better virtual domain
+   support.  Contributed by Jimmy Bergman.
+
+Commands
+--------
+ * `bin/mailman aliases` loses the `--output`, `--format`, and `--simple`
+   arguments, and adds a `--directory` argument.  This is necessary to support
+   the Postfix `relay_domains` support.
+
 
 3.0 beta 2 -- "Freeze"
 ======================
diff --git a/src/mailman/interfaces/mta.py b/src/mailman/interfaces/mta.py
index 34c210edd..303a8e42a 100644
--- a/src/mailman/interfaces/mta.py
+++ b/src/mailman/interfaces/mta.py
@@ -84,12 +84,12 @@ class IMailTransportAgentLifecycle(Interface):
     def delete(mlist):
         """Tell the MTA that the mailing list was deleted."""
 
-    def regenerate(output=None):
+    def regenerate(directory=None):
         """Regenerate the full aliases file.
 
-        :param output: The file name or file object to send the output to.  If
-            not given or None, and MTA specific file is used.
-        :type output: string, file object, None
+        :param directory: The directory to write the MTA specific support
+            files to.  Defaults to $DATA_DIR.
+        :type directory: string
         """
 
 
diff --git a/src/mailman/mta/postfix.py b/src/mailman/mta/postfix.py
index c04e38f02..072581374 100644
--- a/src/mailman/mta/postfix.py
+++ b/src/mailman/mta/postfix.py
@@ -42,6 +42,7 @@ from mailman.utilities.datetime import now
 
 log = logging.getLogger('mailman.error')
 ALIASTMPL = '{0:{2}}lmtp:[{1.mta.lmtp_host}]:{1.mta.lmtp_port}'
+NL = '\n'
 
 
 
@@ -67,52 +68,49 @@ class LMTP:
 
     delete = create
 
-    def regenerate(self, output=None):
-        """See `IMailTransportAgentLifecycle`.
-
-        The format for Postfix's LMTP transport map is defined here:
-        http://www.postfix.org/transport.5.html
-        """
+    def regenerate(self, directory=None):
+        """See `IMailTransportAgentLifecycle`."""
         # Acquire a lock file to prevent other processes from racing us here.
+        if directory is None:
+            directory = config.DATA_DIR
         lock_file = os.path.join(config.LOCK_DIR, 'mta')
         with Lock(lock_file):
-            # If output is a filename, open up a backing file and write the
-            # output there, then do the atomic rename dance.  First though, if
-            # it's None, we use a calculated path.
-            if output is None:
-                path = os.path.join(config.DATA_DIR, 'postfix_lmtp')
-                path_new = path + '.new'
-            elif isinstance(output, basestring):
-                path = output
-                path_new = output + '.new'
-            else:
-                path = path_new = None
-            if path_new is None:
-                self._do_write_file(output)
-                # There's nothing to rename, and we can't generate the .db
-                # file, so we're done.
-                return
-            # Write the file.
-            with open(path_new, 'w') as fp:
-                self._do_write_file(fp)
+            lmtp_path = os.path.join(directory, 'postfix_lmtp')
+            lmtp_path_new = lmtp_path + '.new'
+            with open(lmtp_path_new, 'w') as fp:
+                self._generate_lmtp_file(fp)
             # Atomically rename to the intended path.
-            os.rename(path + '.new', path)
-            # Now that the new file is in place, we must tell Postfix to
-            # generate a new .db file.
-            command = config.mta.postfix_map_cmd + ' ' + path
-            status = (os.system(command) >> 8) & 0xff
-            if status:
-                msg = 'command failure: %s, %s, %s'
-                errstr = os.strerror(status)
-                log.error(msg, command, status, errstr)
-                raise RuntimeError(msg % (command, status, errstr))
+            os.rename(lmtp_path_new, lmtp_path)
+            domains_path = os.path.join(directory, 'postfix_domains')
+            domains_path_new = domains_path + '.new'
+            with open(domains_path_new, 'w') as fp:
+                self._generate_domains_file(fp)
+            # Atomically rename to the intended path.
+            os.rename(domains_path_new, domains_path)
+            # Now, run the postmap command on both newly generated files.  If
+            # one files, still try the other one.
+            errors = []
+            for path in (lmtp_path, domains_path):
+                command = config.mta.postfix_map_cmd + ' ' + path
+                status = (os.system(command) >> 8) & 0xff
+                if status:
+                    msg = 'command failure: %s, %s, %s'
+                    errstr = os.strerror(status)
+                    log.error(msg, command, status, errstr)
+                    errors.append(msg % (command, status, errstr))
+            if errors:
+                raise RuntimeError(NL.join(errors))
 
-    def _do_write_file(self, fp):
-        """Do the actual file writes for list creation."""
-        # Sort all existing mailing list names first by domain, then by local
-        # part.  For postfix we need a dummy entry for the domain.
+    def _generate_lmtp_file(self, fp):
+        # The format for Postfix's LMTP transport map is defined here:
+        # http://www.postfix.org/transport.5.html
+        #
+        # Sort all existing mailing list names first by domain, then by
+        # local part.  For Postfix we need a dummy entry for the domain.
         list_manager = getUtility(IListManager)
+        utility = getUtility(IMailTransportAgentAliases)
         by_domain = {}
+        sort_key = attrgetter('list_name')
         for list_name, mail_host in list_manager.name_components:
             mlist = _FakeList(list_name, mail_host)
             by_domain.setdefault(mlist.mail_host, []).append(mlist)
@@ -123,17 +121,32 @@ class LMTP:
 # file.  YOU SHOULD NOT MANUALLY EDIT THIS FILE unless you know what you're
 # doing, and can keep the two files properly in sync.  If you screw it up,
 # you're on your own.
-""".format(now().replace(microsecond=0)), file=fp)
-        sort_key = attrgetter('list_name')
+    """.format(now().replace(microsecond=0)), file=fp)
         for domain in sorted(by_domain):
             print("""\
-# Aliases which are visible only in the @{0} domain.""".format(domain),
-                file=fp)
+# Aliases which are visible only in the @{0} domain.""".format(domain), 
+                  file=fp)
             for mlist in sorted(by_domain[domain], key=sort_key):
-                utility = getUtility(IMailTransportAgentAliases)
                 aliases = list(utility.aliases(mlist))
                 width = max(len(alias) for alias in aliases) + 3
                 print(ALIASTMPL.format(aliases.pop(0), config, width), file=fp)
                 for alias in aliases:
                     print(ALIASTMPL.format(alias, config, width), file=fp)
                 print(file=fp)
+
+    def _generate_domains_file(self, fp):
+        # Uniquify the domains, then sort them alphabetically.
+        domains = set()
+        for list_name, mail_host in getUtility(IListManager).name_components:
+            domains.add(mail_host)
+        print("""\
+# AUTOMATICALLY GENERATED BY MAILMAN ON {0}
+#
+# This file is generated by Mailman, and is kept in sync with the binary hash
+# file.  YOU SHOULD NOT MANUALLY EDIT THIS FILE unless you know what you're
+# doing, and can keep the two files properly in sync.  If you screw it up,
+# you're on your own.
+""".format(now().replace(microsecond=0)), file=fp)
+        for domain in sorted(domains):
+            print('{0} {0}'.format(domain), file=fp)
+        print(file=fp)
diff --git a/src/mailman/mta/tests/test_aliases.py b/src/mailman/mta/tests/test_aliases.py
index b1a60bc95..cc6b677b3 100644
--- a/src/mailman/mta/tests/test_aliases.py
+++ b/src/mailman/mta/tests/test_aliases.py
@@ -15,28 +15,40 @@
 # You should have received a copy of the GNU General Public License along with
 # GNU Mailman.  If not, see <http://www.gnu.org/licenses/>.
 
-"""Test the template generating utility."""
+"""Test the MTA file generating utility."""
 
 from __future__ import absolute_import, unicode_literals
 
 __metaclass__ = type
 __all__ = [
+    'TestAliases',
+    'TestPostfix',
     ]
 
 
+import os
+import shutil
+import tempfile
 import unittest
 
-from cStringIO import StringIO
 from zope.component import getUtility
 
 from mailman.app.lifecycle import create_list
+from mailman.interfaces.domain import IDomainManager
 from mailman.interfaces.mta import IMailTransportAgentAliases
 from mailman.mta.postfix import LMTP
+from mailman.testing.helpers import configuration
 from mailman.testing.layers import ConfigLayer
 
+
 NL = '\n'
 
 
+def _strip_header(contents):
+    lines = contents.splitlines()
+    return NL.join(lines[7:])
+
+
 
 class TestAliases(unittest.TestCase):
 
@@ -129,21 +141,35 @@ class TestPostfix(unittest.TestCase):
     layer = ConfigLayer
 
     def setUp(self):
+        self.tempdir = tempfile.mkdtemp()
         self.utility = getUtility(IMailTransportAgentAliases)
         self.mlist = create_list('test@example.com')
-        self.output = StringIO()
         self.postfix = LMTP()
         # Python 2.7 has assertMultiLineEqual.  Let this work without bounds.
         self.maxDiff = None
         self.eq = getattr(self, 'assertMultiLineEqual', self.assertEqual)
 
+    def tearDown(self):
+        shutil.rmtree(self.tempdir)
+
+    @configuration('mta', postfix_map_cmd='true')
     def test_aliases(self):
         # Test the format of the Postfix alias generator.
-        self.postfix.regenerate(self.output)
-        # Strip out the variable and unimportant bits of the output.
-        lines = self.output.getvalue().splitlines()
-        output = NL.join(lines[7:])
-        self.eq(output, """\
+        self.postfix.regenerate(self.tempdir)
+        # There are two files in this directory.
+        self.assertEqual(sorted(os.listdir(self.tempdir)),
+                         ['postfix_domains', 'postfix_lmtp'])
+        # The domains file, just contains the example.com domain.  We have to
+        # ignore the file header.
+        with open(os.path.join(self.tempdir, 'postfix_domains')) as fp:
+            contents = _strip_header(fp.read())
+        self.eq(contents, """\
+example.com example.com
+""")
+        # The lmtp file contains transport mappings to the lmtp server.
+        with open(os.path.join(self.tempdir, 'postfix_lmtp')) as fp:
+            contents = _strip_header(fp.read())
+        self.eq(contents, """\
 # Aliases which are visible only in the @example.com domain.
 test@example.com               lmtp:[127.0.0.1]:9024
 test-bounces@example.com       lmtp:[127.0.0.1]:9024
@@ -156,15 +182,26 @@ test-subscribe@example.com     lmtp:[127.0.0.1]:9024
 test-unsubscribe@example.com   lmtp:[127.0.0.1]:9024
 """)
 
+    @configuration('mta', postfix_map_cmd='true')
     def test_two_lists(self):
         # Both lists need to show up in the aliases file.  LP: #874929.
         # Create a second list.
         create_list('other@example.com')
-        self.postfix.regenerate(self.output)
-        # Strip out the variable and unimportant bits of the output.
-        lines = self.output.getvalue().splitlines()
-        output = NL.join(lines[7:])
-        self.eq(output, """\
+        self.postfix.regenerate(self.tempdir)
+        # There are two files in this directory.
+        self.assertEqual(sorted(os.listdir(self.tempdir)),
+                         ['postfix_domains', 'postfix_lmtp'])
+        # Because both lists are in the same domain, there should be only one
+        # entry in the relays file.
+        with open(os.path.join(self.tempdir, 'postfix_domains')) as fp:
+            contents = _strip_header(fp.read())
+        self.eq(contents, """\
+example.com example.com
+""")
+        # The transport file contains entries for both lists.
+        with open(os.path.join(self.tempdir, 'postfix_lmtp')) as fp:
+            contents = _strip_header(fp.read())
+        self.eq(contents, """\
 # Aliases which are visible only in the @example.com domain.
 other@example.com               lmtp:[127.0.0.1]:9024
 other-bounces@example.com       lmtp:[127.0.0.1]:9024
@@ -186,3 +223,49 @@ test-request@example.com       lmtp:[127.0.0.1]:9024
 test-subscribe@example.com     lmtp:[127.0.0.1]:9024
 test-unsubscribe@example.com   lmtp:[127.0.0.1]:9024
 """)
+
+    @configuration('mta', postfix_map_cmd='true')
+    def test_two_lists_two_domains(self):
+        # Now we have two lists in two different domains.  Both lists will
+        # show up in the postfix_lmtp file, and both domains will show up in
+        # the postfix_domains file.
+        getUtility(IDomainManager).add('example.net')
+        create_list('other@example.net')
+        self.postfix.regenerate(self.tempdir)
+        # There are two files in this directory.
+        self.assertEqual(sorted(os.listdir(self.tempdir)),
+                         ['postfix_domains', 'postfix_lmtp'])
+        # Because both lists are in the same domain, there should be only one
+        # entry in the relays file.
+        with open(os.path.join(self.tempdir, 'postfix_domains')) as fp:
+            contents = _strip_header(fp.read())
+        self.eq(contents, """\
+example.com example.com
+example.net example.net
+""")
+        # The transport file contains entries for both lists.
+        with open(os.path.join(self.tempdir, 'postfix_lmtp')) as fp:
+            contents = _strip_header(fp.read())
+        self.eq(contents, """\
+# Aliases which are visible only in the @example.com domain.
+test@example.com               lmtp:[127.0.0.1]:9024
+test-bounces@example.com       lmtp:[127.0.0.1]:9024
+test-confirm@example.com       lmtp:[127.0.0.1]:9024
+test-join@example.com          lmtp:[127.0.0.1]:9024
+test-leave@example.com         lmtp:[127.0.0.1]:9024
+test-owner@example.com         lmtp:[127.0.0.1]:9024
+test-request@example.com       lmtp:[127.0.0.1]:9024
+test-subscribe@example.com     lmtp:[127.0.0.1]:9024
+test-unsubscribe@example.com   lmtp:[127.0.0.1]:9024
+
+# Aliases which are visible only in the @example.net domain.
+other@example.net               lmtp:[127.0.0.1]:9024
+other-bounces@example.net       lmtp:[127.0.0.1]:9024
+other-confirm@example.net       lmtp:[127.0.0.1]:9024
+other-join@example.net          lmtp:[127.0.0.1]:9024
+other-leave@example.net         lmtp:[127.0.0.1]:9024
+other-owner@example.net         lmtp:[127.0.0.1]:9024
+other-request@example.net       lmtp:[127.0.0.1]:9024
+other-subscribe@example.net     lmtp:[127.0.0.1]:9024
+other-unsubscribe@example.net   lmtp:[127.0.0.1]:9024
+""")