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

Adam Roach via Datatracker <noreply@ietf.org> Wed, 15 May 2019 07:20 UTC

Return-Path: <noreply@ietf.org>
X-Original-To: netconf@ietf.org
Delivered-To: netconf@ietfa.amsl.com
Received: from ietfa.amsl.com (localhost [IPv6:::1]) by ietfa.amsl.com (Postfix) with ESMTP id 80882120255; Wed, 15 May 2019 00:20:18 -0700 (PDT)
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit
From: Adam Roach via Datatracker <noreply@ietf.org>
To: The IESG <iesg@ietf.org>
Cc: draft-ietf-netconf-restconf-notif@ietf.org, Kent Watsen <kent+ietf@watsen.net>, netconf-chairs@ietf.org, kent+ietf@watsen.net, netconf@ietf.org
X-Test-IDTracker: no
X-IETF-IDTracker: 6.96.0
Auto-Submitted: auto-generated
Precedence: bulk
Reply-To: Adam Roach <adam@nostrum.com>
Message-ID: <155790481851.17474.3456985672733157831.idtracker@ietfa.amsl.com>
Date: Wed, 15 May 2019 00:20:18 -0700
Archived-At: <https://mailarchive.ietf.org/arch/msg/netconf/rOZDMkuAmmEwjQWpv4c_5ey80fc>
Subject: [netconf] Adam Roach's Discuss on draft-ietf-netconf-restconf-notif-13: (with DISCUSS and COMMENT)
X-BeenThere: netconf@ietf.org
X-Mailman-Version: 2.1.29
List-Id: NETCONF WG list <netconf.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/netconf>, <mailto:netconf-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/netconf/>
List-Post: <mailto:netconf@ietf.org>
List-Help: <mailto:netconf-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/netconf>, <mailto:netconf-request@ietf.org?subject=subscribe>
X-List-Received-Date: Wed, 15 May 2019 07:20:19 -0000

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:


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.



>  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.



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: }


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".


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



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

Please use the boilerplate specified by RFC 8174.



>  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)



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

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