AMWA-TV / is-06

AMWA IS-06 NMOS Network Control Specification (Deprecated)
https://specs.amwa.tv/is-06
Apache License 2.0
14 stars 10 forks source link

Clarify that DELETE /endpoints/{endpointId} also deletes associated Network Flows (and Links) #22

Closed garethsb closed 4 years ago

garethsb commented 5 years ago

We assume that issuing a DELETE /endpoints/{endpointId} also deletes any associated Network Flows. Is that correct? If so, a sentence in the RAML might be a good idea.

garethsb commented 5 years ago

Or alternatively, a 403 Forbidden response could be added to DELETE to indicate the Endpoint cannot be removed while it has associated Network Flows.

vsachin33 commented 5 years ago

Implicitly deleting n/w flows w/ deleting endpoints would be risky. 403/Forbidden is generally relevant to user rights.

HTTP 409 may be more  appropriate. The 409 (Conflict) status code indicates that the request could not be completed due to a conflict with the current state of the target resource ..Sachin

garethsb commented 5 years ago

Agreed, there are pros and cons, and precedents in NMOS APIS, for the options:

  1. Network Flows are implicitly deleted if Endpoint is deleted
    • Sub-resources are implicitly deleted in IS-04 Registration API
  2. Network Flows must be explicitly deleted before Endpoint can be deleted
    • 403 Forbidden is used in IS-04 Query API when trying to DELETE a non-persistent /subscription resource
    • 409 Conflict is used in IS-04 Registration API and Query API e.g. when trying to DELETE a resource using the wrong API version

If we go for the option to require explicit delete (409 Conflict), one issue is that when an Endpoint that is the source of many Network Flows drops off the network, this will require many HTTP DELETE requests on the Network Control API.

Is that acceptable?

andrewbonney commented 5 years ago

In general we've tried to adopt a pragmatic approach so far. The existing APIs try to follow REST principles, but they don't do it to the letter for reasons such as the sub-optimal DELETE case described above.

An implicit DELETE does sound appealing here. If the main concerns around adopting this approach are with regard to doing something accidentally or preventing malicious users, these problems may be better solved via use with IS-10 or by implementing sufficient safety nets and prompts in user interfaces.

garethsb commented 5 years ago

I note this issue also applies to the Network-Link resources, which are apparently created automatically by the network controller at the point the Endpoint is registered by a client (they also reference the Endpoint by its unique id).

NEOAdvancedTechnology commented 4 years ago

Gareth & Sachin to come up with agreed wording on error codes for trying to delete resources that are related to other reserved resources.

garethsb commented 4 years ago

In general we've tried to adopt a pragmatic approach so far. The existing APIs try to follow REST principles, but they don't do it to the letter for reasons such as the sub-optimal DELETE case described above.

An implicit DELETE does sound appealing here. If the main concerns around adopting this approach are with regard to doing something accidentally or preventing malicious users, these problems may be better solved via use with IS-10 or by implementing sufficient safety nets and prompts in user interfaces.

@andrewbonney, FYI, during today's call, @vsachin33 made the point that deleting Network Flows (and in the future, NAT Policies) happens at the Network Device (switch), and if this is being performed implicitly in response to a DELETE operation on an Endpoint, it is challenging to make the whole operation transactional, all-or-nothing, and leave the API data model self-consistent and consistent with reality on the Network Devices. Hence his preference for requiring the BC to make atomic operations of deleting flows, etc. before endpoints.

Does that make sense?

garethsb commented 4 years ago

I note this issue also applies to the Network-Link resources, which are apparently created automatically by the network controller at the point the Endpoint is registered by a client (they also reference the Endpoint by its unique id).

Huh, we're actually therefore going to have a mix of implicit and explicit deletion, though I think it still makes sense... DELETE on an Endpoint with any associated Network Flows (or Network Address Translation policies in v1.1) will result in a 409 Conflict response. On the other hand, DELETE on an Endpoint with no associated Network Flows (or NAT policies) will succeed and implicitly delete the associated Network Link resource.

With the 409 Conflict response we need to determine if there's a way all the conflicting resources could be indicated (we use a single resource Location header in this situation in IS-04 Registration API).

garethsb commented 4 years ago

With the 409 Conflict response we need to determine if there's a way all the conflicting resources could be indicated (we use a single resource Location header in this situation in IS-04 Registration API).

Answering myself... two choices (if we don't invent a new list response header instead of using Location):

  1. Discover all of the Network Flows that refer to the Endpoint, via a query to /network-flows?sender_endpoint_id={endpointId} and/or /network-flows?receiver_endpoint_ids={endpointId} depending on the Endpoint role. DELETE those flows first, then DELETE the endpoint.

    :+1: Efficient :-1: Relies on support for query parameters in the Network Control API

  2. Attempt to DELETE the endpoint. Get a 409 Conflict with a Location header indicating one conflicting Network Flow. DELETE that flow. Repeat until you stop getting 409 Conflict.

    :+1: Doesn't rely on query params being supported :-1: Inefficient

garethsb commented 4 years ago

Another possibility:

  1. Support DELETE with a query string, so you can make a smaller number of requests: DELETE /network-flows?sender_endpoint_id={endpointId} DELETE /network-flows?receiver_endpoint_ids={endpointId} DELETE /endpoint/{endpointId}
garethsb commented 4 years ago

We should bear in mind that if query parameters aren't supported on the Network Control API, and we require explicit deletion of the flows, the BC has to get the entire list of flows to determine which ones to delete.

garethsb commented 4 years ago

This has been resolved sufficiently for a v1.0.x with the addition of the 409 response to DELETE. However, the other possibilities are worth considering in v1.1-dev.