Add config value that gives users options to skip calculating role fo…

…r each lease (#22651) * Add config value that gives users options to skip calculating role for each lease * add changelog * change name * add config for testing * Update changelog/22651.txt Co-authored-by: Violet Hynes <[email protected]> * update tests, docs and reorder logic in conditional * fix comment * update comment * fix comment again * Update comments and change if order * change comment again * add other comment * fix tests * add documentation * edit docs * Update http/util.go Co-authored-by: Mike Palmiotto <[email protected]> * Update vault/core.go * Update vault/core.go * update var name * udpate docs * Update vault/request_handling.go Co-authored-by: Mike Palmiotto <[email protected]> * 1 more docs change --------- Co-authored-by: Violet Hynes <[email protected]> Co-authored-by: Mike Palmiotto <[email protected]>
hashicorp · Sep 1, 2023 · bd36e66 · bd36e66
1 parent 897cbbc
commit bd36e66
Show file tree

Hide file tree

Showing 10 changed files with 40 additions and 4 deletions.
diff --git a/changelog/22651.txt b/changelog/22651.txt
@@ -0,0 +1,3 @@
+```release-note:improvement
+core/quotas: Add configuration to allow skipping of expensive role calculations
+```
diff --git a/command/server.go b/command/server.go
@@ -2998,6 +2998,7 @@ func createCoreConfig(c *ServerCommand, config *server.Config, backend physical.
 		LogicalBackends:                c.LogicalBackends,
 		Logger:                         c.logger,
 		DetectDeadlocks:                config.DetectDeadlocks,
+		ImpreciseLeaseRoleTracking:     config.ImpreciseLeaseRoleTracking,
 		DisableSentinelTrace:           config.DisableSentinelTrace,
 		DisableCache:                   config.DisableCache,
 		DisableMlock:                   config.DisableMlock,

diff --git a/command/server/config.go b/command/server/config.go
@@ -112,6 +112,8 @@ type Config struct {
 
 	DetectDeadlocks string `hcl:"detect_deadlocks"`
 
+	ImpreciseLeaseRoleTracking bool `hcl:"imprecise_lease_role_tracking"`
+
 	EnableResponseHeaderRaftNodeID    bool        `hcl:"-"`
 	EnableResponseHeaderRaftNodeIDRaw interface{} `hcl:"enable_response_header_raft_node_id"`
 
@@ -412,6 +414,11 @@ func (c *Config) Merge(c2 *Config) *Config {
 		result.DetectDeadlocks = c2.DetectDeadlocks
 	}
 
+	result.ImpreciseLeaseRoleTracking = c.ImpreciseLeaseRoleTracking
+	if c2.ImpreciseLeaseRoleTracking {
+		result.ImpreciseLeaseRoleTracking = c2.ImpreciseLeaseRoleTracking
+	}
+
 	result.EnableResponseHeaderRaftNodeID = c.EnableResponseHeaderRaftNodeID
 	if c2.EnableResponseHeaderRaftNodeID {
 		result.EnableResponseHeaderRaftNodeID = c2.EnableResponseHeaderRaftNodeID
@@ -1144,6 +1151,8 @@ func (c *Config) Sanitized() map[string]interface{} {
 		"experiments":        c.Experiments,
 
 		"detect_deadlocks": c.DetectDeadlocks,
+
+		"imprecise_lease_role_tracking": c.ImpreciseLeaseRoleTracking,
 	}
 	for k, v := range sharedResult {
 		result[k] = v

diff --git a/command/server/config_test_helpers.go b/command/server/config_test_helpers.go
@@ -849,6 +849,7 @@ func testConfig_Sanitized(t *testing.T) {
 			"add_mount_point_rollback_metrics":       false,
 		},
 		"administrative_namespace_path": "admin/",
+		"imprecise_lease_role_tracking": false,
 	}
 
 	addExpectedEntSanitizedConfig(expected, []string{"http"})

diff --git a/http/sys_config_state_test.go b/http/sys_config_state_test.go
@@ -175,6 +175,7 @@ func TestSysConfigState_Sanitized(t *testing.T) {
 				},
 				"storage":                       tc.expectedStorageOutput,
 				"administrative_namespace_path": "",
+				"imprecise_lease_role_tracking": false,
 			}
 
 			if tc.expectedHAStorageOutput != nil {

diff --git a/http/util.go b/http/util.go
@@ -67,6 +67,8 @@ func rateLimitQuotaWrapping(handler http.Handler, core *vault.Core) http.Handler
 			NamespacePath: ns.Path,
 			ClientAddress: parseRemoteIPAddress(r),
 		}
+
+		// This checks if any role based quota is required (LCQ or RLQ).
 		requiresResolveRole, err := core.ResolveRoleForQuotas(r.Context(), quotaReq)
 		if err != nil {
 			core.Logger().Error("failed to lookup quotas", "path", path, "error", err)

diff --git a/vault/core.go b/vault/core.go
@@ -699,6 +699,9 @@ type Core struct {
 	// if populated, the callback is called for every request
 	// for testing purposes
 	requestResponseCallback func(logical.Backend, *logical.Request, *logical.Response)
+
+	// If any role based quota (LCQ or RLQ) is enabled, don't track lease counts by role
+	impreciseLeaseRoleTracking bool
 }
 
 // c.stateLock needs to be held in read mode before calling this function.
@@ -760,6 +763,9 @@ type CoreConfig struct {
 	// Use the deadlocks library to detect deadlocks
 	DetectDeadlocks string
 
+	// If any role based quota (LCQ or RLQ) is enabled, don't track lease counts by role
+	ImpreciseLeaseRoleTracking bool
+
 	// Disables the trace display for Sentinel checks
 	DisableSentinelTrace bool
 
@@ -1032,6 +1038,7 @@ func CreateCore(conf *CoreConfig) (*Core, error) {
 		pendingRemovalMountsAllowed:    conf.PendingRemovalMountsAllowed,
 		expirationRevokeRetryBase:      conf.ExpirationRevokeRetryBase,
 		rollbackMountPathMetrics:       conf.MetricSink.TelemetryConsts.RollbackMetricsIncludeMountPoint,
+		impreciseLeaseRoleTracking:     conf.ImpreciseLeaseRoleTracking,
 	}
 
 	c.standbyStopCh.Store(make(chan struct{}))

diff --git a/vault/request_handling.go b/vault/request_handling.go
@@ -1687,10 +1687,16 @@ func (c *Core) handleLoginRequest(ctx context.Context, req *logical.Request) (re
 		// Attach the display name, might be used by audit backends
 		req.DisplayName = auth.DisplayName
 
-		// If this is not a role-based quota, we still need to associate the
-		// login role with this lease for later lease-count quotas to be
-		// accurate.
-		if reqRole == nil && resp.Auth.TokenType != logical.TokenTypeBatch {
+		requiresLease := resp.Auth.TokenType != logical.TokenTypeBatch
+
+		// If role was not already determined by http.rateLimitQuotaWrapping
+		// and a lease will be generated, calculate a role for the leaseEntry.
+		// We can skip this step if there are no pre-existing role-based quotas
+		// for this mount and Vault is configured to skip lease role-based lease counting
+		// until after they're created. This effectively zeroes out the lease count
+		// for new role-based quotas upon creation, rather than counting old leases toward
+		// the total.
+		if reqRole == nil && requiresLease && !c.impreciseLeaseRoleTracking {
 			role = c.DetermineRoleFromLoginRequest(ctx, req.MountPoint, req.Data)
 		}
 

diff --git a/vault/testing.go b/vault/testing.go
@@ -225,6 +225,7 @@ func TestCoreWithSealAndUINoCleanup(t testing.T, opts *CoreConfig) *Core {
 	conf.CensusAgent = opts.CensusAgent
 	conf.AdministrativeNamespacePath = opts.AdministrativeNamespacePath
 	conf.AllLoggers = logger.AllLoggers
+	conf.ImpreciseLeaseRoleTracking = opts.ImpreciseLeaseRoleTracking
 
 	if opts.Logger != nil {
 		conf.Logger = opts.Logger
@@ -1553,6 +1554,7 @@ func NewTestCluster(t testing.T, base *CoreConfig, opts *TestClusterOptions) *Te
 		coreConfig.DisableAutopilot = base.DisableAutopilot
 		coreConfig.AdministrativeNamespacePath = base.AdministrativeNamespacePath
 		coreConfig.ServiceRegistration = base.ServiceRegistration
+		coreConfig.ImpreciseLeaseRoleTracking = base.ImpreciseLeaseRoleTracking
 
 		if base.BuiltinRegistry != nil {
 			coreConfig.BuiltinRegistry = base.BuiltinRegistry

diff --git a/website/content/docs/configuration/index.mdx b/website/content/docs/configuration/index.mdx
@@ -221,6 +221,10 @@ a negative effect on performance due to the tracking of each lock attempt.
   the `VAULT_EXPERIMENTS` environment variable as a comma-separated list, or via the
   [`-experiment`](/vault/docs/commands/server#experiment) flag.
 
+- `imprecise_lease_role_tracking` `(bool: "false")` - Skip lease counting by role if there are no role based quotas enabled.
+  When `imprecise_lease_role_tracking` is set to true and a new role-based quota is enabled, subsequent lease counts start from 0.
+  `imprecise_lease_role_tracking` affects role-based lease count quotas, but reduces latencies when not using role based quotas.
+
 ### High availability parameters
 
 The following parameters are used on backends that support [high availability][high-availability].