Merge lp:~gmb/lpsetup/make-lp-lxc-ip-discoverable-bug-1019181 into lp:lpsetup
| Status: | Merged |
|---|---|
| Approved by: | Graham Binns on 2012-07-09 |
| Approved revision: | 49 |
| Merged at revision: | 46 |
| Proposed branch: | lp:~gmb/lpsetup/make-lp-lxc-ip-discoverable-bug-1019181 |
| Merge into: | lp:lpsetup |
| Diff against target: | 0 lines |
| To merge this branch: | bzr merge lp:~gmb/lpsetup/make-lp-lxc-ip-discoverable-bug-1019181 |
| Related bugs: |
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Gary Poster (community) | 2012-07-09 | Approve on 2012-07-09 | |
|
Review via email:
|
|||
Commit Message
lp-setup now checks both /usr/bin and /usr/local/bin for lp-lxc-ip and blows up if it doesn't exist.
Description of the Change
This branch fixes bug 1019181 by changing the LXC_IP_COMMAND constant in settings.py to point to `/usr/bin/env lp-lxc-ip`, thus sidestepping the problem of the .deb and setup.py putting it in different places.
I've also made a driveby change in this branch to further fix init-host's initialize() function, which, by adding a header when it generated, read in and re-wrote the user's SSH keys, would raise an exception every time it ran. It will now only write keys out if it's been passed them to write. I've also added tests for this behaviour.
| Launchpad QA Bot (lpqabot) wrote : | # |
The attempt to merge lp:~gmb/lpsetup/make-lp-lxc-ip-discoverable-bug-1019181 into lp:lpsetup failed. Below is the output from the failed tests.
nose.plugins.cover: ERROR: Coverage not available: unable to import coverage module
.......
=======
ERROR: Failure: Exception (Unable to find lp-lxc-ip.)
-------
Traceback (most recent call last):
File "/usr/lib/
addr.filename, addr.module)
File "/usr/lib/
return self.importFrom
File "/usr/lib/
mod = load_module(
File "/home/
from lpsetup.subcommands import (
File "/home/
from lpsetup.handlers import handle_directories
File "/home/
from lpsetup.settings import (
File "/home/
raise Exception("Unable to find lp-lxc-ip.")
Exception: Unable to find lp-lxc-ip.
=======
ERROR: Failure: Exception (Unable to find lp-lxc-ip.)
-------
Traceback (most recent call last):
File "/usr/lib/
addr.filename, addr.module)
File "/usr/lib/
return self.importFrom
File "/usr/lib/
mod = load_module(
File "/home/
from lpsetup.settings import (
File "/home/
raise Exception("Unable to find lp-lxc-ip.")
Exception: Unable to find lp-lxc-ip.
=======
ERROR: Failure: Exception (Unable to find lp-lxc-ip.)
-------
Traceback (most recent call last):
File "/usr/lib/
addr.filename, addr.module)
File "/usr/lib/
return self.importFrom
File "/usr/lib/
mod = load_module(
File "/home/
raise Exception("Unable to find lp-lxc-ip.")
Exception: Unable to find lp-lxc-ip.
================...
- 49. By Graham Binns on 2012-07-09
-
Reverted the previous test-breaking change.

Thank you Graham. The initialize change is great. Thank you for the thorough tests. I'm curious as to why you chose to write these as doctests rather than unit tests, but not bothered about it.
The lp-lxc-ip change might be fine, but it is less paranoid than what I had in mind. Using the environment to find and call a script as root is a potential security problem (privilege escalation).
The question is how insane it is to worry about it in this case. In order to trigger this escalation at the moment, someone would minimally need to have sudo for the packaged location of lp-setup. Then they would write their own lp-lxc-ip to do nefarious things, like giving themselves full root access.
As far as I can tell, we only have three use cases for the script.
1. devs on their on systems or on ec2. They already have root.
2. webops on data center machines. They will not have sudo for lp-setup, except possibly within the lxc containers.
3. testing automation, such as what we do now.
In all of those, we are covered.
So in sum, I think this is ok, and a convenient solution both for coding and using, but I'd feel more comfortable in my paranoia if we used hard-coded paths.