Comment 8 for bug 2023366

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Hi,
wow this is different to what I expected by just seeing the title.
This is really - as you say in the readme - essentially is a dummy package.
It has a very special purpose and a way to do that via packaging meta-data.
But it does not hold any code, aside from the apt hooks.
You already have added tests and all that I can see makes me rather confident.

There is no point in copy&pasting the usual MIR review template for this case and say "n/a" to almost every entry, hence you'll see a minimal form with just what is worth to state in this case.

Review for Package: boot-managed-by-snapd

[Summary]
- MIR team ACK
- This does not need a security review
- List of specific binary packages to be promoted to main: boot-managed-by-snapd

[Applicable checks]
- There is no other package in main providing the same functionality.
- no other Dependencies to MIR due to this
  (libjson-c5 and libc6 are in main)
- no embedded source present
- While there is no history on anything, and thereby not on CVEs I can OTOH not see the attack surface either.
- does not FTBFS currently
- does have a non-trivial test suite that runs as autopkgtest

---

Recommended TODOs (all are really optional):

#1
The only thing I'd recommend before calling this package fully complete and ready for prime time is a clear statement of the "possibility" to do what it tries to protect one from doing.
You said this will be a dependency of subiquity for a hybrid install, I haven't heard enough of that architecture yet, but either way I'm sure the moment this is out there will be the question "but I do not want to be protected and be able to do this?".

There should be a place that clearly states either (whatever applies):
a) you really shouldn't do this, but if you insist this is how you'd do (but better prepare for damage)
b) you really shouldn't do this, and you shouldn't try as it is impossible because reason

This place wherever you'd host it should then be linked from the README file and/or set as package homepage or any such to make it easy to find (up to you).

What I mean is that it is great that you want to protect users from shooting in their feet.
But one thing I have learned is that whenever you block something, people want to try it even more.
So how about adding the answer to that question right away?

#2
The purpose of this package and the ways it operates are strange.
How would you feel about extending the Readme.md even more and putting it into docs?
Right now it only had copyright and changelog, both not helpful.
So all I have as a user with this installed is apt-cache show, which isn't too detailed.

#3
One can install this on a non-booting system. E.g. a LXD container.
Can you think of that ever becoming an issue?
If so, should we somehow prevent this to only be installed in the case a system is really managed by snapd (if we can detect that in dependencies in some way)?