diff --git a/lib/s3_cors_rulesets.rb b/lib/s3_cors_rulesets.rb index d982d18d0f4..7178e1a45f4 100644 --- a/lib/s3_cors_rulesets.rb +++ b/lib/s3_cors_rulesets.rb @@ -26,6 +26,10 @@ class S3CorsRulesets max_age_seconds: 3000 }.freeze + RULE_STATUS_SKIPPED = "rules_skipped_from_settings" + RULE_STATUS_EXISTED = "rules_already_existed" + RULE_STATUS_APPLIED = "rules_applied" + ## # Used by the s3:ensure_cors_rules rake task to make sure the # relevant CORS rules are applied to allow for direct uploads to @@ -38,33 +42,33 @@ class S3CorsRulesets return if !SiteSetting.s3_install_cors_rule return if !(GlobalSetting.use_s3? || SiteSetting.enable_s3_uploads) - assets_rules_applied = false - backup_rules_applied = false - direct_upload_rules_applied = false + assets_rules_status = RULE_STATUS_SKIPPED + backup_rules_status = RULE_STATUS_SKIPPED + direct_upload_rules_status = RULE_STATUS_SKIPPED s3_helper = S3Helper.build_from_config( s3_client: s3_client, use_db_s3_config: use_db_s3_config ) puts "Attempting to apply ASSETS S3 CORS ruleset in bucket #{s3_helper.s3_bucket_name}." - assets_rules_applied = s3_helper.ensure_cors!([S3CorsRulesets::ASSETS]) + assets_rules_status = s3_helper.ensure_cors!([S3CorsRulesets::ASSETS]) ? RULE_STATUS_APPLIED : RULE_STATUS_EXISTED if SiteSetting.enable_backups? && SiteSetting.backup_location == BackupLocationSiteSetting::S3 backup_s3_helper = S3Helper.build_from_config( s3_client: s3_client, use_db_s3_config: use_db_s3_config, for_backup: true ) puts "Attempting to apply BACKUP_DIRECT_UPLOAD S3 CORS ruleset in bucket #{backup_s3_helper.s3_bucket_name}." - backup_rules_applied = backup_s3_helper.ensure_cors!([S3CorsRulesets::BACKUP_DIRECT_UPLOAD]) + backup_rules_status = backup_s3_helper.ensure_cors!([S3CorsRulesets::BACKUP_DIRECT_UPLOAD]) ? RULE_STATUS_APPLIED : RULE_STATUS_EXISTED end if SiteSetting.enable_direct_s3_uploads puts "Attempting to apply DIRECT_UPLOAD S3 CORS ruleset in bucket #{s3_helper.s3_bucket_name}." - direct_upload_rules_applied = s3_helper.ensure_cors!([S3CorsRulesets::DIRECT_UPLOAD]) + direct_upload_rules_status = s3_helper.ensure_cors!([S3CorsRulesets::DIRECT_UPLOAD]) ? RULE_STATUS_APPLIED : RULE_STATUS_EXISTED end { - assets_rules_applied: assets_rules_applied, - backup_rules_applied: backup_rules_applied, - direct_upload_rules_applied: direct_upload_rules_applied + assets_rules_status: assets_rules_status, + backup_rules_status: backup_rules_status, + direct_upload_rules_status: direct_upload_rules_status } end end diff --git a/lib/tasks/s3.rake b/lib/tasks/s3.rake index 65b738d064b..954ba60fb78 100644 --- a/lib/tasks/s3.rake +++ b/lib/tasks/s3.rake @@ -177,21 +177,9 @@ task 's3:ensure_cors_rules' => :environment do puts "Installing CORS rules..." result = S3CorsRulesets.sync(use_db_s3_config: use_db_s3_config) - if result[:assets_rules_applied] - puts "Assets rules did not exist and were applied." - else - puts "Assets rules already existed." - end - if result[:backup_rules_applied] - puts "Backup rules did not exist and were applied." - else - puts "Backup rules already existed." - end - if result[:direct_upload_rules_applied] - puts "Direct upload rules did not exist and were applied." - else - puts "Direct upload rules already existed." - end + puts "Assets rules status: #{result[:assets_rules_status]}." + puts "Backup rules status: #{result[:backup_rules_status]}." + puts "Direct upload rules status: #{result[:direct_upload_rules_status]}." end task 's3:upload_assets' => [:environment, 's3:ensure_cors_rules'] do diff --git a/spec/lib/s3_cors_rulesets_spec.rb b/spec/lib/s3_cors_rulesets_spec.rb index cbcd95551a8..92e280b8e44 100644 --- a/spec/lib/s3_cors_rulesets_spec.rb +++ b/spec/lib/s3_cors_rulesets_spec.rb @@ -40,9 +40,9 @@ RSpec.describe S3CorsRulesets do } ) result = sync_rules - expect(result[:assets_rules_applied]).to eq(true) - expect(result[:direct_upload_rules_applied]).to eq(false) - expect(result[:backup_rules_applied]).to eq(false) + expect(result[:assets_rules_status]).to eq(S3CorsRulesets::RULE_STATUS_APPLIED) + expect(result[:direct_upload_rules_status]).to eq(S3CorsRulesets::RULE_STATUS_SKIPPED) + expect(result[:backup_rules_status]).to eq(S3CorsRulesets::RULE_STATUS_SKIPPED) end it "does not apply the ASSETS rules if they already exist" do @@ -51,9 +51,9 @@ RSpec.describe S3CorsRulesets do }) client.expects(:put_bucket_cors).never result = sync_rules - expect(result[:assets_rules_applied]).to eq(false) - expect(result[:direct_upload_rules_applied]).to eq(false) - expect(result[:backup_rules_applied]).to eq(false) + expect(result[:assets_rules_status]).to eq(S3CorsRulesets::RULE_STATUS_EXISTED) + expect(result[:direct_upload_rules_status]).to eq(S3CorsRulesets::RULE_STATUS_SKIPPED) + expect(result[:backup_rules_status]).to eq(S3CorsRulesets::RULE_STATUS_SKIPPED) end it "applies the ASSETS rules and the BACKUP_DIRECT_UPLOAD rules if S3 backups are enabled" do @@ -77,9 +77,9 @@ RSpec.describe S3CorsRulesets do } ) result = sync_rules - expect(result[:assets_rules_applied]).to eq(true) - expect(result[:direct_upload_rules_applied]).to eq(false) - expect(result[:backup_rules_applied]).to eq(true) + expect(result[:assets_rules_status]).to eq(S3CorsRulesets::RULE_STATUS_APPLIED) + expect(result[:direct_upload_rules_status]).to eq(S3CorsRulesets::RULE_STATUS_SKIPPED) + expect(result[:backup_rules_status]).to eq(S3CorsRulesets::RULE_STATUS_APPLIED) end it "applies the ASSETS rules and the DIRECT_UPLOAD rules when S3 direct uploads are enabled" do @@ -103,9 +103,9 @@ RSpec.describe S3CorsRulesets do } ) result = sync_rules - expect(result[:assets_rules_applied]).to eq(true) - expect(result[:direct_upload_rules_applied]).to eq(true) - expect(result[:backup_rules_applied]).to eq(false) + expect(result[:assets_rules_status]).to eq(S3CorsRulesets::RULE_STATUS_APPLIED) + expect(result[:direct_upload_rules_status]).to eq(S3CorsRulesets::RULE_STATUS_APPLIED) + expect(result[:backup_rules_status]).to eq(S3CorsRulesets::RULE_STATUS_SKIPPED) end it "does no changes if all the rules already exist" do @@ -117,9 +117,9 @@ RSpec.describe S3CorsRulesets do }) client.expects(:put_bucket_cors).never result = sync_rules - expect(result[:assets_rules_applied]).to eq(false) - expect(result[:direct_upload_rules_applied]).to eq(false) - expect(result[:backup_rules_applied]).to eq(false) + expect(result[:assets_rules_status]).to eq(S3CorsRulesets::RULE_STATUS_EXISTED) + expect(result[:direct_upload_rules_status]).to eq(S3CorsRulesets::RULE_STATUS_EXISTED) + expect(result[:backup_rules_status]).to eq(S3CorsRulesets::RULE_STATUS_EXISTED) end def setup_backups @@ -148,9 +148,9 @@ RSpec.describe S3CorsRulesets do } ) result = sync_rules - expect(result[:assets_rules_applied]).to eq(true) - expect(result[:direct_upload_rules_applied]).to eq(false) - expect(result[:backup_rules_applied]).to eq(false) + expect(result[:assets_rules_status]).to eq(S3CorsRulesets::RULE_STATUS_APPLIED) + expect(result[:direct_upload_rules_status]).to eq(S3CorsRulesets::RULE_STATUS_SKIPPED) + expect(result[:backup_rules_status]).to eq(S3CorsRulesets::RULE_STATUS_SKIPPED) end it "does not apply the ASSETS rules if they already exist" do @@ -159,9 +159,9 @@ RSpec.describe S3CorsRulesets do }) client.expects(:put_bucket_cors).never result = sync_rules - expect(result[:assets_rules_applied]).to eq(false) - expect(result[:direct_upload_rules_applied]).to eq(false) - expect(result[:backup_rules_applied]).to eq(false) + expect(result[:assets_rules_status]).to eq(S3CorsRulesets::RULE_STATUS_EXISTED) + expect(result[:direct_upload_rules_status]).to eq(S3CorsRulesets::RULE_STATUS_SKIPPED) + expect(result[:backup_rules_status]).to eq(S3CorsRulesets::RULE_STATUS_SKIPPED) end it "applies the ASSETS rules and the BACKUP_DIRECT_UPLOAD rules if S3 backups are enabled" do @@ -187,9 +187,9 @@ RSpec.describe S3CorsRulesets do } ) result = sync_rules - expect(result[:assets_rules_applied]).to eq(true) - expect(result[:direct_upload_rules_applied]).to eq(false) - expect(result[:backup_rules_applied]).to eq(true) + expect(result[:assets_rules_status]).to eq(S3CorsRulesets::RULE_STATUS_APPLIED) + expect(result[:direct_upload_rules_status]).to eq(S3CorsRulesets::RULE_STATUS_SKIPPED) + expect(result[:backup_rules_status]).to eq(S3CorsRulesets::RULE_STATUS_APPLIED) end it "applies the ASSETS rules and the DIRECT_UPLOAD rules when S3 direct uploads are enabled" do @@ -213,9 +213,9 @@ RSpec.describe S3CorsRulesets do } ) result = sync_rules - expect(result[:assets_rules_applied]).to eq(true) - expect(result[:direct_upload_rules_applied]).to eq(true) - expect(result[:backup_rules_applied]).to eq(false) + expect(result[:assets_rules_status]).to eq(S3CorsRulesets::RULE_STATUS_APPLIED) + expect(result[:direct_upload_rules_status]).to eq(S3CorsRulesets::RULE_STATUS_APPLIED) + expect(result[:backup_rules_status]).to eq(S3CorsRulesets::RULE_STATUS_SKIPPED) end end end