diff --git a/app/actions/v3/service_instance_update_managed.rb b/app/actions/v3/service_instance_update_managed.rb index 378b803568e..ae344bf010f 100644 --- a/app/actions/v3/service_instance_update_managed.rb +++ b/app/actions/v3/service_instance_update_managed.rb @@ -159,6 +159,7 @@ def complete_instance_and_save(instance, broker_response) u[:maintenance_info] = maintenance_info if maintenance_info_updated? end updates[:dashboard_url] = broker_response[:dashboard_url] if broker_response.key?(:dashboard_url) + updates[:broker_provided_metadata] = broker_response[:broker_provided_metadata] if broker_response.key?(:broker_provided_metadata) ManagedServiceInstance.db.transaction do service_instance.save_with_new_operation( @@ -174,6 +175,7 @@ def complete_instance_and_save(instance, broker_response) def save_incomplete_instance(instance, broker_response) attributes_to_update = {} attributes_to_update[:dashboard_url] = broker_response[:dashboard_url] if broker_response.key?(:dashboard_url) + attributes_to_update[:broker_provided_metadata] = broker_response[:broker_provided_metadata] if broker_response.key?(:broker_provided_metadata) instance.save_with_new_operation( attributes_to_update, diff --git a/app/models/services/managed_service_instance.rb b/app/models/services/managed_service_instance.rb index 4a01fa11998..692c7a08504 100644 --- a/app/models/services/managed_service_instance.rb +++ b/app/models/services/managed_service_instance.rb @@ -21,7 +21,7 @@ class ManagedServiceInstance < ServiceInstance plugin :after_initialize - serialize_attributes :json, :maintenance_info + serialize_attributes :json, :maintenance_info, :broker_provided_metadata def validation_policies if space diff --git a/app/presenters/v3/service_instance_presenter.rb b/app/presenters/v3/service_instance_presenter.rb index 0e561bb3046..66707981f77 100644 --- a/app/presenters/v3/service_instance_presenter.rb +++ b/app/presenters/v3/service_instance_presenter.rb @@ -77,7 +77,7 @@ def hash_common end def hash_additions_managed - { + base_hash = { type: 'managed', maintenance_info: maintenance_info, upgrade_available: upgrade_available, @@ -101,6 +101,10 @@ def hash_additions_managed } } } + + base_hash[:broker_provided_metadata] = service_instance.broker_provided_metadata if service_instance.broker_provided_metadata.present? + + base_hash end def hash_additions_user_provided diff --git a/db/migrations/20251121174647_add_broker_provided_metadata_to_service_instances.rb b/db/migrations/20251121174647_add_broker_provided_metadata_to_service_instances.rb new file mode 100644 index 00000000000..863ea75e134 --- /dev/null +++ b/db/migrations/20251121174647_add_broker_provided_metadata_to_service_instances.rb @@ -0,0 +1,7 @@ +Sequel.migration do + change do + # rubocop:disable Migration/IncludeStringSize + add_column :service_instances, :broker_provided_metadata, String, size: 4096, text: true, null: true + # rubocop:enable Migration/IncludeStringSize + end +end diff --git a/docs/v3/source/includes/api_resources/_service_instances.erb b/docs/v3/source/includes/api_resources/_service_instances.erb index dbbe5116f4d..d2e8f4cdc29 100644 --- a/docs/v3/source/includes/api_resources/_service_instances.erb +++ b/docs/v3/source/includes/api_resources/_service_instances.erb @@ -43,6 +43,14 @@ }, "upgrade_available": false, "dashboard_url": "https://service-broker.example.org/dashboard", + "broker_provided_metadata": { + "labels": { + "service_engine_version": "16.6" + }, + "attributes": { + "max_connections": "100" + } + }, "last_operation": { "type": "create", "state": "succeeded", @@ -152,6 +160,14 @@ }, "upgrade_available": false, "dashboard_url": "https://service-broker.example.org/dashboard", + "broker_provided_metadata": { + "labels": { + "service_engine_version": "16.6" + }, + "attributes": { + "max_connections": "100" + } + }, "last_operation": { "type": <%= metadata.fetch(:operation, "create").to_json %>, "state": "succeeded", diff --git a/docs/v3/source/includes/resources/service_instances/_object.md.erb b/docs/v3/source/includes/resources/service_instances/_object.md.erb index b188c69c3bb..e5e2074313c 100644 --- a/docs/v3/source/includes/resources/service_instances/_object.md.erb +++ b/docs/v3/source/includes/resources/service_instances/_object.md.erb @@ -27,6 +27,7 @@ Name | Type | Description **maintenance_info** | _[maintenance_info object](#the-maintenance-info-object-for-service-instances)_ | Information about the version of this service instance; only shown when type is `managed` **upgrade_available** | _bool_ | Whether or not an upgrade of this service instance is available on the current Service Plan; details are available in the maintenance_info object; Only shown when type is `managed` **dashboard_url** | _string_ | The URL to the service instance dashboard (or null if there is none); only shown when type is `managed` +**broker_provided_metadata** | _[broker provided metadata object](#the-broker-provided-metadata-object-for-service-instances)_ | Metadata provided by the service broker about this service instance; only shown when type is `managed` **last_operation** | _[last operation object](#the-last-operation-object-for-service-instances)_ | The last operation of this service instance **relationships.service_plan** | [_to-one relationship_](#to-one-relationships) | The service plan the service instance relates to; only shown when type is `managed` **relationships.space** | [_to-one relationship_](#to-one-relationships) | The space the service instance is contained in @@ -43,6 +44,14 @@ Name | Type | Description **description** | _string_ | A textual explanation associated with this version +#### The broker provided metadata object for service instances + +Name | Type | Description +---- | ---- | ----------- +**labels** | _object_ | Broker-specified key-value pairs specifying attributes of Service Instances that do not directly imply behavior changes +**attributes** | _object_ | Broker-specific key-value pairs generated by the Broker that MAY imply behavior changes by the Platform + + #### The last operation object for service instances Name | Type | Description diff --git a/lib/services/service_brokers/v2/client.rb b/lib/services/service_brokers/v2/client.rb index d22433686ae..e80cf656810 100644 --- a/lib/services/service_brokers/v2/client.rb +++ b/lib/services/service_brokers/v2/client.rb @@ -56,7 +56,8 @@ def provision(instance, arbitrary_parameters: {}, accepts_incomplete: false, mai return_values = { instance: { credentials: {}, - dashboard_url: parsed_response['dashboard_url'] + dashboard_url: parsed_response['dashboard_url'], + broker_provided_metadata: extract_broker_provided_metadata(parsed_response) }, last_operation: { type: 'create', @@ -204,6 +205,9 @@ def update(instance, plan, accepts_incomplete: false, arbitrary_parameters: nil, attributes[:dashboard_url] = dashboard_url if dashboard_url + # Add broker_provided_metadata extraction + attributes[:broker_provided_metadata] = extract_broker_provided_metadata(parsed_response) + if state == 'in progress' proposed_changes = { service_plan_guid: plan.guid } proposed_changes[:maintenance_info] = maintenance_info if maintenance_info @@ -417,5 +421,11 @@ def hashified_public_annotations(annotations) end hashified_annotations(public_annotations) end + + def extract_broker_provided_metadata(parsed_response) + return nil unless parsed_response['metadata'].present? && parsed_response['metadata'].is_a?(Hash) + + parsed_response['metadata'] + end end end diff --git a/lib/services/service_brokers/v2/response_parser.rb b/lib/services/service_brokers/v2/response_parser.rb index 759a3a3d8df..e9d89dabbaa 100644 --- a/lib/services/service_brokers/v2/response_parser.rb +++ b/lib/services/service_brokers/v2/response_parser.rb @@ -263,6 +263,17 @@ def provision_response_schema 'operation' => { 'type' => 'string', 'maxLength' => 10_000 + }, + 'metadata' => { + 'type' => 'object', + 'properties' => { + 'labels' => { + 'type' => 'object' + }, + 'attributes' => { + 'type' => 'object' + } + } } } }] @@ -292,6 +303,17 @@ def update_response_schema 'operation' => { 'type' => 'string', 'maxLength' => 10_000 + }, + 'metadata' => { + 'type' => 'object', + 'properties' => { + 'labels' => { + 'type' => 'object' + }, + 'attributes' => { + 'type' => 'object' + } + } } } }] @@ -313,6 +335,17 @@ def fetch_service_instance_response_schema }, 'parameters' => { 'type' => 'object' + }, + 'metadata' => { + 'type' => 'object', + 'properties' => { + 'labels' => { + 'type' => 'object' + }, + 'attributes' => { + 'type' => 'object' + } + } } } }] diff --git a/spec/migrations/20251121174647_add_broker_provided_metadata_to_service_instances_spec.rb b/spec/migrations/20251121174647_add_broker_provided_metadata_to_service_instances_spec.rb new file mode 100644 index 00000000000..eb0797a4559 --- /dev/null +++ b/spec/migrations/20251121174647_add_broker_provided_metadata_to_service_instances_spec.rb @@ -0,0 +1,68 @@ +# Migration test for adding broker_provided_metadata column to service_instances table +# +# This test verifies that the migration correctly adds the broker_provided_metadata +# column to the service_instances table with the correct properties (text type, nullable). + +require 'spec_helper' +require 'migrations/helpers/migration_shared_context' + +RSpec.describe 'migration to add broker_provided_metadata column to service_instances table', isolation: :truncation, type: :migration do + include_context 'migration' do + let(:migration_filename) { '20251121174647_add_broker_provided_metadata_to_service_instances.rb' } + end + + describe 'service_instances table' do + subject(:run_migration) { Sequel::Migrator.run(db, migrations_path, target: current_migration_index, allow_missing_migration_files: true) } + + let(:space) { VCAP::CloudController::Space.make } + + it 'adds a column `broker_provided_metadata`' do + expect(db[:service_instances].columns).not_to include(:broker_provided_metadata) + run_migration + expect(db[:service_instances].columns).to include(:broker_provided_metadata) + end + + it 'allows null values for broker_provided_metadata' do + run_migration + # Insert a service instance without broker_provided_metadata + db[:service_instances].insert( + guid: 'test-service-instance-guid', + name: 'test-instance', + space_id: space.id, + broker_provided_metadata: nil + ) + # Verify the insert succeeded and the column is null + instance = db[:service_instances].first(guid: 'test-service-instance-guid') + expect(instance[:broker_provided_metadata]).to be_nil + end + + it 'accepts text values for broker_provided_metadata' do + run_migration + # Insert a service instance with broker_provided_metadata + metadata_json = '{"labels": {"version": "1.0"}, "attributes": {"engine": "postgresql"}}' + db[:service_instances].insert( + guid: 'test-service-instance-with-metadata', + name: 'test-instance-with-metadata', + space_id: space.id, + broker_provided_metadata: metadata_json + ) + # Verify the metadata was stored correctly + instance = db[:service_instances].first(guid: 'test-service-instance-with-metadata') + expect(instance[:broker_provided_metadata]).to eq(metadata_json) + end + + it 'preserves existing service instances after migration' do + # Insert a service instance before migration + db[:service_instances].insert( + guid: 'existing-service-instance-guid', + name: 'existing-instance', + space_id: space.id + ) + run_migration + # Verify the existing instance still exists and has null metadata + instance = db[:service_instances].first(guid: 'existing-service-instance-guid') + expect(instance).not_to be_nil + expect(instance[:broker_provided_metadata]).to be_nil + end + end +end diff --git a/spec/unit/actions/v3/service_instance_update_managed_spec.rb b/spec/unit/actions/v3/service_instance_update_managed_spec.rb index 1c5deaa7417..87fd7429a89 100644 --- a/spec/unit/actions/v3/service_instance_update_managed_spec.rb +++ b/spec/unit/actions/v3/service_instance_update_managed_spec.rb @@ -939,6 +939,48 @@ module V3 { prefix: nil, key_name: 'release', value: 'stable' }) end + context 'when broker returns broker_provided_metadata' do + let(:broker_metadata) do + { + 'labels' => { + 'service_engine_version' => 'postgresql 16.6' + }, + 'attributes' => { + 'attr_key' => 'attr_value' + } + } + end + + let(:update_response) do + { + dashboard_url: updated_dashboard_url, + broker_provided_metadata: broker_metadata, + last_operation: { + type: 'update', + state: 'succeeded' + } + } + end + + it 'saves the broker_provided_metadata to the instance' do + action.update(accepts_incomplete: true) + + instance = original_instance.reload + expect(instance.broker_provided_metadata).to eq(broker_metadata) + end + end + + context 'when broker does not return broker_provided_metadata' do + it 'does not modify existing broker_provided_metadata' do + original_instance.update(broker_provided_metadata: { 'old' => 'metadata' }) + + action.update(accepts_incomplete: true) + + instance = original_instance.reload + expect(instance.broker_provided_metadata).to eq({ 'old' => 'metadata' }) + end + end + it 'logs an audit event' do action.update(accepts_incomplete: true) @@ -1219,6 +1261,36 @@ module V3 ) end + context 'when broker returns broker_provided_metadata during async update' do + let(:broker_metadata) do + { + 'labels' => { + 'version' => '17.2' + } + } + end + + let(:update_response) do + { + dashboard_url: updated_dashboard_url, + broker_provided_metadata: broker_metadata, + last_operation: { + type: 'update', + state: 'in progress', + description: 'some description', + broker_provided_operation: broker_provided_operation + } + } + end + + it 'saves the broker_provided_metadata to the instance' do + action.update(accepts_incomplete: true) + + instance = original_instance.reload + expect(instance.broker_provided_metadata).to eq(broker_metadata) + end + end + context 'when update does not return dashboard_url' do let(:update_response) do { diff --git a/spec/unit/lib/services/service_brokers/v2/client_spec.rb b/spec/unit/lib/services/service_brokers/v2/client_spec.rb index 2f2122b98ee..209834eb7f0 100644 --- a/spec/unit/lib/services/service_brokers/v2/client_spec.rb +++ b/spec/unit/lib/services/service_brokers/v2/client_spec.rb @@ -259,6 +259,76 @@ module VCAP::Services::ServiceBrokers::V2 end end + describe 'broker_provided_metadata extraction' do + context 'when broker returns metadata in the response' do + let(:response_data) do + { + 'dashboard_url' => 'http://example-dashboard.com/9189kdfsk0vfnku', + 'metadata' => { + 'labels' => { + 'service_engine_version' => 'postgresql 16.6' + }, + 'attributes' => { + 'engine' => 'postgresql' + } + } + } + end + + it 'returns the broker_provided_metadata' do + attributes = client.provision(instance) + expect(attributes[:instance][:broker_provided_metadata]).to eq(response_data['metadata']) + end + end + + context 'when broker returns metadata with only labels' do + let(:response_data) do + { + 'metadata' => { + 'labels' => { + 'service_engine_version' => 'postgresql 16.6' + } + } + } + end + + it 'returns the broker_provided_metadata with only labels' do + attributes = client.provision(instance) + expect(attributes[:instance][:broker_provided_metadata]).to eq(response_data['metadata']) + end + end + + context 'when broker returns metadata with only attributes' do + let(:response_data) do + { + 'metadata' => { + 'attributes' => { + 'engine' => 'postgresql' + } + } + } + end + + it 'returns the broker_provided_metadata with only attributes' do + attributes = client.provision(instance) + expect(attributes[:instance][:broker_provided_metadata]).to eq(response_data['metadata']) + end + end + + context 'when broker does not return metadata' do + let(:response_data) do + { + 'dashboard_url' => 'http://example-dashboard.com/9189kdfsk0vfnku' + } + end + + it 'returns nil for broker_provided_metadata' do + attributes = client.provision(instance) + expect(attributes[:instance][:broker_provided_metadata]).to be_nil + end + end + end + it 'passes arbitrary params in the broker request' do arbitrary_parameters = { 'some_param' => 'some-value' @@ -966,6 +1036,61 @@ module VCAP::Services::ServiceBrokers::V2 end end + context 'when the broker returns broker_provided_metadata' do + context 'and the response is a 202' do + let(:code) { 202 } + let(:message) { 'Accepted' } + + let(:response_data) do + { + 'metadata' => { + 'labels' => { + 'service_engine_version' => 'postgresql 16.6' + } + } + } + end + + it 'returns the broker_provided_metadata from the broker response' do + client = Client.new(client_attrs) + attributes, = client.update(instance, new_plan, accepts_incomplete: true) + expect(attributes[:broker_provided_metadata]).to eq(response_data['metadata']) + end + end + + context 'when metadata is nil' do + let(:code) { 200 } + + let(:response_data) do + { + 'metadata' => nil + } + end + + it 'returns nil for broker_provided_metadata' do + client = Client.new(client_attrs) + attributes, = client.update(instance, new_plan, accepts_incomplete: true) + expect(attributes[:broker_provided_metadata]).to be_nil + end + end + + context 'when metadata is not present' do + let(:code) { 200 } + + let(:response_data) do + { + 'dashboard_url' => 'http://foo.com' + } + end + + it 'returns nil for broker_provided_metadata' do + client = Client.new(client_attrs) + attributes, = client.update(instance, new_plan, accepts_incomplete: true) + expect(attributes[:broker_provided_metadata]).to be_nil + end + end + end + describe 'error handling' do let(:response_parser) { instance_double(ResponseParser) } diff --git a/spec/unit/presenters/v3/service_instance_presenter_spec.rb b/spec/unit/presenters/v3/service_instance_presenter_spec.rb index ada59a3886b..cf91f9d5f41 100644 --- a/spec/unit/presenters/v3/service_instance_presenter_spec.rb +++ b/spec/unit/presenters/v3/service_instance_presenter_spec.rb @@ -165,6 +165,70 @@ module VCAP::CloudController::Presenters::V3 end end end + + describe 'broker_provided_metadata' do + context 'when broker_provided_metadata is present' do + let(:broker_metadata) do + { + 'labels' => { + 'service_engine_version' => 'postgresql 16.6' + }, + 'attributes' => { + 'attr_key' => 'attr_value' + } + } + end + + let(:service_instance) do + VCAP::CloudController::ManagedServiceInstance.make( + service_plan: plan, + name: 'my-db', + tags: %w[tag1 tag2], + maintenance_info: maintenance_info, + dashboard_url: 'https://my-fantistic-service.com', + broker_provided_metadata: broker_metadata + ) + end + + it 'includes broker_provided_metadata in the response' do + expect(result[:broker_provided_metadata]).to eq(broker_metadata.deep_symbolize_keys) + end + end + + context 'when broker_provided_metadata is nil' do + let(:service_instance) do + VCAP::CloudController::ManagedServiceInstance.make( + service_plan: plan, + name: 'my-db-no-metadata', + tags: %w[tag1 tag2], + maintenance_info: maintenance_info, + dashboard_url: 'https://my-fantistic-service.com', + broker_provided_metadata: nil + ) + end + + it 'does not include broker_provided_metadata in the response' do + expect(result).not_to have_key(:broker_provided_metadata) + end + end + + context 'when broker_provided_metadata is an empty hash' do + let(:service_instance) do + VCAP::CloudController::ManagedServiceInstance.make( + service_plan: plan, + name: 'my-db-empty-metadata', + tags: %w[tag1 tag2], + maintenance_info: maintenance_info, + dashboard_url: 'https://my-fantistic-service.com', + broker_provided_metadata: {} + ) + end + + it 'does not include broker_provided_metadata in the response' do + expect(result).not_to have_key(:broker_provided_metadata) + end + end + end end context 'user-provided service instance' do