added POST, PUT, DELETE routes to modify Service objects through API

[jb-adams]

PR for #65

• Added POST operation to /services
• Added PUT and DELETE operations to /services/{serviceId}
• a common requestBody is submitted to POST or PUT operations to create or update a service, respectively

Some questions for the group:

• should we also include a PATCH operation for simple modification of a few properties?
• should we add additional auth scopes to differentiate between read and write of objects?

@mcupak @jrambla

[jrambla]

How has the following been resolved?: should we add additional auth scopes to differentiate between read and write of objects?

[mcupak]

LGTM. My only requirement is that we treat these operations as optional somehow, since there will definitely be read-only registries (maybe most of them?). Let's make that explicit in the descriptions and couple these modifications with a section in the README explaining that? Or is there a better OAS way to mark this?

With optionality, I'd be supportive of PATCH too.

What would the auth scopes get us?

We should also bump the spec version, I think this is the first PR since the last release.

[mcupak]

A couple of clarifying questions after the discussion on the call today:

As for scopes, would specifying them in any way restrict us? For example, would having a scope for the list endpoint prevent us from having it accessible publicly to anonymous users? Would prescribing scopes in any way interfere with AAI and Passports?

If the answer to these questions is no, I'm +1 for adding scopes.

As for PATCH, I'm not sure where we landed. What I've heard from Jordi is that ELIXIR essentially don't have use case for PATCH, unless we make it more prescriptive, and would use PUT. I guess we have 2 options here:

1. Take a look at existing adopters (ELIXIR, DNAstack, GA4GH). See if they would want to use PATCH. If not, we can just prescribe what's needed, i.e. PUT.
2. Assume each operation is optional, but if people choose to implement them, we want them to be consistent. I.e. add PATCH the way it's usually done and people may or may not implement it.
3. Add a more prescriptive PATCH with versioning support, as per ELIXIR's ask.

I think this order corresponds to my preference. WDYT?

[jb-adams]

Auth
Agree with Miro on Auth, multiple scopes should only be used if it works well with AAI + Passports. Regarding the list /services endpoint, I'd say the endpoint should be publicly accessible, but the list of services in the response may differ whether the client is unauthorized (can only retrieve public services) or authorized (retrieves public and private services they're authorized to view). Though I'm not sure if Auth scopes in OpenAPI is the correct way to capture this behaviour.

This is the behaviour that the GA4GH registry is aiming for with /services, but we should also clarify if all adopters are thinking the same thing.

Patch
The GA4GH registry likely won't support PATCH as we can get by with PUT, going with (1) makes sense to me so we ensure it'll be used.

Agree the writable endpoints should all be optional. Should we include some mechanism to inform the client what write operations are supported, if any? this could be done via /service-info, eg:

"supportedWriteEndpoints" : {
    "post": true,
    "put": true,
    "delete": true,
    "patch": false
}

Do we also want to delineate scenarios in which a write operation may be blocked? (e.g. what Jordi mentioned about unexpected service types or versions, data race, etc). Do these apply to just PATCH or all write routes?

[mcupak]

As for auth, we definitely can't mandate that the /services endpoint is public in all cases. There are use cases, e.g. FASP, where you'd use the registry for internal service discovery in a completely secured platform.

On our side we would get by without PATCH as well.

As for writable endpoints, I don't think we need to disclose them under service-info. I see service info as general, likely public, service metadata, whereas what operations are allowed could depend on context (e.g. only allowed for specific users pending auth).

In terms of blocking/validation, that seems like an implementation-specific detail. How would that affect the spec? Another response code? If it's just a response code and you have one in mind, I'm supportive of adding it.

[jb-adams]

Agreed about private endpoints and not broadcasting write endpoints via /service-info. With PATCH, I'll hold off on putting this into the spec until we get confirmation of someone wanting it. Lastly, the blocking/validation seems to me like it falls into the realm of 400 Bad Request, though maybe 409 Conflict would work here? I'm good to keep the current list of response codes unless someone requests another.

[mcupak]

Sounds like a good plan to me, thanks! I also prefer converging on a small number of generic errors, so I think 400 is fine.

[jb-adams]

Pushed another commit based on discussions here and Aug 5th Discovery Networks meeting. There weren't many changes to make, as we decided to:

• keep bearer auth as is
• keep the same error response codes, allowing implementers add more based on implementation-specific behaviour
• not adding PATCH endpoint

The only modification was an additional write endpoints section in the description. It simply states the new addition, and that these endpoints are not mandatory. I can move this info to another location if that's best

[jb-adams]

At the Discovery Networks meeting (Nov. 25, 2020), we decided to only merge this PR once the feature is implemented in the Service Registry Reference Implementation.

For reference, this feature is already implemented in the GA4GH Registry