Code review comment for lp://qastaging/~jdstrand/snap-confine/ubuntu-core-launcher.pts

Thanks for the review:

> - AppArmor "ought to" support something like "options=newinstance" so that
> we can reject any mounts that may violate our expectations. "Ought to"
> since it looks like the parser and kernel were written with this in mind
> but I can't figure out how to make it work.

When AppArmor handles this, we can adjust the policy accordingly.

> - We really shouldn't be afraid to modify our startup routines to do the
> right thing. We can either adjust now and deal with the fallout or we
> can continue to live on the assumption that no one will ever screw up,
> ever. Setting -o newinstance on the very first mount sounds like the
> best way to reduce our long-term technical debt.

I wasn't afraid per se to change the startup routines, but Debian and Ubuntu are currently set as 'single-instance' and this is the recommendation on systems in general. I was aiming for the simplest code and simply punting on legacy systems (unlike lxc which will convert a legacy mode to a multi-instance) while working on both single-instance and multi-instance with simple code. Creating a delta with Debian seemed unneeded when the code itself works fine in either mode.

> - I'm not sure the gid=5 is correct if a gid map is in place
> (/proc/pid/guid_map). I can't find documentation on this one way or
> another. I thought I'd call this out in case things don't look correct
> elsewhere (say, gids are wrong in containers) then this may be recalled.

Thanks-- I wondered about this myself but saw the lxc hard-coded this as gid=5 too. Considering its extensive use of nesting and user containers, I just went with it and like you said, figured we could revisit as needed.