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 e9fb97b10a..fe2fcc28f3 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 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 ead5a6e64f..789e5435a1 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,7 +516,7 @@ 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 + it 'returns a StorageCliBlob 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)]) @@ -493,9 +544,13 @@ def build_client(resource_type) 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 +559,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 +568,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('')