Merge pull request #184 from trlinkin/refactor_vhost_logformat

Refactor vhost logformat

README.md

@@ -279,6 +279,14 @@ Specifies whether `*_access.log` directives should be configured. Valid values a
 
 Points to the `*_access.log` file. Defaults to 'undef'.
 
+#####`access_log_pipe`
+
+Specifies a pipe to send access log messages to. Defaults to 'undef'.
+
+#####`access_log_format`
+
+Specifies either a LogFormat nickname or custom format string for access log. Defaults to 'undef'.
+
 #####`add_listen`
 
 Determines whether the vhost creates a listen statement. The default value is 'true'. 
@@ -321,6 +329,10 @@ Specifies whether `*_error.log` directives should be configured. Defaults to 'tr
 
 Points to the `*_error.log` file. Defaults to 'undef'.
 
+#####`error_log_pipe`
+
+Specifies a pipe to send error log messages to. Defaults to 'undef'.
+
 #####`ensure`
 
 Specifies if the vhost file is present or absent.

manifests/vhost.pp

@@ -83,8 +83,11 @@
     $logroot            = "/var/log/${apache::params::apache_name}",
     $access_log         = true,
     $access_log_file    = undef,
+    $access_log_pipe    = undef,
+    $access_log_format  = undef,
     $error_log          = true,
     $error_log_file     = undef,
+    $error_log_pipe     = undef,
     $scriptalias        = undef,
     $proxy_dest         = undef,
     $no_proxy_uris      = [],
@@ -117,6 +120,14 @@
   validate_bool($ssl)
   validate_bool($default_vhost)
 
+  if $access_log_file and $access_log_pipe {
+    fail("Apache::Vhost[${name}]: 'access_log_file' and 'access_log_pipe' cannot be defined at the same time")
+  }
+
+  if $error_log_file and $error_log_pipe {
+    fail("Apache::Vhost[${name}]: 'error_log_file' and 'error_log_pipe' cannot be defined at the same time")
+  }
+
   if $ssl {
     include apache::mod::ssl
   }
@@ -148,25 +159,38 @@
   }
 
   # Define log file names
-  if ! $access_log_file {
+  if $access_log_file {
+    $access_log_destination = "${logroot}/${access_log_file}"
+  } elsif $access_log_pipe {
+    $access_log_destination = "\"${access_log_pipe}\""
+  } else {
     if $ssl {
-      $access_log_file_real = "${servername_real}_access_ssl.log"
+      $access_log_destination = "${logroot}/${servername_real}_access_ssl.log"
     } else {
-      $access_log_file_real = "${servername_real}_access.log"
+      $access_log_destination = "${logroot}/${servername_real}_access.log"
     }
-  } else {
-    $access_log_file_real = $access_log_file
   }
-  if ! $error_log_file {
+
+  if $error_log_file {
+    $error_log_destination = "${logroot}/${error_log_file}"
+  } elsif $error_log_pipe {
+    $error_log_destination = "\"${error_log_pipe}\""
+  } else {
     if $ssl {
-      $error_log_file_real = "${servername_real}_error_ssl.log"
+      $error_log_destination = "${logroot}/${servername_real}_error_ssl.log"
     } else {
-      $error_log_file_real = "${servername_real}_error.log"
+      $error_log_destination = "${logroot}/${servername_real}_error.log"
     }
+  }
+
+  # Set access log format
+  if $access_log_format {
+    $_access_log_format = "\"${access_log_format}\""
   } else {
-    $error_log_file_real = $error_log_file
+    $_access_log_format = 'combined'
   }
 
+
   if $ip {
     if $port {
       $listen_addr_port = "${ip}:${port}"
@@ -271,9 +295,10 @@
   # - $logroot
   # - $name
   # - $access_log
-  # - $access_log_file_real
+  # - $access_log_destination
+  # - $_access_log_format
   # - $error_log
-  # - $error_log_file_real
+  # - $error_log_destination
   # - $custom_fragment
   # block fragment:
   #   - $block

spec/defines/vhost_spec.rb

@@ -134,6 +134,24 @@
           :value => '/fake/log',
           :match => [/CustomLog \/fake\/log\//,/ErrorLog \/fake\/log\//],
         },
+        {
+          :title => 'should accept pipe destination for access log',
+          :attr  => 'access_log_pipe',
+          :value => '| /bin/fake/logging',
+          :match => /CustomLog "| \/bin\/fake\/logging" combined$/,
+        },
+        {
+          :title => 'should accept pipe destination for error log',
+          :attr  => 'error_log_pipe',
+          :value => '| /bin/fake/logging',
+          :match => /ErrorLog "| \/bin\/fake\/logging" combined$/,
+        },
+        {
+          :title => 'should accept custom format for access logs',
+          :attr  => 'access_log_format',
+          :value => '%h %{X-Forwarded-For}i %l %u %t \"%r\" %s %b  \"%{Referer}i\" \"%{User-agent}i\" \"Host: %{Host}i\" %T %D',
+          :match => /CustomLog \/var\/log\/.+_access\.log "%h %\{X-Forwarded-For\}i %l %u %t \\"%r\\" %s %b  \\"%\{Referer\}i\\" \\"%\{User-agent\}i\\" \\"Host: %\{Host\}i\\" %T %D"$/,
+        },
         {
           :title => 'should contain access logs',
           :attr  => 'access_log',
@@ -214,6 +232,24 @@
     end
 
     context 'attribute resources' do
+      describe 'when access_log_file and access_log_pipe are specified' do
+        let :params do default_params.merge({
+          :access_log_file => 'fake.log',
+          :access_log_pipe => '| /bin/fake',
+        }) end
+        it 'should cause a failure' do
+          expect {should raise_error(Puppet::Error, 'Apache::Vhost[${name}]: \'access_log_file\' and \'access_log_pipe\' cannot be defined at the same time') }
+        end
+      end
+      describe 'when error_log_file and error_log_pipe are specified' do
+        let :params do default_params.merge({
+          :error_log_file => 'fake.log',
+          :error_log_pipe => '| /bin/fake',
+        }) end
+        it 'should cause a failure' do
+          expect { should raise_error(Puppet::Error, 'Apache::Vhost[${name}]: \'error_log_file\' and \'error_log_pipe\' cannot be defined at the same time') }
+        end
+      end
       describe 'when docroot owner is specified' do
         let :params do default_params.merge({
           :docroot_owner => 'testuser',

templates/vhost.conf.erb

@@ -20,12 +20,12 @@
 
   ## Logging
 <% if @error_log -%>
-  ErrorLog <%= @logroot %>/<%= @error_log_file_real %>
+  ErrorLog <%= @error_log_destination %>
 <% end -%>
   LogLevel warn
   ServerSignature Off
 <% if @access_log -%>
-  CustomLog <%= @logroot %>/<%= @access_log_file_real %> combined
+  CustomLog <%= @access_log_destination %> <%= @_access_log_format %>
 <% end -%>
 <%= scope.function_template(['apache/vhost/_block.erb']) -%>
 <%= scope.function_template(['apache/vhost/_proxy.erb']) -%>