Skip to content

Cloud Foundry do not parse SB response and do not include custom metadata #4618

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
atanas-attodorov-wq wants to merge 1 commit into from
Closed

Cloud Foundry do not parse SB response and do not include custom metadata #4618

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions app/models/services/managed_service_instance.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,16 @@ class ManagedServiceInstance < ServiceInstance

export_attributes :name, :credentials, :service_plan_guid,
:space_guid, :gateway_data, :dashboard_url, :type, :last_operation,
:tags, :maintenance_info
:tags, :maintenance_info, :broker_metadata

import_attributes :name, :service_plan_guid,
:space_guid, :gateway_data, :maintenance_info
:space_guid, :gateway_data, :maintenance_info, :broker_metadata

strip_attributes :name

plugin :after_initialize

serialize_attributes :json, :maintenance_info
serialize_attributes :json, :maintenance_info, :broker_metadata

def validation_policies
if space
Expand Down
26 changes: 24 additions & 2 deletions app/presenters/v3/service_instance_presenter.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ def hash_common
}
},
metadata: {
labels: hashified_labels(service_instance.labels),
labels: merged_labels,
annotations: hashified_annotations(service_instance.annotations)
},
links: {
Expand All @@ -77,7 +77,7 @@ def hash_common
end

def hash_additions_managed
{
hash = {
type: 'managed',
maintenance_info: maintenance_info,
upgrade_available: upgrade_available,
Expand All @@ -101,6 +101,13 @@ def hash_additions_managed
}
}
}

# Add broker metadata if present
if service_instance.broker_metadata.present?
hash[:broker_metadata] = service_instance.broker_metadata
end

hash
end

def hash_additions_user_provided
Expand Down Expand Up @@ -145,6 +152,21 @@ def last_operation

service_instance.last_operation.to_hash({})
end

def merged_labels
# Start with existing CF labels
cf_labels = hashified_labels(service_instance.labels)

# Get broker metadata labels if they exist (only for managed service instances)
broker_labels = {}
if service_instance.is_a?(ManagedServiceInstance) && service_instance.broker_metadata.present?
broker_metadata_labels = service_instance.broker_metadata.dig('labels')
broker_labels = broker_metadata_labels if broker_metadata_labels.is_a?(Hash)
end

# Merge broker labels into CF labels (broker labels take precedence for conflicts)
cf_labels.merge(broker_labels)
Copy link
Copy Markdown
Member

@Benjamintf1 Benjamintf1 Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure merging the broker labels in with the user defined labels is the correct solution here.

While, I think, without passing the broker attributes to the user it remains terribly useful, i think conflating the osabi labels with user labels because they are specified with the same word might be the wrong choice.

Copy link
Copy Markdown
Contributor Author

@atanas-attodorov-wq atanas-attodorov-wq Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree . Can you have a look at this discussion?
#4633
In the project i am working ,this will not be needed anymore. I am closing this PR. If someone say it is feasible , i can re-open the PR and contribute.

end
end
end
end
Expand Down
7 changes: 7 additions & 0 deletions db/migrations/20251023214733_add_broker_metadata_to_service_instances.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
Sequel.migration do
change do
alter_table :service_instances do
add_column :broker_metadata, String, text: true
end
end
end
4 changes: 3 additions & 1 deletion lib/services/service_brokers/v2/client.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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_metadata: parsed_response['metadata']
},
last_operation: {
type: 'create',
Expand Down Expand Up @@ -203,6 +204,7 @@ def update(instance, plan, accepts_incomplete: false, arbitrary_parameters: nil,
}

attributes[:dashboard_url] = dashboard_url if dashboard_url
attributes[:broker_metadata] = parsed_response['metadata'] if parsed_response['metadata']

if state == 'in progress'
proposed_changes = { service_plan_guid: plan.guid }
Expand Down
22 changes: 22 additions & 0 deletions lib/services/service_brokers/v2/response_parser.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,17 @@ def provision_response_schema
'operation' => {
'type' => 'string',
'maxLength' => 10_000
},
'metadata' => {
'type' => 'object',
'properties' => {
'labels' => {
'type' => 'object'
},
'attributes' => {
'type' => 'object'
}
}
}
}
}]
Expand Down Expand Up @@ -292,6 +303,17 @@ def update_response_schema
'operation' => {
'type' => 'string',
'maxLength' => 10_000
},
'metadata' => {
'type' => 'object',
'properties' => {
'labels' => {
'type' => 'object'
},
'attributes' => {
'type' => 'object'
}
}
}
}
}]
Expand Down
Loading