Re: [netconf] RESTCONF support in private candidates draft

[Kent Watsen <[e-mail]>]

Hi Robert,


> On Mar 17, 2024, at 4:21 PM, Robert Wills (rowills) <[e-mail]> wrote:
> 
> Kent, Andy,
>  
> Thank you both for taking the time to provide feedback.
>  
> In draft-ietf-netconf-privcand-02, which was recently published, I have attempted to incorporate this feedback.
>  
> Summarising:
> With RESTCONF, the private candidate is tied to the client, using the client identity as described in RFC 8040 Section 2.5
> When the private candidate datastore is explicitly referenced in the request, then edits are made to the client’s private candidate and are NOT committed. The client must execute an ietf-netconf:commit operation to commit them.
> When the private candidate datastore is not explicitly referenced in the request, changes are made in a private candidate and the private candidate is instantly committed.
>  
> The bulk of the changes are in Section 4.4.3, with small changes elsewhere to make it hang together.
>  
> Elaborating on each of the points above:
>  
> (1) Private candidate is tied to the client
> On re-reading the draft, I think it should be more explicit that it’s tied to the client identity from 8040. I will make this change in -03.

That would be good.  For example: NETCONF and RESTCONF clients, as identified by the “username".

I would suggest treating NC/RC the same as much as possible.  For instance:
	- replace:	"NETCONF sessions and RESTCONF clients"
	- with: 	"NETCONF and RESTCONF clients"

There may be other places too.




Also this:
	"A private candidate is valid for the duration of the NETCONF session,
	 or the duration of the existence of the RESTCONF client."

Doesn’t make sense to me.  For one, RC clients “existence” are ephemeral.  Next, IDK why we’d ever want an authenticated user’s priv-cand DS to auto-delete.  I think that this draft should, in general, move away from mentioning “clients” and instead focus on “users”.  For instance, as a user, I’d expect to be able to configure my priv-cand DS using NC and be able to see it using RC.   Makes sense?



> (2) Privcand datastore explicitly referenced in the request
> Kent and Andy, you both agree that a candidate datastore that’s instantly committed is not useful. Hopefully what I’ve now written (in Section 4.4.3.2) provides something more useful in implementations that support privcand.
>  
> (3) Privcand datastore not explicitly referenced
> In this case, the private candidate datastore IS automatically committed. This is for backwards compatibility with clients that aren’t aware of private candidates: they are expecting their changes to be committed. Kent, I hope this captures the spirit of what you were after?

Sections 4.4.3.1 and 4.4.3.2 are in the pointing in the right direction, but...

In 4.4.3.1 Imagine :writeable-running, :candidate, and :private-candidate, or some combination thereof, all advertised by the server at the same time.   The general gist of RFC 8040, in all cases, is that the edit goes to the “unified” datastore and the unified datastore is auto-committed.  That said, RFC 8040 stops short of formally defining a term for “unified datastore”, so this section needs to say the same thing using more words.   Specifically, this sentence seems problematic: "...all edits are made to the client's private candidate, and the private candidate is automatically committed.”...

In 4.4.3.2, please add what is meant by “referenced”.  I assume you mean when the URL is like this?  {+restconf}/ds/ietf-datastores:privcand


>  Further comments are very welcome.

Please add Andy as a Contributor too!  ;)

 
> Many thanks,
> Robert.

Kent

 
> From: Andy Bierman <[e-mail] <mailto:[e-mail]>>
> Date: Thursday, 8 February 2024 at 22:02
> To: Kent Watsen <[e-mail] <mailto:[e-mail]>>
> Cc: "Robert Wills (rowills)" <[e-mail] <mailto:[e-mail]>>, "[e-mail] <mailto:[e-mail]>" <[e-mail] <mailto:[e-mail]>>
> Subject: Re: [netconf] RESTCONF support in private candidates draft
>  
>  
>  
> On Thu, Feb 8, 2024 at 1:30 PM Kent Watsen <[e-mail] <mailto:[e-mail]>> wrote:
>> Robert, Andy, et. al.,
>>  
>> 
>> 
>>> On Feb 6, 2024, at 12:35 PM, Andy Bierman <[e-mail] <mailto:[e-mail]>> wrote:
>>>  
>>>  
>>>  
>>> On Mon, Feb 5, 2024 at 4:32 AM Robert Wills (rowills) <rowills=[e-mail] <mailto:[e-mail]>> wrote:
>>>> Hi Netconf Working Group,
>>>>  
>>>> After James and I presented an update on draft-ietf-netconf-privcand-01 at IETF 118, Kent asked about RESTCONF support in the draft, and specifically about the lifecycle of a private candidate when using RESTCONF. This email is to follow up on that question, and to ask the Working Group whether RESTCONF support should be kept in this draft.
>>>> Currently, the draft states that
>>>> "When a private candidate is used by RESTCONF, it exists only for the duration of the RESTCONF request." (draft-ietf-netconf-privcand-01 Section 2.3).
>>>> Later, the draft expands on this by stating that changes made to a private candidate will be committed as part of the same request:
>>>> "When the server advertises the :private-candidate capability and the client does not explicitly reference a datastore in their request, all edits are made to a new private candidate, and the private candidate is committed. This is analagous to the behavior of RESTCONF when the :candidate capability is specified by the server.
>>>> "When the private-candidate datastore is explicitly referenced, edits are made to a new private candidate and the private candidate is committed."
>>>> (draft-ietf-netconf-privcand-01 Section 4.4.3) 
>> It would be better for this draft to define nothing than to define it this way.  
>> But all is not lost.  This is an opportunity to fix it.
>>  
>>>> This wording is intended to preserve the high-level goal of private candidates, namely allowing multiple clients to make independent configuration changes, and to behave similarly to RESTCONF's current behavior for the (shared) candidate. (RESTCONF RFC 8040 Section 1.4: "The candidate MUST be automatically committed to running immediately after each successful edit").
>> RFC 8040 should’ve just said to use the operations supported by the various NETCONF’s capabilities under the {+restconf}/operations resource. 
>>  
>  
> I disagree.
> RESTCONF does not have sessions.
> NETCONF :candidate is session-specific.
> No changes to the existing URI s (e.g. /restconf/data) should be made.
>  
>  
>>>> As far as I'm aware, RESTCONF doesn't currently have a mechanism for editing a datastore and then committing the changes in a separate request. The authors wanted to avoid having to specify such a mechanism, partly because neither of us are RESTCONF experts, and partly because it could also apply to other datastores (such as the “shared” candidate).
>> As a RESTCONF expert, I will help. 
>>  
>> RFC 8040 only says what happens when certain datastores are supported.   It says nothing about what happens when a datastore called “private-candidate” is implemented.  Thus this draft gets to define that behavior, and it doesn’t have to match RFC 8040’s “support” for :candidate.
>>  
>  
> As long as new mechanisms and URIs are used, this should be OK.
>  
> IMO RESTCONF does not really need its own way of doing everything, because the /restconf/operations interface 
> is very easy to use.
>  
> Andy
>  
>> This draft can state:
>>  
>> - when "“private-candidate” is implemented:
>> - edits accumulate in the client-specific candidate datastore
>> - the client-specific candidate datastore can be committed 
>>   using  {+restconf}/operations/ietf-netconf:commit
>> - the client-specific candidate datastore can be discarded
>>   using {+restconf}/operations/ietf-netconf:discard-changes
>>  
>> - if :validate is also implemented:
>> - the client-specific candidate datastore can be validated
>>   using {+restconf}/operations/ietf-netconf:validate
>>  
>> - if :confirmed-commit is also implemented:
>> - the {+restconf}/operations/ietf-netconf:commit operation
>>   can take the “confirmed” attribute
>> - the commit can be canceled using 
>>   {+restconf}/operations/ietf-netconf:cancel-commit
>>  
>> Note:  I am not trying at all to introduce to RESTCONF an equivalency to NETCONF’s <target> attribute.  I’m perfectly okay with the following auto-selection:
>>  
>> if :running implemented:
>> - RC-edits go to <running> and are auto-committed
>> - this is from RFC 8040 and is fine (matches :writable-running behavior)
>>  
>> else if :candidate is implemented:
>> - RC-edits go to <candidate> and are auto-committed
>> - this is from RFC 8040 and is not good (explained above)
>> - but let’s hope that :candidate becomes a thing of the 
>>   past in an NBC-update to RESTCONF
>>  
>> else if :private-candidate is implemented:
>> - see behavior described above.
>> - but, to work, this draft needs to say that neither
>>   :running nor :candidate can be implemented…
>> - this would be goodness 
>>  
>>>> If more work is needed on the RESTCONF support for the private candidate datastore, I think it would be better to remove RESTCONF from this draft and address it in a separate draft later, with an author who's a RESTCONF expert.
>> A common strategy is to have one “place” where protocol-independent behavior is defined, and then a “place” for each protocol, where protocol-specific syntax is defined.   The only question is if the *places* are sections in a draft, or separate drafts (e.g., in the list-pagination work).
>>  
>> IMO, if you want to isolate RESTCONF, then NETCONF should be also be isolated (i.e.: there would be 3 drafts).  Good reasons for doing this are 1) because each is a heavy topic, perhaps too big for all be in one draft) and 2) because independent documents make for better references.
>>  
>>>> Thoughts and comments are very welcome.
>> Thank you for sending out this message.
>>  
>>  
>> From Andy:
>> 
>>> A candidate datastore that can only accept one edit and then automatically commit is not useful.
>>  
>> I agree that it would not be useful.    
>> It was also not useful when RFC 8040 defined it.
>>  
>>  
>> Also from Andy:
>> 
>>> I agree RESTCONF should be removed from this draft.
>>  
>> This is too important to not support in RESTCONF (and CORECONF, I assume).
>>  
>>  
>> Kent // contributor