Re: [netconf] [Last-Call] Yangdoctors last call review of draft-ietf-netconf-ssh-client-server-24

[Andy Bierman <[e-mail]>]

On Thu, May 27, 2021 at 12:05 PM Kent Watsen <[e-mail]> wrote:

> [moving all alias except “netconf” to the BCC list]
>
>
> Hi Andy,
>
> Thank you for your review!
>
> > On May 25, 2021, at 3:21 PM, Andy Bierman via Datatracker <
> [e-mail]> wrote:
> >
> > Reviewer: Andy Bierman
> > Review result: Ready
> >
> > Comments:
> >
> > 1) Measuring Interoperability for groupings and identities
> >
> > [same comment for SSH and TLS drafts]
> >
> > These modules are intentionally abstract.
> > There are no protocol-accessible objects defined at all.
> > Interoperability is usually measured in the context of a
> > specific protocol (e.g., NETCONF).
> >
> > There is an assumption that interoperability will be achieved
> > by some other RFCs that will have "uses" statements to create
> > protocol-accessible or otherwise implementable objects.
>
> True, and we have, e.g., the "netconf-client-server" and
> "restconf-client-server" drafts, and the PCE WG has the "pcep-yang” draft.
>  There’s a leap of faith, but also some running code, so hopefully within
> the intentions of an “RFC”.
>
>
> > There is also an assumption that the groupings will be used the
> > same everywhere, and the only difference will be the
> > path from root to the objects in these groupings.
> > In fact, the "refine" statement allows each usage to be
> > different.
>
> What assumption?  Do you mean the description statements in the various
> "[ssh/tls]-[client/server]-grouping” groupings.  For instance, see the 2nd
> paragraph below:
>
>      grouping ssh-client-grouping {
>        description
>          "A reusable grouping for configuring a SSH client without
>           any consideration for how an underlying TCP session is
>           established.
>
>           Note that this grouping uses fairly typical descendent
>           node names such that a stack of 'uses' statements will
>           have name conflicts.  It is intended that the consuming
>           data model will resolve the issue (e.g., by wrapping
>           the 'uses' statement in a container called
>           'ssh-client-parameters').  This model purposely does
>           not do this itself so as to provide maximum flexibility
>           to consuming models.";
>
> While it is true that “stacks” (or sibling “uses” statements) are expected
> (from a "best practice” POV), I’m unaware of any Xpath expressions
> enforcing this.
>
>

I was re-reading RFC 7950 and there is nothing to add in these drafts about
it.
Groupings do not define any nodes in the schema tree.

In the old days the IETF used to be concerned with Interoperability Reports
in order to advance an RFC from Proposed Standard to Draft Standard.
Now that that is no longer done, it is not that important to point out that
interoperability can only be evaluated for the module with the "uses"
statement,
since that is how schema tree nodes are defined from a grouping template.



> > Perhaps the drafts should mention these interoperability issues.
>
> Could you be more specific in what text you’d like to see in which
> sections?
>
>
see bove -- no changes requested


>
>
> > 2) same feature names in 2 modules
> >
> >  - feature userauth-hostbased
> >  - feature userauth-none
> >  - feature userauth-password
> >  - feature userauth-publickey
> >
> > The ietf-ssh-client and ietf-ssh-server modules both use these
> > feature names. IMO users will not expect this, and this will cause
> > confusion.
> >
> > Why can't these features be defined once in ietf-ssh-common.yang?
> > Seems like client and server will advertise the feature for
> > implementing their relevant values.
>
> Well, these were named differently in just the previous version of the ssh
> draft…from the change log:
>
>  *  Renamed "client-auth-*" to "userauth-*"
>  *  Renamed "client-identity-*" to "userauth-*”
>
> Juergen felt that these were more natural names for SSH.   I agreed,
> mostly just to get the “monkey off my back”, but it breaks with the
> convention used by the collection if client-server drafts (where all the
> variations of "[client/server]-[auth/identity]” are typically used).
>
> Yes, “userauth” is the term used in the SSH drafts, and I didn’t think is
> was a big deal or, really, any deal at all, for the same feature names to
> be used in both drafts, such that I'm surprised by your "users will not
> expect this, and this will cause confusion” comment...
>
> Moving the definitions to the “common” module is possible, of course, but
> tricky since the description statements say different things, to address
> the different contexts in which they’re used.  The description statement
> could be expanded to address each context, but this will get wordy…I wonder
> if it wouldn’t be better to just go back to the previous
> “client-[auth/identity]-*” prefixes - thoughts?
>
>
OK -- not a big deal except possibly when the feature name is used outside
of YANG,
without any prefix to tell which one is meant.  I agree the different
description-stmts
make it difficult to write a common feature definition, so this is not
important to change either.




>
>
>
> K.
>
>
Andy