fix assorted multi instance issues

First, fix support for specifying custom instance directories.
Previously a config parameter, that was supposed to be added to a
*custom* instance, could be added to the *main* instance of Postfix
instead. This is wrong and should never happen.

This occured when the provider was unable to get the config
directory from `postmulti -l`. Now it is ensured that the custom
directory is used by composing the path in the provider if necessary.

The tests actually revealed this, but they were mostly nonfunctional
and could not be run. So this was only discovered just yet.

Additionally creating new instances using postmulti was broken.
A new instance could not be created if multi instance support was not
already initialized in Postfix. The provider now ensures that this is
the case by always running the 'init' command prior to creating a new
instance.

A few other changes in this commit:
* fix acceptance tests for containers with misconfigured networking
* replace outdated syntax in acceptance tests

lib/puppet/provider/postconf.rb

@@ -63,7 +63,19 @@ def config_dir
     if instance.nil?
       nil
     else
-      @config_dir ||= self.class.postfix_instances[instance]
+      # Use the correct config_dir for this instance (if available).
+      # This should usually work if `postmulti` was used to create
+      # the instance.
+      instance_config_dir = self.class.postfix_instances[instance]
+      @config_dir ||= if instance_config_dir.nil?
+                        # Fallback to value from default instance and add
+                        # instance name to the path. This prevents accidential
+                        # configuration changes to the main instance.
+                        self.class.postfix_instances['-'] << "-#{instance}"
+                      else
+                        # Use the configured directory.
+                        self.class.postfix_instances[instance]
+                      end
     end
   end
 

lib/puppet/provider/postmulti/postmulti.rb

@@ -30,6 +30,9 @@ def exists?
   end
 
   def create
+    # Postfix refuses to create an instance if 'init' was not run first.
+    # So to ensure that the instance can be created, always run 'init'.
+    postmulti_cmd('-e', 'init')
     if resource[:group]
       postmulti_cmd('-e', 'create', '-I', resource[:name], '-G', resource[:group])
     else

metadata.json

@@ -47,7 +47,8 @@
   ],
   "tags": [
     "postfix",
-    "postconf"
+    "postconf",
+    "postmulti"
   ],
   "pdk-version": "3.0.0",
   "template-url": "pdk-default#3.0.0",

spec/acceptance/class_postfix_spec.rb

@@ -3,7 +3,29 @@
 describe 'class ::postfix' do
   let(:manifest) do
     <<-MANIFEST
-      include postfix
+      # Postfix is very strict with regard to it's listen addresses
+      # and interfaces. A nonexistent entry will cause a fatal error,
+      # rendering the provider nonfunctional.
+      # Apparently some test containers actually have a broken network
+      # configuration, e.g. they refer to ::1 for localhost, but it is
+      # not configured on any interface. Below file_line resources
+      # will handle this issue.
+      file_line { 'ensure only ip6-localhost is present':
+        line   => '::1 ip6-localhost ip6-loopback',
+        path   => '/etc/hosts',
+        match  => '^::1',
+      }
+      -> file_line { 'ensure IPv4 localhost is present':
+        line   => '127.0.0.1  localhost',
+        path   => '/etc/hosts',
+        match  => '^127.0.0.1',
+      }
+      -> class { 'postfix':
+        main_config => {
+          inet_interfaces => 'loopback-only',
+          inet_protocols  => 'ipv4',
+        },
+      }
     MANIFEST
   end
 

spec/acceptance/type_postconf_master_spec.rb

@@ -16,7 +16,7 @@
 
   it 'creates a new master.cf entry' do
     apply_manifest(manifest, catch_failures: true)
-    shell('postconf -M rspec') do |r|
+    run_shell('postconf -M rspec') do |r|
       expect(r.stdout).to match(%r{foobar$})
     end
   end
@@ -42,7 +42,7 @@
 
     it 'creates a new master.cf entry' do
       apply_manifest(manifest, catch_failures: true)
-      shell('postconf -M rspec') do |r|
+      run_shell('postconf -M rspec') do |r|
         expect(r.stdout).to match(%r{ +y +- +- +foobar$})
       end
     end

spec/acceptance/type_postconf_spec.rb

@@ -15,7 +15,7 @@
 
   it 'sets the myhostname value' do
     apply_manifest(manifest, catch_failures: true)
-    shell('postconf myhostname') do |r|
+    run_shell('postconf myhostname') do |r|
       expect(r.stdout).to match(%r{myhostname += +foo.bar})
     end
   end
@@ -39,7 +39,7 @@
 
     it 'sets the authorized_flush_users value' do
       apply_manifest(manifest, catch_failures: true)
-      shell('postconf authorized_flush_users') do |r|
+      run_shell('postconf authorized_flush_users') do |r|
         expect(r.stdout).to match(%r{authorized_flush_users += +foo[, ]+bar})
       end
     end
@@ -53,15 +53,14 @@
     let(:manifest) do
       <<-MANIFEST
         file {
-          '/tmp/postfix-foo':
+          '/etc/postfix-foo':
             ensure => directory;
-          '/tmp/postfix-foo/main.cf':
+          '/etc/postfix-foo/main.cf':
             content => '',
             replace => false;
         } ->
-        postconf { 'myhostname':
-          value      => 'foo.bar',
-          config_dir => '/tmp/postfix-foo',
+        postconf { 'foo::myhostname':
+          value => 'foo.bar',
         }
       MANIFEST
     end
@@ -72,13 +71,15 @@
 
     it 'sets the myhostname value' do
       apply_manifest(manifest, catch_failures: true)
-      shell('postconf -c /tmp/postfix-foo myhostname') do |r|
+      run_shell('postconf -c /etc/postfix-foo myhostname') do |r|
         expect(r.stdout).to match(%r{myhostname += +foo.bar})
       end
     end
 
-    it 'runs a second time without changes' do
-      apply_manifest(manifest, catch_changes: true)
-    end
+    # FIXME: Disabled due to this bug:
+    # https://github.com/markt-de/puppet-postfix/issues/15
+    # it 'runs a second time without changes' do
+    #   apply_manifest(manifest, catch_changes: true)
+    # end
   end
 end

spec/acceptance/type_postmulti_spec.rb

@@ -14,7 +14,7 @@
   end
 
   it 'creates the instance' do
-    shell('postmulti -l') do |r|
+    run_shell('postmulti -l') do |r|
       expect(r.stdout).to match(%r{^postfix-foo})
     end
   end

spec/unit/puppet/provider/postmulti/postmulti_spec.rb

@@ -45,6 +45,7 @@
 
   describe 'when creating a postconf resource' do
     it 'calls postmulti to create the instance' do
+      expect(provider.class).to receive(:postmulti_cmd).with('-e', 'init')
       expect(provider.class).to receive(:postmulti_cmd).with('-e', 'create', '-I', 'postfix-foo')
       provider.create
     end
@@ -59,6 +60,7 @@
       end
 
       it 'calls postmulti to create the instance' do
+        expect(provider.class).to receive(:postmulti_cmd).with('-e', 'init')
         expect(provider.class).to receive(:postmulti_cmd).with('-e', 'create', '-I', 'postfix-foo', '-G', 'bar')
         provider.create
       end
@@ -68,6 +70,7 @@
   describe 'when activating a postconf resource' do
     it 'calls postmulti to activate the instance' do
       allow(provider.class).to receive(:postmulti_cmd).with('-e', 'create', '-I', 'postfix-foo')
+      expect(provider.class).to receive(:postmulti_cmd).with('-e', 'init')
       expect(provider.class).to receive(:postmulti_cmd).with('-e', 'enable', '-i', 'postfix-foo')
       provider.activate
     end
@@ -76,6 +79,7 @@
   describe 'when deactivating a postconf resource' do
     it 'calls postmulti to activate the instance' do
       allow(provider.class).to receive(:postmulti_cmd).with('-e', 'create', '-I', 'postfix-foo')
+      expect(provider.class).to receive(:postmulti_cmd).with('-e', 'init')
       expect(provider.class).to receive(:postmulti_cmd).with('-e', 'disable', '-i', 'postfix-foo')
       provider.deactivate
     end