From 622c4df14c09fd0a1599c54d00bdbe46453bd05e Mon Sep 17 00:00:00 2001 From: James Bardin Date: Tue, 26 Oct 2021 15:54:09 -0400 Subject: [PATCH] remove the use of panicwrap Stop using panicwrap, and execute terraform in the main process. --- go.mod | 1 - go.sum | 2 - internal/command/console_interactive.go | 5 +- internal/terminal/panicwrap_ugh.go | 78 ---------------------- main.go | 89 ++----------------------- main_test.go | 6 +- 6 files changed, 11 insertions(+), 170 deletions(-) delete mode 100644 internal/terminal/panicwrap_ugh.go diff --git a/go.mod b/go.mod index 646309e2a..11095b765 100644 --- a/go.mod +++ b/go.mod @@ -63,7 +63,6 @@ require ( github.com/mitchellh/go-wordwrap v1.0.1 github.com/mitchellh/gox v1.0.1 github.com/mitchellh/mapstructure v1.1.2 - github.com/mitchellh/panicwrap v1.0.0 github.com/mitchellh/reflectwalk v1.0.2 github.com/nishanths/exhaustive v0.2.3 github.com/packer-community/winrmcp v0.0.0-20180921211025-c76d91c1e7db diff --git a/go.sum b/go.sum index bd8b487fb..0fb3794c2 100644 --- a/go.sum +++ b/go.sum @@ -515,8 +515,6 @@ github.com/mitchellh/iochan v1.0.0/go.mod h1:JwYml1nuB7xOzsp52dPpHFffvOCDupsG0Qu github.com/mitchellh/mapstructure v0.0.0-20160808181253-ca63d7c062ee/go.mod h1:FVVH3fgwuzCH5S8UJGiWEs2h04kUh9fWfEaFds41c1Y= github.com/mitchellh/mapstructure v1.1.2 h1:fmNYVwqnSfB9mZU6OS2O6GsXM+wcskZDuKQzvN1EDeE= github.com/mitchellh/mapstructure v1.1.2/go.mod h1:FVVH3fgwuzCH5S8UJGiWEs2h04kUh9fWfEaFds41c1Y= -github.com/mitchellh/panicwrap v1.0.0 h1:67zIyVakCIvcs69A0FGfZjBdPleaonSgGlXRSRlb6fE= -github.com/mitchellh/panicwrap v1.0.0/go.mod h1:pKvZHwWrZowLUzftuFq7coarnxbBXU4aQh3N0BJOeeA= github.com/mitchellh/reflectwalk v1.0.0/go.mod h1:mSTlrgnPZtwu0c4WaC2kGObEpuNDbx0jmZXqmk4esnw= github.com/mitchellh/reflectwalk v1.0.2 h1:G2LzWKi524PWgd3mLHV8Y5k7s6XUvT0Gef6zxSIeXaQ= github.com/mitchellh/reflectwalk v1.0.2/go.mod h1:mSTlrgnPZtwu0c4WaC2kGObEpuNDbx0jmZXqmk4esnw= diff --git a/internal/command/console_interactive.go b/internal/command/console_interactive.go index c9d63245c..15fde13b4 100644 --- a/internal/command/console_interactive.go +++ b/internal/command/console_interactive.go @@ -10,7 +10,6 @@ import ( "fmt" "io" - "github.com/hashicorp/terraform/internal/helper/wrappedreadline" "github.com/hashicorp/terraform/internal/repl" "github.com/chzyer/readline" @@ -19,12 +18,12 @@ import ( func (c *ConsoleCommand) modeInteractive(session *repl.Session, ui cli.Ui) int { // Configure input - l, err := readline.NewEx(wrappedreadline.Override(&readline.Config{ + l, err := readline.NewEx(&readline.Config{ Prompt: "> ", InterruptPrompt: "^C", EOFPrompt: "exit", HistorySearchFold: true, - })) + }) if err != nil { c.Ui.Error(fmt.Sprintf( "Error initializing console: %s", diff --git a/internal/terminal/panicwrap_ugh.go b/internal/terminal/panicwrap_ugh.go deleted file mode 100644 index b17165b2c..000000000 --- a/internal/terminal/panicwrap_ugh.go +++ /dev/null @@ -1,78 +0,0 @@ -package terminal - -import "os" - -// This file has some annoying nonsense to, yet again, work around the -// panicwrap hack. -// -// Specifically, typically when we're running Terraform the stderr handle is -// not directly connected to the terminal but is instead a pipe into a parent -// process gathering up the output just in case a panic message appears. -// However, this package needs to know whether the _real_ stderr is connected -// to a terminal and what its width is. -// -// To work around that, we'll first initialize the terminal in the parent -// process, and then capture information about stderr into an environment -// variable so we can pass it down to the child process. The child process -// will then use the environment variable to pretend that the panicwrap pipe -// has the same characteristics as the terminal that it's indirectly writing -// to. -// -// This file has some helpers for implementing that awkward handshake, but the -// handshake itself is in package main, interspersed with all of the other -// panicwrap machinery. -// -// You might think that the code in helper/wrappedstreams could avoid this -// problem, but that package is broken on Windows: it always fails to recover -// the real stderr, and it also gets an incorrect result if the user was -// redirecting or piping stdout/stdin. So... we have this hack instead, which -// gets a correct result even on Windows and even with I/O redirection. - -// StateForAfterPanicWrap is part of the workaround for panicwrap that -// captures some characteristics of stderr that the caller can pass to the -// panicwrap child process somehow and then use ReinitInsidePanicWrap. -func (s *Streams) StateForAfterPanicWrap() *PrePanicwrapState { - return &PrePanicwrapState{ - StderrIsTerminal: s.Stderr.IsTerminal(), - StderrWidth: s.Stderr.Columns(), - } -} - -// ReinitInsidePanicwrap is part of the workaround for panicwrap that -// produces a Streams containing a potentially-lying Stderr that might -// claim to be a terminal even if it's actually a pipe connected to the -// parent process. -// -// That's an okay lie in practice because the parent process will copy any -// data it recieves via that pipe verbatim to the real stderr anyway. (The -// original call to Init in the parent process should've already done any -// necessary modesetting on the Stderr terminal, if any.) -// -// The state argument can be nil if we're not running in panicwrap mode, -// in which case this function behaves exactly the same as Init. -func ReinitInsidePanicwrap(state *PrePanicwrapState) (*Streams, error) { - ret, err := Init() - if err != nil { - return ret, err - } - if state != nil { - // A lying stderr, then. - ret.Stderr = &OutputStream{ - File: ret.Stderr.File, - isTerminal: func(f *os.File) bool { - return state.StderrIsTerminal - }, - getColumns: func(f *os.File) int { - return state.StderrWidth - }, - } - } - return ret, nil -} - -// PrePanicwrapState is a horrible thing we use to work around panicwrap, -// related to both Streams.StateForAfterPanicWrap and ReinitInsidePanicwrap. -type PrePanicwrapState struct { - StderrIsTerminal bool - StderrWidth int -} diff --git a/main.go b/main.go index 793db83c8..99e2c58a3 100644 --- a/main.go +++ b/main.go @@ -3,7 +3,6 @@ package main import ( "encoding/json" "fmt" - "io/ioutil" "log" "net" "os" @@ -24,7 +23,6 @@ import ( "github.com/mattn/go-shellwords" "github.com/mitchellh/cli" "github.com/mitchellh/colorstring" - "github.com/mitchellh/panicwrap" backendInit "github.com/hashicorp/terraform/internal/backend/init" ) @@ -35,12 +33,6 @@ const ( // The parent process will create a file to collect crash logs envTmpLogPath = "TF_TEMP_LOG_PATH" - - // Environment variable name used for smuggling true stderr terminal - // settings into a panicwrap child process. This is an implementation - // detail, subject to change in future, and should not ever be directly - // set by an end-user. - envTerminalPanicwrapWorkaround = "TF_PANICWRAP_STDERR" ) // ui wraps the primary output cli.Ui, and redirects Warn calls to Output @@ -54,67 +46,6 @@ func (u *ui) Warn(msg string) { u.Ui.Output(msg) } -func main() { - os.Exit(realMain()) -} - -func realMain() int { - var wrapConfig panicwrap.WrapConfig - - // don't re-exec terraform as a child process for easier debugging - if os.Getenv("TF_FORK") == "0" { - return wrappedMain() - } - - if !panicwrap.Wrapped(&wrapConfig) { - // We always send logs to a temporary file that we use in case - // there is a panic. Otherwise, we delete it. - logTempFile, err := ioutil.TempFile("", "terraform-log") - if err != nil { - fmt.Fprintf(os.Stderr, "Couldn't set up logging tempfile: %s", err) - return 1 - } - // Now that we have the file, close it and leave it for the wrapped - // process to write to. - logTempFile.Close() - defer os.Remove(logTempFile.Name()) - - // store the path in the environment for the wrapped executable - os.Setenv(envTmpLogPath, logTempFile.Name()) - - // We also need to do our terminal initialization before we fork, - // because the child process doesn't necessarily have access to - // the true stderr in order to initialize it. - streams, err := terminal.Init() - if err != nil { - fmt.Fprintf(os.Stderr, "Failed to initialize terminal: %s", err) - return 1 - } - - // We need the child process to behave _as if_ connected to the real - // stderr, even though panicwrap is about to add a pipe in the way, - // so we'll smuggle the true stderr information in an environment - // varible. - streamState := streams.StateForAfterPanicWrap() - os.Setenv(envTerminalPanicwrapWorkaround, fmt.Sprintf("%t:%d", streamState.StderrIsTerminal, streamState.StderrWidth)) - - // Create the configuration for panicwrap and wrap our executable - wrapConfig.Handler = logging.PanicHandler(logTempFile.Name()) - wrapConfig.IgnoreSignals = ignoreSignals - wrapConfig.ForwardSignals = forwardSignals - exitStatus, err := panicwrap.Wrap(&wrapConfig) - if err != nil { - fmt.Fprintf(os.Stderr, "Couldn't start Terraform: %s", err) - return 1 - } - - return exitStatus - } - - // Call the real main - return wrappedMain() -} - func init() { Ui = &ui{&cli.BasicUi{ Writer: os.Stdout, @@ -123,7 +54,11 @@ func init() { }} } -func wrappedMain() int { +func main() { + os.Exit(realMain()) +} + +func realMain() int { var err error tmpLogPath := os.Getenv(envTmpLogPath) @@ -145,19 +80,7 @@ func wrappedMain() int { log.Printf("[INFO] Go runtime version: %s", runtime.Version()) log.Printf("[INFO] CLI args: %#v", os.Args) - // This is the recieving end of our workaround to retain the metadata - // about the real stderr even though we're talking to it via the panicwrap - // pipe. See the call to StateForAfterPanicWrap above for the producer - // part of this. - var streamState *terminal.PrePanicwrapState - if raw := os.Getenv(envTerminalPanicwrapWorkaround); raw != "" { - streamState = &terminal.PrePanicwrapState{} - if _, err := fmt.Sscanf(raw, "%t:%d", &streamState.StderrIsTerminal, &streamState.StderrWidth); err != nil { - log.Printf("[WARN] %s is set but is incorrectly-formatted: %s", envTerminalPanicwrapWorkaround, err) - streamState = nil // leave it unset for a normal init, then - } - } - streams, err := terminal.ReinitInsidePanicwrap(streamState) + streams, err := terminal.Init() if err != nil { Ui.Error(fmt.Sprintf("Failed to configure the terminal: %s", err)) return 1 diff --git a/main_test.go b/main_test.go index 102ea44bb..15a09f283 100644 --- a/main_test.go +++ b/main_test.go @@ -130,7 +130,7 @@ func TestMain_cliArgsFromEnv(t *testing.T) { // Run it! os.Args = args testCommand.Args = nil - exit := wrappedMain() + exit := realMain() if (exit != 0) != tc.Err { t.Fatalf("bad: %d", exit) } @@ -237,7 +237,7 @@ func TestMain_cliArgsFromEnvAdvanced(t *testing.T) { // Run it! os.Args = args testCommand.Args = nil - exit := wrappedMain() + exit := realMain() if (exit != 0) != tc.Err { t.Fatalf("unexpected exit status %d; want 0", exit) } @@ -275,7 +275,7 @@ func TestMain_autoComplete(t *testing.T) { // Run it! os.Args = []string{"terraform", "terraform", "versio"} - exit := wrappedMain() + exit := realMain() if exit != 0 { t.Fatalf("unexpected exit status %d; want 0", exit) }