Revert of classic snap

I have a classic snap installed, and two older revisions of it:

andreas@nsn7:~/git/packages/samba$ snap list --all git-ubuntu
Name        Version  Rev  Developer  Notes
git-ubuntu  0        206  nacc       disabled,classic
git-ubuntu  0        207  nacc       disabled,classic
git-ubuntu  0        209  nacc       classic

Because the current snap is broken, I decided to revert. Note I didn’t pass --classic:

andreas@nsn7:~/git/packages/samba$ snap revert --revision=207 git-ubuntu
git-ubuntu reverted to 0
andreas@nsn7:~/git/packages/samba$ snap list 
Name                 Version    Rev   Developer  Notes
asciinema            1.4.0      9     sabdfl     classic
canonical-livepatch  7          22    canonical  -
charm                2.2        15    charms     classic
core                 16-2.27.5  2774  canonical  core
git-ubuntu           0          207   nacc       classic
ubuntu-image         1.1+snap3  75    canonical  classic

Now the snap stopped working, with a lot of permission denied errors (see the pastebin for all of it):

09/01/2017 10:24:20 - ERROR:stderr: warning: unable to access '/home/andreas/.gitconfig': Permission denied
(...)
[ 5160.966933] audit: type=1400 audit(1504272252.972:92): apparmor="DENIED" operation="open" profile="snap.git-ubuntu.git-ubuntu" name="/home/andreas/.gitconfig" pid=32598 comm="git" requested_mask="r" denied_mask="r" fsuid=1000 ouid=1000

Turns out it was reverted as a confined snap, not classic. The snap list output earlier above, after the revert, is saying it’s classic. That in itself is probably a bug.

The other bug would be that it allowed the revert without --classic to happen. I think it should either give the same kind of error you get when you try to install a classic snap without passing --classic, or it should assume --classic on its own, since the version you are reverting to, already installed previously, was a classic snap with classic confinement.

@ahasenack does this also happen on 2.26? (snap refresh core --stable)

Yes. I downgraded core to 16-2.26.14 and tried the snap revert on git-ubuntu one more time, same result. It keeps being listed as a classic snap, but has normal confinement applied and doesn’t work.

Yes, it should definitely assume the prior flags on revert, and I believe we already do that today with devmode for exactly the same reason.

@chipaca Was it you fixing the case of devmode, and if so would you mind having a look at this when you have a sec?

already on my plate :slight_smile:

In a standup last week we talked about this issue, and how it’s actually a symptom of not having flags per revision in state: fixing it to behave properly and naturally is actually currently not possible for all corner cases (there’s currently 144 cases per verb and it grows exponentially).

Unfortunately this means the general fix is not just a quick PR I can push between priority items on my plate; I need to schedule it. My scheduler is a heap :slight_smile:.

I could push a PR that fixes just this issue and not the general case; I’d have to de-generalise the tests I already have written for it. It shouldn’t make anything worse

@chipaca If you have it started, the case here sounds like a problem worth fixing on itself as it’s probably cheap given it was just a revert losing a flag in a simple case, and it’s probably also worth fixing the presentation issue of it saying it’s classic when it’s not.

the presentation issue is separate: what it’s displaying is confinement:classic, not the installation flag

to be clear, what I have written would mean that the “classic” flag is sticky. This behaves correctly for this case, but breaks if a snap jumps from strict to classic. It’s just moving the breakage into a smaller corner case. Which might be worth it!

PR#3941 addresses this.