Merge lp://staging/~fwereade/juju-core/machine-0-units into lp://staging/~juju/juju-core/trunk

Reviewers: mp+161344_code.launchpad.net,

Message:
Please take a look.

Description:
jujud: enable JobHostUnits on machine 0

https://code.launchpad.net/~fwereade/juju-core/machine-0-units/+merge/161344

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/8953044/

Affected files:
   A [revision details]
   M cmd/jujud/bootstrap.go
   M cmd/jujud/bootstrap_test.go

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: <email address hidden>
+New revision: <email address hidden>

Index: cmd/jujud/bootstrap.go
=== modified file 'cmd/jujud/bootstrap.go'
--- cmd/jujud/bootstrap.go 2013-04-10 15:49:40 +0000
+++ cmd/jujud/bootstrap.go 2013-04-28 20:50:44 +0000
@@ -71,11 +71,23 @@
   if err := st.SetEnvironConstraints(c.Constraints); err != nil {
    return err
   }
- // TODO: we need to be able to customize machine jobs, not just hardcode
these.
- m, err := st.InjectMachine(
- version.Current.Series, instanceId,
- state.JobManageEnviron, state.JobServeAPI,
- )
+ // TODO(fwereade): we need to be able to customize machine jobs,
+ // not just hardcode these values; in particular, JobHostUnits
+ // on a machine, like this one, that is running JobManageEnviron
+ // (not to mention the actual state server itself...) will allow
+ // a malicious or compromised unit to trivially access to the
+ // user's environment credentials. However, given that this point
+ // is currently moot (see Upgrader in this package), the pseudo-
+ // local provider mode (in which everything is deployed with
+ // `--force-machine 0`) offers enough value to enough people that
+ // JobHostUnits is currently always enabled. This will one day
+ // have to change, but it's strictly less important than fixing
+ // Upgrader, and it's a capability we'll always want to have
+ // available for the aforementioned use case.
+ jobs := []state.MachineJob{
+ state.JobManageEnviron, state.JobServeAPI, state.JobHostUnits,
+ }
+ m, err := st.InjectMachine(version.Current.Series, instanceId, jobs...)
   if err != nil {
    return err
   }

Index: cmd/jujud/bootstrap_test.go
=== modified file 'cmd/jujud/bootstrap_test.go'
--- cmd/jujud/bootstrap_test.go 2013-04-15 10:28:50 +0000
+++ cmd/jujud/bootstrap_test.go 2013-04-28 20:50:44 +0000
@@ -125,7 +125,9 @@
   defer st.Close()
   m, err := st.Machine("0")
   c.Assert(err, IsNil)
- c.Assert(m.Jobs(), DeepEquals, []state.MachineJob{state.JobManageEnviron,
state.JobServeAPI})
+ c.Assert(m.Jobs(), DeepEquals, []state.MachineJob{
+ state.JobManageEnviron, state.JobServeAPI, state.JobHostUnits,
+ })
  }

  func testOpenState(c *C, info *state.Info, expectErr error) {

On 2013/04/28 20:53:29, fwereade wrote:
> Please take a look.

LGTM - now to move on to containerisation :-)

https://codereview.appspot.com/8953044/

*** Submitted:

jujud: enable JobHostUnits on machine 0

R=thumper, dfc
CC=
https://codereview.appspot.com/8953044

https://codereview.appspot.com/8953044/