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

[Reshad Rahman <[e-mail]>]

 Thank you Jürgen for the detailed review. Apologies for the delayed response, some of the comments have led to substantial discussions in the weekly meetings.

    On Sunday, March 6, 2022, 06:08:04 AM EST, Jürgen Schönwälder <[e-mail]> wrote:  
 
 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.

<RR> We have updated text, captured in issues #145 and #146.
* 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.

<RR> Yes we should remove the RFC 2119 language. We would like to keep the option of having the # form since having the revision in the name is convenient. Issue #148.
* 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.
<RR> We can only have 1 branch in the revision history of a specific module revision. If we apply changes in 1 branch to another branch (e.g. the changes in 2.x branch applied into the 3.x branch), it would be 1 update in the 3.x branch with new date and revision. We would not be getting any of the history from the 2.x branch. <RR> The rev:non-backwards-compatible marker is specific to a particular revision in that branch. e.g if it's used with version 3.0, that means 3.0 is NBC with the previous revision, e.g. 2.5.3, in the linear history of 3.0.  It does not convey any information wrt 3.0's compatibility with other branches.Issue #147 for text clarification.
* 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?

<RR> This has already been discussed in another thread. Issue #156.
  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?<RR> If the imported module has a revision label then the revision label should be used in the import. If not, we can use the date: this allows use of revision-or-derived even when the imported module does not have a revision label.<RR> There is no partial order on the labels, i.e just looking at a label is not enough, you need to look at the revision history.
      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)?
<RR> No you can't.
  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.
<RR> This was mentioned in IETF114 presentation. Please see slides 11 and 12 in https://datatracker.ietf.org/meeting/114/materials/slides-114-netmod-42-netmod-module-versioning-draft-update-01.pdf
  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.

<RR> This is not a compatibility constraint, as per the slides above, it's to improve the import functionality in RFC 7950.
      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.<RR> We are currently discussing making revision-or-derived a "hint" rather than a hard rule. Issue #158.

  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).
<RR> We need to clarify the text. 
    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...<RR> The goal is to have both the client and server to have the same (predictable) schema tree.
  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.)<RR> Correct, this applies for vendor models and we should clarify.
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?
<RR> Sounds good.
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.
<RR> Sounds good to me. We still need to discuss this with the weekly meeting participants.
9. Contributors

  - Typo 'Discussons'

  - Perhaps just list the names of the contributors comma separated
    instead of making this a longish list.
<RR> Ack.
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.
<RR> Currently many vendors keep making NBC changes to YANG modules, even though it is prohibited by RFC 7950. We believe versioning improves the current situation since there shouldn't be any surprises anymore. Not sure what exploitable situations you have in mind. 
  - 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.
<RR> Wouldn't that just be a defective implementation, i.e. a bug?
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?
<RR> Yes although reducing/increasing could also be confusing, e.g. if we change 1..50 to 25..100, we are actually increasing the allowed value set but the change is NBC. What about "any changes that remove any previously allowed values from the allowed value set of the data node..."? 
* 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.
<RR> Please see slide 13 @ https://datatracker.ietf.org/meeting/114/materials/slides-114-netmod-42-netmod-module-versioning-draft-update-01.pdf
Regards,Reshad (on behalf of authors + contributors).
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/>

_______________________________________________
netmod mailing list
[e-mail]
https://www.ietf.org/mailman/listinfo/netmod