From 6c286232100ed3497796cded4ec15db26958acec Mon Sep 17 00:00:00 2001
From: Gordon Bleux <33967640+UiP9AV6Y@users.noreply.github.com>
Date: Mon, 29 May 2023 16:00:14 +0200
Subject: [PATCH] fpm: manage runtime- and log directory based on params

instead of hardcoding the paths for the log- and runtime directory,
calculate them based on the error_log and pid_file params respectively.
the directories will not be managed if they are shared system locations
like `/var/log` or `/run` (this is checked by ensuring the directories
have *php* in their name, signalling that those are paths dedicated
to PHP)
---
 REFERENCE.md                        |  7 +++-
 manifests/fpm/config.pp             | 20 ++++++---
 spec/classes/php_fpm_config_spec.rb | 64 +++++++++++++++++++++++++++++
 3 files changed, 84 insertions(+), 7 deletions(-)

diff --git a/REFERENCE.md b/REFERENCE.md
index 3d3e618a..f99425aa 100644
--- a/REFERENCE.md
+++ b/REFERENCE.md
@@ -1119,6 +1119,9 @@ Configure php-fpm service
 [*error_log*]
   Path to error log file. If it's set to "syslog", log is
   sent to syslogd instead of being written in a local file.
+  The base directory will be managed if it is a directory
+  dedicated to PHP (i.e. has "php" in its name and is not
+  a shared location like /var/log)
 
 [*log_level*]
   The php-fpm log level
@@ -1160,7 +1163,9 @@ Configure php-fpm service
   UNIX group of the root user
 
 [*pid_file*]
-  Path to fpm pid file
+  Path to fpm pid file. The base directory will be managed if it is
+  a directory dedicated to PHP (i.e. has "php" in its name and is not
+  a shared location like /var/run)
 
 [*manage_run_dir*]
   Manage the run directory
diff --git a/manifests/fpm/config.pp b/manifests/fpm/config.pp
index 3ac787f0..7d9a58c0 100644
--- a/manifests/fpm/config.pp
+++ b/manifests/fpm/config.pp
@@ -27,6 +27,9 @@
 # [*error_log*]
 #   Path to error log file. If it's set to "syslog", log is
 #   sent to syslogd instead of being written in a local file.
+#   The base directory will be managed if it is a directory
+#   dedicated to PHP (i.e. has "php" in its name and is not
+#   a shared location like /var/log)
 #
 # [*log_level*]
 #   The php-fpm log level
@@ -68,7 +71,9 @@
 #   UNIX group of the root user
 #
 # [*pid_file*]
-#   Path to fpm pid file
+#   Path to fpm pid file. The base directory will be managed if it is
+#   a directory dedicated to PHP (i.e. has "php" in its name and is not
+#   a shared location like /var/run)
 #
 # [*manage_run_dir*]
 #   Manage the run directory
@@ -100,6 +105,9 @@
 ) inherits php::params {
   assert_private()
 
+  $pid_dir = dirname($pid_file)
+  $log_dir = dirname($error_log)
+
   file { $config_file:
     ensure  => file,
     content => template('php/fpm/php-fpm.conf.erb'),
@@ -108,8 +116,8 @@
     mode    => '0644',
   }
 
-  if $manage_run_dir {
-    file { '/var/run/php-fpm':
+  if $manage_run_dir and 'php' in $pid_dir {
+    file { $pid_dir:
       ensure => directory,
       owner  => 'root',
       group  => $root_group,
@@ -117,14 +125,14 @@
     }
   }
 
-  ensure_resource('file', '/var/log/php-fpm/',
-    {
+  if $error_log != 'syslog' and 'php' in $log_dir {
+    file { $log_dir:
       ensure => directory,
       owner  => 'root',
       group  => $root_group,
       mode   => $log_dir_mode,
     }
-  )
+  }
 
   file { $pool_base_dir:
     ensure => directory,
diff --git a/spec/classes/php_fpm_config_spec.rb b/spec/classes/php_fpm_config_spec.rb
index 9f0a7b13..94aa7837 100644
--- a/spec/classes/php_fpm_config_spec.rb
+++ b/spec/classes/php_fpm_config_spec.rb
@@ -44,6 +44,70 @@
           )
         end
       end
+
+      describe 'manages a log directory' do
+        context 'with dedicated path' do
+          let(:params) do
+            {
+              error_log: '/var/log/php/fpm.log',
+            }
+          end
+
+          it do
+            is_expected.to contain_file('/var/log/php')
+          end
+        end
+
+        context 'without dedicated path' do
+          let(:params) do
+            {
+              error_log: '/var/log/php-fpm.log',
+            }
+          end
+
+          it do
+            is_expected.not_to contain_file('/var/log')
+          end
+        end
+
+        context 'without syslog logging' do
+          let(:params) do
+            {
+              error_log: 'syslog',
+            }
+          end
+
+          it do
+            is_expected.not_to contain_file('syslog')
+          end
+        end
+      end
+
+      describe 'manages a runtime directory' do
+        context 'with dedicated path' do
+          let(:params) do
+            {
+              pid_file: '/var/run/php/fpm.pid',
+            }
+          end
+
+          it do
+            is_expected.to contain_file('/var/run/php')
+          end
+        end
+
+        context 'without dedicated path' do
+          let(:params) do
+            {
+              pid_file: '/var/run/fpm.pid',
+            }
+          end
+
+          it do
+            is_expected.not_to contain_file('/var/run')
+          end
+        end
+      end
     end
   end
 end