Code review comment for lp:~thumper/juju-core/local-ensure-root

Reviewers: mp+211849_code.launchpad.net,

Message:
Please take a look.

Description:
Implement a helper function for sudo.

Most of the local plugin helper commands will need
to run as sudo. Instead of each writing their own
code to handle this, provide a helper function.

https://code.launchpad.net/~thumper/juju-core/local-ensure-root/+merge/211849

(do not edit description out of merge proposal)

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

Affected files (+79, -4 lines):
   A [revision details]
   M cmd/plugins/local/export_test.go
   M cmd/plugins/local/main.go
   M cmd/plugins/local/main_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: tarmac-20140320011142-7ljymhlzvpq44apl
+New revision: <email address hidden>

Index: cmd/plugins/local/export_test.go
=== modified file 'cmd/plugins/local/export_test.go'
--- cmd/plugins/local/export_test.go 2014-02-27 05:16:54 +0000
+++ cmd/plugins/local/export_test.go 2014-03-19 21:53:07 +0000
@@ -3,4 +3,11 @@

  package local

-var JujuLocalPlugin = jujuLocalPlugin
+var (
+ // Variable is the address so we can PatchValue
+ CheckIfRoot = &checkIfRoot
+
+ // function exports for tests
+ EnsureRoot = ensureRoot
+ JujuLocalPlugin = jujuLocalPlugin
+)

Index: cmd/plugins/local/main.go
=== modified file 'cmd/plugins/local/main.go'
--- cmd/plugins/local/main.go 2014-03-05 19:41:34 +0000
+++ cmd/plugins/local/main.go 2014-03-19 22:29:47 +0000
@@ -5,6 +5,7 @@

  import (
   "os"
+ "os/exec"

   "github.com/juju/loggo"

@@ -39,3 +40,32 @@
   plugin := jujuLocalPlugin()
   os.Exit(cmd.Main(plugin, cmd.DefaultContext(), args[1:]))
  }
+
+var checkIfRoot = func() bool {
+ return os.Getuid() == 0
+}
+
+func ensureRoot(args []string, context *cmd.Context, call
func(*cmd.Context) error) error {
+ if checkIfRoot() {
+ logger.Debugf("running as root")
+ return call(context)
+ }
+
+ logger.Debugf("running as user")
+
+ fullpath, err := exec.LookPath(args[0])
+ if err != nil {
+ return err
+ }
+
+ sudoArgs := []string{"--preserve-env", fullpath}
+ sudoArgs = append(sudoArgs, args[1:]...)
+
+ command := exec.Command("sudo", sudoArgs...)
+ // Now hook up stdin, stdout, stderr
+ command.Stdin = context.Stdin
+ command.Stdout = context.Stdout
+ command.Stderr = context.Stderr
+ // And run it!
+ return command.Run()
+}

Index: cmd/plugins/local/main_test.go
=== modified file 'cmd/plugins/local/main_test.go'
--- cmd/plugins/local/main_test.go 2014-02-27 05:16:54 +0000
+++ cmd/plugins/local/main_test.go 2014-03-20 01:58:59 +0000
@@ -4,12 +4,17 @@
  package local_test

  import (
+ "fmt"
+ "os/exec"
   "strings"

+ "github.com/juju/testing"
+ jc "github.com/juju/testing/checkers"
   gc "launchpad.net/gocheck"

+ "launchpad.net/juju-core/cmd"
   "launchpad.net/juju-core/cmd/plugins/local"
- "launchpad.net/juju-core/testing"
+ coretesting "launchpad.net/juju-core/testing"
   "launchpad.net/juju-core/testing/testbase"
  )

@@ -25,10 +30,10 @@
    // TODO: add some as they get registered
   }
   plugin := local.JujuLocalPlugin()
- ctx, err := testing.RunCommand(c, plugin, []string{"help", "commands"})
+ ctx, err := coretesting.RunCommand(c, plugin,
[]string{"help", "commands"})
   c.Assert(err, gc.IsNil)

- lines := strings.Split(testing.Stdout(ctx), "\n")
+ lines := strings.Split(coretesting.Stdout(ctx), "\n")
   var names []string
   for _, line := range lines {
    f := strings.Fields(line)
@@ -40,3 +45,34 @@
   // The names should be output in alphabetical order, so don't sort.
   c.Assert(names, gc.DeepEquals, expectedSubcommands)
  }
+
+func (s *mainSuite) TestEnsureRootCallsFuncIfRoot(c *gc.C) {
+ s.PatchValue(local.CheckIfRoot, func() bool { return true })
+ called := false
+ call := func(*cmd.Context) error {
+ called = true
+ return nil
+ }
+ args := []string{"juju-magic", "ignored..."}
+ err := local.EnsureRoot(args, coretesting.Context(c), call)
+ c.Assert(err, gc.IsNil)
+ c.Assert(called, jc.IsTrue)
+}
+
+func (s *mainSuite) TestEnsureRootCallsSudoIfNotRoot(c *gc.C) {
+ s.PatchValue(local.CheckIfRoot, func() bool { return false })
+ testing.PatchExecutableAsEchoArgs(c, s, "sudo")
+ // the command needs to be in the path...
+ testing.PatchExecutableAsEchoArgs(c, s, "juju-magic")
+ magicPath, err := exec.LookPath("juju-magic")
+ c.Assert(err, gc.IsNil)
+ callIgnored := func(*cmd.Context) error {
+ panic("unreachable")
+ }
+ args := []string{"juju-magic", "passed"}
+ context := coretesting.Context(c)
+ err = local.EnsureRoot(args, context, callIgnored)
+ c.Assert(err, gc.IsNil)
+ expected := fmt.Sprintf("sudo \"--preserve-env\" %q \"passed\"\n",
magicPath)
+ c.Assert(coretesting.Stdout(context), gc.Equals, expected)
+}