Subject: [Netconf] Comments on draft-ietf-netconf-restconf-13

From: [email-masked] (Dale R. Worley)
Date: Wed, 15 Jun 2016 21:12:30 -0400

This is a bunch of comments on draft-ietf-netconf-restconf-13.  Some of
these comments might be redundant or due to misunderstanding because I'm
coming into this process late.  But as I'm likely to be the Gen-ART
reviewer, it's best to take care of these now.

I think almost all of these comments are at the nits level.  The two
comments that seem to have technical content are:

- Leaf-list nodes may now have duplicated values, so identifying
  leaf-list instances by value may not always work.  But perhaps that
  ambiguity never arises in practice due to restrictions on the CRUD
  operations.

- Any identifier that is materialized as an XML element or attribute
  can't start with "xml" (with any case).  This includes the data
  nodes that are materialized in XML and extension keywords.  But this
  restriction isn't specified in draft-ietf-netmod-rfc6020.

1.1.4

   o  event stream resource: This resource represents an SSE (Server-
      Sent Events) event stream.  The content consists of text using the
      media type "text/event-stream", as defined by the HTML5
      [W3C.REC-html5-20141028] specification.  Each event represents one
      <notification> message generated by the server.  It contains a
      conceptual system or data-model specific event that is delivered
      within an event notification stream.  Also called a "stream
      resource".

Is this correct?  I can't find "event-stream" within
<http://www.w3.org/TR/2014/REC-html5-20141028>.  It appears that the
latest Server-Sent Events specification is
<http://www.w3.org/TR/2015/REC-eventsource-20150203/>.

3.5.1

   If a data node in the path expression is a YANG leaf-list node, then
   the leaf-list value MUST be encoded according to the following rules:

   o  The instance-identifier for the leaf-list MUST be encoded using
      one path segment [RFC3986].

   o  The path segment is constructed by having the leaf-list name,
      followed by an "=" character, followed by the leaf-list value.
      (e.g., /restconf/data/top-leaflist=fred).

In item 2, does "leaf-list value" mean the encoded instance-identifier
of item 1?  But the example below does not show a full
instance-identifier as one path segment, but only the leaf-list
element name and value as the last path segment:

    /restconf/data/example-top:top/Y=instance-value

Perhaps item 1 means "the leaf-list name MUST be encoded ...".

   o  Resource URI values returned in Location headers for data
      resources MUST identify the module name, even if there are no
      conflicting local names when the resource is created.  This

Is this restricted to list nodes, as its position indicates?  Or is it
generally applicable, and should be pulled out to a top-level
paragraph?

"identify the module name" is vague.  I think this means that each
element name must be qualified with its module name.

   For the above YANG definition, a target resource URI for leaf-list
   "Y" would be encoded as follows:

    /restconf/data/example-top:top/Y=instance-value

Non-unique values are now allowed for leaf-lists, so this syntax may
be ambiguous.  (Does this apply to instance-identifiers in modules?)
Perhaps the 'insert' and 'point' query parameters are used to
disambiguate non-unique leaf-list values?

It seems that any netconf data node that is materialized as an XML
element can't start with "xml" (case-insensitive).  But this is not
noted in draft-ietf-netmod-rfc6020 as a restriction.  The existence of
YIN requires that extension keywords also may not start with "xml"
(case-insensitive).

3.6

   POST /restconf/operations/module-A:reset HTTP/1.1
   Server example.com

In two locations, the "Server" header is shown without a colon.

   If the operation is invoked without errors, and if the "rpc" or
   "action" statement has an "output" section, then a message-body MAY
   be sent by the server in the response, otherwise the response message
   MUST NOT include a message-body in the response message, and MUST
   send a "204 No Content" status-line instead.

It seems odd that if an RPC is performed, then it is optional for the
output data to be returned to the client.

   <input xmlns="https://example.com/ns/example-ops">

This means the 'input' element of an RPC body is in the namespace of
the module.  Similarly for 'output' elements.  This is not specified
anywhere.  (Errors is specified to be in "ietf-restconf" in section
7.1.)

4

   Operations are applied to a single data resource instance at once.

IMO it would be useful to use the word "atomically", as that has
a well-understood meaning.

4.3

   If a retrieval request for a data resource representing a YANG leaf-
   list or list object identifies more than one instance, and XML
   encoding is used in the response, then an error response containing a
   "400 Bad Request" status-line MUST be returned by the server.

What about zero instances?  Presumably it gets a 404 response rather
than an empty body, but it would be good to specify that.

   Note that the way that access control is applied to data resources is
   completely incompatible with HTTP caching.  The Last-Modified and
   ETag headers maintained for a data resource are not affected by
   changes to the access control rules for that data resource.  It is
   possible for the representation of a data resource that is visible to
   a particular client to be changed without detection via the Last-
   Modified or ETag values.

Note that it's possible for the client to detect changes in
configuration data that it is not allowed to read by detecting changes
in the Last-Modified/ETag values.  This probably isn't a security
problem, but the authors should be aware of it.

What does a GET of the datastore URL return?  I would guess an XML
document with a top node of "datastore" (in the module namespace), but
that doesn't seem to be specified.  Perhaps it is a known thing that
the datastore contains one instance of every top-level data node in
the module definition... but there can be multiple modules defined in
the server...  Similarly, one can ask how PUT, POST, PATCH, and DELETE
are applied to the datastore resource.

4.4.1

   The message-body MUST NOT contain more than one instance
   of the expected data resource.

It seems like you want "MUST contain exactly one", since it seems that
you don't want to allow zero instances.

4.6

   This document defines one patch mechanism (Section 4.6.1).  The YANG
   PATCH mechanism is defined in [I-D.ietf-netconf-yang-patch].  Other
   patch mechanisms may be defined by future specifications.

This sentence isn't clear that "one patch mechanism" is different from
"The YANG PATCH mechanism ...".  Perhaps:

   This document defines one patch mechanism (Section 4.6.1).  Another
   patch mechanism, the YANG PATCH mechanism, is defined in
   [I-D.ietf-netconf-yang-patch].  Other patch mechanisms may be
   defined by future specifications.

4.6.1

   Plain patch can be used to create or update, but not delete, a child
   resource within the target resource.  Please see
   [I-D.ietf-netconf-yang-patch] for an alternate media-type supporting
   more granular control.

I assume this means "supporting deletion of child resources".

4.8

   | point             | POST, PUT   | Insertion point for ordered-yb  |

"ordered-yb" should be "ordered-by".

   o  Each parameter can appear at most once in a request URI.

Despite being given here as a general rule, it is repeated in 4.8.2,
4.8.3, 4.8.4, 4.8.5, 4.8.6, 4.8.7, 4.8.8, and 4.8.9 (but not 4.8.1).
Also, if the repetition is to be removed, the general rule needs to
add "If more than one instance is present, then a "400 Bad Request"
status-line MUST be returned by the server."

4.8.2

   By default, the server will include all sub-resources within a
   retrieved resource, which have the same resource type as the
   requested resource.  Only one level of sub-resources with a different
   media type than the target resource will be returned.  The exception
   is the datastore resource.  If this resource type is retrieved then
   by default the datastore and all child data resources are returned.

Does the second sentence do anything?  The only instance I know of of
a node whose media type is different from that of its child nodes is
the the datastore resource, and that is specifically exempted from the
effect of the 2nd sentence.

4.8.3.

I take it that 

   "fields=admin(label;catalogue-number)".

is equivalent to

   "fields=admin/label;admin/catalogue-number"

But the lack of a / before the ( is not really standard practice and
might trip up users.  (Compare with Un*x shell "/a/b/c/{d,e}" and
"/a/b/c{d,e}".)

4.8.7

   It is not valid to specify start times that are later than the
   current time.

It seems that there needs to be a way for the client to determine the
time the *server* thinks it is.  The client can't use its own clock,
because there can be clock skew between the client and the server, and
also the server may not know what time it is, so its time scale could
have an arbitrary offset from real time.

7

   Since an operation resource is defined with a YANG "rpc" statement,
   and an action is defined with a YANG "action" statement, a mapping
   between the NETCONF <error-tag> value and the HTTP status code is
   needed.  The specific error condition and response code to use are
   data-model specific and might be contained in the YANG "description"
   statement for the "action" or "rpc" statement.

I assume this means "The specific error-tag value and response code
..."; the word "condition" appears nowhere else in the draft.

                 +-------------------------+-------------+
                 | <error&#8209;tag>       | status code |
                 +-------------------------+-------------+

Why is "&#8209;" (&#x2011;, NON-BREAKING HYPHEN) used here?

7.1

   [...] then the server SHOULD send a response
   message-body containing the information described by the "errors"
   container definition within the YANG module Section 8.

There's probably a missing word before "Section 8".  Specifically,
this sentence needs to say that "errors" is defined in the YANG module
"ietf-restconf" -- as the sentence stands, it's easy to assume that
"errors" is within "the YANG module", i.e., the one defining the
operation.

11.2

Looking at RFC 3688 and the registry
(http://www.iana.org/assignments/xml-registry/xml-registry.xhtml#ns)
it looks like you want to explicitly say that the URIs are being
registered as namespaces in the IETF XML registry.

11.4

   o  the name of the RESTCONF capability.  By convention, this name is
      prefixed with the colon ':' character.

I think "begins" rather than "prefixed" is preferable, as it makes
clear that the colon is part of the name, rather than being prepended
to the name.

13

Who is The Space & Terrestrial Communications Directorate?  Maybe it
would add to the credit of the S&TCD by prefixing the country that
funds it ... which seems to be "United States Army".

8.

         draft-ietf-netmod-yang-json defines
         JSON encoding rules for all RESTCONF media types that
         use the '+json' suffix.

There needs to be an RFC Editor note to replace
"draft-ietf-netmod-yang-json" with the eventual RFC name.

D.1.3

   <capabilities xmlns="">

Is it true that there's no NS URN?  Section 9.3 suggests it should
have namespace "urn:ietf:params:xml:ns:yang:ietf-restconf-monitoring".

D.2.1

   Note that the "Location" header line is wrapped for display purposes
   only:

You might want to promote this comment to a higher section, as
wrapping is done in other places.

D.3.4

   Location: https://example.com/restconf/data/
       example-jukebox:jukebox/playlist=Foo-One/song=1

But the text doesn't say that the Location header is wrapped only for
display purposes.

---

From: Benoit Claise <[email-masked]>
Date: Fri, 17 Jun 2016 09:29:36 +0200

> reviewer, it's best to take care of these now.
Yes. Dale did such a good job on the gen-ART review of RFC6020bis that 
he accepted to review the RESTCONF document the same way.

Thanks Dale.

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

---

From: Martin Bjorklund <[email-masked]>
Date: Sun, 19 Jun 2016 19:57:05 +0200 (CEST)

>   restriction isn't specified in draft-ietf-netmod-rfc6020.

In section 14 (grammar) 6020bis says:

;; An identifier MUST NOT start with (('X'|'x') ('M'|'m') ('L'|'l'))

/martin

---

From: [email-masked] (Dale R. Worley)
Date: Tue, 21 Jun 2016 11:13:34 -0400

> ;; An identifier MUST NOT start with (('X'|'x') ('M'|'m') ('L'|'l'))

Good.  It might be worth mentioning that in the prose as well.

Dale