Skip to content

Commit

Permalink
logmon: Refactor fifo access for windows safety
Browse files Browse the repository at this point in the history
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)
  • Loading branch information
endocrimes committed Jun 28, 2019
1 parent c7723c7 commit efda81c
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 12 deletions.
8 changes: 5 additions & 3 deletions client/lib/fifo/fifo_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 9 additions & 2 deletions client/lib/fifo/fifo_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
25 changes: 18 additions & 7 deletions client/logmon/logmon.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package logmon
import (
"fmt"
"io"
"os"
"strings"
"sync"
"time"
Expand Down Expand Up @@ -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)
}
Expand All @@ -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
Expand Down Expand Up @@ -284,5 +296,4 @@ func (l *logRotatorWrapper) Close() {
}

l.rotatorWriter.Close()
return
}

0 comments on commit efda81c

Please sign in to comment.