[yang-doctors] Yangdoctors early review of draft-ietf-ippm-ioam-yang-03

[Andy Bierman via Datatracker <[e-mail]>]

Reviewer: Andy Bierman
Review result: Ready with Issues


Draft: draft-ietf-ippm-ioam-yang-03
Module: ietf-ioam
Revision: 2022-01-25

Major Issues: none

Minor Issues:

1) when-stmts using identities

The preferred method is to use the 'derived-from-or-self'
https://datatracker.ietf.org/doc/html/rfc7950#section-10.4.2

OLD:

  when "../filter-type = 'ioam:acl-filter'";


NEW:

  when "derived-from-or-self(../filter-type, 'ioam:acl-filter')";

This correction allows for derived identities,
not just the specified identity.  The problem is that the
prefix to module mapping is only defined for the XPath function,
not for a plain string. The 'ioam' prefix must be resolved correctly
by the YANG compiler.

2)  /ioam/ioam-profiles/admin-config/enabled leaf description

        description
          "When true, IOAM configuration is enabled for the system.
           Meanwhile, the IOAM data-plane functionality is enabled.";

This object needs more clarification.

  - this object must be true before anything in the
    /ioam/ioam-profiles/ioam-profile subtree can be edited.
  - This means it cannot be changed in the same edit as the
    objects it affects.
  - if false, any config in place is used (is this true?)
  - The 2nd sentence is not clear how it relates to the YANG module.
  - Need to be clear (in terms of YANG datastore editing)
    what true or false exactly means.


3) Terse descriptions

A few description-stmts are too terse.
There must be something that can be said in a full sentence.

      leaf node-action {
        type ioam-node-action;
        description "node action";
      }

4) Use interface-ref data type

Object /ioam/ioam-info/available-interface/if-name

OLD:

     type leafref {
        path "/if:interfaces/if:interface/if:name";
     }

NEW:

     type if:interface-ref;

5) Use of ordered-by user

List /ioam/ioam-profiles/ioam-profile is user-ordered.
Describe all server processing requirements that are
related to the order of these list entries.

6) Use of plain 'string' as a list key

List key /ioam/ioam-profiles/ioam-profile/profile-name is
an unconstrained string.

Most servers do not allow corner-case string values
(such as an empty string).

Consider using yang:yang-identifier type or at least
length 1..max to disallow empty strings.

7) No mandatory functionality

There is a YANG feature for each profile so all profiles
are optional-to-implement.

Is this because there is no WG agreement on any mandatory
functionality?  Some explanation that a server is expected
to support at least one profile should be added in this case.


Nits
------

1) Some description text starts with a lowercase letter,
   and should be capitalized.

2) Some sentences start with 'And ...' which could be
   added to the previous sentence instead.

3) Remove the 'mandatory true;' statement in leaf profile-name.
   A list key leaf is already mandatory.