VAULT-19681 allow users to specify files for agent child process stdout/stderr

changelog/22812.txt

@@ -0,0 +1,3 @@
+```release-note:improvement
+agent: allow users to specify files for child process stdout/stderr
+```

command/agent.go

@@ -729,13 +729,17 @@ func (c *AgentCommand) Run(args []string) int {
 			ExitAfterAuth: config.ExitAfterAuth,
 		})
 
-		es := exec.NewServer(&exec.ServerConfig{
+		es, err := exec.NewServer(&exec.ServerConfig{
 			AgentConfig: c.config,
 			Namespace:   templateNamespace,
 			Logger:      c.logger.Named("exec.server"),
 			LogLevel:    c.logger.GetLevel(),
 			LogWriter:   c.logWriter,
 		})
+		if err != nil {
+			c.logger.Error("could not create exec server", "error", err)
+			return 1
+		}
 
 		g.Add(func() error {
 			return ah.Run(ctx, method)
@@ -800,6 +804,7 @@ func (c *AgentCommand) Run(args []string) int {
 				leaseCache.SetShuttingDown(true)
 			}
 			cancelFunc()
+			es.Close()
 		})
 
 	}

command/agent/config/config.go

@@ -171,6 +171,8 @@ type ExecConfig struct {
 	Command                []string  `hcl:"command,attr" mapstructure:"command"`
 	RestartOnSecretChanges string    `hcl:"restart_on_secret_changes,optional" mapstructure:"restart_on_secret_changes"`
 	RestartStopSignal      os.Signal `hcl:"-" mapstructure:"restart_stop_signal"`
+	ChildProcessStdout     string    `mapstructure:"child_process_stdout"`
+	ChildProcessStderr     string    `mapstructure:"child_process_stderr"`
 }
 
 func NewConfig() *Config {

command/agent/exec/exec.go

@@ -65,9 +65,13 @@ type Server struct {
 
 	logger hclog.Logger
 
-	childProcess      *child.Child
-	childProcessState childProcessState
-	childProcessLock  sync.Mutex
+	childProcess            *child.Child
+	childProcessState       childProcessState
+	childProcessLock        sync.Mutex
+	childProcessStdout      io.WriteCloser
+	childProcessStderr      io.WriteCloser
+	childProcessCloseStdOut bool
+	childProcessCloseStderr bool
 
 	// exit channel of the child process
 	childProcessExitCh chan int
@@ -85,15 +89,44 @@ func (e *ProcessExitError) Error() string {
 	return fmt.Sprintf("process exited with %d", e.ExitCode)
 }
 
-func NewServer(cfg *ServerConfig) *Server {
+func NewServer(cfg *ServerConfig) (*Server, error) {
+	var err error
+
+	childProcessCloseStdout := false
+	childProcessStdout := os.Stdout
+	childProcessCloseStderr := false
+	childProcessStderr := os.Stderr
+
+	if cfg.AgentConfig.Exec != nil {
+		if cfg.AgentConfig.Exec.ChildProcessStdout != "" {
+			childProcessStdout, err = os.OpenFile(cfg.AgentConfig.Exec.ChildProcessStdout, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0o644)
+			if err != nil {
+				return nil, fmt.Errorf("could not open %q, %w", cfg.AgentConfig.Exec.ChildProcessStdout, err)
+			}
+			childProcessCloseStdout = true
+		}
+
+		if cfg.AgentConfig.Exec.ChildProcessStderr != "" {
+			childProcessStderr, err = os.OpenFile(cfg.AgentConfig.Exec.ChildProcessStderr, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0o644)
+			if err != nil {
+				return nil, fmt.Errorf("could not open %q, %w", cfg.AgentConfig.Exec.ChildProcessStdout, err)
+			}
+			childProcessCloseStderr = true
+		}
+	}
+
 	server := Server{
-		logger:             cfg.Logger,
-		config:             cfg,
-		childProcessState:  childProcessStateNotStarted,
-		childProcessExitCh: make(chan int),
+		logger:                  cfg.Logger,
+		config:                  cfg,
+		childProcessState:       childProcessStateNotStarted,
+		childProcessExitCh:      make(chan int),
+		childProcessStdout:      childProcessStdout,
+		childProcessStderr:      childProcessStderr,
+		childProcessCloseStdOut: childProcessCloseStdout,
+		childProcessCloseStderr: childProcessCloseStderr,
 	}
 
-	return &server
+	return &server, nil
 }
 
 func (s *Server) Run(ctx context.Context, incomingVaultToken chan string) error {
@@ -152,6 +185,7 @@ func (s *Server) Run(ctx context.Context, incomingVaultToken chan string) error
 				s.childProcess.Stop()
 			}
 			s.childProcessState = childProcessStateStopped
+			s.close()
 			s.childProcessLock.Unlock()
 			return nil
 
@@ -291,8 +325,8 @@ func (s *Server) restartChildProcess(newEnvVars []string) error {
 
 	childInput := &child.NewInput{
 		Stdin:        os.Stdin,
-		Stdout:       os.Stdout,
-		Stderr:       os.Stderr,
+		Stdout:       s.childProcessStdout,
+		Stderr:       s.childProcessStderr,
 		Command:      args[0],
 		Args:         args[1:],
 		Timeout:      0, // let it run forever
@@ -333,3 +367,19 @@ func (s *Server) restartChildProcess(newEnvVars []string) error {
 
 	return nil
 }
+
+func (s *Server) Close() {
+	s.childProcessLock.Lock()
+	defer s.childProcessLock.Unlock()
+	s.close()
+}
+
+func (s *Server) close() {
+	if s.childProcessCloseStdOut {
+		_ = s.childProcessStdout.Close()
+	}
+
+	if s.childProcessCloseStderr {
+		_ = s.childProcessStderr.Close()
+	}
+}

command/agent/exec/exec_test.go

@@ -4,10 +4,12 @@
 package exec
 
 import (
+	"bytes"
 	"context"
 	"encoding/json"
 	"errors"
 	"fmt"
+	"io"
 	"net/http"
 	"net/http/httptest"
 	"os"
@@ -257,7 +259,7 @@ func TestExecServer_Run(t *testing.T) {
 				strconv.Itoa(testCase.testAppPort),
 			}
 
-			execServer := NewServer(&ServerConfig{
+			execServer, err := NewServer(&ServerConfig{
 				Logger: logging.NewVaultLogger(hclog.Trace),
 				AgentConfig: &config.Config{
 					Vault: &config.Vault{
@@ -280,6 +282,9 @@ func TestExecServer_Run(t *testing.T) {
 				LogLevel:  hclog.Trace,
 				LogWriter: hclog.DefaultOutput,
 			})
+			if err != nil {
+				t.Fatalf("could not create exec server: %q", err)
+			}
 
 			// start the exec server
 			var (
@@ -380,3 +385,164 @@ func TestExecServer_Run(t *testing.T) {
 		})
 	}
 }
+
+func TestExecServer_LogFiles(t *testing.T) {
+	goBinary, err := exec.LookPath("go")
+	if err != nil {
+		t.Fatalf("could not find go binary on path: %s", err)
+	}
+
+	testAppBinary := filepath.Join(os.TempDir(), "test-app")
+
+	if err := exec.Command(goBinary, "build", "-o", testAppBinary, "./test-app").Run(); err != nil {
+		t.Fatalf("could not build the test application: %s", err)
+	}
+	t.Cleanup(func() {
+		if err := os.Remove(testAppBinary); err != nil {
+			t.Fatalf("could not remove %q test application: %s", testAppBinary, err)
+		}
+	})
+
+	tempStderr := filepath.Join(os.TempDir(), "vault-exec-test.stderr.log")
+	t.Cleanup(func() {
+		_ = os.Remove(tempStderr)
+	})
+
+	testCases := map[string]struct {
+		testAppPort   int
+		stderrFile    string
+		expectedError error
+	}{
+		"can_log_to_file": {
+			testAppPort: 34001,
+			stderrFile:  tempStderr,
+		},
+		"cant_open_file": {
+			testAppPort:   34002,
+			stderrFile:    "/file/does/not/exist",
+			expectedError: os.ErrNotExist,
+		},
+	}
+
+	for tcName, testCase := range testCases {
+		t.Run(tcName, func(t *testing.T) {
+			fakeVault := fakeVaultServer(t)
+			defer fakeVault.Close()
+
+			testAppCommand := []string{
+				testAppBinary,
+				"--port",
+				strconv.Itoa(testCase.testAppPort),
+				"--stop-after",
+				"60s",
+			}
+
+			execServer, err := NewServer(&ServerConfig{
+				Logger: logging.NewVaultLogger(hclog.Trace),
+				AgentConfig: &config.Config{
+					Vault: &config.Vault{
+						Address: fakeVault.URL,
+						Retry: &config.Retry{
+							NumRetries: 3,
+						},
+					},
+					Exec: &config.ExecConfig{
+						RestartOnSecretChanges: "always",
+						Command:                testAppCommand,
+						ChildProcessStderr:     testCase.stderrFile,
+					},
+					EnvTemplates: []*ctconfig.TemplateConfig{{
+						Contents:                 pointerutil.StringPtr(`{{ with secret "kv/my-app/creds" }}{{ .Data.data.user }}{{ end }}`),
+						MapToEnvironmentVariable: pointerutil.StringPtr("MY_USER"),
+					}},
+					TemplateConfig: &config.TemplateConfig{
+						ExitOnRetryFailure:    true,
+						StaticSecretRenderInt: 5 * time.Second,
+					},
+				},
+				LogLevel:  hclog.Trace,
+				LogWriter: hclog.DefaultOutput,
+			})
+			if err != nil {
+				if testCase.expectedError != nil {
+					if errors.Is(err, testCase.expectedError) {
+						t.Log("test passes! caught expected err")
+						return
+					} else {
+						t.Fatalf("caught error %q did not match expected error %q", err, testCase.expectedError)
+					}
+				}
+				t.Fatalf("could not create exec server: %q", err)
+			}
+
+			// replace the tempfile created with one owned by this test
+			var stdoutBuffer bytes.Buffer
+			execServer.childProcessStderr = noopCloser{&stdoutBuffer}
+
+			ctx, cancel := context.WithCancel(context.Background())
+			defer cancel()
+
+			// start the exec server
+			var (
+				execServerErrCh   = make(chan error)
+				execServerTokenCh = make(chan string, 1)
+			)
+			go func() {
+				execServerErrCh <- execServer.Run(ctx, execServerTokenCh)
+			}()
+
+			// send a dummy token to kick off the server
+			execServerTokenCh <- "my-token"
+
+			// ensure the test app is running after 3 seconds
+			var (
+				testAppAddr      = fmt.Sprintf("http://localhost:%d", testCase.testAppPort)
+				testAppStartedCh = make(chan error)
+			)
+			time.AfterFunc(500*time.Millisecond, func() {
+				_, err := retryablehttp.Head(testAppAddr)
+				testAppStartedCh <- err
+			})
+
+			select {
+			case <-ctx.Done():
+				t.Fatal("timeout reached before templates were rendered")
+
+			case err := <-execServerErrCh:
+				if testCase.expectedError == nil && err != nil {
+					t.Fatalf("exec server did not expect an error, got: %v", err)
+				}
+
+				if errors.Is(err, testCase.expectedError) {
+					t.Fatalf("exec server expected error %v; got %v", testCase.expectedError, err)
+				}
+
+				t.Log("exec server exited without an error")
+
+				return
+
+			case <-testAppStartedCh:
+				t.Log("test app started successfully")
+			}
+
+			// let the app run a bit
+			time.Sleep(5 * time.Second)
+			// stop the app
+			cancel()
+			// wait for app to stop
+			time.Sleep(5 * time.Second)
+
+			if stdoutBuffer.Len() == 0 {
+				t.Fatalf("stdout log file does not have any data!")
+			}
+		})
+	}
+}
+
+type noopCloser struct {
+	io.Writer
+}
+
+func (_ noopCloser) Close() error {
+	return nil
+}