VAULT-9900: Log rotation for 'agent' and 'server' commands (#18031)

* Work to unify log-file for agent/server and add rotation
* Updates to rotation code, tried to centralise the log config setup
* logging + tests
* Move LogFile to ShareConfig in test
* Docs

changelog/18031.txt

@@ -0,0 +1,3 @@
+```release-note:feature
+logging: Vault agent and server commands support log file and log rotation.
+```

command/agent.go

@@ -19,6 +19,7 @@ import (
 	systemd "github.com/coreos/go-systemd/daemon"
 	log "github.com/hashicorp/go-hclog"
 	"github.com/hashicorp/go-secure-stdlib/gatedwriter"
+	"github.com/hashicorp/go-secure-stdlib/parseutil"
 	"github.com/hashicorp/vault/api"
 	"github.com/hashicorp/vault/command/agent/auth"
 	"github.com/hashicorp/vault/command/agent/auth/alicloud"
@@ -69,6 +70,7 @@ const (
 
 type AgentCommand struct {
 	*BaseCommand
+	logFlags logFlags
 
 	ShutdownCh chan struct{}
 	SighupCh   chan struct{}
@@ -84,13 +86,9 @@ type AgentCommand struct {
 
 	startedCh chan (struct{}) // for tests
 
-	flagConfigs       []string
-	flagLogLevel      string
-	flagLogFile       string
-	flagExitAfterAuth bool
-
+	flagConfigs        []string
+	flagExitAfterAuth  bool
 	flagTestVerifyOnly bool
-	flagCombineLogs    bool
 }
 
 func (c *AgentCommand) Synopsis() string {
@@ -119,6 +117,9 @@ func (c *AgentCommand) Flags() *FlagSets {
 
 	f := set.NewFlagSet("Command Options")
 
+	// Augment with the log flags
+	f.addLogFlags(&c.logFlags)
+
 	f.StringSliceVar(&StringSliceVar{
 		Name:   "config",
 		Target: &c.flagConfigs,
@@ -130,23 +131,6 @@ func (c *AgentCommand) Flags() *FlagSets {
 			"contain only agent directives.",
 	})
 
-	f.StringVar(&StringVar{
-		Name:       flagNameLogLevel,
-		Target:     &c.flagLogLevel,
-		Default:    "info",
-		EnvVar:     EnvVaultLogLevel,
-		Completion: complete.PredictSet("trace", "debug", "info", "warn", "error"),
-		Usage: "Log verbosity level. Supported values (in order of detail) are " +
-			"\"trace\", \"debug\", \"info\", \"warn\", and \"error\".",
-	})
-
-	f.StringVar(&StringVar{
-		Name:   flagNameLogFile,
-		Target: &c.flagLogFile,
-		EnvVar: EnvVaultLogFile,
-		Usage:  "Path to the log file that Vault should use for logging",
-	})
-
 	f.BoolVar(&BoolVar{
 		Name:    flagNameAgentExitAfterAuth,
 		Target:  &c.flagExitAfterAuth,
@@ -163,15 +147,6 @@ func (c *AgentCommand) Flags() *FlagSets {
 	// no warranty or backwards-compatibility promise. Do not use these flags
 	// in production. Do not build automation using these flags. Unless you are
 	// developing against Vault, you should not need any of these flags.
-
-	// TODO: should the below flags be public?
-	f.BoolVar(&BoolVar{
-		Name:    "combine-logs",
-		Target:  &c.flagCombineLogs,
-		Default: false,
-		Hidden:  true,
-	})
-
 	f.BoolVar(&BoolVar{
 		Name:    "test-verify-only",
 		Target:  &c.flagTestVerifyOnly,
@@ -204,7 +179,8 @@ func (c *AgentCommand) Run(args []string) int {
 	// start logging too early.
 	c.logGate = gatedwriter.NewWriter(os.Stderr)
 	c.logWriter = c.logGate
-	if c.flagCombineLogs {
+
+	if c.logFlags.flagCombineLogs {
 		c.logWriter = os.Stdout
 	}
 
@@ -237,9 +213,9 @@ func (c *AgentCommand) Run(args []string) int {
 		c.UI.Info("No auto_auth block found in config file, not starting automatic authentication feature")
 	}
 
-	config = c.aggregateConfig(f, config)
+	c.updateConfig(f, config)
 
-	// Build the logger using level, format and path
+	// Parse all the log related config
 	logLevel, err := logging.ParseLogLevel(config.LogLevel)
 	if err != nil {
 		c.UI.Error(err.Error())
@@ -252,7 +228,34 @@ func (c *AgentCommand) Run(args []string) int {
 		return 1
 	}
 
-	logCfg := logging.NewLogConfig("agent", logLevel, logFormat, config.LogFile)
+	logRotateDuration, err := parseutil.ParseDurationSecond(config.LogRotateDuration)
+	if err != nil {
+		c.UI.Error(err.Error())
+		return 1
+	}
+
+	logRotateBytes, err := parseutil.ParseInt(config.LogRotateBytes)
+	if err != nil {
+		c.UI.Error(err.Error())
+		return 1
+	}
+
+	logRotateMaxFiles, err := parseutil.ParseInt(config.LogRotateMaxFiles)
+	if err != nil {
+		c.UI.Error(err.Error())
+		return 1
+	}
+
+	logCfg := &logging.LogConfig{
+		Name:              "vault-agent",
+		LogLevel:          logLevel,
+		LogFormat:         logFormat,
+		LogFilePath:       config.LogFile,
+		LogRotateDuration: logRotateDuration,
+		LogRotateBytes:    int(logRotateBytes),
+		LogRotateMaxFiles: int(logRotateMaxFiles),
+	}
+
 	l, err := logging.Setup(logCfg, c.logWriter)
 	if err != nil {
 		c.UI.Error(err.Error())
@@ -263,7 +266,7 @@ func (c *AgentCommand) Run(args []string) int {
 
 	infoKeys := make([]string, 0, 10)
 	info := make(map[string]string)
-	info["log level"] = c.flagLogLevel
+	info["log level"] = config.LogLevel
 	infoKeys = append(infoKeys, "log level")
 
 	infoKeys = append(infoKeys, "version")
@@ -457,7 +460,7 @@ func (c *AgentCommand) Run(args []string) int {
 	}
 
 	// Output the header that the agent has started
-	if !c.flagCombineLogs {
+	if !c.logFlags.flagCombineLogs {
 		c.UI.Output("==> Vault agent started! Log data will stream in below:\n")
 	}
 
@@ -926,31 +929,19 @@ func (c *AgentCommand) Run(args []string) int {
 	return 0
 }
 
-// aggregateConfig ensures that the config object accurately reflects the desired
+// updateConfig ensures that the config object accurately reflects the desired
 // settings as configured by the user. It applies the relevant config setting based
 // on the precedence (env var overrides file config, cli overrides env var).
-// It mutates the config object supplied and returns the updated object.
-func (c *AgentCommand) aggregateConfig(f *FlagSets, config *agentConfig.Config) *agentConfig.Config {
+// It mutates the config object supplied.
+func (c *AgentCommand) updateConfig(f *FlagSets, config *agentConfig.Config) {
+	f.updateLogConfig(config.SharedConfig)
+
 	f.Visit(func(fl *flag.Flag) {
 		if fl.Name == flagNameAgentExitAfterAuth {
 			config.ExitAfterAuth = c.flagExitAfterAuth
 		}
 	})
 
-	c.setStringFlag(f, config.LogFile, &StringVar{
-		Name:   flagNameLogFile,
-		EnvVar: EnvVaultLogFile,
-		Target: &c.flagLogFile,
-	})
-	config.LogFile = c.flagLogFile
-
-	c.setStringFlag(f, config.LogLevel, &StringVar{
-		Name:   flagNameLogLevel,
-		EnvVar: EnvVaultLogLevel,
-		Target: &c.flagLogLevel,
-	})
-	config.LogLevel = c.flagLogLevel
-
 	c.setStringFlag(f, config.Vault.Address, &StringVar{
 		Name:    flagNameAddress,
 		Target:  &c.flagAddress,
@@ -1000,8 +991,6 @@ func (c *AgentCommand) aggregateConfig(f *FlagSets, config *agentConfig.Config)
 		EnvVar:  api.EnvVaultTLSServerName,
 	})
 	config.Vault.TLSServerName = c.flagTLSServerName
-
-	return config
 }
 
 // verifyRequestHeader wraps an http.Handler inside a Handler that checks for

command/agent/config/config.go

@@ -37,7 +37,6 @@ type Config struct {
 	DisableKeepAlivesCaching    bool                       `hcl:"-"`
 	DisableKeepAlivesTemplating bool                       `hcl:"-"`
 	DisableKeepAlivesAutoAuth   bool                       `hcl:"-"`
-	LogFile                     string                     `hcl:"log_file"`
 }
 
 const (

command/agent/config/config_test.go

@@ -198,6 +198,7 @@ func TestLoadConfigFile(t *testing.T) {
 	expected := &Config{
 		SharedConfig: &configutil.SharedConfig{
 			PidFile: "./pidfile",
+			LogFile: "/var/log/vault/vault-agent.log",
 		},
 		AutoAuth: &AutoAuth{
 			Method: &Method{
@@ -237,7 +238,6 @@ func TestLoadConfigFile(t *testing.T) {
 				NumRetries: 12,
 			},
 		},
-		LogFile: "/var/log/vault/vault-agent.log",
 	}
 
 	config.Prune()

command/agent_test.go

@@ -2250,7 +2250,7 @@ cache {}
 	wg.Wait()
 }
 
-func TestAgent_LogFile_EnvVarOverridesConfig(t *testing.T) {
+func TestAgent_LogFile_CliOverridesConfig(t *testing.T) {
 	// Create basic config
 	configFile := populateTempFile(t, "agent-config.hcl", BasicHclConfig)
 	cfg, err := agentConfig.LoadConfig(configFile.Name())
@@ -2261,50 +2261,6 @@ func TestAgent_LogFile_EnvVarOverridesConfig(t *testing.T) {
 	// Sanity check that the config value is the current value
 	assert.Equal(t, "/foo/bar/juan.log", cfg.LogFile)
 
-	// Make sure the env var is configured
-	oldEnvVarLogFile := os.Getenv(EnvVaultLogFile)
-	os.Setenv(EnvVaultLogFile, "/squiggle/logs.txt")
-	if oldEnvVarLogFile == "" {
-		defer os.Unsetenv(EnvVaultLogFile)
-	} else {
-		defer os.Setenv(EnvVaultLogFile, oldEnvVarLogFile)
-	}
-
-	// Initialize the command and parse any flags
-	cmd := &AgentCommand{BaseCommand: &BaseCommand{}}
-	f := cmd.Flags()
-	err = f.Parse([]string{})
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	// Update the config based on the inputs.
-	cfg = cmd.aggregateConfig(f, cfg)
-
-	assert.NotEqual(t, "/foo/bar/juan.log", cfg.LogFile)
-	assert.Equal(t, "/squiggle/logs.txt", cfg.LogFile)
-}
-
-func TestAgent_LogFile_CliOverridesEnvVar(t *testing.T) {
-	// Create basic config
-	configFile := populateTempFile(t, "agent-config.hcl", BasicHclConfig)
-	cfg, err := agentConfig.LoadConfig(configFile.Name())
-	if err != nil {
-		t.Fatal("Cannot load config to test update/merge", err)
-	}
-
-	// Sanity check that the config value is the current value
-	assert.Equal(t, "/foo/bar/juan.log", cfg.LogFile)
-
-	// Make sure the env var is configured
-	oldEnvVarLogFile := os.Getenv(EnvVaultLogFile)
-	os.Setenv(EnvVaultLogFile, "/squiggle/logs.txt")
-	if oldEnvVarLogFile == "" {
-		defer os.Unsetenv(EnvVaultLogFile)
-	} else {
-		defer os.Setenv(EnvVaultLogFile, oldEnvVarLogFile)
-	}
-
 	// Initialize the command and parse any flags
 	cmd := &AgentCommand{BaseCommand: &BaseCommand{}}
 	f := cmd.Flags()
@@ -2315,17 +2271,14 @@ func TestAgent_LogFile_CliOverridesEnvVar(t *testing.T) {
 	}
 
 	// Update the config based on the inputs.
-	cfg = cmd.aggregateConfig(f, cfg)
+	cmd.updateConfig(f, cfg)
 
 	assert.NotEqual(t, "/foo/bar/juan.log", cfg.LogFile)
 	assert.NotEqual(t, "/squiggle/logs.txt", cfg.LogFile)
 	assert.Equal(t, "/foo/bar/test.log", cfg.LogFile)
 }
 
 func TestAgent_LogFile_Config(t *testing.T) {
-	// Sanity check, remove any env var
-	os.Unsetenv(EnvVaultLogFile)
-
 	configFile := populateTempFile(t, "agent-config.hcl", BasicHclConfig)
 
 	cfg, err := agentConfig.LoadConfig(configFile.Name())
@@ -2344,7 +2297,7 @@ func TestAgent_LogFile_Config(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	cfg = cmd.aggregateConfig(f, cfg)
+	cmd.updateConfig(f, cfg)
 
 	assert.Equal(t, "/foo/bar/juan.log", cfg.LogFile, "actual config check")
 }

command/commands.go

@@ -82,8 +82,8 @@ const (
 	EnvVaultLicensePath = "VAULT_LICENSE_PATH"
 	// EnvVaultDetailed is to output detailed information (e.g., ListResponseWithInfo).
 	EnvVaultDetailed = `VAULT_DETAILED`
-	// EnvVaultLogFile is used to specify the path to the log file that Vault should use for logging
-	EnvVaultLogFile = "VAULT_LOG_FILE"
+	// EnvVaultLogFormat is used to specify the log format. Supported values are "standard" and "json"
+	EnvVaultLogFormat = "VAULT_LOG_FORMAT"
 	// EnvVaultLogLevel is used to specify the log level applied to logging
 	// Supported log levels: Trace, Debug, Error, Warn, Info
 	EnvVaultLogLevel = "VAULT_LOG_LEVEL"
@@ -141,8 +141,18 @@ const (
 	flagNameUserLockoutDisable = "user-lockout-disable"
 	// flagNameDisableRedirects is used to prevent the client from honoring a single redirect as a response to a request
 	flagNameDisableRedirects = "disable-redirects"
+	// flagNameCombineLogs is used to specify whether log output should be combined and sent to stdout
+	flagNameCombineLogs = "combine-logs"
 	// flagNameLogFile is used to specify the path to the log file that Vault should use for logging
 	flagNameLogFile = "log-file"
+	// flagNameLogRotateBytes is the flag used to specify the number of bytes a log file should be before it is rotated.
+	flagNameLogRotateBytes = "log-rotate-bytes"
+	// flagNameLogRotateDuration is the flag used to specify the duration after which a log file should be rotated.
+	flagNameLogRotateDuration = "log-rotate-duration"
+	// flagNameLogRotateMaxFiles is the flag used to specify the maximum number of older/archived log files to keep.
+	flagNameLogRotateMaxFiles = "log-rotate-max-files"
+	// flagNameLogFormat is the flag used to specify the log format. Supported values are "standard" and "json"
+	flagNameLogFormat = "log-format"
 	// flagNameLogLevel is used to specify the log level applied to logging
 	// Supported log levels: Trace, Debug, Error, Warn, Info
 	flagNameLogLevel = "log-level"