netconf-wg / restconf

9 stars 4 forks source link

SH read-through comments on draft-ietf-netconf-restconf-09 #49

Closed abierman closed 8 years ago

abierman commented 8 years ago

From Sue Hares

https://www.ietf.org/mail-archive/web/netconf/current/msg10907.html

abierman commented 8 years ago

+2.1 – what other protocols than TLS are possible for HTTP for RESTCONF

        [Joe Clarke Fri 1/8/2016 2:10 PM also mentions]

NONE -- ADDED TEXT MUST NOT IMPLEMENT OVER HTTP WITHOUT TLS

abierman commented 8 years ago

+3.4.1 - This is Edit Collision detection for the {+restconf}/data nodes by the server for Patch entities

            may be useful for I2RS (see I2RS comments below).  What is not clear from the text is whether you can

CLARIFIED THAT DATASTORE MUST BE USED INSTEAD

abierman commented 8 years ago

+3. Section 4.6 – there is a plain patch, but no mention is made of a more complicated patch. Is this on purpose, or did you want to refer to another draft?

IGNORED

abierman commented 8 years ago

+4. List actions that apply to all list entries: GET, DELETE, sequence of PATCHes for a list (multiple entries from Lada (+1 on Lada: Mon 1/11/2016 5:36 AM),

ADDED TEXT:

Operations are applied to a single data resource instance at once. The server MUST NOT allow any operation to be applied to multiple instances of a YANG list or leaf-list.

abierman commented 8 years ago

+5 Application of multiple filters in order for Query plus start-time/stop time. Is this possible?

NO - CLARIFIED THAT QUERY PARAMETERS MUST NOT APPEAR MORE THAN ONCE IN A REQUEST

abierman commented 8 years ago

+6 – Does Data timestamp and E-Tag apply to operational state data?

NO -CLARIFIED THAT NON-CONFIG MUST NOT AFFECT timestamp or entity tag

abierman commented 8 years ago

+7: You do not mention leaf-lists (this is +1 on Lada Mon 1/1/2016 5:36AM comment).

LEAF-LIST HAS BEEN ADDED AS A DATA RESOURCE

abierman commented 8 years ago

+8: CAN RESTCONF and NETCONF both attach to a data store?

  If so the question regarding netconf lock (persist-id), and RESTCONF edit collisions

  Needs to be reference in section n3.4.1.  If not, it needs to be clearly stated

 (+1 Martin Bjorklund’s comment on revising locking 1/8/2016 9:45pm)  

 (see Tom Petch’s comment A) on 1/22/2016 at 11:20am)

DO NOT UNDERSTAND COMMENT. RESTCONF PROVIDES NO LOCKING AT ALL

abierman commented 8 years ago

+9: +1 on Security review that more needs to be in the security section.

· It would be helpful to consider whether the I2RS extensions suggested below might break the data security of RESTCONF. I believe RESTCONF will fulfill the I2RS protocol security requirements minus the ephemeral state (not created yet).

· Note that Tom Petch (comment A on 1/22/2016 at 11:20am) – asks if “authenticated NETCONF username [MUST] be associated with each request”.

· NACM is clearly specified in order to allow some filtering of results, but the link to authenticated NETCONF user name is not as strongly linked. Since I2RS has suggested NACM may prevent some DoS attacks, this may be useful.

LEFT FOR KENT TO ADDRESS

abierman commented 8 years ago

+10- +1 on (Rodney Cummings 1/11/2016 5:30pm) – we should have a capability that indicates if we can pull module schema if this is optional. Security may restrict a full pull of the module schema if there is a module augmentation by a vendor that has security add-ons.

NO -- THE schema LEAF WILL BE PRESENT IF THE YANG MODULE CAN BE RETRIEVED THIS IS CLEARLY STATED IN THE YANG MODULE

abierman commented 8 years ago

+11 – Since insert and point is required for PUT/POST on the server, can modules be designed that do not support it? The comment from Rodney Cummings 1/11/2016 5:30pm] surprised me. Can this be explained or defined some-how? If modules do not support it for lists, then please explain the error handling.

NO CHANGES -- YANG CLEARLY DEFINES ordered-by user. NO WAY FOR A RESTCONF SERVER TO OMIT THIS FUNCTIONALITY

ADDED TEXT:

If the server implements any YANG module that contains a configuration data node that is "ordered-by" user, then the "insert" and "point" query parameters MUST be supported. If not, then these parameters are not applicable.

abierman commented 8 years ago

+12 - +1 to Robert Wilton’s question on http 1.2? Section 2.1 specifies http 1.1 [RFC7230]. Will RESTCONF run over HTTP 1.2? I did not find any restrictions on http 1.1 pipelining. Does this mean that RESTCONF can access different parts of a tree via two different sessions? What happens if overwrites – Last one wins edit collisions – unless you have the model timestamp and ETAG ID flags on?

NO CHANGES MADE

abierman commented 8 years ago

PIPELINING:

ADDED TEXT: HTTP/1.1 pipelining MUST be supported by the server. Responses MUST be sent in the same order that requests are received. No other versions of HTTP are supported for use with RESTCONF.

abierman commented 8 years ago

+13 - +1 on the question or edit ordering from Robert Wilton [1/19/2016 5:46am]

NO CHANGES -- NO NOT UNDERSTAND EDITS THAT ARE NEEDED

abierman commented 8 years ago

+14 - +1 – where netmod-opsstate-reqs fit into this document? It would be good to determine if the module that is “read-only” can get a query such as “op-state-only”. It would be good to query for multiple flags (op-state, with-default, ephemeral). Have you considered how you will merge this in? It would be good if these would be incremental upgrades.

NOT WILLING TO POLLUTE RESTCONF WITH OPSTATE POORLY DEFINED REQUIREMENTS; NOT CLEAR RESTCONF NEEDS ANY CHANGES

abierman commented 8 years ago

1 Multiple places /operational data/operational state data/

[+ to Lada: Mon 1/11/2016 5:36am concern, but different resolution]

NO CHANGES -- WILL ALTER TERMINOLOGY IF FORCED BY IESG

abierman commented 8 years ago

2 p. 6: HATEOAS ( at least deserves an expansion, and a definition) since it is the scheme you are not choosing. (+1 on Lada: Mon 1/11/2016 5:36 AM),

ADDED TO TERMS

3 p. 10: ABNF – first time ABNF is used, it should be expanded.

EXPANDED FIRST USE

abierman commented 8 years ago

4 p. 25 – In section 3.6.3 in paragraph 2

/Using the “reset” operation/ Using the “reset” operation from the example in section 3.6

CORRECTED -- SHOULD BE reboot NOT reset

abierman commented 8 years ago

Section 4 – I2RS and ephemeral state.

NOT WILLING TO MAKE ANY CHANGES TO RESTCONF WHATSOEVER TO SUPPORT I2RS. NOT PART OF CHARTER. WILL REQUIRE NEW CHARTER AND NEW RFC TO ADD I2RS SUPPORT

abierman commented 8 years ago

closed in draft-10