Re: [netconf] Yangdoctors last call review of draft-ietf-netconf-https-notif-06

Mahesh Jethanandani <mjethanandani@gmail.com> Tue, 09 February 2021 04:27 UTC

Return-Path: <mjethanandani@gmail.com>
X-Original-To: netconf@ietfa.amsl.com
Delivered-To: netconf@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 4980A3A18D5; Mon, 8 Feb 2021 20:27:05 -0800 (PST)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -2.097
X-Spam-Level:
X-Spam-Status: No, score=-2.097 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, FREEMAIL_FROM=0.001, HTML_MESSAGE=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, URIBL_BLOCKED=0.001] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (2048-bit key) header.d=gmail.com
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id hT0td8yn_JdN; Mon, 8 Feb 2021 20:27:02 -0800 (PST)
Received: from mail-pj1-x102d.google.com (mail-pj1-x102d.google.com [IPv6:2607:f8b0:4864:20::102d]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 14CD63A18DA; Mon, 8 Feb 2021 20:26:58 -0800 (PST)
Received: by mail-pj1-x102d.google.com with SMTP id my11so869211pjb.1; Mon, 08 Feb 2021 20:26:58 -0800 (PST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:message-id:mime-version:subject:date:in-reply-to:cc:to :references; bh=OVOUtULNLl2QWPqSgPYp4Q9vKtyUjmbFyyZlBlBQgLY=; b=fwCKcPZNsr0QXnqNsPvRjK47MD3OEmqhp1Wx4s31/OcBgVLzc+rxaAt8LtQD4a2T9x YHlVN5xc3n9AqIv4lr6nwgPtA5HjlP3pHYz4iDQgE9atJe3HZp2POvgvDV0v7seyil9d oKXcGkIbTpffz63WyXHe1t70BRprJDTxdGk2gja21ht+SezZi+6YTKMeOPSM36Tb1+zH zhnL0EwS6CPmTakqLFft8TCVOsQDw12JUQSYA6tR6RGmyaKqfS6DgrWGSH9+Mi8Vykjh 3BTfqBJ8NTvUkgFCqgeUqGhktTAWJrmcD5V4cM0yVDuSifwwfeRxfWCpLMVOr8Qdr3zD NWyA==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:message-id:mime-version:subject:date :in-reply-to:cc:to:references; bh=OVOUtULNLl2QWPqSgPYp4Q9vKtyUjmbFyyZlBlBQgLY=; b=ot3QTS+PGrSgYwU8hu5zi0Bz1hc3uyG4hE7eVs+3scmmGob68Ceqmz/4i824BDKQXp YM3T/+lF2FK/dp8JTwyg1Ya1A1IDiY5UmKPsLvpmhsc7deKRcgwF6QeTMCXS4q4JNRCn 4oRr2ACT56jel+JI71FlUxriEXDKs2oWL75cBKSRdNwrLx6U4ZBz6hn2oqZC6UnT0Day cab1DJbheGtCehMJMO/Xhzf8GWHC6YgjHQZlsoqYxELkc8wAhukxWxGdbo9cp6r452oO XKP+m7G7Wr1kHjm2kOiat9SFk3NDBAXvwKgaoTzAOuYvQ5yCmSvWT0YB0EbBWWzUo0MV gZkg==
X-Gm-Message-State: AOAM532BCGdIRouzwwaLuk/2LdqO56ufRTlzRwBeiXhDixcn7XuGGi0b DOYwFy81jqYBimlV0cvOXtc=
X-Google-Smtp-Source: ABdhPJw4f6x6dH8HnDrTYxG3Co47/1ECszb39TZaJUcaHi+Xbdh4ydf0VCwq4EtPJ9N4Pl/QuATyTw==
X-Received: by 2002:a17:902:7c11:b029:e1:8692:9137 with SMTP id x17-20020a1709027c11b02900e186929137mr19869673pll.50.1612844818276; Mon, 08 Feb 2021 20:26:58 -0800 (PST)
Received: from ?IPv6:2601:647:5600:5020:a506:3d59:a640:5116? ([2601:647:5600:5020:a506:3d59:a640:5116]) by smtp.gmail.com with ESMTPSA id h15sm16860260pfo.193.2021.02.08.20.26.56 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Mon, 08 Feb 2021 20:26:57 -0800 (PST)
From: Mahesh Jethanandani <mjethanandani@gmail.com>
Message-Id: <DFCF64EB-5CCF-44EA-832B-C72EC6B1B17D@gmail.com>
Content-Type: multipart/alternative; boundary="Apple-Mail=_4B7053B2-CE3A-4C68-87ED-47E84A1402F2"
Mime-Version: 1.0 (Mac OS X Mail 13.4 \(3608.120.23.2.4\))
Date: Mon, 8 Feb 2021 20:26:56 -0800
In-Reply-To: <161176691048.5494.2373231790931856179@ietfa.amsl.com>
Cc: yang-doctors@ietf.org, draft-ietf-netconf-https-notif.all@ietf.org, last-call@ietf.org, netconf@ietf.org
To: Acee Lindem <acee@cisco.com>
References: <161176691048.5494.2373231790931856179@ietfa.amsl.com>
X-Mailer: Apple Mail (2.3608.120.23.2.4)
Archived-At: <https://mailarchive.ietf.org/arch/msg/netconf/1SCMDiLWjwSDHgQS2XXe4oe0EO8>
Subject: Re: [netconf] Yangdoctors last call review of draft-ietf-netconf-https-notif-06
X-BeenThere: netconf@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: NETCONF WG list <netconf.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/netconf>, <mailto:netconf-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/netconf/>
List-Post: <mailto:netconf@ietf.org>
List-Help: <mailto:netconf-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/netconf>, <mailto:netconf-request@ietf.org?subject=subscribe>
X-List-Received-Date: Tue, 09 Feb 2021 04:27:05 -0000

Hi Acee,

Thank you first of all for the review of the document. You reviewed -06, and we have since submitted -07. I will in my responses reflect the changes in -07 that might affect the comments you have provided.

> On Jan 27, 2021, at 9:01 AM, Acee Lindem via Datatracker <noreply@ietf.org> wrote:
> 
> Reviewer: Acee Lindem
> Review result: Almost Ready
> 
> The document is almost ready for WG Last Call. I have the following comments:
> 
> Model ietf-sub-notif-recv-list:
> 
>    Seems odd to have a model that adds a choice statement without any
>    valid options. I realize you expect other transports in the future but
>    having a separate module the one defined in the same draft seems like a
>    profligate design choice.

The draft contains two models as you have noted. While the ietf-sub-notif-recv-list adds a choice statement, the second model goes on to adding a case statement for that choice statement. We could have hard coded the two models together, but believe that by separating them, we offer the flexibility to implementations that may choose not to implement both the models. Also, other transports can augment "receiver-instances", without including the entire http transport model.

If it helps, we can add a comment to that effect in the model to explain our motivation.

> 
> Model ietf-sub-notif-recv-list:
>    1. The prefix "hn" seems inconsistent to lose the context. I'd use
>        "snrlhn".  This wouldn't be an issue if you combined the modules.

The updated model in -07 now carries the prefix “hnt”.

>    2. This comment is incomprehensible:
> 
>               // create the logical impossibility of enabling "tcp"
>               // transport

Would it help if we expanded the comment to say:

// The "http-client-stack-grouping” supports both HTTP and HTTPS stacks by default. This draft, however supports HTTPS only. The following 
// if-feature statement makes it logically impossible to configure HTTP transport.

> 
> In the IANA section, the security considerations for the media types
> point to this draft's security considerations. However, they don't
> include the referenced information. It seems these IANA references
> should point to other RFCs.

The updated IANA section no longer carries the media-types.

> 
> It should be noted that ietf-subscribed-notifications has a number of YANG
> errors and warnings.
> 
> I also have the following editorial comments:

> Acee-Lindems-iMac-2:Desktop acee$ diff -c
> draft-ietf-netconf-https-notif-06.txt.orig
> draft-ietf-netconf-https-notif-06.txt ***
> draft-ietf-netconf-https-notif-06.txt.orig  2021-01-27 10:06:29.000000000 -0500
> --- draft-ietf-netconf-https-notif-06.txt       2021-01-27 11:57:13.000000000
> -0500
> ***************
> *** 17,23 ****
>     This document defines a YANG data module for configuring HTTPS based
>     configured subscription, as defined in RFC 8639.  The use of HTTPS
>     maximizes transport-level interoperability, while allowing for
> !    encoding selection from text, e.g.  XML or JSON, to binary.
> 
>  Status of This Memo
> 
> --- 17,23 ----
>     This document defines a YANG data module for configuring HTTPS based
>     configured subscription, as defined in RFC 8639.  The use of HTTPS
>     maximizes transport-level interoperability, while allowing for
> !    encoding selection from text, e.g.,  XML or JSON, to binary.

The abstract section has been updated.

> 
>  Status of This Memo
> 
> ***************
> *** 103,109 ****
>     documents.  This document defines two YANG 1.1 [RFC7950] data
>     modules, one for augmenting the Subscription to YANG Notifications
>     [RFC8639] to add a transport type, and another for configuring and
> !    managing HTTPS based receivers for the notifications.
> 
> --- 103,109 ----
>     documents.  This document defines two YANG 1.1 [RFC7950] data
>     modules, one for augmenting the Subscription to YANG Notifications
>     [RFC8639] to add a transport type, and another for configuring and
> !    managing HTTPS-based receivers for the notifications.

Fixed.

> 
> ***************
> *** 118,124 ****
>     the same receiver instance.  The second module describes how to
>     enable the transmission of YANG modeled notifications, in the
>     configured encoding (i.e., XML, JSON) over HTTPS.  Notifications are
> !    delivered in the form of a HTTPS POST.  The use of HTTPS maximizes
>     transport-level interoperability, while the encoding selection pivots
>     between implementation simplicity (XML, JSON) and throughput (text
>     versus binary).
> --- 118,124 ----
>     the same receiver instance.  The second module describes how to
>     enable the transmission of YANG modeled notifications, in the
>     configured encoding (i.e., XML, JSON) over HTTPS.  Notifications are
> !    delivered in the form of an HTTPS POST.  The use of HTTPS maximizes
>     transport-level interoperability, while the encoding selection pivots
>     between implementation simplicity (XML, JSON) and throughput (text
>     versus binary).
> ***************

Fixed.

> *** 196,202 ****
> 
>     In the case of "pipelining", the flow of messages would look
>     something like this.  This example shows the flow assuming that
> !    Subscribed Notifications is used and therefore a <subscription-
>     started> notification is sent before sending the first notification.
>     The example would be the same for when Subscribed Notification is not
>     used by removing the first POST message for <susbscription-started>.
> --- 196,202 ----
> 
>     In the case of "pipelining", the flow of messages would look
>     something like this.  This example shows the flow assuming that
> !    Subscribed Notification is used and therefore a <subscription-
>     started> notification is sent before sending the first notification.
>     The example would be the same for when Subscribed Notification is not
>     used by removing the first POST message for <susbscription-started>.
> ***************

This section has been rewritten.

> *** 342,350 ****
> 
>  2.1.  Introduction
> 
> !    To learn the capabilities of the receiver, the publisher can issue a
>     HTTPS GET request with Accept-Type set to application/ietf-https-
> !    notif-cap+xml or application/ietf-https-notif-cap+json, with latter
>     as the mandatory to implement, and the default in case the type is
>     not specified.  If the receiver supports capabilities such as binary
>     encoding of data, it can return that as a capability in a response.
> --- 342,350 ----
> 
>  2.1.  Introduction
> 
> !    To learn the capabilities of the receiver, the publisher can issue an
>     HTTPS GET request with Accept-Type set to application/ietf-https-
> !    notif-cap+xml or application/ietf-https-notif-cap+json, with the latter
>     as the mandatory to implement, and the default in case the type is
>     not specified.  If the receiver supports capabilities such as binary
>     encoding of data, it can return that as a capability in a response.
> ***************

Fixed.

> *** 450,456 ****
>  Internet-Draft        HTTPS Configured Subscription        November 2020
> 
> !      Copyright (c) 2018 IETF Trust and the persons identified as
>       the document authors.  All rights reserved.
>       Redistribution and use in source and binary forms, with or
>       without modification, is permitted pursuant to, and subject
> --- 450,456 ----
>  Internet-Draft        HTTPS Configured Subscription        November 2020
> 
> !      Copyright (c) 2021 IETF Trust and the persons identified as
>       the document authors.  All rights reserved.
>       Redistribution and use in source and binary forms, with or
>       without modification, is permitted pursuant to, and subject
> ***************

Fixed.

> *** 536,545 ****
> 
>     This YANG module is a definition of a set of receivers that are
>     interested in the notifications published by the publisher.  The
> !    module contains the TCP, TLS and HTTPS parameters that are needed to
>     communicate with the receiver.  The module augments the ietf-sub-
>     notif-recv-list module to define a transport specific receiver.  As
> !    mentioned earlier, it uses POST method to deliver the notification.
>     The attribute 'path' defines the path for the resource on the
>     receiver, as defined by 'path-absolute' in URI Generic Syntax
>     [RFC3986].  The user-id used by Network Configuration Access Control
> --- 536,545 ----
> 
>     This YANG module is a definition of a set of receivers that are
>     interested in the notifications published by the publisher.  The
> !    module contains the TCP, TLS, and HTTPS parameters that are needed to
>     communicate with the receiver.  The module augments the ietf-sub-
>     notif-recv-list module to define a transport specific receiver.  As
> !    mentioned earlier, it uses the POST method to deliver the notification.
>     The attribute 'path' defines the path for the resource on the
>     receiver, as defined by 'path-absolute' in URI Generic Syntax
>     [RFC3986].  The user-id used by Network Configuration Access Control
> ***************

This section of the module has been rewritten.

> *** 639,645 ****
>       description
>         "YANG module for configuring HTTPS base configuration.
> 
> !         Copyright (c) 2018 IETF Trust and the persons identified as
>          the document authors.  All rights reserved.
>          Redistribution and use in source and binary forms, with or
>          without modification, is permitted pursuant to, and subject
> --- 639,645 ----
>       description
>         "YANG module for configuring HTTPS base configuration.
> 
> !         Copyright (c) 2021 IETF Trust and the persons identified as
>          the document authors.  All rights reserved.
>          Redistribution and use in source and binary forms, with or
>          without modification, is permitted pursuant to, and subject
> ***************

Fixed.

> *** 704,710 ****
> 
>             container receiver-identity {
>               description
> !                "Specifies mechanism for identifying the receiver.
>                  The publisher MUST NOT include any content in a
>                  notification that the user is not authorized to view.";
> 
> --- 704,710 ----
> 
>             container receiver-identity {
>               description
> !                "Specifies the mechanism for identifying the receiver.
>                  The publisher MUST NOT include any content in a
>                  notification that the user is not authorized to view.";
> 
> ***************

This description statement has been updated.

> *** 791,801 ****
> 
>  6.  Receiving Event Notifications
> 
> !    Encoding notifications for the HTTPS notifications is the same as the
>     encoding notifications as defined in RESTCONF [RFC8040] Section 6.4,
>     with the following changes.  Instead of saying that for JSON-encoding
>     purposes, the module name for "notification" element will be "ietf-
> !    restconf, it will say that for JSON-encoding purposes, the module
>     name for "notification" element will be "ietf-https-notif".
> 
>     With those changes, the SSE event notification encoded JSON example
> --- 791,801 ----
> 
>  6.  Receiving Event Notifications
> 
> !    Encoding notifications for the HTTPS notifications is the same as for
>     encoding notifications as defined in RESTCONF [RFC8040] Section 6.4,
>     with the following changes.  Instead of saying that for JSON-encoding
>     purposes, the module name for "notification" element will be "ietf-
> !    restconf", it will say that for JSON-encoding purposes, the module
>     name for "notification" element will be "ietf-https-notif".
> 
>     With those changes, the SSE event notification encoded JSON example
> ***************

This and all the following sections have been updated. 

Thanks.

> *** 815,826 ****
> 
>  7.  IANA Considerations
> 
> !    This document registers two URI, two YANG module and two Media Types.
> 
>  7.1.  URI Registration
> 
>     in the IETF XML registry [RFC3688].  Following the format in RFC
> !    3688, the following registration is requested to be made:
> 
>     URI: urn:ietf:params:xml:ns:yang:ietf-http-notif
>     URI: urn:ietf:params:xml:ns:yang:ietf-sub-notif-recv-list
> --- 815,826 ----
> 
>  7.  IANA Considerations
> 
> !    This document registers two URIs, two YANG modules, and two Media Types.
> 
>  7.1.  URI Registration
> 
>     in the IETF XML registry [RFC3688].  Following the format in RFC
> !    3688, the following registrations are requested to be made:
> 
>     URI: urn:ietf:params:xml:ns:yang:ietf-http-notif
>     URI: urn:ietf:params:xml:ns:yang:ietf-sub-notif-recv-list
> ***************
> *** 854,860 ****
> 
>  7.3.  Media Types
> 
> ! 7.3.1.  Media Type "application/ietf-https-notif-cap+xml
> 
>  Type name: application
> 
> --- 854,860 ----
> 
>  7.3.  Media Types
> 
> ! 7.3.1.  Media Type "application/ietf-https-notif-cap+xml"
> 
>  Type name: application
> 
> ***************
> *** 917,923 ****
> 
>  Provisional registration? (standards tree only): no
> 
> ! 7.3.2.  Media Type "application/ietf-https-notif-cap+json
> 
>  Type name: application
> 
> --- 917,923 ----
> 
>  Provisional registration? (standards tree only): no
> 
> ! 7.3.2.  Media Type "application/ietf-https-notif-cap+json"
> 
>  Type name: application
> 
> ***************
> *** 1106,1112 ****
>  </config>
> 
> ! 8.2.  Non Subscribed Notification based Configuration
> 
>     In the case that it is desired to use HTTPS notif outside of
>     Subscribed Notifications, there would have to be a module to define
> --- 1106,1112 ----
>  </config>
> 
> ! 8.2.  Non-Subscribed Notification based Configuration
> 
>     In the case that it is desired to use HTTPS notif outside of
>     Subscribed Notifications, there would have to be a module to define
> 
> Thanks,
> Acee
> 
> 

Mahesh Jethanandani
mjethanandani@gmail.com