From efda81cbbb2d3926bb6a2b9b219d0e49a9c4df82 Mon Sep 17 00:00:00 2001 From: Danielle Lancashire Date: Tue, 18 Jun 2019 14:56:24 +0200 Subject: [PATCH] logmon: Refactor fifo access for windows safety On unix platforms, it is safe to re-open fifo's for reading after the first creation if the file is already a fifo, however this is not possible on windows where this triggers a permissions error on the socket path, as you cannot recreate it. We can't transparently handle this in the CreateAndRead handle, because the Access Is Denied error is too generic to reliably be an IO error. Instead, we add an explict API for opening a reader to an existing FIFO, and check to see if the fifo already exists inside the calling package (e.g logmon) --- client/lib/fifo/fifo_unix.go | 8 +++++--- client/lib/fifo/fifo_windows.go | 11 +++++++++-- client/logmon/logmon.go | 25 ++++++++++++++++++------- 3 files changed, 32 insertions(+), 12 deletions(-) diff --git a/client/lib/fifo/fifo_unix.go b/client/lib/fifo/fifo_unix.go index 4d902fb03..1533b0bee 100644 --- a/client/lib/fifo/fifo_unix.go +++ b/client/lib/fifo/fifo_unix.go @@ -21,11 +21,13 @@ func CreateAndRead(path string) (func() (io.ReadCloser, error), error) { return nil, fmt.Errorf("error creating fifo %v: %v", path, err) } - openFn := func() (io.ReadCloser, error) { + return func() (io.ReadCloser, error) { return os.OpenFile(path, unix.O_RDONLY, os.ModeNamedPipe) - } + }, nil +} - return openFn, nil +func OpenReader(path string) (io.ReadCloser, error) { + return os.OpenFile(path, unix.O_RDONLY, os.ModeNamedPipe) } // OpenWriter opens a fifo file for writer, assuming it already exists, returns io.WriteCloser diff --git a/client/lib/fifo/fifo_windows.go b/client/lib/fifo/fifo_windows.go index 24d9c1e42..0c3a88ba0 100644 --- a/client/lib/fifo/fifo_windows.go +++ b/client/lib/fifo/fifo_windows.go @@ -78,13 +78,20 @@ func CreateAndRead(path string) (func() (io.ReadCloser, error), error) { return nil, err } - openFn := func() (io.ReadCloser, error) { + return func() (io.ReadCloser, error) { return &winFIFO{ listener: l, }, nil + }, nil +} + +func OpenReader(path string) (io.ReadCloser, error) { + conn, err := winio.DialPipe(path, nil) + if err != nil { + return nil, err } - return openFn, nil + return &winFIFO{conn: conn}, nil } // OpenWriter opens a fifo that already exists and returns an io.WriteCloser for it diff --git a/client/logmon/logmon.go b/client/logmon/logmon.go index 74a13631d..a92257649 100644 --- a/client/logmon/logmon.go +++ b/client/logmon/logmon.go @@ -3,6 +3,7 @@ package logmon import ( "fmt" "io" + "os" "strings" "sync" "time" @@ -199,7 +200,18 @@ func (l *logRotatorWrapper) isRunning() bool { // processOutWriter to attach to the stdout or stderr of a process. func newLogRotatorWrapper(path string, logger hclog.Logger, rotator *logging.FileRotator) (*logRotatorWrapper, error) { logger.Info("opening fifo", "path", path) - fifoOpenFn, err := fifo.CreateAndRead(path) + + var openFn func() (io.ReadCloser, error) + var err error + + if _, ferr := os.Stat(path); os.IsNotExist(ferr) { + openFn, err = fifo.CreateAndRead(path) + } else { + openFn = func() (io.ReadCloser, error) { + return fifo.OpenReader(path) + } + } + if err != nil { return nil, fmt.Errorf("failed to create fifo for extracting logs: %v", err) } @@ -211,20 +223,20 @@ func newLogRotatorWrapper(path string, logger hclog.Logger, rotator *logging.Fil openCompleted: make(chan struct{}), logger: logger, } - wrap.start(fifoOpenFn) + + wrap.start(openFn) return wrap, nil } // start starts a goroutine that copies from the pipe into the rotator. This is // called by the constructor and not the user of the wrapper. -func (l *logRotatorWrapper) start(readerOpenFn func() (io.ReadCloser, error)) { +func (l *logRotatorWrapper) start(openFn func() (io.ReadCloser, error)) { go func() { defer close(l.hasFinishedCopied) - reader, err := readerOpenFn() + reader, err := openFn() if err != nil { - close(l.openCompleted) - l.logger.Warn("failed to open log fifo", "error", err) + l.logger.Warn("failed to open fifo", "error", err) return } l.processOutReader = reader @@ -284,5 +296,4 @@ func (l *logRotatorWrapper) Close() { } l.rotatorWriter.Close() - return }