Merge pull request #274 from secrethub/feature/refactor-ui

Refactor ui package

Author: SimonBarendse

internals/cli/ui/ask.go

@@ -62,14 +62,14 @@ func AskSecret(io IO, question string) (string, error) {
 		return "", err
 	}
 
-	raw, err := promptIn.ReadPassword()
+	secret, err := readPassword(promptIn)
 	if err != nil {
 		return "", ErrReadInput(err)
 	}
 
 	fmt.Fprintln(promptOut, "")
 
-	return string(raw), nil
+	return secret, nil
 }
 
 // AskMultiline prints out the question and reads back the input until an EOF is reached.

internals/cli/ui/io.go

@@ -18,35 +18,37 @@ var (
 
 // IO is an interface to work with input/output.
 type IO interface {
-	Stdin() Reader
-	Stdout() Writer
-	Prompts() (Reader, Writer, error)
+	Stdin() io.Reader
+	Stdout() io.Writer
+	Prompts() (io.Reader, io.Writer, error)
+	IsStdinPiped() bool
+	IsStdoutPiped() bool
 }
 
 // UserIO is a middleware between input and output to the CLI program.
 // It implements userIO.Prompter and can be passed to libraries.
 type UserIO struct {
-	Input        Reader
-	Output       Writer
-	tty          file
+	Input        *os.File
+	Output       *os.File
+	tty          *os.File
 	ttyAvailable bool
 }
 
 // NewStdUserIO creates a new UserIO middleware only from os.Stdin and os.Stdout.
 func NewStdUserIO() UserIO {
 	return UserIO{
-		Input:  file{os.Stdin},
-		Output: file{os.Stdout},
+		Input:  os.Stdin,
+		Output: os.Stdout,
 	}
 }
 
 // Stdin returns the UserIO's Input.
-func (o UserIO) Stdin() Reader {
+func (o UserIO) Stdin() io.Reader {
 	return o.Input
 }
 
 // Stdout returns the UserIO's Output.
-func (o UserIO) Stdout() Writer {
+func (o UserIO) Stdout() io.Writer {
 	return o.Output
 }
 
@@ -55,8 +57,8 @@ func (o UserIO) Stdout() Writer {
 // bypass stdin and stdout by connecting to /dev/tty on Unix systems when
 // available. On systems where tty is not available and when either input
 // or output is piped, prompting is not possible so an error is returned.
-func (o UserIO) Prompts() (Reader, Writer, error) {
-	if o.Input.IsPiped() || o.Output.IsPiped() {
+func (o UserIO) Prompts() (io.Reader, io.Writer, error) {
+	if o.IsStdoutPiped() || o.IsStdinPiped() {
 		if o.ttyAvailable {
 			return o.tty, o.tty, nil
 		}
@@ -65,12 +67,30 @@ func (o UserIO) Prompts() (Reader, Writer, error) {
 	return o.Input, o.Output, nil
 }
 
-// Reader can read input for a CLI program.
-type Reader interface {
-	io.Reader
-	// ReadPassword reads a line of input from a terminal without local echo.
-	ReadPassword() ([]byte, error)
-	IsPiped() bool
+func (o UserIO) IsStdinPiped() bool {
+	return isPiped(o.Input)
+}
+
+func (o UserIO) IsStdoutPiped() bool {
+	return isPiped(o.Output)
+}
+
+// readPassword reads one line of input from the terminal without echoing the user input.
+func readPassword(r io.Reader) (string, error) {
+	file, ok := r.(*os.File)
+	if !ok {
+		return "", ErrCannotAsk
+	}
+	// this case happens among other things when input is piped and ReadPassword is called.
+	if !terminal.IsTerminal(int(file.Fd())) {
+		return "", ErrCannotAsk
+	}
+
+	password, err := terminal.ReadPassword(int(file.Fd()))
+	if err != nil {
+		return "", err
+	}
+	return string(password), nil
 }
 
 // Readln reads 1 line of input from a io.Reader. The newline character is not included in the response.
@@ -84,31 +104,10 @@ func Readln(r io.Reader) (string, error) {
 	return s.Text(), nil
 }
 
-// Writer can write output for a CLI program.
-type Writer interface {
-	io.Writer
-	IsPiped() bool
-}
-
-// file implements the Reader and Writer interface.
-type file struct {
-	*os.File
-}
-
-// ReadPassword reads from a terminal without echoing back the typed input.
-func (f file) ReadPassword() ([]byte, error) {
-	// this case happens among other things when input is piped and ReadPassword is called.
-	if !terminal.IsTerminal(int(f.Fd())) {
-		return nil, ErrCannotAsk
-	}
-
-	return terminal.ReadPassword(int(f.Fd()))
-}
-
-// IsPiped checks whether the file is a pipe.
+// isPiped checks whether the file is a pipe.
 // If the file does not exist, it returns false.
-func (f file) IsPiped() bool {
-	stat, err := f.Stat()
+func isPiped(file *os.File) bool {
+	stat, err := file.Stat()
 	if err != nil {
 		return false
 	}

internals/cli/ui/io_unix.go

@@ -9,9 +9,9 @@ func NewUserIO() UserIO {
 	tty, err := os.OpenFile("/dev/tty", os.O_RDWR, 0)
 	if err == nil {
 		return UserIO{
-			Input:        file{os.Stdin},
-			Output:       file{os.Stdout},
-			tty:          file{tty},
+			Input:        os.Stdin,
+			Output:       os.Stdout,
+			tty:          tty,
 			ttyAvailable: true,
 		}
 	}

internals/cli/ui/testing.go

@@ -5,6 +5,7 @@ package ui
 import (
 	"bytes"
 	"errors"
+	"io"
 )
 
 // FakeIO is a helper type for testing that implements the ui.IO interface
@@ -35,20 +36,28 @@ func NewFakeIO() *FakeIO {
 }
 
 // Stdin returns the mocked StdIn.
-func (f *FakeIO) Stdin() Reader {
+func (f *FakeIO) Stdin() io.Reader {
 	return f.StdIn
 }
 
 // Stdout returns the mocked StdOut.
-func (f *FakeIO) Stdout() Writer {
+func (f *FakeIO) Stdout() io.Writer {
 	return f.StdOut
 }
 
 // Prompts returns the mocked prompts and error.
-func (f *FakeIO) Prompts() (Reader, Writer, error) {
+func (f *FakeIO) Prompts() (io.Reader, io.Writer, error) {
 	return f.PromptIn, f.PromptOut, f.PromptErr
 }
 
+func (f *FakeIO) IsStdinPiped() bool {
+	return f.StdIn.Piped
+}
+
+func (f *FakeIO) IsStdoutPiped() bool {
+	return f.StdOut.Piped
+}
+
 // FakeReader implements the Reader interface.
 type FakeReader struct {
 	*bytes.Buffer
@@ -58,20 +67,6 @@ type FakeReader struct {
 	ReadErr error
 }
 
-// ReadPassword reads a line from the mocked buffer.
-func (f *FakeReader) ReadPassword() ([]byte, error) {
-	pass, err := Readln(f)
-	if err != nil {
-		return nil, err
-	}
-	return []byte(pass), nil
-}
-
-// IsPiped returns the mocked Piped.
-func (f *FakeReader) IsPiped() bool {
-	return f.Piped
-}
-
 // Read returns the mocked ReadErr or reads from the mocked buffer.
 func (f *FakeReader) Read(p []byte) (n int, err error) {
 	if f.ReadErr != nil {
@@ -92,8 +87,3 @@ type FakeWriter struct {
 	*bytes.Buffer
 	Piped bool
 }
-
-// IsPiped returns the mocked Piped.
-func (f *FakeWriter) IsPiped() bool {
-	return f.Piped
-}

internals/secrethub/inject.go

@@ -91,7 +91,7 @@ func (cmd *InjectCommand) Run() error {
 			return ErrReadFile(cmd.inFile, err)
 		}
 	} else {
-		if !cmd.io.Stdin().IsPiped() {
+		if !cmd.io.IsStdinPiped() {
 			return ErrNoDataOnStdin
 		}
 
@@ -139,7 +139,7 @@ func (cmd *InjectCommand) Run() error {
 	} else if cmd.outFile != "" {
 		_, err := os.Stat(cmd.outFile)
 		if err == nil && !cmd.force {
-			if cmd.io.Stdout().IsPiped() {
+			if cmd.io.IsStdoutPiped() {
 				return ErrFileAlreadyExists
 			}
 

internals/secrethub/service_deploy_winrm.go

@@ -179,7 +179,7 @@ func (cmd *ServiceDeployWinRmCommand) Run() error {
 
 	deployer := newWindowsDeployer(client, destinationPath)
 
-	if !cmd.io.Stdin().IsPiped() {
+	if !cmd.io.IsStdinPiped() {
 		return ErrNoDataOnStdin
 	}
 

internals/secrethub/write.go

@@ -22,6 +22,7 @@ var (
 // WriteCommand is a command to write content to a secret.
 type WriteCommand struct {
 	io           ui.IO
+	askSecret    func(io ui.IO, question string) (string, error)
 	path         api.SecretPath
 	inFile       string
 	multiline    bool
@@ -36,6 +37,7 @@ func NewWriteCommand(io ui.IO, newClient newClientFunc) *WriteCommand {
 	return &WriteCommand{
 		clipper:   clip.NewClipboard(),
 		io:        io,
+		askSecret: ui.AskSecret,
 		newClient: newClient,
 	}
 }
@@ -82,7 +84,7 @@ func (cmd *WriteCommand) Run() error {
 		if err != nil {
 			return ErrReadFile(cmd.inFile, err)
 		}
-	} else if cmd.io.Stdin().IsPiped() {
+	} else if cmd.io.IsStdinPiped() {
 		data, err = ioutil.ReadAll(cmd.io.Stdin())
 		if err != nil {
 			return ui.ErrReadInput(err)
@@ -94,7 +96,7 @@ func (cmd *WriteCommand) Run() error {
 			return err
 		}
 	} else {
-		str, err := ui.AskSecret(cmd.io, "Please type in the value of the secret, followed by an [ENTER]:")
+		str, err := cmd.askSecret(cmd.io, "Please type in the value of the secret, followed by an [ENTER]:")
 		if err != nil {
 			return err
 		}

internals/secrethub/write_test.go

@@ -1,6 +1,7 @@
 package secrethub
 
 import (
+	"bufio"
 	"bytes"
 	"testing"
 
@@ -18,6 +19,19 @@ import (
 func TestWriteCommand_Run(t *testing.T) {
 	testErr := errio.Namespace("test").Code("test").Error("test error")
 
+	fakeAskSecretFunc := func(io ui.IO, question string) (s string, err error) {
+		reader, writer, err := io.Prompts()
+		if err != nil {
+			return "", err
+		}
+		_, err = writer.Write([]byte(question + "\n"))
+		if err != nil {
+			return "", err
+		}
+		line, _, err := bufio.NewReader(reader).ReadLine()
+		return string(line), err
+	}
+
 	cases := map[string]struct {
 		cmd       WriteCommand
 		writeFunc func(path string, data []byte) (*api.SecretVersion, error)
@@ -121,7 +135,8 @@ func TestWriteCommand_Run(t *testing.T) {
 		},
 		"ask secret success": {
 			cmd: WriteCommand{
-				path: "namespace/repo/secret",
+				path:      "namespace/repo/secret",
+				askSecret: fakeAskSecretFunc,
 			},
 			promptIn:  "asked secret value",
 			promptOut: "Please type in the value of the secret, followed by an [ENTER]:\n",
@@ -137,7 +152,8 @@ func TestWriteCommand_Run(t *testing.T) {
 		},
 		"ask secret error": {
 			cmd: WriteCommand{
-				path: "namespace/repo/secret",
+				path:      "namespace/repo/secret",
+				askSecret: fakeAskSecretFunc,
 			},
 			promptErr: testErr,
 			err:       testErr,