[Ivy] Re: YANG Doctors Early Review for "A YANG Module for Entitlement Inventory" - draft-ietf-ivy-inventory-entitlement-04

[Marisol <[e-mail]>]

We want to thank you Acee, on behalf of the authors, for the detailed review.

Really appreciate it!,
In the next coming days we will work on it addressing all the points
that you are highlighting.
Many thanks,
Marisol

On Mon, Jul 27, 2026 at 7:56 PM Acee Lindem <[e-mail]> wrote:
>
> Document: draft-ietf-ivy-inventory-entitlement
> Reviewer: Acee Lindem
> Review Date: 2026-07-27
> IETF LC End Date: N/A (Early Review)
> Intended Status: Proposed Standard.
>
> This is YANG doctor early review on the YANG data module
> ietf-entitlement-inventory.yang. The YANG data module includes augments
> to the network-inventroy.yang YANG module to expose the entitlements
> for network elements in the network inventory. Additionally, the data
> module includes the entitlement restrictions and relationships between
> entitlements (i.e., the hierarchy).
>
> The draft also includes a description of the framework for entitlements.
> Interestly, the provisioning of entitlements is outside the scope of the
> draft and all the data nodes in the module are read-only. Finally, the
> draft contains a large number of examples (8) for various entitlement
> use cases.
>
> The ietf-network-entitlement.yang data module augments the
> ietf-network-inventory.yang data module and is well-structured. As this
> is an early review, there are a number of issues that need to be fixed
> prior to WG last call. I've characterized these by the type of issue.
>
> Formating issues:
>
>  1. All the YANG trees are formatted without line constraints and
>     exceed the supported RFC page length. Please reformat with
>
>         pyang -f tree --tree-line-length 66 ietf-network-inventory.yang
>
>  2. Many of the JSON example lines also exceed 69, the RFC maximum.
>
>  3. There are a number of unprintable characters in the draft. I've tried
>     to removed these in the nits.
>
> RFC 9907 Issues:
>
>   1. Missing References for imported YANG data modules:
>
>      RFC 7950 — the module is yang-version 1.1. RFC 6020 is cited
>                 correctly the registry.
>      RFC 9911 — ietf-yang-types is imported for date-and-time.
>      RFC 8340 — tree diagrams in sections 3.2, 3.3, 3.5, and 3.7.1.
>      RFC 7951 — all of section 4 is JSON encoding.
>      RFC 6241/8040/8446 for the security boilerplate.
>
>    2. Similiarly, the imported modules should contain YANG
>       reference statements for the corresponding drafts for RFCs.
>
>    3. The YANG description statement and the YANG revision statement
>       should reference the current version of the draft. Also, the
>       revision statement should a YANG refernce statement. There are
>       plenty of examples. Also, no need to have revision
>       statements for revisions prior to publication - just update the
>       single revision.
>
>    4. Reference to [BaseInventory] draft should be normative rather
>       than informative.
>
>    5. Need <CODE BEGINS> / <CODE ENDS> for ietf-network-entitlements.yang
>       source code.
>
>    6. The "Security Considerations" doesn't follow the template in
>       RFC 9907. Also, the data paths of the sensitive leaves need
>       to be listed.
>
>    7. Drop the "-t" on "entitlement-state-t". While this is sometimes
>       done in C, it shouldn't be done in YANG for typedefs.
>
>
> YANG Module Problems and Suggestions:
>
>    1. The installed-entitlements/entitlement/entitlement-id references a
>
>            type leafref {
>                path "/inv:network-inventory/ei:entitlements"
>                   + "/ei:entitlement/ei:entitlement-id";
>             }
>
>       Since require-instance is true, an entitlement can't be returned
>       unless the corresponding network-element exists. Consider
>       "require-instance false;".
>
>
>    2. Same probllem entitlement-attachment/assets/{elements,components}
>       since the assets can't be returned unless both the network-element
>       and the compoent exists. Consider "require-instance false;".
>
>            type leafref {
>                        path "/inv:network-inventory"
>                           + "/inv:network-elements"
>                           + "/inv:network-element"
>                           + "[inv:ne-id=current()/../network-element]"
>                           + "/inv:components/"
>                           + "inv:component/inv:component-id";
>             }
>
>    3. Should "max-value" and "current-value" be uint64 rather than uint32
>       for extendibility to restrictions with units that can exceed 4.2G?
>
>    4. Section 5.2 says:
>
>         Network elements SHOULD generate notifications when installed
>         entitlements are approaching expiration.  The notification timing and
>
>       However, the YANG module doesn't include definitiions for notifications.
>
>    5. Rename "parent-entitlement-uid" to "parent-entitlement-id" consistent
>       with the referenced leaf. Fix in json example as well.
>
>    6. The identity hierarchy of "basic-capability-class" and
>       "basic-capability-description" is awkward. Why isn't it
>       "basic-capability-list" rather than "basic-capability-description"?
>
>           *Basic capability class*: The module defines basic-capability-
>       description as a simple capability class using only identifiers
>       and descriptions.  This supports implementations that present
>       capabilities as straightforward lists.
>
>    7. For the holder leaf lists, organization-names/organizations and
>       user-names/users, do we really need an enclosing containter? I'm not
>       sure what it buys you. If needed, maybe organizations/organization
>       and users/user for the leaf lists.
>
>    8. If "universal-access" is true for entitlement attachments, does this
>       mean that holders and assets cannot be specified? What are the intended
>       semantics?
>
> Other Issues:
>
>    1. Section 3.5 says "This point requires further exploration in future
>       instances of this document."
>
>    2. Section 2 includes:
>
>            *  ToBeUpdated(TBU) Open Issue for the IVY WG, to include:
>
>          <<Update Glossary under Network Inventory draft, [BaseInventory].  We
>          need at least formal definitions of "capability" and "entitlement".>>
>
>        However, it seems these are already convered in the document.
>
>    3. In Section 3.6.4:
>
>        When a capability lists multiple supporting entitlements, the
>        entitlement-state/allowed field MUST reflect the combined effect of
>        all required entitlements.  If any required entitlement is missing,
>        expired, or revoked, the allowed leaf should be false.
>
>      Since there is "MUST reflect", shouldn't it be "the allowed leaf SHOULD
>      be false."? Uppercase "SHOULD"?
>
>    4. The tree disagrams in sections 3.2, 3.3, and 3.5 are snipped without
>       including the  augment line from the full tree. It would be clearer to
>       include the augment line.
>
> Editorial suggestions:
>
> 183c183
> <    Network elements provide capabilities‚ i.e., functions related to
> ---
> >    Network elements provide capabilities, i.e., functions related to
> 288c288
> <    rights to use that capability—even if access to the device itself is
> ---
> >    rights to use that capability, even if access to the device itself is
> 589c589
> <    entitlement—whether active or not.
> ---
> >    entitlement, whether active or not.
> 961c961
> <    when necessary—specifically when each component has its own set of
> ---
> >    when necessary, specifically when each component has its own set of
> 1052c1052
> <    Other licenses should be openly constrained to a geographic location.
> ---
> >    Other licenses should be specifically constrained to a geographic location.
> 1069c1069
> <    Some entitlements are inherently associated with a holder, such as
> ---
> >    Some entitlements are inherently associated with a holder, such as an
> 1074c1074
> <    holders, such as people under a jurisdiction or a geographical area.
> ---
> >    holders, such as people under a jurisdiction or in a geographical area.
> 1195c1195
> <    expired, or revoked, allowed should be false.  The in-use field
> ---
> >    expired, or revoked, the allowed leaf should be false.  The in-use leaf
> 1260c1260
> <         Copyright (c) 2025 IETF Trust and the persons identified as
> ---
> >         Copyright (c) 2026 IETF Trust and the persons identified as
> 1423c1423
> <                 capabilities list exist and the information system
> ---
> >                 capabilities list entry exists and the information system
> 1521c1521
> <                   current allowed state. this container
> ---
> >                   current allowed state. This container
> 1523c1523
> <                   An empty list of supporting-entitlement means
> ---
> >                   An empty list of supporting-entitlements means
> 1662,1664c1662,1664
> <                "Stock Keeping Unit - vendor's catalog/ordering number
> <                 for this entitlement. Used for procurement and asset
> <                 management integration.";
> ---
> >                "Stock Keeping Unit (SKU) - vendor's catalog/ordering
> >                 number for this entitlement. Used for procurement and
> >                 asset management integration.";
> 1756c1756
> <                 statements due to XPath 1.0 limitations, management
> ---
> >                 statements due to XPath 1.1 limitations, management
> 2429c2429
> <                       "description": "Maximum OSPF neighbor adjacencies, just to give an example :)",
> ---
> >                       "description": "Maximum OSPF neighbor adjacencies",
> 4443c4443
> <    handling is implementation-specific but SHOULD provide sufficient
> ---
> >    handling are implementation-specific but SHOULD provide sufficient
>
> Thanks,
> Acee
>
>
> _______________________________________________
> Ivy mailing list -- [e-mail]
> To unsubscribe send an email to [e-mail]