From e226ba8c340acd9eb6f5e887d9958205ba9fb221 Mon Sep 17 00:00:00 2001
From: Mason Malone <mason@masonm.org>
Date: Wed, 21 Oct 2015 13:03:20 -0400
Subject: [PATCH] Fix ordering issue with conf_file and ports_file

The httpd.conf.erb template explicitly includes the $ports_file, but the resource that uses that
template doesn't have a dependency on $ports_file. This means it's possible for a declaration of the
apache::custom_config resource to get run between when $conf_file is written and $ports_file is
written. This will cause syntax verification via "apachectl -t" to always fail, causing the
custom_config to be removed if the $verify_config flag is set to "true". Example:

    ==> ops: Notice: /Stage[main]/Jci_nagios::Server/Apache::Custom_config[cgid]/Exec[service notify for cgid]/returns: httpd: Syntax error on line 37 of /etc/httpd/conf/httpd.conf: Could not open configuration file /etc/httpd/conf/ports.conf: No such file or directory
    ==> ops: Error: /Stage[main]/Jci_nagios::Server/Apache::Custom_config[cgid]/Exec[service notify for cgid]: Failed to call refresh: /usr/sbin/apachectl -t returned 1 instead of one of [0]
    ==> ops: Error: /Stage[main]/Jci_nagios::Server/Apache::Custom_config[cgid]/Exec[service notify for cgid]: /usr/sbin/apachectl -t returned 1 instead of one of [0]
    ==> ops: Notice: /Stage[main]/Jci_nagios::Server/Apache::Custom_config[cgid]/Exec[remove cgid if invalid]: Triggered 'refresh' from 1 events

I wrote a test that reproduces this behavior by using ordering arrows to force apache::custom_config
to run before $ports_file is written. This is rather artificial, but I wasn't able to get this is
happen "naturally" in the test environment. Take my word for it that it's possible.
---
 manifests/init.pp                     |  2 +-
 spec/acceptance/custom_config_spec.rb | 21 +++++++++++++++++++++
 spec/classes/apache_spec.rb           |  2 +-
 3 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/manifests/init.pp b/manifests/init.pp
index d894e324f..6aaa57cac 100644
--- a/manifests/init.pp
+++ b/manifests/init.pp
@@ -327,7 +327,7 @@
       ensure  => file,
       content => template($conf_template),
       notify  => Class['Apache::Service'],
-      require => Package['httpd'],
+      require => [Package['httpd'], File[$ports_file]],
     }
 
     # preserve back-wards compatibility to the times when default_mods was
diff --git a/spec/acceptance/custom_config_spec.rb b/spec/acceptance/custom_config_spec.rb
index 8b59f703f..4bfd02f8d 100644
--- a/spec/acceptance/custom_config_spec.rb
+++ b/spec/acceptance/custom_config_spec.rb
@@ -52,4 +52,25 @@ class { 'apache': }
       it { is_expected.to be_file }
     end
   end
+
+  describe 'custom_config only applied after configs are written' do
+    it 'applies in the right order' do
+      pp = <<-EOS
+        class { 'apache': }
+
+        apache::custom_config { 'ordering_test':
+          content => '# just a comment',
+        }
+
+        # Try to wedge the apache::custom_config call between when httpd.conf is written and
+        # ports.conf is written. This should trigger a dependency cycle
+        File["#{$conf_file}"] -> Apache::Custom_config['ordering_test'] -> File["#{$ports_file}"]
+      EOS
+      expect(apply_manifest(pp, :expect_failures => true).stderr).to match(/Failed to apply catalog: Found 1 dependency cycle/i)
+    end
+
+    describe file("#{$confd_dir}/25-ordering_test.conf") do
+      it { is_expected.not_to be_file }
+    end
+  end
 end
diff --git a/spec/classes/apache_spec.rb b/spec/classes/apache_spec.rb
index 0f4d0c5ba..60c03495e 100644
--- a/spec/classes/apache_spec.rb
+++ b/spec/classes/apache_spec.rb
@@ -480,7 +480,7 @@
       it { is_expected.to contain_file("/opt/rh/root/etc/httpd/conf/httpd.conf").with(
         'ensure'  => 'file',
         'notify'  => 'Class[Apache::Service]',
-        'require' => 'Package[httpd]'
+        'require' => ['Package[httpd]', 'File[/etc/httpd/conf/ports.conf]'],
       ) }
     end