diff --git a/helper/testhelpers/consul/cluster_storage.go b/helper/testhelpers/consul/cluster_storage.go index ed3c1b4f75..52c7dec036 100644 --- a/helper/testhelpers/consul/cluster_storage.go +++ b/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 } diff --git a/helper/testhelpers/consul/consulhelper.go b/helper/testhelpers/consul/consulhelper.go index eaa0ff9d4c..b0a2b2603e 100644 --- a/helper/testhelpers/consul/consulhelper.go +++ b/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"), diff --git a/physical/consul/consul.go b/physical/consul/consul.go index 25fb88c5c8..8cde037583 100644 --- a/physical/consul/consul.go +++ b/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 } diff --git a/physical/raft/raft.go b/physical/raft/raft.go index 2f6b946a6a..29b4ee07b4 100644 --- a/physical/raft/raft.go +++ b/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 diff --git a/sdk/physical/inmem/inmem.go b/sdk/physical/inmem/inmem.go index 4365d7ff04..aac7feaac9 100644 --- a/sdk/physical/inmem/inmem.go +++ b/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) diff --git a/sdk/physical/physical.go b/sdk/physical/physical.go index 9baa60795b..add5f1ede0 100644 --- a/sdk/physical/physical.go +++ b/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, diff --git a/vault/mount.go b/vault/mount.go index a396106189..590cf34ef4 100644 --- a/vault/mount.go +++ b/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 }