Comment 4 for bug 1815991

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

[Summary]
Coming from the same source the review results seemed virtually identical so let us handle it as one.

=> MIR Team ack to masakari and masakari-monitors.

@Security:
- IMHO this needs a review and AFAIK it is 20.04 material so it would have
  to be fast

@Openstack TODOs:
- some tests trigger errors which might be true issues, please check (low prio)

[Duplication]
Masakari provides Virtual Machine High Availability (VMHA) service for OpenStack
clouds, nothing else provides that.
The monitors are essentially the probes it uses to detect these events.

[Embedded sources and static linking]
OK:
- no embedded source present
- no static linking

[Security]
OK:
- history of CVEs does not look concerning
- does not use webkit1,2
- does not use lib*v8 directly
- does not process arbitrary web content
- does not integrate arbitrary javascript into the desktop
- does not deal with system authentication (eg, pam), etc)
- does not use centralized online accounts

Problems:
- It does listen on a port (default conf says 15868) for its API interactions
- It does parse data formats as part of the API handling
- does not run a daemon as root, but is reachable as integrated through libapache2-mod-wsgi-py3 which could be running as root

As usual the approach is better safe then sorry, therefore this will need a security review.

[Common blockers]
OK:
- does not FTBFS currently
- does have a test suite that runs at build time
  - test suite fails will fail the build upon error.
- does have a test suite that runs as autopkgtest
- openstack is subscribed
- no translation present, but none needed for this case (not non-admin visible)
- no new python2 dependency
- used dh_python

[Packaging red flags]
OK:
- Ubuntu does carry not carry a delta, it is Ubuntu-only right now
- symbols tracking not applicable for this kind of code.
- d/watch is present
- Upstream update history is good
- Ubuntu update history is good (Is Ubuntu only atm)
- the current release is packaged
- promoting this does not seem to cause issues for MOTUs that so far maintained the package
- no massive Lintian warnings
- d/rules is rather clean
- not using Built-Using
- not a golang package for further checks in that regard

Problems:
- d/watch didn't follow the update to version 9, but that isn't too important as long as the openstack team actively maintains it

[Upstream red flags]
OK:
- no hard Errors/warnings during the build (I've found a few, see below)
- no incautious use of malloc/sprintf (python code)
- no use of sudo, gksu, pkexec, or LD_LIBRARY_PATH
- no use of user nobody
- no use of setuid
- no important open bugs (crashers, etc) in Debian or Ubuntu
  - the only bugs known are driven and under control by the openstack Team
- no dependency on webkit, qtwebkit, seed or libgoa-*
- no embedded source copies
- not part of the UI for extra checks

Problems:
The tests trigger a bunch of non fatal issues like:
- TypeError: 'int' object is not an iterator
- TypeError: foo() missing 1 required positional
- TypeError: 'NoneType' object is not callable
- KeyError: 'hypervisor_name'
- KeyError: 'id'
- LookupError: No section 'nonexistent app'
I've skipped those that seem intentional due to the test, but if one could give
the full length test output a look and double-check that there are no hidden
errors "we could have known" that would be great.