Merge pull request #1096 from igalic/log_reject

Do not offload overriding LogFormats to httpd

Author: Morgan Haskel

README.md

@@ -357,7 +357,7 @@ Specifies the location where apache module files are stored. It should not be co
 
 #####`loadfile_name`
 
-Sets the file name for the module loadfile. Should be in the format *.load.  This can be used to set the module load order.
+Sets the file name for the module loadfile. Should be in the format \*.load.  This can be used to set the module load order.
 
 #####`log_level`
 
@@ -371,6 +371,17 @@ Define additional [LogFormats](https://httpd.apache.org/docs/current/mod/mod_log
   $log_formats = { vhost_common => '%v %h %l %u %t \"%r\" %>s %b' }
 ```
 
+There are a number of predefined LogFormats in the httpd.conf that Puppet writes out:
+
+```httpd
+LogFormat "%h %l %u %t \"%r\" %>s %b \"%{Referer}i\" \"%{User-Agent}i\"" combined
+LogFormat "%h %l %u %t \"%r\" %>s %b" common
+LogFormat "%{Referer}i -> %U" referer
+LogFormat "%{User-agent}i" agent
+```
+
+If your `$log_formats` contains one of those, they will be overwritten with **your** definition.
+
 #####`logroot`
 
 Changes the directory where Apache log files for the virtual host are placed. Defaults to '/var/log/httpd' on RedHat, '/var/log/apache2' on Debian, '/var/log/apache22' on FreeBSD, and '/var/log/apache2' on Gentoo.

spec/classes/apache_spec.rb

@@ -218,6 +218,23 @@
       end
     end
 
+    describe "Override existing LogFormats" do
+      context "When parameter log_formats is a hash" do
+        let :params do
+          { :log_formats => {
+            'common'   => "%v %h %l %u %t \"%r\" %>s %b",
+            'combined' => "%v %h %l %u %t \"%r\" %>s %b \"%{Referer}i\" \"%{User-agent}i\""
+          } }
+        end
+
+        it { is_expected.to contain_file("/etc/apache2/apache2.conf").with_content %r{^LogFormat "%v %h %l %u %t \"%r\" %>s %b" common\n} }
+        it { is_expected.to contain_file("/etc/apache2/apache2.conf").without_content %r{^LogFormat "%h %l %u %t \"%r\" %>s %b \"%\{Referer\}i\" \"%\{User-agent\}i\"" combined\n} }
+        it { is_expected.to contain_file("/etc/apache2/apache2.conf").with_content %r{^LogFormat "%v %h %l %u %t \"%r\" %>s %b" common\n} }
+        it { is_expected.to contain_file("/etc/apache2/apache2.conf").with_content %r{^LogFormat "%v %h %l %u %t \"%r\" %>s %b \"%\{Referer\}i\" \"%\{User-agent\}i\"" combined\n} }
+        it { is_expected.to contain_file("/etc/apache2/apache2.conf").without_content %r{^LogFormat "%h %l %u %t \"%r\" %>s %b \"%\{Referer\}i\" \"%\{User-agent\}i\"" combined\n} }
+      end
+    end
+
     context "on Ubuntu" do
       let :facts do
         super().merge({

templates/httpd.conf.erb

@@ -60,10 +60,18 @@ Include "<%= @mod_load_dir %>/*.conf"
 <% end -%>
 Include "<%= @ports_file %>"
 
+<% unless @log_formats.has_key?('combined') -%>
 LogFormat "%h %l %u %t \"%r\" %>s %b \"%{Referer}i\" \"%{User-Agent}i\"" combined
+<% end -%>
+<% unless @log_formats.has_key?('common') -%>
 LogFormat "%h %l %u %t \"%r\" %>s %b" common
+<% end -%>
+<% unless @log_formats.has_key?('referer') -%>
 LogFormat "%{Referer}i -> %U" referer
+<% end -%>
+<% unless @log_formats.has_key?('agent') -%>
 LogFormat "%{User-agent}i" agent
+<% end -%>
 <% if @log_formats and !@log_formats.empty? -%>
   <%- @log_formats.sort.each do |nickname,format| -%>
 LogFormat "<%= format -%>" <%= nickname %>