[Summary]
It generally looks good already for being at such an early stage, the following
list covers what I think need to be added/improved to make it acceptable.
- go issue of embedded libs, can we resolve reduce this?
Please answer my questions below.
- I know it makes no sense in a container, but fix it so that it properly
installs by changing default config/postinst or whatever you see fit
- add watch file to your github
- please resolve the lintian copyright complains
- needs a security review
- enable more isolation features for for zsys-commit.service please
- add some documentation
- extend autopkgtests to cover some real use-cases
- this seems to be at a very early stage, every here and there more features
are mentioned but not yet implemented. To review this it should sort of
feature complete at least to the extend that are planned for the coming
release. Otherwise reviews cover what is there in 0.1 but e.g. 1.0 will
look much much different (If the feature is too far out at least add the
docs and specs to check if the design fits main inclusion)
[Duplication]
There is no other tool doing that job.
[Embedded sources and static linking]
No static (C) linking, but this is golang and uses the classic approach of
bundling a lot of vendor libs.
Of your bundled libs I found almost all in the archive (package names):
golang-gopkg-yaml.v2-dev golang-golang-x-xerrors-dev golang-golang-x-sys-dev
golang-github-spf13-cobra-dev golang-github-spf13-pflag-dev
golang-github-sirupsen-logrus-dev golang-github-mattn-go-colorable-dev
golang-github-mattn-go-isatty-dev
golang-github-konsorten-go-windows-terminal-sequences-dev
golang-github-k0kubun-pp-dev golang-github-inconshreveable-mousetrap-dev
golang-github-google-go-cmp-dev
The only one I didn't find was vendor/github.com/bicomsystems/go-libzfs/
Of course this would also mean you'd have to MIR all of those as well.
But that is what the code review here has to do.
So the amount of code to be verified/reviewed/maintained would not change
And otherwise things have to maintained multiple times.
I know ABI instabilities are an issue, but you'd know which release you target.
Or are you planning to maintain one version of zsys for all future Ubuntu
releases?
In that case I see why you bundle the versions, but be aware of the tracking
and updating that you'll have to do for all of them.
Same for the security team, they need to track all those components for
potentially many Go packages.
Currently the rule to keep them bundled is:
"the requesting team must state their commitment to testing no-change-rebuilds
triggered by a dependent library/compiler and to fix any issues found for the
lifetime of the release."
Hence I'd like the Desktop and the Security Team state here on the bug that
a) they are aware
b) they are willing and able to do that tracking and updating
[Security]
OK:
- history of CVEs
I have not found any known CVEs.
But zsys obviously is brand new and the golang components are very hard to
search/track.
So I think it is ok from the "known CVEs"
- no use of webkit, lib*v8
- does not process arbitrary web content
- does not manage centralized online accounts
- does not integrates arbitrary javascript into the desktop
- does not deal with system authentication
- does not open a port (at least I think so)
- runs a daemon as root - sort of ok
Just the one that commits the passed boot, but could we throw that one into
all sorts of privTMP/priv*** and other systemd restrictions?
Security sensitive for:
- parsing data formats (on disk)
IMHO this is security sensitive as it needs to get access (and can prevent) to
most of the disk data.
If exploited it could pass data on.
Therefore I request a security review of this as well.
[Common blockers]
OK:
- no FTBFS currently?
- build time internal tests seem excessive thanks for those
- desktop-packages is already subscribed
- no python package, so no extra python checks apply
Not bad but improvable:
- The autopkgtest seems a bit too light, in isolation-machine I'm sure quite a
bunch of self-tests could be added right?
- I don't mind so early to not have translations, but do you have the
infrastructure in place to do so later?
[Packaging red flags]
OK:
- Does not carry Ubuntu delta? (native)
- no lib, no symbols tracking needed
- too new to rate the update history, but is under canonical maintenance, so we
have a good feeling for it
- the current release packaged
- no issues hamstringing MOTUs with updating it
- d/rules is small and clean
- uses dh_golang (via dh --with=golang)
- Built-Using: ${misc:Built-Using} for golang is in place
Please improve these:
- Install doesn't work by default (not everywhere at least)
- it currently doesn't install in containers
I know it isn't needed/working there anyway.
But please update the packaging in a way that the package installs with a
disabled/detecting default config.
- no watch file, less bad than usual as it is native, but add one to track your
github please
- there is a bunch of copyright lintian warnings - it would be great to resolve
those on the initial inclusion
- documentation umm ... where?
I know this is WIP, but then we don't promote it at too early stages
The -h test is minimal, no man page, nothing in docs.
Even the frontpage on https://github.com/ubuntu/zsys still is a bit too light.
Before being acceptable there should be something users can start with.
[Upstream red flags]
OK
- no Errors/warnings during the build
- no Incautious use of malloc/sprintf
- no Uses of sudo, gksu, pkexec, or LD_LIBRARY_PATH
- no use of User nobody
- no use of setuid (some generated code for all syscalls in
vendor/golang.org/x/sys/unix/ but no use of it)
- no known Important bugs (crashers, etc) in Debian or Ubuntu
- no Dependency on webkit, qtwebkit, seed or libgoa-*
Not so good:
- Embedded source copies (we talked about that already above)
[ Notes ]
This will eventually do much more than just setting the successful boot
(zsys-commit.service).
If I might ask how will this be later integrated into the system?
Reading
https://didrocks.fr/2019/08/06/ubuntu-zfs-support-in-19.10-introduction/ I'm
not so sure if:
a) this is mostly just the commit code
b) this will grow a lot
(BTW as mentioned before, more docs would have helped).
** Changed in: zsys (Ubuntu)
Status: New => Incomplete
--
You received this bug notification because you are a member of Ubuntu
Bugs, which is subscribed to Ubuntu.
https://bugs.launchpad.net/bugs/1839271
Title:
[MIR] zsys
To manage notifications about this bug go to:
https://bugs.launchpad.net/ubuntu/+source/zsys/+bug/1839271/+subscriptions
--
ubuntu-bugs mailing list
[email protected]
https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
