More strict key check in snap/snapctl get/set broke existing snap

[Paweł Stołowski]

The handling of config option keys became more strict on 2016-10-01, with the following:

var validKey = regexp.MustCompile("^(?:[a-z0-9]+-?)*[a-z](?:-?[a-z0-9])*$")

so capital letters are not accepted now. There is at least one known snap (openhab) whose configure script broke because of that.

Also, it seems that the related validation error is not propagated correctly as expected, e.g.
$ sudo snap set openhab OPENHAB_HTTPS_PORT=4445

is silently accepted (no error). The option is of course not set:

pawel@ubuntu:~$ sudo snap get openhab OPENHAB_HTTPS_PORT
error: invalid option name: "OPENHAB_HTTPS_PORT"

[John Lenton (chipaca)]

fwiw just adding (?i) to the start of the regexp would address this, if it's what's wanted.

[Gustavo Niemeyer (niemeyer)]

Apologies for the inconvenience. By Oct 1st we had configuration support still in development. The freeze of Ubuntu Core 16, which was the target date for the feature, happened a few days after that. So this was actually never changed after the feature landed in a stable release and was announced.

The reason this was constrained is to encourage a pattern that looks alike across multiple snaps. We may well end up relaxing that constraint, but I'd prefer to stick to it for a bit longer to see if we can reach a point where we're all using a similar convention.

[Ondrej Kubik (ondrak)]

This has been a bit unfortunate change:
a) if snap already implemented set/get feature and released it before this change, then we broke it without communicating back that it's broken. Which was my case. True this was developed agains dev core, but that was only way at the time.
b) now with this syntax restriction, we get got this feature even harder to use.

Let me explain.
First I believe it should be possible to call snapctl get/set from within snap, not only in configure hook, so snap does not need to maintain copy of key-value database.
Now we actually make this even worse as we further force snap to also keep mapping table between keys used by snap app itself and keys blessed by snapd. At least before this change one could keep only copy of database and that was it.
Also what is the point to dictate this across multiple snaps. Most of the snaps are already existing applications, which are snapped, not developed from scratch. And adding rule that if you want to make set/get feature implementation to be sensible, you need to rename keys used internally by app you are snapping, to comply with snapd rules is a bit too much. What if another platform dictates differently?

Also as minimum we should throw error already when calling $ snap set <snap> key=value if key does not comply. Instead we consume it from snap side silently and run config and fail there, which does not actually throw any error back to user either….

[Michael Vogt (mvo)]

This change is in effect for more than 1y now. I assume all snaps have migrated now and I will close the bug. Please reopen if you disagree.