replace s3 config service

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(Configuration::S3Service.new(name: ENV["EXPORT_PAAS_INSTANCE"]))
     export_service = Exports::LettingsLogExportService.new(storage_service)
 
     export_service.export_xml_lettings_logs(full_update:)

app/jobs/email_csv_job.rb

@@ -11,7 +11,7 @@ class EmailCsvJob < ApplicationJob
 
     filename = organisation.present? ? "logs-#{organisation.name}-#{Time.zone.now}.csv" : "logs-#{Time.zone.now}.csv"
 
-    storage_service = Storage::S3Service.new(Configuration::EnvConfigurationService.new, ENV["CSV_DOWNLOAD_PAAS_INSTANCE"])
+    storage_service = Storage::S3Service.new(Configuration::S3Service.new(name: ENV["CSV_DOWNLOAD_PAAS_INSTANCE"]))
     storage_service.write_file(filename, BYTE_ORDER_MARK + filtered_logs.to_csv(user, codes_only_export:))
 
     url = storage_service.get_presigned_url(filename, EXPIRATION_TIME)

app/models/forms/bulk_upload_lettings/upload_your_file.rb

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

app/models/forms/bulk_upload_sales/upload_your_file.rb

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

app/services/bulk_upload/downloader.rb

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

app/services/configuration/s3_service.rb

@@ -0,0 +1,25 @@
+module Configuration
+  class S3Service
+    attr_reader :name
+
+    def initialize(name:)
+      @name = name
+    end
+
+    def credentials
+      return @credentials if @credentials
+
+      raise "bucket: #{name} not found" if vcap_services[:"aws-s3-bucket"].find { |e| e[:instance_name] == name }.blank?
+
+      @credentials = vcap_services[:"aws-s3-bucket"].find { |e| e[:instance_name] == name }[:credentials]
+    end
+
+  private
+
+    def vcap_services
+      raise "VCAP_SERVICES not found" if ENV["VCAP_SERVICES"].blank?
+
+      @vcap_services ||= JSON.parse(ENV["VCAP_SERVICES"], symbolize_names: true)
+    end
+  end
+end

app/services/storage/s3_service.rb

@@ -2,10 +2,9 @@ module Storage
   class S3Service < StorageService
     attr_reader :configuration
 
-    def initialize(config_service, paas_instance_name)
+    def initialize(config_service)
       super()
       @config_service = config_service
-      @instance_name = (paas_instance_name || "").to_sym
       @configuration = create_configuration
       @client = create_client
     end
@@ -41,16 +40,10 @@ module Storage
 
   private
 
-    def create_configuration
-      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"
-      end
+    attr_reader :config_service
 
-      bucket_config = @config_service.s3_buckets[@instance_name]
-      StorageConfiguration.new(bucket_config[:credentials])
+    def create_configuration
+      StorageConfiguration.new(config_service.credentials)
     end
 
     def create_client

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(Configuration::S3Service.new(name: ENV["IMPORT_PAAS_INSTANCE"]))
 
     case type
     when "organisation"

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(Configuration::PaasConfigurationService.new, ENV["IMPORT_PAAS_INSTANCE"])
+    storage_service = Storage::S3Service.new(Configuration::S3Service.new(name: ENV["IMPORT_PAAS_INSTANCE"]))
 
     # We only allow a reduced list of known fields to be updatable
     case field

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(Configuration::PaasConfigurationService.new, ENV["IMPORT_PAAS_INSTANCE"])
+    s3_service = Storage::S3Service.new(Configuration::S3Service.new(name: ENV["IMPORT_PAAS_INSTANCE"]))
     archive_io = s3_service.get_file_io(archive_path)
     archive_service = Storage::ArchiveService.new(archive_io)
 

spec/jobs/data_export_xml_job_spec.rb

@@ -2,12 +2,10 @@ require "rails_helper"
 
 describe DataExportXmlJob do
   let(:storage_service) { instance_double(Storage::S3Service) }
-  let(:paas_config_service) { instance_double(Configuration::PaasConfigurationService) }
   let(:export_service) { instance_double(Exports::LettingsLogExportService) }
 
   before do
     allow(Storage::S3Service).to receive(:new).and_return(storage_service)
-    allow(Configuration::PaasConfigurationService).to receive(:new).and_return(paas_config_service)
     allow(Exports::LettingsLogExportService).to receive(:new).and_return(export_service)
   end
 

spec/lib/tasks/data_export_spec.rb

@@ -4,7 +4,6 @@ require "rake"
 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(Configuration::PaasConfigurationService) }
   let(:export_service) { instance_double(Exports::LettingsLogExportService) }
 
   before do
@@ -13,7 +12,6 @@ describe "rake core:data_export", type: task do
     task.reenable
 
     allow(Storage::S3Service).to receive(:new).and_return(storage_service)
-    allow(Configuration::PaasConfigurationService).to receive(:new).and_return(paas_config_service)
     allow(Exports::LettingsLogExportService).to receive(:new).and_return(export_service)
     allow(ENV).to receive(:[])
     allow(ENV).to receive(:[]).with("EXPORT_PAAS_INSTANCE").and_return(paas_instance)

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(Configuration::PaasConfigurationService) }
+  let(:paas_config_service) { instance_double(Configuration::S3Service) }
 
   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(Configuration::PaasConfigurationService).to receive(:new).and_return(paas_config_service)
+    allow(Configuration::S3Service).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
@@ -29,7 +29,7 @@ describe "rake core:data_import", type: :task do
     end
 
     it "creates an organisation from the given XML file" do
-      expect(Storage::S3Service).to receive(:new).with(paas_config_service, instance_name)
+      expect(Storage::S3Service).to receive(:new).with(paas_config_service)
       expect(Imports::OrganisationImportService).to receive(:new).with(storage_service)
       expect(import_service).to receive(:create_organisations).with(fixture_path)
 
@@ -47,7 +47,7 @@ describe "rake core:data_import", type: :task do
     end
 
     it "creates a user from the given XML file" do
-      expect(Storage::S3Service).to receive(:new).with(paas_config_service, instance_name)
+      expect(Storage::S3Service).to receive(:new).with(paas_config_service)
       expect(Imports::UserImportService).to receive(:new).with(storage_service)
       expect(import_service).to receive(:create_users).with(fixture_path)
 
@@ -65,7 +65,7 @@ describe "rake core:data_import", type: :task do
     end
 
     it "creates an organisation from the given XML file" do
-      expect(Storage::S3Service).to receive(:new).with(paas_config_service, instance_name)
+      expect(Storage::S3Service).to receive(:new).with(paas_config_service)
       expect(Imports::DataProtectionConfirmationImportService).to receive(:new).with(storage_service)
       expect(import_service).to receive(:create_data_protection_confirmations).with(fixture_path)
 
@@ -83,7 +83,7 @@ describe "rake core:data_import", type: :task do
     end
 
     it "creates an organisation la from the given XML file" do
-      expect(Storage::S3Service).to receive(:new).with(paas_config_service, instance_name)
+      expect(Storage::S3Service).to receive(:new).with(paas_config_service)
       expect(Imports::OrganisationRentPeriodImportService).to receive(:new).with(storage_service)
       expect(import_service).to receive(:create_organisation_rent_periods).with(fixture_path)
 
@@ -101,7 +101,7 @@ describe "rake core:data_import", type: :task do
     end
 
     it "creates lettings logs from the given XML file" do
-      expect(Storage::S3Service).to receive(:new).with(paas_config_service, instance_name)
+      expect(Storage::S3Service).to receive(:new).with(paas_config_service)
       expect(Imports::LettingsLogsImportService).to receive(:new).with(storage_service)
       expect(import_service).to receive(:create_logs).with(fixture_path)
 
@@ -119,7 +119,7 @@ describe "rake core:data_import", type: :task do
     end
 
     it "creates sales logs from the given XML file" do
-      expect(Storage::S3Service).to receive(:new).with(paas_config_service, instance_name)
+      expect(Storage::S3Service).to receive(:new).with(paas_config_service)
       expect(Imports::SalesLogsImportService).to receive(:new).with(storage_service)
       expect(import_service).to receive(:create_logs).with(fixture_path)
 
@@ -137,7 +137,7 @@ describe "rake core:data_import", type: :task do
     end
 
     it "creates a scheme from the given XML file" do
-      expect(Storage::S3Service).to receive(:new).with(paas_config_service, instance_name)
+      expect(Storage::S3Service).to receive(:new).with(paas_config_service)
       expect(Imports::SchemeImportService).to receive(:new).with(storage_service)
       expect(import_service).to receive(:create_schemes).with(fixture_path)
 
@@ -155,7 +155,7 @@ describe "rake core:data_import", type: :task do
     end
 
     it "creates a scheme location from the given XML file" do
-      expect(Storage::S3Service).to receive(:new).with(paas_config_service, instance_name)
+      expect(Storage::S3Service).to receive(:new).with(paas_config_service)
       expect(Imports::SchemeLocationImportService).to receive(:new).with(storage_service)
       expect(import_service).to receive(:create_scheme_locations).with(fixture_path)
 

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(Configuration::PaasConfigurationService) }
+  let(:paas_config_service) { instance_double(Configuration::S3Service) }
 
   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(Configuration::PaasConfigurationService).to receive(:new).and_return(paas_config_service)
+    allow(Configuration::S3Service).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
@@ -32,7 +32,7 @@ describe "rake core:data_import_field", type: :task do
       let(:field) { "tenant_code" }
 
       it "properly configures the storage service" do
-        expect(Storage::S3Service).to receive(:new).with(paas_config_service, instance_name)
+        expect(Storage::S3Service).to receive(:new).with(paas_config_service)
         task.invoke(field, fixture_path)
       end
 
@@ -46,7 +46,7 @@ describe "rake core:data_import_field", type: :task do
       let(:field) { "lettings_allocation" }
 
       it "properly configures the storage service" do
-        expect(Storage::S3Service).to receive(:new).with(paas_config_service, instance_name)
+        expect(Storage::S3Service).to receive(:new).with(paas_config_service)
         task.invoke(field, fixture_path)
       end
 
@@ -60,7 +60,7 @@ describe "rake core:data_import_field", type: :task do
       let(:field) { "major_repairs" }
 
       it "properly configures the storage service" do
-        expect(Storage::S3Service).to receive(:new).with(paas_config_service, instance_name)
+        expect(Storage::S3Service).to receive(:new).with(paas_config_service)
         task.invoke(field, fixture_path)
       end
 

spec/lib/tasks/full_import_spec.rb

@@ -7,14 +7,12 @@ 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(Configuration::PaasConfigurationService) }
 
   before do
     Rake.application.rake_require("tasks/full_import")
     Rake::Task.define_task(:environment)
     task.reenable
 
-    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)

spec/services/configuration/s3_service_spec.rb

@@ -0,0 +1,156 @@
+require "rails_helper"
+
+RSpec.describe Configuration::S3Service do
+  subject(:service) { described_class.new(name:) }
+
+  let(:name) { "dluhc-core-review-export-bucket" }
+
+  let(:vcap_services) do
+    {
+      "aws-s3-bucket" => [
+        {
+          "label" => "aws-s3-bucket",
+          "provider" => nil,
+          "plan" => "default",
+          "name" => "dluhc-core-review-export-bucket",
+          "tags" => %w[s3],
+          "instance_guid" => "some-guid",
+          "instance_name" => "dluhc-core-review-export-bucket",
+          "binding_guid" => "some-guid",
+          "binding_name" => nil,
+          "credentials" => {
+            "bucket_name" => "actual-export-bucket-name",
+            "aws_access_key_id" => "access_key_123",
+            "aws_secret_access_key" => "secret_access_key_123",
+            "aws_region" => "region-1",
+            "deploy_env" => "",
+          },
+          "syslog_drain_url" => nil,
+          "volume_mounts" => [],
+        },
+        {
+          "label" => "aws-s3-bucket",
+          "provider" => nil,
+          "plan" => "default",
+          "name" => "dluhc-core-review-csv-bucket",
+          "tags" => %w[s3],
+          "instance_guid" => "some-guid",
+          "instance_name" => "dluhc-core-review-csv-bucket",
+          "binding_guid" => "some-guid",
+          "binding_name" => nil,
+          "credentials" => {
+            "bucket_name" => "paas-s3-broker-prod-lon-some-guid",
+            "aws_access_key_id" => "access_key",
+            "aws_secret_access_key" => "secret_access_key",
+            "aws_region" => "eu-west-2",
+            "deploy_env" => "",
+          },
+          "syslog_drain_url" => nil,
+          "volume_mounts" => [],
+        },
+        {
+          "label" => "aws-s3-bucket",
+          "provider" => nil,
+          "plan" => "default",
+          "name" => "dluhc-core-review-import-bucket",
+          "tags" => %w[s3],
+          "instance_guid" => "some-guid",
+          "instance_name" => "dluhc-core-review-import-bucket",
+          "binding_guid" => "some-guid",
+          "binding_name" => nil,
+          "credentials" => {
+            "bucket_name" => "paas-s3-broker-prod-lon-some-guid",
+            "aws_access_key_id" => "access_key",
+            "aws_secret_access_key" => "secret_access_key",
+            "aws_region" => "eu-west-2",
+            "deploy_env" => "",
+          },
+          "syslog_drain_url" => nil,
+          "volume_mounts" => [],
+        },
+      ],
+      "postgres" => [
+        {
+          "label" => "postgres",
+          "provider" => nil,
+          "plan" => "some-plan",
+          "name" => "dluhc-core-review-id-postgres",
+          "tags" => %w[postgres relational],
+          "instance_guid" => "some-guid",
+          "instance_name" => "dluhc-core-review-id-postgres",
+          "binding_guid" => "some-guid",
+          "binding_name" => nil,
+          "credentials" => {
+            "host" => "rdsbroker-some-guid.some-guid.eu-west-2.rds.amazonaws.com",
+            "port" => 5432,
+            "name" => "rdsbroker_some_guid",
+            "username" => "some_user",
+            "password" => "some_password",
+            "uri" => "postgres://some_user:some_password@rdsbroker-some-guid.some-guid.com:5432/rdsbroker_some_guid",
+            "jdbcuri" => "some-connection-string",
+            "syslog_drain_url" => nil,
+            "volume_mounts" => [],
+          },
+        },
+      ],
+      "redis" => [
+        {
+          "label" => "redis",
+          "provider" => nil,
+          "plan" => "micro-6.x",
+          "name" => "dluhc-core-review-id-redis",
+          "tags" => %w[elasticache redis],
+          "instance_guid" => "some-guid",
+          "instance_name" => "dluhc-core-review-id-redis",
+          "binding_guid" => "some-guid",
+          "binding_name" => nil,
+          "credentials" => {
+            "host" => "somewhere.cache.amazonaws.com",
+            "port" => 6379,
+            "name" => "some-name",
+            "password" => "some-password",
+            "uri" => "rediss://key@master.cf-somewhere.euw2.cache.amazonaws.com:6379",
+            "tls_enabled" => true,
+          },
+          "syslog_drain_url" => nil,
+          "volume_mounts" => [],
+        },
+      ],
+      "user-provided" => [
+        {
+          "label" => "user-provided",
+          "name" => "logit-ssl-drain",
+          "tags" => [],
+          "instance_guid" => "some-guid",
+          "instance_name" => "logit-ssl-drain",
+          "binding_guid" => "some-guid",
+          "binding_name" => nil,
+          "credentials" => nil,
+          "syslog_drain_url" => "syslog-tls://some-guid.service.com:123",
+          "volume_mounts" => [],
+        },
+      ],
+    }
+  end
+
+  before do
+    stub_const("ENV", { "VCAP_SERVICES" => vcap_services.to_json })
+  end
+
+  describe "#credentials" do
+    it "returns correct credentials" do
+      expect(service.credentials[:aws_access_key_id]).to eql("access_key_123")
+      expect(service.credentials[:aws_secret_access_key]).to eql("secret_access_key_123")
+      expect(service.credentials[:bucket_name]).to eql("actual-export-bucket-name")
+      expect(service.credentials[:aws_region]).to eql("region-1")
+    end
+
+    context "when bucket not present" do
+      let(:name) { "foo" }
+
+      it "raises an error" do
+        expect { service.credentials[:aws_access_key_id] }.to raise_error.with_message("bucket: foo not found")
+      end
+    end
+  end
+end

spec/services/storage/s3_service_spec.rb

@@ -20,15 +20,15 @@ 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(Configuration::PaasConfigurationService.new, "random_instance") }
+    subject(:storage_service) { described_class.new(Configuration::S3Service.new(name: "random_instance")) }
 
     it "raises an exception" do
-      expect { storage_service }.to raise_error(RuntimeError, "No S3 bucket is present in the PaaS configuration")
+      expect { storage_service }.to raise_error(RuntimeError, "VCAP_SERVICES not found")
     end
   end
 
   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") }
+    subject(:storage_service) { described_class.new(Configuration::S3Service.new(name: "random_instance")) }
 
     let(:vcap_services) do
       <<-JSON
@@ -41,22 +41,18 @@ RSpec.describe Storage::S3Service do
     end
 
     it "raises an exception" do
-      expect { storage_service }.to raise_error(RuntimeError, /instance name could not be found/)
+      expect { storage_service }.to raise_error(RuntimeError, "bucket: random_instance not found")
     end
   end
 
   context "when we create a storage service with a valid instance name" do
-    subject(:storage_service) { described_class.new(Configuration::PaasConfigurationService.new, instance_name) }
+    subject(:storage_service) { described_class.new(Configuration::S3Service.new(name: instance_name)) }
 
     before do
       allow(ENV).to receive(:[])
       allow(ENV).to receive(:[]).with("VCAP_SERVICES").and_return(vcap_services)
     end
 
-    it "creates a Storage Configuration" do
-      expect(storage_service.configuration).to be_an(Storage::StorageConfiguration)
-    end
-
     it "sets the expected parameters in the configuration" do
       expected_configuration = Storage::StorageConfiguration.new(
         {
@@ -71,7 +67,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(Configuration::PaasConfigurationService.new, instance_name) }
+    subject(:storage_service) { described_class.new(Configuration::S3Service.new(name: instance_name)) }
 
     let(:filename) { "my_file" }
     let(:content) { "content" }
@@ -106,7 +102,7 @@ RSpec.describe Storage::S3Service do
   end
 
   context "when we create a storage service" do
-    subject(:storage_service) { described_class.new(Configuration::PaasConfigurationService.new, instance_name) }
+    subject(:storage_service) { described_class.new(Configuration::S3Service.new(name: instance_name)) }
 
     let(:s3_client_stub) { Aws::S3::Client.new(stub_responses: true) }