[yang-doctors] draft-ietf-netconf-notif-envelope-05 early Yangdoctors review

[Ebben Aries via Datatracker <[e-mail]>]

Document: draft-ietf-netconf-notif-envelope
Title: Extensible YANG Model for YANG-Push Notifications
Reviewer: Ebben Aries
Review result: Almost Ready

2 modules in this draft:
- [e-mail]
- [e-mail]

YANG compiler errors or warnings (pyang 2.7.1, yanglint 5.8.6)
- Both primary modules compile clean under pyang --ietf --strict and yanglint
  with no errors or warnings.
- The companion example module (example-foo-extension.yang) compiles clean now
  under yanglint 5.8.6.  The prior -02 review noted a libyang error on this
  module (augment extension target not found) - that has been resolved in a
  newer libyang release and is addressed.  pyang --ietf --strict still flags
  the module with:

  example-foo-extension.yang:27: error: node ietf-yp-notification::envelope of
  type ('ietf-yang-structure-ext', 'structure') cannot be target node

  This appears rather a pyang tooling gap in RFC8791 (sx:augment-structure)
  support
- Unrelated to these modules specifically but just to note that a dependency
  module fails `pyang --ietf`

  yang/[e-mail]:222: error: keyword
  "if-feature" not in canonical order (see RFC 7950, Section 14)
  yang/[e-mail]:276: error: keyword
  "if-feature" not in canonical order (see RFC 7950, Section 14)
  yang/[e-mail]:335: error: keyword
  "if-feature" not in canonical order (see RFC 7950, Section 14)
  yang/[e-mail]:344: error: keyword
  "if-feature" not in canonical order (see RFC 7950, Section 14)

Prior Review Status
--------------------
The following items from Juergen's -02 YANG Doctors review are now addressed in
-05:
- RFC3339 citation in the event-time description: removed, addressed
- RFC1213 reference in the hostname description: removed, addressed
- Inconsistent RFC title strings across reference statements: resolved, all
  references now consistently use "RFC XXXX: Extensible YANG Model for YANG-Push
  Notifications"

The following items from the Joe's -03 OpsDir review are addressed in -05:
- Per-subscription vs. network-wide control rationale: Section 4 now includes an
  explanation of the design decision, addressed
- enable-notification-envelope description omitting subscription-termination
  behavior: the leaf description now includes termination behavior and uses MUST
  language, addressed
- Typo "messsage" in Section 1.1: no longer present, addressed

General Comments
--------------------
This is a follow-up review against draft-05.  The modules are in good shape and
the majority of prior review comments have been resolved.  The remaining issues
below are largely nits and minor inconsistencies, with one functional concern
regarding the optionality of the `contents` anydata node that should be resolved
before publication.

YANG Module - ietf-yp-notification
--------------------
- (Nit) L#57: (module description): "encoded as defined in RFCXXX." is
  inconsistent with all other RFC placeholder occurrences in this file using
  "RFC XXXX".

- L#162: The `contents` anydata node within the sx:structure envelope is
  optional (no `mandatory` statement), yet Sections 3.1.2.1 and 3.1.2.2 of the
  draft both state "Two mandatory child nodes within the 'envelope' container
  are expected, representing the event time and the Notification payload."

- L#190: The `metadata` container added by `augment "/sn:subscriptions"` is
  empty with no children defined in this module.  RFC8407 section 4.14.1 states
  that use of non-presence containers SHOULD be considered carefully. An empty
  container used purely as an extension anchor is unusual - the description of
  the container itself should likely make the intent explicit, e.g. "This
  container is intended to be augmented by modules defining optional
  notification metadata extensions."

- No data node in this module carries a `reference` statement.  Per RFC8407
  section 4.14, if the data definition semantics are defined in an external
  document then a `reference` statement MUST be present.  This applies to all
  nodes: `envelope` (L#106), `metadata` (L#115), `hostname-sequence-number`
  (L#119), `event-time` (L#136), `hostname` (L#143), `sequence-number` (L#151),
  `contents` (L#162), `enable-notification-envelope` (L#174), `metadata`
  (L#190), and `notification-metadata` (L#204).  Several of these embed "Refer
  to Section 4 of RFC XXXX for more details." directly in the description (e.g.
  L#188, L#193) - that normative pointer belongs in a `reference` substatement,
  not the `description`.

- (Nit) L#178: The description of `enable-notification-envelope` wraps
  continuation lines flush with the opening quote rather than one space past it,
  as used consistently elsewhere in the module.  This causes line breaks to fall
  mid-phrase (e.g. "notification-envelope / defined in RFC XXXX", "using / the
  header configured", "Refer to / Section 4"), making the text read as dangling
  fragments.

YANG Module - ietf-yp-observation
--------------------
- L#33: (contact): The WG Web URL is malformed and stale:
    <http:/tools.ietf.org/wg/netconf/>
  This has a single slash after the colon and uses plain HTTP.  Update to match
  ietf-yp-notification:
    <https://datatracker.ietf.org/group/netconf/>

- L#132: The `yang-push-observation-time-supported` leaf description reads:
    "If set to true, publisher supports. If set to false, the
    observation-timestamp is not supported."
  The first sentence appears grammatically incomplete.  Suggest:
    "If set to true, the publisher supports the observation-timestamp and
    point-in-time in notifications. If set to false, these are not supported."

- L#74, L#79: The `timestamp` and `point-in-time` nodes in the
  `yang-push-observation` grouping carry no `reference` statement.  Per RFC8407
  section 4.14, a `reference` statement MUST be present when the semantics are
  defined in an external document.  Notably
  `yang-push-observation-time-supported` (L#126) does carry a `reference`,
  making the omission on the grouping nodes inconsistent.

- (Nit) L#110, L#117: The augment statements reference the grouping with a
  self-prefix (`uses iypo:yang-push-observation;`).  Per RFC7950 section 5.4,
  the module prefix is not required when referencing a grouping defined in the
  same module.  Suggest `uses yang-push-observation;`.

Example validated instance data:

<subscriptions
xmlns="urn:ietf:params:xml:ns:yang:ietf-subscribed-notifications">
  <enable-notification-envelope
      xmlns="urn:ietf:params:xml:ns:yang:ietf-yp-notification">true</enable-notification-envelope>
</subscriptions>

<system-capabilities
xmlns="urn:ietf:params:xml:ns:yang:ietf-system-capabilities">
  <subscription-capabilities
      xmlns="urn:ietf:params:xml:ns:yang:ietf-notification-capabilities">
    <notification-metadata
    xmlns="urn:ietf:params:xml:ns:yang:ietf-yp-notification">
      <envelope>true</envelope>
      <metadata>
        <hostname-sequence-number>true</hostname-sequence-number>
      </metadata>
    </notification-metadata>
    <yang-push-observation-time-supported
        xmlns="urn:ietf:params:xml:ns:yang:ietf-yp-observation">true</yang-push-observation-time-supported>
  </subscription-capabilities>
</system-capabilities>

<envelope xmlns="urn:ietf:params:xml:ns:yang:ietf-yp-notification">
  <event-time>2026-06-24T10:00:00Z</event-time>
  <hostname>router.example.com</hostname>
  <sequence-number>42</sequence-number>
  <contents>
    <push-update xmlns="urn:ietf:params:xml:ns:yang:ietf-yang-push">
      <id>101</id>
      <timestamp
      xmlns="urn:ietf:params:xml:ns:yang:ietf-yp-observation">2026-06-24T09:59:55Z</timestamp>
      <point-in-time
      xmlns="urn:ietf:params:xml:ns:yang:ietf-yp-observation">current-accounting</point-in-time>
      <datastore-contents>
        <interfaces xmlns="urn:ietf:params:xml:ns:yang:ietf-interfaces">
          <interface>
            <name>eth0</name>
            <admin-status>up</admin-status>
            <oper-status>up</oper-status>
            <type
            xmlns:ianaift="urn:ietf:params:xml:ns:yang:iana-if-type">ianaift:ethernetCsmacd</type>
          </interface>
        </interfaces>
      </datastore-contents>
    </push-update>
  </contents>
</envelope>

<push-update xmlns="urn:ietf:params:xml:ns:yang:ietf-yang-push">
  <id>101</id>
  <datastore-contents>
    <interfaces xmlns="urn:ietf:params:xml:ns:yang:ietf-interfaces">
      <interface>
        <name>eth0</name>
        <admin-status>up</admin-status>
        <oper-status>up</oper-status>
        <type
        xmlns:ianaift="urn:ietf:params:xml:ns:yang:iana-if-type">ianaift:ethernetCsmacd</type>
      </interface>
    </interfaces>
  </datastore-contents>
  <timestamp
  xmlns="urn:ietf:params:xml:ns:yang:ietf-yp-observation">2026-06-24T09:59:55Z</timestamp>
  <point-in-time
  xmlns="urn:ietf:params:xml:ns:yang:ietf-yp-observation">current-accounting</point-in-time>
</push-update>

<push-change-update xmlns="urn:ietf:params:xml:ns:yang:ietf-yang-push">
  <id>102</id>
  <datastore-changes>
    <yang-patch>
      <patch-id>patch-1</patch-id>
      <edit>
        <edit-id>1</edit-id>
        <operation>replace</operation>
        <target>/ietf-interfaces:interfaces/interface[name='eth0']/oper-status</target>
        <value>
          <oper-status
          xmlns="urn:ietf:params:xml:ns:yang:ietf-interfaces">down</oper-status>
        </value>
      </edit>
    </yang-patch>
  </datastore-changes>
  <timestamp
  xmlns="urn:ietf:params:xml:ns:yang:ietf-yp-observation">2026-06-24T10:00:01Z</timestamp>
  <point-in-time
  xmlns="urn:ietf:params:xml:ns:yang:ietf-yp-observation">state-changed</point-in-time>
</push-change-update>