openservicebrokerapi / servicebroker

Open Service Broker API Specification
https://openservicebrokerapi.org/
Apache License 2.0
1.19k stars 436 forks source link

Add attributes field to metadata for broker generated information #721

Closed rsampaio closed 3 years ago

rsampaio commented 4 years ago

What is the problem this PR solves? This PR adds the attributes field to the response of fetching a service instance, more details can be found in this issue: https://github.com/openservicebrokerapi/servicebroker/issues/720

Checklist:

Samze commented 3 years ago

We chatted about this on the call today.

General:

Specific:

It would be good to chat through these on the next call.

rsampaio commented 3 years ago

@Samze Thanks for the feedback!

I agree that the term context is overused and even in the spec it has many meanings and the main motivation for adding this is field is to be able to use clients generated from the openapi.yaml to parse this information from brokers as an object.

The spec already allows brokers and platforms to send arbitrary fields to eachother off-spec. So this change gives us a context container to place them inside to avoid future naming collisions?

Name collision is a good point and indeed requires a different name than context to avoid that, will push a different name suggestion in a moment.

Do you imagine that we document the fields inside this context like we do in profile.md? (There are a lot more brokers than platforms so this may be difficult unless we choose some specific fields platforms know about)

Yes, that is a great idea, with a different name like attributes we can definitely identify common fields that platforms are interested and document the use, it is a potential tool to create a platform spec.

Does your use case for this (setting up network connectivity) be solved by another mechanism? e.g. The networking proposal by @fmui.

Possibly and I believe it is still something we should consider for more complex network setups like VPNs and VPC peering and I believe the field can still be useful for this proposal and still enable information passing.

I propose a rename to attributes since it describes better the intent and avoid collision with user-provided fields and will push changes to this PR shortly to rename the field and also include this field when provisioning is made.