Subject: [Netconf] RESTCONF-10 draft

From: Andy Bierman <[email-masked]>
Date: Sun, 20 Mar 2016 12:02:48 -0700

Hi,

Please review the latest version of the RESTCONF protocol:
https://datatracker.ietf.org/doc/draft-ietf-netconf-restconf/

There have been many updates based on the extensive list
of review comments:

https://tools.ietf.org/html/draft-ietf-netconf-restconf-10#appendix-A.1

This draft addresses all open issues found on github:
https://github.com/netconf-wg/restconf/issues

Not all requested edits were made. Please review the
github issue tracker for details about the editing process.

thanks,
Andy

---

From: "Ersue, Mehmet (Nokia - DE/Munich)" <[email-masked]>
Date: Mon, 21 Mar 2016 11:30:23 +0000

Dear NETCONF WG,

first of all Many Thanks to Andy, Martin and Kent for addressing the numerous WGLC comments.

RESTCONF is a major NETCONF document going to be published soon.

I would like to ask all to review and verify the changes made in the document.
If there is anything missing or insufficiently addressed, please send your comments to NETCONF ML at the latest by March 31, 2016 EOB PDT.

Our aim is to go to AD and IESG review after that.

BR,
Mehmet

From: Netconf [mailto:[email-masked]] On Behalf Of EXT Andy Bierman
Sent: Sunday, March 20, 2016 8:03 PM
To: Netconf <[email-masked]>
Subject: [Netconf] RESTCONF-10 draft

Hi,

thanks,
Andy

---

From: Robert Wilton <[email-masked]>
Date: Tue, 22 Mar 2016 18:10:50 +0000

Hi Andy, Kent, Martin,

I think that you have addresses some of my recent comments on 09, but 
not all of them.

I've reproduced what I think are my outstanding review comments here 
which I think also apply to -10;

The first 6 questions related to section 4.8: Query Parameters:
1. Are query parameters and paths case sensitive?  I would guess so.  
The draft doesn't make any explicit mention of this, does it need to?

2. Should an unexpected query parameter cause an error or should it just 
be ignored?  For some (but not all) of the query parameters in sections 
4.8.1 through 4.8.9 it indicates that an explicit error should be 
returned if the query parameter is used for an resource type on which it 
isn't supported, but section 4.8 allows vendor specific query parameters 
to also be defined.  I think that interop might be easier if unknown 
query parameters are just ignored.

3. Both the depth and fields query parameters (4.8.2 & 4.8.3) indicate 
that they apply to the API resource (3.3), but I wasn't sure that they 
served any purpose since my reading of the draft is that the api 
resource is just the top level and effectively fixed. I.e. it doesn't 
return any data from the datastore or the operations, and they need to 
be explicitly queried separately if required.  Proposed fix:

4.8.2 & 4.8.3 Old:

    This parameter is only allowed for GET methods on*API,*  datastore, and
    data resources.  A "400 Bad Request" status-line is returned if it
    used for other methods or resource types.

4.8.2 & 4.8.3 New:

    This parameter is only allowed for GET methods on datastore, and
    data resources.  A "400 Bad Request" status-line is returned if it
    used for other methods or resource types.

4. Minor nit.  It might be helpful if the options were listed in the 
same order as they are listed in the table in section 4.8.  E.g. this 
would mean that the section on "filter" would need to move up above the 
section on "insert".

5. Section 4.8.2: The "depth" Query Parameter
The last but one paragraph reads:

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

I'm not convinced that this paragraph is correct, e.g. for a GET request 
on /restconf/data.  By default I would expect it to return all levels of 
child nodes (which have a different resource type) rather than just the 
first layer of children.

If the depth parameter is changed to not apply to the API resource, then 
could this paragraph just be changed to:

New:

    "By default, the server will include all sub-resources within a
    retrieved resource."

6. Section 8: "error-info". Should the type be anydata rather than the 
deprecated anyxml?

Old:

            *anyxml *error-info {
               description
                 "This*anyxml *value MUST represent a container with
                 zero or more data nodes representing additional
                 error information.";
            }

New:

            anydata error-info {
               description
                 "This anydata value MUST represent a container with
                 zero or more data nodes representing additional
                 error information.";
            }

These remaining questions are those that I raised previously (eliding 
those which have been addressed in -10):

7. Section 3.4.1 Edit Collision Detection:
In -10, this has been updated to make it clear that it only applies to 
configuration resources which is now internally consistent.

However, it feels like the timestamp is potentially be a bit inaccurate 
for a REST like interface.

It is probably too late to raise this now, but I was wondering whether 
the server shouldn't also track operational resources using parallel 
timestamps/etags?  Whether the config or operational timestamp/etag is 
returned would depend on which node had been queried and the "content" 
GET query parameter:
  - If the content query was for "config" only then the config resources 
timestamp/etag would be returned.
  - if the content query was for "nonconfig" then the operational 
resources timestamp/etag would be returned.
  - If the content query was for "all" then the latest of the 
config/operational resource timestamp would be returned and a combined 
config/operational etag could be returned.  In addition, the config and 
operational timestamps and etags could be returned using separate HTTP 
headers.

8. Section 3.5 Data Resource, last paragraph:

    The resource definition version for a data resource is identified by
    the revision date of the YANG module containing the YANG definition
    for the data resource.

I don't understand the purpose of this last paragraph means, or what its 
relevance is.  I suggest that either "resource definition version" is 
defined in 1.1.4 Terms, or this paragraph is deleted.

9. Section 4.1. OPTIONS:

    The OPTIONS method is sent by the client to discover which methods
    are supported by the server for a specific resource (e.g., GET, POST,
    DELETE, etc.).

    The server SHOULD implement this method, however the same information
    could be extracted from the YANG modules and the RESTCONF protocol
    specification.

I suggest adding the sentence:

    The server SHOULD NOT include any body text for the OPTIONS method.

10. Section 7.1 Error Response message:

    When an error occurs for a request message on a*data*  resource or an
    *operation *resource, and a "4xx" class of status codes will be
    returned (except for status code "403 Forbidden"), then the server
    SHOULD send a response message-body containing the information
    described by the "errors" container definition within the YANG module
    Section 8 
<https://tools.ietf.org/html/draft-ietf-netconf-restconf-10#section-8>.  The Content-Type of this response message*MUST be application/yang.errors*  (see example below).

    The client*MAY *specify the desired encoding for error messages by
    specifying the appropriate media-type in the Accept header.*If no error media is specified, then the media type of the request 
message SHOULD be used, or the server MAY choose any supported message 
encoding format.*   If there is no request message the server MUST
    select "application/yang.errors+xml" or "application/
    yang.errors+json", depending on server preference.  All of the
    examples in this document, except for the one below, assume that XML
    encoding will be returned if there is an error.

i) The beginning of the error paragraph makes the description 
conditional on the request message being on a data resource.  Should 
this cover other resource types as well, or otherwise what is the error 
handling strategy for other resource types?
ii) Should the last sentence of the first paragraph be "The Content-Type 
of this response message MUST be a subtype of application/yang.errors 
(see example below). "?
iii) Would it be wise to indicate that the client SHOULD specify desired 
encoding for error messages?
iv) The second paragraph states "If no error media is specified, then 
the media type of the request message is used. " but this directly 
conflicts with "The Content- Type of this response message MUST be 
application/yang.errors (see example below). "
v) "If there is no request message the server ..." - I would think that 
there must always be a request message.

In summary, what about this text instead:

    When an error occurs for a request message on an*api, data,****datastore, or operation*  resource, and a "4xx" class of status codes will be
    returned (except for status code "403 Forbidden"), then the server
    SHOULD send a response message-body containing the information
    described by the "errors" container definition within the YANG module
    Section 8 
<https://tools.ietf.org/html/draft-ietf-netconf-restconf-10#section-8>.  The Content-Type of this response message*MUST**be a subtype of**application/yang.errors*  (see example below).

    The client*SHOULD *specify the desired encoding for error messages by
    specifying the appropriate media-type in the Accept header.*If no error media is specified, then the server SHOULD choose the error 
media****type with the clients preferred encoding, or otherwise the server MAY****choose any supported message encoding format. *If there is no request message
    the server MUST select "application/yang.errors+xml" or "application/
    yang.errors+json", depending on server preference.  All of the
    examples in this document, except for the one below, assume that XML
    encoding will be returned if there is an error.

Thanks,
Rob

On 20/03/2016 19:02, Andy Bierman wrote:
> _______________________________________________
> Netconf mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/netconf

---

From: t.petch <[email-masked]>
Date: Thu, 24 Mar 2016 16:18:23 +0000

It's BIG (which means I include comments up to the end of section 3 now
and hope to send the rest next week; questions of clarity, mostly).

I do like section 1, much more to my taste.

- 'state data is defined in 1.1.1, operational data is not defined.  The
former is used twice, that latter six times - I would rather 'state
data' were used throughout.  (I do see it as a persistent source of
confusion as others come to use NETCONF).

- s1.2 it might be better to give a REST reference [rest-dissertation]
where REST first appears.

-s1.4 "   If the device supports :startup, the device automatically
copies the "
sounds like a MUST

- s2.2 "The exclusive use the X.509v3 based certificates is consistent
with "
 use of?

- s2.3
"MUST use X.509 certificate path validation"
 and
"MUST also be considered valid if it matches a locally configured
certificate fingerprint"
seem contradictory to me; suggest something along the lines of

"MUST
 - either use X.509 certificate path validation
   [RFC5280] to verify the integrity of the RESTCONF server's TLS
   certificate
- or match the presented X.509 certificate with a locally configured
certificate fingerprints"

- s.2.5
"If the RESTCONF client is not authorized to access a resource, the
server MUST send an HTTP response with "401 Unauthorized"
cf s.4.3
"If the user is not authorized to read the target resource, an error
response containing a "403 Forbidden" "
seems contradictory

- s3.1 Can there be more than one restconf link relation?  I am not
thinking of the future postulated above with a version 2, but more of
SNMP with multiple MIBs under different community.

- s3.4
"Each RESTCONF edit of a datastore resource is saved to non-volatile
storage by the server. "
Is this a MUST?

- s3.5
"If not maintained, then the resource timestamp for the datastore MUST
be used instead."
I am unclear what the timestamp is being used for.

"This timestamp is only affected by configuration data resources, and
   MUST NOT be updated for changes to non-configuration data."

Which timestamp?  resource timestamp for the datastore?  Since it is a
new paragraph, I am unclear.

"If not maintained, then the resource entity tag  for the datastore MUST
be used instead.

This entity tag is only affected by configuration data resources, and
MUST NOT be updated for changes to non-configuration data"

Same pair of questions

- s3.5.2
"If a leaf or leaf-list is missing from the configuration ..."

"and the leaf has not been given a value yet, .. "

Do these mean that it has not been configured?  If so, I would find it
clearer if it said 'configured'; if not, I am unclear of the nuance.

- s3.6

"If the "input" section contains mandatory parameters, then a
message-body MUST be sent by the client."

6020bis says
"   If a leaf in the input tree has a "mandatory" statement with the
   value "true", the leaf MUST be present in an RPC invocation.
which I think more precise.  Suggest something like

"   If a leaf in the input tree has a "mandatory" statement with the
    value "true", the leaf MUST be present in an RPC invocation and
    a message-body MUST be sent by the client."

- should a similar statement be present for output?  I see no difference
between input and output in this regard.

s3.6.1
"      POST /restconf/data/example-action:interfaces/interface=eth0 ..."
"      { "example-action:input" : { ..."

module name is 'example-actions'

s3.6.2
"   The server might respond:

      HTTP/1.1 200 OK
      Date: Mon, 25 Apr 2012 11:10:30 GMT
      Server: example-server
      Content-Type: application/yang.operation+json
      {   "example-actions:output" : {
          "last-reset" : "2015-10-10T02:14:11Z"     }    } "

How does the client marry the output to the request? Should the rest of
the path be there?
  interfaces/interface=eth0/get-last-reset-time

Tom Petch