VAULT 2844: remove legacy lease revocation strategy (hashicorp#12888)

* remove legacy lease revocation strategy

* add deprecation change log note

* remove VAULT_16_REVOKE_PERMITPOOL

* update changelog

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.
+```

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 {

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,
 	}

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)