FEATURE: Add SiteSetting for s3_configure_tombstone_policy

Add SiteSetting for s3_configure_tombstone_policy, skip policy generation if turned off (default on)

config/locales/server.en.yml

@@ -1350,6 +1350,7 @@ en:
     s3_backup_bucket: "The remote bucket to hold backups. WARNING: Make sure it is a private bucket."
     s3_endpoint: "The endpoint can be modified to backup to an S3 compatible service like DigitalOcean Spaces or Minio. WARNING: Use default if using AWS S3"
     s3_force_path_style: "Enforce path-style addressing for your custom endpoint. IMPORTANT: Required for using Minio uploads and backups."
+    s3_configure_tombstone_policy: "Enable automatic deletion policy for tombstone uploads. IMPORTANT: If disabled, no space will be reclaimed after uploads are deleted."
     s3_disable_cleanup: "Disable the removal of backups from S3 when removed locally."
     backup_time_of_day: "Time of day UTC when the backup should occur."
     backup_with_uploads: "Include uploads in scheduled backups. Disabling this will only backup the database."

config/site_settings.yml

@@ -964,6 +964,9 @@ files:
     regex: '^https?:\/\/.+[^\/]$'
   s3_force_path_style:
     default: false
+  s3_configure_tombstone_policy:
+    default: true
+    shadowed_by_global: true
   allow_profile_backgrounds:
     client: true
     default: true

lib/s3_helper.rb

@@ -131,6 +131,7 @@ class S3Helper
   end
 
   def update_tombstone_lifecycle(grace_period)
+    return if !SiteSetting.s3_configure_tombstone_policy
     return if @tombstone_prefix.blank?
     update_lifecycle("purge_tombstone", grace_period, prefix: @tombstone_prefix)
   end

spec/components/s3_helper_spec.rb

@@ -2,13 +2,12 @@ require "s3_helper"
 require "rails_helper"
 
 describe "S3Helper" do
+  before(:each) do
+    SiteSetting.enable_s3_uploads = true
+    SiteSetting.s3_access_key_id = "abc"
+    SiteSetting.s3_secret_access_key = "def"
 
-  it "can correctly set the purge policy" do
-
-    stub_request(:get, "http://169.254.169.254/latest/meta-data/iam/security-credentials/").
-      to_return(status: 404, body: "", headers: {})
-
-    lifecycle = <<~XML
+    @lifecycle = <<~XML
       <?xml version="1.0" encoding="UTF-8"?>
       <LifecycleConfiguration xmlns="http://s3.amazonaws.com/doc/2006-03-01/">
         <Rule>
@@ -29,9 +28,16 @@ describe "S3Helper" do
         </Rule>
       </LifecycleConfiguration>
     XML
+  end
+
+  it "can correctly set the purge policy" do
+    SiteSetting.s3_configure_tombstone_policy = true
+
+    stub_request(:get, "http://169.254.169.254/latest/meta-data/iam/security-credentials/").
+      to_return(status: 404, body: "", headers: {})
 
     stub_request(:get, "https://bob.s3.amazonaws.com/?lifecycle").
-      to_return(status: 200, body: lifecycle, headers: {})
+      to_return(status: 200, body: @lifecycle, headers: {})
 
     stub_request(:put, "https://bob.s3.amazonaws.com/?lifecycle").
       with do |req|
@@ -40,14 +46,17 @@ describe "S3Helper" do
       rules = hash["LifecycleConfiguration"]["Rule"]
 
       expect(rules.length).to eq(2)
-
+      expect(rules[1]["Expiration"]["Days"]).to eq("100")
       # fixes the bad filter
       expect(rules[0]["Filter"]["Prefix"]).to eq("projectdocs/")
     end.to_return(status: 200, body: "", headers: {})
 
-    SiteSetting.enable_s3_uploads = true
-    SiteSetting.s3_access_key_id = "abc"
-    SiteSetting.s3_secret_access_key = "def"
+    helper = S3Helper.new('bob', 'tomb')
+    helper.update_tombstone_lifecycle(100)
+  end
+
+  it "can skip policy update when s3_configure_tombstone_policy is false" do
+    SiteSetting.s3_configure_tombstone_policy = false
 
     helper = S3Helper.new('bob', 'tomb')
     helper.update_tombstone_lifecycle(100)