MAAS

Merge lp:~andreserl/maas/maas_lp1040047 into lp:maas/trunk

  1. maas_lp1040047
  2. Merge into trunk
Proposed by Andres Rodriguez on 2012-08-22
Status: Merged
Approved by: Andres Rodriguez on 2012-08-22
Approved revision: 910
Merged at revision: 910
Proposed branch: lp:~andreserl/maas/maas_lp1040047
Merge into: lp:maas/trunk
Diff against target: 15 lines (+5/-1)
1 file modified
src/provisioningserver/omshell.py (+5/-1)
To merge this branch: bzr merge lp:~andreserl/maas/maas_lp1040047
Reviewer Review Type Date Requested Status
Gavin Panella (community) 2012-08-22 Approve on 2012-08-22
Review via email: mp+120769@code.launchpad.net

Commit Message

Use dnssec-keygen absolute path otherwise MAAS fails (LP: #1040047)

To post a comment you must log in.
Gavin Panella (allenap) wrote :

> === modified file 'src/provisioningserver/omshell.py'
> --- src/provisioningserver/omshell.py   2012-08-07 10:20:38 +0000
> +++ src/provisioningserver/omshell.py   2012-08-22 13:35:57 +0000
> @@ -32,8 +32,12 @@
>
>
>  def call_dnssec_keygen(tmpdir):
> +    dnssec_keygen = 'dnssec-keygen'
> +    if os.path.exists('/usr/sbin/dnssec-keygen'):
> +        dnssec_keygen = '/usr/sbin/dnssec-keygen'
> +
>      return check_output(
> -        ['dnssec-keygen', '-r', '/dev/urandom', '-a', 'HMAC-MD5',
> +        [dnssec_keygen, '-r', '/dev/urandom', '-a', 'HMAC-MD5',
>           '-b', '512', '-n', 'HOST', '-K', tmpdir, '-q', 'omapi_key'])

This is okay, but can I suggest an alternative approach?

I think it would be better to run the command in an environment with a
modified PATH, where /usr/sbin is appended. That way, if the caller
wants to use a different dnssec-keygen they can. The attached diff
demonstrates what I mean.

Another alternative would be to always use /usr/sbin/dnssec-keygen. If
they don't have it, they need to install it.

 +1

alt.diff
1=== modified file 'src/provisioningserver/omshell.py'
2--- src/provisioningserver/omshell.py 2012-08-07 10:20:38 +0000
3+++ src/provisioningserver/omshell.py 2012-08-22 16:24:23 +0000
4@@ -32,9 +32,13 @@
5
6
7 def call_dnssec_keygen(tmpdir):
8+ path = os.environ.get("PATH", "").split(os.pathsep)
9+ path.append("/usr/sbin")
10+ env = dict(os.environ, PATH=os.pathsep.join(path))
11 return check_output(
12 ['dnssec-keygen', '-r', '/dev/urandom', '-a', 'HMAC-MD5',
13- '-b', '512', '-n', 'HOST', '-K', tmpdir, '-q', 'omapi_key'])
14+ '-b', '512', '-n', 'HOST', '-K', tmpdir, '-q', 'omapi_key'],
15+ env=env)
16
17
18 def generate_omapi_key():
Gavin Panella (allenap) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/provisioningserver/omshell.py'
2--- src/provisioningserver/omshell.py 2012-08-07 10:20:38 +0000
3+++ src/provisioningserver/omshell.py 2012-08-22 13:35:57 +0000
4@@ -32,8 +32,12 @@
5
6
7 def call_dnssec_keygen(tmpdir):
8+ dnssec_keygen = 'dnssec-keygen'
9+ if os.path.exists('/usr/sbin/dnssec-keygen'):
10+ dnssec_keygen = '/usr/sbin/dnssec-keygen'
11+
12 return check_output(
13- ['dnssec-keygen', '-r', '/dev/urandom', '-a', 'HMAC-MD5',
14+ [dnssec_keygen, '-r', '/dev/urandom', '-a', 'HMAC-MD5',
15 '-b', '512', '-n', 'HOST', '-K', tmpdir, '-q', 'omapi_key'])
16
17