VAULT-45341: Only add required mounts if there's space in mount table (#15014) (#15079)

* only add required mounts if there's space

* correct the docker version, add comments

* fix base version

Co-authored-by: miagilepner <mia.epner@hashicorp.com>

helper/testhelpers/consul/cluster_storage.go

@@ -6,6 +6,7 @@ package consul
 import (
 	"context"
 	"fmt"
+	"sync/atomic"
 
 	"github.com/hashicorp/vault/sdk/helper/testcluster"
 )
@@ -16,9 +17,10 @@ type ClusterStorage struct {
 	// your test. Leave empty for latest OSS (defined in consulhelper.go).
 	ConsulVersion    string
 	ConsulEnterprise bool
-
-	cleanup func()
-	config  *Config
+	ConsulConfig     string
+	cleanup          func()
+	config           *Config
+	started          atomic.Bool
 }
 
 var _ testcluster.ClusterStorage = &ClusterStorage{}
@@ -28,16 +30,20 @@ func NewClusterStorage() *ClusterStorage {
 }
 
 func (s *ClusterStorage) Start(ctx context.Context, opts *testcluster.ClusterOptions) error {
+	if s.started.Load() {
+		return nil
+	}
 	prefix := ""
 	if opts != nil && opts.ClusterName != "" {
 		prefix = fmt.Sprintf("%s-", opts.ClusterName)
 	}
-	cleanup, config, err := RunContainer(ctx, prefix, s.ConsulVersion, s.ConsulEnterprise, true)
+	cleanup, config, err := RunContainerConfig(ctx, prefix, s.ConsulVersion, s.ConsulEnterprise, true, s.ConsulConfig)
 	if err != nil {
 		return err
 	}
 	s.cleanup = cleanup
 	s.config = config
+	s.started.Store(true)
 
 	return nil
 }

helper/testhelpers/consul/consulhelper.go

@@ -61,6 +61,13 @@ func PrepareTestContainer(t *testing.T, version string, isEnterprise bool, doBoo
 // is used by `PrepareTestContainer` which is used typically in tests that rely
 // on Consul but run tested code within the test process.
 func RunContainer(ctx context.Context, namePrefix, version string, isEnterprise bool, doBootstrapSetup bool) (func(), *Config, error) {
+	return RunContainerConfig(ctx, namePrefix, version, isEnterprise, doBootstrapSetup, "")
+}
+
+// RunContainerConfig starts Consul with an optional additional config string.
+// The addlConfig variable should be hcl and will be added to the default
+// Consul configuration
+func RunContainerConfig(ctx context.Context, namePrefix, version string, isEnterprise bool, doBootstrapSetup bool, addlConfig string) (func(), *Config, error) {
 	if retAddress := os.Getenv("CONSUL_HTTP_ADDR"); retAddress != "" {
 		shp, err := docker.NewServiceHostPortParse(retAddress)
 		if err != nil {
@@ -105,12 +112,16 @@ func RunContainer(ctx context.Context, namePrefix, version string, isEnterprise
 		repo = dockerRepo
 	}
 
+	cmd := []string{"agent", "-dev", "-client", "0.0.0.0", "-hcl", config}
+	if addlConfig != "" {
+		cmd = append(cmd, "-hcl", addlConfig)
+	}
 	dockerOpts := docker.RunOptions{
 		ContainerName: name,
 		ImageRepo:     repo,
 		ImageTag:      version,
 		Env:           envVars,
-		Cmd:           []string{"agent", "-dev", "-client", "0.0.0.0", "-hcl", config},
+		Cmd:           cmd,
 		Ports:         []string{"8500/tcp"},
 		AuthUsername:  os.Getenv("CONSUL_DOCKER_USERNAME"),
 		AuthPassword:  os.Getenv("CONSUL_DOCKER_PASSWORD"),

physical/consul/consul.go

@@ -348,8 +348,8 @@ func (c *ConsulBackend) txnInternal(ctx context.Context, txns []*physical.TxnEnt
 
 	ok, resp, _, err := c.txn.Txn(ops, queryOpts)
 	if err != nil {
-		if strings.Contains(err.Error(), "is too large") {
-			return fmt.Errorf("%s: %w", physical.ErrValueTooLarge, err)
+		if strings.Contains(err.Error(), " too large") {
+			return errors.Join(physical.ErrValueSize, err)
 		}
 		return err
 	}

physical/raft/raft.go

@@ -2013,7 +2013,7 @@ func (b *RaftBackend) validateCommandEntrySizes(command *LogData) (uint64, error
 		if op.OpType == putOp {
 			entrySize := b.entrySizeLimitForPath(op.Key)
 			if len(op.Value) > int(entrySize) {
-				return 0, fmt.Errorf("%s, max value size for key %s is %d, got %d", physical.ErrValueTooLarge, op.Key, entrySize, len(op.Value))
+				return 0, fmt.Errorf("%w, max value size for key %s is %d, got %d", physical.ErrValueSize, op.Key, entrySize, len(op.Value))
 			}
 			if entrySize > largestEntryLimit {
 				largestEntryLimit = entrySize

sdk/physical/inmem/inmem.go

@@ -6,7 +6,6 @@ package inmem
 import (
 	"context"
 	"errors"
-	"fmt"
 	"os"
 	"strconv"
 	"strings"
@@ -176,7 +175,7 @@ func (i *InmemBackend) PutInternal(ctx context.Context, entry *physical.Entry) e
 	}
 
 	if i.maxValueSize > 0 && len(entry.Value) > i.maxValueSize {
-		return fmt.Errorf("%s", physical.ErrValueTooLarge)
+		return physical.ErrValueSize
 	}
 
 	i.root.Insert(entry.Key, entry.Value)

sdk/physical/physical.go

@@ -5,6 +5,7 @@ package physical
 
 import (
 	"context"
+	"errors"
 	"strings"
 
 	log "github.com/hashicorp/go-hclog"
@@ -28,6 +29,9 @@ const (
 	ErrKeyTooLarge   = "put failed due to key being too large"
 )
 
+// ErrValueSize is the error returned when the value is too large
+var ErrValueSize = errors.New(ErrValueTooLarge)
+
 // Backend is the interface required for a physical
 // backend. A physical backend is used to durably store
 // data outside of Vault. As such, it is completely untrusted,

vault/mount.go

@@ -25,6 +25,7 @@ import (
 	"github.com/hashicorp/vault/sdk/helper/consts"
 	"github.com/hashicorp/vault/sdk/helper/jsonutil"
 	"github.com/hashicorp/vault/sdk/logical"
+	"github.com/hashicorp/vault/sdk/physical"
 	"github.com/hashicorp/vault/vault/observations"
 	"github.com/hashicorp/vault/vault/plugincatalog"
 	"github.com/mitchellh/copystructure"
@@ -157,6 +158,18 @@ type MountTable struct {
 	Entries []*MountEntry `json:"entries"`
 }
 
+func (m *MountTable) Clone() (*MountTable, error) {
+	entries := make([]*MountEntry, 0, len(m.Entries))
+	for _, entry := range m.Entries {
+		clonedEntry, err := entry.Clone()
+		if err != nil {
+			return nil, err
+		}
+		entries = append(entries, clonedEntry)
+	}
+	return &MountTable{Type: m.Type, Entries: entries}, nil
+}
+
 //go:generate enumer -type=MountMigrationStatus -trimprefix=MigrationStatus -transform=kebab
 
 type MountMigrationStatus int
@@ -1360,7 +1373,17 @@ func (c *Core) loadMounts(ctx context.Context) error {
 	// If this node is a performance standby we do not want to attempt to
 	// upgrade the mount table, this will be the active node's responsibility.
 	if !c.perfStandby {
-		err := c.runMountUpdates(ctx, needPersist)
+		oldMounts, err := c.mounts.Clone()
+		if err != nil {
+			c.logger.Error("failed to clone mount table", "error", err)
+			return err
+		}
+		err = c.runMountUpdates(ctx, needPersist, true)
+		if errors.Is(err, physical.ErrValueSize) {
+			c.logger.Error("Cannot add default mounts because the mount table is too large to write to storage. If you are using integrated storage, increase the max_mount_and_namespace_table_entry_size in your Vault config file to add all default mounts to all namespaces. If you are using Consul storage, increase the txn_max_req_len in your Consul config file to add all default mounts to all namespaces. Continuing without default mounts")
+			c.mounts = oldMounts
+			err = c.runMountUpdates(ctx, needPersist, false)
+		}
 		if err != nil {
 			c.logger.Error("failed to run mount table upgrades", "error", err)
 			return err
@@ -1406,7 +1429,7 @@ func (c *Core) loadMounts(ctx context.Context) error {
 
 // Note that this is only designed to work with singletons, as it checks by
 // type only.
-func (c *Core) runMountUpdates(ctx context.Context, needPersist bool) error {
+func (c *Core) runMountUpdates(ctx context.Context, needPersist, tryAddingRequiredMounts bool) error {
 	// Upgrade to typed mount table
 	if c.mounts.Type == "" {
 		c.mounts.Type = mountTableType
@@ -1430,13 +1453,13 @@ func (c *Core) runMountUpdates(ctx context.Context, needPersist bool) error {
 		// ensure this comes over. If we upgrade first, we simply don't
 		// create the mount, so we won't conflict when we sync. If this is
 		// local (e.g. cubbyhole) we do still add it.
-		if !foundRequired && (!c.IsPerfSecondary() || requiredMount.Local) {
+		if !foundRequired && (!c.IsPerfSecondary() || requiredMount.Local) && tryAddingRequiredMounts {
 			c.mounts.Entries = append(c.mounts.Entries, requiredMount)
 			needPersist = true
 		}
 	}
 
-	if !c.IsPerfSecondary() {
+	if !c.IsPerfSecondary() && tryAddingRequiredMounts {
 		var modified bool
 		var err error
 		c.mounts.Entries, modified, err = c.addRequiredNamespaceMounts(c.mounts.Entries)
@@ -1499,7 +1522,7 @@ func (c *Core) runMountUpdates(ctx context.Context, needPersist bool) error {
 	// Persist both mount tables
 	if err := c.persistMounts(ctx, c.mounts, nil); err != nil {
 		c.logger.Error("failed to persist mount table", "error", err)
-		return errLoadMountsFailed
+		return errors.Join(errLoadMountsFailed, err)
 	}
 	return nil
 }