Subject: [netconf] Adam Roach's Discuss on draft-ietf-netconf-restconf-notif-13: (with DISCUSS and COMMENT)

From: Adam Roach via Datatracker <[email-masked]>
Date: Wed, 15 May 2019 00:20:18 -0700

Adam Roach has entered the following ballot position for
draft-ietf-netconf-restconf-notif-13: Discuss

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)

Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.

The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-netconf-restconf-notif/

----------------------------------------------------------------------
DISCUSS:
----------------------------------------------------------------------

Thanks for all the work that the authors and other contributors have
put into this document. I have two comments that need to be addressed
before publication, but they should both be very easy to fix.

---------------------------------------------------------------------------

§3.3:

>  If a publisher fails to serve the RPC request for one of the reasons
>  indicated in [I-D.draft-ietf-netconf-subscribed-notifications]
>  Section 2.4.6 or [I-D.ietf-netconf-yang-push] Appendix A, this will
>  be indicated by "406" status code transported in the HTTP response.

This really isn't what 406 means. 406 means "you sent one or more of the
'Accept', 'Accept-Charset', 'Accept-Encoding', or 'Accept-Language' header
fields, and I can't generate a response that satisfies what you've asked for."

For some of the errors listed in the two cited sections, there is a reasonable
semantic mapping onto existing HTTP response codes; e.g. the
"no-such-subscription" errors could all reasonably map on to HTTP 404.  I'll
note that RFC 8040 section 7 performs exactly this kind of mapping, so the
approach seems to be consistent with the way that RESTCONF has elected to use
HTTP response codes. In fact, this document already maps from the cited errors
to error tags already, and that table maps from error-tag to
HTTP response codes, so fixing this should be the relatively straightforward
exercise of updaing the tables in this section to also include the HTTP response
code that RFC 8040 maps to the indicated error-tag. For example:

     error identity         uses error-tag             HTTP Response
     ---------------------- --------------             -------------
     dscp-unavailable       invalid-value              400
     encoding-unsupported   invalid-value              400
     filter-unsupported     invalid-value              400
     insufficient-resources resource-denied            409
     no-such-subscription   invalid-value              404
     replay-unsupported     operation-not-supported    501

     error identity              uses error-tag          HTTP Response
     ----------------------      --------------          -------------
     cant-exclude                operation-not-supported 501
     datastore-not-subscribable  invalid-value           400
     no-such-subscription-resync invalid-value           404
     on-change-unsupported       operation-not-supported 501
     on-change-sync-unsupported  operation-not-supported 501
     period-unsupported          invalid-value           400
     update-too-big              too-big                 400
     sync-too-big                too-big                 400
     unchanging-selection        operation-failed        500

However you choose to address this, if the error isn't related to any of the
four header fields I mention above, then you can't specify the use of a 406.

---------------------------------------------------------------------------

§3.4:

This section is unclear about how Server-Sent Events are to be used (in
particular, they don't say anything about event type to be used). Based on the
one example in Appendix A that shows SSE syntax, I'm assuming you probably do
not intend to use SSE "event type" fields to distinguish between events in any
way.  This would mean that you need to specify that all SSE messages are sent
with an event type of "message," which the server may omit (as it is the
default specified in the Server-Side Events specification).  This means that
clients will need to accept both:

data: {
data:   "ietf-restconf:notification" : {
data:     "eventTime": "2007-09-01T10:00:00Z",
data:     "ietf-subscribed-notifications:subscription-modified": {
data:       "id": "39",
data:       "uri": "https://example.com/restconf/subscriptions/22"
data:       "stream-xpath-filter": "/example-module:foo",
data:       "stream": {
data:          "ietf-netconf-subscribed-notifications" : "NETCONF"
data:       }
data:     }
data:   }
data: }

...and...

event: message
data: {
data:   "ietf-restconf:notification" : {
data:     "eventTime": "2007-09-01T10:00:00Z",
data:     "ietf-subscribed-notifications:subscription-modified": {
data:       "id": "39",
data:       "uri": "https://example.com/restconf/subscriptions/22"
data:       "stream-xpath-filter": "/example-module:foo",
data:       "stream": {
data:          "ietf-netconf-subscribed-notifications" : "NETCONF"
data:       }
data:     }
data:   }
data: }

It may be helpful to incorporate the SSE syntax into all of the notification
examples in Appendix A (that is, all of the examples in A.2 and A.3). I would
recommend a mix of examples with and without "event: message".

----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

General comment:

It's a bit unclear about what the normative relationship between a Server-Sent
Events connection and a subscription is intended to be. For example: if I send
an RPC command to create a subscription, and then make two different SSE
connections to the resulting URL, will they both receive events associated
with that subscription? If so, does a TLS heartbeat failure on one of them
cause the entire subscription to go away? (If not, what happens?)

If I have only one connection related to a subscription, and I close the TCP
connection, does that necessarily make the subscription go away? What if I
set up a new TCP connection right away after closing the first one? Will
that work?

What if I use RPC to set up a new subscription, and then wait a few minutes
before connecting to the subscription URL?

I don't think you need to answer all of these corner cases per se (although it
would be nice), but minimally a couple of statements that clearly spell out
the relationship between subscriptions and the connections used to deliver
related events would help implementors figure out the answers to these
questions.

---------------------------------------------------------------------------

§2:

>  The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT",
>  "SHOULD", "SHOULD NOT", "RECOMMENDED", "MAY", and "OPTIONAL" in this
>  document are to be interpreted as described in RFC 2119 [RFC2119].

Please use the boilerplate specified by RFC 8174.

---------------------------------------------------------------------------

§3.1:

>  As stated in Section 2.1 of [RFC8040], a subscriber MUST establish
>  the HTTP session over TLS [RFC5246] in order to secure the content in
>  transit.

Is the intention here to restrict clients to TLS 1.2? If not, please cite
RFC 8446 instead of RFC 5246. If so, please add text that explains why.

(This comment also applies to section 9)

---------------------------------------------------------------------------

§3.1:

>  Loss of the heartbeat MUST result in any subscription related TCP

nit: "...subcription-related..."

---

From: "Reshad Rahman (rrahman)" <[email-masked]>
Date: Fri, 17 May 2019 02:27:09 +0000

Hi,

Thanks for the review, please see inline.

On 2019-05-15, 3:20 AM, "netconf on behalf of Adam Roach via Datatracker" <[email-masked] on behalf of [email-masked]> wrote:

    

    

    

    However you choose to address this, if the error isn't related to any of the
    four header fields I mention above, then you can't specify the use of a 406.
<RR> Thank you for suggesting status codes for the different errors, much appreciated. I agree that 406 is not appropriate. Regarding 400, my understanding is that it is for invalid syntax, so for errors such as dscp-unavailable or encoding-unsupported, would 409 be more appropriate than 400?

    §3.4:

    data: {
    
    ...and...
    
    event: message
    data: {
    
    It may be helpful to incorporate the SSE syntax into all of the notification
    examples in Appendix A (that is, all of the examples in A.2 and A.3). I would
    recommend a mix of examples with and without "event: message".
<RR> Andy Bierman (one of the co-authors) mentioned that RESTCONF does not define any values for "event" and pointed us to this text from RFC8040 P72    
   A RESTCONF
   server SHOULD NOT send the "event" or "id" fields, as there are no
   meaningful values that could be used for them that would not be
   redundant to the contents of the notification itself.    

    General comment:
    
<RR> The resource created via the establish-subscription is for 1 subscription. There can only be 1 GET on the URI at the same time, this does not seem to be explicitly stated anywhere, so probably a good idea to add text to that effect.
    
    If I have only one connection related to a subscription, and I close the TCP
    connection, does that necessarily make the subscription go away? What if I
    set up a new TCP connection right away after closing the first one? Will
    that work?
<RR> The subscription will not go away when the TCP connection is closed. As mentioned at the end of 3.4, the subscription goes away on receipt of delete-subscription/kill-subscription RPCs or on loss of TLS heartbeat. If a client does a GET after closing the first GET, yes this is supposed to work.
    
    What if I use RPC to set up a new subscription, and then wait a few minutes
    before connecting to the subscription URL?
<RR> That is also supposed to work as long as the subscription wasn't terminated as per section 3.4.

    

<RR> Will do.    

    
    
    (This comment also applies to section 9)
<RR> Will update to RFC8446.

    
    
    nit: "...subcription-related..."
<RR> Will change to subscription-related.    

Regards,
Reshad.
    
    _______________________________________________
    netconf mailing list
    [email-masked]
    https://www.ietf.org/mailman/listinfo/netconf

---

From: Adam Roach <[email-masked]>
Date: Fri, 17 May 2019 15:35:35 -0500

Thanks for your quick response. I have some answers inline below (and 
have removed the part of the conversation that requires no response).

>      https://datatracker.ietf.org/doc/draft-ietf-netconf-restconf-notif/

400 is a generic "there is something wrong with your request for which 
there is no more specific error code," and it's used pretty extensively 
by the base RESTCONF specification. 409 generally means that the server 
is in a (typically temporary) state that is incompatible with the action 
indicated by the request.

Once of the things that I took pains to ensure in my suggestions is that 
the HTTP response I suggest above is one of the ones that RFC 8040 
indicates for the corresponding error-tag (even for those cases where I 
don't think RFC 8040 made a strictly accurate choice). I suspect that it 
is best if the RESTCONF notification behavior is kept in sync with the 
base RESTCONF behavior, but I'm not an expert on the whole YANG 
ecosystem, so I may be mistaken.

Ah, okay. That seems to cover the situation, although it might bear 
reiterating in this document also.

Thanks for the explanation; that all makes sense. I suggest you add 
whatever text you feel is appropriate so that other readers understand 
it as well as I do now. :)

Thanks!

/a