Re: [Netconf] Adam Roach's No Objection on draft-ietf-netconf-nmda-restconf-04: (with COMMENT)

[Martin Bjorklund <[e-mail]>]

Hi,

Adam Roach <[e-mail]> wrote:
> Adam Roach has entered the following ballot position for
> draft-ietf-netconf-nmda-restconf-04: No Objection
> 
> 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-nmda-restconf/
> 
> 
> 
> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
> 
> Thanks to everyone who worked on this mechanism.
> 
> I agree with Alexey's general point that implementors could benefit from
> guidance regarding the "with-defaults" values, as the current mechanism
> requires a potentially cumbersome process of probing for support of
> desired functionality.

I will reply to Alexey's email.

> Beyond that, I have only a handful of minor comments.
> 
> ---------------------------------------------------------------------------
> 
> §3.1:
> 
> >  The <datastore> path component is encoded as an "identity" according
> >  to the JSON encoding rules for identities, defined in Section 4 of
> >  [RFC7951].
> 
> I don't find anything in section 4 of RFC 7951 that indicates encoding for an
> "identity." Should this text say "identityref" instead? And if that's the
> intention, it seems that pointing to section 6.8 would be more direct than
> pointing to section 4.

Yes.  I suggest changing the reference and adding a clarification:

OLD:

   The <datastore> path component is encoded as an "identity" according
   to the JSON encoding rules for identities, defined in Section 4 of
   [RFC7951].  Such an identity MUST be derived from the "datastore"
   identity defined in the "ietf-datastores" YANG module [RFC8342].

NEW:

   The <datastore> path component is encoded as an "identityref"
   according to the JSON encoding rules for identities, defined in
   Section 6.8 of [RFC7951].  The namespace-qualified form MUST be used.
   Such an identity MUST be derived from the "datastore" identity
   defined in the "ietf-datastores" YANG module [RFC8342].

> ---------------------------------------------------------------------------
> 
> §5:
> 
> >  This documents extends the RESTCONF protocol by introducing new
> >  datastore resources.  The lowest RESTCONF layer is HTTPS, and the
> >  mandatory-to-implement secure transport is TLS [RFC5246].
> 
> Unless this mechanism relies on some behavior that is different
> between TLS 1.2
> and TLS 1.3, please update this reference to RFC 8446.

Ok.  This update needs to go into the security-considerations-template
that is used for documents with YANG modules.

> Nit: "This document extends..."
>                    ^

Fixed.

> >  The security constraints for the base RESTCONF protocol (see
> >  Section 12 of [RFC8040] apply to the new RESTCONF datastore resources
> >  defined in this document.
> 
> Nit: Missing a closing parenthesis.

Fixed.


Thanks for the review!



/martin