feat(creds): Add edit credentials command (#921)

* feat(creds): Add edit credentials command

* ensure EDITOR env var is checked

* Edit the entire credential set instead of picking each one

* fix spaces in EDITOR not opening editor correctly, e.g. using code.exe --wait

* add test, change editor to use context.CommandBuilder so it works

* add test for windows editor path

* Changes from feedback:

 - shift editor into separate package, get it through editor.New()
 - embed the context struct into editor directly
 - use context.FileSystem instead of ioutil/os calls
 - fix editor to split on spaces, add comment on why and examples
 - remove "tempStrategy" stuff, edit the actual credentials directly
 - shift windows-specific check into conditional compilation
   with editor_windows.go, editor_nix.go etc

* fix parsing of editor command, pass this to the shell to do it for us

* fix tests

* remove restriction on editing an empty credential set (it's possible)

* use yaml extension for temp credential file for editor syntax, colors etc

* minor variable naming, use Wrap instead of Wrapf, and unnecessary byte casting

* minor tweaks to comment

* validate source keys are correct after editing credentials

* use multierrors for validation errors

* Adding tests for CredentialStorage.Validate

* show some slightly friendlier errors

Author: halkyon

cmd/porter/credentials.go

@@ -2,7 +2,6 @@ package main
 
 import (
 	"get.porter.sh/porter/pkg/porter"
-	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
 )
 
@@ -24,15 +23,17 @@ func buildCredentialsCommands(p *porter.Porter) *cobra.Command {
 }
 
 func buildCredentialsEditCommand(p *porter.Porter) *cobra.Command {
+	opts := porter.CredentialEditOptions{}
+
 	cmd := &cobra.Command{
-		Use:    "edit",
-		Short:  "Edit Credential",
-		Hidden: true,
+		Use:   "edit",
+		Short: "Edit Credential",
+		Long:  `Edit a named credential set.`,
 		PreRunE: func(cmd *cobra.Command, args []string) error {
-			return nil
+			return opts.Validate(args)
 		},
 		RunE: func(cmd *cobra.Command, args []string) error {
-			return errors.New("Not implemented")
+			return p.EditCredential(opts)
 		},
 	}
 	return cmd

docs/content/cli/credentials.md

@@ -27,6 +27,7 @@ Credentials commands
 
 * [porter](/cli/porter/)	 - I am porter 👩🏽‍✈️, the friendly neighborhood CNAB authoring tool
 * [porter credentials delete](/cli/porter_credentials_delete/)	 - Delete a Credential
+* [porter credentials edit](/cli/porter_credentials_edit/)	 - Edit Credential
 * [porter credentials generate](/cli/porter_credentials_generate/)	 - Generate Credential Set
 * [porter credentials list](/cli/porter_credentials_list/)	 - List credentials
 * [porter credentials show](/cli/porter_credentials_show/)	 - Show a Credential

docs/content/cli/credentials_edit.md

@@ -0,0 +1,33 @@
+---
+title: "porter credentials edit"
+slug: porter_credentials_edit
+url: /cli/porter_credentials_edit/
+---
+## porter credentials edit
+
+Edit Credential
+
+### Synopsis
+
+Edit a named credential set.
+
+```
+porter credentials edit [flags]
+```
+
+### Options
+
+```
+  -h, --help   help for edit
+```
+
+### Options inherited from parent commands
+
+```
+      --debug   Enable debug logging
+```
+
+### SEE ALSO
+
+* [porter credentials](/cli/porter_credentials/)	 - Credentials commands
+

pkg/credentials/credentialProvider.go

@@ -8,6 +8,7 @@ import (
 type CredentialProvider interface {
 	CredentialStore
 	ResolveAll(creds credentials.CredentialSet) (credentials.Set, error)
+	Validate(credentials.CredentialSet) error
 }
 
 // CredentialStore is an interface representing cnab-go's credentials.Store

pkg/credentials/credentialStorage.go

@@ -1,12 +1,16 @@
 package credentials
 
 import (
+	"fmt"
+	"strings"
+
 	"get.porter.sh/porter/pkg/config"
 	"get.porter.sh/porter/pkg/secrets"
 	secretplugins "get.porter.sh/porter/pkg/secrets/pluginstore"
 	crudplugins "get.porter.sh/porter/pkg/storage/pluginstore"
 	"github.com/cnabio/cnab-go/credentials"
 	cnabsecrets "github.com/cnabio/cnab-go/secrets"
+	"github.com/cnabio/cnab-go/secrets/host"
 	"github.com/hashicorp/go-multierror"
 	"github.com/pkg/errors"
 )
@@ -49,3 +53,27 @@ func (s CredentialStorage) ResolveAll(creds credentials.CredentialSet) (credenti
 
 	return resolvedCreds, resolveErrors
 }
+
+func (s CredentialStorage) Validate(creds credentials.CredentialSet) error {
+	validSources := []string{"secret", host.SourceValue, host.SourceEnv, host.SourcePath, host.SourceCommand}
+	var errors error
+
+	for _, cs := range creds.Credentials {
+		valid := false
+		for _, validSource := range validSources {
+			if cs.Source.Key == validSource {
+				valid = true
+				break
+			}
+		}
+		if valid == false {
+			errors = multierror.Append(errors, fmt.Errorf(
+				"%s is not a valid source. Valid sources are: %s",
+				cs.Source.Key,
+				strings.Join(validSources, ", "),
+			))
+		}
+	}
+
+	return errors
+}

pkg/credentials/credentialStorage_test.go

@@ -0,0 +1,54 @@
+package credentials
+
+import (
+	"testing"
+
+	"github.com/cnabio/cnab-go/credentials"
+	"github.com/stretchr/testify/require"
+)
+
+func TestCredentialStorage_Validate_GoodSources(t *testing.T) {
+	s := CredentialStorage{}
+	testCreds := credentials.CredentialSet{
+		Credentials: []credentials.CredentialStrategy{
+			{
+				Source: credentials.Source{
+					Key:   "env",
+					Value: "SOME_ENV",
+				},
+			},
+			{
+				Source: credentials.Source{
+					Key:   "value",
+					Value: "somevalue",
+				},
+			},
+		},
+	}
+
+	err := s.Validate(testCreds)
+	require.NoError(t, err, "Validate did not return errors")
+}
+
+func TestCredentialStorage_Validate_BadSources(t *testing.T) {
+	s := CredentialStorage{}
+	testCreds := credentials.CredentialSet{
+		Credentials: []credentials.CredentialStrategy{
+			{
+				Source: credentials.Source{
+					Key:   "wrongthing",
+					Value: "SOME_ENV",
+				},
+			},
+			{
+				Source: credentials.Source{
+					Key:   "anotherwrongthing",
+					Value: "somevalue",
+				},
+			},
+		},
+	}
+
+	err := s.Validate(testCreds)
+	require.Error(t, err, "Validate returned errors")
+}

pkg/editor/editor.go

@@ -0,0 +1,88 @@
+package editor
+
+import (
+	"fmt"
+	"os"
+	"path/filepath"
+
+	"get.porter.sh/porter/pkg/context"
+)
+
+// Editor displays content to a user using an external text editor, like vi or notepad.
+// The content is captured and returned.
+//
+// The `EDITOR` environment variable is checked to find an editor.
+// Failing that, use some sensible default depending on the operating system.
+//
+// This is useful for editing things like configuration files, especially those
+// that might be stored on a remote server. For example: the content could be retrieved
+// from the remote store, edited locally, then saved back.
+type Editor struct {
+	*context.Context
+	contents     []byte
+	tempFilename string
+}
+
+// New returns a new Editor with the temp filename and contents provided.
+func New(context *context.Context, tempFilename string, contents []byte) *Editor {
+	return &Editor{
+		Context:      context,
+		tempFilename: tempFilename,
+		contents:     contents,
+	}
+}
+
+func editorArgs(filename string) []string {
+	shell := defaultShell
+	if os.Getenv("SHELL") != "" {
+		shell = os.Getenv("SHELL")
+	}
+	editor := defaultEditor
+	if os.Getenv("EDITOR") != "" {
+		editor = os.Getenv("EDITOR")
+	}
+
+	// Example of what will be run:
+	// on *nix: sh -c "vi /tmp/test.txt"
+	// on windows: cmd /C "C:\Program Files\Visual Studio Code\Code.exe --wait C:\somefile.txt"
+	//
+	// Pass the editor command to the shell so we don't have to parse the command ourselves.
+	// Passing the editor command that could possibly have an argument (e.g. --wait for VSCode) to the
+	// shell means we don't have to parse this ourselves, like splitting on spaces.
+	return []string{shell, shellCommandFlag, fmt.Sprintf("%s %s", editor, filename)}
+}
+
+// Run opens the editor, displaying the contents through a temporary file.
+// The content is returned once the editor closes.
+func (e *Editor) Run() ([]byte, error) {
+	tempFile, err := e.FileSystem.OpenFile(filepath.Join(os.TempDir(), e.tempFilename), os.O_RDWR|os.O_CREATE|os.O_EXCL, 0600)
+	if err != nil {
+		return nil, err
+	}
+	defer e.FileSystem.Remove(tempFile.Name())
+
+	_, err = tempFile.Write(e.contents)
+	if err != nil {
+		return nil, err
+	}
+
+	// close here without defer so cmd can grab the file
+	tempFile.Close()
+
+	args := editorArgs(tempFile.Name())
+	cmd := e.NewCommand(args[0], args[1:]...)
+	cmd.Stdout = e.Out
+	cmd.Stderr = e.Err
+	cmd.Stdin = e.In
+	err = cmd.Run()
+	if err != nil {
+		return nil, err
+	}
+
+	contents, err := e.FileSystem.ReadFile(tempFile.Name())
+	if err != nil {
+		return nil, err
+	}
+
+	return contents, nil
+}

pkg/editor/editor_nix.go

@@ -0,0 +1,7 @@
+// +build !windows
+
+package editor
+
+const defaultEditor = "vi"
+const defaultShell = "sh"
+const shellCommandFlag = "-c"

pkg/editor/editor_windows.go

@@ -0,0 +1,7 @@
+// +build windows
+
+package editor
+
+const defaultEditor = "notepad"
+const defaultShell = "cmd"
+const shellCommandFlag = "/C"

pkg/porter/credentials.go

@@ -7,7 +7,9 @@ import (
 
 	"get.porter.sh/porter/pkg/context"
 	"get.porter.sh/porter/pkg/credentialsgenerator"
+	"get.porter.sh/porter/pkg/editor"
 	"get.porter.sh/porter/pkg/printer"
+	"gopkg.in/yaml.v2"
 
 	dtprinter "github.com/carolynvs/datetime-printer"
 	credentials "github.com/cnabio/cnab-go/credentials"
@@ -22,6 +24,10 @@ type CredentialShowOptions struct {
 	Name string
 }
 
+type CredentialEditOptions struct {
+	Name string
+}
+
 // ListCredentials lists saved credential sets.
 func (p *Porter) ListCredentials(opts ListOptions) error {
 	creds, err := p.Credentials.ReadAll()
@@ -137,7 +143,7 @@ func (p *Porter) GenerateCredentials(opts CredentialOptions) error {
 	return errors.Wrapf(err, "unable to save credentials")
 }
 
-// Validate validates the args provided Porter's credential show command
+// Validate validates the args provided to Porter's credential show command
 func (o *CredentialShowOptions) Validate(args []string) error {
 	if err := validateCredentialName(args); err != nil {
 		return err
@@ -146,6 +152,52 @@ func (o *CredentialShowOptions) Validate(args []string) error {
 	return o.ParseFormat()
 }
 
+// Validate validates the args provided to Porter's credential edit command
+func (o *CredentialEditOptions) Validate(args []string) error {
+	if err := validateCredentialName(args); err != nil {
+		return err
+	}
+	o.Name = args[0]
+	return nil
+}
+
+// EditCredential edits the credentials of the provided name.
+func (p *Porter) EditCredential(opts CredentialEditOptions) error {
+	credSet, err := p.Credentials.Read(opts.Name)
+	if err != nil {
+		return err
+	}
+
+	contents, err := yaml.Marshal(credSet)
+	if err != nil {
+		return errors.Wrap(err, "unable to load credentials")
+	}
+
+	editor := editor.New(p.Context, fmt.Sprintf("porter-%s.yaml", credSet.Name), contents)
+	output, err := editor.Run()
+	if err != nil {
+		return errors.Wrap(err, "unable to open editor to edit credentials")
+	}
+
+	err = yaml.Unmarshal(output, &credSet)
+	if err != nil {
+		return errors.Wrap(err, "unable to process credentials")
+	}
+
+	err = p.Credentials.Validate(credSet)
+	if err != nil {
+		return errors.Wrap(err, "credentials are invalid")
+	}
+
+	credSet.Modified = time.Now()
+	err = p.Credentials.Save(credSet)
+	if err != nil {
+		return errors.Wrap(err, "unable to save credentials")
+	}
+
+	return nil
+}
+
 // ShowCredential shows the credential set corresponding to the provided name, using
 // the provided printer.PrintOptions for display.
 func (p *Porter) ShowCredential(opts CredentialShowOptions) error {