CLDC-2539: configure application for PaaS and AWS (#1822)

* feat: update application to use non paas and paas configuration services, add helper to determine if platform is paas

feat: don't double-instantiate the configuration service

feat: add self. to is_paas? definition, and require module in rack_attack

chore: lint

* test: update suite to work with new paas/env config service logic

test: new PlatformHelper

* fix: update full import tests to check paas and non-paas config services

* fix: update test descriptions and remove unrequired line

---------

Co-authored-by: Sam Seed <sam.seed@softwire.com>

app/helpers/platform_helper.rb

@@ -0,0 +1,5 @@
+module PlatformHelper
+  def self.is_paas?
+    !ENV["VCAP_SERVICES"].nil?
+  end
+end

app/jobs/data_export_xml_job.rb

@@ -2,7 +2,7 @@ class DataExportXmlJob < ApplicationJob
   queue_as :default
 
   def perform(full_update: false)
-    storage_service = Storage::S3Service.new(Configuration::PaasConfigurationService.new, ENV["EXPORT_PAAS_INSTANCE"])
+    storage_service = Storage::S3Service.new(PlatformHelper.is_paas? ? Configuration::PaasConfigurationService.new : Configuration::EnvConfigurationService.new, ENV["EXPORT_PAAS_INSTANCE"])
     export_service = Exports::LettingsLogExportService.new(storage_service)
 
     export_service.export_xml_lettings_logs(full_update:)

app/models/forms/bulk_upload_lettings/upload_your_file.rb

@@ -57,7 +57,7 @@ module Forms
       def storage_service
         @storage_service ||= if upload_enabled?
                                Storage::S3Service.new(
-                                 Configuration::PaasConfigurationService.new,
+                                 PlatformHelper.is_paas? ? Configuration::PaasConfigurationService.new : Configuration::EnvConfigurationService.new,
                                  ENV["CSV_DOWNLOAD_PAAS_INSTANCE"],
                                )
                              else

app/models/forms/bulk_upload_sales/upload_your_file.rb

@@ -50,7 +50,7 @@ module Forms
       def storage_service
         @storage_service ||= if FeatureToggle.upload_enabled?
                                Storage::S3Service.new(
-                                 Configuration::PaasConfigurationService.new,
+                                 PlatformHelper.is_paas? ? Configuration::PaasConfigurationService.new : Configuration::EnvConfigurationService.new,
                                  ENV["CSV_DOWNLOAD_PAAS_INSTANCE"],
                                )
                              else

app/services/bulk_upload/downloader.rb

@@ -38,7 +38,7 @@ private
 
   def s3_storage_service
     Storage::S3Service.new(
-      Configuration::PaasConfigurationService.new,
+      PlatformHelper.is_paas? ? Configuration::PaasConfigurationService.new : Configuration::EnvConfigurationService.new,
       ENV["CSV_DOWNLOAD_PAAS_INSTANCE"],
     )
   end

config/initializers/rack_attack.rb

@@ -1,14 +1,18 @@
 require "configuration/configuration_service"
 require "configuration/paas_configuration_service"
+require "configuration/env_configuration_service"
+require Rails.root.join("app/helpers/platform_helper")
+
+configuration_service = PlatformHelper.is_paas? ? Configuration::PaasConfigurationService.new : Configuration::EnvConfigurationService.new
 
 if Rails.env.development? || Rails.env.test?
   Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new
   Rack::Attack.enabled = false
 elsif Rails.env.review?
-  redis_url = Configuration::PaasConfigurationService.new.redis_uris.to_a[0][1]
+  redis_url = configuration_service.redis_uris.to_a[0][1]
   Rack::Attack.cache.store = ActiveSupport::Cache::RedisCacheStore.new(url: redis_url)
 else
-  redis_url = Configuration::PaasConfigurationService.new.redis_uris[:"dluhc-core-#{Rails.env}-redis"]
+  redis_url = configuration_service.redis_uris[:"dluhc-core-#{Rails.env}-redis"]
   Rack::Attack.cache.store = ActiveSupport::Cache::RedisCacheStore.new(url: redis_url)
 end
 

config/initializers/sidekiq.rb

@@ -1,8 +1,10 @@
 require "sidekiq/web"
 require "sidekiq/cron/web"
 
+configuration_service = PlatformHelper.is_paas? ? Configuration::PaasConfigurationService.new : Configuration::EnvConfigurationService.new
+
 if Rails.env.staging? || Rails.env.production?
-  redis_url = Configuration::PaasConfigurationService.new.redis_uris[:"dluhc-core-#{Rails.env}-redis"]
+  redis_url = configuration_service.redis_uris[:"dluhc-core-#{Rails.env}-redis"]
 
   Sidekiq.configure_server do |config|
     config.redis = { url: redis_url }
@@ -14,7 +16,7 @@ if Rails.env.staging? || Rails.env.production?
 end
 
 if Rails.env.review?
-  redis_url = Configuration::PaasConfigurationService.new.redis_uris.to_a[0][1]
+  redis_url = configuration_service.redis_uris.to_a[0][1]
 
   Sidekiq.configure_server do |config|
     config.redis = { url: redis_url }

lib/tasks/data_import.rake

@@ -5,7 +5,7 @@ namespace :core do
     path = args[:path]
     raise "Usage: rake core:data_import['data_type', 'path/to/xml_files']" if path.blank? || type.blank?
 
-    storage_service = Storage::S3Service.new(Configuration::PaasConfigurationService.new, ENV["IMPORT_PAAS_INSTANCE"])
+    storage_service = Storage::S3Service.new(PlatformHelper.is_paas? ? Configuration::PaasConfigurationService.new : Configuration::EnvConfigurationService.new, ENV["IMPORT_PAAS_INSTANCE"])
 
     case type
     when "organisation"

lib/tasks/data_import_field.rake

@@ -8,7 +8,7 @@ namespace :core do
     # We only allow a reduced list of known fields to be updatable
     case field
     when "tenancycode", "major_repairs", "lettings_allocation", "offered"
-      s3_service = Storage::S3Service.new(Configuration::PaasConfigurationService.new, ENV["IMPORT_PAAS_INSTANCE"])
+      s3_service = Storage::S3Service.new(PlatformHelper.is_paas? ? Configuration::PaasConfigurationService.new : Configuration::EnvConfigurationService.new, ENV["IMPORT_PAAS_INSTANCE"])
       archive_io = s3_service.get_file_io(path)
       archive_service = Storage::ArchiveService.new(archive_io)
       if archive_service.folder_present?("logs")

lib/tasks/full_import.rake

@@ -6,7 +6,7 @@ namespace :import do
     institutions_csv_name = args[:institutions_csv_name]
     raise "Usage: rake import:initial['institutions_csv_name']" if institutions_csv_name.blank?
 
-    s3_service = Storage::S3Service.new(Configuration::PaasConfigurationService.new, ENV["IMPORT_PAAS_INSTANCE"])
+    s3_service = Storage::S3Service.new(PlatformHelper.is_paas? ? Configuration::PaasConfigurationService.new : Configuration::EnvConfigurationService.new, ENV["IMPORT_PAAS_INSTANCE"])
     csv = CSV.parse(s3_service.get_file_io(institutions_csv_name), headers: true)
     org_count = csv.length
 
@@ -43,7 +43,7 @@ namespace :import do
     institutions_csv_name = args[:institutions_csv_name]
     raise "Usage: rake import:logs['institutions_csv_name']" if institutions_csv_name.blank?
 
-    s3_service = Storage::S3Service.new(Configuration::PaasConfigurationService.new, ENV["IMPORT_PAAS_INSTANCE"])
+    s3_service = Storage::S3Service.new(PlatformHelper.is_paas? ? Configuration::PaasConfigurationService.new : Configuration::EnvConfigurationService.new, ENV["IMPORT_PAAS_INSTANCE"])
     csv = CSV.parse(s3_service.get_file_io(institutions_csv_name), headers: true)
     org_count = csv.length
 
@@ -77,7 +77,7 @@ namespace :import do
     institutions_csv_name = args[:institutions_csv_name]
     raise "Usage: rake import:trigger_invites['institutions_csv_name']" if institutions_csv_name.blank?
 
-    s3_service = Storage::S3Service.new(Configuration::PaasConfigurationService.new, ENV["IMPORT_PAAS_INSTANCE"])
+    s3_service = Storage::S3Service.new(PlatformHelper.is_paas? ? Configuration::PaasConfigurationService.new : Configuration::EnvConfigurationService.new, ENV["IMPORT_PAAS_INSTANCE"])
     csv = CSV.parse(s3_service.get_file_io(institutions_csv_name), headers: true)
 
     Rails.logger.info("Triggering user invite emails")
@@ -98,7 +98,7 @@ namespace :import do
     institutions_csv_name = args[:institutions_csv_name]
     raise "Usage: rake import:generate_reports['institutions_csv_name']" if institutions_csv_name.blank?
 
-    s3_service = Storage::S3Service.new(Configuration::PaasConfigurationService.new, ENV["IMPORT_PAAS_INSTANCE"])
+    s3_service = Storage::S3Service.new(PlatformHelper.is_paas? ? Configuration::PaasConfigurationService.new : Configuration::EnvConfigurationService.new, ENV["IMPORT_PAAS_INSTANCE"])
     institutions_csv = CSV.parse(s3_service.get_file_io(institutions_csv_name), headers: true)
 
     Imports::ImportReportService.new(s3_service, institutions_csv).create_reports(institutions_csv_name)

spec/helpers/platform_helper_spec.rb

@@ -0,0 +1,15 @@
+require "rails_helper"
+
+RSpec.describe PlatformHelper do
+  describe "is_paas?" do
+    it "returns true if the VCAP_SERVICES environment variable exists" do
+      allow(ENV).to receive(:[]).with("VCAP_SERVICES").and_return("dummy")
+      expect(described_class.is_paas?).to eq(true)
+    end
+
+    it "returns false if the VCAP_SERVICES environment variable doesn't exist" do
+      allow(ENV).to receive(:[]).with("VCAP_SERVICES")
+      expect(described_class.is_paas?).to eq(false)
+    end
+  end
+end

spec/lib/tasks/data_import_spec.rb

@@ -3,6 +3,7 @@ require "rake"
 
 describe "data import", type: :task do
   let(:instance_name) { "paas_import_instance" }
+  let(:env_config_service) { instance_double(Configuration::EnvConfigurationService) }
   let(:paas_config_service) { instance_double(Configuration::PaasConfigurationService) }
   let(:storage_service) { instance_double(Storage::S3Service) }
 
@@ -16,8 +17,10 @@ describe "data import", type: :task do
 
       allow(Storage::S3Service).to receive(:new).and_return(storage_service)
       allow(Configuration::PaasConfigurationService).to receive(:new).and_return(paas_config_service)
+      allow(Configuration::EnvConfigurationService).to receive(:new).and_return(env_config_service)
       allow(ENV).to receive(:[])
       allow(ENV).to receive(:[]).with("IMPORT_PAAS_INSTANCE").and_return(instance_name)
+      allow(ENV).to receive(:[]).with("VCAP_SERVICES").and_return("dummy")
     end
 
     context "when importing organisation data" do
@@ -29,13 +32,22 @@ describe "data import", type: :task do
         allow(Imports::OrganisationImportService).to receive(:new).and_return(import_service)
       end
 
-      it "creates an organisation from the given XML file" do
+      it "creates an organisation from the given XML file when the VCAP_SERVICES environment variable exists" do
         expect(Storage::S3Service).to receive(:new).with(paas_config_service, instance_name)
         expect(Imports::OrganisationImportService).to receive(:new).with(storage_service)
         expect(import_service).to receive(:create_organisations).with(fixture_path)
 
         task.invoke(type, fixture_path)
       end
+
+      it "creates an organisation from the given XML file when the VCAP_SERVICES environment variable does not exist" do
+        allow(ENV).to receive(:[]).with("VCAP_SERVICES")
+        expect(Storage::S3Service).to receive(:new).with(env_config_service, instance_name)
+        expect(Imports::OrganisationImportService).to receive(:new).with(storage_service)
+        expect(import_service).to receive(:create_organisations).with(fixture_path)
+
+        task.invoke(type, fixture_path)
+      end
     end
 
     context "when importing user data" do

spec/lib/tasks/date_import_field_spec.rb

@@ -6,6 +6,7 @@ describe "rake core:data_import_field", type: :task do
 
   let(:instance_name) { "paas_import_instance" }
   let(:storage_service) { instance_double(Storage::S3Service) }
+  let(:env_config_service) { instance_double(Configuration::EnvConfigurationService) }
   let(:paas_config_service) { instance_double(Configuration::PaasConfigurationService) }
 
   before do
@@ -14,9 +15,11 @@ describe "rake core:data_import_field", type: :task do
     task.reenable
 
     allow(Storage::S3Service).to receive(:new).and_return(storage_service)
+    allow(Configuration::EnvConfigurationService).to receive(:new).and_return(env_config_service)
     allow(Configuration::PaasConfigurationService).to receive(:new).and_return(paas_config_service)
     allow(ENV).to receive(:[])
     allow(ENV).to receive(:[]).with("IMPORT_PAAS_INSTANCE").and_return(instance_name)
+    allow(ENV).to receive(:[]).with("VCAP_SERVICES").and_return("dummy")
     allow(Imports::LettingsLogsFieldImportService).to receive(:new).and_return(import_service)
   end
 
@@ -34,13 +37,22 @@ describe "rake core:data_import_field", type: :task do
     context "and we update the tenancycode field" do
       let(:field) { "tenancycode" }
 
-      it "updates the logs from the given XML file" do
+      it "updates the logs from the given XML file when the VCAP_SERVICES environment variable exists" do
         expect(Storage::S3Service).to receive(:new).with(paas_config_service, instance_name)
         expect(storage_service).to receive(:get_file_io).with("spec/fixtures/imports/logs")
         expect(Imports::LettingsLogsFieldImportService).to receive(:new).with(archive_service)
         expect(import_service).to receive(:update_field).with(field, "logs")
         task.invoke(field, fixture_path)
       end
+
+      it "updates the logs from the given XML file when the VCAP_SERVICES environment variable does not exist" do
+        allow(ENV).to receive(:[]).with("VCAP_SERVICES")
+        expect(Storage::S3Service).to receive(:new).with(env_config_service, instance_name)
+        expect(storage_service).to receive(:get_file_io).with("spec/fixtures/imports/logs")
+        expect(Imports::LettingsLogsFieldImportService).to receive(:new).with(archive_service)
+        expect(import_service).to receive(:update_field).with(field, "logs")
+        task.invoke(field, fixture_path)
+      end
     end
 
     context "and we update the lettings_allocation fields" do

spec/lib/tasks/full_import_spec.rb

@@ -4,6 +4,7 @@ require "rake"
 describe "full import", type: :task do
   let(:instance_name) { "paas_import_instance" }
   let(:paas_config_service) { instance_double(Configuration::PaasConfigurationService) }
+  let(:env_config_service) { instance_double(Configuration::EnvConfigurationService) }
   let(:storage_service) { instance_double(Storage::S3Service) }
   let(:orgs_list) { "Institution name,Id,Old Completed lettings logs,Old In progress lettings logs,Old Completed sales logs,Old In progress sales logs\norg1,1.zip,0,0,0,0\norg2,2.zip,0,0,0,0" }
 
@@ -12,6 +13,7 @@ describe "full import", type: :task do
     allow(storage_service).to receive(:write_file).and_return(nil)
     allow(storage_service).to receive(:get_file_io).and_return(orgs_list)
     allow(Configuration::PaasConfigurationService).to receive(:new).and_return(paas_config_service)
+    allow(Configuration::EnvConfigurationService).to receive(:new).and_return(env_config_service)
     allow(ENV).to receive(:[])
     allow(ENV).to receive(:[]).with("IMPORT_PAAS_INSTANCE").and_return(instance_name)
   end
@@ -32,13 +34,23 @@ describe "full import", type: :task do
         allow(Imports::ImportReportService).to receive(:new).and_return(import_report_service)
       end
 
-      it "creates a report using given organisation csv" do
+      it "creates a report using given organisation csv when the VCAP_SERVICES environment variable exists" do
+        allow(ENV).to receive(:[]).with("VCAP_SERVICES").and_return("dummy")
         expect(Storage::S3Service).to receive(:new).with(paas_config_service, instance_name)
         expect(Imports::ImportReportService).to receive(:new).with(storage_service, CSV.parse(orgs_list, headers: true))
         expect(import_report_service).to receive(:create_reports).with("some_name")
 
         task.invoke("some_name")
       end
+
+      it "creates a report using given organisation csv when the VCAP_SERVICES environment variable does not exist" do
+        allow(ENV).to receive(:[]).with("VCAP_SERVICES")
+        expect(Storage::S3Service).to receive(:new).with(env_config_service, instance_name)
+        expect(Imports::ImportReportService).to receive(:new).with(storage_service, CSV.parse(orgs_list, headers: true))
+        expect(import_report_service).to receive(:create_reports).with("some_name")
+
+        task.invoke("some_name")
+      end
     end
   end
 end