Internal error: could not marshal value for state entry "hook-context"

snapd
1

Matteo reported a panic bug when installing wifi-ap snap from edge channel today - see https://bugs.launchpad.net/snapd/+bug/1680471.

2017/04/06 14:01:21.523100 api.go:891: Installing snap "wifi-ap" revision unset
2017/04/06 14:01:21.731705 state.go:63: PANIC internal error: could not marshal value for state entry "hook-context": json: unsupported type: map[interface {}]interface {}
2017/04/06 14:01:21 http: panic serving uid=0;@: internal error: could not marshal value for state entry "hook-context": json: unsupported type: map[interface {}]interface {}

Here here is nicely formatted state.json for the affected system - http://pastebin.ubuntu.com/24327232/

I’ve tried to reproduce this bug in various ways but haven’t managed so far: I can install the wifi-ap snap from egde successfuly on my desktop, on a clean 16.04 VM and on clean ubuntu core image (in all cases running snapd 2.23.6). Also, I can run sudo snap set wifi-ap automatic-setup.disable=false to trigger configure hook just fine.

Also I don’t see anything suspicious in the state.json attached to this bug (I guess that’s expected, the failure indicates the state couldn’t be updated with provided value, so we will not find it there).

Any suggestions what else to test/check/look for?

2

The problem is the key in that map, and this indeed looks like an internal error in the sense that a value that shouldn’t be used as configuration is reaching the configuration code path.

Looking at the JSON won’t help because by definition you’ll never get an interface{} key when manipulating it. Those maps will have string keys instead, and that’s what the configuration subsystem expects and knows how to handle.

As a suggested next step, look at all the code paths we’ve got which make data land on “hook-context”, and see how we could get such an alien type passing through.

3

Ok, I got it reproduced with spread tests of that snap from Matteo, and then also implemented a very minimal unit test that triggers it using the data from that spread test. The problem is with gadget.yaml and its defaults, we cleary fail on:

defaults:
  19adjald109e1jlasd:
      apconfig: {"wifi.ssid":"GadgetSnap","wifi.security":"wpa2","wifi.security-passphrase":"GadgetGadgetGadgetGadgetMushroomMushroom"}

We clearly don’t support that. I’m trying to figure out what is that we wanted here, will continue investigating.

4

Actually the example above is missing quotes. With proper quotes this should be supported as its just a string handed to the snap through the configuration like shown here https://github.com/snapcore/snapd/wiki/Gadget-snap

5

@morphis are you saying that the {…} should be quoted, making it a string? That looks a bit suspicious… It’s a bit hard to tell if it’s doing the right thing when applied through the configuration without some kind of test.

6

That is what we do already for those things being set from the prepare-device hook.

7

that’s just quoting to pass json on the command line, yaml should work without turning the dict into a string

8

@pstolowski encoding/json doesn’t support map[interface{}]interface even if all the key values are strings, it supports only map[string]interface{}

package main

import "fmt"
import "encoding/json"

func main() {
       x := map[interface{}]interface{}{
         "foo": "bar",
       }

	fmt.Println(json.Marshal(x))
}

=> [] json: unsupported type: map[interface {}]interface {}

we really need code like the one we use for interface attributes, to make sure what we take in from the gadget.yaml defaults is JSON-serialisable and turning dicts into map[string]interface{}

See validateAttr in snap/info_snap_yaml.go

1 Like
9

@morphis Having blobs of JSON into a configuration option is definitely discouraged, because it’s much messier to operate with those, much harder to get/set values individually, hard to validate properly, and so on. It would be much nicer to have these individually set, and build the JSON inside the hook if that’s what’s needed.

That said, there’s obviously no reason for it to be breaking. That’s a bug that needs fixing.

As a minor suggestion, rather than quoting, use the | style so that the string remains clean.