Move listener config from 'cache' block to top-level 'listener' blocks. Allow cache without auto-auth. (#6421)

* Since we want to use the Agent listener for #6384, move listener config
from top-level 'cache' block to new top-level 'listeners' block.

* Make agent config allow cache and listener blocks without auto-auth
configured.

.gitignore

@@ -51,6 +51,9 @@ Vagrantfile
 !command/agent/config/test-fixtures/config-cache.hcl
 !command/agent/config/test-fixtures/config-embedded-type.hcl
 !command/agent/config/test-fixtures/config-cache-embedded-type.hcl
+!command/agent/config/test-fixtures/bad-config-cache-inconsistent-auto_auth.hcl
+!command/agent/config/test-fixtures/bad-config-cache-no-listeners.hcl
+!command/agent/config/test-fixtures/config-cache-no-auto_auth.hcl
 
 .DS_Store
 .idea

command/agent.go

@@ -202,10 +202,13 @@ func (c *AgentCommand) Run(args []string) int {
 				"-config flag."))
 		return 1
 	}
-	if config.AutoAuth == nil {
-		c.UI.Error("No auto_auth block found in config file")
+	if config.AutoAuth == nil && config.Cache == nil {
+		c.UI.Error("No auto_auth or cache block found in config file")
 		return 1
 	}
+	if config.AutoAuth == nil {
+		c.UI.Info("No auto_auth block found in config file, not starting automatic authentication feature")
+	}
 
 	if config.Vault != nil {
 		c.setStringFlag(f, config.Vault.Address, &StringVar{
@@ -288,60 +291,62 @@ func (c *AgentCommand) Run(args []string) int {
 
 	ctx, cancelFunc := context.WithCancel(context.Background())
 
+	var method auth.AuthMethod
 	var sinks []*sink.SinkConfig
-	for _, sc := range config.AutoAuth.Sinks {
-		switch sc.Type {
-		case "file":
-			config := &sink.SinkConfig{
-				Logger:  c.logger.Named("sink.file"),
-				Config:  sc.Config,
-				Client:  client,
-				WrapTTL: sc.WrapTTL,
-				DHType:  sc.DHType,
-				DHPath:  sc.DHPath,
-				AAD:     sc.AAD,
-			}
-			s, err := file.NewFileSink(config)
-			if err != nil {
-				c.UI.Error(errwrap.Wrapf("Error creating file sink: {{err}}", err).Error())
+	if config.AutoAuth != nil {
+		for _, sc := range config.AutoAuth.Sinks {
+			switch sc.Type {
+			case "file":
+				config := &sink.SinkConfig{
+					Logger:  c.logger.Named("sink.file"),
+					Config:  sc.Config,
+					Client:  client,
+					WrapTTL: sc.WrapTTL,
+					DHType:  sc.DHType,
+					DHPath:  sc.DHPath,
+					AAD:     sc.AAD,
+				}
+				s, err := file.NewFileSink(config)
+				if err != nil {
+					c.UI.Error(errwrap.Wrapf("Error creating file sink: {{err}}", err).Error())
+					return 1
+				}
+				config.Sink = s
+				sinks = append(sinks, config)
+			default:
+				c.UI.Error(fmt.Sprintf("Unknown sink type %q", sc.Type))
 				return 1
 			}
-			config.Sink = s
-			sinks = append(sinks, config)
+		}
+
+		authConfig := &auth.AuthConfig{
+			Logger:    c.logger.Named(fmt.Sprintf("auth.%s", config.AutoAuth.Method.Type)),
+			MountPath: config.AutoAuth.Method.MountPath,
+			Config:    config.AutoAuth.Method.Config,
+		}
+		switch config.AutoAuth.Method.Type {
+		case "alicloud":
+			method, err = alicloud.NewAliCloudAuthMethod(authConfig)
+		case "aws":
+			method, err = aws.NewAWSAuthMethod(authConfig)
+		case "azure":
+			method, err = azure.NewAzureAuthMethod(authConfig)
+		case "gcp":
+			method, err = gcp.NewGCPAuthMethod(authConfig)
+		case "jwt":
+			method, err = jwt.NewJWTAuthMethod(authConfig)
+		case "kubernetes":
+			method, err = kubernetes.NewKubernetesAuthMethod(authConfig)
+		case "approle":
+			method, err = approle.NewApproleAuthMethod(authConfig)
 		default:
-			c.UI.Error(fmt.Sprintf("Unknown sink type %q", sc.Type))
+			c.UI.Error(fmt.Sprintf("Unknown auth method %q", config.AutoAuth.Method.Type))
+			return 1
+		}
+		if err != nil {
+			c.UI.Error(errwrap.Wrapf(fmt.Sprintf("Error creating %s auth method: {{err}}", config.AutoAuth.Method.Type), err).Error())
 			return 1
 		}
-	}
-
-	var method auth.AuthMethod
-	authConfig := &auth.AuthConfig{
-		Logger:    c.logger.Named(fmt.Sprintf("auth.%s", config.AutoAuth.Method.Type)),
-		MountPath: config.AutoAuth.Method.MountPath,
-		Config:    config.AutoAuth.Method.Config,
-	}
-	switch config.AutoAuth.Method.Type {
-	case "alicloud":
-		method, err = alicloud.NewAliCloudAuthMethod(authConfig)
-	case "aws":
-		method, err = aws.NewAWSAuthMethod(authConfig)
-	case "azure":
-		method, err = azure.NewAzureAuthMethod(authConfig)
-	case "gcp":
-		method, err = gcp.NewGCPAuthMethod(authConfig)
-	case "jwt":
-		method, err = jwt.NewJWTAuthMethod(authConfig)
-	case "kubernetes":
-		method, err = kubernetes.NewKubernetesAuthMethod(authConfig)
-	case "approle":
-		method, err = approle.NewApproleAuthMethod(authConfig)
-	default:
-		c.UI.Error(fmt.Sprintf("Unknown auth method %q", config.AutoAuth.Method.Type))
-		return 1
-	}
-	if err != nil {
-		c.UI.Error(errwrap.Wrapf(fmt.Sprintf("Error creating %s auth method: {{err}}", config.AutoAuth.Method.Type), err).Error())
-		return 1
 	}
 
 	// Output the header that the server has started
@@ -355,21 +360,8 @@ func (c *AgentCommand) Run(args []string) int {
 	default:
 	}
 
-	ss := sink.NewSinkServer(&sink.SinkServerConfig{
-		Logger:        c.logger.Named("sink.server"),
-		Client:        client,
-		ExitAfterAuth: config.ExitAfterAuth,
-	})
-
-	ah := auth.NewAuthHandler(&auth.AuthHandlerConfig{
-		Logger:                       c.logger.Named("auth.handler"),
-		Client:                       c.client,
-		WrapTTL:                      config.AutoAuth.Method.WrapTTL,
-		EnableReauthOnNewCredentials: config.AutoAuth.EnableReauthOnNewCredentials,
-	})
-
 	// Parse agent listener configurations
-	if config.Cache != nil && len(config.Cache.Listeners) != 0 {
+	if config.Cache != nil && len(config.Listeners) != 0 {
 		cacheLogger := c.logger.Named("cache")
 
 		// Create the API proxier
@@ -418,7 +410,7 @@ func (c *AgentCommand) Run(args []string) int {
 		mux.Handle("/", cache.Handler(ctx, cacheLogger, leaseCache, inmemSink))
 
 		var listeners []net.Listener
-		for i, lnConfig := range config.Cache.Listeners {
+		for i, lnConfig := range config.Listeners {
 			ln, tlsConf, err := cache.StartListener(lnConfig)
 			if err != nil {
 				c.UI.Error(fmt.Sprintf("Error starting listener: %v", err))
@@ -461,9 +453,27 @@ func (c *AgentCommand) Run(args []string) int {
 		defer c.cleanupGuard.Do(listenerCloseFunc)
 	}
 
+	var ssDoneCh, ahDoneCh chan struct{}
 	// Start auto-auth and sink servers
-	go ah.Run(ctx, method)
-	go ss.Run(ctx, ah.OutputCh, sinks)
+	if method != nil {
+		ah := auth.NewAuthHandler(&auth.AuthHandlerConfig{
+			Logger:                       c.logger.Named("auth.handler"),
+			Client:                       c.client,
+			WrapTTL:                      config.AutoAuth.Method.WrapTTL,
+			EnableReauthOnNewCredentials: config.AutoAuth.EnableReauthOnNewCredentials,
+		})
+		ahDoneCh = ah.DoneCh
+
+		ss := sink.NewSinkServer(&sink.SinkServerConfig{
+			Logger:        c.logger.Named("sink.server"),
+			Client:        client,
+			ExitAfterAuth: config.ExitAfterAuth,
+		})
+		ssDoneCh = ss.DoneCh
+
+		go ah.Run(ctx, method)
+		go ss.Run(ctx, ah.OutputCh, sinks)
+	}
 
 	// Server configuration output
 	padding := 24
@@ -494,14 +504,18 @@ func (c *AgentCommand) Run(args []string) int {
 	}()
 
 	select {
-	case <-ss.DoneCh:
+	case <-ssDoneCh:
 		// This will happen if we exit-on-auth
 		c.logger.Info("sinks finished, exiting")
 	case <-c.ShutdownCh:
 		c.UI.Output("==> Vault agent shutdown triggered")
 		cancelFunc()
-		<-ah.DoneCh
-		<-ss.DoneCh
+		if ahDoneCh != nil {
+			<-ahDoneCh
+		}
+		if ssDoneCh != nil {
+			<-ssDoneCh
+		}
 	}
 
 	return 0

command/agent/config/config.go

@@ -10,7 +10,7 @@ import (
 
 	"github.com/hashicorp/errwrap"
 	log "github.com/hashicorp/go-hclog"
-	multierror "github.com/hashicorp/go-multierror"
+	"github.com/hashicorp/go-multierror"
 	"github.com/hashicorp/vault/helper/parseutil"
 
 	"github.com/hashicorp/hcl"
@@ -19,11 +19,12 @@ import (
 
 // Config is the configuration for the vault server.
 type Config struct {
-	AutoAuth      *AutoAuth `hcl:"auto_auth"`
-	ExitAfterAuth bool      `hcl:"exit_after_auth"`
-	PidFile       string    `hcl:"pid_file"`
-	Cache         *Cache    `hcl:"cache"`
-	Vault         *Vault    `hcl:"vault"`
+	AutoAuth      *AutoAuth   `hcl:"auto_auth"`
+	ExitAfterAuth bool        `hcl:"exit_after_auth"`
+	PidFile       string      `hcl:"pid_file"`
+	Listeners     []*Listener `hcl:"listeners"`
+	Cache         *Cache      `hcl:"cache"`
+	Vault         *Vault      `hcl:"vault"`
 }
 
 type Vault struct {
@@ -36,8 +37,7 @@ type Vault struct {
 }
 
 type Cache struct {
-	UseAutoAuthToken bool        `hcl:"use_auto_auth_token"`
-	Listeners        []*Listener `hcl:"listeners"`
+	UseAutoAuthToken bool `hcl:"use_auto_auth_token"`
 }
 
 type Listener struct {
@@ -112,11 +112,26 @@ func LoadConfig(path string, logger log.Logger) (*Config, error) {
 		return nil, errwrap.Wrapf("error parsing 'auto_auth': {{err}}", err)
 	}
 
+	err = parseListeners(&result, list)
+	if err != nil {
+		return nil, errwrap.Wrapf("error parsing 'listeners': {{err}}", err)
+	}
+
 	err = parseCache(&result, list)
 	if err != nil {
 		return nil, errwrap.Wrapf("error parsing 'cache':{{err}}", err)
 	}
 
+	if result.Cache != nil {
+		if len(result.Listeners) < 1 {
+			return nil, fmt.Errorf("at least one listener required when cache enabled")
+		}
+
+		if result.Cache.UseAutoAuthToken && result.AutoAuth == nil {
+			return nil, fmt.Errorf("cache.use_auto_auth_token is true but auto_auth not configured")
+		}
+	}
+
 	err = parseVault(&result, list)
 	if err != nil {
 		return nil, errwrap.Wrapf("error parsing 'vault':{{err}}", err)
@@ -171,18 +186,6 @@ func parseCache(result *Config, list *ast.ObjectList) error {
 	}
 
 	result.Cache = &c
-
-	subs, ok := item.Val.(*ast.ObjectType)
-	if !ok {
-		return fmt.Errorf("could not parse %q as an object", name)
-	}
-	subList := subs.List
-
-	err = parseListeners(result, subList)
-	if err != nil {
-		return errwrap.Wrapf("error parsing 'listener' stanzas: {{err}}", err)
-	}
-
 	return nil
 }
 
@@ -190,9 +193,6 @@ func parseListeners(result *Config, list *ast.ObjectList) error {
 	name := "listener"
 
 	listenerList := list.Filter(name)
-	if len(listenerList.Items) < 1 {
-		return fmt.Errorf("at least one %q block is required", name)
-	}
 
 	var listeners []*Listener
 	for _, item := range listenerList.Items {
@@ -225,7 +225,7 @@ func parseListeners(result *Config, list *ast.ObjectList) error {
 		})
 	}
 
-	result.Cache.Listeners = listeners
+	result.Listeners = listeners
 
 	return nil
 }
@@ -234,8 +234,11 @@ func parseAutoAuth(result *Config, list *ast.ObjectList) error {
 	name := "auto_auth"
 
 	autoAuthList := list.Filter(name)
-	if len(autoAuthList.Items) != 1 {
-		return fmt.Errorf("one and only one %q block is required", name)
+	if len(autoAuthList.Items) == 0 {
+		return nil
+	}
+	if len(autoAuthList.Items) > 1 {
+		return fmt.Errorf("at most one %q block is allowed", name)
 	}
 
 	// Get our item

command/agent/config/config_test.go

@@ -42,31 +42,31 @@ func TestLoadConfigFile_AgentCache(t *testing.T) {
 		},
 		Cache: &Cache{
 			UseAutoAuthToken: true,
-			Listeners: []*Listener{
-				&Listener{
-					Type: "unix",
-					Config: map[string]interface{}{
-						"address":      "/path/to/socket",
-						"tls_disable":  true,
-						"socket_mode":  "configmode",
-						"socket_user":  "configuser",
-						"socket_group": "configgroup",
-					},
+		},
+		Listeners: []*Listener{
+			&Listener{
+				Type: "unix",
+				Config: map[string]interface{}{
+					"address":      "/path/to/socket",
+					"tls_disable":  true,
+					"socket_mode":  "configmode",
+					"socket_user":  "configuser",
+					"socket_group": "configgroup",
 				},
-				&Listener{
-					Type: "tcp",
-					Config: map[string]interface{}{
-						"address":     "127.0.0.1:8300",
-						"tls_disable": true,
-					},
+			},
+			&Listener{
+				Type: "tcp",
+				Config: map[string]interface{}{
+					"address":     "127.0.0.1:8300",
+					"tls_disable": true,
 				},
-				&Listener{
-					Type: "tcp",
-					Config: map[string]interface{}{
-						"address":       "127.0.0.1:8400",
-						"tls_key_file":  "/path/to/cakey.pem",
-						"tls_cert_file": "/path/to/cacert.pem",
-					},
+			},
+			&Listener{
+				Type: "tcp",
+				Config: map[string]interface{}{
+					"address":       "127.0.0.1:8400",
+					"tls_key_file":  "/path/to/cakey.pem",
+					"tls_cert_file": "/path/to/cacert.pem",
 				},
 			},
 		},
@@ -154,3 +154,48 @@ func TestLoadConfigFile(t *testing.T) {
 		t.Fatal(diff)
 	}
 }
+
+func TestLoadConfigFile_AgentCache_NoAutoAuth(t *testing.T) {
+	logger := logging.NewVaultLogger(log.Debug)
+
+	config, err := LoadConfig("./test-fixtures/config-cache-no-auto_auth.hcl", logger)
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	expected := &Config{
+		Cache: &Cache{},
+		Listeners: []*Listener{
+			&Listener{
+				Type: "tcp",
+				Config: map[string]interface{}{
+					"address":     "127.0.0.1:8300",
+					"tls_disable": true,
+				},
+			},
+		},
+		PidFile: "./pidfile",
+	}
+
+	if diff := deep.Equal(config, expected); diff != nil {
+		t.Fatal(diff)
+	}
+}
+
+func TestLoadConfigFile_Bad_AgentCache_InconsisentAutoAuth(t *testing.T) {
+	logger := logging.NewVaultLogger(log.Debug)
+
+	_, err := LoadConfig("./test-fixtures/bad-config-cache-inconsistent-auto_auth.hcl", logger)
+	if err == nil {
+		t.Fatal("LoadConfig should return an error when use_auto_auth_token=true and no auto_auth section present")
+	}
+}
+
+func TestLoadConfigFile_Bad_AgentCache_NoListeners(t *testing.T) {
+	logger := logging.NewVaultLogger(log.Debug)
+
+	_, err := LoadConfig("./test-fixtures/bad-config-cache-no-listeners.hcl", logger)
+	if err == nil {
+		t.Fatal("LoadConfig should return an error when cache section present and no listeners present")
+	}
+}

command/agent/config/test-fixtures/bad-config-cache-inconsistent-auto_auth.hcl

@@ -0,0 +1,12 @@
+pid_file = "./pidfile"
+
+cache {
+	use_auto_auth_token = true
+}
+
+listener "tcp" {
+    address = "127.0.0.1:8300"
+    tls_disable = true
+}
+
+

command/agent/config/test-fixtures/bad-config-cache-no-listeners.hcl

@@ -0,0 +1,5 @@
+pid_file = "./pidfile"
+
+cache {
+}
+

command/agent/config/test-fixtures/config-cache-embedded-type.hcl

@@ -22,28 +22,28 @@ auto_auth {
 
 cache {
 	use_auto_auth_token = true
+}
 
-	listener {
-		type = "unix"
-		address = "/path/to/socket"
-		tls_disable = true
-		socket_mode = "configmode"
-		socket_user = "configuser"
-		socket_group = "configgroup"
-	}
+listener {
+    type = "unix"
+    address = "/path/to/socket"
+    tls_disable = true
+    socket_mode = "configmode"
+    socket_user = "configuser"
+    socket_group = "configgroup"
+}
 
-	listener {
-		type = "tcp"
-		address = "127.0.0.1:8300"
-		tls_disable = true
-	}
+listener {
+    type = "tcp"
+    address = "127.0.0.1:8300"
+    tls_disable = true
+}
 
-	listener {
-		type = "tcp"
-		address = "127.0.0.1:8400"
-		tls_key_file = "/path/to/cakey.pem"
-		tls_cert_file = "/path/to/cacert.pem"
-	}
+listener {
+    type = "tcp"
+    address = "127.0.0.1:8400"
+    tls_key_file = "/path/to/cakey.pem"
+    tls_cert_file = "/path/to/cacert.pem"
 }
 
 vault {

command/agent/config/test-fixtures/config-cache-no-auto_auth.hcl

@@ -0,0 +1,11 @@
+pid_file = "./pidfile"
+
+cache {
+}
+
+listener "tcp" {
+    address = "127.0.0.1:8300"
+    tls_disable = true
+}
+
+

command/agent/config/test-fixtures/config-cache.hcl

@@ -22,25 +22,25 @@ auto_auth {
 
 cache {
 	use_auto_auth_token = true
+}
 
-	listener "unix" {
-		address = "/path/to/socket"
-		tls_disable = true
-		socket_mode = "configmode"
-		socket_user = "configuser"
-		socket_group = "configgroup"
-	}
+listener "unix" {
+    address = "/path/to/socket"
+    tls_disable = true
+    socket_mode = "configmode"
+    socket_user = "configuser"
+    socket_group = "configgroup"
+}
 
-	listener "tcp" {
-		address = "127.0.0.1:8300"
-		tls_disable = true
-	}
+listener "tcp" {
+    address = "127.0.0.1:8300"
+    tls_disable = true
+}
 
-	listener "tcp" {
-		address = "127.0.0.1:8400"
-		tls_key_file = "/path/to/cakey.pem"
-		tls_cert_file = "/path/to/cacert.pem"
-	}
+listener "tcp" {
+    address = "127.0.0.1:8400"
+    tls_key_file = "/path/to/cakey.pem"
+    tls_cert_file = "/path/to/cacert.pem"
 }
 
 vault {

website/source/docs/agent/caching/index.html.md

@@ -177,11 +177,11 @@ The top level `cache` block has the following configuration entries:
   configuration will be overridden and the token in the request will be used to
   forward the request to the Vault server.
 
+## Configuration (`listener`)
+
 - `listener` `(array of objects: required)` - Configuration for the listeners.
 
-### Configuration (`listener`)
-
-There can be one or more `listener` blocks inside the top-level `cache` block.
+There can be one or more `listener` blocks at the top level.
 These configuration values are common to all `listener` blocks.
 
 - `type` `(string: required)` - The type of the listener to use. Valid values
@@ -207,17 +207,38 @@ These configuration values are common to all `listener` blocks.
 An example configuration, with very contrived values, follows:
 
 ```javascript
-cache {
-  use_auto_auth_token = true
-
-  listener "unix" {
-    address = "/path/to/socket"
-    tls_disable = true
+auto_auth {
+  method {
+    type = "aws"
+    wrap_ttl = 300
+    config = {
+      role = "foobar"
+    }
   }
 
-  listener "tcp" {
-    address = "127.0.0.1:8200"
-    tls_disable = true
+  sink {
+    type = "file"
+    config = {
+      path = "/tmp/file-foo"
+    }
   }
 }
+
+cache {
+  use_auto_auth_token = true
+}
+
+listener "unix" {
+  address = "/path/to/socket"
+  tls_disable = true
+}
+
+listener "tcp" {
+  address = "127.0.0.1:8200"
+  tls_disable = true
+}
+
+vault {
+  address = "http://127.0.0.1:8200"
+}
 ```