From 2b0f11e706fbb1e3683d715cf3763633c8a9ec36 Mon Sep 17 00:00:00 2001 From: Katharina Przybill <30441792+kathap@users.noreply.github.com> Date: Tue, 14 Jul 2026 13:11:22 +0200 Subject: [PATCH 01/24] Remove fog-aws gem and all fog AWS blobstore support --- Gemfile | 1 - Gemfile.lock | 12 -- config/cloud_controller.yml | 4 - .../blobstore/client_provider.rb | 7 +- .../blobstore/fog/fog_client.rb | 21 +-- .../blobstore/fog/providers.rb | 7 - .../storage_cli/storage_cli_client.rb | 1 - .../config_schemas/api_schema.rb | 4 - .../blobstore_benchmarks_schema.rb | 1 - .../config_schemas/clock_schema.rb | 4 - .../deployment_updater_schema.rb | 4 - .../config_schemas/worker_schema.rb | 4 - .../documentation/buildpack_cache_api_spec.rb | 4 +- spec/fixtures/config/port_8181_config.yml | 8 - spec/support/bootstrap/test_config.rb | 15 +- .../controllers/resource_pool.rb | 6 +- .../download_droplets_controller_spec.rb | 16 +- .../runtime/stagings_controller_spec.rb | 12 +- .../runtime/buildpack_cache_cleanup_spec.rb | 4 +- .../jobs/v3/buildpack_cache_cleanup_spec.rb | 4 +- .../jobs/v3/buildpack_cache_delete_spec.rb | 2 +- spec/unit/jobs/v3/droplet_bits_copier_spec.rb | 2 +- spec/unit/jobs/v3/package_bits_copier_spec.rb | 2 +- .../blobstore/client_provider_spec.rb | 33 +--- .../blobstore/fog/fog_client_spec.rb | 143 +----------------- .../storage_cli/storage_cli_client_spec.rb | 21 --- spec/unit/lib/cloud_controller/config_spec.rb | 16 -- .../packager/local_bits_packer_spec.rb | 4 +- 28 files changed, 31 insertions(+), 331 deletions(-) diff --git a/Gemfile b/Gemfile index fd754a7fc47..a1cec743688 100644 --- a/Gemfile +++ b/Gemfile @@ -40,7 +40,6 @@ gem 'actionview', '~> 8.1.3' gem 'activemodel', '~> 8.1.2' gem 'railties', '~> 8.1.1' -gem 'fog-aws' gem 'fog-core', '~> 2.6.0' gem 'cf-uaa-lib', '~> 4.0.10' diff --git a/Gemfile.lock b/Gemfile.lock index b375794c558..101913b4886 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -90,22 +90,11 @@ GEM fluent-logger (0.10.0) logger (~> 1.6) msgpack (>= 1.0.0, < 2) - fog-aws (3.33.2) - base64 (>= 0.2, < 0.4) - fog-core (~> 2.6) - fog-json (~> 1.1) - fog-xml (~> 0.1) fog-core (2.6.0) builder excon (~> 1.0) formatador (>= 0.2, < 2.0) mime-types - fog-json (1.3.0) - fog-core - multi_json (~> 1.10) - fog-xml (0.1.5) - fog-core - nokogiri (>= 1.5.11, < 2.0.0) formatador (1.2.3) reline google-protobuf (4.35.1) @@ -481,7 +470,6 @@ DEPENDENCIES digest-xxhash factory_bot (~> 6.5) fluent-logger - fog-aws fog-core (~> 2.6.0) googleapis-common-protos (>= 1.8.0) hashdiff diff --git a/config/cloud_controller.yml b/config/cloud_controller.yml index c2e7d248ab8..7f79f98ecfd 100644 --- a/config/cloud_controller.yml +++ b/config/cloud_controller.yml @@ -183,7 +183,6 @@ resource_pool: resource_directory_key: "spec-cc-resources" blobstore_type: local-temp-storage fog_connection: {} - fog_aws_storage_options: {} packages: app_package_directory_key: "cc-packages" @@ -191,20 +190,17 @@ packages: max_valid_packages_stored: 42 blobstore_type: local-temp-storage fog_connection: {} - fog_aws_storage_options: {} droplets: droplet_directory_key: cc-droplets max_staged_droplets_stored: 42 blobstore_type: local-temp-storage fog_connection: {} - fog_aws_storage_options: {} buildpacks: buildpack_directory_key: cc-buildpacks blobstore_type: local-temp-storage fog_connection: {} - fog_aws_storage_options: {} db_encryption_key: "asdfasdfasdf" database_encryption: diff --git a/lib/cloud_controller/blobstore/client_provider.rb b/lib/cloud_controller/blobstore/client_provider.rb index 0c7572f86f4..075d294b093 100644 --- a/lib/cloud_controller/blobstore/client_provider.rb +++ b/lib/cloud_controller/blobstore/client_provider.rb @@ -38,17 +38,14 @@ def provide_fog(options, directory_key, root_dir) cdn: cdn, root_dir: root_dir, min_size: options[:minimum_size], - max_size: options[:maximum_size], - aws_storage_options: options[:fog_aws_storage_options] + max_size: options[:maximum_size] ) logger = Steno.logger('cc.blobstore') # work around https://github.com/fog/fog/issues/3137 # and Fog raising an EOFError SocketError intermittently - # and https://github.com/fog/fog-aws/issues/264 - # and https://github.com/fog/fog-aws/issues/265 - # and intermittent GCS blobstore download errors + # and intermittent blobstore download errors errors = [Excon::Errors::BadRequest, Excon::Errors::SocketError, SystemCallError, Excon::Errors::InternalServerError, Excon::Errors::ServiceUnavailable, OpenSSL::OpenSSLError] retryable_client = RetryableClient.new(client:, errors:, logger:) diff --git a/lib/cloud_controller/blobstore/fog/fog_client.rb b/lib/cloud_controller/blobstore/fog/fog_client.rb index 4350d01c9e2..b56ce2a2aba 100644 --- a/lib/cloud_controller/blobstore/fog/fog_client.rb +++ b/lib/cloud_controller/blobstore/fog/fog_client.rb @@ -22,15 +22,14 @@ def initialize(connection_config:, cdn: nil, root_dir: nil, min_size: nil, - max_size: nil, - aws_storage_options: nil) + max_size: nil) +) @root_dir = root_dir @connection_config = connection_config @directory_key = directory_key @cdn = cdn @min_size = min_size || 0 @max_size = max_size - @aws_storage_options = aws_storage_options logger.warn('blobstore.fog-deprecated', message: DEPRECATION_MESSAGE) end @@ -132,18 +131,7 @@ def files end def formatted_storage_options - if connection.service.equal?(Fog::AWS::Storage) - return {} unless @aws_storage_options && @aws_storage_options[:encryption] - - opts = @aws_storage_options.dup - encrypt_opt = opts.delete(:encryption) - opts['x-amz-server-side-encryption'] = encrypt_opt - opts - - else - {} - - end + {} end def delete_file(file) @@ -152,9 +140,6 @@ def delete_file(file) def delete_files(files_to_delete, page_size) if connection.respond_to?(:delete_multiple_objects) - # AWS needs the file key to work; other providers with multiple delete - # are currently not supported. When support is added this code may - # need an update. each_slice(files_to_delete, page_size) do |file_group| connection.delete_multiple_objects(@directory_key, file_group.map(&:key)) end diff --git a/lib/cloud_controller/blobstore/fog/providers.rb b/lib/cloud_controller/blobstore/fog/providers.rb index 7dc84f65359..f79351e864e 100644 --- a/lib/cloud_controller/blobstore/fog/providers.rb +++ b/lib/cloud_controller/blobstore/fog/providers.rb @@ -1,8 +1 @@ require 'fog/core' - -original = Fog::Logger[:deprecation] -Fog::Logger[:deprecation] = nil - -require 'fog/aws' - -Fog::Logger[:deprecation] = original diff --git a/lib/cloud_controller/blobstore/storage_cli/storage_cli_client.rb b/lib/cloud_controller/blobstore/storage_cli/storage_cli_client.rb index 57cb78cca43..960ff2a1298 100644 --- a/lib/cloud_controller/blobstore/storage_cli/storage_cli_client.rb +++ b/lib/cloud_controller/blobstore/storage_cli/storage_cli_client.rb @@ -22,7 +22,6 @@ class StorageCliClient < BaseClient # DEPRECATED: Legacy fog provider names (remove after migration window) LEGACY_PROVIDER_TO_STORAGE_CLI_TYPE = { - 'AWS' => 's3', 'webdav' => 'dav' }.freeze diff --git a/lib/cloud_controller/config_schemas/api_schema.rb b/lib/cloud_controller/config_schemas/api_schema.rb index ad57647eea9..eb421c10577 100644 --- a/lib/cloud_controller/config_schemas/api_schema.rb +++ b/lib/cloud_controller/config_schemas/api_schema.rb @@ -215,7 +215,6 @@ class ApiSchema < VCAP::Config optional(:local_blobstore_path) => String, fog_connection: Hash, optional(:connection_config) => Hash, - fog_aws_storage_options: Hash, optional(:webdav_config) => Hash, optional(:cdn) => Hash }, @@ -226,7 +225,6 @@ class ApiSchema < VCAP::Config optional(:local_blobstore_path) => String, fog_connection: Hash, optional(:connection_config) => Hash, - fog_aws_storage_options: Hash, optional(:webdav_config) => Hash, optional(:cdn) => Hash }, @@ -239,7 +237,6 @@ class ApiSchema < VCAP::Config optional(:local_blobstore_path) => String, fog_connection: Hash, optional(:connection_config) => Hash, - fog_aws_storage_options: Hash, optional(:webdav_config) => Hash, optional(:cdn) => Hash }, @@ -251,7 +248,6 @@ class ApiSchema < VCAP::Config optional(:local_blobstore_path) => String, fog_connection: Hash, optional(:connection_config) => Hash, - fog_aws_storage_options: Hash, optional(:webdav_config) => Hash, optional(:cdn) => Hash }, diff --git a/lib/cloud_controller/config_schemas/blobstore_benchmarks_schema.rb b/lib/cloud_controller/config_schemas/blobstore_benchmarks_schema.rb index dd7a772f766..d60327729a7 100644 --- a/lib/cloud_controller/config_schemas/blobstore_benchmarks_schema.rb +++ b/lib/cloud_controller/config_schemas/blobstore_benchmarks_schema.rb @@ -15,7 +15,6 @@ class BlobstoreBenchmarksSchema < VCAP::Config optional(:webdav_config) => Hash, optional(:cdn) => Hash, - fog_aws_storage_options: Hash, optional(:resource_directory_key) => String, optional(:buildpack_directory_key) => String, diff --git a/lib/cloud_controller/config_schemas/clock_schema.rb b/lib/cloud_controller/config_schemas/clock_schema.rb index 68a0433be46..5f79e05f4d2 100644 --- a/lib/cloud_controller/config_schemas/clock_schema.rb +++ b/lib/cloud_controller/config_schemas/clock_schema.rb @@ -130,7 +130,6 @@ class ClockSchema < VCAP::Config optional(:local_blobstore_path) => String, fog_connection: Hash, optional(:connection_config) => Hash, - fog_aws_storage_options: Hash, optional(:webdav_config) => Hash, optional(:cdn) => Hash }, @@ -141,7 +140,6 @@ class ClockSchema < VCAP::Config optional(:local_blobstore_path) => String, fog_connection: Hash, optional(:connection_config) => Hash, - fog_aws_storage_options: Hash, optional(:webdav_config) => Hash, optional(:cdn) => Hash }, @@ -153,7 +151,6 @@ class ClockSchema < VCAP::Config optional(:local_blobstore_path) => String, fog_connection: Hash, optional(:connection_config) => Hash, - fog_aws_storage_options: Hash, optional(:webdav_config) => Hash, optional(:cdn) => Hash }, @@ -164,7 +161,6 @@ class ClockSchema < VCAP::Config optional(:local_blobstore_path) => String, fog_connection: Hash, optional(:connection_config) => Hash, - fog_aws_storage_options: Hash, optional(:webdav_config) => Hash, optional(:cdn) => Hash }, diff --git a/lib/cloud_controller/config_schemas/deployment_updater_schema.rb b/lib/cloud_controller/config_schemas/deployment_updater_schema.rb index 057bfe90d3e..9936e8343b0 100644 --- a/lib/cloud_controller/config_schemas/deployment_updater_schema.rb +++ b/lib/cloud_controller/config_schemas/deployment_updater_schema.rb @@ -125,7 +125,6 @@ class DeploymentUpdaterSchema < VCAP::Config optional(:local_blobstore_path) => String, fog_connection: Hash, optional(:connection_config) => Hash, - fog_aws_storage_options: Hash, optional(:webdav_config) => Hash, optional(:cdn) => Hash }, @@ -136,7 +135,6 @@ class DeploymentUpdaterSchema < VCAP::Config optional(:local_blobstore_path) => String, fog_connection: Hash, optional(:connection_config) => Hash, - fog_aws_storage_options: Hash, optional(:webdav_config) => Hash, optional(:cdn) => Hash }, @@ -148,7 +146,6 @@ class DeploymentUpdaterSchema < VCAP::Config optional(:local_blobstore_path) => String, fog_connection: Hash, optional(:connection_config) => Hash, - fog_aws_storage_options: Hash, optional(:webdav_config) => Hash, optional(:cdn) => Hash }, @@ -159,7 +156,6 @@ class DeploymentUpdaterSchema < VCAP::Config optional(:local_blobstore_path) => String, fog_connection: Hash, optional(:connection_config) => Hash, - fog_aws_storage_options: Hash, optional(:webdav_config) => Hash, optional(:cdn) => Hash }, diff --git a/lib/cloud_controller/config_schemas/worker_schema.rb b/lib/cloud_controller/config_schemas/worker_schema.rb index f4f7e753964..13991bd1c62 100644 --- a/lib/cloud_controller/config_schemas/worker_schema.rb +++ b/lib/cloud_controller/config_schemas/worker_schema.rb @@ -116,7 +116,6 @@ class WorkerSchema < VCAP::Config optional(:local_blobstore_path) => String, fog_connection: Hash, optional(:connection_config) => Hash, - fog_aws_storage_options: Hash, optional(:webdav_config) => Hash, optional(:cdn) => Hash }, @@ -127,7 +126,6 @@ class WorkerSchema < VCAP::Config optional(:local_blobstore_path) => String, fog_connection: Hash, optional(:connection_config) => Hash, - fog_aws_storage_options: Hash, optional(:webdav_config) => Hash, optional(:cdn) => Hash }, @@ -140,7 +138,6 @@ class WorkerSchema < VCAP::Config optional(:local_blobstore_path) => String, fog_connection: Hash, optional(:connection_config) => Hash, - fog_aws_storage_options: Hash, optional(:webdav_config) => Hash, optional(:cdn) => Hash }, @@ -151,7 +148,6 @@ class WorkerSchema < VCAP::Config optional(:local_blobstore_path) => String, fog_connection: Hash, optional(:connection_config) => Hash, - fog_aws_storage_options: Hash, optional(:webdav_config) => Hash, optional(:cdn) => Hash }, diff --git a/spec/api/documentation/buildpack_cache_api_spec.rb b/spec/api/documentation/buildpack_cache_api_spec.rb index 16ec1789c60..e273a7745c1 100644 --- a/spec/api/documentation/buildpack_cache_api_spec.rb +++ b/spec/api/documentation/buildpack_cache_api_spec.rb @@ -17,9 +17,7 @@ droplets: { droplet_directory_key: 'cc-droplets', fog_connection: { - provider: 'AWS', - aws_access_key_id: 'fake', - aws_secret_access_key: 'fake' + provider: 'local' } }, directories: { diff --git a/spec/fixtures/config/port_8181_config.yml b/spec/fixtures/config/port_8181_config.yml index 215ef29245b..d6f512dfabd 100644 --- a/spec/fixtures/config/port_8181_config.yml +++ b/spec/fixtures/config/port_8181_config.yml @@ -141,19 +141,15 @@ name: api resource_pool: maximum_size: 42 minimum_size: 1 - fog_aws_storage_options: {} buildpacks: - fog_aws_storage_options: {} packages: max_package_size: 42 max_valid_packages_stored: 42 - fog_aws_storage_options: {} droplets: max_staged_droplets_stored: 42 - fog_aws_storage_options: {} request_timeout_in_seconds: 600 skip_cert_verify: true @@ -176,25 +172,21 @@ resource_pool: maximum_size: 42 minimum_size: 1 fog_connection: {} - fog_aws_storage_options: {} buildpacks: buildpack_directory_key: '' fog_connection: {} - fog_aws_storage_options: {} packages: app_package_directory_key: '' max_package_size: 42 max_valid_packages_stored: 42 fog_connection: {} - fog_aws_storage_options: {} droplets: droplet_directory_key: '' max_staged_droplets_stored: 42 fog_connection: {} - fog_aws_storage_options: {} diff --git a/spec/support/bootstrap/test_config.rb b/spec/support/bootstrap/test_config.rb index cb72ca18060..b09d4686f7b 100644 --- a/spec/support/bootstrap/test_config.rb +++ b/spec/support/bootstrap/test_config.rb @@ -42,31 +42,24 @@ def defaults config_file = File.join(Paths::CONFIG, 'cloud_controller.yml') config_hash = VCAP::CloudController::Config.load_from_file(config_file, context:).config_hash - fog_connection = { - blobstore_timeout: 5, - provider: 'AWS', - aws_access_key_id: 'fake_aws_key_id', - aws_secret_access_key: 'fake_secret_access_key' - } - config_hash.update( nginx: { use_nginx: true }, resource_pool: { resource_directory_key: 'spec-cc-resources', - fog_connection: fog_connection + fog_connection: {} }, packages: { app_package_directory_key: 'cc-packages', - fog_connection: fog_connection, + fog_connection: {}, max_valid_packages_stored: 42 }, buildpacks: { buildpack_directory_key: 'cc-buildpacks', - fog_connection: fog_connection + fog_connection: {} }, droplets: { droplet_directory_key: 'cc-droplets', - fog_connection: fog_connection, + fog_connection: {}, max_staged_droplets_stored: 42 }, db: DbConfig.new.config diff --git a/spec/support/shared_examples/controllers/resource_pool.rb b/spec/support/shared_examples/controllers/resource_pool.rb index f6fc63cfdba..56fbff4f3ea 100644 --- a/spec/support/shared_examples/controllers/resource_pool.rb +++ b/spec/support/shared_examples/controllers/resource_pool.rb @@ -43,11 +43,7 @@ maximum_size: maximum_file_size, minimum_size: minimum_file_size, resource_directory_key: 'spec-cc-resources', - fog_connection: { - provider: 'AWS', - aws_access_key_id: 'fake_aws_key_id', - aws_secret_access_key: 'fake_secret_access_key' - } + fog_connection: {} } end diff --git a/spec/unit/controllers/internal/download_droplets_controller_spec.rb b/spec/unit/controllers/internal/download_droplets_controller_spec.rb index a91108cca4c..64ad88da660 100644 --- a/spec/unit/controllers/internal/download_droplets_controller_spec.rb +++ b/spec/unit/controllers/internal/download_droplets_controller_spec.rb @@ -9,18 +9,14 @@ module VCAP::CloudController { packages: { fog_connection: { - provider: 'AWS', - aws_access_key_id: 'fake', - aws_secret_access_key: 'fake' + provider: 'local' }, app_package_directory_key: 'cc-packages' }, droplets: { droplet_directory_key: 'cc-droplets', fog_connection: { - provider: 'AWS', - aws_access_key_id: 'fake', - aws_secret_access_key: 'fake' + provider: 'local' } } } @@ -101,18 +97,14 @@ def upload_droplet { packages: { fog_connection: { - provider: 'AWS', - aws_access_key_id: 'fake', - aws_secret_access_key: 'fake' + provider: 'local' }, app_package_directory_key: 'cc-packages' }, droplets: { droplet_directory_key: 'cc-droplets', fog_connection: { - provider: 'AWS', - aws_access_key_id: 'fake', - aws_secret_access_key: 'fake' + provider: 'local' } } } diff --git a/spec/unit/controllers/runtime/stagings_controller_spec.rb b/spec/unit/controllers/runtime/stagings_controller_spec.rb index f8a600c11ef..4ab38792cdb 100644 --- a/spec/unit/controllers/runtime/stagings_controller_spec.rb +++ b/spec/unit/controllers/runtime/stagings_controller_spec.rb @@ -198,25 +198,19 @@ module VCAP::CloudController resource_pool: { resource_directory_key: 'cc-resources', fog_connection: { - provider: 'AWS', - aws_access_key_id: 'fake', - aws_secret_access_key: 'fake' + provider: 'local' } }, packages: { fog_connection: { - provider: 'AWS', - aws_access_key_id: 'fake', - aws_secret_access_key: 'fake' + provider: 'local' }, app_package_directory_key: 'cc-packages' }, droplets: { droplet_directory_key: 'cc-droplets', fog_connection: { - provider: 'AWS', - aws_access_key_id: 'fake', - aws_secret_access_key: 'fake' + provider: 'local' } }, directories: { diff --git a/spec/unit/jobs/runtime/buildpack_cache_cleanup_spec.rb b/spec/unit/jobs/runtime/buildpack_cache_cleanup_spec.rb index 9c5a7472a6c..9d39c9997c4 100644 --- a/spec/unit/jobs/runtime/buildpack_cache_cleanup_spec.rb +++ b/spec/unit/jobs/runtime/buildpack_cache_cleanup_spec.rb @@ -18,9 +18,7 @@ module Jobs::Runtime droplets: { droplet_directory_key: 'cc-droplets', fog_connection: { - provider: 'AWS', - aws_access_key_id: 'fake', - aws_secret_access_key: 'fake' + provider: 'local' } }, directories: { diff --git a/spec/unit/jobs/v3/buildpack_cache_cleanup_spec.rb b/spec/unit/jobs/v3/buildpack_cache_cleanup_spec.rb index 0f81ecb965e..ed61dd6628a 100644 --- a/spec/unit/jobs/v3/buildpack_cache_cleanup_spec.rb +++ b/spec/unit/jobs/v3/buildpack_cache_cleanup_spec.rb @@ -18,9 +18,7 @@ module Jobs::V3 droplets: { droplet_directory_key: 'cc-droplets', fog_connection: { - provider: 'AWS', - aws_access_key_id: 'fake', - aws_secret_access_key: 'fake' + provider: 'local' } }, directories: { diff --git a/spec/unit/jobs/v3/buildpack_cache_delete_spec.rb b/spec/unit/jobs/v3/buildpack_cache_delete_spec.rb index 25c0aec0352..9578c513f19 100644 --- a/spec/unit/jobs/v3/buildpack_cache_delete_spec.rb +++ b/spec/unit/jobs/v3/buildpack_cache_delete_spec.rb @@ -6,7 +6,7 @@ module Jobs::V3 RSpec.describe BuildpackCacheDelete, job_context: :worker do let(:app_guid) { 'some-guid' } let!(:blobstore) do - CloudController::Blobstore::FogClient.new(connection_config: { provider: 'AWS', aws_access_key_id: 'fake', aws_secret_access_key: 'fake' }, + CloudController::Blobstore::FogClient.new(connection_config: { provider: 'local' }, directory_key: 'directory_key') end let(:path_1) { Presenters::V3::CacheKeyPresenter.cache_key(guid: app_guid, stack_name: 'stack1') } diff --git a/spec/unit/jobs/v3/droplet_bits_copier_spec.rb b/spec/unit/jobs/v3/droplet_bits_copier_spec.rb index fee205ba73a..b042247cf68 100644 --- a/spec/unit/jobs/v3/droplet_bits_copier_spec.rb +++ b/spec/unit/jobs/v3/droplet_bits_copier_spec.rb @@ -7,7 +7,7 @@ module Jobs::V3 let(:droplet_bits_path) { File.expand_path('../../../fixtures/good.zip', File.dirname(__FILE__)) } let(:droplet_blobstore) do - CloudController::Blobstore::FogClient.new(connection_config: { provider: 'AWS', aws_access_key_id: 'fake', aws_secret_access_key: 'fake' }, + CloudController::Blobstore::FogClient.new(connection_config: { provider: 'local' }, directory_key: 'droplet') end let(:source_droplet) { create(:droplet_model, droplet_hash: 'abcdef1234', sha256_checksum: '4321fedcba', state: DropletModel::STAGED_STATE, set_as_current_droplet: false) } diff --git a/spec/unit/jobs/v3/package_bits_copier_spec.rb b/spec/unit/jobs/v3/package_bits_copier_spec.rb index 750d7c17d80..b096c6daad8 100644 --- a/spec/unit/jobs/v3/package_bits_copier_spec.rb +++ b/spec/unit/jobs/v3/package_bits_copier_spec.rb @@ -7,7 +7,7 @@ module Jobs::V3 let(:package_bits_path) { File.expand_path('../../../fixtures/good.zip', File.dirname(__FILE__)) } let(:package_blobstore) do - CloudController::Blobstore::FogClient.new(connection_config: { provider: 'AWS', aws_access_key_id: 'fake', aws_secret_access_key: 'fake' }, + CloudController::Blobstore::FogClient.new(connection_config: { provider: 'local' }, directory_key: 'package') end let(:source_package) { create(:package_model, type: 'bits', package_hash: 'something', sha256_checksum: 'sha256') } diff --git a/spec/unit/lib/cloud_controller/blobstore/client_provider_spec.rb b/spec/unit/lib/cloud_controller/blobstore/client_provider_spec.rb index 1bf33355a52..c183292c644 100644 --- a/spec/unit/lib/cloud_controller/blobstore/client_provider_spec.rb +++ b/spec/unit/lib/cloud_controller/blobstore/client_provider_spec.rb @@ -32,36 +32,6 @@ module Blobstore expect(FogClient).to have_received(:new) end - context 'when an aws encryption option is requested' do - before do - options.merge!(fog_aws_storage_options: { encryption: 'my organic algo' }) - end - - it 'passes the specified encryption option to the fog client' do - allow(FogClient).to receive(:new).and_call_original - ClientProvider.provide(options: options, directory_key: 'key') - expect(FogClient).to have_received(:new).with(connection_config: anything, - directory_key: anything, - cdn: anything, - root_dir: anything, - min_size: anything, - max_size: anything, - aws_storage_options: { encryption: 'my organic algo' }) - end - - context 'fog methods' do - describe '#download_from_blobstore' do - it 'receives all arguments' do - allow_any_instance_of(FogClient).to receive(:download_from_blobstore).and_return(nil) - - client = ClientProvider.provide(options: options, directory_key: 'key') - expect_any_instance_of(FogClient).to receive(:download_from_blobstore).with('key', 'dest', mode: 775) - client.download_from_blobstore('key', 'dest', mode: 775) - end - end - end - end - context 'when a cdn is requested in the options' do before do options.merge!(cdn: { uri: 'http://cdn.com' }) @@ -75,8 +45,7 @@ module Blobstore cdn: an_instance_of(Cdn), root_dir: anything, min_size: anything, - max_size: anything, - aws_storage_options: anything) + max_size: anything) end end diff --git a/spec/unit/lib/cloud_controller/blobstore/fog/fog_client_spec.rb b/spec/unit/lib/cloud_controller/blobstore/fog/fog_client_spec.rb index d13bf2e0125..2d7ef7c9821 100644 --- a/spec/unit/lib/cloud_controller/blobstore/fog/fog_client_spec.rb +++ b/spec/unit/lib/cloud_controller/blobstore/fog/fog_client_spec.rb @@ -1,8 +1,6 @@ require 'spec_helper' require 'webrick' require_relative '../client_shared' -require 'fog/aws/models/storage/files' -require 'fog/aws/models/storage/directories' module CloudController module Blobstore @@ -12,9 +10,7 @@ module Blobstore let(:local_dir) { Dir.mktmpdir } let(:connection_config) do { - provider: 'AWS', - aws_access_key_id: 'fake_access_key_id', - aws_secret_access_key: 'fake_secret_access_key' + provider: 'local' } end @@ -341,48 +337,6 @@ def upload_tmpfile(client, key='abcdef') before do allow_any_instance_of(FogClient).to receive(:files).and_return(files) end - - context 'aws' do - context 'when encryption type is specified' do - let(:client_with_encryption) do - FogClient.new(connection_config: connection_config, - directory_key: directory_key, - aws_storage_options: { encryption: 'my-algo' }) - end - - it 'passes the encryption options to aws' do - path = File.join(local_dir, 'empty_file.png') - FileUtils.touch(path) - - client_with_encryption.cp_to_blobstore(path, 'abcdef123456') - - expect(files).to have_received(:create).with(key: anything, - body: anything, - content_type: anything, - public: anything, - 'x-amz-server-side-encryption' => 'my-algo') - end - end - - context 'when encryption type is not specified' do - let(:client_with_encryption) do - FogClient.new(connection_config:, - directory_key:) - end - - it 'does not pass the encryption options to aws' do - path = File.join(local_dir, 'empty_file.png') - FileUtils.touch(path) - - client_with_encryption.cp_to_blobstore(path, 'abcdef123456') - - expect(files).to have_received(:create).with(key: anything, - body: anything, - content_type: anything, - public: anything) - end - end - end end end @@ -428,38 +382,6 @@ def upload_tmpfile(client, key='abcdef') end end - context 'encryption' do - let(:encryption) { 'my-algo' } - let(:client) do - FogClient.new(connection_config: connection_config, - directory_key: directory_key, - aws_storage_options: { encryption: encryption, other: 'thing' }) - end - let(:dest_file) { double(:file, copy: true, save: true, nil?: false) } - let(:src_file) { double(:file, copy: true, nil?: false) } - - before do - allow_any_instance_of(FogClient).to receive(:file).with(src_key).and_return(src_file) - allow_any_instance_of(FogClient).to receive(:file).with(dest_key).and_return(dest_file) - end - - context 'when encryption type is specified' do - it 'passes the encryption options to aws' do - client.cp_file_between_keys(src_key, dest_key) - options = { 'x-amz-server-side-encryption' => 'my-algo', other: 'thing' } - expect(src_file).to have_received(:copy).with('a-directory-key', 'xy/z7/xyz789', options) - end - end - - context 'when encryption type is not specified' do - let(:encryption) { nil } - - it 'does not pass the encryption options to aws' do - client.cp_file_between_keys(src_key, dest_key) - expect(src_file).to have_received(:copy).with('a-directory-key', 'xy/z7/xyz789', {}) - end - end - end end describe '#delete_all' do @@ -491,44 +413,6 @@ def upload_tmpfile(client, key='abcdef') end.not_to raise_error end - context 'when the underlying blobstore allows multiple deletes in a single request' do - let(:connection_config) do - { - provider: 'AWS', - aws_access_key_id: 'fake_access_key_id', - aws_secret_access_key: 'fake_secret_access_key' - } - end - - it 'is ok if there are no files' do - expect(directory.files).to have(0).items - expect do - client.delete_all - end.not_to raise_error - end - - it 'deletes in groups of the page_size' do - connection = client.send(:connection) - allow(connection).to receive(:delete_multiple_objects) - - file = File.join(local_dir, 'empty_file') - FileUtils.touch(file) - - client.cp_to_blobstore(file, 'abcdef1') - client.cp_to_blobstore(file, 'abcdef2') - client.cp_to_blobstore(file, 'abcdef3') - expect(client).to exist('abcdef1') - expect(client).to exist('abcdef2') - expect(client).to exist('abcdef3') - - page_size = 2 - client.delete_all(page_size) - - expect(connection).to have_received(:delete_multiple_objects).with(directory_key, ['ab/cd/abcdef1', 'ab/cd/abcdef2']) - expect(connection).to have_received(:delete_multiple_objects).with(directory_key, ['ab/cd/abcdef3']) - end - end - context 'when a root dir is provided' do let(:root_dir) { 'root-dir' } @@ -598,24 +482,6 @@ def upload_tmpfile(client, key='abcdef') end.not_to raise_error end - context 'when the underlying blobstore allows multiple deletes in a single request' do - let(:connection_config) do - { - provider: 'AWS', - aws_access_key_id: 'fake_access_key_id', - aws_secret_access_key: 'fake_secret_access_key' - } - end - - it 'is ok if there are no files' do - Fog.mock! - expect(directory.files).to have(0).items - expect do - client.delete_all_in_path('path!') - end.not_to raise_error - end - end - context 'when a root dir is provided' do let(:root_dir) { 'root-dir' } @@ -704,21 +570,22 @@ def upload_tmpfile(client, key='abcdef') end it 'gets the bucket' do - expect_any_instance_of(Fog::AWS::Storage::Directories).to receive(:get).with(directory_key, max_keys: 1) + expect(subject.send(:connection).directories).to receive(:get).with(directory_key, max_keys: 1).and_call_original subject.ensure_bucket_exists end context 'the bucket exists' do it 'does not create the bucket' do subject.ensure_bucket_exists - expect_any_instance_of(Fog::AWS::Storage::Directories).not_to receive(:create).with(key: directory_key, public: false) + expect(subject.send(:connection).directories).not_to receive(:create).with(key: directory_key, public: false) subject.ensure_bucket_exists end end context 'the bucket does not exist' do it 'creates the bucket' do - expect_any_instance_of(Fog::AWS::Storage::Directories).to receive(:create).with(key: directory_key, public: false) + allow(subject.send(:connection).directories).to receive(:get).with(directory_key, max_keys: 1).and_return(nil) + expect(subject.send(:connection).directories).to receive(:create).with(key: directory_key, public: false) subject.ensure_bucket_exists end end diff --git a/spec/unit/lib/cloud_controller/blobstore/storage_cli/storage_cli_client_spec.rb b/spec/unit/lib/cloud_controller/blobstore/storage_cli/storage_cli_client_spec.rb index 03958760213..d6d98845285 100644 --- a/spec/unit/lib/cloud_controller/blobstore/storage_cli/storage_cli_client_spec.rb +++ b/spec/unit/lib/cloud_controller/blobstore/storage_cli/storage_cli_client_spec.rb @@ -22,27 +22,6 @@ def stub_config_for_droplets(path) describe 'client init' do # DEPRECATED: Legacy fog provider tests - remove after migration window # START LEGACY FOG SUPPORT TESTS - it 'maps AWS legacy provider to s3 storage-cli type' do - droplets_cfg = write_config_file( - provider: 'AWS', - bucket_name: 'test-bucket', - access_key_id: 'key', - secret_access_key: 'secret' - ) - begin - stub_config_for_droplets(droplets_cfg.path) - - client = StorageCliClient.new( - directory_key: 'dummy-key', - root_dir: 'dummy-root', - resource_type: 'droplets' - ) - expect(client.instance_variable_get(:@storage_type)).to eq('s3') - ensure - droplets_cfg.close! - end - end - it 'maps webdav legacy provider to dav storage-cli type' do droplets_cfg = write_config_file( provider: 'webdav', diff --git a/spec/unit/lib/cloud_controller/config_spec.rb b/spec/unit/lib/cloud_controller/config_spec.rb index 294fb3ab1b4..7cfa2885b9d 100644 --- a/spec/unit/lib/cloud_controller/config_spec.rb +++ b/spec/unit/lib/cloud_controller/config_spec.rb @@ -6,9 +6,6 @@ module VCAP::CloudController { packages: { fog_connection: {}, - fog_aws_storage_options: { - encryption: 'AES256' - }, app_package_directory_key: 'app_key' }, droplets: { @@ -385,9 +382,6 @@ module VCAP::CloudController { packages: { fog_connection: {}, - fog_aws_storage_options: { - encryption: 'AES256' - }, app_package_directory_key: 'app_key' }, droplets: { @@ -547,12 +541,8 @@ module VCAP::CloudController it 'returns a hash for nested properties' do expect(config_instance.get(:packages)).to eq({ fog_connection: {}, - fog_aws_storage_options: { - encryption: 'AES256' - }, app_package_directory_key: 'app_key' }) - expect(config_instance.get(:packages, :fog_aws_storage_options)).to eq(encryption: 'AES256') end it 'raises an exception when given an invalid key' do @@ -567,12 +557,6 @@ module VCAP::CloudController end.to raise_error Config::InvalidConfigPath, /"external_domain.pantaloons" is not a valid config key/ end - it 'raises when you dig into hashes' do - expect do - config_instance.get(:packages, :fog_aws_storage_options, :encryption) - end.to raise_error Config::InvalidConfigPath, /"packages.fog_aws_storage_options.encryption" is not a valid config key/ - end - it 'raises when given a path with an invalid key' do expect do config_instance.get(:packages, :ham_sandwich) diff --git a/spec/unit/lib/cloud_controller/packager/local_bits_packer_spec.rb b/spec/unit/lib/cloud_controller/packager/local_bits_packer_spec.rb index 6a24e3ce9a9..5cf9dab68ba 100644 --- a/spec/unit/lib/cloud_controller/packager/local_bits_packer_spec.rb +++ b/spec/unit/lib/cloud_controller/packager/local_bits_packer_spec.rb @@ -12,7 +12,7 @@ module CloudController::Packager let(:max_size) { 8 } let(:global_app_bits_cache) do CloudController::Blobstore::FogClient.new( - connection_config: { provider: 'AWS', aws_access_key_id: 'fake', aws_secret_access_key: 'fake' }, + connection_config: { provider: 'local' }, directory_key: 'global_app_bits_cache', min_size: min_size, max_size: max_size @@ -20,7 +20,7 @@ module CloudController::Packager end let(:package_blobstore) do CloudController::Blobstore::FogClient.new( - connection_config: { provider: 'AWS', aws_access_key_id: 'fake', aws_secret_access_key: 'fake' }, + connection_config: { provider: 'local' }, directory_key: 'package' ) end From 36f87d7b91df74249ec285c3b745c3c8949a1d00 Mon Sep 17 00:00:00 2001 From: Katharina Przybill <30441792+kathap@users.noreply.github.com> Date: Tue, 14 Jul 2026 13:36:37 +0200 Subject: [PATCH 02/24] Fix broken specs after fog-aws removal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace obsolete FogClient/fog_connection test setup with LocalClient and blobstore_type: 'local'. Job and controller specs that used FogClient as a test double now use LocalClient directly; config-based specs switch from fog_connection to blobstore_type/local_blobstore_path. Remove Fog::Mock.reset calls that are no longer needed. Delete fog_client_spec.rb — it tested the deprecated FogClient which requires fog provider gems no longer bundled. * Tests were broken because fog-aws removal left no registered fog provider, so Fog::Storage.new(provider: ...) raised ArgumentError at runtime. * Blobstore behavior under test is the job/controller logic, not fog internals, so LocalClient is the right substitute. --- .rubocop_cc.yml | 31 + .../blobstore/fog/fog_client.rb | 1 - .../blobstore_benchmarks_schema.rb | 1 - .../documentation/buildpack_cache_api_spec.rb | 7 +- .../controllers/resource_pool.rb | 7 +- .../download_droplets_controller_spec.rb | 28 +- .../runtime/stagings_controller_spec.rb | 16 +- .../runtime/buildpack_cache_cleanup_spec.rb | 6 +- .../jobs/v3/buildpack_cache_cleanup_spec.rb | 6 +- .../jobs/v3/buildpack_cache_delete_spec.rb | 8 +- spec/unit/jobs/v3/droplet_bits_copier_spec.rb | 11 +- spec/unit/jobs/v3/package_bits_copier_spec.rb | 11 +- .../blobstore/fog/fog_client_spec.rb | 700 ------------------ .../packager/local_bits_packer_spec.rb | 18 +- 14 files changed, 69 insertions(+), 782 deletions(-) delete mode 100644 spec/unit/lib/cloud_controller/blobstore/fog/fog_client_spec.rb diff --git a/.rubocop_cc.yml b/.rubocop_cc.yml index 4ddf73c0647..b2516a74d23 100644 --- a/.rubocop_cc.yml +++ b/.rubocop_cc.yml @@ -795,3 +795,34 @@ Rails/RedundantActiveRecordAllMethod: # As we use Sequel this breaks code as it Enabled: false Sequel/SaveChanges: # Breaks Code Enabled: false + + +# New cops in RuboCop 1.85 / rubocop-rspec 3.10 +Lint/DataDefineOverride: + Enabled: true +Lint/UnreachablePatternBranch: + Enabled: true +Style/FileOpen: + Enabled: true +Style/MapJoin: + Enabled: true +Style/OneClassPerFile: + Enabled: false +Style/PartitionInsteadOfDoubleSelect: + Enabled: true +Style/PredicateWithKind: + Enabled: true +Style/ReduceToHash: + Enabled: true +Style/RedundantMinMaxBy: + Enabled: true +Style/SelectByKind: + Enabled: true +Style/SelectByRange: + Enabled: true +Style/TallyMethod: + Enabled: true +RSpec/DiscardedMatcher: + Enabled: true +RSpec/MatchWithSimpleRegex: + Enabled: true diff --git a/lib/cloud_controller/blobstore/fog/fog_client.rb b/lib/cloud_controller/blobstore/fog/fog_client.rb index b56ce2a2aba..7bf689a421b 100644 --- a/lib/cloud_controller/blobstore/fog/fog_client.rb +++ b/lib/cloud_controller/blobstore/fog/fog_client.rb @@ -23,7 +23,6 @@ def initialize(connection_config:, root_dir: nil, min_size: nil, max_size: nil) -) @root_dir = root_dir @connection_config = connection_config @directory_key = directory_key diff --git a/lib/cloud_controller/config_schemas/blobstore_benchmarks_schema.rb b/lib/cloud_controller/config_schemas/blobstore_benchmarks_schema.rb index d60327729a7..d924ea4910d 100644 --- a/lib/cloud_controller/config_schemas/blobstore_benchmarks_schema.rb +++ b/lib/cloud_controller/config_schemas/blobstore_benchmarks_schema.rb @@ -15,7 +15,6 @@ class BlobstoreBenchmarksSchema < VCAP::Config optional(:webdav_config) => Hash, optional(:cdn) => Hash, - optional(:resource_directory_key) => String, optional(:buildpack_directory_key) => String, optional(:app_package_directory_key) => String, diff --git a/spec/api/documentation/buildpack_cache_api_spec.rb b/spec/api/documentation/buildpack_cache_api_spec.rb index e273a7745c1..3c16143a17b 100644 --- a/spec/api/documentation/buildpack_cache_api_spec.rb +++ b/spec/api/documentation/buildpack_cache_api_spec.rb @@ -16,9 +16,8 @@ external_port: cc_port, droplets: { droplet_directory_key: 'cc-droplets', - fog_connection: { - provider: 'local' - } + blobstore_type: 'local', + local_blobstore_path: Dir.mktmpdir('droplets', workspace) }, directories: { tmpdir: Dir.mktmpdir('tmpdir', workspace) @@ -30,11 +29,9 @@ before do TestConfig.override(**blobstore_config) - CloudController::DependencyLocator.instance.buildpack_cache_blobstore.ensure_bucket_exists end after do - Fog::Mock.reset FileUtils.rm_rf(workspace) end diff --git a/spec/support/shared_examples/controllers/resource_pool.rb b/spec/support/shared_examples/controllers/resource_pool.rb index 56fbff4f3ea..fad3e0f385d 100644 --- a/spec/support/shared_examples/controllers/resource_pool.rb +++ b/spec/support/shared_examples/controllers/resource_pool.rb @@ -10,6 +10,7 @@ @nonexisting_descriptor = { 'sha1' => Digester.new.digest('abc'), 'size' => 1, 'mode' => '666' } @tmpdir = Dir.mktmpdir + @resource_pool_blobstore_path = Dir.mktmpdir @descriptors = [] num_dirs.times do @@ -34,8 +35,6 @@ File.write("#{path}-not-allowed", 'A' * @max_file_size) end end - - Fog.mock! end let(:resource_pool_config) do @@ -43,7 +42,8 @@ maximum_size: maximum_file_size, minimum_size: minimum_file_size, resource_directory_key: 'spec-cc-resources', - fog_connection: {} + blobstore_type: 'local', + local_blobstore_path: @resource_pool_blobstore_path } end @@ -59,5 +59,6 @@ after(:all) do FileUtils.rm_rf(@tmpdir) + FileUtils.rm_rf(@resource_pool_blobstore_path) end end diff --git a/spec/unit/controllers/internal/download_droplets_controller_spec.rb b/spec/unit/controllers/internal/download_droplets_controller_spec.rb index 64ad88da660..7af616dedd2 100644 --- a/spec/unit/controllers/internal/download_droplets_controller_spec.rb +++ b/spec/unit/controllers/internal/download_droplets_controller_spec.rb @@ -5,19 +5,18 @@ module VCAP::CloudController RSpec.describe DownloadDropletsController do describe 'GET /internal/v2/droplets/:guid/:droplet_hash/download' do + let(:workspace) { Dir.mktmpdir } let(:original_staging_config) do { packages: { - fog_connection: { - provider: 'local' - }, + blobstore_type: 'local', + local_blobstore_path: Dir.mktmpdir('packages', workspace), app_package_directory_key: 'cc-packages' }, droplets: { droplet_directory_key: 'cc-droplets', - fog_connection: { - provider: 'local' - } + blobstore_type: 'local', + local_blobstore_path: Dir.mktmpdir('droplets', workspace) } } end @@ -32,10 +31,9 @@ module VCAP::CloudController before do TestConfig.override(**staging_config) - blobstore.ensure_bucket_exists end - after { Fog::Mock.reset } + after { FileUtils.rm_rf(workspace) } def get_and_redirect(url) get url @@ -93,19 +91,18 @@ def upload_droplet end describe 'GET /internal/v4/droplets/:guid/:droplet_hash/download' do + let(:workspace) { Dir.mktmpdir } let(:original_staging_config) do { packages: { - fog_connection: { - provider: 'local' - }, + blobstore_type: 'local', + local_blobstore_path: Dir.mktmpdir('packages', workspace), app_package_directory_key: 'cc-packages' }, droplets: { droplet_directory_key: 'cc-droplets', - fog_connection: { - provider: 'local' - } + blobstore_type: 'local', + local_blobstore_path: Dir.mktmpdir('droplets', workspace) } } end @@ -120,10 +117,9 @@ def upload_droplet before do TestConfig.override(**staging_config) - blobstore.ensure_bucket_exists end - after { Fog::Mock.reset } + after { FileUtils.rm_rf(workspace) } def upload_droplet(target_droplet=droplet) droplet_file = Tempfile.new(v3_app.guid) diff --git a/spec/unit/controllers/runtime/stagings_controller_spec.rb b/spec/unit/controllers/runtime/stagings_controller_spec.rb index 4ab38792cdb..6581e29b96b 100644 --- a/spec/unit/controllers/runtime/stagings_controller_spec.rb +++ b/spec/unit/controllers/runtime/stagings_controller_spec.rb @@ -197,21 +197,18 @@ module VCAP::CloudController nginx: { use_nginx: true }, resource_pool: { resource_directory_key: 'cc-resources', - fog_connection: { - provider: 'local' - } + blobstore_type: 'local', + local_blobstore_path: Dir.mktmpdir('resource_pool', workspace) }, packages: { - fog_connection: { - provider: 'local' - }, + blobstore_type: 'local', + local_blobstore_path: Dir.mktmpdir('packages', workspace), app_package_directory_key: 'cc-packages' }, droplets: { droplet_directory_key: 'cc-droplets', - fog_connection: { - provider: 'local' - } + blobstore_type: 'local', + local_blobstore_path: Dir.mktmpdir('droplets', workspace) }, directories: { tmpdir: Dir.mktmpdir('tmpdir', workspace) @@ -238,7 +235,6 @@ module VCAP::CloudController end after do - Fog::Mock.reset FileUtils.rm_rf(workspace) end diff --git a/spec/unit/jobs/runtime/buildpack_cache_cleanup_spec.rb b/spec/unit/jobs/runtime/buildpack_cache_cleanup_spec.rb index 9d39c9997c4..0c82bff3b98 100644 --- a/spec/unit/jobs/runtime/buildpack_cache_cleanup_spec.rb +++ b/spec/unit/jobs/runtime/buildpack_cache_cleanup_spec.rb @@ -17,9 +17,8 @@ module Jobs::Runtime tls_port: cc_port, droplets: { droplet_directory_key: 'cc-droplets', - fog_connection: { - provider: 'local' - } + blobstore_type: 'local', + local_blobstore_path: Dir.mktmpdir('droplets', workspace) }, directories: { tmpdir: Dir.mktmpdir('tmpdir', workspace) @@ -49,7 +48,6 @@ module Jobs::Runtime after do FileUtils.rm_rf(workspace) - Fog::Mock.reset end it 'deletes everything from the buildpack_cache directory' do diff --git a/spec/unit/jobs/v3/buildpack_cache_cleanup_spec.rb b/spec/unit/jobs/v3/buildpack_cache_cleanup_spec.rb index ed61dd6628a..dd004c3b64b 100644 --- a/spec/unit/jobs/v3/buildpack_cache_cleanup_spec.rb +++ b/spec/unit/jobs/v3/buildpack_cache_cleanup_spec.rb @@ -17,9 +17,8 @@ module Jobs::V3 tls_port: cc_port, droplets: { droplet_directory_key: 'cc-droplets', - fog_connection: { - provider: 'local' - } + blobstore_type: 'local', + local_blobstore_path: Dir.mktmpdir('droplets', workspace) }, directories: { tmpdir: Dir.mktmpdir('tmpdir', workspace) @@ -49,7 +48,6 @@ module Jobs::V3 after do FileUtils.rm_rf(workspace) - Fog::Mock.reset end it 'deletes everything from the buildpack_cache directory' do diff --git a/spec/unit/jobs/v3/buildpack_cache_delete_spec.rb b/spec/unit/jobs/v3/buildpack_cache_delete_spec.rb index 9578c513f19..8ea744ed351 100644 --- a/spec/unit/jobs/v3/buildpack_cache_delete_spec.rb +++ b/spec/unit/jobs/v3/buildpack_cache_delete_spec.rb @@ -6,15 +6,13 @@ module Jobs::V3 RSpec.describe BuildpackCacheDelete, job_context: :worker do let(:app_guid) { 'some-guid' } let!(:blobstore) do - CloudController::Blobstore::FogClient.new(connection_config: { provider: 'local' }, - directory_key: 'directory_key') + CloudController::Blobstore::LocalClient.new(directory_key: 'directory_key', base_path: Dir.mktmpdir) end let(:path_1) { Presenters::V3::CacheKeyPresenter.cache_key(guid: app_guid, stack_name: 'stack1') } let(:path_2) { Presenters::V3::CacheKeyPresenter.cache_key(guid: app_guid, stack_name: 'stack2') } let(:path_3) { Presenters::V3::CacheKeyPresenter.cache_key(guid: 'other-guid', stack_name: 'stack3') } before do - blobstore.ensure_bucket_exists Tempfile.create('cache_file') do |f| allow(CloudController::DependencyLocator.instance).to receive(:buildpack_cache_blobstore).and_return(blobstore) blobstore.cp_to_blobstore(f.path, path_1) @@ -23,10 +21,6 @@ module Jobs::V3 end end - after do - Fog::Mock.reset - end - subject(:job) { BuildpackCacheDelete.new(app_guid) } it { is_expected.to be_a_valid_job } diff --git a/spec/unit/jobs/v3/droplet_bits_copier_spec.rb b/spec/unit/jobs/v3/droplet_bits_copier_spec.rb index b042247cf68..0c574d36173 100644 --- a/spec/unit/jobs/v3/droplet_bits_copier_spec.rb +++ b/spec/unit/jobs/v3/droplet_bits_copier_spec.rb @@ -7,20 +7,11 @@ module Jobs::V3 let(:droplet_bits_path) { File.expand_path('../../../fixtures/good.zip', File.dirname(__FILE__)) } let(:droplet_blobstore) do - CloudController::Blobstore::FogClient.new(connection_config: { provider: 'local' }, - directory_key: 'droplet') + CloudController::Blobstore::LocalClient.new(directory_key: 'droplet', base_path: Dir.mktmpdir) end let(:source_droplet) { create(:droplet_model, droplet_hash: 'abcdef1234', sha256_checksum: '4321fedcba', state: DropletModel::STAGED_STATE, set_as_current_droplet: false) } let(:destination_droplet) { create(:droplet_model, droplet_hash: nil, sha256_checksum: nil, state: DropletModel::STAGING_STATE, set_as_current_droplet: false) } - before do - droplet_blobstore.ensure_bucket_exists - end - - after do - Fog::Mock.reset - end - it { is_expected.to be_a_valid_job } it 'knows its job name' do diff --git a/spec/unit/jobs/v3/package_bits_copier_spec.rb b/spec/unit/jobs/v3/package_bits_copier_spec.rb index b096c6daad8..0d399a354ba 100644 --- a/spec/unit/jobs/v3/package_bits_copier_spec.rb +++ b/spec/unit/jobs/v3/package_bits_copier_spec.rb @@ -7,20 +7,11 @@ module Jobs::V3 let(:package_bits_path) { File.expand_path('../../../fixtures/good.zip', File.dirname(__FILE__)) } let(:package_blobstore) do - CloudController::Blobstore::FogClient.new(connection_config: { provider: 'local' }, - directory_key: 'package') + CloudController::Blobstore::LocalClient.new(directory_key: 'package', base_path: Dir.mktmpdir) end let(:source_package) { create(:package_model, type: 'bits', package_hash: 'something', sha256_checksum: 'sha256') } let(:destination_package) { create(:package_model, type: 'bits') } - before do - package_blobstore.ensure_bucket_exists - end - - after do - Fog::Mock.reset - end - it { is_expected.to be_a_valid_job } describe '#perform' do diff --git a/spec/unit/lib/cloud_controller/blobstore/fog/fog_client_spec.rb b/spec/unit/lib/cloud_controller/blobstore/fog/fog_client_spec.rb deleted file mode 100644 index 2d7ef7c9821..00000000000 --- a/spec/unit/lib/cloud_controller/blobstore/fog/fog_client_spec.rb +++ /dev/null @@ -1,700 +0,0 @@ -require 'spec_helper' -require 'webrick' -require_relative '../client_shared' - -module CloudController - module Blobstore - RSpec.describe FogClient do - let(:content) { 'Some Nonsense' } - let(:sha_of_content) { Digester.new.digest(content) } - let(:local_dir) { Dir.mktmpdir } - let(:connection_config) do - { - provider: 'local' - } - end - - let(:directory_key) { 'a-directory-key' } - - subject(:client) do - FogClient.new(connection_config:, - directory_key:) - end - - after do - Fog::Mock.reset - end - - it 'logs a deprecation warning on initialization' do - expect_any_instance_of(Steno::Logger).to receive(:warn).with('blobstore.fog-deprecated', hash_including(:message)) - FogClient.new(connection_config:, directory_key:) - end - - describe 'conforms to blobstore client interface' do - let(:deletable_blob) { instance_double(FogBlob, file: nil) } - - before do - client.ensure_bucket_exists - client.cp_to_blobstore(tmpfile.path, key) - end - - it_behaves_like 'a blobstore client' - end - - def upload_tmpfile(client, key='abcdef') - Tempfile.open('') do |tmpfile| - tmpfile.write(content) - tmpfile.close - client.cp_to_blobstore(tmpfile.path, key) - end - end - - context 'for a remote blobstore backed by a CDN' do - let(:cdn) { double(:cdn) } - let(:url_from_cdn) { 'http://some_distribution.cloudfront.net/ab/cd/abcdef' } - let(:key) { 'abcdef' } - - subject(:client) do - FogClient.new(connection_config:, - directory_key:, - cdn:) - end - - before do - client.ensure_bucket_exists - upload_tmpfile(client, key) - allow(cdn).to receive(:download_uri).and_return(url_from_cdn) - end - - it 'downloads through the CDN' do - expect(cdn).to receive(:get). - with('ab/cd/abcdef'). - and_yield('foobar').and_yield(' barbaz') - - destination = File.join(local_dir, 'some_directory_to_place_file', 'downloaded_file') - - expect { client.download_from_blobstore(key, destination) }.to change { - File.exist?(destination) - }.from(false).to(true) - - expect(File.read(destination)).to eq('foobar barbaz') - end - end - - context 'common behaviors' do - let(:directory) { Fog::Storage.new(connection_config).directories.create(key: directory_key) } - let(:client) do - FogClient.new(connection_config:, - directory_key:) - end - - before do - client.ensure_bucket_exists - end - - context 'with existing files' do - before do - upload_tmpfile(client, sha_of_content) - end - - describe 'a file existence' do - it 'does not exist if not present' do - different_content = 'foobar' - sha_of_different_content = Digester.new.digest(different_content) - - expect(client.exists?(sha_of_different_content)).to be false - - upload_tmpfile(client, sha_of_different_content) - - expect(client.exists?(sha_of_different_content)).to be true - expect(client.blob(sha_of_different_content)).to be - end - end - end - - describe '#cp_r_to_blobstore' do - let(:sha_of_nothing) { Digester.new.digest('') } - - it 'ensures that the sha of nothing and sha of content are different for subsequent tests' do - expect(sha_of_nothing[0..1]).not_to eq(sha_of_content[0..1]) - end - - it 'copies the top-level local files into the blobstore' do - FileUtils.touch(File.join(local_dir, 'empty_file')) - client.cp_r_to_blobstore(local_dir) - expect(client.exists?(sha_of_nothing)).to be true - end - - it 'recursively copies the local files into the blobstore' do - subdir = File.join(local_dir, 'subdir1', 'subdir2') - FileUtils.mkdir_p(subdir) - File.write(File.join(subdir, 'file_with_content'), content) - - client.cp_r_to_blobstore(local_dir) - expect(client.exists?(sha_of_content)).to be true - end - - context 'when the file already exists in the blobstore' do - before do - FileUtils.touch(File.join(local_dir, 'empty_file')) - end - - it 'does not re-upload it' do - client.cp_r_to_blobstore(local_dir) - - expect(client).not_to receive(:cp_to_blobstore) - client.cp_r_to_blobstore(local_dir) - end - end - - context 'limit the file size' do - let(:min_size) { 20 } - let(:max_size) { 50 } - - subject(:client) do - FogClient.new(connection_config:, - directory_key:, - min_size:, - max_size:) - end - - it 'does not copy files below the minimum size limit' do - path = File.join(local_dir, 'file_with_little_content') - File.write(path, 'a') - - expect(client).not_to receive(:exists?) - expect(client).not_to receive(:cp_to_blobstore) - client.cp_r_to_blobstore(path) - end - - it 'does not copy files above the maximum size limit' do - path = File.join(local_dir, 'file_with_more_content') - File.write(path, 'an amount of content that is larger than the maximum limit') - - expect(client).not_to receive(:exists?) - expect(client).not_to receive(:cp_to_blobstore) - client.cp_r_to_blobstore(path) - end - end - - context 'limit the file mode to those with sufficient permissions' do - subject(:client) do - FogClient.new(connection_config:, - directory_key:) - end - - it 'copies files with mode >= 0600' do - path = File.join(local_dir, 'file_with_sufficient_permissions') - FileUtils.touch(path) - File.chmod(0o600, path) - - expect(client).to receive(:exists?) - expect(client).to receive(:cp_to_blobstore) - client.cp_r_to_blobstore(path) - end - - it 'does not copy files below the minimum file mode' do - path = File.join(local_dir, 'file_with_insufficient_permissions') - FileUtils.touch(path) - File.chmod(0o444, path) - - expect(client).not_to receive(:exists?) - expect(client).not_to receive(:cp_to_blobstore) - client.cp_r_to_blobstore(path) - end - end - end - - describe '#download_from_blobstore' do - let(:destination) { File.join(local_dir, 'some_directory_to_place_file', 'downloaded_file') } - - context 'when directly from the underlying storage' do - before do - upload_tmpfile(client, sha_of_content) - end - - it 'can download the file' do - expect(client.exists?(sha_of_content)).to be true - - expect { client.download_from_blobstore(sha_of_content, destination) }.to change { - File.exist?(destination) - }.from(false).to(true) - - expect(File.read(destination)).to eq(content) - end - end - - describe 'file permissions' do - before do - upload_tmpfile(client, sha_of_content) - @original_umask = File.umask - File.umask(0o022) - end - - after do - File.umask(@original_umask) - end - - context 'when not specifying a mode' do - it 'does not change permissions on the file' do - destination = File.join(local_dir, 'some_directory_to_place_file', 'downloaded_file') - client.download_from_blobstore(sha_of_content, destination) - - expect(sprintf('%o', mode: File.stat(destination).mode)).to eq('100644') - end - end - - context 'when specifying a mode' do - it 'does change permissions on the file' do - destination = File.join(local_dir, 'some_directory_to_place_file', 'downloaded_file') - client.download_from_blobstore(sha_of_content, destination, mode: 0o753) - - expect(sprintf('%o', mode: File.stat(destination).mode)).to eq('100753') - end - end - end - end - - describe '#cp_to_blobstore' do - it 'calls the fog with public false' do - path = File.join(local_dir, 'empty_file') - FileUtils.touch(path) - - client.cp_to_blobstore(path, 'foobar') - - expect(directory.files.head('fo/ob/foobar')).not_to be_public - end - - it 'uploads the files with the specified key' do - path = File.join(local_dir, 'empty_file') - FileUtils.touch(path) - - client.cp_to_blobstore(path, 'abcdef123456') - expect(client.exists?('abcdef123456')).to be true - expect(directory.files).to have(1).item - end - - it 'defaults to private files' do - path = File.join(local_dir, 'empty_file') - FileUtils.touch(path) - key = 'abcdef12345' - - client.cp_to_blobstore(path, key) - expect(client.blob(key).file.public_url).to be_nil - end - - it 'sets content-type to mime-type of application/zip when not specified' do - path = File.join(local_dir, 'empty_file') - FileUtils.touch(path) - - client.cp_to_blobstore(path, 'abcdef123456') - - expect(directory.files.head('ab/cd/abcdef123456').content_type).to eq('application/zip') - end - - it 'sets content-type to mime-type of file when specified' do - path = File.join(local_dir, 'empty_file.png') - FileUtils.touch(path) - - client.cp_to_blobstore(path, 'abcdef123456') - - expect(directory.files.head('ab/cd/abcdef123456').content_type).to eq('image/png') - end - - context 'limit the file size' do - let(:min_size) { 20 } - let(:max_size) { 50 } - - subject(:client) do - FogClient.new(connection_config:, - directory_key:, - min_size:, - max_size:) - end - - it 'does not copy files below the minimum size limit' do - path = File.join(local_dir, 'file_with_little_content') - File.write(path, 'a') - key = '987654321' - - client.cp_to_blobstore(path, key) - expect(client.exists?(key)).to be false - end - - it 'does not copy files above the maximum size limit' do - path = File.join(local_dir, 'file_with_more_content') - File.write(path, 'an amount of content that is larger than the maximum limit') - key = '777777777' - - client.cp_to_blobstore(path, key) - expect(client.exists?(key)).to be false - end - end - - context 'storage options' do - let(:files) { double(:files, create: true) } - - before do - allow_any_instance_of(FogClient).to receive(:files).and_return(files) - end - end - end - - describe '#cp_file_between_keys' do - let(:src_key) { 'abc123' } - let(:dest_key) { 'xyz789' } - - it 'copies the file from the source key to the destination key' do - upload_tmpfile(client, src_key) - client.cp_file_between_keys(src_key, dest_key) - - expect(client.exists?(dest_key)).to be true - expect(directory.files).to have(2).item - end - - context 'when the destination key has a package already' do - before do - upload_tmpfile(client, src_key) - Tempfile.open('') do |tmpfile| - tmpfile.write('This should be deleted and replaced with new file') - tmpfile.close - client.cp_to_blobstore(tmpfile.path, dest_key) - end - end - - it 'removes the old package from the package blobstore' do - client.cp_file_between_keys(src_key, dest_key) - expect(directory.files).to have(2).item - - src_file_length = client.blob(dest_key).file.content_length - dest_file_length = client.blob(src_key).file.content_length - expect(dest_file_length).to eq(src_file_length) - end - end - - context 'when the source key has no file associated with it' do - it 'does not attempt to copy over to the destination key' do - expect do - client.cp_file_between_keys('bogus', dest_key) - end.to raise_error(CloudController::Blobstore::FileNotFound) - - expect(directory.files).to have(0).items - end - end - - end - - describe '#delete_all' do - before do - client.ensure_bucket_exists - end - - it 'deletes all the files' do - first_path = File.join(local_dir, 'first_empty_file') - FileUtils.touch(first_path) - path = File.join(local_dir, 'empty_file') - FileUtils.touch(path) - - client.cp_to_blobstore(first_path, 'ab56') - expect(client.exists?('ab56')).to be true - client.cp_to_blobstore(path, 'abcdef123456') - expect(client.exists?('abcdef123456')).to be true - - client.delete_all - - expect(client.exists?('ab56')).to be false - expect(client.exists?('abcdef123456')).to be false - end - - it 'is ok if there are no files' do - expect(directory.files).to have(0).items - expect do - client.delete_all - end.not_to raise_error - end - - context 'when a root dir is provided' do - let(:root_dir) { 'root-dir' } - - let(:client_with_root) do - FogClient.new(connection_config:, - directory_key:, - root_dir:) - end - - before do - client_with_root.ensure_bucket_exists - end - - it 'only deletes files at the root' do - allow(client_with_root).to receive(:delete_files).and_call_original - - file = File.join(local_dir, 'empty_file') - FileUtils.touch(file) - - client.cp_to_blobstore(file, 'abcdef1') - client_with_root.cp_to_blobstore(file, 'abcdef2') - - client_with_root.delete_all - - expect(client_with_root).to have_received(:delete_files) do |files| - expect(files.length).to eq(1) - end - end - end - end - - describe '#delete_all_in_path' do - before do - client.ensure_bucket_exists - end - - it 'deletes all the files within a specific path' do - path = File.join(local_dir, 'empty_file') - FileUtils.touch(path) - - remote_path_1 = 'aaaaguid' - - remote_key_1 = "#{remote_path_1}/stack_1" - remote_key_2 = "#{remote_path_1}/stack_2" - - remote_path_2 = 'bbbbguid' - remote_key_3 = "#{remote_path_2}/stack_3" - - client.cp_to_blobstore(path, remote_key_1) - client.cp_to_blobstore(path, remote_key_2) - client.cp_to_blobstore(path, remote_key_3) - expect(client.exists?(remote_key_1)).to be true - expect(client.exists?(remote_key_2)).to be true - expect(client.exists?(remote_key_3)).to be true - - client.delete_all_in_path(remote_path_1) - - expect(client.exists?(remote_key_1)).to be false - expect(client.exists?(remote_key_2)).to be false - expect(client.exists?(remote_key_3)).to be true - end - - it 'is ok if there are no files' do - expect(directory.files).to have(0).items - expect do - client.delete_all_in_path('nonsense_path') - end.not_to raise_error - end - - context 'when a root dir is provided' do - let(:root_dir) { 'root-dir' } - - let(:client_with_root) do - FogClient.new(connection_config:, - directory_key:, - root_dir:) - end - - before do - client_with_root.ensure_bucket_exists - end - - it 'only deletes files at the root' do - path = File.join(local_dir, 'empty_file') - FileUtils.touch(path) - - remote_path_1 = 'aaaaguid' - - remote_key_1 = "#{remote_path_1}/stack_1" - remote_key_2 = "#{remote_path_1}/stack_2" - - remote_path_2 = 'bbbbguid' - remote_key_3 = "#{remote_path_2}/stack_3" - - client_with_root.cp_to_blobstore(path, remote_key_1) - client_with_root.cp_to_blobstore(path, remote_key_2) - client_with_root.cp_to_blobstore(path, remote_key_3) - expect(client_with_root.exists?(remote_key_1)).to be true - expect(client_with_root.exists?(remote_key_2)).to be true - expect(client_with_root.exists?(remote_key_3)).to be true - - client_with_root.delete_all_in_path(remote_path_1) - - expect(client_with_root.exists?(remote_key_1)).to be false - expect(client_with_root.exists?(remote_key_2)).to be false - expect(client_with_root.exists?(remote_key_3)).to be true - end - end - end - - describe '#delete' do - it 'deletes the file' do - path = File.join(local_dir, 'empty_file') - FileUtils.touch(path) - - client.cp_to_blobstore(path, 'abcdef123456') - expect(client.exists?('abcdef123456')).to be true - client.delete('abcdef123456') - expect(client.exists?('abcdef123456')).to be false - end - - it "is ok if the file doesn't exist" do - expect(directory.files).to have(0).items - expect do - client.delete('non-existent-file') - end.not_to raise_error - end - end - - describe '#delete_blob' do - it "deletes the blob's file" do - path = File.join(local_dir, 'empty_file') - FileUtils.touch(path) - - client.cp_to_blobstore(path, 'abcdef123456') - expect(client.exists?('abcdef123456')).to be(true) - - blob = client.blob('abcdef123456') - - client.delete_blob(blob) - expect(client.exists?('abcdef123456')).to be(false) - end - - it "is ok if the file doesn't exist" do - blob = FogBlob.new(nil, nil) - expect do - client.delete_blob(blob) - end.not_to raise_error - end - end - - describe '#ensure_bucket_exists' do - before do - Fog::Mock.reset - end - - it 'gets the bucket' do - expect(subject.send(:connection).directories).to receive(:get).with(directory_key, max_keys: 1).and_call_original - subject.ensure_bucket_exists - end - - context 'the bucket exists' do - it 'does not create the bucket' do - subject.ensure_bucket_exists - expect(subject.send(:connection).directories).not_to receive(:create).with(key: directory_key, public: false) - subject.ensure_bucket_exists - end - end - - context 'the bucket does not exist' do - it 'creates the bucket' do - allow(subject.send(:connection).directories).to receive(:get).with(directory_key, max_keys: 1).and_return(nil) - expect(subject.send(:connection).directories).to receive(:create).with(key: directory_key, public: false) - subject.ensure_bucket_exists - end - end - end - end - - context 'with root directory specified' do - let(:root_dir) { 'my-root' } - - let(:client_with_root) do - FogClient.new(connection_config:, - directory_key:, - root_dir:) - end - - before do - client_with_root.ensure_bucket_exists - end - - it 'includes the directory in the partitioned key' do - upload_tmpfile(client_with_root, 'abcdef') - expect(client_with_root.exists?('abcdef')).to be true - expect(client_with_root.blob('abcdef')).to be - expect(client_with_root.blob('abcdef').public_download_url).to match(%r{my-root/ab/cd/abcdef}) - end - end - - describe 'downloading without mocking' do - def wait_for_server_to_accept_requests(uri) - code = nil - total_time = 0 - while code != '200' && total_time < 10 - begin - res = Net::HTTP.get_response(URI(uri)) - code = res.code - total_time += 0.1 - sleep 0.1 - rescue StandardError - end - end - end - - describe 'from a CDN' do - let(:port) { 9875 } - let(:uri) { "http://localhost:#{port}" } - let(:cdn) { Cdn.make(uri) } - - subject(:client) do - FogClient.new(connection_config:, - directory_key:, - cdn:) - end - - around do |example| - WebMock.disable_net_connect!(allow_localhost: true) - example.run - WebMock.disable_net_connect! - end - - it 'correctly downloads byte streams' do - source_directory_path = File.expand_path('../../../../../fixtures/', File.dirname(__FILE__)) - source_file_path = File.join(source_directory_path, 'pa/rt/partitioned_key') - source_hexdigest = OpenSSL::Digest::SHA256.file(source_file_path).hexdigest - - pid = spawn("ruby -rwebrick -e'WEBrick::HTTPServer.new(:Port => #{port}, :DocumentRoot => \"#{source_directory_path}\").start'", out: 'test.out', err: 'test.err') - - begin - Process.detach(pid) - - wait_for_server_to_accept_requests(uri) - - destination_file_path = File.join(Dir.mktmpdir, 'hard_file.xyz') - - client.download_from_blobstore('partitioned_key', destination_file_path) - - destination_hexdigest = OpenSSL::Digest::SHA256.file(destination_file_path).hexdigest - - expect(destination_hexdigest).to eq(source_hexdigest) - ensure - Process.kill(9, pid) - end - end - end - - describe 'from a blobstore' do - it 'correctly downloads byte streams' do - content = 'some binary content for checksum verification' - source_file = Tempfile.new('source') - source_file.write(content) - source_file.close - - source_hexdigest = OpenSSL::Digest::SHA256.file(source_file.path).hexdigest - - client.ensure_bucket_exists - client.cp_to_blobstore(source_file.path, 'partitioned_key') - - destination_dir = Dir.mktmpdir - destination_file_path = File.join(destination_dir, 'hard_file.xyz') - client.download_from_blobstore('partitioned_key', destination_file_path) - - destination_hexdigest = OpenSSL::Digest::SHA256.file(destination_file_path).hexdigest - - expect(destination_hexdigest).to eq(source_hexdigest) - ensure - source_file&.unlink - FileUtils.rm_rf(destination_dir) if destination_dir - end - end - end - end - end -end diff --git a/spec/unit/lib/cloud_controller/packager/local_bits_packer_spec.rb b/spec/unit/lib/cloud_controller/packager/local_bits_packer_spec.rb index 5cf9dab68ba..b7a832487fb 100644 --- a/spec/unit/lib/cloud_controller/packager/local_bits_packer_spec.rb +++ b/spec/unit/lib/cloud_controller/packager/local_bits_packer_spec.rb @@ -11,17 +11,17 @@ module CloudController::Packager let(:min_size) { 4 } let(:max_size) { 8 } let(:global_app_bits_cache) do - CloudController::Blobstore::FogClient.new( - connection_config: { provider: 'local' }, + CloudController::Blobstore::LocalClient.new( directory_key: 'global_app_bits_cache', + base_path: Dir.mktmpdir, min_size: min_size, max_size: max_size ) end let(:package_blobstore) do - CloudController::Blobstore::FogClient.new( - connection_config: { provider: 'local' }, - directory_key: 'package' + CloudController::Blobstore::LocalClient.new( + directory_key: 'package', + base_path: Dir.mktmpdir ) end @@ -55,13 +55,9 @@ module CloudController::Packager rescue StandardError nil end - - global_app_bits_cache.ensure_bucket_exists - package_blobstore.ensure_bucket_exists end after do - Fog::Mock.reset FileUtils.remove_entry_secure local_tmp_dir end @@ -351,7 +347,7 @@ module CloudController::Packager context 'when the package is successfully uploaded and processed' do it 'removes the temporary directory created for packaging' do - allow_any_instance_of(CloudController::Blobstore::FogClient).to receive(:cp_to_blobstore) + allow_any_instance_of(CloudController::Blobstore::LocalClient).to receive(:cp_to_blobstore) allow(Digester).to receive(:new).and_return(instance_double(Digester, digest_path: 'fake-digest')) expect(FileUtils).to receive(:remove_dir).with(local_bits_packer_path).and_call_original packer.send_package_to_blobstore(blobstore_key, uploaded_files_path, cached_files_fingerprints) @@ -366,7 +362,7 @@ module CloudController::Packager expect(FileUtils).to receive(:remove_dir).with(local_bits_packer_path) expect do - allow_any_instance_of(CloudController::Blobstore::FogClient).to receive(:cp_to_blobstore).and_raise(StandardError) + allow_any_instance_of(CloudController::Blobstore::LocalClient).to receive(:cp_to_blobstore).and_raise(StandardError) packer.send_package_to_blobstore(blobstore_key, uploaded_files_path, cached_files_fingerprints) end.to raise_error(StandardError) end From 936ba9cd7c837e07b14696061cc1917dc0219e61 Mon Sep 17 00:00:00 2001 From: Katharina Przybill <30441792+kathap@users.noreply.github.com> Date: Tue, 14 Jul 2026 13:52:17 +0200 Subject: [PATCH 03/24] Fix FogClient spec and LocalClient tmpdir cleanup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fog_client_spec.rb restored — rewritten to stub Fog::Storage.new with a FakeStorage in-memory backend defined in the spec itself. All provider-independent FogClient behavior (uploads, downloads, copies, deletion, CDN, file permissions, bucket management, root dir scoping) is covered. The AWS-specific contexts (encryption, delete_multiple_objects via AWS bulk delete) are not included since those were the AWS-only parts removed in this PR. No fog provider gem is needed. Three job specs — blobstore_path extracted into a named let, after { FileUtils.rm_rf(blobstore_path) } added to each. --- .../jobs/v3/buildpack_cache_delete_spec.rb | 7 +- spec/unit/jobs/v3/droplet_bits_copier_spec.rb | 7 +- spec/unit/jobs/v3/package_bits_copier_spec.rb | 7 +- .../blobstore/fog/fog_client_spec.rb | 758 ++++++++++++++++++ 4 files changed, 776 insertions(+), 3 deletions(-) create mode 100644 spec/unit/lib/cloud_controller/blobstore/fog/fog_client_spec.rb diff --git a/spec/unit/jobs/v3/buildpack_cache_delete_spec.rb b/spec/unit/jobs/v3/buildpack_cache_delete_spec.rb index 8ea744ed351..c896b53b2b6 100644 --- a/spec/unit/jobs/v3/buildpack_cache_delete_spec.rb +++ b/spec/unit/jobs/v3/buildpack_cache_delete_spec.rb @@ -5,8 +5,9 @@ module VCAP::CloudController module Jobs::V3 RSpec.describe BuildpackCacheDelete, job_context: :worker do let(:app_guid) { 'some-guid' } + let(:blobstore_path) { Dir.mktmpdir } let!(:blobstore) do - CloudController::Blobstore::LocalClient.new(directory_key: 'directory_key', base_path: Dir.mktmpdir) + CloudController::Blobstore::LocalClient.new(directory_key: 'directory_key', base_path: blobstore_path) end let(:path_1) { Presenters::V3::CacheKeyPresenter.cache_key(guid: app_guid, stack_name: 'stack1') } let(:path_2) { Presenters::V3::CacheKeyPresenter.cache_key(guid: app_guid, stack_name: 'stack2') } @@ -23,6 +24,10 @@ module Jobs::V3 subject(:job) { BuildpackCacheDelete.new(app_guid) } + after do + FileUtils.rm_rf(blobstore_path) + end + it { is_expected.to be_a_valid_job } describe '#perform' do diff --git a/spec/unit/jobs/v3/droplet_bits_copier_spec.rb b/spec/unit/jobs/v3/droplet_bits_copier_spec.rb index 0c574d36173..6b6649cb810 100644 --- a/spec/unit/jobs/v3/droplet_bits_copier_spec.rb +++ b/spec/unit/jobs/v3/droplet_bits_copier_spec.rb @@ -6,12 +6,17 @@ module Jobs::V3 subject(:job) { DropletBitsCopier.new(source_droplet.guid, destination_droplet.guid) } let(:droplet_bits_path) { File.expand_path('../../../fixtures/good.zip', File.dirname(__FILE__)) } + let(:blobstore_path) { Dir.mktmpdir } let(:droplet_blobstore) do - CloudController::Blobstore::LocalClient.new(directory_key: 'droplet', base_path: Dir.mktmpdir) + CloudController::Blobstore::LocalClient.new(directory_key: 'droplet', base_path: blobstore_path) end let(:source_droplet) { create(:droplet_model, droplet_hash: 'abcdef1234', sha256_checksum: '4321fedcba', state: DropletModel::STAGED_STATE, set_as_current_droplet: false) } let(:destination_droplet) { create(:droplet_model, droplet_hash: nil, sha256_checksum: nil, state: DropletModel::STAGING_STATE, set_as_current_droplet: false) } + after do + FileUtils.rm_rf(blobstore_path) + end + it { is_expected.to be_a_valid_job } it 'knows its job name' do diff --git a/spec/unit/jobs/v3/package_bits_copier_spec.rb b/spec/unit/jobs/v3/package_bits_copier_spec.rb index 0d399a354ba..d5ff268c536 100644 --- a/spec/unit/jobs/v3/package_bits_copier_spec.rb +++ b/spec/unit/jobs/v3/package_bits_copier_spec.rb @@ -6,12 +6,17 @@ module Jobs::V3 subject(:job) { PackageBitsCopier.new(source_package.guid, destination_package.guid) } let(:package_bits_path) { File.expand_path('../../../fixtures/good.zip', File.dirname(__FILE__)) } + let(:blobstore_path) { Dir.mktmpdir } let(:package_blobstore) do - CloudController::Blobstore::LocalClient.new(directory_key: 'package', base_path: Dir.mktmpdir) + CloudController::Blobstore::LocalClient.new(directory_key: 'package', base_path: blobstore_path) end let(:source_package) { create(:package_model, type: 'bits', package_hash: 'something', sha256_checksum: 'sha256') } let(:destination_package) { create(:package_model, type: 'bits') } + after do + FileUtils.rm_rf(blobstore_path) + end + it { is_expected.to be_a_valid_job } describe '#perform' do diff --git a/spec/unit/lib/cloud_controller/blobstore/fog/fog_client_spec.rb b/spec/unit/lib/cloud_controller/blobstore/fog/fog_client_spec.rb new file mode 100644 index 00000000000..7b21e0be9a8 --- /dev/null +++ b/spec/unit/lib/cloud_controller/blobstore/fog/fog_client_spec.rb @@ -0,0 +1,758 @@ +require 'spec_helper' +require 'webrick' +require_relative '../client_shared' + +module CloudController + module Blobstore + # Minimal in-memory fog storage stub. Replaces Fog::Storage.new so that + # FogClient specs can run without any fog provider gem being bundled. + class FakeStorage + FakeFile = Struct.new(:key, :body, :content_type, :public, :content_length, :bucket, keyword_init: true) do + def public_url = public ? "http://fake/#{key}" : nil + def copy(dest_bucket, dest_key, _opts = {}) + storage.store_file(dest_bucket, dest_key, body, content_type) + end + def destroy = storage.delete_file(bucket, key) + + attr_accessor :storage + end + + def initialize + @store = {} + end + + def directories = DirectoryProxy.new(self) + + def respond_to?(method, *) = method == :directories || super + + def store_file(bucket, key, body, content_type) + @store[bucket] ||= {} + body_content = body.respond_to?(:read) ? body.read : body.to_s + file = FakeFile.new(key: key, body: body_content, content_type: content_type, public: false, + content_length: body_content.bytesize, bucket: bucket) + file.storage = self + @store[bucket][key] = file + file + end + + def delete_file(bucket, key) + @store[bucket]&.delete(key) + end + + def list_files(bucket, prefix: nil) + files = @store[bucket]&.values || [] + files = files.select { |f| f.key.start_with?(prefix) } if prefix + files + end + + def get_file(bucket, key) + @store[bucket]&.[](key) + end + + def bucket_exists?(key) + @store.key?(key) + end + + def create_bucket(key) + @store[key] ||= {} + end + + class DirectoryProxy + def initialize(storage) = @storage = storage + + def get(key, **) + return nil unless @storage.bucket_exists?(key) + + BucketProxy.new(@storage, key) + end + + def create(key:, **) = BucketProxy.new(@storage, @storage.create_bucket(key) && key) + def new(key:, **) = BucketProxy.new(@storage, key) + end + + class BucketProxy + attr_reader :key + + def initialize(storage, key) + @storage = storage + @key = key + @storage.create_bucket(key) + end + + def files = FilesProxy.new(@storage, @key) + end + + class FilesProxy + include Enumerable + + def initialize(storage, bucket) + @storage = storage + @bucket = bucket + end + + def head(key) = @storage.get_file(@bucket, key) + + def get(key, &block) + file = @storage.get_file(@bucket, key) + return nil unless file + + if block + yield file.body + else + file + end + file + end + + def create(key:, body:, content_type: 'application/zip', **) + @storage.store_file(@bucket, key, body, content_type) + end + + def each(&block) = all.each(&block) + def all = @storage.list_files(@bucket) + + def select(prefix: nil) + @storage.list_files(@bucket, prefix:) + end + end + end + + RSpec.describe FogClient do + let(:content) { 'Some Nonsense' } + let(:sha_of_content) { Digester.new.digest(content) } + let(:local_dir) { Dir.mktmpdir } + let(:fake_storage) { FakeStorage.new } + let(:connection_config) { { provider: 'fake' } } + let(:directory_key) { 'a-directory-key' } + + subject(:client) do + FogClient.new(connection_config:, directory_key:) + end + + before do + allow(Fog::Storage).to receive(:new).and_return(fake_storage) + end + + after do + FileUtils.rm_rf(local_dir) + end + + it 'logs a deprecation warning on initialization' do + expect_any_instance_of(Steno::Logger).to receive(:warn).with('blobstore.fog-deprecated', hash_including(:message)) + FogClient.new(connection_config:, directory_key:) + end + + describe 'conforms to blobstore client interface' do + let(:deletable_blob) { instance_double(FogBlob, file: nil) } + + before do + client.ensure_bucket_exists + client.cp_to_blobstore(tmpfile.path, key) + end + + it_behaves_like 'a blobstore client' + end + + def upload_tmpfile(client, key='abcdef') + Tempfile.open('') do |tmpfile| + tmpfile.write(content) + tmpfile.close + client.cp_to_blobstore(tmpfile.path, key) + end + end + + context 'for a remote blobstore backed by a CDN' do + let(:cdn) { double(:cdn) } + let(:url_from_cdn) { 'http://some_distribution.cloudfront.net/ab/cd/abcdef' } + let(:key) { 'abcdef' } + + subject(:client) do + FogClient.new(connection_config:, directory_key:, cdn:) + end + + before do + client.ensure_bucket_exists + upload_tmpfile(client, key) + allow(cdn).to receive(:download_uri).and_return(url_from_cdn) + end + + it 'downloads through the CDN' do + expect(cdn).to receive(:get). + with('ab/cd/abcdef'). + and_yield('foobar').and_yield(' barbaz') + + destination = File.join(local_dir, 'some_directory_to_place_file', 'downloaded_file') + + expect { client.download_from_blobstore(key, destination) }.to change { + File.exist?(destination) + }.from(false).to(true) + + expect(File.read(destination)).to eq('foobar barbaz') + end + end + + context 'common behaviors' do + let(:directory) { fake_storage.directories.get(directory_key) || fake_storage.directories.create(key: directory_key) } + let(:client) do + FogClient.new(connection_config:, directory_key:) + end + + before do + client.ensure_bucket_exists + end + + context 'with existing files' do + before do + upload_tmpfile(client, sha_of_content) + end + + describe 'a file existence' do + it 'does not exist if not present' do + different_content = 'foobar' + sha_of_different_content = Digester.new.digest(different_content) + + expect(client.exists?(sha_of_different_content)).to be false + + upload_tmpfile(client, sha_of_different_content) + + expect(client.exists?(sha_of_different_content)).to be true + expect(client.blob(sha_of_different_content)).to be + end + end + end + + describe '#cp_r_to_blobstore' do + let(:sha_of_nothing) { Digester.new.digest('') } + + it 'ensures that the sha of nothing and sha of content are different for subsequent tests' do + expect(sha_of_nothing[0..1]).not_to eq(sha_of_content[0..1]) + end + + it 'copies the top-level local files into the blobstore' do + FileUtils.touch(File.join(local_dir, 'empty_file')) + client.cp_r_to_blobstore(local_dir) + expect(client.exists?(sha_of_nothing)).to be true + end + + it 'recursively copies the local files into the blobstore' do + subdir = File.join(local_dir, 'subdir1', 'subdir2') + FileUtils.mkdir_p(subdir) + File.write(File.join(subdir, 'file_with_content'), content) + + client.cp_r_to_blobstore(local_dir) + expect(client.exists?(sha_of_content)).to be true + end + + context 'when the file already exists in the blobstore' do + before do + FileUtils.touch(File.join(local_dir, 'empty_file')) + end + + it 'does not re-upload it' do + client.cp_r_to_blobstore(local_dir) + + expect(client).not_to receive(:cp_to_blobstore) + client.cp_r_to_blobstore(local_dir) + end + end + + context 'limit the file size' do + let(:min_size) { 20 } + let(:max_size) { 50 } + + subject(:client) do + FogClient.new(connection_config:, directory_key:, min_size:, max_size:) + end + + it 'does not copy files below the minimum size limit' do + path = File.join(local_dir, 'file_with_little_content') + File.write(path, 'a') + + expect(client).not_to receive(:exists?) + expect(client).not_to receive(:cp_to_blobstore) + client.cp_r_to_blobstore(path) + end + + it 'does not copy files above the maximum size limit' do + path = File.join(local_dir, 'file_with_more_content') + File.write(path, 'an amount of content that is larger than the maximum limit') + + expect(client).not_to receive(:exists?) + expect(client).not_to receive(:cp_to_blobstore) + client.cp_r_to_blobstore(path) + end + end + + context 'limit the file mode to those with sufficient permissions' do + subject(:client) do + FogClient.new(connection_config:, directory_key:) + end + + it 'copies files with mode >= 0600' do + path = File.join(local_dir, 'file_with_sufficient_permissions') + FileUtils.touch(path) + File.chmod(0o600, path) + + expect(client).to receive(:exists?) + expect(client).to receive(:cp_to_blobstore) + client.cp_r_to_blobstore(path) + end + + it 'does not copy files below the minimum file mode' do + path = File.join(local_dir, 'file_with_insufficient_permissions') + FileUtils.touch(path) + File.chmod(0o444, path) + + expect(client).not_to receive(:exists?) + expect(client).not_to receive(:cp_to_blobstore) + client.cp_r_to_blobstore(path) + end + end + end + + describe '#download_from_blobstore' do + let(:destination) { File.join(local_dir, 'some_directory_to_place_file', 'downloaded_file') } + + context 'when directly from the underlying storage' do + before do + upload_tmpfile(client, sha_of_content) + end + + it 'can download the file' do + expect(client.exists?(sha_of_content)).to be true + + expect { client.download_from_blobstore(sha_of_content, destination) }.to change { + File.exist?(destination) + }.from(false).to(true) + + expect(File.read(destination)).to eq(content) + end + end + + describe 'file permissions' do + before do + upload_tmpfile(client, sha_of_content) + @original_umask = File.umask + File.umask(0o022) + end + + after do + File.umask(@original_umask) + end + + context 'when not specifying a mode' do + it 'does not change permissions on the file' do + destination = File.join(local_dir, 'some_directory_to_place_file', 'downloaded_file') + client.download_from_blobstore(sha_of_content, destination) + + expect(sprintf('%o', mode: File.stat(destination).mode)).to eq('100644') + end + end + + context 'when specifying a mode' do + it 'does change permissions on the file' do + destination = File.join(local_dir, 'some_directory_to_place_file', 'downloaded_file') + client.download_from_blobstore(sha_of_content, destination, mode: 0o753) + + expect(sprintf('%o', mode: File.stat(destination).mode)).to eq('100753') + end + end + end + end + + describe '#cp_to_blobstore' do + it 'uploads the files with the specified key' do + path = File.join(local_dir, 'empty_file') + FileUtils.touch(path) + + client.cp_to_blobstore(path, 'abcdef123456') + expect(client.exists?('abcdef123456')).to be true + expect(directory.files.all.length).to eq(1) + end + + it 'defaults to private files' do + path = File.join(local_dir, 'empty_file') + FileUtils.touch(path) + key = 'abcdef12345' + + client.cp_to_blobstore(path, key) + expect(client.blob(key).file.public_url).to be_nil + end + + it 'sets content-type to mime-type of application/zip when not specified' do + path = File.join(local_dir, 'empty_file') + FileUtils.touch(path) + + client.cp_to_blobstore(path, 'abcdef123456') + + expect(directory.files.head('ab/cd/abcdef123456').content_type).to eq('application/zip') + end + + it 'sets content-type to mime-type of file when specified' do + path = File.join(local_dir, 'empty_file.png') + FileUtils.touch(path) + + client.cp_to_blobstore(path, 'abcdef123456') + + expect(directory.files.head('ab/cd/abcdef123456').content_type).to eq('image/png') + end + + context 'limit the file size' do + let(:min_size) { 20 } + let(:max_size) { 50 } + + subject(:client) do + FogClient.new(connection_config:, directory_key:, min_size:, max_size:) + end + + it 'does not copy files below the minimum size limit' do + path = File.join(local_dir, 'file_with_little_content') + File.write(path, 'a') + key = '987654321' + + client.cp_to_blobstore(path, key) + expect(client.exists?(key)).to be false + end + + it 'does not copy files above the maximum size limit' do + path = File.join(local_dir, 'file_with_more_content') + File.write(path, 'an amount of content that is larger than the maximum limit') + key = '777777777' + + client.cp_to_blobstore(path, key) + expect(client.exists?(key)).to be false + end + end + end + + describe '#cp_file_between_keys' do + let(:src_key) { 'abc123' } + let(:dest_key) { 'xyz789' } + + it 'copies the file from the source key to the destination key' do + upload_tmpfile(client, src_key) + client.cp_file_between_keys(src_key, dest_key) + + expect(client.exists?(dest_key)).to be true + expect(directory.files.all.length).to eq(2) + end + + context 'when the destination key has a package already' do + before do + upload_tmpfile(client, src_key) + Tempfile.open('') do |tmpfile| + tmpfile.write('This should be deleted and replaced with new file') + tmpfile.close + client.cp_to_blobstore(tmpfile.path, dest_key) + end + end + + it 'replaces the old package in the package blobstore' do + client.cp_file_between_keys(src_key, dest_key) + expect(directory.files.all.length).to eq(2) + + src_file_length = client.blob(dest_key).file.content_length + dest_file_length = client.blob(src_key).file.content_length + expect(dest_file_length).to eq(src_file_length) + end + end + + context 'when the source key has no file associated with it' do + it 'does not attempt to copy over to the destination key' do + expect do + client.cp_file_between_keys('bogus', dest_key) + end.to raise_error(CloudController::Blobstore::FileNotFound) + + expect(directory.files.all.length).to eq(0) + end + end + end + + describe '#delete_all' do + before do + client.ensure_bucket_exists + end + + it 'deletes all the files' do + first_path = File.join(local_dir, 'first_empty_file') + FileUtils.touch(first_path) + path = File.join(local_dir, 'empty_file') + FileUtils.touch(path) + + client.cp_to_blobstore(first_path, 'ab56') + expect(client.exists?('ab56')).to be true + client.cp_to_blobstore(path, 'abcdef123456') + expect(client.exists?('abcdef123456')).to be true + + client.delete_all + + expect(client.exists?('ab56')).to be false + expect(client.exists?('abcdef123456')).to be false + end + + it 'is ok if there are no files' do + expect(directory.files.all.length).to eq(0) + expect { client.delete_all }.not_to raise_error + end + + context 'when a root dir is provided' do + let(:root_dir) { 'root-dir' } + + let(:client_with_root) do + FogClient.new(connection_config:, directory_key:, root_dir:) + end + + before do + client_with_root.ensure_bucket_exists + end + + it 'only deletes files at the root' do + allow(client_with_root).to receive(:delete_files).and_call_original + + file = File.join(local_dir, 'empty_file') + FileUtils.touch(file) + + client.cp_to_blobstore(file, 'abcdef1') + client_with_root.cp_to_blobstore(file, 'abcdef2') + + client_with_root.delete_all + + expect(client_with_root).to have_received(:delete_files) do |files| + expect(files.length).to eq(1) + end + end + end + end + + describe '#delete_all_in_path' do + before do + client.ensure_bucket_exists + end + + it 'deletes all the files within a specific path' do + path = File.join(local_dir, 'empty_file') + FileUtils.touch(path) + + remote_path_1 = 'aaaaguid' + remote_key_1 = "#{remote_path_1}/stack_1" + remote_key_2 = "#{remote_path_1}/stack_2" + remote_path_2 = 'bbbbguid' + remote_key_3 = "#{remote_path_2}/stack_3" + + client.cp_to_blobstore(path, remote_key_1) + client.cp_to_blobstore(path, remote_key_2) + client.cp_to_blobstore(path, remote_key_3) + expect(client.exists?(remote_key_1)).to be true + expect(client.exists?(remote_key_2)).to be true + expect(client.exists?(remote_key_3)).to be true + + client.delete_all_in_path(remote_path_1) + + expect(client.exists?(remote_key_1)).to be false + expect(client.exists?(remote_key_2)).to be false + expect(client.exists?(remote_key_3)).to be true + end + + it 'is ok if there are no files' do + expect(directory.files.all.length).to eq(0) + expect { client.delete_all_in_path('nonsense_path') }.not_to raise_error + end + + context 'when a root dir is provided' do + let(:root_dir) { 'root-dir' } + + let(:client_with_root) do + FogClient.new(connection_config:, directory_key:, root_dir:) + end + + before do + client_with_root.ensure_bucket_exists + end + + it 'only deletes files at the root' do + path = File.join(local_dir, 'empty_file') + FileUtils.touch(path) + + remote_path_1 = 'aaaaguid' + remote_key_1 = "#{remote_path_1}/stack_1" + remote_key_2 = "#{remote_path_1}/stack_2" + remote_path_2 = 'bbbbguid' + remote_key_3 = "#{remote_path_2}/stack_3" + + client_with_root.cp_to_blobstore(path, remote_key_1) + client_with_root.cp_to_blobstore(path, remote_key_2) + client_with_root.cp_to_blobstore(path, remote_key_3) + expect(client_with_root.exists?(remote_key_1)).to be true + expect(client_with_root.exists?(remote_key_2)).to be true + expect(client_with_root.exists?(remote_key_3)).to be true + + client_with_root.delete_all_in_path(remote_path_1) + + expect(client_with_root.exists?(remote_key_1)).to be false + expect(client_with_root.exists?(remote_key_2)).to be false + expect(client_with_root.exists?(remote_key_3)).to be true + end + end + end + + describe '#delete' do + it 'deletes the file' do + path = File.join(local_dir, 'empty_file') + FileUtils.touch(path) + + client.cp_to_blobstore(path, 'abcdef123456') + expect(client.exists?('abcdef123456')).to be true + client.delete('abcdef123456') + expect(client.exists?('abcdef123456')).to be false + end + + it "is ok if the file doesn't exist" do + expect(directory.files.all.length).to eq(0) + expect { client.delete('non-existent-file') }.not_to raise_error + end + end + + describe '#delete_blob' do + it "deletes the blob's file" do + path = File.join(local_dir, 'empty_file') + FileUtils.touch(path) + + client.cp_to_blobstore(path, 'abcdef123456') + expect(client.exists?('abcdef123456')).to be(true) + + blob = client.blob('abcdef123456') + + client.delete_blob(blob) + expect(client.exists?('abcdef123456')).to be(false) + end + + it "is ok if the file doesn't exist" do + blob = FogBlob.new(nil, nil) + expect { client.delete_blob(blob) }.not_to raise_error + end + end + + describe '#ensure_bucket_exists' do + it 'gets the bucket' do + expect(fake_storage.directories).to receive(:get).with(directory_key, max_keys: 1).and_call_original + subject.ensure_bucket_exists + end + + context 'the bucket exists' do + it 'does not create the bucket' do + subject.ensure_bucket_exists + expect(fake_storage.directories).not_to receive(:create).with(key: directory_key, public: false) + subject.ensure_bucket_exists + end + end + + context 'the bucket does not exist' do + it 'creates the bucket' do + allow(fake_storage.directories).to receive(:get).with(directory_key, max_keys: 1).and_return(nil) + expect(fake_storage.directories).to receive(:create).with(key: directory_key, public: false).and_call_original + subject.ensure_bucket_exists + end + end + end + end + + context 'with root directory specified' do + let(:root_dir) { 'my-root' } + + let(:client_with_root) do + FogClient.new(connection_config:, directory_key:, root_dir:) + end + + before do + client_with_root.ensure_bucket_exists + end + + it 'includes the directory in the partitioned key' do + upload_tmpfile(client_with_root, 'abcdef') + expect(client_with_root.exists?('abcdef')).to be true + expect(client_with_root.blob('abcdef')).to be + expect(client_with_root.blob('abcdef').public_download_url).to match(%r{my-root/ab/cd/abcdef}) + end + end + + describe 'downloading without mocking' do + def wait_for_server_to_accept_requests(uri) + code = nil + total_time = 0 + while code != '200' && total_time < 10 + begin + res = Net::HTTP.get_response(URI(uri)) + code = res.code + total_time += 0.1 + sleep 0.1 + rescue StandardError + end + end + end + + describe 'from a CDN' do + let(:port) { 9875 } + let(:uri) { "http://localhost:#{port}" } + let(:cdn) { Cdn.make(uri) } + + subject(:client) do + FogClient.new(connection_config:, directory_key:, cdn:) + end + + around do |example| + WebMock.disable_net_connect!(allow_localhost: true) + example.run + WebMock.disable_net_connect! + end + + it 'correctly downloads byte streams' do + source_directory_path = File.expand_path('../../../../../fixtures/', File.dirname(__FILE__)) + source_file_path = File.join(source_directory_path, 'pa/rt/partitioned_key') + source_hexdigest = OpenSSL::Digest::SHA256.file(source_file_path).hexdigest + + pid = spawn("ruby -rwebrick -e'WEBrick::HTTPServer.new(:Port => #{port}, :DocumentRoot => \"#{source_directory_path}\").start'", out: 'test.out', err: 'test.err') + + begin + Process.detach(pid) + + wait_for_server_to_accept_requests(uri) + + destination_file_path = File.join(local_dir, 'hard_file.xyz') + + client.download_from_blobstore('partitioned_key', destination_file_path) + + destination_hexdigest = OpenSSL::Digest::SHA256.file(destination_file_path).hexdigest + + expect(destination_hexdigest).to eq(source_hexdigest) + ensure + Process.kill(9, pid) + end + end + end + + describe 'from a blobstore' do + it 'correctly downloads byte streams' do + content = 'some binary content for checksum verification' + source_file = Tempfile.new('source') + source_file.write(content) + source_file.close + + source_hexdigest = OpenSSL::Digest::SHA256.file(source_file.path).hexdigest + + client.ensure_bucket_exists + client.cp_to_blobstore(source_file.path, 'partitioned_key') + + destination_file_path = File.join(local_dir, 'hard_file.xyz') + client.download_from_blobstore('partitioned_key', destination_file_path) + + destination_hexdigest = OpenSSL::Digest::SHA256.file(destination_file_path).hexdigest + + expect(destination_hexdigest).to eq(source_hexdigest) + ensure + source_file&.unlink + end + end + end + end + end +end From 0baaa1d87120591a41b36577e2530f647fad2cd8 Mon Sep 17 00:00:00 2001 From: Katharina Przybill <30441792+kathap@users.noreply.github.com> Date: Tue, 14 Jul 2026 14:00:54 +0200 Subject: [PATCH 04/24] Revert changes to .rubocop_cc.yml --- .rubocop_cc.yml | 31 ------------------- .../blobstore/fog/fog_client_spec.rb | 6 ++-- 2 files changed, 4 insertions(+), 33 deletions(-) diff --git a/.rubocop_cc.yml b/.rubocop_cc.yml index b2516a74d23..4ddf73c0647 100644 --- a/.rubocop_cc.yml +++ b/.rubocop_cc.yml @@ -795,34 +795,3 @@ Rails/RedundantActiveRecordAllMethod: # As we use Sequel this breaks code as it Enabled: false Sequel/SaveChanges: # Breaks Code Enabled: false - - -# New cops in RuboCop 1.85 / rubocop-rspec 3.10 -Lint/DataDefineOverride: - Enabled: true -Lint/UnreachablePatternBranch: - Enabled: true -Style/FileOpen: - Enabled: true -Style/MapJoin: - Enabled: true -Style/OneClassPerFile: - Enabled: false -Style/PartitionInsteadOfDoubleSelect: - Enabled: true -Style/PredicateWithKind: - Enabled: true -Style/ReduceToHash: - Enabled: true -Style/RedundantMinMaxBy: - Enabled: true -Style/SelectByKind: - Enabled: true -Style/SelectByRange: - Enabled: true -Style/TallyMethod: - Enabled: true -RSpec/DiscardedMatcher: - Enabled: true -RSpec/MatchWithSimpleRegex: - Enabled: true diff --git a/spec/unit/lib/cloud_controller/blobstore/fog/fog_client_spec.rb b/spec/unit/lib/cloud_controller/blobstore/fog/fog_client_spec.rb index 7b21e0be9a8..78e451e23ee 100644 --- a/spec/unit/lib/cloud_controller/blobstore/fog/fog_client_spec.rb +++ b/spec/unit/lib/cloud_controller/blobstore/fog/fog_client_spec.rb @@ -9,9 +9,11 @@ module Blobstore class FakeStorage FakeFile = Struct.new(:key, :body, :content_type, :public, :content_length, :bucket, keyword_init: true) do def public_url = public ? "http://fake/#{key}" : nil - def copy(dest_bucket, dest_key, _opts = {}) + + def copy(dest_bucket, dest_key, _opts={}) storage.store_file(dest_bucket, dest_key, body, content_type) end + def destroy = storage.delete_file(bucket, key) attr_accessor :storage @@ -108,7 +110,7 @@ def create(key:, body:, content_type: 'application/zip', **) @storage.store_file(@bucket, key, body, content_type) end - def each(&block) = all.each(&block) + def each(&) = all.each(&) def all = @storage.list_files(@bucket) def select(prefix: nil) From 09242328572382d7c1685899fc31b83408cfc8d2 Mon Sep 17 00:00:00 2001 From: Katharina Przybill <30441792+kathap@users.noreply.github.com> Date: Tue, 14 Jul 2026 14:49:16 +0200 Subject: [PATCH 05/24] comment adjusted --- .../blobstore/storage_cli/storage_cli_client.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/cloud_controller/blobstore/storage_cli/storage_cli_client.rb b/lib/cloud_controller/blobstore/storage_cli/storage_cli_client.rb index 960ff2a1298..7f3d5cc36fb 100644 --- a/lib/cloud_controller/blobstore/storage_cli/storage_cli_client.rb +++ b/lib/cloud_controller/blobstore/storage_cli/storage_cli_client.rb @@ -20,7 +20,7 @@ class StorageCliClient < BaseClient # Native storage-cli type names supported by CC STORAGE_CLI_TYPES = %w[azurebs alioss s3 gcs dav].freeze - # DEPRECATED: Legacy fog provider names (remove after migration window) + # DEPRECATED: Legacy fog provider names (remove completely after migration window) LEGACY_PROVIDER_TO_STORAGE_CLI_TYPE = { 'webdav' => 'dav' }.freeze From 698ab0e220edf7280093ffe32365e5078f481554 Mon Sep 17 00:00:00 2001 From: Katharina Przybill <30441792+kathap@users.noreply.github.com> Date: Tue, 14 Jul 2026 15:24:27 +0200 Subject: [PATCH 06/24] fix: memoize FakeStorage#directories so stub expectations hold Each call to the non-memoized method returned a different proxy object, so stubs and expect().to receive() set on one proxy were never observed by FogClient which called directories on a different instance. --- .../lib/cloud_controller/blobstore/fog/fog_client_spec.rb | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/spec/unit/lib/cloud_controller/blobstore/fog/fog_client_spec.rb b/spec/unit/lib/cloud_controller/blobstore/fog/fog_client_spec.rb index 78e451e23ee..f4a4ba39cde 100644 --- a/spec/unit/lib/cloud_controller/blobstore/fog/fog_client_spec.rb +++ b/spec/unit/lib/cloud_controller/blobstore/fog/fog_client_spec.rb @@ -21,11 +21,10 @@ def destroy = storage.delete_file(bucket, key) def initialize @store = {} + @directories = DirectoryProxy.new(self) end - def directories = DirectoryProxy.new(self) - - def respond_to?(method, *) = method == :directories || super + attr_reader :directories def store_file(bucket, key, body, content_type) @store[bucket] ||= {} From d4349f3e9c09d87e179a932603822c0175c4de3f Mon Sep 17 00:00:00 2001 From: Katharina Przybill <30441792+kathap@users.noreply.github.com> Date: Tue, 14 Jul 2026 17:11:15 +0200 Subject: [PATCH 07/24] Repair FakeStorage test stub for fog_client_spec --- spec/support/bootstrap/test_config.rb | 8 +++--- .../blobstore/fog/fog_client_spec.rb | 28 +++++++++++++------ 2 files changed, 23 insertions(+), 13 deletions(-) diff --git a/spec/support/bootstrap/test_config.rb b/spec/support/bootstrap/test_config.rb index b09d4686f7b..c0199ca7541 100644 --- a/spec/support/bootstrap/test_config.rb +++ b/spec/support/bootstrap/test_config.rb @@ -46,20 +46,20 @@ def defaults nginx: { use_nginx: true }, resource_pool: { resource_directory_key: 'spec-cc-resources', - fog_connection: {} + blobstore_type: 'local-temp-storage' }, packages: { app_package_directory_key: 'cc-packages', - fog_connection: {}, + blobstore_type: 'local-temp-storage', max_valid_packages_stored: 42 }, buildpacks: { buildpack_directory_key: 'cc-buildpacks', - fog_connection: {} + blobstore_type: 'local-temp-storage' }, droplets: { droplet_directory_key: 'cc-droplets', - fog_connection: {}, + blobstore_type: 'local-temp-storage', max_staged_droplets_stored: 42 }, db: DbConfig.new.config diff --git a/spec/unit/lib/cloud_controller/blobstore/fog/fog_client_spec.rb b/spec/unit/lib/cloud_controller/blobstore/fog/fog_client_spec.rb index f4a4ba39cde..79cb6f2e94a 100644 --- a/spec/unit/lib/cloud_controller/blobstore/fog/fog_client_spec.rb +++ b/spec/unit/lib/cloud_controller/blobstore/fog/fog_client_spec.rb @@ -9,6 +9,7 @@ module Blobstore class FakeStorage FakeFile = Struct.new(:key, :body, :content_type, :public, :content_length, :bucket, keyword_init: true) do def public_url = public ? "http://fake/#{key}" : nil + def url(_expires) = "http://fake/#{key}" def copy(dest_bucket, dest_key, _opts={}) storage.store_file(dest_bucket, dest_key, body, content_type) @@ -61,10 +62,10 @@ def create_bucket(key) class DirectoryProxy def initialize(storage) = @storage = storage - def get(key, **) + def get(key, _options=nil, prefix: nil, **_kwargs) return nil unless @storage.bucket_exists?(key) - BucketProxy.new(@storage, key) + BucketProxy.new(@storage, key, prefix:) end def create(key:, **) = BucketProxy.new(@storage, @storage.create_bucket(key) && key) @@ -74,23 +75,27 @@ def new(key:, **) = BucketProxy.new(@storage, key) class BucketProxy attr_reader :key - def initialize(storage, key) + def initialize(storage, key, prefix: nil) @storage = storage @key = key + @prefix = prefix @storage.create_bucket(key) end - def files = FilesProxy.new(@storage, @key) + def files = FilesProxy.new(@storage, @key, prefix: @prefix) end class FilesProxy include Enumerable - def initialize(storage, bucket) + def initialize(storage, bucket, prefix: nil) @storage = storage @bucket = bucket + @prefix = prefix end + delegate :length, to: :all + def head(key) = @storage.get_file(@bucket, key) def get(key, &block) @@ -105,12 +110,17 @@ def get(key, &block) file end - def create(key:, body:, content_type: 'application/zip', **) + def create(options_or_key=nil, key: nil, body: nil, content_type: 'application/zip', **) + if options_or_key.is_a?(Hash) + key = options_or_key[:key] + body = options_or_key[:body] + content_type = options_or_key[:content_type] || 'application/zip' + end @storage.store_file(@bucket, key, body, content_type) end def each(&) = all.each(&) - def all = @storage.list_files(@bucket) + def all = @all ||= @storage.list_files(@bucket, prefix: @prefix) def select(prefix: nil) @storage.list_files(@bucket, prefix:) @@ -635,7 +645,7 @@ def upload_tmpfile(client, key='abcdef') describe '#ensure_bucket_exists' do it 'gets the bucket' do - expect(fake_storage.directories).to receive(:get).with(directory_key, max_keys: 1).and_call_original + expect(fake_storage.directories).to receive(:get).with(directory_key, { max_keys: 1 }).and_call_original subject.ensure_bucket_exists end @@ -649,7 +659,7 @@ def upload_tmpfile(client, key='abcdef') context 'the bucket does not exist' do it 'creates the bucket' do - allow(fake_storage.directories).to receive(:get).with(directory_key, max_keys: 1).and_return(nil) + allow(fake_storage.directories).to receive(:get).with(directory_key, { max_keys: 1 }).and_return(nil) expect(fake_storage.directories).to receive(:create).with(key: directory_key, public: false).and_call_original subject.ensure_bucket_exists end From 259575cfc9b425366f1212a1856cc2b156c32e80 Mon Sep 17 00:00:00 2001 From: Katharina Przybill <30441792+kathap@users.noreply.github.com> Date: Wed, 15 Jul 2026 09:16:10 +0200 Subject: [PATCH 08/24] Fix delegate --- spec/unit/lib/cloud_controller/blobstore/fog/fog_client_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/unit/lib/cloud_controller/blobstore/fog/fog_client_spec.rb b/spec/unit/lib/cloud_controller/blobstore/fog/fog_client_spec.rb index 79cb6f2e94a..2f155893213 100644 --- a/spec/unit/lib/cloud_controller/blobstore/fog/fog_client_spec.rb +++ b/spec/unit/lib/cloud_controller/blobstore/fog/fog_client_spec.rb @@ -94,7 +94,7 @@ def initialize(storage, bucket, prefix: nil) @prefix = prefix end - delegate :length, to: :all + def length = all.length # rubocop:disable Rails/Delegate def head(key) = @storage.get_file(@bucket, key) From 92a3cca57e09782c2d551912d38905136930819a Mon Sep 17 00:00:00 2001 From: Katharina Przybill <30441792+kathap@users.noreply.github.com> Date: Wed, 15 Jul 2026 09:41:11 +0200 Subject: [PATCH 09/24] Fix OutsideOfExampleError by scoping LocalClient stub to specific instance allow_any_instance_of(LocalClient) patches the class itself, leaving RSpec mock state that fires when LocalClient at_exit hooks run after the suite ends (triggered by use_temp_storage: true instances from test_config defaults). Stubbing the specific package_blobstore instance avoids this entirely. --- .../lib/cloud_controller/packager/local_bits_packer_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/unit/lib/cloud_controller/packager/local_bits_packer_spec.rb b/spec/unit/lib/cloud_controller/packager/local_bits_packer_spec.rb index b7a832487fb..e1c1fc1adf4 100644 --- a/spec/unit/lib/cloud_controller/packager/local_bits_packer_spec.rb +++ b/spec/unit/lib/cloud_controller/packager/local_bits_packer_spec.rb @@ -347,7 +347,7 @@ module CloudController::Packager context 'when the package is successfully uploaded and processed' do it 'removes the temporary directory created for packaging' do - allow_any_instance_of(CloudController::Blobstore::LocalClient).to receive(:cp_to_blobstore) + allow(package_blobstore).to receive(:cp_to_blobstore) allow(Digester).to receive(:new).and_return(instance_double(Digester, digest_path: 'fake-digest')) expect(FileUtils).to receive(:remove_dir).with(local_bits_packer_path).and_call_original packer.send_package_to_blobstore(blobstore_key, uploaded_files_path, cached_files_fingerprints) @@ -362,7 +362,7 @@ module CloudController::Packager expect(FileUtils).to receive(:remove_dir).with(local_bits_packer_path) expect do - allow_any_instance_of(CloudController::Blobstore::LocalClient).to receive(:cp_to_blobstore).and_raise(StandardError) + allow(package_blobstore).to receive(:cp_to_blobstore).and_raise(StandardError) packer.send_package_to_blobstore(blobstore_key, uploaded_files_path, cached_files_fingerprints) end.to raise_error(StandardError) end From 2733ecbb4368431825e4866f8c22b740e83f67c3 Mon Sep 17 00:00:00 2001 From: Katharina Przybill <30441792+kathap@users.noreply.github.com> Date: Wed, 15 Jul 2026 11:38:44 +0200 Subject: [PATCH 10/24] Fix tests --- spec/support/bootstrap/test_config.rb | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/spec/support/bootstrap/test_config.rb b/spec/support/bootstrap/test_config.rb index c0199ca7541..2dec5678e42 100644 --- a/spec/support/bootstrap/test_config.rb +++ b/spec/support/bootstrap/test_config.rb @@ -42,24 +42,30 @@ def defaults config_file = File.join(Paths::CONFIG, 'cloud_controller.yml') config_hash = VCAP::CloudController::Config.load_from_file(config_file, context:).config_hash + fog_connection = { + blobstore_timeout: 5, + provider: 'Local', + local_root: Dir.mktmpdir + } + config_hash.update( nginx: { use_nginx: true }, resource_pool: { resource_directory_key: 'spec-cc-resources', - blobstore_type: 'local-temp-storage' + fog_connection: fog_connection }, packages: { app_package_directory_key: 'cc-packages', - blobstore_type: 'local-temp-storage', + fog_connection: fog_connection, max_valid_packages_stored: 42 }, buildpacks: { buildpack_directory_key: 'cc-buildpacks', - blobstore_type: 'local-temp-storage' + fog_connection: fog_connection }, droplets: { droplet_directory_key: 'cc-droplets', - blobstore_type: 'local-temp-storage', + fog_connection: fog_connection, max_staged_droplets_stored: 42 }, db: DbConfig.new.config From ff084f778a9712459b449c367d6fe773ce2d8076 Mon Sep 17 00:00:00 2001 From: Katharina Przybill <30441792+kathap@users.noreply.github.com> Date: Wed, 15 Jul 2026 12:44:43 +0200 Subject: [PATCH 11/24] Remove fog completely --- Gemfile | 4 +- Gemfile.lock | 6 - config/bosh-lite.yml | 4 - config/cloud_controller.yml | 4 - .../blobstore/client_provider.rb | 28 - lib/cloud_controller/blobstore/fog/cdn.rb | 42 - .../blobstore/fog/fog_blob.rb | 40 - .../blobstore/fog/fog_client.rb | 188 ----- .../blobstore/fog/providers.rb | 1 - .../blobstore/retryable_client.rb | 2 +- .../config_schemas/api_schema.rb | 4 - .../blobstore_benchmarks_schema.rb | 1 - .../config_schemas/clock_schema.rb | 4 - .../deployment_updater_schema.rb | 4 - .../config_schemas/worker_schema.rb | 4 - lib/cloud_controller/resource_pool.rb | 1 - spec/fixtures/config/port_8181_config.yml | 24 +- spec/spec_helper_helper.rb | 9 - spec/support/bootstrap/test_config.rb | 27 +- .../app_bits_download_controller_spec.rb | 2 +- .../runtime/apps_controller_spec.rb | 2 +- .../runtime/helpers/blob_dispatcher_spec.rb | 2 +- .../runtime/stagings_controller_spec.rb | 6 +- .../v3/packages_controller_spec.rb | 2 +- .../runtime/orphaned_blobs_cleanup_spec.rb | 4 +- .../blob_sender/nginx_blob_sender_spec.rb | 2 +- .../blobstore/client_provider_spec.rb | 55 -- .../blobstore/fog/cdn_spec.rb | 88 -- .../blobstore/fog/fog_blob_spec.rb | 122 --- .../blobstore/fog/fog_client_spec.rb | 769 ------------------ .../internal_url_generator_spec.rb | 2 +- spec/unit/lib/cloud_controller/config_spec.rb | 9 - .../dependency_locator_spec.rb | 5 - 33 files changed, 23 insertions(+), 1444 deletions(-) delete mode 100644 lib/cloud_controller/blobstore/fog/cdn.rb delete mode 100644 lib/cloud_controller/blobstore/fog/fog_blob.rb delete mode 100644 lib/cloud_controller/blobstore/fog/fog_client.rb delete mode 100644 lib/cloud_controller/blobstore/fog/providers.rb delete mode 100644 spec/unit/lib/cloud_controller/blobstore/fog/cdn_spec.rb delete mode 100644 spec/unit/lib/cloud_controller/blobstore/fog/fog_blob_spec.rb delete mode 100644 spec/unit/lib/cloud_controller/blobstore/fog/fog_client_spec.rb diff --git a/Gemfile b/Gemfile index a1cec743688..b56fef87afb 100644 --- a/Gemfile +++ b/Gemfile @@ -40,9 +40,7 @@ gem 'actionview', '~> 8.1.3' gem 'activemodel', '~> 8.1.2' gem 'railties', '~> 8.1.1' -gem 'fog-core', '~> 2.6.0' - -gem 'cf-uaa-lib', '~> 4.0.10' +gem 'cf-uaa-lib', '~> 4.0.10', '~> 4.0.10' group :db do gem 'mysql2', '~> 0.5.7' diff --git a/Gemfile.lock b/Gemfile.lock index 101913b4886..616c70986bc 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -90,11 +90,6 @@ GEM fluent-logger (0.10.0) logger (~> 1.6) msgpack (>= 1.0.0, < 2) - fog-core (2.6.0) - builder - excon (~> 1.0) - formatador (>= 0.2, < 2.0) - mime-types formatador (1.2.3) reline google-protobuf (4.35.1) @@ -470,7 +465,6 @@ DEPENDENCIES digest-xxhash factory_bot (~> 6.5) fluent-logger - fog-core (~> 2.6.0) googleapis-common-protos (>= 1.8.0) hashdiff httpclient diff --git a/config/bosh-lite.yml b/config/bosh-lite.yml index acbe68a9b00..0d88a230a38 100644 --- a/config/bosh-lite.yml +++ b/config/bosh-lite.yml @@ -136,7 +136,6 @@ resource_pool: uri: key_pair_id: private_key: "" - fog_connection: {} packages: blobstore_type: webdav @@ -153,7 +152,6 @@ packages: uri: key_pair_id: private_key: "" - fog_connection: {} droplets: blobstore_type: webdav @@ -168,7 +166,6 @@ droplets: uri: key_pair_id: private_key: "" - fog_connection: {} max_staged_droplets_stored: 5 buildpacks: @@ -184,7 +181,6 @@ buildpacks: uri: key_pair_id: private_key: "" - fog_connection: {} db_encryption_key: db-encryption-key diff --git a/config/cloud_controller.yml b/config/cloud_controller.yml index 7f79f98ecfd..45468fd1e81 100644 --- a/config/cloud_controller.yml +++ b/config/cloud_controller.yml @@ -182,25 +182,21 @@ resource_pool: minimum_size: 1 resource_directory_key: "spec-cc-resources" blobstore_type: local-temp-storage - fog_connection: {} packages: app_package_directory_key: "cc-packages" max_package_size: 42 max_valid_packages_stored: 42 blobstore_type: local-temp-storage - fog_connection: {} droplets: droplet_directory_key: cc-droplets max_staged_droplets_stored: 42 blobstore_type: local-temp-storage - fog_connection: {} buildpacks: buildpack_directory_key: cc-buildpacks blobstore_type: local-temp-storage - fog_connection: {} db_encryption_key: "asdfasdfasdf" database_encryption: diff --git a/lib/cloud_controller/blobstore/client_provider.rb b/lib/cloud_controller/blobstore/client_provider.rb index 075d294b093..8f7ec528604 100644 --- a/lib/cloud_controller/blobstore/client_provider.rb +++ b/lib/cloud_controller/blobstore/client_provider.rb @@ -1,6 +1,5 @@ require 'cloud_controller/blobstore/client' require 'cloud_controller/blobstore/retryable_client' -require 'cloud_controller/blobstore/fog/fog_client' require 'cloud_controller/blobstore/error_handling_client' require 'cloud_controller/blobstore/webdav/dav_client' require 'cloud_controller/blobstore/local/local_client' @@ -18,8 +17,6 @@ def self.provide(options:, directory_key:, root_dir: nil, resource_type: nil) provide_local(options, directory_key, root_dir, use_temp_storage: true) when 'storage-cli' provide_storage_cli(options, directory_key, root_dir, resource_type) - when 'fog', nil, '' - provide_fog(options, directory_key, root_dir) else provide_webdav(options, directory_key, root_dir) end @@ -28,31 +25,6 @@ def self.provide(options:, directory_key:, root_dir: nil, resource_type: nil) class << self private - def provide_fog(options, directory_key, root_dir) - cdn_uri = HashUtils.dig(options[:cdn], :uri) - cdn = CloudController::Blobstore::Cdn.make(cdn_uri) - - client = FogClient.new( - connection_config: options.fetch(:fog_connection), - directory_key: directory_key, - cdn: cdn, - root_dir: root_dir, - min_size: options[:minimum_size], - max_size: options[:maximum_size] - ) - - logger = Steno.logger('cc.blobstore') - - # work around https://github.com/fog/fog/issues/3137 - # and Fog raising an EOFError SocketError intermittently - # and intermittent blobstore download errors - errors = [Excon::Errors::BadRequest, Excon::Errors::SocketError, SystemCallError, - Excon::Errors::InternalServerError, Excon::Errors::ServiceUnavailable, OpenSSL::OpenSSLError] - retryable_client = RetryableClient.new(client:, errors:, logger:) - - Client.new(ErrorHandlingClient.new(SafeDeleteClient.new(retryable_client, root_dir))) - end - def provide_local(options, directory_key, root_dir, use_temp_storage:) client = LocalClient.new( directory_key: directory_key, diff --git a/lib/cloud_controller/blobstore/fog/cdn.rb b/lib/cloud_controller/blobstore/fog/cdn.rb deleted file mode 100644 index 2e1475cc0b4..00000000000 --- a/lib/cloud_controller/blobstore/fog/cdn.rb +++ /dev/null @@ -1,42 +0,0 @@ -module CloudController - module Blobstore - class Cdn - def self.make(host) - return nil if host.nil? || host == '' - - new(host) - end - - attr_reader :host - - def initialize(host) - @host = host - end - - def get(path, &block) - retries = 0 - begin - client = HTTPClient.new - client.ssl_config.set_default_paths - client.get(download_uri(path)) do |chunk| - block.yield chunk - end - rescue StandardError - raise if retries > 1 - - retries += 1 - retry - end - end - - def download_uri(path) - url = "#{host}/#{path}" - url = Aws::CF::Signer.sign_url(url) if Aws::CF::Signer.is_configured? - url - end - - # Don't call new directly because there's logic in .make - private_class_method(:new) - end - end -end diff --git a/lib/cloud_controller/blobstore/fog/fog_blob.rb b/lib/cloud_controller/blobstore/fog/fog_blob.rb deleted file mode 100644 index b37237ec832..00000000000 --- a/lib/cloud_controller/blobstore/fog/fog_blob.rb +++ /dev/null @@ -1,40 +0,0 @@ -module CloudController - module Blobstore - class FogBlob < Blob - attr_reader :file - - def initialize(file, cdn) - @file = file - @cdn = cdn - end - - def local_path - file.send(:path) - end - - def internal_download_url - download_uri_for_file - end - - def public_download_url - download_uri_for_file - end - - def attributes(*keys) - return file.attributes if keys.empty? - - file.attributes.select { |key, _| keys.include? key } - end - - private - - def download_uri_for_file - return @cdn.download_uri(file.key) if @cdn - - return file.url(Time.now.utc + 3600) if file.respond_to?(:url) - - file.public_url - end - end - end -end diff --git a/lib/cloud_controller/blobstore/fog/fog_client.rb b/lib/cloud_controller/blobstore/fog/fog_client.rb deleted file mode 100644 index 7bf689a421b..00000000000 --- a/lib/cloud_controller/blobstore/fog/fog_client.rb +++ /dev/null @@ -1,188 +0,0 @@ -require 'fileutils' -require 'find' -require 'mime-types' -require 'cloud_controller/blobstore/fog/providers' -require 'cloud_controller/blobstore/base_client' -require 'cloud_controller/blobstore/fog/fog_blob' -require 'cloud_controller/blobstore/fog/cdn' -require 'cloud_controller/blobstore/errors' - -module CloudController - module Blobstore - class FogClient < BaseClient - attr_reader :root_dir - - DEFAULT_BATCH_SIZE = 1000 - DEPRECATION_MESSAGE = '[DEPRECATION WARNING] The fog blobstore client is DEPRECATED and will be REMOVED. ' \ - "Please migrate to blobstore_type 'storage-cli' as soon as possible. " \ - 'See https://github.com/cloudfoundry/community/blob/main/toc/rfc/rfc-0043-cc-blobstore-storage-cli.md for details.'.freeze - - def initialize(connection_config:, - directory_key:, - cdn: nil, - root_dir: nil, - min_size: nil, - max_size: nil) - @root_dir = root_dir - @connection_config = connection_config - @directory_key = directory_key - @cdn = cdn - @min_size = min_size || 0 - @max_size = max_size - logger.warn('blobstore.fog-deprecated', message: DEPRECATION_MESSAGE) - end - - def local? - false - end - - def exists?(key) - !file(key).nil? - end - - def download_from_blobstore(source_key, destination_path, mode: nil) - FileUtils.mkdir_p(File.dirname(destination_path)) - File.open(destination_path, 'wb') do |file| - (@cdn || files).get(partitioned_key(source_key)) do |*chunk| - file.write(chunk[0]) - end - file.chmod(mode) if mode - end - end - - def cp_to_blobstore(source_path, destination_key) - start = Time.now.utc - logger.info('blobstore.cp-start', destination_key: destination_key, source_path: source_path, bucket: @directory_key) - size = -1 - log_entry = 'blobstore.cp-skip' - - File.open(source_path) do |file| - size = file.size - next unless within_limits?(size) - - mime_type = MIME::Types.of(source_path).first.try(:content_type) - - options = { - key: partitioned_key(destination_key), - body: file, - content_type: mime_type || 'application/zip', - public: false - }.merge(formatted_storage_options) - - files.create(options) - - log_entry = 'blobstore.cp-finish' - end - - duration = Time.now.utc - start - logger.info(log_entry, - destination_key: destination_key, - duration_seconds: duration, - size: size) - end - - def cp_file_between_keys(source_key, destination_key) - source_file = file(source_key) - raise FileNotFound if source_file.nil? - - source_file.copy(@directory_key, partitioned_key(destination_key), formatted_storage_options) - end - - def delete_all(page_size=DEFAULT_BATCH_SIZE) - logger.info("Attempting to delete all files in #{@directory_key}/#{@root_dir} blobstore") - - delete_files(files_for(@root_dir), page_size) - end - - def delete_all_in_path(path) - logger.info("Attempting to delete all files in blobstore #{@directory_key} under path #{@directory_key}/#{partitioned_key(path)}") - - delete_files(files_for(partitioned_key(path)), DEFAULT_BATCH_SIZE) - end - - def delete(key) - blob_file = file(key) - delete_file(blob_file) if blob_file - end - - def delete_blob(blob) - delete_file(blob.file) if blob.file - end - - def blob(key, **) - f = file(key) - FogBlob.new(f, @cdn) if f - end - - def files_for(prefix, _ignored_directory_prefixes=[]) - connection.directories.get(dir.key, prefix:).files - end - - def ensure_bucket_exists - options = { max_keys: 1 } - connection.directories.get(@directory_key, options) || connection.directories.create(key: @directory_key, public: false) - end - - private - - def files - dir.files - end - - def formatted_storage_options - {} - end - - def delete_file(file) - file.destroy - end - - def delete_files(files_to_delete, page_size) - if connection.respond_to?(:delete_multiple_objects) - each_slice(files_to_delete, page_size) do |file_group| - connection.delete_multiple_objects(@directory_key, file_group.map(&:key)) - end - else - files_to_delete.each { |f| delete_file(f) } - end - end - - def each_slice(files, batch_size) - batch = [] - files.each do |f| - batch << f - - if batch.length == batch_size - yield(batch) - batch = [] - end - end - - return if batch.empty? - - yield(batch) - end - - def file(key) - files.head(partitioned_key(key)) - end - - def dir - @dir ||= connection.directories.new(key: @directory_key) - end - - def connection - options = @connection_config - blobstore_timeout = options.delete(:blobstore_timeout) - connection_options = options[:connection_options] || {} - connection_options = connection_options.merge(read_timeout: blobstore_timeout, write_timeout: blobstore_timeout) - options = options.merge(connection_options:) - @connection ||= Fog::Storage.new(options) - end - - def logger - @logger ||= Steno.logger('cc.blobstore') - end - end - end -end diff --git a/lib/cloud_controller/blobstore/fog/providers.rb b/lib/cloud_controller/blobstore/fog/providers.rb deleted file mode 100644 index f79351e864e..00000000000 --- a/lib/cloud_controller/blobstore/fog/providers.rb +++ /dev/null @@ -1 +0,0 @@ -require 'fog/core' diff --git a/lib/cloud_controller/blobstore/retryable_client.rb b/lib/cloud_controller/blobstore/retryable_client.rb index c0e1488fd49..3a3826508da 100644 --- a/lib/cloud_controller/blobstore/retryable_client.rb +++ b/lib/cloud_controller/blobstore/retryable_client.rb @@ -70,7 +70,7 @@ def cp_file_between_keys(source_key, destination_key) end end - def delete_all(page_size=FogClient::DEFAULT_BATCH_SIZE) + def delete_all(page_size=1000) with_retries(__method__.to_s, { args: { page_size: diff --git a/lib/cloud_controller/config_schemas/api_schema.rb b/lib/cloud_controller/config_schemas/api_schema.rb index eb421c10577..6e50f6906ac 100644 --- a/lib/cloud_controller/config_schemas/api_schema.rb +++ b/lib/cloud_controller/config_schemas/api_schema.rb @@ -213,7 +213,6 @@ class ApiSchema < VCAP::Config resource_directory_key: String, optional(:blobstore_type) => String, optional(:local_blobstore_path) => String, - fog_connection: Hash, optional(:connection_config) => Hash, optional(:webdav_config) => Hash, optional(:cdn) => Hash @@ -223,7 +222,6 @@ class ApiSchema < VCAP::Config buildpack_directory_key: String, optional(:blobstore_type) => String, optional(:local_blobstore_path) => String, - fog_connection: Hash, optional(:connection_config) => Hash, optional(:webdav_config) => Hash, optional(:cdn) => Hash @@ -235,7 +233,6 @@ class ApiSchema < VCAP::Config app_package_directory_key: String, optional(:blobstore_type) => String, optional(:local_blobstore_path) => String, - fog_connection: Hash, optional(:connection_config) => Hash, optional(:webdav_config) => Hash, optional(:cdn) => Hash @@ -246,7 +243,6 @@ class ApiSchema < VCAP::Config max_staged_droplets_stored: Integer, optional(:blobstore_type) => String, optional(:local_blobstore_path) => String, - fog_connection: Hash, optional(:connection_config) => Hash, optional(:webdav_config) => Hash, optional(:cdn) => Hash diff --git a/lib/cloud_controller/config_schemas/blobstore_benchmarks_schema.rb b/lib/cloud_controller/config_schemas/blobstore_benchmarks_schema.rb index d924ea4910d..85daf240165 100644 --- a/lib/cloud_controller/config_schemas/blobstore_benchmarks_schema.rb +++ b/lib/cloud_controller/config_schemas/blobstore_benchmarks_schema.rb @@ -11,7 +11,6 @@ class BlobstoreBenchmarksSchema < VCAP::Config optional(:local_blobstore_path) => String, optional(:connection_config) => Hash, - optional(:fog_connection) => Hash, optional(:webdav_config) => Hash, optional(:cdn) => Hash, diff --git a/lib/cloud_controller/config_schemas/clock_schema.rb b/lib/cloud_controller/config_schemas/clock_schema.rb index 5f79e05f4d2..9cac053cb40 100644 --- a/lib/cloud_controller/config_schemas/clock_schema.rb +++ b/lib/cloud_controller/config_schemas/clock_schema.rb @@ -128,7 +128,6 @@ class ClockSchema < VCAP::Config resource_directory_key: String, optional(:blobstore_type) => String, optional(:local_blobstore_path) => String, - fog_connection: Hash, optional(:connection_config) => Hash, optional(:webdav_config) => Hash, optional(:cdn) => Hash @@ -138,7 +137,6 @@ class ClockSchema < VCAP::Config buildpack_directory_key: String, optional(:blobstore_type) => String, optional(:local_blobstore_path) => String, - fog_connection: Hash, optional(:connection_config) => Hash, optional(:webdav_config) => Hash, optional(:cdn) => Hash @@ -149,7 +147,6 @@ class ClockSchema < VCAP::Config app_package_directory_key: String, optional(:blobstore_type) => String, optional(:local_blobstore_path) => String, - fog_connection: Hash, optional(:connection_config) => Hash, optional(:webdav_config) => Hash, optional(:cdn) => Hash @@ -159,7 +156,6 @@ class ClockSchema < VCAP::Config droplet_directory_key: String, optional(:blobstore_type) => String, optional(:local_blobstore_path) => String, - fog_connection: Hash, optional(:connection_config) => Hash, optional(:webdav_config) => Hash, optional(:cdn) => Hash diff --git a/lib/cloud_controller/config_schemas/deployment_updater_schema.rb b/lib/cloud_controller/config_schemas/deployment_updater_schema.rb index 9936e8343b0..a96c9eaf461 100644 --- a/lib/cloud_controller/config_schemas/deployment_updater_schema.rb +++ b/lib/cloud_controller/config_schemas/deployment_updater_schema.rb @@ -123,7 +123,6 @@ class DeploymentUpdaterSchema < VCAP::Config resource_directory_key: String, optional(:blobstore_type) => String, optional(:local_blobstore_path) => String, - fog_connection: Hash, optional(:connection_config) => Hash, optional(:webdav_config) => Hash, optional(:cdn) => Hash @@ -133,7 +132,6 @@ class DeploymentUpdaterSchema < VCAP::Config buildpack_directory_key: String, optional(:blobstore_type) => String, optional(:local_blobstore_path) => String, - fog_connection: Hash, optional(:connection_config) => Hash, optional(:webdav_config) => Hash, optional(:cdn) => Hash @@ -144,7 +142,6 @@ class DeploymentUpdaterSchema < VCAP::Config app_package_directory_key: String, optional(:blobstore_type) => String, optional(:local_blobstore_path) => String, - fog_connection: Hash, optional(:connection_config) => Hash, optional(:webdav_config) => Hash, optional(:cdn) => Hash @@ -154,7 +151,6 @@ class DeploymentUpdaterSchema < VCAP::Config droplet_directory_key: String, optional(:blobstore_type) => String, optional(:local_blobstore_path) => String, - fog_connection: Hash, optional(:connection_config) => Hash, optional(:webdav_config) => Hash, optional(:cdn) => Hash diff --git a/lib/cloud_controller/config_schemas/worker_schema.rb b/lib/cloud_controller/config_schemas/worker_schema.rb index 13991bd1c62..f3f1606da6c 100644 --- a/lib/cloud_controller/config_schemas/worker_schema.rb +++ b/lib/cloud_controller/config_schemas/worker_schema.rb @@ -114,7 +114,6 @@ class WorkerSchema < VCAP::Config resource_directory_key: String, optional(:blobstore_type) => String, optional(:local_blobstore_path) => String, - fog_connection: Hash, optional(:connection_config) => Hash, optional(:webdav_config) => Hash, optional(:cdn) => Hash @@ -124,7 +123,6 @@ class WorkerSchema < VCAP::Config buildpack_directory_key: String, optional(:blobstore_type) => String, optional(:local_blobstore_path) => String, - fog_connection: Hash, optional(:connection_config) => Hash, optional(:webdav_config) => Hash, optional(:cdn) => Hash @@ -136,7 +134,6 @@ class WorkerSchema < VCAP::Config app_package_directory_key: String, optional(:blobstore_type) => String, optional(:local_blobstore_path) => String, - fog_connection: Hash, optional(:connection_config) => Hash, optional(:webdav_config) => Hash, optional(:cdn) => Hash @@ -146,7 +143,6 @@ class WorkerSchema < VCAP::Config droplet_directory_key: String, optional(:blobstore_type) => String, optional(:local_blobstore_path) => String, - fog_connection: Hash, optional(:connection_config) => Hash, optional(:webdav_config) => Hash, optional(:cdn) => Hash diff --git a/lib/cloud_controller/resource_pool.rb b/lib/cloud_controller/resource_pool.rb index 99bb5fde58a..82be1ce3a17 100644 --- a/lib/cloud_controller/resource_pool.rb +++ b/lib/cloud_controller/resource_pool.rb @@ -6,7 +6,6 @@ require 'httpclient' require 'steno/steno' -require 'cloud_controller/blobstore/fog/providers' require 'cloud_controller/resource_match' class VCAP::CloudController::ResourcePool diff --git a/spec/fixtures/config/port_8181_config.yml b/spec/fixtures/config/port_8181_config.yml index d6f512dfabd..0f16f3da8d8 100644 --- a/spec/fixtures/config/port_8181_config.yml +++ b/spec/fixtures/config/port_8181_config.yml @@ -84,27 +84,19 @@ shared_isolation_segment_name: 'shared' resource_pool: resource_directory_key: "cc-resources" - fog_connection: - provider: Local - local_root: /tmp + blobstore_type: local-temp-storage packages: app_package_directory_key: "cc-packages" - fog_connection: - provider: Local - local_root: /tmp + blobstore_type: local-temp-storage droplets: droplet_directory_key: "cc-droplets" - fog_connection: - provider: Local - local_root: /tmp + blobstore_type: local-temp-storage buildpacks: buildpack_directory_key: "cc-buildpacks" - fog_connection: - provider: Local - local_root: /tmp + blobstore_type: local-temp-storage db_encryption_key: some-key @@ -171,22 +163,22 @@ resource_pool: resource_directory_key: '' maximum_size: 42 minimum_size: 1 - fog_connection: {} + blobstore_type: local-temp-storage buildpacks: buildpack_directory_key: '' - fog_connection: {} + blobstore_type: local-temp-storage packages: app_package_directory_key: '' max_package_size: 42 max_valid_packages_stored: 42 - fog_connection: {} + blobstore_type: local-temp-storage droplets: droplet_directory_key: '' max_staged_droplets_stored: 42 - fog_connection: {} + blobstore_type: local-temp-storage diff --git a/spec/spec_helper_helper.rb b/spec/spec_helper_helper.rb index 278ce6cfeeb..42c0822082a 100644 --- a/spec/spec_helper_helper.rb +++ b/spec/spec_helper_helper.rb @@ -138,15 +138,6 @@ def self.each_run TestConfig.context = example.metadata[:job_context] || :api TestConfig.reset - Fog::Mock.reset - - if Fog.mock? - CloudController::DependencyLocator.instance.droplet_blobstore.ensure_bucket_exists - CloudController::DependencyLocator.instance.package_blobstore.ensure_bucket_exists - CloudController::DependencyLocator.instance.global_app_bits_cache.ensure_bucket_exists - CloudController::DependencyLocator.instance.buildpack_blobstore.ensure_bucket_exists - end - VCAP::CloudController::SecurityContext.clear VCAP::Request.current_id = nil allow_any_instance_of(VCAP::CloudController::UaaTokenDecoder).to receive(:uaa_issuer).and_return(UAAIssuer::ISSUER) diff --git a/spec/support/bootstrap/test_config.rb b/spec/support/bootstrap/test_config.rb index 2dec5678e42..ea43300b3b8 100644 --- a/spec/support/bootstrap/test_config.rb +++ b/spec/support/bootstrap/test_config.rb @@ -1,7 +1,6 @@ require 'support/bootstrap/db_config' require 'support/paths' require 'cloud_controller/config' -require 'fog/core/mock' require 'cloud_controller/dependency_locator' module TestConfig @@ -42,30 +41,24 @@ def defaults config_file = File.join(Paths::CONFIG, 'cloud_controller.yml') config_hash = VCAP::CloudController::Config.load_from_file(config_file, context:).config_hash - fog_connection = { - blobstore_timeout: 5, - provider: 'Local', - local_root: Dir.mktmpdir - } - config_hash.update( nginx: { use_nginx: true }, resource_pool: { resource_directory_key: 'spec-cc-resources', - fog_connection: fog_connection + blobstore_type: 'local-temp-storage' }, packages: { app_package_directory_key: 'cc-packages', - fog_connection: fog_connection, + blobstore_type: 'local-temp-storage', max_valid_packages_stored: 42 }, buildpacks: { buildpack_directory_key: 'cc-buildpacks', - fog_connection: fog_connection + blobstore_type: 'local-temp-storage' }, droplets: { droplet_directory_key: 'cc-droplets', - fog_connection: fog_connection, + blobstore_type: 'local-temp-storage', max_staged_droplets_stored: 42 }, db: DbConfig.new.config @@ -77,11 +70,6 @@ def defaults end def configure_components(config) - Fog.mock! unless - context == :route_syncer || - context == :deployment_updater || - is_using_local_blobstore?(config) - # reset dependency locator dependency_locator = CloudController::DependencyLocator.instance dependency_locator.reset(config) @@ -90,12 +78,5 @@ def configure_components(config) stacks_file = File.join(Paths::FIXTURES, 'config/stacks.yml') VCAP::CloudController::Stack.configure(stacks_file) end - - def is_using_local_blobstore?(config) - res_pool_blobstore_type = config.get(:resource_pool, :blobstore_type) - packages_blobstore_type = config.get(:packages, :blobstore_type) - - res_pool_blobstore_type&.start_with?('local') || packages_blobstore_type&.start_with?('local') - end end end diff --git a/spec/unit/controllers/runtime/app_bits_download_controller_spec.rb b/spec/unit/controllers/runtime/app_bits_download_controller_spec.rb index ccc43599e83..f05132dae3c 100644 --- a/spec/unit/controllers/runtime/app_bits_download_controller_spec.rb +++ b/spec/unit/controllers/runtime/app_bits_download_controller_spec.rb @@ -20,7 +20,7 @@ module VCAP::CloudController end context 'when the package is valid' do - let(:blob) { instance_double(CloudController::Blobstore::FogBlob) } + let(:blob) { instance_double(CloudController::Blobstore::Blob) } before do allow(blob).to receive(:public_download_url).and_return('http://example.com/somewhere/else') diff --git a/spec/unit/controllers/runtime/apps_controller_spec.rb b/spec/unit/controllers/runtime/apps_controller_spec.rb index 96be7475581..eee55c91bf2 100644 --- a/spec/unit/controllers/runtime/apps_controller_spec.rb +++ b/spec/unit/controllers/runtime/apps_controller_spec.rb @@ -1806,7 +1806,7 @@ def delete_app describe 'downloading the droplet' do let(:process) { ProcessModelFactory.make } - let(:blob) { instance_double(CloudController::Blobstore::FogBlob) } + let(:blob) { instance_double(CloudController::Blobstore::Blob) } let(:developer) { make_developer_for_space(process.space) } before do diff --git a/spec/unit/controllers/runtime/helpers/blob_dispatcher_spec.rb b/spec/unit/controllers/runtime/helpers/blob_dispatcher_spec.rb index d082389836f..3160d09b814 100644 --- a/spec/unit/controllers/runtime/helpers/blob_dispatcher_spec.rb +++ b/spec/unit/controllers/runtime/helpers/blob_dispatcher_spec.rb @@ -16,7 +16,7 @@ module VCAP::CloudController end describe '#send_or_redirect' do - let(:blob) { instance_double(CloudController::Blobstore::FogBlob) } + let(:blob) { instance_double(CloudController::Blobstore::Blob) } let(:package_guid) { 'package-guid' } before do diff --git a/spec/unit/controllers/runtime/stagings_controller_spec.rb b/spec/unit/controllers/runtime/stagings_controller_spec.rb index 6581e29b96b..30396fb62e3 100644 --- a/spec/unit/controllers/runtime/stagings_controller_spec.rb +++ b/spec/unit/controllers/runtime/stagings_controller_spec.rb @@ -265,11 +265,7 @@ module VCAP::CloudController before { authorize(staging_user, staging_password) } def create_test_blob - tmpdir = Dir.mktmpdir - file = File.new(File.join(tmpdir, 'afile.txt'), 'w') - file.print('test blob contents') - file.close - CloudController::Blobstore::FogBlob.new(file, nil) + instance_double(CloudController::Blobstore::Blob, internal_download_url: '/cc-packages/test/blob') end context 'when using with nginx' do diff --git a/spec/unit/controllers/v3/packages_controller_spec.rb b/spec/unit/controllers/v3/packages_controller_spec.rb index fa656c80b84..5cb4cc91b97 100644 --- a/spec/unit/controllers/v3/packages_controller_spec.rb +++ b/spec/unit/controllers/v3/packages_controller_spec.rb @@ -245,7 +245,7 @@ let(:user) { set_current_user(create(:user), email: 'utako') } before do - blob = instance_double(CloudController::Blobstore::FogBlob, public_download_url: 'http://package.example.com') + blob = instance_double(CloudController::Blobstore::Blob, public_download_url: 'http://package.example.com') allow_any_instance_of(CloudController::Blobstore::Client).to receive(:blob).and_return(blob) allow_any_instance_of(CloudController::Blobstore::Client).to receive(:local?).and_return(false) allow_user_read_access_for(user, spaces: [space]) diff --git a/spec/unit/jobs/runtime/orphaned_blobs_cleanup_spec.rb b/spec/unit/jobs/runtime/orphaned_blobs_cleanup_spec.rb index 71e1060dbc7..e1f94e181ae 100644 --- a/spec/unit/jobs/runtime/orphaned_blobs_cleanup_spec.rb +++ b/spec/unit/jobs/runtime/orphaned_blobs_cleanup_spec.rb @@ -50,10 +50,10 @@ module Jobs::Runtime let(:package_blobstore) { instance_double(CloudController::Blobstore::DavClient, files_for: package_files, root_dir: package_root_dir) } let(:package_files) { [] } let(:package_root_dir) { nil } - let(:buildpack_blobstore) { instance_double(CloudController::Blobstore::FogClient, files_for: buildpack_files, root_dir: buildpack_root_dir) } + let(:buildpack_blobstore) { instance_double(CloudController::Blobstore::Client, files_for: buildpack_files, root_dir: buildpack_root_dir) } let(:buildpack_files) { [] } let(:buildpack_root_dir) { nil } - let(:legacy_resources_blobstore) { instance_double(CloudController::Blobstore::FogClient, files_for: legacy_resource_files, root_dir: legacy_resource_root_dir) } + let(:legacy_resources_blobstore) { instance_double(CloudController::Blobstore::Client, files_for: legacy_resource_files, root_dir: legacy_resource_root_dir) } let(:legacy_resource_files) { [] } let(:legacy_resource_root_dir) { nil } diff --git a/spec/unit/lib/cloud_controller/blob_sender/nginx_blob_sender_spec.rb b/spec/unit/lib/cloud_controller/blob_sender/nginx_blob_sender_spec.rb index 0153fb10a48..6d485e01edc 100644 --- a/spec/unit/lib/cloud_controller/blob_sender/nginx_blob_sender_spec.rb +++ b/spec/unit/lib/cloud_controller/blob_sender/nginx_blob_sender_spec.rb @@ -7,7 +7,7 @@ module BlobSender NginxLocalBlobSender.new end - let(:blob) { instance_double(Blobstore::FogBlob, internal_download_url: 'http://url/to/blob') } + let(:blob) { instance_double(Blobstore::Blob, internal_download_url: 'http://url/to/blob') } describe '#send_blob' do context 'when the controller is a v2 controller' do diff --git a/spec/unit/lib/cloud_controller/blobstore/client_provider_spec.rb b/spec/unit/lib/cloud_controller/blobstore/client_provider_spec.rb index c183292c644..5dfdf0b7c2e 100644 --- a/spec/unit/lib/cloud_controller/blobstore/client_provider_spec.rb +++ b/spec/unit/lib/cloud_controller/blobstore/client_provider_spec.rb @@ -5,61 +5,6 @@ module Blobstore RSpec.describe ClientProvider do let(:options) { { blobstore_type: } } - context 'when no type is requested' do - let(:blobstore_type) { nil } - - before do - options.merge!(fog_connection: {}) - end - - it 'provides a fog client' do - allow(FogClient).to receive(:new).and_call_original - ClientProvider.provide(options: options, directory_key: 'key') - expect(FogClient).to have_received(:new) - end - end - - context 'when fog is requested' do - let(:blobstore_type) { 'fog' } - - before do - options.merge!(fog_connection: {}) - end - - it 'provides a fog client' do - allow(FogClient).to receive(:new).and_call_original - ClientProvider.provide(options: options, directory_key: 'key') - expect(FogClient).to have_received(:new) - end - - context 'when a cdn is requested in the options' do - before do - options.merge!(cdn: { uri: 'http://cdn.com' }) - end - - it 'sets up a cdn for the fog client' do - allow(FogClient).to receive(:new).and_call_original - ClientProvider.provide(options: options, directory_key: 'key') - expect(FogClient).to have_received(:new).with(connection_config: anything, - directory_key: anything, - cdn: an_instance_of(Cdn), - root_dir: anything, - min_size: anything, - max_size: anything) - end - end - - context 'when fog_connection is not provided' do - before do - options.delete(:fog_connection) - end - - it 'raises an error' do - expect { ClientProvider.provide(options: options, directory_key: 'key') }.to raise_error(KeyError) - end - end - end - context 'when webdav is requested' do let(:blobstore_type) { 'webdav' } diff --git a/spec/unit/lib/cloud_controller/blobstore/fog/cdn_spec.rb b/spec/unit/lib/cloud_controller/blobstore/fog/cdn_spec.rb deleted file mode 100644 index 509deaed6eb..00000000000 --- a/spec/unit/lib/cloud_controller/blobstore/fog/cdn_spec.rb +++ /dev/null @@ -1,88 +0,0 @@ -require 'spec_helper' -require 'cloud_controller/blobstore/fog/cdn' -require 'cloudfront-signer' - -module CloudController - module Blobstore - RSpec.describe Cdn do - let(:cdn_host) { 'https://some_distribution.cloudfront.net' } - let(:cdn) { Cdn.make(cdn_host) } - - describe '.make' do - it 'returns nil for an empty host' do - expect(Cdn.make(nil)).to be_nil - expect(Cdn.make('')).to be_nil - end - - it 'returns a real Cdn for a non-empty host' do - expect(Cdn.make('example.com')).to be_a(Cdn) - end - end - - describe '.new' do - it 'is private' do - expect do - Cdn.new('foo') - end.to raise_error(/private method/) - end - end - - describe '#get' do - let(:path_location) { 'ab/cd/abcdefghi' } - - context 'on http errors' do - let(:fake_client) { instance_double(HTTPClient, ssl_config: double(set_default_paths: true)) } - - before do - allow(HTTPClient).to receive(:new).and_return(fake_client) - end - - it 'tries 3 times' do - allow(fake_client).to receive(:get).and_raise('nope') - expect do - cdn.get(path_location) - end.to raise_error('nope') - expect(fake_client).to have_received(:get).exactly(3).times - end - end - - context 'when CloudFront Signer is not configured' do - before do - allow(Aws::CF::Signer).to receive(:is_configured?).and_return(false) - @stub = stub_request(:get, "#{cdn_host}/#{path_location}").to_return(body: 'barbaz') - end - - it 'yields' do - expect do |yielded| - cdn.get(path_location, &yielded) - end.to yield_control - end - - it 'downloads the file' do - cdn.get(path_location) do |chunk| - expect(chunk).to eq('barbaz') - end - end - - it 'requests the correct url' do - cdn.get(path_location) {} - expect(@stub).to have_been_requested - end - end - - context 'when CloudFront Signer is configured' do - before { allow(Aws::CF::Signer).to receive(:is_configured?).and_return(true) } - - it 'returns a signed URI using the CDN' do - expect(Aws::CF::Signer).to receive(:sign_url).with("#{cdn_host}/#{path_location}").and_return('http://signed_url') - stub = stub_request(:get, 'signed_url').to_return(body: 'foobar') - - cdn.get(path_location) {} - - expect(stub).to have_been_requested - end - end - end - end - end -end diff --git a/spec/unit/lib/cloud_controller/blobstore/fog/fog_blob_spec.rb b/spec/unit/lib/cloud_controller/blobstore/fog/fog_blob_spec.rb deleted file mode 100644 index 34e75422d40..00000000000 --- a/spec/unit/lib/cloud_controller/blobstore/fog/fog_blob_spec.rb +++ /dev/null @@ -1,122 +0,0 @@ -require 'spec_helper' -require_relative '../blob_shared' - -module CloudController - module Blobstore - RSpec.describe FogBlob do - subject(:blob) { FogBlob.new(file, cdn) } - let(:attrs) { { 'a' => 'b', 'c' => 'd' } } - let(:file) { double('file', key: 'abcdef', attributes: attrs, destroy: nil) } - let(:cdn) { double(:cdn, download_uri: 'http://localhost') } - - it_behaves_like 'a blob' - - describe '#internal_download_url' do - context 'it is backed by a CDN' do - let(:url_from_cdn) { 'http://some_distribution.cloudfront.net/ab/cd/abcdef' } - - before do - allow(cdn).to receive(:download_uri).and_return(url_from_cdn) - end - - it 'returns a url to the cdn' do - expect(blob.internal_download_url).to eql(url_from_cdn) - end - end - - context 'when is not backed by a CDN' do - let(:cdn) { nil } - - context 'a file responds to url' do - before do - allow(file).to receive(:url).and_return('http://example.com') - end - - it 'returns a url from file' do - expect(blob.internal_download_url).to eql('http://example.com') - end - - it 'is valid for an hour' do - Timecop.freeze do - now = Time.now.utc - expect(file).to receive(:url).with(now + 3600) - blob.internal_download_url - end - end - end - - context 'a file does not respond to url' do - before do - allow(file).to receive_messages(url: nil, public_url: 'http://example.com/public') - it 'returns a public url from file' do - expect(blob.internal_download_url).to eql('http://example.com/public') - end - end - end - end - end - - describe '#public_download_url' do - context 'it is backed by a CDN' do - let(:url_from_cdn) { 'http://some_distribution.cloudfront.net/ab/cd/abcdef' } - - before do - allow(cdn).to receive(:download_uri).and_return(url_from_cdn) - end - - it 'returns a url to the cdn' do - expect(blob.public_download_url).to eql(url_from_cdn) - end - end - - context 'when is not backed by a CDN' do - let(:cdn) { nil } - - context 'a file responds to url' do - before do - allow(file).to receive(:url).and_return('http://example.com') - end - - it 'returns a url from file' do - expect(blob.public_download_url).to eql('http://example.com') - end - - it 'is valid for an hour' do - Timecop.freeze do - now = Time.now.utc - expect(file).to receive(:url).with(now + 3600) - blob.public_download_url - end - end - end - - context 'a file does not respond to url' do - before do - allow(file).to receive_messages(url: nil, public_url: 'http://example.com/public') - it 'returns a public url from file' do - expect(blob.public_download_url).to eql('http://example.com/public') - end - end - end - end - end - - describe 'local_path' do - it 'comes path of the file' do - expect(file).to receive(:path).and_return('path') - expect(blob.local_path).to eql('path') - end - end - - describe 'attributes' do - it "returns the blob's attributes" do - expect(blob.attributes).to eq(attrs) - end - - it 'returns attributes for a set of keys' do - expect(blob.attributes('c')).to eq({ 'c' => 'd' }) - end - end - end - end -end diff --git a/spec/unit/lib/cloud_controller/blobstore/fog/fog_client_spec.rb b/spec/unit/lib/cloud_controller/blobstore/fog/fog_client_spec.rb deleted file mode 100644 index 2f155893213..00000000000 --- a/spec/unit/lib/cloud_controller/blobstore/fog/fog_client_spec.rb +++ /dev/null @@ -1,769 +0,0 @@ -require 'spec_helper' -require 'webrick' -require_relative '../client_shared' - -module CloudController - module Blobstore - # Minimal in-memory fog storage stub. Replaces Fog::Storage.new so that - # FogClient specs can run without any fog provider gem being bundled. - class FakeStorage - FakeFile = Struct.new(:key, :body, :content_type, :public, :content_length, :bucket, keyword_init: true) do - def public_url = public ? "http://fake/#{key}" : nil - def url(_expires) = "http://fake/#{key}" - - def copy(dest_bucket, dest_key, _opts={}) - storage.store_file(dest_bucket, dest_key, body, content_type) - end - - def destroy = storage.delete_file(bucket, key) - - attr_accessor :storage - end - - def initialize - @store = {} - @directories = DirectoryProxy.new(self) - end - - attr_reader :directories - - def store_file(bucket, key, body, content_type) - @store[bucket] ||= {} - body_content = body.respond_to?(:read) ? body.read : body.to_s - file = FakeFile.new(key: key, body: body_content, content_type: content_type, public: false, - content_length: body_content.bytesize, bucket: bucket) - file.storage = self - @store[bucket][key] = file - file - end - - def delete_file(bucket, key) - @store[bucket]&.delete(key) - end - - def list_files(bucket, prefix: nil) - files = @store[bucket]&.values || [] - files = files.select { |f| f.key.start_with?(prefix) } if prefix - files - end - - def get_file(bucket, key) - @store[bucket]&.[](key) - end - - def bucket_exists?(key) - @store.key?(key) - end - - def create_bucket(key) - @store[key] ||= {} - end - - class DirectoryProxy - def initialize(storage) = @storage = storage - - def get(key, _options=nil, prefix: nil, **_kwargs) - return nil unless @storage.bucket_exists?(key) - - BucketProxy.new(@storage, key, prefix:) - end - - def create(key:, **) = BucketProxy.new(@storage, @storage.create_bucket(key) && key) - def new(key:, **) = BucketProxy.new(@storage, key) - end - - class BucketProxy - attr_reader :key - - def initialize(storage, key, prefix: nil) - @storage = storage - @key = key - @prefix = prefix - @storage.create_bucket(key) - end - - def files = FilesProxy.new(@storage, @key, prefix: @prefix) - end - - class FilesProxy - include Enumerable - - def initialize(storage, bucket, prefix: nil) - @storage = storage - @bucket = bucket - @prefix = prefix - end - - def length = all.length # rubocop:disable Rails/Delegate - - def head(key) = @storage.get_file(@bucket, key) - - def get(key, &block) - file = @storage.get_file(@bucket, key) - return nil unless file - - if block - yield file.body - else - file - end - file - end - - def create(options_or_key=nil, key: nil, body: nil, content_type: 'application/zip', **) - if options_or_key.is_a?(Hash) - key = options_or_key[:key] - body = options_or_key[:body] - content_type = options_or_key[:content_type] || 'application/zip' - end - @storage.store_file(@bucket, key, body, content_type) - end - - def each(&) = all.each(&) - def all = @all ||= @storage.list_files(@bucket, prefix: @prefix) - - def select(prefix: nil) - @storage.list_files(@bucket, prefix:) - end - end - end - - RSpec.describe FogClient do - let(:content) { 'Some Nonsense' } - let(:sha_of_content) { Digester.new.digest(content) } - let(:local_dir) { Dir.mktmpdir } - let(:fake_storage) { FakeStorage.new } - let(:connection_config) { { provider: 'fake' } } - let(:directory_key) { 'a-directory-key' } - - subject(:client) do - FogClient.new(connection_config:, directory_key:) - end - - before do - allow(Fog::Storage).to receive(:new).and_return(fake_storage) - end - - after do - FileUtils.rm_rf(local_dir) - end - - it 'logs a deprecation warning on initialization' do - expect_any_instance_of(Steno::Logger).to receive(:warn).with('blobstore.fog-deprecated', hash_including(:message)) - FogClient.new(connection_config:, directory_key:) - end - - describe 'conforms to blobstore client interface' do - let(:deletable_blob) { instance_double(FogBlob, file: nil) } - - before do - client.ensure_bucket_exists - client.cp_to_blobstore(tmpfile.path, key) - end - - it_behaves_like 'a blobstore client' - end - - def upload_tmpfile(client, key='abcdef') - Tempfile.open('') do |tmpfile| - tmpfile.write(content) - tmpfile.close - client.cp_to_blobstore(tmpfile.path, key) - end - end - - context 'for a remote blobstore backed by a CDN' do - let(:cdn) { double(:cdn) } - let(:url_from_cdn) { 'http://some_distribution.cloudfront.net/ab/cd/abcdef' } - let(:key) { 'abcdef' } - - subject(:client) do - FogClient.new(connection_config:, directory_key:, cdn:) - end - - before do - client.ensure_bucket_exists - upload_tmpfile(client, key) - allow(cdn).to receive(:download_uri).and_return(url_from_cdn) - end - - it 'downloads through the CDN' do - expect(cdn).to receive(:get). - with('ab/cd/abcdef'). - and_yield('foobar').and_yield(' barbaz') - - destination = File.join(local_dir, 'some_directory_to_place_file', 'downloaded_file') - - expect { client.download_from_blobstore(key, destination) }.to change { - File.exist?(destination) - }.from(false).to(true) - - expect(File.read(destination)).to eq('foobar barbaz') - end - end - - context 'common behaviors' do - let(:directory) { fake_storage.directories.get(directory_key) || fake_storage.directories.create(key: directory_key) } - let(:client) do - FogClient.new(connection_config:, directory_key:) - end - - before do - client.ensure_bucket_exists - end - - context 'with existing files' do - before do - upload_tmpfile(client, sha_of_content) - end - - describe 'a file existence' do - it 'does not exist if not present' do - different_content = 'foobar' - sha_of_different_content = Digester.new.digest(different_content) - - expect(client.exists?(sha_of_different_content)).to be false - - upload_tmpfile(client, sha_of_different_content) - - expect(client.exists?(sha_of_different_content)).to be true - expect(client.blob(sha_of_different_content)).to be - end - end - end - - describe '#cp_r_to_blobstore' do - let(:sha_of_nothing) { Digester.new.digest('') } - - it 'ensures that the sha of nothing and sha of content are different for subsequent tests' do - expect(sha_of_nothing[0..1]).not_to eq(sha_of_content[0..1]) - end - - it 'copies the top-level local files into the blobstore' do - FileUtils.touch(File.join(local_dir, 'empty_file')) - client.cp_r_to_blobstore(local_dir) - expect(client.exists?(sha_of_nothing)).to be true - end - - it 'recursively copies the local files into the blobstore' do - subdir = File.join(local_dir, 'subdir1', 'subdir2') - FileUtils.mkdir_p(subdir) - File.write(File.join(subdir, 'file_with_content'), content) - - client.cp_r_to_blobstore(local_dir) - expect(client.exists?(sha_of_content)).to be true - end - - context 'when the file already exists in the blobstore' do - before do - FileUtils.touch(File.join(local_dir, 'empty_file')) - end - - it 'does not re-upload it' do - client.cp_r_to_blobstore(local_dir) - - expect(client).not_to receive(:cp_to_blobstore) - client.cp_r_to_blobstore(local_dir) - end - end - - context 'limit the file size' do - let(:min_size) { 20 } - let(:max_size) { 50 } - - subject(:client) do - FogClient.new(connection_config:, directory_key:, min_size:, max_size:) - end - - it 'does not copy files below the minimum size limit' do - path = File.join(local_dir, 'file_with_little_content') - File.write(path, 'a') - - expect(client).not_to receive(:exists?) - expect(client).not_to receive(:cp_to_blobstore) - client.cp_r_to_blobstore(path) - end - - it 'does not copy files above the maximum size limit' do - path = File.join(local_dir, 'file_with_more_content') - File.write(path, 'an amount of content that is larger than the maximum limit') - - expect(client).not_to receive(:exists?) - expect(client).not_to receive(:cp_to_blobstore) - client.cp_r_to_blobstore(path) - end - end - - context 'limit the file mode to those with sufficient permissions' do - subject(:client) do - FogClient.new(connection_config:, directory_key:) - end - - it 'copies files with mode >= 0600' do - path = File.join(local_dir, 'file_with_sufficient_permissions') - FileUtils.touch(path) - File.chmod(0o600, path) - - expect(client).to receive(:exists?) - expect(client).to receive(:cp_to_blobstore) - client.cp_r_to_blobstore(path) - end - - it 'does not copy files below the minimum file mode' do - path = File.join(local_dir, 'file_with_insufficient_permissions') - FileUtils.touch(path) - File.chmod(0o444, path) - - expect(client).not_to receive(:exists?) - expect(client).not_to receive(:cp_to_blobstore) - client.cp_r_to_blobstore(path) - end - end - end - - describe '#download_from_blobstore' do - let(:destination) { File.join(local_dir, 'some_directory_to_place_file', 'downloaded_file') } - - context 'when directly from the underlying storage' do - before do - upload_tmpfile(client, sha_of_content) - end - - it 'can download the file' do - expect(client.exists?(sha_of_content)).to be true - - expect { client.download_from_blobstore(sha_of_content, destination) }.to change { - File.exist?(destination) - }.from(false).to(true) - - expect(File.read(destination)).to eq(content) - end - end - - describe 'file permissions' do - before do - upload_tmpfile(client, sha_of_content) - @original_umask = File.umask - File.umask(0o022) - end - - after do - File.umask(@original_umask) - end - - context 'when not specifying a mode' do - it 'does not change permissions on the file' do - destination = File.join(local_dir, 'some_directory_to_place_file', 'downloaded_file') - client.download_from_blobstore(sha_of_content, destination) - - expect(sprintf('%o', mode: File.stat(destination).mode)).to eq('100644') - end - end - - context 'when specifying a mode' do - it 'does change permissions on the file' do - destination = File.join(local_dir, 'some_directory_to_place_file', 'downloaded_file') - client.download_from_blobstore(sha_of_content, destination, mode: 0o753) - - expect(sprintf('%o', mode: File.stat(destination).mode)).to eq('100753') - end - end - end - end - - describe '#cp_to_blobstore' do - it 'uploads the files with the specified key' do - path = File.join(local_dir, 'empty_file') - FileUtils.touch(path) - - client.cp_to_blobstore(path, 'abcdef123456') - expect(client.exists?('abcdef123456')).to be true - expect(directory.files.all.length).to eq(1) - end - - it 'defaults to private files' do - path = File.join(local_dir, 'empty_file') - FileUtils.touch(path) - key = 'abcdef12345' - - client.cp_to_blobstore(path, key) - expect(client.blob(key).file.public_url).to be_nil - end - - it 'sets content-type to mime-type of application/zip when not specified' do - path = File.join(local_dir, 'empty_file') - FileUtils.touch(path) - - client.cp_to_blobstore(path, 'abcdef123456') - - expect(directory.files.head('ab/cd/abcdef123456').content_type).to eq('application/zip') - end - - it 'sets content-type to mime-type of file when specified' do - path = File.join(local_dir, 'empty_file.png') - FileUtils.touch(path) - - client.cp_to_blobstore(path, 'abcdef123456') - - expect(directory.files.head('ab/cd/abcdef123456').content_type).to eq('image/png') - end - - context 'limit the file size' do - let(:min_size) { 20 } - let(:max_size) { 50 } - - subject(:client) do - FogClient.new(connection_config:, directory_key:, min_size:, max_size:) - end - - it 'does not copy files below the minimum size limit' do - path = File.join(local_dir, 'file_with_little_content') - File.write(path, 'a') - key = '987654321' - - client.cp_to_blobstore(path, key) - expect(client.exists?(key)).to be false - end - - it 'does not copy files above the maximum size limit' do - path = File.join(local_dir, 'file_with_more_content') - File.write(path, 'an amount of content that is larger than the maximum limit') - key = '777777777' - - client.cp_to_blobstore(path, key) - expect(client.exists?(key)).to be false - end - end - end - - describe '#cp_file_between_keys' do - let(:src_key) { 'abc123' } - let(:dest_key) { 'xyz789' } - - it 'copies the file from the source key to the destination key' do - upload_tmpfile(client, src_key) - client.cp_file_between_keys(src_key, dest_key) - - expect(client.exists?(dest_key)).to be true - expect(directory.files.all.length).to eq(2) - end - - context 'when the destination key has a package already' do - before do - upload_tmpfile(client, src_key) - Tempfile.open('') do |tmpfile| - tmpfile.write('This should be deleted and replaced with new file') - tmpfile.close - client.cp_to_blobstore(tmpfile.path, dest_key) - end - end - - it 'replaces the old package in the package blobstore' do - client.cp_file_between_keys(src_key, dest_key) - expect(directory.files.all.length).to eq(2) - - src_file_length = client.blob(dest_key).file.content_length - dest_file_length = client.blob(src_key).file.content_length - expect(dest_file_length).to eq(src_file_length) - end - end - - context 'when the source key has no file associated with it' do - it 'does not attempt to copy over to the destination key' do - expect do - client.cp_file_between_keys('bogus', dest_key) - end.to raise_error(CloudController::Blobstore::FileNotFound) - - expect(directory.files.all.length).to eq(0) - end - end - end - - describe '#delete_all' do - before do - client.ensure_bucket_exists - end - - it 'deletes all the files' do - first_path = File.join(local_dir, 'first_empty_file') - FileUtils.touch(first_path) - path = File.join(local_dir, 'empty_file') - FileUtils.touch(path) - - client.cp_to_blobstore(first_path, 'ab56') - expect(client.exists?('ab56')).to be true - client.cp_to_blobstore(path, 'abcdef123456') - expect(client.exists?('abcdef123456')).to be true - - client.delete_all - - expect(client.exists?('ab56')).to be false - expect(client.exists?('abcdef123456')).to be false - end - - it 'is ok if there are no files' do - expect(directory.files.all.length).to eq(0) - expect { client.delete_all }.not_to raise_error - end - - context 'when a root dir is provided' do - let(:root_dir) { 'root-dir' } - - let(:client_with_root) do - FogClient.new(connection_config:, directory_key:, root_dir:) - end - - before do - client_with_root.ensure_bucket_exists - end - - it 'only deletes files at the root' do - allow(client_with_root).to receive(:delete_files).and_call_original - - file = File.join(local_dir, 'empty_file') - FileUtils.touch(file) - - client.cp_to_blobstore(file, 'abcdef1') - client_with_root.cp_to_blobstore(file, 'abcdef2') - - client_with_root.delete_all - - expect(client_with_root).to have_received(:delete_files) do |files| - expect(files.length).to eq(1) - end - end - end - end - - describe '#delete_all_in_path' do - before do - client.ensure_bucket_exists - end - - it 'deletes all the files within a specific path' do - path = File.join(local_dir, 'empty_file') - FileUtils.touch(path) - - remote_path_1 = 'aaaaguid' - remote_key_1 = "#{remote_path_1}/stack_1" - remote_key_2 = "#{remote_path_1}/stack_2" - remote_path_2 = 'bbbbguid' - remote_key_3 = "#{remote_path_2}/stack_3" - - client.cp_to_blobstore(path, remote_key_1) - client.cp_to_blobstore(path, remote_key_2) - client.cp_to_blobstore(path, remote_key_3) - expect(client.exists?(remote_key_1)).to be true - expect(client.exists?(remote_key_2)).to be true - expect(client.exists?(remote_key_3)).to be true - - client.delete_all_in_path(remote_path_1) - - expect(client.exists?(remote_key_1)).to be false - expect(client.exists?(remote_key_2)).to be false - expect(client.exists?(remote_key_3)).to be true - end - - it 'is ok if there are no files' do - expect(directory.files.all.length).to eq(0) - expect { client.delete_all_in_path('nonsense_path') }.not_to raise_error - end - - context 'when a root dir is provided' do - let(:root_dir) { 'root-dir' } - - let(:client_with_root) do - FogClient.new(connection_config:, directory_key:, root_dir:) - end - - before do - client_with_root.ensure_bucket_exists - end - - it 'only deletes files at the root' do - path = File.join(local_dir, 'empty_file') - FileUtils.touch(path) - - remote_path_1 = 'aaaaguid' - remote_key_1 = "#{remote_path_1}/stack_1" - remote_key_2 = "#{remote_path_1}/stack_2" - remote_path_2 = 'bbbbguid' - remote_key_3 = "#{remote_path_2}/stack_3" - - client_with_root.cp_to_blobstore(path, remote_key_1) - client_with_root.cp_to_blobstore(path, remote_key_2) - client_with_root.cp_to_blobstore(path, remote_key_3) - expect(client_with_root.exists?(remote_key_1)).to be true - expect(client_with_root.exists?(remote_key_2)).to be true - expect(client_with_root.exists?(remote_key_3)).to be true - - client_with_root.delete_all_in_path(remote_path_1) - - expect(client_with_root.exists?(remote_key_1)).to be false - expect(client_with_root.exists?(remote_key_2)).to be false - expect(client_with_root.exists?(remote_key_3)).to be true - end - end - end - - describe '#delete' do - it 'deletes the file' do - path = File.join(local_dir, 'empty_file') - FileUtils.touch(path) - - client.cp_to_blobstore(path, 'abcdef123456') - expect(client.exists?('abcdef123456')).to be true - client.delete('abcdef123456') - expect(client.exists?('abcdef123456')).to be false - end - - it "is ok if the file doesn't exist" do - expect(directory.files.all.length).to eq(0) - expect { client.delete('non-existent-file') }.not_to raise_error - end - end - - describe '#delete_blob' do - it "deletes the blob's file" do - path = File.join(local_dir, 'empty_file') - FileUtils.touch(path) - - client.cp_to_blobstore(path, 'abcdef123456') - expect(client.exists?('abcdef123456')).to be(true) - - blob = client.blob('abcdef123456') - - client.delete_blob(blob) - expect(client.exists?('abcdef123456')).to be(false) - end - - it "is ok if the file doesn't exist" do - blob = FogBlob.new(nil, nil) - expect { client.delete_blob(blob) }.not_to raise_error - end - end - - describe '#ensure_bucket_exists' do - it 'gets the bucket' do - expect(fake_storage.directories).to receive(:get).with(directory_key, { max_keys: 1 }).and_call_original - subject.ensure_bucket_exists - end - - context 'the bucket exists' do - it 'does not create the bucket' do - subject.ensure_bucket_exists - expect(fake_storage.directories).not_to receive(:create).with(key: directory_key, public: false) - subject.ensure_bucket_exists - end - end - - context 'the bucket does not exist' do - it 'creates the bucket' do - allow(fake_storage.directories).to receive(:get).with(directory_key, { max_keys: 1 }).and_return(nil) - expect(fake_storage.directories).to receive(:create).with(key: directory_key, public: false).and_call_original - subject.ensure_bucket_exists - end - end - end - end - - context 'with root directory specified' do - let(:root_dir) { 'my-root' } - - let(:client_with_root) do - FogClient.new(connection_config:, directory_key:, root_dir:) - end - - before do - client_with_root.ensure_bucket_exists - end - - it 'includes the directory in the partitioned key' do - upload_tmpfile(client_with_root, 'abcdef') - expect(client_with_root.exists?('abcdef')).to be true - expect(client_with_root.blob('abcdef')).to be - expect(client_with_root.blob('abcdef').public_download_url).to match(%r{my-root/ab/cd/abcdef}) - end - end - - describe 'downloading without mocking' do - def wait_for_server_to_accept_requests(uri) - code = nil - total_time = 0 - while code != '200' && total_time < 10 - begin - res = Net::HTTP.get_response(URI(uri)) - code = res.code - total_time += 0.1 - sleep 0.1 - rescue StandardError - end - end - end - - describe 'from a CDN' do - let(:port) { 9875 } - let(:uri) { "http://localhost:#{port}" } - let(:cdn) { Cdn.make(uri) } - - subject(:client) do - FogClient.new(connection_config:, directory_key:, cdn:) - end - - around do |example| - WebMock.disable_net_connect!(allow_localhost: true) - example.run - WebMock.disable_net_connect! - end - - it 'correctly downloads byte streams' do - source_directory_path = File.expand_path('../../../../../fixtures/', File.dirname(__FILE__)) - source_file_path = File.join(source_directory_path, 'pa/rt/partitioned_key') - source_hexdigest = OpenSSL::Digest::SHA256.file(source_file_path).hexdigest - - pid = spawn("ruby -rwebrick -e'WEBrick::HTTPServer.new(:Port => #{port}, :DocumentRoot => \"#{source_directory_path}\").start'", out: 'test.out', err: 'test.err') - - begin - Process.detach(pid) - - wait_for_server_to_accept_requests(uri) - - destination_file_path = File.join(local_dir, 'hard_file.xyz') - - client.download_from_blobstore('partitioned_key', destination_file_path) - - destination_hexdigest = OpenSSL::Digest::SHA256.file(destination_file_path).hexdigest - - expect(destination_hexdigest).to eq(source_hexdigest) - ensure - Process.kill(9, pid) - end - end - end - - describe 'from a blobstore' do - it 'correctly downloads byte streams' do - content = 'some binary content for checksum verification' - source_file = Tempfile.new('source') - source_file.write(content) - source_file.close - - source_hexdigest = OpenSSL::Digest::SHA256.file(source_file.path).hexdigest - - client.ensure_bucket_exists - client.cp_to_blobstore(source_file.path, 'partitioned_key') - - destination_file_path = File.join(local_dir, 'hard_file.xyz') - client.download_from_blobstore('partitioned_key', destination_file_path) - - destination_hexdigest = OpenSSL::Digest::SHA256.file(destination_file_path).hexdigest - - expect(destination_hexdigest).to eq(source_hexdigest) - ensure - source_file&.unlink - end - end - end - end - end -end diff --git a/spec/unit/lib/cloud_controller/blobstore/url_generator/internal_url_generator_spec.rb b/spec/unit/lib/cloud_controller/blobstore/url_generator/internal_url_generator_spec.rb index a5b384a8ee2..fe2c5522abc 100644 --- a/spec/unit/lib/cloud_controller/blobstore/url_generator/internal_url_generator_spec.rb +++ b/spec/unit/lib/cloud_controller/blobstore/url_generator/internal_url_generator_spec.rb @@ -28,7 +28,7 @@ module Blobstore let(:droplet_blobstore) { instance_double(Blobstore::Client, blob:) } let(:internal_url) { 'http://s3.internal.example.com/signed' } - let(:blob) { instance_double(Blobstore::FogBlob, internal_download_url: internal_url) } + let(:blob) { instance_double(Blobstore::Blob, internal_download_url: internal_url) } subject(:url_generator) do InternalUrlGenerator.new(connection_options, diff --git a/spec/unit/lib/cloud_controller/config_spec.rb b/spec/unit/lib/cloud_controller/config_spec.rb index 7cfa2885b9d..c47847e15e4 100644 --- a/spec/unit/lib/cloud_controller/config_spec.rb +++ b/spec/unit/lib/cloud_controller/config_spec.rb @@ -5,21 +5,17 @@ module VCAP::CloudController let(:test_config_hash) do { packages: { - fog_connection: {}, app_package_directory_key: 'app_key' }, droplets: { - fog_connection: {}, droplet_directory_key: 'droplet_key' }, buildpacks: { - fog_connection: {}, buildpack_directory_key: 'bp_key' }, resource_pool: { minimum_size: 9001, maximum_size: 0, - fog_connection: {}, resource_directory_key: 'resource_key' }, external_domain: 'host', @@ -381,21 +377,17 @@ module VCAP::CloudController let(:test_config_hash) do { packages: { - fog_connection: {}, app_package_directory_key: 'app_key' }, droplets: { - fog_connection: {}, droplet_directory_key: 'droplet_key' }, buildpacks: { - fog_connection: {}, buildpack_directory_key: 'bp_key' }, resource_pool: { minimum_size: 9001, maximum_size: 0, - fog_connection: {}, resource_directory_key: 'resource_key' }, external_host: 'host', @@ -540,7 +532,6 @@ module VCAP::CloudController it 'returns a hash for nested properties' do expect(config_instance.get(:packages)).to eq({ - fog_connection: {}, app_package_directory_key: 'app_key' }) end diff --git a/spec/unit/lib/cloud_controller/dependency_locator_spec.rb b/spec/unit/lib/cloud_controller/dependency_locator_spec.rb index 5c3cc3fe68d..77220f48654 100644 --- a/spec/unit/lib/cloud_controller/dependency_locator_spec.rb +++ b/spec/unit/lib/cloud_controller/dependency_locator_spec.rb @@ -13,7 +13,6 @@ let(:config) do VCAP::CloudController::Config.new({ droplets: { - fog_connection: 'fog_connection', droplet_directory_key: 'key' } }) @@ -30,7 +29,6 @@ let(:config) do VCAP::CloudController::Config.new({ droplets: { - fog_connection: 'fog_connection', droplet_directory_key: 'key' } }) @@ -51,7 +49,6 @@ let(:config) do VCAP::CloudController::Config.new({ packages: { - fog_connection: 'fog_connection', app_package_directory_key: 'key' } }) @@ -68,7 +65,6 @@ let(:config) do VCAP::CloudController::Config.new({ resource_pool: { - fog_connection: 'fog_connection', resource_directory_key: 'key' } }) @@ -88,7 +84,6 @@ let(:config) do VCAP::CloudController::Config.new({ resource_pool: { - fog_connection: 'fog_connection', resource_directory_key: 'key' } }) From fbf160f6b66f60cb05cd27e0bfb5e6d1fcd46142 Mon Sep 17 00:00:00 2001 From: Katharina Przybill <30441792+kathap@users.noreply.github.com> Date: Wed, 15 Jul 2026 12:58:49 +0200 Subject: [PATCH 12/24] fix tests --- spec/request/packages_spec.rb | 6 ++---- spec/support/bootstrap/test_config.rb | 14 ++++++++++---- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/spec/request/packages_spec.rb b/spec/request/packages_spec.rb index f247c3db46a..978804bd08f 100644 --- a/spec/request/packages_spec.rb +++ b/spec/request/packages_spec.rb @@ -1017,7 +1017,6 @@ create(:app_model, guid: 'woof-guid', space: space, name: 'meow') end let(:space) { create(:space) } - let(:bits_download_url) { CloudController::DependencyLocator.instance.blobstore_url_generator.package_download_url(package_model) } let(:guid) { package_model.guid } let(:temp_file) do file = File.join(Dir.mktmpdir, 'application.zip') @@ -1043,8 +1042,7 @@ Timecop.freeze do get "/v3/packages/#{guid}/download", {}, user_header - expect(last_response.status).to eq(302) - expect(last_response.headers['Location']).to eq(bits_download_url) + expect(last_response.status).to eq(200) expected_metadata = { package_guid: package_model.guid }.to_json @@ -1076,7 +1074,7 @@ let(:org) { space.organization } let(:user) { create(:user) } let(:expected_codes_and_responses) do - h = Hash.new({ code: 302 }.freeze) + h = Hash.new({ code: 200 }.freeze) h['global_auditor'] = { code: 403 } h['org_auditor'] = { code: 404 } h['org_billing_manager'] = { code: 404 } diff --git a/spec/support/bootstrap/test_config.rb b/spec/support/bootstrap/test_config.rb index ea43300b3b8..4925a40b0f1 100644 --- a/spec/support/bootstrap/test_config.rb +++ b/spec/support/bootstrap/test_config.rb @@ -4,6 +4,8 @@ require 'cloud_controller/dependency_locator' module TestConfig + BLOBSTORE_PATH = Dir.mktmpdir('cc-test-blobstore') + class << self def context @context || :api @@ -45,20 +47,24 @@ def defaults nginx: { use_nginx: true }, resource_pool: { resource_directory_key: 'spec-cc-resources', - blobstore_type: 'local-temp-storage' + blobstore_type: 'local', + local_blobstore_path: BLOBSTORE_PATH }, packages: { app_package_directory_key: 'cc-packages', - blobstore_type: 'local-temp-storage', + blobstore_type: 'local', + local_blobstore_path: BLOBSTORE_PATH, max_valid_packages_stored: 42 }, buildpacks: { buildpack_directory_key: 'cc-buildpacks', - blobstore_type: 'local-temp-storage' + blobstore_type: 'local', + local_blobstore_path: BLOBSTORE_PATH }, droplets: { droplet_directory_key: 'cc-droplets', - blobstore_type: 'local-temp-storage', + blobstore_type: 'local', + local_blobstore_path: BLOBSTORE_PATH, max_staged_droplets_stored: 42 }, db: DbConfig.new.config From 890f43e2c477f4f796b61f802876638f0646f632 Mon Sep 17 00:00:00 2001 From: Katharina Przybill <30441792+kathap@users.noreply.github.com> Date: Wed, 15 Jul 2026 13:37:17 +0200 Subject: [PATCH 13/24] Revert test_config to local-temp-storage and restore 302 download expectations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit OutsideOfExampleError was already fixed by 92a3cca (instance-specific stub). The BLOBSTORE_PATH constant and local blobstore_type were unnecessary — revert to local-temp-storage. Restore packages_spec download expectations to 302 + Location header, matching actual blobstore URL generator behavior. --- spec/request/packages_spec.rb | 6 ++++-- spec/support/bootstrap/test_config.rb | 14 ++++---------- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/spec/request/packages_spec.rb b/spec/request/packages_spec.rb index 978804bd08f..f247c3db46a 100644 --- a/spec/request/packages_spec.rb +++ b/spec/request/packages_spec.rb @@ -1017,6 +1017,7 @@ create(:app_model, guid: 'woof-guid', space: space, name: 'meow') end let(:space) { create(:space) } + let(:bits_download_url) { CloudController::DependencyLocator.instance.blobstore_url_generator.package_download_url(package_model) } let(:guid) { package_model.guid } let(:temp_file) do file = File.join(Dir.mktmpdir, 'application.zip') @@ -1042,7 +1043,8 @@ Timecop.freeze do get "/v3/packages/#{guid}/download", {}, user_header - expect(last_response.status).to eq(200) + expect(last_response.status).to eq(302) + expect(last_response.headers['Location']).to eq(bits_download_url) expected_metadata = { package_guid: package_model.guid }.to_json @@ -1074,7 +1076,7 @@ let(:org) { space.organization } let(:user) { create(:user) } let(:expected_codes_and_responses) do - h = Hash.new({ code: 200 }.freeze) + h = Hash.new({ code: 302 }.freeze) h['global_auditor'] = { code: 403 } h['org_auditor'] = { code: 404 } h['org_billing_manager'] = { code: 404 } diff --git a/spec/support/bootstrap/test_config.rb b/spec/support/bootstrap/test_config.rb index 4925a40b0f1..ea43300b3b8 100644 --- a/spec/support/bootstrap/test_config.rb +++ b/spec/support/bootstrap/test_config.rb @@ -4,8 +4,6 @@ require 'cloud_controller/dependency_locator' module TestConfig - BLOBSTORE_PATH = Dir.mktmpdir('cc-test-blobstore') - class << self def context @context || :api @@ -47,24 +45,20 @@ def defaults nginx: { use_nginx: true }, resource_pool: { resource_directory_key: 'spec-cc-resources', - blobstore_type: 'local', - local_blobstore_path: BLOBSTORE_PATH + blobstore_type: 'local-temp-storage' }, packages: { app_package_directory_key: 'cc-packages', - blobstore_type: 'local', - local_blobstore_path: BLOBSTORE_PATH, + blobstore_type: 'local-temp-storage', max_valid_packages_stored: 42 }, buildpacks: { buildpack_directory_key: 'cc-buildpacks', - blobstore_type: 'local', - local_blobstore_path: BLOBSTORE_PATH + blobstore_type: 'local-temp-storage' }, droplets: { droplet_directory_key: 'cc-droplets', - blobstore_type: 'local', - local_blobstore_path: BLOBSTORE_PATH, + blobstore_type: 'local-temp-storage', max_staged_droplets_stored: 42 }, db: DbConfig.new.config From 1b70f87bad91a1941899a508c15f7a5a8fa300be Mon Sep 17 00:00:00 2001 From: Katharina Przybill <30441792+kathap@users.noreply.github.com> Date: Thu, 16 Jul 2026 12:50:00 +0200 Subject: [PATCH 14/24] Memoize blobstore clients in DependencyLocator; fix BlobNotFound with local-temp-storage --- lib/cloud_controller/dependency_locator.rb | 94 ++++++++++++---------- spec/request/packages_spec.rb | 6 +- 2 files changed, 52 insertions(+), 48 deletions(-) diff --git a/lib/cloud_controller/dependency_locator.rb b/lib/cloud_controller/dependency_locator.rb index 0760b8b4a34..5d71b1390ee 100644 --- a/lib/cloud_controller/dependency_locator.rb +++ b/lib/cloud_controller/dependency_locator.rb @@ -142,65 +142,71 @@ def index_stopper end def droplet_blobstore - options = config.get(:droplets) - - Blobstore::ClientProvider.provide( - options: options, - directory_key: options.fetch(:droplet_directory_key), - resource_type: :droplets - ) + @dependencies[:droplet_blobstore] ||= begin + options = config.get(:droplets) + Blobstore::ClientProvider.provide( + options: options, + directory_key: options.fetch(:droplet_directory_key), + resource_type: :droplets + ) + end end def buildpack_cache_blobstore - options = config.get(:droplets) - - Blobstore::ClientProvider.provide( - options: options, - directory_key: options.fetch(:droplet_directory_key), - root_dir: BUILDPACK_CACHE_DIR, - resource_type: :buildpack_cache - ) + @dependencies[:buildpack_cache_blobstore] ||= begin + options = config.get(:droplets) + Blobstore::ClientProvider.provide( + options: options, + directory_key: options.fetch(:droplet_directory_key), + root_dir: BUILDPACK_CACHE_DIR, + resource_type: :buildpack_cache + ) + end end def package_blobstore - options = config.get(:packages) - - Blobstore::ClientProvider.provide( - options: options, - directory_key: options.fetch(:app_package_directory_key), - resource_type: :packages - ) + @dependencies[:package_blobstore] ||= begin + options = config.get(:packages) + Blobstore::ClientProvider.provide( + options: options, + directory_key: options.fetch(:app_package_directory_key), + resource_type: :packages + ) + end end def legacy_global_app_bits_cache - options = config.get(:resource_pool) - - Blobstore::ClientProvider.provide( - options: options, - directory_key: options.fetch(:resource_directory_key), - resource_type: :resource_pool - ) + @dependencies[:legacy_global_app_bits_cache] ||= begin + options = config.get(:resource_pool) + Blobstore::ClientProvider.provide( + options: options, + directory_key: options.fetch(:resource_directory_key), + resource_type: :resource_pool + ) + end end def global_app_bits_cache - options = config.get(:resource_pool) - - Blobstore::ClientProvider.provide( - options: options, - directory_key: options.fetch(:resource_directory_key), - root_dir: RESOURCE_POOL_DIR, - resource_type: :resource_pool - ) + @dependencies[:global_app_bits_cache] ||= begin + options = config.get(:resource_pool) + Blobstore::ClientProvider.provide( + options: options, + directory_key: options.fetch(:resource_directory_key), + root_dir: RESOURCE_POOL_DIR, + resource_type: :resource_pool + ) + end end def buildpack_blobstore - options = config.get(:buildpacks) - - Blobstore::ClientProvider.provide( - options: options, - directory_key: options.fetch(:buildpack_directory_key, 'cc-buildpacks'), - resource_type: :buildpacks - ) + @dependencies[:buildpack_blobstore] ||= begin + options = config.get(:buildpacks) + Blobstore::ClientProvider.provide( + options: options, + directory_key: options.fetch(:buildpack_directory_key, 'cc-buildpacks'), + resource_type: :buildpacks + ) + end end def blobstore_url_generator diff --git a/spec/request/packages_spec.rb b/spec/request/packages_spec.rb index f247c3db46a..978804bd08f 100644 --- a/spec/request/packages_spec.rb +++ b/spec/request/packages_spec.rb @@ -1017,7 +1017,6 @@ create(:app_model, guid: 'woof-guid', space: space, name: 'meow') end let(:space) { create(:space) } - let(:bits_download_url) { CloudController::DependencyLocator.instance.blobstore_url_generator.package_download_url(package_model) } let(:guid) { package_model.guid } let(:temp_file) do file = File.join(Dir.mktmpdir, 'application.zip') @@ -1043,8 +1042,7 @@ Timecop.freeze do get "/v3/packages/#{guid}/download", {}, user_header - expect(last_response.status).to eq(302) - expect(last_response.headers['Location']).to eq(bits_download_url) + expect(last_response.status).to eq(200) expected_metadata = { package_guid: package_model.guid }.to_json @@ -1076,7 +1074,7 @@ let(:org) { space.organization } let(:user) { create(:user) } let(:expected_codes_and_responses) do - h = Hash.new({ code: 302 }.freeze) + h = Hash.new({ code: 200 }.freeze) h['global_auditor'] = { code: 403 } h['org_auditor'] = { code: 404 } h['org_billing_manager'] = { code: 404 } From a503a3d0248e7f47000931829f7580ba60051061 Mon Sep 17 00:00:00 2001 From: Katharina Przybill <30441792+kathap@users.noreply.github.com> Date: Thu, 16 Jul 2026 13:25:45 +0200 Subject: [PATCH 15/24] Fix dependency locator --- lib/cloud_controller/dependency_locator.rb | 6 ++++-- .../dependency_locator_spec.rb | 18 +++++++++++++++++- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/lib/cloud_controller/dependency_locator.rb b/lib/cloud_controller/dependency_locator.rb index 5d71b1390ee..f29ecca2083 100644 --- a/lib/cloud_controller/dependency_locator.rb +++ b/lib/cloud_controller/dependency_locator.rb @@ -35,8 +35,6 @@ class DependencyLocator BUILDPACK_CACHE_DIR = 'buildpack_cache'.freeze RESOURCE_POOL_DIR = 'app_bits_cache'.freeze - attr_writer :config - def initialize @config = VCAP::CloudController::Config.config @dependencies = {} @@ -46,6 +44,10 @@ def config @config || raise('config not set') end + def config=(config) + reset(config) + end + def reset(config) @config = config @dependencies = {} diff --git a/spec/unit/lib/cloud_controller/dependency_locator_spec.rb b/spec/unit/lib/cloud_controller/dependency_locator_spec.rb index 77220f48654..d8a4a428ef8 100644 --- a/spec/unit/lib/cloud_controller/dependency_locator_spec.rb +++ b/spec/unit/lib/cloud_controller/dependency_locator_spec.rb @@ -7,7 +7,7 @@ let(:config) { TestConfig.config_instance } - before { locator.config = config } + before { locator.reset(config) } describe '#droplet_blobstore' do let(:config) do @@ -59,6 +59,22 @@ with(options: config.get(:packages), directory_key: 'key', resource_type: :packages) locator.package_blobstore end + + it 'memoizes the blobstore' do + client = instance_double(CloudController::Blobstore::Client) + expect(CloudController::Blobstore::ClientProvider).to receive(:provide).once.and_return(client) + expect(locator.package_blobstore).to be(client) + expect(locator.package_blobstore).to be(client) + end + + it 'rebuilds the blobstore after reset' do + first_client = instance_double(CloudController::Blobstore::Client) + second_client = instance_double(CloudController::Blobstore::Client) + allow(CloudController::Blobstore::ClientProvider).to receive(:provide).and_return(first_client, second_client) + expect(locator.package_blobstore).to be(first_client) + locator.reset(config) + expect(locator.package_blobstore).to be(second_client) + end end describe '#legacy_global_app_bits_cache' do From 108952fa817f349520759efdca3b6c3b0738c6d9 Mon Sep 17 00:00:00 2001 From: Katharina Przybill <30441792+kathap@users.noreply.github.com> Date: Thu, 16 Jul 2026 14:14:28 +0200 Subject: [PATCH 16/24] Don't call logger in at_exit cleanup hook --- lib/cloud_controller/blobstore/local/local_client.rb | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/cloud_controller/blobstore/local/local_client.rb b/lib/cloud_controller/blobstore/local/local_client.rb index fa858bd6c96..b1ce6119689 100644 --- a/lib/cloud_controller/blobstore/local/local_client.rb +++ b/lib/cloud_controller/blobstore/local/local_client.rb @@ -151,10 +151,9 @@ def register_cleanup_hook def cleanup_temp_storage return unless use_temp_storage? && @base_path && File.directory?(@base_path) - logger.info('temp-storage-cleanup', directory_key: @directory_key, path: @base_path) FileUtils.rm_rf(@base_path) - rescue StandardError => e - logger.error('temp-storage-cleanup-failed', error: e.message, path: @base_path) + rescue StandardError + # at_exit: swallow errors silently, logger may be unavailable end def logger From c2902090d22c225968ae615758d569ec6b617b8d Mon Sep 17 00:00:00 2001 From: Katharina Przybill <30441792+kathap@users.noreply.github.com> Date: Thu, 16 Jul 2026 14:42:26 +0200 Subject: [PATCH 17/24] Fix fog-aws removal artifacts --- .rubocop_todo.yml | 1 - Gemfile | 2 +- README.md | 1 - 3 files changed, 1 insertion(+), 3 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 4fd9bda6042..b976c449464 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -581,7 +581,6 @@ Style/HashSlice: - 'app/controllers/services/service_instances_controller.rb' - 'app/controllers/services/service_plans_controller.rb' - 'app/presenters/system_environment/service_binding_presenter.rb' - - 'lib/cloud_controller/blobstore/fog/fog_blob.rb' - 'lib/cloud_controller/blobstore/storage_cli/storage_cli_blob.rb' - 'lib/cloud_controller/blobstore/webdav/dav_blob.rb' - 'lib/cloud_controller/telemetry_logger.rb' diff --git a/Gemfile b/Gemfile index b56fef87afb..ced08672190 100644 --- a/Gemfile +++ b/Gemfile @@ -40,7 +40,7 @@ gem 'actionview', '~> 8.1.3' gem 'activemodel', '~> 8.1.2' gem 'railties', '~> 8.1.1' -gem 'cf-uaa-lib', '~> 4.0.10', '~> 4.0.10' +gem 'cf-uaa-lib', '~> 4.0.10' group :db do gem 'mysql2', '~> 0.5.7' diff --git a/README.md b/README.md index 5c785f69d42..ca64ba13876 100644 --- a/README.md +++ b/README.md @@ -40,7 +40,6 @@ When deployed via capi-release only: | Provider | `blobstore_type` | Backends | Notes | |----------|------------------|-----------------------------------------------|-------| | Storage CLI | `storage-cli` | S3, S3-compatible, GCS, Azure, Alibaba Cloud | | -| Fog | `fog` | AWS, GCS, Alibaba Cloud, OpenStack, Local/NFS | **Default.** Local/NFS not recommended for production | | WebDAV | `webdav` | WebDAV servers | | | Local | `local`, `local-temp-storage` | Filesystem, NFS | Development and testing only | From c8c0993f9b1cdaf05edba5d0b3690c6e06d23d21 Mon Sep 17 00:00:00 2001 From: Katharina Przybill <30441792+kathap@users.noreply.github.com> Date: Thu, 16 Jul 2026 15:39:05 +0200 Subject: [PATCH 18/24] Fix upload tests --- spec/unit/jobs/v3/buildpack_cache_upload_spec.rb | 5 +---- spec/unit/jobs/v3/droplet_upload_spec.rb | 6 +----- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/spec/unit/jobs/v3/buildpack_cache_upload_spec.rb b/spec/unit/jobs/v3/buildpack_cache_upload_spec.rb index 47ea364e3bc..b4358513fcd 100644 --- a/spec/unit/jobs/v3/buildpack_cache_upload_spec.rb +++ b/spec/unit/jobs/v3/buildpack_cache_upload_spec.rb @@ -84,11 +84,8 @@ module Jobs::V3 before { app.destroy } it 'does not try to upload the droplet' do + expect(blobstore).not_to receive(:cp_to_blobstore) job.perform - - downloaded_file = Tempfile.new('downloaded_file') - blobstore.download_from_blobstore(blobstore_key, downloaded_file.path) - expect(downloaded_file.read).to eql('') end it 'deletes the local file' do diff --git a/spec/unit/jobs/v3/droplet_upload_spec.rb b/spec/unit/jobs/v3/droplet_upload_spec.rb index b37ddd2042d..56fdfc27548 100644 --- a/spec/unit/jobs/v3/droplet_upload_spec.rb +++ b/spec/unit/jobs/v3/droplet_upload_spec.rb @@ -75,12 +75,8 @@ module Jobs::V3 end it 'does not try to upload the droplet' do - digest = Digester.new.digest_file(local_file) + expect(blobstore).not_to receive(:cp_to_blobstore) job.perform - - downloaded_file = Tempfile.new('downloaded_file') - blobstore.download_from_blobstore(File.join('bad-guid', digest), downloaded_file.path) - expect(downloaded_file.read).to eql('') end it 'deletes the local file' do From 97450cc180ecb61b9cf1ebcb94ca8217a9e935a2 Mon Sep 17 00:00:00 2001 From: Katharina Przybill <30441792+kathap@users.noreply.github.com> Date: Thu, 16 Jul 2026 15:55:18 +0200 Subject: [PATCH 19/24] fix specs --- lib/cloud_controller/blobstore/blob.rb | 8 ++++++++ lib/cloud_controller/blobstore/local/local_client.rb | 4 ++-- spec/request/droplets_spec.rb | 4 +++- .../runtime/app_bits_upload_controller_spec.rb | 11 +++++++++++ 4 files changed, 24 insertions(+), 3 deletions(-) diff --git a/lib/cloud_controller/blobstore/blob.rb b/lib/cloud_controller/blobstore/blob.rb index 9b3acf4a19c..a0fa014fd44 100644 --- a/lib/cloud_controller/blobstore/blob.rb +++ b/lib/cloud_controller/blobstore/blob.rb @@ -2,6 +2,14 @@ module CloudController module Blobstore class Blob CACHE_ATTRIBUTES = %i[etag last_modified created_at content_length].freeze + + def internal_download_url + raise NotImplementedError + end + + def public_download_url + raise NotImplementedError + end end end end diff --git a/lib/cloud_controller/blobstore/local/local_client.rb b/lib/cloud_controller/blobstore/local/local_client.rb index b1ce6119689..5a3964ca2ed 100644 --- a/lib/cloud_controller/blobstore/local/local_client.rb +++ b/lib/cloud_controller/blobstore/local/local_client.rb @@ -152,8 +152,8 @@ def cleanup_temp_storage return unless use_temp_storage? && @base_path && File.directory?(@base_path) FileUtils.rm_rf(@base_path) - rescue StandardError - # at_exit: swallow errors silently, logger may be unavailable + rescue StandardError => e + logger.error('temp-storage-cleanup-failed', error: e.message, path: @base_path) end def logger diff --git a/spec/request/droplets_spec.rb b/spec/request/droplets_spec.rb index 10521192108..1e643111039 100644 --- a/spec/request/droplets_spec.rb +++ b/spec/request/droplets_spec.rb @@ -368,7 +368,7 @@ bits_path: droplet_file } end - let(:bits_download_url) { CloudController::DependencyLocator.instance.blobstore_url_generator.droplet_download_url(droplet_model) } + let(:bits_download_url) { 'http://blobstore.example.com/droplet-download' } context 'when the droplet is uploaded' do let(:api_call) { ->(user_headers) { get "/v3/droplets/#{guid}/download", nil, user_headers } } @@ -398,6 +398,8 @@ successes, failures = Delayed::Worker.new.work_off expect(successes).to eq(1) expect(failures).to eq(0) + allow_any_instance_of(CloudController::Blobstore::LocalClient).to receive(:local?).and_return(false) + allow_any_instance_of(CloudController::Blobstore::LocalBlob).to receive(:public_download_url).and_return(bits_download_url) end it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS diff --git a/spec/unit/controllers/runtime/app_bits_upload_controller_spec.rb b/spec/unit/controllers/runtime/app_bits_upload_controller_spec.rb index 531786c8269..698f0142f29 100644 --- a/spec/unit/controllers/runtime/app_bits_upload_controller_spec.rb +++ b/spec/unit/controllers/runtime/app_bits_upload_controller_spec.rb @@ -122,6 +122,10 @@ def make_request before do # rack_test overrides 'CONTENT_TYPE' header with 'boundary' which causes errors when the request does not contain an application headers[:multipart] = false + # Seed the resource pool with the empty-file SHA so the packer can match it + empty_file = Tempfile.new('empty') + CloudController::DependencyLocator.instance.global_app_bits_cache.cp_to_blobstore(empty_file.path, 'da39a3ee5e6b4b0d3255bfef95601890afd80709') + empty_file.close end it 'succeeds to upload' do @@ -134,6 +138,13 @@ def make_request context 'with at least one resource and an application' do let(:req_body) { { resources: Oj.dump([{ 'fn' => 'lol', 'sha1' => 'da39a3ee5e6b4b0d3255bfef95601890afd80709', 'size' => 2048 }]), application: valid_zip } } + before do + # Seed the resource pool with the empty-file SHA so the packer can match it + empty_file = Tempfile.new('empty') + CloudController::DependencyLocator.instance.global_app_bits_cache.cp_to_blobstore(empty_file.path, 'da39a3ee5e6b4b0d3255bfef95601890afd80709') + empty_file.close + end + it 'succeeds to upload' do make_request expect(last_response.status).to eq(201) From c8c0bdf4dff87c463fe22f2fa2be1d77a1fcb353 Mon Sep 17 00:00:00 2001 From: Katharina Przybill <30441792+kathap@users.noreply.github.com> Date: Thu, 16 Jul 2026 16:11:23 +0200 Subject: [PATCH 20/24] Change failed job output logs --- lib/tasks/spec.rake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/tasks/spec.rake b/lib/tasks/spec.rake index a5e250a1675..ef0f209e4f3 100644 --- a/lib/tasks/spec.rake +++ b/lib/tasks/spec.rake @@ -45,7 +45,7 @@ namespace :spec do def run_specs_parallel(path, env_vars='') command = <<~CMD #{env_vars} bundle exec parallel_rspec \ - --test-options '--order rand #{rspec_profile_option} --format ParallelTests::RSpec::RuntimeLogger --out tmp/parallel_runtime_rspec_main.log' \ + --test-options '--order rand #{rspec_profile_option} --format progress --format ParallelTests::RSpec::RuntimeLogger --out tmp/parallel_runtime_rspec_main.log' \ --runtime-log tmp/parallel_runtime_rspec_main.log \ --single spec/integration/ \ --single spec/acceptance/ \ From 5f5301104421b969cee49a4ea69169b9b685d24c Mon Sep 17 00:00:00 2001 From: Katharina Przybill <30441792+kathap@users.noreply.github.com> Date: Thu, 16 Jul 2026 17:00:33 +0200 Subject: [PATCH 21/24] Fix tests --- lib/cloud_controller/blobstore/blob.rb | 4 ++ .../blobstore/local/local_client.rb | 5 ++- spec/api/documentation/app_bits_api_spec.rb | 13 +++++- spec/request/v2/apps_spec.rb | 15 ++++++- .../download_droplets_controller_spec.rb | 2 + .../runtime/stagings_controller_spec.rb | 11 ++++- .../blobstore/error_handling_client_spec.rb | 44 +++++++++---------- .../runtime/buildpack_bits_delete_spec.rb | 11 ++--- 8 files changed, 68 insertions(+), 37 deletions(-) diff --git a/lib/cloud_controller/blobstore/blob.rb b/lib/cloud_controller/blobstore/blob.rb index a0fa014fd44..f56145eb632 100644 --- a/lib/cloud_controller/blobstore/blob.rb +++ b/lib/cloud_controller/blobstore/blob.rb @@ -10,6 +10,10 @@ def internal_download_url def public_download_url raise NotImplementedError end + + def local_path + raise NotImplementedError + end end end end diff --git a/lib/cloud_controller/blobstore/local/local_client.rb b/lib/cloud_controller/blobstore/local/local_client.rb index 5a3964ca2ed..2a077370c0a 100644 --- a/lib/cloud_controller/blobstore/local/local_client.rb +++ b/lib/cloud_controller/blobstore/local/local_client.rb @@ -92,8 +92,9 @@ def delete_blob(blob) end def delete_all(_=nil) - FileUtils.rm_rf(@base_path) - FileUtils.mkdir_p(@base_path) + target = @root_dir ? File.join(@base_path, @root_dir) : @base_path + FileUtils.rm_rf(target) + FileUtils.mkdir_p(target) end def delete_all_in_path(path) diff --git a/spec/api/documentation/app_bits_api_spec.rb b/spec/api/documentation/app_bits_api_spec.rb index d6a5f479e8e..9c51b465c2e 100644 --- a/spec/api/documentation/app_bits_api_spec.rb +++ b/spec/api/documentation/app_bits_api_spec.rb @@ -45,6 +45,11 @@ directories: { tmpdir: File.dirname(valid_zip.path) }, kubernetes: {} ) + empty_file = Tempfile.new('empty') + CloudController::DependencyLocator.instance.global_app_bits_cache.cp_to_blobstore( + empty_file.path, 'da39a3ee5e6b4b0d3255bfef95601890afd80709' + ) + empty_file.close end put '/v2/apps/:guid/bits' do @@ -123,8 +128,10 @@ EOS no_doc { client.put "/v2/apps/#{process.guid}/bits", app_bits_put_params, headers } + allow_any_instance_of(CloudController::Blobstore::LocalClient).to receive(:local?).and_return(false) + allow_any_instance_of(CloudController::Blobstore::LocalBlob).to receive(:public_download_url).and_return('http://blobstore.example.com/download') client.get "/v2/apps/#{process.guid}/download", {}, headers - expect(response_headers['Location']).to include('cc-packages.s3.amazonaws.com') + expect(response_headers['Location']).to eq('http://blobstore.example.com/download') expect(status).to eq(302) end end @@ -150,9 +157,11 @@ Some blobstores may reject the request in that case. Clients may need to follow the redirect without including the OAuth token. EOS + allow_any_instance_of(CloudController::Blobstore::LocalClient).to receive(:local?).and_return(false) + allow_any_instance_of(CloudController::Blobstore::LocalBlob).to receive(:public_download_url).and_return('http://blobstore.example.com/droplet-download') client.get "/v2/apps/#{process.guid}/droplet/download", {}, headers expect(status).to eq(302) - expect(response_headers['Location']).to include('cc-droplets.s3.amazonaws.com') + expect(response_headers['Location']).to eq('http://blobstore.example.com/droplet-download') end end diff --git a/spec/request/v2/apps_spec.rb b/spec/request/v2/apps_spec.rb index 2fac4913281..7050fa666ad 100644 --- a/spec/request/v2/apps_spec.rb +++ b/spec/request/v2/apps_spec.rb @@ -1882,6 +1882,11 @@ def make_actual_lrp(instance_guid:, index:, state:, error:, since:) before do TestConfig.config[:directories][:tmpdir] = File.dirname(valid_zip.path) + empty_file = Tempfile.new('empty') + CloudController::DependencyLocator.instance.global_app_bits_cache.cp_to_blobstore( + empty_file.path, 'da39a3ee5e6b4b0d3255bfef95601890afd80709' + ) + empty_file.close upload_params = { application: valid_zip, resources: [{ fn: 'a/b/c', size: 1, sha1: 'da39a3ee5e6b4b0d3255bfef95601890afd80709' }].to_json @@ -1890,9 +1895,12 @@ def make_actual_lrp(instance_guid:, index:, state:, error:, since:) end it 'redirects to a blobstore url' do + allow_any_instance_of(CloudController::Blobstore::LocalClient).to receive(:local?).and_return(false) + allow_any_instance_of(CloudController::Blobstore::LocalBlob).to receive(:public_download_url).and_return('http://blobstore.example.com/download') + get "/v2/apps/#{process.guid}/download", nil, headers_for(user) - expect(last_response.headers['Location']).to include('cc-packages.s3.amazonaws.com') + expect(last_response.headers['Location']).to eq('http://blobstore.example.com/download') expect(last_response.status).to eq(302) end end @@ -1909,9 +1917,12 @@ def make_actual_lrp(instance_guid:, index:, state:, error:, since:) end it 'redirects to a blobstore to download the droplet' do + allow_any_instance_of(CloudController::Blobstore::LocalClient).to receive(:local?).and_return(false) + allow_any_instance_of(CloudController::Blobstore::LocalBlob).to receive(:public_download_url).and_return('http://blobstore.example.com/droplet-download') + get "/v2/apps/#{process.guid}/droplet/download", nil, headers_for(user) expect(last_response.status).to eq(302) - expect(last_response.headers['Location']).to include('cc-droplets.s3.amazonaws.com') + expect(last_response.headers['Location']).to eq('http://blobstore.example.com/droplet-download') end end diff --git a/spec/unit/controllers/internal/download_droplets_controller_spec.rb b/spec/unit/controllers/internal/download_droplets_controller_spec.rb index 7af616dedd2..792ddfd3b0b 100644 --- a/spec/unit/controllers/internal/download_droplets_controller_spec.rb +++ b/spec/unit/controllers/internal/download_droplets_controller_spec.rb @@ -135,6 +135,7 @@ def upload_droplet(target_droplet=droplet) it 'redirects to the correct droplet when revision droplet differs from desired droplet' do upload_droplet upload_droplet(new_droplet) + allow_any_instance_of(CloudController::Blobstore::LocalClient).to receive(:local?).and_return(false) allow_any_instance_of(CloudController::Blobstore::UrlGenerator).to receive(:droplet_download_url).with(droplet).and_return('http://example.com/wrong/droplet') allow_any_instance_of(CloudController::Blobstore::UrlGenerator).to receive(:droplet_download_url).with(new_droplet).and_return('http://example.com/correct/droplet') @@ -153,6 +154,7 @@ def upload_droplet(target_droplet=droplet) it 'redirects to the url provided by the blobstore_url_generator' do upload_droplet + allow_any_instance_of(CloudController::Blobstore::LocalClient).to receive(:local?).and_return(false) allow_any_instance_of(CloudController::Blobstore::UrlGenerator).to receive(:droplet_download_url).and_return('http://example.com/somewhere/else') get "/internal/v4/droplets/#{process.guid}/#{process.droplet_checksum}/download" diff --git a/spec/unit/controllers/runtime/stagings_controller_spec.rb b/spec/unit/controllers/runtime/stagings_controller_spec.rb index 30396fb62e3..a7936f3db94 100644 --- a/spec/unit/controllers/runtime/stagings_controller_spec.rb +++ b/spec/unit/controllers/runtime/stagings_controller_spec.rb @@ -265,7 +265,12 @@ module VCAP::CloudController before { authorize(staging_user, staging_password) } def create_test_blob - instance_double(CloudController::Blobstore::Blob, internal_download_url: '/cc-packages/test/blob') + tmpfile = Tempfile.new('staging-test-blob') + tmpfile.write('test blob contents') + tmpfile.flush + instance_double(CloudController::Blobstore::Blob, + internal_download_url: '/cc-packages/test/blob', + local_path: tmpfile.path) end context 'when using with nginx' do @@ -304,6 +309,8 @@ def create_test_blob end it 'fails if blobstore is not local' do + non_local_blobstore = instance_double(CloudController::Blobstore::Client, local?: false) + allow(CloudController::DependencyLocator.instance).to receive(:package_blobstore).and_return(non_local_blobstore) get '/staging/packages/some-guid' expect(last_response.status).to eq(400) end @@ -457,6 +464,8 @@ def make_request(guid=app_model.guid) before { authorize(staging_user, staging_password) } it 'fails if blobstore is not local' do + non_local_blobstore = instance_double(CloudController::Blobstore::Client, local?: false) + allow(CloudController::DependencyLocator.instance).to receive(:droplet_blobstore).and_return(non_local_blobstore) get '/staging/v3/droplets/some-guid/download' expect(last_response.status).to eq(400) end diff --git a/spec/unit/lib/cloud_controller/blobstore/error_handling_client_spec.rb b/spec/unit/lib/cloud_controller/blobstore/error_handling_client_spec.rb index 760306a8457..d9f77c0c9b7 100644 --- a/spec/unit/lib/cloud_controller/blobstore/error_handling_client_spec.rb +++ b/spec/unit/lib/cloud_controller/blobstore/error_handling_client_spec.rb @@ -22,131 +22,131 @@ module Blobstore describe '#delete_all' do before do - allow(wrapped_client).to receive(:delete_all).and_raise(Excon::Errors::Error.new('error message')) + allow(wrapped_client).to receive(:delete_all).and_raise(StandardError.new('error message')) end it 'handles errors and delegates to wrapped client' do expect do client.delete_all end.to raise_error(BlobstoreError, 'error message') - expect(logger).to have_received(:error).with('Error with blobstore: Excon::Error - error message') + expect(logger).to have_received(:error).with('Error with blobstore: StandardError - error message') end end describe '#delete_all_in_path' do before do - allow(wrapped_client).to receive(:delete_all_in_path).and_raise(Excon::Errors::Error.new('error message')) + allow(wrapped_client).to receive(:delete_all_in_path).and_raise(StandardError.new('error message')) end it 'handles errors and delegates to wrapped client' do expect do client.delete_all_in_path('sallow\\dossy\\path') end.to raise_error(BlobstoreError, 'error message') - expect(logger).to have_received(:error).with('Error with blobstore: Excon::Error - error message') + expect(logger).to have_received(:error).with('Error with blobstore: StandardError - error message') end end describe '#exists?' do before do - allow(wrapped_client).to receive(:exists?).and_raise(Excon::Errors::Error.new('error message')) + allow(wrapped_client).to receive(:exists?).and_raise(StandardError.new('error message')) end it 'handles errors and delegates to wrapped client' do expect do client.exists?('off') end.to raise_error(BlobstoreError, 'error message') - expect(logger).to have_received(:error).with('Error with blobstore: Excon::Error - error message') + expect(logger).to have_received(:error).with('Error with blobstore: StandardError - error message') end end describe '#blob' do before do - allow(wrapped_client).to receive(:blob).and_raise(Excon::Errors::Error.new('error message')) + allow(wrapped_client).to receive(:blob).and_raise(StandardError.new('error message')) end it 'handles errors and delegates to wrapped client' do expect do client.blob('a minor') end.to raise_error(BlobstoreError, 'error message') - expect(logger).to have_received(:error).with('Error with blobstore: Excon::Error - error message') + expect(logger).to have_received(:error).with('Error with blobstore: StandardError - error message') end end describe '#delete_blob' do before do - allow(wrapped_client).to receive(:delete_blob).and_raise(Excon::Errors::Error.new('error message')) + allow(wrapped_client).to receive(:delete_blob).and_raise(StandardError.new('error message')) end it 'handles errors and delegates to wrapped client' do expect do client.delete_blob('herbie') end.to raise_error(BlobstoreError, 'error message') - expect(logger).to have_received(:error).with('Error with blobstore: Excon::Error - error message') + expect(logger).to have_received(:error).with('Error with blobstore: StandardError - error message') end end describe '#cp_file_between_keys' do before do - allow(wrapped_client).to receive(:cp_file_between_keys).and_raise(Excon::Errors::Error.new('error message')) + allow(wrapped_client).to receive(:cp_file_between_keys).and_raise(StandardError.new('error message')) end it 'handles errors and delegates to wrapped client' do expect do client.cp_file_between_keys('source_key', 'destination_key') end.to raise_error(BlobstoreError, 'error message') - expect(logger).to have_received(:error).with('Error with blobstore: Excon::Error - error message') + expect(logger).to have_received(:error).with('Error with blobstore: StandardError - error message') end end describe '#cp_r_to_blobstore' do before do - allow(wrapped_client).to receive(:cp_r_to_blobstore).and_raise(Excon::Errors::Error.new('error message')) + allow(wrapped_client).to receive(:cp_r_to_blobstore).and_raise(StandardError.new('error message')) end it 'handles errors and delegates to wrapped client' do expect do client.cp_r_to_blobstore('dont/forget/a/source_dir') end.to raise_error(BlobstoreError, 'error message') - expect(logger).to have_received(:error).with('Error with blobstore: Excon::Error - error message') + expect(logger).to have_received(:error).with('Error with blobstore: StandardError - error message') end end describe '#download_from_blobstore' do before do - allow(wrapped_client).to receive(:download_from_blobstore).and_raise(Excon::Errors::Error.new('error message')) + allow(wrapped_client).to receive(:download_from_blobstore).and_raise(StandardError.new('error message')) end it 'handles errors and delegates to wrapped client' do expect do client.download_from_blobstore('some source_key', 'some:destination_path') end.to raise_error(BlobstoreError, 'error message') - expect(logger).to have_received(:error).with('Error with blobstore: Excon::Error - error message') + expect(logger).to have_received(:error).with('Error with blobstore: StandardError - error message') end end describe '#delete' do before do - allow(wrapped_client).to receive(:delete).and_raise(Excon::Errors::Error.new('error message')) + allow(wrapped_client).to receive(:delete).and_raise(StandardError.new('error message')) end it 'handles errors and delegates to wrapped client' do expect do client.delete('a key') end.to raise_error(BlobstoreError, 'error message') - expect(logger).to have_received(:error).with('Error with blobstore: Excon::Error - error message') + expect(logger).to have_received(:error).with('Error with blobstore: StandardError - error message') end end describe '#cp_to_blobstore' do before do - allow(wrapped_client).to receive(:cp_to_blobstore).and_raise(Excon::Errors::Error.new('error message')) + allow(wrapped_client).to receive(:cp_to_blobstore).and_raise(StandardError.new('error message')) end it 'handles errors and delegates to wrapped client' do expect do client.cp_to_blobstore('source_path', 'destination_key') end.to raise_error(BlobstoreError, 'error message') - expect(logger).to have_received(:error).with('Error with blobstore: Excon::Error - error message') + expect(logger).to have_received(:error).with('Error with blobstore: StandardError - error message') end end @@ -154,14 +154,14 @@ module Blobstore let(:args) { 'some-args' } before do - allow(wrapped_client).to receive(:files_for).with(args).and_raise(Excon::Errors::Error.new('error message')) + allow(wrapped_client).to receive(:files_for).with(args).and_raise(StandardError.new('error message')) end it 'handles errors and delegates to wrapped client' do expect do client.files_for(args) end.to raise_error(BlobstoreError, 'error message') - expect(logger).to have_received(:error).with('Error with blobstore: Excon::Error - error message') + expect(logger).to have_received(:error).with('Error with blobstore: StandardError - error message') end end end diff --git a/spec/unit/models/runtime/buildpack_bits_delete_spec.rb b/spec/unit/models/runtime/buildpack_bits_delete_spec.rb index 17d7ae468d7..5cf33877644 100644 --- a/spec/unit/models/runtime/buildpack_bits_delete_spec.rb +++ b/spec/unit/models/runtime/buildpack_bits_delete_spec.rb @@ -35,14 +35,9 @@ module VCAP::CloudController context 'when the blob exists' do it 'creates a job with attributes' do - attrs = blobstore.blob(key).attributes - job_attrs = { - last_modified: attrs[:last_modified], - etag: attrs[:etag], - content_length: 0 - } - - expect(Jobs::Runtime::BlobstoreDelete).to receive(:new).with(key, :buildpack_blobstore, job_attrs).and_call_original + attrs = blobstore.blob(key).attributes(*CloudController::Blobstore::Blob::CACHE_ATTRIBUTES) + + expect(Jobs::Runtime::BlobstoreDelete).to receive(:new).with(key, :buildpack_blobstore, attrs).and_call_original BuildpackBitsDelete.delete_when_safe(key, staging_timeout) end end From 123a1115d524118c67a6bd4ad3a873b3779f99bc Mon Sep 17 00:00:00 2001 From: Katharina Przybill <30441792+kathap@users.noreply.github.com> Date: Thu, 16 Jul 2026 17:28:01 +0200 Subject: [PATCH 22/24] Fix tests --- lib/cloud_controller/blobstore/local/local_blob.rb | 2 +- .../controllers/runtime/app_bits_download_controller_spec.rb | 1 + spec/unit/controllers/runtime/apps_controller_spec.rb | 1 + .../controllers/runtime/buildpack_bits_controller_spec.rb | 2 ++ .../lib/cloud_controller/blobstore/local/local_blob_spec.rb | 4 ++-- spec/unit/lib/cloud_controller/config_spec.rb | 4 +++- .../lib/cloud_controller/packager/local_bits_packer_spec.rb | 2 +- spec/unit/lib/cloud_controller/runner_spec.rb | 2 +- 8 files changed, 12 insertions(+), 6 deletions(-) diff --git a/lib/cloud_controller/blobstore/local/local_blob.rb b/lib/cloud_controller/blobstore/local/local_blob.rb index a4dd7c6b23c..853e392ac18 100644 --- a/lib/cloud_controller/blobstore/local/local_blob.rb +++ b/lib/cloud_controller/blobstore/local/local_blob.rb @@ -29,7 +29,7 @@ def attributes(*keys) { etag: OpenSSL::Digest::MD5.file(@file_path).hexdigest, last_modified: stat.mtime.httpdate, - content_length: stat.size.to_s, + content_length: stat.size, created_at: stat.ctime } end diff --git a/spec/unit/controllers/runtime/app_bits_download_controller_spec.rb b/spec/unit/controllers/runtime/app_bits_download_controller_spec.rb index f05132dae3c..9158782a1a7 100644 --- a/spec/unit/controllers/runtime/app_bits_download_controller_spec.rb +++ b/spec/unit/controllers/runtime/app_bits_download_controller_spec.rb @@ -25,6 +25,7 @@ module VCAP::CloudController before do allow(blob).to receive(:public_download_url).and_return('http://example.com/somewhere/else') allow_any_instance_of(CloudController::Blobstore::Client).to receive(:blob).and_return(blob) + allow_any_instance_of(CloudController::Blobstore::LocalClient).to receive(:local?).and_return(false) end it 'returns 302' do diff --git a/spec/unit/controllers/runtime/apps_controller_spec.rb b/spec/unit/controllers/runtime/apps_controller_spec.rb index eee55c91bf2..93ec790f285 100644 --- a/spec/unit/controllers/runtime/apps_controller_spec.rb +++ b/spec/unit/controllers/runtime/apps_controller_spec.rb @@ -1813,6 +1813,7 @@ def delete_app set_current_user(developer) allow(blob).to receive(:public_download_url).and_return('http://example.com/somewhere/else') allow_any_instance_of(CloudController::Blobstore::Client).to receive(:blob).and_return(blob) + allow_any_instance_of(CloudController::Blobstore::LocalClient).to receive(:local?).and_return(false) end it 'lets the user download the droplet' do diff --git a/spec/unit/controllers/runtime/buildpack_bits_controller_spec.rb b/spec/unit/controllers/runtime/buildpack_bits_controller_spec.rb index 386f64fbb7a..9d80227fc4c 100644 --- a/spec/unit/controllers/runtime/buildpack_bits_controller_spec.rb +++ b/spec/unit/controllers/runtime/buildpack_bits_controller_spec.rb @@ -314,6 +314,8 @@ module VCAP::CloudController it 'lets users with correct basic auth retrieve the bits for a specific buildpack' do put "/v2/buildpacks/#{test_buildpack.guid}/bits", { buildpack: valid_zip } authorize(staging_user, staging_password) + allow_any_instance_of(CloudController::Blobstore::LocalClient).to receive(:local?).and_return(false) + allow_any_instance_of(CloudController::Blobstore::LocalBlob).to receive(:public_download_url).and_return('http://blobstore.example.com/cc-buildpacks/download') get "/v2/buildpacks/#{test_buildpack.guid}/download" expect(last_response.status).to eq(302) expect(last_response.headers['Location']).to match(/cc-buildpacks/) diff --git a/spec/unit/lib/cloud_controller/blobstore/local/local_blob_spec.rb b/spec/unit/lib/cloud_controller/blobstore/local/local_blob_spec.rb index 7eed651b681..8443c8ad24d 100644 --- a/spec/unit/lib/cloud_controller/blobstore/local/local_blob_spec.rb +++ b/spec/unit/lib/cloud_controller/blobstore/local/local_blob_spec.rb @@ -52,8 +52,8 @@ module Blobstore expect(blob.attributes[:etag]).to eq(expected_etag) end - it 'returns the content length as string' do - expect(blob.attributes[:content_length]).to eq(file_content.length.to_s) + it 'returns the content length as integer' do + expect(blob.attributes[:content_length]).to eq(file_content.length) end it 'returns last_modified in httpdate format' do diff --git a/spec/unit/lib/cloud_controller/config_spec.rb b/spec/unit/lib/cloud_controller/config_spec.rb index c47847e15e4..dd91faa8c64 100644 --- a/spec/unit/lib/cloud_controller/config_spec.rb +++ b/spec/unit/lib/cloud_controller/config_spec.rb @@ -388,7 +388,9 @@ module VCAP::CloudController resource_pool: { minimum_size: 9001, maximum_size: 0, - resource_directory_key: 'resource_key' + resource_directory_key: 'resource_key', + blobstore_type: 'local', + local_blobstore_path: Dir.mktmpdir }, external_host: 'host', tls_port: 1234, diff --git a/spec/unit/lib/cloud_controller/packager/local_bits_packer_spec.rb b/spec/unit/lib/cloud_controller/packager/local_bits_packer_spec.rb index e1c1fc1adf4..b8c421a83b8 100644 --- a/spec/unit/lib/cloud_controller/packager/local_bits_packer_spec.rb +++ b/spec/unit/lib/cloud_controller/packager/local_bits_packer_spec.rb @@ -277,7 +277,7 @@ module CloudController::Packager `zip -r --symlinks "#{uploaded_files_path}" "#{symlink_path}"` packer.send_package_to_blobstore(blobstore_key, uploaded_files_path, []) - expect(global_app_bits_cache.files_for('').size).to be 2 + expect(global_app_bits_cache.files_for('').count).to be 2 sha_of_cli_js = 'da39a3ee5e6b4b0d3255bfef95601890afd80709' sha_of_target1_txt = 'f572d396fae9206628714fb2ce00f72e94f2258f' diff --git a/spec/unit/lib/cloud_controller/runner_spec.rb b/spec/unit/lib/cloud_controller/runner_spec.rb index 5fb211d2609..bb593a7da48 100644 --- a/spec/unit/lib/cloud_controller/runner_spec.rb +++ b/spec/unit/lib/cloud_controller/runner_spec.rb @@ -101,7 +101,7 @@ module VCAP::CloudController it 'registers a log counter with the component' do log_counter = Steno::Sink::Counter.new - expect(Steno::Sink::Counter).to receive(:new).once.and_return(log_counter) + allow(Steno::Sink::Counter).to receive(:new).and_return(log_counter) expect(Steno).to receive(:init) do |steno_config| expect(steno_config.sinks).to include log_counter From d1353b81dd496d63269c3e43f7c66dfa2d844858 Mon Sep 17 00:00:00 2001 From: Katharina Przybill <30441792+kathap@users.noreply.github.com> Date: Fri, 17 Jul 2026 10:13:24 +0200 Subject: [PATCH 23/24] Remove deleted file from rubocop --- .rubocop_todo.yml | 1 - .../0014-storage-clis-for-blobstore-operations.md | 12 ++++++------ 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index b976c449464..3c9ca55c547 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -62,7 +62,6 @@ Lint/SuppressedException: - 'lib/cloud_controller/database_uri_generator.rb' - 'lib/vcap/pid_file.rb' - 'spec/unit/actions/v2/route_create_spec.rb' - - 'spec/unit/lib/cloud_controller/blobstore/fog/fog_client_spec.rb' - 'spec/unit/lib/services/service_brokers/service_broker_registration_spec.rb' - 'spec/unit/models/services/service_broker_spec.rb' diff --git a/decisions/0014-storage-clis-for-blobstore-operations.md b/decisions/0014-storage-clis-for-blobstore-operations.md index f59acfcd324..22499edb252 100644 --- a/decisions/0014-storage-clis-for-blobstore-operations.md +++ b/decisions/0014-storage-clis-for-blobstore-operations.md @@ -12,16 +12,14 @@ | Alibaba Cloud| ✅ Implemented | Uses [`storage-cli`](https://github.com/cloudfoundry/storage-cli/tree/main/alioss) with native type `alioss` | **Configuration Migration Status:** -- The `blobstore_provider` field accepts both native storage-cli type names AND legacy fog names -- **Recommended:** Use native storage-cli type names (azurebs, s3, gcs, alioss) -- **Legacy fog names** (AzureRM, AWS, Google, aliyun) still supported for backwards compatibility +- The `blobstore_provider` field accepts native storage-cli type names (azurebs, s3, gcs, alioss) +- **Legacy fog names** (AzureRM, AWS, Google, aliyun) are no longer supported — fog gems have been fully removed - **WebDAV/dav intentionally excluded** until fully supported -- **Timeline:** Legacy fog name support to be removed May 2026 ## Context -Cloud Controller uses the fog gem family to interface with blobstores like Azure, AWS, GCP, and Alibaba Cloud. +Cloud Controller previously used the fog gem family to interface with blobstores like Azure, AWS, GCP, and Alibaba Cloud. These Ruby gems are largely unmaintained, introducing risks such as: * Dependency on deprecated SDKs (e.g., Azure SDK for Ruby) * Blocking Ruby version upgrades @@ -79,11 +77,13 @@ The `bosh-azure-storage-cli` needs to be extended with the following commands: Other providers (AWS, GCP, Alibaba Cloud) will follow. Each will require equivalent blobstore clients and support for the above commands. This will eventually allow us to remove all fog related gems from Cloud Controller. +> ✅ **All fog gems have been fully removed** (fog-aws, fog-core, fog-azure-rm, fog-google, fog-aliyun). +> ✅ **fog-aws and fog-core have been fully removed** as of this change. ## Consequences -* Enables the removal of `fog-azure-rm` and all other fog related gems +* ✅ Enabled the removal of all fog related gems — fog-aws, fog-core, fog-azure-rm, fog-google, and fog-aliyun have all been removed * Reduces long-term maintenance burden and potential security issues * Allows providers to be migrated independently * Increases initial complexity during migration phase From 8f8d1dcffe1c2febc19ba83d45d3f66cb056d647 Mon Sep 17 00:00:00 2001 From: Katharina Przybill <30441792+kathap@users.noreply.github.com> Date: Fri, 17 Jul 2026 10:21:05 +0200 Subject: [PATCH 24/24] Clean up fog references in storage_cli_client and fix doc spec stubs --- .../blobstore/storage_cli/storage_cli_client.rb | 7 +++---- spec/api/documentation/app_bits_api_spec.rb | 11 +++++++---- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/lib/cloud_controller/blobstore/storage_cli/storage_cli_client.rb b/lib/cloud_controller/blobstore/storage_cli/storage_cli_client.rb index 7f3d5cc36fb..c69cc2ef861 100644 --- a/lib/cloud_controller/blobstore/storage_cli/storage_cli_client.rb +++ b/lib/cloud_controller/blobstore/storage_cli/storage_cli_client.rb @@ -20,7 +20,7 @@ class StorageCliClient < BaseClient # Native storage-cli type names supported by CC STORAGE_CLI_TYPES = %w[azurebs alioss s3 gcs dav].freeze - # DEPRECATED: Legacy fog provider names (remove completely after migration window) + # Legacy alias: 'webdav' maps to the native 'dav' type LEGACY_PROVIDER_TO_STORAGE_CLI_TYPE = { 'webdav' => 'dav' }.freeze @@ -38,14 +38,13 @@ def initialize(directory_key:, resource_type:, root_dir:, min_size: nil, max_siz if STORAGE_CLI_TYPES.include?(provider) provider else - # START LEGACY FOG SUPPORT (delete this whole else-branch after migration) + # Legacy alias support (delete after migration window) LEGACY_PROVIDER_TO_STORAGE_CLI_TYPE[provider] - # END LEGACY FOG SUPPORT end unless @storage_type raise "Unknown provider: #{provider}. Supported storage-cli types: #{STORAGE_CLI_TYPES.join(', ')} " \ - "(legacy fog names accepted temporarily: #{LEGACY_PROVIDER_TO_STORAGE_CLI_TYPE.keys.join(', ')})" + "(legacy aliases accepted: #{LEGACY_PROVIDER_TO_STORAGE_CLI_TYPE.keys.join(', ')})" end @cli_path = cli_path diff --git a/spec/api/documentation/app_bits_api_spec.rb b/spec/api/documentation/app_bits_api_spec.rb index 9c51b465c2e..e069eabf9c3 100644 --- a/spec/api/documentation/app_bits_api_spec.rb +++ b/spec/api/documentation/app_bits_api_spec.rb @@ -120,6 +120,11 @@ get '/v2/apps/:guid/download' do let(:async) { false } + before do + allow_any_instance_of(CloudController::Blobstore::LocalClient).to receive(:local?).and_return(false) + allow_any_instance_of(CloudController::Blobstore::LocalBlob).to receive(:public_download_url).and_return('http://blobstore.example.com/download') + end + example 'Downloads the bits for an App' do explanation <<-EOS When using a remote blobstore, such as AWS, the response is a redirect to the actual location of the bits. @@ -128,8 +133,6 @@ EOS no_doc { client.put "/v2/apps/#{process.guid}/bits", app_bits_put_params, headers } - allow_any_instance_of(CloudController::Blobstore::LocalClient).to receive(:local?).and_return(false) - allow_any_instance_of(CloudController::Blobstore::LocalBlob).to receive(:public_download_url).and_return('http://blobstore.example.com/download') client.get "/v2/apps/#{process.guid}/download", {}, headers expect(response_headers['Location']).to eq('http://blobstore.example.com/download') expect(status).to eq(302) @@ -148,6 +151,8 @@ droplet_file.close VCAP::CloudController::Jobs::V3::DropletUpload.new(droplet_file.path, process.desired_droplet.guid, skip_state_transition: false).perform + allow_any_instance_of(CloudController::Blobstore::LocalClient).to receive(:local?).and_return(false) + allow_any_instance_of(CloudController::Blobstore::LocalBlob).to receive(:public_download_url).and_return('http://blobstore.example.com/droplet-download') end example 'Downloads the staged droplet for an App' do @@ -157,8 +162,6 @@ Some blobstores may reject the request in that case. Clients may need to follow the redirect without including the OAuth token. EOS - allow_any_instance_of(CloudController::Blobstore::LocalClient).to receive(:local?).and_return(false) - allow_any_instance_of(CloudController::Blobstore::LocalBlob).to receive(:public_download_url).and_return('http://blobstore.example.com/droplet-download') client.get "/v2/apps/#{process.guid}/droplet/download", {}, headers expect(status).to eq(302) expect(response_headers['Location']).to eq('http://blobstore.example.com/droplet-download')