Merge lp:~gmb/lpsetup/make-lp-lxc-ip-discoverable-bug-1019181 into lp:lpsetup

Proposed by Graham Binns on 2012-07-09
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
Reviewer Review Type Date Requested Status
Gary Poster (community) 2012-07-09 Approve on 2012-07-09
Review via email: mp+113955@code.launchpad.net

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.

To post a comment you must log in.
Gary Poster (gary) wrote :

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.

review: Approve
Launchpad QA Bot (lpqabot) wrote :
Download full text (19.6 KiB)

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
........EEEEEEEEEEEEEEEEE...................EE
======================================================================
ERROR: Failure: Exception (Unable to find lp-lxc-ip.)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/lib/python2.7/dist-packages/nose/loader.py", line 390, in loadTestsFromName
    addr.filename, addr.module)
  File "/usr/lib/python2.7/dist-packages/nose/importer.py", line 39, in importFromPath
    return self.importFromDir(dir_path, fqname)
  File "/usr/lib/python2.7/dist-packages/nose/importer.py", line 86, in importFromDir
    mod = load_module(part_fqname, fh, filename, desc)
  File "/home/tarmac/repos/lpsetup/trunk/lpsetup/cli.py", line 18, in <module>
    from lpsetup.subcommands import (
  File "/home/tarmac/repos/lpsetup/trunk/lpsetup/subcommands/finish_inithost.py", line 24, in <module>
    from lpsetup.handlers import handle_directories
  File "/home/tarmac/repos/lpsetup/trunk/lpsetup/handlers.py", line 30, in <module>
    from lpsetup.settings import (
  File "/home/tarmac/repos/lpsetup/trunk/lpsetup/settings.py", line 78, in <module>
    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/python2.7/dist-packages/nose/loader.py", line 390, in loadTestsFromName
    addr.filename, addr.module)
  File "/usr/lib/python2.7/dist-packages/nose/importer.py", line 39, in importFromPath
    return self.importFromDir(dir_path, fqname)
  File "/usr/lib/python2.7/dist-packages/nose/importer.py", line 86, in importFromDir
    mod = load_module(part_fqname, fh, filename, desc)
  File "/home/tarmac/repos/lpsetup/trunk/lpsetup/handlers.py", line 30, in <module>
    from lpsetup.settings import (
  File "/home/tarmac/repos/lpsetup/trunk/lpsetup/settings.py", line 78, in <module>
    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/python2.7/dist-packages/nose/loader.py", line 390, in loadTestsFromName
    addr.filename, addr.module)
  File "/usr/lib/python2.7/dist-packages/nose/importer.py", line 39, in importFromPath
    return self.importFromDir(dir_path, fqname)
  File "/usr/lib/python2.7/dist-packages/nose/importer.py", line 86, in importFromDir
    mod = load_module(part_fqname, fh, filename, desc)
  File "/home/tarmac/repos/lpsetup/trunk/lpsetup/settings.py", line 78, in <module>
    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.

Preview Diff

Empty

Subscribers

People subscribed via source and target branches