Confusion from "Error: missing update value/file or path"

[candlerb]

$ gnmic version
version : 0.26.0
 commit : ba387ba
   date : 2022-06-29T06:52:10Z
 gitURL : https://github.com/karimra/gnmic
   docs : https://gnmic.kmrd.dev

I was confused by an error message from gnmic.

The following works (action is performed, response is returned):

gnmic ... -e json_ietf set --update-path '/interfaces/interface[name="Bundle-Ether2"]/subinterfaces' --update-value '{"subinterface":[{"index":9876}]}'

The following doesn't work:

gnmic ... -e json_ietf set --update-path '/interfaces/interface[name="Bundle-Ether2"]/subinterfaces' --update-value '{"subinterface":[{"index":9876,"config":{"index":9876}}]}'

Instead this returns "Error: missing update value/file or path". And yet I've provided both a path and a value, and jq confirms the JSON value is valid:

$ echo '{"subinterface":{"index":9876,"config":{"index":9876}}}' | jq .
{
  "subinterface": {
    "index": 9876,
    "config": {
      "index": 9876
    }
  }
}

Looking for this error message in the code, I find it here:

        if len(c.LocalFlags.SetUpdatePath) != len(c.LocalFlags.SetUpdateValue) && len(c.LocalFlags.SetUpdatePath) != len(c.LocalFlags.SetUpdateFile) {
		return errors.New("missing update value/file or path")
	}

These are []string. But from the point of view of a dumb user like me, I think I've provided 1 path and 1 value. Adding -d flag seems to confirm this:

2022/08/20 14:42:49.968783 /home/runner/work/gnmic/gnmic/app/app.go:264: [gnmic] set-update-path='[/interfaces/interface[name="Bundle-Ether2"]/subinterfaces]'([]string)
2022/08/20 14:42:49.968802 /home/runner/work/gnmic/gnmic/app/app.go:264: [gnmic] set-update-value='[{"subinterface":[{"index":9876,"config":{"index":9876}}]}]'([]string)
...
2022/08/20 14:42:49.969326 /home/runner/work/gnmic/gnmic/config/config.go:343: [config] cmd=set, flagName=update-path, changed=true, isSetInFile=true
2022/08/20 14:42:49.969341 /home/runner/work/gnmic/gnmic/config/config.go:343: [config] cmd=set, flagName=update-value, changed=true, isSetInFile=true

Note that it says that set-update-value is []string, with apparently one element in the list.

After digging through the code, I believe the issue is introduced in SanitizeArrayFlagValue. First, it strips off any outer pairs of [ and ]. Then it breaks the string on comma, unconditionally. In my case, I have a comma in there, but it's part of a compound value, and it didn't occur to me that it would be split here. The debug output shows the value before the split, and the error message seemed to be telling me I hadn't provided a value, when I had.

I suggest the error message could be clarified like this:

"number of update-paths (1) does not match number of update-values (2) or update-files (0)"

Other ideas:

• The help could say --update-value stringArray (comma separated)
• The debug output could be shown after the string splitting, and with %#v instead of %+v
• In an ideal world, I'd like to be able to provide any valid JSON value inline, with repeated args if necessary: --update-value foo --update-value bar. But I realise that would be an incompatible change to the CLI.

Anyway, I hope you don't mind this feedback - thanks for an otherwise great tool!

[candlerb]

Just to throw one more into the pot, here's a different confusing error message which may not be gnmic's fault, but if it's a server error response it could be displayed more clearly.

This works:

$ gnmic -a 192.168.0.2:57400 -u ... -p ... --insecure -e json_ietf \
    get --path '/interfaces/interface[name="Bundle-Ether2"]/subinterfaces/subinterface[index=9876]'

Now I add something invalid to the end of the path:

$ gnmic -a 192.168.0.2:57400 -u ... -p ... --insecure -e json_ietf \
    get --path '/interfaces/interface[name="Bundle-Ether2"]/subinterfaces/subinterface[index=9876]/ipv4'
error marshaling message: invalid character ':' after top-level valuetarget "192.168.0.2:57400": invalid character ':' after top-level value
Error: one or more requests failed

I didn't add a colon though. Looking at tcpdump, some communication does take place, and I can see the path in cleartext.

"Marshaling" to me normally means "encoding". However i's not clear to me whether the issue is client side with encoding the request, or the Cisco's response is somehow invalid (this is IOS-XR 7.2.2, BTW) in which case the problem is in decoding the response.

[hellt]

Now I add something invalid to the end of the path:

Hi
can you try using another formatting option by adding -f prototext to see what cisco returns
it seems that whatever it returns in a gnmi message can't be marshalled to json string that is used by default

[candlerb]

Sure (it's --format not -f). This is the bad one with /ipv4 on the path:

notification: {
  timestamp: 1661012775069334955
  update: {
    path: {
      elem: {
        name: "interfaces"
      }
      elem: {
        name: "interface"
        key: {
          key: "name"
          value: "\"Bundle-Ether2\""
        }
      }
      elem: {
        name: "subinterfaces"
      }
      elem: {
        name: "subinterface"
        key: {
          key: "index"
          value: "9876"
        }
      }
      elem: {
        name: "ipv4"
      }
    }
    val: {
      json_ietf_val: "\"index\":9876"
    }
  }
}
error: {}

And the good one without:

notification: {
  timestamp: 1661012877664384155
  update: {
    path: {
      elem: {
        name: "interfaces"
      }
      elem: {
        name: "interface"
        key: {
          key: "name"
          value: "\"Bundle-Ether2\""
        }
      }
      elem: {
        name: "subinterfaces"
      }
      elem: {
        name: "subinterface"
        key: {
          key: "index"
          value: "9876"
        }
      }
    }
    val: {
      json_ietf_val: "[{\"index\":9876,\"state\":{\"index\":9876,\"name\":\"Bundle-Ether2.9876\",\"enabled\":true,\"admin-status\":\"UP\",\"oper-status\":\"UP\",\"last-change\":\"1661012733839605487\",\"counters\":{\"in-unicast-pkts\":\"0\",\"in-pkts\":\"0\",\"in-broadcast-pkts\":\"0\",\"in-multicast-pkts\":\"0\",\"in-octets\":\"0\",\"out-unicast-pkts\":\"0\",\"out-broadcast-pkts\":\"0\",\"out-multicast-pkts\":\"0\",\"out-pkts\":\"0\",\"out-octets\":\"0\",\"out-discards\":\"0\",\"in-discards\":\"0\",\"in-unknown-protos\":\"0\",\"in-errors\":\"0\",\"in-fcs-errors\":\"0\",\"out-errors\":\"0\",\"carrier-transitions\":\"0\",\"last-clear\":\"2022-08-18T11:55:34.737+01:00\"},\"ifindex\":129,\"logical\":true},\"openconfig-if-ip:ipv4\":{\"state\":{\"counters\":{\"in-octets\":\"0\",\"in-pkts\":\"0\",\"out-octets\":\"0\",\"out-pkts\":\"0\"}}},\"openconfig-if-ip:ipv6\":{\"state\":{\"counters\":{\"in-octets\":\"0\",\"in-pkts\":\"0\",\"out-octets\":\"0\",\"out-pkts\":\"0\"}}}}]"
    }
  }
}
error: {}

Hmm, so the bad one really is bad (a JSON object without {...} ?!)

Might be nice to display this as part of the error though.

[karimra]

Hi,

The comma issue in flags --update-value and --replace-value is a known issue, it ultimately comes from the underlying packages used to parse the flags and set the config values (pflag and viper).

Instead if trying to figure out how different shells and different repeated flags will behave, there are a few other options that can be used to set a more complex value.

If you would like to set a compound json value you have a couple of options:

• Use --update-file instead of --update-value:
  You can create a file with the desired value and run the same command with --update-file.
  Or set the value using stdin to --update-file: gnmic .... --update-file - | echo '{JSON_BLOB_HERE}'
• Use --request-file:
  This flag allows you to set the whole request (updates, replaces and deletes) in a single file (YAML or JSON).
  As a bonus, the request-file can be a Go Text template that will be rendered per target, so a single file can result in different set requests for each target. See here.
  As an example, your request above would be:

$ cat req_file.yaml

updates:
  - path: /interfaces/interface[name="Bundle-Ether2"]/subinterfaces
    encoding: json_ietf
    value:
      subinterface:
        index: 9876
        config:
          index: 9876         

$ gnmic ... set --request-file req_file.yaml

[karimra]

Hmm, so the bad one really is bad (a JSON object without {...} ?!)

Might be nice to display this as part of the error though.

Yes and the error is displayed: error marshaling message: invalid character ':' after top-level value

[candlerb]

I got it working with --update-file, thanks.

it ultimately comes from the underlying packages used to parse the flags and set the config values (pflag and viper).

There is StringArray:

package main

import flag "github.com/spf13/pflag"
import "fmt"

func main() {
	uvp := flag.StringArray("update-value", nil, "set update request value")
	flag.Parse()
	fmt.Printf("%#v\n", *uvp)
}

$ go run main.go --update-value foo --update-value bar
[]string{"foo", "bar"}

Yes and the error is displayed: error marshaling message: invalid character ':' after top-level value

Of course, although it doesn't show the actual value it's complaining about (I didn't know about --format prototext)

The confusion for me as a user was from seeing "error marshaling message", when to me the problem is unmarshaling the response. Looking at code, I think this is an implementation detail, with a marshal and unmarshal pair. Also, "message" could refer to either the request or the response.

Perhaps "error decoding response", or even "error decoding json_ietf_val in response", would be clearer?

Cheers,

Brian.

[karimra]

I got it working with --update-file, thanks.

it ultimately comes from the underlying packages used to parse the flags and set the config values (pflag and viper).

There is StringArray:

package main

import flag "github.com/spf13/pflag"
import "fmt"

func main() {
	uvp := flag.StringArray("update-value", nil, "set update request value")
	flag.Parse()
	fmt.Printf("%#v\n", *uvp)
}

$ go run main.go --update-value foo --update-value bar
[]string{"foo", "bar"}

StringArray is what is used right now because StringSlice did too many things under the hood (e.g reading the flag as a CSV line).

The flags can also be set from ENV or from a config file, viper does not handle string arrays and slices properly (returns them as a string of comma separated values in both cases), hence the usage of StringArray and SanitizeArrayFlagValue func.

Yes and the error is displayed: error marshaling message: invalid character ':' after top-level value

Of course, although it doesn't show the actual value it's complaining about (I didn't know about --format prototext)

The confusion for me as a user was from seeing "error marshaling message", when to me the problem is unmarshaling the response. Looking at code, I think this is an implementation detail, with a marshal and unmarshal pair. Also, "message" could refer to either the request or the response.

Perhaps "error decoding response", or even "error decoding json_ietf_val in response", would be clearer?

Cheers,

Brian.

The error does not happen when reading the value from the received message, it happens when trying to marshal it to JSON to print it out on the terminal. gNMIc reads the json/json_ietf values as []byte, it does not validate that the bytes are valid json on reception.

I'm not against adding the name of the message to the logged error.