RpcReply.OK not set correctly due to XML unmarshalling of boolean is broken

[emasean]

The following rpc-reply is received:

 <rpc-reply message-id="101"
            xmlns="urn:ietf:params:xml:ns:netconf:base:1.0">
   <ok/>
 </rpc-reply>

But the unmarshalled RpcReply.Ok did not set to true.

After checking the unmarshalling format:

 // RPCReply defines a reply to a RPC request
 type RPCReply struct {
   XMLName  xml.Name   `xml:"rpc-reply"`
   Errors   []RPCError `xml:"rpc-error,omitempty"`
   Data     string     `xml:",innerxml"`
   Ok       bool       `xml:",omitempty"`
   RawReply string     `xml:"-"`
 }

the boolean unmarshalling seems to be broken according to the following site

So making the following RPCReply:

 // RPCReply defines a reply to a RPC request
 type RPCReply struct {
   XMLName  xml.Name   `xml:"rpc-reply"`
   Errors   []RPCError `xml:"rpc-error,omitempty"`
   Data     string     `xml:",innerxml"`
   Ok       *struct{}       `xml:"ok,omitempty"`
   RawReply string     `xml:"-"`
 }

and check if Ok != nil will make it work

[nemith]

I think we can create a custom unmarshaler here.

[wtucker]

Does the RPCReply.Ok flag tell us anything useful? From what I've been able to find the RPCReply is only returned if no error was generated. If something does go wrong, RPCError is raised instead.

[wtucker]

https://www.juniper.net/documentation/en_US/junos/topics/reference/tag-summary/netconf-ok.html

"Indicates that the NETCONF server successfully performed a requested operation that changes the state or contents of the device configuration."

Using the validate command as an example:
<rpc><validate><source><candidate/></source></validate></rpc>

The <ok/> tag is outside of the <commit-results/> tree, so I guess the intent is to save you from having to look inside that blob to understand if everything was "OK"

<commit-results>
</commit-results>
<ok/>
</rpc-reply>
]]>]]>```

[wtucker]

Sample of validate response with bad config (should raise RPCError):

<rpc-reply xmlns="urn:ietf:params:xml:ns:netconf:base:1.0" xmlns:junos="http://xml.juniper.net/junos/15.1F4/junos">
<commit-results>
<rpc-error>
<error-type>application</error-type>
<error-tag>invalid-value</error-tag>
<error-severity>error</error-severity>
<error-path>[edit]</error-path>
<error-message>mgd: Missing mandatory statement: 'root-authentication'</error-message>
<error-info>
<bad-element>system</bad-element>
</error-info>
</rpc-error>
<rpc-error>
<error-type>protocol</error-type>
<error-tag>operation-failed</error-tag>
<error-severity>error</error-severity>
<error-message>
configuration check-out failed: (missing mandatory statements)
</error-message>
</rpc-error>
</commit-results>
</rpc-reply>

[wtucker]

Sample from a lab QFX that had MPLS enabled but not licensed:

<rpc><validate><source><candidate/></source></validate></rpc>
<rpc-reply xmlns="urn:ietf:params:xml:ns:netconf:base:1.0" xmlns:junos="http://xml.juniper.net/junos/16.1R3/junos">
<commit-results>
<rpc-error>
<error-severity>warning</error-severity>
<error-path>[edit protocols]</error-path>
<error-message>mgd: requires 'mpls' license</error-message>
<error-info>
<bad-element>mpls</bad-element>
</error-info>
</rpc-error>
<rpc-error>
<error-severity>warning</error-severity>
<error-path>[edit protocols]</error-path>
<error-message>mgd: requires 'bgp' license</error-message>
<error-info>
<bad-element>bgp</bad-element>
</error-info>
</rpc-error>
<routing-engine junos:style="normal">
<name>fpc0</name>
<commit-check-success/>
</routing-engine>
</commit-results>
<ok/>
</rpc-reply>
]]>]]>

[yhzs8]

Does the RPCReply.Ok flag tell us anything useful? From what I've been able to find the RPCReply is only returned if no error was generated. If something does go wrong, RPCError is raised instead.

For me it is more intuitive to check if the response is OK first (the normal flow) rather than if response contains error first (the exception flow). I believe this is another workaround that works but The protocol wrapper should work as stated in the RFC: https://tools.ietf.org/html/rfc6241#page-19

[wtucker]

For me it is more intuitive to check if the response is OK first (the normal flow) rather than if response contains error first (the exception flow). I believe this is another workaround that works but The protocol wrapper should work as stated in the RFC: https://tools.ietf.org/html/rfc6241#page-19

I've filed #55 to update documentation so it explains that the Ok flag doesn't mean what one might expect.

There are two branches that fix setting the RPCReply.Ok flag processing, but both have drawbacks. I'm open to alternate suggestions for handle the unmarshaling.

[nemith]

Agreed that it should follow the RFC. We worked on this package through the NANOG hackathon and may have some ideas on unmarshaling it, but still experimenting.

[nemith]

This should work: https://play.golang.org/p/Qyl_i0Ihf96 @wtucker can you update your code/tests with this method?

[nemith]

This is fixed in v2. (I understand the xml package so much better than 10 years ago)