openconfig / ygot

A YANG-centric Go toolkit - Go/Protobuf Code Generation; Validation; Marshaling/Unmarshaling
Apache License 2.0
286 stars 106 forks source link

Serializing structures to XML #486

Open egr3s opened 3 years ago

egr3s commented 3 years ago

Hi,

I've looked through the ygot's code and couldn't find anything related to XML as the output format that can be used in Netconf's edit-config command. Do you have any plans to add capability to serialize to XML?

wenovus commented 3 years ago

See #144

GiacomoCortesi commented 2 years ago

Ehi guys, I'd like to add an EmitXML functionality, so I'd love to have some deeper design discussion.

Based on points described by @robshakir I started the work as described below:

  1. The YANG namespace for each struct field needs to be stored somewhere, this could either be in the compressed JSON schema (preferable), or as struct tags.

To do so, edit goyang/pkg/yang/entry.go:

Couple of questions here:

Inside ygot/render.go, create following functions: func ConstructXML(s GoStruct) (map[string]interface{}, error) For now I'm calling structJSON from ConstructXML, with RFC7951 format, I should then be able to marshal the resulting map[string]interface{} into XML (given I also have namespace information of course). Do you agree?

Everything I mentioned looks feasible to me, what looks pretty difficult is how to use the generated go structs and code into a Netconf client implementation. For instance, let's assume I'm using NETCONF and I want to perform an edit-config operation to delete some data from the target device. In such a case we need to build up an xml that looks like:

  <rpc message-id="101"
          xmlns="urn:ietf:params:xml:ns:netconf:base:1.0">
       <edit-config>
         <target>
           <running/>
         </target>
         <default-operation>none</default-operation>
         <config xmlns:xc="urn:ietf:params:xml:ns:netconf:base:1.0">
           <top xmlns="http://example.com/schema/1.2/config">
             <interface xc:operation="delete">
               <name>Ethernet0/0</name>
             </interface>
           </top>
         </config>
       </edit-config>
     </rpc>

considering that the xc:operation attribute can be placed on whatever element under config, I'm struggling to find a solution that is not too impacting on the existing code. Do you have any considerations/suggestions about this last point?

I'm very happy to provide further explanation/use-cases if it's not clear what I'm trying to achieve :)

GiacomoCortesi commented 2 years ago

eg: https://github.com/damianoneill/net/tree/master/v2/netconf

how is this related to my questions? It's just a netconf client/server implementation

robshakir commented 2 years ago

@GiacomoCortesi - this is an interesting question. I do agree that adding a struct tag would likely be the simplest approach. I think that we were preferring another approach just to restrict the number of struct tags, but it's likely that some symmetry between JSON and XML is better.

I was thinking about the way that you could create the kind of XML that you described here, and was wondering whether ygot.Diff could help here. The approach could be:

Since the diff would already give you a way that describes /interfaces/interface[name=ethernet42] has been deleted then it seems easier to form your content from this. Add and modify would be represented in the same way - but you could detect any 'update' and then turn this into a replace in the configuration.

The gap in our tooling here today is that ygot is really designed around a declarative config model... generate a new config and issue a replace at some part of the tree, rather than one that tries to make individual transactions like the one that you showed. A clean way to make this change might be to implement a new package that can take you from orig->modified in a way that is agnostic to transport protocol (e.g., returning a list of added, deleted, modified transactions) that could then be translated into a new underlying approach.

Thoughts? I'm happy to brainstorm this further.

denis2glez commented 2 years ago

@robshakir thanks for sharing your insights above. @GiacomoCortesi and I have been thinking of using ygot.Diff as you suggested to get the changes taking into account not only leaves but also a container. Generally speaking, I think we could proceed with the following changes in:

Do you have any recommendations on the necessary modifications around TypedValue?