diff --git a/lib/cloud_controller/blobstore/storage_cli/storage_cli_blob.rb b/lib/cloud_controller/blobstore/storage_cli/storage_cli_blob.rb index b16f9f19777..8c4c98563c2 100644 --- a/lib/cloud_controller/blobstore/storage_cli/storage_cli_blob.rb +++ b/lib/cloud_controller/blobstore/storage_cli/storage_cli_blob.rb @@ -3,18 +3,26 @@ module Blobstore class StorageCliBlob < Blob attr_reader :key - def initialize(key, properties: nil, signed_url: nil) + def initialize(key, properties: nil, signed_url: nil, storage_cli_client: nil, expires_in_seconds: 3600) @key = key @signed_url = signed_url if signed_url + @storage_cli_client = storage_cli_client + @expires_in_seconds = expires_in_seconds # Set properties to an empty hash if nil to avoid nil errors @properties = properties || {} end def internal_download_url + # For DAV with lazy signing support, generate URL on-demand + return @storage_cli_client.sign_internal_url(@key, verb: 'get', expires_in_seconds: @expires_in_seconds) if @storage_cli_client&.supports_lazy_signing? + signed_url end def public_download_url + # For DAV with lazy signing support, generate URL on-demand + return @storage_cli_client.sign_public_url(@key, verb: 'get', expires_in_seconds: @expires_in_seconds) if @storage_cli_client&.supports_lazy_signing? + signed_url end 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 e9fb97b10a2..f532fd86c75 100644 --- a/lib/cloud_controller/blobstore/storage_cli/storage_cli_client.rb +++ b/lib/cloud_controller/blobstore/storage_cli/storage_cli_client.rb @@ -17,16 +17,16 @@ class StorageCliClient < BaseClient 'resource_pool' => :storage_cli_config_file_resource_pool }.freeze - # Native storage-cli type names supported by CC (dav intentionally excluded for now) - STORAGE_CLI_TYPES = %w[azurebs alioss s3 gcs].freeze + # 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) LEGACY_PROVIDER_TO_STORAGE_CLI_TYPE = { 'AzureRM' => 'azurebs', 'aliyun' => 'alioss', 'AWS' => 's3', - 'Google' => 'gcs' - # 'webdav' => 'dav', # intentionally not enabled yet + 'Google' => 'gcs', + 'webdav' => 'dav' }.freeze def initialize(directory_key:, resource_type:, root_dir:, min_size: nil, max_size: nil) @@ -39,10 +39,6 @@ def initialize(directory_key:, resource_type:, root_dir:, min_size: nil, max_siz provider = cfg['provider']&.to_s raise BlobstoreError.new("No provider specified in config file: #{File.basename(config_file_path)}") if provider.nil? || provider.empty? - # Explicitly block unfinished webdav storage-cli support to avoid confusion and wasted effort on debugging - # unsupported providers. Remove this check when webdav support is added. - raise "provider '#{provider}' is not supported yet" if %w[webdav dav].include?(provider) - @storage_type = if STORAGE_CLI_TYPES.include?(provider) provider @@ -172,8 +168,29 @@ def blob(key) properties = properties(key) return nil if properties.nil? || properties.empty? - signed_url = sign_url(partitioned_key(key), verb: 'get', expires_in_seconds: 3600) - StorageCliBlob.new(key, properties:, signed_url:) + # For DAV with lazy signing support, pass client reference for on-demand signing + # For other providers, generate signed URL eagerly + if supports_lazy_signing? + StorageCliBlob.new(key, properties: properties, storage_cli_client: self, expires_in_seconds: 3600) + else + signed_url = sign_url(partitioned_key(key), verb: 'get', expires_in_seconds: 3600) + StorageCliBlob.new(key, properties:, signed_url:) + end + end + + def supports_lazy_signing? + # Only DAV with external signer needs lazy signing for internal vs public endpoints + @storage_type == 'dav' + end + + def sign_internal_url(key, verb:, expires_in_seconds:) + stdout, _status = run_cli('sign-internal', partitioned_key(key), verb.to_s.downcase, "#{expires_in_seconds}s") + stdout.strip + end + + def sign_public_url(key, verb:, expires_in_seconds:) + stdout, _status = run_cli('sign-public', partitioned_key(key), verb.to_s.downcase, "#{expires_in_seconds}s") + stdout.strip end def files_for(prefix, _ignored_directory_prefixes=[]) diff --git a/spec/unit/lib/cloud_controller/blobstore/storage_cli/storage_cli_blob_spec.rb b/spec/unit/lib/cloud_controller/blobstore/storage_cli/storage_cli_blob_spec.rb index 1595f3caa7a..3fecb0549c3 100644 --- a/spec/unit/lib/cloud_controller/blobstore/storage_cli/storage_cli_blob_spec.rb +++ b/spec/unit/lib/cloud_controller/blobstore/storage_cli/storage_cli_blob_spec.rb @@ -18,15 +18,134 @@ module Blobstore end describe 'download_urls' do - it 'returns the internal download URL of the blob' do - expect(blob.internal_download_url).to eq(signed_url) + context 'with pre-generated signed URL (eager signing for non-DAV providers)' do + it 'returns the internal download URL of the blob' do + expect(blob.internal_download_url).to eq(signed_url) + end + + it 'returns the public download URL of the blob' do + expect(blob.public_download_url).to eq(signed_url) + end + end + + context 'with lazy signing (for DAV provider)' do + let(:storage_cli_client) { double('StorageCliClient') } + + before do + allow(storage_cli_client).to receive(:supports_lazy_signing?).and_return(true) + end + + subject(:lazy_blob) do + StorageCliBlob.new( + 'dr/op/droplet-guid', + properties: properties, + storage_cli_client: storage_cli_client, + expires_in_seconds: 3600 + ) + end + + describe '#internal_download_url' do + it 'calls sign_internal_url on the storage_cli_client' do + expect(storage_cli_client).to receive(:sign_internal_url).with( + 'dr/op/droplet-guid', + verb: 'get', + expires_in_seconds: 3600 + ).and_return('https://blobstore.internal:4443/read/cc-droplets/dr/op/droplet-guid?md5=internal123&expires=789') + + url = lazy_blob.internal_download_url + + expect(url).to eq('https://blobstore.internal:4443/read/cc-droplets/dr/op/droplet-guid?md5=internal123&expires=789') + end + + it 'generates URL on-demand each time it is called' do + call_count = 0 + allow(storage_cli_client).to receive(:sign_internal_url) do + call_count += 1 + "https://blobstore.internal/url-#{call_count}" + end + + url1 = lazy_blob.internal_download_url + url2 = lazy_blob.internal_download_url + + expect(url1).to eq('https://blobstore.internal/url-1') + expect(url2).to eq('https://blobstore.internal/url-2') + expect(call_count).to eq(2) + end + end + + describe '#public_download_url' do + it 'calls sign_public_url on the storage_cli_client' do + expect(storage_cli_client).to receive(:sign_public_url).with( + 'dr/op/droplet-guid', + verb: 'get', + expires_in_seconds: 3600 + ).and_return('https://blobstore.example.com/read/cc-droplets/dr/op/droplet-guid?md5=public456&expires=999') + + url = lazy_blob.public_download_url + + expect(url).to eq('https://blobstore.example.com/read/cc-droplets/dr/op/droplet-guid?md5=public456&expires=999') + end + + it 'generates URL on-demand each time it is called' do + call_count = 0 + allow(storage_cli_client).to receive(:sign_public_url) do + call_count += 1 + "https://blobstore.public/url-#{call_count}" + end + + url1 = lazy_blob.public_download_url + url2 = lazy_blob.public_download_url + + expect(url1).to eq('https://blobstore.public/url-1') + expect(url2).to eq('https://blobstore.public/url-2') + expect(call_count).to eq(2) + end + end + + it 'uses custom expires_in_seconds when provided' do + custom_blob = StorageCliBlob.new( + 'test-key', + properties: properties, + storage_cli_client: storage_cli_client, + expires_in_seconds: 7200 + ) + + expect(storage_cli_client).to receive(:sign_internal_url).with( + 'test-key', + verb: 'get', + expires_in_seconds: 7200 + ).and_return('url') + + custom_blob.internal_download_url + end end - it 'returns the public download URL of the blob' do - expect(blob.public_download_url).to eq(signed_url) + context 'when storage_cli_client does not support lazy signing' do + let(:storage_cli_client) { double('StorageCliClient') } + + before do + allow(storage_cli_client).to receive(:supports_lazy_signing?).and_return(false) + end + + subject(:non_lazy_blob) do + StorageCliBlob.new( + 'test-blob', + properties: properties, + signed_url: signed_url, + storage_cli_client: storage_cli_client + ) + end + + it 'falls back to using pre-generated signed_url for internal_download_url' do + expect(non_lazy_blob.internal_download_url).to eq(signed_url) + end + + it 'falls back to using pre-generated signed_url for public_download_url' do + expect(non_lazy_blob.public_download_url).to eq(signed_url) + end end - context 'when signed_url is not provided' do + context 'when signed_url is not provided and no storage_cli_client' do subject(:blob_without_signed_url) { StorageCliBlob.new('test-blob', properties:) } it 'raises an error when accessing internal_download_url' do 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 ead5a6e64f8..d5cfc2f8a9e 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 @@ -4,154 +4,240 @@ module CloudController module Blobstore RSpec.describe StorageCliClient do + # Helper methods + def write_config_file(hash) + file = Tempfile.new(['storage-cli', '.json']) + file.write(hash.to_json) + file.flush + file + end + + def stub_config_for_droplets(path) + config_double = instance_double(VCAP::CloudController::Config) + allow(VCAP::CloudController::Config).to receive(:config).and_return(config_double) + allow(config_double).to receive(:get).with(:storage_cli_config_file_droplets).and_return(path) + allow(Steno).to receive(:logger).and_return(double(info: nil, error: nil, debug: nil)) + end + describe 'client init' do # DEPRECATED: Legacy fog provider tests - remove after migration window # START LEGACY FOG SUPPORT TESTS - it 'init the correct client when JSON has provider AzureRM (legacy fog name)' do - droplets_cfg = Tempfile.new(['droplets', '.json']) - droplets_cfg.write({ provider: 'AzureRM', - account_key: 'bommelkey', - account_name: 'bommel', - container_name: 'bommelcontainer', - environment: 'BommelCloud' }.to_json) - droplets_cfg.flush - - config_double = instance_double(VCAP::CloudController::Config) - allow(VCAP::CloudController::Config).to receive(:config).and_return(config_double) - allow(config_double).to receive(:get).with(:storage_cli_config_file_droplets).and_return(droplets_cfg.path) - - client = StorageCliClient.new( - directory_key: 'dummy-key', - root_dir: 'dummy-root', - resource_type: 'droplets' + it 'maps AzureRM legacy provider to azurebs storage-cli type' do + droplets_cfg = write_config_file( + provider: 'AzureRM', + account_key: 'bommelkey', + account_name: 'bommel', + container_name: 'bommelcontainer', + environment: 'BommelCloud' ) - expect(client.instance_variable_get(:@storage_type)).to eq('azurebs') - expect(client.instance_variable_get(:@resource_type)).to eq('droplets') - expect(client.instance_variable_get(:@root_dir)).to eq('dummy-root') - expect(client.instance_variable_get(:@directory_key)).to eq('dummy-key') + begin + stub_config_for_droplets(droplets_cfg.path) - droplets_cfg.close! + client = StorageCliClient.new( + directory_key: 'dummy-key', + root_dir: 'dummy-root', + resource_type: 'droplets' + ) + expect(client.instance_variable_get(:@storage_type)).to eq('azurebs') + expect(client.instance_variable_get(:@resource_type)).to eq('droplets') + expect(client.instance_variable_get(:@root_dir)).to eq('dummy-root') + expect(client.instance_variable_get(:@directory_key)).to eq('dummy-key') + ensure + droplets_cfg.close! + end end - # END LEGACY FOG SUPPORT TESTS - - it 'init the correct client when JSON has provider azurebs (native storage-cli type)' do - droplets_cfg = Tempfile.new(['droplets', '.json']) - droplets_cfg.write({ provider: 'azurebs', - account_key: 'bommelkey', - account_name: 'bommel', - container_name: 'bommelcontainer', - environment: 'BommelCloud' }.to_json) - droplets_cfg.flush - - config_double = instance_double(VCAP::CloudController::Config) - allow(VCAP::CloudController::Config).to receive(:config).and_return(config_double) - allow(config_double).to receive(:get).with(:storage_cli_config_file_droplets).and_return(droplets_cfg.path) - client = StorageCliClient.new( - directory_key: 'dummy-key', - root_dir: 'dummy-root', - resource_type: 'droplets' + 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' ) - expect(client.instance_variable_get(:@storage_type)).to eq('azurebs') - expect(client.instance_variable_get(:@resource_type)).to eq('droplets') - expect(client.instance_variable_get(:@root_dir)).to eq('dummy-root') - expect(client.instance_variable_get(:@directory_key)).to eq('dummy-key') + begin + stub_config_for_droplets(droplets_cfg.path) - droplets_cfg.close! + 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 - # DEPRECATED: Legacy fog provider tests - remove after migration window - # START LEGACY FOG SUPPORT TESTS - it 'raises an error for an unknown legacy provider' do - droplets_cfg = Tempfile.new(['droplets', '.json']) - droplets_cfg.write( - { provider: 'UnknownProvider', - account_key: 'bommelkey', - account_name: 'bommel', - container_name: 'bommelcontainer', - environment: 'BommelCloud' }.to_json + it 'maps Google legacy provider to gcs storage-cli type' do + droplets_cfg = write_config_file( + provider: 'Google', + bucket_name: 'test-bucket', + json_key: '{}' ) - droplets_cfg.flush + begin + stub_config_for_droplets(droplets_cfg.path) - config_double = instance_double(VCAP::CloudController::Config) - allow(VCAP::CloudController::Config).to receive(:config).and_return(config_double) - allow(config_double).to receive(:get).with(:storage_cli_config_file_droplets).and_return(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('gcs') + ensure + droplets_cfg.close! + end + end - expect do - StorageCliClient.new(directory_key: 'dummy-key', root_dir: 'dummy-root', resource_type: 'droplets') - end.to raise_error(RuntimeError, /Unknown provider: UnknownProvider/) + it 'maps aliyun legacy provider to alioss storage-cli type' do + droplets_cfg = write_config_file( + provider: 'aliyun', + access_key_id: 'key', + access_key_secret: 'secret', + endpoint: 'aliyun.com', + bucket_name: 'bucket' + ) + begin + stub_config_for_droplets(droplets_cfg.path) - droplets_cfg.close! + client = StorageCliClient.new( + directory_key: 'dummy-key', + root_dir: 'dummy-root', + resource_type: 'droplets' + ) + expect(client.instance_variable_get(:@storage_type)).to eq('alioss') + ensure + droplets_cfg.close! + end end - it 'blocks webdav/dav provider explicitly' do - droplets_cfg = Tempfile.new(['droplets', '.json']) - droplets_cfg.write( - { provider: 'webdav', - account_key: 'bommelkey' }.to_json + it 'maps webdav legacy provider to dav storage-cli type' do + droplets_cfg = write_config_file( + provider: 'webdav', + endpoint: 'https://webdav.example.com', + user: 'testuser', + password: 'testpass' ) - droplets_cfg.flush + begin + stub_config_for_droplets(droplets_cfg.path) - config_double = instance_double(VCAP::CloudController::Config) - allow(VCAP::CloudController::Config).to receive(:config).and_return(config_double) - allow(config_double).to receive(:get).with(:storage_cli_config_file_droplets).and_return(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('dav') + ensure + droplets_cfg.close! + end + end - expect do - StorageCliClient.new(directory_key: 'dummy-key', root_dir: 'dummy-root', resource_type: 'droplets') - end.to raise_error(RuntimeError, /is not supported yet/) + it 'raises an error for an unknown legacy provider' do + droplets_cfg = write_config_file( + provider: 'UnknownProvider', + account_key: 'bommelkey', + account_name: 'bommel', + container_name: 'bommelcontainer', + environment: 'BommelCloud' + ) + begin + stub_config_for_droplets(droplets_cfg.path) - droplets_cfg.close! + expect do + StorageCliClient.new(directory_key: 'dummy-key', root_dir: 'dummy-root', resource_type: 'droplets') + end.to raise_error(RuntimeError, /Unknown provider: UnknownProvider/) + ensure + droplets_cfg.close! + end end # END LEGACY FOG SUPPORT TESTS - it 'raises an error for an unknown storage-cli type' do - droplets_cfg = Tempfile.new(['droplets', '.json']) - droplets_cfg.write( - { provider: 'unknown_type', - account_key: 'bommelkey', - account_name: 'bommel', - container_name: 'bommelcontainer', - environment: 'BommelCloud' }.to_json + it 'init the correct client when JSON has provider azurebs (native storage-cli type)' do + droplets_cfg = write_config_file( + provider: 'azurebs', + account_key: 'bommelkey', + account_name: 'bommel', + container_name: 'bommelcontainer', + environment: 'BommelCloud' ) - droplets_cfg.flush + begin + stub_config_for_droplets(droplets_cfg.path) - config_double = instance_double(VCAP::CloudController::Config) - allow(VCAP::CloudController::Config).to receive(:config).and_return(config_double) - allow(config_double).to receive(:get).with(:storage_cli_config_file_droplets).and_return(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('azurebs') + expect(client.instance_variable_get(:@resource_type)).to eq('droplets') + expect(client.instance_variable_get(:@root_dir)).to eq('dummy-root') + expect(client.instance_variable_get(:@directory_key)).to eq('dummy-key') + ensure + droplets_cfg.close! + end + end - expect do - StorageCliClient.new(directory_key: 'dummy-key', root_dir: 'dummy-root', resource_type: 'droplets') - end.to raise_error(RuntimeError, /Unknown provider: unknown_type/) + it 'init the correct client when JSON has provider dav (native storage-cli type)' do + droplets_cfg = write_config_file( + provider: 'dav', + endpoint: 'https://webdav.example.com', + user: 'testuser', + password: 'testpass' + ) + begin + stub_config_for_droplets(droplets_cfg.path) - droplets_cfg.close! + client = StorageCliClient.new( + directory_key: 'dummy-key', + root_dir: 'dummy-root', + resource_type: 'droplets' + ) + expect(client.instance_variable_get(:@storage_type)).to eq('dav') + expect(client.instance_variable_get(:@resource_type)).to eq('droplets') + expect(client.instance_variable_get(:@root_dir)).to eq('dummy-root') + expect(client.instance_variable_get(:@directory_key)).to eq('dummy-key') + ensure + droplets_cfg.close! + end end - # DEPRECATED: Legacy fog provider test - remove after migration window - # START LEGACY FOG SUPPORT TESTS - it 'raises an error when provider is missing' do - droplets_cfg = Tempfile.new(['droplets', '.json']) - droplets_cfg.write( - { account_key: 'bommelkey', - account_name: 'bommel', - container_name: 'bommelcontainer', - environment: 'BommelCloud' }.to_json + it 'raises an error for an unknown storage-cli type' do + droplets_cfg = write_config_file( + provider: 'unknown_type', + account_key: 'bommelkey', + account_name: 'bommel', + container_name: 'bommelcontainer', + environment: 'BommelCloud' ) - droplets_cfg.flush + begin + stub_config_for_droplets(droplets_cfg.path) - config_double = instance_double(VCAP::CloudController::Config) - allow(VCAP::CloudController::Config).to receive(:config).and_return(config_double) - allow(config_double).to receive(:get).with(:storage_cli_config_file_droplets).and_return(droplets_cfg.path) + expect do + StorageCliClient.new(directory_key: 'dummy-key', root_dir: 'dummy-root', resource_type: 'droplets') + end.to raise_error(RuntimeError, /Unknown provider: unknown_type/) + ensure + droplets_cfg.close! + end + end - expect do - StorageCliClient.new(directory_key: 'dummy-key', root_dir: 'dummy-root', resource_type: 'droplets') - end.to raise_error(BlobstoreError, /No provider specified/) + it 'raises an error when provider is missing' do + droplets_cfg = write_config_file( + account_key: 'bommelkey', + account_name: 'bommel', + container_name: 'bommelcontainer', + environment: 'BommelCloud' + ) + begin + stub_config_for_droplets(droplets_cfg.path) - droplets_cfg.close! + expect do + StorageCliClient.new(directory_key: 'dummy-key', root_dir: 'dummy-root', resource_type: 'droplets') + end.to raise_error(BlobstoreError, /No provider specified/) + ensure + droplets_cfg.close! + end end - # END LEGACY FOG SUPPORT TESTS - # After removal, change error message expectation to /No storage_type specified/ - it 'raise when no resource type' do + it 'raises when resource_type is missing' do expect do StorageCliClient.new(directory_key: 'dummy-key', root_dir: 'dummy-root', resource_type: nil) end.to raise_error(RuntimeError, 'Missing resource_type') @@ -161,21 +247,12 @@ module Blobstore describe 'resource_type → config file selection & validation' do let(:config_double) { instance_double(VCAP::CloudController::Config) } - let(:droplets_cfg) { Tempfile.new(['droplets', '.json']) } - let(:buildpacks_cfg) { Tempfile.new(['buildpacks', '.json']) } - let(:packages_cfg) { Tempfile.new(['packages', '.json']) } - let(:resource_pool_cfg) { Tempfile.new(['resource_pool', '.json']) } + let(:droplets_cfg) { write_config_file(provider: 'azurebs', account_key: 'key', account_name: 'acc', container_name: 'cont', environment: 'Cloud') } + let(:buildpacks_cfg) { write_config_file(provider: 'azurebs', account_key: 'key', account_name: 'acc', container_name: 'cont', environment: 'Cloud') } + let(:packages_cfg) { write_config_file(provider: 'azurebs', account_key: 'key', account_name: 'acc', container_name: 'cont', environment: 'Cloud') } + let(:resource_pool_cfg) { write_config_file(provider: 'azurebs', account_key: 'key', account_name: 'acc', container_name: 'cont', environment: 'Cloud') } before do - [droplets_cfg, buildpacks_cfg, packages_cfg, resource_pool_cfg].each do |f| - f.write({ provider: 'AzureRM', - account_key: 'bommelkey', - account_name: 'bommel', - container_name: 'bommelcontainer', - environment: 'BommelCloud' }.to_json) - f.flush - end - allow(VCAP::CloudController::Config).to receive(:config).and_return(config_double) allow(config_double).to receive(:get) do |key| @@ -264,22 +341,10 @@ def build_client(resource_type) describe 'client helper operations' do describe 'Json operations' do - let(:droplets_cfg) do - f = Tempfile.new(['droplets', '.json']) - f.write({ provider: 'AzureRM', - account_key: 'bommelkey', - account_name: 'bommel', - container_name: 'bommelcontainer', - environment: 'BommelCloud' }.to_json) - f.flush - f - end - - let(:config_double) { instance_double(VCAP::CloudController::Config) } + let(:droplets_cfg) { write_config_file(provider: 'azurebs', account_key: 'key', account_name: 'acc', container_name: 'cont', environment: 'Cloud') } before do - allow(VCAP::CloudController::Config).to receive(:config).and_return(config_double) - allow(config_double).to receive(:get).with(:storage_cli_config_file_droplets).and_return(droplets_cfg.path) + stub_config_for_droplets(droplets_cfg.path) end after do @@ -302,19 +367,9 @@ def build_client(resource_type) end describe 'with valid client' do + let(:droplets_cfg) { write_config_file(provider: 'azurebs', account_key: 'key', account_name: 'acc', container_name: 'cont', environment: 'Cloud') } let(:client) do - droplets_cfg = Tempfile.new(['droplets', '.json']) - droplets_cfg.write({ provider: 'AzureRM', - account_key: 'bommelkey', - account_name: 'bommel', - container_name: 'bommelcontainer', - environment: 'BommelCloud' }.to_json) - droplets_cfg.flush - - config_double = instance_double(VCAP::CloudController::Config) - allow(VCAP::CloudController::Config).to receive(:config).and_return(config_double) - allow(config_double).to receive(:get).with(:storage_cli_config_file_droplets).and_return(droplets_cfg.path) - + stub_config_for_droplets(droplets_cfg.path) StorageCliClient.new( directory_key: 'dummy-key', root_dir: 'dummy-root', @@ -322,6 +377,10 @@ def build_client(resource_type) ) end + after do + droplets_cfg.close! + end + it '#local? returns false' do expect(client.local?).to be false end @@ -340,14 +399,13 @@ def build_client(resource_type) describe 'client operations' do let!(:tmp_cfg) do - f = Tempfile.new(['storage_cli_config', '.json']) - f.write({ provider: 'AzureRM', - account_name: 'some-account-name', - account_key: 'some-access-key', - container_name: directory_key, - environment: 'AzureCloud' }.to_json) - f.flush - f + write_config_file( + provider: 'azurebs', + account_name: 'some-account-name', + account_key: 'some-access-key', + container_name: directory_key, + environment: 'AzureCloud' + ) end before do @@ -371,13 +429,6 @@ def build_client(resource_type) subject(:client) { StorageCliClient.new(directory_key: directory_key, resource_type: resource_type, root_dir: 'bommel') } let(:directory_key) { 'my-bucket' } let(:resource_type) { 'resource_pool' } - let(:downloaded_file) do - Tempfile.open('') do |tmpfile| - tmpfile.write('downloaded file content') - tmpfile - end - end - let(:deletable_blob) { StorageCliBlob.new('deletable-blob') } let(:dest_path) { File.join(Dir.mktmpdir, SecureRandom.uuid) } @@ -397,7 +448,7 @@ def build_client(resource_type) allow(VCAP::CloudController::Config.config).to receive(:get).with(:storage_cli_optional_flags).and_return('-log-level warn -log-file some/path/storage-cli.log') end - it('returns empty list') { + it('returns list with parsed flags') { expect(client.send(:additional_flags)).to eq(['-log-level', 'warn', '-log-file', 'some/path/storage-cli.log']) } end @@ -465,20 +516,80 @@ def build_client(resource_type) describe '#blob' do let(:properties_json) { '{"etag": "test-etag", "last_modified": "2024-10-01T00:00:00Z", "content_length": 1024}' } - it 'returns a list of StorageCliBlob instances for a given key' do - allow(client).to receive(:run_cli).with('properties', 'bommel/va/li/valid-blob').and_return([properties_json, instance_double(Process::Status, exitstatus: 0)]) - allow(client).to receive(:run_cli).with('sign', 'bommel/va/li/valid-blob', 'get', '3600s').and_return(['some-url', instance_double(Process::Status, exitstatus: 0)]) + context 'for non-DAV providers (eager signing)' do + it 'returns a StorageCliBlob with pre-generated signed URL' do + allow(client).to receive(:run_cli).with('properties', 'bommel/va/li/valid-blob').and_return([properties_json, instance_double(Process::Status, exitstatus: 0)]) + allow(client).to receive(:run_cli).with('sign', 'bommel/va/li/valid-blob', 'get', '3600s').and_return(['some-url', instance_double(Process::Status, exitstatus: 0)]) + + blob = client.blob('valid-blob') + expect(blob).to be_a(StorageCliBlob) + expect(blob.key).to eq('valid-blob') + expect(blob.attributes(:etag, :last_modified, :content_length)).to eq({ + etag: 'test-etag', + last_modified: '2024-10-01T00:00:00Z', + content_length: 1024 + }) + expect(blob.internal_download_url).to eq('some-url') + expect(blob.public_download_url).to eq('some-url') + end + end + + context 'for DAV provider (lazy signing)' do + let!(:dav_cfg) do + write_config_file( + provider: 'dav', + endpoint: 'https://blobstore.internal:4443/admin/cc-droplets', + public_endpoint: 'https://blobstore.example.com/admin/cc-droplets', + user: 'testuser', + password: 'testpass', + signed_url_format: 'external-nginx-secure-link-signer' + ) + end + + let(:dav_client) do + stub_config_for_droplets(dav_cfg.path) + StorageCliClient.new( + directory_key: 'cc-droplets', + root_dir: nil, + resource_type: 'droplets' + ) + end + + after { dav_cfg.close! } + + it 'returns a StorageCliBlob with storage_cli_client reference for lazy signing' do + allow(dav_client).to receive(:run_cli).with('properties', 'dr/op/droplet-guid').and_return([properties_json, instance_double(Process::Status, exitstatus: 0)]) + + blob = dav_client.blob('droplet-guid') + expect(blob).to be_a(StorageCliBlob) + expect(blob.key).to eq('droplet-guid') + expect(blob.instance_variable_get(:@storage_cli_client)).to eq(dav_client) + expect(blob.instance_variable_get(:@signed_url)).to be_nil + end + + it 'generates internal URL on-demand when internal_download_url is called' do + allow(dav_client).to receive(:run_cli).with('properties', 'dr/op/droplet-guid').and_return([properties_json, instance_double(Process::Status, exitstatus: 0)]) + allow(dav_client).to receive(:run_cli).with('sign-internal', 'dr/op/droplet-guid', 'get', + '3600s').and_return(['https://blobstore.internal:4443/read/cc-droplets/dr/op/droplet-guid?md5=abc&expires=123', + instance_double(Process::Status, exitstatus: 0)]) + + blob = dav_client.blob('droplet-guid') + internal_url = blob.internal_download_url + + expect(internal_url).to eq('https://blobstore.internal:4443/read/cc-droplets/dr/op/droplet-guid?md5=abc&expires=123') + end + + it 'generates public URL on-demand when public_download_url is called' do + allow(dav_client).to receive(:run_cli).with('properties', 'dr/op/droplet-guid').and_return([properties_json, instance_double(Process::Status, exitstatus: 0)]) + allow(dav_client).to receive(:run_cli).with('sign-public', 'dr/op/droplet-guid', 'get', + '3600s').and_return(['https://blobstore.example.com/read/cc-droplets/dr/op/droplet-guid?md5=xyz&expires=456', + instance_double(Process::Status, exitstatus: 0)]) - blob = client.blob('valid-blob') - expect(blob).to be_a(StorageCliBlob) - expect(blob.key).to eq('valid-blob') - expect(blob.attributes(:etag, :last_modified, :content_length)).to eq({ - etag: 'test-etag', - last_modified: '2024-10-01T00:00:00Z', - content_length: 1024 - }) - expect(blob.internal_download_url).to eq('some-url') - expect(blob.public_download_url).to eq('some-url') + blob = dav_client.blob('droplet-guid') + public_url = blob.public_download_url + + expect(public_url).to eq('https://blobstore.example.com/read/cc-droplets/dr/op/droplet-guid?md5=xyz&expires=456') + end end it 'raises an error if the cli output is empty' do @@ -492,10 +603,148 @@ def build_client(resource_type) end end + describe '#supports_lazy_signing?' do + context 'for DAV provider' do + let!(:dav_cfg) do + write_config_file( + provider: 'dav', + endpoint: 'https://blobstore.internal:4443', + user: 'testuser', + password: 'testpass' + ) + end + + let(:dav_client) do + stub_config_for_droplets(dav_cfg.path) + StorageCliClient.new( + directory_key: 'cc-droplets', + root_dir: nil, + resource_type: 'droplets' + ) + end + + after { dav_cfg.close! } + + it 'returns true' do + expect(dav_client.supports_lazy_signing?).to be true + end + end + + context 'for non-DAV providers' do + let!(:s3_cfg) do + write_config_file( + provider: 's3', + bucket_name: 'test-bucket', + access_key_id: 'key', + secret_access_key: 'secret' + ) + end + + let(:s3_client) do + stub_config_for_droplets(s3_cfg.path) + StorageCliClient.new( + directory_key: 'cc-droplets', + root_dir: nil, + resource_type: 'droplets' + ) + end + + after { s3_cfg.close! } + + it 'returns false for S3' do + expect(s3_client.supports_lazy_signing?).to be false + end + end + end + + describe '#sign_internal_url' do + let!(:dav_cfg) do + write_config_file( + provider: 'dav', + endpoint: 'https://blobstore.internal:4443/admin/cc-droplets', + public_endpoint: 'https://blobstore.example.com/admin/cc-droplets', + user: 'testuser', + password: 'testpass' + ) + end + + let(:dav_client) do + stub_config_for_droplets(dav_cfg.path) + StorageCliClient.new( + directory_key: 'cc-droplets', + root_dir: nil, + resource_type: 'droplets' + ) + end + + after { dav_cfg.close! } + + it 'calls storage-cli sign-internal command and returns signed URL' do + expect(dav_client).to receive(:run_cli).with('sign-internal', 'dr/o/dr/op/droplet-guid', 'get', + '7200s').and_return(['https://blobstore.internal:4443/read/cc-droplets/dr/op/droplet-guid?md5=internal123&expires=789', + instance_double(Process::Status, exitstatus: 0)]) + + signed_url = dav_client.sign_internal_url('dr/op/droplet-guid', verb: 'get', expires_in_seconds: 7200) + + expect(signed_url).to eq('https://blobstore.internal:4443/read/cc-droplets/dr/op/droplet-guid?md5=internal123&expires=789') + end + + it 'converts verb to lowercase' do + expect(dav_client).to receive(:run_cli).with('sign-internal', 'dr/o/dr/op/droplet-guid', 'get', + '3600s').and_return(['url', instance_double(Process::Status, exitstatus: 0)]) + + dav_client.sign_internal_url('dr/op/droplet-guid', verb: :GET, expires_in_seconds: 3600) + end + end + + describe '#sign_public_url' do + let!(:dav_cfg) do + write_config_file( + provider: 'dav', + endpoint: 'https://blobstore.internal:4443/admin/cc-droplets', + public_endpoint: 'https://blobstore.example.com/admin/cc-droplets', + user: 'testuser', + password: 'testpass' + ) + end + + let(:dav_client) do + stub_config_for_droplets(dav_cfg.path) + StorageCliClient.new( + directory_key: 'cc-droplets', + root_dir: nil, + resource_type: 'droplets' + ) + end + + after { dav_cfg.close! } + + it 'calls storage-cli sign-public command and returns signed URL' do + expect(dav_client).to receive(:run_cli).with('sign-public', 'pa/c/pa/ck/package-guid', 'get', + '1800s').and_return(['https://blobstore.example.com/read/cc-packages/pa/ck/package-guid?md5=public456&expires=999', + instance_double(Process::Status, exitstatus: 0)]) + + signed_url = dav_client.sign_public_url('pa/ck/package-guid', verb: 'get', expires_in_seconds: 1800) + + expect(signed_url).to eq('https://blobstore.example.com/read/cc-packages/pa/ck/package-guid?md5=public456&expires=999') + end + + it 'converts verb to lowercase' do + expect(dav_client).to receive(:run_cli).with('sign-public', 'pa/c/pa/ck/package-guid', 'put', + '3600s').and_return(['url', instance_double(Process::Status, exitstatus: 0)]) + + dav_client.sign_public_url('pa/ck/package-guid', verb: :PUT, expires_in_seconds: 3600) + end + end + describe '#run_cli' do + before do + allow(client).to receive(:additional_flags).and_return([]) + end + it 'returns output and status on success' do status = instance_double(Process::Status, success?: true, exitstatus: 0) - allow(Open3).to receive(:capture3).with(anything, '-s', anything, '-c', anything, 'list', 'arg1').and_return(['ok', '', status]) + allow(Open3).to receive(:capture3).and_return(['ok', '', status]) output, returned_status = client.send(:run_cli, 'list', 'arg1') expect(output).to eq('ok') @@ -504,7 +753,7 @@ def build_client(resource_type) it 'raises an error on failure' do status = instance_double(Process::Status, success?: false, exitstatus: 1) - allow(Open3).to receive(:capture3).with(anything, '-s', anything, '-c', anything, 'list', 'arg1').and_return(['', 'error message', status]) + allow(Open3).to receive(:capture3).and_return(['', 'error message', status]) expect do client.send(:run_cli, 'list', 'arg1') @@ -513,7 +762,7 @@ def build_client(resource_type) it 'allows exit code 3 if specified' do status = instance_double(Process::Status, success?: false, exitstatus: 3) - allow(Open3).to receive(:capture3).with(anything, '-s', anything, '-c', anything, 'list', 'arg1').and_return(['', 'error message', status]) + allow(Open3).to receive(:capture3).and_return(['', 'error message', status]) output, returned_status = client.send(:run_cli, 'list', 'arg1', allow_exit_code_three: true) expect(output).to eq('')