mirror of
https://github.com/open-policy-agent/opa.git
synced 2026-08-12 19:32:48 -06:00
repl: use a plain line reader for non-terminal input (#8941)
Switching the REPL to reeflective/readline (#8882) dropped liner's fallback to a plain reader for non-terminal stdin. This made `opa run` with piped/redirected stdin spin at 100% CPU instead of exiting, and made the v1/runtime REPL tests flaky via leaked spinning goroutines. Loop now uses the readline editor only for a real terminal and a plain line reader (stops cleanly at EOF) otherwise, configurable via WithConsoleInput / Params.ConsoleInput. Signed-off-by: Sebastian Spaink <sebastianspaink@gmail.com>
This commit is contained in:
@@ -48,6 +48,7 @@ require (
|
||||
go.opentelemetry.io/proto/otlp v1.10.0
|
||||
go.yaml.in/yaml/v3 v3.0.4
|
||||
golang.org/x/sync v0.22.0
|
||||
golang.org/x/term v0.44.0
|
||||
golang.org/x/text v0.40.0
|
||||
golang.org/x/time v0.15.0
|
||||
google.golang.org/grpc v1.82.0
|
||||
|
||||
@@ -292,6 +292,8 @@ golang.org/x/term v0.8.0/go.mod h1:xPskH00ivmX89bAKVGSKKtLOWNx2+17Eiy94tnKShWo=
|
||||
golang.org/x/term v0.12.0/go.mod h1:owVbMEjm3cBLCHdkQu9b1opXd4ETQWc3BhuQGKgXgvU=
|
||||
golang.org/x/term v0.13.0/go.mod h1:LTmsnFJwVN6bCy1rVCoS+qHT1HhALEFxKncY3WNNh4U=
|
||||
golang.org/x/term v0.14.0/go.mod h1:TySc+nGkYR6qt8km8wUhuFRTVSMIX3XPR58y2lC8vww=
|
||||
golang.org/x/term v0.44.0 h1:0rLvDRCtNj0gZkyIXhCyOb2OAzEhLVqc4B+hrsBhrmc=
|
||||
golang.org/x/term v0.44.0/go.mod h1:7ze4MdzUzLXpSAoFP1H0bOI9aXDqveSvatT5vKcFh2Y=
|
||||
golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
|
||||
golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
|
||||
golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ=
|
||||
|
||||
+69
-1
@@ -24,6 +24,7 @@ import (
|
||||
"time"
|
||||
|
||||
"github.com/reeflective/readline"
|
||||
"golang.org/x/term"
|
||||
|
||||
"github.com/open-policy-agent/opa/internal/future"
|
||||
pr "github.com/open-policy-agent/opa/internal/presentation"
|
||||
@@ -44,6 +45,7 @@ import (
|
||||
type REPL struct {
|
||||
output io.Writer
|
||||
stderr io.Writer
|
||||
input io.Reader
|
||||
store storage.Store
|
||||
runtime *ast.Term
|
||||
|
||||
@@ -118,6 +120,7 @@ func New(store storage.Store, historyPath string, output io.Writer, outputFormat
|
||||
|
||||
return &REPL{
|
||||
output: output,
|
||||
input: os.Stdin,
|
||||
store: store,
|
||||
modules: map[string]*ast.Module{},
|
||||
capabilities: ast.CapabilitiesForThisVersion(),
|
||||
@@ -171,6 +174,16 @@ func (r *REPL) WithStderrWriter(w io.Writer) *REPL {
|
||||
return r
|
||||
}
|
||||
|
||||
// WithConsoleInput sets the reader the REPL reads query input from. It defaults
|
||||
// to os.Stdin; a nil reader is ignored. Non-terminal input (a pipe, a file,
|
||||
// /dev/null) uses a plain line reader instead of the terminal editor.
|
||||
func (r *REPL) WithConsoleInput(in io.Reader) *REPL {
|
||||
if in != nil {
|
||||
r.input = in
|
||||
}
|
||||
return r
|
||||
}
|
||||
|
||||
// newShell initializes the readline line-reader used by Loop. Bracketed paste
|
||||
// is enabled so that pasted tabs/newlines are inserted literally instead of
|
||||
// triggering tab-completion or submitting the line (issue #962).
|
||||
@@ -187,8 +200,63 @@ func (r *REPL) newShell() *readline.Shell {
|
||||
return line
|
||||
}
|
||||
|
||||
// Loop will run until the user enters "exit", Ctrl+C, Ctrl+D, or an unexpected error occurs.
|
||||
// Loop reads, evaluates, and prints query results until the input is exhausted
|
||||
// (EOF), the user exits, or an unexpected error occurs.
|
||||
//
|
||||
// An interactive terminal gets the full readline editor (history, completion,
|
||||
// bracketed paste); any non-terminal input (a pipe, a file, /dev/null) gets a
|
||||
// plain line reader that stops cleanly at EOF. The readline editor must not be
|
||||
// driven against a non-TTY: on macOS it busy-loops at 100% CPU without ever
|
||||
// seeing EOF.
|
||||
func (r *REPL) Loop(ctx context.Context) error {
|
||||
if r.inputIsTerminal() {
|
||||
return r.loopInteractive(ctx)
|
||||
}
|
||||
return r.loopPiped(ctx)
|
||||
}
|
||||
|
||||
// inputIsTerminal reports whether the configured input is an interactive
|
||||
// terminal. Any non-*os.File reader (e.g. a pipe used in tests) is not.
|
||||
func (r *REPL) inputIsTerminal() bool {
|
||||
f, ok := r.input.(*os.File)
|
||||
if !ok {
|
||||
return false
|
||||
}
|
||||
return term.IsTerminal(int(f.Fd()))
|
||||
}
|
||||
|
||||
// maxConsoleLineBytes bounds a single line read from non-interactive input.
|
||||
const maxConsoleLineBytes = 1024 * 1024
|
||||
|
||||
// loopPiped reads and evaluates input line-by-line, returning at EOF.
|
||||
func (r *REPL) loopPiped(ctx context.Context) error {
|
||||
if len(r.banner) > 0 {
|
||||
fmt.Fprintln(r.output, r.banner)
|
||||
}
|
||||
|
||||
scanner := bufio.NewScanner(r.input)
|
||||
scanner.Buffer(make([]byte, 0, bufio.MaxScanTokenSize), maxConsoleLineBytes)
|
||||
|
||||
for scanner.Scan() {
|
||||
if err := ctx.Err(); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
if err := r.OneShot(ctx, scanner.Text()); err != nil {
|
||||
switch err.(type) {
|
||||
case stop:
|
||||
return nil
|
||||
default:
|
||||
fmt.Fprintln(r.output, err)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return scanner.Err()
|
||||
}
|
||||
|
||||
// loopInteractive will run until the user enters "exit", Ctrl+C, Ctrl+D, or an unexpected error occurs.
|
||||
func (r *REPL) loopInteractive(ctx context.Context) error {
|
||||
|
||||
line := r.newShell()
|
||||
|
||||
|
||||
@@ -16,6 +16,7 @@ import (
|
||||
"slices"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/reeflective/readline"
|
||||
|
||||
@@ -319,6 +320,57 @@ func TestREPLBracketedPasteTabNotCompleted(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestREPLLoopNonInteractiveInput(t *testing.T) {
|
||||
// runLoop feeds input as a non-terminal reader; it fails if Loop doesn't
|
||||
// return before the deadline (a regression would hang or spin here).
|
||||
runLoop := func(t *testing.T, input string) (string, error) {
|
||||
t.Helper()
|
||||
|
||||
var buf bytes.Buffer
|
||||
repl := New(newTestStore(), "", &buf, "", 0, "").
|
||||
WithStderrWriter(&buf).
|
||||
WithConsoleInput(strings.NewReader(input))
|
||||
|
||||
done := make(chan error, 1)
|
||||
go func() { done <- repl.Loop(t.Context()) }()
|
||||
|
||||
select {
|
||||
case err := <-done:
|
||||
return buf.String(), err
|
||||
case <-time.After(10 * time.Second):
|
||||
t.Fatal("Loop did not return for non-interactive input; it should stop at EOF, not block or spin")
|
||||
return "", nil
|
||||
}
|
||||
}
|
||||
|
||||
t.Run("evaluates piped lines and stops at EOF", func(t *testing.T) {
|
||||
out, err := runLoop(t, "1 + 1\n2 + 3\n")
|
||||
if err != nil {
|
||||
t.Fatalf("Loop returned error: %v", err)
|
||||
}
|
||||
if !strings.Contains(out, "2") || !strings.Contains(out, "5") {
|
||||
t.Fatalf("expected both piped queries to be evaluated, got: %q", out)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("exit command stops the loop", func(t *testing.T) {
|
||||
// Anything after "exit" must not be evaluated.
|
||||
out, err := runLoop(t, "exit\n1 + 1\n")
|
||||
if err != nil {
|
||||
t.Fatalf("Loop returned error: %v", err)
|
||||
}
|
||||
if strings.Contains(out, "2") {
|
||||
t.Fatalf("expected input after exit to be ignored, got: %q", out)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("empty input returns immediately at EOF", func(t *testing.T) {
|
||||
if _, err := runLoop(t, ""); err != nil {
|
||||
t.Fatalf("Loop returned error for empty input: %v", err)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
func TestREPLHistoryMigratesLegacyFile(t *testing.T) {
|
||||
path := filepath.Join(t.TempDir(), "history")
|
||||
|
||||
|
||||
@@ -210,6 +210,10 @@ type Params struct {
|
||||
// is mostly for test purposes.
|
||||
Output io.Writer
|
||||
|
||||
// ConsoleInput is the reader the interactive shell reads query input from.
|
||||
// When nil, os.Stdin is used. Mostly for tests and non-terminal hosts.
|
||||
ConsoleInput io.Reader
|
||||
|
||||
// GracefulShutdownPeriod is the time (in seconds) to wait for the http
|
||||
// server to shutdown gracefully.
|
||||
GracefulShutdownPeriod int
|
||||
@@ -846,6 +850,7 @@ func (rt *Runtime) StartREPL(ctx context.Context) error {
|
||||
WithRuntime(rt.Manager.Info).
|
||||
WithRegoVersion(rt.Params.regoVersion()).
|
||||
WithInitBundles(rt.loadedPathsResult.Bundles).
|
||||
WithConsoleInput(rt.Params.ConsoleInput).
|
||||
WithStderrWriter(rt.Params.Output)
|
||||
|
||||
if rt.Params.Watch {
|
||||
|
||||
+31
-12
@@ -283,6 +283,35 @@ func testRuntimeProcessWatchEventPolicyError(t *testing.T, asBundle bool) {
|
||||
})
|
||||
}
|
||||
|
||||
func startREPLForTest(t *testing.T, ctx context.Context, params *Params) *Runtime {
|
||||
t.Helper()
|
||||
|
||||
pr, pw := io.Pipe()
|
||||
params.ConsoleInput = pr
|
||||
|
||||
rt, err := NewRuntime(ctx, *params)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
done := make(chan struct{})
|
||||
go func() {
|
||||
defer close(done)
|
||||
_ = rt.StartREPL(ctx)
|
||||
}()
|
||||
|
||||
t.Cleanup(func() {
|
||||
_ = pw.Close() // signal EOF so the REPL loop returns
|
||||
select {
|
||||
case <-done:
|
||||
case <-time.After(5 * time.Second):
|
||||
t.Error("timed out waiting for REPL goroutine to exit after closing input")
|
||||
}
|
||||
})
|
||||
|
||||
return rt
|
||||
}
|
||||
|
||||
func TestRuntimeReplWithBundleBuiltWithV1Compatibility(t *testing.T) {
|
||||
ctx := t.Context()
|
||||
|
||||
@@ -313,12 +342,7 @@ func TestRuntimeReplWithBundleBuiltWithV1Compatibility(t *testing.T) {
|
||||
params.Paths = []string{p}
|
||||
params.BundleMode = true
|
||||
|
||||
rt, err := NewRuntime(ctx, params)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
go func() { _ = rt.StartREPL(ctx) }()
|
||||
rt := startREPLForTest(t, ctx, ¶ms)
|
||||
|
||||
if !test.Eventually(t, 5*time.Second, func() bool {
|
||||
return strings.Contains(output.String(), "Run 'help' to see a list of commands and check for updates.")
|
||||
@@ -451,12 +475,7 @@ p contains 1 if {
|
||||
params.V0Compatible = tc.v0Compatible
|
||||
params.V1Compatible = tc.v1Compatible
|
||||
|
||||
rt, err := NewRuntime(ctx, params)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
go func() { _ = rt.StartREPL(ctx) }()
|
||||
_ = startREPLForTest(t, ctx, ¶ms)
|
||||
|
||||
if !test.Eventually(t, 5*time.Second, func() bool {
|
||||
return strings.Contains(output.String(), "Run 'help' to see a list of commands and check for updates.")
|
||||
|
||||
Reference in New Issue
Block a user