store should detect and reject changing snap types

[Jamie Strandboge]

This came up in a meeting today between Samuele and I and we felt it would be good if the store verified that the snap type does not change in a new upload. Eg, if the last upload's snap type was 'app', then we should reject the upload if the next is 'gadget'.

Today the review-tools flag for review when specifying anything other than 'app' which helps, but would not discover going from a non-app snap type to 'app' (not to mention, we won't always flag for non-app). The review-tools also are stateless and cannot be adjusted to check for this (without the store passing the review-tools the previous snap type).

I'm not sure if the store shouldn't just perform this check or if it should pass this in to the snap (perhaps --previous-yaml=/path/to/last/yaml). There are problems with passing this to the review tools since determining what is 'last yaml' is tricky. Best is probably for the store to store the value of the first upload's snap type and comparing on every upload (or passing that to the review tools: --previous-snap-type='app').

[Daniel Manrique (roadmr)]

It sounds simpler for the store to check this. Type seems to be stored per upload, and we can check an upload always matches the previous one's type. We'd need a bit of scouring the database for any snaps which have changed types so we can decide what to do in those cases, looking back, but if implemented as I describe, this should never happen again going forward.

This is probably oversimplistic and I may be missing some subtleties but we can hammer those out during the specification phase if this sounds acceptable in general.

I have a question though: do we want to allow some way of changing a snap's type? or would the type be set "in stone" after the first upload? If the latter, I think we need to be careful and transparent in publicizing this change of behavior.

[Jamie Strandboge (jdstrand)]

The snap types are vastly different from one another so something fairly rigid sounds ok to me. It seems plausible that someone might've registered a name and accidentally sent up the first snap with the wrong type, so some mechanism to change it, even if it is quite manual, seems like it would be good.

[Daniel Manrique (roadmr)]

A problem that arises if we allow changing a snap's type is that it may differ from what's actually in the snap. Example: someone uploads a snap with type: app when they meant it to be kernel. We change it in the database to be kernel, but suppose someone tries to install the existing upload (which internally in its snap.yaml says type: app). What would snapd do? Would we have undesirable results?

Even if a scenario where published revisions of the same snap have differing types internally (assuming the database has been frobbed into lying and saying they're all of the same type) is unlikely, we need to consider the edge cases it introduces, and they sound quite sharp and dangerous to me :(

In that light, I believe disallowing changing it altogether and requiring registering a new snap would be easier to implement and safer in that we don't risk missing some obscure case down the road.

Also - the behavior would be "rejected", not "held for manual review". Does that sound OK?

[Jamie Strandboge (jdstrand)]

This sounds fine to me. If the case I mentioned in comment #3 becomes a problem, we can reassess.