Re: [netmod] WG Last Call: draft-ietf-netmod-yang-module-versioning-05

[Jürgen Schönwälder <[e-mail]>]

Hi,

below is my last call review.

Document: draft-ietf-netmod-yang-module-versioning-05
Date: 2022-03-06

* 1. Introduction

  Is it "module lifecyle problems" or "module versioning problems"?
  Can we perhaps be even more explicit? I also note that
  [I-D.ietf-netmod-yang-solutions] has seemingly expired, perhaps this
  document is not needed if we revise the introduction a bit and make
  it more descriptive. And why are SDOs, vendors and industry
  mentioned as explicit targets? Here is an attempt to make a
  constructive proposal:

  NEW

   The current YANG [RFC7950] module update rules require that updates
   of YANG modules preserve strict backwards compatibility. This has
   caused problems as described in [I-D.ietf-netmod-yang-versioning-reqs].
   This document recognizes the need to sometimes allow YANG modules
   to evolve with non-backwards-compatible changes, which can cause
   breakage to clients and importing YANG modules.  Accepting that
   non-backwards-compatible changes do sometimes occur, it is
   important to have mechanisms to report where these changes occur,
   and to manage their effect on clients and the broader YANG
   ecosystem.

   This document defines the foundation of a flexible versioning
   solution. A companion document [I-D.ietf-netmod-yang-semver]
   extends this work by introducing a semantic versioning scheme. In
   addition, [I-D.ietf-netmod-yang-ver-selection] provides a schema
   selection scheme that can be used by clients to choose which
   schemas to use when interacting with a server from the available
   schema that are supported and advertised by the server. This builds
   on [I-D.ietf-netmod-yang-packages], which provides a mechanism to
   group sets of related YANG modules revisions together into so
   called YANG packages. Finally,
   [I-D.ietf-netmod-yang-schema-comparison] specifies an algorithm
   that can be used to compare two revisions of a YANG schema.

* 3.4.1. File names

  - This looks like a SHOULD/MAY clash, what do you really suggest
    here? Better don't use RFC 2119 language if the guideline is kind
    of contradictory. ;-)

      YANG module (or submodule) files MAY be identified using either
      revision-date or revision-label. Typically, only one file name
      SHOULD exist for the same module (or submodule) revision.  Two file
      names, one with the revision date and another with the revision
      label, MAY exist for the same module (or submodule) revision

    An implementation needs to be able to find the modules. If the
    revision date is the unique key used by imports, then probably the
    @ form needs to be there while the # form is nice to have? Well, I
    am not really sure what the text in the I-D suggests. Perhaps it
    says it SHMAY be implementation specific. ;-)

    If we want interoperability at the file name level, a clear and
    unambiguous baseline may be helpful. If we do not expect that,
    there is no need to write rules using RFC 2119 keywords.

* 3.5. Examples for updating the YANG module revision history

  Assuming the 2.x line is backwards compatible but the 3.x line is
  not. How do I figure this out from the linear revision history
  sorted by dates? Is the idea that I always only have a single branch
  in the revision history in a module? Or can there be multiple
  branches documented? Can branches merge again? If I can include the
  history of multiple branches, is the idea that tools have to
  understand the semantics of the revision labels to reconstruct the
  revision history graph in order to make sense of the
  rev:non-backwards-compatible markers?  The linear order could easily
  lead to misinterpretations, revision 2019-04-01 is BC but it would
  appear in chronological order after 2019-03-01, to which it is NBC.

* 4. Import by derived revision

  I agree that import by revision or derived is better than import by
  revision and it would work great with the existing YANG module
  update rules. That said, I do not see how it will work great with
  update rules that allow BC and NBC updates. If we support richer
  update semantics, we will need richer mechanisms to express
  compatibility and it seems wrong to put that information into the
  YANG import statement. To me, it seems much more reasonable to
  maintain the compatibility contraints outside the module such that
  updates on the compatibility rules can be made without having to
  touch the YANG modules.

  To fully automate things, it might be desirable to actually tag the
  specific definitions that changed. When module A imports from B, it
  matters what A is using from B. Perhaps the idea is that algorithms
  compute this on the fly. I fear not all changes may be decidable to
  be either BC or NBC. Has it been considered to have markup at the
  statement level that provides details such as when something was
  added, when something had an NBC change etc? Once you have markup at
  that level, tools can decide whether an importing module is affected
  by changes in an imported module or not.

  It seems we are doing things backwards, we mark modules as having
  NBC changes and then tooling may try to figure out what the causes
  are. Would it not be easier to tag what has changed and then tooling
  can calculate the rest?

  I am not sure of this:

      The argument to the "revision-or-derived" extension statement is a
      revision date or a revision label.

  Why is it useful that the value can be a revision date or a revision
  label? I assume you also expect there to be a partial order on the
  labels, no?

      A particular revision of an imported module satisfies an import's
      "revision-or-derived" extension statement if the imported module's
      revision history contains a revision statement with a matching
      revision date or revision label.

  I am not sure, perhaps because I am not sure whether the revision
  history can reflect branches. If I say revision-or-derived 2.1.0,
  can I then get 3.0.0 (which is in a different branch)?

  Yes, revision-or-derived is what we should have had in YANG. I fail
  to see how it is useful in a versioning scheme that supports NBC
  changes and some level of branching.

      The "revision-or-derived" extension statement does not guarantee
      that all module revisions that satisfy an import statement are
      necessarily compatible; it only gives an indication that the
      revisions are more likely to be compatible.

  So how is this useful? The name "revision-or-derived" may be a
  misnomer, it seems you really have "none-before". The name
  "revision-or-derived" suggests a promise that is not really there.

  I believe compatibility constraints need maintenance and grow to get
  complex. Managing them inside of imports is in my view not a good
  and scalable idea.

      Adding, modifying or removing a "revision-or-derived" extension
      statement is considered to be a BC change.

  How can that be? Such changes fundamentally change how imports are
  resolved.

  Example 2 finally tells me by way of example what derived means.  I
  suggest to define the notion of "derived" earlier. The example seems
  to suggest that derived is along the partial order induced by the
  version labeling scheme (and this means tools need to be able to
  reconstruct the versioning tree (can it be more than a strict
  tree?). If this document is expected to work with different
  versioning schemes, it seems this document needs to spell out which
  requirements a versioning scheme has to satisfy for this document to
  work properly (e.g., that there is a defined partial order on
  version labels).

     If a module import statement could resolve to more than one module
     revision defined in the datastore schema, and none of those revisions
     are implemented, then the import MUST resolve to the module revision
     with the latest revision date.

  Why has the latest revision date been chosen here? Just asking...
  And has this anything to do with the versioning work or was this
  just a convenient place to fix the ambiguity?

* 5.1. Updates to ietf-yang-library

  Is this section here because it is essential to fix this ambiguity
  for the versioning work or is this here just because it was a
  convenient moment to fix this ambiguity?

7.1. Guidelines for YANG module authors

     [...] If all dependencies have been updated to not import specific
     revisions of a module, then the corresponding revision statements can
     be removed from that module.

  This seems to be somewhat pointless for open standard YANG modules
  since there is no effective procedure I can imagine to determine
  whether all dependencies have been updated. (Well, not sure what
  dependencies means, I assume it means there are no modules anymore
  importing a given revision.)

7.1.1. Making non-backwards-compatible changes to a YANG module

     o  NBC changes can be sometimes be done incrementally using the
        "deprecated" status to provide clients time to adapt to NBC
        changes.

   Not sure what this means. Also note the double 'be'.

     o  NBC changes are done at once, i.e. without using "status"
        statements.  Depending on the change, this may have a big impact
        on clients.

   Not sure either, likely because I did not understand the previous
   item. The third bullet also leaves me a bit puzzled. It seems as if
   there is a relationship between NBC changes and status changes.  If
   so, this may need to be spelled out more clearly.

   The detailed description of the "incremental" approach that follows
   is helpful. Perhaps the bullets can simply be removed and we keep
   the incremental approach as the suggested one?

8. Module Versioning Extension YANG Modules

   - Replace '\d{4}-\d{2}-\d{2}' with the pattern used in RFC6991bis
     (the \d does some surprising things, so I dropped it everywhere).

   - RFC6991bis introduces the type revision-identifier but given this
     work is getting stable it seems more logical to call the type
     revision-date. This is also the term RFC 7950 uses. And I think
     revision-date should be defined in ietf-yang-revisions and not in
     RFC6991bis. (I guess we once called it revision-identifier
     because there was no clear understanding yet that we will have a
     revision-date and a revision-label.) Note that dropping the
     definition of revision-identifier from RFC6991bis and defining a
     revision-date type in ietf-yang-revisions also causes the
     dependency on ietf-yang-types to go away.

9. Contributors

  - Typo 'Discussons'

  - Perhaps just list the names of the contributors comma separated
    instead of making this a longish list.

10. Security Considerations

  - I wonder whether confusion about versions can lead to exploitable
    vulnerabilities. If client and server disagree on the meaning of
    values, this may lead to exploitable situations. With the move to
    allow NBC changes, a server may implement different semantics for
    a given leaf and this probably deserves a warning. In a world
    where semantics of definitions can change, client and server need
    to be sure they work with compatible definitions.

  - Another scenario to mention could be a system provisioning NACM
    rules. Such a system may fail generating proper access control
    rules if there is ambiguity about the version of a YANG module
    implemented by the server for which NACM rules are provisioned.
    In fact, a system provisioning NACM rules may need to provision
    rules that continue to work even if client and server negotiate a
    version for their interactions.

A. Examples of changes that are NBC

   - I believe enlarging the value set of a data node is allowed, the
     wording 'any changes that change or reduce the allowed value set'
     seems to disallow this. Perhaps you wanted to say 'any changes
     that reduce the allowed value set' instead?

* RFC 7950 compatibility

  An RFC 7950 compliant implementation will treat

     import example-module {
       rev:revision-or-derived 2.0.0;
     }

  as if	were

     import example-module;

  and hence it may import a revision that is not in the 2.0.0 branch.
  I believe this is a problem. An RFC 7950 implementation will also
  consider deprecate -> obsolete a legal BC change. I believe that
  changing the YANG RFC 7950 update rules fundamentally must be done
  in such a way that parsers not understanding the new rules fail
  instead of producing surprising results (e.g. by bumping the YANG
  version number.)

  Even if this document is implemented by a parser, the parser may get

     import example-module {
       rev:revision-or-derived omicron;
     }

  without knowing what the possible labels are and what the order
  relationship of them is. It seems that the revision-label-scheme
  statement has to be mandatory and a parser needs to stop if the
  announced revision-label-scheme is unknown to the parser.


On Mon, Feb 21, 2022 at 12:20:34PM -0500, Lou Berger wrote:
> 
> 
> All,
> 
> This starts working group last call on
> https://datatracker.ietf.org/doc/draft-ietf-netmod-yang-module-versioning/
> 
> The working group last call ends on March 7 th.
> Please send your comments to the working group mailing list.
> 
> Positive comments, e.g., "I've reviewed this document
> and believe it is ready for publication", are welcome!
> This is useful and important, even from authors.
> 
> Please note that once WG Last call is complete, this document will be held
> and submitted as a set with the other versioning documents (once they are
> ready) for publication request to the IESG.
> 
> Thank you,
> Lou (Co-Chair & doc Shepherd)
> 
> 
> 
> 
> _______________________________________________
> netmod mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/netmod

-- 
Jürgen Schönwälder              Jacobs University Bremen gGmbH
Phone: [phone]         Campus Ring 1 | 28759 Bremen | Germany
Fax:   [phone]         <https://www.jacobs-university.de/>