ygot icon indicating copy to clipboard operation
ygot copied to clipboard

Serializing structures to XML

Open egr3s opened this issue 4 years ago • 5 comments

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?

egr3s avatar Dec 09 '20 06:12 egr3s

See #144

wenovus avatar Dec 09 '20 15:12 wenovus

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:

  • Refactor annotateEntry (schemaparse.go) to add namespace information into the Entry struct (goyang/pkg/yang/entry.go) as exported field (type Entry already has a "namespace" field, but this is not exported)
  • add an inclNamespaces bool parameter to annotateEntry to optionally store the namespace information into the compressed JSON schema

Couple of questions here:

  • do we need to store namespace prefix information also? (I think so)
  • if we store the namespace information in the compressed JSON schema, then we have to pass the schema to EmitXML and things get more complex. Why do you prefer to have it in the compressed JSON schema? I would have added namespace information as XML tag as you are doing for the module name for instance, so that it is available from the struct.
  1. A new render function would need to be added to ygot/render.go to handle a GoStruct and return the relevant XML.

Inside ygot/struct_validation_map.go, create following functions: func EmitXML(s ValidatedGoStruct, opts *EmitXMLConfig) (string, error) func makeXML(s GoStruct, opts *EmitXMLConfig) (map[string]interface{}, error)

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 avatar Feb 25 '22 16:02 GiacomoCortesi

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

GiacomoCortesi avatar Mar 10 '22 15:03 GiacomoCortesi

@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:

  • take in 'current config' and 'new config'
  • run ygot.Diff to get the changes that need to be made between the two
  • translate these into the XML
    • we would need to check that ygot.Diff does the right thing in terms of subtrees -- I think it might not be as succinct as it could be when considering diffs today - so it would give you a delete of every leaf rather than any container. We could either add an option to the diff to change this, or post-process the output.

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.

robshakir avatar Mar 10 '22 16:03 robshakir

@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:

  • ygot.Diff adding ConsiderNonLeafNodes as a DiffOpt that indicates that YANG lists (Go maps) and containers (Go structs) should be considered.
  • ygot.findSetLeaves removing the above cases from the ignored values if the ConsiderNonLeafNodes option is passed.
  • util.ForEachDataField adding an argument opts ...ReflectOpt with an option similar to ConsiderNonLeafNodes.
  • util.forEachDataFieldInternal Idem to the above and taking this option into account in cases IsTypeStruct(t) and IsTypeMap(t) to avoid going recursively into the items.
  • ygot.appendUpdate that calls ygot.EncodeTypedValue to encode a value into a gNMI TypedValue message, being a current challenge.

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

denis2glez avatar May 31 '22 08:05 denis2glez