Juniper/go-netconf

Add support for <xnm:error /> management protocol operational errors

Closed this issue · 9 comments

I get the following response (from a srx that does not currently have a cx111 attached to one of the USB ports):

<rpc-reply xmlns="urn:ietf:params:xml:ns:netconf:base:1.0" xmlns:junos="http://xml.juniper.net/junos/11.4R9/junos" message-id="8fe866c66684646c42df2beace371f37">
  <xnm:error xmlns="http://xml.juniper.net/xnm/1.1/xnm" xmlns:xnm="http://xml.juniper.net/xnm/1.1/xnm">
    <source-daemon>wireless-wan-service</source-daemon>
    <message>Adapter is not present.</message>
  </xnm:error>
</rpc-

One can unmarshal the <xnm:error /> payload with a struct like:

type XnmError struct {
    SourceDaemon string `xml:"source-daemon"`
    Message      string `xml:"message"`
}

type RPCReply struct {
    XMLName  xml.Name    `xml:"rpc-reply"`
    XnmErrors []XnmError `xml:"error"`
}

Here's a reference implementation.

What would be the best way to present these errors?

Do we create a unified error interface that rpc-error and xnm:error are part of or do we keep rpc errors separate from management protocol operational errors?

This is pretty interesting.

I would prefer that anything in the base package is pretty well defined by the RFC or can be standard practice by the vendors implementing them.

Seems that this error may by custom Juniper. I am open to having vendor specific addons (and common rpc commands) in a subpackage (for example netconf/jnpr or netconf/juniper) but I am not sure the best way to add hooks into error reporting to allow that.

What about something like this?

We would then modify anything that returns a RPCReply or *RPCReply to rather take a RPCReplier as a parameter that it then fills with the rpc reply via the Parse() function for the concrete implementation.

So ExecRPC() and Exec() would become something like:

func (s *Session) ExecRPC(rpc *RPCMessage, reply RPCReplier) error {
        return s.Exec(rpc.String(), reply)
}

func (s *Session) Exec(msg string, reply RPCReplier) error {
        err := s.Transport.Send([]byte(msg))
        if err != nil {
                return err
        }

        rawXml, err := s.Transport.Receive()
        if err != nil {
                return err
        }
        reply.RawReply = string(rawXml)

        if err := reply.Parse(rawXml); err != nil {
                return err
        }

        if len(reply.Errors) > 0 {
                // We have errors, lets see if it's a warning or an error.
                for _, rpcErr := range reply.Errors {
                        if rpcErr.Severity == "error" || s.ErrOnWarning {
                                return &rpcErr
                        }
                }
        }

        return nil
}

And you'd use them like this:

[...]
        s, err := netconf.DialSSH(flag.Arg(0),
                netconf.SSHConfigPassword(*username, *password))
        if err != nil {
                panic(err)
        }
        defer s.Close()

       // To use the base RPCReply
       reply := &RPCReply{}
       err := s.ExecRPC(netconf.RPCGetConfig("running"), reply)
        if err != nil {
                panic(err)
        }
        fmt.Printf("Reply: %+v", reply)

       // To use the Juniper enhanced RPCReply
       reply := &JuniperRPCReply{}
       err := s.ExecRPC(netconf.RPCGetConfig("running"), reply)
        if err != nil {
                panic(err)
        }
        fmt.Printf("Reply: %+v", reply) 

This code is untested and meant to convey how we could accomplish this. If this looks like a good approach I'll work on an implementation and submit a pull request.

I like the idea of making an interface, other option would to find a way to serialize additional options into a map to be handled by the library user.

Also defaults should be sane. So need a way to default to standard errors first and allow customization on top of it. (i.e RPCReplier shouldn't be mandatory)

I think we should try to solve this with an Extra field on the RPC Reply that uses ,any in encoding/xml

  • If the XML element has an attribute not handled by the previous
    rule and the struct has a field with an associated tag containing
    ",any,attr", Unmarshal records the attribute value in the first
    such field.

Actually I am closing this. It's in violation of the RFC and we have RawReply in the struct to allow you to parse the error directly.