diff --git a/app/services/configuration/configuration_service.rb b/app/services/configuration/configuration_service.rb new file mode 100644 index 000000000..efc2c4907 --- /dev/null +++ b/app/services/configuration/configuration_service.rb @@ -0,0 +1,13 @@ +module Configuration + class ConfigurationService + attr_reader :s3_buckets, :redis_uris + + def s3_config_present? + raise NotImplementedError + end + + def redis_config_present? + raise NotImplementedError + end + end +end diff --git a/app/services/configuration/paas_configuration_service.rb b/app/services/configuration/paas_configuration_service.rb new file mode 100644 index 000000000..af7aee19e --- /dev/null +++ b/app/services/configuration/paas_configuration_service.rb @@ -0,0 +1,63 @@ +module Configuration + class PaasConfigurationService < ConfigurationService + def initialize(logger = Rails.logger) + super() + @logger = logger + @paas_config = read_pass_config + @s3_buckets = read_s3_buckets + @redis_uris = read_redis_uris + end + + def s3_config_present? + config_present? && @paas_config.key?(:"aws-s3-bucket") + end + + def redis_config_present? + config_present? && @paas_config.key?(:redis) + end + + private + + def config_present? + !ENV["VCAP_SERVICES"].nil? + end + + def read_pass_config + unless config_present? + @logger.warn("Could not find VCAP_SERVICES in the environment!") + return {} + end + + begin + JSON.parse(ENV["VCAP_SERVICES"], { symbolize_names: true }) + rescue StandardError + @logger.warn("Could not parse VCAP_SERVICES!") + {} + end + end + + def read_s3_buckets + return {} unless s3_config_present? + + s3_buckets = {} + @paas_config[:"aws-s3-bucket"].each do |bucket_config| + if bucket_config.key?(:instance_name) + s3_buckets[bucket_config[:instance_name].to_sym] = bucket_config + end + end + s3_buckets + end + + def read_redis_uris + return {} unless redis_config_present? + + redis_uris = {} + @paas_config[:redis].each do |redis_config| + if redis_config.key?(:instance_name) + redis_uris[redis_config[:instance_name].to_sym] = redis_config.dig(:credentials, :uri) + end + end + redis_uris + end + end +end diff --git a/app/services/paas_configuration_service.rb b/app/services/paas_configuration_service.rb deleted file mode 100644 index 008b4d2ab..000000000 --- a/app/services/paas_configuration_service.rb +++ /dev/null @@ -1,62 +0,0 @@ -class PaasConfigurationService - attr_reader :s3_buckets, :redis_uris - - def initialize(logger = Rails.logger) - @logger = logger - @paas_config = read_pass_config - @s3_buckets = read_s3_buckets - @redis_uris = read_redis_uris - end - - def config_present? - !ENV["VCAP_SERVICES"].nil? - end - - def s3_config_present? - config_present? && @paas_config.key?(:"aws-s3-bucket") - end - - def redis_config_present? - config_present? && @paas_config.key?(:redis) - end - -private - - def read_pass_config - unless config_present? - @logger.warn("Could not find VCAP_SERVICES in the environment!") - return {} - end - - begin - JSON.parse(ENV["VCAP_SERVICES"], { symbolize_names: true }) - rescue StandardError - @logger.warn("Could not parse VCAP_SERVICES!") - {} - end - end - - def read_s3_buckets - return {} unless s3_config_present? - - s3_buckets = {} - @paas_config[:"aws-s3-bucket"].each do |bucket_config| - if bucket_config.key?(:instance_name) - s3_buckets[bucket_config[:instance_name].to_sym] = bucket_config - end - end - s3_buckets - end - - def read_redis_uris - return {} unless redis_config_present? - - redis_uris = {} - @paas_config[:redis].each do |redis_config| - if redis_config.key?(:instance_name) - redis_uris[redis_config[:instance_name].to_sym] = redis_config.dig(:credentials, :uri) - end - end - redis_uris - end -end diff --git a/app/services/storage/s3_service.rb b/app/services/storage/s3_service.rb index 9aa043e98..aee76398d 100644 --- a/app/services/storage/s3_service.rb +++ b/app/services/storage/s3_service.rb @@ -36,8 +36,8 @@ module Storage private def create_configuration - unless @config_service.config_present? - raise "No PaaS configuration present" + unless @config_service.s3_config_present? + raise "No S3 bucket is present in the PaaS configuration" end unless @config_service.s3_buckets.key?(@instance_name) raise "#{@instance_name} instance name could not be found" diff --git a/config/initializers/rack_attack.rb b/config/initializers/rack_attack.rb index e130adc50..3bfcff86b 100644 --- a/config/initializers/rack_attack.rb +++ b/config/initializers/rack_attack.rb @@ -1,10 +1,11 @@ -require "paas_configuration_service" +require "configuration/configuration_service" +require "configuration/paas_configuration_service" if Rails.env.development? || Rails.env.test? Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new Rack::Attack.enabled = false else - redis_url = PaasConfigurationService.new.redis_uris[:"dluhc-core-#{Rails.env}-redis-rate-limit"] + redis_url = Configuration::PaasConfigurationService.new.redis_uris[:"dluhc-core-#{Rails.env}-redis-rate-limit"] Rack::Attack.cache.store = ActiveSupport::Cache::RedisCacheStore.new(url: redis_url) end diff --git a/lib/tasks/data_export.rake b/lib/tasks/data_export.rake index c173bd5f0..fb7af1466 100644 --- a/lib/tasks/data_export.rake +++ b/lib/tasks/data_export.rake @@ -4,7 +4,7 @@ namespace :core do format = args[:format] full_update = args[:full_update].present? && args[:full_update] == "true" - storage_service = Storage::S3Service.new(PaasConfigurationService.new, ENV["EXPORT_PAAS_INSTANCE"]) + storage_service = Storage::S3Service.new(Configuration::PaasConfigurationService.new, ENV["EXPORT_PAAS_INSTANCE"]) export_service = Exports::CaseLogExportService.new(storage_service) if format.present? && format == "CSV" diff --git a/lib/tasks/data_import.rake b/lib/tasks/data_import.rake index 80254b93b..b8baac256 100644 --- a/lib/tasks/data_import.rake +++ b/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(PaasConfigurationService.new, ENV["IMPORT_PAAS_INSTANCE"]) + storage_service = Storage::S3Service.new(Configuration::PaasConfigurationService.new, ENV["IMPORT_PAAS_INSTANCE"]) case type when "organisation" diff --git a/lib/tasks/data_import_field.rake b/lib/tasks/data_import_field.rake index 4c9362093..523ed6146 100644 --- a/lib/tasks/data_import_field.rake +++ b/lib/tasks/data_import_field.rake @@ -5,7 +5,7 @@ namespace :core do path = args[:path] raise "Usage: rake core:data_import_field['field','path/to/xml_files']" if path.blank? || field.blank? - storage_service = Storage::S3Service.new(PaasConfigurationService.new, ENV["IMPORT_PAAS_INSTANCE"]) + storage_service = Storage::S3Service.new(Configuration::PaasConfigurationService.new, ENV["IMPORT_PAAS_INSTANCE"]) # We only allow a reduced list of known fields to be updatable case field diff --git a/lib/tasks/full_import.rake b/lib/tasks/full_import.rake index 5f4ed52bd..82879a15c 100644 --- a/lib/tasks/full_import.rake +++ b/lib/tasks/full_import.rake @@ -6,7 +6,7 @@ namespace :core do archive_path = args[:archive_path] raise "Usage: rake core:full_import['path/to/archive']" if archive_path.blank? - s3_service = Storage::S3Service.new(PaasConfigurationService.new, ENV["IMPORT_PAAS_INSTANCE"]) + s3_service = Storage::S3Service.new(Configuration::PaasConfigurationService.new, ENV["IMPORT_PAAS_INSTANCE"]) archive_io = s3_service.get_file_io(archive_path) archive_service = Storage::ArchiveService.new(archive_io) diff --git a/spec/lib/tasks/data_export_spec.rb b/spec/lib/tasks/data_export_spec.rb index da87f6bf2..00d3414d4 100644 --- a/spec/lib/tasks/data_export_spec.rb +++ b/spec/lib/tasks/data_export_spec.rb @@ -6,7 +6,7 @@ describe "rake core:data_export", type: task do let(:paas_instance) { "paas_export_instance" } let(:storage_service) { instance_double(Storage::S3Service) } - let(:paas_config_service) { instance_double(PaasConfigurationService) } + let(:paas_config_service) { instance_double(Configuration::PaasConfigurationService) } let(:export_service) { instance_double(Exports::CaseLogExportService) } before do @@ -15,7 +15,7 @@ describe "rake core:data_export", type: task do task.reenable allow(Storage::S3Service).to receive(:new).and_return(storage_service) - allow(PaasConfigurationService).to receive(:new).and_return(paas_config_service) + allow(Configuration::PaasConfigurationService).to receive(:new).and_return(paas_config_service) allow(Exports::CaseLogExportService).to receive(:new).and_return(export_service) allow(ENV).to receive(:[]) allow(ENV).to receive(:[]).with("EXPORT_PAAS_INSTANCE").and_return(paas_instance) diff --git a/spec/lib/tasks/data_import_spec.rb b/spec/lib/tasks/data_import_spec.rb index 7d36c2a8d..e861c2ad2 100644 --- a/spec/lib/tasks/data_import_spec.rb +++ b/spec/lib/tasks/data_import_spec.rb @@ -6,7 +6,7 @@ describe "rake core:data_import", type: :task do let(:instance_name) { "paas_import_instance" } let(:storage_service) { instance_double(Storage::S3Service) } - let(:paas_config_service) { instance_double(PaasConfigurationService) } + let(:paas_config_service) { instance_double(Configuration::PaasConfigurationService) } before do Rake.application.rake_require("tasks/data_import") @@ -14,7 +14,7 @@ describe "rake core:data_import", type: :task do task.reenable allow(Storage::S3Service).to receive(:new).and_return(storage_service) - allow(PaasConfigurationService).to receive(:new).and_return(paas_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) end diff --git a/spec/lib/tasks/date_import_field_spec.rb b/spec/lib/tasks/date_import_field_spec.rb index a79a1d379..69c58697a 100644 --- a/spec/lib/tasks/date_import_field_spec.rb +++ b/spec/lib/tasks/date_import_field_spec.rb @@ -6,7 +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(:paas_config_service) { instance_double(PaasConfigurationService) } + let(:paas_config_service) { instance_double(Configuration::PaasConfigurationService) } before do Rake.application.rake_require("tasks/data_import_field") @@ -14,7 +14,7 @@ describe "rake core:data_import_field", type: :task do task.reenable allow(Storage::S3Service).to receive(:new).and_return(storage_service) - allow(PaasConfigurationService).to receive(:new).and_return(paas_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) end diff --git a/spec/lib/tasks/full_import_spec.rb b/spec/lib/tasks/full_import_spec.rb index 78f336197..1c7ab5c1b 100644 --- a/spec/lib/tasks/full_import_spec.rb +++ b/spec/lib/tasks/full_import_spec.rb @@ -7,14 +7,14 @@ describe "rake core:full_import", type: :task do let(:s3_service) { instance_double(Storage::S3Service) } let(:archive_service) { instance_double(Storage::ArchiveService) } - let(:paas_config_service) { instance_double(PaasConfigurationService) } + let(:paas_config_service) { instance_double(Configuration::PaasConfigurationService) } before do Rake.application.rake_require("tasks/full_import") Rake::Task.define_task(:environment) task.reenable - allow(PaasConfigurationService).to receive(:new).and_return(paas_config_service) + allow(Configuration::PaasConfigurationService).to receive(:new).and_return(paas_config_service) allow(Storage::S3Service).to receive(:new).and_return(s3_service) allow(s3_service).to receive(:get_file_io) allow(Storage::ArchiveService).to receive(:new).and_return(archive_service) diff --git a/spec/services/paas_configuration_service_spec.rb b/spec/services/configuration/paas_configuration_service_spec.rb similarity index 64% rename from spec/services/paas_configuration_service_spec.rb rename to spec/services/configuration/paas_configuration_service_spec.rb index 3338ad5ec..5c6430e53 100644 --- a/spec/services/paas_configuration_service_spec.rb +++ b/spec/services/configuration/paas_configuration_service_spec.rb @@ -1,6 +1,6 @@ require "rails_helper" -RSpec.describe PaasConfigurationService do +RSpec.describe Configuration::PaasConfigurationService do subject(:config_service) { described_class.new(logger) } let(:logger) { instance_double(ActiveSupport::LogSubscriber) } @@ -8,14 +8,38 @@ RSpec.describe PaasConfigurationService do context "when the paas configuration is unavailable" do before { allow(logger).to receive(:warn) } - it "returns the configuration as not present" do - expect(config_service.config_present?).to be(false) + it "returns the S3 configuration as not present" do + expect(config_service.s3_config_present?).to be(false) + end + + it "returns the redis configuration as not present" do + expect(config_service.redis_config_present?).to be(false) + end + + it "does not retrieve any S3 bucket configuration" do + expect(config_service.s3_buckets).to be_a(Hash) + expect(config_service.s3_buckets).to be_empty + end + + it "does not retrieve any redis configuration" do + expect(config_service.redis_uris).to be_a(Hash) + expect(config_service.redis_uris).to be_empty + end + end + + context "when the paas configuration is present but empty" do + before do + allow(ENV).to receive(:[]).with("VCAP_SERVICES").and_return("{}") end it "returns the S3 configuration as not present" do expect(config_service.s3_config_present?).to be(false) end + it "returns the redis configuration as not present" do + expect(config_service.redis_config_present?).to be(false) + end + it "does not retrieve any S3 bucket configuration" do expect(config_service.s3_buckets).to be_a(Hash) expect(config_service.s3_buckets).to be_empty @@ -41,7 +65,7 @@ RSpec.describe PaasConfigurationService do end end - context "when the paas configuration is present with S3 buckets" do + context "when the paas configuration is present with S3 configured" do let(:vcap_services) do <<-JSON {"aws-s3-bucket": [{"instance_name": "bucket_1"},{"instance_name": "bucket_2"}]} @@ -52,14 +76,14 @@ RSpec.describe PaasConfigurationService do allow(ENV).to receive(:[]).with("VCAP_SERVICES").and_return(vcap_services) end - it "returns the configuration as present" do - expect(config_service.config_present?).to be(true) - end - it "returns the S3 configuration as present" do expect(config_service.s3_config_present?).to be(true) end + it "returns the redis configuration as not present" do + expect(config_service.redis_config_present?).to be(false) + end + it "does retrieve the S3 bucket configurations" do s3_buckets = config_service.s3_buckets @@ -70,29 +94,42 @@ RSpec.describe PaasConfigurationService do end end - context "when the paas configuration is present without S3 buckets" do + context "when the paas configuration is present with redis configured" do + let(:vcap_services) do + <<-JSON + {"redis": [{"instance_name": "redis_1", "credentials": {"uri": "redis_uri" }}]} + JSON + end + before do - allow(ENV).to receive(:[]).with("VCAP_SERVICES").and_return("{}") + allow(ENV).to receive(:[]).with("VCAP_SERVICES").and_return(vcap_services) end - it "returns the configuration as present" do - expect(config_service.config_present?).to be(true) + it "returns the redis configuration as present" do + expect(config_service.redis_config_present?).to be(true) end it "returns the S3 configuration as not present" do expect(config_service.s3_config_present?).to be(false) end - it "does not retrieve any S3 bucket configuration" do - expect(config_service.s3_buckets).to be_a(Hash) - expect(config_service.s3_buckets).to be_empty + it "does retrieve the redis configurations" do + redis_uris = config_service.redis_uris + + expect(redis_uris).not_to be_empty + expect(redis_uris.count).to be(1) + expect(redis_uris).to have_key(:redis_1) + expect(redis_uris[:redis_1]).to eq("redis_uri") end end - context "when the paas configuration is present with redis configurations" do + context "when the paas configuration is present with both S3 and redis configured" do let(:vcap_services) do <<-JSON - {"redis": [{"instance_name": "redis_1", "credentials": {"uri": "redis_uri" }}]} + { + "aws-s3-bucket": [{"instance_name": "bucket_1"},{"instance_name": "bucket_2"}], + "redis": [{"instance_name": "redis_1", "credentials": {"uri": "redis_uri" }}] + } JSON end @@ -100,8 +137,8 @@ RSpec.describe PaasConfigurationService do allow(ENV).to receive(:[]).with("VCAP_SERVICES").and_return(vcap_services) end - it "returns the configuration as present" do - expect(config_service.config_present?).to be(true) + it "returns the S3configuration as present" do + expect(config_service.s3_config_present?).to be(true) end it "returns the redis configuration as present" do @@ -116,24 +153,14 @@ RSpec.describe PaasConfigurationService do expect(redis_uris).to have_key(:redis_1) expect(redis_uris[:redis_1]).to eq("redis_uri") end - end - context "when the paas configuration is present without redis configuration" do - before do - allow(ENV).to receive(:[]).with("VCAP_SERVICES").and_return("{}") - end - - it "returns the configuration as present" do - expect(config_service.config_present?).to be(true) - end - - it "returns the redis configuration as not present" do - expect(config_service.redis_config_present?).to be(false) - end + it "does retrieve the S3 bucket configurations" do + s3_buckets = config_service.s3_buckets - it "does not retrieve any redis uris from configuration" do - expect(config_service.redis_uris).to be_a(Hash) - expect(config_service.redis_uris).to be_empty + expect(s3_buckets).not_to be_empty + expect(s3_buckets.count).to be(2) + expect(s3_buckets).to have_key(:bucket_1) + expect(s3_buckets).to have_key(:bucket_2) end end end diff --git a/spec/services/storage/s3_service_spec.rb b/spec/services/storage/s3_service_spec.rb index 6c0f0eeb1..36a549b21 100644 --- a/spec/services/storage/s3_service_spec.rb +++ b/spec/services/storage/s3_service_spec.rb @@ -20,18 +20,24 @@ RSpec.describe Storage::S3Service do end context "when we create a storage service with no PaaS Configuration present" do - subject(:storage_service) { described_class.new(PaasConfigurationService.new, "random_instance") } + subject(:storage_service) { described_class.new(Configuration::PaasConfigurationService.new, "random_instance") } it "raises an exception" do - expect { storage_service }.to raise_error(RuntimeError, /No PaaS configuration present/) + expect { storage_service }.to raise_error(RuntimeError, "No S3 bucket is present in the PaaS configuration") end end - context "when we create a storage service with an unknown instance name" do - subject(:storage_service) { described_class.new(PaasConfigurationService.new, "random_instance") } + context "when we create a storage service and the S3 instance name is not found in the PaaS configuration" do + subject(:storage_service) { described_class.new(Configuration::PaasConfigurationService.new, "random_instance") } + + let(:vcap_services) do + <<-JSON + {"aws-s3-bucket": []} + JSON + end before do - allow(ENV).to receive(:[]).with("VCAP_SERVICES").and_return("{}") + allow(ENV).to receive(:[]).with("VCAP_SERVICES").and_return(vcap_services) end it "raises an exception" do @@ -40,7 +46,7 @@ RSpec.describe Storage::S3Service do end context "when we create a storage service with a valid instance name" do - subject(:storage_service) { described_class.new(PaasConfigurationService.new, instance_name) } + subject(:storage_service) { described_class.new(Configuration::PaasConfigurationService.new, instance_name) } before do allow(ENV).to receive(:[]) @@ -65,7 +71,7 @@ RSpec.describe Storage::S3Service do end context "when we create a storage service and write a stubbed object" do - subject(:storage_service) { described_class.new(PaasConfigurationService.new, instance_name) } + subject(:storage_service) { described_class.new(Configuration::PaasConfigurationService.new, instance_name) } let(:filename) { "my_file" } let(:content) { "content" } @@ -100,7 +106,7 @@ RSpec.describe Storage::S3Service do end context "when we create a storage service" do - subject(:storage_service) { described_class.new(PaasConfigurationService.new, instance_name) } + subject(:storage_service) { described_class.new(Configuration::PaasConfigurationService.new, instance_name) } let(:s3_client_stub) { Aws::S3::Client.new(stub_responses: true) }