From f8932702774a7d1c62c02a34f91fda200c9ddb24 Mon Sep 17 00:00:00 2001
From: Jeff Ohrstrom <johrstrom@osc.edu>
Date: Tue, 17 Dec 2024 10:40:53 -0500
Subject: [PATCH] determin inactive users and clean their PUNs too (#3942)

Determine inactive users and clean their PUNs too. This sends a SIGTERM to the PUN
becuase stopping it through nginx will fail if the user has already been deleted.

It then gathers a list of directories which will be empty soon, and rmdir() them
after sending the SIGTERM signal to all invalid user PUNs. This will result
in the least sleeping, and will avoid using `rm_rf()` as root, which seems
very risky.

---------

Co-authored-by: Simon Westersund <85734631+CSC-swesters@users.noreply.github.com>
---
 nginx_stage/lib/nginx_stage.rb                | 15 ++++++++
 .../generators/nginx_clean_generator.rb       | 35 +++++++++++++++++++
 spec/e2e/nginx_stage_spec.rb                  |  7 +++-
 3 files changed, 56 insertions(+), 1 deletion(-)

diff --git a/nginx_stage/lib/nginx_stage.rb b/nginx_stage/lib/nginx_stage.rb
index f5cb71ea59..a74ecf0edb 100644
--- a/nginx_stage/lib/nginx_stage.rb
+++ b/nginx_stage/lib/nginx_stage.rb
@@ -192,6 +192,21 @@ def self.active_users
     end.compact
   end
 
+  # List of inactive users.
+  # @return [Array<String>] the list of inactive users.
+  def self.inactive_users
+    Dir[pun_pid_path(user: '*')].map do |v|
+      name = v[/#{pun_pid_path(user: '(.+)')}/, 1]
+      User.new(name)
+
+      # return nil here becasue we actually _want_ to rescue.
+      # i.e., a User is inactive if it _can't_ be instanitated.
+      nil
+    rescue ArgumentError => e
+      name
+    end.compact
+  end
+
   # Get a hash of all the staged app configs
   # @example List of all staged app configs
   #   staged_apps
diff --git a/nginx_stage/lib/nginx_stage/generators/nginx_clean_generator.rb b/nginx_stage/lib/nginx_stage/generators/nginx_clean_generator.rb
index 60c2ea67ee..63a812ae76 100644
--- a/nginx_stage/lib/nginx_stage/generators/nginx_clean_generator.rb
+++ b/nginx_stage/lib/nginx_stage/generators/nginx_clean_generator.rb
@@ -79,6 +79,41 @@ class NginxCleanGenerator < Generator
           $stderr.puts "#{$!.to_s}"
         end
       end
+
+      pid_parent_dirs_to_remove_later = []
+      NginxStage.inactive_users.each do |u|
+        begin
+          puts "#{u} (disabled)"
+          pid_path = PidFile.new NginxStage.pun_pid_path(user: u)
+
+          # Send a SIGTERM to the master nginx process to kill the PUN.
+          # 'nginx stop' won't work, since getpwnam(3) will cause an error.
+          `kill -s TERM #{pid_path.pid}`
+          FileUtils.rm(NginxStage.pun_secret_key_base_path(user: u).to_s)
+          FileUtils.rm(NginxStage.pun_config_path(user: u).to_s)
+          pid_path_parent_dir = Pathname.new(pid_path.to_s).parent
+          pid_parent_dirs_to_remove_later.push(pid_path_parent_dir)
+        rescue StandardError => e
+          warn "Error trying to clean up disabled user #{u}: #{e.message}"
+        end
+      end
+
+      # Remove the PID path parent directories now that the nginx processes have
+      # had time to clean up their Passenger PID file and socket.
+      pid_parent_dirs_to_remove_later.each do |dir|
+        begin
+          begin
+            FileUtils.rmdir(dir)
+          rescue Errno::ENOTEMPTY
+            # Wait for a short time, while Nginx cleans up its PID file.
+            sleep(0.5)
+            # Then try again once.
+            FileUtils.rmdir(dir)
+          end
+        rescue StandardError => e
+          warn "Error trying to clean up the PID file directory '#{dir}' of disabled user: #{e.message}"
+        end
+      end
     end
 
     def cleanup_stale_files(pid_path, socket)
diff --git a/spec/e2e/nginx_stage_spec.rb b/spec/e2e/nginx_stage_spec.rb
index 4a5f029fc2..dc644ab807 100644
--- a/spec/e2e/nginx_stage_spec.rb
+++ b/spec/e2e/nginx_stage_spec.rb
@@ -17,6 +17,8 @@ def browser
       on hosts, 'mkdir /var/run/ondemand-nginx/deleted_user'
       on hosts, 'chmod 600 /var/run/ondemand-nginx/deleted_user'
       on hosts, 'echo -n 11111111 > /var/run/ondemand-nginx/deleted_user/passenger.pid'
+      on hosts, 'echo -n 11111111 > /var/lib/ondemand-nginx/config/puns/deleted_user.conf'
+      on hosts, 'echo -n 11111111 > /var/lib/ondemand-nginx/config/puns/deleted_user.secret_key_base.txt'
     end
 
     after(:all) do
@@ -31,7 +33,10 @@ def browser
 
       # Note there's no error here about 'deleted_user'
       on hosts, '/opt/ood/nginx_stage/sbin/nginx_stage nginx_clean --force' do
-        assert_equal stdout, "ood\n"
+        assert_equal stdout, "ood\ndeleted_user (disabled)\n"
+        refute(File.exists?('/var/run/ondemand-nginx/deleted_user'))
+        refute(File.exists?('/var/lib/ondemand-nginx/config/puns/deleted_user.conf'))
+        refute(File.exists?('/var/lib/ondemand-nginx/config/puns/deleted_user.secret_key_base.txt'))
       end
     end
   end