Read storage-cli config JSON from capi (AzureRM)

kathap · kathap · commit d4ca4ba094f3 · 2025-09-26T15:57:59.000+02:00
Stop building storage-cli JSON inside CC; instead consume files
  rendered by capi job templates. StorageCliClient now selects the JSON
  file by `resource_type`.
diff --git a/config/cloud_controller.yml b/config/cloud_controller.yml
@@ -320,6 +320,12 @@ directories:
   diagnostics: /tmp
 
 stacks_file: config/stacks.yml
+
+storage_cli_config_file_droplets: config/storage_cli_config_droplets.json
+storage_cli_config_file_packages: config/storage_cli_config_packages.json
+storage_cli_config_file_buildpacks: config/storage_cli_config_buildpacks.json
+storage_cli_config_file_resource_pool: config/storage_cli_config_resource_pool.json
+
 newrelic_enabled: false
 
 max_annotations_per_resource: 200
diff --git a/lib/cloud_controller/blobstore/client_provider.rb b/lib/cloud_controller/blobstore/client_provider.rb
@@ -16,7 +16,7 @@ def self.provide(options:, directory_key:, root_dir: nil, resource_type: nil)
           provide_fog(options, directory_key, root_dir)
         elsif options[:blobstore_type] == 'storage-cli'
           # storage-cli is an experimental feature and not yet fully implemented. !!! DO NOT USE IN PRODUCTION !!!
-          provide_storage_cli(options, directory_key, root_dir)
+          provide_storage_cli(options, directory_key, root_dir, resource_type)
         else
           provide_webdav(options, directory_key, root_dir)
         end
@@ -71,11 +71,12 @@ def provide_webdav(options, directory_key, root_dir)
           Client.new(SafeDeleteClient.new(retryable_client, root_dir))
         end
 
-        def provide_storage_cli(options, directory_key, root_dir)
+        def provide_storage_cli(options, directory_key, root_dir, resource_type)
           raise BlobstoreError.new('connection_config for storage-cli is not provided') unless options[:connection_config]
 
           client = StorageCliClient.build(connection_config: options.fetch(:connection_config),
                                           directory_key: directory_key,
+                                          resource_type: resource_type,
                                           root_dir: root_dir,
                                           min_size: options[:minimum_size],
                                           max_size: options[:maximum_size])
diff --git a/lib/cloud_controller/blobstore/storage_cli/azure_storage_cli_client.rb b/lib/cloud_controller/blobstore/storage_cli/azure_storage_cli_client.rb
@@ -5,15 +5,6 @@ def cli_path
         ENV['AZURE_STORAGE_CLI_PATH'] || '/var/vcap/packages/azure-storage-cli/bin/azure-storage-cli'
       end
 
-      def build_config(connection_config)
-        {
-          account_name: connection_config[:azure_storage_account_name],
-          account_key: connection_config[:azure_storage_access_key],
-          container_name: @directory_key,
-          environment: connection_config[:environment]
-        }.compact
-      end
-
       CloudController::Blobstore::StorageCliClient.register('AzureRM', AzureStorageCliClient)
     end
   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
@@ -19,26 +19,53 @@ def register(provider, klass)
           registry[provider] = klass
         end
 
-        def build(connection_config:, directory_key:, root_dir:, min_size: nil, max_size: nil)
+        def build(connection_config:, directory_key:, root_dir:, resource_type: nil, min_size: nil, max_size: nil)
           provider = connection_config[:provider]
           raise 'Missing connection_config[:provider]' if provider.nil?
+          raise 'Missing resource_type' if resource_type.nil?
 
           impl_class = registry[provider]
           raise "No storage CLI client registered for provider #{provider}" unless impl_class
 
-          impl_class.new(connection_config:, directory_key:, root_dir:, min_size:, max_size:)
+          impl_class.new(connection_config:, directory_key:, root_dir:, resource_type:, min_size:, max_size:)
         end
       end
 
-      def initialize(connection_config:, directory_key:, root_dir:, min_size: nil, max_size: nil)
+      def initialize(connection_config:, directory_key:, resource_type:, root_dir:, min_size: nil, max_size: nil)
         @cli_path = cli_path
         @directory_key = directory_key
+        @resource_type = resource_type.to_s
         @root_dir = root_dir
         @min_size = min_size || 0
         @max_size = max_size
-        config = build_config(connection_config)
-        @config_file = write_config_file(config)
         @fork = connection_config.fetch(:fork, false)
+
+        file_path = case @resource_type
+                    when 'droplets', 'buildpack_cache'
+                      VCAP::CloudController::Config.config.get(:storage_cli_config_file_droplets)
+                    when 'buildpacks'
+                      VCAP::CloudController::Config.config.get(:storage_cli_config_file_buildpacks)
+                    when 'packages'
+                      VCAP::CloudController::Config.config.get(:storage_cli_config_file_packages)
+                    when 'resource_pool'
+                      VCAP::CloudController::Config.config.get(:storage_cli_config_file_resource_pool)
+                    else
+                      raise BlobstoreError.new("Unknown resource_type: #{@resource_type}")
+                    end
+
+        unless file_path && File.file?(file_path) && File.readable?(file_path)
+          raise BlobstoreError.new("storage-cli config file not found or not readable at: #{file_path.inspect}")
+        end
+
+        begin
+          VCAP::CloudController::YAMLConfig.safe_load_file(file_path)
+        rescue => e
+          raise BlobstoreError.new("Failed to load storage-cli config at #{file_path}: #{e.message}")
+        end
+
+        @config_file = file_path
+        logger.info('storage_cli_config_selected', resource_type: @resource_type, path: @config_file)
+
       end
 
       def local?
diff --git a/lib/cloud_controller/config_schemas/api_schema.rb b/lib/cloud_controller/config_schemas/api_schema.rb
@@ -95,6 +95,12 @@ class ApiSchema < VCAP::Config
           },
 
           stacks_file: String,
+
+          optional(:storage_cli_config_file_buildpacks) => String,
+          optional(:storage_cli_config_file_packages) => String,
+          optional(:storage_cli_config_file_resource_pool) => String,
+          optional(:storage_cli_config_file_droplets) => String,
+
           newrelic_enabled: bool,
 
           optional(:max_migration_duration_in_minutes) => Integer,
diff --git a/lib/cloud_controller/config_schemas/clock_schema.rb b/lib/cloud_controller/config_schemas/clock_schema.rb
@@ -50,6 +50,11 @@ class ClockSchema < VCAP::Config
 
           pid_filename: String, # Pid filename to use
 
+          optional(:storage_cli_config_file_buildpacks) => String,
+          optional(:storage_cli_config_file_packages) => String,
+          optional(:storage_cli_config_file_resource_pool) => String,
+          optional(:storage_cli_config_file_droplets) => String,
+
           newrelic_enabled: bool,
 
           optional(:max_migration_duration_in_minutes) => Integer,
diff --git a/lib/cloud_controller/config_schemas/worker_schema.rb b/lib/cloud_controller/config_schemas/worker_schema.rb
@@ -41,6 +41,12 @@ class WorkerSchema < VCAP::Config
           },
 
           stacks_file: String,
+
+          optional(:storage_cli_config_file_buildpacks) => String,
+          optional(:storage_cli_config_file_packages) => String,
+          optional(:storage_cli_config_file_resource_pool) => String,
+          optional(:storage_cli_config_file_droplets) => String,
+
           newrelic_enabled: bool,
 
           optional(:max_migration_duration_in_minutes) => Integer,
diff --git a/lib/cloud_controller/dependency_locator.rb b/lib/cloud_controller/dependency_locator.rb
@@ -167,7 +167,8 @@ def legacy_global_app_bits_cache
 
       Blobstore::ClientProvider.provide(
         options: options,
-        directory_key: options.fetch(:resource_directory_key)
+        directory_key: options.fetch(:resource_directory_key),
+        resource_type: :resource_pool
       )
     end
 
@@ -177,7 +178,8 @@ def global_app_bits_cache
       Blobstore::ClientProvider.provide(
         options: options,
         directory_key: options.fetch(:resource_directory_key),
-        root_dir: RESOURCE_POOL_DIR
+        root_dir: RESOURCE_POOL_DIR,
+        resource_type: :resource_pool
       )
     end
 
diff --git a/lib/cloud_controller/resource_pool.rb b/lib/cloud_controller/resource_pool.rb
@@ -25,7 +25,8 @@ def initialize(config)
     @blobstore = CloudController::Blobstore::ClientProvider.provide(
       options: options,
       directory_key: options.fetch(:resource_directory_key),
-      root_dir: CloudController::DependencyLocator::RESOURCE_POOL_DIR
+      root_dir: CloudController::DependencyLocator::RESOURCE_POOL_DIR,
+      resource_type: 'resource_pool'
     )
 
     @minimum_size = options[:minimum_size] || 0 # TODO: move default into config object?
diff --git a/spec/unit/lib/cloud_controller/blobstore/client_provider_spec.rb b/spec/unit/lib/cloud_controller/blobstore/client_provider_spec.rb
@@ -129,6 +129,7 @@ module Blobstore
       context 'when storage-cli is requested' do
         let(:blobstore_type) { 'storage-cli' }
         let(:directory_key) { 'some-bucket' }
+        let(:resource_type) { 'some-resource-type' }
         let(:root_dir) { 'some-root-dir' }
         let(:storage_cli_client_mock) { class_double(CloudController::Blobstore::StorageCliClient) }
 
@@ -138,8 +139,8 @@ module Blobstore
 
         it 'provides a storage-cli client' do
           allow(StorageCliClient).to receive(:build).and_return(storage_cli_client_mock)
-          ClientProvider.provide(options:, directory_key:, root_dir:)
-          expect(StorageCliClient).to have_received(:build).with(connection_config: {}, directory_key: directory_key, root_dir: root_dir, min_size: 100, max_size: 1000)
+          ClientProvider.provide(options:, directory_key:, root_dir:, resource_type:)
+          expect(StorageCliClient).to have_received(:build).with(connection_config: {}, directory_key: directory_key, resource_type: resource_type, root_dir: root_dir, min_size: 100, max_size: 1000)
         end
 
         it 'raises an error if connection_config is not provided' 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
@@ -5,23 +5,182 @@ module CloudController
   module Blobstore
     RSpec.describe StorageCliClient do
       describe 'registry build and lookup' do
+
         it 'builds the correct client' do
-          client_from_registry = StorageCliClient.build(connection_config: { provider: 'AzureRM' }, directory_key: 'dummy-key', root_dir: 'dummy-root')
+          droplets_cfg = Tempfile.new(['droplets', '.json'])
+          droplets_cfg.write({ connection_config: { provider: 'AzureRM' } }.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_from_registry = StorageCliClient.build(connection_config: { provider: 'AzureRM' }, directory_key: 'dummy-key', root_dir: 'dummy-root', resource_type: 'droplets')
           expect(client_from_registry).to be_a(AzureStorageCliClient)
+
+          droplets_cfg.close!
         end
 
         it 'raises an error for an unregistered provider' do
           expect do
-            StorageCliClient.build(connection_config: { provider: 'UnknownProvider' }, directory_key: 'dummy-key', root_dir: 'dummy-root')
+            StorageCliClient.build(connection_config: { provider: 'UnknownProvider' }, directory_key: 'dummy-key', root_dir: 'dummy-root', resource_type: 'droplets')
           end.to raise_error(RuntimeError, 'No storage CLI client registered for provider UnknownProvider')
         end
 
         it 'raises an error if provider is missing' do
           expect do
-            StorageCliClient.build(connection_config: {}, directory_key: 'dummy-key', root_dir: 'dummy-root')
+            StorageCliClient.build(connection_config: {}, directory_key: 'dummy-key', root_dir: 'dummy-root', resource_type: 'droplets')
           end.to raise_error(RuntimeError, 'Missing connection_config[:provider]')
         end
       end
+
+      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']) }
+
+        before do
+          # Valid JSON (YAML can parse JSON)
+          [droplets_cfg, buildpacks_cfg, packages_cfg, resource_pool_cfg].each do |f|
+            f.write({ connection_config: { provider: 'AzureRM' } }.to_json)
+            f.flush
+          end
+
+          allow(VCAP::CloudController::Config).to receive(:config).and_return(config_double)
+
+          allow(config_double).to receive(:get) do |key|
+            case key
+            when :storage_cli_config_file_droplets      then droplets_cfg.path
+            when :storage_cli_config_file_buildpacks    then buildpacks_cfg.path
+            when :storage_cli_config_file_packages      then packages_cfg.path
+            when :storage_cli_config_file_resource_pool then resource_pool_cfg.path
+            else
+              nil
+            end
+          end
+
+          # Quiet logger noise in specs
+          allow(Steno).to receive(:logger).and_return(double(info: nil, error: nil))
+        end
+
+        after do
+          [droplets_cfg, buildpacks_cfg, packages_cfg, resource_pool_cfg].each(&:close!)
+        end
+
+        def build_client(rt)
+          StorageCliClient.build(
+            connection_config: { provider: 'AzureRM' },
+            directory_key: 'dir-key',
+            root_dir: 'root',
+            resource_type: rt
+          )
+        end
+
+        it 'picks droplets file for "droplets"' do
+          client = build_client('droplets')
+          expect(client.instance_variable_get(:@config_file)).to eq(droplets_cfg.path)
+        end
+
+        it 'picks droplets file for "buildpack_cache" too' do
+          client = build_client('buildpack_cache')
+          expect(client.instance_variable_get(:@config_file)).to eq(droplets_cfg.path)
+        end
+
+        it 'picks buildpacks file for "buildpacks"' do
+          client = build_client('buildpacks')
+          expect(client.instance_variable_get(:@config_file)).to eq(buildpacks_cfg.path)
+        end
+
+        it 'picks packages file for "packages"' do
+          client = build_client('packages')
+          expect(client.instance_variable_get(:@config_file)).to eq(packages_cfg.path)
+        end
+
+        it 'picks resource_pool file for "resource_pool"' do
+          client = build_client('resource_pool')
+          expect(client.instance_variable_get(:@config_file)).to eq(resource_pool_cfg.path)
+        end
+
+        it 'accepts a symbol and normalizes it' do
+          client = build_client(:droplets)
+          expect(client.instance_variable_get(:@config_file)).to eq(droplets_cfg.path)
+        end
+
+        it 'raises for unknown resource_type' do
+          expect {
+            build_client('nope')
+          }.to raise_error(CloudController::Blobstore::BlobstoreError, /Unknown resource_type: nope/)
+        end
+
+        it 'raises when file missing/unreadable' do
+          allow(config_double).to receive(:get).with(:storage_cli_config_file_packages).and_return('/no/such/file.json')
+          expect {
+            build_client('packages')
+          }.to raise_error(CloudController::Blobstore::BlobstoreError, /not found or not readable/)
+        end
+
+        it 'raises when YAML load fails' do
+          File.write(packages_cfg.path, "{ this is: [not, valid }")
+          expect {
+            build_client('packages')
+          }.to raise_error(CloudController::Blobstore::BlobstoreError, /Failed to load storage-cli config/)
+        end
+      end
+
+      describe '#exists? exit code handling' do
+        let(:config_double) { instance_double(VCAP::CloudController::Config) }
+        let(:droplets_cfg)  { Tempfile.new(['droplets', '.json']) }
+
+        before do
+          droplets_cfg.write({ connection_config: { provider: 'AzureRM' } }.to_json)
+          droplets_cfg.flush
+
+          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)
+
+          # Avoid logger noise
+          allow(Steno).to receive(:logger).and_return(double(info: nil, error: nil))
+        end
+
+        after { droplets_cfg.close! }
+
+        let(:client) do
+          StorageCliClient.build(
+            connection_config: { provider: 'AzureRM' },
+            directory_key: 'dir',
+            root_dir: 'root',
+            resource_type: 'droplets'
+          )
+        end
+
+        it 'returns true on exitstatus 0' do
+          expect(Open3).to receive(:capture3)
+                             .with(kind_of(String), '-c', droplets_cfg.path, 'exists', kind_of(String))
+                             .and_return(['', '', instance_double(Process::Status, success?: true, exitstatus: 0)])
+
+          expect(client.exists?('key')).to be true
+        end
+
+        it 'returns false on exitstatus 3' do
+          expect(Open3).to receive(:capture3)
+                             .with(kind_of(String), '-c', droplets_cfg.path, 'exists', kind_of(String))
+                             .and_return(['', '', instance_double(Process::Status, success?: false, exitstatus: 3)])
+
+          expect(client.exists?('key')).to be false
+        end
+
+        it 'raises for other non-zero exit codes' do
+          expect(Open3).to receive(:capture3)
+                             .with(kind_of(String), '-c', droplets_cfg.path, 'exists', kind_of(String))
+                             .and_return(['', 'boom', instance_double(Process::Status, success?: false, exitstatus: 2)])
+
+          expect { client.exists?('key') }.to raise_error(/storage-cli exists failed/)
+        end
+      end
+
     end
   end
 end

Original file line number	Diff line number	Diff line change
`@@ -25,7 +25,8 @@ def initialize(config)`
`25`	`25`	`@blobstore = CloudController::Blobstore::ClientProvider.provide(`
`26`	`26`	`options: options,`
`27`	`27`	`directory_key: options.fetch(:resource_directory_key),`
`28`		`- root_dir: CloudController::DependencyLocator::RESOURCE_POOL_DIR`
	`28`	`+ root_dir: CloudController::DependencyLocator::RESOURCE_POOL_DIR,`
	`29`	`+ resource_type: 'resource_pool'`
`29`	`30`	`)`
`30`	`31`
`31`	`32`	`@minimum_size = options[:minimum_size] \|\| 0 # TODO: move default into config object?`