From 0ed0896ca6cf5bc281aa6db1df2fb90e155f1e8c Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Thu, 16 Apr 2015 14:13:58 -0400 Subject: [PATCH 1/3] UPSTREAM: It should be possible to have lists with mixed versions --- .../kubernetes/pkg/kubectl/resource/result.go | 49 +++++++++++++++++++ .../kubernetes/pkg/runtime/scheme.go | 11 +++-- 2 files changed, 57 insertions(+), 3 deletions(-) diff --git a/Godeps/_workspace/src/github.com/GoogleCloudPlatform/kubernetes/pkg/kubectl/resource/result.go b/Godeps/_workspace/src/github.com/GoogleCloudPlatform/kubernetes/pkg/kubectl/resource/result.go index 13e5b89638b1..e1d5526c88f6 100644 --- a/Godeps/_workspace/src/github.com/GoogleCloudPlatform/kubernetes/pkg/kubectl/resource/result.go +++ b/Godeps/_workspace/src/github.com/GoogleCloudPlatform/kubernetes/pkg/kubectl/resource/result.go @@ -21,6 +21,7 @@ import ( "reflect" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + "github.com/GoogleCloudPlatform/kubernetes/pkg/api/latest" "github.com/GoogleCloudPlatform/kubernetes/pkg/api/meta" "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" "github.com/GoogleCloudPlatform/kubernetes/pkg/util" @@ -200,3 +201,51 @@ func (r *Result) Watch(resourceVersion string) (watch.Interface, error) { } return w.Watch(resourceVersion) } + +// AsVersionedObject converts a list of infos into a single object - either a List containing +// the objects as children, or if only a single Object is present, as that object. The provided +// version will be preferred as the conversion target, but the Object's mapping version will be +// used if that version is not present. +func AsVersionedObject(infos []*Info, version string) (runtime.Object, error) { + objects := []runtime.Object{} + for _, info := range infos { + if info.Object == nil { + continue + } + converted, err := tryConvert(info.Mapping.ObjectConvertor, info.Object, version, info.Mapping.APIVersion) + if err != nil { + return nil, err + } + objects = append(objects, converted) + } + var object runtime.Object + if len(objects) == 1 { + object = objects[0] + } else { + object = &api.List{Items: objects} + converted, err := tryConvert(api.Scheme, object, version, latest.Version) + if err != nil { + return nil, err + } + object = converted + } + return object, nil +} + +// tryConvert attempts to convert the given object to the provided versions in order. This function assumes +// the object is in internal version. +func tryConvert(convertor runtime.ObjectConvertor, object runtime.Object, versions ...string) (runtime.Object, error) { + var last error + for _, version := range versions { + if len(version) == 0 { + return object, nil + } + obj, err := convertor.ConvertToVersion(object, version) + if err != nil { + last = err + continue + } + return obj, nil + } + return nil, last +} diff --git a/Godeps/_workspace/src/github.com/GoogleCloudPlatform/kubernetes/pkg/runtime/scheme.go b/Godeps/_workspace/src/github.com/GoogleCloudPlatform/kubernetes/pkg/runtime/scheme.go index 0e0ed67070bd..b30ee800d8fa 100644 --- a/Godeps/_workspace/src/github.com/GoogleCloudPlatform/kubernetes/pkg/runtime/scheme.go +++ b/Godeps/_workspace/src/github.com/GoogleCloudPlatform/kubernetes/pkg/runtime/scheme.go @@ -147,8 +147,8 @@ func (self *Scheme) rawExtensionToEmbeddedObject(in *RawExtension, out *Embedded } // runtimeObjectToRawExtensionArray takes a list of objects and encodes them as RawExtension in the output version -// defined by the conversion.Scope. If objects must be encoded to different schema versions you should set them as -// runtime.Unknown in the internal version instead. +// defined by the conversion.Scope. If objects must be encoded to different schema versions than the default, you +// should encode them yourself with runtime.Unknown, or convert the object prior to invoking conversion. func (self *Scheme) runtimeObjectToRawExtensionArray(in *[]Object, out *[]RawExtension, s conversion.Scope) error { src := *in dest := make([]RawExtension, len(src)) @@ -160,7 +160,12 @@ func (self *Scheme) runtimeObjectToRawExtensionArray(in *[]Object, out *[]RawExt case *Unknown: dest[i].RawJSON = t.RawJSON default: - data, err := scheme.EncodeToVersion(src[i], outVersion) + version := outVersion + // if the object exists + if inVersion, _, err := scheme.ObjectVersionAndKind(src[i]); err == nil && len(inVersion) != 0 { + version = inVersion + } + data, err := scheme.EncodeToVersion(src[i], version) if err != nil { return err } From a4b4fecf6ebea3456e93c2c6c438931f45ce3430 Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Mon, 13 Apr 2015 18:06:47 -0400 Subject: [PATCH 2/3] Add an edit command `osc edit` allows any resource on the server to be edited directly from the CLI, which greatly simplifies the process of making changes to objects. The edited object will be the API object in YAML format by default. Errors will be captured to the comment at the top of the file and then output to the user. Validation errors will allow the user to attempt to edit the file a second time. --- hack/test-cmd.sh | 4 + pkg/cmd/cli/cli.go | 1 + pkg/cmd/cli/cmd/edit.go | 335 ++++++++++++++++++++++++ pkg/cmd/util/editor/editor.go | 160 +++++++++++ pkg/cmd/util/editor/editor_test.go | 46 ++++ pkg/cmd/util/editor/term.go | 11 + pkg/cmd/util/editor/term_unsupported.go | 10 + 7 files changed, 567 insertions(+) create mode 100644 pkg/cmd/cli/cmd/edit.go create mode 100644 pkg/cmd/util/editor/editor.go create mode 100644 pkg/cmd/util/editor/editor_test.go create mode 100644 pkg/cmd/util/editor/term.go create mode 100644 pkg/cmd/util/editor/term_unsupported.go diff --git a/hack/test-cmd.sh b/hack/test-cmd.sh index 63465d01cb73..0f5d4125dfba 100755 --- a/hack/test-cmd.sh +++ b/hack/test-cmd.sh @@ -370,6 +370,10 @@ osc status [ "$(osc status | grep frontend-service)" ] echo "template+config: ok" +[ "$(OSC_EDITOR='cat' osc edit svc/kubernetes 2>&1 | grep 'Edit cancelled')" ] +[ "$(OSC_EDITOR='cat' osc edit svc/kubernetes | grep 'provider: kubernetes')" ] +echo "edit: ok" + openshift kube resize --replicas=2 rc guestbook osc get pods echo "resize: ok" diff --git a/pkg/cmd/cli/cli.go b/pkg/cmd/cli/cli.go index 364bccb1387d..00af69fddbd5 100644 --- a/pkg/cmd/cli/cli.go +++ b/pkg/cmd/cli/cli.go @@ -75,6 +75,7 @@ func NewCommandCLI(name, fullName string) *cobra.Command { // Deprecate 'osc apply' with 'osc create' command. cmds.AddCommand(applyToCreate(cmd.NewCmdCreate(fullName, f, out))) cmds.AddCommand(cmd.NewCmdProcess(fullName, f, out)) + cmds.AddCommand(cmd.NewCmdEdit(fullName, f, out)) cmds.AddCommand(cmd.NewCmdUpdate(fullName, f, out)) cmds.AddCommand(cmd.NewCmdDelete(fullName, f, out)) cmds.AddCommand(cmd.NewCmdLog(fullName, f, out)) diff --git a/pkg/cmd/cli/cmd/edit.go b/pkg/cmd/cli/cmd/edit.go new file mode 100644 index 000000000000..111a7e373428 --- /dev/null +++ b/pkg/cmd/cli/cmd/edit.go @@ -0,0 +1,335 @@ +package cmd + +import ( + "bufio" + "bytes" + "fmt" + "io" + "os" + "strings" + + "github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors" + kclient "github.com/GoogleCloudPlatform/kubernetes/pkg/client" + "github.com/GoogleCloudPlatform/kubernetes/pkg/kubectl" + cmdutil "github.com/GoogleCloudPlatform/kubernetes/pkg/kubectl/cmd/util" + "github.com/GoogleCloudPlatform/kubernetes/pkg/kubectl/resource" + "github.com/GoogleCloudPlatform/kubernetes/pkg/util" + + "github.com/golang/glog" + "github.com/spf13/cobra" + + "github.com/openshift/origin/pkg/cmd/util/clientcmd" + "github.com/openshift/origin/pkg/cmd/util/editor" +) + +const ( + edit_long = `Edit a resource from the default editor. + +The edit command allows you to directly edit any API resource you can retrieve via the +command line tools. It will open the editor defined by your OSC_EDITOR, GIT_EDITOR, +or EDITOR environment variables, or fall back to 'vi'. You can edit multiple objects, +although changes are applied one at a time. The command accepts filenames as well as +command line arguments, although the files you point to must be previously saved +versions of resources. + +The files to edit will be output in the default API version, or a version specified +by --output-version. The default format is YAML - if you would like to edit in JSON +pass -o json. + +In the event an error occurs while updating, a temporary file will be created on disk +that contains your unapplied changes. The most common error when updating a resource +is another editor changing the resource on the server. When this occurs, you will have +to apply your changes to the newer version of the resource, or update your temporary +saved copy to include the latest resource version. + +Examples: + + # Edit the service named 'docker-registry': + $ %[1]s edit svc/docker-registry + + # Edit the deployment config named 'my-deployment': + $ %[1]s edit dc/my-deployment + + # Use an alternative editor + $ OSC_EDITOR="nano" %[1]s edit dc/my-deployment + + # Edit the service 'docker-registry' in JSON using the v1beta3 API format: + $ %[1]s edit svc/docker-registry --output-version=v1beta3 -o json +` +) + +func NewCmdEdit(fullName string, f *clientcmd.Factory, out io.Writer) *cobra.Command { + var filenames util.StringList + cmd := &cobra.Command{ + Use: "edit -f FILENAME", + Short: "Edit a resource on the server and apply the update.", + Long: fmt.Sprintf(edit_long, fullName), + Run: func(cmd *cobra.Command, args []string) { + err := RunEdit(fullName, f, out, cmd, args, filenames) + if err == errExit { + os.Exit(1) + } + cmdutil.CheckErr(err) + }, + } + cmd.Flags().StringP("output", "o", "yaml", "Output format. One of: yaml|json.") + cmd.Flags().String("output-version", "", "Output the formatted object with the given version (default api-version).") + cmd.Flags().VarP(&filenames, "filename", "f", "Filename, directory, or URL to file to use to edit the resource.") + return cmd +} + +func RunEdit(fullName string, f *clientcmd.Factory, out io.Writer, cmd *cobra.Command, args []string, filenames util.StringList) error { + var printer kubectl.ResourcePrinter + switch format := cmdutil.GetFlagString(cmd, "output"); format { + case "json": + printer = &kubectl.JSONPrinter{} + case "yaml": + printer = &kubectl.YAMLPrinter{} + default: + return cmdutil.UsageError(cmd, "The flag 'output' must be one of yaml|json") + } + + cmdNamespace, err := f.DefaultNamespace() + if err != nil { + return err + } + + mapper, typer := f.Object() + rmap := &resource.Mapper{ + ObjectTyper: typer, + RESTMapper: mapper, + ClientMapper: f.ClientMapperForCommand(), + } + + b := resource.NewBuilder(mapper, typer, f.ClientMapperForCommand()). + NamespaceParam(cmdNamespace).DefaultNamespace(). + FilenameParam(filenames...). + //SelectorParam(selector). + ResourceTypeOrNameArgs(true, args...). + Latest() + if err != nil { + return err + } + + clientConfig, err := f.ClientConfig() + if err != nil { + return err + } + + r := b.Flatten().Do() + infos, err := r.Infos() + if err != nil { + return err + } + + defaultVersion := cmdutil.OutputVersion(cmd, clientConfig.Version) + results := editResults{} + for { + obj, err := resource.AsVersionedObject(infos, defaultVersion) + if err != nil { + return preservedFile(err, results.file, cmd.Out()) + } + + // TODO: add an annotating YAML printer that can print inline comments on each field, + // including descriptions or validation errors + + // generate the file to edit + buf := &bytes.Buffer{} + if err := results.header.WriteTo(buf); err != nil { + return preservedFile(err, results.file, cmd.Out()) + } + if err := printer.PrintObj(obj, buf); err != nil { + return preservedFile(err, results.file, cmd.Out()) + } + original := buf.Bytes() + + // launch the editor + edit := editor.NewDefaultEditor() + edited, file, err := edit.LaunchTempFile("", "osc-edit-", buf) + if err != nil { + return preservedFile(err, results.file, cmd.Out()) + } + + // cleanup any file from the previous pass + if len(results.file) > 0 { + os.Remove(results.file) + } + + glog.V(4).Infof("User edited:\n%s", string(edited)) + lines, err := hasLines(bytes.NewBuffer(edited)) + if err != nil { + return preservedFile(err, file, cmd.Out()) + } + if bytes.Equal(original, edited) { + if len(results.edit) > 0 { + preservedFile(nil, file, cmd.Out()) + } else { + os.Remove(file) + } + fmt.Fprintln(cmd.Out(), "Edit cancelled, no changes made.") + return nil + } + if !lines { + if len(results.edit) > 0 { + preservedFile(nil, file, cmd.Out()) + } else { + os.Remove(file) + } + fmt.Fprintln(cmd.Out(), "Edit cancelled, saved file was empty.") + return nil + } + + results = editResults{ + file: file, + } + + // parse the edited file + updates, err := rmap.InfoForData(edited, "edited-file") + if err != nil { + results.header.reasons = append(results.header.reasons, editReason{ + head: fmt.Sprintf("The edited file had a syntax error: %v", err), + }) + continue + } + + err = resource.NewFlattenListVisitor(updates, rmap).Visit(func(info *resource.Info) error { + data, err := info.Mapping.Codec.Encode(info.Object) + if err != nil { + return err + } + updated, err := resource.NewHelper(info.Client, info.Mapping).Update(info.Namespace, info.Name, false, data) + if err != nil { + fmt.Fprintln(cmd.Out(), results.AddError(err, info)) + return nil + } + info.Refresh(updated, true) + fmt.Fprintf(out, "%s/%s\n", info.Mapping.Resource, info.Name) + return nil + }) + if err != nil { + return preservedFile(err, file, cmd.Out()) + } + + if results.retryable > 0 { + fmt.Fprintf(cmd.Out(), "You can run `%s update -f %s` to try this update again.\n", fullName, file) + return errExit + } + if results.conflict > 0 { + fmt.Fprintf(cmd.Out(), "You must update your resource version and run `%s update -f %s` to overwrite the remote changes.\n", fullName, file) + return errExit + } + if len(results.edit) == 0 { + if results.notfound == 0 { + os.Remove(file) + } else { + fmt.Fprintf(cmd.Out(), "The edits you made on deleted resources have been saved to %q\n", file) + } + return nil + } + + // loop again and edit the remaining items + infos = results.edit + } + return nil +} + +// editReason preserves a message about the reason this file must be edited again +type editReason struct { + head string + other []string +} + +// editHeader includes a list of reasons the edit must be retried +type editHeader struct { + reasons []editReason +} + +// WriteTo outputs the current header information into a stream +func (h *editHeader) WriteTo(w io.Writer) error { + fmt.Fprint(w, `# Please edit the object below. Lines beginning with a '#' will be ignored, +# and an empty file will abort the edit. If an error occurs while saving this file will be +# reopened with the relevant failures. +# +`) + for _, r := range h.reasons { + if len(r.other) > 0 { + fmt.Fprintf(w, "# %s:\n", r.head) + } else { + fmt.Fprintf(w, "# %s\n", r.head) + } + for _, o := range r.other { + fmt.Fprintf(w, "# * %s\n", o) + } + fmt.Fprintln(w, "#") + } + return nil +} + +// editResults capture the result of an update +type editResults struct { + header editHeader + retryable int + notfound int + conflict int + edit []*resource.Info + file string +} + +func (r *editResults) AddError(err error, info *resource.Info) string { + switch { + case errors.IsInvalid(err): + r.edit = append(r.edit, info) + reason := editReason{ + head: fmt.Sprintf("%s %s was not valid", info.Mapping.Kind, info.Name), + } + if err, ok := err.(kclient.APIStatus); ok { + if details := err.Status().Details; details != nil { + for _, cause := range details.Causes { + reason.other = append(reason.other, cause.Message) + } + } + } + r.header.reasons = append(r.header.reasons, reason) + return fmt.Sprintf("Error: the %s %s is invalid", info.Mapping.Kind, info.Name) + case errors.IsNotFound(err): + r.notfound++ + return fmt.Sprintf("Error: the %s %s has been deleted on the server", info.Mapping.Kind, info.Name) + case errors.IsConflict(err): + r.conflict++ + // TODO: make this better by extracting the resource version of the new version and allowing the user + // to know what command they need to run to update again (by rewriting the version?) + return fmt.Sprintf("Error: %v", err) + default: + r.retryable++ + return fmt.Sprintf("Error: the %s %s could not be updated: %v", info.Mapping.Kind, info.Name, err) + } +} + +// preservedFile writes out a message about the provided file if it exists to the +// provided output stream when an error happens. Used to notify the user where +// their updates were preserved. +func preservedFile(err error, path string, out io.Writer) error { + if len(path) > 0 { + if _, err := os.Stat(path); !os.IsNotExist(err) { + fmt.Fprintf(out, "A copy of your changes has been stored to %q\n", path) + } + } + return err +} + +// hasLines returns true if any line in the provided stream is non empty - has non-whitespace +// characters, or the first non-whitespace character is a '#' indicating a comment. Returns +// any errors encountered reading the stream. +func hasLines(r io.Reader) (bool, error) { + // TODO: if any files we read have > 64KB lines, we'll need to switch to bytes.ReadLine + s := bufio.NewScanner(r) + for s.Scan() { + if line := strings.TrimSpace(s.Text()); len(line) > 0 && line[0] != '#' { + return true, nil + } + } + if err := s.Err(); err != nil && err != io.EOF { + return false, err + } + return false, nil +} diff --git a/pkg/cmd/util/editor/editor.go b/pkg/cmd/util/editor/editor.go new file mode 100644 index 000000000000..cde848e1fa42 --- /dev/null +++ b/pkg/cmd/util/editor/editor.go @@ -0,0 +1,160 @@ +package editor + +import ( + "fmt" + "io" + "io/ioutil" + "os" + "os/exec" + "os/signal" + "path/filepath" + "strings" + + "github.com/docker/docker/pkg/term" + "github.com/golang/glog" +) + +const ( + // sorry, blame Git + defaultEditor = "vi" + defaultShell = "/bin/bash" +) + +type Editor struct { + Args []string + Shell bool +} + +// NewDefaultEditor creates an Editor that uses the OS environment to +// locate the editor program, looking at OSC_EDITOR, GIT_EDITOR, and +// EDITOR in order to find the proper command line. If the provided +// editor has no spaces, or no quotes, it is treated as a bare command +// to be loaded. Otherwise, the string will be passed to the user's +// shell for execution. +func NewDefaultEditor() Editor { + args, shell := defaultEnvEditor() + return Editor{ + Args: args, + Shell: shell, + } +} + +func defaultEnvShell() []string { + shell := os.Getenv("SHELL") + if len(shell) == 0 { + shell = defaultShell + } + return []string{shell, "-c"} +} + +func defaultEnvEditor() ([]string, bool) { + editor := os.Getenv("OSC_EDITOR") + if len(editor) == 0 { + editor = os.Getenv("GIT_EDITOR") + } + if len(editor) == 0 { + editor = os.Getenv("EDITOR") + } + if len(editor) == 0 { + editor = defaultEditor + } + if !strings.Contains(editor, " ") { + return []string{editor}, false + } + if !strings.ContainsAny(editor, "\"'\\") { + return strings.Split(editor, " "), false + } + // rather than parse the shell arguments ourselves, punt to the shell + shell := defaultEnvShell() + return append(shell, editor), true +} + +func (e Editor) args(path string) []string { + args := make([]string, len(e.Args)) + copy(args, e.Args) + if e.Shell { + last := args[len(args)-1] + args[len(args)-1] = fmt.Sprintf("%s %q", last, path) + } else { + args = append(args, path) + } + return args +} + +// Launch opens the described or returns an error. The TTY will be protected, and +// SIGQUIT, SIGTERM, and SIGINT will all be trapped. +func (e Editor) Launch(path string) error { + if len(e.Args) == 0 { + return fmt.Errorf("no editor defined, can't open %s", path) + } + abs, err := filepath.Abs(path) + if err != nil { + return err + } + args := e.args(abs) + cmd := exec.Command(args[0], args[1:]...) + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + cmd.Stdin = os.Stdin + glog.V(5).Infof("Opening file with editor %v", args) + if err := withSafeTTYAndInterrupts(os.Stdin, cmd.Run); err != nil { + if err, ok := err.(*exec.Error); ok { + if err.Err == exec.ErrNotFound { + return fmt.Errorf("unable to launch the editor %q", strings.Join(e.Args, " ")) + } + } + return fmt.Errorf("there was a problem with the editor %q", strings.Join(e.Args, " ")) + } + return nil +} + +// LaunchTempFile reads the provided stream into a temporary file in the given directory +// and file prefix, and then invokes Launch with the path of that file. It will return +// the contents of the file after launch, any errors that occur, and the path of the +// temporary file so the caller can clean it up as needed. +func (e Editor) LaunchTempFile(dir, prefix string, r io.Reader) ([]byte, string, error) { + f, err := ioutil.TempFile(dir, prefix) + if err != nil { + return nil, "", err + } + defer f.Close() + path := f.Name() + if _, err := io.Copy(f, r); err != nil { + os.Remove(path) + return nil, path, err + } + if err := e.Launch(path); err != nil { + return nil, path, err + } + bytes, err := ioutil.ReadFile(path) + return bytes, path, err +} + +// withSafeTTYAndInterrupts invokes the provided function after the terminal +// state has been stored, and then on any error or termination attempts to +// restore the terminal state to its prior behavior. It also eats signals +// for the duration of the function. +func withSafeTTYAndInterrupts(r io.Reader, fn func() error) error { + ch := make(chan os.Signal, 1) + signal.Notify(ch, childSignals...) + defer signal.Stop(ch) + + if file, ok := r.(*os.File); ok { + inFd := file.Fd() + if term.IsTerminal(inFd) { + state, err := term.SaveState(inFd) + if err != nil { + return err + } + go func() { + if _, ok := <-ch; !ok { + return + } + term.RestoreTerminal(inFd, state) + }() + defer term.RestoreTerminal(inFd, state) + return fn() + } + } + return fn() +} diff --git a/pkg/cmd/util/editor/editor_test.go b/pkg/cmd/util/editor/editor_test.go new file mode 100644 index 000000000000..60870a1bf18b --- /dev/null +++ b/pkg/cmd/util/editor/editor_test.go @@ -0,0 +1,46 @@ +package editor + +import ( + "bytes" + "io/ioutil" + "os" + "reflect" + "strings" + "testing" +) + +func TestArgs(t *testing.T) { + if e, a := []string{"/bin/bash", "-c \"test\""}, (Editor{Args: []string{"/bin/bash", "-c"}, Shell: true}).args("test"); !reflect.DeepEqual(e, a) { + t.Errorf("unexpected args: %v", a) + } + if e, a := []string{"/bin/bash", "-c", "test"}, (Editor{Args: []string{"/bin/bash", "-c"}, Shell: false}).args("test"); !reflect.DeepEqual(e, a) { + t.Errorf("unexpected args: %v", a) + } + if e, a := []string{"/bin/bash", "-i -c \"test\""}, (Editor{Args: []string{"/bin/bash", "-i -c"}, Shell: true}).args("test"); !reflect.DeepEqual(e, a) { + t.Errorf("unexpected args: %v", a) + } + if e, a := []string{"/test", "test"}, (Editor{Args: []string{"/test"}}).args("test"); !reflect.DeepEqual(e, a) { + t.Errorf("unexpected args: %v", a) + } +} + +func TestEditor(t *testing.T) { + edit := Editor{Args: []string{"cat"}} + contents, path, err := edit.LaunchTempFile("", "someprefix", bytes.NewBufferString("test something")) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if _, err := os.Stat(path); err != nil { + t.Fatalf("no temp file: %s", path) + } + defer os.Remove(path) + if disk, err := ioutil.ReadFile(path); err != nil || !bytes.Equal(contents, disk) { + t.Errorf("unexpected file on disk: %v %s", err, string(disk)) + } + if !bytes.Equal(contents, []byte("test something")) { + t.Errorf("unexpected contents: %s", string(contents)) + } + if !strings.Contains(path, "someprefix") { + t.Errorf("path not expected: %s", path) + } +} diff --git a/pkg/cmd/util/editor/term.go b/pkg/cmd/util/editor/term.go new file mode 100644 index 000000000000..f21991f3e802 --- /dev/null +++ b/pkg/cmd/util/editor/term.go @@ -0,0 +1,11 @@ +// +build !windows + +package editor + +import ( + "os" + "syscall" +) + +// childSignals are the allowed signals that can be sent to children in Unix variant OS's +var childSignals = []os.Signal{syscall.SIGINT, syscall.SIGTERM, syscall.SIGQUIT} diff --git a/pkg/cmd/util/editor/term_unsupported.go b/pkg/cmd/util/editor/term_unsupported.go new file mode 100644 index 000000000000..799c203aa22a --- /dev/null +++ b/pkg/cmd/util/editor/term_unsupported.go @@ -0,0 +1,10 @@ +// +build windows + +package editor + +import ( + "os" +) + +// childSignals are the allowed signals that can be sent to children in Windows to terminate +var childSignals = []os.Signal{os.Interrupt} From dde93d104963116541e70f2ecb4cb070a94fae02 Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Thu, 16 Apr 2015 23:23:48 -0400 Subject: [PATCH 3/3] Check /dev/tty if STDIN is not a tty Should allow piping input streams to edit. --- pkg/cmd/util/editor/editor.go | 39 ++++++++++++++++++++--------------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/pkg/cmd/util/editor/editor.go b/pkg/cmd/util/editor/editor.go index cde848e1fa42..d07a5664903f 100644 --- a/pkg/cmd/util/editor/editor.go +++ b/pkg/cmd/util/editor/editor.go @@ -97,7 +97,7 @@ func (e Editor) Launch(path string) error { cmd.Stderr = os.Stderr cmd.Stdin = os.Stdin glog.V(5).Infof("Opening file with editor %v", args) - if err := withSafeTTYAndInterrupts(os.Stdin, cmd.Run); err != nil { + if err := withSafeTTYAndInterrupts(cmd.Run); err != nil { if err, ok := err.(*exec.Error); ok { if err.Err == exec.ErrNotFound { return fmt.Errorf("unable to launch the editor %q", strings.Join(e.Args, " ")) @@ -134,27 +134,32 @@ func (e Editor) LaunchTempFile(dir, prefix string, r io.Reader) ([]byte, string, // state has been stored, and then on any error or termination attempts to // restore the terminal state to its prior behavior. It also eats signals // for the duration of the function. -func withSafeTTYAndInterrupts(r io.Reader, fn func() error) error { +func withSafeTTYAndInterrupts(fn func() error) error { ch := make(chan os.Signal, 1) signal.Notify(ch, childSignals...) defer signal.Stop(ch) - if file, ok := r.(*os.File); ok { - inFd := file.Fd() - if term.IsTerminal(inFd) { - state, err := term.SaveState(inFd) - if err != nil { - return err - } - go func() { - if _, ok := <-ch; !ok { - return - } - term.RestoreTerminal(inFd, state) - }() - defer term.RestoreTerminal(inFd, state) - return fn() + inFd := os.Stdin.Fd() + if !term.IsTerminal(inFd) { + if f, err := os.Open("/dev/tty"); err == nil { + defer f.Close() + inFd = f.Fd() } } + + if term.IsTerminal(inFd) { + state, err := term.SaveState(inFd) + if err != nil { + return err + } + go func() { + if _, ok := <-ch; !ok { + return + } + term.RestoreTerminal(inFd, state) + }() + defer term.RestoreTerminal(inFd, state) + return fn() + } return fn() }