Merge lp:~thumper/juju-core/local-ensure-root into lp:~go-bot/juju-core/trunk

[Tim Penhey]

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://codereview.appspot.com/78030044/

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://codereview.appspot.com/78030044/

[Tim Penhey (thumper)]

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.JujuLocal...

[Ian Booth (wallyworld)]

LGTM with suggestion considered

https://codereview.appspot.com/78030044/diff/1/cmd/plugins/local/main.go
File cmd/plugins/local/main.go (right):

https://codereview.appspot.com/78030044/diff/1/cmd/plugins/local/main.go#newcode48
cmd/plugins/local/main.go:48: func ensureRoot(args []string, context
*cmd.Context, call func(*cmd.Context) error) error {
Even though not exported, a doc comment would be nice. Maybe include
that args contains cmd to run in args[0] etc. I think I'd prefer args
split up so that command is passed in explicitly

func ensureRoot(cmd string, cmdArgs []string, context *cmd.Context, call
func(*cmd.Context) error {
}

And maybe call the method runAsRoot()

https://codereview.appspot.com/78030044/

=== 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-20 04:14:57 +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
+	RunAsRoot       = runAsRoot
+	JujuLocalPlugin = jujuLocalPlugin
+)

=== 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-20 04:14:57 +0000
@@ -5,6 +5,7 @@
 
 import (
 	"os"
+	"os/exec"
 
 	"github.com/juju/loggo"
 
@@ -39,3 +40,36 @@
 	plugin := jujuLocalPlugin()
 	os.Exit(cmd.Main(plugin, cmd.DefaultContext(), args[1:]))
 }
+
+var checkIfRoot = func() bool {
+	return os.Getuid() == 0
+}
+
+// runAsRoot ensures that the executable is running as root.
+// If checkIfRoot returns true, the call function is called,
+// otherwise executable is executed using sudo and the extra args
+// passed through.
+func runAsRoot(executable string, 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(executable)
+	if err != nil {
+		return err
+	}
+
+	sudoArgs := []string{"--preserve-env", fullpath}
+	sudoArgs = append(sudoArgs, args...)
+
+	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()
+}

=== 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 04:14:57 +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) TestRunAsRootCallsFuncIfRoot(c *gc.C) {
+	s.PatchValue(local.CheckIfRoot, func() bool { return true })
+	called := false
+	call := func(*cmd.Context) error {
+		called = true
+		return nil
+	}
+	args := []string{"ignored..."}
+	err := local.RunAsRoot("juju-magic", args, coretesting.Context(c), call)
+	c.Assert(err, gc.IsNil)
+	c.Assert(called, jc.IsTrue)
+}
+
+func (s *mainSuite) TestRunAsRootCallsSudoIfNotRoot(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{"passed"}
+	context := coretesting.Context(c)
+	err = local.RunAsRoot("juju-magic", 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)
+}