diff --git a/changelog/12888.txt b/changelog/12888.txt new file mode 100644 index 0000000000000..1595a62ee8bcf --- /dev/null +++ b/changelog/12888.txt @@ -0,0 +1,5 @@ +```release-note:deprecation +expiration: VAULT_LEASE_USE_LEGACY_REVOCATION_STRATEGY environment variable has +been removed. +expiration: VAULT_16_REVOKE_PERMITPOOL environment variable has been removed. +``` \ No newline at end of file diff --git a/vault/core.go b/vault/core.go index ce4ff1a86bcff..8594f7cc496f6 100644 --- a/vault/core.go +++ b/vault/core.go @@ -14,7 +14,6 @@ import ( "net" "net/http" "net/url" - "os" "path/filepath" "strings" "sync" @@ -2037,13 +2036,7 @@ func (s standardUnsealStrategy) unseal(ctx context.Context, logger log.Logger, c if err := c.startRollback(); err != nil { return err } - var expirationStrategy ExpireLeaseStrategy - if os.Getenv("VAULT_LEASE_USE_LEGACY_REVOCATION_STRATEGY") != "" { - expirationStrategy = expireLeaseStrategyRevoke - } else { - expirationStrategy = expireLeaseStrategyFairsharing - } - if err := c.setupExpiration(expirationStrategy); err != nil { + if err := c.setupExpiration(expireLeaseStrategyFairsharing); err != nil { return err } if err := c.loadAudits(ctx); err != nil { diff --git a/vault/expiration.go b/vault/expiration.go index 349bc30d066f1..403ac8434423a 100644 --- a/vault/expiration.go +++ b/vault/expiration.go @@ -28,7 +28,6 @@ import ( "github.com/hashicorp/vault/sdk/helper/jsonutil" "github.com/hashicorp/vault/sdk/helper/locksutil" "github.com/hashicorp/vault/sdk/logical" - "github.com/hashicorp/vault/sdk/physical" "github.com/hashicorp/vault/vault/quotas" uberAtomic "go.uber.org/atomic" ) @@ -145,8 +144,6 @@ type ExpirationManager struct { logLeaseExpirations bool expireFunc ExpireLeaseStrategy - revokePermitPool *physical.PermitPool - // testRegisterAuthFailure, if set to true, triggers an explicit failure on // RegisterAuth to simulate a partial failure during a token creation // request. This value should only be set by tests. @@ -295,62 +292,6 @@ func revokeExponentialBackoff(attempt uint8) time.Duration { return time.Duration(backoffTime) } -// revokeIDFunc is invoked when a given ID is expired -func expireLeaseStrategyRevoke(ctx context.Context, m *ExpirationManager, leaseID string, ns *namespace.Namespace) { - for attempt := uint(0); attempt < maxRevokeAttempts; attempt++ { - releasePermit := func() {} - if m.revokePermitPool != nil { - m.logger.Trace("expiring lease; waiting for permit pool") - m.revokePermitPool.Acquire() - releasePermit = m.revokePermitPool.Release - m.logger.Trace("expiring lease; got permit pool") - } - - metrics.IncrCounterWithLabels([]string{"expire", "lease_expiration"}, 1, []metrics.Label{{"namespace", ns.ID}}) - - revokeCtx, cancel := context.WithTimeout(ctx, DefaultMaxRequestDuration) - revokeCtx = namespace.ContextWithNamespace(revokeCtx, ns) - - go func() { - select { - case <-ctx.Done(): - case <-m.quitCh: - cancel() - case <-revokeCtx.Done(): - } - }() - - select { - case <-m.quitCh: - m.logger.Error("shutting down, not attempting further revocation of lease", "lease_id", leaseID) - releasePermit() - cancel() - return - case <-m.quitContext.Done(): - m.logger.Error("core context canceled, not attempting further revocation of lease", "lease_id", leaseID) - releasePermit() - cancel() - return - default: - } - - m.coreStateLock.RLock() - err := m.Revoke(revokeCtx, leaseID) - m.coreStateLock.RUnlock() - releasePermit() - cancel() - if err == nil { - return - } - - metrics.IncrCounterWithLabels([]string{"expire", "lease_expiration", "error"}, 1, []metrics.Label{{"namespace", ns.ID}}) - - m.logger.Error("failed to revoke lease", "lease_id", leaseID, "error", err) - time.Sleep((1 << attempt) * revokeRetryBase) - } - m.logger.Error("maximum revoke attempts reached", "lease_id", leaseID) -} - func getNumExpirationWorkers(c *Core, l log.Logger) int { numWorkers := c.numExpirationWorkers @@ -372,18 +313,6 @@ func getNumExpirationWorkers(c *Core, l log.Logger) int { // NewExpirationManager creates a new ExpirationManager that is backed // using a given view, and uses the provided router for revocation. func NewExpirationManager(c *Core, view *BarrierView, e ExpireLeaseStrategy, logger log.Logger) *ExpirationManager { - var permitPool *physical.PermitPool - if os.Getenv("VAULT_16_REVOKE_PERMITPOOL") != "" { - permitPoolSize := 50 - permitPoolSizeRaw, err := strconv.Atoi(os.Getenv("VAULT_16_REVOKE_PERMITPOOL")) - if err == nil && permitPoolSizeRaw > 0 { - permitPoolSize = permitPoolSizeRaw - } - - permitPool = physical.NewPermitPool(permitPoolSize) - - } - jobManager := fairshare.NewJobManager("expire", getNumExpirationWorkers(c, logger), logger.Named("job-manager"), c.metricSink) jobManager.Start() @@ -416,7 +345,6 @@ func NewExpirationManager(c *Core, view *BarrierView, e ExpireLeaseStrategy, log logLeaseExpirations: os.Getenv("VAULT_SKIP_LOGGING_LEASE_EXPIRATIONS") == "", expireFunc: e, - revokePermitPool: permitPool, jobManager: jobManager, } diff --git a/vault/expiration_test.go b/vault/expiration_test.go index 6fe12f6d6dffd..daaf9ba3d337d 100644 --- a/vault/expiration_test.go +++ b/vault/expiration_test.go @@ -36,11 +36,11 @@ func mockExpiration(t testing.TB) *ExpirationManager { // Wait until the expiration manager is out of restore mode. // This was added to prevent sporadic failures of TestExpiration_unrecoverableErrorMakesIrrevocable. timeout := time.Now().Add(time.Second * 10) - for c.expiration.inRestoreMode() { + for c.expiration.inRestoreMode() { if time.Now().After(timeout) { t.Fatal("ExpirationManager is still in restore mode after 10 seconds") } - time.Sleep(50*time.Millisecond) + time.Sleep(50 * time.Millisecond) } return c.expiration @@ -2181,9 +2181,7 @@ func TestExpiration_renewEntry(t *testing.T) { } } -func revokeEntryRejectedCore(t *testing.T, e ExpireLeaseStrategy) { - t.Helper() - +func TestExpiration_revokeEntry_rejected_fairsharing(t *testing.T) { core, _, _ := TestCoreUnsealed(t) exp := core.expiration @@ -2250,7 +2248,7 @@ func revokeEntryRejectedCore(t *testing.T, e ExpireLeaseStrategy) { t.Fatal(err) } - err = core.setupExpiration(e) + err = core.setupExpiration(expireLeaseStrategyFairsharing) if err != nil { t.Fatal(err) } @@ -2275,14 +2273,6 @@ func revokeEntryRejectedCore(t *testing.T, e ExpireLeaseStrategy) { } } -func TestExpiration_revokeEntry_rejected_revoke(t *testing.T) { - revokeEntryRejectedCore(t, expireLeaseStrategyRevoke) -} - -func TestExpiration_revokeEntry_rejected_fairsharing(t *testing.T) { - revokeEntryRejectedCore(t, expireLeaseStrategyFairsharing) -} - func TestExpiration_renewAuthEntry(t *testing.T) { exp := mockExpiration(t)